fix(commands): render templates per node in the apply callback

PR #119 moved talm apply's template rendering from offline to online.
The new online path runs inside withApplyClient, whose
wrapWithNodeContext helper batches every node from GlobalArgs.Nodes
into a single gRPC context. engine.Render then calls
helpers.FailIfMultiNodes which rejects multi-node contexts, so a
node file with `nodes=[ip1, ip2]` (or `--nodes A,B,C` on the
command line) fails before any rendering happens. The pre-#119
direct-patch flow handled multi-node fan-out at the wire level inside
ApplyConfiguration, so this never surfaced.

Render once per node instead. Add applyTemplatesPerNode that takes a
nodes slice and injection points for the render and apply functions,
then iterates: for each node it builds a single-node context with
client.WithNodes, calls engine.Render (which now sees one node and
satisfies the FailIfMultiNodes guard), merges the modeline'd file as
a patch, and applies the result. Per-node iteration is also the
correct semantic — discovery via lookup() resolves each node's own
network topology rather than mashing everything together.

Split withApplyClient into a public form (still wraps with the legacy
multi-node context for the direct-patch branch) and withApplyClientBare
which skips the wrap so the per-node loop can attach contexts itself.

Three tests pin the contract: that each render and apply call sees
exactly one node in its outgoing-context metadata; that engine.Render
with a batched multi-node context still errors (the pre-condition the
loop exists to satisfy); that an empty nodes slice errors loudly
rather than silently doing nothing. Manually verified the loop assertion
fails when the per-iteration context is built with all nodes instead
of one.

Closes #120

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

Author: lexfrei

pkg/commands/apply.go

