From 2022b6558f280fc71861d8f8f1dd0318980b8e8a Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 08:49:40 -0700 Subject: [PATCH 01/22] hack: fix DNS on IPv6-only kind clusters On a fresh IP_FAMILY=ipv6 cluster nothing resolves from inside a pod and no actor boots: CoreDNS inherits the node's IPv4 resolver, which a v6-only pod cannot reach, and "kind-registry" NXDOMAINs in atelet's own netns. Point the forward at an IPv6 upstream, overridable with IPV6_DNS_UPSTREAM, and give the registry its own server block, so it is asked for nothing but its own name. IPv4 and dual-stack clusters are unchanged, and atenet-egress still crashloops on v6-only for an unrelated Envoy bind bug. Asking once was not enough to prove that: about half of fresh clusters do not answer the first query, and a pod that goes unanswered stays unanswered, so the check re-asks with a new pod and prints what the pod saw when it gives up. It lives in hack/verify-ipv6-dns.sh rather than inline, because the registry block records an address the registry can move off and there was no way to re-check a cluster without rebuilding it. --- hack/create-kind-cluster.sh | 58 ++++++++++++++++++++ hack/verify-ipv6-dns.sh | 106 ++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100755 hack/verify-ipv6-dns.sh 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. +# +# --attach gives one stream and only the last leg's exit status, so each leg +# reports a marker on stdout and no failure message may contain one; PROBE_RAN +# separates a failed leg from a pod that never ran. 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 + attempt_out="$(kubectl --context="${KUBECTL_CONTEXT}" run "coredns-probe-$$-${probe_attempt}" \ + --rm --attach --quiet --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")" || 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. + [[ "${probe}" == *RESOLVE_OK* ]] && break + if ((probe_attempt < probe_max)); then + echo " the cluster is not resolving yet; 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 + 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 From d74e67737126f40d68439503840615a7c0ee22da Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Sat, 8 Aug 2026 10:58:33 -0700 Subject: [PATCH 02/22] atenet/router: bind the Envoy ingress listeners dual-stack The HTTP and HTTPS ingress listeners bound 0.0.0.0 only, so on a dual-stack cluster Envoy answered on the router Service's IPv4 ClusterIP and on nothing at all for IPv6. Each primary socket now carries an additional "::" address on the same port. Ipv4Compat stays false on the additional address: clearing IPV6_V6ONLY would collide with the primary already bound to that port. Leaving the primary alone is what keeps an IPv4-only cluster unchanged -- with the caveat that a node lacking AF_INET6 entirely could not bind "::" and the listener would not come up. First of three commits binding atenet's gateways dual-stack. (cherry picked from commit 501991d285a1efeb9a146a029136e2eb140001f4) --- cmd/atenet/internal/router/xds.go | 23 ++++++++++++++ cmd/atenet/internal/router/xds_test.go | 44 +++++++++++++++++++------- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index e2d76d8bf..8a02f4e23 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{ diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 6fa5c428b..6a1569275 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 From 7684e39bb21b0731d220ffbfcd7e00bd92db7142 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 12 Aug 2026 21:41:31 -0700 Subject: [PATCH 03/22] atenet/router: bind the admin socket and Service dual-stack The Envoy admin socket bound 0.0.0.0, and the atenet-router Service carried no ipFamilyPolicy -- which the API server defaults to SingleStack, one IPv4 ClusterIP and nothing else. Between them the router had no IPv6 address to answer on. The socket now binds "::" with ipv4_compat, one socket for both families, and the Service asks for PreferDualStack. bootstrap.v3.Admin takes a single address and has no additional_addresses, so the ingress listeners' shape is not available here; ipv4_compat is what makes the one socket serve both families. It is load-bearing: dataplane.go health-checks the admin listener over http://127.0.0.1:9901/ready, so a bare "::" would report the dataplane component of /statusz unhealthy. Prefer, not Require, keeps the Service valid on a single-stack cluster; spec.ipFamilies is left alone because the primary family is immutable and the API server appends the secondary itself. (cherry picked from commit 2a21292a3262d4a642c723550881d253737e410d) --- cmd/atenet/internal/router/dataplane.go | 2 ++ manifests/ate-install/atenet-router.yaml | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) 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/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: From eb2579402d1bed1f359266b9561ec8714598a825 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 13 Aug 2026 07:20:18 -0700 Subject: [PATCH 04/22] atenet/egress: bind the Envoy sockets and Service dual-stack The gateway's admin and :443 sockets bound 0.0.0.0, so on an IPv6-primary cluster the kubelet probed the pod on its only address and atenet-egress crashlooped -- Envoy started fine and logged "admin address: 0.0.0.0:15000" -- while an actor's CONNECT had no v6 path in. Both sockets now bind "::" with ipv4_compat, and the Service asks for PreferDualStack so a dual-stack cluster hands out an IPv6 ClusterIP to reach them on. One socket here rather than the ingress listeners' pair: IPv4 peers then arrive as ::ffff: addresses, and nothing on this path reads the peer -- actor identity comes from the client certificate and the access log records the cert SAN. ipv4_compat also has to stay on the admin socket, because the ext-proc sidecar's drainer dials 127.0.0.1:15000 and envoydrain.go reads a refusal there as "Envoy already exited", skipping the drain silently. Last of three. (cherry picked from commit 2549657bc13650207b28ae49a80b7e2e5e790c5e) --- manifests/ate-install/atenet-egress.yaml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/manifests/ate-install/atenet-egress.yaml b/manifests/ate-install/atenet-egress.yaml index 591ef31fc..a655a3759 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 @@ -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: From 504170a287a62d4bdeb36a91715ec473f6a762ad Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 20:39:32 -0700 Subject: [PATCH 05/22] hack/verify: keep the gateway Envoy admin sockets dual-stack Both gateway admin sockets bind "::" with ipv4_compat, and the flag is what keeps their in-pod callers working: dataplane.go health-checks the router's over IPv4 loopback, and envoydrain.go dials the egress one the same way and reads a refusal as "Envoy already exited", skipping the drain without reporting an error. No Go test, golden file, or verify script read either manifest, so dropping the flag would have failed silently. make verify now rejects an admin socket that binds "::" without it. (cherry picked from commit 4b478a0ead243a0f8f77af9e2ed969996874d2cd) --- hack/verify/atenet-admin-bind.sh | 41 ++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100755 hack/verify/atenet-admin-bind.sh 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}" From 9146916cc422e9a5d33c9dfa1cda836249f4e735 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 10:24:50 -0700 Subject: [PATCH 06/22] atenet/router: bind the CONNECT listeners dual-stack too The CONNECT-terminating listeners landed after the first commit of this series, so they kept a bare 0.0.0.0 socket while ingress HTTP and HTTPS gained their "::" pair. Give them the same additional address, so all four of the router's socket listeners answer on both families. Both are port-gated and no e2e suite configures them yet, which is why nothing caught this; the internal main_internal listener has no socket and needs nothing. (cherry picked from commit 54c727d22843556dfcbd06a4910087efe2114ba2) --- cmd/atenet/internal/router/xds.go | 2 ++ cmd/atenet/internal/router/xds_test.go | 8 +++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 8a02f4e23..903ac1fff 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -1236,6 +1236,7 @@ func (x *XdsServer) buildConnectTerminateListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.connectPlainTextPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ @@ -1269,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 6a1569275..ea39fb7c4 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -374,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()) From 729f95f177ac22dee06e578e44363185e88a3055 Mon Sep 17 00:00:00 2001 From: lubingtan Date: Wed, 5 Aug 2026 20:58:54 +0800 Subject: [PATCH 07/22] atunnel: support IPv6 original destination lookup TCPOriginalDestination read only the IPv4 SOL_IP/SO_ORIGINAL_DST, so an actor's IPv6 connection redirected into the transparent egress listener had no destination to dial and the proxy failed it. Read IP6T_SO_ORIGINAL_DST too, falling back to it only when the IPv4 lookup returns ENOENT, so unrelated IPv4 failures keep their own error. One step towards dual-stack actor networking; the actor veth and its nftables rules are still IPv4-only. Co-authored-by: Yuan Gao (cherry picked from commit d8527b5e8ea24e588f694da726ee76784d11c41b) --- internal/atunnel/original_dst_linux.go | 68 +++- internal/atunnel/original_dst_linux_test.go | 395 ++++++++++++++++++++ 2 files changed, 444 insertions(+), 19 deletions(-) create mode 100644 internal/atunnel/original_dst_linux_test.go 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) + } + }) +} From 656d0009c54ab2e5e08d729a4772c382c37fb79a Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 10:22:32 -0700 Subject: [PATCH 08/22] ateom: drop the family from the atunnel ingress listen defaults Both ateom herders defaulted the actor ingress flags to "0.0.0.0:443" and "0.0.0.0:444", which reads as IPv4-only. It never was: Go treats an unspecified address as a wildcard and binds it dual-stack, so the sockets already served both families. Spell the defaults ":443" and ":444" so the flag says what it does, and note why in a comment. Part of the dual-stack actor networking series; no behavior change. (cherry picked from commit 51b2cbef37e1e2c6699fb7d8eee64e968f222be9) --- cmd/ateom-gvisor/main.go | 7 +++++-- cmd/ateom-microvm/main.go | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) 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") From d6a11c9d90882915f1ae95291af651048de3f2f2 Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Wed, 19 Aug 2026 09:53:17 -0700 Subject: [PATCH 09/22] ateomnet: enable IPv6 forwarding in the worker pod netns The actor veth and pod eth0 sit in the same netns, and only net.ipv4.ip_forward was enabled there, so ip6_forward() dropped every actor IPv6 packet, DNS queries included, on a dual-stack or IPv6-only cluster. Write net.ipv6.conf.all.forwarding as well, treating a missing path as nothing to enable so a netns with IPv6 compiled out still comes up. Part of #945; the actor veth itself is still IPv4-only, so nothing generates that traffic yet. Co-authored-by: Yuan Gao (cherry picked from commit 29fe4bc8852137c10e0a91b7499e3a86475a8a6b) --- internal/ateomnet/net.go | 41 ++++++++++--- internal/ateomnet/write_sysctl_test.go | 84 ++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 internal/ateomnet/write_sysctl_test.go diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 91203a8e0..1efb5ce8a 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -192,31 +192,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 } @@ -565,7 +586,7 @@ func SetupActorNetwork(ctx context.Context, cfg NetworkConfig) (retErr error) { 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/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) + } + }) +} From dd7b8610995193c94a7c35918c283008f2dd3f8f Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Mon, 10 Aug 2026 20:57:30 +0000 Subject: [PATCH 10/22] ateomnet: give the actor veth an IPv6 address and dual-stack nftables rules Add an fd00:169:254::/126 point-to-point pair alongside the existing IPv4 addresses, with a matching ::/0 default route, and move the actor nftables table from ip to inet so a single table carries both families. Each payload match now guards on NFPROTO to stay off the other family's packets, and teardown lists the inet family too: naming the wrong family there dumps empty, takes the "already clean" path, and silently leaks the table. Assign the IPv6 addresses with IFA_F_NODAD instead of writing the accept_dad sysctl. The ateom container is unprivileged, so containerd mounts /proc/sys read-only and the write failed with EROFS, taking SetupActorNetwork and every actor start down with it on both sandbox classes. A root-gated assertion pins the flag; the existing tests run as real root, where the sysctl is writable and the bug is invisible. (cherry picked from commit 1d36862f1749289edd1c20b68ced8f831a41eadd) --- internal/ateomnet/net.go | 109 ++++++++++++++++++++++++++-- internal/ateomnet/net_linux_test.go | 63 ++++++++++++++++ 2 files changed, 167 insertions(+), 5 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 1efb5ce8a..692947ac9 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,15 @@ 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 +} + // MustParseMAC parses a MAC address string into a net.HardwareAddr, panicking on error. func MustParseMAC(s string) net.HardwareAddr { m, err := net.ParseMAC(s) @@ -107,6 +137,10 @@ 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 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 +151,13 @@ func ConfigureActorVeth(ctx context.Context) error { }); err != nil { return fmt.Errorf("while installing actor default route: %w", err) } + 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 } @@ -250,9 +291,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 @@ -269,7 +307,7 @@ func InstallActorNftablesRules(egressPort uint16) error { c := &nftables.Conn{} table := &nftables.Table{ - Family: nftables.TableFamilyIPv4, + Family: nftables.TableFamilyINet, Name: ActorNftTableName, } c.AddTable(table) @@ -284,6 +322,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", @@ -297,6 +338,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{ @@ -327,7 +373,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) } @@ -350,6 +396,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, @@ -364,6 +416,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}, @@ -393,6 +471,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) { @@ -578,6 +674,9 @@ 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) } + if err := netlink.AddrReplace(hostLink, HostVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning host veth ipv6 address: %w", err) + } if err := netlink.LinkSetUp(hostLink); err != nil { return fmt.Errorf("while bringing up host veth: %w", err) } diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index b9c8ac45f..d74442da6 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -26,6 +26,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 @@ -86,6 +87,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 +136,32 @@ 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) +} + // 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 @@ -143,6 +190,7 @@ 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, @@ -162,6 +210,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") @@ -215,9 +264,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 +288,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) From 061f8cf1e4f463dfd64937216c5c9b04cb834433 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 20:52:29 -0700 Subject: [PATCH 11/22] ateomnet: skip the actor veth IPv6 setup when the netns has IPv6 disabled An IPv4-only cluster leaves net.ipv6.conf.all.disable_ipv6=1 in the worker pod netns, which is the default on IPv4-only GKE, and netlink there rejects the veth's IPv6 address with EPERM. The assignment sits on the path of every SetupActorNetwork call site, so actor startup went from working to failing outright and the actor never left ResumeGoldenActor. Gate the IPv6 address and default route on a per-link disable_ipv6 read, leaving the interior IPv4-only on those clusters instead of failing. A root-gated test covers a netns with IPv6 disabled; reverting the gate reproduces the EPERM against it. (cherry picked from commit 42e7c46aa1c337473b9b46dd948bec70d457f2ed) --- internal/ateomnet/net.go | 43 ++++++++++--- internal/ateomnet/net_linux_test.go | 99 +++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 10 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 692947ac9..8c6d90cad 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -101,6 +101,20 @@ func MustParseIPv6(s string) net.IP { return ip } +// LinkIPv6Enabled reports whether IPv6 addresses can be assigned to the named +// link in the current netns. An IPv4-only cluster leaves disable_ipv6=1 in the +// worker pod netns — the default on IPv4-only GKE — and netlink then rejects +// every IPv6 address with EPERM; a kernel built without IPv6 has no sysctl at +// all. Either way the actor interior stays IPv4-only rather than failing to +// start. +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' +} + // MustParseMAC parses a MAC address string into a net.HardwareAddr, panicking on error. func MustParseMAC(s string) net.HardwareAddr { m, err := net.ParseMAC(s) @@ -137,8 +151,11 @@ 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 err := netlink.AddrReplace(actorLink, ActorVethIPv6Addr); err != nil { - return fmt.Errorf("while assigning actor veth ipv6 address: %w", err) + actorIPv6 := LinkIPv6Enabled(ActorVethName) + if actorIPv6 { + 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 { @@ -151,12 +168,14 @@ func ConfigureActorVeth(ctx context.Context) error { }); err != nil { return fmt.Errorf("while installing actor default route: %w", err) } - 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) + if actorIPv6 { + 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 @@ -674,8 +693,12 @@ 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) } - if err := netlink.AddrReplace(hostLink, HostVethIPv6Addr); err != nil { - return fmt.Errorf("while assigning host veth ipv6 address: %w", err) + if LinkIPv6Enabled(HostVethName) { + if err := netlink.AddrReplace(hostLink, HostVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning host veth ipv6 address: %w", err) + } + } else { + slog.Info("IPv6 disabled in the worker pod netns; actor networking is IPv4-only", "link", HostVethName) } if err := netlink.LinkSetUp(hostLink); err != nil { return fmt.Errorf("while bringing up host veth: %w", err) diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index d74442da6..3897529f8 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -19,6 +19,7 @@ package ateomnet import ( "context" "errors" + "os" "runtime" "testing" @@ -247,6 +248,104 @@ func TestSetupActorNetworkFinalState(t *testing.T) { }) } +// 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) + } + } +} + +// 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) + } +} + +// TestSetupActorNetworkIPv4OnlyNetNS covers a worker pod whose netns has IPv6 +// disabled, which is the default on an IPv4-only GKE cluster. Assigning the veth +// its IPv6 address there fails with EPERM, and because that happens on the path +// of every SetupActorNetwork call site an ungated attempt takes actor startup +// from working to totally broken — the actor never leaves ResumeGoldenActor. +// The IPv4 half must come up exactly as it does with IPv6 available. +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) + + routes, err := netlink.RouteList(actor, netlink.FAMILY_V4) + if err != nil { + t.Fatalf("listing interior routes: %v", err) + } + var haveDefault bool + for _, route := range routes { + ones := 0 + if route.Dst != nil { + ones, _ = route.Dst.Mask.Size() + } + if (route.Dst == nil || ones == 0) && route.Gw.Equal(ActorVethGwIP) { + haveDefault = true + } + } + if !haveDefault { + t.Errorf("interior netns has no default route via %s, got %v", ActorVethGateway, routes) + } + return nil + }); err != nil { + t.Fatalf("inspecting interior netns: %v", err) + } + }) +} + // TestSetupActorNetworkIsRepeatable covers the activation cycle a reused worker // runs: set up, tear down, set up again. The second setup has to succeed against // whatever the first one left behind. From 4476b8007d4026e950c23da0850e61189fd5c061 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 22:19:45 -0700 Subject: [PATCH 12/22] atenet/egress: resolve upstream names on both address families The egress Envoy pinned dns_lookup_family to V4_ONLY, so it asked only for A records. On an IPv6-only cluster no upstream name resolves and no actor can reach the internet. AUTO tries AAAA and falls back to A, so IPv4-only clusters behave as before. One step of the IPv6 egress work, and not the one that unblocks it -- actor egress still stops earlier, in atunnel's original-destination lookup. (cherry picked from commit de81578a5f9387adc7f4d626986426c632f3be85) --- manifests/ate-install/atenet-egress-with-sdsmint.yaml | 8 ++++---- manifests/ate-install/atenet-egress.yaml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) 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 a655a3759..64153ea6f 100644 --- a/manifests/ate-install/atenet-egress.yaml +++ b/manifests/ate-install/atenet-egress.yaml @@ -145,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 @@ -182,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 From 2bb2cc94deb337d861bfd5c37b5e262a65c3fa82 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Fri, 14 Aug 2026 11:03:44 -0700 Subject: [PATCH 13/22] ci: add an IPv6-only kind e2e job Runs the full install plus the demo and networking e2e suites against a single-stack IPv6-only kind cluster, and asserts the cluster really is v6-only so a green run cannot quietly become a second IPv4 run. It stays out of the e2e-test merge gate, so it reports IPv6 status without being able to block a PR, and it runs on every PR for now so the results are visible; the TODO on the trigger records the intended ci/ipv6 label gate. ubuntu-latest has no IPv6 egress, so the job stands up tayga for NAT64 and points CoreDNS at an upstream resolver through the well-known prefix. DNS64 is scoped to a catch-all server block: synthesizing AAAA over the cluster zones destroys the v6-only ClusterIP answers and the control plane never comes up. (cherry picked from commit 748e8412183ab0f85f4beb7bbaa8f39a5dcb2494) --- .github/workflows/e2e-ipv6.yaml | 366 ++++++++++++++++++++++++++++++++ 1 file changed, 366 insertions(+) create mode 100644 .github/workflows/e2e-ipv6.yaml 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 From bb6bcca6842be8815a5e0774178d610da17ad812 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 12:23:57 -0700 Subject: [PATCH 14/22] hack: read the IPv6 DNS probe from the pod log The probe attached to the pod to collect its markers, and an attach can end before the last write arrives. A CI run lost the registry marker that way, so the check reported a registry it could not reach -- and then refused to re-probe, because only the resolve leg was treated as a settling race. Wait for the pod to terminate and read its log instead, and close the probe with a PROBE_DONE marker so a short read is re-probed rather than read as a failed fetch. A registry that really is down still fails on the first attempt. --- hack/verify-ipv6-dns.sh | 42 +++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/hack/verify-ipv6-dns.sh b/hack/verify-ipv6-dns.sh index 2afd300ae..7f21c041c 100755 --- a/hack/verify-ipv6-dns.sh +++ b/hack/verify-ipv6-dns.sh @@ -57,16 +57,20 @@ echo "Verifying DNS from a pod..." # The registry leg fetches rather than resolves -- the hosts entry is AAAA-only, # which fails nslookup's A query but satisfies getaddrinfo. # -# --attach gives one stream and only the last leg's exit status, so each leg +# 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 -# separates a failed leg from a pod that never ran. 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. +# 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 - attempt_out="$(kubectl --context="${KUBECTL_CONTEXT}" run "coredns-probe-$$-${probe_attempt}" \ - --rm --attach --quiet --restart=Never --image=busybox:1.36 --command -- \ + 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 @@ -77,13 +81,28 @@ for ((probe_attempt = 1; probe_attempt <= probe_max; probe_attempt++)); do echo REGISTRY_OK else echo \"registry fetch failed: \$(echo \"\$out\" | tail -1)\" - fi")" || true + 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. - [[ "${probe}" == *RESOLVE_OK* ]] && break + # 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 cluster is not resolving yet; re-probing (attempt $((probe_attempt + 1)) of ${probe_max})..." + echo " the probe did not come back clean; re-probing (attempt $((probe_attempt + 1)) of ${probe_max})..." sleep 10 fi done @@ -94,6 +113,9 @@ if [[ "${probe}" != *RESOLVE_OK* || "${probe}" != *REGISTRY_OK* ]]; then 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 From 9b7fceb0e019f02ae2238f5e1da8dbd8ed846316 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 07:39:50 -0700 Subject: [PATCH 15/22] atenet/dns: answer non-A actor queries instead of SERVFAIL Before, the actor zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor. A failure reads as a temporary error rather than an answer, so clients retry it and then give up on the name; Alpine actors could not resolve each other at all, even on an IPv4-only cluster. After, those queries return a correct empty answer, and one that resolvers can cache. A unit test pins the whole rendered zone as a literal, so editing the name pattern or the suffix fails there rather than passing silently. --- cmd/atenet/internal/dns/README.md | 22 ++++++- cmd/atenet/internal/dns/corefile.go | 25 +++++++- cmd/atenet/internal/dns/corefile_test.go | 75 ++++++++++++++---------- 3 files changed, 87 insertions(+), 35 deletions(-) diff --git a/cmd/atenet/internal/dns/README.md b/cmd/atenet/internal/dns/README.md index a134e829a..e89b06b62 100644 --- a/cmd/atenet/internal/dns/README.md +++ b/cmd/atenet/internal/dns/README.md @@ -10,7 +10,6 @@ Cluster resources: * Deployment `ate-system:dns`. Label: app=dns * Service `ate-system:dns`. -* ConfigMap `ate-system:dns`. These are defined in manifests/ate-install/atenet-dns.yaml. @@ -20,16 +19,33 @@ These are defined in manifests/ate-install/atenet-dns.yaml. * Deployment `ate-system:dns`. * Service `ate-system:dns` pointing to the Deployment. -ConfigMap `ate-system:dns`: +Corefile, rendered by `corefile.go`: ``` -# Match any 'A' query for an actor name + atespace pattern under actors.resources.substrate.ate.dev +# Answer any 'A' query for an actor name + atespace pattern under actors.resources.substrate.ate.dev template IN A actors.resources.substrate.ate.dev { match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.actors\\.resources\\.substrate\\.ate\\.dev\\.$" answer "{{ .Name }} 60 IN A " + fallthrough + } +# NODATA for a well-formed actor name on any other qtype (AAAA, HTTPS, SRV, ...). + template ANY ANY actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.actors\\.resources\\.substrate\\.ate\\.dev\\.$" + rcode NOERROR + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + fallthrough + } +# Terminal catch-all: NXDOMAIN for anything else in the zone. + template ANY ANY actors.resources.substrate.ate.dev { + rcode NXDOMAIN + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" } ``` +The last two blocks keep the zone from ever answering SERVFAIL, which musl libc +maps to `EAI_AGAIN` — sinking the paired A query with it — and which cannot be +cached negatively. + ## Integration * CoreDNS: Update CoreDNS ConfigMap to add the stub resolver. diff --git a/cmd/atenet/internal/dns/corefile.go b/cmd/atenet/internal/dns/corefile.go index 0b301e7e2..2869e3a24 100644 --- a/cmd/atenet/internal/dns/corefile.go +++ b/cmd/atenet/internal/dns/corefile.go @@ -30,6 +30,11 @@ func init() { } func buildTemplate() string { + const ( + fallthroughDirective = " fallthrough" + soaDirective = ` authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)"` + ) + // Build up the corefileTemplate programmatically to make it easier to understand. var directives []string // Plugins to enable. @@ -44,9 +49,27 @@ func buildTemplate() string { directives = append(directives, fmt.Sprintf("template IN A %s {", resources.ActorDNSSuffix)) // Escape the suffix's dots so they match literally; the final \. matches the FQDN's trailing dot. escapedSuffix := strings.ReplaceAll(resources.ActorDNSSuffix, ".", `\.`) - directives = append(directives, fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix)) + actorMatch := fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix) + directives = append(directives, actorMatch) // Note the %s -- this will be filled with the router IP. directives = append(directives, ` answer "{{ .Name }} 60 IN A %s"`) + directives = append(directives, fallthroughDirective) + directives = append(directives, "}") + + // Valid actor names return NOERROR (NODATA) for non-A queries. + directives = append(directives, fmt.Sprintf("template ANY ANY %s {", resources.ActorDNSSuffix)) + directives = append(directives, actorMatch) + directives = append(directives, " rcode NOERROR") + directives = append(directives, soaDirective) + directives = append(directives, fallthroughDirective) + directives = append(directives, "}") + + // Returns rcode NXDOMAIN (Non-Existent Domain) for any query that did not + // match the valid actor regex in the previous blocks. + // TODO(#922): answer empty non-terminals with NODATA. + directives = append(directives, fmt.Sprintf("template ANY ANY %s {", resources.ActorDNSSuffix)) + directives = append(directives, " rcode NXDOMAIN") + directives = append(directives, soaDirective) directives = append(directives, "}") // Generate the template. diff --git a/cmd/atenet/internal/dns/corefile_test.go b/cmd/atenet/internal/dns/corefile_test.go index f13429e47..c8653ad7e 100644 --- a/cmd/atenet/internal/dns/corefile_test.go +++ b/cmd/atenet/internal/dns/corefile_test.go @@ -15,50 +15,63 @@ package dns import ( + "fmt" "strings" "testing" - - "github.com/agent-substrate/substrate/internal/resources" ) +// Spelled out rather than built from resources.ResourceNameRegexPattern and +// ActorDNSSuffix: the rendered zone is a wire contract, so a change to either +// constant should fail here instead of being tracked silently. +const wantCorefileFmt = `actors.resources.substrate.ate.dev:53 { + log + errors + health :8080 + ready :8181 + reload + template IN A actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\.actors\.resources\.substrate\.ate\.dev\.$" + answer "{{ .Name }} 60 IN A %s" + fallthrough + } + template ANY ANY actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\.actors\.resources\.substrate\.ate\.dev\.$" + rcode NOERROR + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + fallthrough + } + template ANY ANY actors.resources.substrate.ate.dev { + rcode NXDOMAIN + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + } +} +` + +// zoneBody strips the "# Generated at " header. +func zoneBody(t *testing.T, corefile string) string { + t.Helper() + header, body, ok := strings.Cut(corefile, "\n") + if !ok || !strings.HasPrefix(header, "# Generated at ") { + t.Fatalf("makeCoreFile() has no generated-at header, got first line %q", header) + } + return body +} + func TestMakeCoreFile(t *testing.T) { tests := []struct { name string routerIP string - expected []string }{ - { - name: "standard local IP", - routerIP: "10.240.0.10", - expected: []string{ - "actors.resources.substrate.ate.dev:53 {", - "log", - "errors", - "health :8080", - "ready :8181", - "reload", - "template IN A actors.resources.substrate.ate.dev {", - `match "^` + resources.ResourceNameRegexPattern + `\.` + resources.ResourceNameRegexPattern + `\.actors\.resources\.substrate\.ate\.dev\.$"`, - `answer "{{ .Name }} 60 IN A 10.240.0.10"`, - }, - }, - { - name: "different IP", - routerIP: "192.168.1.1", - expected: []string{ - "actors.resources.substrate.ate.dev:53 {", - `answer "{{ .Name }} 60 IN A 192.168.1.1"`, - }, - }, + {name: "cluster IP", routerIP: "10.240.0.10"}, + {name: "different cluster IP", routerIP: "192.168.1.1"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := makeCoreFile(tc.routerIP) - for _, exp := range tc.expected { - if !strings.Contains(got, exp) { - t.Errorf("makeCoreFile(%q) missing expected substring %q\nGot:\n%s", tc.routerIP, exp, got) - } + got := zoneBody(t, makeCoreFile(tc.routerIP)) + want := fmt.Sprintf(wantCorefileFmt, tc.routerIP) + if got != want { + t.Errorf("makeCoreFile(%q) rendered an unexpected Corefile\nGot:\n%s\nWant:\n%s", tc.routerIP, got, want) } }) } From 584ab0bfcb9ed61bb8db87e280e3e28bad0b8073 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Fri, 7 Aug 2026 15:56:35 -0700 Subject: [PATCH 16/22] internal/ipfamily: add ClusterIPsByFamily Splits a Service's cluster IPs into its IPv4 and IPv6 entries, returning "" for a family the Service has no address in. No behavior change on its own -- nothing calls it until the AAAA change later in this series. It is shared rather than package-local because a Service with no ipFamilyPolicy is SingleStack, so one empty family is the steady state on every cluster, not an error, and each caller would otherwise have to decide that for itself. Unit tests cover single- and dual-stack Services and the unallocated and malformed cases. --- internal/ipfamily/ipfamily.go | 60 +++++++++++++++++++ internal/ipfamily/ipfamily_test.go | 95 ++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 internal/ipfamily/ipfamily.go create mode 100644 internal/ipfamily/ipfamily_test.go 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) + } + }) + } +} From 80313cb1d9b50cdb4719f886a31fc4e21b4695e5 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 08:44:24 -0700 Subject: [PATCH 17/22] atenet/dns: hoist the Corefile generation stamp to a package var No behavior change -- buildTemplate() already ran once, from init(). The next commit renders the Corefile on every call instead, where a stamp taken inline would differ each time: reconcile compares the render against the file on disk, so it would rewrite and reload CoreDNS every tick. --- cmd/atenet/internal/dns/corefile.go | 8 +++++++- cmd/atenet/internal/dns/corefile_test.go | 13 +++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/cmd/atenet/internal/dns/corefile.go b/cmd/atenet/internal/dns/corefile.go index 2869e3a24..7b22cb521 100644 --- a/cmd/atenet/internal/dns/corefile.go +++ b/cmd/atenet/internal/dns/corefile.go @@ -25,6 +25,12 @@ import ( // corefileTemplate is a Sprintf template for the CoreDNS configuration. var corefileTemplate string +// generatedAt stamps the rendered Corefile once per process, and must not be +// recomputed per render: 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 init() { corefileTemplate = buildTemplate() } @@ -74,7 +80,7 @@ func buildTemplate() string { // Generate the template. 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") diff --git a/cmd/atenet/internal/dns/corefile_test.go b/cmd/atenet/internal/dns/corefile_test.go index c8653ad7e..58921b135 100644 --- a/cmd/atenet/internal/dns/corefile_test.go +++ b/cmd/atenet/internal/dns/corefile_test.go @@ -76,3 +76,16 @@ func TestMakeCoreFile(t *testing.T) { }) } } + +// 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") + second := makeCoreFile("10.240.0.10") + 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) + } +} From b109618d13787a450fa125b8f7fd5db755b9a05f Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 08:44:44 -0700 Subject: [PATCH 18/22] atenet/dns: publish the router's IPv6 ClusterIP as an AAAA Before, an actor name never resolved over IPv6: the zone published the router's primary cluster IP, always as an A record whatever family it was. On a dual-stack cluster the v6 address went unpublished; on an IPv6-only cluster the record was malformed, so every A query for an actor name failed. After, the zone publishes an address record per family the router has an address in, and answers empty for a family it has none in. Unit tests pin the rendered zone for each family combination, verified against the pinned coredns/coredns:1.11.1. --- cmd/atenet/internal/dns/README.md | 28 ++++- cmd/atenet/internal/dns/corefile.go | 65 +++++++----- cmd/atenet/internal/dns/corefile_test.go | 118 +++++++++++++++++---- cmd/atenet/internal/dns/dns.go | 18 ++-- cmd/atenet/internal/dns/dns_test.go | 127 +++++++++++++++++++++++ 5 files changed, 297 insertions(+), 59 deletions(-) diff --git a/cmd/atenet/internal/dns/README.md b/cmd/atenet/internal/dns/README.md index e89b06b62..cb28e837f 100644 --- a/cmd/atenet/internal/dns/README.md +++ b/cmd/atenet/internal/dns/README.md @@ -19,16 +19,26 @@ These are defined in manifests/ate-install/atenet-dns.yaml. * Deployment `ate-system:dns`. * Service `ate-system:dns` pointing to the Deployment. -Corefile, rendered by `corefile.go`: +`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. ``` # 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 } -# NODATA for a well-formed actor name on any other qtype (AAAA, HTTPS, SRV, ...). +# 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 @@ -42,9 +52,19 @@ Corefile, rendered by `corefile.go`: } ``` +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. +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 diff --git a/cmd/atenet/internal/dns/corefile.go b/cmd/atenet/internal/dns/corefile.go index 7b22cb521..32d4043b7 100644 --- a/cmd/atenet/internal/dns/corefile.go +++ b/cmd/atenet/internal/dns/corefile.go @@ -22,26 +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)"` +) // generatedAt stamps the rendered Corefile once per process, and must not be -// recomputed per render: reconcileCoreDNSConfig decides whether to rewrite the +// 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 init() { - corefileTemplate = buildTemplate() -} - -func buildTemplate() string { - const ( - fallthroughDirective = " fallthrough" - soaDirective = ` authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)"` - ) - - // Build up the corefileTemplate programmatically to make it easier to understand. +// 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") @@ -52,17 +57,19 @@ 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, ".", `\.`) actorMatch := fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix) - directives = append(directives, actorMatch) - // Note the %s -- this will be filled with the router IP. - directives = append(directives, ` answer "{{ .Name }} 60 IN A %s"`) - directives = append(directives, fallthroughDirective) - directives = append(directives, "}") - // Valid actor names return NOERROR (NODATA) for non-A queries. + 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") @@ -78,7 +85,7 @@ func buildTemplate() string { directives = append(directives, soaDirective) directives = append(directives, "}") - // Generate the template. + // Generate the Corefile. b := strings.Builder{} fmt.Fprintf(&b, "# Generated at %s\n", generatedAt) fmt.Fprintf(&b, "%s:53 {\n ", resources.ActorDNSSuffix) @@ -88,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 58921b135..c20792662 100644 --- a/cmd/atenet/internal/dns/corefile_test.go +++ b/cmd/atenet/internal/dns/corefile_test.go @@ -15,37 +15,81 @@ package dns import ( - "fmt" "strings" "testing" ) -// Spelled out rather than built from resources.ResourceNameRegexPattern and -// ActorDNSSuffix: the rendered zone is a wire contract, so a change to either -// constant should fail here instead of being tracked silently. -const wantCorefileFmt = `actors.resources.substrate.ate.dev:53 { +// 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 - template IN A actors.resources.substrate.ate.dev { - match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\.actors\.resources\.substrate\.ate\.dev\.$" - answer "{{ .Name }} 60 IN A %s" - fallthrough - } - template ANY ANY actors.resources.substrate.ate.dev { - match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\.actors\.resources\.substrate\.ate\.dev\.$" +` + wantZoneTail = ` template ANY ANY actors.resources.substrate.ate.dev { + ` + actorMatchDirective + ` rcode NOERROR - authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + ` + soaAuthorityDirective + ` fallthrough } template ANY ANY actors.resources.substrate.ate.dev { rcode NXDOMAIN - authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + ` + 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 + + wantZoneNoAddresses = wantZoneHead + wantZoneTail +) // zoneBody strips the "# Generated at " header. func zoneBody(t *testing.T, corefile string) string { @@ -60,18 +104,46 @@ func zoneBody(t *testing.T, corefile string) string { func TestMakeCoreFile(t *testing.T) { tests := []struct { name string - routerIP string + routerV4 string + routerV6 string + want string }{ - {name: "cluster IP", routerIP: "10.240.0.10"}, - {name: "different cluster IP", routerIP: "192.168.1.1"}, + { + // 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: "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 := zoneBody(t, makeCoreFile(tc.routerIP)) - want := fmt.Sprintf(wantCorefileFmt, tc.routerIP) - if got != want { - t.Errorf("makeCoreFile(%q) rendered an unexpected Corefile\nGot:\n%s\nWant:\n%s", tc.routerIP, got, want) + 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) } }) } @@ -83,8 +155,8 @@ func TestMakeCoreFile(t *testing.T) { // 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") - second := makeCoreFile("10.240.0.10") + 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) From 97500223cd1e3e87d0a58aa4d6fe7a1755852414 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 16:35:16 -0700 Subject: [PATCH 19/22] e2e: add an in-cluster DNS client and router IP family helpers Nothing in the e2e harness could query the actor DNS zone. Suites reach actors by port-forwarding atenet-router and passing the actor name as a Host header, so the zone CoreDNS actually serves went unasserted, and a suite that wanted to check it had no way to distinguish an empty answer from a server failure. Adds a DNS client that port-forwards the atenet DNS Service and reports the rcode class alongside the addresses, plus a helper for the router's ClusterIP in each family. First of two commits; the tests that use these follow. clusterIPsByFamily here is a stopgap that #938 replaces with internal/ipfamily. --- internal/e2e/dns_client.go | 162 ++++++++++++++++++++ internal/e2e/ipfamily.go | 69 +++++++++ internal/e2e/router_client.go | 11 +- internal/e2e/statusz.go | 2 +- internal/e2e/suites/networking/dns_test.go | 163 +++++++++++++++++++++ 5 files changed, 402 insertions(+), 5 deletions(-) create mode 100644 internal/e2e/dns_client.go create mode 100644 internal/e2e/ipfamily.go create mode 100644 internal/e2e/suites/networking/dns_test.go 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..cf1f6f57b --- /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.ActorRef{Atespace: networkingAtespace, Name: "dns-probe"}.DNSName() +} + +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) + } +} From e3299763610a454a70cffe4791dc73274c036f64 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 16:35:23 -0700 Subject: [PATCH 20/22] e2e: assert the actor DNS zone answers benign rcodes The zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor -- and no test caught it, because Go's resolver masks a SERVFAIL that musl treats as fatal. These assert the rcode class rather than the record: a non-A qtype and a name that misses the actor regex must come back NODATA or NXDOMAIN, and an A query must carry the router's ClusterIP. A separate test covers the AAAA record, skipped where the router has no v6 address. Second of two commits. The assertions are red until #874 and #938 land, so this stays a draft until then. Part of #246. --- internal/e2e/suites/networking/dns_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/e2e/suites/networking/dns_test.go b/internal/e2e/suites/networking/dns_test.go index cf1f6f57b..b8abf001c 100644 --- a/internal/e2e/suites/networking/dns_test.go +++ b/internal/e2e/suites/networking/dns_test.go @@ -28,7 +28,7 @@ import ( // These tests therefore need no actor fixture — they are asserting the zone's // behavior, not an actor's. func probeActorDNSName() string { - return resources.ActorRef{Atespace: networkingAtespace, Name: "dns-probe"}.DNSName() + return resources.ActorDNSName(resources.ActorRef{Atespace: networkingAtespace, Name: "dns-probe"}) } func mustDNSClient(t *testing.T, ctx context.Context) *e2e.DNSClient { From b816e3bdd8c6cade294a117ce7c2efb460ce70be Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 16:37:14 -0700 Subject: [PATCH 21/22] e2e: assert the router ingress works on every IP family Nothing checked that the router's dataplane listeners bind more than an IPv4 socket, and nothing reached an actor over the router's IPv6 ClusterIP. Every other path a test has into the router -- a port-forward, the pods/proxy and services/proxy subresources -- is mediated by the API server, which picks the family, so no existing test could have caught a listener that lost its IPv6 socket. Reads the bound addresses from Envoy's own admin /listeners, and drives an in-cluster probe pod at the router over each ClusterIP in turn. Red until #911 binds those sockets, so this stays a draft until then. The per-family probe skips on a single-stack cluster. Part of #246. --- .../suites/networking/ingress_family_test.go | 280 ++++++++++++++++++ 1 file changed, 280 insertions(+) create mode 100644 internal/e2e/suites/networking/ingress_family_test.go 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 +} From 54783ec054e397af42ac8ff88db67186b9017e79 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 19:03:24 -0700 Subject: [PATCH 22/22] ateomnet: give the actor IPv6 only when the worker pod has it LinkIPv6Enabled answers whether the kernel will accept an IPv6 address on a link, not whether the cluster routes IPv6. IPv4-only kind leaves disable_ipv6 at 0, so the actor got fd00:169:254::2 and a ::/0 route it could not use, Go's destination sorting preferred the AAAA of any dual-stack host, and the egress fetch died mid-response -- the IPv4 e2e job has been red since. Pair the capability read with a check that the worker pod's own eth0 carries a global IPv6 address, and the actor stays IPv4-only wherever the pod is. Decide it once in the pod netns and pass it into ConfigureActorVeth. The interior namespace is created fresh, so its own sysctl always said IPv6 was available whatever the pod's families were. Root-gated tests cover a pod without IPv6 and a pod whose new links have IPv6 disabled per link; dropping either half of the check reproduces its failure. (cherry picked from commit 298b6d3b67f8b581f38ee4c92dc7012a45c36b17) --- internal/ateomnet/net.go | 60 ++++-- internal/ateomnet/net_linux_test.go | 298 +++++++++++++++++++++++----- 2 files changed, 301 insertions(+), 57 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 8c6d90cad..5c41cc84f 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -102,11 +102,12 @@ func MustParseIPv6(s string) net.IP { } // LinkIPv6Enabled reports whether IPv6 addresses can be assigned to the named -// link in the current netns. An IPv4-only cluster leaves disable_ipv6=1 in the -// worker pod netns — the default on IPv4-only GKE — and netlink then rejects -// every IPv6 address with EPERM; a kernel built without IPv6 has no sysctl at -// all. Either way the actor interior stays IPv4-only rather than failing to -// start. +// 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 { @@ -115,6 +116,31 @@ func LinkIPv6Enabled(name string) bool { 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) @@ -126,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 @@ -151,8 +179,7 @@ func ConfigureActorVeth(ctx context.Context) error { if err := netlink.AddrReplace(actorLink, ActorVethAddr); err != nil { return fmt.Errorf("while assigning actor veth address: %w", err) } - actorIPv6 := LinkIPv6Enabled(ActorVethName) - if actorIPv6 { + if ipv6 { if err := netlink.AddrReplace(actorLink, ActorVethIPv6Addr); err != nil { return fmt.Errorf("while assigning actor veth ipv6 address: %w", err) } @@ -168,7 +195,7 @@ func ConfigureActorVeth(ctx context.Context) error { }); err != nil { return fmt.Errorf("while installing actor default route: %w", err) } - if actorIPv6 { + if ipv6 { if err := netlink.RouteReplace(&netlink.Route{ LinkIndex: actorLink.Attrs().Index, Gw: ActorVethIPv6GwIP, @@ -693,18 +720,27 @@ 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) } - if LinkIPv6Enabled(HostVethName) { + // 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.Info("IPv6 disabled in the worker pod netns; actor networking is IPv4-only", "link", HostVethName) + 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) } diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index 3897529f8..1f7634f8d 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -19,6 +19,7 @@ package ateomnet import ( "context" "errors" + "net" "os" "runtime" "testing" @@ -77,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. @@ -163,6 +192,43 @@ func assertIPv6AddrNoDAD(t *testing.T, link netlink.Link, cidr string) { 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 @@ -176,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) } @@ -194,10 +263,14 @@ func TestSetupActorNetworkFinalState(t *testing.T) { 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 { @@ -219,28 +292,8 @@ 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) - } - // 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 - } - var haveDefault bool - for _, route := range routes { - if isDefault(route) && route.Gw.Equal(ActorVethGwIP) { - haveDefault = true - } - } - if !haveDefault { - t.Errorf("interior netns has no default route via %s, got %v", ActorVethGateway, routes) - } + 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) @@ -260,6 +313,20 @@ func disableIPv6(t *testing.T) { } } +// 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() @@ -272,12 +339,31 @@ func assertNoIPv6Addr(t *testing.T, link netlink.Link) { } } +// 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, which is the default on an IPv4-only GKE cluster. Assigning the veth -// its IPv6 address there fails with EPERM, and because that happens on the path -// of every SetupActorNetwork call site an ungated attempt takes actor startup -// from working to totally broken — the actor never leaves ResumeGoldenActor. -// The IPv4 half must come up exactly as it does with IPv6 available. +// 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() @@ -322,23 +408,145 @@ func TestSetupActorNetworkIPv4OnlyNetNS(t *testing.T) { } assertNoIPv6Addr(t, actor) - 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, 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) + } + 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 { - ones := 0 - if route.Dst != nil { - ones, _ = route.Dst.Mask.Size() - } - if (route.Dst == nil || ones == 0) && 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)