NodeConfig defaults + code-quality pass + fuzz tests + README

NodeConfig.Spec.Defaults adds per-node IPv6/IPv4 family defaults that pod annotations can override; built-in baseline (v6=true, v4=false) still applies when the field is omitted. bird.Render now validates every operator-supplied value (peer addresses, CIDRs, anycast IPs, source addresses) before templating — fuzz found a peer address containing `}` produced unbalanced braces in bird.conf. Failing input preserved as a regression seed. Fuzz targets added for ParseAnnotations, ParseCNIArgs, HostIfaceName, canonical, IPAM allocate sequences, embed.Embed, and bird.Render. Hardened canonical/ipToU32 against nil and non-IPv4 inputs. README rewritten for outside readers — quickstart, NodeConfig + annotation reference with worked examples, anycast use cases, comparison vs Calico and Cilium, requirements, limitations. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
2026-04-25 09:25:45 -05:00
parent 677aec2a42
commit 71e584cf96
17 changed files with 1583 additions and 100 deletions
@@ -5,77 +5,153 @@ import (
 	"net"
 	"strings"

+	flockv1alpha1 "code.fritzlab.net/fritzlab/flock/pkg/api/v1alpha1"
 	"code.fritzlab.net/fritzlab/flock/pkg/embed"
 )

+// annotationPrefix is the namespace under which all flock pod annotations
+// live. Anything not starting with this prefix is ignored by the parser.
 const annotationPrefix = "flock.fritzlab.net/"