@@ -113,40 +113,34 @@ func apply(args []string) error {
 		withSecretsPath := ResolveSecretsPath(applyCmdFlags.withSecrets)
 
 		if len(modelineTemplates) > 0 {
-			// Template rendering path: connect to the node first, render templates
-			// online (so lookup() functions resolve real discovery data), then apply.
+			// Template rendering path: connect to the node first, render
+			// templates online per node (so lookup() functions resolve each
+			// node's own discovery data), merge the node file as a patch,
+			// then apply. The bare client wrapper is used so the per-node
+			// loop can attach a single-node gRPC context per iteration —
+			// engine.Render's FailIfMultiNodes guard rejects a batched
+			// multi-node context.
 			opts := buildApplyRenderOptions(modelineTemplates, withSecretsPath)
+			nodes := append([]string(nil), GlobalArgs.Nodes...)
 
-			if err := withApplyClient(func(ctx context.Context, c *client.Client) error {
+			if err := withApplyClientBare(func(ctx context.Context, c *client.Client) error {
 				fmt.Printf("- talm: file=%s, nodes=%s, endpoints=%s\n", configFile, GlobalArgs.Nodes, GlobalArgs.Endpoints)
-
-				result, err := engine.Render(ctx, c, opts)
-				if err != nil {
-					return fmt.Errorf("template rendering error: %w", err)
-				}
-
-				// Overlay any per-node config from the modeline'd file on top
-				// of the rendered template. Without this merge, hostname,
-				// secondary interfaces, VIP placement and other per-node
-				// fields defined in the node file are silently lost.
-				result, err = engine.MergeFileAsPatch(result, configFile)
-				if err != nil {
-					return fmt.Errorf("merging node file as patch: %w", err)
-				}
-
-				resp, err := c.ApplyConfiguration(ctx, &machineapi.ApplyConfigurationRequest{
-					Data:           result,
-					Mode:           applyCmdFlags.Mode.Mode,
-					DryRun:         applyCmdFlags.dryRun,
-					TryModeTimeout: durationpb.New(applyCmdFlags.configTryTimeout),
-				})
-				if err != nil {
-					return fmt.Errorf("error applying new configuration: %w", err)
-				}
-
-				helpers.PrintApplyResults(resp)
-
-				return nil
+				return applyTemplatesPerNode(ctx, c, opts, configFile, nodes,
+					engine.Render,
+					func(ctx context.Context, c *client.Client, data []byte) error {
+						resp, err := c.ApplyConfiguration(ctx, &machineapi.ApplyConfigurationRequest{
+							Data:           data,
+							Mode:           applyCmdFlags.Mode.Mode,
+							DryRun:         applyCmdFlags.dryRun,
+							TryModeTimeout: durationpb.New(applyCmdFlags.configTryTimeout),
+						})
+						if err != nil {
+							return fmt.Errorf("error applying new configuration: %w", err)
+						}
+						helpers.PrintApplyResults(resp)
+						return nil
+					},
+				)
 			}); err != nil {
 				return err
 			}
@@ -197,22 +191,70 @@ func apply(args []string) error {
 }
 
 // withApplyClient creates a Talos client appropriate for the current apply mode
-// and invokes the given action with it.
+// and invokes the given action with it. The action receives a context with
+// every node from GlobalArgs.Nodes batched into the gRPC metadata, matching
+// the legacy direct-patch fan-out behaviour.
 func withApplyClient(f func(ctx context.Context, c *client.Client) error) error {
+	return withApplyClientBare(wrapWithNodeContext(f))
+}
+
+// withApplyClientBare connects to Talos for the current apply mode but does
+// NOT inject GlobalArgs.Nodes into the context. The template-rendering path
+// uses this so its per-node loop can attach a single-node context per
+// iteration instead — engine.Render's FailIfMultiNodes guard rejects a
+// batched multi-node context, and discovery via lookup() is per-node anyway.
+func withApplyClientBare(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(wrappedF)
+		return WithClientSkipVerify(f)
 	}
 
-	return WithClientNoNodes(wrappedF)
+	return WithClientNoNodes(f)
+}
+
+// renderFunc and applyFunc are injection points for applyTemplatesPerNode so
+// unit tests can drive the loop with fakes instead of a real Talos client.
+type renderFunc func(ctx context.Context, c *client.Client, opts engine.Options) ([]byte, error)
+type applyFunc func(ctx context.Context, c *client.Client, data []byte) error
+
+// applyTemplatesPerNode runs render → MergeFileAsPatch → apply once per node,
+// each iteration carrying a single-node gRPC context. Enables multi-node
+// modelines and `--nodes A,B,C` against the template-rendering branch:
+// engine.Render's FailIfMultiNodes guard requires a single-node context, and
+// each node should resolve its own discovery via lookup() in any case.
+func applyTemplatesPerNode(
+	parentCtx context.Context,
+	c *client.Client,
+	opts engine.Options,
+	configFile string,
+	nodes []string,
+	render renderFunc,
+	apply applyFunc,
+) error {
+	if len(nodes) == 0 {
+		return fmt.Errorf("no nodes specified for template-rendering apply")
+	}
+	for _, node := range nodes {
+		perCtx := client.WithNodes(parentCtx, node)
+		rendered, err := render(perCtx, c, opts)
+		if err != nil {
+			return fmt.Errorf("node %s: template rendering: %w", node, err)
+		}
+		merged, err := engine.MergeFileAsPatch(rendered, configFile)
+		if err != nil {
+			return fmt.Errorf("node %s: merging node file as patch: %w", node, err)
+		}
+		if err := apply(perCtx, c, merged); err != nil {
+			return fmt.Errorf("node %s: %w", node, err)
+		}
+	}
+	return nil
 }
 
 // buildApplyRenderOptions constructs engine.Options for the template rendering path.

pkg/commands/apply_test.go

@@ -7,6 +7,7 @@ import (
 	"slices"
 	"testing"
 
+	"github.com/cozystack/talm/pkg/engine"
 	"github.com/siderolabs/talos/pkg/machinery/client"
 	"google.golang.org/grpc/metadata"
 )
@@ -241,3 +242,108 @@ func TestWrapWithNodeContext_NoNodesNoClient(t *testing.T) {
 		t.Error("expected error when no nodes and no client config context, got nil")
 	}
 }
+
+// nodesFromOutgoingCtx pulls the gRPC outgoing-metadata "nodes" key — the same
+// place client.WithNodes writes to. Used by the per-node loop tests to assert
+// each iteration sees a single-node context.
+func nodesFromOutgoingCtx(t *testing.T, ctx context.Context) []string {
+	t.Helper()
+	md, ok := metadata.FromOutgoingContext(ctx)
+	if !ok {
+		return nil
+	}
+	return md.Get("nodes")
+}
+
+// TestApplyTemplatesPerNode_LoopsOncePerNodeWithSingleNodeContext covers #120:
+// the multi-node fan-out previously batched every node into a single gRPC
+// context, which engine.Render's FailIfMultiNodes guard then rejected. The
+// per-node loop must run render + merge + apply once per node, each with a
+// context carrying exactly that one node, so the guard passes and each node
+// resolves its own discovery.
+func TestApplyTemplatesPerNode_LoopsOncePerNodeWithSingleNodeContext(t *testing.T) {
+	dir := t.TempDir()
+	configFile := filepath.Join(dir, "node.yaml")
+	// Modeline-only file so MergeFileAsPatch is a no-op and the test stays
+	// focused on the loop semantics rather than patch behaviour.
+	if err := os.WriteFile(configFile, []byte("# talm: nodes=[\"a\",\"b\",\"c\"], templates=[\"templates/controlplane.yaml\"]\n"), 0o644); err != nil {
+		t.Fatalf("write configFile: %v", err)
+	}
+
+	want := []string{"10.0.0.1", "10.0.0.2", "10.0.0.3"}
+	var renderCalls, applyCalls []string
+
+	render := func(ctx context.Context, _ *client.Client, _ engine.Options) ([]byte, error) {
+		got := nodesFromOutgoingCtx(t, ctx)
+		if len(got) != 1 {
+			t.Errorf("render: expected single-node ctx, got %v", got)
+		}
+		renderCalls = append(renderCalls, got...)
+		return []byte("version: v1alpha1\nmachine:\n  type: worker\n"), nil
+	}
+	apply := func(ctx context.Context, _ *client.Client, _ []byte) error {
+		got := nodesFromOutgoingCtx(t, ctx)
+		if len(got) != 1 {
+			t.Errorf("apply: expected single-node ctx, got %v", got)
+		}
+		applyCalls = append(applyCalls, got...)
+		return nil
+	}
+
+	if err := applyTemplatesPerNode(context.Background(), nil, engine.Options{}, configFile, want, render, apply); err != nil {
+		t.Fatalf("applyTemplatesPerNode: %v", err)
+	}
+
+	if !slices.Equal(renderCalls, want) {
+		t.Errorf("render calls = %v, want %v", renderCalls, want)
+	}
+	if !slices.Equal(applyCalls, want) {
+		t.Errorf("apply calls  = %v, want %v", applyCalls, want)
+	}
+}
+
+// TestApplyTemplatesPerNode_BatchedContextIsRejected covers the regression
+// vector that motivated the per-node loop: feeding engine.Render a context
+// with multiple nodes produces a FailIfMultiNodes error. The per-node loop is
+// the cure; this test pins the disease.
+func TestApplyTemplatesPerNode_BatchedContextIsRejected(t *testing.T) {
+	dir := t.TempDir()
+	configFile := filepath.Join(dir, "node.yaml")
+	if err := os.WriteFile(configFile, []byte("# talm: nodes=[\"a\",\"b\"]\n"), 0o644); err != nil {
+		t.Fatalf("write configFile: %v", err)
+	}
+
+	// Sanity: when the loop hands engine.Render a single-node ctx, the
+	// guard is satisfied. We exercise this above. Here we assert that if
+	// somebody tried to feed a multi-node ctx to render directly, the
+	// real engine.Render would reject it — the bug we are working around.
+	multiCtx := client.WithNodes(context.Background(), "10.0.0.1", "10.0.0.2")
+	_, err := engine.Render(multiCtx, nil, engine.Options{Offline: false, CommandName: "talm apply"})
+	if err == nil {
+		t.Fatal("engine.Render expected to reject multi-node ctx, got nil")
+	}
+}
+
+// TestApplyTemplatesPerNode_NoNodesIsAnError guards against silently
+// short-circuiting when GlobalArgs.Nodes is empty. The previous structure
+// would happily run zero iterations — this pin makes it loud.
+func TestApplyTemplatesPerNode_NoNodesIsAnError(t *testing.T) {
+	dir := t.TempDir()
+	configFile := filepath.Join(dir, "node.yaml")
+	if err := os.WriteFile(configFile, []byte("# talm: nodes=[]\n"), 0o644); err != nil {
+		t.Fatalf("write configFile: %v", err)
+	}
+
+	render := func(_ context.Context, _ *client.Client, _ engine.Options) ([]byte, error) {
+		t.Fatal("render must not be called when there are zero nodes")
+		return nil, nil
+	}
+	apply := func(_ context.Context, _ *client.Client, _ []byte) error {
+		t.Fatal("apply must not be called when there are zero nodes")
+		return nil
+	}
+
+	if err := applyTemplatesPerNode(context.Background(), nil, engine.Options{}, configFile, nil, render, apply); err == nil {
+		t.Fatal("expected an error for empty nodes list, got nil")
+	}
+}