diff --git a/pkg/commands/apply.go b/pkg/commands/apply.go index 86d770b..6cab784 100644 --- a/pkg/commands/apply.go +++ b/pkg/commands/apply.go @@ -16,8 +16,9 @@ package commands import ( "context" - "errors" "fmt" + "path/filepath" + "strings" "time" "github.com/cozystack/talm/pkg/engine" @@ -77,7 +78,7 @@ var applyCmd = &cobra.Command{ } applyCmdFlags.nodesFromArgs = len(GlobalArgs.Nodes) > 0 applyCmdFlags.endpointsFromArgs = len(GlobalArgs.Endpoints) > 0 - // Set dummy endpoint to avoid errors on building clinet + // Set dummy endpoint to avoid errors on building client if len(GlobalArgs.Endpoints) == 0 { GlobalArgs.Endpoints = append(GlobalArgs.Endpoints, "127.0.0.1") } @@ -104,54 +105,51 @@ func apply(args []string) error { } for _, configFile := range expandedFiles { - if err := processModelineAndUpdateGlobals(configFile, applyCmdFlags.nodesFromArgs, applyCmdFlags.endpointsFromArgs, true); err != nil { + modelineTemplates, err := processModelineAndUpdateGlobals(configFile, applyCmdFlags.nodesFromArgs, applyCmdFlags.endpointsFromArgs, true) + if err != nil { return err } - // Resolve secrets.yaml path relative to project root if not absolute withSecretsPath := ResolveSecretsPath(applyCmdFlags.withSecrets) - opts := engine.Options{ - TalosVersion: applyCmdFlags.talosVersion, - WithSecrets: withSecretsPath, - KubernetesVersion: applyCmdFlags.kubernetesVersion, - Debug: applyCmdFlags.debug, - } - - patches := []string{"@" + configFile} - configBundle, machineType, err := engine.FullConfigProcess(ctx, opts, patches) - if err != nil { - return fmt.Errorf("full config processing error: %s", err) - } + var result []byte + if len(modelineTemplates) > 0 { + // Template rendering path: render templates via Helm engine to produce full config + opts := buildApplyRenderOptions(modelineTemplates, withSecretsPath) + result, err = engine.Render(ctx, nil, opts) + if err != nil { + return fmt.Errorf("template rendering error: %w", err) + } + } else { + // Direct patch path: apply config file as patch against empty bundle + opts := buildApplyPatchOptions(withSecretsPath) + patches := []string{"@" + configFile} + configBundle, machineType, err := engine.FullConfigProcess(ctx, opts, patches) + if err != nil { + return fmt.Errorf("full config processing error: %w", err) + } - result, err := engine.SerializeConfiguration(configBundle, machineType) - if err != nil { - return fmt.Errorf("error serializing configuration: %s", err) + result, err = engine.SerializeConfiguration(configBundle, machineType) + if err != nil { + return fmt.Errorf("error serializing configuration: %w", err) + } } withClient := func(f func(ctx context.Context, c *client.Client) error) error { if applyCmdFlags.insecure { + // Maintenance mode connects directly to the node IP without talosconfig; + // node context injection is not needed — the maintenance client handles + // node targeting internally via GlobalArgs.Nodes. return WithClientMaintenance(applyCmdFlags.certFingerprints, f) } + wrappedF := wrapWithNodeContext(f) + if GlobalArgs.SkipVerify { - return WithClientSkipVerify(f) + return WithClientSkipVerify(wrappedF) } - return WithClientNoNodes(func(ctx context.Context, cli *client.Client) error { - if len(GlobalArgs.Nodes) < 1 { - configContext := cli.GetConfigContext() - if configContext == nil { - return errors.New("failed to resolve config context") - } - - GlobalArgs.Nodes = configContext.Nodes - } - - ctx = client.WithNodes(ctx, GlobalArgs.Nodes...) - - return f(ctx, cli) - }) + return WithClientNoNodes(wrappedF) } err = withClient(func(ctx context.Context, c *client.Client) error { @@ -186,6 +184,109 @@ func apply(args []string) error { return nil } +// buildApplyRenderOptions constructs engine.Options for the template rendering path. +// Offline is set to true because at this point we don't have a Talos client for +// Helm lookup functions. Templates that use lookup() should be rendered via +// 'talm template' which supports online mode. +func buildApplyRenderOptions(modelineTemplates []string, withSecretsPath string) engine.Options { + resolvedTemplates := resolveTemplatePaths(modelineTemplates, Config.RootDir) + return engine.Options{ + TalosVersion: applyCmdFlags.talosVersion, + WithSecrets: withSecretsPath, + KubernetesVersion: applyCmdFlags.kubernetesVersion, + Debug: applyCmdFlags.debug, + Full: true, + Offline: true, + Root: Config.RootDir, + TemplateFiles: resolvedTemplates, + } +} + +// buildApplyPatchOptions constructs engine.Options for the direct patch path. +func buildApplyPatchOptions(withSecretsPath string) engine.Options { + return engine.Options{ + TalosVersion: applyCmdFlags.talosVersion, + WithSecrets: withSecretsPath, + KubernetesVersion: applyCmdFlags.kubernetesVersion, + Debug: applyCmdFlags.debug, + } +} + +// wrapWithNodeContext wraps a client action function to resolve and inject node +// context. If GlobalArgs.Nodes is already set, uses those directly. Otherwise, +// attempts to resolve nodes from the client's config context. +// This function does not mutate GlobalArgs. It reads GlobalArgs.Nodes at +// invocation time (not at wrapper creation time) and makes a defensive copy. +func wrapWithNodeContext(f func(ctx context.Context, c *client.Client) error) func(ctx context.Context, c *client.Client) error { + return func(ctx context.Context, c *client.Client) error { + nodes := append([]string(nil), GlobalArgs.Nodes...) + if len(nodes) < 1 { + if c == nil { + return fmt.Errorf("failed to resolve config context: no client available") + } + configContext := c.GetConfigContext() + if configContext == nil { + return fmt.Errorf("failed to resolve config context") + } + nodes = configContext.Nodes + } + + ctx = client.WithNodes(ctx, nodes...) + return f(ctx, c) + } +} + +// resolveTemplatePaths resolves template file paths relative to the project root, +// normalizing them for the Helm engine (forward slashes). +// Relative paths from the modeline are resolved against rootDir, not CWD. +// +// Note: template.go has similar path resolution in generateOutput() but resolves +// against CWD via filepath.Abs and has an additional fallback that tries +// templates/ when a path resolves outside the root. This function +// intentionally resolves against rootDir (modeline paths are root-relative by +// convention) and does not perform the basename fallback to avoid silently +// substituting a different file. +func resolveTemplatePaths(templates []string, rootDir string) []string { + resolved := make([]string, len(templates)) + if rootDir == "" { + // No rootDir specified — normalize paths only, don't resolve against CWD + for i, p := range templates { + resolved[i] = engine.NormalizeTemplatePath(p) + } + return resolved + } + absRootDir, rootErr := filepath.Abs(rootDir) + if rootErr != nil { + for i, p := range templates { + resolved[i] = engine.NormalizeTemplatePath(p) + } + return resolved + } + + for i, templatePath := range templates { + var absTemplatePath string + if filepath.IsAbs(templatePath) { + absTemplatePath = templatePath + } else { + // Resolve relative paths against rootDir, not CWD + absTemplatePath = filepath.Join(absRootDir, templatePath) + } + relPath, relErr := filepath.Rel(absRootDir, absTemplatePath) + if relErr != nil { + resolved[i] = engine.NormalizeTemplatePath(templatePath) + continue + } + relPath = filepath.Clean(relPath) + if strings.HasPrefix(relPath, "..") { + // Path goes outside project root — use original path as-is + resolved[i] = engine.NormalizeTemplatePath(templatePath) + continue + } + resolved[i] = engine.NormalizeTemplatePath(relPath) + } + return resolved +} + func init() { applyCmd.Flags().BoolVarP(&applyCmdFlags.insecure, "insecure", "i", false, "apply using the insecure (encrypted with no auth) maintenance service") applyCmd.Flags().StringSliceVarP(&applyCmdFlags.configFiles, "file", "f", nil, "specify config files or patches in a YAML file (can specify multiple)") diff --git a/pkg/commands/apply_test.go b/pkg/commands/apply_test.go new file mode 100644 index 0000000..e642929 --- /dev/null +++ b/pkg/commands/apply_test.go @@ -0,0 +1,243 @@ +package commands + +import ( + "context" + "os" + "path/filepath" + "slices" + "testing" + + "github.com/siderolabs/talos/pkg/machinery/client" + "google.golang.org/grpc/metadata" +) + +func TestBuildApplyRenderOptions(t *testing.T) { + origTalosVersion := applyCmdFlags.talosVersion + origKubeVersion := applyCmdFlags.kubernetesVersion + origDebug := applyCmdFlags.debug + origRootDir := Config.RootDir + defer func() { + applyCmdFlags.talosVersion = origTalosVersion + applyCmdFlags.kubernetesVersion = origKubeVersion + applyCmdFlags.debug = origDebug + Config.RootDir = origRootDir + }() + + applyCmdFlags.talosVersion = "v1.12" + applyCmdFlags.kubernetesVersion = "1.31.0" + applyCmdFlags.debug = false + Config.RootDir = "/project" + + opts := buildApplyRenderOptions( + []string{"templates/controlplane.yaml"}, + "/project/secrets.yaml", + ) + + if !opts.Full { + t.Error("expected Full=true for template rendering path") + } + if !opts.Offline { + t.Error("expected Offline=true for template rendering path") + } + if opts.Root != "/project" { + t.Errorf("expected Root=/project, got %s", opts.Root) + } + if opts.TalosVersion != "v1.12" { + t.Errorf("expected TalosVersion=v1.12, got %s", opts.TalosVersion) + } + if opts.WithSecrets != "/project/secrets.yaml" { + t.Errorf("expected WithSecrets=/project/secrets.yaml, got %s", opts.WithSecrets) + } + if len(opts.TemplateFiles) != 1 || opts.TemplateFiles[0] != "templates/controlplane.yaml" { + t.Errorf("expected TemplateFiles=[templates/controlplane.yaml], got %v", opts.TemplateFiles) + } +} + +func TestBuildApplyPatchOptions(t *testing.T) { + origTalosVersion := applyCmdFlags.talosVersion + origKubeVersion := applyCmdFlags.kubernetesVersion + origDebug := applyCmdFlags.debug + defer func() { + applyCmdFlags.talosVersion = origTalosVersion + applyCmdFlags.kubernetesVersion = origKubeVersion + applyCmdFlags.debug = origDebug + }() + + applyCmdFlags.talosVersion = "v1.12" + applyCmdFlags.kubernetesVersion = "1.31.0" + applyCmdFlags.debug = false + + opts := buildApplyPatchOptions("/project/secrets.yaml") + + if opts.Full { + t.Error("expected Full=false for direct patch path") + } + if opts.Offline { + t.Error("expected Offline=false for direct patch path") + } + if opts.Root != "" { + t.Errorf("expected empty Root for direct patch path, got %s", opts.Root) + } + if len(opts.TemplateFiles) != 0 { + t.Errorf("expected no TemplateFiles for direct patch path, got %v", opts.TemplateFiles) + } +} + +func TestResolveTemplatePaths(t *testing.T) { + // Create a real rootDir with template files for testing + tmpRoot := t.TempDir() + if err := os.MkdirAll(filepath.Join(tmpRoot, "templates"), 0o755); err != nil { + t.Fatalf("failed to create templates dir: %v", err) + } + + tests := []struct { + name string + templates []string + rootDir string + want []string + }{ + { + name: "relative path with empty rootDir", + templates: []string{"templates/controlplane.yaml"}, + rootDir: "", + want: []string{"templates/controlplane.yaml"}, + }, + { + name: "relative path resolved against rootDir", + templates: []string{"templates/controlplane.yaml"}, + rootDir: tmpRoot, + want: []string{"templates/controlplane.yaml"}, + }, + { + name: "multiple paths with rootDir", + templates: []string{"templates/controlplane.yaml", "templates/worker.yaml"}, + rootDir: tmpRoot, + want: []string{"templates/controlplane.yaml", "templates/worker.yaml"}, + }, + { + name: "absolute path inside rootDir", + templates: []string{filepath.Join(tmpRoot, "templates", "controlplane.yaml")}, + rootDir: tmpRoot, + want: []string{"templates/controlplane.yaml"}, + }, + { + name: "path outside rootDir is kept as-is", + templates: []string{"/other/project/templates/controlplane.yaml"}, + rootDir: tmpRoot, + want: []string{"/other/project/templates/controlplane.yaml"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := resolveTemplatePaths(tt.templates, tt.rootDir) + if len(got) != len(tt.want) { + t.Fatalf("resolveTemplatePaths() returned %d items, want %d", len(got), len(tt.want)) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("resolveTemplatePaths()[%d] = %q, want %q", i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestWrapWithNodeContext_SetsNodesInContext(t *testing.T) { + origNodes := GlobalArgs.Nodes + defer func() { GlobalArgs.Nodes = origNodes }() + + GlobalArgs.Nodes = []string{"10.0.0.1", "10.0.0.2"} + + var capturedCtx context.Context + inner := func(ctx context.Context, c *client.Client) error { + capturedCtx = ctx + return nil + } + + wrapped := wrapWithNodeContext(inner) + err := wrapped(context.Background(), nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if capturedCtx == nil { + t.Fatal("inner function was not called") + } + + // Verify the actual nodes injected via gRPC metadata + md, ok := metadata.FromOutgoingContext(capturedCtx) + if !ok { + t.Fatal("expected outgoing gRPC metadata in context, got none") + } + gotNodes := md.Get("nodes") + wantNodes := []string{"10.0.0.1", "10.0.0.2"} + if !slices.Equal(gotNodes, wantNodes) { + t.Errorf("nodes in context metadata = %v, want %v", gotNodes, wantNodes) + } +} + +func TestWrapWithNodeContext_DoesNotMutateGlobalArgs(t *testing.T) { + origNodes := GlobalArgs.Nodes + defer func() { GlobalArgs.Nodes = origNodes }() + + // Use a slice with extra capacity so append inside the closure could + // theoretically leak back to GlobalArgs if the copy is shallow + nodes := make([]string, 1, 10) + nodes[0] = "10.0.0.1" + GlobalArgs.Nodes = nodes + + inner := func(ctx context.Context, c *client.Client) error { + return nil + } + + wrapped := wrapWithNodeContext(inner) + if err := wrapped(context.Background(), nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify GlobalArgs.Nodes is unchanged after wrapWithNodeContext call + if !slices.Equal(GlobalArgs.Nodes, []string{"10.0.0.1"}) { + t.Errorf("GlobalArgs.Nodes was mutated to %v, expected [10.0.0.1]", GlobalArgs.Nodes) + } + + // Verify that the defensive copy is independent: mutating GlobalArgs + // after wrapper creation doesn't affect a subsequent call + GlobalArgs.Nodes = []string{"10.0.0.2"} + + var capturedCtx context.Context + inner2 := func(ctx context.Context, c *client.Client) error { + capturedCtx = ctx + return nil + } + wrapped2 := wrapWithNodeContext(inner2) + if err := wrapped2(context.Background(), nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + md, ok := metadata.FromOutgoingContext(capturedCtx) + if !ok { + t.Fatal("expected outgoing gRPC metadata in context") + } + gotNodes := md.Get("nodes") + if !slices.Equal(gotNodes, []string{"10.0.0.2"}) { + t.Errorf("nodes in context = %v, want [10.0.0.2]", gotNodes) + } +} + +func TestWrapWithNodeContext_NoNodesNoClient(t *testing.T) { + origNodes := GlobalArgs.Nodes + defer func() { GlobalArgs.Nodes = origNodes }() + + GlobalArgs.Nodes = []string{} + + inner := func(ctx context.Context, c *client.Client) error { + return nil + } + + wrapped := wrapWithNodeContext(inner) + err := wrapped(context.Background(), nil) + if err == nil { + t.Error("expected error when no nodes and no client config context, got nil") + } +} diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 1f206e5..c5c6871 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -219,34 +219,37 @@ func DetectRootForTemplate(templatePath string) (string, error) { return DetectProjectRootForFile(templatePath) } -func processModelineAndUpdateGlobals(configFile string, nodesFromArgs bool, endpointsFromArgs bool, owerwrite bool) error { +func processModelineAndUpdateGlobals(configFile string, nodesFromArgs bool, endpointsFromArgs bool, overwrite bool) ([]string, error) { modelineConfig, err := modeline.ReadAndParseModeline(configFile) if err != nil { fmt.Printf("Warning: modeline parsing failed: %v\n", err) - return err + return nil, err } + var templates []string + // Update global settings if modeline was successfully parsed if modelineConfig != nil { if !nodesFromArgs && len(modelineConfig.Nodes) > 0 { - if owerwrite { + if overwrite { GlobalArgs.Nodes = modelineConfig.Nodes } else { GlobalArgs.Nodes = append(GlobalArgs.Nodes, modelineConfig.Nodes...) } } if !endpointsFromArgs && len(modelineConfig.Endpoints) > 0 { - if owerwrite { + if overwrite { GlobalArgs.Endpoints = modelineConfig.Endpoints } else { GlobalArgs.Endpoints = append(GlobalArgs.Endpoints, modelineConfig.Endpoints...) } } + templates = modelineConfig.Templates } if len(GlobalArgs.Nodes) < 1 { - return errors.New("nodes are not set for the command: please use `--nodes` flag or configuration file to set the nodes to run the command against") + return nil, errors.New("nodes are not set for the command: please use `--nodes` flag or configuration file to set the nodes to run the command against") } - return nil + return templates, nil } diff --git a/pkg/commands/root_test.go b/pkg/commands/root_test.go new file mode 100644 index 0000000..ccd4756 --- /dev/null +++ b/pkg/commands/root_test.go @@ -0,0 +1,133 @@ +package commands + +import ( + "os" + "path/filepath" + "testing" +) + +func TestProcessModelineAndUpdateGlobals_ReturnsTemplates(t *testing.T) { + origNodes := GlobalArgs.Nodes + origEndpoints := GlobalArgs.Endpoints + defer func() { + GlobalArgs.Nodes = origNodes + GlobalArgs.Endpoints = origEndpoints + }() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "node0.yaml") + content := `# talm: nodes=["10.0.0.1"], endpoints=["10.0.0.1"], templates=["templates/controlplane.yaml"] +machine: + type: controlplane +` + if err := os.WriteFile(configFile, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write temp file: %v", err) + } + + GlobalArgs.Nodes = []string{} + GlobalArgs.Endpoints = []string{} + + templates, err := processModelineAndUpdateGlobals(configFile, false, false, true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(templates) != 1 { + t.Fatalf("expected 1 template, got %d", len(templates)) + } + if templates[0] != "templates/controlplane.yaml" { + t.Errorf("expected templates/controlplane.yaml, got %s", templates[0]) + } +} + +func TestProcessModelineAndUpdateGlobals_NoTemplates(t *testing.T) { + origNodes := GlobalArgs.Nodes + origEndpoints := GlobalArgs.Endpoints + defer func() { + GlobalArgs.Nodes = origNodes + GlobalArgs.Endpoints = origEndpoints + }() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "node0.yaml") + content := `# talm: nodes=["10.0.0.1"], endpoints=["10.0.0.1"] +machine: + type: controlplane +` + if err := os.WriteFile(configFile, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write temp file: %v", err) + } + + GlobalArgs.Nodes = []string{} + GlobalArgs.Endpoints = []string{} + + templates, err := processModelineAndUpdateGlobals(configFile, false, false, true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(templates) != 0 { + t.Errorf("expected 0 templates, got %d: %v", len(templates), templates) + } +} + +func TestProcessModelineAndUpdateGlobals_InvalidModeline(t *testing.T) { + origNodes := GlobalArgs.Nodes + origEndpoints := GlobalArgs.Endpoints + defer func() { + GlobalArgs.Nodes = origNodes + GlobalArgs.Endpoints = origEndpoints + }() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "node0.yaml") + content := `# talm: this is not valid modeline syntax +machine: + type: controlplane +` + if err := os.WriteFile(configFile, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write temp file: %v", err) + } + + GlobalArgs.Nodes = []string{} + GlobalArgs.Endpoints = []string{} + + templates, err := processModelineAndUpdateGlobals(configFile, false, false, true) + if err == nil { + t.Fatal("expected error for invalid modeline, got nil") + } + if templates != nil { + t.Errorf("expected nil templates on error, got %v", templates) + } +} + +func TestProcessModelineAndUpdateGlobals_EmptyNodesError(t *testing.T) { + origNodes := GlobalArgs.Nodes + origEndpoints := GlobalArgs.Endpoints + defer func() { + GlobalArgs.Nodes = origNodes + GlobalArgs.Endpoints = origEndpoints + }() + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "node0.yaml") + // Valid modeline but with empty nodes + content := `# talm: nodes=[], endpoints=["10.0.0.1"], templates=["templates/controlplane.yaml"] +machine: + type: controlplane +` + if err := os.WriteFile(configFile, []byte(content), 0o644); err != nil { + t.Fatalf("failed to write temp file: %v", err) + } + + GlobalArgs.Nodes = []string{} + GlobalArgs.Endpoints = []string{} + + templates, err := processModelineAndUpdateGlobals(configFile, false, false, true) + if err == nil { + t.Fatal("expected error for empty nodes, got nil") + } + if templates != nil { + t.Errorf("expected nil templates on error, got %v", templates) + } +} diff --git a/pkg/commands/talosctl_wrapper.go b/pkg/commands/talosctl_wrapper.go index f8fd7ba..f331680 100644 --- a/pkg/commands/talosctl_wrapper.go +++ b/pkg/commands/talosctl_wrapper.go @@ -127,7 +127,7 @@ func wrapTalosCommand(cmd *cobra.Command, cmdName string) *cobra.Command { } for _, configFile := range expandedFiles { - if err := processModelineAndUpdateGlobals(configFile, nodesFromArgs, endpointsFromArgs, false); err != nil { + if _, err := processModelineAndUpdateGlobals(configFile, nodesFromArgs, endpointsFromArgs, false); err != nil { return err } } diff --git a/pkg/commands/upgrade_handler.go b/pkg/commands/upgrade_handler.go index 21130a9..aa7849a 100644 --- a/pkg/commands/upgrade_handler.go +++ b/pkg/commands/upgrade_handler.go @@ -55,7 +55,7 @@ func wrapUpgradeCommand(wrappedCmd *cobra.Command, originalRunE func(*cobra.Comm // Process modeline to update GlobalArgs nodesFromArgs := len(GlobalArgs.Nodes) > 0 endpointsFromArgs := len(GlobalArgs.Endpoints) > 0 - if err := processModelineAndUpdateGlobals(configFile, nodesFromArgs, endpointsFromArgs, true); err != nil { + if _, err := processModelineAndUpdateGlobals(configFile, nodesFromArgs, endpointsFromArgs, true); err != nil { return fmt.Errorf("failed to process modeline: %w", err) }