diff --git a/cmd/atenet/internal/dns/README.md b/cmd/atenet/internal/dns/README.md index a134e829ab..cb28e837f2 100644 --- a/cmd/atenet/internal/dns/README.md +++ b/cmd/atenet/internal/dns/README.md @@ -10,7 +10,6 @@ Cluster resources: * Deployment `ate-system:dns`. Label: app=dns * Service `ate-system:dns`. -* ConfigMap `ate-system:dns`. These are defined in manifests/ate-install/atenet-dns.yaml. @@ -20,16 +19,53 @@ These are defined in manifests/ate-install/atenet-dns.yaml. * Deployment `ate-system:dns`. * Service `ate-system:dns` pointing to the Deployment. -ConfigMap `ate-system:dns`: +`corefile.go` renders the zone below; the controller writes it to +`--corefile-path` on an emptyDir shared with the CoreDNS container and signals +a reload. The excerpt is illustrative — `corefile.go` is authoritative, and +`TestMakeCoreFile` pins the exact rendering for each family combination. ``` -# Match any 'A' query for an actor name + atespace pattern under actors.resources.substrate.ate.dev +# Answer any 'A' query for an actor name + atespace pattern under actors.resources.substrate.ate.dev template IN A actors.resources.substrate.ate.dev { match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.actors\\.resources\\.substrate\\.ate\\.dev\\.$" - answer "{{ .Name }} 60 IN A " + answer "{{ .Name }} 60 IN A " + fallthrough + } +# The same for 'AAAA', when the router Service has an IPv6 ClusterIP. + template IN AAAA actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.actors\\.resources\\.substrate\\.ate\\.dev\\.$" + answer "{{ .Name }} 60 IN AAAA " + fallthrough + } +# NODATA for a well-formed actor name on any other qtype (HTTPS, SRV, ...), and +# for the family the router has no ClusterIP in. + template ANY ANY actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.actors\\.resources\\.substrate\\.ate\\.dev\\.$" + rcode NOERROR + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + fallthrough + } +# Terminal catch-all: NXDOMAIN for anything else in the zone. + template ANY ANY actors.resources.substrate.ate.dev { + rcode NXDOMAIN + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" } ``` +An address block is emitted only for a family the atenet-router Service +actually has a ClusterIP in, which on any cluster where `ipFamilyPolicy` is +unset means exactly one of the two. That is not tidiness: the `answer` line is a +literal RR, so an `IN A` carrying an IPv6 address parses fine as a Corefile and +then fails `dns.NewRR` on every query, SERVFAILing the whole zone. Leaving the +family out hands it to the NODATA block instead, which is the right answer for a +name with no address of that type. + +The last two blocks keep the zone from ever answering SERVFAIL, which musl libc +maps to `EAI_AGAIN` — sinking the paired A query with it — and which cannot be +cached negatively. The `fallthrough` on every block that carries a `match` is +load-bearing: the template plugin walks past a class or qtype mismatch on its +own, but a regex miss returns SERVFAIL immediately unless the block declares it. + ## Integration * CoreDNS: Update CoreDNS ConfigMap to add the stub resolver. diff --git a/cmd/atenet/internal/dns/corefile.go b/cmd/atenet/internal/dns/corefile.go index 0b301e7e29..32d4043b74 100644 --- a/cmd/atenet/internal/dns/corefile.go +++ b/cmd/atenet/internal/dns/corefile.go @@ -22,15 +22,31 @@ import ( "github.com/agent-substrate/substrate/internal/resources" ) -// corefileTemplate is a Sprintf template for the CoreDNS configuration. -var corefileTemplate string +const ( + fallthroughDirective = " fallthrough" + soaDirective = ` authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)"` +) -func init() { - corefileTemplate = buildTemplate() -} +// generatedAt stamps the rendered Corefile once per process, and must not be +// recomputed per call: reconcileCoreDNSConfig decides whether to rewrite the +// file and signal CoreDNS by comparing the render against what is on disk, so a +// moving timestamp would reload the server on every tick of the reconcile loop. +var generatedAt = time.Now() -func buildTemplate() string { - // Build up the corefileTemplate programmatically to make it easier to understand. +// makeCoreFile renders the actor zone for the router Service's ClusterIPs. +// +// A family gets an address template only when the router actually has an +// address in it. That is not an optimization: an address template is a literal +// RR, so emitting `IN A ` on a v6-only cluster produces a Corefile +// that loads clean and then fails dns.NewRR on every query, turning the whole +// zone into SERVFAIL. Omitting the block instead leaves the family to the +// NODATA template below, which is the correct answer for a name with no address +// of that type. +// +// Either argument may be empty, and on any cluster where ipFamilyPolicy is +// unset exactly one of them will be. +func makeCoreFile(routerV4, routerV6 string) string { + // Build up the Corefile programmatically to make it easier to understand. var directives []string // Plugins to enable. directives = append(directives, "log") @@ -41,17 +57,37 @@ func buildTemplate() string { // Construct match pattern for ... Both the // actor name and the atespace are DNS-1123 labels (same regex). - directives = append(directives, fmt.Sprintf("template IN A %s {", resources.ActorDNSSuffix)) // Escape the suffix's dots so they match literally; the final \. matches the FQDN's trailing dot. escapedSuffix := strings.ReplaceAll(resources.ActorDNSSuffix, ".", `\.`) - directives = append(directives, fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix)) - // Note the %s -- this will be filled with the router IP. - directives = append(directives, ` answer "{{ .Name }} 60 IN A %s"`) + actorMatch := fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix) + + if routerV4 != "" { + directives = append(directives, addressTemplate("A", routerV4, actorMatch)...) + } + if routerV6 != "" { + directives = append(directives, addressTemplate("AAAA", routerV6, actorMatch)...) + } + + // Valid actor names return NOERROR (NODATA) for the qtypes not answered + // above, which includes the family the router has no address in. + directives = append(directives, fmt.Sprintf("template ANY ANY %s {", resources.ActorDNSSuffix)) + directives = append(directives, actorMatch) + directives = append(directives, " rcode NOERROR") + directives = append(directives, soaDirective) + directives = append(directives, fallthroughDirective) + directives = append(directives, "}") + + // Returns rcode NXDOMAIN (Non-Existent Domain) for any query that did not + // match the valid actor regex in the previous blocks. + // TODO(#922): answer empty non-terminals with NODATA. + directives = append(directives, fmt.Sprintf("template ANY ANY %s {", resources.ActorDNSSuffix)) + directives = append(directives, " rcode NXDOMAIN") + directives = append(directives, soaDirective) directives = append(directives, "}") - // Generate the template. + // Generate the Corefile. b := strings.Builder{} - fmt.Fprintf(&b, "# Generated at %s\n", time.Now()) + fmt.Fprintf(&b, "# Generated at %s\n", generatedAt) fmt.Fprintf(&b, "%s:53 {\n ", resources.ActorDNSSuffix) fmt.Fprint(&b, strings.Join(directives, "\n ")) fmt.Fprint(&b, "\n}\n") @@ -59,6 +95,16 @@ func buildTemplate() string { return b.String() } -func makeCoreFile(routerIP string) string { - return fmt.Sprintf(corefileTemplate, routerIP) +// addressTemplate returns the template block that answers qtype ("A" or "AAAA") +// for actor names with addr. addr is interpolated into an RR verbatim, so it +// must already be known to be an address of that family -- see +// ipfamily.ClusterIPsByFamily, which is where callers get it. +func addressTemplate(qtype, addr, actorMatch string) []string { + return []string{ + fmt.Sprintf("template IN %s %s {", qtype, resources.ActorDNSSuffix), + actorMatch, + fmt.Sprintf(` answer "{{ .Name }} 60 IN %s %s"`, qtype, addr), + fallthroughDirective, + "}", + } } diff --git a/cmd/atenet/internal/dns/corefile_test.go b/cmd/atenet/internal/dns/corefile_test.go index f13429e475..c207926623 100644 --- a/cmd/atenet/internal/dns/corefile_test.go +++ b/cmd/atenet/internal/dns/corefile_test.go @@ -17,49 +17,147 @@ package dns import ( "strings" "testing" +) + +// actorMatchDirective is the match line every template that scopes itself to +// real actor names carries; soaAuthorityDirective is the record that makes the +// negative answers cacheable. Both are spelled out rather than built from +// resources.ResourceNameRegexPattern and ActorDNSSuffix: the rendered zone is a +// wire contract, so a change to either constant should fail here instead of +// being tracked silently. +const ( + actorMatchDirective = `match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\.actors\.resources\.substrate\.ate\.dev\.$"` + soaAuthorityDirective = `authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)"` +) + +// The zones below are compared whole rather than by substring because every +// part of them is behavior: templates are evaluated in Corefile order, every +// block carrying a "match" needs a "fallthrough" to reach the blocks after it, +// the catch-all must be last and must not declare one, and the indentation has +// to parse. See README.md for what the template plugin does with each. +// +// The head and tail are shared to keep the four goldens readable. That does not +// weaken the ordering assertion: each golden is still the whole expected file, +// with the address blocks spelled out between them. +const ( + wantZoneHead = `actors.resources.substrate.ate.dev:53 { + log + errors + health :8080 + ready :8181 + reload +` + wantZoneTail = ` template ANY ANY actors.resources.substrate.ate.dev { + ` + actorMatchDirective + ` + rcode NOERROR + ` + soaAuthorityDirective + ` + fallthrough + } + template ANY ANY actors.resources.substrate.ate.dev { + rcode NXDOMAIN + ` + soaAuthorityDirective + ` + } +} +` +) + +const ( + wantZoneIPv4 = wantZoneHead + ` template IN A actors.resources.substrate.ate.dev { + ` + actorMatchDirective + ` + answer "{{ .Name }} 60 IN A 10.240.0.10" + fallthrough + } +` + wantZoneTail + + wantZoneIPv6 = wantZoneHead + ` template IN AAAA actors.resources.substrate.ate.dev { + ` + actorMatchDirective + ` + answer "{{ .Name }} 60 IN AAAA fd00:10:96::8857" + fallthrough + } +` + wantZoneTail + + wantZoneDualStack = wantZoneHead + ` template IN A actors.resources.substrate.ate.dev { + ` + actorMatchDirective + ` + answer "{{ .Name }} 60 IN A 10.96.233.69" + fallthrough + } + template IN AAAA actors.resources.substrate.ate.dev { + ` + actorMatchDirective + ` + answer "{{ .Name }} 60 IN AAAA fd00:10:96::7373" + fallthrough + } +` + wantZoneTail - "github.com/agent-substrate/substrate/internal/resources" + wantZoneNoAddresses = wantZoneHead + wantZoneTail ) +// zoneBody strips the "# Generated at " header. +func zoneBody(t *testing.T, corefile string) string { + t.Helper() + header, body, ok := strings.Cut(corefile, "\n") + if !ok || !strings.HasPrefix(header, "# Generated at ") { + t.Fatalf("makeCoreFile() has no generated-at header, got first line %q", header) + } + return body +} + func TestMakeCoreFile(t *testing.T) { tests := []struct { name string - routerIP string - expected []string + routerV4 string + routerV6 string + want string }{ { - name: "standard local IP", - routerIP: "10.240.0.10", - expected: []string{ - "actors.resources.substrate.ate.dev:53 {", - "log", - "errors", - "health :8080", - "ready :8181", - "reload", - "template IN A actors.resources.substrate.ate.dev {", - `match "^` + resources.ResourceNameRegexPattern + `\.` + resources.ResourceNameRegexPattern + `\.actors\.resources\.substrate\.ate\.dev\.$"`, - `answer "{{ .Name }} 60 IN A 10.240.0.10"`, - }, + // AAAA is left to the NODATA template in the tail, which is the + // right answer for a name with no address of that type. Publishing + // the v4 ClusterIP as an AAAA instead would render a literal RR that + // loads clean and then fails dns.NewRR on every query. + name: "IPv4 only", + routerV4: "10.240.0.10", + want: wantZoneIPv4, + }, + { + // The bug this change exists for: a v6-only cluster's sole ClusterIP + // used to be published as an A record, SERVFAILing the whole zone. + name: "IPv6 only", + routerV6: "fd00:10:96::8857", + want: wantZoneIPv6, }, { - name: "different IP", - routerIP: "192.168.1.1", - expected: []string{ - "actors.resources.substrate.ate.dev:53 {", - `answer "{{ .Name }} 60 IN A 192.168.1.1"`, - }, + name: "dual stack", + routerV4: "10.96.233.69", + routerV6: "fd00:10:96::7373", + want: wantZoneDualStack, + }, + { + // The controller does not call makeCoreFile in this state, but the + // zone still has to be a loadable Corefile if it ever does: negative + // answers only, never a template with an empty address in it. + name: "no addresses", + want: wantZoneNoAddresses, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := makeCoreFile(tc.routerIP) - for _, exp := range tc.expected { - if !strings.Contains(got, exp) { - t.Errorf("makeCoreFile(%q) missing expected substring %q\nGot:\n%s", tc.routerIP, exp, got) - } + got := zoneBody(t, makeCoreFile(tc.routerV4, tc.routerV6)) + if got != tc.want { + t.Errorf("makeCoreFile(%q, %q) rendered an unexpected Corefile\nGot:\n%s\nWant:\n%s", tc.routerV4, tc.routerV6, got, tc.want) } }) } } + +// TestMakeCoreFileStable pins the property that keeps the reconcile loop quiet: +// the render depends only on its arguments. reconcileCoreDNSConfig rewrites the +// Corefile and signals CoreDNS whenever the render differs from what is on +// disk, so anything time-varying in the output -- the "Generated at" stamp, in +// particular -- would reload the DNS server on every tick. +func TestMakeCoreFileStable(t *testing.T) { + first := makeCoreFile("10.240.0.10", "fd00:10:96::8857") + second := makeCoreFile("10.240.0.10", "fd00:10:96::8857") + if first != second { + t.Errorf("makeCoreFile() is not stable across calls; the reconcile loop would rewrite and reload every tick\nFirst:\n%s\nSecond:\n%s", first, second) + } +} diff --git a/cmd/atenet/internal/dns/dns.go b/cmd/atenet/internal/dns/dns.go index cf2db99b69..96bc7df249 100644 --- a/cmd/atenet/internal/dns/dns.go +++ b/cmd/atenet/internal/dns/dns.go @@ -26,6 +26,7 @@ import ( "syscall" "time" + "github.com/agent-substrate/substrate/internal/ipfamily" "github.com/agent-substrate/substrate/internal/resources" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -50,7 +51,6 @@ type Controller struct { // Run the DNS orchestration loop until ctx is canceled. func (c *Controller) Run(ctx context.Context) error { slog.InfoContext(ctx, "DNS Controller started", slog.Duration("interval", c.Interval), slog.String("corefile", c.CorefilePath)) - slog.InfoContext(ctx, "Using template", "template", corefileTemplate) ticker := time.NewTicker(c.Interval) defer ticker.Stop() @@ -81,8 +81,10 @@ func (c *Controller) reconcile(ctx context.Context) error { return fmt.Errorf("failed to get atenet-router service: %w", err) } - routerIP := routerSvc.Spec.ClusterIP - if routerIP == "" || routerIP == "None" { + // Both families, not just Spec.ClusterIP: on an IPv6-only cluster the sole + // ClusterIP is a v6 address, and the zone has to publish it as an AAAA. + routerV4, routerV6 := ipfamily.ClusterIPsByFamily(routerSvc) + if routerV4 == "" && routerV6 == "" { slog.WarnContext(ctx, "atenet-router service has no ClusterIP yet, waiting...") return nil } @@ -104,7 +106,7 @@ func (c *Controller) reconcile(ctx context.Context) error { } // 3. Reconcile CoreDNS Corefile on shared volume - if err := c.reconcileCoreDNSConfig(ctx, routerIP); err != nil { + if err := c.reconcileCoreDNSConfig(ctx, routerV4, routerV6); err != nil { return fmt.Errorf("failed to reconcile CoreDNS config file: %w", err) } @@ -116,13 +118,13 @@ func (c *Controller) reconcile(ctx context.Context) error { return nil } -func (c *Controller) reconcileCoreDNSConfig(ctx context.Context, routerIP string) error { - expectedCorefile := makeCoreFile(routerIP) +func (c *Controller) reconcileCoreDNSConfig(ctx context.Context, routerV4, routerV6 string) error { + expectedCorefile := makeCoreFile(routerV4, routerV6) // Read Corefile from local shared volume path to see if it needs updating corefileBytes, err := os.ReadFile(c.CorefilePath) if err == nil && string(corefileBytes) == expectedCorefile { - slog.DebugContext(ctx, "CoreDNS Corefile is up-to-date", slog.String("routerIP", routerIP)) + slog.DebugContext(ctx, "CoreDNS Corefile is up-to-date", slog.String("routerIPv4", routerV4), slog.String("routerIPv6", routerV6)) return nil } @@ -130,7 +132,7 @@ func (c *Controller) reconcileCoreDNSConfig(ctx context.Context, routerIP string if err := os.WriteFile(c.CorefilePath, []byte(expectedCorefile), 0644); err != nil { return fmt.Errorf("failed to write updated Corefile to %s: %w", c.CorefilePath, err) } - slog.InfoContext(ctx, "CoreDNS Corefile updated", slog.String("routerIP", routerIP)) + slog.InfoContext(ctx, "CoreDNS Corefile updated", slog.String("routerIPv4", routerV4), slog.String("routerIPv6", routerV6)) // Signal CoreDNS process to reload if err := c.Reloader.Reload(ctx); err != nil { diff --git a/cmd/atenet/internal/dns/dns_test.go b/cmd/atenet/internal/dns/dns_test.go index 34116db284..34d23405d2 100644 --- a/cmd/atenet/internal/dns/dns_test.go +++ b/cmd/atenet/internal/dns/dns_test.go @@ -32,10 +32,12 @@ import ( type mockConfigReloader struct { reloaded bool + reloads int } func (m *mockConfigReloader) Reload(ctx context.Context) error { m.reloaded = true + m.reloads++ return nil } @@ -145,6 +147,131 @@ func TestReconcile(t *testing.T) { } } +// TestReconcileRouterIPFamilies covers what the controller publishes for each +// shape the atenet-router Service takes. The v6-only row is the one that used +// to be broken: the sole ClusterIP was read out of Spec.ClusterIP and written +// into an `IN A` answer, which loads as a valid Corefile and then SERVFAILs +// every query in the zone. +func TestReconcileRouterIPFamilies(t *testing.T) { + tests := []struct { + name string + // routerSpec is the atenet-router Service's spec; the dns Service is + // always a plain single-stack v4 one, since it feeds kube-dns rather + // than the zone under test. + routerSpec corev1.ServiceSpec + // wantAnswers are the answer lines the rendered Corefile must have. + wantAnswers []string + // wantNoAnswer, when true, means reconcile should leave the Corefile + // untouched rather than publish anything. + wantNoAnswer bool + }{ + { + name: "single stack IPv4", + routerSpec: corev1.ServiceSpec{ClusterIP: "10.0.0.1", ClusterIPs: []string{"10.0.0.1"}}, + wantAnswers: []string{`answer "{{ .Name }} 60 IN A 10.0.0.1"`}, + }, + { + name: "single stack IPv6", + routerSpec: corev1.ServiceSpec{ClusterIP: "fd00:10:96::8857", ClusterIPs: []string{"fd00:10:96::8857"}}, + wantAnswers: []string{`answer "{{ .Name }} 60 IN AAAA fd00:10:96::8857"`}, + }, + { + name: "dual stack", + routerSpec: corev1.ServiceSpec{ClusterIP: "10.0.0.1", ClusterIPs: []string{"10.0.0.1", "fd00:10:96::8857"}}, + wantAnswers: []string{ + `answer "{{ .Name }} 60 IN A 10.0.0.1"`, + `answer "{{ .Name }} 60 IN AAAA fd00:10:96::8857"`, + }, + }, + { + name: "not yet allocated", + routerSpec: corev1.ServiceSpec{}, + wantNoAnswer: true, + }, + { + name: "headless", + routerSpec: corev1.ServiceSpec{ClusterIP: corev1.ClusterIPNone, ClusterIPs: []string{corev1.ClusterIPNone}}, + wantNoAnswer: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + + routerSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "atenet-router", Namespace: "ate-system"}, + Spec: tc.routerSpec, + } + dnsSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "dns", Namespace: "ate-system"}, + Spec: corev1.ServiceSpec{ClusterIP: "10.0.0.2"}, + } + + const placeholder = "# not written yet\n" + corefilePath := filepath.Join(t.TempDir(), "Corefile") + if err := os.WriteFile(corefilePath, []byte(placeholder), 0644); err != nil { + t.Fatalf("failed to write initial Corefile: %v", err) + } + + reloader := &mockConfigReloader{} + controller := &Controller{ + Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(routerSvc, dnsSvc).Build(), + Interval: 1 * time.Second, + CorefilePath: corefilePath, + Reloader: reloader, + } + + ctx := context.Background() + if err := controller.reconcile(ctx); err != nil { + t.Fatalf("reconcile failed: %v", err) + } + + corefileBytes, err := os.ReadFile(corefilePath) + if err != nil { + t.Fatalf("failed to read Corefile: %v", err) + } + got := string(corefileBytes) + + if tc.wantNoAnswer { + if got != placeholder { + t.Errorf("reconcile() rewrote the Corefile for a Service with no usable ClusterIP; want it left alone\nGot:\n%s", got) + } + if reloader.reloads != 0 { + t.Errorf("reconcile() reloaded CoreDNS %d times for a Service with no usable ClusterIP, want 0", reloader.reloads) + } + return + } + + for _, want := range tc.wantAnswers { + if !strings.Contains(got, want) { + t.Errorf("reconcile() wrote a Corefile missing %q\nGot:\n%s", want, got) + } + } + // Exactly the expected answers and no others: an address template for + // a family the Service does not have would publish an unreachable + // address, and on the A side would not even parse as an RR. + if answers := strings.Count(got, `answer "`); answers != len(tc.wantAnswers) { + t.Errorf("reconcile() wrote %d answer directives, want %d\nGot:\n%s", answers, len(tc.wantAnswers), got) + } + if reloader.reloads != 1 { + t.Errorf("reconcile() reloaded CoreDNS %d times, want 1", reloader.reloads) + } + + // A second pass must be a no-op. The controller reconciles on a + // ticker, so anything unstable in the render -- a timestamp, most + // easily -- would rewrite the file and signal CoreDNS every interval. + if err := controller.reconcile(ctx); err != nil { + t.Fatalf("second reconcile failed: %v", err) + } + if reloader.reloads != 1 { + t.Errorf("second reconcile() reloaded CoreDNS again (%d total), want it to recognise the Corefile as up to date", reloader.reloads) + } + }) + } +} + func TestReconcileKubeDNSNotFound(t *testing.T) { scheme := runtime.NewScheme() _ = corev1.AddToScheme(scheme) diff --git a/internal/ipfamily/ipfamily.go b/internal/ipfamily/ipfamily.go new file mode 100644 index 0000000000..a6a3641087 --- /dev/null +++ b/internal/ipfamily/ipfamily.go @@ -0,0 +1,60 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package ipfamily sorts Kubernetes addresses into their IP families. +package ipfamily + +import ( + "net/netip" + + corev1 "k8s.io/api/core/v1" +) + +// ClusterIPsByFamily splits a Service's cluster IPs into its IPv4 and IPv6 +// entries, returning "" for a family the Service does not have. +// +// A Service with no ipFamilyPolicy is SingleStack, so even on a dual-stack +// cluster it has exactly one ClusterIP and one of the two return values is +// empty. Callers must handle that: it is the steady state everywhere +// ipFamilyPolicy has not been set, and it is what distinguishes "this cluster +// has no address to offer in that family" from a misconfiguration. +// +// Spec.ClusterIPs is preferred over the singular Spec.ClusterIP, with a +// fallback to the latter because a Service built by hand (or by a fake client) +// may only set the scalar. Headless Services, and any entry that is not a +// parseable address, are skipped rather than returned. +func ClusterIPsByFamily(svc *corev1.Service) (v4, v6 string) { + ips := svc.Spec.ClusterIPs + if len(ips) == 0 && svc.Spec.ClusterIP != "" { + ips = []string{svc.Spec.ClusterIP} + } + for _, ip := range ips { + if ip == "" || ip == corev1.ClusterIPNone { + continue + } + // netip rather than net.IP: net.IP.To4 returns non-nil for a v4-mapped + // v6 address and would misfile it as IPv4. + addr, err := netip.ParseAddr(ip) + if err != nil { + continue + } + switch { + case addr.Is4() && v4 == "": + v4 = ip + case addr.Is6() && !addr.Is4In6() && v6 == "": + v6 = ip + } + } + return v4, v6 +} diff --git a/internal/ipfamily/ipfamily_test.go b/internal/ipfamily/ipfamily_test.go new file mode 100644 index 0000000000..9b60972534 --- /dev/null +++ b/internal/ipfamily/ipfamily_test.go @@ -0,0 +1,95 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ipfamily + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" +) + +func TestClusterIPsByFamily(t *testing.T) { + tests := []struct { + name string + spec corev1.ServiceSpec + wantV4 string + wantV6 string + wantReason string + }{ + { + name: "single stack IPv4", + spec: corev1.ServiceSpec{ClusterIP: "10.96.0.10", ClusterIPs: []string{"10.96.0.10"}}, + wantV4: "10.96.0.10", + wantReason: "the default policy on an IPv4 cluster", + }, + { + name: "single stack IPv6", + spec: corev1.ServiceSpec{ClusterIP: "fd00:10:96::8857", ClusterIPs: []string{"fd00:10:96::8857"}}, + wantV6: "fd00:10:96::8857", + wantReason: "an IPv6-only cluster allocates a v6 ClusterIP with no ipFamilyPolicy set", + }, + { + name: "dual stack IPv4 primary", + spec: corev1.ServiceSpec{ClusterIP: "10.96.0.10", ClusterIPs: []string{"10.96.0.10", "fd00:10:96::8857"}}, + wantV4: "10.96.0.10", + wantV6: "fd00:10:96::8857", + wantReason: "both families are usable regardless of which one is primary", + }, + { + name: "dual stack IPv6 primary", + spec: corev1.ServiceSpec{ClusterIP: "fd00:10:96::8857", ClusterIPs: []string{"fd00:10:96::8857", "10.96.0.10"}}, + wantV4: "10.96.0.10", + wantV6: "fd00:10:96::8857", + wantReason: "the order of ClusterIPs is the family preference, not a family label", + }, + { + name: "scalar only", + spec: corev1.ServiceSpec{ClusterIP: "10.96.0.10"}, + wantV4: "10.96.0.10", + wantReason: "a hand-built Service may set only the singular field", + }, + { + name: "headless", + spec: corev1.ServiceSpec{ClusterIP: corev1.ClusterIPNone, ClusterIPs: []string{corev1.ClusterIPNone}}, + wantReason: "None is a sentinel, not an address", + }, + { + name: "not yet allocated", + spec: corev1.ServiceSpec{}, + wantReason: "a Service observed before the allocator has run has neither", + }, + { + name: "v4-mapped v6 belongs to neither family", + spec: corev1.ServiceSpec{ClusterIPs: []string{"::ffff:10.96.0.10"}}, + wantReason: "net.IP.To4 would misfile this as IPv4; kube never allocates one, so dropping it beats guessing", + }, + { + name: "unparseable entries are skipped", + spec: corev1.ServiceSpec{ClusterIPs: []string{"not-an-ip", "10.96.0.10"}}, + wantV4: "10.96.0.10", + wantReason: "a junk entry must not shadow a usable one", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := &corev1.Service{Spec: tc.spec} + v4, v6 := ClusterIPsByFamily(svc) + if v4 != tc.wantV4 || v6 != tc.wantV6 { + t.Errorf("ClusterIPsByFamily(%+v) = (%q, %q), want (%q, %q): %s", tc.spec, v4, v6, tc.wantV4, tc.wantV6, tc.wantReason) + } + }) + } +}