Skip to content

Commit e56f6e6

Browse files
committed
fix(apply): scope COSI preflight ctx to singular node key
The auth template-rendering apply path puts the target node under the plural "nodes" metadata key so helpers.ForEachResource and apid's machine-API backend resolver can read it. Reusing that ctx for the COSI version preflight breaks the preflight: Talos's apid director rejects every COSI method whose outgoing context carries the plural key, regardless of slice length, and cosiVersionReader swallows errors and returns ok=false on rejection. End user sees no version-mismatch warning even when the running Talos predates the configured talosVersion -- defeating the whole point of the preflight added in PR #133. Add cosiPreflightContext: clone the outgoing metadata, drop "nodes", attach "node" with the same target, hand the rebuilt context to the COSI caller. ApplyConfiguration keeps the original ctx unchanged. The helper is a noop on the insecure (maintenance) path that carries no node metadata at all and on multi-node ctx where the single-target shortcut would be unsafe. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent 7a0e793 commit e56f6e6

2 files changed

Lines changed: 117 additions & 9 deletions

File tree

pkg/commands/apply.go

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"github.com/cozystack/talm/pkg/engine"
2626
"github.com/spf13/cobra"
27+
"google.golang.org/grpc/metadata"
2728
"google.golang.org/protobuf/types/known/durationpb"
2829

2930
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers"
@@ -122,15 +123,25 @@ func apply(args []string) error {
122123
fmt.Printf("- talm: file=%s, nodes=%s, endpoints=%s\n", configFile, nodes, GlobalArgs.Endpoints)
123124

124125
applyClosure := func(ctx context.Context, c *client.Client, data []byte) error {
125-
// ctx here is single-target on every apply path: the auth
126-
// branch sets `nodes` (plural, one element) via
127-
// openClientPerNodeAuth (apid treats len("nodes") == 1 as
128-
// single-target); the insecure branch goes through a fresh
129-
// per-node maintenance client whose ctx carries no node
130-
// metadata at all, but the maintenance client itself dials
131-
// a single endpoint per call. COSI reads are safe under
132-
// either shape.
133-
preflightCheckTalosVersion(ctx, cosiVersionReader(c), applyCmdFlags.talosVersion, os.Stderr)
126+
// ctx is shaped for ApplyConfiguration on every apply path:
127+
// the auth branch sets `nodes` (plural, one element) via
128+
// openClientPerNodeAuth so apid resolves a single backend
129+
// and helpers.ForEachResource can read the plural key from
130+
// inside template lookups; the insecure branch carries no
131+
// node metadata at all and the maintenance client dials a
132+
// single endpoint per call.
133+
//
134+
// The COSI preflight needs a different context shape:
135+
// Talos's apid director rejects every COSI method whose
136+
// ctx carries the plural "nodes" key, regardless of slice
137+
// length (its COSI guard is unconditional). cosiVersionReader
138+
// swallows errors and returns ok=false on rejection, so the
139+
// preflight would silently no-op on the auth path — defeating
140+
// the whole point of preflightCheckTalosVersion (PR #133).
141+
// cosiPreflightContext rebuilds ctx with the singular "node"
142+
// key so the COSI router accepts the call; ApplyConfiguration
143+
// keeps the original ctx unchanged.
144+
preflightCheckTalosVersion(cosiPreflightContext(ctx), cosiVersionReader(c), applyCmdFlags.talosVersion, os.Stderr)
134145

135146
resp, err := c.ApplyConfiguration(ctx, &machineapi.ApplyConfigurationRequest{
136147
Data: data,
@@ -378,6 +389,38 @@ func openClientPerNodeAuth(parentCtx context.Context, c *client.Client) openClie
378389
}
379390
}
380391

392+
// cosiPreflightContext returns a context suitable for a COSI call
393+
// against the same single target the caller's ctx addresses on the
394+
// machine API. Talos's apid director rejects every COSI method whose
395+
// outgoing context carries the plural "nodes" metadata key, regardless
396+
// of how many entries the slice has — the COSI router insists on the
397+
// singular "node" key. The auth template-rendering apply path uses
398+
// client.WithNodes (plural, single-element slice) so that
399+
// helpers.ForEachResource and the apid backend resolver can both read
400+
// the plural key from template lookups; that ctx is therefore
401+
// unsuitable for COSI reads as is.
402+
//
403+
// The fix is local to the COSI preflight: clone the outgoing
404+
// metadata, drop "nodes", attach "node" with the same target, and
405+
// hand the rebuilt context to the COSI caller. ctx is unchanged for
406+
// the insecure (maintenance) path that carries no node metadata at
407+
// all, and unchanged when the auth path's plural slice has a length
408+
// other than one (multi-node scenarios already error out earlier in
409+
// applyTemplatesPerNode, but the helper is defensive).
410+
func cosiPreflightContext(ctx context.Context) context.Context {
411+
md, ok := metadata.FromOutgoingContext(ctx)
412+
if !ok {
413+
return ctx
414+
}
415+
nodes := md.Get("nodes")
416+
if len(nodes) != 1 {
417+
return ctx
418+
}
419+
mdClone := md.Copy()
420+
mdClone.Delete("nodes")
421+
return client.WithNode(metadata.NewOutgoingContext(ctx, mdClone), nodes[0])
422+
}
423+
381424
// resolveAuthTemplateNodes returns the node list the authenticated
382425
// template-rendering path should iterate over. cliNodes (from --nodes
383426
// or the modeline) takes precedence; when empty, the talosconfig

pkg/commands/apply_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,71 @@ func TestApplyTemplatesPerNode_AuthModeUsesPluralNodesMetadataKey(t *testing.T)
697697
}
698698
}
699699

