diff --git a/pkg/agent/annotations.go b/pkg/agent/annotations.go index 0195806..56a3765 100644 --- a/pkg/agent/annotations.go +++ b/pkg/agent/annotations.go @@ -2,6 +2,7 @@ package agent import ( "fmt" + "log/slog" "net" "strings" @@ -87,9 +88,6 @@ type ParsedAnnotations struct { CIDR6 []*net.IPNet // CIDR4 narrows IPv4 allocation. nil/empty means "use any node CIDR4". CIDR4 []*net.IPNet - // IPAlgo is the ordered list of identity fields used to build the IID. - // nil/empty means "random IID". - IPAlgo []embed.Field // Anycast is the set of anycast IPs to bind on the pod's loopback. // nil/empty means "no anycast". Anycast []net.IP @@ -144,14 +142,6 @@ func ParseAnnotations(in map[string]string, defaults FamilyDefaults) (*ParsedAnn out.CIDR4 = nets } - if v, ok := in[annotationPrefix+annIPAlgo]; ok { - fields, err := parseIPAlgo(v) - if err != nil { - return nil, fmt.Errorf("annotation %s: %w", annIPAlgo, err) - } - out.IPAlgo = fields - } - if v, ok := in[annotationPrefix+annAnycast]; ok { ips, err := parseIPList(v) if err != nil { @@ -247,30 +237,85 @@ func parseIPList(s string) ([]net.IP, error) { return out, nil } -// parseIPAlgo parses the ip-algo annotation. Each comma-separated token must -// match one of: namespace, pod, image. Empty tokens are dropped; unknown -// tokens are reported. -func parseIPAlgo(s string) ([]embed.Field, error) { +// ResolveIPAlgo resolves the effective ip-algo for a pod. Precedence: +// +// pod annotation → NodeConfig annotation → nil (random IID). +// +// Empty, missing, or invalid annotations at any level fall through to the +// next. Invalid input emits a warning via log; a nil log is silent. A nil +// return value means "no algo, generate a fully random IID". +// +// "Invalid" is everything tryParseIPAlgo cannot turn into a non-empty, +// duplicate-free subset of {namespace, pod, image} — unrecognised tokens, +// duplicates, lists that resolve to zero fields after trimming. +func ResolveIPAlgo(podAnn, nodeAnn map[string]string, log *slog.Logger) []embed.Field { + if v, ok := podAnn[annotationPrefix+annIPAlgo]; ok { + if fields := tryParseIPAlgo(v); fields != nil { + return fields + } + warnIPAlgo(log, "pod", v) + } + if v, ok := nodeAnn[annotationPrefix+annIPAlgo]; ok { + if fields := tryParseIPAlgo(v); fields != nil { + return fields + } + warnIPAlgo(log, "NodeConfig", v) + } + return nil +} + +// warnIPAlgo logs a single warning when an ip-algo annotation is present +// but cannot be parsed. Empty values are not worth a warn — they are +// indistinguishable from "key absent" by the user's design rule, so we +// only warn when a non-empty value failed parsing. +func warnIPAlgo(log *slog.Logger, source, value string) { + if log == nil { + return + } + if strings.TrimSpace(value) == "" { + return + } + log.Warn("ignoring invalid ip-algo annotation; falling through", + "source", source, "value", value) +} + +// tryParseIPAlgo parses an ip-algo annotation value under the relaxed +// "invalid → unset" rules. Returns nil for: empty input, unrecognised +// tokens, duplicate fields, or anything that resolves to zero fields after +// trimming. Returns the ordered field list otherwise. +// +// Duplicates collapse to nil rather than dedup-and-keep so the operator +// notices their malformed annotation via the warn log instead of silently +// losing a field they thought they had specified. +func tryParseIPAlgo(s string) []embed.Field { var out []embed.Field + seen := map[embed.Field]struct{}{} for _, part := range strings.Split(s, ",") { part = strings.TrimSpace(part) - switch part { - case "": + if part == "" { continue - case string(embed.FieldNamespace): - out = append(out, embed.FieldNamespace) - case string(embed.FieldPod): - out = append(out, embed.FieldPod) - case string(embed.FieldImage): - out = append(out, embed.FieldImage) - default: - return nil, fmt.Errorf("unknown ip-algo field %q (allowed: namespace, pod, image)", part) } + var f embed.Field + switch part { + case string(embed.FieldNamespace): + f = embed.FieldNamespace + case string(embed.FieldPod): + f = embed.FieldPod + case string(embed.FieldImage): + f = embed.FieldImage + default: + return nil + } + if _, dup := seen[f]; dup { + return nil + } + seen[f] = struct{}{} + out = append(out, f) } if len(out) == 0 { - return nil, fmt.Errorf("empty ip-algo") + return nil } - return out, nil + return out } // CNIArgs is the typed view of the K=V;K=V CNI_ARGS string passed by kubelet. diff --git a/pkg/agent/annotations_fuzz_test.go b/pkg/agent/annotations_fuzz_test.go index 2385d73..83bf127 100644 --- a/pkg/agent/annotations_fuzz_test.go +++ b/pkg/agent/annotations_fuzz_test.go @@ -5,9 +5,10 @@ import ( ) // FuzzParseAnnotations explores the joint space of {ipv6, ipv4, cidr6, cidr4, -// ip-algo, anycast} annotations with random byte strings. Every recognised -// key is exercised by deriving a deterministic input map from the fuzzed -// bytes; this gives the fuzzer reach into all parser branches at once. +// anycast} annotations with random byte strings. ip-algo is handled by +// ResolveIPAlgo (separate fuzz target below) and is no longer touched by +// ParseAnnotations. Every recognised key is exercised by deriving a +// deterministic input map from the fuzzed bytes. // // Properties checked: // @@ -15,15 +16,15 @@ import ( // 2. On nil-error return, the result satisfies the design-doc invariant // that at least one of WantV6 / WantV4 is true (a pod always has at // least one address). -// 3. Anycast IPs and IPAlgo fields are non-nil/empty only when the +// 3. Anycast IPs and CIDR slices are non-nil/empty only when the // annotation was supplied; never spontaneously populated. // // Seed corpus covers known edge cases the spec must handle. func FuzzParseAnnotations(f *testing.F) { - // Seeds: each entry is six strings — the literal raw values for the - // six parsed keys. Empty string for "key absent". + // Seeds: each entry is five strings — the literal raw values for the + // five parsed keys. Empty string for "key absent". type seed struct { - ipv6, ipv4, cidr6, cidr4, ipAlgo, anycast string + ipv6, ipv4, cidr6, cidr4, anycast string } seeds := []seed{ {}, @@ -43,11 +44,6 @@ func FuzzParseAnnotations(f *testing.F) { {cidr4: "172.25.210.0/24"}, // valid {cidr4: "172.25.210.0/24,172.25.211.0/24"}, // multiple {cidr4: "2602:817::/32"}, // family mismatch - {ipAlgo: "namespace,pod,image"}, - {ipAlgo: "namespace, pod , image"}, // whitespace - {ipAlgo: "namespace,unknown"}, // invalid - {ipAlgo: ""}, // invalid (empty) - {ipAlgo: ","}, // invalid {anycast: "2602:817:3000:ac::1"}, {anycast: "2602:817:3000:ac::1, 172.25.255.1"}, {anycast: "::1"}, // loopback (allowed at parse time) @@ -62,15 +58,14 @@ func FuzzParseAnnotations(f *testing.F) { {anycast: "\x00\x00"}, // Unicode {ipv4: "trüe"}, - {ipAlgo: "námespace"}, // Very long {cidr6: longString("2602:817:3000:f001::/64,", 4096)}, } for _, s := range seeds { - f.Add(s.ipv6, s.ipv4, s.cidr6, s.cidr4, s.ipAlgo, s.anycast) + f.Add(s.ipv6, s.ipv4, s.cidr6, s.cidr4, s.anycast) } - f.Fuzz(func(t *testing.T, ipv6, ipv4, cidr6, cidr4, ipAlgo, anycast string) { + f.Fuzz(func(t *testing.T, ipv6, ipv4, cidr6, cidr4, anycast string) { in := map[string]string{} // Treat empty as "key absent" so the seed table matches the run-time // shape; Kubernetes annotations cannot have a nil value but they CAN @@ -88,9 +83,6 @@ func FuzzParseAnnotations(f *testing.F) { if cidr4 != "" { in[annotationPrefix+annCIDR4] = cidr4 } - if ipAlgo != "" { - in[annotationPrefix+annIPAlgo] = ipAlgo - } if anycast != "" { in[annotationPrefix+annAnycast] = anycast } @@ -104,9 +96,6 @@ func FuzzParseAnnotations(f *testing.F) { t.Fatalf("parser accepted but produced no family: in=%#v", in) } // Property: optional fields populated only when their key was set. - if _, hasAlgo := in[annotationPrefix+annIPAlgo]; !hasAlgo && len(got.IPAlgo) != 0 { - t.Fatalf("IPAlgo populated without annotation") - } if _, hasAny := in[annotationPrefix+annAnycast]; !hasAny && len(got.Anycast) != 0 { t.Fatalf("Anycast populated without annotation") } diff --git a/pkg/agent/annotations_test.go b/pkg/agent/annotations_test.go index 691d283..40f30ed 100644 --- a/pkg/agent/annotations_test.go +++ b/pkg/agent/annotations_test.go @@ -174,32 +174,105 @@ func TestParseAnnotations_BoolCaseInsensitive(t *testing.T) { } } -func TestParseAnnotations_IPAlgo(t *testing.T) { - a, err := ParseAnnotations(map[string]string{ - annotationPrefix + "ip-algo": "namespace,pod,image", - }, BuiltinFamilyDefaults()) - if err != nil { - t.Fatal(err) +// ResolveIPAlgo: precedence is pod → node → nil. Empty / missing / invalid +// at any level falls through to the next under the relaxed user-defined rule +// "all three mean unset". + +func TestResolveIPAlgo_PodWins(t *testing.T) { + pod := map[string]string{annotationPrefix + annIPAlgo: "namespace,pod"} + node := map[string]string{annotationPrefix + annIPAlgo: "image"} + got := ResolveIPAlgo(pod, node, nil) + want := []embed.Field{embed.FieldNamespace, embed.FieldPod} + if !equalFields(got, want) { + t.Fatalf("got %v, want %v", got, want) } - want := []embed.Field{embed.FieldNamespace, embed.FieldPod, embed.FieldImage} - if len(a.IPAlgo) != len(want) { - t.Fatalf("ip-algo len=%d, want %d", len(a.IPAlgo), len(want)) +} + +func TestResolveIPAlgo_PodAbsentFallsToNode(t *testing.T) { + node := map[string]string{annotationPrefix + annIPAlgo: "image"} + got := ResolveIPAlgo(nil, node, nil) + want := []embed.Field{embed.FieldImage} + if !equalFields(got, want) { + t.Fatalf("got %v, want %v", got, want) } - for i := range want { - if a.IPAlgo[i] != want[i] { - t.Fatalf("ip-algo[%d]=%s, want %s", i, a.IPAlgo[i], want[i]) +} + +func TestResolveIPAlgo_PodEmptyFallsToNode(t *testing.T) { + pod := map[string]string{annotationPrefix + annIPAlgo: ""} + node := map[string]string{annotationPrefix + annIPAlgo: "image"} + got := ResolveIPAlgo(pod, node, nil) + want := []embed.Field{embed.FieldImage} + if !equalFields(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestResolveIPAlgo_PodInvalidFallsToNode(t *testing.T) { + for _, podVal := range []string{"namespace,bogus", "ns", ",", "namespace,namespace"} { + pod := map[string]string{annotationPrefix + annIPAlgo: podVal} + node := map[string]string{annotationPrefix + annIPAlgo: "pod"} + got := ResolveIPAlgo(pod, node, nil) + want := []embed.Field{embed.FieldPod} + if !equalFields(got, want) { + t.Fatalf("podVal=%q: got %v, want %v", podVal, got, want) } } } -func TestParseAnnotations_IPAlgo_Unknown(t *testing.T) { - if _, err := ParseAnnotations(map[string]string{ - annotationPrefix + "ip-algo": "namespace,foo", - }, BuiltinFamilyDefaults()); err == nil { - t.Fatalf("expected unknown-field error") +func TestResolveIPAlgo_BothInvalidReturnsNil(t *testing.T) { + pod := map[string]string{annotationPrefix + annIPAlgo: "bogus"} + node := map[string]string{annotationPrefix + annIPAlgo: "also-bogus"} + if got := ResolveIPAlgo(pod, node, nil); got != nil { + t.Fatalf("got %v, want nil", got) } } +func TestResolveIPAlgo_BothAbsentReturnsNil(t *testing.T) { + if got := ResolveIPAlgo(nil, nil, nil); got != nil { + t.Fatalf("got %v, want nil", got) + } +} + +func TestResolveIPAlgo_NilNodeMap(t *testing.T) { + pod := map[string]string{annotationPrefix + annIPAlgo: "image"} + got := ResolveIPAlgo(pod, nil, nil) + want := []embed.Field{embed.FieldImage} + if !equalFields(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestResolveIPAlgo_Whitespace(t *testing.T) { + pod := map[string]string{annotationPrefix + annIPAlgo: " namespace , pod "} + got := ResolveIPAlgo(pod, nil, nil) + want := []embed.Field{embed.FieldNamespace, embed.FieldPod} + if !equalFields(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestResolveIPAlgo_DuplicateInvalidates(t *testing.T) { + pod := map[string]string{annotationPrefix + annIPAlgo: "pod,pod"} + node := map[string]string{annotationPrefix + annIPAlgo: "namespace"} + got := ResolveIPAlgo(pod, node, nil) + want := []embed.Field{embed.FieldNamespace} + if !equalFields(got, want) { + t.Fatalf("got %v, want %v (duplicate must collapse to invalid)", got, want) + } +} + +func equalFields(a, b []embed.Field) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + func TestParseAnnotations_CIDR(t *testing.T) { a, err := ParseAnnotations(map[string]string{ annotationPrefix + "cidr6": "2602:817:3000:f001::/64, 2602:817:3000:f002::/64", diff --git a/pkg/agent/handlers.go b/pkg/agent/handlers.go index eff35b9..4d1c021 100644 --- a/pkg/agent/handlers.go +++ b/pkg/agent/handlers.go @@ -3,6 +3,7 @@ package agent import ( "context" "fmt" + "log/slog" "net" "time" @@ -22,6 +23,7 @@ type PodHandler struct { IPAM *IPAM Pods *PodCache NodeConfig *NodeConfigCache + Logger *slog.Logger // SetupFunc and TeardownFunc are injected at startup; in production // they point at the Linux netlink ops, in tests they're fakes. SetupFunc func(SetupRequest) error @@ -49,12 +51,19 @@ func (h *PodHandler) Add(ctx context.Context, req flockcni.Request) (*current.Re return nil, fmt.Errorf("lookup pod: %w", err) } - defaults := FamilyDefaultsFromNodeConfig(h.NodeConfig.Load()) + nc := h.NodeConfig.Load() + defaults := FamilyDefaultsFromNodeConfig(nc) parsed, err := ParseAnnotations(pod.Annotations, defaults) if err != nil { return nil, fmt.Errorf("parse annotations: %w", err) } + var nodeAnn map[string]string + if nc != nil { + nodeAnn = nc.GetAnnotations() + } + ipAlgo := ResolveIPAlgo(pod.Annotations, nodeAnn, h.Logger) + allocReq := AllocRequest{ ContainerID: req.ContainerID, Namespace: args.PodNamespace, @@ -63,7 +72,7 @@ func (h *PodHandler) Add(ctx context.Context, req flockcni.Request) (*current.Re WantV4: parsed.WantV4, AnnCIDR6: parsed.CIDR6, AnnCIDR4: parsed.CIDR4, - IPAlgo: parsed.IPAlgo, + IPAlgo: ipAlgo, } res, err := h.IPAM.Allocate(allocReq) if err != nil { diff --git a/pkg/agent/runtime_linux.go b/pkg/agent/runtime_linux.go index 517fa5c..73ec4fd 100644 --- a/pkg/agent/runtime_linux.go +++ b/pkg/agent/runtime_linux.go @@ -149,6 +149,7 @@ func (s *Server) configureRuntime(ctx context.Context) error { IPAM: ipam, Pods: pods, NodeConfig: s.NodeConfig, + Logger: s.Logger, SetupFunc: Setup, TeardownFunc: Teardown, AfterCommit: func() {