-// ParsedAnnotations is the typed view of a Pod's flock annotations.
-type ParsedAnnotations struct {
-	WantV6   bool
-	WantV4   bool
-	CIDR6    []*net.IPNet
-	CIDR4    []*net.IPNet
-	IPAlgo   []embed.Field
-	Anycast  []net.IP
+// Recognised annotation keys (without the prefix).
+const (
+	annIPv6    = "ipv6"
+	annIPv4    = "ipv4"
+	annCIDR6   = "cidr6"
+	annCIDR4   = "cidr4"
+	annIPAlgo  = "ip-algo"
+	annAnycast = "anycast"
+)
+
+// FamilyDefaults is the per-call baseline for whether a pod receives an IPv6
+// and/or IPv4 address. It is the merge of:
+//
+//  1. flock's built-in baseline (IPv6=true, IPv4=false), then
+//  2. any NodeConfig.Spec.Defaults override the operator has applied to
+//     the local node.
+//
+// Pod-level `flock.fritzlab.net/ipv{6,4}` annotations override this baseline.
+//
+// Use FamilyDefaultsFromNodeConfig to compute a value from a NodeConfig,
+// or BuiltinFamilyDefaults() if no NodeConfig is in scope.
+type FamilyDefaults struct {
+	// WantV6 is the default-on value for IPv6 inclusion when the pod has no
+	// explicit ipv6 annotation.
+	WantV6 bool
+	// WantV4 is the default-on value for IPv4 inclusion when the pod has no
+	// explicit ipv4 annotation.
+	WantV4 bool
 }

-// ParseAnnotations applies the design-doc defaults (ipv6=true, ipv4=false)
-// and validates the post-merge combination.
-func ParseAnnotations(in map[string]string) (*ParsedAnnotations, error) {
-	out := &ParsedAnnotations{WantV6: true, WantV4: false}
+// BuiltinFamilyDefaults returns flock's hard-coded fallback: IPv6 only.
+// This is the policy applied when no NodeConfig override is in effect.
+//
+// We define it as a function rather than a var so callers can't mutate the
+// shared baseline at runtime.
+func BuiltinFamilyDefaults() FamilyDefaults {
+	return FamilyDefaults{WantV6: true, WantV4: false}
+}

-	if v, ok := in[annotationPrefix+"ipv6"]; ok {
-		switch strings.ToLower(strings.TrimSpace(v)) {
-		case "true":
-			out.WantV6 = true
-		case "false":
-			out.WantV6 = false
-		default:
-			return nil, fmt.Errorf("annotation ipv6=%q: must be true or false", v)
-		}
+// FamilyDefaultsFromNodeConfig resolves the effective per-node defaults,
+// falling back to BuiltinFamilyDefaults for any field the NodeConfig leaves
+// unset. A nil NodeConfig (or nil Spec.Defaults) returns the built-in
+// baseline unchanged.
+func FamilyDefaultsFromNodeConfig(nc *flockv1alpha1.NodeConfig) FamilyDefaults {
+	out := BuiltinFamilyDefaults()
+	if nc == nil || nc.Spec.Defaults == nil {
+		return out
 	}
-	if v, ok := in[annotationPrefix+"ipv4"]; ok {
-		switch strings.ToLower(strings.TrimSpace(v)) {
-		case "true":
-			out.WantV4 = true
-		case "false":
-			out.WantV4 = false
-		default:
-			return nil, fmt.Errorf("annotation ipv4=%q: must be true or false", v)
+	if nc.Spec.Defaults.IPv6 != nil {
+		out.WantV6 = *nc.Spec.Defaults.IPv6
+	}
+	if nc.Spec.Defaults.IPv4 != nil {
+		out.WantV4 = *nc.Spec.Defaults.IPv4
+	}
+	return out
+}
+
+// ParsedAnnotations is the typed view of a pod's flock annotations after the
+// node-level defaults have been merged in. All slices are non-nil only when
+// the corresponding annotation was present and parsed cleanly.
+type ParsedAnnotations struct {
+	// WantV6 is true when the pod should receive an IPv6 address.
+	WantV6 bool
+	// WantV4 is true when the pod should receive an IPv4 address.
+	WantV4 bool
+	// CIDR6 narrows IPv6 allocation to specific operator-approved sub-ranges
+	// of the node's CIDR6 set. nil/empty means "use any node CIDR6".
+	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
+}
+
+// ParseAnnotations applies the supplied per-node defaults and validates the
+// post-merge combination. It is pure — it does not consult NodeConfig or any
+// global state — so it is safe to call from tests and fuzz targets.
+//
+// Annotation precedence: pod annotation > FamilyDefaults > built-in baseline.
+// Callers compute FamilyDefaults via FamilyDefaultsFromNodeConfig and pass it
+// in.
+//
+// Errors:
+//   - any unknown ipv6/ipv4 value (must be "true" or "false", case-insensitive)
+//   - any malformed cidr6/cidr4/anycast/ip-algo value
+//   - the post-merge combination resolves to neither IPv6 nor IPv4 (a pod
+//     must have at least one address)
+func ParseAnnotations(in map[string]string, defaults FamilyDefaults) (*ParsedAnnotations, error) {
+	out := &ParsedAnnotations{WantV6: defaults.WantV6, WantV4: defaults.WantV4}
+
+	if v, ok := in[annotationPrefix+annIPv6]; ok {
+		b, err := parseBoolAnnotation(annIPv6, v)
+		if err != nil {
+			return nil, err
 		}
+		out.WantV6 = b
+	}
+	if v, ok := in[annotationPrefix+annIPv4]; ok {
+		b, err := parseBoolAnnotation(annIPv4, v)
+		if err != nil {
+			return nil, err
+		}
+		out.WantV4 = b
 	}
 	if !out.WantV6 && !out.WantV4 {
-		return nil, fmt.Errorf("ipv6=false requires ipv4=true (pod must have at least one address)")
+		return nil, fmt.Errorf("annotations + defaults resolve to no address family (need at least one of ipv6/ipv4)")
 	}

-	if v, ok := in[annotationPrefix+"cidr6"]; ok {
-		nets, err := parseCIDRList(v)
+	if v, ok := in[annotationPrefix+annCIDR6]; ok {
+		nets, err := parseCIDRList(v, familyV6)
 		if err != nil {
-			return nil, fmt.Errorf("annotation cidr6: %w", err)
+			return nil, fmt.Errorf("annotation %s: %w", annCIDR6, err)
 		}
 		out.CIDR6 = nets
 	}
-	if v, ok := in[annotationPrefix+"cidr4"]; ok {
-		nets, err := parseCIDRList(v)
+	if v, ok := in[annotationPrefix+annCIDR4]; ok {
+		nets, err := parseCIDRList(v, familyV4)
 		if err != nil {
-			return nil, fmt.Errorf("annotation cidr4: %w", err)
+			return nil, fmt.Errorf("annotation %s: %w", annCIDR4, err)
 		}
 		out.CIDR4 = nets
 	}

-	if v, ok := in[annotationPrefix+"ip-algo"]; ok {
+	if v, ok := in[annotationPrefix+annIPAlgo]; ok {
 		fields, err := parseIPAlgo(v)
 		if err != nil {
-			return nil, fmt.Errorf("annotation ip-algo: %w", err)
+			return nil, fmt.Errorf("annotation %s: %w", annIPAlgo, err)
 		}
 		out.IPAlgo = fields
 	}

-	if v, ok := in[annotationPrefix+"anycast"]; ok {
+	if v, ok := in[annotationPrefix+annAnycast]; ok {
 		ips, err := parseIPList(v)
 		if err != nil {
-			return nil, fmt.Errorf("annotation anycast: %w", err)
+			return nil, fmt.Errorf("annotation %s: %w", annAnycast, err)
 		}
 		out.Anycast = ips
 	}
@@ -83,7 +159,39 @@ func ParseAnnotations(in map[string]string) (*ParsedAnnotations, error) {
 	return out, nil
 }

-func parseCIDRList(s string) ([]*net.IPNet, error) {
+// parseBoolAnnotation accepts only "true" or "false" (case-insensitive,
+// surrounding whitespace tolerated). All other values — including "1", "0",
+// "yes", "no" — are rejected so operator typos are caught loudly rather
+// than silently producing the "false" default.
+func parseBoolAnnotation(key, v string) (bool, error) {
+	switch strings.ToLower(strings.TrimSpace(v)) {
+	case "true":
+		return true, nil
+	case "false":
+		return false, nil
+	default:
+		return false, fmt.Errorf("annotation %s=%q: must be \"true\" or \"false\"", key, v)
+	}
+}
+
+// addressFamily distinguishes IPv6 vs IPv4 in places where the parser must
+// validate the family of supplied CIDRs.
+type addressFamily int
+
+const (
+	familyAny addressFamily = iota
+	familyV6
+	familyV4
+)
+
+// parseCIDRList parses a comma-separated CIDR list. Whitespace around items
+// is trimmed; empty items are silently dropped. The list must contain at
+// least one entry post-trim.
+//
+// If `want` is familyV6 or familyV4 each entry's family is checked and a
+// mismatch is reported, so an `flock.fritzlab.net/cidr6` annotation cannot
+// silently slip a v4 prefix into the v6 allocator.
+func parseCIDRList(s string, want addressFamily) ([]*net.IPNet, error) {
 	var out []*net.IPNet
 	for _, part := range strings.Split(s, ",") {
 		part = strings.TrimSpace(part)
@@ -94,6 +202,17 @@ func parseCIDRList(s string) ([]*net.IPNet, error) {
 		if err != nil {
 			return nil, fmt.Errorf("invalid CIDR %q: %w", part, err)
 		}
+		isV4 := n.IP.To4() != nil
+		switch want {
+		case familyV6:
+			if isV4 {
+				return nil, fmt.Errorf("CIDR %q is IPv4, expected IPv6", part)
+			}
+		case familyV4:
+			if !isV4 {
+				return nil, fmt.Errorf("CIDR %q is IPv6, expected IPv4", part)
+			}
+		}
 		out = append(out, n)
 	}
 	if len(out) == 0 {
@@ -102,6 +221,9 @@ func parseCIDRList(s string) ([]*net.IPNet, error) {
 	return out, nil
 }

+// parseIPList parses a comma-separated literal-IP list. Same trim/empty
+// semantics as parseCIDRList. Mixed v4 and v6 entries are allowed (anycast
+// pods can advertise both families together).
 func parseIPList(s string) ([]net.IP, error) {
 	var out []net.IP
 	for _, part := range strings.Split(s, ",") {
@@ -121,6 +243,9 @@ 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) {
 	var out []embed.Field
 	for _, part := range strings.Split(s, ",") {
@@ -128,11 +253,11 @@ func parseIPAlgo(s string) ([]embed.Field, error) {
 		switch part {
 		case "":
 			continue
-		case "namespace":
+		case string(embed.FieldNamespace):
 			out = append(out, embed.FieldNamespace)
-		case "pod":
+		case string(embed.FieldPod):
 			out = append(out, embed.FieldPod)
-		case "image":
+		case string(embed.FieldImage):
 			out = append(out, embed.FieldImage)
 		default:
 			return nil, fmt.Errorf("unknown ip-algo field %q (allowed: namespace, pod, image)", part)
@@ -144,8 +269,8 @@ func parseIPAlgo(s string) ([]embed.Field, error) {
 	return out, nil
 }

-// CNIArgs parses the K=V;K=V CNI_ARGS string for the kubelet keys we care
-// about. Other keys are ignored.
+// CNIArgs is the typed view of the K=V;K=V CNI_ARGS string passed by kubelet.
+// We only keep the fields the agent uses; unknown keys are ignored.
 type CNIArgs struct {
 	PodNamespace string
 	PodName      string
@@ -153,6 +278,10 @@ type CNIArgs struct {
 	InfraID      string
 }

+// ParseCNIArgs is permissive by design — kubelet versions and runtime
+// shims pass varying sets of keys. Malformed entries are skipped silently
+// rather than failing the whole ADD; required-key validation is the
+// caller's responsibility.
 func ParseCNIArgs(s string) CNIArgs {
 	var a CNIArgs
 	for _, kv := range strings.Split(s, ";") {