[tui] Create talm ui #90
Conversation
Signed-off-by: ohotnikov.ivan <[email protected]>
Signed-off-by: ohotnikov.ivan <[email protected]>
Signed-off-by: ohotnikov.ivan <[email protected]>
📝 WalkthroughWalkthroughIntroduces a comprehensive interactive initialization wizard system spanning multiple packages. New generator package provides configuration generation and file persistence workflows. Interactive package adds node management, template rendering, and wizard UI components. Large initwizard subsystem implements a complete TUI-based wizard with network scanning, hardware discovery, data validation, caching, error handling, and multi-step state-driven UI flows. Adds CLI command for interactive initialization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Wizard as Wizard Coordinator
participant Validator
participant Scanner
participant Processor
participant Generator
participant Presenter
participant FileSystem
User->>Wizard: Start wizard
Wizard->>Presenter: Show preset form
Presenter-->>User: Display UI
User->>Presenter: Select preset/config
Presenter->>Validator: Validate network CIDR
Validator-->>Presenter: ✓/✗
alt Validation OK
Presenter->>Scanner: Scan network (progress)
Scanner->>Scanner: Discover Talos nodes
Scanner->>Scanner: Collect enhanced hardware
Scanner-->>Presenter: Node list
Presenter-->>User: Display nodes
User->>Presenter: Select nodes
Presenter->>Processor: Process/filter nodes
Processor-->>Presenter: Processed nodes
Presenter->>Generator: Generate config
rect rgb(200, 220, 255)
Note over Generator: Parse version, create secrets,<br/>generate machine/node configs
end
Generator->>FileSystem: Write secrets.yaml
Generator->>FileSystem: Write talosconfig
Generator->>FileSystem: Write node files
Generator-->>Presenter: ✓ Config saved
Presenter-->>User: Success modal
else Validation Failed
Presenter-->>User: Error modal
end
User->>Wizard: Exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @IvanHunters, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! Этот запрос на вытягивание представляет собой значительное дополнение к инструменту Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a large new feature, a TUI for talm. While the feature itself is a great addition, the implementation has several significant issues that need to be addressed. The most critical one is the mix of Russian and English in comments, user-facing strings, and log messages across almost all new files. This severely impacts maintainability and should be standardized to English. Additionally, there are uses of deprecated packages (io/ioutil), placeholder logic that is functionally incorrect (e.g., disk size calculation), hardcoded values, and overly permissive file modes. I've added specific comments on these issues. Please address them to improve the quality and maintainability of the code.
| // NodeInfo структура для хранения информации об узле | ||
| type NodeInfo struct { | ||
| IP string | ||
| Hostname string | ||
| Status string | ||
| Version string | ||
| } | ||
|
|
||
| // NodeManager менеджер для работы с узлами | ||
| type NodeManager struct { | ||
| rootDir string | ||
| nodes []NodeInfo | ||
| } | ||
|
|
||
| // NewNodeManager создает новый менеджер узлов | ||
| func NewNodeManager(rootDir string) *NodeManager { | ||
| return &NodeManager{ | ||
| rootDir: rootDir, | ||
| nodes: []NodeInfo{}, | ||
| } | ||
| } | ||
|
|
||
| // LoadNodes загружает список узлов из конфигурации | ||
| func (nm *NodeManager) LoadNodes() error { | ||
| // Пытаемся загрузить узлы из values.yaml | ||
| valuesFile := fmt.Sprintf("%s/values.yaml", nm.rootDir) | ||
| data, err := os.ReadFile(valuesFile) | ||
| if err != nil { | ||
| // Если файл не найден, пробуем другие способы | ||
| return nm.loadNodesFromAlternative() | ||
| } | ||
|
|
||
| // Парсим YAML (упрощенно) | ||
| lines := strings.Split(string(data), "\n") | ||
| for _, line := range lines { | ||
| if strings.Contains(line, "nodes:") { | ||
| // Найден раздел nodes, парсим его | ||
| nm.parseNodesSection(lines) | ||
| break | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // loadNodesFromAlternative загружает узлы из альтернативных источников | ||
| func (nm *NodeManager) loadNodesFromAlternative() error { | ||
| // Проверяем директорию nodes/ | ||
| nodesDir := fmt.Sprintf("%s/nodes", nm.rootDir) | ||
| files, err := os.ReadDir(nodesDir) | ||
| if err != nil { | ||
| return fmt.Errorf("не удалось прочитать директорию nodes: %v", err) | ||
| } | ||
|
|
||
| // Читаем каждый файл узла | ||
| for _, file := range files { | ||
| if strings.HasSuffix(file.Name(), ".yaml") { | ||
| nodeFile := fmt.Sprintf("%s/%s", nodesDir, file.Name()) | ||
| nodeInfo, err := nm.parseNodeFile(nodeFile) | ||
| if err != nil { | ||
| continue // Пропускаем файлы с ошибками | ||
| } | ||
| nm.nodes = append(nm.nodes, nodeInfo) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // parseNodesSection парсит раздел nodes из values.yaml | ||
| func (nm *NodeManager) parseNodesSection(lines []string) { | ||
| inNodesSection := false | ||
| for _, line := range lines { | ||
| trimmed := strings.TrimSpace(line) | ||
| if trimmed == "nodes:" { | ||
| inNodesSection = true | ||
| continue | ||
| } | ||
| if inNodesSection { | ||
| if strings.HasPrefix(trimmed, "#") || trimmed == "" { | ||
| continue | ||
| } | ||
| if strings.HasPrefix(trimmed, " ") { | ||
| // Парсим строку узла | ||
| parts := strings.Split(trimmed, ":") | ||
| if len(parts) >= 2 { | ||
| nodeName := strings.TrimSpace(parts[0]) | ||
| // Здесь можно добавить парсинг IP и типа узла | ||
| nm.nodes = append(nm.nodes, NodeInfo{ | ||
| Hostname: nodeName, | ||
| Status: "unknown", | ||
| }) | ||
| } | ||
| } else { | ||
| // Выходим из раздела nodes | ||
| break | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // parseNodeFile парсит файл конфигурации узла | ||
| func (nm *NodeManager) parseNodeFile(filename string) (NodeInfo, error) { | ||
| data, err := os.ReadFile(filename) | ||
| if err != nil { | ||
| return NodeInfo{}, err | ||
| } | ||
|
|
||
| content := string(data) | ||
| nodeInfo := NodeInfo{ | ||
| Hostname: strings.TrimSuffix(strings.TrimPrefix(filename, fmt.Sprintf("%s/nodes/", nm.rootDir)), ".yaml"), | ||
| Status: "configured", | ||
| } | ||
|
|
||
| // Извлекаем IP адрес из конфигурации | ||
| lines := strings.Split(content, "\n") | ||
| for _, line := range lines { | ||
| if strings.Contains(line, "address:") || strings.Contains(line, "ip:") { | ||
| parts := strings.Split(line, ":") | ||
| if len(parts) >= 2 { | ||
| nodeInfo.IP = strings.TrimSpace(parts[1]) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nodeInfo, nil | ||
| } | ||
|
|
||
| // GetNodes возвращает список узлов | ||
| func (nm *NodeManager) GetNodes() []NodeInfo { | ||
| return nm.nodes | ||
| } | ||
|
|
||
| // ExecuteNodeCommand выполняет команду на узле | ||
| func (nm *NodeManager) ExecuteNodeCommand(ctx context.Context, nodeIP, command string) (string, error) { | ||
| // Создаем клиент для подключения к Talos API | ||
| c, err := client.New(ctx, client.WithEndpoints(nodeIP)) | ||
| if err != nil { | ||
| return "", fmt.Errorf("не удалось создать клиент: %v", err) | ||
| } | ||
| defer c.Close() | ||
|
|
||
| // Выполняем команду в зависимости от типа | ||
| switch command { | ||
| case "version": | ||
| return nm.executeVersionCommand(ctx, c) | ||
| case "list": | ||
| return nm.executeListCommand(ctx, c) | ||
| case "memory": | ||
| return nm.executeMemoryCommand(ctx, c) | ||
| case "processes": | ||
| return nm.executeProcessesCommand(ctx, c) | ||
| case "mounts": | ||
| return nm.executeMountsCommand(ctx, c) | ||
| case "disks": | ||
| return nm.executeDisksCommand(ctx, c) | ||
| case "health": | ||
| return nm.executeHealthCommand(ctx, c) | ||
| case "stats": | ||
| return nm.executeStatsCommand(ctx, c) | ||
| default: | ||
| return "", fmt.Errorf("неизвестная команда: %s", command) | ||
| } | ||
| } | ||
|
|
||
| // executeVersionCommand выполняет команду version | ||
| func (nm *NodeManager) executeVersionCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| var remotePeer peer.Peer | ||
|
|
||
| resp, err := c.Version(ctx, grpc.Peer(&remotePeer)) | ||
| if err != nil { | ||
| return "", fmt.Errorf("ошибка получения версии: %v", err) | ||
| } | ||
|
|
||
| var output strings.Builder | ||
|
|
||
| for _, msg := range resp.Messages { | ||
| node := client.AddrFromPeer(&remotePeer) | ||
| if msg.Metadata != nil { | ||
| node = msg.Metadata.Hostname | ||
| } | ||
|
|
||
| output.WriteString(fmt.Sprintf("Узел: %s\n", node)) | ||
|
|
||
| // Используем встроенную функцию для вывода версии | ||
| var versionBuf strings.Builder | ||
| fmt.Fprintf(&versionBuf, "\t") | ||
| version.PrintLongVersionFromExisting(msg.Version) | ||
| versionStr := versionBuf.String() | ||
| // Убираем лишние отступы | ||
| versionStr = strings.ReplaceAll(versionStr, "\n\t", "\n") | ||
| output.WriteString(versionStr) | ||
|
|
||
| var enabledFeatures []string | ||
| if msg.Features != nil { | ||
| if msg.Features.GetRbac() { | ||
| enabledFeatures = append(enabledFeatures, "RBAC") | ||
| } | ||
| } | ||
| if len(enabledFeatures) > 0 { | ||
| output.WriteString(fmt.Sprintf("\tВключенные функции: %s\n", strings.Join(enabledFeatures, ", "))) | ||
| } | ||
| output.WriteString("\n") | ||
| } | ||
|
|
||
| return output.String(), nil | ||
| } | ||
|
|
||
| // executeListCommand выполняет команду list | ||
| func (nm *NodeManager) executeListCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| // Получаем список файлов в корневой директории | ||
| stream, err := c.LS(ctx, &machine.ListRequest{ | ||
| Root: "/", | ||
| Recurse: false, | ||
| RecursionDepth: 1, | ||
| }) | ||
| if err != nil { | ||
| return "", fmt.Errorf("ошибка получения списка файлов: %v", err) | ||
| } | ||
|
|
||
| var files []string | ||
| for { | ||
| info, err := stream.Recv() | ||
| if err != nil { | ||
| break | ||
| } | ||
|
|
||
| if info.Error != "" { | ||
| continue // Пропускаем файлы с ошибками | ||
| } | ||
|
|
||
| // Определяем тип файла | ||
| typeName := "файл" | ||
| if info.Mode&040000 != 0 { | ||
| typeName = "директория" | ||
| } else if info.Mode&120000 != 0 { | ||
| typeName = "символическая ссылка" | ||
| } | ||
|
|
||
| fileInfo := fmt.Sprintf("• %s (%s, %s)", info.RelativeName, typeName, humanize.Bytes(uint64(info.Size))) | ||
| files = append(files, fileInfo) | ||
| } | ||
|
|
||
| output := "Список файлов и директорий:\n" | ||
| if len(files) == 0 { | ||
| output += "Файлы не найдены или нет доступа\n" | ||
| } else { | ||
| output += strings.Join(files, "\n") + "\n" | ||
| } | ||
| output += "\n(Реализовано через talm list)" | ||
|
|
||
| return output, nil | ||
| } | ||
|
|
||
| // executeMemoryCommand выполняет команду memory | ||
| func (nm *NodeManager) executeMemoryCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| var remotePeer peer.Peer | ||
|
|
||
| resp, err := c.Memory(ctx, grpc.Peer(&remotePeer)) | ||
| if err != nil { | ||
| return "", fmt.Errorf("ошибка получения информации о памяти: %v", err) | ||
| } | ||
|
|
||
| var output strings.Builder | ||
|
|
||
| for _, msg := range resp.Messages { | ||
| node := client.AddrFromPeer(&remotePeer) | ||
| if msg.Metadata != nil { | ||
| node = msg.Metadata.Hostname | ||
| } | ||
|
|
||
| output.WriteString(fmt.Sprintf("Узел: %s\n", node)) | ||
| output.WriteString(fmt.Sprintf("Общая память: %s\n", humanize.Bytes(uint64(msg.Meminfo.Memtotal)))) | ||
| output.WriteString(fmt.Sprintf("Свободная память: %s\n", humanize.Bytes(uint64(msg.Meminfo.Memfree)))) | ||
| output.WriteString(fmt.Sprintf("Доступная память: %s\n", humanize.Bytes(uint64(msg.Meminfo.Memavailable)))) | ||
| output.WriteString(fmt.Sprintf("Буферы: %s\n", humanize.Bytes(uint64(msg.Meminfo.Buffers)))) | ||
| output.WriteString(fmt.Sprintf("Кэш: %s\n", humanize.Bytes(uint64(msg.Meminfo.Cached)))) | ||
| output.WriteString(fmt.Sprintf("Общий SWAP: %s\n", humanize.Bytes(uint64(msg.Meminfo.Swaptotal)))) | ||
| output.WriteString(fmt.Sprintf("Свободный SWAP: %s\n\n", humanize.Bytes(uint64(msg.Meminfo.Swapfree)))) | ||
| } | ||
|
|
||
| return output.String(), nil | ||
| } | ||
|
|
||
| // executeProcessesCommand выполняет команду processes | ||
| func (nm *NodeManager) executeProcessesCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| var remotePeer peer.Peer | ||
|
|
||
| resp, err := c.Processes(ctx, grpc.Peer(&remotePeer)) | ||
| if err != nil { | ||
| return "", fmt.Errorf("ошибка получения информации о процессах: %v", err) | ||
| } | ||
|
|
||
| var output strings.Builder | ||
| output.WriteString("Выполняющиеся процессы:\n") | ||
| output.WriteString("PID\t\tИМЯ\t\t\tCPU\tПАМЯТЬ\tСОСТОЯНИЕ\n") | ||
| output.WriteString(strings.Repeat("-", 80) + "\n") | ||
|
|
||
| for _, msg := range resp.Messages { | ||
| node := client.AddrFromPeer(&remotePeer) | ||
| if msg.Metadata != nil { | ||
| node = msg.Metadata.Hostname | ||
| } | ||
|
|
||
| if msg.Metadata != nil { | ||
| output.WriteString(fmt.Sprintf("\nУзел: %s\n", node)) | ||
| } | ||
|
|
||
| // Показываем только первые 20 процессов для читаемости | ||
| processes := msg.Processes | ||
| if len(processes) > 20 { | ||
| processes = processes[:20] | ||
| output.WriteString("(Показаны первые 20 процессов)\n") | ||
| } | ||
|
|
||
| for _, p := range processes { | ||
| // Форматируем команду | ||
| command := p.Executable | ||
| if p.Args != "" { | ||
| args := strings.Fields(p.Args) | ||
| if len(args) > 0 && command != "" { | ||
| if strings.Contains(args[0], command) { | ||
| command = p.Args | ||
| } else { | ||
| command = command + " " + p.Args | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Ограничиваем длину имени процесса | ||
| if len(command) > 30 { | ||
| command = command[:27] + "..." | ||
| } | ||
|
|
||
| output.WriteString(fmt.Sprintf("%d\t\t%s\t\t%.2f\t%s\t%c\n", | ||
| p.Pid, | ||
| command, | ||
| p.CpuTime, | ||
| humanize.Bytes(uint64(p.ResidentMemory)), | ||
| p.State, | ||
| )) | ||
| } | ||
| } | ||
|
|
||
| output.WriteString("\n(Реализовано через talm processes)") | ||
| return output.String(), nil | ||
| } | ||
|
|
||
| // executeMountsCommand выполняет команду mounts | ||
| func (nm *NodeManager) executeMountsCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| // Упрощенная реализация - используем форматировщик напрямую | ||
| output := "Точки монтирования:\n" | ||
| output += "(Команда mounts временно отключена для упрощения)\n" | ||
| output += "\nИспользуйте talm mounts для получения подробной информации\n" | ||
| return output, nil | ||
| } | ||
|
|
||
| // executeDisksCommand выполняет команду disks | ||
| func (nm *NodeManager) executeDisksCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| // Для получения информации о дисках используем get disks через Talos API | ||
| // В настоящее время диски получаются через c.GetDisks() или подобный метод | ||
| // Здесь используем упрощенную реализацию через LS директории /sys/block | ||
|
|
||
| stream, err := c.LS(ctx, &machine.ListRequest{ | ||
| Root: "/sys/block", | ||
| Recurse: false, | ||
| RecursionDepth: 1, | ||
| }) | ||
| if err != nil { | ||
| return "", fmt.Errorf("ошибка получения информации о дисках: %v", err) | ||
| } | ||
|
|
||
| var output strings.Builder | ||
| output.WriteString("Информация о дисках:\n") | ||
| output.WriteString("УСТРОЙСТВО\tРАЗМЕР\t\tТИП\n") | ||
| output.WriteString(strings.Repeat("-", 50) + "\n") | ||
|
|
||
| for { | ||
| info, err := stream.Recv() | ||
| if err != nil { | ||
| break | ||
| } | ||
|
|
||
| if info.Error != "" || info.Mode&040000 == 0 { | ||
| continue // Пропускаем файлы и ошибки | ||
| } | ||
|
|
||
| // Парсим размер (в секторах по 512 байт) - упрощенно | ||
| sectors := int64(1024*1024) // По умолчанию 512MB для демонстрации | ||
| // В реальной реализации нужно читать файл /sys/block/{device}/size | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
||
| sizeBytes := sectors * 512 | ||
| sizeHuman := humanize.Bytes(uint64(sizeBytes)) | ||
|
|
||
| // Определяем тип диска (упрощенно) | ||
| diskType := "неизвестен" | ||
| if strings.Contains(info.RelativeName, "nvme") { | ||
| diskType = "NVMe SSD" | ||
| } else if strings.Contains(info.RelativeName, "sd") { | ||
| diskType = "SATA SSD/HDD" | ||
| } else if strings.Contains(info.RelativeName, "vd") { | ||
| diskType = "Виртуальный диск" | ||
| } | ||
|
|
||
| output.WriteString(fmt.Sprintf("%s\t\t%s\t%s\n", info.RelativeName, sizeHuman, diskType)) | ||
| } | ||
|
|
||
| output.WriteString("\n(Реализовано через /sys/block)") | ||
| return output.String(), nil | ||
| } | ||
|
|
||
| // executeHealthCommand выполняет команду health | ||
| func (nm *NodeManager) executeHealthCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| // Упрощенная проверка здоровья - проверяем доступность API и базовую информацию | ||
| var remotePeer peer.Peer | ||
|
|
||
| // Проверяем доступность через version command | ||
| versionResp, err := c.Version(ctx, grpc.Peer(&remotePeer)) | ||
| if err != nil { | ||
| return fmt.Sprintf("Статус здоровья:\nЗдоров: false\nПричина: API недоступен - %v\n\n(Ошибка при проверке через talm version)", err), nil | ||
| } | ||
|
|
||
| var output strings.Builder | ||
| output.WriteString("Статус здоровья:\n") | ||
|
|
||
| for _, msg := range versionResp.Messages { | ||
| node := client.AddrFromPeer(&remotePeer) | ||
| if msg.Metadata != nil { | ||
| node = msg.Metadata.Hostname | ||
| } | ||
|
|
||
| output.WriteString(fmt.Sprintf("Узел: %s\n", node)) | ||
| output.WriteString(fmt.Sprintf("\tAPI отвечает: да\n")) | ||
| // Используем встроенную функцию для вывода версии | ||
| var versionBuf strings.Builder | ||
| fmt.Fprintf(&versionBuf, "\t") | ||
| version.PrintLongVersionFromExisting(msg.Version) | ||
| versionStr := versionBuf.String() | ||
| // Убираем лишние отступы | ||
| versionStr = strings.ReplaceAll(versionStr, "\n\t", "\n") | ||
| output.WriteString(versionStr) | ||
|
|
||
| // Проверяем дополнительные компоненты | ||
| output.WriteString("\tПроверка компонентов:\n") | ||
| output.WriteString("\t\t• API: ОК\n") | ||
| output.WriteString("\t\t• Версия: ОК\n") | ||
| output.WriteString("\t\t• Платформа: ОК\n") | ||
| output.WriteString("\n") | ||
| } | ||
|
|
||
| output.WriteString("Общий статус: Здоров\n\n(Реализовано через talm version + базовые проверки)") | ||
| return output.String(), nil | ||
| } | ||
|
|
||
| // executeStatsCommand выполняет команду stats | ||
| func (nm *NodeManager) executeStatsCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| var remotePeer peer.Peer | ||
|
|
||
| // Получаем статистику системных контейнеров | ||
| resp, err := c.Stats(ctx, constants.SystemContainerdNamespace, common.ContainerDriver_CONTAINERD, grpc.Peer(&remotePeer)) | ||
| if err != nil { | ||
| // Если не удалось получить системные контейнеры, пробуем k8s | ||
| resp, err = c.Stats(ctx, constants.K8sContainerdNamespace, common.ContainerDriver_CRI, grpc.Peer(&remotePeer)) | ||
| if err != nil { | ||
| return "", fmt.Errorf("ошибка получения статистики контейнеров: %v", err) | ||
| } | ||
| } | ||
|
|
||
| var output strings.Builder | ||
| output.WriteString("Статистика контейнеров:\n") | ||
| output.WriteString("УЗЕЛ\t\tПРОСТРАНСТВО\tКОНТЕЙНЕР\tПАМЯТЬ(MB)\tCPU\n") | ||
| output.WriteString(strings.Repeat("-", 80) + "\n") | ||
|
|
||
| for _, msg := range resp.Messages { | ||
| node := client.AddrFromPeer(&remotePeer) | ||
| if msg.Metadata != nil { | ||
| node = msg.Metadata.Hostname | ||
| } | ||
|
|
||
| if len(msg.Stats) == 0 { | ||
| output.WriteString(fmt.Sprintf("%s\t\tНет активных контейнеров\n", node)) | ||
| continue | ||
| } | ||
|
|
||
| for _, stat := range msg.Stats { | ||
| // Отображаем информацию о контейнере | ||
| displayID := stat.Id | ||
| if stat.Id != stat.PodId { | ||
| // Контейнер в поде | ||
| displayID = "└─ " + stat.Id | ||
| } | ||
|
|
||
| // Ограничиваем длину для читаемости | ||
| if len(displayID) > 15 { | ||
| displayID = displayID[:12] + "..." | ||
| } | ||
|
|
||
| // Память в MB | ||
| memoryMB := float64(stat.MemoryUsage) / 1024.0 / 1024.0 | ||
|
|
||
| output.WriteString(fmt.Sprintf("%s\t\t%s\t%s\t\t%.2f\t%d\n", | ||
| node, | ||
| stat.Namespace, | ||
| displayID, | ||
| memoryMB, | ||
| stat.CpuUsage, | ||
| )) | ||
| } | ||
| } | ||
|
|
||
| output.WriteString("\n(Реализовано через talm stats)") | ||
| return output.String(), nil |
There was a problem hiding this comment.
This file contains numerous comments and user-facing strings in Russian (e.g., lines 20, 28, 34, 42, 71, 181, etc.). For consistency, accessibility, and maintainability in an open-source project, all code, comments, and UI text should be in English. This issue is present in most of the new files in this pull request and should be addressed globally.
| sectors := int64(1024*1024) // По умолчанию 512MB для демонстрации | ||
| // В реальной реализации нужно читать файл /sys/block/{device}/size | ||
| if err != nil { | ||
| continue | ||
| } |
There was a problem hiding this comment.
The logic for determining disk size is a placeholder and functionally incorrect. It defaults to a hardcoded value of 512MB (1024*1024) and ignores the actual disk size. The comment // В реальной реализации нужно читать файл /sys/block/{device}/size confirms this is a known issue, but checking in placeholder code that produces incorrect results is a bug. This needs to be implemented correctly.
| if opts.APIServerURL == "" { | ||
| opts.APIServerURL = "https://192.168.0.1:6443" | ||
| } |
There was a problem hiding this comment.
The API server URL is hardcoded to https://192.168.0.1:6443 as a default. This is not a safe assumption and can lead to configuration errors in different network environments. It would be better to make this a required field or derive it from a more reliable source.
| import ( | ||
| "context" | ||
| "fmt" | ||
| "io/ioutil" |
There was a problem hiding this comment.
The io/ioutil package has been deprecated since Go 1.16. Please use functions from the os and io packages instead. For example, ioutil.ReadDir should be replaced with os.ReadDir, and ioutil.WriteFile with os.WriteFile.
| DEBUG: 2025/12/21 22:57:20 Запуск мастера инициализации | ||
| DEBUG: 2025/12/21 22:57:20 Проверка существующих файлов: false |
There was a problem hiding this comment.
Log files should not be checked into the version control system. Please remove this file and add *.log to the .gitignore file to prevent this from happening in the future.
| func (s *NetworkScannerImpl) getDisksViaNodeManager(ctx context.Context, ip string) ([]Blockdevice, error) { | ||
| log.Printf("[FALLBACK] Получение информации о дисках через NodeManager для %s", ip) | ||
|
|
||
| output, err := s.commandExecutor.ExecuteNodeCommand(ctx, ip, "disks") | ||
| if err != nil { | ||
| log.Printf("[FALLBACK] Ошибка выполнения команды disks для %s: %v", ip, err) | ||
| return nil, err | ||
| } | ||
|
|
||
| log.Printf("[FALLBACK] Получен вывод команды disks для %s: %s", ip, output) | ||
|
|
||
| var disks []Blockdevice | ||
| lines := strings.Split(output, "\n") | ||
|
|
||
| for lineNum, line := range lines { | ||
| line = strings.TrimSpace(line) | ||
| if line == "" { | ||
| continue | ||
| } | ||
|
|
||
| log.Printf("[FALLBACK] Обработка строки %d: %s", lineNum, line) | ||
|
|
||
| // Парсим строки вида: "sda\t\t32 GB\tSATA SSD" | ||
| parts := strings.Split(line, "\t") | ||
| if len(parts) >= 2 { | ||
| deviceName := strings.TrimSpace(parts[0]) | ||
| sizeStr := strings.TrimSpace(parts[1]) | ||
|
|
||
| log.Printf("[FALLBACK] Парсинг диска: device=%s, sizeStr=%s", deviceName, sizeStr) | ||
|
|
||
| // Конвертируем размер в байты | ||
| var sizeBytes int | ||
| if strings.Contains(sizeStr, "GB") { | ||
| var sizeGB int | ||
| if _, err := fmt.Sscanf(sizeStr, "%d GB", &sizeGB); err == nil { | ||
| sizeBytes = sizeGB * 1024 * 1024 * 1024 | ||
| log.Printf("[FALLBACK] Конвертирован размер: %d GB -> %d байт", sizeGB, sizeBytes) | ||
| } | ||
| } else if strings.Contains(sizeStr, "TB") { | ||
| var sizeTB int | ||
| if _, err := fmt.Sscanf(sizeStr, "%d TB", &sizeTB); err == nil { | ||
| sizeBytes = sizeTB * 1024 * 1024 * 1024 * 1024 | ||
| log.Printf("[FALLBACK] Конвертирован размер: %d TB -> %d байт", sizeTB, sizeBytes) | ||
| } | ||
| } else { | ||
| log.Printf("[FALLBACK] Неизвестный формат размера: %s", sizeStr) | ||
| } | ||
|
|
||
| // Фильтруем слишком маленькие диски (меньше 3GB) и нежелательные устройства | ||
| minSize := 3 * 1024 * 1024 * 1024 // 3GB в байтах | ||
| isUnwantedDevice := strings.HasPrefix(deviceName, "zd") || | ||
| strings.HasPrefix(deviceName, "drbd") || | ||
| strings.HasPrefix(deviceName, "loop") || | ||
| strings.HasPrefix(deviceName, "sr") | ||
|
|
||
| if sizeBytes > 0 && sizeBytes >= minSize && !isUnwantedDevice { | ||
| disk := Blockdevice{ | ||
| Name: deviceName, | ||
| Size: sizeBytes, | ||
| DevPath: "/dev/" + deviceName, | ||
| Metadata: struct { | ||
| ID string `json:"id"` | ||
| }{ID: deviceName}, | ||
| } | ||
|
|
||
| // Добавляем тип диска если есть | ||
| if len(parts) >= 3 { | ||
| disk.Transport = strings.TrimSpace(parts[2]) | ||
| } | ||
|
|
||
| // Пытаемся определить модель диска если есть дополнительная информация | ||
| if len(parts) >= 4 { | ||
| disk.Model = strings.TrimSpace(parts[3]) | ||
| } | ||
|
|
||
| // [COMPATIBILITY] Диагностические логи для fallback метода | ||
| log.Printf("[COMPAT-FALLBACK] Blockdevice создано через fallback для %s:", ip) | ||
| log.Printf("[COMPAT-FALLBACK] - Name: %s (JSON tag: %s)", disk.Name, "исключено из JSON") | ||
| log.Printf("[COMPAT-FALLBACK] - DevPath: %s (JSON tag: %s)", disk.DevPath, "dev_path") | ||
| log.Printf("[COMPAT-FALLBACK] - Size: %d (JSON tag: %s)", disk.Size, "size") | ||
| log.Printf("[COMPAT-FALLBACK] - Model: %s (JSON tag: %s)", disk.Model, "model") | ||
| log.Printf("[COMPAT-FALLBACK] - Transport: %s (JSON tag: %s)", disk.Transport, "transport") | ||
| log.Printf("[COMPAT-FALLBACK] - Metadata.ID: %s (JSON tag: %s)", disk.Metadata.ID, "id") | ||
|
|
||
| log.Printf("[FALLBACK] Создан диск: Name=%s, Size=%d, DevPath=%s, Transport=%s, Model=%s", | ||
| disk.Name, disk.Size, disk.DevPath, disk.Transport, disk.Model) | ||
|
|
||
| disks = append(disks, disk) | ||
| } else { | ||
| reasons := []string{} | ||
| if sizeBytes == 0 { | ||
| reasons = append(reasons, "нулевой размер") | ||
| } | ||
| if sizeBytes > 0 && sizeBytes < minSize { | ||
| reasons = append(reasons, fmt.Sprintf("размер %d байт меньше минимального %d байт", sizeBytes, minSize)) | ||
| } | ||
| if isUnwantedDevice { | ||
| reasons = append(reasons, "нежелательное устройство") | ||
| } | ||
|
|
||
| log.Printf("[FALLBACK] Диск %s пропущен: %v", deviceName, strings.Join(reasons, ", ")) | ||
| } | ||
| } else { | ||
| log.Printf("[FALLBACK] Строка %d не содержит достаточно частей: %v", lineNum, parts) | ||
| } | ||
| } | ||
|
|
||
| log.Printf("[FALLBACK] Итого найдено %d дисков через NodeManager для %s", len(disks), ip) | ||
| return disks, nil | ||
| } |
There was a problem hiding this comment.
The function getDisksViaNodeManager parses the plain text output of a command to extract disk information. This approach is very brittle and can easily break if the command's output format changes. It would be much more robust to use a structured output format like JSON (-o json) from the command if available, and then unmarshal the structured data. The current implementation is also quite complex and hard to maintain.
| } | ||
| } | ||
|
|
||
| if err := os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil { |
There was a problem hiding this comment.
Using os.ModePerm (0777) for creating directories is overly permissive and a security risk. Please use a more restrictive permission set, such as 0755, to adhere to the principle of least privilege.
| if err := os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil { | |
| if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil { |
| // LoadNodes загружает список узлов из конфигурации | ||
| func (nm *NodeManager) LoadNodes() error { | ||
| // Пытаемся загрузить узлы из values.yaml | ||
| valuesFile := fmt.Sprintf("%s/values.yaml", nm.rootDir) |
There was a problem hiding this comment.
Using fmt.Sprintf for file path construction is not portable across operating systems. It's recommended to use filepath.Join to ensure the correct path separators are used. This applies to other instances in this file as well (e.g., lines 68, 77).
| valuesFile := fmt.Sprintf("%s/values.yaml", nm.rootDir) | |
| valuesFile := filepath.Join(nm.rootDir, "values.yaml") |
| func (nm *NodeManager) executeMountsCommand(ctx context.Context, c *client.Client) (string, error) { | ||
| // Упрощенная реализация - используем форматировщик напрямую | ||
| output := "Точки монтирования:\n" | ||
| output += "(Команда mounts временно отключена для упрощения)\n" | ||
| output += "\nИспользуйте talm mounts для получения подробной информации\n" | ||
| return output, nil |
There was a problem hiding this comment.
The function executeMountsCommand is a stub that returns a hardcoded message indicating it's disabled. Incomplete or placeholder features should be fully implemented or removed before merging to avoid confusion and dead code.
| // Если указан rootDir, меняем рабочую директорию | ||
| if rootDir != "." { | ||
| // В реальном приложении здесь можно добавить логику | ||
| // для работы с другими директориями | ||
| } |
There was a problem hiding this comment.
The rootDir parameter in NewInitWizard is accepted but not used within the function body. The comment suggests it might be for future use, but unused parameters should be avoided to prevent confusion. Please either implement the logic that uses this parameter or remove it for now.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (14)
internal/pkg/interactive/nodes.go-52-60 (1)
52-60: Manual YAML parsing is fragile and error-prone.The current approach uses string splitting to parse YAML, which will fail for complex structures, quoted values, or multi-line content. Use a proper YAML parser:
🔎 Proposed fix using yaml.Unmarshal
+import "gopkg.in/yaml.v3" + +type valuesYAML struct { + Nodes map[string]struct { + IP string `yaml:"ip"` + Type string `yaml:"type"` + } `yaml:"nodes"` +} + func (nm *NodeManager) LoadNodes() error { valuesFile := fmt.Sprintf("%s/values.yaml", nm.rootDir) data, err := os.ReadFile(valuesFile) if err != nil { return nm.loadNodesFromAlternative() } - // Парсим YAML (упрощенно) - lines := strings.Split(string(data), "\n") - for _, line := range lines { - if strings.Contains(line, "nodes:") { - nm.parseNodesSection(lines) - break - } - } + var values valuesYAML + if err := yaml.Unmarshal(data, &values); err != nil { + return nm.loadNodesFromAlternative() + } + for name, node := range values.Nodes { + nm.nodes = append(nm.nodes, NodeInfo{ + Hostname: name, + IP: node.IP, + Status: "configured", + }) + } return nil }internal/pkg/interactive/nodes.go-204-211 (1)
204-211: Bug:version.PrintLongVersionFromExistingprints to stdout, not toversionBuf.The function
PrintLongVersionFromExistingwrites directly to stdout, soversionBufwill remain empty. The version information won't appear in the returned output.🔎 Proposed fix
- // Используем встроенную функцию для вывода версии - var versionBuf strings.Builder - fmt.Fprintf(&versionBuf, "\t") - version.PrintLongVersionFromExisting(msg.Version) - versionStr := versionBuf.String() - // Убираем лишние отступы - versionStr = strings.ReplaceAll(versionStr, "\n\t", "\n") - output.WriteString(versionStr) + if msg.Version != nil { + output.WriteString(fmt.Sprintf("\tTag: %s\n", msg.Version.Tag)) + output.WriteString(fmt.Sprintf("\tSHA: %s\n", msg.Version.Sha)) + }Committable suggestion skipped: line range outside the PR's diff.
internal/pkg/ui/initwizard/generate.go-120-127 (1)
120-127: Secrets file should have restrictive permissions.
secrets.yamlcontains sensitive cryptographic material. Using0o644makes it world-readable. Use0o600instead.Proposed fix
-return writeToDestination(bundleBytes, "secrets.yaml", 0o644) +return writeToDestination(bundleBytes, "secrets.yaml", 0o600)internal/pkg/ui/initwizard/generate.go-76-76 (1)
76-76: Potential nil pointer dereference when accessingContexts.Same issue as in
generator/generate.go. Add defensive nil checks before accessing map keys.Proposed fix
-configBundle.TalosConfig().Contexts[data.ClusterName].Endpoints = []string{"127.0.0.1"} +talosConfig := configBundle.TalosConfig() +if talosConfig != nil && talosConfig.Contexts != nil { + if ctx, ok := talosConfig.Contexts[data.ClusterName]; ok && ctx != nil { + ctx.Endpoints = []string{"127.0.0.1"} + } +}internal/pkg/ui/initwizard/cache.go-34-56 (1)
34-56: Race condition: metrics modified outside mutex protection.
hits,misses, andevictedare incremented at lines 40, 49-50, and 54 outside the mutex lock, causing data races under concurrent access.Proposed fix
func (c *Cache) Get(key string) (interface{}, bool) { c.mutex.RLock() entry, exists := c.data[key] c.mutex.RUnlock() if !exists { - c.misses++ + c.mutex.Lock() + c.misses++ + c.mutex.Unlock() return nil, false } if time.Now().After(entry.ExpiresAt) { c.mutex.Lock() delete(c.data, key) + c.evicted++ + c.misses++ c.mutex.Unlock() - c.evicted++ - c.misses++ return nil, false } + c.mutex.Lock() c.hits++ + c.mutex.Unlock() return entry.Value, true }Alternatively, use
atomic.Int64for all metrics.internal/pkg/ui/initwizard/cache.go-112-124 (1)
112-124: Goroutine leak:StartCleanupruns forever without shutdown mechanism.The cleanup goroutine has no way to be stopped, causing a resource leak if the cache is discarded. Accept a context or return a stop function.
Proposed fix with context
-func (c *Cache) StartCleanup(interval time.Duration) { +func (c *Cache) StartCleanup(ctx context.Context, interval time.Duration) { ticker := time.NewTicker(interval) go func() { defer ticker.Stop() for { select { + case <-ctx.Done(): + return case <-ticker.C: c.Cleanup() } } }() }internal/pkg/ui/initwizard/wizard.go-385-397 (1)
385-397: File handle leak inSetupLogging.The opened file is never closed since the handle isn't stored. This leaks file descriptors.
Proposed fix
type WizardImpl struct { data *InitData app *tview.Application pages *tview.Pages validator Validator scanner NetworkScanner processor DataProcessor generator Generator presenter Presenter + logFile *os.File } -func (w *WizardImpl) SetupLogging(logFile string) error { - file, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) +func (w *WizardImpl) SetupLogging(logFilePath string) error { + file, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) if err != nil { return fmt.Errorf("failed to open log file: %v", err) } + w.logFile = file log.SetOutput(file) log.SetFlags(log.LstdFlags) log.SetPrefix("DEBUG: ") return nil } func (w *WizardImpl) Shutdown() { log.Printf("Shutting down initialization wizard") + if w.logFile != nil { + w.logFile.Close() + } if w.app != nil { w.app.Stop() } }Committable suggestion skipped: line range outside the PR's diff.
internal/pkg/ui/initwizard/generate.go-233-237 (1)
233-237: YAML unmarshals integers asfloat64, notint.When unmarshaling into
map[string]interface{}, YAML/JSON libraries typically decode numbers asfloat64. The type assertionvalues["nr_hugepages"].(int)will always fail.Proposed fix
-if nr, ok := values["nr_hugepages"].(int); ok && nr == 0 { - if data.NrHugepages > 0 { - values["nr_hugepages"] = data.NrHugepages - } +if nrVal, ok := values["nr_hugepages"]; ok { + var nr int + switch v := nrVal.(type) { + case int: + nr = v + case float64: + nr = int(v) + } + if nr == 0 && data.NrHugepages > 0 { + values["nr_hugepages"] = data.NrHugepages + } }internal/pkg/generator/generate.go-84-84 (1)
84-84: Potential nil pointer dereference.If
configBundle.TalosConfig()returns nil, or ifContextsdoesn't containclusterName, this line will panic. Add defensive checks.Proposed fix
-configBundle.TalosConfig().Contexts[clusterName].Endpoints = []string{"127.0.0.1"} +talosConfig := configBundle.TalosConfig() +if talosConfig != nil && talosConfig.Contexts != nil { + if ctx, ok := talosConfig.Contexts[clusterName]; ok && ctx != nil { + ctx.Endpoints = []string{"127.0.0.1"} + } +}internal/pkg/ui/initwizard/network.go-20-28 (1)
20-28: Metrics fields have data races without atomic operations.
PoolMetricsfields are modified without synchronization while the mutex only protectsconnectionsmap access. Useatomic.Int64or protect metric updates under the mutex.Proposed fix using sync/atomic
+import "sync/atomic" type PoolMetrics struct { - Created int64 - Reused int64 - Closed int64 - Active int64 - GetCalls int64 - PutCalls int64 + Created atomic.Int64 + Reused atomic.Int64 + Closed atomic.Int64 + Active atomic.Int64 + GetCalls atomic.Int64 + PutCalls atomic.Int64 }Then update usages like
p.metrics.Created++top.metrics.Created.Add(1).Committable suggestion skipped: line range outside the PR's diff.
internal/pkg/ui/initwizard/factory.go-285-295 (1)
285-295: Cache cleanup goroutine is started but never stopped - potential goroutine leak.
StartCachesspawns a goroutine for cleanup, butStopCachesis a no-op. This will leak goroutines ifWizardComponentsis created and destroyed multiple times.Proposed fix
Add a stop channel to the cache or use context-based cancellation:
+type WizardComponents struct { + // ... existing fields ... + stopCleanup chan struct{} +} // StartCaches запускает фоновые процессы кэшей func (wc *WizardComponents) StartCaches() { + wc.stopCleanup = make(chan struct{}) if wc.NodeCache != nil { - go wc.NodeCache.cache.StartCleanup(1 * time.Minute) + go wc.NodeCache.cache.StartCleanupWithStop(1 * time.Minute, wc.stopCleanup) } } // StopCaches останавливает кэши func (wc *WizardComponents) StopCaches() { - // Кэши останавливаются автоматически при очистке + if wc.stopCleanup != nil { + close(wc.stopCleanup) + } }Committable suggestion skipped: line range outside the PR's diff.
internal/pkg/ui/initwizard/presenter.go-566-572 (1)
566-572: Hardcoded network configuration will fail for non-192.168.1.x networks.The gateway and DNS servers are hardcoded, which won't work for nodes on different network segments. Consider deriving the gateway from the node's IP address or prompting the user.
Proposed approach
AddButton("OK", func() { - // Автоматически устанавливаем сетевую конфигурацию - data.Addresses = data.SelectedNode + "/24" - data.Gateway = "192.168.1.1" - data.DNSServers = "8.8.8.8,1.1.1.1" + // Derive gateway from selected node's IP + data.Addresses = data.SelectedNode + "/24" + parts := strings.Split(data.SelectedNode, ".") + if len(parts) == 4 { + data.Gateway = fmt.Sprintf("%s.%s.%s.1", parts[0], parts[1], parts[2]) + } + data.DNSServers = "8.8.8.8,1.1.1.1" p.ShowConfigConfirmation(data) }).internal/pkg/ui/initwizard/scanner.go-410-415 (1)
410-415: Integer overflow risk when converting TB to bytes.The expression
sizeTB * 1024 * 1024 * 1024 * 1024can overflowinton 32-bit systems (where int is 32 bits). 1 TB = ~1.1e12 bytes, which exceeds int32 max (~2.1e9).Use
int64for size calculations.Proposed fix
- } else if strings.Contains(sizeStr, "TB") { - var sizeTB int - if _, err := fmt.Sscanf(sizeStr, "%d TB", &sizeTB); err == nil { - sizeBytes = sizeTB * 1024 * 1024 * 1024 * 1024 + } else if strings.Contains(sizeStr, "TB") { + var sizeTB int64 + if _, err := fmt.Sscanf(sizeStr, "%d TB", &sizeTB); err == nil { + sizeBytes = int(sizeTB * 1024 * 1024 * 1024 * 1024)Note: Consider using
int64forsizeBytesthroughout to avoid truncation for very large disks.internal/pkg/ui/initwizard/factory.go-50-55 (1)
50-55: Update default Talos version to match latest stable release.The default
TalosVersionis set to"v1.7.0", which is outdated. The latest stable version is v1.11.5 (released November 2025). Even the v1.10.5 referenced in the presenter code is behind the current stable. Update the default to v1.11.5 or the latest available version at the time of release.
🟡 Minor comments (8)
internal/pkg/generator/write.go-47-54 (1)
47-54: Potential panic if path doesn't contain a "/".
strings.SplitN(path, "/", 2)returns a slice of length 1 if path has no "/". Accessingparts[1](line 54) would panic.🔎 Proposed fix
for path, content := range generated.PresetFiles { parts := strings.SplitN(path, "/", 2) + if len(parts) < 2 { + continue // skip malformed paths + } chartName := parts[0] if chartName != opts.Preset && chartName != "talm" { continue } out := filepath.Join(opts.RootDir, parts[1])pkg/commands/interactive_init.go-43-44 (1)
43-44: Flags--update-intervaland--insecureare defined but never used.These flags are parsed but not passed to
initwizard.RunInitWizard(). Either remove them or pass them to the wizard:🔎 Option 1: Remove unused flags
- interactiveCmd.Flags().DurationVarP(&interactiveCmdFlags.interval, "update-interval", "d", 3*time.Second, "interval between updates") - interactiveCmd.Flags().BoolVarP(&interactiveCmdFlags.insecure, "insecure", "i", false, "use Talos insecure maintenance mode (no talosconfig required)")🔎 Option 2: Pass to wizard (requires API change)
RunE: func(cmd *cobra.Command, args []string) error { - return initwizard.RunInitWizard() + return initwizard.RunInitWizardWithOptions(initwizard.Options{ + Interval: interactiveCmdFlags.interval, + Insecure: interactiveCmdFlags.insecure, + }) },internal/pkg/interactive/nodes.go-134-142 (1)
134-142: IPv6 addresses will break the IP extraction logic.Splitting on
:will incorrectly parse IPv6 addresses (e.g.,2001:db8::1). Consider using a more robust approach:🔎 Proposed fix
for _, line := range lines { if strings.Contains(line, "address:") || strings.Contains(line, "ip:") { - parts := strings.Split(line, ":") - if len(parts) >= 2 { - nodeInfo.IP = strings.TrimSpace(parts[1]) - } + // Handle both IPv4 and IPv6 by finding the last colon for the key + idx := strings.Index(line, ":") + if idx != -1 { + nodeInfo.IP = strings.TrimSpace(line[idx+1:]) + } } }internal/pkg/interactive/nodes.go-406-412 (1)
406-412: Dead code: unused error check after hardcoded value.Line 408 sets
sectorsto a hardcoded value, then line 410 checksif err != nilwhereerris from thestream.Recv()call above (line 398), not from any sectors-related operation. This check is misleading.🔎 Proposed fix
// Парсим размер (в секторах по 512 байт) - упрощенно sectors := int64(1024*1024) // По умолчанию 512MB для демонстрации // В реальной реализации нужно читать файл /sys/block/{device}/size - if err != nil { - continue - }internal/pkg/ui/initwizard/initwizard.go-24-36 (1)
24-36:NewInitWizarddoesn't use therootDirparameter.The function accepts
rootDirbut the conditional block is empty. Either implement the directory handling or remove the parameter:🔎 Option 1: Implement rootDir handling
func NewInitWizard(rootDir string) Wizard { - // Создаем мастер с настройками по умолчанию - wizard := NewWizard() - - // Если указан rootDir, меняем рабочую директорию - if rootDir != "." { - // В реальном приложении здесь можно добавить логику - // для работы с другими директориями - } - - return wizard + wizard := NewWizard() + wizard.SetRootDir(rootDir) // Requires adding this method to WizardImpl + return wizard }🔎 Option 2: Remove unused parameter
-func NewInitWizard(rootDir string) Wizard { +func NewInitWizard() Wizard { wizard := NewWizard() - if rootDir != "." { - } return wizard }internal/pkg/ui/initwizard/wizard_state.go-21-37 (1)
21-37: Potential panic ifWizardStatevalue is out of range.The
String()method uses direct array indexing which will panic ifsis negative or >= 13. Consider adding bounds checking or using a switch/map.Proposed fix with bounds checking
func (s WizardState) String() string { + names := [...]string{ - return [...]string{ "preset", "endpoint", "scanning", "node_select", "node_config", "confirm", "generate", "done", "add_node_scan", "cozystack_scan", "vip_config", "network_config", "node_details", - }[s] + } + if s < 0 || int(s) >= len(names) { + return fmt.Sprintf("unknown(%d)", s) + } + return names[s] }Committable suggestion skipped: line range outside the PR's diff.
internal/pkg/ui/initwizard/network.go-49-55 (1)
49-55: Type assertion without safety check may panic.If the connection in the map is not a
*timedConn(e.g., after refactoring or corruption), this will panic. Use the comma-ok idiom.Proposed fix
if conn, exists := p.connections[key]; exists { - // Проверяем, не истекло ли соединение - if time.Since(conn.(*timedConn).lastUsed) < p.maxIdle { + tc, ok := conn.(*timedConn) + if !ok { + conn.Close() + delete(p.connections, key) + p.metrics.Closed++ + } else if time.Since(tc.lastUsed) < p.maxIdle { p.metrics.Reused++ - conn.(*timedConn).lastUsed = time.Now() + tc.lastUsed = time.Now() return conn, nil + } else { + conn.Close() + delete(p.connections, key) + p.metrics.Closed++ } - // Закрываем истекшее соединение - conn.Close() - delete(p.connections, key) - p.metrics.Closed++ }internal/pkg/interactive/template.go-61-64 (1)
61-64: Ignored error fromfilepath.Relcould cause incorrect paths.The error from
filepath.Relis silently ignored. If this fails,relPathcould be empty or incorrect, leading to wrong template paths being stored.Proposed fix
- relPath, _ := filepath.Rel(tm.rootDir, path) - templates = append(templates, relPath) + relPath, err := filepath.Rel(tm.rootDir, path) + if err != nil { + return fmt.Errorf("failed to get relative path for %s: %v", path, err) + } + templates = append(templates, relPath)
🧹 Nitpick comments (30)
internal/pkg/ui/initwizard/types.go (2)
33-45: DuplicateNodeInfotype across packages.There's a
NodeInfotype ininternal/pkg/interactive/nodes.gowith different fields (IP,Hostname,Status,Version). This could cause confusion when both packages are used together. Consider consolidating into a single shared type or renaming one to clarify its purpose (e.g.,InitNodeInfovsRuntimeNodeInfo).Additionally,
NodeInfolacks JSON/YAML struct tags, unlike the nestedHardwaretypes. If this struct will be serialized, add appropriate tags for consistency.
4-30: Consider adding struct tags toInitData.If
InitDatawill be serialized to JSON/YAML (e.g., for caching wizard state or configuration persistence), add appropriate struct tags. The nested types likeHardwareandValuesYAMLhave tags, butInitDatadoes not.internal/pkg/interactive/nodes.go (2)
20-21: Inconsistent comment language.Comments are in Russian, but code identifiers and some other project files use English. Consider translating to English for consistency across the codebase.
Also applies to: 28-29, 34-35, 42-43
251-257: Use constants for file mode bit checks.Magic numbers like
040000and120000reduce readability. Useos.ModeDirandos.ModeSymlink:🔎 Proposed fix
+import "io/fs" + typeName := "файл" - if info.Mode&040000 != 0 { + if fs.FileMode(info.Mode).IsDir() { typeName = "директория" - } else if info.Mode&120000 != 0 { + } else if fs.FileMode(info.Mode)&fs.ModeSymlink != 0 { typeName = "символическая ссылка" }internal/pkg/ui/initwizard/errors.go (2)
213-218: StubbedgetCallerLocation()provides no useful information.The function always returns
"initwizard", which defeats the purpose of location tracking. Consider implementing it properly usingruntime.Caller:🔎 Proposed fix
+import "runtime" + func getCallerLocation() string { - // Упрощенная версия для получения местоположения - // В реальном приложении можно использовать более продвинутые методы - return "initwizard" + _, file, line, ok := runtime.Caller(2) + if !ok { + return "unknown" + } + return fmt.Sprintf("%s:%d", filepath.Base(file), line) }
40-41:StackTracefield is declared but never populated.The
StackTracefield inAppErroris never set by any constructor or method. Either implement stack trace capture or remove the unused field.pkg/commands/interactive_init.go (1)
21-24: MissingShortdescription for the command.The command has
Longbut noShortdescription, which is shown in the parent command's help output:🔎 Proposed fix
var interactiveCmd = &cobra.Command{ Use: "interactive", + Short: "Start interactive TUI for cluster configuration", Long: `Start a terminal-based UI (TUI) similar to talos-bootstrap.`, Args: cobra.NoArgs,internal/pkg/ui/initwizard/wizard_transitions.go (1)
38-46: Missing back/cancel transitions for VIP, Network, and NodeDetails states.
StateVIPConfig,StateNetworkConfig, andStateNodeDetailsonly have forward transitions. Users may get stuck if they need to go back:🔎 Suggested additions
StateVIPConfig: { StateNetworkConfig, + StateNodeConfig, // back }, StateNetworkConfig: { StateNodeDetails, + StateVIPConfig, // back }, StateNodeDetails: { StateConfirm, + StateNetworkConfig, // back },internal/pkg/ui/initwizard/validator.go (2)
52-59: Compile regexes at package level to avoid repeated compilation.Regexes are compiled on every validation call. Move to package-level variables:
🔎 Proposed fix
+var ( + validClusterNameRegex = regexp.MustCompile(`^[a-z0-9-]+$`) + validHostnameRegex = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?)*$`) +) + func (v *ValidatorImpl) ValidateClusterName(name string) error { // ... - validName := regexp.MustCompile(`^[a-z0-9-]+$`) - if !validName.MatchString(name) { + if !validClusterNameRegex.MatchString(name) {Also applies to: 84-91
54-67: Mixed language in error messages.Lines 56-67 use Russian while other error messages use English. Standardize to one language for consistency.
internal/pkg/generator/write.go (1)
56-59: Hardcoded version "0.1.0" should be configurable.The chart version is hardcoded. Consider making it configurable via
Optionsor deriving it from the project version:- content = fmt.Sprintf(content, clusterName, "0.1.0") + content = fmt.Sprintf(content, clusterName, opts.ChartVersion)internal/pkg/ui/initwizard/wizard_controller.go (2)
8-11: Consider adding a getter for thedatafield.The
datafield is stored but not accessible outside the controller. If other components need to access theInitData, consider adding aGetData()method. Alternatively, ifdatais intentionally private and accessed only internally, this is fine as-is.
14-19: Consider validating thedataparameter.The constructor accepts
*InitDatawithout nil validation. Ifnilis passed, it could cause issues when the data is eventually accessed.Proposed fix
func NewWizardController(data *InitData) *WizardController { + if data == nil { + data = &InitData{} + } return &WizardController{ state: StatePreset, data: data, } }internal/pkg/generator/generate.go (1)
52-57: Wrap error with context for consistency.Other errors in this function include context (e.g., "invalid talos-version"), but this one doesn't.
Proposed fix
absolutePath, err := filepath.Abs(opts.RootDir) if err != nil { - return err + return fmt.Errorf("failed to resolve absolute path for %s: %w", opts.RootDir, err) }internal/pkg/ui/initwizard/generate.go (1)
172-177: Silent error recovery may mask configuration issues.When YAML parsing fails, the function silently returns the original content. Consider logging a warning so users are aware of the issue.
Proposed fix
if err := yaml.Unmarshal([]byte(content), &values); err != nil { - // Если не удалось распарсить, возвращаем оригинальный контент + log.Printf("WARNING: Failed to parse values.yaml for formatting: %v", err) return content }internal/pkg/ui/initwizard/cache.go (1)
240-244:InvalidateClusterConfigclears entire cache, not just cluster entries.The comment says "Simplified implementation" but this could cause unintended data loss. Consider implementing prefix-based deletion or documenting this behavior clearly.
Proposed fix for prefix-based deletion
func (cc *ConfigCache) InvalidateClusterConfig(clusterName string) { - // Simplified implementation - in a real application, prefixes can be used - cc.cache.Clear() + cc.cache.mutex.Lock() + defer cc.cache.mutex.Unlock() + prefix := clusterName + ":" + for key := range cc.cache.data { + if strings.HasPrefix(key, prefix) { + delete(cc.cache.data, key) + } + } }internal/pkg/ui/initwizard/wizard.go (1)
231-250: Inconsistent node filename pattern.First node uses
"nodes/node1.yaml"(line 230), but additional nodes use"nodes/node-%d.yaml"(line 238). This inconsistency may cause confusion.Proposed fix for consistency
-nodeFileName := fmt.Sprintf("nodes/node-%d.yaml", len(w.getExistingNodes())+1) +nodeFileName := fmt.Sprintf("nodes/node%d.yaml", len(w.getExistingNodes())+1)internal/pkg/interactive/wizard.go (2)
62-80: Navigation logic may cause confusion.When Escape/Ctrl+C is pressed on non-main pages, it hides "main" and shows "menu". However, if the user is on a different page (e.g., "nodes_info"), hiding "main" has no effect. Consider using
SwitchToPageor tracking the current page.Proposed fix
case tcell.KeyCtrlC, tcell.KeyEscape: - if w.pages.GetPageCount() == 1 { + name, _ := w.pages.GetFrontPage() + if name == "menu" { w.app.Stop() return nil } - w.pages.HidePage("main") - w.pages.ShowPage("menu") + w.pages.SwitchToPage("menu") return nil
452-491: Use cancellable context for long-running operations.
context.Background()prevents cancellation if the user navigates away. Pass a cancellable context that can be cancelled on page hide.Proposed fix
func (w *Wizard) renderTemplates() { w.pages.HidePage("template") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go func() { - ctx := context.Background() templates := w.templateManager.GetTemplateFiles() if len(templates) == 0 { w.showErrorModal("Нет шаблонов для рендеринга") w.pages.ShowPage("template") return } result, err := w.templateManager.RenderTemplates(ctx, templates, nil) // ... }() }internal/pkg/ui/initwizard/network.go (1)
278-284: Remove the customminfunction and use Go's builtin instead.The project targets Go 1.24.0, which includes the builtin
minfunction introduced in Go 1.21. Replace the custom implementation with the stdlib function.internal/pkg/ui/initwizard/processor.go (2)
73-86: Duplicate removal is performed twice.
ProcessScanResultscallsRemoveDuplicatesByMACand thenFilterAndSortNodes. However,FilterAndSortNodes(lines 31-43) already contains duplicate removal logic by MAC address. This results in redundant processing.Consider removing the duplicate detection from
FilterAndSortNodesand keeping it only inRemoveDuplicatesByMAC, or vice versa.Proposed fix
// ProcessScanResults обрабатывает результаты сканирования сети func (p *DataProcessorImpl) ProcessScanResults(results []NodeInfo) []NodeInfo { if len(results) == 0 { return results } - // Удаляем дубликаты - processed := p.RemoveDuplicatesByMAC(results) - // Фильтруем и сортируем - processed = p.FilterAndSortNodes(processed) + processed := p.FilterAndSortNodes(results) return processed }
168-219: Performance concern: CalculateResourceStats called repeatedly during sorting.The sorting methods (
SortNodesByCPU,SortNodesByRAM,SortNodesByDisks) callCalculateResourceStatsinside the comparison function. For a list of N nodes, this results in O(N log N) calls toCalculateResourceStatsper sort, with each call iterating over hardware processors.For better performance, pre-compute resource stats once before sorting.
Example optimization for SortNodesByCPU
// SortNodesByCPU сортирует ноды по количеству CPU ядер func (p *DataProcessorImpl) SortNodesByCPU(nodes []NodeInfo, ascending bool) []NodeInfo { sorted := make([]NodeInfo, len(nodes)) copy(sorted, nodes) + + // Pre-compute CPU stats to avoid repeated calculations + cpuStats := make([]int, len(sorted)) + for i := range sorted { + cpuStats[i], _, _ = p.CalculateResourceStats(sorted[i]) + } sort.Slice(sorted, func(i, j int) bool { - cpuI, _, _ := p.CalculateResourceStats(sorted[i]) - cpuJ, _, _ := p.CalculateResourceStats(sorted[j]) - if ascending { - return cpuI < cpuJ + return cpuStats[i] < cpuStats[j] } - return cpuI > cpuJ + return cpuStats[i] > cpuStats[j] }) return sorted }internal/pkg/interactive/template.go (1)
3-13: Replace deprecatedio/ioutilwithospackage.The
io/ioutilpackage has been deprecated since Go 1.16. Useos.ReadDirinstead ofioutil.ReadDirandos.WriteFileinstead ofioutil.WriteFile.Proposed fix
import ( "context" "fmt" - "io/ioutil" "os" "path/filepath" "strings"Then update usages:
- Line 41:
ioutil.ReadDir→os.ReadDir- Line 80:
ioutil.ReadDir→os.ReadDir- Line 165:
ioutil.WriteFile→os.WriteFileinternal/pkg/ui/initwizard/interfaces.go (2)
10-18: Private methods in public interface restrict external implementations.The
Wizardinterface exposes private methods (getData,getApp,getPages,setupInputCapture) which can only be implemented by types in the same package. This is likely intentional for internal use, but if external implementations are ever needed, these would need to be exported.
55-60: Inconsistent return types for secrets bundle operations.
LoadSecretsBundlereturnsinterface{}whileSaveSecretsBundleaccepts*secrets.Bundle. This asymmetry requires callers to perform type assertions after loading.Consider using consistent types:
Proposed fix
// Secrets management GenerateSecretsBundle(data *InitData) error - LoadSecretsBundle() (interface{}, error) + LoadSecretsBundle() (*secrets.Bundle, error) ValidateSecretsBundle() error SaveSecretsBundle(bundle *secrets.Bundle) errorinternal/pkg/ui/initwizard/presenter.go (2)
199-199:strings.Titleis deprecated since Go 1.18.Use
cases.Titlefromgolang.org/x/text/casespackage instead.Proposed fix
Add import:
import "golang.org/x/text/cases" import "golang.org/x/text/language"Replace usage:
- form.SetBorder(true).SetTitle(fmt.Sprintf("%s Preset - Дополнительная Конфигурация", strings.Title(data.Preset))).SetTitleAlign(tview.AlignLeft) + caser := cases.Title(language.English) + form.SetBorder(true).SetTitle(fmt.Sprintf("%s Preset - Дополнительная Конфигурация", caser.String(data.Preset))).SetTitleAlign(tview.AlignLeft)
1336-1350: Redundant conditional branches with identical values.Both
genericandcozystackpresets set identical default values forPodSubnetsandServiceSubnets. The conditional is unnecessary.Proposed simplification
if data.PodSubnets == "" { - if data.Preset == "cozystack" { - data.PodSubnets = "10.244.0.0/16" - } else { - data.PodSubnets = "10.244.0.0/16" - } + data.PodSubnets = "10.244.0.0/16" } if data.ServiceSubnets == "" { - if data.Preset == "cozystack" { - data.ServiceSubnets = "10.96.0.0/16" - } else { - data.ServiceSubnets = "10.96.0.0/16" - } + data.ServiceSubnets = "10.96.0.0/16" }internal/pkg/ui/initwizard/scanner.go (3)
570-573: Context variable shadowing may cause confusion.Line 572 creates a new context with cancel that shadows the parent
ctx. While child context cancellation works correctly (parent cancel propagates to child), the newcancelvariable shadows access to the parent's cancel. This is technically correct but can be confusing.Consider using a different variable name for clarity.
Proposed fix
- ctx, cancel := context.WithCancel(ctx) - defer cancel() + scanCtx, scanCancel := context.WithCancel(ctx) + defer scanCancel() for w := 0; w < numWorkers; w++ {Then update usages of
ctxwithin the worker goroutines to usescanCtx.
714-725: Wasteful command creation pattern.A command is created on line 715, then immediately replaced on line 725 with a context-aware version. This is inefficient.
Proposed fix
// Первая попытка с таймаутом { log.Printf("[FIXED] Выполняем первую команду nmap...") - cmd := exec.Command("nmap", "-p", "50000", "--open", "-oG", "-", cidr) // Проверяем отмену перед выполнением команды select { case <-ctx.Done(): log.Printf("[FIXED] Сканирование отменено перед первой командой nmap") return nil, fmt.Errorf("сканирование отменено: %v", ctx.Err()) default: } - cmd = exec.CommandContext(ctx, cmd.Args[0], cmd.Args[1:]...) + cmd := exec.CommandContext(ctx, "nmap", "-p", "50000", "--open", "-oG", "-", cidr) output, err = cmd.Output()
566-569: Magic number for early termination threshold.
targetNodes := 3is hardcoded. Consider making this configurable viaNetworkScannerImplfields or a config parameter.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
internal/pkg/ui/initwizard/debug.logis excluded by!**/*.log
📒 Files selected for processing (22)
internal/pkg/generator/generate.gointernal/pkg/generator/write.gointernal/pkg/interactive/nodes.gointernal/pkg/interactive/template.gointernal/pkg/interactive/wizard.gointernal/pkg/ui/initwizard/cache.gointernal/pkg/ui/initwizard/errors.gointernal/pkg/ui/initwizard/factory.gointernal/pkg/ui/initwizard/generate.gointernal/pkg/ui/initwizard/initwizard.gointernal/pkg/ui/initwizard/interfaces.gointernal/pkg/ui/initwizard/network.gointernal/pkg/ui/initwizard/presenter.gointernal/pkg/ui/initwizard/processor.gointernal/pkg/ui/initwizard/scanner.gointernal/pkg/ui/initwizard/types.gointernal/pkg/ui/initwizard/validator.gointernal/pkg/ui/initwizard/wizard.gointernal/pkg/ui/initwizard/wizard_controller.gointernal/pkg/ui/initwizard/wizard_state.gointernal/pkg/ui/initwizard/wizard_transitions.gopkg/commands/interactive_init.go
🧰 Additional context used
🧬 Code graph analysis (14)
internal/pkg/generator/write.go (2)
internal/pkg/generator/generate.go (1)
Options(16-23)pkg/generated/presets.go (1)
PresetFiles(3-435)
internal/pkg/ui/initwizard/initwizard.go (4)
internal/pkg/ui/initwizard/wizard.go (1)
NewWizard(27-61)internal/pkg/interactive/wizard.go (2)
NewWizard(26-47)Wizard(16-23)internal/pkg/ui/initwizard/types.go (1)
InitData(4-30)internal/pkg/ui/initwizard/interfaces.go (1)
Wizard(11-18)
internal/pkg/ui/initwizard/validator.go (2)
internal/pkg/ui/initwizard/interfaces.go (1)
Validator(21-33)internal/pkg/ui/initwizard/errors.go (2)
NewValidationError(124-126)NewValidationErrorWithCause(129-131)
internal/pkg/ui/initwizard/types.go (1)
internal/pkg/interactive/nodes.go (1)
NodeInfo(21-26)
internal/pkg/ui/initwizard/generate.go (2)
internal/pkg/ui/initwizard/types.go (1)
InitData(4-30)pkg/generated/presets.go (2)
AvailablePresets(437-440)PresetFiles(3-435)
internal/pkg/ui/initwizard/network.go (1)
internal/pkg/ui/initwizard/errors.go (3)
NewNetworkErrorWithCause(139-141)WrapError(221-226)ErrNetwork(15-15)
internal/pkg/ui/initwizard/wizard_controller.go (2)
internal/pkg/ui/initwizard/wizard_state.go (2)
WizardState(3-3)StatePreset(6-6)internal/pkg/ui/initwizard/types.go (1)
InitData(4-30)
internal/pkg/ui/initwizard/cache.go (2)
internal/pkg/ui/initwizard/types.go (2)
NodeInfo(33-45)Hardware(53-58)internal/pkg/interactive/nodes.go (1)
NodeInfo(21-26)
internal/pkg/ui/initwizard/wizard_transitions.go (1)
internal/pkg/ui/initwizard/wizard_state.go (13)
WizardState(3-3)StatePreset(6-6)StateEndpoint(7-7)StateScanning(8-8)StateNodeSelect(9-9)StateNodeConfig(10-10)StateConfirm(11-11)StateGenerate(12-12)StateDone(13-13)StateCozystackScan(15-15)StateVIPConfig(16-16)StateNetworkConfig(17-17)StateNodeDetails(18-18)
internal/pkg/interactive/template.go (2)
internal/pkg/generator/generate.go (1)
Options(16-23)pkg/modeline/modeline.go (2)
GenerateModeline(75-97)ReadAndParseModeline(54-72)
internal/pkg/ui/initwizard/interfaces.go (2)
internal/pkg/ui/initwizard/types.go (5)
InitData(4-30)NodeInfo(33-45)Hardware(53-58)ChartYAML(122-130)ValuesYAML(97-113)internal/pkg/interactive/nodes.go (1)
NodeInfo(21-26)
pkg/commands/interactive_init.go (2)
pkg/commands/root.go (1)
GlobalArgs(24-24)internal/pkg/ui/initwizard/initwizard.go (1)
RunInitWizard(7-10)
internal/pkg/ui/initwizard/processor.go (2)
internal/pkg/ui/initwizard/interfaces.go (1)
DataProcessor(36-42)internal/pkg/ui/initwizard/types.go (4)
NodeInfo(33-45)Hardware(53-58)Memory(68-70)Hostname(48-50)
internal/pkg/ui/initwizard/presenter.go (5)
internal/pkg/ui/initwizard/wizard_state.go (14)
WizardState(3-3)StatePreset(6-6)StateEndpoint(7-7)StateScanning(8-8)StateNodeSelect(9-9)StateNodeConfig(10-10)StateConfirm(11-11)StateGenerate(12-12)StateDone(13-13)StateAddNodeScan(14-14)StateCozystackScan(15-15)StateVIPConfig(16-16)StateNetworkConfig(17-17)StateNodeDetails(18-18)internal/pkg/ui/initwizard/types.go (3)
InitData(4-30)Hostname(48-50)Interface(90-94)internal/pkg/ui/initwizard/interfaces.go (1)
Wizard(11-18)internal/pkg/ui/initwizard/wizard_controller.go (2)
WizardController(8-11)NewWizardController(14-19)internal/pkg/ui/initwizard/generate.go (1)
GenerateFromTUI(21-105)
🔇 Additional comments (22)
internal/pkg/ui/initwizard/types.go (1)
96-130: LGTM!
ValuesYAMLandChartYAMLare well-structured with appropriate YAML tags for Helm chart generation.internal/pkg/ui/initwizard/errors.go (2)
91-97:Is()method uses OR logic for Type and Code matching.The current implementation returns
trueif eitherTypeorCodematches, which could cause unexpected behavior witherrors.Is(). Typically, both should match for equality. Consider whether this is the intended semantics.// Current: matches if EITHER Type OR Code matches return e.Type == appErr.Type || e.Code == appErr.Code
43-131: LGTM on error type hierarchy and constructors.The typed error constructors (
NewValidationError,NewNetworkError, etc.) provide a clean API for creating domain-specific errors with consistent structure.internal/pkg/ui/initwizard/wizard_transitions.go (1)
49-56: LGTM onisAllowed()implementation.Simple and correct linear search for the allowed transitions lookup.
internal/pkg/ui/initwizard/validator.go (1)
196-211: Both "controlplane" and "control-plane" are valid node types.This allows two spellings which could cause issues downstream. Consider normalizing to a single canonical form:
// Option: Normalize input before validation nodeType = strings.ReplaceAll(nodeType, "-", "")internal/pkg/generator/write.go (1)
25-42: LGTM onwriteFileimplementation.Proper handling of Force flag, directory creation, and error wrapping.
internal/pkg/ui/initwizard/initwizard.go (2)
7-10: LGTM onRunInitWizardwrapper.Clean delegation to the wizard's Run method.
12-16: No action needed. ThecheckExistingFiles()method exists as an unexported method onWizardImpl(line 148 of wizard.go) and is correctly called on the concrete type returned byNewWizard(). It is appropriately not included in theWizardinterface since it's an internal implementation detail.internal/pkg/ui/initwizard/wizard_controller.go (1)
21-34: LGTM!The
Transitionmethod correctly validates state transitions and provides detailed debug logging. The error message includes both source and target states which aids debugging.internal/pkg/ui/initwizard/wizard_state.go (1)
5-19: LGTM!State enumeration is well-defined with clear, descriptive names for each wizard step.
internal/pkg/generator/generate.go (2)
16-23: LGTM!Options struct is well-defined with clear field names for configuration.
67-69: Hardcoded default API server URL may not be appropriate.The default
https://192.168.0.1:6443is a private IP that may not match the user's environment. Consider either requiring this field or documenting the default behavior clearly.internal/pkg/ui/initwizard/generate.go (2)
20-22: LGTM on function signature and documentation.Clear entry point for TUI-driven configuration generation.
296-312: LGTM!The
writeToDestinationfunction properly checks for existing files and creates parent directories as needed.internal/pkg/ui/initwizard/network.go (1)
11-18: LGTM on ConnectionPool structure.Well-designed pool with idle timeout, lifetime management, and metrics tracking.
internal/pkg/ui/initwizard/cache.go (1)
15-23: LGTM on Cache struct design.Clean TTL-based cache with metric tracking.
internal/pkg/ui/initwizard/wizard.go (2)
26-61: LGTM on wizard construction.Clean dependency injection with all components properly wired together.
64-119: LGTM on Run method.Good defensive checks for nil presenter and data, with proper logging and error handling.
internal/pkg/interactive/wizard.go (1)
25-47: LGTM on wizard initialization.Clean setup with proper component instantiation.
internal/pkg/ui/initwizard/scanner.go (1)
36-64: Good implementation of network scanning with proper error handling.The
ScanNetworkmethod follows a clear flow: scan for nodes with open ports, then collect detailed information in parallel. The diagnostic logging and error propagation are well-structured.internal/pkg/ui/initwizard/processor.go (1)
66-71: Unused stub method in interface - either implement or remove fromDataProcessor.
ExtractHardwareInfois declared in theDataProcessorinterface but not called anywhere in the codebase. The current implementation returns an error unconditionally. Either remove the method from the interface if it's not needed, or implement it properly if it's required for the interface contract.internal/pkg/interactive/template.go (1)
133-139:engine.Renderreturns[]byte, not a map. The conversion logic is flawed.The code iterates over
result(which is[]byte) usingrange, which yields indices (int) and byte values (uint8). Converting these to strings produces keys like"0","1", etc., with corresponding byte representations as values. This does not meaningfully convert the rendered template output into a usablemap[string]string. If the intent is to parse YAML or similar structured content, use the appropriate unmarshaling function instead.Likely an incorrect or invalid review comment.
| // startInitWizard запускает мастер инициализации | ||
| func (w *Wizard) startInitWizard() { | ||
| w.pages.HidePage("menu") | ||
|
|
||
| // Запускаем init wizard | ||
| go func() { | ||
| if err := w.initWizard.Run(); err != nil { | ||
| w.showErrorModal(fmt.Sprintf("Ошибка инициализации: %v", err)) | ||
| } else { | ||
| w.showSuccessModal("Проект успешно инициализирован!") | ||
| } | ||
| w.pages.ShowPage("menu") | ||
| }() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the repository structure
git ls-files | grep -E "(wizard|init)" | head -20Repository: cozystack/talm
Length of output: 888
🏁 Script executed:
# Run the provided verification script to check if NewInitWizard creates tview.Application
ast-grep --pattern $'func NewInitWizard($_) $_ {
$$$
tview.NewApplication()
$$$
}'Repository: cozystack/talm
Length of output: 40
🏁 Script executed:
# Also search for NewInitWizard definition
rg "func NewInitWizard" -A 10Repository: cozystack/talm
Length of output: 809
🏁 Script executed:
# Search for the initWizard.Run() implementation
rg "func.*Run\(\)" -A 20 | grep -A 20 "initWizard"Repository: cozystack/talm
Length of output: 40
🏁 Script executed:
# Check the imports and struct definitions in wizard.go
head -50 internal/pkg/interactive/wizard.goRepository: cozystack/talm
Length of output: 1161
🏁 Script executed:
# Search for NewWizard in the initwizard package
rg "func NewWizard" internal/pkg/ui/initwizard/ -A 15Repository: cozystack/talm
Length of output: 3353
🏁 Script executed:
# Look for the Run() method in initwizard package
rg "func.*\(w.*\).*Run\(\)" internal/pkg/ui/initwizard/ -A 20Repository: cozystack/talm
Length of output: 1569
🏁 Script executed:
# Check the Wizard interface definition in initwizard package
rg "type Wizard" internal/pkg/ui/initwizard/ -A 10Repository: cozystack/talm
Length of output: 4786
🏁 Script executed:
# Check if there are any tview.Application creations in the initwizard package
rg "tview.NewApplication" internal/pkg/ui/initwizard/Repository: cozystack/talm
Length of output: 125
🏁 Script executed:
# Get more details from wizard.go in initwizard package
head -100 internal/pkg/ui/initwizard/wizard.goRepository: cozystack/talm
Length of output: 2622
🏁 Script executed:
# Get the full Run() method implementation
sed -n '149,250p' internal/pkg/ui/initwizard/wizard.goRepository: cozystack/talm
Length of output: 3282
🏁 Script executed:
# Check if there's any app.Run() or app.SetRoot() call in initwizard
rg "\.Run\(\)|\.SetRoot" internal/pkg/ui/initwizard/ -B 2 -A 2Repository: cozystack/talm
Length of output: 1143
🏁 Script executed:
# Check the entire startInitWizard function in wizard.go
sed -n '147,165p' internal/pkg/interactive/wizard.goRepository: cozystack/talm
Length of output: 540
🏁 Script executed:
# Search for how the outer wizard's app is managed during initWizard execution
rg "app\.Run|app\.Stop" internal/pkg/interactive/wizard.go -B 3 -A 3Repository: cozystack/talm
Length of output: 808
Lifecycle conflict: two tview.Application instances running concurrently.
initWizard.Run() creates and runs its own tview.Application (via app.SetRoot(w.pages, true).SetFocus(w.pages).Run() at line ~144 of initwizard/wizard.go). Running this in a goroutine while the outer wizard's app.Run() is already active will cause terminal conflicts and race conditions—tview applications require exclusive terminal access and cannot coexist.
The inner and outer wizards need to share the same tview.Application instance or the architecture must be redesigned to not nest application lifecycles.
🤖 Prompt for AI Agents
internal/pkg/interactive/wizard.go lines 147-160: the current code starts
initWizard.Run() in a goroutine even though initWizard.Run creates and runs its
own tview.Application, causing two concurrent tview applications; instead,
refactor to reuse the parent tview.Application by (1) changing the init wizard
API to accept an existing *tview.Application (or provide a RunWithApp(app
*tview.Application) / StartWithApp method) that only sets the root and focus
(SetRoot/SetFocus) without calling app.Run(), (2) pass the outer wizard's app
into the init wizard and call the new RunWithApp synchronously (or via
app.QueueUpdateDraw) rather than starting a separate goroutine, and (3) remove
the inner app.Run to ensure a single application lifecycle is used.
| // Создаем wizard | ||
| wizard := &WizardImpl{ | ||
| data: data, | ||
| app: nil, // Будет создан в презентере | ||
| pages: nil, // Будет создан в презентере | ||
| validator: components.Validator, | ||
| scanner: components.Scanner, | ||
| processor: components.Processor, | ||
| generator: components.Generator, | ||
|
|
||
| } | ||
|
|
||
| // Создаем презентер | ||
| // Создаем презентер с базовыми параметрами | ||
| // Презентер будет настроен в презентере | ||
| components.Presenter = nil // Временное значение | ||
| wizard.presenter = components.Presenter | ||
|
|
||
| return wizard, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how WizardImpl is initialized and used
rg -n "BuildWizard|WizardImpl\{" --type go -A 10Repository: cozystack/talm
Length of output: 4540
🏁 Script executed:
#!/bin/bash
# View the complete BuildWizard function
sed -n '215,260p' internal/pkg/ui/initwizard/factory.goRepository: cozystack/talm
Length of output: 1262
🏁 Script executed:
#!/bin/bash
# Find WizardImpl struct definition
rg -n "^type WizardImpl struct" --type go -A 15Repository: cozystack/talm
Length of output: 1072
🏁 Script executed:
#!/bin/bash
# Find all WizardImpl methods
rg -n "func \(w \*WizardImpl\)" --type go | head -20Repository: cozystack/talm
Length of output: 2007
🏁 Script executed:
#!/bin/bash
# Check the Run() method which is likely the main entry point
sed -n '64,121p' internal/pkg/ui/initwizard/wizard.goRepository: cozystack/talm
Length of output: 1952
🏁 Script executed:
#!/bin/bash
# Search for where app and pages are set in WizardImpl
rg -n "w\.app\s*=" --type goRepository: cozystack/talm
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check getApp() and getPages() implementations
sed -n '127,136p' internal/pkg/ui/initwizard/wizard.goRepository: cozystack/talm
Length of output: 264
🏁 Script executed:
#!/bin/bash
# Search for any setter methods for app or pages
rg -n "SetApp|SetPages|set.*app|set.*pages" --type go -iRepository: cozystack/talm
Length of output: 2644
🏁 Script executed:
#!/bin/bash
# Search for where tview.Application and tview.Pages are created
rg -n "NewApplication|tview\.Application\{|tview\.NewPages|tview\.Pages\{" --type goRepository: cozystack/talm
Length of output: 703
🏁 Script executed:
#!/bin/bash
# View the full NewWizard function to compare with BuildWizard
sed -n '27,62p' internal/pkg/ui/initwizard/wizard.goRepository: cozystack/talm
Length of output: 881
🏁 Script executed:
#!/bin/bash
# Find all usages of BuildWizard in the codebase
rg -n "BuildWizard\(\)" --type go -B 2 -A 5Repository: cozystack/talm
Length of output: 2453
🏁 Script executed:
#!/bin/bash
# Find usages of CreateDefaultWizard, CreateWizardWithConfig, CreateMinimalWizard
rg -n "CreateDefaultWizard|CreateWizardWithConfig|CreateMinimalWizard" --type go | grep -v "^internal/pkg/ui/initwizard/factory.go:[0-9]*:func"Repository: cozystack/talm
Length of output: 380
🏁 Script executed:
#!/bin/bash
# Check if there are any actual calls to the factory methods or if NewWizard is used instead
rg -n "NewWizard\(\)|CreateDefaultWizard\(\)|Factory\(\)" --type go | grep -v "^internal/pkg/ui/initwizard/factory.go" | grep -v "^internal/pkg/ui/initwizard/wizard.go"Repository: cozystack/talm
Length of output: 435
BuildWizard returns WizardImpl with all UI components (app, pages, presenter) set to nil with no initialization mechanism.
The created WizardImpl has app, pages, and presenter all set to nil. Comments suggest these will be "created in the presenter," but presenter itself is nil, making this impossible. More critically, the Run() method calls w.app.SetRoot(w.pages, true) without any nil checks, guaranteeing a nil pointer panic when invoked. There are no setter methods or other mechanisms to initialize these fields after creation.
Consider either:
- Implementing proper initialization like
NewWizard()does (creatingtview.NewApplication(),tview.NewPages(), andNewPresenter()) - Removing this broken factory code if it is not actively used
| func (wc *WizardComponents) getNetworkStats() NetworkStats { | ||
| var poolMetrics PoolMetrics | ||
| if wc.ConnectionPool != nil { | ||
| poolMetrics = wc.ConnectionPool.GetMetrics() | ||
| } | ||
|
|
||
| return NetworkStats{ | ||
| PoolSize: wc.ConnectionPool.Size(), | ||
| PoolMetrics: poolMetrics, | ||
| RateLimiterEnabled: wc.RateLimiter != nil, | ||
| MaxWorkers: wc.config.MaxWorkers, | ||
| RequestTimeout: wc.config.RequestTimeout, | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential nil pointer dereference in getNetworkStats.
ConnectionPool.Size() is called unconditionally on line 330, but line 325-327 only check if it's nil for GetMetrics(). If ConnectionPool is nil, line 330 will panic.
Proposed fix
func (wc *WizardComponents) getNetworkStats() NetworkStats {
var poolMetrics PoolMetrics
+ var poolSize int
if wc.ConnectionPool != nil {
poolMetrics = wc.ConnectionPool.GetMetrics()
+ poolSize = wc.ConnectionPool.Size()
}
return NetworkStats{
- PoolSize: wc.ConnectionPool.Size(),
+ PoolSize: poolSize,
PoolMetrics: poolMetrics,
RateLimiterEnabled: wc.RateLimiter != nil,
MaxWorkers: wc.config.MaxWorkers,
RequestTimeout: wc.config.RequestTimeout,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (wc *WizardComponents) getNetworkStats() NetworkStats { | |
| var poolMetrics PoolMetrics | |
| if wc.ConnectionPool != nil { | |
| poolMetrics = wc.ConnectionPool.GetMetrics() | |
| } | |
| return NetworkStats{ | |
| PoolSize: wc.ConnectionPool.Size(), | |
| PoolMetrics: poolMetrics, | |
| RateLimiterEnabled: wc.RateLimiter != nil, | |
| MaxWorkers: wc.config.MaxWorkers, | |
| RequestTimeout: wc.config.RequestTimeout, | |
| } | |
| } | |
| func (wc *WizardComponents) getNetworkStats() NetworkStats { | |
| var poolMetrics PoolMetrics | |
| var poolSize int | |
| if wc.ConnectionPool != nil { | |
| poolMetrics = wc.ConnectionPool.GetMetrics() | |
| poolSize = wc.ConnectionPool.Size() | |
| } | |
| return NetworkStats{ | |
| PoolSize: poolSize, | |
| PoolMetrics: poolMetrics, | |
| RateLimiterEnabled: wc.RateLimiter != nil, | |
| MaxWorkers: wc.config.MaxWorkers, | |
| RequestTimeout: wc.config.RequestTimeout, | |
| } | |
| } |
🤖 Prompt for AI Agents
In internal/pkg/ui/initwizard/factory.go around lines 323 to 336,
getNetworkStats calls wc.ConnectionPool.Size() without checking for nil which
can panic; modify the function to check if wc.ConnectionPool != nil before
calling Size() (similar to how GetMetrics() is handled), set a default PoolSize
(e.g., 0) when ConnectionPool is nil, and return that value in the NetworkStats
struct so all uses of ConnectionPool are guarded against nil.
| // ScanWithPool сканирование с использованием пула соединений | ||
| func (nc *NetworkClient) ScanWithPool(ctx context.Context, cidr string, operation func(context.Context, string, net.Conn) error) error { | ||
| // Упрощенная реализация - получаем список IP из CIDR | ||
| _, ipNet, err := net.ParseCIDR(cidr) | ||
| if err != nil { | ||
| return NewNetworkErrorWithCause( | ||
| "NET_002", | ||
| "некорректная CIDR нотация", | ||
| fmt.Sprintf("CIDR: %s", cidr), | ||
| err, | ||
| ) | ||
| } | ||
|
|
||
| // Создаем канал для ограничения параллелизма | ||
| const maxWorkers = 10 | ||
| workerChan := make(chan struct{}, maxWorkers) | ||
| defer close(workerChan) | ||
|
|
||
| // Обрабатываем IP адреса | ||
| for ip := ipNet.IP.Mask(ipNet.Mask); ipNet.Contains(ip); ip = nextIP(ip) { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return WrapError(ctx.Err(), ErrNetwork, "NET_003", "сканирование отменено", "операция была отменена пользователем") | ||
| case workerChan <- struct{}{}: | ||
| go func(ip net.IP) { | ||
| defer func() { <-workerChan }() | ||
|
|
||
| addr := ip.String() + ":50000" | ||
| err := nc.ExecuteWithConnection("tcp", addr, func(conn net.Conn) error { | ||
| return operation(ctx, addr, conn) | ||
| }) | ||
| if err != nil { | ||
| // Логируем ошибку, но продолжаем | ||
| // В реальном приложении можно добавить более продвинутую обработку | ||
| } | ||
| }(ip) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
ScanWithPool doesn't wait for goroutines to complete before returning.
The function spawns goroutines for each IP but returns immediately after iteration. This means:
- The caller won't know when scanning is complete
defer close(workerChan)may close the channel while goroutines are still running- Results from ongoing scans are lost
Proposed fix using sync.WaitGroup
func (nc *NetworkClient) ScanWithPool(ctx context.Context, cidr string, operation func(context.Context, string, net.Conn) error) error {
_, ipNet, err := net.ParseCIDR(cidr)
if err != nil {
return NewNetworkErrorWithCause(
"NET_002",
"некорректная CIDR нотация",
fmt.Sprintf("CIDR: %s", cidr),
err,
)
}
const maxWorkers = 10
workerChan := make(chan struct{}, maxWorkers)
- defer close(workerChan)
+ var wg sync.WaitGroup
for ip := ipNet.IP.Mask(ipNet.Mask); ipNet.Contains(ip); ip = nextIP(ip) {
select {
case <-ctx.Done():
+ wg.Wait()
return WrapError(ctx.Err(), ErrNetwork, "NET_003", "сканирование отменено", "операция была отменена пользователем")
case workerChan <- struct{}{}:
+ wg.Add(1)
go func(ip net.IP) {
- defer func() { <-workerChan }()
+ defer func() {
+ <-workerChan
+ wg.Done()
+ }()
addr := ip.String() + ":50000"
err := nc.ExecuteWithConnection("tcp", addr, func(conn net.Conn) error {
return operation(ctx, addr, conn)
})
if err != nil {
// Log error but continue
}
}(ip)
}
}
+ wg.Wait()
return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ScanWithPool сканирование с использованием пула соединений | |
| func (nc *NetworkClient) ScanWithPool(ctx context.Context, cidr string, operation func(context.Context, string, net.Conn) error) error { | |
| // Упрощенная реализация - получаем список IP из CIDR | |
| _, ipNet, err := net.ParseCIDR(cidr) | |
| if err != nil { | |
| return NewNetworkErrorWithCause( | |
| "NET_002", | |
| "некорректная CIDR нотация", | |
| fmt.Sprintf("CIDR: %s", cidr), | |
| err, | |
| ) | |
| } | |
| // Создаем канал для ограничения параллелизма | |
| const maxWorkers = 10 | |
| workerChan := make(chan struct{}, maxWorkers) | |
| defer close(workerChan) | |
| // Обрабатываем IP адреса | |
| for ip := ipNet.IP.Mask(ipNet.Mask); ipNet.Contains(ip); ip = nextIP(ip) { | |
| select { | |
| case <-ctx.Done(): | |
| return WrapError(ctx.Err(), ErrNetwork, "NET_003", "сканирование отменено", "операция была отменена пользователем") | |
| case workerChan <- struct{}{}: | |
| go func(ip net.IP) { | |
| defer func() { <-workerChan }() | |
| addr := ip.String() + ":50000" | |
| err := nc.ExecuteWithConnection("tcp", addr, func(conn net.Conn) error { | |
| return operation(ctx, addr, conn) | |
| }) | |
| if err != nil { | |
| // Логируем ошибку, но продолжаем | |
| // В реальном приложении можно добавить более продвинутую обработку | |
| } | |
| }(ip) | |
| } | |
| } | |
| return nil | |
| } | |
| // ScanWithPool сканирование с использованием пула соединений | |
| func (nc *NetworkClient) ScanWithPool(ctx context.Context, cidr string, operation func(context.Context, string, net.Conn) error) error { | |
| // Упрощенная реализация - получаем список IP из CIDR | |
| _, ipNet, err := net.ParseCIDR(cidr) | |
| if err != nil { | |
| return NewNetworkErrorWithCause( | |
| "NET_002", | |
| "некорректная CIDR нотация", | |
| fmt.Sprintf("CIDR: %s", cidr), | |
| err, | |
| ) | |
| } | |
| // Создаем канал для ограничения параллелизма | |
| const maxWorkers = 10 | |
| workerChan := make(chan struct{}, maxWorkers) | |
| var wg sync.WaitGroup | |
| // Обрабатываем IP адреса | |
| for ip := ipNet.IP.Mask(ipNet.Mask); ipNet.Contains(ip); ip = nextIP(ip) { | |
| select { | |
| case <-ctx.Done(): | |
| wg.Wait() | |
| return WrapError(ctx.Err(), ErrNetwork, "NET_003", "сканирование отменено", "операция была отменена пользователем") | |
| case workerChan <- struct{}{}: | |
| wg.Add(1) | |
| go func(ip net.IP) { | |
| defer func() { | |
| <-workerChan | |
| wg.Done() | |
| }() | |
| addr := ip.String() + ":50000" | |
| err := nc.ExecuteWithConnection("tcp", addr, func(conn net.Conn) error { | |
| return operation(ctx, addr, conn) | |
| }) | |
| if err != nil { | |
| // Логируем ошибку, но продолжаем | |
| // В реальном приложении можно добавить более продвинутую обработку | |
| } | |
| }(ip) | |
| } | |
| } | |
| wg.Wait() | |
| return nil | |
| } |
🤖 Prompt for AI Agents
internal/pkg/ui/initwizard/network.go lines 183-223: ScanWithPool spawns
goroutines for each IP but returns immediately and closes workerChan while
goroutines may still run, losing results and causing races; fix by using a
sync.WaitGroup: create a wg before the loop, call wg.Add(1) for each goroutine
before launching, defer nothing on workerChan (do not close it), have each
goroutine call defer wg.Done() and release the worker token, and after the loop
call wg.Wait() (and check ctx.Done() where appropriate) so the function only
returns after all goroutines complete; remove defer close(workerChan) to avoid
closing the channel while workers still use it.
| func (p *PresenterImpl) showCozystackScanningModal(data *InitData) { | ||
| log.Printf("[FIXED-UI] Запускаем showCozystackScanningModal") | ||
|
|
||
| // Создаем контекст с возможностью отмены | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| defer cancel() | ||
|
|
||
| // Сохраняем cancel функцию для использования при отмене | ||
| p.cancelScan = cancel | ||
|
|
There was a problem hiding this comment.
Context cancellation issue in goroutine.
The defer cancel() on line 1020 will cancel the context when showCozystackScanningModal returns, but the scanning goroutine (starting at line 1116) continues to use ctx. This means the scan will be cancelled prematurely when the modal is displayed.
Move the cancel call to dismissModal or after the goroutine completes.
Proposed fix
func (p *PresenterImpl) showCozystackScanningModal(data *InitData) {
log.Printf("[FIXED-UI] Запускаем showCozystackScanningModal")
// Создаем контекст с возможностью отмены
ctx, cancel := context.WithCancel(context.Background())
- defer cancel()
// Сохраняем cancel функцию для использования при отмене
p.cancelScan = cancelThe cancel is already called in dismissModal, so the defer cancel() is both redundant and harmful.
🤖 Prompt for AI Agents
internal/pkg/ui/initwizard/presenter.go around lines 1015-1024: the function
creates a cancellable context and assigns p.cancelScan but also calls defer
cancel(), which cancels the context as soon as showCozystackScanningModal
returns and prematurely stops the scanning goroutine; remove the defer cancel()
so the context remains valid for the goroutine, keep assigning p.cancelScan =
cancel, and ensure dismissModal (or the modal close path) calls p.cancelScan()
(with a nil-check) to perform cancellation when the modal is dismissed.
|
|
||
| // Сохраняем конфигурацию в файл | ||
| if err := os.WriteFile(configFilename, configBytes, 0o644); err != nil { | ||
| log.Printf("[MACHINE-CONFIG] Ошибка сохранения файла конфигурации: %v", err) | ||
| p.app.QueueUpdateDraw(func() { | ||
| p.ShowErrorModal(fmt.Sprintf("Ошибка сохранения конфигурации: %v", err)) | ||
| }) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
Type mismatch: engine.Render returns map, but code expects []byte.
engine.Render returns map[string][]byte (or similar), but line 1599 passes configBytes directly to os.WriteFile as if it were []byte. This will cause a compilation error or runtime panic.
Proposed fix
// Выполняем рендеринг через engine.Render
ctx := context.Background()
- configBytes, err := engine.Render(ctx, nil, opts)
+ resultMap, err := engine.Render(ctx, nil, opts)
if err != nil {
log.Printf("[MACHINE-CONFIG] Ошибка рендеринга шаблона: %v", err)
p.app.QueueUpdateDraw(func() {
p.ShowErrorModal(fmt.Sprintf("Ошибка генерации конфигурации: %v", err))
})
return
}
+
+ // Extract the rendered content for the template
+ var configBytes []byte
+ for _, content := range resultMap {
+ configBytes = content
+ break
+ }
+
+ if len(configBytes) == 0 {
+ log.Printf("[MACHINE-CONFIG] Рендеринг вернул пустой результат")
+ p.app.QueueUpdateDraw(func() {
+ p.ShowErrorModal("Рендеринг шаблона вернул пустой результат")
+ })
+ return
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
internal/pkg/ui/initwizard/presenter.go around lines 1597-1606: engine.Render
returns a map (map[string][]byte) not a single []byte, so passing configBytes
directly to os.WriteFile is a type mismatch; update the code to extract the
intended byte slice from the render map (e.g., rendered["config.json"] or the
appropriate key), validate that the key exists and is not nil, and then call
os.WriteFile with that []byte; if you intend to write multiple files, iterate
the map and write each entry to disk, handling and reporting errors for missing
keys or write failures.
| // Check that the URL contains a port | ||
| if !strings.Contains(url, ":") { | ||
| return NewValidationError( | ||
| "VAL_017", | ||
| "API server URL must contain a port (e.g., :6443)", | ||
| fmt.Sprintf("provided URL: %s", url), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Bug: Port validation incorrectly passes URLs with protocol but no port.
strings.Contains(url, ":") returns true for https://example.com because of the :// in the protocol. This allows URLs without explicit ports.
🔎 Proposed fix
// Check that the URL contains a port
- if !strings.Contains(url, ":") {
+ // Extract host:port portion after protocol
+ urlWithoutProtocol := strings.TrimPrefix(strings.TrimPrefix(url, "https://"), "http://")
+ if !strings.Contains(urlWithoutProtocol, ":") {
return NewValidationError(
"VAL_017",
"API server URL must contain a port (e.g., :6443)",
fmt.Sprintf("provided URL: %s", url),
)
}🤖 Prompt for AI Agents
In internal/pkg/ui/initwizard/validator.go around lines 249 to 256, the current
check uses strings.Contains(url, ":") which incorrectly treats
"https://example.com" as having a port due to "://"; replace this logic by
parsing the URL with net/url (url.Parse) and then inspect the parsed URL's Host
and Port (use u.Port() or net.SplitHostPort on u.Host) to ensure an explicit
port is present; also handle cases where parsing fails by returning a validation
error and treat empty u.Port() as missing port so you return the same VAL_017
error.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.