700+
// TestCosiPreflightContext_StripsPluralAndAttachesSingular pins the
701+
// COSI preflight ctx contract: the auth template-rendering apply path
702+
// puts the target node under the plural "nodes" metadata key (so
703+
// helpers.ForEachResource and apid's machine-API backend resolver can
704+
// read it), but Talos's apid director rejects every COSI method whose
705+
// outgoing context carries the plural key, regardless of slice
706+
// length. cosiPreflightContext rebuilds ctx with the singular "node"
707+
// key so the COSI router accepts the call. Without this, the version
708+
// preflight added in PR #133 silently no-ops on the auth path:
709+
// cosiVersionReader swallows errors and returns ok=false on rejection.
710+
func TestCosiPreflightContext_StripsPluralAndAttachesSingular(t *testing.T) {
711+
const node = "10.0.0.1"
712+
in := client.WithNodes(context.Background(), node)
713+
714+
out := cosiPreflightContext(in)
715+
716+
md, ok := metadata.FromOutgoingContext(out)
717+
if !ok {
718+
t.Fatal("expected outgoing metadata on preflight ctx")
719+
}
720+
if got := md.Get("nodes"); len(got) != 0 {
721+
t.Errorf(`metadata key "nodes" must be unset on COSI preflight ctx, got %v (apid's COSI router rejects every call carrying it)`, got)
722+
}
723+
if got := md.Get("node"); !slices.Equal(got, []string{node}) {
724+
t.Errorf(`metadata key "node" = %v, want [%q] (apid's COSI router routes by the singular key)`, got, node)
725+
}
726+
}
727+
728+
// TestCosiPreflightContext_LeavesNoMetadataAlone pins the noop case
729+
// for the insecure (maintenance) apply path, whose ctx carries no
730+
// outgoing metadata at all — the maintenance client dials a single
731+
// endpoint per call and routing-by-key is irrelevant. The helper
732+
// must return ctx unchanged, not synthesize an empty "node" key
733+
// that apid would route to the wrong target.
734+
func TestCosiPreflightContext_LeavesNoMetadataAlone(t *testing.T) {
735+
in := context.Background()
736+
out := cosiPreflightContext(in)
737+
738+
if md, ok := metadata.FromOutgoingContext(out); ok && len(md) > 0 {
739+
t.Errorf("expected no outgoing metadata on preflight ctx for maintenance path, got %v", md)
740+
}
741+
}
742+
743+
// TestCosiPreflightContext_LeavesMultiNodeAlone pins the defensive
744+
// branch for plural slices with a length other than one: the
745+
// applyTemplatesPerNode loop runs one iteration per node, so plural
746+
// metadata should always be a single-element slice in production,
747+
// but the helper must not silently collapse a multi-element slice
748+
// into a fabricated single-target call.
749+
func TestCosiPreflightContext_LeavesMultiNodeAlone(t *testing.T) {
750+
in := client.WithNodes(context.Background(), "a", "b")
751+
out := cosiPreflightContext(in)
752+
753+
md, ok := metadata.FromOutgoingContext(out)
754+
if !ok {
755+
t.Fatal("expected outgoing metadata on preflight ctx")
756+
}
757+
if got := md.Get("nodes"); !slices.Equal(got, []string{"a", "b"}) {
758+
t.Errorf(`expected "nodes" passthrough on multi-node ctx, got %v`, got)
759+
}
760+
if got := md.Get("node"); len(got) != 0 {
761+
t.Errorf(`expected no synthesized "node" on multi-node ctx, got %v`, got)
762+
}
763+
}
764+
700765
// TestTemplateAndApplyDiverge_NodeBodyOverlayLimitation pins a known
701766
// trade-off: `talm apply -f node.yaml` overlays the node file body on the
702767
// rendered template before sending the result to ApplyConfiguration, but

0 commit comments

Comments
 (0)