From 9055f6f4d2cc1ca022f6b2782773f1240b341cd5 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 07:39:50 -0700 Subject: [PATCH 1/4] atenet/dns: answer non-A actor queries instead of SERVFAIL Before, the actor zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor. A failure reads as a temporary error rather than an answer, so clients retry it and then give up on the name; Alpine actors could not resolve each other at all, even on an IPv4-only cluster. After, those queries return a correct empty answer, and one that resolvers can cache. A unit test pins the whole rendered zone as a literal, so editing the name pattern or the suffix fails there rather than passing silently. --- cmd/atenet/internal/dns/README.md | 22 ++++++- cmd/atenet/internal/dns/corefile.go | 25 +++++++- cmd/atenet/internal/dns/corefile_test.go | 75 ++++++++++++++---------- 3 files changed, 87 insertions(+), 35 deletions(-) diff --git a/cmd/atenet/internal/dns/README.md b/cmd/atenet/internal/dns/README.md index a134e829ab..e89b06b62f 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,33 @@ 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, rendered by `corefile.go`: ``` -# 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 " + fallthrough + } +# NODATA for a well-formed actor name on any other qtype (AAAA, HTTPS, SRV, ...). + 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)" } ``` +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. + ## 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..2869e3a24d 100644 --- a/cmd/atenet/internal/dns/corefile.go +++ b/cmd/atenet/internal/dns/corefile.go @@ -30,6 +30,11 @@ func init() { } func buildTemplate() string { + const ( + fallthroughDirective = " fallthrough" + soaDirective = ` authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)"` + ) + // Build up the corefileTemplate programmatically to make it easier to understand. var directives []string // Plugins to enable. @@ -44,9 +49,27 @@ func buildTemplate() string { 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)) + actorMatch := fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix) + directives = append(directives, actorMatch) // Note the %s -- this will be filled with the router IP. directives = append(directives, ` answer "{{ .Name }} 60 IN A %s"`) + directives = append(directives, fallthroughDirective) + directives = append(directives, "}") + + // Valid actor names return NOERROR (NODATA) for non-A queries. + 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. diff --git a/cmd/atenet/internal/dns/corefile_test.go b/cmd/atenet/internal/dns/corefile_test.go index f13429e475..c8653ad7ef 100644 --- a/cmd/atenet/internal/dns/corefile_test.go +++ b/cmd/atenet/internal/dns/corefile_test.go @@ -15,50 +15,63 @@ package dns import ( + "fmt" "strings" "testing" - - "github.com/agent-substrate/substrate/internal/resources" ) +// 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 wantCorefileFmt = `actors.resources.substrate.ate.dev:53 { + log + errors + health :8080 + ready :8181 + reload + 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 %s" + fallthrough + } + 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 + } + 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)" + } +} +` + +// 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 }{ - { - 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"`, - }, - }, - { - 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: "cluster IP", routerIP: "10.240.0.10"}, + {name: "different cluster IP", routerIP: "192.168.1.1"}, } 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.routerIP)) + want := fmt.Sprintf(wantCorefileFmt, tc.routerIP) + if got != want { + t.Errorf("makeCoreFile(%q) rendered an unexpected Corefile\nGot:\n%s\nWant:\n%s", tc.routerIP, got, want) } }) } From 44a965e312f74ab3b67d702865b14e56c32297e6 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Aug 2026 12:20:18 -0700 Subject: [PATCH 2/4] e2e: add an in-cluster DNS client and router IP family helpers Nothing in the e2e harness could query the actor DNS zone. Suites reach actors by port-forwarding atenet-router and passing the actor name as a Host header, so the zone CoreDNS actually serves went unasserted, and a suite that wanted to check it had no way to distinguish an empty answer from a server failure. Adds a DNS client that port-forwards the atenet DNS Service and reports the rcode class alongside the addresses, plus a helper for the router's ClusterIP in each family. The tests that use these follow. clusterIPsByFamily here is a stopgap that #938 replaces with internal/ipfamily. --- internal/e2e/dns_client.go | 162 ++++++++++++++++++++++++++++++++++ internal/e2e/ipfamily.go | 69 +++++++++++++++ internal/e2e/router_client.go | 11 ++- internal/e2e/statusz.go | 2 +- 4 files changed, 239 insertions(+), 5 deletions(-) create mode 100644 internal/e2e/dns_client.go create mode 100644 internal/e2e/ipfamily.go diff --git a/internal/e2e/dns_client.go b/internal/e2e/dns_client.go new file mode 100644 index 0000000000..76db25d4f6 --- /dev/null +++ b/internal/e2e/dns_client.go @@ -0,0 +1,162 @@ +// 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 e2e + +import ( + "context" + "errors" + "fmt" + "net" + "strconv" + "strings" + "time" + + "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/portforward" + "k8s.io/client-go/kubernetes" +) + +const ( + dnsNamespace = "ate-system" + dnsService = "dns" + // dnsServicePort is the Service port; the Service exposes 53 twice, once + // UDP and once TCP, and the port-forward tunnel is TCP either way. + dnsServicePort = 53 +) + +// DNSRcode is how the server answered, at the granularity net.Resolver exposes. +type DNSRcode int + +const ( + // DNSAnswered is NOERROR with at least one address of the queried family. + DNSAnswered DNSRcode = iota + // DNSEmpty is "this name has no address in this family": NODATA (NOERROR + // with an empty answer section) or NXDOMAIN. net.Resolver reports both as + // DNSError.IsNotFound and the standard library offers no way to tell them + // apart, which is fine here — both are benign to every stub resolver, and + // that benign-ness is the property under test. + DNSEmpty + // DNSFailed is SERVFAIL, REFUSED, a timeout, or a malformed reply: anything + // net.Resolver classifies as the server misbehaving. No query into the actor + // zone should ever produce it — not a non-A qtype, not a name that fails the + // actor regex — and that is the regression these tests exist to catch. + DNSFailed +) + +func (r DNSRcode) String() string { + switch r { + case DNSAnswered: + return "answered" + case DNSEmpty: + return "no-such-host (NODATA or NXDOMAIN)" + case DNSFailed: + return "server failure (SERVFAIL/REFUSED/timeout)" + default: + return "unknown" + } +} + +// DNSClient resolves names against the ate-system/dns CoreDNS Service over a +// port-forward. +// +// Querying that Service directly, rather than going through the cluster's own +// resolver, is deliberate: the delegation that would make actor names resolvable +// cluster-wide is a patch to the kube-system/kube-dns ConfigMap, which only +// exists on GKE (cmd/atenet/internal/dns/dns.go reconcileKubeDNSConfig hits the +// IsNotFound branch on kind and upstream Kubernetes). Pointing at the Service is +// the only way to assert the zone's behavior on every cluster we test on. +type DNSClient struct { + resolver *net.Resolver + stop func() +} + +// NewDNSClient establishes a port-forward to the atenet DNS Service. Call Close +// to tear it down. +func NewDNSClient(ctx context.Context) (*DNSClient, error) { + config, err := ateclient.LoadConfig(KubeConfig, KubeContext) + if err != nil { + return nil, fmt.Errorf("loading kubeconfig: %w", err) + } + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return nil, fmt.Errorf("creating k8s client: %w", err) + } + + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, dnsNamespace, dnsService, dnsServicePort) + if err != nil { + return nil, err + } + addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(localPort)) + + return &DNSClient{ + stop: stop, + resolver: &net.Resolver{ + // PreferGo keeps us on Go's own resolver on every platform. cgo's + // would ignore Dial entirely and query the host's nameservers. + PreferGo: true, + // Surface a per-family failure instead of hiding it behind the + // other family's success. + StrictErrors: true, + Dial: func(ctx context.Context, _, _ string) (net.Conn, error) { + // The port-forward is a TCP tunnel, so every query goes over TCP + // whatever network the resolver asked for. net.Resolver selects + // stream framing for any conn that is not a net.PacketConn, so + // returning a TCP conn here is transparent to it. The requested + // server address is ignored: there is exactly one server. + var d net.Dialer + return d.DialContext(ctx, "tcp", addr) + }, + }, + }, nil +} + +// Close tears down the port-forward. +func (c *DNSClient) Close() { + if c.stop != nil { + c.stop() + } +} + +// Lookup resolves name in a single address family — network is "ip4" for an A +// query or "ip6" for a AAAA query — and reports the addresses alongside how the +// server answered. A DNSFailed result is returned with the underlying error for +// the failure message; DNSEmpty is returned with a nil error because it is a +// valid answer, not a fault. +func (c *DNSClient) Lookup(ctx context.Context, network, name string) ([]string, DNSRcode, error) { + // Root the name so the resolver skips the host's search list and ndots + // handling, which would otherwise make the query depend on where the test + // runs. + if !strings.HasSuffix(name, ".") { + name += "." + } + + lookupCtx, cancel := context.WithTimeout(ctx, 15*time.Second) + defer cancel() + + addrs, err := c.resolver.LookupNetIP(lookupCtx, network, name) + if err == nil { + ips := make([]string, 0, len(addrs)) + for _, a := range addrs { + ips = append(ips, a.Unmap().String()) + } + return ips, DNSAnswered, nil + } + + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) && dnsErr.IsNotFound { + return nil, DNSEmpty, nil + } + return nil, DNSFailed, fmt.Errorf("%s query for %q: %w", network, name, err) +} diff --git a/internal/e2e/ipfamily.go b/internal/e2e/ipfamily.go new file mode 100644 index 0000000000..3d72fcc040 --- /dev/null +++ b/internal/e2e/ipfamily.go @@ -0,0 +1,69 @@ +// 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 e2e + +import ( + "context" + "fmt" + "net/netip" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/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 on a dual-stack cluster it still has +// exactly one ClusterIP and one of the two return values is empty — which is +// what makes this the right thing to gate a dual-stack assertion on. +// +// Spec.ClusterIPs is preferred over the singular Spec.ClusterIP, with a +// fallback for the latter because a Service object built by hand (or by a fake +// client) may only set the scalar. +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 +} + +// RouterClusterIPs returns the atenet-router Service's IPv4 and IPv6 +// ClusterIPs. Either may be "". +func RouterClusterIPs(ctx context.Context) (v4, v6 string, err error) { + svc, err := GetClients().K8s.CoreV1().Services(RouterNamespace).Get(ctx, RouterService, metav1.GetOptions{}) + if err != nil { + return "", "", fmt.Errorf("getting Service %s/%s: %w", RouterNamespace, RouterService, err) + } + v4, v6 = clusterIPsByFamily(svc) + return v4, v6, nil +} diff --git a/internal/e2e/router_client.go b/internal/e2e/router_client.go index 4a0c006b22..7992ba036b 100644 --- a/internal/e2e/router_client.go +++ b/internal/e2e/router_client.go @@ -36,8 +36,11 @@ import ( ) const ( - routerNamespace = "ate-system" - routerService = "atenet-router" + // RouterNamespace and RouterService locate the atenet router. Exported so + // that suites addressing the same Service or its pods do not have to + // redeclare them. + RouterNamespace = "ate-system" + RouterService = "atenet-router" // routerConnectServicePort is atenet-router's Service port for // CONNECT-tunneled traffic (see manifests/ate-install/atenet-router.yaml). // It is a distinct listener from the plain HTTP one Get/PostJSON use: @@ -79,7 +82,7 @@ func NewRouterClient(ctx context.Context) (*RouterClient, error) { return nil, fmt.Errorf("creating k8s client: %w", err) } - localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, routerNamespace, routerService, 80) + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, RouterNamespace, RouterService, 80) if err != nil { return nil, err } @@ -183,7 +186,7 @@ func (c *RouterClient) Connect(ctx context.Context, actorRef resources.ActorRef, // in one test don't each pay for a fresh port-forward. func (c *RouterClient) ensureConnectPortForward(ctx context.Context) error { c.connectOnce.Do(func() { - localPort, stop, err := portforward.ServicePortForward(ctx, c.config, c.clientset, routerNamespace, routerService, routerConnectServicePort) + localPort, stop, err := portforward.ServicePortForward(ctx, c.config, c.clientset, RouterNamespace, RouterService, routerConnectServicePort) if err != nil { c.connectErr = fmt.Errorf("port-forwarding to the router's CONNECT listener: %w", err) return diff --git a/internal/e2e/statusz.go b/internal/e2e/statusz.go index a04a07141d..1890a036fe 100644 --- a/internal/e2e/statusz.go +++ b/internal/e2e/statusz.go @@ -51,7 +51,7 @@ func NewStatuszClient(ctx context.Context) (*StatuszClient, error) { return nil, fmt.Errorf("creating k8s client: %w", err) } - localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, routerNamespace, routerService, routerStatusPort) + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, RouterNamespace, RouterService, routerStatusPort) if err != nil { return nil, err } From 337ee749754f10f4d2d544f481722252823e03c5 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Aug 2026 12:20:38 -0700 Subject: [PATCH 3/4] e2e: assert the actor DNS zone answers benign rcodes The zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor -- and no test caught it, because Go's resolver masks a SERVFAIL that musl treats as fatal. These assert the rcode class rather than the record: a non-A qtype and a name that misses the actor regex must come back NODATA or NXDOMAIN, and an A query must carry the router's ClusterIP. Both rcode assertions fail on a tree without the first commit here and pass with it, measured on a single-stack IPv4 kind cluster. Part of #246. --- internal/e2e/suites/networking/dns_test.go | 129 +++++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 internal/e2e/suites/networking/dns_test.go diff --git a/internal/e2e/suites/networking/dns_test.go b/internal/e2e/suites/networking/dns_test.go new file mode 100644 index 0000000000..a9b0ce93cb --- /dev/null +++ b/internal/e2e/suites/networking/dns_test.go @@ -0,0 +1,129 @@ +// 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 networking + +import ( + "context" + "slices" + "testing" + + "github.com/agent-substrate/substrate/internal/e2e" + "github.com/agent-substrate/substrate/internal/resources" +) + +// The actor zone is served by a CoreDNS `template` block, which answers for any +// name matching .. whether or not that actor exists. +// These tests therefore need no actor fixture — they are asserting the zone's +// behavior, not an actor's. +func probeActorDNSName() string { + return resources.ActorDNSName(resources.ActorRef{Atespace: networkingAtespace, Name: "dns-probe"}) +} + +func mustDNSClient(t *testing.T, ctx context.Context) *e2e.DNSClient { + t.Helper() + dns, err := e2e.NewDNSClient(ctx) + if err != nil { + t.Fatalf("NewDNSClient: %v", err) + } + t.Cleanup(dns.Close) + return dns +} + +// TestActorDNSZone asserts that the actor zone answers an A query with the +// router's ClusterIP, and — the part no other test covers — that everything +// else it is asked returns a *benign* rcode rather than SERVFAIL. +// +// The rcode matters more than the missing record. NODATA and NXDOMAIN are what +// every stub resolver expects for "there is no address here"; SERVFAIL is a +// transport fault, and resolvers disagree about it. musl maps it to EAI_AGAIN +// and abandons the whole getaddrinfo — and because musl issues the A and AAAA +// queries in parallel, one SERVFAIL sinks the other with it, so an Alpine-based +// client cannot resolve an actor name at all, not even its A record. glibc +// retries and pays the resolver timeout instead. And unlike NODATA and +// NXDOMAIN, SERVFAIL carries no SOA to cache negatively against, so every +// request re-pays that cost. Go's resolver masks all of this, which is why no +// test in this repo caught it before these. +// +// The zone gets the rcodes right with three `template` blocks in +// cmd/atenet/internal/dns/corefile.go: the `IN A` block that answers actor +// names, a regex-matched `template ANY ANY` returning NOERROR plus an SOA +// authority (NODATA) for other qtypes on a well-formed actor name, and a +// terminal `template ANY ANY` returning NXDOMAIN plus an SOA authority for +// everything else in the zone. The first two carry a bare `fallthrough`, which +// is load-bearing: the plugin walks past a class or qtype mismatch by itself, +// but a *regex* miss returns SERVFAIL immediately unless the block declares it. +// The two subtests below are what keeps that from being collapsed back into a +// single block. +// +// This test is family-agnostic and is expected to run, not skip, on a +// single-stack cluster. +func TestActorDNSZone(t *testing.T) { + ctx := context.Background() + dns := mustDNSClient(t, ctx) + + routerV4, _, err := e2e.RouterClusterIPs(ctx) + if err != nil { + t.Fatalf("reading atenet-router ClusterIPs: %v", err) + } + + name := probeActorDNSName() + + t.Run("A answers with the router ClusterIP", func(t *testing.T) { + if routerV4 == "" { + // A v6-only cluster: there is no IPv4 ClusterIP to answer with, and + // emitting an A record at all would be the bug. + t.Skip("atenet-router has no IPv4 ClusterIP") + } + addrs, rcode, err := dns.Lookup(ctx, "ip4", name) + if rcode != e2e.DNSAnswered { + t.Fatalf("A %s: %v (%v); want the router ClusterIP %s", name, rcode, err, routerV4) + } + if !slices.Contains(addrs, routerV4) { + t.Fatalf("A %s = %v; want it to contain the atenet-router ClusterIP %s", name, addrs, routerV4) + } + }) + + t.Run("AAAA is not a server failure", func(t *testing.T) { + // The name is well-formed, so NODATA is the answer owed here on a + // single-stack cluster: it exists, it just has no address in this + // family. That needs a block that matches the qtype. Were the `IN A` + // template the zone's only one, a qtype mismatch would fall through to + // a plugin chain with nothing after it, and plugin.NextOrFailure with a + // nil Next returns SERVFAIL. + _, rcode, err := dns.Lookup(ctx, "ip6", name) + if rcode == e2e.DNSFailed { + t.Fatalf("AAAA %s: %v (%v); want NODATA. A SERVFAIL on a non-A qtype in this "+ + "zone breaks musl-based clients on IPv4-only clusters too, because it takes "+ + "their parallel A query down with it", name, rcode, err) + } + }) + + t.Run("a name outside the actor pattern is not a server failure", func(t *testing.T) { + // A single-label name inside the zone: the zone matches, the qtype + // matches, the actor regex does not. This is the case that depends on + // both halves of the corefile fix at once. A regex miss is the one kind + // of non-match the template plugin does not walk past on its own -- it + // consults fall.Through() and, absent a bare `fallthrough`, answers + // SERVFAIL without evaluating any later block. So the two regex-matched + // templates each need `fallthrough` to decline the name, and the + // terminal catch-all `template ANY ANY` is what turns it into NXDOMAIN. + // Drop either piece and this subtest goes red. + bogus := "not-an-actor." + resources.ActorDNSSuffix + _, rcode, err := dns.Lookup(ctx, "ip4", bogus) + if rcode == e2e.DNSFailed { + t.Fatalf("A %s: %v (%v); want NXDOMAIN", bogus, rcode, err) + } + }) +} From 0c5c6a87aaa3cadf9a3e9d59c885ae28066cd710 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Aug 2026 17:53:43 -0700 Subject: [PATCH 4/4] e2e: assert the actor zone publishes the router's AAAA The rcode assertions in the previous commit prove the zone stops failing an AAAA query, not that it ever answers one. Nothing checks that the record the zone does publish is the router's IPv6 ClusterIP, so #938 could regress to an empty answer and every existing test would still be green. Kept separate from TestActorDNSZone because it is the only assertion here whose expected result changes with the cluster: it skips wherever atenet-router has a single ClusterIP, which is every cluster until #911 gives the Service a dual-stack policy. Part of #246. --- internal/e2e/suites/networking/dns_test.go | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/internal/e2e/suites/networking/dns_test.go b/internal/e2e/suites/networking/dns_test.go index a9b0ce93cb..b8abf001c2 100644 --- a/internal/e2e/suites/networking/dns_test.go +++ b/internal/e2e/suites/networking/dns_test.go @@ -127,3 +127,37 @@ func TestActorDNSZone(t *testing.T) { } }) } + +// TestActorDNSAAAA asserts the zone publishes the router's IPv6 ClusterIP. +// +// Skipped unless the atenet-router Service actually has one, which is the +// steady state on every single-stack cluster: a Service with no +// ipFamilyPolicy is SingleStack and never gets a second ClusterIP, so there is +// nothing an AAAA could correctly point at. +// +// Deliberately separate from TestActorDNSZone: it is the only assertion here +// whose expected result changes when the cluster becomes dual-stack, so keeping +// it its own function lets a dual-stack CI job exclude it while the AAAA +// generator is still in flight. +func TestActorDNSAAAA(t *testing.T) { + ctx := context.Background() + + _, routerV6, err := e2e.RouterClusterIPs(ctx) + if err != nil { + t.Fatalf("reading atenet-router ClusterIPs: %v", err) + } + if routerV6 == "" { + t.Skip("atenet-router has no IPv6 ClusterIP; single-stack cluster, nothing to publish") + } + + dns := mustDNSClient(t, ctx) + name := probeActorDNSName() + + addrs, rcode, err := dns.Lookup(ctx, "ip6", name) + if rcode != e2e.DNSAnswered { + t.Fatalf("AAAA %s: %v (%v); want the router IPv6 ClusterIP %s", name, rcode, err, routerV6) + } + if !slices.Contains(addrs, routerV6) { + t.Fatalf("AAAA %s = %v; want it to contain the atenet-router IPv6 ClusterIP %s", name, addrs, routerV6) + } +}