From 2022b6558f280fc71861d8f8f1dd0318980b8e8a Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 08:49:40 -0700 Subject: [PATCH 01/26] 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 f413e5c953..c2ffdab58b 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/26] 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 e2d76d8bf9..8a02f4e237 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 6fa5c428b7..6a15692757 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/26] 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 bd9f2abfc4..ff886dcaaa 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 e05e06efb7..462e2a8ad0 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/26] 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 591ef31fcb..a655a37592 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/26] 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 0000000000..86090c0168 --- /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/26] 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 8a02f4e237..903ac1fffb 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 6a15692757..ea39fb7c4d 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/26] 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 07dd0f9344..8b4309119d 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 0000000000..4f26d98055 --- /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 1771134883504452547abf2dde073f7c65605d40 Mon Sep 17 00:00:00 2001 From: lubingtan Date: Fri, 21 Aug 2026 11:46:21 +0800 Subject: [PATCH 08/26] atunnel: preserve IPv4 original destination errors --- internal/atunnel/original_dst_linux.go | 16 +++++-- internal/atunnel/original_dst_linux_test.go | 47 +++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/internal/atunnel/original_dst_linux.go b/internal/atunnel/original_dst_linux.go index 8b4309119d..b9d1717479 100644 --- a/internal/atunnel/original_dst_linux.go +++ b/internal/atunnel/original_dst_linux.go @@ -42,15 +42,23 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { if err != nil { return "", fmt.Errorf("atunnel: acquiring TCP syscall connection: %w", err) } + // The IPv6 option is only meaningful on an AF_INET6 socket: on AF_INET the + // kernel returns EOPNOTSUPP, which would mask the real IPv4 error. A + // v4-mapped local address still means an IPv4 flow, so To4 is the test. + local, ok := tcpConn.LocalAddr().(*net.TCPAddr) + if !ok { + return "", fmt.Errorf("atunnel: original destination requires a TCP local address, got %T", tcpConn.LocalAddr()) + } + isIPv6 := local.IP.To4() == nil var sockoptErr error var destination string if err := rawConn.Control(func(fd uintptr) { 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) { + // A pure-IPv6 socket leaves the inet addresses zeroed, so the IPv4 + // conntrack lookup always misses with ENOENT. That is the redirected + // IPv6 connection, and the only case worth retrying. + if isIPv6 && errors.Is(sockoptErr, unix.ENOENT) { destination, sockoptErr = originalIPv6Destination(fd) } }); err != nil { diff --git a/internal/atunnel/original_dst_linux_test.go b/internal/atunnel/original_dst_linux_test.go index 4f26d98055..ef647c331b 100644 --- a/internal/atunnel/original_dst_linux_test.go +++ b/internal/atunnel/original_dst_linux_test.go @@ -37,6 +37,53 @@ import ( "github.com/agent-substrate/substrate/internal/roottest" ) +// TestTCPOriginalDestinationPreservesErrno covers the failure path on an +// ordinary connection that no REDIRECT rule touched. The IPv4 lookup misses +// and reports ENOENT; that error must reach the caller. Retrying the IPv6 +// option on an AF_INET socket would replace it with EOPNOTSUPP, which says +// nothing about why the lookup failed. +// +// It runs in a fresh namespace because conntrack tracks loopback in any +// namespace that has nftables rules — including the one Docker runs in — and a +// tracked connection returns its real destination instead of missing. +func TestTCPOriginalDestinationPreservesErrno(t *testing.T) { + roottest.Require(t, "CAP_SYS_ADMIN for a network namespace with no conntrack hooks") + + ns := newTestNetNS(t) + if err := ateomnet.NetNSDo(context.Background(), ns, func(context.Context) error { + loopback, err := netlink.LinkByName("lo") + if err != nil { + return err + } + if err := netlink.LinkSetUp(loopback); err != nil { + return err + } + + listener, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + return err + } + defer listener.Close() + client, err := net.DialTimeout("tcp4", listener.Addr().String(), time.Second) + if err != nil { + return err + } + defer client.Close() + server, err := listener.Accept() + if err != nil { + return err + } + defer server.Close() + + if _, err := TCPOriginalDestination(server); !errors.Is(err, unix.ENOENT) { + return fmt.Errorf("want the IPv4 lookup's ENOENT, got %w", err) + } + return nil + }); err != nil { + t.Fatal(err) + } +} + func TestTCPOriginalDestination(t *testing.T) { roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") From 5728f51ab67117aca5693fa77681b01e1ca80540 Mon Sep 17 00:00:00 2001 From: lubingtan Date: Fri, 21 Aug 2026 11:50:58 +0800 Subject: [PATCH 09/26] atunnel: stabilize original destination tests --- internal/atunnel/original_dst_linux_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/atunnel/original_dst_linux_test.go b/internal/atunnel/original_dst_linux_test.go index ef647c331b..3a9dbfca2a 100644 --- a/internal/atunnel/original_dst_linux_test.go +++ b/internal/atunnel/original_dst_linux_test.go @@ -114,7 +114,7 @@ func TestTCPOriginalDestination(t *testing.T) { // 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) + conn, err := net.DialTimeout("tcp4", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), 10*time.Second) if err == nil { _ = conn.Close() } @@ -122,7 +122,7 @@ func TestTCPOriginalDestination(t *testing.T) { }) }() - if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(10 * time.Second)); err != nil { t.Fatal(err) } redirected, err := redirectListener.Accept() @@ -164,7 +164,7 @@ func TestTCPOriginalDestinationIPv6(t *testing.T) { 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) + conn, err := net.DialTimeout("tcp6", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), 10*time.Second) if err == nil { _ = conn.Close() } @@ -172,7 +172,7 @@ func TestTCPOriginalDestinationIPv6(t *testing.T) { }) }() - if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(10 * time.Second)); err != nil { t.Fatal(err) } redirected, err := redirectListener.Accept() From a87ff5afecc0cda94d9b238dabce9b8297b4a841 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 10:22:32 -0700 Subject: [PATCH 10/26] 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 0620c09e3f..9e77aae2f2 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 6613fcd9b3..d4dde4680c 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 b7fd047d946db4cd205235bf405fbd2b7152b903 Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Sat, 15 Aug 2026 12:57:09 +0000 Subject: [PATCH 11/26] ateomnet: enable IPv6 forwarding in worker pod netns EnableIPv4Forwarding now also writes /proc/sys/net/ipv6/conf/all/forwarding so actor IPv6 traffic (including DNS queries) is routed between the actor veth and pod eth0 instead of being dropped by ip6_forward() on dual-stack / IPv6-only clusters. Factor the sysctl write into writeSysctlIfUnset preserving the original read-only remount/restore behavior, and add unit coverage for its fast paths. Fixes: agent-substrate/substrate#945 --- internal/ateomnet/net.go | 28 ++++++++- internal/ateomnet/write_sysctl_test.go | 84 ++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 internal/ateomnet/write_sysctl_test.go diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 91203a8e04..2d3daa4737 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -193,6 +193,9 @@ func PodIPv4() (net.IP, error) { } // EnableIPv4Forwarding enables IPv4 forwarding in the current network namespace. +// It also enables IPv6 forwarding so actor IPv6 traffic (including DNS queries +// on IPv6-capable clusters) is routed between the veth and eth0 instead of +// being dropped by ip6_forward(). func EnableIPv4Forwarding() 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 @@ -203,20 +206,41 @@ func EnableIPv4Forwarding() error { // 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" + if err := writeSysctlIfUnset(path); err != nil { + return fmt.Errorf("while enabling IPv4 forwarding in worker pod netns: %w", err) + } + // IPv6 forwarding: actor packets that arrive on the veth and leave via eth0 + // are IPv6 on dual-stack / IPv6-only clusters. Without + // net.ipv6.conf.all.forwarding the kernel drops every IPv6 packet in + // ip6_forward(), including the actor's DNS queries. conf.all.forwarding=1 + // also implies the per-interface default, so a single write covers the veth + // and eth0. + const v6path = "/proc/sys/net/ipv6/conf/all/forwarding" + if err := writeSysctlIfUnset(v6path); err != nil { + return fmt.Errorf("while enabling IPv6 forwarding in worker pod netns: %w", err) + } + return nil +} + +// writeSysctlIfUnset writes "1\n" to a sysctl path unless it already reads "1". +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 } + // 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 } diff --git a/internal/ateomnet/write_sysctl_test.go b/internal/ateomnet/write_sysctl_test.go new file mode 100644 index 0000000000..b4cb1db169 --- /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 a1e5ca55b2bbb4f586bd6ab98e768d4079e6d268 Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Sun, 16 Aug 2026 19:12:06 +0000 Subject: [PATCH 12/26] ateomnet: return nil when sysctl path missing in writeSysctlIfUnset IPv6 sysctls are absent on kernels with IPv6 disabled (e.g. some containers set net.ipv6.conf.* only when IPv6 is enabled). Treat a missing path as 'nothing to enable' instead of forcing a remount and failing, matching the documented behavior. --- internal/ateomnet/net.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 2d3daa4737..a2a9501787 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -223,6 +223,8 @@ func EnableIPv4Forwarding() error { } // writeSysctlIfUnset writes "1\n" to a sysctl path unless it already reads "1". +// If the path does not exist (e.g. IPv6 sysctls on a kernel with IPv6 disabled), +// it returns nil — IPv6 forwarding is simply unavailable, not an error. func writeSysctlIfUnset(path string) error { if b, err := os.ReadFile(path); err == nil && len(b) > 0 && b[0] == '1' { return nil @@ -230,6 +232,10 @@ func writeSysctlIfUnset(path string) error { if err := os.WriteFile(path, []byte("1\n"), 0o644); err == nil { return nil } + if _, err := os.Stat(path); os.IsNotExist(err) { + // Path absent (e.g. IPv6 disabled in kernel): nothing to enable. + 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. From 3753982725aaf3272d8e3bce5b73a36d40f03ad0 Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Fri, 21 Aug 2026 13:04:19 +0000 Subject: [PATCH 13/26] ateomnet: rename EnableIPv4Forwarding to EnableForwarding The helper has enabled both address families since IPv6 forwarding was added; the name now says so. The single call site in SetupActorNetwork is updated along with the doc comment. --- internal/ateomnet/net.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index a2a9501787..1f0fbc92a8 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -192,11 +192,11 @@ func PodIPv4() (net.IP, error) { return nil, fmt.Errorf("pod eth0 has no IPv4 address") } -// EnableIPv4Forwarding enables IPv4 forwarding in the current network namespace. -// It also enables IPv6 forwarding so actor IPv6 traffic (including DNS queries -// on IPv6-capable clusters) is routed between the veth and eth0 instead of -// being dropped by ip6_forward(). -func EnableIPv4Forwarding() error { +// EnableForwarding enables IPv4 and IPv6 forwarding in the current network +// namespace, so actor traffic (including DNS queries on IPv6-capable clusters) +// is routed between the veth and eth0 instead of being dropped by ip_forward() +// or ip6_forward(). +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 @@ -595,7 +595,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 { From d2b61eda91172ccb2c610bc952149ea8765d869f Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Fri, 21 Aug 2026 13:04:19 +0000 Subject: [PATCH 14/26] ateomnet: cover writeSysctlIfUnset's missing-path branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The os.Stat/IsNotExist fallback was never executed by the unit tests: every existing subtest's temp path could be created, so each returned at the os.WriteFile fast path. Point the new subtest at a node under a directory that does not exist — what procfs always does in production — and assert the file stays absent. --- internal/ateomnet/write_sysctl_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/ateomnet/write_sysctl_test.go b/internal/ateomnet/write_sysctl_test.go index b4cb1db169..d687932d8c 100644 --- a/internal/ateomnet/write_sysctl_test.go +++ b/internal/ateomnet/write_sysctl_test.go @@ -65,6 +65,21 @@ func TestWriteSysctlIfUnset(t *testing.T) { } }) + t.Run("missing_path_is_noop", func(t *testing.T) { + // A node under a directory that does not exist stands in for + // /proc/sys/net/ipv6/... on a kernel with IPv6 disabled. The other + // subtests' paths can be created, so they return at the os.WriteFile + // fast path; this is the only one that reaches the os.Stat branch, + // which is what procfs always does in production. + p := filepath.Join(dir, "no-such-dir", "forwarding") + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset on a missing path: %v", err) + } + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Fatalf("expected %s to stay absent, stat err = %v", p, err) + } + }) + 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 { From 9185af7179b421b38c271d59bba790df2a166d5a Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 22:19:45 -0700 Subject: [PATCH 15/26] 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 72b3e19795..f283a0c34a 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 a655a37592..64153ea6f5 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 5190881894f7a7136d988bea3d5f8f9237603a47 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Fri, 14 Aug 2026 11:03:44 -0700 Subject: [PATCH 16/26] 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 0000000000..c7a0e9a519 --- /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 30b4ab2b8555bf6414217109cb72f4b6cd880a6a Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 12:23:57 -0700 Subject: [PATCH 17/26] 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 2afd300ae8..7f21c041c3 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 81ae66aef3323fdac0e54fea6aaf94e2b2cc69fb Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 07:39:50 -0700 Subject: [PATCH 18/26] atenet/dns: answer non-A actor queries instead of SERVFAIL Before, the actor zone answered A queries and failed everything else -- AAAA for a valid actor, and any name in the zone that is not an actor. A failure reads as a temporary error rather than an answer, so clients retry it and then give up on the name; Alpine actors could not resolve each other at all, even on an IPv4-only cluster. After, those queries return a correct empty answer, and one that resolvers can cache. A unit test pins the whole rendered zone as a literal, so editing the name pattern or the suffix fails there rather than passing silently. --- cmd/atenet/internal/dns/README.md | 22 ++++++- cmd/atenet/internal/dns/corefile.go | 25 +++++++- cmd/atenet/internal/dns/corefile_test.go | 75 ++++++++++++++---------- 3 files changed, 87 insertions(+), 35 deletions(-) diff --git a/cmd/atenet/internal/dns/README.md b/cmd/atenet/internal/dns/README.md index a134e829ab..e89b06b62f 100644 --- a/cmd/atenet/internal/dns/README.md +++ b/cmd/atenet/internal/dns/README.md @@ -10,7 +10,6 @@ Cluster resources: * Deployment `ate-system:dns`. Label: app=dns * Service `ate-system:dns`. -* ConfigMap `ate-system:dns`. These are defined in manifests/ate-install/atenet-dns.yaml. @@ -20,16 +19,33 @@ These are defined in manifests/ate-install/atenet-dns.yaml. * Deployment `ate-system:dns`. * Service `ate-system:dns` pointing to the Deployment. -ConfigMap `ate-system:dns`: +Corefile, rendered by `corefile.go`: ``` -# Match any 'A' query for an actor name + atespace pattern under actors.resources.substrate.ate.dev +# Answer any 'A' query for an actor name + atespace pattern under actors.resources.substrate.ate.dev template IN A actors.resources.substrate.ate.dev { match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.actors\\.resources\\.substrate\\.ate\\.dev\\.$" answer "{{ .Name }} 60 IN A " + fallthrough + } +# NODATA for a well-formed actor name on any other qtype (AAAA, HTTPS, SRV, ...). + template ANY ANY actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\\.actors\\.resources\\.substrate\\.ate\\.dev\\.$" + rcode NOERROR + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + fallthrough + } +# Terminal catch-all: NXDOMAIN for anything else in the zone. + template ANY ANY actors.resources.substrate.ate.dev { + rcode NXDOMAIN + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" } ``` +The last two blocks keep the zone from ever answering SERVFAIL, which musl libc +maps to `EAI_AGAIN` — sinking the paired A query with it — and which cannot be +cached negatively. + ## Integration * CoreDNS: Update CoreDNS ConfigMap to add the stub resolver. diff --git a/cmd/atenet/internal/dns/corefile.go b/cmd/atenet/internal/dns/corefile.go index 0b301e7e29..2869e3a24d 100644 --- a/cmd/atenet/internal/dns/corefile.go +++ b/cmd/atenet/internal/dns/corefile.go @@ -30,6 +30,11 @@ func init() { } func buildTemplate() string { + const ( + fallthroughDirective = " fallthrough" + soaDirective = ` authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)"` + ) + // Build up the corefileTemplate programmatically to make it easier to understand. var directives []string // Plugins to enable. @@ -44,9 +49,27 @@ func buildTemplate() string { directives = append(directives, fmt.Sprintf("template IN A %s {", resources.ActorDNSSuffix)) // Escape the suffix's dots so they match literally; the final \. matches the FQDN's trailing dot. escapedSuffix := strings.ReplaceAll(resources.ActorDNSSuffix, ".", `\.`) - directives = append(directives, fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix)) + actorMatch := fmt.Sprintf(` match "^%s\.%s\.%s\.$"`, resources.ResourceNameRegexPattern, resources.ResourceNameRegexPattern, escapedSuffix) + directives = append(directives, actorMatch) // Note the %s -- this will be filled with the router IP. directives = append(directives, ` answer "{{ .Name }} 60 IN A %s"`) + directives = append(directives, fallthroughDirective) + directives = append(directives, "}") + + // Valid actor names return NOERROR (NODATA) for non-A queries. + directives = append(directives, fmt.Sprintf("template ANY ANY %s {", resources.ActorDNSSuffix)) + directives = append(directives, actorMatch) + directives = append(directives, " rcode NOERROR") + directives = append(directives, soaDirective) + directives = append(directives, fallthroughDirective) + directives = append(directives, "}") + + // Returns rcode NXDOMAIN (Non-Existent Domain) for any query that did not + // match the valid actor regex in the previous blocks. + // TODO(#922): answer empty non-terminals with NODATA. + directives = append(directives, fmt.Sprintf("template ANY ANY %s {", resources.ActorDNSSuffix)) + directives = append(directives, " rcode NXDOMAIN") + directives = append(directives, soaDirective) directives = append(directives, "}") // Generate the template. diff --git a/cmd/atenet/internal/dns/corefile_test.go b/cmd/atenet/internal/dns/corefile_test.go index f13429e475..c8653ad7ef 100644 --- a/cmd/atenet/internal/dns/corefile_test.go +++ b/cmd/atenet/internal/dns/corefile_test.go @@ -15,50 +15,63 @@ package dns import ( + "fmt" "strings" "testing" - - "github.com/agent-substrate/substrate/internal/resources" ) +// Spelled out rather than built from resources.ResourceNameRegexPattern and +// ActorDNSSuffix: the rendered zone is a wire contract, so a change to either +// constant should fail here instead of being tracked silently. +const wantCorefileFmt = `actors.resources.substrate.ate.dev:53 { + log + errors + health :8080 + ready :8181 + reload + template IN A actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\.actors\.resources\.substrate\.ate\.dev\.$" + answer "{{ .Name }} 60 IN A %s" + fallthrough + } + template ANY ANY actors.resources.substrate.ate.dev { + match "^[a-z0-9]([-a-z0-9]*[a-z0-9])?\.[a-z0-9]([-a-z0-9]*[a-z0-9])?\.actors\.resources\.substrate\.ate\.dev\.$" + rcode NOERROR + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + fallthrough + } + template ANY ANY actors.resources.substrate.ate.dev { + rcode NXDOMAIN + authority "{{ .Zone }} 60 IN SOA ns.dns.{{ .Zone }} hostmaster.{{ .Zone }} (1 60 60 60 60)" + } +} +` + +// zoneBody strips the "# Generated at " header. +func zoneBody(t *testing.T, corefile string) string { + t.Helper() + header, body, ok := strings.Cut(corefile, "\n") + if !ok || !strings.HasPrefix(header, "# Generated at ") { + t.Fatalf("makeCoreFile() has no generated-at header, got first line %q", header) + } + return body +} + func TestMakeCoreFile(t *testing.T) { tests := []struct { name string routerIP string - expected []string }{ - { - name: "standard local IP", - routerIP: "10.240.0.10", - expected: []string{ - "actors.resources.substrate.ate.dev:53 {", - "log", - "errors", - "health :8080", - "ready :8181", - "reload", - "template IN A actors.resources.substrate.ate.dev {", - `match "^` + resources.ResourceNameRegexPattern + `\.` + resources.ResourceNameRegexPattern + `\.actors\.resources\.substrate\.ate\.dev\.$"`, - `answer "{{ .Name }} 60 IN A 10.240.0.10"`, - }, - }, - { - name: "different IP", - routerIP: "192.168.1.1", - expected: []string{ - "actors.resources.substrate.ate.dev:53 {", - `answer "{{ .Name }} 60 IN A 192.168.1.1"`, - }, - }, + {name: "cluster IP", routerIP: "10.240.0.10"}, + {name: "different cluster IP", routerIP: "192.168.1.1"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := makeCoreFile(tc.routerIP) - for _, exp := range tc.expected { - if !strings.Contains(got, exp) { - t.Errorf("makeCoreFile(%q) missing expected substring %q\nGot:\n%s", tc.routerIP, exp, got) - } + got := zoneBody(t, makeCoreFile(tc.routerIP)) + want := fmt.Sprintf(wantCorefileFmt, tc.routerIP) + if got != want { + t.Errorf("makeCoreFile(%q) rendered an unexpected Corefile\nGot:\n%s\nWant:\n%s", tc.routerIP, got, want) } }) } From cf39993e4160f4cb9048ba79031dcc64156ef9cf Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Fri, 7 Aug 2026 15:56:35 -0700 Subject: [PATCH 19/26] 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 0000000000..a6a3641087 --- /dev/null +++ b/internal/ipfamily/ipfamily.go @@ -0,0 +1,60 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package ipfamily sorts Kubernetes addresses into their IP families. +package ipfamily + +import ( + "net/netip" + + corev1 "k8s.io/api/core/v1" +) + +// ClusterIPsByFamily splits a Service's cluster IPs into its IPv4 and IPv6 +// entries, returning "" for a family the Service does not have. +// +// A Service with no ipFamilyPolicy is SingleStack, so even on a dual-stack +// cluster it has exactly one ClusterIP and one of the two return values is +// empty. Callers must handle that: it is the steady state everywhere +// ipFamilyPolicy has not been set, and it is what distinguishes "this cluster +// has no address to offer in that family" from a misconfiguration. +// +// Spec.ClusterIPs is preferred over the singular Spec.ClusterIP, with a +// fallback to the latter because a Service built by hand (or by a fake client) +// may only set the scalar. Headless Services, and any entry that is not a +// parseable address, are skipped rather than returned. +func ClusterIPsByFamily(svc *corev1.Service) (v4, v6 string) { + ips := svc.Spec.ClusterIPs + if len(ips) == 0 && svc.Spec.ClusterIP != "" { + ips = []string{svc.Spec.ClusterIP} + } + for _, ip := range ips { + if ip == "" || ip == corev1.ClusterIPNone { + continue + } + // netip rather than net.IP: net.IP.To4 returns non-nil for a v4-mapped + // v6 address and would misfile it as IPv4. + addr, err := netip.ParseAddr(ip) + if err != nil { + continue + } + switch { + case addr.Is4() && v4 == "": + v4 = ip + case addr.Is6() && !addr.Is4In6() && v6 == "": + v6 = ip + } + } + return v4, v6 +} diff --git a/internal/ipfamily/ipfamily_test.go b/internal/ipfamily/ipfamily_test.go new file mode 100644 index 0000000000..9b60972534 --- /dev/null +++ b/internal/ipfamily/ipfamily_test.go @@ -0,0 +1,95 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ipfamily + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" +) + +func TestClusterIPsByFamily(t *testing.T) { + tests := []struct { + name string + spec corev1.ServiceSpec + wantV4 string + wantV6 string + wantReason string + }{ + { + name: "single stack IPv4", + spec: corev1.ServiceSpec{ClusterIP: "10.96.0.10", ClusterIPs: []string{"10.96.0.10"}}, + wantV4: "10.96.0.10", + wantReason: "the default policy on an IPv4 cluster", + }, + { + name: "single stack IPv6", + spec: corev1.ServiceSpec{ClusterIP: "fd00:10:96::8857", ClusterIPs: []string{"fd00:10:96::8857"}}, + wantV6: "fd00:10:96::8857", + wantReason: "an IPv6-only cluster allocates a v6 ClusterIP with no ipFamilyPolicy set", + }, + { + name: "dual stack IPv4 primary", + spec: corev1.ServiceSpec{ClusterIP: "10.96.0.10", ClusterIPs: []string{"10.96.0.10", "fd00:10:96::8857"}}, + wantV4: "10.96.0.10", + wantV6: "fd00:10:96::8857", + wantReason: "both families are usable regardless of which one is primary", + }, + { + name: "dual stack IPv6 primary", + spec: corev1.ServiceSpec{ClusterIP: "fd00:10:96::8857", ClusterIPs: []string{"fd00:10:96::8857", "10.96.0.10"}}, + wantV4: "10.96.0.10", + wantV6: "fd00:10:96::8857", + wantReason: "the order of ClusterIPs is the family preference, not a family label", + }, + { + name: "scalar only", + spec: corev1.ServiceSpec{ClusterIP: "10.96.0.10"}, + wantV4: "10.96.0.10", + wantReason: "a hand-built Service may set only the singular field", + }, + { + name: "headless", + spec: corev1.ServiceSpec{ClusterIP: corev1.ClusterIPNone, ClusterIPs: []string{corev1.ClusterIPNone}}, + wantReason: "None is a sentinel, not an address", + }, + { + name: "not yet allocated", + spec: corev1.ServiceSpec{}, + wantReason: "a Service observed before the allocator has run has neither", + }, + { + name: "v4-mapped v6 belongs to neither family", + spec: corev1.ServiceSpec{ClusterIPs: []string{"::ffff:10.96.0.10"}}, + wantReason: "net.IP.To4 would misfile this as IPv4; kube never allocates one, so dropping it beats guessing", + }, + { + name: "unparseable entries are skipped", + spec: corev1.ServiceSpec{ClusterIPs: []string{"not-an-ip", "10.96.0.10"}}, + wantV4: "10.96.0.10", + wantReason: "a junk entry must not shadow a usable one", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + svc := &corev1.Service{Spec: tc.spec} + v4, v6 := ClusterIPsByFamily(svc) + if v4 != tc.wantV4 || v6 != tc.wantV6 { + t.Errorf("ClusterIPsByFamily(%+v) = (%q, %q), want (%q, %q): %s", tc.spec, v4, v6, tc.wantV4, tc.wantV6, tc.wantReason) + } + }) + } +} From 7c0a2f8ececeeb5729148c4f2b11f93a6674b40f Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 08:44:24 -0700 Subject: [PATCH 20/26] 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 2869e3a24d..7b22cb5219 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 c8653ad7ef..58921b135b 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 8148ad84b85e584258e2ccdfd5565db8594377db Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 08:44:44 -0700 Subject: [PATCH 21/26] 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 e89b06b62f..cb28e837f2 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 7b22cb5219..32d4043b74 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 58921b135b..c207926623 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 cf2db99b69..96bc7df249 100644 --- a/cmd/atenet/internal/dns/dns.go +++ b/cmd/atenet/internal/dns/dns.go @@ -26,6 +26,7 @@ import ( "syscall" "time" + "github.com/agent-substrate/substrate/internal/ipfamily" "github.com/agent-substrate/substrate/internal/resources" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -50,7 +51,6 @@ type Controller struct { // Run the DNS orchestration loop until ctx is canceled. func (c *Controller) Run(ctx context.Context) error { slog.InfoContext(ctx, "DNS Controller started", slog.Duration("interval", c.Interval), slog.String("corefile", c.CorefilePath)) - slog.InfoContext(ctx, "Using template", "template", corefileTemplate) ticker := time.NewTicker(c.Interval) defer ticker.Stop() @@ -81,8 +81,10 @@ func (c *Controller) reconcile(ctx context.Context) error { return fmt.Errorf("failed to get atenet-router service: %w", err) } - routerIP := routerSvc.Spec.ClusterIP - if routerIP == "" || routerIP == "None" { + // Both families, not just Spec.ClusterIP: on an IPv6-only cluster the sole + // ClusterIP is a v6 address, and the zone has to publish it as an AAAA. + routerV4, routerV6 := ipfamily.ClusterIPsByFamily(routerSvc) + if routerV4 == "" && routerV6 == "" { slog.WarnContext(ctx, "atenet-router service has no ClusterIP yet, waiting...") return nil } @@ -104,7 +106,7 @@ func (c *Controller) reconcile(ctx context.Context) error { } // 3. Reconcile CoreDNS Corefile on shared volume - if err := c.reconcileCoreDNSConfig(ctx, routerIP); err != nil { + if err := c.reconcileCoreDNSConfig(ctx, routerV4, routerV6); err != nil { return fmt.Errorf("failed to reconcile CoreDNS config file: %w", err) } @@ -116,13 +118,13 @@ func (c *Controller) reconcile(ctx context.Context) error { return nil } -func (c *Controller) reconcileCoreDNSConfig(ctx context.Context, routerIP string) error { - expectedCorefile := makeCoreFile(routerIP) +func (c *Controller) reconcileCoreDNSConfig(ctx context.Context, routerV4, routerV6 string) error { + expectedCorefile := makeCoreFile(routerV4, routerV6) // Read Corefile from local shared volume path to see if it needs updating corefileBytes, err := os.ReadFile(c.CorefilePath) if err == nil && string(corefileBytes) == expectedCorefile { - slog.DebugContext(ctx, "CoreDNS Corefile is up-to-date", slog.String("routerIP", routerIP)) + slog.DebugContext(ctx, "CoreDNS Corefile is up-to-date", slog.String("routerIPv4", routerV4), slog.String("routerIPv6", routerV6)) return nil } @@ -130,7 +132,7 @@ func (c *Controller) reconcileCoreDNSConfig(ctx context.Context, routerIP string if err := os.WriteFile(c.CorefilePath, []byte(expectedCorefile), 0644); err != nil { return fmt.Errorf("failed to write updated Corefile to %s: %w", c.CorefilePath, err) } - slog.InfoContext(ctx, "CoreDNS Corefile updated", slog.String("routerIP", routerIP)) + slog.InfoContext(ctx, "CoreDNS Corefile updated", slog.String("routerIPv4", routerV4), slog.String("routerIPv6", routerV6)) // Signal CoreDNS process to reload if err := c.Reloader.Reload(ctx); err != nil { diff --git a/cmd/atenet/internal/dns/dns_test.go b/cmd/atenet/internal/dns/dns_test.go index 34116db284..34d23405d2 100644 --- a/cmd/atenet/internal/dns/dns_test.go +++ b/cmd/atenet/internal/dns/dns_test.go @@ -32,10 +32,12 @@ import ( type mockConfigReloader struct { reloaded bool + reloads int } func (m *mockConfigReloader) Reload(ctx context.Context) error { m.reloaded = true + m.reloads++ return nil } @@ -145,6 +147,131 @@ func TestReconcile(t *testing.T) { } } +// TestReconcileRouterIPFamilies covers what the controller publishes for each +// shape the atenet-router Service takes. The v6-only row is the one that used +// to be broken: the sole ClusterIP was read out of Spec.ClusterIP and written +// into an `IN A` answer, which loads as a valid Corefile and then SERVFAILs +// every query in the zone. +func TestReconcileRouterIPFamilies(t *testing.T) { + tests := []struct { + name string + // routerSpec is the atenet-router Service's spec; the dns Service is + // always a plain single-stack v4 one, since it feeds kube-dns rather + // than the zone under test. + routerSpec corev1.ServiceSpec + // wantAnswers are the answer lines the rendered Corefile must have. + wantAnswers []string + // wantNoAnswer, when true, means reconcile should leave the Corefile + // untouched rather than publish anything. + wantNoAnswer bool + }{ + { + name: "single stack IPv4", + routerSpec: corev1.ServiceSpec{ClusterIP: "10.0.0.1", ClusterIPs: []string{"10.0.0.1"}}, + wantAnswers: []string{`answer "{{ .Name }} 60 IN A 10.0.0.1"`}, + }, + { + name: "single stack IPv6", + routerSpec: corev1.ServiceSpec{ClusterIP: "fd00:10:96::8857", ClusterIPs: []string{"fd00:10:96::8857"}}, + wantAnswers: []string{`answer "{{ .Name }} 60 IN AAAA fd00:10:96::8857"`}, + }, + { + name: "dual stack", + routerSpec: corev1.ServiceSpec{ClusterIP: "10.0.0.1", ClusterIPs: []string{"10.0.0.1", "fd00:10:96::8857"}}, + wantAnswers: []string{ + `answer "{{ .Name }} 60 IN A 10.0.0.1"`, + `answer "{{ .Name }} 60 IN AAAA fd00:10:96::8857"`, + }, + }, + { + name: "not yet allocated", + routerSpec: corev1.ServiceSpec{}, + wantNoAnswer: true, + }, + { + name: "headless", + routerSpec: corev1.ServiceSpec{ClusterIP: corev1.ClusterIPNone, ClusterIPs: []string{corev1.ClusterIPNone}}, + wantNoAnswer: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + + routerSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "atenet-router", Namespace: "ate-system"}, + Spec: tc.routerSpec, + } + dnsSvc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "dns", Namespace: "ate-system"}, + Spec: corev1.ServiceSpec{ClusterIP: "10.0.0.2"}, + } + + const placeholder = "# not written yet\n" + corefilePath := filepath.Join(t.TempDir(), "Corefile") + if err := os.WriteFile(corefilePath, []byte(placeholder), 0644); err != nil { + t.Fatalf("failed to write initial Corefile: %v", err) + } + + reloader := &mockConfigReloader{} + controller := &Controller{ + Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(routerSvc, dnsSvc).Build(), + Interval: 1 * time.Second, + CorefilePath: corefilePath, + Reloader: reloader, + } + + ctx := context.Background() + if err := controller.reconcile(ctx); err != nil { + t.Fatalf("reconcile failed: %v", err) + } + + corefileBytes, err := os.ReadFile(corefilePath) + if err != nil { + t.Fatalf("failed to read Corefile: %v", err) + } + got := string(corefileBytes) + + if tc.wantNoAnswer { + if got != placeholder { + t.Errorf("reconcile() rewrote the Corefile for a Service with no usable ClusterIP; want it left alone\nGot:\n%s", got) + } + if reloader.reloads != 0 { + t.Errorf("reconcile() reloaded CoreDNS %d times for a Service with no usable ClusterIP, want 0", reloader.reloads) + } + return + } + + for _, want := range tc.wantAnswers { + if !strings.Contains(got, want) { + t.Errorf("reconcile() wrote a Corefile missing %q\nGot:\n%s", want, got) + } + } + // Exactly the expected answers and no others: an address template for + // a family the Service does not have would publish an unreachable + // address, and on the A side would not even parse as an RR. + if answers := strings.Count(got, `answer "`); answers != len(tc.wantAnswers) { + t.Errorf("reconcile() wrote %d answer directives, want %d\nGot:\n%s", answers, len(tc.wantAnswers), got) + } + if reloader.reloads != 1 { + t.Errorf("reconcile() reloaded CoreDNS %d times, want 1", reloader.reloads) + } + + // A second pass must be a no-op. The controller reconciles on a + // ticker, so anything unstable in the render -- a timestamp, most + // easily -- would rewrite the file and signal CoreDNS every interval. + if err := controller.reconcile(ctx); err != nil { + t.Fatalf("second reconcile failed: %v", err) + } + if reloader.reloads != 1 { + t.Errorf("second reconcile() reloaded CoreDNS again (%d total), want it to recognise the Corefile as up to date", reloader.reloads) + } + }) + } +} + func TestReconcileKubeDNSNotFound(t *testing.T) { scheme := runtime.NewScheme() _ = corev1.AddToScheme(scheme) From 8e9ff3ac52130e5126860cc1ffdbacc18c67fc08 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 16:35:16 -0700 Subject: [PATCH 22/26] 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 0000000000..76db25d4f6 --- /dev/null +++ b/internal/e2e/dns_client.go @@ -0,0 +1,162 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package e2e + +import ( + "context" + "errors" + "fmt" + "net" + "strconv" + "strings" + "time" + + "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/portforward" + "k8s.io/client-go/kubernetes" +) + +const ( + dnsNamespace = "ate-system" + dnsService = "dns" + // dnsServicePort is the Service port; the Service exposes 53 twice, once + // UDP and once TCP, and the port-forward tunnel is TCP either way. + dnsServicePort = 53 +) + +// DNSRcode is how the server answered, at the granularity net.Resolver exposes. +type DNSRcode int + +const ( + // DNSAnswered is NOERROR with at least one address of the queried family. + DNSAnswered DNSRcode = iota + // DNSEmpty is "this name has no address in this family": NODATA (NOERROR + // with an empty answer section) or NXDOMAIN. net.Resolver reports both as + // DNSError.IsNotFound and the standard library offers no way to tell them + // apart, which is fine here — both are benign to every stub resolver, and + // that benign-ness is the property under test. + DNSEmpty + // DNSFailed is SERVFAIL, REFUSED, a timeout, or a malformed reply: anything + // net.Resolver classifies as the server misbehaving. No query into the actor + // zone should ever produce it — not a non-A qtype, not a name that fails the + // actor regex — and that is the regression these tests exist to catch. + DNSFailed +) + +func (r DNSRcode) String() string { + switch r { + case DNSAnswered: + return "answered" + case DNSEmpty: + return "no-such-host (NODATA or NXDOMAIN)" + case DNSFailed: + return "server failure (SERVFAIL/REFUSED/timeout)" + default: + return "unknown" + } +} + +// DNSClient resolves names against the ate-system/dns CoreDNS Service over a +// port-forward. +// +// Querying that Service directly, rather than going through the cluster's own +// resolver, is deliberate: the delegation that would make actor names resolvable +// cluster-wide is a patch to the kube-system/kube-dns ConfigMap, which only +// exists on GKE (cmd/atenet/internal/dns/dns.go reconcileKubeDNSConfig hits the +// IsNotFound branch on kind and upstream Kubernetes). Pointing at the Service is +// the only way to assert the zone's behavior on every cluster we test on. +type DNSClient struct { + resolver *net.Resolver + stop func() +} + +// NewDNSClient establishes a port-forward to the atenet DNS Service. Call Close +// to tear it down. +func NewDNSClient(ctx context.Context) (*DNSClient, error) { + config, err := ateclient.LoadConfig(KubeConfig, KubeContext) + if err != nil { + return nil, fmt.Errorf("loading kubeconfig: %w", err) + } + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + return nil, fmt.Errorf("creating k8s client: %w", err) + } + + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, dnsNamespace, dnsService, dnsServicePort) + if err != nil { + return nil, err + } + addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(localPort)) + + return &DNSClient{ + stop: stop, + resolver: &net.Resolver{ + // PreferGo keeps us on Go's own resolver on every platform. cgo's + // would ignore Dial entirely and query the host's nameservers. + PreferGo: true, + // Surface a per-family failure instead of hiding it behind the + // other family's success. + StrictErrors: true, + Dial: func(ctx context.Context, _, _ string) (net.Conn, error) { + // The port-forward is a TCP tunnel, so every query goes over TCP + // whatever network the resolver asked for. net.Resolver selects + // stream framing for any conn that is not a net.PacketConn, so + // returning a TCP conn here is transparent to it. The requested + // server address is ignored: there is exactly one server. + var d net.Dialer + return d.DialContext(ctx, "tcp", addr) + }, + }, + }, nil +} + +// Close tears down the port-forward. +func (c *DNSClient) Close() { + if c.stop != nil { + c.stop() + } +} + +// Lookup resolves name in a single address family — network is "ip4" for an A +// query or "ip6" for a AAAA query — and reports the addresses alongside how the +// server answered. A DNSFailed result is returned with the underlying error for +// the failure message; DNSEmpty is returned with a nil error because it is a +// valid answer, not a fault. +func (c *DNSClient) Lookup(ctx context.Context, network, name string) ([]string, DNSRcode, error) { + // Root the name so the resolver skips the host's search list and ndots + // handling, which would otherwise make the query depend on where the test + // runs. + if !strings.HasSuffix(name, ".") { + name += "." + } + + lookupCtx, cancel := context.WithTimeout(ctx, 15*time.Second) + defer cancel() + + addrs, err := c.resolver.LookupNetIP(lookupCtx, network, name) + if err == nil { + ips := make([]string, 0, len(addrs)) + for _, a := range addrs { + ips = append(ips, a.Unmap().String()) + } + return ips, DNSAnswered, nil + } + + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) && dnsErr.IsNotFound { + return nil, DNSEmpty, nil + } + return nil, DNSFailed, fmt.Errorf("%s query for %q: %w", network, name, err) +} diff --git a/internal/e2e/ipfamily.go b/internal/e2e/ipfamily.go new file mode 100644 index 0000000000..3d72fcc040 --- /dev/null +++ b/internal/e2e/ipfamily.go @@ -0,0 +1,69 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package e2e + +import ( + "context" + "fmt" + "net/netip" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// clusterIPsByFamily splits a Service's cluster IPs into its IPv4 and IPv6 +// entries, returning "" for a family the Service does not have. A Service with +// no ipFamilyPolicy is SingleStack, so on a dual-stack cluster it still has +// exactly one ClusterIP and one of the two return values is empty — which is +// what makes this the right thing to gate a dual-stack assertion on. +// +// Spec.ClusterIPs is preferred over the singular Spec.ClusterIP, with a +// fallback for the latter because a Service object built by hand (or by a fake +// client) may only set the scalar. +func clusterIPsByFamily(svc *corev1.Service) (v4, v6 string) { + ips := svc.Spec.ClusterIPs + if len(ips) == 0 && svc.Spec.ClusterIP != "" { + ips = []string{svc.Spec.ClusterIP} + } + for _, ip := range ips { + if ip == "" || ip == corev1.ClusterIPNone { + continue + } + // netip rather than net.IP: net.IP.To4 returns non-nil for a v4-mapped + // v6 address and would misfile it as IPv4. + addr, err := netip.ParseAddr(ip) + if err != nil { + continue + } + switch { + case addr.Is4() && v4 == "": + v4 = ip + case addr.Is6() && !addr.Is4In6() && v6 == "": + v6 = ip + } + } + return v4, v6 +} + +// RouterClusterIPs returns the atenet-router Service's IPv4 and IPv6 +// ClusterIPs. Either may be "". +func RouterClusterIPs(ctx context.Context) (v4, v6 string, err error) { + svc, err := GetClients().K8s.CoreV1().Services(RouterNamespace).Get(ctx, RouterService, metav1.GetOptions{}) + if err != nil { + return "", "", fmt.Errorf("getting Service %s/%s: %w", RouterNamespace, RouterService, err) + } + v4, v6 = clusterIPsByFamily(svc) + return v4, v6, nil +} diff --git a/internal/e2e/router_client.go b/internal/e2e/router_client.go index 4a0c006b22..7992ba036b 100644 --- a/internal/e2e/router_client.go +++ b/internal/e2e/router_client.go @@ -36,8 +36,11 @@ import ( ) const ( - routerNamespace = "ate-system" - routerService = "atenet-router" + // RouterNamespace and RouterService locate the atenet router. Exported so + // that suites addressing the same Service or its pods do not have to + // redeclare them. + RouterNamespace = "ate-system" + RouterService = "atenet-router" // routerConnectServicePort is atenet-router's Service port for // CONNECT-tunneled traffic (see manifests/ate-install/atenet-router.yaml). // It is a distinct listener from the plain HTTP one Get/PostJSON use: @@ -79,7 +82,7 @@ func NewRouterClient(ctx context.Context) (*RouterClient, error) { return nil, fmt.Errorf("creating k8s client: %w", err) } - localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, routerNamespace, routerService, 80) + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, RouterNamespace, RouterService, 80) if err != nil { return nil, err } @@ -183,7 +186,7 @@ func (c *RouterClient) Connect(ctx context.Context, actorRef resources.ActorRef, // in one test don't each pay for a fresh port-forward. func (c *RouterClient) ensureConnectPortForward(ctx context.Context) error { c.connectOnce.Do(func() { - localPort, stop, err := portforward.ServicePortForward(ctx, c.config, c.clientset, routerNamespace, routerService, routerConnectServicePort) + localPort, stop, err := portforward.ServicePortForward(ctx, c.config, c.clientset, RouterNamespace, RouterService, routerConnectServicePort) if err != nil { c.connectErr = fmt.Errorf("port-forwarding to the router's CONNECT listener: %w", err) return diff --git a/internal/e2e/statusz.go b/internal/e2e/statusz.go index a04a07141d..1890a036fe 100644 --- a/internal/e2e/statusz.go +++ b/internal/e2e/statusz.go @@ -51,7 +51,7 @@ func NewStatuszClient(ctx context.Context) (*StatuszClient, error) { return nil, fmt.Errorf("creating k8s client: %w", err) } - localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, routerNamespace, routerService, routerStatusPort) + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, RouterNamespace, RouterService, routerStatusPort) if err != nil { return nil, err } diff --git a/internal/e2e/suites/networking/dns_test.go b/internal/e2e/suites/networking/dns_test.go new file mode 100644 index 0000000000..cf1f6f57bc --- /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 84e02b60e21c699c1a85b42c72bb2edadf5f26d2 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 16:35:23 -0700 Subject: [PATCH 23/26] 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 cf1f6f57bc..b8abf001c2 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 57c179127e622f3e55a12c44ff7ab82675ced657 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 16:37:14 -0700 Subject: [PATCH 24/26] 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 0000000000..4a2e50d1d8 --- /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 a369c2baa85075d7794d7bf1504fb0fc4d14563c Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Aug 2026 14:48:33 -0700 Subject: [PATCH 25/26] ateomnet: move the actor nftables table to the inet family The actor's NAT and filter rules lived in an ip table, which can only ever carry IPv4. They are now in an inet table, so one table can hold both address families when the actor veth becomes dual-stack. Every IPv4 match opens with an NFPROTO comparison, because a bare payload match in an inet table would read an IPv4 offset out of an IPv6 header. IPv4 behaviour is unchanged, but the move is not a no-op on a dual-stack pod: inet nat chains register the nat hooks for both families, so IPv6 traffic in the worker pod netns is now conntracked, and the forward accept now covers IPv6. NAT in the inet family needs Linux 5.2 or later. Teardown sweeps ip as well as inet. A table name is unique per family, so the ip table an earlier ateom left behind is invisible to an inet-only cleanup: the dump comes back empty, the "already clean" path reports success, and the stale table keeps redirecting alongside the new one. Part of #246 --- internal/ateomnet/net.go | 69 ++++++++---- internal/ateomnet/net_linux_test.go | 148 +++++++++++++++++++++++++- internal/ateomnet/rules_linux_test.go | 88 +++++++++++++++ 3 files changed, 281 insertions(+), 24 deletions(-) create mode 100644 internal/ateomnet/rules_linux_test.go diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 1f0fbc92a8..a63377fc2c 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -259,8 +259,15 @@ 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 table is in the inet family so one table can carry both address + // families once the actor veth is dual-stack. NAT there needs Linux 5.2. + // A bare payload match in that family is ambiguous, so every IPv4 match + // opens with an NFPROTO comparison. An inet nat chain registers the nat + // hooks for both families, so IPv6 traffic in this netns is conntracked + // where an ip table left it untracked. + // + // TODO(#246): Add the IPv6 veth addressing and forwarding this table is + // waiting on. The actor network itself is still IPv4-only. // // The rules do three things: // @@ -278,7 +285,7 @@ func InstallActorNftablesRules(egressPort uint16) error { c := &nftables.Conn{} table := &nftables.Table{ - Family: nftables.TableFamilyIPv4, + Family: nftables.TableFamilyINet, Name: ActorNftTableName, } c.AddTable(table) @@ -304,7 +311,7 @@ func InstallActorNftablesRules(egressPort uint16) error { c.AddRule(&nftables.Rule{ Table: table, Chain: postrouting, - Exprs: append(IPSourceEqual(ActorVethIP), &expr.Masq{}), + Exprs: append(ipv4SourceEqual(ActorVethIP), &expr.Masq{}), }) acceptPolicy := nftables.ChainPolicyAccept @@ -316,6 +323,9 @@ func InstallActorNftablesRules(egressPort uint16) error { Priority: nftables.ChainPriorityFilter, Policy: &acceptPolicy, }) + // Unqualified, so this accepts forwarded IPv6 too -- what the actor needs + // once it is dual-stack. accept is per-table, so it cannot override a drop + // from the CNI's own forward chains. c.AddRule(&nftables.Rule{ Table: table, Chain: forward, @@ -335,30 +345,51 @@ func RemoveActorNftablesRules() error { // Delete the whole ateom nftables table if it exists. The table is // 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. + // + // Both families are swept, not just the inet one this installs into: a table + // name is unique per family, so an ip table left by an earlier ateom would + // survive every later cleanup and keep redirecting alongside the new one. + // The pod netns outlives an in-place container restart, so an ateom that + // predates the inet table can share a netns with one that follows it + // wherever WorkerPool.spec.ateomImage is a mutable tag -- the dev loop. + // TODO(ypgao): Drop the ip sweep once no live pod can predate this change. c := &nftables.Conn{} - tables, err := c.ListTablesOfFamily(nftables.TableFamilyIPv4) - if err != nil { - return fmt.Errorf("while listing nftables tables: %w", err) - } - for _, table := range tables { - if table.Name != ActorNftTableName { - continue + for _, family := range []struct { + name string + id nftables.TableFamily + }{{"inet", nftables.TableFamilyINet}, {"ip", nftables.TableFamilyIPv4}} { + tables, err := c.ListTablesOfFamily(family.id) + if err != nil { + return fmt.Errorf("while listing %s nftables tables: %w", family.name, err) } - c.DelTable(table) - if err := c.Flush(); err != nil { - return fmt.Errorf("while deleting actor nftables table: %w", err) + for _, table := range tables { + if table.Name != ActorNftTableName { + continue + } + c.DelTable(table) + if err := c.Flush(); err != nil { + return fmt.Errorf("while deleting the %s actor nftables table: %w", family.name, err) + } } - return nil } return nil } -func IPSourceEqual(ip string) []expr.Any { - return IPPayloadEqual(12, ip) +func ipv4SourceEqual(ip string) []expr.Any { + return ipv4PayloadEqual(12, ip) } -func IPPayloadEqual(offset uint32, ip string) []expr.Any { +// ipv4PayloadEqual matches a 4-byte IPv4 network-header field. The leading +// nfproto comparison is what makes it safe in the inet table: without it the +// payload load would read the same offset out of an IPv6 header. +func ipv4PayloadEqual(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, @@ -391,7 +422,7 @@ func ActorEgressRedirectRule(table *nftables.Table, chain *nftables.Chain, port if port == 0 { return nil } - exprs := append(IPSourceEqual(ActorVethIP), TCPProtocol()...) + exprs := append(ipv4SourceEqual(ActorVethIP), TCPProtocol()...) exprs = append(exprs, &expr.Immediate{ Register: 1, diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index b9c8ac45f4..0216585f4c 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -17,6 +17,7 @@ package ateomnet import ( + "bytes" "context" "errors" "runtime" @@ -24,6 +25,8 @@ import ( "github.com/agent-substrate/substrate/internal/roottest" "github.com/google/nftables" + "github.com/google/nftables/binaryutil" + "github.com/google/nftables/expr" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" ) @@ -75,17 +78,51 @@ func withTestNetNS(t *testing.T, fn func(interior netns.NsHandle)) { fn(interior) } -// 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. +// requireNftables skips when this kernel cannot serve what SetupActorNetwork +// installs, which is a property of the machine rather than of the code under +// test. Listing the inet family is not a sufficient probe: inet filter is far +// older than inet nat, which needs Linux 5.2, so this builds and drops a nat +// chain in the family the actor table uses. func requireNftables(t *testing.T) { t.Helper() c := &nftables.Conn{} - if _, err := c.ListTablesOfFamily(nftables.TableFamilyIPv4); err != nil { - t.Skipf("nftables unavailable in this environment: %v", err) + probe := c.AddTable(&nftables.Table{Family: nftables.TableFamilyINet, Name: "ateom_nft_probe"}) + c.AddChain(&nftables.Chain{ + Name: "prerouting", + Table: probe, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityNATDest, + }) + if err := c.Flush(); err != nil { + t.Skipf("nftables inet nat unavailable in this environment: %v", err) + } + c.DelTable(probe) + if err := c.Flush(); err != nil { + t.Fatalf("deleting the nftables probe table: %v", err) } } +// 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() @@ -215,9 +252,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 +276,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) @@ -239,6 +290,93 @@ func TestSetupActorNetworkIsRepeatable(t *testing.T) { }) } +// TestRemoveActorNftablesRulesSweepsIPv4Family covers the upgrade case: a +// worker whose previous ateom created the actor table in the ip family. Table +// names are unique per family, so an inet-only cleanup could never see that +// table, and it would have kept redirecting alongside the inet one installed +// next to it. +func TestRemoveActorNftablesRulesSweepsIPv4Family(t *testing.T) { + roottest.Require(t, "creating network namespaces and nftables rules") + + withTestNetNS(t, func(netns.NsHandle) { + requireNftables(t) + + c := &nftables.Conn{} + c.AddTable(&nftables.Table{Family: nftables.TableFamilyIPv4, Name: ActorNftTableName}) + if err := c.Flush(); err != nil { + t.Fatalf("creating the stand-in ip actor table: %v", err) + } + + if err := RemoveActorNftablesRules(); err != nil { + t.Fatalf("RemoveActorNftablesRules: %v", err) + } + + tables, err := c.ListTablesOfFamily(nftables.TableFamilyIPv4) + if err != nil { + t.Fatalf("listing ip nftables tables: %v", err) + } + for _, table := range tables { + if table.Name == ActorNftTableName { + t.Fatal("the ip actor table survived cleanup") + } + } + }) +} + +// TestSetupActorNetworkInstallsEgressRedirect covers the rule no other test in +// this package builds: they all leave EgressRedirectPort zero, so the kernel +// never sees the redirect. Its acceptance is not implied by the masquerade rule +// next to it -- redirect in the inet family is separate kernel support from the +// nat chain type -- and it is the rule the whole actor egress path rides on. +func TestSetupActorNetworkInstallsEgressRedirect(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + const egressPort = 15001 + + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + if err := SetupActorNetwork(ctx, NetworkConfig{ + InteriorNetNS: interior, + EgressRedirectPort: egressPort, + }); err != nil { + t.Fatalf("SetupActorNetwork: %v", err) + } + + c := &nftables.Conn{} + rules, err := c.GetRules( + &nftables.Table{Family: nftables.TableFamilyINet, Name: ActorNftTableName}, + &nftables.Chain{Name: "prerouting"}, + ) + if err != nil { + t.Fatalf("listing prerouting rules of the actor table: %v", err) + } + if len(rules) != 1 { + t.Fatalf("prerouting holds %d rules, want the egress redirect alone", len(rules)) + } + + // Read back what the kernel stored rather than what the builder emitted: + // TestActorNftablesRuleExprs already pins the builder, and what is in + // doubt here is whether an inet nat chain takes these expressions at all. + var haveNFProto, havePort, haveRedir bool + for _, e := range rules[0].Exprs { + switch e := e.(type) { + case *expr.Meta: + haveNFProto = haveNFProto || e.Key == expr.MetaKeyNFPROTO + case *expr.Immediate: + havePort = havePort || bytes.Equal(e.Data, binaryutil.BigEndian.PutUint16(egressPort)) + case *expr.Redir: + haveRedir = true + } + } + if !haveNFProto || !havePort || !haveRedir { + t.Errorf("installed redirect has nfproto=%t port=%t redir=%t, want all three, got %v", + haveNFProto, havePort, haveRedir, rules[0].Exprs) + } + }) +} + // TestSetupActorNetworkHostVethHWAddr covers the micro-VM requirement: a CH // snapshot freezes the guest's ARP entry for the gateway, so the worker-side // veth MAC has to be exactly the one the caller asked for, on every pod. diff --git a/internal/ateomnet/rules_linux_test.go b/internal/ateomnet/rules_linux_test.go new file mode 100644 index 0000000000..b6b8b53f3e --- /dev/null +++ b/internal/ateomnet/rules_linux_test.go @@ -0,0 +1,88 @@ +//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 ( + "fmt" + "reflect" + "testing" + + "github.com/google/nftables/expr" +) + +// TestActorNftablesRuleExprs pins the expressions installed into the inet actor +// table. The nfproto guard in front of every IPv4 match is what makes those +// matches safe there -- without it the payload load reads an IPv4 offset out of +// an IPv6 header -- and no other test in the package can see it: an IPv4-only +// datapath still behaves correctly with the guard removed. +// +// The wants are spelled out as literal bytes rather than built from the same +// helpers as the code, so they pin the wire encoding and not just its spelling. +func TestActorNftablesRuleExprs(t *testing.T) { + // meta nfproto ipv4; ip saddr 169.254.17.2 + actorSourceIsIPv4 := []expr.Any{ + &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{2}}, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{169, 254, 17, 2}}, + } + + tests := []struct { + name string + got []expr.Any + want []expr.Any + }{{ + name: "source match guards the payload load with nfproto", + got: ipv4SourceEqual(ActorVethIP), + want: actorSourceIsIPv4, + }, { + name: "egress redirect matches actor IPv4 TCP and redirects to the port", + got: ActorEgressRedirectRule(nil, nil, 15001).Exprs, + want: append(append([]expr.Any{}, actorSourceIsIPv4...), + // meta l4proto tcp; redirect to :15001 + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{6}}, + &expr.Immediate{Register: 1, Data: []byte{0x3a, 0x99}}, + &expr.Redir{RegisterProtoMin: 1}, + ), + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if !reflect.DeepEqual(test.got, test.want) { + t.Errorf("rule exprs mismatch:\ngot:\n%s\nwant:\n%s", formatExprs(test.got), formatExprs(test.want)) + } + }) + } +} + +// TestActorEgressRedirectRuleDisabled covers the zero port: no rule at all, so +// actor egress stays on the masquerade path instead of being redirected to a +// listener that is not there. +func TestActorEgressRedirectRuleDisabled(t *testing.T) { + if rule := ActorEgressRedirectRule(nil, nil, 0); rule != nil { + t.Errorf("ActorEgressRedirectRule(0) = %v, want nil", rule.Exprs) + } +} + +func formatExprs(exprs []expr.Any) string { + var s string + for _, e := range exprs { + s += fmt.Sprintf(" %T%+v\n", e, e) + } + return s +} From 4ea369f4b952dbc281ef674939b079bb773be501 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Aug 2026 14:56:11 -0700 Subject: [PATCH 26/26] ateomnet: give the actor an IPv6 address when the pod has one Actor networking was IPv4-only, so an actor on a dual-stack worker pod could not reach an IPv6-only destination at all. SetupActorNetwork now assigns the fd00:169:254::/126 counterparts of the existing point-to-point pair to both ends of the actor veth, installs an IPv6 default route in the interior netns, and adds the matching rules to the inet-family actor table. Whether the actor gets IPv6 is decided once in the worker pod netns and carried into the interior one, which is created fresh and so always reports IPv6 available whatever the cluster's families are. Both halves have to hold: the pod needs a global IPv6 address of its own, and the veth has to accept an IPv6 address -- IPv4-only GKE sets disable_ipv6 and netlink then rejects the assignment with EPERM. Addresses carry IFA_F_NODAD rather than the accept_dad sysctl, which the unprivileged ateom container cannot write. Part of #246 --- cmd/ateom-microvm/run.go | 4 + internal/ateomnet/net.go | 194 +++++++++++++++++-- internal/ateomnet/net_linux_test.go | 268 ++++++++++++++++++++++++-- internal/ateomnet/rules_linux_test.go | 39 +++- 4 files changed, 467 insertions(+), 38 deletions(-) diff --git a/cmd/ateom-microvm/run.go b/cmd/ateom-microvm/run.go index fd575d7daf..112e2a894a 100644 --- a/cmd/ateom-microvm/run.go +++ b/cmd/ateom-microvm/run.go @@ -1076,6 +1076,10 @@ func tailString(s string, n int) string { // agent: configure eth0 (IP/MAC/MTU), install the connected + default routes, and // pin the gateway's ARP entry to its fixed MAC (so a restored guest's frozen // neighbor entry stays valid). +// +// TODO(#246): the guest is configured IPv4-only, so a micro-VM actor sees no +// IPv6 even on a dual-stack pod where the host veth has one. gVisor reads the +// interior netns and picks the address up; this path has to be told. func (s *AteomService) configureGuestNetwork(ctx context.Context, ac *kata.AgentClient, mtu uint64) error { if err := ac.UpdateInterface(ctx, &agentpb.Interface{ Device: ateomnet.ActorVethName, diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index a63377fc2c..2576669c1a 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -43,6 +43,22 @@ const ( ActorVethIP = "169.254.17.2" ActorNftTableName = "ateom_actor" + // podPrimaryIfaceName is the worker pod's own interface, the one the CNI + // gave it. It is not ActorVethName despite the identical value: that one + // names the actor's end of the veth, which lives in the interior netns. + podPrimaryIfaceName = "eth0" + + // The IPv6 counterparts of the point-to-point pair above, chosen to echo the + // v4 addresses digit for digit. A fixed ULA rather than an RFC 4193 random + // prefix so the pair stays as readable in a packet dump as 169.254.17.x, and + // not fe80::/10 because a link-local source would need a scope id everywhere + // it is used. It must not overlap the cluster's pod CIDR; kind's dual-stack + // default is fd00:10:244::/56. + 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 +67,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 +82,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 +103,57 @@ func MustParseIP(s string) net.IP { return ip } +// MustParseIPv6 parses an IPv6 string into a net.IP, panicking on error. An +// IPv4 string is an error: net.IP holds it as a 16-byte v4-mapped address, so +// it would pass a length check and then compare against no IPv6 header. +func MustParseIPv6(s string) net.IP { + ip := net.ParseIP(s) + if ip == nil || ip.To4() != nil { + panic(fmt.Sprintf("parsing constant IPv6 %q", s)) + } + return ip.To16() +} + +// linkIPv6Enabled reports whether IPv6 addresses can be assigned to the named +// link in the current netns. It answers a kernel capability question, not a +// cluster one: IPv4-only GKE leaves disable_ipv6=1 and netlink then rejects +// every IPv6 address with EPERM, but IPv4-only kind leaves it at 0 because the +// node kernel has IPv6 compiled in. A kernel built without IPv6 has no sysctl +// at all. Pair it with linkHasGlobalIPv6 to decide whether the actor gets IPv6; +// on its own it says yes on clusters that have no IPv6 anywhere. +func linkIPv6Enabled(name string) bool { + b, err := os.ReadFile("/proc/sys/net/ipv6/conf/" + name + "/disable_ipv6") + if err != nil { + return false + } + return len(b) > 0 && b[0] == '0' +} + +// linkHasGlobalIPv6 reports whether link carries a global IPv6 address. Called +// on the worker pod's own interface, that is what decides the families the +// actor can egress on. +// +// It answers whether the pod has an address to egress from, not whether that +// address routes anywhere: IsGlobalUnicast is true for a ULA, and dual-stack +// kind hands pods a ULA with no path off the host. Reachability is the +// cluster's problem, not something this can decide from inside the netns. +func linkHasGlobalIPv6(ctx context.Context, link netlink.Link) bool { + // 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, err := netlink.AddrList(link, netlink.FAMILY_V6) + if err != nil { + slog.WarnContext(ctx, "listing IPv6 addresses of the worker pod interface", + "link", link.Attrs().Name, "error", err, "addressesRead", len(addrs)) + } + for _, addr := range addrs { + if addr.IP.IsGlobalUnicast() { + return true + } + } + return false +} + // MustParseMAC parses a MAC address string into a net.HardwareAddr, panicking on error. func MustParseMAC(s string) net.HardwareAddr { m, err := net.ParseMAC(s) @@ -82,7 +165,9 @@ func MustParseMAC(s string) net.HardwareAddr { // ConfigureActorVeth configures the actor veth inside the interior netns. // It assumes it is already running inside the target network namespace. -func ConfigureActorVeth(ctx context.Context) error { +// ipv6 comes from SetupActorNetwork, which decides it in the worker pod netns; +// this namespace cannot answer the question for itself. +func ConfigureActorVeth(ctx context.Context, ipv6 bool) error { // Run inside the gVisor interior netns. SetupActorNetwork has already created // the veth peer here, under its final name, so this only has to address it. // gVisor reads link names, addresses, and routes from this namespace when the @@ -107,6 +192,12 @@ func ConfigureActorVeth(ctx context.Context) error { if err := netlink.AddrReplace(actorLink, ActorVethAddr); err != nil { return fmt.Errorf("while assigning actor veth address: %w", err) } + if ipv6 { + if err := netlink.AddrReplace(actorLink, ActorVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning actor veth ipv6 address: %w", err) + } + } + if err := netlink.LinkSetUp(actorLink); err != nil { return fmt.Errorf("while bringing up actor veth: %w", err) } @@ -117,6 +208,15 @@ func ConfigureActorVeth(ctx context.Context) error { }); err != nil { return fmt.Errorf("while installing actor default route: %w", err) } + if ipv6 { + if err := netlink.RouteReplace(&netlink.Route{ + LinkIndex: actorLink.Attrs().Index, + Gw: ActorVethIPv6GwIP, + Dst: &net.IPNet{IP: net.ParseIP("::"), Mask: net.CIDRMask(0, 128)}, + }); err != nil { + return fmt.Errorf("while installing actor default ipv6 route: %w", err) + } + } return nil } @@ -173,7 +273,7 @@ func PodIPv4() (net.IP, error) { // Resolve the worker pod IPv4 address from the pod namespace's real eth0. // Because eth0 now stays in the pod namespace, this IP remains available for // both normal worker connectivity and the temporary inbound DNAT rule. - eth0Link, err := netlink.LinkByName("eth0") + eth0Link, err := netlink.LinkByName(podPrimaryIfaceName) if err != nil { return nil, fmt.Errorf("while getting pod eth0: %w", err) } @@ -259,15 +359,10 @@ func InstallActorNftablesRules(egressPort uint16) error { // rules in an ateom-owned table makes cleanup simple and avoids mutating // Kubernetes or CNI-managed chains directly. // - // The table is in the inet family so one table can carry both address - // families once the actor veth is dual-stack. NAT there needs Linux 5.2. - // A bare payload match in that family is ambiguous, so every IPv4 match - // opens with an NFPROTO comparison. An inet nat chain registers the nat - // hooks for both families, so IPv6 traffic in this netns is conntracked - // where an ip table left it untracked. - // - // TODO(#246): Add the IPv6 veth addressing and forwarding this table is - // waiting on. The actor network itself is still IPv4-only. + // The table is in the inet family so one table carries both address + // families. NAT there needs Linux 5.2. A bare payload match in that family + // is ambiguous, so every family-specific match opens with an NFPROTO + // comparison. // // The rules do three things: // @@ -300,6 +395,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", @@ -313,6 +411,11 @@ func InstallActorNftablesRules(egressPort uint16) error { Chain: postrouting, Exprs: append(ipv4SourceEqual(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{ @@ -404,6 +507,35 @@ func ipv4PayloadEqual(offset uint32, ip string) []expr.Any { } } +func ipv6SourceEqual(ip string) []expr.Any { + return ipv6PayloadEqual(8, ip) +} + +// ipv6PayloadEqual matches a 16-byte IPv6 network-header field. Offset 8 is the +// source address, where the IPv4 source sits at 12: the nfproto comparison in +// front is what keeps each rule off the other family's packets. +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}, @@ -433,6 +565,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) { @@ -618,11 +768,31 @@ func SetupActorNetwork(ctx context.Context, cfg NetworkConfig) (retErr error) { if err := netlink.AddrReplace(hostLink, HostVethAddr); err != nil { return fmt.Errorf("while assigning host veth address: %w", err) } + // Decided once, here in the worker pod netns, and carried into the interior + // netns below. Probing separately on each side would let them disagree: the + // interior netns is freshly created, so its sysctl is always the permissive + // kernel default whatever the pod's families are. + var podIPv6 bool + if podLink, err := netlink.LinkByName(podPrimaryIfaceName); err == nil { + podIPv6 = linkHasGlobalIPv6(ctx, podLink) + } + vethIPv6 := linkIPv6Enabled(HostVethName) + actorIPv6 := podIPv6 && vethIPv6 + if actorIPv6 { + if err := netlink.AddrReplace(hostLink, HostVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning host veth ipv6 address: %w", err) + } + } else { + slog.InfoContext(ctx, "actor networking is IPv4-only", + "link", HostVethName, "podHasGlobalIPv6", podIPv6, "vethIPv6Enabled", vethIPv6) + } 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 0216585f4c..2e4dd38f03 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -20,6 +20,8 @@ import ( "bytes" "context" "errors" + "net" + "os" "runtime" "testing" @@ -29,6 +31,7 @@ import ( "github.com/google/nftables/expr" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" + "golang.org/x/sys/unix" ) // withTestNetNS runs fn with the calling thread inside a throwaway netns @@ -325,9 +328,10 @@ func TestRemoveActorNftablesRulesSweepsIPv4Family(t *testing.T) { // TestSetupActorNetworkInstallsEgressRedirect covers the rule no other test in // this package builds: they all leave EgressRedirectPort zero, so the kernel -// never sees the redirect. Its acceptance is not implied by the masquerade rule -// next to it -- redirect in the inet family is separate kernel support from the -// nat chain type -- and it is the rule the whole actor egress path rides on. +// never sees either redirect. Their acceptance is not implied by the masquerade +// rule next to them -- redirect in the inet family is separate kernel support +// from the nat chain type -- and they are what the whole actor egress path +// rides on. func TestSetupActorNetworkInstallsEgressRedirect(t *testing.T) { roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") ctx := context.Background() @@ -352,31 +356,259 @@ func TestSetupActorNetworkInstallsEgressRedirect(t *testing.T) { if err != nil { t.Fatalf("listing prerouting rules of the actor table: %v", err) } - if len(rules) != 1 { - t.Fatalf("prerouting holds %d rules, want the egress redirect alone", len(rules)) + if len(rules) != 2 { + t.Fatalf("prerouting holds %d rules, want one redirect per family", len(rules)) } // Read back what the kernel stored rather than what the builder emitted: // TestActorNftablesRuleExprs already pins the builder, and what is in // doubt here is whether an inet nat chain takes these expressions at all. - var haveNFProto, havePort, haveRedir bool - for _, e := range rules[0].Exprs { - switch e := e.(type) { - case *expr.Meta: - haveNFProto = haveNFProto || e.Key == expr.MetaKeyNFPROTO - case *expr.Immediate: - havePort = havePort || bytes.Equal(e.Data, binaryutil.BigEndian.PutUint16(egressPort)) - case *expr.Redir: - haveRedir = true + // Both rules are installed whatever the pod's families are; the one whose + // source address the actor never gets simply matches nothing. + for i, rule := range rules { + var nfproto []byte + var havePort, haveRedir bool + for _, e := range rule.Exprs { + switch e := e.(type) { + case *expr.Cmp: + if nfproto == nil && len(e.Data) == 1 { + nfproto = e.Data + } + case *expr.Immediate: + havePort = havePort || bytes.Equal(e.Data, binaryutil.BigEndian.PutUint16(egressPort)) + case *expr.Redir: + haveRedir = true + } + } + wantNFProto := []byte{unix.NFPROTO_IPV4} + if i == 1 { + wantNFProto = []byte{unix.NFPROTO_IPV6} + } + if !bytes.Equal(nfproto, wantNFProto) || !havePort || !haveRedir { + t.Errorf("prerouting rule %d has nfproto=%v port=%t redir=%t, want nfproto=%v and both, got %v", + i, nfproto, havePort, haveRedir, wantNFProto, rule.Exprs) } - } - if !haveNFProto || !havePort || !haveRedir { - t.Errorf("installed redirect has nfproto=%t port=%t redir=%t, want all three, got %v", - haveNFProto, havePort, haveRedir, rules[0].Exprs) } }) } +// 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 podPrimaryIfaceName: the probe is link-scoped, so +// under any other name it answers false and the test asserts the opposite of +// what it means to. +func addPodEth0(t *testing.T, cidrs ...string) { + t.Helper() + + link := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: podPrimaryIfaceName}} + if err := netlink.LinkAdd(link); err != nil { + t.Fatalf("creating the stand-in pod %s: %v", podPrimaryIfaceName, err) + } + if err := netlink.LinkSetUp(link); err != nil { + t.Fatalf("bringing up the stand-in pod %s: %v", podPrimaryIfaceName, 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 %s: %v", cidr, podPrimaryIfaceName, err) + } + } +} + +// writeSysctl turns an IPv6 knob off in the current netns. "all" flushes the +// addresses already assigned; "default" only reaches links created afterwards. +func writeSysctl(t *testing.T, knob string) { + t.Helper() + path := "/proc/sys/net/ipv6/conf/" + knob + "/disable_ipv6" + if err := os.WriteFile(path, []byte("1\n"), 0o644); err != nil { + t.Fatalf("disabling IPv6 via %s: %v", path, err) + } +} + +// 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) +} + +// 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. +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() { + t.Errorf("%q carries global IPv6 address %s, want none", link.Attrs().Name, addr) + } + } +} + +// assertDefaultRoute requires link to carry -- or, when want is false, to not +// carry -- a default route via gw in the given family. +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) + } +} + +// TestSetupActorNetworkIPv6Gate is the truth table for who gets actor IPv6. +// Both halves have to hold: the worker pod needs a global IPv6 address of its +// own, or the actor prefers the AAAA of a dual-stack destination and the +// connection dies with nowhere to go; and the veth has to accept an IPv6 +// address at all, or the assignment fails with EPERM on the path of every +// SetupActorNetwork call and the actor never starts. +// +// The IPv4 half must come out identical in every case. +func TestSetupActorNetworkIPv6Gate(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + for _, tc := range []struct { + name string + // podAddrs go on the stand-in pod interface before setup runs. + podAddrs []string + // disable, when set, runs in the pod netns after podAddrs are assigned. + disable func(*testing.T) + wantIPv6 bool + }{{ + name: "dual-stack pod", + podAddrs: []string{"10.244.0.7/24", "fd00:10:244::7/64"}, + wantIPv6: true, + }, { + // A probe that reads the wrong link or the wrong scope fails closed, + // which every IPv4 case here would happily accept. This one notices. + name: "IPv6-only pod", + podAddrs: []string{"fd00:10:244::7/64"}, + wantIPv6: true, + }, { + // An IPv4-only cluster whose kernel still has IPv6 compiled in, so every + // capability probe says yes. This is the case that turned the IPv4 e2e + // job red. + name: "pod without IPv6", + podAddrs: []string{"10.244.0.7/24"}, + }, { + // The default on IPv4-only GKE. Writing "all" also flushes podAddrs, so + // both halves of the gate are false here. + name: "IPv6 disabled for the whole netns", + podAddrs: []string{"10.244.0.7/24", "fd00:10:244::7/64"}, + disable: func(t *testing.T) { + writeSysctl(t, "all") + writeSysctl(t, "default") + }, + }, { + // The one case the capability half is there for: the pod keeps its + // address, but the veth created next inherits disable_ipv6=1. + name: "IPv6 disabled per link", + podAddrs: []string{"10.244.0.7/24", "fd00:10:244::7/64"}, + disable: func(t *testing.T) { writeSysctl(t, "default") }, + }} { + t.Run(tc.name, func(t *testing.T) { + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + addPodEth0(t, tc.podAddrs...) + if tc.disable != nil { + tc.disable(t) + } + + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { + t.Fatalf("SetupActorNetwork: %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 tc.wantIPv6 { + assertIPv6AddrNoDAD(t, host, HostVethIPv6CIDR) + } else { + 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) + } + assertDefaultRoute(t, actor, netlink.FAMILY_V4, ActorVethGwIP, true) + + // 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 gets this right. + if tc.wantIPv6 { + assertIPv6AddrNoDAD(t, actor, ActorVethIPv6CIDR) + } else { + assertNoGlobalIPv6Addr(t, actor) + } + assertDefaultRoute(t, actor, netlink.FAMILY_V6, ActorVethIPv6GwIP, tc.wantIPv6) + return nil + }); err != nil { + t.Fatalf("inspecting interior netns: %v", err) + } + }) + }) + } +} + // TestSetupActorNetworkHostVethHWAddr covers the micro-VM requirement: a CH // snapshot freezes the guest's ARP entry for the gateway, so the worker-side // veth MAC has to be exactly the one the caller asked for, on every pod. diff --git a/internal/ateomnet/rules_linux_test.go b/internal/ateomnet/rules_linux_test.go index b6b8b53f3e..d0bd59301c 100644 --- a/internal/ateomnet/rules_linux_test.go +++ b/internal/ateomnet/rules_linux_test.go @@ -41,24 +41,44 @@ func TestActorNftablesRuleExprs(t *testing.T) { &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{169, 254, 17, 2}}, } + // meta nfproto ipv6; ip6 saddr fd00:169:254::2 + actorSourceIsIPv6 := []expr.Any{ + &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{10}}, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 8, Len: 16}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{ + 0xfd, 0x00, 0x01, 0x69, 0x02, 0x54, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x02, + }}, + } + + // meta l4proto tcp; redirect to :15001 + tcpRedirectTo15001 := []expr.Any{ + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{6}}, + &expr.Immediate{Register: 1, Data: []byte{0x3a, 0x99}}, + &expr.Redir{RegisterProtoMin: 1}, + } + tests := []struct { name string got []expr.Any want []expr.Any }{{ - name: "source match guards the payload load with nfproto", + name: "IPv4 source match guards the payload load with nfproto", got: ipv4SourceEqual(ActorVethIP), want: actorSourceIsIPv4, + }, { + name: "IPv6 source match guards the payload load with nfproto", + got: ipv6SourceEqual(ActorVethIPv6IP), + want: actorSourceIsIPv6, }, { name: "egress redirect matches actor IPv4 TCP and redirects to the port", got: ActorEgressRedirectRule(nil, nil, 15001).Exprs, - want: append(append([]expr.Any{}, actorSourceIsIPv4...), - // meta l4proto tcp; redirect to :15001 - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{6}}, - &expr.Immediate{Register: 1, Data: []byte{0x3a, 0x99}}, - &expr.Redir{RegisterProtoMin: 1}, - ), + want: append(append([]expr.Any{}, actorSourceIsIPv4...), tcpRedirectTo15001...), + }, { + name: "egress redirect matches actor IPv6 TCP and redirects to the port", + got: ActorIPv6EgressRedirectRule(nil, nil, 15001).Exprs, + want: append(append([]expr.Any{}, actorSourceIsIPv6...), tcpRedirectTo15001...), }} for _, test := range tests { @@ -77,6 +97,9 @@ func TestActorEgressRedirectRuleDisabled(t *testing.T) { if rule := ActorEgressRedirectRule(nil, nil, 0); rule != nil { t.Errorf("ActorEgressRedirectRule(0) = %v, want nil", rule.Exprs) } + if rule := ActorIPv6EgressRedirectRule(nil, nil, 0); rule != nil { + t.Errorf("ActorIPv6EgressRedirectRule(0) = %v, want nil", rule.Exprs) + } } func formatExprs(exprs []expr.Any) string {