fix: address remaining round-3 review notes

- Sync openClientFunc docstring with the WithNode (single-target)
  metadata key the auth-mode opener actually uses; the old docstring
  still mentioned WithNodes from the previous version.

- TestOpenClientPerNodeMaintenance_NarrowsAndRestoresGlobalNodes now
  drives the production openClientPerNodeMaintenance with an injected
  fake maintenanceClientFunc instead of an inline stub. The fake
  snapshots GlobalArgs.Nodes at the moment WithClientMaintenance reads
  it, proving the narrow-and-restore-via-defer logic is exercised end
  to end. A regression in the production function now fails this test
  (previously it would have continued to pass against the test-local
  stub).

- New TestApplyTemplatesPerNode_AuthModeUsesSingleNodeMetadataKey pins
  the gRPC metadata key the auth-mode opener writes so a future swap
  back to WithNodes (which would round-trip through nodesFromOutgoingCtx
  unnoticed) gets caught.

- README documents that node files can carry per-node patches in their
  body, that talm apply applies them as a strategic merge over the
  rendered template, and that talm template intentionally does not.
  Recommends apply --dry-run for previewing the exact bytes apply will
  send.

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

Author: lexfrei

README.md

@@ -140,6 +140,23 @@ Re-template and update generated file in place (this will overwrite it):
 talm template -f nodes/node1.yaml -I
 ```
 
+> **Per-node patches inside node files.** A node file can carry Talos config
+> below its modeline (for example, a custom `hostname`, secondary
+> interfaces with `deviceSelector`, VIP placement, or extra etcd args).
+> When `talm apply -f node.yaml` runs the template-rendering branch, that
+> body is applied as a strategic merge patch on top of the rendered
+> template before the result is sent to the node — so per-node fields
+> survive even when the template auto-generates conflicting values
+> (e.g. `hostname: talos-XXXXX`).
+>
+> `talm template -f node.yaml` (with or without `-I`) does **not** apply
+> the same overlay: its output is the rendered template plus the modeline
+> and the auto-generated warning, byte-identical to what the template
+> alone would produce. Routing it through the patcher would drop every
+> YAML comment (including the modeline) and re-sort keys, breaking
+> downstream commands that read the file back. Use `apply --dry-run` if
+> you want to preview the exact bytes that will be sent to the node.
+
 ## Using talosctl commands
 
 Talm offers a similar set of commands to those provided by talosctl.

pkg/commands/apply.go

@@ -135,7 +135,7 @@ func apply(args []string) error {
 			}
 
 			if applyCmdFlags.insecure {
-				openClient := openClientPerNodeMaintenance(applyCmdFlags.certFingerprints)
+				openClient := openClientPerNodeMaintenance(applyCmdFlags.certFingerprints, WithClientMaintenance)
 				if err := applyTemplatesPerNode(opts, configFile, nodes, openClient, engine.Render, applyClosure); err != nil {
 					return err
 				}
@@ -229,10 +229,10 @@ type applyFunc func(ctx context.Context, c *client.Client, data []byte) error
 
 // openClientFunc opens a Talos client suitable for a single node and runs
 // action with it. Authenticated mode reuses one parent client and rotates
-// the node via gRPC metadata (client.WithNodes); insecure (maintenance)
-// mode opens a fresh single-endpoint client per node because Talos's
-// maintenance client ignores nodes-in-context and round-robins between its
-// configured endpoints.
+// the node via single-target gRPC metadata (client.WithNode); insecure
+// (maintenance) mode opens a fresh single-endpoint client per node because
+// Talos's maintenance client ignores node metadata in the context and
+// round-robins between its configured endpoints.
 type openClientFunc func(node string, action func(ctx context.Context, c *client.Client) error) error
 
 // applyTemplatesPerNode runs render → MergeFileAsPatch → apply once per
@@ -269,19 +269,29 @@ func applyTemplatesPerNode(
 	return nil
 }
 
+// maintenanceClientFunc is the contract WithClientMaintenance satisfies in
+// production and a fake satisfies in tests. Injection lets the unit tests
+// run the real openClientPerNodeMaintenance body without dialing a Talos
+// node.
+type maintenanceClientFunc func(fingerprints []string, action func(ctx context.Context, c *client.Client) error) error
+
 // openClientPerNodeMaintenance returns an openClientFunc that opens a
 // fresh single-endpoint maintenance client per node. Multi-node insecure
 // apply (first bootstrap of a multi-node cluster) needs this because
 // WithClientMaintenance creates a client with all endpoints and gRPC then
 // round-robins ApplyConfiguration across them — most nodes never see the
 // config. Narrowing GlobalArgs.Nodes to the current iteration's node and
 // restoring it via defer keeps the wrapper's signature unchanged.
-func openClientPerNodeMaintenance(fingerprints []string) openClientFunc {
+//
+// mkClient is normally WithClientMaintenance; tests pass a fake that
+// captures the GlobalArgs.Nodes value at the moment WithClientMaintenance
+// would have read it.
+func openClientPerNodeMaintenance(fingerprints []string, mkClient maintenanceClientFunc) openClientFunc {
 	return func(node string, action func(ctx context.Context, c *client.Client) error) error {
 		savedNodes := append([]string(nil), GlobalArgs.Nodes...)
 		GlobalArgs.Nodes = []string{node}
 		defer func() { GlobalArgs.Nodes = savedNodes }()
-		return WithClientMaintenance(fingerprints, action)
+		return mkClient(fingerprints, action)
 	}
 }
 

pkg/commands/apply_test.go

@@ -431,48 +431,96 @@ func TestApplyTemplatesPerNode_MaintenanceModeOpensFreshClientPerNode(t *testing
 	}
 }
 
-// TestOpenClientPerNodeMaintenance_NarrowsAndRestoresGlobalNodes verifies
-// the production maintenance opener narrows GlobalArgs.Nodes to the
-// iteration's single endpoint while WithClientMaintenance reads it, and
-// restores the prior value afterwards regardless of whether the action
-// succeeded. Without this narrowing, WithClientMaintenance would build a
-// client with every endpoint and gRPC would round-robin
-// ApplyConfiguration.
+// TestOpenClientPerNodeMaintenance_NarrowsAndRestoresGlobalNodes drives
+// the real openClientPerNodeMaintenance with an injected
+// maintenanceClientFunc fake. The fake captures GlobalArgs.Nodes at the
+// moment a real WithClientMaintenance would have read it for endpoint
+// resolution. The contract: every iteration narrows GlobalArgs.Nodes to
+// exactly the iteration's node, and the prior value is restored after
+// the action returns regardless of success. Without the narrowing, the
+// real WithClientMaintenance would dial every endpoint at once and gRPC
+// would round-robin ApplyConfiguration across them.
 func TestOpenClientPerNodeMaintenance_NarrowsAndRestoresGlobalNodes(t *testing.T) {
 	saved := append([]string(nil), GlobalArgs.Nodes...)
 	defer func() { GlobalArgs.Nodes = saved }()
 
 	GlobalArgs.Nodes = []string{"original-A", "original-B"}
 
-	// We can't invoke the real WithClientMaintenance without a Talos
-	// endpoint, but openClientPerNodeMaintenance's narrowing is
-	// observable: stub the action to capture GlobalArgs.Nodes at the
-	// moment WithClientMaintenance reads them. WithClientMaintenance
-	// dials a TCP socket; stubbing it requires a fake. We instead
-	// replicate the narrow/defer/restore logic on an inline maintenance
-	// stub of the same shape.
-	openWithStub := func(node string, action func(ctx context.Context, c *client.Client) error) error {
-		savedNodes := append([]string(nil), GlobalArgs.Nodes...)
-		GlobalArgs.Nodes = []string{node}
-		defer func() { GlobalArgs.Nodes = savedNodes }()
-
-		// In production WithClientMaintenance reads GlobalArgs.Nodes here.
-		// Capture the value and exit without doing real network IO.
-		if got := GlobalArgs.Nodes; len(got) != 1 || got[0] != node {
-			t.Errorf("expected GlobalArgs.Nodes pinned to %q during open; got %v", node, got)
-		}
+	type call struct {
+		fingerprints []string
+		nodesAtCall  []string
+	}
+	var calls []call
+
+	fakeMaintenance := func(fingerprints []string, action func(ctx context.Context, c *client.Client) error) error {
+		// WithClientMaintenance reads GlobalArgs.Nodes for its endpoints
+		// at this point. Snapshot the value so the test can inspect it.
+		calls = append(calls, call{
+			fingerprints: append([]string(nil), fingerprints...),
+			nodesAtCall:  append([]string(nil), GlobalArgs.Nodes...),
+		})
 		return action(context.Background(), nil)
 	}
 
+	openClient := openClientPerNodeMaintenance([]string{"fp-1"}, fakeMaintenance)
+
 	for _, node := range []string{"10.0.0.1", "10.0.0.2"} {
-		if err := openWithStub(node, func(_ context.Context, _ *client.Client) error { return nil }); err != nil {
-			t.Fatalf("openWithStub(%q): %v", node, err)
+		if err := openClient(node, func(_ context.Context, _ *client.Client) error { return nil }); err != nil {
+			t.Fatalf("openClient(%q): %v", node, err)
 		}
 	}
 
 	if !slices.Equal(GlobalArgs.Nodes, []string{"original-A", "original-B"}) {
 		t.Errorf("GlobalArgs.Nodes not restored after maintenance loop: got %v", GlobalArgs.Nodes)
 	}
+	if len(calls) != 2 {
+		t.Fatalf("maintenance fake should have been called twice, got %d times", len(calls))
+	}
+	for i, want := range []string{"10.0.0.1", "10.0.0.2"} {
+		if !slices.Equal(calls[i].nodesAtCall, []string{want}) {
+			t.Errorf("call %d: GlobalArgs.Nodes at WithClientMaintenance time = %v, want [%q]", i, calls[i].nodesAtCall, want)
+		}
+		if !slices.Equal(calls[i].fingerprints, []string{"fp-1"}) {
+			t.Errorf("call %d: fingerprints passed through = %v, want [\"fp-1\"]", i, calls[i].fingerprints)
+		}
+	}
+}
+
+// TestApplyTemplatesPerNode_AuthModeUsesSingleNodeMetadataKey pins the
+// gRPC metadata key the auth-mode opener writes. WithNode sets "node"
+// (single-target proxy); WithNodes sets "nodes" (apid aggregation).
+// engine.Render's FailIfMultiNodes guard treats len("nodes") > 1 as the
+// multi-node case, so single-target metadata under "node" passes
+// trivially. A future refactor that swaps WithNode back to WithNodes
+// would slip past nodesFromOutgoingCtx (which reads either key) — this
+// assertion catches that regression directly.
+func TestApplyTemplatesPerNode_AuthModeUsesSingleNodeMetadataKey(t *testing.T) {
+	dir := t.TempDir()
+	configFile := filepath.Join(dir, "node.yaml")
+	if err := os.WriteFile(configFile, []byte("# talm: nodes=[\"a\"]\n"), 0o644); err != nil {
+		t.Fatalf("write configFile: %v", err)
+	}
+
+	const node = "10.0.0.1"
+	render := func(ctx context.Context, _ *client.Client, _ engine.Options) ([]byte, error) {
+		md, ok := metadata.FromOutgoingContext(ctx)
+		if !ok {
+			t.Fatal("expected outgoing metadata on per-iteration ctx")
+		}
+		if got := md.Get("node"); !slices.Equal(got, []string{node}) {
+			t.Errorf(`metadata key "node" = %v, want [%q]`, got, node)
+		}
+		if got := md.Get("nodes"); len(got) != 0 {
+			t.Errorf(`metadata key "nodes" must be unset for single-target apply, got %v`, got)
+		}
+		return []byte("version: v1alpha1\nmachine:\n  type: worker\n"), nil
+	}
+	apply := func(_ context.Context, _ *client.Client, _ []byte) error { return nil }
+
+	openClient := openClientPerNodeAuth(context.Background(), nil)
+	if err := applyTemplatesPerNode(engine.Options{}, configFile, []string{node}, openClient, render, apply); err != nil {
+		t.Fatalf("applyTemplatesPerNode: %v", err)
+	}
 }
 
 // TestTemplateAndApplyDiverge_NodeBodyOverlayLimitation pins a known