diff --git a/.github/workflows/e2e-ipv6.yaml b/.github/workflows/e2e-ipv6.yaml new file mode 100644 index 000000000..c7a0e9a51 --- /dev/null +++ b/.github/workflows/e2e-ipv6.yaml @@ -0,0 +1,366 @@ +# 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. + +name: e2e-ipv6 +# Separate from pr-workflow.yaml so this can be gated independently -- and so a +# 40-minute IPv6 run never delays that workflow's merge-gating jobs. +# +# TODO(246): gate this before merging. It runs on every PR today so the IPv6-only +# results are visible without a maintainer having to act first; the intended +# steady state is a `ci/ipv6` label, which needs the label created upstream: +# +# on: {pull_request: {types: [labeled, opened, synchronize, reopened]}} +# if: contains(github.event.pull_request.labels.*.name, 'ci/ipv6') +# +# Either way this job stays out of the `e2e-test` gate, so it never blocks a PR. +on: + pull_request: +permissions: + contents: read +jobs: + e2e-test-ipv6: + runs-on: ubuntu-latest + # Nothing else in this workflow sets a timeout, so jobs inherit GitHub's + # 6-hour default. A broken IPv6 cluster does not crash, it misses + # 10-minute ActorTemplate deadlines, so an uncapped job burns hours. + timeout-minutes: 40 + env: + # Non-default name so these steps can be replayed locally without + # touching an existing cluster. install-ate-kind.sh does not derive + # KUBECTL_CONTEXT from the cluster name the way run-e2e-kind.sh does, + # so both have to be set here. + KIND_CLUSTER_NAME: ate-ipv6 + KUBECTL_CONTEXT: kind-ate-ipv6 + # 8.8.8.8 reached through the well-known NAT64 prefix. CoreDNS is a + # v6-only pod on a runner with no IPv6 egress of its own, so this is the + # only shape of upstream resolver it can reach. See "Set up NAT64". + IPV6_DNS_UPSTREAM: 64:ff9b::808:808 + steps: + - name: Checkout + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 + - name: Setup Go + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 + with: + go-version-file: 'go.mod' + - name: Free disk space + # kind node image + control-plane images + snapshots are tight on the + # ~14GB runner disk even without the micro-VM assets. + run: | + sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /opt/hostedtoolcache/CodeQL + df -h / + - name: Enable IPv6 in the Docker daemon + # ubuntu-latest ships dockerd with IPv6 off, so kind would create its + # network v4-only and create-kind-cluster.sh would reject the cluster. + # Merge the two keys into whatever daemon.json the runner image ships + # rather than replacing the file. + run: | + sudo mkdir -p /etc/docker + [ -s /etc/docker/daemon.json ] || echo '{}' | sudo tee /etc/docker/daemon.json >/dev/null + sudo cat /etc/docker/daemon.json \ + | jq '. + {"ipv6": true, "ip6tables": true}' \ + | sudo tee /etc/docker/daemon.json.new >/dev/null + sudo mv /etc/docker/daemon.json.new /etc/docker/daemon.json + sudo systemctl restart docker + docker network inspect bridge --format 'bridge EnableIPv6={{.EnableIPv6}}' + - name: Set up NAT64 on the runner + # ubuntu-latest has no IPv6 egress whatsoever -- measured, not assumed: + # every curl -6 fails in ~2ms. A v6-only cluster still has to reach real + # v4 destinations (atelet fetches the gVisor tarball from GCS, + # TestActorEgress fetches example.com), so the runner translates for it. + # Ordered after the dockerd restart, which rebuilds the iptables chains + # these rules live in. + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq tayga dnsutils + # tayga answers to .1/::1; the tun holds .2/::2 so host-originated + # traffic is not sourced from tayga's own address, which is + # self-addressed rather than translatable. The pool avoids both. + sudo tee /etc/tayga.conf >/dev/null <<'EOF' + tun-device nat64 + ipv4-addr 192.168.255.1 + # tayga refuses the well-known prefix with an RFC1918 pool unless it + # also holds a v6 address of its own, outside that prefix. + ipv6-addr 2001:db8:64::1 + prefix 64:ff9b::/96 + dynamic-pool 192.168.255.128/25 + data-dir /var/spool/tayga + EOF + sudo mkdir -p /var/spool/tayga + sudo tayga --mktun + sudo ip link set nat64 up + sudo ip addr add 192.168.255.2/24 dev nat64 + sudo ip -6 addr add 2001:db8:64::2/128 dev nat64 + sudo ip -6 route add 64:ff9b::/96 dev nat64 src 2001:db8:64::2 + sudo sysctl -qw net.ipv4.ip_forward=1 + sudo sysctl -qw net.ipv6.conf.all.forwarding=1 + sudo iptables -t nat -A POSTROUTING -s 192.168.255.0/24 -j MASQUERADE + # Insert, not append: docker sets the FORWARD policy to DROP. + sudo iptables -I FORWARD 1 -i nat64 -j ACCEPT + sudo iptables -I FORWARD 1 -o nat64 -j ACCEPT + sudo ip6tables -I FORWARD 1 -i nat64 -j ACCEPT + sudo ip6tables -I FORWARD 1 -o nat64 -j ACCEPT + # -d keeps tayga in the foreground and logs every dropped packet with a + # reason; detaching hides exactly the failures worth diagnosing. + sudo sh -c 'nohup tayga -d --config /etc/tayga.conf >/tmp/tayga.log 2>&1 &' + sleep 3 + pgrep -a tayga || { + echo "::error::tayga is not running"; sudo cat /tmp/tayga.log; exit 1; + } + - name: Verify NAT64 before building anything on it + # Hard gate. The cluster takes ~4 minutes and every step after it depends + # on translation working, so a broken translator should fail here with + # one clear message rather than as a rollout timeout ten minutes later. + run: | + # Map a live A record rather than hardcoding one: example.com's old + # 93.184.216.34 is retired and would fail for the wrong reason. + v4=$(getent ahostsv4 storage.googleapis.com | awk 'NR==1{print $1}') + # shellcheck disable=SC2086 + set -- ${v4//./ } + v6=$(printf '64:ff9b::%02x%02x:%02x%02x' "$1" "$2" "$3" "$4") + echo "NAT64 maps ${v4} -> ${v6}" + dig +timeout=5 +tries=1 @"${IPV6_DNS_UPSTREAM}" storage.googleapis.com A +short + code=$(curl -6 -sS -m 15 -o /dev/null -w '%{http_code}' \ + --resolve "storage.googleapis.com:443:[${v6}]" \ + https://storage.googleapis.com/ || echo 000) + echo "NAT64 HTTPS probe returned ${code}" + case "${code}" in + # Any HTTP status proves the translator carried a TCP stream; GCS + # answers a bare / with 400. ICMP is separately blocked, so a ping + # test here would report a failure that does not matter. + 2*|3*|4*) ;; + *) echo "::error::NAT64 is not translating; the cluster cannot egress" + sudo cat /tmp/tayga.log || true + exit 1 ;; + esac + - name: Create cluster + env: + IP_FAMILY: ipv6 + run: hack/create-kind-cluster.sh + - name: Assert the cluster is single-stack IPv6 + # This job is worthless if the cluster is not actually v6-only, and a + # green run leaves no evidence either way -- the diagnostics dump below + # only runs on failure. A kind default change or an IP_FAMILY regression + # would otherwise turn this into a second IPv4 run that reports success. + # Checked here rather than later so it fails as itself. + # + # PreferDualStack Services resolving to a single clusterIP is the + # positive signal: on a dual-stack cluster they would get two. + run: | + k() { kubectl --context="$KUBECTL_CONTEXT" "$@"; } + pod_cidrs=$(k get nodes -o jsonpath='{.items[*].spec.podCIDRs[*]}') + svc_ips=$(k -n default get svc kubernetes -o jsonpath='{.spec.clusterIPs[*]}') + node_ips=$(k get nodes -o jsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') + for pair in "podCIDRs=${pod_cidrs}" "kubernetes.clusterIPs=${svc_ips}" "node.InternalIP=${node_ips}"; do + case "${pair#*=}" in + *.*) echo "::error::not single-stack IPv6 -- ${pair}"; exit 1 ;; + "") echo "::error::empty, cannot confirm IP family -- ${pair}"; exit 1 ;; + esac + echo " ${pair}" + done + echo "single-stack IPv6 confirmed" + - name: Apply DNS64 to external names only + # create-kind-cluster.sh already points CoreDNS at IPV6_DNS_UPSTREAM, so + # names resolve -- but the answers are unusable. Plain DNS64 synthesizes + # only for names with no AAAA, and the external names this job needs + # (storage.googleapis.com, example.com) do have AAAA records, pointing at + # real IPv6 addresses the runner cannot reach. Only translate_all forces + # them through the prefix. + # + # translate_all cannot go in the same server block as the cluster zones. + # dns64 wraps the whole plugin chain below it, and it answers a AAAA query + # by synthesizing from A -- so for an AAAA-only name it synthesizes from + # nothing and returns an empty answer. Every ClusterIP on a v6-only + # cluster is AAAA-only, so a single-block Corefile takes out all + # in-cluster service discovery: ate-api-server cannot find + # valkey-cluster.ate-system.svc and the install times out. + # + # So: cluster zones keep the chain kind shipped, the registry keeps the + # block create-kind-cluster.sh gave it, and dns64 sits in the catch-all + # with the forwarder. + run: | + kubectl --context="$KUBECTL_CONTEXT" -n kube-system get cm coredns \ + -o jsonpath='{.data.Corefile}' > /tmp/Corefile + # Re-zone the block kind shipped and lift out its forwarder, which moves + # to the catch-all below; health/ready/kubernetes/cache stay as-is. The + # rules are gated on "first" so they stop at that block's closing brace + # and leave the registry's own block untouched. + awk ' + NR == 1 && /^\.:53[[:space:]]*\{/ { + print "cluster.local:53 in-addr.arpa:53 ip6.arpa:53 {"; first = 1; next + } + first && /^ forward([[:space:]].*)?\{$/ { skip = 1; next } + first && skip && /^ \}$/ { skip = 0; next } + first && skip { next } + first && /^\}$/ { first = 0 } + { print } + ' /tmp/Corefile > /tmp/Corefile.new + if ! grep -q '^cluster.local:53' /tmp/Corefile.new; then + echo "::error::Corefile did not start with the .:53 block kind ships" + cat /tmp/Corefile; exit 1 + fi + # create-kind-cluster.sh owns this block. If it ever goes back to a + # hosts entry inside .:53, the split above silently drops the registry. + if ! grep -q '^kind-registry:53' /tmp/Corefile.new; then + echo "::error::no kind-registry server block in the Corefile" + cat /tmp/Corefile; exit 1 + fi + if grep -q 'forward' /tmp/Corefile.new; then + echo "::error::the forward block survived the split" + cat /tmp/Corefile.new; exit 1 + fi + cat >>/tmp/Corefile.new < /tmp/coredns-dns64.yaml + kubectl --context="$KUBECTL_CONTEXT" -n kube-system patch cm coredns \ + --type=merge --patch-file /tmp/coredns-dns64.yaml + kubectl --context="$KUBECTL_CONTEXT" -n kube-system rollout restart deploy/coredns + kubectl --context="$KUBECTL_CONTEXT" -n kube-system rollout status deploy/coredns --timeout=120s + cat /tmp/Corefile.new + - name: Verify cluster DNS answers both internal and external names + # The install is the next step and it takes ten minutes to fail. A DNS + # regression is the failure this Corefile is most likely to cause, so + # assert all three cases here where the message is unambiguous. + run: | + set -o pipefail + kubectl --context="$KUBECTL_CONTEXT" run dnscheck --rm --attach --quiet \ + --restart=Never --image=busybox:1.36 --command -- \ + sh -c ' + nslookup kubernetes.default.svc.cluster.local >/dev/null 2>&1 \ + || { echo "FAIL: an in-cluster Service does not resolve"; exit 1; } + nslookup storage.googleapis.com 2>/dev/null | grep -q "64:ff9b" \ + || { echo "FAIL: external names are not synthesized through NAT64"; exit 1; } + # Informational. Nothing downstream resolves this from a pod -- + # containerd pulls images on the node, and create-kind-cluster.sh + # runs its own registry probe before DNS64 is applied -- so a miss + # here is not a reason to fail the job. Printed because a change + # here would still be worth seeing. + echo "--- kind-registry, informational" + nslookup kind-registry 2>&1 | tail -4 + echo "DNS-OK" + ' | tee /tmp/dnscheck.log + grep -q DNS-OK /tmp/dnscheck.log + - name: Install Agent Substrate + run: hack/install-ate-kind.sh --deploy-ate-system + - name: Assert the control plane is up + # install-ate.sh runs under pipefail, so a failed apply does propagate. + # What it would not catch is a Deployment that rolls out and then + # crash-loops. Re-check everything deploy_ate_system waits on -- + # atenet-egress included, since a non-dual-stack Envoy listener fails + # there first, by way of a readiness probe the kubelet cannot reach. + run: | + for r in deployment/ate-api-server deployment/ate-controller \ + deployment/atenet-router deployment/atenet-egress \ + statefulset/valkey-cluster daemonset/atelet; do + kubectl --context="$KUBECTL_CONTEXT" -n ate-system rollout status "$r" --timeout=120s + done + kubectl --context="$KUBECTL_CONTEXT" -n podcertificate-controller-system \ + rollout status deployment/podcertificate-controller --timeout=120s + if kubectl --context="$KUBECTL_CONTEXT" -n ate-system get pods \ + -o jsonpath='{.items[*].status.containerStatuses[*].state.waiting.reason}' \ + | grep -q CrashLoopBackOff; then + echo "::error::a pod in ate-system is in CrashLoopBackOff" + exit 1 + fi + - name: Deploy gVisor counter demo + run: hack/install-ate-kind.sh --deploy-demo-counter + - name: Deploy egress demo + run: hack/install-ate-kind.sh --deploy-demo-egress + - name: Assert the demo fixtures exist + # A failed demo deploy exits 0: install-ate.sh dispatches demos through + # `if "${demo}_cmdline" "$1"`, which suspends errexit, and _cmdline ends + # in an unconditional `return 0`. Without this the suites fail later with + # "ActorTemplate not found", pointing at the tests instead of the install. + run: | + for ns_tmpl in ate-demo-counter/counter ate-demo-egress/egress; do + ns=${ns_tmpl%/*}; tmpl=${ns_tmpl#*/} + kubectl --context="$KUBECTL_CONTEXT" -n "${ns}" get actortemplate "${tmpl}" \ + || { echo "::error::${ns_tmpl} was not created -- the demo deploy failed silently"; exit 1; } + done + # One suite per step: run-e2e.sh takes exactly one target path, and this + # way a failure names the suite that produced it. + - name: Run E2E tests (demo) + id: e2e-demo + run: | + set -o pipefail + hack/run-e2e-kind.sh ./internal/e2e/suites/demo -v -args --no-color 2>&1 \ + | tee /tmp/e2e-demo.log + - name: Run E2E tests (networking) + # Runs even when demo failed -- networking is the half most likely to + # expose a single-family bug -- but stays skipped when an earlier step + # left no cluster to test against. always() is required, not decorative: + # an if: without a status function is implicitly ANDed with success(), + # which skips this step on exactly the failure it is meant to survive. + if: always() && steps.e2e-demo.outcome != 'skipped' + run: | + set -o pipefail + hack/run-e2e-kind.sh ./internal/e2e/suites/networking -v -args --no-color 2>&1 \ + | tee /tmp/e2e-networking.log + - name: Guard against a vacuously green run + # A suite that gates on a dual-stack Service and skips itself on v6-only + # exits 0, so a suite that only skipped would otherwise read as a pass. + # The bar is one real PASS per suite, not zero skips: demo legitimately + # skips the micro-VM-only Golden resume and the CSI volume tests on any + # family. Skips are printed so a growing list gets noticed. + # Skipped when the suites never ran: with no logs to count, this step + # would otherwise report a reassuring zero on a job that failed earlier. + if: always() && steps.e2e-demo.outcome != 'skipped' + run: | + for f in /tmp/e2e-demo.log /tmp/e2e-networking.log; do + [ -s "$f" ] || { echo "::error::${f} is missing or empty"; exit 1; } + passed=$(grep -c -- '--- PASS' "$f" || true) + echo "${f}: ${passed} passed, $(grep -c -- '--- SKIP' "$f" || true) skipped" + grep -h -- '--- SKIP' "$f" || true + if [ "${passed}" -eq 0 ]; then + echo "::error::${f} has no passing tests -- a suite that only skips proves nothing" + exit 1 + fi + done + - name: Dump diagnostics on failure + if: failure() + run: | + kubectl --context="$KUBECTL_CONTEXT" get actortemplate,workerpool,pods -A -o wide || true + dump() { + echo "=== logs: $1/$2 ===" + kubectl --context="$KUBECTL_CONTEXT" logs -n "$1" "$2" --all-containers --tail=300 2>/dev/null || true + } + for p in $(kubectl --context="$KUBECTL_CONTEXT" get pods -n ate-system -o name 2>/dev/null); do + dump ate-system "$p" + done + # Every worker pod in any namespace: the demo pools plus the e2e suites' + # randomly-named per-test namespaces, which the suites keep on failure. + kubectl --context="$KUBECTL_CONTEXT" get pods -A -l ate.dev/worker-pool \ + -o 'custom-columns=:.metadata.namespace,:.metadata.name' --no-headers 2>/dev/null \ + | while read -r ns name; do dump "$ns" "$name"; done + # IPv6-specific: the rewritten Corefile, and what each Service actually + # got assigned, are the two things that differ from the IPv4 job. + kubectl --context="$KUBECTL_CONTEXT" -n kube-system logs -l k8s-app=kube-dns --tail=100 || true + kubectl --context="$KUBECTL_CONTEXT" -n kube-system get cm coredns -o jsonpath='{.data.Corefile}' || true + kubectl --context="$KUBECTL_CONTEXT" get svc -A \ + -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,POLICY:.spec.ipFamilyPolicy,IPS:.spec.clusterIPs || true + # tayga logs a reason for every packet it declines to translate, which + # is the only view of an egress failure that is not a bare timeout. + echo "=== tayga ===" + sudo tail -100 /tmp/tayga.log || true diff --git a/cmd/atenet/internal/dns/README.md b/cmd/atenet/internal/dns/README.md index a134e829a..cb28e837f 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 0b301e7e2..32d4043b7 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 f13429e47..c20792662 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 cf2db99b6..96bc7df24 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 34116db28..34d23405d 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/cmd/atenet/internal/router/dataplane.go b/cmd/atenet/internal/router/dataplane.go index bd9f2abfc..ff886dcaa 100644 --- a/cmd/atenet/internal/router/dataplane.go +++ b/cmd/atenet/internal/router/dataplane.go @@ -36,6 +36,8 @@ type dataplaneHealthCheck struct { // untouched, so atunnel always authorizes by the actor's own DNS name -- // ingress.New needs no per-dataplane routing mode. +// healthCheck dials IPv4 loopback, which is why the admin socket in +// manifests/ate-install/atenet-router.yaml needs ipv4_compat. func (r atenetRouter) healthCheck() dataplaneHealthCheck { switch r { case atenetRouterEnvoy: diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index e2d76d8bf..903ac1fff 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -1108,6 +1108,27 @@ func (x *XdsServer) buildTracing() *hcmv3.HttpConnectionManager_Tracing { } } +// dualStackAdditionalAddresses returns the IPv6 half of a dual-stack ingress +// listener, to pair with a primary 0.0.0.0 socket on the same port. Ipv4Compat +// stays false: clearing IPV6_V6ONLY would collide with that primary. +func dualStackAdditionalAddresses(port uint32) []*listenerv3.AdditionalAddress { + return []*listenerv3.AdditionalAddress{ + { + Address: &corev3.Address{ + Address: &corev3.Address_SocketAddress{ + SocketAddress: &corev3.SocketAddress{ + Address: "::", + Ipv4Compat: false, + PortSpecifier: &corev3.SocketAddress_PortValue{ + PortValue: port, + }, + }, + }, + }, + }, + } +} + func (x *XdsServer) buildListener() *listenerv3.Listener { hcm := x.buildHcm("ingress_http", true) @@ -1123,6 +1144,7 @@ func (x *XdsServer) buildListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.ingressPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ @@ -1182,6 +1204,7 @@ func (x *XdsServer) buildHttpsListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.httpsPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ @@ -1213,6 +1236,7 @@ func (x *XdsServer) buildConnectTerminateListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.connectPlainTextPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ @@ -1246,6 +1270,7 @@ func (x *XdsServer) buildConnectTerminateTLSListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.connectTLSPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 6fa5c428b..ea39fb7c4 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -48,6 +48,36 @@ import ( "github.com/agent-substrate/substrate/internal/atunnel" ) +// assertDualStackIngress checks an ingress listener keeps its 0.0.0.0 primary +// and gains exactly one "::" socket on the same port. +func assertDualStackIngress(t *testing.T, l *listenerv3.Listener, wantPort uint32) { + t.Helper() + + sa := l.GetAddress().GetSocketAddress() + if sa.GetAddress() != "0.0.0.0" { + t.Errorf("Expected address '0.0.0.0', got %s", sa.GetAddress()) + } + if sa.GetPortValue() != wantPort { + t.Errorf("Expected port %d, got %d", wantPort, sa.GetPortValue()) + } + + addrs := l.GetAdditionalAddresses() + if len(addrs) != 1 { + t.Fatalf("Expected 1 additional address on %s, got %d", l.GetName(), len(addrs)) + } + + asa := addrs[0].GetAddress().GetSocketAddress() + if asa.GetAddress() != "::" { + t.Errorf("Expected additional address '::', got %s", asa.GetAddress()) + } + if asa.GetIpv4Compat() { + t.Error("Expected additional address Ipv4Compat to be false") + } + if asa.GetPortValue() != wantPort { + t.Errorf("Expected additional port %d, got %d", wantPort, asa.GetPortValue()) + } +} + func TestXdsServer_UpdateSnapshot(t *testing.T) { server := NewXdsServer(18000) server.SetConfig(8081, 50052, "10.0.0.1") @@ -150,14 +180,7 @@ func TestXdsServer_UpdateSnapshot(t *testing.T) { if raw, exists := listenersMap[IngressHTTPListener]; !exists { t.Errorf("Listener name '%s' is missing from snapshot listeners", IngressHTTPListener) } else { - l := raw.(*listenerv3.Listener) - sa := l.GetAddress().GetSocketAddress() - if sa.GetPortValue() != 8081 { - t.Errorf("Expected port 8081, got %d", sa.GetPortValue()) - } - if sa.GetAddress() != "0.0.0.0" { - t.Errorf("Expected address '0.0.0.0', got %s", sa.GetAddress()) - } + assertDualStackIngress(t, raw.(*listenerv3.Listener), 8081) } } @@ -192,10 +215,7 @@ func TestXdsServer_UpdateSnapshot_WithHttps(t *testing.T) { t.Errorf("Listener name '%s' is missing from snapshot listeners", IngressHTTPSListener) } else { l := raw.(*listenerv3.Listener) - sa := l.GetAddress().GetSocketAddress() - if sa.GetPortValue() != 8443 { - t.Errorf("Expected port 8443, got %d", sa.GetPortValue()) - } + assertDualStackIngress(t, l, 8443) // Verify the TLS config references the serving cert via SDS rather // than embedding it: inline filename DataSources are read only once @@ -354,16 +374,14 @@ func TestXdsServer_UpdateSnapshot_WithConnect(t *testing.T) { } if raw, exists := listenersMap["connect_terminate"]; !exists { t.Error("connect_terminate listener missing") - } else if sa := raw.(*listenerv3.Listener).GetAddress().GetSocketAddress(); sa.GetPortValue() != 8081 { - t.Errorf("Expected connect_terminate port 8081, got %d", sa.GetPortValue()) + } else { + assertDualStackIngress(t, raw.(*listenerv3.Listener), 8081) } if raw, exists := listenersMap["connect_terminate_tls"]; !exists { t.Error("connect_terminate_tls listener missing") } else { l := raw.(*listenerv3.Listener) - if sa := l.GetAddress().GetSocketAddress(); sa.GetPortValue() != 8444 { - t.Errorf("Expected connect_terminate_tls port 8444, got %d", sa.GetPortValue()) - } + assertDualStackIngress(t, l, 8444) ts := l.GetFilterChains()[0].GetTransportSocket() if ts.GetName() != "envoy.transport_sockets.tls" { t.Errorf("Expected connect_terminate_tls to be TLS-wrapped, got transport socket %q", ts.GetName()) diff --git a/cmd/ateom-gvisor/main.go b/cmd/ateom-gvisor/main.go index 0620c09e3..9e77aae2f 100644 --- a/cmd/ateom-gvisor/main.go +++ b/cmd/ateom-gvisor/main.go @@ -65,8 +65,11 @@ var ( podUID = pflag.String("pod-uid", "", "The UID of the current pod") // TODO(liorlieberman) have a sub package for all atunnel releated things like that - atunnelListenAddress = pflag.String("atunnel-listen-address", "0.0.0.0:443", "Address for actor ingress HTTPS") - atunnelConnectListenAddress = pflag.String("atunnel-connect-listen-address", "0.0.0.0:444", "Address for actor ingress mTLS CONNECT") + // + // Every listen address here is an unspecified wildcard, which Go binds as a + // dual-stack socket. + atunnelListenAddress = pflag.String("atunnel-listen-address", ":443", "Address for actor ingress HTTPS") + atunnelConnectListenAddress = pflag.String("atunnel-connect-listen-address", ":444", "Address for actor ingress mTLS CONNECT") workerCredentialBundle = pflag.String("atunnel-credential-bundle", "/run/podidentity.podcert.ate.dev/credential-bundle.pem", "Worker Pod credential bundle used by atunnel for inbound serving and outbound mTLS") podIdentityTrustBundle = pflag.String("atunnel-trust-bundle", "/run/podidentity.podcert.ate.dev/trust-bundle.pem", "Pod identity trust bundle used for router clients and the node-local atelet") atunnelClientIdentity = pflag.String("atunnel-client-identity", "spiffe://cluster.local/ns/ate-system/sa/atenet-router", "SPIFFE identity allowed to call actor ingress HTTPS") diff --git a/cmd/ateom-microvm/main.go b/cmd/ateom-microvm/main.go index 6613fcd9b..d4dde4680 100644 --- a/cmd/ateom-microvm/main.go +++ b/cmd/ateom-microvm/main.go @@ -71,8 +71,10 @@ var ( otlpRelaySocket = flag.String("otlp-relay-socket", ateompath.AteletOTLPSocketPath(), "Unix socket of atelet's OTLP relay to export telemetry through, keeping it off the pod network. Empty, or absent at startup, exports directly to OTEL_EXPORTER_OTLP_ENDPOINT instead.") - atunnelListenAddress = flag.String("atunnel-listen-address", "0.0.0.0:443", "Address for actor ingress HTTPS") - atunnelConnectListenAddress = flag.String("atunnel-connect-listen-address", "0.0.0.0:444", "Address for actor ingress mTLS CONNECT") + // Every listen address here is an unspecified wildcard, which Go binds as a + // dual-stack socket. + atunnelListenAddress = flag.String("atunnel-listen-address", ":443", "Address for actor ingress HTTPS") + atunnelConnectListenAddress = flag.String("atunnel-connect-listen-address", ":444", "Address for actor ingress mTLS CONNECT") workerCredentialBundle = flag.String("atunnel-credential-bundle", "/run/podidentity.podcert.ate.dev/credential-bundle.pem", "Worker Pod credential bundle used by atunnel for inbound serving and outbound mTLS") podIdentityTrustBundle = flag.String("atunnel-trust-bundle", "/run/podidentity.podcert.ate.dev/trust-bundle.pem", "Pod identity trust bundle used for router clients and the node-local atelet") atunnelClientIdentity = flag.String("atunnel-client-identity", "spiffe://cluster.local/ns/ate-system/sa/atenet-router", "SPIFFE identity allowed to call actor ingress HTTPS") diff --git a/hack/create-kind-cluster.sh b/hack/create-kind-cluster.sh index f413e5c95..c2ffdab58 100755 --- a/hack/create-kind-cluster.sh +++ b/hack/create-kind-cluster.sh @@ -21,6 +21,7 @@ KIND_CLUSTER_NAME="${KIND_CLUSTER_NAME:-kind}" KUBECTL_CONTEXT="kind-${KIND_CLUSTER_NAME}" reg_name="kind-registry" reg_port="${KIND_REGISTRY_PORT:-5001}" +IPV6_DNS_UPSTREAM="${IPV6_DNS_UPSTREAM:-2001:4860:4860::8888 2001:4860:4860::8844}" if [[ $# -gt 0 ]]; then case "$1" in @@ -31,6 +32,8 @@ if [[ $# -gt 0 ]]; then echo "Configured through the environment:" echo " KIND_CLUSTER_NAME Name of the cluster to create (default: kind)." echo " IP_FAMILY Address families for pods and Services: ipv4, ipv6 or dual (default: ipv4)." + echo " IPV6_DNS_UPSTREAM Space-separated IPv6 resolvers CoreDNS forwards to when IP_FAMILY=ipv6" + echo " (default: Google Public DNS). Override where those are unreachable." exit 0 ;; esac @@ -196,6 +199,61 @@ if [ "$(docker inspect -f='{{json .NetworkSettings.Networks.kind}}' "${reg_name} docker network connect "kind" "${reg_name}" fi +# 4.5. Give CoreDNS an IPv6 forwarder and a registry entry +# +# CoreDNS runs dnsPolicy: Default, inheriting the node's IPv4 resolver, which +# no pod here can reach, so external lookups SERVFAIL. Step 3's registry +# wiring is node-side, so it misses atelet too: that pull runs in atelet's own +# netns, where "kind-registry" NXDOMAINs. +if [[ "${IP_FAMILY}" == "ipv6" ]]; then + echo "Repointing CoreDNS at an IPv6 resolver and teaching it '${reg_name}'..." + reg_v6="$(docker inspect "${reg_name}" \ + --format '{{.NetworkSettings.Networks.kind.GlobalIPv6Address}}')" + if [[ -z "${reg_v6}" ]]; then + echo "error: '${reg_name}' has no IPv6 address on the 'kind' network" >&2 + exit 1 + fi + + corefile="$(kubectl --context="${KUBECTL_CONTEXT}" -n kube-system get cm coredns \ + -o jsonpath='{.data.Corefile}')" + search="forward . /etc/resolv.conf" + replace="forward . ${IPV6_DNS_UPSTREAM}" + # $search unquoted: bash 3.2 splices the quotes in literally. Replacing just + # the target leaves kind's trailing "{ max_concurrent 1000 }" in place. + patched="${corefile/$search/$replace}" + if [[ "${patched}" == "${corefile}" ]]; then + echo "error: '${search}' not found in the CoreDNS Corefile" >&2 + echo " the Corefile layout changed upstream; update this block" >&2 + exit 1 + fi + + # Its own server block, not a hosts entry in .:53. A query is served by the + # one block whose zone is its longest suffix, so only "${reg_name}" arrives + # here -- which is why this hosts needs no fallthrough to avoid NXDOMAINing + # every other name. + patched="${patched} +${reg_name}:53 { + errors + hosts { + ${reg_v6} ${reg_name} + } +}" + + # A YAML patch file avoids escaping the Corefile's newlines into JSON. + { printf 'data:\n Corefile: |\n'; printf '%s\n' "${patched}" | sed 's/^/ /'; } \ + > "${ROOT}/bin/coredns-patch.yaml" + kubectl --context="${KUBECTL_CONTEXT}" -n kube-system patch cm coredns \ + --type=merge --patch-file "${ROOT}/bin/coredns-patch.yaml" + kubectl --context="${KUBECTL_CONTEXT}" -n kube-system rollout restart deploy/coredns + kubectl --context="${KUBECTL_CONTEXT}" -n kube-system rollout status deploy/coredns \ + --timeout=120s + + # Its own script so it can be re-run against a live cluster: the hosts entry + # above is a snapshot of an address the registry can move off (#1049). + KUBECTL_CONTEXT="${KUBECTL_CONTEXT}" REG_NAME="${reg_name}" \ + IPV6_DNS_UPSTREAM="${IPV6_DNS_UPSTREAM}" "${ROOT}"/hack/verify-ipv6-dns.sh +fi + # 5. Document the local registry in kube-public ConfigMap echo "Documenting local registry in cluster..." cat <&2 + exit 1 + ;; + esac +fi + +# Best-effort: only used to make the registry failure message actionable. +reg_v6="$(docker inspect "${REG_NAME}" \ + --format '{{.NetworkSettings.Networks.kind.GlobalIPv6Address}}' 2>/dev/null || true)" +reg_at="${reg_v6:+ at [${reg_v6}]:5000}" + +echo "Verifying DNS from a pod..." +# Probe from a pod, not the node: the node is dual-stack and passes either way. +# The registry leg fetches rather than resolves -- the hosts entry is AAAA-only, +# which fails nslookup's A query but satisfies getaddrinfo. +# +# One stream carries every leg and only the last one's exit status, so each leg +# reports a marker on stdout and no failure message may contain one; PROBE_RAN +# and PROBE_DONE bracket the run so a short read is told apart from a leg that +# failed. Read the log once the pod has terminated rather than attaching to it: +# an attach can drop the tail, and a lost registry marker then reads as an +# unreachable registry. Retry the pod, not the query: one that asks before +# CoreDNS settles stays broken for ~30s, while a fresh pod 10s later resolves +# first try. +probe="" +probe_max=4 +for ((probe_attempt = 1; probe_attempt <= probe_max; probe_attempt++)); do + probe_pod="coredns-probe-$$-${probe_attempt}" + kubectl --context="${KUBECTL_CONTEXT}" run "${probe_pod}" \ + --restart=Never --image=busybox:1.36 --command -- \ + sh -c "echo PROBE_RAN + if out=\$(nslookup storage.googleapis.com 2>&1); then + echo RESOLVE_OK + else + echo \"resolve failed: \$(echo \"\$out\" | tail -2 | tr '\n' ' ')\" + fi + if out=\$(wget -T10 -O/dev/null http://${REG_NAME}:5000/v2/ 2>&1); then + echo REGISTRY_OK + else + echo \"registry fetch failed: \$(echo \"\$out\" | tail -1)\" + fi + echo PROBE_DONE" >/dev/null || true + for ((probe_wait = 0; probe_wait < 120; probe_wait++)); do + phase="$(kubectl --context="${KUBECTL_CONTEXT}" get pod "${probe_pod}" \ + -o jsonpath='{.status.phase}' 2>/dev/null || true)" + [[ "${phase}" == "Succeeded" || "${phase}" == "Failed" ]] && break + sleep 1 + done + attempt_out="$(kubectl --context="${KUBECTL_CONTEXT}" logs "${probe_pod}" 2>/dev/null || true)" + kubectl --context="${KUBECTL_CONTEXT}" delete pod "${probe_pod}" \ + --now --ignore-not-found --wait=false >/dev/null 2>&1 || true + # A pod that never started must not bury an earlier one's real failure. + if [[ "${attempt_out}" == *PROBE_RAN* ]]; then probe="${attempt_out}"; fi + # Only the resolve leg is a settling race; a down registry will not fix + # itself, so a finished probe is a verdict either way. An unfinished one + # reported no registry result at all, which is not the same as a failure. + if [[ "${probe}" == *RESOLVE_OK* ]] && + [[ "${probe}" == *REGISTRY_OK* || "${probe}" == *PROBE_DONE* ]]; then + break + fi + if ((probe_attempt < probe_max)); then + echo " the probe did not come back clean; re-probing (attempt $((probe_attempt + 1)) of ${probe_max})..." + sleep 10 + fi +done +if [[ "${probe}" != *RESOLVE_OK* || "${probe}" != *REGISTRY_OK* ]]; then + if [[ "${probe}" != *PROBE_RAN* ]]; then + echo "error: the probe pod never ran, so CoreDNS is unverified" >&2 + echo " check that it scheduled and that 'busybox:1.36' pulled" >&2 + elif [[ "${probe}" != *RESOLVE_OK* ]]; then + echo "error: a pod cannot resolve an external name" >&2 + echo " IPV6_DNS_UPSTREAM is '${IPV6_DNS_UPSTREAM}'; set it to a reachable resolver" >&2 + elif [[ "${probe}" != *PROBE_DONE* ]]; then + echo "error: the probe stopped early, so the registry leg is unverified" >&2 + echo " re-run this script; DNS itself answered" >&2 + else + echo "error: DNS works but a pod cannot reach '${REG_NAME}'${reg_at}" >&2 + echo " check the registry container is up and on the 'kind' network" >&2 + fi + if [[ -n "${probe}" ]]; then + echo " probe output was:" >&2 + printf '%s\n' "${probe}" | sed 's/^/ /' >&2 + fi + exit 1 +fi diff --git a/hack/verify/atenet-admin-bind.sh b/hack/verify/atenet-admin-bind.sh new file mode 100755 index 000000000..86090c016 --- /dev/null +++ b/hack/verify/atenet-admin-bind.sh @@ -0,0 +1,41 @@ +#!/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. + +# Dropping ipv4_compat from a gateway's Envoy admin socket fails silently: the +# drain sequence reads the refused IPv4 loopback dial as "Envoy already exited" +# and reports a drain it never performed. No Go test reads these manifests. + +set -o errexit -o nounset -o pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "${ROOT}" + +rc=0 +for f in manifests/ate-install/atenet-router.yaml manifests/ate-install/atenet-egress.yaml; do + block="$(grep -A 6 -E '^ *admin:$' "${f}" || true)" + if [[ -z "${block}" ]]; then + echo "${f}: no Envoy admin block found; this check needs updating" >&2 + rc=1 + elif ! grep -q '"::"' <<<"${block}"; then + echo "${f}: Envoy admin socket does not bind \"::\"; an IPv6-primary pod cannot be probed" >&2 + rc=1 + elif ! grep -q 'ipv4_compat: true' <<<"${block}"; then + echo "${f}: Envoy admin socket binds \"::\" without ipv4_compat; IPv4 loopback dials will be refused" >&2 + rc=1 + fi +done + +exit "${rc}" diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 91203a8e0..5c41cc84f 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -43,6 +43,11 @@ const ( ActorVethIP = "169.254.17.2" ActorNftTableName = "ateom_actor" + HostVethIPv6CIDR = "fd00:169:254::1/126" + ActorVethIPv6CIDR = "fd00:169:254::2/126" + ActorVethIPv6Gateway = "fd00:169:254::1" + ActorVethIPv6IP = "fd00:169:254::2" + // ActorVethSubnet is the point-to-point /30 the actor veth lives on. ActorVethSubnet = "169.254.17.0/30" ) @@ -51,6 +56,10 @@ var ( HostVethAddr = MustParseAddr(HostVethCIDR) ActorVethAddr = MustParseAddr(ActorVethCIDR) ActorVethGwIP = MustParseIP(ActorVethGateway) + + HostVethIPv6Addr = mustParseNoDADAddr(HostVethIPv6CIDR) + ActorVethIPv6Addr = mustParseNoDADAddr(ActorVethIPv6CIDR) + ActorVethIPv6GwIP = MustParseIPv6(ActorVethIPv6Gateway) ) // MustParseAddr parses a CIDR string into a netlink.Addr, panicking on error. @@ -62,6 +71,18 @@ func MustParseAddr(cidr string) *netlink.Addr { return a } +// mustParseNoDADAddr parses a CIDR into an address flagged IFA_F_NODAD. +// +// Per-address flag rather than the interface-wide accept_dad sysctl because the +// ateom container is unprivileged, so containerd mounts /proc/sys read-only. +// DAD is pointless on a point-to-point veth nobody else can reach, and it would +// otherwise hold the address tentative for ~1s on every resume. +func mustParseNoDADAddr(cidr string) *netlink.Addr { + a := MustParseAddr(cidr) + a.Flags = unix.IFA_F_NODAD + return a +} + // MustParseIP parses an IPv4 string into a net.IP, panicking on error. func MustParseIP(s string) net.IP { ip := net.ParseIP(s).To4() @@ -71,6 +92,55 @@ func MustParseIP(s string) net.IP { return ip } +// MustParseIPv6 parses an IPv6 string into a net.IP, panicking on error. +func MustParseIPv6(s string) net.IP { + ip := net.ParseIP(s).To16() + if ip == nil { + panic(fmt.Sprintf("parsing constant IPv6 %q", s)) + } + return ip +} + +// LinkIPv6Enabled reports whether IPv6 addresses can be assigned to the named +// link in the current netns. It answers a kernel capability question, not a +// cluster one: IPv4-only GKE leaves disable_ipv6=1 and netlink then rejects +// every IPv6 address with EPERM, but IPv4-only kind leaves it at 0 because the +// node kernel has IPv6 compiled in. A kernel built without IPv6 has no sysctl +// at all. Pair it with podHasGlobalIPv6 to decide whether the actor gets IPv6; +// on its own it says yes on clusters that have no IPv6 anywhere. +func LinkIPv6Enabled(name string) bool { + b, err := os.ReadFile("/proc/sys/net/ipv6/conf/" + name + "/disable_ipv6") + if err != nil { + return false + } + return len(b) > 0 && b[0] == '0' +} + +// podHasGlobalIPv6 reports whether the worker pod's own eth0 carries a global +// IPv6 address, which is what decides the families the actor can egress on. +// It must run in the worker pod netns. +// +// Scoped to eth0 rather than the whole netns on purpose: ActorVethName is also +// "eth0", so a netns-wide scan that filtered out the ateomnet link names by +// name would filter out the pod's own interface too and report false on every +// cluster. +func podHasGlobalIPv6() bool { + eth0Link, err := netlink.LinkByName("eth0") + if err != nil { + return false + } + // netlink can report ErrDumpInterrupted alongside a valid partial answer. + // Trust a positive result either way: reporting false on a dual-stack pod + // silently strands the actor on IPv4, which is the costlier mistake. + addrs, _ := netlink.AddrList(eth0Link, netlink.FAMILY_V6) + for _, addr := range addrs { + if addr.IP.IsGlobalUnicast() && !addr.IP.IsLinkLocalUnicast() { + return true + } + } + return false +} + // MustParseMAC parses a MAC address string into a net.HardwareAddr, panicking on error. func MustParseMAC(s string) net.HardwareAddr { m, err := net.ParseMAC(s) @@ -82,7 +152,9 @@ func MustParseMAC(s string) net.HardwareAddr { // ConfigureActorVeth configures the actor veth inside the interior netns. // It assumes it is already running inside the target network namespace. -func ConfigureActorVeth(ctx context.Context) error { +// ipv6 comes from SetupActorNetwork, which decides it in the worker pod netns; +// this namespace cannot answer the question for itself. +func ConfigureActorVeth(ctx context.Context, ipv6 bool) error { // Run inside the gVisor interior netns. SetupActorNetwork has already created // the veth peer here, under its final name, so this only has to address it. // gVisor reads link names, addresses, and routes from this namespace when the @@ -107,6 +179,12 @@ func ConfigureActorVeth(ctx context.Context) error { if err := netlink.AddrReplace(actorLink, ActorVethAddr); err != nil { return fmt.Errorf("while assigning actor veth address: %w", err) } + if ipv6 { + if err := netlink.AddrReplace(actorLink, ActorVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning actor veth ipv6 address: %w", err) + } + } + if err := netlink.LinkSetUp(actorLink); err != nil { return fmt.Errorf("while bringing up actor veth: %w", err) } @@ -117,6 +195,15 @@ func ConfigureActorVeth(ctx context.Context) error { }); err != nil { return fmt.Errorf("while installing actor default route: %w", err) } + if ipv6 { + if err := netlink.RouteReplace(&netlink.Route{ + LinkIndex: actorLink.Attrs().Index, + Gw: ActorVethIPv6GwIP, + Dst: &net.IPNet{IP: net.ParseIP("::"), Mask: net.CIDRMask(0, 128)}, + }); err != nil { + return fmt.Errorf("while installing actor default ipv6 route: %w", err) + } + } return nil } @@ -192,31 +279,52 @@ func PodIPv4() (net.IP, error) { return nil, fmt.Errorf("pod eth0 has no IPv4 address") } -// EnableIPv4Forwarding enables IPv4 forwarding in the current network namespace. -func EnableIPv4Forwarding() error { +// EnableForwarding enables IPv4 and IPv6 forwarding in the current network +// namespace. +func EnableForwarding() error { // Forwarding is required because actor packets now enter the worker pod via // the host-side veth and then leave through the pod's eth0. Without this, the // kernel would not route traffic between those interfaces even though both - // live in the worker pod network namespace. + // live in the worker pod network namespace, and ip6_forward() would drop the + // IPv6 half outright, the actor's DNS queries included. // - // Without privileged, the container runtime bind-mounts /proc/sys read-only. - // The worker holds CAP_SYS_ADMIN and uses no user namespace, so the ro flag - // is not locked: clear it, write the sysctl, restore ro. - const path = "/proc/sys/net/ipv4/ip_forward" + // conf.all.forwarding implies the per-interface default, so one write covers + // both the veth and eth0. + for _, path := range []string{ + "/proc/sys/net/ipv4/ip_forward", + "/proc/sys/net/ipv6/conf/all/forwarding", + } { + if err := writeSysctlIfUnset(path); err != nil { + return fmt.Errorf("while enabling forwarding in worker pod netns: %w", err) + } + } + return nil +} + +// writeSysctlIfUnset writes "1" to a sysctl path unless it already reads "1". A +// missing path is not an error: the IPv6 sysctls are absent on a kernel with +// IPv6 compiled out, and there is then nothing to enable. +func writeSysctlIfUnset(path string) error { if b, err := os.ReadFile(path); err == nil && len(b) > 0 && b[0] == '1' { return nil } if err := os.WriteFile(path, []byte("1\n"), 0o644); err == nil { return nil } + if _, err := os.Stat(path); os.IsNotExist(err) { + return nil + } + // Without privileged, the container runtime bind-mounts /proc/sys read-only. + // The worker holds CAP_SYS_ADMIN and uses no user namespace, so the ro flag + // is not locked: clear it, write the sysctl, restore ro. if err := unix.Mount("none", "/proc/sys", "", unix.MS_BIND|unix.MS_REMOUNT, ""); err != nil { - return fmt.Errorf("while remounting /proc/sys read-write to enable IPv4 forwarding: %w", err) + return fmt.Errorf("while remounting /proc/sys read-write to enable forwarding: %w", err) } defer func() { _ = unix.Mount("none", "/proc/sys", "", unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, "") }() if err := os.WriteFile(path, []byte("1\n"), 0o644); err != nil { - return fmt.Errorf("while enabling IPv4 forwarding in worker pod netns: %w", err) + return fmt.Errorf("while writing %s: %w", path, err) } return nil } @@ -229,9 +337,6 @@ func InstallActorNftablesRules(egressPort uint16) error { // rules in an ateom-owned table makes cleanup simple and avoids mutating // Kubernetes or CNI-managed chains directly. // - // TODO: Add IPv6 veth addressing, forwarding, and nftables rules once actor - // networking supports dual-stack pods. The current actor network is IPv4-only. - // // The rules do three things: // // * prerouting: redirect new actor TCP connections to atunnel's local @@ -248,7 +353,7 @@ func InstallActorNftablesRules(egressPort uint16) error { c := &nftables.Conn{} table := &nftables.Table{ - Family: nftables.TableFamilyIPv4, + Family: nftables.TableFamilyINet, Name: ActorNftTableName, } c.AddTable(table) @@ -263,6 +368,9 @@ func InstallActorNftablesRules(egressPort uint16) error { if redirectRule := ActorEgressRedirectRule(table, prerouting, egressPort); redirectRule != nil { c.AddRule(redirectRule) } + if redirectRuleIPv6 := ActorIPv6EgressRedirectRule(table, prerouting, egressPort); redirectRuleIPv6 != nil { + c.AddRule(redirectRuleIPv6) + } postrouting := c.AddChain(&nftables.Chain{ Name: "postrouting", @@ -276,6 +384,11 @@ func InstallActorNftablesRules(egressPort uint16) error { Chain: postrouting, Exprs: append(IPSourceEqual(ActorVethIP), &expr.Masq{}), }) + c.AddRule(&nftables.Rule{ + Table: table, + Chain: postrouting, + Exprs: append(IPv6SourceEqual(ActorVethIPv6IP), &expr.Masq{}), + }) acceptPolicy := nftables.ChainPolicyAccept forward := c.AddChain(&nftables.Chain{ @@ -306,7 +419,7 @@ func RemoveActorNftablesRules() error { // per-worker and currently per-active-actor because this worker path runs at // most one actor at a time. Missing tables are treated as already clean. c := &nftables.Conn{} - tables, err := c.ListTablesOfFamily(nftables.TableFamilyIPv4) + tables, err := c.ListTablesOfFamily(nftables.TableFamilyINet) if err != nil { return fmt.Errorf("while listing nftables tables: %w", err) } @@ -329,6 +442,12 @@ func IPSourceEqual(ip string) []expr.Any { func IPPayloadEqual(offset uint32, ip string) []expr.Any { return []expr.Any{ + &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: []byte{unix.NFPROTO_IPV4}, + }, &expr.Payload{ DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, @@ -343,6 +462,32 @@ func IPPayloadEqual(offset uint32, ip string) []expr.Any { } } +func IPv6SourceEqual(ip string) []expr.Any { + return IPv6PayloadEqual(8, ip) +} + +func IPv6PayloadEqual(offset uint32, ip string) []expr.Any { + return []expr.Any{ + &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: []byte{unix.NFPROTO_IPV6}, + }, + &expr.Payload{ + DestRegister: 1, + Base: expr.PayloadBaseNetworkHeader, + Offset: offset, + Len: 16, + }, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: MustParseIPv6(ip), + }, + } +} + func TCPProtocol() []expr.Any { return []expr.Any{ &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, @@ -372,6 +517,24 @@ func ActorEgressRedirectRule(table *nftables.Table, chain *nftables.Chain, port return &nftables.Rule{Table: table, Chain: chain, Exprs: exprs} } +// ActorIPv6EgressRedirectRule is ActorEgressRedirectRule for the actor's IPv6 +// source address. Both rules live in the same inet table, so each carries its +// own NFPROTO match to keep it off the other family's packets. +func ActorIPv6EgressRedirectRule(table *nftables.Table, chain *nftables.Chain, port uint16) *nftables.Rule { + if port == 0 { + return nil + } + exprs := append(IPv6SourceEqual(ActorVethIPv6IP), TCPProtocol()...) + exprs = append(exprs, + &expr.Immediate{ + Register: 1, + Data: binaryutil.BigEndian.PutUint16(port), + }, + &expr.Redir{RegisterProtoMin: 1}, + ) + return &nftables.Rule{Table: table, Chain: chain, Exprs: exprs} +} + // CreateNetNSWithoutSwitching creates a named netns and returns its handle, // restoring the caller's current netns before returning. func CreateNetNSWithoutSwitching(name string) (netns.NsHandle, error) { @@ -557,15 +720,31 @@ func SetupActorNetwork(ctx context.Context, cfg NetworkConfig) (retErr error) { if err := netlink.AddrReplace(hostLink, HostVethAddr); err != nil { return fmt.Errorf("while assigning host veth address: %w", err) } + // Decided once, here in the worker pod netns, and carried into the interior + // netns below. Probing separately on each side would let them disagree: the + // interior netns is freshly created, so its sysctl is always the permissive + // kernel default whatever the pod's families are. + podIPv6, linkIPv6 := podHasGlobalIPv6(), LinkIPv6Enabled(HostVethName) + actorIPv6 := podIPv6 && linkIPv6 + if actorIPv6 { + if err := netlink.AddrReplace(hostLink, HostVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning host veth ipv6 address: %w", err) + } + } else { + slog.InfoContext(ctx, "actor networking is IPv4-only", + "link", HostVethName, "podHasGlobalIPv6", podIPv6, "linkIPv6Enabled", linkIPv6) + } if err := netlink.LinkSetUp(hostLink); err != nil { return fmt.Errorf("while bringing up host veth: %w", err) } - if err := NetNSDo(ctx, cfg.InteriorNetNS, ConfigureActorVeth); err != nil { + if err := NetNSDo(ctx, cfg.InteriorNetNS, func(ctx context.Context) error { + return ConfigureActorVeth(ctx, actorIPv6) + }); err != nil { return fmt.Errorf("while configuring actor veth in interior netns: %w", err) } - if err := EnableIPv4Forwarding(); err != nil { + if err := EnableForwarding(); err != nil { return err } if err := InstallActorNftablesRules(cfg.EgressRedirectPort); err != nil { diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index b9c8ac45f..1f7634f8d 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -19,6 +19,8 @@ package ateomnet import ( "context" "errors" + "net" + "os" "runtime" "testing" @@ -26,6 +28,7 @@ import ( "github.com/google/nftables" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" + "golang.org/x/sys/unix" ) // withTestNetNS runs fn with the calling thread inside a throwaway netns @@ -75,6 +78,34 @@ func withTestNetNS(t *testing.T, fn func(interior netns.NsHandle)) { fn(interior) } +// addPodEth0 plants a dummy link carrying cidrs in the current netns, standing +// in for the worker pod's own primary interface. withTestNetNS hands out a bare +// namespace, and the families on that interface are what SetupActorNetwork reads +// to decide the families the actor gets. +// +// The name has to be exactly "eth0": the probe is link-scoped, so under any +// other name it answers false and the test asserts the opposite of what it +// means to. It collides with ActorVethName by design -- that collision is the +// reason the probe cannot just scan the namespace. +func addPodEth0(t *testing.T, cidrs ...string) { + t.Helper() + + link := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "eth0"}} + if err := netlink.LinkAdd(link); err != nil { + t.Fatalf("creating the stand-in pod eth0: %v", err) + } + if err := netlink.LinkSetUp(link); err != nil { + t.Fatalf("bringing up the stand-in pod eth0: %v", err) + } + for _, cidr := range cidrs { + addr := MustParseAddr(cidr) + addr.Flags |= unix.IFA_F_NODAD // else an IPv6 address stays tentative + if err := netlink.AddrAdd(link, addr); err != nil { + t.Fatalf("assigning %s to the stand-in pod eth0: %v", cidr, err) + } + } +} + // requireNftables skips when the kernel in this environment cannot serve the // nftables netlink API at all, which SetupActorNetwork needs and which is a // property of the machine rather than of the code under test. @@ -86,6 +117,26 @@ func requireNftables(t *testing.T) { } } +// actorNftTableExists reports whether the actor table is present in the family +// InstallActorNftablesRules creates it in. The family is load-bearing: +// ListTablesOfFamily puts it in the netlink dump header, so the kernel filters +// the dump and a query for the wrong family comes back empty rather than +// erroring. +func actorNftTableExists(t *testing.T) bool { + t.Helper() + c := &nftables.Conn{} + tables, err := c.ListTablesOfFamily(nftables.TableFamilyINet) + if err != nil { + t.Fatalf("listing inet nftables tables: %v", err) + } + for _, table := range tables { + if table.Name == ActorNftTableName { + return true + } + } + return false +} + // linkByName returns the link, or nil when it does not exist. func linkByName(t *testing.T, name string) netlink.Link { t.Helper() @@ -115,6 +166,69 @@ func hasAddr(t *testing.T, link netlink.Link, cidr string) bool { return false } +// assertIPv6AddrNoDAD requires cidr to be present on link and to carry +// IFA_F_NODAD. +// +// The flag is the whole point: the ateom container is unprivileged, so the +// accept_dad sysctl this replaced could not be written and setup failed outright +// on a real worker. It passes as root, where /proc/sys is writable either way, +// so nothing else here would catch a regression back to the sysctl. +func assertIPv6AddrNoDAD(t *testing.T, link netlink.Link, cidr string) { + t.Helper() + addrs, err := netlink.AddrList(link, netlink.FAMILY_V6) + if err != nil { + t.Fatalf("listing IPv6 addresses of %q: %v", link.Attrs().Name, err) + } + want := MustParseAddr(cidr) + for _, addr := range addrs { + if addr.IPNet == nil || addr.IPNet.String() != want.IPNet.String() { + continue + } + if addr.Flags&unix.IFA_F_NODAD == 0 { + t.Errorf("%s on %q has flags %#x, want IFA_F_NODAD (%#x) set", cidr, link.Attrs().Name, addr.Flags, unix.IFA_F_NODAD) + } + return + } + t.Errorf("%q does not carry %s, got %v", link.Attrs().Name, cidr, addrs) +} + +// assertDefaultRoute requires link to carry -- or, when want is false, to not +// carry -- a default route via gw in the given family. +// +// The IPv6 case is the half that matters most: it is the ::/0 route, not the +// address, that lets Go's RFC 6724 sorting find a source for a AAAA and put it +// first. +func assertDefaultRoute(t *testing.T, link netlink.Link, family int, gw net.IP, want bool) { + t.Helper() + + dst := "0.0.0.0/0" + if family == netlink.FAMILY_V6 { + dst = "::/0" + } + routes, err := netlink.RouteList(link, family) + if err != nil { + t.Fatalf("listing %s routes of %q: %v", dst, link.Attrs().Name, err) + } + var got bool + for _, route := range routes { + // A default route reports its destination either as nil or as an + // explicit zero-length mask, depending on how the kernel rendered it. + ones := 0 + if route.Dst != nil { + ones, _ = route.Dst.Mask.Size() + } + if ones == 0 && route.Gw.Equal(gw) { + got = true + } + } + switch { + case want && !got: + t.Errorf("%q has no %s route via %s, got %v", link.Attrs().Name, dst, gw, routes) + case !want && got: + t.Errorf("%q has a %s route via %s, want none, got %v", link.Attrs().Name, dst, gw, routes) + } +} + // TestSetupActorNetworkFinalState pins the namespace state gVisor and the // micro-VM guest read after an activation: what links exist, where, with which // addresses and routes. It deliberately asserts the end state rather than the @@ -128,6 +242,9 @@ func TestSetupActorNetworkFinalState(t *testing.T) { withTestNetNS(t, func(interior netns.NsHandle) { requireNftables(t) + // A dual-stack pod: the actor gets IPv6 only because this does. + addPodEth0(t, "10.244.0.7/24", "fd00:10:244::7/64") + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { t.Fatalf("SetupActorNetwork: %v", err) } @@ -143,12 +260,17 @@ func TestSetupActorNetworkFinalState(t *testing.T) { if host.Attrs().Flags&1 == 0 { // net.FlagUp t.Errorf("host veth %q is not up", HostVethName) } + assertIPv6AddrNoDAD(t, host, HostVethIPv6CIDR) // The actor interface must exist ONLY in the interior netns. A peer left - // in the pod netns would mean the pair was built the old way, and worse, - // would collide with the pod's own eth0 on a real worker. - if stray := linkByName(t, ActorVethName); stray != nil { - t.Errorf("actor interface %q must not exist in the pod netns", ActorVethName) + // in the pod netns would mean the pair was built the old way -- and since + // ActorVethName is "eth0", it would land on top of the pod's own + // interface. What must still answer to that name here is the stand-in. + switch stray := linkByName(t, ActorVethName); { + case stray == nil: + t.Errorf("the stand-in pod %q disappeared", ActorVethName) + case stray.Type() != "dummy": + t.Errorf("%q in the pod netns is a %s, want the stand-in dummy: the veth peer was left behind", ActorVethName, stray.Type()) } if err := NetNSDo(ctx, interior, func(context.Context) error { @@ -162,6 +284,7 @@ func TestSetupActorNetworkFinalState(t *testing.T) { if actor.Attrs().Flags&1 == 0 { t.Errorf("actor veth %q is not up", ActorVethName) } + assertIPv6AddrNoDAD(t, actor, ActorVethIPv6CIDR) if lo := linkByName(t, "lo"); lo == nil { t.Error("interior netns has no loopback") @@ -169,28 +292,261 @@ func TestSetupActorNetworkFinalState(t *testing.T) { t.Error("interior loopback is not up") } - routes, err := netlink.RouteList(actor, netlink.FAMILY_V4) - if err != nil { - t.Fatalf("listing interior routes: %v", err) + assertDefaultRoute(t, actor, netlink.FAMILY_V4, ActorVethGwIP, true) + assertDefaultRoute(t, actor, netlink.FAMILY_V6, ActorVethIPv6GwIP, true) + return nil + }); err != nil { + t.Fatalf("inspecting interior netns: %v", err) + } + }) +} + +// disableIPv6 turns IPv6 off in the current netns the way an IPv4-only cluster +// does, so a link created afterwards rejects every IPv6 address. +func disableIPv6(t *testing.T) { + t.Helper() + for _, knob := range []string{"all", "default"} { + path := "/proc/sys/net/ipv6/conf/" + knob + "/disable_ipv6" + if err := os.WriteFile(path, []byte("1\n"), 0o644); err != nil { + t.Skipf("cannot disable IPv6 in this netns (%s): %v", path, err) + } + } +} + +// disableIPv6ForNewLinks turns IPv6 off for links created after it runs and +// leaves the ones that already exist -- and their addresses -- alone. +// +// disableIPv6 cannot stand in for it: writing "all" flushes the pod eth0's IPv6 +// address too, so both halves of the gate go false at once and either one alone +// would produce the same result. +func disableIPv6ForNewLinks(t *testing.T) { + t.Helper() + const path = "/proc/sys/net/ipv6/conf/default/disable_ipv6" + if err := os.WriteFile(path, []byte("1\n"), 0o644); err != nil { + t.Skipf("cannot disable IPv6 for new links in this netns (%s): %v", path, err) + } +} + +// assertNoIPv6Addr requires link to carry no IPv6 address at all. +func assertNoIPv6Addr(t *testing.T, link netlink.Link) { + t.Helper() + addrs, err := netlink.AddrList(link, netlink.FAMILY_V6) + if err != nil { + t.Fatalf("listing IPv6 addresses of %q: %v", link.Attrs().Name, err) + } + if len(addrs) != 0 { + t.Errorf("%q carries IPv6 addresses %v in an IPv4-only netns, want none", link.Attrs().Name, addrs) + } +} + +// assertNoGlobalIPv6Addr requires link to carry no IPv6 address beyond the +// fe80::/64 the kernel gives every up link wherever IPv6 is enabled at all. +// That link-local is not what strands an actor -- the routable address is -- +// and only turning IPv6 off for the whole netns suppresses it, which is why +// assertNoIPv6Addr does not fit a pod that merely has no IPv6 of its own. +func assertNoGlobalIPv6Addr(t *testing.T, link netlink.Link) { + t.Helper() + addrs, err := netlink.AddrList(link, netlink.FAMILY_V6) + if err != nil { + t.Fatalf("listing IPv6 addresses of %q: %v", link.Attrs().Name, err) + } + for _, addr := range addrs { + if addr.IP.IsGlobalUnicast() && !addr.IP.IsLinkLocalUnicast() { + t.Errorf("%q carries global IPv6 address %s, want none", link.Attrs().Name, addr) + } + } +} + +// TestSetupActorNetworkIPv4OnlyNetNS covers a worker pod whose netns has IPv6 +// disabled outright, which is the default on an IPv4-only GKE cluster. Writing +// the "all" knob also flushes the addresses already assigned, so both halves of +// the gate are false here and the actor must come up IPv4-only. The IPv4 half +// must come up exactly as it does with IPv6 available -- an ungated attempt +// fails with EPERM on the path of every SetupActorNetwork call site, and the +// actor never leaves ResumeGoldenActor. +func TestSetupActorNetworkIPv4OnlyNetNS(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + disableIPv6(t) + if err := NetNSDo(ctx, interior, func(context.Context) error { + disableIPv6(t) + return nil + }); err != nil { + t.Fatalf("disabling IPv6 in the interior netns: %v", err) + } + + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { + t.Fatalf("SetupActorNetwork on an IPv4-only netns: %v", err) + } + + host := linkByName(t, HostVethName) + if host == nil { + t.Fatalf("host veth %q missing from the pod netns", HostVethName) + } + if !hasAddr(t, host, HostVethCIDR) { + t.Errorf("host veth %q does not carry %s", HostVethName, HostVethCIDR) + } + if host.Attrs().Flags&1 == 0 { // net.FlagUp + t.Errorf("host veth %q is not up", HostVethName) + } + assertNoIPv6Addr(t, host) + + if err := NetNSDo(ctx, interior, func(context.Context) error { + actor := linkByName(t, ActorVethName) + if actor == nil { + t.Fatalf("actor veth %q missing from the interior netns", ActorVethName) + } + if !hasAddr(t, actor, ActorVethCIDR) { + t.Errorf("actor veth %q does not carry %s", ActorVethName, ActorVethCIDR) + } + if actor.Attrs().Flags&1 == 0 { + t.Errorf("actor veth %q is not up", ActorVethName) + } + assertNoIPv6Addr(t, actor) + + assertDefaultRoute(t, actor, netlink.FAMILY_V4, ActorVethGwIP, true) + assertDefaultRoute(t, actor, netlink.FAMILY_V6, ActorVethIPv6GwIP, false) + return nil + }); err != nil { + t.Fatalf("inspecting interior netns: %v", err) + } + }) +} + +// TestSetupActorNetworkNoPodIPv6 covers the case that turned the IPv4 e2e job +// red: a worker pod on an IPv4-only cluster whose kernel has IPv6 compiled in. +// Nothing is disabled, so every capability probe says yes, but the pod's own +// eth0 carries no IPv6 and there is nowhere for an actor's IPv6 packet to go. +// Giving the actor an address and a ::/0 route anyway makes it prefer the AAAA +// of any dual-stack destination and the connection dies mid-response. +func TestSetupActorNetworkNoPodIPv6(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + addPodEth0(t, "10.244.0.7/24") + + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { + t.Fatalf("SetupActorNetwork on a pod without IPv6: %v", err) + } + + host := linkByName(t, HostVethName) + if host == nil { + t.Fatalf("host veth %q missing from the pod netns", HostVethName) + } + if !hasAddr(t, host, HostVethCIDR) { + t.Errorf("host veth %q does not carry %s", HostVethName, HostVethCIDR) + } + assertNoGlobalIPv6Addr(t, host) + + if err := NetNSDo(ctx, interior, func(context.Context) error { + actor := linkByName(t, ActorVethName) + if actor == nil { + t.Fatalf("actor veth %q missing from the interior netns", ActorVethName) } - // A default route reports its destination either as nil or as an - // explicit 0.0.0.0/0, depending on how the kernel rendered it. - isDefault := func(route netlink.Route) bool { - if route.Dst == nil { - return true - } - ones, _ := route.Dst.Mask.Size() - return ones == 0 + if !hasAddr(t, actor, ActorVethCIDR) { + t.Errorf("actor veth %q does not carry %s", ActorVethName, ActorVethCIDR) + } + // The interior netns is created fresh, so its own sysctls always say + // IPv6 is available whatever the pod's families are. Only a decision + // carried across from the pod netns can get this right. + assertNoGlobalIPv6Addr(t, actor) + assertDefaultRoute(t, actor, netlink.FAMILY_V4, ActorVethGwIP, true) + assertDefaultRoute(t, actor, netlink.FAMILY_V6, ActorVethIPv6GwIP, false) + return nil + }); err != nil { + t.Fatalf("inspecting interior netns: %v", err) + } + }) +} + +// TestSetupActorNetworkPerLinkIPv6Disabled covers the one case the capability +// half of the gate is there for: a dual-stack pod on a node that disables IPv6 +// per link, so the veth created for the actor inherits disable_ipv6=1 while the +// pod's own eth0 keeps its address. Assigning IPv6 to that veth fails with +// EPERM, and because it happens on the path of every SetupActorNetwork call an +// ungated attempt takes actor startup from working to totally broken. +func TestSetupActorNetworkPerLinkIPv6Disabled(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + addPodEth0(t, "10.244.0.7/24", "fd00:10:244::7/64") + disableIPv6ForNewLinks(t) + + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { + t.Fatalf("SetupActorNetwork with IPv6 disabled per link: %v", err) + } + + host := linkByName(t, HostVethName) + if host == nil { + t.Fatalf("host veth %q missing from the pod netns", HostVethName) + } + if !hasAddr(t, host, HostVethCIDR) { + t.Errorf("host veth %q does not carry %s", HostVethName, HostVethCIDR) + } + assertNoGlobalIPv6Addr(t, host) + + if err := NetNSDo(ctx, interior, func(context.Context) error { + actor := linkByName(t, ActorVethName) + if actor == nil { + t.Fatalf("actor veth %q missing from the interior netns", ActorVethName) } - var haveDefault bool - for _, route := range routes { - if isDefault(route) && route.Gw.Equal(ActorVethGwIP) { - haveDefault = true - } + if !hasAddr(t, actor, ActorVethCIDR) { + t.Errorf("actor veth %q does not carry %s", ActorVethName, ActorVethCIDR) } - if !haveDefault { - t.Errorf("interior netns has no default route via %s, got %v", ActorVethGateway, routes) + assertNoGlobalIPv6Addr(t, actor) + assertDefaultRoute(t, actor, netlink.FAMILY_V6, ActorVethIPv6GwIP, false) + return nil + }); err != nil { + t.Fatalf("inspecting interior netns: %v", err) + } + }) +} + +// TestSetupActorNetworkPodIPv6Only is the other direction of the gate: a pod on +// an IPv6-only cluster must still get actor IPv6. A probe that reads the wrong +// link or the wrong scope fails closed, which every IPv4 test in this file would +// happily accept, so this and TestSetupActorNetworkFinalState are the two that +// notice. +// +// The actor still gets its IPv4 address and 0.0.0.0/0 route here, unconditionally +// and unusably -- the mirror image of the bug this gate fixes, left alone because +// nothing in the tree runs an actor on a v6-only cluster yet. +func TestSetupActorNetworkPodIPv6Only(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + addPodEth0(t, "fd00:10:244::7/64") + + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { + t.Fatalf("SetupActorNetwork on an IPv6-only pod: %v", err) + } + + host := linkByName(t, HostVethName) + if host == nil { + t.Fatalf("host veth %q missing from the pod netns", HostVethName) + } + assertIPv6AddrNoDAD(t, host, HostVethIPv6CIDR) + + if err := NetNSDo(ctx, interior, func(context.Context) error { + actor := linkByName(t, ActorVethName) + if actor == nil { + t.Fatalf("actor veth %q missing from the interior netns", ActorVethName) } + assertIPv6AddrNoDAD(t, actor, ActorVethIPv6CIDR) + assertDefaultRoute(t, actor, netlink.FAMILY_V6, ActorVethIPv6GwIP, true) return nil }); err != nil { t.Fatalf("inspecting interior netns: %v", err) @@ -215,9 +571,20 @@ func TestSetupActorNetworkIsRepeatable(t *testing.T) { if linkByName(t, HostVethName) == nil { t.Fatalf("host veth %q missing after activation %d", HostVethName, i) } + if !actorNftTableExists(t) { + t.Fatalf("nftables table %q missing after activation %d", ActorNftTableName, i) + } if err := CleanupActorNetwork(ctx, interior); err != nil { t.Fatalf("CleanupActorNetwork (activation %d): %v", i, err) } + // Install and teardown have to name the same family. When they do not, + // teardown's dump comes back empty, its "missing tables are already + // clean" path reports success, and the table survives -- so the next + // activation stacks another copy of every chain and rule onto it and + // the leak is invisible to every other assertion here. + if actorNftTableExists(t) { + t.Fatalf("nftables table %q survived cleanup after activation %d", ActorNftTableName, i) + } } // Cleanup is idempotent: the extra call after the loop's last one must @@ -228,6 +595,9 @@ func TestSetupActorNetworkIsRepeatable(t *testing.T) { if stray := linkByName(t, HostVethName); stray != nil { t.Errorf("host veth %q survived cleanup", HostVethName) } + if actorNftTableExists(t) { + t.Errorf("nftables table %q survived a repeated cleanup", ActorNftTableName) + } if err := NetNSDo(ctx, interior, func(context.Context) error { if stray := linkByName(t, ActorVethName); stray != nil { t.Errorf("actor veth %q survived cleanup", ActorVethName) diff --git a/internal/ateomnet/write_sysctl_test.go b/internal/ateomnet/write_sysctl_test.go new file mode 100644 index 000000000..b4cb1db16 --- /dev/null +++ b/internal/ateomnet/write_sysctl_test.go @@ -0,0 +1,84 @@ +//go:build linux + +// 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 ateomnet + +import ( + "os" + "path/filepath" + "testing" +) + +// TestWriteSysctlIfUnset verifies writeSysctlIfUnset's fast paths against a +// temp file standing in for a /proc/sys node: it must not rewrite a value +// that already reads "1", and it must write "1\n" when the value is missing +// or unset. The privileged bind-remount path is covered by the netns +// integration tests (withTestNetNS), which require root. +func TestWriteSysctlIfUnset(t *testing.T) { + dir := t.TempDir() + + t.Run("already_set", func(t *testing.T) { + p := filepath.Join(dir, "already") + // Sentinel content: if writeSysctlIfUnset rewrote the file, the value + // would change to "1\n" and this assertion would fail. Keeping the + // file larger than the helper's output makes a silent rewrite + // detectable. + if err := os.WriteFile(p, []byte("1 other-content\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if string(b) != "1 other-content\n" { + t.Fatalf("already-set file was rewritten: %q", b) + } + }) + + t.Run("unset_written", func(t *testing.T) { + p := filepath.Join(dir, "unset") + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if len(b) < 1 || b[0] != '1' { + t.Fatalf("expected '1' written, got %q", b) + } + }) + + t.Run("zero_is_rewritten", func(t *testing.T) { + p := filepath.Join(dir, "zero") + if err := os.WriteFile(p, []byte("0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if len(b) < 1 || b[0] != '1' { + t.Fatalf("expected '1' written, got %q", b) + } + }) +} diff --git a/internal/atunnel/original_dst_linux.go b/internal/atunnel/original_dst_linux.go index 07dd0f934..8b4309119 100644 --- a/internal/atunnel/original_dst_linux.go +++ b/internal/atunnel/original_dst_linux.go @@ -18,6 +18,7 @@ package atunnel import ( "encoding/binary" + "errors" "fmt" "net" "strconv" @@ -26,10 +27,12 @@ import ( "golang.org/x/sys/unix" ) -// TCPOriginalDestination reads the IPv4 destination preserved by a Linux -// REDIRECT rule. Actor networking is currently IPv4-only. -// TODO(liorlieberman) add the IPv6 IP6T_SO_ORIGINAL_DST variant -// when actor veth setup gains dual-stack support. +// IP6T_SO_ORIGINAL_DST is not generated by golang.org/x/sys/unix. It is +// defined as 80 in linux/netfilter_ipv6/ip6_tables.h. +const ip6tSOOriginalDst = 80 + +// TCPOriginalDestination reads the IPv4 or IPv6 destination preserved by a +// Linux REDIRECT rule. func TCPOriginalDestination(conn net.Conn) (string, error) { tcpConn, ok := conn.(*net.TCPConn) if !ok { @@ -40,21 +43,15 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { return "", fmt.Errorf("atunnel: acquiring TCP syscall connection: %w", err) } - var addr unix.RawSockaddrInet4 var sockoptErr error + var destination string if err := rawConn.Control(func(fd uintptr) { - size := uint32(unsafe.Sizeof(addr)) - _, _, errno := unix.Syscall6( - unix.SYS_GETSOCKOPT, - fd, - unix.SOL_IP, - unix.SO_ORIGINAL_DST, - uintptr(unsafe.Pointer(&addr)), - uintptr(unsafe.Pointer(&size)), - 0, - ) - if errno != 0 { - sockoptErr = errno + destination, sockoptErr = originalIPv4Destination(fd) + // Linux returns ENOENT when the IPv4 original-destination option is + // queried on a redirected IPv6 connection. Only then try the IPv6 + // equivalent, so unrelated IPv4 failures retain their original error. + if errors.Is(sockoptErr, unix.ENOENT) { + destination, sockoptErr = originalIPv6Destination(fd) } }); err != nil { return "", fmt.Errorf("atunnel: accessing TCP socket: %w", err) @@ -62,11 +59,44 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { if sockoptErr != nil { return "", fmt.Errorf("atunnel: reading original TCP destination: %w", sockoptErr) } + return destination, nil +} + +func originalIPv4Destination(fd uintptr) (string, error) { + var addr unix.RawSockaddrInet4 + if errno := getOriginalDestination(fd, unix.SOL_IP, unix.SO_ORIGINAL_DST, unsafe.Pointer(&addr), unsafe.Sizeof(addr)); errno != 0 { + return "", errno + } + return formatOriginalDestination(addr.Addr[:], addr.Port) +} + +func originalIPv6Destination(fd uintptr) (string, error) { + var addr unix.RawSockaddrInet6 + if errno := getOriginalDestination(fd, unix.SOL_IPV6, ip6tSOOriginalDst, unsafe.Pointer(&addr), unsafe.Sizeof(addr)); errno != 0 { + return "", errno + } + return formatOriginalDestination(addr.Addr[:], addr.Port) +} + +func getOriginalDestination(fd uintptr, level, option int, addr unsafe.Pointer, addrSize uintptr) unix.Errno { + size := uint32(addrSize) + _, _, errno := unix.Syscall6( + unix.SYS_GETSOCKOPT, + fd, + uintptr(level), + uintptr(option), + uintptr(addr), + uintptr(unsafe.Pointer(&size)), + 0, + ) + return errno +} - portBytes := (*[2]byte)(unsafe.Pointer(&addr.Port)) +func formatOriginalDestination(ip []byte, rawPort uint16) (string, error) { + portBytes := (*[2]byte)(unsafe.Pointer(&rawPort)) port := binary.BigEndian.Uint16(portBytes[:]) if port == 0 { return "", fmt.Errorf("atunnel: original TCP destination has port zero") } - return net.JoinHostPort(net.IP(addr.Addr[:]).String(), strconv.Itoa(int(port))), nil + return net.JoinHostPort(net.IP(ip).String(), strconv.Itoa(int(port))), nil } diff --git a/internal/atunnel/original_dst_linux_test.go b/internal/atunnel/original_dst_linux_test.go new file mode 100644 index 000000000..4f26d9805 --- /dev/null +++ b/internal/atunnel/original_dst_linux_test.go @@ -0,0 +1,395 @@ +//go:build linux + +// 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 atunnel + +import ( + "context" + "errors" + "fmt" + "net" + "os" + "strings" + "testing" + "time" + + "github.com/google/nftables" + "github.com/google/nftables/binaryutil" + "github.com/google/nftables/expr" + "github.com/vishvananda/netlink" + "github.com/vishvananda/netns" + "golang.org/x/sys/unix" + + "github.com/agent-substrate/substrate/internal/ateomnet" + "github.com/agent-substrate/substrate/internal/roottest" +) + +func TestTCPOriginalDestination(t *testing.T) { + roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") + + // Model the production path rather than redirecting a locally generated + // connection through OUTPUT. Actor egress enters the worker netns through a + // veth and is redirected in PREROUTING; that is the path on which Linux + // preserves SO_ORIGINAL_DST for atunnel. + actorNS := newTestNetNS(t) + actorIP, hostIP := setupTestVeth(t, actorNS) + // targetListener reserves the port the actor intends to reach. The NAT rule + // below must prevent connections from reaching it. + // + // redirectListener represents atunnel's local egress listener. It receives + // the redirected connection and is therefore the connection on which we ask + // Linux for the original destination. + redirectListener := listenTCP(t, hostIP) + defer redirectListener.Close() + targetListener := listenTCP(t, hostIP) + defer targetListener.Close() + targetPort := targetListener.Addr().(*net.TCPAddr).Port + + table := &nftables.Table{Family: nftables.TableFamilyIPv4, Name: fmt.Sprintf("atunnel_original_dst_test_%d", os.Getpid())} + installOriginalDstRedirect(t, table, actorIP, targetPort, redirectListener.Addr().(*net.TCPAddr).Port) + + clientDone := make(chan error, 1) + go func() { + // From the actor's perspective this is an ordinary connection to + // hostIP:targetPort. The worker's PREROUTING rule redirects it before + // it reaches the host network stack's local delivery path. + clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + conn, err := net.DialTimeout("tcp4", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), time.Second) + if err == nil { + _ = conn.Close() + } + return err + }) + }() + + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + t.Fatal(err) + } + redirected, err := redirectListener.Accept() + if err != nil { + t.Fatalf("accepting redirected connection: %v", err) + } + defer redirected.Close() + + // The accepted socket is addressed to redirectListener, but the kernel's + // SO_ORIGINAL_DST record must still contain the destination chosen by the + // actor before nftables rewrote it. + got, err := TCPOriginalDestination(redirected) + if err != nil { + t.Fatalf("TCPOriginalDestination: %v", err) + } + want := net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)) + if got != want { + t.Errorf("original destination = %q, want %q", got, want) + } + if err := <-clientDone; err != nil { + t.Fatalf("dialing redirected connection: %v", err) + } +} + +func TestTCPOriginalDestinationIPv6(t *testing.T) { + roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") + + actorNS := newTestNetNS(t) + actorIP, hostIP := setupTestIPv6Veth(t, actorNS) + redirectListener := listenTCP6(t, hostIP) + defer redirectListener.Close() + targetListener := listenTCP6(t, hostIP) + defer targetListener.Close() + targetPort := targetListener.Addr().(*net.TCPAddr).Port + + table := &nftables.Table{Family: nftables.TableFamilyIPv6, Name: fmt.Sprintf("atunnel_original_dst_ipv6_test_%d", os.Getpid())} + installOriginalDstIPv6Redirect(t, table, actorIP, targetPort, redirectListener.Addr().(*net.TCPAddr).Port) + + clientDone := make(chan error, 1) + go func() { + clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + conn, err := net.DialTimeout("tcp6", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), time.Second) + if err == nil { + _ = conn.Close() + } + return err + }) + }() + + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + t.Fatal(err) + } + redirected, err := redirectListener.Accept() + if err != nil { + t.Fatalf("accepting redirected IPv6 connection: %v", err) + } + defer redirected.Close() + + // This assertion captures the IPv6 behavior required by #686. + got, err := TCPOriginalDestination(redirected) + if err != nil { + t.Fatalf("TCPOriginalDestination: %v", err) + } + want := net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)) + if got != want { + t.Errorf("original IPv6 destination = %q, want %q", got, want) + } + if err := <-clientDone; err != nil { + t.Fatalf("dialing redirected IPv6 connection: %v", err) + } +} + +func newTestNetNS(t *testing.T) netns.NsHandle { + t.Helper() + name := fmt.Sprintf("atunnel-original-dst-%d", os.Getpid()) + ns, err := ateomnet.CreateNetNSWithoutSwitching(name) + if err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_SYS_ADMIN to create network namespace: %v", err) + } + t.Fatal(err) + } + t.Cleanup(func() { + _ = ns.Close() + if err := netns.DeleteNamed(name); err != nil { + t.Errorf("deleting test network namespace: %v", err) + } + }) + return ns +} + +func setupTestVeth(t *testing.T, actorNS netns.NsHandle) (actorIP, hostIP net.IP) { + t.Helper() + hostName := fmt.Sprintf("atod%d", os.Getpid()) + peerName := fmt.Sprintf("atop%d", os.Getpid()) + if err := netlink.LinkAdd(&netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: hostName}, PeerName: peerName}); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to create veth: %v", err) + } + t.Fatal(err) + } + t.Cleanup(func() { + if link, err := netlink.LinkByName(hostName); err == nil { + if err := netlink.LinkDel(link); err != nil { + t.Errorf("deleting test veth: %v", err) + } + } + }) + hostLink, err := netlink.LinkByName(hostName) + if err != nil { + t.Fatal(err) + } + // Allocate one of the /30s in 198.18.0.0/16 from the PID so concurrent + // test processes do not try to use the same host-side address. + network := uint16(os.Getpid() % (1 << 14)) + thirdOctet := byte(network >> 6) + fourthOctet := byte(network&0x3f) << 2 + hostIP = net.IPv4(198, 18, thirdOctet, fourthOctet+1) + actorIP = net.IPv4(198, 18, thirdOctet, fourthOctet+2) + if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: &net.IPNet{IP: hostIP, Mask: net.CIDRMask(30, 32)}}); err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetUp(hostLink); err != nil { + t.Fatal(err) + } + peer, err := netlink.LinkByName(peerName) + if err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetNsFd(peer, int(actorNS)); err != nil { + t.Fatal(err) + } + // Complete the actor end of the point-to-point link inside its own netns. + if err := ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + lo, err := netlink.LinkByName("lo") + if err != nil { + return err + } + if err := netlink.LinkSetUp(lo); err != nil { + return err + } + link, err := netlink.LinkByName(peerName) + if err != nil { + return err + } + if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: &net.IPNet{IP: actorIP, Mask: net.CIDRMask(30, 32)}}); err != nil { + return err + } + return netlink.LinkSetUp(link) + }); err != nil { + t.Fatal(err) + } + return actorIP, hostIP +} + +func listenTCP(t *testing.T, hostIP net.IP) net.Listener { + t.Helper() + listener, err := net.ListenTCP("tcp4", &net.TCPAddr{IP: hostIP, Port: 0}) + if err != nil { + t.Fatal(err) + } + return listener +} + +func setupTestIPv6Veth(t *testing.T, actorNS netns.NsHandle) (actorIP, hostIP net.IP) { + t.Helper() + hostName := fmt.Sprintf("atod6%d", os.Getpid()) + peerName := fmt.Sprintf("atop6%d", os.Getpid()) + if err := netlink.LinkAdd(&netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: hostName}, PeerName: peerName}); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to create veth: %v", err) + } + t.Fatal(err) + } + t.Cleanup(func() { + if link, err := netlink.LinkByName(hostName); err == nil { + if err := netlink.LinkDel(link); err != nil { + t.Errorf("deleting test IPv6 veth: %v", err) + } + } + }) + hostLink, err := netlink.LinkByName(hostName) + if err != nil { + t.Fatal(err) + } + prefix := uint16(os.Getpid()) + hostIP = net.ParseIP(fmt.Sprintf("fd00:198:18:%x::1", prefix)) + actorIP = net.ParseIP(fmt.Sprintf("fd00:198:18:%x::2", prefix)) + // This isolated veth has no competing IPv6 peers. Suppress DAD so the + // address can be bound immediately instead of remaining tentative while + // the test is trying to start its listener. + if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: &net.IPNet{IP: hostIP, Mask: net.CIDRMask(64, 128)}, Flags: unix.IFA_F_NODAD}); err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetUp(hostLink); err != nil { + t.Fatal(err) + } + peer, err := netlink.LinkByName(peerName) + if err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetNsFd(peer, int(actorNS)); err != nil { + t.Fatal(err) + } + if err := ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + lo, err := netlink.LinkByName("lo") + if err != nil { + return err + } + if err := netlink.LinkSetUp(lo); err != nil { + return err + } + link, err := netlink.LinkByName(peerName) + if err != nil { + return err + } + if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: &net.IPNet{IP: actorIP, Mask: net.CIDRMask(64, 128)}, Flags: unix.IFA_F_NODAD}); err != nil { + return err + } + return netlink.LinkSetUp(link) + }); err != nil { + t.Fatal(err) + } + return actorIP, hostIP +} + +func listenTCP6(t *testing.T, hostIP net.IP) net.Listener { + t.Helper() + listener, err := net.ListenTCP("tcp6", &net.TCPAddr{IP: hostIP, Port: 0}) + if err != nil { + t.Fatal(err) + } + return listener +} + +func installOriginalDstRedirect(t *testing.T, table *nftables.Table, actorIP net.IP, targetPort, redirectPort int) { + t.Helper() + c := &nftables.Conn{} + c.AddTable(table) + chain := c.AddChain(&nftables.Chain{ + Name: "prerouting", + Table: table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityNATDest, + }) + c.AddRule(&nftables.Rule{ + Table: table, + Chain: chain, + Exprs: []expr.Any{ + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_TCP}}, + // Restrict the rule to this test's actor so the temporary table cannot + // affect unrelated local TCP traffic. + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: actorIP.To4()}, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(targetPort))}, + &expr.Immediate{Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(redirectPort))}, + &expr.Redir{RegisterProtoMin: 1}, + }, + }) + if err := c.Flush(); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to install nftables rule: %v", err) + } + t.Fatalf("installing nftables redirect: %v", err) + } + t.Cleanup(func() { + cleanup := &nftables.Conn{} + cleanup.DelTable(table) + if err := cleanup.Flush(); err != nil { + t.Errorf("removing nftables redirect: %v", err) + } + }) +} + +func installOriginalDstIPv6Redirect(t *testing.T, table *nftables.Table, actorIP net.IP, targetPort, redirectPort int) { + t.Helper() + c := &nftables.Conn{} + c.AddTable(table) + chain := c.AddChain(&nftables.Chain{ + Name: "prerouting", + Table: table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityNATDest, + }) + c.AddRule(&nftables.Rule{ + Table: table, + Chain: chain, + Exprs: []expr.Any{ + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_TCP}}, + // An IPv6 source address begins eight bytes into the IPv6 header. + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 8, Len: 16}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: actorIP.To16()}, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(targetPort))}, + &expr.Immediate{Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(redirectPort))}, + &expr.Redir{RegisterProtoMin: 1}, + }, + }) + if err := c.Flush(); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to install IPv6 nftables rule: %v", err) + } + t.Fatalf("installing IPv6 nftables redirect: %v", err) + } + t.Cleanup(func() { + cleanup := &nftables.Conn{} + cleanup.DelTable(table) + if err := cleanup.Flush(); err != nil { + t.Errorf("removing IPv6 nftables redirect: %v", err) + } + }) +} diff --git a/internal/e2e/dns_client.go b/internal/e2e/dns_client.go new file mode 100644 index 000000000..76db25d4f --- /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 000000000..3d72fcc04 --- /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 4a0c006b2..7992ba036 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 a04a07141..1890a036f 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 } diff --git a/internal/e2e/suites/networking/dns_test.go b/internal/e2e/suites/networking/dns_test.go new file mode 100644 index 000000000..b8abf001c --- /dev/null +++ b/internal/e2e/suites/networking/dns_test.go @@ -0,0 +1,163 @@ +// 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) + } + }) +} + +// 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) + } +} diff --git a/internal/e2e/suites/networking/ingress_family_test.go b/internal/e2e/suites/networking/ingress_family_test.go new file mode 100644 index 000000000..4a2e50d1d --- /dev/null +++ b/internal/e2e/suites/networking/ingress_family_test.go @@ -0,0 +1,280 @@ +// 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" + "encoding/json" + "fmt" + "maps" + "net" + "os/exec" + "slices" + "strconv" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/agent-substrate/substrate/internal/e2e" + "github.com/agent-substrate/substrate/internal/portforward" + "github.com/agent-substrate/substrate/internal/resources" +) + +const ( + routerAppLabel = "app=atenet-router" + // envoyAdminPort is the admin listener in the router pod's envoy container. + // It is not published by the Service, so the pod proxy subresource is the + // only way at it from a test. + envoyAdminPort = 9901 + + // Listener names from cmd/atenet/internal/router/xds.go. They cannot be + // imported: that package is under cmd/atenet/internal, so only cmd/atenet + // may import it. + ingressHTTPListener = "ingress_http_listener" + ingressHTTPSListener = "ingress_https_listener" + connectTerminateListener = "connect_terminate" + connectTerminateTLSListener = "connect_terminate_tls" + + // Same digest-pinned image the networkpolicy suite probes with. BusyBox's + // wget handles bracketed IPv6 URLs and honors a user-supplied Host header + // instead of adding its own, which is exactly what is needed here. + probeImage = "busybox@sha256:1487d0af5f52b4ba31c7e465126ee2123fe3f2305d638e7827681e7cf6c83d5e" +) + +// envoyListeners is the subset of Envoy's admin /listeners?format=json response +// this test reads. additional_local_addresses is how a listener with +// Listener.additional_addresses reports its extra sockets. +type envoyListeners struct { + ListenerStatuses []struct { + Name string `json:"name"` + LocalAddress struct { + SocketAddress envoySocketAddress `json:"socket_address"` + } `json:"local_address"` + AdditionalLocalAddresses []struct { + SocketAddress envoySocketAddress `json:"socket_address"` + } `json:"additional_local_addresses"` + } `json:"listener_statuses"` +} + +type envoySocketAddress struct { + Address string `json:"address"` + PortValue int `json:"port_value"` +} + +// TestRouterListenerAddresses asserts, from Envoy's own view of itself, that +// each of the router's dataplane listeners — the two ingress ones and the two +// CONNECT ones — bound both an IPv4 and an IPv6 socket. +// +// This is the cheap half of the ingress coverage and the one that runs +// everywhere: the "::" socket binds on a single-stack IPv4 cluster too, it just +// carries no traffic there. It is also the only assertion in the suite that can +// fail when someone removes the IPv6 socket, because every other path a test has +// into the router — a port-forward, the pods/proxy subresource, the +// services/proxy subresource — is mediated by the API server and reaches the +// pod over whatever family the *kubelet or apiserver* chooses. None of them let +// the test select an address family, so none of them can select a listener +// socket. +func TestRouterListenerAddresses(t *testing.T) { + ctx := context.Background() + clients := e2e.GetClients() + pod := mustRouterPodName(t, ctx) + + raw, err := clients.K8s.CoreV1().RESTClient().Get(). + Namespace(e2e.RouterNamespace). + Resource("pods"). + Name(pod+":"+strconv.Itoa(envoyAdminPort)). + SubResource("proxy"). + Suffix("listeners"). + Param("format", "json"). + DoRaw(ctx) + if err != nil { + // The pods/proxy subresource reaches the pod on its primary-family + // PodIP, so this hop used to be family-sensitive. It no longer is: the + // admin listener binds "::" with ipv4_compat + // (manifests/ate-install/atenet-router.yaml), which accepts connections + // from either family. A failure here means the admin interface is not + // answering — the container is not up, or the proxy path is blocked. + t.Fatalf("reading Envoy admin /listeners from %s/%s: %v", e2e.RouterNamespace, pod, err) + } + + var listeners envoyListeners + if err := json.Unmarshal(raw, &listeners); err != nil { + t.Fatalf("decoding /listeners response %q: %v", raw, err) + } + if len(listeners.ListenerStatuses) == 0 { + t.Fatalf("Envoy reports no listeners at all; xDS has not converged. Body: %s", raw) + } + + // One entry per listener name: every address it is bound on. + bound := map[string][]string{} + for _, ls := range listeners.ListenerStatuses { + addrs := []string{ls.LocalAddress.SocketAddress.Address} + for _, extra := range ls.AdditionalLocalAddresses { + addrs = append(addrs, extra.SocketAddress.Address) + } + bound[ls.Name] = addrs + } + + // Only the plain HTTP listener is unconditional. The other three exist + // only when --port-https, --port-connect and --port-connect-tls are set; + // all are set in the shipped manifest, but do not make this test the thing + // that fails if that changes. + for _, l := range []struct { + name string + required bool + }{ + {ingressHTTPListener, true}, + {ingressHTTPSListener, false}, + {connectTerminateListener, false}, + {connectTerminateTLSListener, false}, + } { + name := l.name + t.Run(name, func(t *testing.T) { + addrs, ok := bound[name] + if !ok { + if !l.required { + t.Skipf("router has no %s; listeners present: %v", name, slices.Sorted(maps.Keys(bound))) + } + t.Fatalf("router has no %s; listeners present: %v", name, slices.Sorted(maps.Keys(bound))) + } + // The IPv4 socket is the one that carries all production traffic + // today; losing it is the expensive regression, so assert it first. + if !slices.Contains(addrs, "0.0.0.0") { + t.Errorf("%s is bound on %v; want an IPv4 wildcard socket (0.0.0.0)", name, addrs) + } + if !slices.Contains(addrs, "::") { + t.Errorf("%s is bound on %v; want an IPv6 wildcard socket (::) as well. "+ + "Envoy binds it on a single-stack cluster too, so this failing means the "+ + "listener lost its additional_addresses entry", name, addrs) + } + }) + } +} + +// TestActorIngressPerFamily reaches an actor through the router over each of the +// router Service's ClusterIPs, from a pod inside the cluster. +// +// The client has to be in-cluster: e2e.RouterClient port-forwards to +// 127.0.0.1, which tunnels through the API server to the kubelet, so its own +// address family says nothing about which of the router's sockets served the +// request. +// +// Skipped unless the router Service is dual-stack. On a single-stack cluster +// there is exactly one ClusterIP and TestActorDirectAccess already covers it. +func TestActorIngressPerFamily(t *testing.T) { + ctx := context.Background() + + routerV4, routerV6, err := e2e.RouterClusterIPs(ctx) + if err != nil { + t.Fatalf("reading atenet-router ClusterIPs: %v", err) + } + if routerV4 == "" || routerV6 == "" { + t.Skipf("atenet-router is single-stack (v4=%q v6=%q); nothing to compare", routerV4, routerV6) + } + + actorName, _ := createAndResumeActor(t, ctx, "family", e2e.CounterFixture()) + dnsName := resources.ActorDNSName(resources.ActorRef{Atespace: networkingAtespace, Name: actorName}) + + probeNS := e2e.CreateNamespace(t) + probePod := startProbePod(t, ctx, probeNS.Name) + + // Both families, in one test: the point of dual-stack is that both work, + // and a change that turns the IPv4 socket off is the costly failure. + for _, tc := range []struct{ family, clusterIP string }{ + {"ipv4", routerV4}, + {"ipv6", routerV6}, + } { + t.Run(tc.family, func(t *testing.T) { + // The request must carry the actor's DNS name as the Host: it is + // the only routing key the router's ext_proc has. Only the + // *connection* goes to the literal. + url := fmt.Sprintf("http://%s/readyz", net.JoinHostPort(tc.clusterIP, "80")) + out, err := execInPod(probeNS.Name, probePod, + "wget", "-q", "-T", "10", "-O", "-", "--header", "Host: "+dnsName, url) + if err != nil { + t.Fatalf("GET %s (Host: %s) from %s/%s over %s failed: %v; output: %s", + url, dnsName, probeNS.Name, probePod, tc.family, err, out) + } + t.Logf("actor reached over %s via %s; body: %s", tc.family, tc.clusterIP, strings.TrimSpace(out)) + }) + } +} + +func mustRouterPodName(t *testing.T, ctx context.Context) string { + t.Helper() + pods, err := e2e.GetClients().K8s.CoreV1().Pods(e2e.RouterNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: routerAppLabel, + }) + if err != nil { + t.Fatalf("listing atenet-router pods: %v", err) + } + for i := range pods.Items { + if portforward.IsPodReady(&pods.Items[i]) { + return pods.Items[i].Name + } + } + t.Fatalf("no ready atenet-router pod in %s", e2e.RouterNamespace) + return "" +} + +func startProbePod(t *testing.T, ctx context.Context, namespace string) string { + t.Helper() + clients := e2e.GetClients() + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "ingress-probe", Namespace: namespace}, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{{ + Name: "probe", + Image: probeImage, + Command: []string{"/bin/sleep", "3600"}, + }}, + }, + } + if _, err := clients.K8s.CoreV1().Pods(namespace).Create(ctx, pod, metav1.CreateOptions{}); err != nil { + t.Fatalf("creating probe pod %s/%s: %v", namespace, pod.Name, err) + } + + deadline := time.Now().Add(60 * time.Second) + for time.Now().Before(deadline) { + got, err := clients.K8s.CoreV1().Pods(namespace).Get(ctx, pod.Name, metav1.GetOptions{}) + if err == nil && got.Status.Phase == corev1.PodRunning { + return pod.Name + } + time.Sleep(time.Second) + } + t.Fatalf("timed out waiting for probe pod %s/%s to run", namespace, pod.Name) + return "" +} + +// execInPod runs a command in a pod. It shells out to kubectl, matching what +// the networkpolicy suite already does, rather than pulling in client-go's +// remotecommand plumbing for two calls. +func execInPod(namespace, pod string, command ...string) (string, error) { + args := []string{} + if e2e.KubeConfig != "" { + args = append(args, "--kubeconfig="+e2e.KubeConfig) + } + if e2e.KubeContext != "" { + args = append(args, "--context="+e2e.KubeContext) + } + args = append(args, "exec", "-n", namespace, pod, "--") + args = append(args, command...) + out, err := exec.Command("kubectl", args...).CombinedOutput() + return string(out), err +} diff --git a/internal/ipfamily/ipfamily.go b/internal/ipfamily/ipfamily.go new file mode 100644 index 000000000..a6a364108 --- /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 000000000..9b6097253 --- /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) + } + }) + } +} diff --git a/manifests/ate-install/atenet-egress-with-sdsmint.yaml b/manifests/ate-install/atenet-egress-with-sdsmint.yaml index 72b3e1979..f283a0c34 100644 --- a/manifests/ate-install/atenet-egress-with-sdsmint.yaml +++ b/manifests/ate-install/atenet-egress-with-sdsmint.yaml @@ -236,7 +236,7 @@ data: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO - name: envoy.filters.http.router typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router @@ -323,7 +323,7 @@ data: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO - name: envoy.filters.http.router typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router @@ -410,7 +410,7 @@ data: "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO # The MITM must not weaken upstream authentication. Envoy decrypted the # actor's TLS with a leaf of its own; it still sends the real SNI here # and still validates the real origin's certificate against the public @@ -453,7 +453,7 @@ data: "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO # Envoy refuses to build a dynamic forward proxy cluster without # auto_sni and auto_san_validation unless this is set, because for # the usual TLS case resolving the host from a header and then not diff --git a/manifests/ate-install/atenet-egress.yaml b/manifests/ate-install/atenet-egress.yaml index 591ef31fc..64153ea6f 100644 --- a/manifests/ate-install/atenet-egress.yaml +++ b/manifests/ate-install/atenet-egress.yaml @@ -37,12 +37,15 @@ data: envoy.yaml: | admin: address: - socket_address: { address: 0.0.0.0, port_value: 15000 } + # ipv4_compat is load-bearing: see --envoy-admin-address below. + socket_address: { address: "::", ipv4_compat: true, port_value: 15000 } static_resources: listeners: - name: egress address: - socket_address: { address: 0.0.0.0, port_value: 443 } + # ipv4_compat rather than a second socket: IPv4 peers arrive as + # ::ffff: and nothing here reads the peer -- identity is the cert. + socket_address: { address: "::", ipv4_compat: true, port_value: 443 } filter_chains: # Named so ext_proc can read it back as xds.filter_chain_name. Must # match EgressFilterChainName in @@ -142,7 +145,7 @@ data: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO - name: envoy.filters.http.router typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router @@ -179,7 +182,7 @@ data: "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO --- apiVersion: apps/v1 kind: Deployment @@ -379,6 +382,8 @@ metadata: namespace: ate-system spec: type: ClusterIP + # Prefer, not Require: Require fails Service creation on a single-stack cluster. + ipFamilyPolicy: PreferDualStack selector: app: atenet-egress ports: diff --git a/manifests/ate-install/atenet-router.yaml b/manifests/ate-install/atenet-router.yaml index e05e06efb..462e2a8ad 100644 --- a/manifests/ate-install/atenet-router.yaml +++ b/manifests/ate-install/atenet-router.yaml @@ -86,7 +86,9 @@ data: admin: address: socket_address: - address: 0.0.0.0 + # ipv4_compat is load-bearing: dataplane.go probes /ready over IPv4 loopback. + address: "::" + ipv4_compat: true port_value: 9901 node: @@ -354,6 +356,8 @@ metadata: namespace: ate-system spec: type: ClusterIP + # Prefer, not Require: Require fails Service creation on a single-stack cluster. + ipFamilyPolicy: PreferDualStack selector: app: atenet-router ports: