From 8eaf2423bd999c9a6cc406b5c6b556c08ca0269d Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 07:39:50 -0700 Subject: [PATCH 1/2] 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 a134e829a..e89b06b62 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 0b301e7e2..2869e3a24 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 f13429e47..c8653ad7e 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 d543dd5a2765b9accd231c00911c62bb29bea7fc Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 07:28:36 -0700 Subject: [PATCH 2/2] atenet/dns: manual CoreDNS rcode harness (test signals only) Not intended to merge. The unit test in #874 pins the rendered zone but nothing exercises CoreDNS, so this serves that zone with the pinned coredns/coredns:1.11.1 and checks the rcode returned for A, AAAA, HTTPS, SRV, a name in the zone that is not an actor, and a malformed one, plus an Alpine getent for the musl path. Pointing --corefile at a zone rendered before the fix turns the run into a negative control: the AAAA case SERVFAILs and getent stops resolving, while plain dig A still succeeds. --- cmd/atenet/internal/dns/README.md | 3 + cmd/atenet/internal/dns/corefile_test.go | 20 +++ hack/dns-manual-test.sh | 166 +++++++++++++++++++++++ 3 files changed, 189 insertions(+) create mode 100755 hack/dns-manual-test.sh diff --git a/cmd/atenet/internal/dns/README.md b/cmd/atenet/internal/dns/README.md index e89b06b62..ba92fbd31 100644 --- a/cmd/atenet/internal/dns/README.md +++ b/cmd/atenet/internal/dns/README.md @@ -46,6 +46,9 @@ 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. +`hack/dns-manual-test.sh` serves this zone with the pinned CoreDNS image and +checks the rcode for each query type, including an Alpine `getent`. + ## Integration * CoreDNS: Update CoreDNS ConfigMap to add the stub resolver. diff --git a/cmd/atenet/internal/dns/corefile_test.go b/cmd/atenet/internal/dns/corefile_test.go index c8653ad7e..2dd7e50d8 100644 --- a/cmd/atenet/internal/dns/corefile_test.go +++ b/cmd/atenet/internal/dns/corefile_test.go @@ -16,6 +16,8 @@ package dns import ( "fmt" + "os" + "path/filepath" "strings" "testing" ) @@ -76,3 +78,21 @@ func TestMakeCoreFile(t *testing.T) { }) } } + +// TestDumpCorefile writes the rendered zone to $COREFILE_DUMP_DIR so +// hack/dns-manual-test.sh serves the real thing rather than a copy. Skipped +// unless that variable is set. +func TestDumpCorefile(t *testing.T) { + dir := os.Getenv("COREFILE_DUMP_DIR") + if dir == "" { + t.Skip("COREFILE_DUMP_DIR is unset") + } + routerIP := os.Getenv("COREFILE_DUMP_ROUTER_IP") + if routerIP == "" { + routerIP = "10.240.0.10" + } + path := filepath.Join(dir, "Corefile") + if err := os.WriteFile(path, []byte(makeCoreFile(routerIP)), 0o600); err != nil { + t.Fatalf("os.WriteFile(%q) = %v", path, err) + } +} diff --git a/hack/dns-manual-test.sh b/hack/dns-manual-test.sh new file mode 100755 index 000000000..540939259 --- /dev/null +++ b/hack/dns-manual-test.sh @@ -0,0 +1,166 @@ +#!/usr/bin/env bash + +# 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. + +# Serves the rendered actor DNS zone with the pinned CoreDNS image and checks +# the response code CoreDNS returns for each query type. The unit tests pin +# what we generate; this checks what CoreDNS does with it. + +set -o errexit -o nounset -o pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +COREDNS_IMAGE="coredns/coredns:1.11.1" +ALPINE_IMAGE="alpine:3.19" +BUSYBOX_IMAGE="busybox:1" + +ROUTER_IP="10.240.0.10" +SUFFIX="actors.resources.substrate.ate.dev" +ACTOR="demo.default.${SUFFIX}" + +NET="ate-dns-manual" +SERVER="ate-dns-manual-server" +CLIENT="ate-dns-manual-client" +VOLUME="ate-dns-manual-conf" + +corefile="" +tmpdir="" + +usage() { + cat <&2; usage >&2; exit 1 ;; + esac +done + +read -ra docker_cmd <<< "${DOCKER:-docker}" +dk() { "${docker_cmd[@]}" "$@"; } + +remove_containers() { + dk rm -f "$SERVER" "$CLIENT" >/dev/null 2>&1 || true + dk volume rm "$VOLUME" >/dev/null 2>&1 || true + dk network rm "$NET" >/dev/null 2>&1 || true +} +cleanup() { + remove_containers + [[ -n "$tmpdir" ]] && rm -rf "$tmpdir" +} +trap cleanup EXIT + +if ! dk version >/dev/null 2>&1; then + echo "cannot reach a Docker daemon; see --help" >&2 + exit 1 +fi + +if [[ -z "$corefile" ]]; then + tmpdir="$(mktemp -d)" + ( cd "$ROOT" && COREFILE_DUMP_DIR="$tmpdir" COREFILE_DUMP_ROUTER_IP="$ROUTER_IP" \ + go test ./cmd/atenet/internal/dns/... -run TestDumpCorefile -count=1 >/dev/null ) + corefile="$tmpdir/Corefile" +fi +echo "Serving ${corefile}:" +sed 's/^/ /' "$corefile" +echo + +remove_containers +dk network create "$NET" >/dev/null +dk volume create "$VOLUME" >/dev/null +# Stream the Corefile into a volume rather than bind-mounting it: the daemon may +# be in a VM that cannot see this path. +dk run -i --rm -v "$VOLUME":/out "$BUSYBOX_IMAGE" sh -c 'cat > /out/Corefile' < "$corefile" +dk run -d --name "$SERVER" --network "$NET" -v "$VOLUME":/etc/coredns \ + "$COREDNS_IMAGE" -conf /etc/coredns/Corefile >/dev/null +dk run -d --name "$CLIENT" --network "$NET" "$ALPINE_IMAGE" sleep 600 >/dev/null +dk exec "$CLIENT" apk add --no-cache bind-tools >/dev/null 2>&1 + +for _ in $(seq 20); do + if dk exec "$CLIENT" dig +time=1 +tries=1 "@${SERVER}" -q "$ACTOR" -t A >/dev/null 2>&1; then + break + fi + sleep 0.5 +done + +failures=0 + +# check LABEL NAME QTYPE WANT_STATUS WANT_ANSWERS WANT_AUTHORITY +check() { + local label="$1" name="$2" qtype="$3" want_status="$4" want_answers="$5" want_authority="$6" + local out status answers authority + # -q/-t, not positional: a malformed name starting with "-" is otherwise + # parsed as a dig flag. + out="$(dk exec "$CLIENT" dig +noall +comment "@${SERVER}" -q "$name" -t "$qtype" 2>&1 || true)" + status="$(sed -n 's/.*status: \([A-Z]*\).*/\1/p' <<<"$out" | head -1)" + answers="$(sed -n 's/.*ANSWER: \([0-9]*\).*/\1/p' <<<"$out" | head -1)" + authority="$(sed -n 's/.*AUTHORITY: \([0-9]*\).*/\1/p' <<<"$out" | head -1)" + if [[ "$status" == "$want_status" && "$answers" == "$want_answers" && "$authority" == "$want_authority" ]]; then + printf 'PASS %-34s %-8s %s answers=%s authority=%s\n' "$label" "$qtype" "$status" "$answers" "$authority" + else + printf 'FAIL %-34s %-8s got %s answers=%s authority=%s, want %s answers=%s authority=%s\n' \ + "$label" "$qtype" "${status:-none}" "${answers:-none}" "${authority:-none}" \ + "$want_status" "$want_answers" "$want_authority" + failures=$((failures + 1)) + fi +} + +# A already worked; it is here so the NODATA blocks cannot silently break it. +check "valid actor" "$ACTOR" A NOERROR 1 0 +# The fix: an empty answer with an SOA, not SERVFAIL. +check "valid actor" "$ACTOR" AAAA NOERROR 0 1 +check "valid actor" "$ACTOR" HTTPS NOERROR 0 1 +check "valid actor" "$ACTOR" SRV NOERROR 0 1 +check "name in zone, no actor" "nope.${SUFFIX}" A NXDOMAIN 0 1 +check "malformed actor name" "-bad.default.${SUFFIX}" A NXDOMAIN 0 1 + +got_a="$(dk exec "$CLIENT" dig +short "@${SERVER}" -q "$ACTOR" -t A 2>&1 | tr -d '\r')" +if [[ "$got_a" == "$ROUTER_IP" ]]; then + printf 'PASS %-34s %-8s %s\n' "A record points at the router" "A" "$got_a" +else + printf 'FAIL %-34s %-8s got %q, want %q\n' "A record points at the router" "A" "$got_a" "$ROUTER_IP" + failures=$((failures + 1)) +fi + +# The user-visible bug: musl resolves A and AAAA in parallel and fails the pair +# when either half errors, so this is what SERVFAIL actually costs. +server_ip="$(dk inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$SERVER")" +if dk run --rm --network "$NET" --dns "$server_ip" "$ALPINE_IMAGE" \ + getent ahosts "$ACTOR" 2>/dev/null | grep -q "$ROUTER_IP"; then + printf 'PASS %-34s %-8s resolved %s\n' "alpine getent ahosts" "musl" "$ROUTER_IP" +else + printf 'FAIL %-34s %-8s did not resolve\n' "alpine getent ahosts" "musl" + failures=$((failures + 1)) +fi + +echo +if (( failures > 0 )); then + echo "${failures} check(s) failed" + exit 1 +fi +echo "all checks passed"