From a1ae1b47e72e13e95e3594dc89fe5a5912fc11ad Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Mon, 12 Oct 2026 18:33:06 -0500 Subject: [PATCH 01/12] feat: Keep persistent service paths available --- .../private-service-connect/README.md | 5 +- docs/router/private-service-routes.md | 28 +++++++--- go.mod | 2 +- go.sum | 2 + internal/controller/serviceroute_access.go | 8 ++- .../controller/serviceroute_access_test.go | 56 +++++++++++++++++++ .../controller/serviceroute_programming.go | 2 +- internal/serviceroute/dataplane_test.go | 15 +++++ internal/serviceroute/planner.go | 2 +- 9 files changed, 105 insertions(+), 15 deletions(-) diff --git a/docs/enhancements/networking/private-service-connect/README.md b/docs/enhancements/networking/private-service-connect/README.md index 6729ff81..b11c343f 100644 --- a/docs/enhancements/networking/private-service-connect/README.md +++ b/docs/enhancements/networking/private-service-connect/README.md @@ -389,7 +389,8 @@ Service owners withdraw unhealthy producers. Networking integration deletes access when the consumer or service is deleted. Galactic removes obsolete programming and reconciles current lifetimes after restart. Recreated VPCs and endpoints must not inherit previous grants. The current route policy has no -authorization lease; rollout requires a bounded revocation policy. +authorization policy. Persistent access requires explicit revocation; bounded +access requires a renewed deadline. ### Data plane @@ -443,7 +444,7 @@ networking integration maps that frontend to an authorized service-side destination for the network's DNS context. Galactic enforces the private path; DNS interprets the destination and isolates zones and answers. -DNS contexts, record publication, health-aware discovery, and resolver leases +DNS contexts, record publication, health-aware discovery, and resolver access belong to the [DNS component design](https://github.com/datum-cloud/dns-operator/blob/docs/internal-dns-architecture/docs/architecture/internal-dns/README.md). The generic private-service path does not interpret DNS resources. diff --git a/docs/router/private-service-routes.md b/docs/router/private-service-routes.md index 75ef2ebe..4775ed29 100644 --- a/docs/router/private-service-routes.md +++ b/docs/router/private-service-routes.md @@ -77,16 +77,25 @@ The publishing controller supplies the producer address in `ServiceEndpoint`; `ServiceRoutePolicy.spec.frontend.address` supplies the consumer address. Both addresses must use the same IP family. Ports are not translated. -A translated policy must pin the consumer VPC by name and UID and include an -`authorization.validUntil` deadline within the next two minutes. Selected -attachments must match that VPC's allocated network identity. Kernel checks -stop new requests and established replies when the authorization expires, even -if the controller or its API is unavailable. Renew authorization explicitly to -restore access. +A translated policy pins the consumer VPC by name and UID. Selected attachments +must match that VPC's allocated network identity. Persistent access remains +active during control plane outages and ends when the policy is withdrawn or +the VPC lifetime changes. + +```yaml +spec: + authorization: + # Keep connectivity until an explicit policy or VPC lifetime change. + persistent: true +``` + +Time-bounded access sets `authorization.validUntil` within the next two minutes +and omits `persistent`. Kernel checks enforce its deadline on requests and +established replies. Persistent access must omit the deadline. Policies claiming the same frontend address, protocol, and port on one consumer attachment are rejected together. Separate VPC attachments can reuse that tuple. -The publisher owns authorization renewal and service health; Galactic enforces +The publisher owns access policy and service health; Galactic enforces network access and forwards packets. It does not interpret DNS records or zones. ## Node programming reports @@ -97,8 +106,9 @@ and input digest. It expires within 60 seconds or when authorization expires, whichever comes first. Consumer and producer nodes report independently for remote delivery. -Controllers using these reports must require the current topology digest and -all required nodes. `Accepted=True` means the policy is valid; it does not prove +Controllers require the current topology digest and all required nodes to +establish a new path. Expired reports indicate unknown observation health; they +do not revoke a persistent path. `Accepted=True` means the policy is valid; it does not prove that forwarding is installed. Reports cover network programming, not the health of the producer application. Startup withdraws previous local readiness before rebuilding the node's state. diff --git a/go.mod b/go.mod index bacbf5a8..8f8a1afd 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/spf13/viper v1.21.0 github.com/vishvananda/netlink v1.3.2-0.20261002201547-1d1c62507e3c go.datum.net/cloud v0.1.2 - go.datum.net/network v0.4.1-0.20261009192728-36ee528904ff + go.datum.net/network v0.4.1-0.20261012232803-f3b1d90d0806 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.72.0 go.opentelemetry.io/otel v1.47.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.47.0 diff --git a/go.sum b/go.sum index 0632385e..1ff25679 100644 --- a/go.sum +++ b/go.sum @@ -206,6 +206,8 @@ go.datum.net/cloud v0.1.2 h1:Bs8qR+ZCxf70bZGvjscyXeqoIIy8uNo2zjfdrWzIlGE= go.datum.net/cloud v0.1.2/go.mod h1:HTeaepXL0EqiqNsQ13K4/kGEwv+VLJ//BGrUfVIFfEA= go.datum.net/network v0.4.1-0.20261009192728-36ee528904ff h1:tHhqHnOxM3nDs7eCNeoUOcLTyTV05M3QXy4tR6CV9Hc= go.datum.net/network v0.4.1-0.20261009192728-36ee528904ff/go.mod h1:LawI34k+ddtv3kRbmjiss0Lb6QKpSr60YP3fX1IG5RU= +go.datum.net/network v0.4.1-0.20261012232803-f3b1d90d0806 h1:9/1ysKLyPQqp5fA5uwV92BtELJfhhizmIwVSmp4EOYQ= +go.datum.net/network v0.4.1-0.20261012232803-f3b1d90d0806/go.mod h1:LawI34k+ddtv3kRbmjiss0Lb6QKpSr60YP3fX1IG5RU= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.72.0 h1:Tq+E65HaNbZTeqcLw6QUhsV+/EIWdkfUH2CQzcrlE5M= diff --git a/internal/controller/serviceroute_access.go b/internal/controller/serviceroute_access.go index a9d73621..1fecaa60 100644 --- a/internal/controller/serviceroute_access.go +++ b/internal/controller/serviceroute_access.go @@ -31,7 +31,13 @@ func (r *ServiceRoutePolicyReconciler) validateAuthorization(policy *networkv1al return nil } if policy.Spec.Authorization == nil { - return errors.New("frontend translation requires authorization.validUntil") + return errors.New("frontend translation requires access authorization") + } + if policy.Spec.Authorization.Persistent { + if !policy.Spec.Authorization.ValidUntil.IsZero() { + return errors.New("persistent access must omit validUntil") + } + return nil } deadline := policy.Spec.Authorization.ValidUntil.Time now := r.now() diff --git a/internal/controller/serviceroute_access_test.go b/internal/controller/serviceroute_access_test.go index 031e2e90..501e2b97 100644 --- a/internal/controller/serviceroute_access_test.go +++ b/internal/controller/serviceroute_access_test.go @@ -321,3 +321,59 @@ func TestConflictingPoliciesFailClosedAndRecover(t *testing.T) { }) } } + +func TestPersistentAuthorizationRequiresNoDeadline(t *testing.T) { + r := &ServiceRoutePolicyReconciler{} + p := &networkv1alpha1.ServiceRoutePolicy{Spec: networkv1alpha1.ServiceRoutePolicySpec{ + Frontend: &networkv1alpha1.ServiceRouteFrontend{}, + Authorization: &networkv1alpha1.ServiceRouteAuthorization{Persistent: true}, + }} + if err := r.validateAuthorization(p); err != nil { + t.Fatal(err) + } + p.Spec.Authorization.ValidUntil = metav1.NewTime(time.Now().Add(time.Minute)) + if err := r.validateAuthorization(p); err == nil { + t.Fatal("persistent authorization accepted a deadline") + } +} + +func TestPersistentPathSurvivesControllerOutageAndWithdraws(t *testing.T) { + r, p, _ := pscFixture(t) + ctx := context.Background() + now := time.Now().UTC() + r.Now = func() time.Time { return now } + p.Spec.Authorization = &networkv1alpha1.ServiceRouteAuthorization{Persistent: true} + if err := r.Update(ctx, p); err != nil { + t.Fatal(err) + } + key := types.NamespacedName{Namespace: p.Namespace, Name: p.Name} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key}); err != nil { + t.Fatal(err) + } + now = now.Add(time.Hour) + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key}); err != nil { + t.Fatal(err) + } + if len(r.Applied) == 0 { + t.Fatal("persistent path requires authorization renewal") + } + for _, intents := range r.Applied { + for _, intent := range intents { + if !intent.AuthorizationValidUntil.IsZero() { + t.Fatal("persistent path has a dataplane deadline") + } + } + } + if err := r.Get(ctx, key, p); err != nil { + t.Fatal(err) + } + if err := r.Delete(ctx, p); err != nil { + t.Fatal(err) + } + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key}); err != nil { + t.Fatal(err) + } + if len(r.Applied) != 0 { + t.Fatal("withdrawn policy retained forwarding") + } +} diff --git a/internal/controller/serviceroute_programming.go b/internal/controller/serviceroute_programming.go index 67167426..efc4cbdf 100644 --- a/internal/controller/serviceroute_programming.go +++ b/internal/controller/serviceroute_programming.go @@ -24,7 +24,7 @@ func (r *ServiceRoutePolicyReconciler) setNodeProgramming( ctx context.Context, policy *networkv1alpha1.ServiceRoutePolicy, digest string, ready bool, ) error { deadline := r.now().Add(serviceProgrammingAckDuration) - if policy.Spec.Authorization != nil && policy.Spec.Authorization.ValidUntil.Before(&metav1.Time{Time: deadline}) { + if policy.Spec.Authorization != nil && !policy.Spec.Authorization.Persistent && policy.Spec.Authorization.ValidUntil.Before(&metav1.Time{Time: deadline}) { deadline = policy.Spec.Authorization.ValidUntil.Time } report := networkv1alpha1.ServiceRouteNodeStatus{ diff --git a/internal/serviceroute/dataplane_test.go b/internal/serviceroute/dataplane_test.go index f4642255..7a824beb 100644 --- a/internal/serviceroute/dataplane_test.go +++ b/internal/serviceroute/dataplane_test.go @@ -842,3 +842,18 @@ func TestMapReplayPrunesExpiredAuthorizationAndPreservesStartupGate(t *testing.T }) } } + +func TestPersistentDataplaneHasNoExpiry(t *testing.T) { + routes, access, grants := &recordingTable{}, &recordingTable{}, &recordingTable{} + programmer := testRouteProgrammer(routes, access, grants) + intent := localTestIntent() + if err := programmer.Apply(intent); err != nil { + t.Fatal(err) + } + if expired := programmer.expiredDesiredKeys(time.Now().Add(24 * time.Hour)); len(expired) != 0 { + t.Fatalf("persistent intents expired: %v", expired) + } + if err := programmer.Remove(intent); err != nil { + t.Fatal(err) + } +} diff --git a/internal/serviceroute/planner.go b/internal/serviceroute/planner.go index 4cbe21f0..beaf64e7 100644 --- a/internal/serviceroute/planner.go +++ b/internal/serviceroute/planner.go @@ -371,7 +371,7 @@ func consumerMatchesVPC(policy *networkv1alpha1.ServiceRoutePolicy, } func setIntentAuthorization(policy *networkv1alpha1.ServiceRoutePolicy, intents []RouteIntent) { - if policy.Spec.Frontend == nil || policy.Spec.Authorization == nil { + if policy.Spec.Frontend == nil || policy.Spec.Authorization == nil || policy.Spec.Authorization.Persistent { return } for i := range intents { From d5e5275de9a402bbde94a922cf61ded41bd279f7 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Mon, 12 Oct 2026 18:43:41 -0500 Subject: [PATCH 02/12] test: Validate persistent access with memory-backed dataplane --- internal/controller/serviceroute_programming.go | 3 ++- internal/serviceroute/dataplane_test.go | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/serviceroute_programming.go b/internal/controller/serviceroute_programming.go index efc4cbdf..c1bf1160 100644 --- a/internal/controller/serviceroute_programming.go +++ b/internal/controller/serviceroute_programming.go @@ -24,7 +24,8 @@ func (r *ServiceRoutePolicyReconciler) setNodeProgramming( ctx context.Context, policy *networkv1alpha1.ServiceRoutePolicy, digest string, ready bool, ) error { deadline := r.now().Add(serviceProgrammingAckDuration) - if policy.Spec.Authorization != nil && !policy.Spec.Authorization.Persistent && policy.Spec.Authorization.ValidUntil.Before(&metav1.Time{Time: deadline}) { + if policy.Spec.Authorization != nil && !policy.Spec.Authorization.Persistent && + policy.Spec.Authorization.ValidUntil.Before(&metav1.Time{Time: deadline}) { deadline = policy.Spec.Authorization.ValidUntil.Time } report := networkv1alpha1.ServiceRouteNodeStatus{ diff --git a/internal/serviceroute/dataplane_test.go b/internal/serviceroute/dataplane_test.go index 7a824beb..7a31860c 100644 --- a/internal/serviceroute/dataplane_test.go +++ b/internal/serviceroute/dataplane_test.go @@ -844,8 +844,7 @@ func TestMapReplayPrunesExpiredAuthorizationAndPreservesStartupGate(t *testing.T } func TestPersistentDataplaneHasNoExpiry(t *testing.T) { - routes, access, grants := &recordingTable{}, &recordingTable{}, &recordingTable{} - programmer := testRouteProgrammer(routes, access, grants) + programmer, _, _, _ := leasedTestProgrammer() intent := localTestIntent() if err := programmer.Apply(intent); err != nil { t.Fatal(err) From b235a82a5262f30ce6ce38dcd9bf6c942e27f8b0 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 10:27:48 -0500 Subject: [PATCH 03/12] ci: refresh checks after the main update From f9018c0cc5b89f98107a80390bb799a07261dd6d Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 18:16:24 -0500 Subject: [PATCH 04/12] Preserve primary routes for secondary VPC interfaces --- internal/cni/netns.go | 9 ++++++ internal/cni/netns_test.go | 57 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/internal/cni/netns.go b/internal/cni/netns.go index 6b4f0487..a5a005d3 100644 --- a/internal/cni/netns.go +++ b/internal/cni/netns.go @@ -136,6 +136,15 @@ func addDefaultRouteIfMissing(handle *netlink.Handle, link netlink.Link, ifName return fmt.Errorf("default route on %q already points via %s, refusing to add conflicting route via %s", ifName, r.Gw, gateway) } + routes, err := handle.RouteList(nil, addrFamily(gateway)) + if err != nil { + return fmt.Errorf("list namespace routes: %w", err) + } + for _, route := range routes { + if isDefaultRouteDst(route.Dst) && route.LinkIndex != link.Attrs().Index { + return nil + } + } // The on-link flag: a VPC gateway sits outside the prefix allocated to the // attachment, for either family, so nothing on the interface tells the diff --git a/internal/cni/netns_test.go b/internal/cni/netns_test.go index 1d3749f8..f1f7a860 100644 --- a/internal/cni/netns_test.go +++ b/internal/cni/netns_test.go @@ -476,3 +476,60 @@ func TestFlushGuestNetnsConfigThenRetryAddSucceeds(t *testing.T) { t.Fatalf("default route gateway = %v, want %v", gw, testGateway) } } + +func TestSecondaryInterfacePreservesPrimaryDefault(t *testing.T) { + netnsPath, cleanup := createTestNetnsWithDummy(t) + defer cleanup() + netns, err := ns.GetNS(netnsPath) + if err != nil { + t.Fatal(err) + } + defer netns.Close() + primaryGateway := net.ParseIP("fd00:99::1") + err = netns.Do(func(_ ns.NetNS) error { + handle, err := netlink.NewHandle() + if err != nil { + return err + } + defer handle.Close() + primary := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "primary"}} + if err := handle.LinkAdd(primary); err != nil { + return err + } + if err := handle.LinkSetUp(primary); err != nil { + return err + } + return handle.RouteAdd(&netlink.Route{Gw: primaryGateway, LinkIndex: primary.Attrs().Index, Flags: int(netlink.FLAG_ONLINK)}) + }) + if err != nil { + t.Fatal(err) + } + for range 2 { + if err := configureInterfaceInNetns(netnsPath, "test-dummy", testIPNet, testGateway, nil, nil); err != nil { + t.Fatal(err) + } + } + if gateway := defaultRouteVia(t, netnsPath); gateway != nil { + t.Fatalf("secondary interface installed default route via %s", gateway) + } + err = netns.Do(func(_ ns.NetNS) error { + handle, err := netlink.NewHandle() + if err != nil { + return err + } + defer handle.Close() + routes, err := handle.RouteList(nil, netlink.FAMILY_V6) + if err != nil { + return err + } + for _, route := range routes { + if isDefaultRouteDst(route.Dst) && route.Gw.Equal(primaryGateway) { + return nil + } + } + return fmt.Errorf("primary default route is missing") + }) + if err != nil { + t.Fatal(err) + } +} From 1f8361ad6ebc9e0bb08b7deffaee690816cbc03e Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 18:39:42 -0500 Subject: [PATCH 05/12] fix: route delegated subnets through veth guests --- internal/gc/datapath_repair.go | 10 ++++++---- internal/gc/datapath_repair_test.go | 20 ++++++++++++++++++++ internal/hostgw/hostgw.go | 12 ++++++++++++ internal/hostgw/hostgw_test.go | 29 +++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/internal/gc/datapath_repair.go b/internal/gc/datapath_repair.go index d02c73aa..ff4d67b1 100644 --- a/internal/gc/datapath_repair.go +++ b/internal/gc/datapath_repair.go @@ -557,7 +557,8 @@ func (c routeCache) get(tableID uint32) ([]netlink.Route, error) { // localAddressing recovers what CNI ADD registered as link's local prefixes and // gateways from the kernel. The guest prefixes are the pod-subnet routes the // master plugin installs in the VRF's table out link: unicast, not -// kernel-generated, with no gateway and not link-scoped. The master plugin +// kernel-generated, and not link-scoped. Delegated veth prefixes use their +// guest address as an on-link next hop. The master plugin // installs one only for a family with a gateway, so a family without one is // missing here; discoverAttachments adds it from the BGPAdvertisement. The // gateways are link's own addresses that are not link-local, each also a host @@ -610,9 +611,10 @@ func setGateway(gw *attachreg.Gateways, ip net.IP) { } // isPodSubnetRoute reports whether r is a pod-subnet route out ifindex, as -// opposed to a kernel-generated route or a termination route with a gateway or -// link scope. +// opposed to a kernel-generated route or a termination route. A delegated +// prefix can use its first address as an on-link guest next hop. func isPodSubnetRoute(r *netlink.Route, ifindex int) bool { - return r.Dst != nil && r.LinkIndex == ifindex && r.Gw == nil && len(r.MultiPath) == 0 && + return r.Dst != nil && r.LinkIndex == ifindex && (r.Gw == nil || + (r.Gw.Equal(r.Dst.IP) && r.Flags&int(netlink.FLAG_ONLINK) != 0)) && len(r.MultiPath) == 0 && r.Type == unix.RTN_UNICAST && r.Protocol != unix.RTPROT_KERNEL && r.Scope != netlink.SCOPE_LINK } diff --git a/internal/gc/datapath_repair_test.go b/internal/gc/datapath_repair_test.go index 0c53941f..cc8949ba 100644 --- a/internal/gc/datapath_repair_test.go +++ b/internal/gc/datapath_repair_test.go @@ -1054,3 +1054,23 @@ func TestRepairAttachmentDatapath_NodeIdentity(t *testing.T) { } }) } + +func TestRepairAttachmentDatapathDelegatedGuestRoute(t *testing.T) { + f := newRepairFixture(t) + vrf := f.addVRF("G0000000jUV", 10, 7) + f.addVRFInstance(crdnames.BGPVRFInstanceName("jU", repairNode), 0x21) + f.addHostLink(attachreg.InterfaceTypeVeth, "G0000000jUabcH", 11, vrf) + f.routes[7] = append(f.routes[7], netlink.Route{ + Dst: mustCIDR(t, "fd20:30:ff01::/96"), LinkIndex: 11, + Type: unix.RTN_UNICAST, Protocol: unix.RTPROT_BOOT, + Gw: net.ParseIP("fd20:30:ff01::"), Flags: int(netlink.FLAG_ONLINK), + }) + result, err := f.run() + if err != nil { + t.Fatal(err) + } + if result.Rebuilt.LocalEgressRoutes != 1 { + t.Fatalf("rebuilt %d routes, want 1", result.Rebuilt.LocalEgressRoutes) + } + wantPassThrough(t, f.mem, 7, "fd20:30:ff01::/96") +} diff --git a/internal/hostgw/hostgw.go b/internal/hostgw/hostgw.go index 139f7257..4ffc3975 100644 --- a/internal/hostgw/hostgw.go +++ b/internal/hostgw/hostgw.go @@ -225,6 +225,10 @@ func installPodSubnetRoute(hostLink netlink.Link, subnet *net.IPNet, family, tab LinkIndex: hostLink.Attrs().Index, Table: tableID, } + if ones, bits := subnet.Mask.Size(); hostLink.Type() == "veth" && bits == 128 && ones < bits { + desiredRoute.Gw = subnet.IP + desiredRoute.Flags = int(netlink.FLAG_ONLINK) + } existingRoutes, err := netlink.RouteListFiltered( family, @@ -241,6 +245,14 @@ func installPodSubnetRoute(hostLink netlink.Link, subnet *net.IPNet, family, tab continue } if r.Dst.String() == desiredRoute.Dst.String() { + if r.LinkIndex == desiredRoute.LinkIndex && r.Gw == nil && desiredRoute.Gw != nil && + isPodSubnetRouteCandidate(&r) && len(r.MultiPath) == 0 { + if err := netlink.RouteReplace(desiredRoute); err != nil { + return fmt.Errorf("route delegated subnet through guest address: %w", err) + } + alreadyPresent = true + continue + } if routeConflicts(&r, desiredRoute) { return fmt.Errorf( "existing route %v to %s conflicts with desired route %v", diff --git a/internal/hostgw/hostgw_test.go b/internal/hostgw/hostgw_test.go index 115dbf5d..19249ff2 100644 --- a/internal/hostgw/hostgw_test.go +++ b/internal/hostgw/hostgw_test.go @@ -303,3 +303,32 @@ func addDummyLink(t *testing.T, name string) netlink.Link { }) return link } + +func TestDelegatedVethSubnetUsesGuestNextHop(t *testing.T) { + requireRoot(t) + link := &netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: "hgwdns0"}, PeerName: "hgwdns1"} + if err := netlink.LinkAdd(link); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = netlink.LinkDel(link) }) + if err := netlink.LinkSetUp(link); err != nil { + t.Fatal(err) + } + subnet := mustParseCIDR(t, "fd20:0:e::2:0:0/96") + const tableID = 251 + if err := netlink.RouteAdd(&netlink.Route{Dst: subnet, LinkIndex: link.Attrs().Index, Table: tableID}); err != nil { + t.Fatal(err) + } + for i := 0; i < 2; i++ { + if err := installPodSubnetRoute(link, subnet, netlink.FAMILY_V6, tableID, true); err != nil { + t.Fatal(err) + } + } + routes, err := netlink.RouteListFiltered(netlink.FAMILY_V6, &netlink.Route{Table: tableID}, netlink.RT_FILTER_TABLE) + if err != nil { + t.Fatal(err) + } + if len(routes) != 1 || !routes[0].Gw.Equal(subnet.IP) || routes[0].Flags&int(netlink.FLAG_ONLINK) == 0 { + t.Fatalf("delegated route must use the guest next hop: %+v", routes) + } +} From 2c8ab59841bd9fafe15b36a117ad55fd54b5d0da Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 19:11:56 -0500 Subject: [PATCH 06/12] fix: deliver private service traffic through attachment next hops --- internal/plumbing/ebpf/prog/doc.go | 2 +- internal/plumbing/ebpf/prog/usid.c | 35 ++++++++- internal/plumbing/ebpf/prog/usid_test.go | 15 ++++ .../plumbing/ebpf/serviceroutemap/table.go | 76 ++++++++++++++++++- .../ebpf/serviceroutemap/table_test.go | 36 +++++++++ internal/serviceroute/dataplane.go | 33 +++++--- internal/serviceroute/dataplane_test.go | 48 ++++++------ internal/serviceroute/planner.go | 14 ++++ internal/serviceroute/planner_test.go | 16 ++++ 9 files changed, 236 insertions(+), 39 deletions(-) diff --git a/internal/plumbing/ebpf/prog/doc.go b/internal/plumbing/ebpf/prog/doc.go index b25a5eb4..fd910248 100644 --- a/internal/plumbing/ebpf/prog/doc.go +++ b/internal/plumbing/ebpf/prog/doc.go @@ -40,4 +40,4 @@ package prog // -cc is deliberately omitted: bpf2go defaults to $BPF2GO_CC or "clang", so CI // can pin an exact version without this directive changing. // -//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cflags "-O2 -g -Wall -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include/aarch64-linux-gnu" -target bpfel,bpfeb -type locator_value -type function_value -type vrf_value -type ifindex_vrf_value -type nptv6_value -type vip_xlat_key -type vip_xlat_value -type egress_route_key -type egress_route_value -type service_route_key -type service_route_value -type service_access_key -type service_access_value -type service_deny_key -type service_deny_value -type service_reverse_key -type service_reverse_value -type service_remote_grant_key -type service_remote_grant_value -type service_policy_state_value -type public_uplink_value -type egress_shard_group_value -type egress_shard_member -type egress_shard_pin_key -type egress_shard_pin_value -type egress_shard_counter -type mss_clamp_value -type tenant_gw_value -type vpc_attribution_value -type vrf_direction_counter -type vrf_traffic_value -type tp_state_value -type tp_iface_value -type tp_iface_addr_key -type tp_binding_key -type tp_entry -type tp_header -type tp_counter -type tp_fallback_key Usid usid.c +//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cflags "-O2 -g -Wall -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include/aarch64-linux-gnu" -target bpfel,bpfeb -type locator_value -type function_value -type vrf_value -type ifindex_vrf_value -type nptv6_value -type vip_xlat_key -type vip_xlat_value -type egress_route_key -type egress_route_value -type service_route_key -type service_route_value -type service_access_key -type service_access_value -type service_deny_key -type service_deny_value -type service_reverse_key -type service_reverse_value -type service_remote_grant_key -type service_remote_grant_value -type service_peer_key -type service_peer_value -type service_policy_state_value -type public_uplink_value -type egress_shard_group_value -type egress_shard_member -type egress_shard_pin_key -type egress_shard_pin_value -type egress_shard_counter -type mss_clamp_value -type tenant_gw_value -type vpc_attribution_value -type vrf_direction_counter -type vrf_traffic_value -type tp_state_value -type tp_iface_value -type tp_iface_addr_key -type tp_binding_key -type tp_entry -type tp_header -type tp_counter -type tp_fallback_key Usid usid.c diff --git a/internal/plumbing/ebpf/prog/usid.c b/internal/plumbing/ebpf/prog/usid.c index bb963dd8..4dbfb005 100644 --- a/internal/plumbing/ebpf/prog/usid.c +++ b/internal/plumbing/ebpf/prog/usid.c @@ -1454,6 +1454,27 @@ struct { __type(value, __u64); } attachment_identity_table SEC(".maps"); +struct service_peer_key { + __u32 ifindex; + __u8 family; + __u8 pad[3]; +}; + +struct service_peer_value { + __u64 token; + __u8 address[16]; +}; + +// One next hop per attachment and family delivers virtual service addresses +// through the guest's neighbor entry without resolving each virtual address. +struct { + __uint(type, BPF_MAP_TYPE_HASH); + __uint(map_flags, BPF_F_NO_PREALLOC); + __uint(max_entries, 65536); + __type(key, struct service_peer_key); + __type(value, struct service_peer_value); +} service_peer_table SEC(".maps"); + struct { __uint(type, BPF_MAP_TYPE_HASH); __uint(map_flags, BPF_F_NO_PREALLOC); @@ -4537,12 +4558,22 @@ static USID_ALWAYS_INLINE long service_redirect(struct __sk_buff *skb, if (service_decrement_hop(skb, scratch)) return TC_ACT_SHOT; __builtin_memset(&scratch->neigh, 0, sizeof(scratch->neigh)); + struct service_peer_key key = {.ifindex = target_ifindex, .family = scratch->meta_family}; + struct service_peer_value *peer = bpf_map_lookup_elem(&service_peer_table, &key); + if (peer && !service_identity_matches(target_ifindex, peer->token)) + return TC_ACT_SHOT; if (scratch->meta_family == USID_EGRESS_ROUTE_FAMILY_INET6) { scratch->neigh.nh_family = USID_AF_INET6; - __builtin_memcpy(scratch->neigh.ipv6_nh, scratch->dest_addr, 16); + if (peer) + __builtin_memcpy(scratch->neigh.ipv6_nh, peer->address, 16); + else + __builtin_memcpy(scratch->neigh.ipv6_nh, scratch->dest_addr, 16); } else { scratch->neigh.nh_family = USID_AF_INET; - __builtin_memcpy(&scratch->neigh.ipv4_nh, scratch->dest_addr, 4); + if (peer) + __builtin_memcpy(&scratch->neigh.ipv4_nh, peer->address, 4); + else + __builtin_memcpy(&scratch->neigh.ipv4_nh, scratch->dest_addr, 4); } return bpf_redirect_neigh(target_ifindex, &scratch->neigh, sizeof(scratch->neigh), 0); } diff --git a/internal/plumbing/ebpf/prog/usid_test.go b/internal/plumbing/ebpf/prog/usid_test.go index 9d6d7238..3c040cce 100644 --- a/internal/plumbing/ebpf/prog/usid_test.go +++ b/internal/plumbing/ebpf/prog/usid_test.go @@ -1567,6 +1567,21 @@ func TestUsidEgress_ServiceRouteAllowsAuthorizedPort(t *testing.T) { if got := sumPerCPU(t, objs.DropReasons, 40); got != 0 { t.Errorf("drop_reasons[service_route_denied] = %d, want 0", got) } + peerKey := UsidServicePeerKey{Ifindex: loopback, Family: egressRouteFamilyINET6} + peer := UsidServicePeerValue{Token: 99, Address: netip.MustParseAddr("fd20::4").As16()} + if err := objs.ServicePeerTable.Put(peerKey, peer); err != nil { + t.Fatal(err) + } + if ret, _, err := objs.UsidServiceEgress.Test(pkt); err != nil || ret != tcActShot { + t.Fatalf("stale attachment next hop verdict = %d, err = %v; want drop", ret, err) + } + peer.Token = 1 + if err := objs.ServicePeerTable.Put(peerKey, peer); err != nil { + t.Fatal(err) + } + if ret, _, err := objs.UsidServiceEgress.Test(pkt); err != nil || ret != tcActRedirect { + t.Fatalf("current attachment next hop verdict = %d, err = %v; want redirect", ret, err) + } // Simulate deletion and immediate kernel ifindex reuse. The stale route and // access keys still match ifindex 1, but their old incarnation token must // not authorize the replacement interface. diff --git a/internal/plumbing/ebpf/serviceroutemap/table.go b/internal/plumbing/ebpf/serviceroutemap/table.go index a9d72916..8c1dfff4 100644 --- a/internal/plumbing/ebpf/serviceroutemap/table.go +++ b/internal/plumbing/ebpf/serviceroutemap/table.go @@ -27,6 +27,7 @@ const ( attachmentIdentityMapName = "attachment_identity_table" servicePolicyStateMapName = "service_policy_state_table" serviceDenyMapName = "service_deny_table" + servicePeerMapName = "service_peer_table" familyIPv6 = uint8(0) familyIPv4 = uint8(1) @@ -196,6 +197,7 @@ type Tables struct { identities usidmap.Table policyState usidmap.Table deny usidmap.Table + peers usidmap.Table leaseClockFn func() (time.Time, uint64, error) } @@ -248,12 +250,19 @@ func OpenPinned(pinDir string) (*Tables, []*ebpf.Map, error) { } return nil, nil, fmt.Errorf("serviceroutemap: open %s: %w", serviceDenyMapName, err) } + peers, err := ebpf.LoadPinnedMap(filepath.Join(pinDir, servicePeerMapName), nil) + if err != nil { + for _, opened := range []*ebpf.Map{routes, access, reverse, remoteGrants, identities, policyState, deny} { + _ = opened.Close() + } + return nil, nil, fmt.Errorf("serviceroutemap: open %s: %w", servicePeerMapName, err) + } return New( usidmap.KernelTable{Map: routes}, usidmap.KernelTable{Map: access}, usidmap.KernelTable{Map: reverse}, usidmap.KernelTable{Map: remoteGrants}, usidmap.KernelTable{Map: identities}, - usidmap.KernelTable{Map: policyState}, usidmap.KernelTable{Map: deny}, + usidmap.KernelTable{Map: policyState}, usidmap.KernelTable{Map: deny}, usidmap.KernelTable{Map: peers}, ), - []*ebpf.Map{routes, access, reverse, remoteGrants, identities, policyState, deny}, nil + []*ebpf.Map{routes, access, reverse, remoteGrants, identities, policyState, deny, peers}, nil } // New wraps map implementations. Production uses OpenPinned; tests use fakes. @@ -271,9 +280,71 @@ func New(routes, access, reverse usidmap.Table, remoteGrants ...usidmap.Table) * if len(remoteGrants) > 3 { t.deny = remoteGrants[3] } + if len(remoteGrants) > 4 { + t.peers = remoteGrants[4] + } return t } +type servicePeerKey struct { + Ifindex uint32 + Family uint8 + Pad [3]uint8 +} + +type servicePeerValue struct { + Token uint64 + Address [16]uint8 +} + +// SetPeer records an attachment's next hop, fenced by its current identity. +func (t *Tables) SetPeer(ifindex uint32, token uint64, address net.IP) error { + if address == nil { + return nil + } + if t.peers == nil { + return errors.New("serviceroutemap: service peer table unavailable") + } + key := servicePeerKey{Ifindex: ifindex} + if address.To4() != nil { + key.Family = familyIPv4 + } + value := servicePeerValue{Token: token} + copy(value.Address[:], addressBytes(address)) + if err := t.peers.Put(key, value); err != nil { + return fmt.Errorf("serviceroutemap: register attachment next hop: %w", err) + } + return nil +} + +func (t *Tables) sweepPeers() error { + if t.peers == nil { + return nil + } + iterator := t.peers.Iterate() + var key servicePeerKey + var value servicePeerValue + var stale []servicePeerKey + for iterator.Next(&key, &value) { + token, err := t.IdentityToken(key.Ifindex) + if err != nil && !errors.Is(err, ebpf.ErrKeyNotExist) { + return err + } + if err != nil || token != value.Token { + stale = append(stale, key) + } + } + if err := iterator.Err(); err != nil { + return fmt.Errorf("serviceroutemap: iterate attachment next hops: %w", err) + } + for _, key := range stale { + if err := t.peers.Delete(key); err != nil && !errors.Is(err, ebpf.ErrKeyNotExist) { + return fmt.Errorf("serviceroutemap: remove attachment next hop: %w", err) + } + } + return nil +} + // SetPolicyEnabled gates all service authorization while the controller is // rebuilding a complete cache-synchronized generation. func (t *Tables) SetPolicyEnabled(enabled bool) error { @@ -519,6 +590,7 @@ func (t *Tables) SweepPolicy(snapshot PolicySnapshot) error { deleteErrs = append(deleteErrs, fmt.Errorf("serviceroutemap: sweep deny catalog: %w", err)) } } + deleteErrs = append(deleteErrs, t.sweepPeers()) return errors.Join(deleteErrs...) } diff --git a/internal/plumbing/ebpf/serviceroutemap/table_test.go b/internal/plumbing/ebpf/serviceroutemap/table_test.go index ab56518a..dda3f429 100644 --- a/internal/plumbing/ebpf/serviceroutemap/table_test.go +++ b/internal/plumbing/ebpf/serviceroutemap/table_test.go @@ -422,3 +422,39 @@ func TestAuthorizationLeasePinnedReplayAndRenewal(t *testing.T) { }) } } + +func TestPeerNextHopFollowsAttachmentIdentity(t *testing.T) { + identities := &memoryTable{} + peers := &memoryTable{} + tables := &Tables{identities: identities, peers: peers} + if err := identities.Put(uint32(42), uint64(7)); err != nil { + t.Fatal(err) + } + for _, address := range []string{"fd20::4", "192.0.2.4"} { + if err := tables.SetPeer(42, 7, net.ParseIP(address)); err != nil { + t.Fatal(err) + } + } + if err := tables.sweepPeers(); err != nil { + t.Fatal(err) + } + if len(peers.entries) != 2 { + t.Fatalf("live peer entries = %d, want 2", len(peers.entries)) + } + var v6 servicePeerValue + if err := peers.Lookup(servicePeerKey{Ifindex: 42}, &v6); err != nil { + t.Fatal(err) + } + if !net.IP(v6.Address[:]).Equal(net.ParseIP("fd20::4")) { + t.Fatalf("IPv6 next hop = %x", v6.Address) + } + if err := identities.Put(uint32(42), uint64(8)); err != nil { + t.Fatal(err) + } + if err := tables.sweepPeers(); err != nil { + t.Fatal(err) + } + if len(peers.entries) != 0 { + t.Fatal("recycled attachment retained next hops") + } +} diff --git a/internal/serviceroute/dataplane.go b/internal/serviceroute/dataplane.go index 5d978879..d7e65bae 100644 --- a/internal/serviceroute/dataplane.go +++ b/internal/serviceroute/dataplane.go @@ -111,7 +111,7 @@ type EBPFRouteProgrammer struct { mu sync.Mutex tables *serviceroutemap.Tables mapHandles []*ebpf.Map - mapIDs [7]ebpf.MapID + mapIDs [8]ebpf.MapID mapIDsValid bool routeRefs map[string]routeState accessRefs map[accessKey]accessState @@ -128,8 +128,8 @@ type EBPFRouteProgrammer struct { // These hooks keep map replacement and interface recreation tests // deterministic without requiring a privileged kernel eBPF setup. - openMapSetFn func(string) (*serviceroutemap.Tables, []*ebpf.Map, [7]ebpf.MapID, error) - readMapIDsFn func(string) ([7]ebpf.MapID, error) + openMapSetFn func(string) (*serviceroutemap.Tables, []*ebpf.Map, [8]ebpf.MapID, error) + readMapIDsFn func(string) ([8]ebpf.MapID, error) targetIndexFn func(string) (uint32, error) } @@ -388,6 +388,7 @@ var serviceRouteMapNames = [...]string{ "attachment_identity_table", "service_policy_state_table", "service_deny_table", + "service_peer_table", } func (p *EBPFRouteProgrammer) mapSetChanged() (bool, error) { @@ -401,32 +402,32 @@ func (p *EBPFRouteProgrammer) mapSetChanged() (bool, error) { return ids != p.mapIDs, nil } -func (p *EBPFRouteProgrammer) openMapSet(pinDir string) (*serviceroutemap.Tables, []*ebpf.Map, [7]ebpf.MapID, error) { +func (p *EBPFRouteProgrammer) openMapSet(pinDir string) (*serviceroutemap.Tables, []*ebpf.Map, [8]ebpf.MapID, error) { if p.openMapSetFn != nil { return p.openMapSetFn(pinDir) } tables, handles, err := serviceroutemap.OpenPinned(pinDir) if err != nil { - return nil, nil, [7]ebpf.MapID{}, err + return nil, nil, [8]ebpf.MapID{}, err } - var ids [7]ebpf.MapID + var ids [8]ebpf.MapID for index, handle := range handles { ids[index], err = kernelMapID(handle) if err != nil { for _, opened := range handles { _ = opened.Close() } - return nil, nil, [7]ebpf.MapID{}, err + return nil, nil, [8]ebpf.MapID{}, err } } return tables, handles, ids, nil } -func (p *EBPFRouteProgrammer) readMapIDs(pinDir string) ([7]ebpf.MapID, error) { +func (p *EBPFRouteProgrammer) readMapIDs(pinDir string) ([8]ebpf.MapID, error) { if p.readMapIDsFn != nil { return p.readMapIDsFn(pinDir) } - var ids [7]ebpf.MapID + var ids [8]ebpf.MapID for index, name := range serviceRouteMapNames { current, err := ebpf.LoadPinnedMap(filepath.Join(pinDir, name), nil) if err != nil { @@ -459,7 +460,7 @@ func (p *EBPFRouteProgrammer) closeHandles() { } p.mapHandles = nil p.tables = nil - p.mapIDs = [7]ebpf.MapID{} + p.mapIDs = [8]ebpf.MapID{} p.mapIDsValid = false } @@ -657,6 +658,9 @@ func (p *EBPFRouteProgrammer) entries(intent RouteIntent) ([]appliedEntry, error if err != nil { return nil, err } + if err := p.tables.SetPeer(serviceIfindex, producerToken, intent.ServiceNextHop); err != nil { + return nil, err + } // Producer-side source marker for fragments, whose missing transport // tuple cannot be matched against an individual grant. entries = append(entries, appliedEntry{access: &accessRef{ @@ -688,6 +692,9 @@ func (p *EBPFRouteProgrammer) entries(intent RouteIntent) ([]appliedEntry, error if err != nil { return nil, err } + if err := p.tables.SetPeer(consumerIfindex, consumerToken, intent.ConsumerNextHop); err != nil { + return nil, err + } // A protocol/port-zero marker makes malformed or non-TCP/UDP traffic to a // configured service fail closed without conflating distinct real tuples. entries = append(entries, appliedEntry{access: &accessRef{ @@ -715,6 +722,9 @@ func (p *EBPFRouteProgrammer) entries(intent RouteIntent) ([]appliedEntry, error if err != nil { return nil, err } + if err := p.tables.SetPeer(targetIfindex, producerToken, intent.ServiceNextHop); err != nil { + return nil, err + } // Local replies need the same address-only producer marker. Its // refcount is independent from the consumer marker because ifindex is // part of the key. @@ -1014,6 +1024,7 @@ func intentKey(intent RouteIntent) string { fmt.Fprintf(&builder, "%s|%s|%s|%s|%s|%s|%s|%s|%s|%x", intent.Attachment, intent.ProducerAttachment, intent.Kind, intent.Service, intent.Frontend, intent.ConsumerDevice, intent.ServiceDevice, intent.ServiceSID, intent.ConsumerSID, intent.GrantID) + fmt.Fprintf(&builder, "|peers:%s,%s", intent.ConsumerNextHop, intent.ServiceNextHop) fmt.Fprintf(&builder, "|lease:%d", authorizationUnixNano(intent.AuthorizationValidUntil)) for _, port := range intent.Ports { fmt.Fprintf(&builder, "|%s:%d", port.Protocol, port.Port) @@ -1035,6 +1046,8 @@ func cloneRouteIntent(intent RouteIntent) RouteIntent { } cloned.ServiceSID = append(net.IP(nil), intent.ServiceSID...) cloned.ConsumerSID = append(net.IP(nil), intent.ConsumerSID...) + cloned.ConsumerNextHop = append(net.IP(nil), intent.ConsumerNextHop...) + cloned.ServiceNextHop = append(net.IP(nil), intent.ServiceNextHop...) cloned.Ports = append([]api.ServiceRouteProtocolPort(nil), intent.Ports...) return cloned } diff --git a/internal/serviceroute/dataplane_test.go b/internal/serviceroute/dataplane_test.go index c99f11bc..a4cdfcb1 100644 --- a/internal/serviceroute/dataplane_test.go +++ b/internal/serviceroute/dataplane_test.go @@ -264,10 +264,10 @@ func TestInitializePreservesPinnedState(t *testing.T) { reverse := &recordingTable{iterateCount: 1} grants := &recordingTable{iterateCount: 1} programmer := &EBPFRouteProgrammer{ - openMapSetFn: func(string) (*serviceroutemap.Tables, []*ebpf.Map, [7]ebpf.MapID, error) { + openMapSetFn: func(string) (*serviceroutemap.Tables, []*ebpf.Map, [8]ebpf.MapID, error) { return serviceroutemap.New(routes, access, reverse, grants, &recordingTable{identityTokens: map[uint32]uint64{}}, &recordingTable{}, &recordingTable{}), nil, - [7]ebpf.MapID{1, 2, 3, 4, 5, 6}, nil + [8]ebpf.MapID{1, 2, 3, 4, 5, 6}, nil }, } @@ -282,7 +282,7 @@ func TestInitializePreservesPinnedState(t *testing.T) { func TestInitializeDoesNotDisableFinalizedPolicy(t *testing.T) { policyState := &recordingTable{} - ids := [7]ebpf.MapID{1, 2, 3, 4, 5, 6} + ids := [8]ebpf.MapID{1, 2, 3, 4, 5, 6} programmer := &EBPFRouteProgrammer{ tables: serviceroutemap.New( &recordingTable{}, &recordingTable{}, &recordingTable{}, &recordingTable{}, @@ -297,7 +297,7 @@ func TestInitializeDoesNotDisableFinalizedPolicy(t *testing.T) { appliedRefs: make(map[string][][]appliedEntry), desiredIntents: make(map[string][]RouteIntent), pendingRollbacks: make(map[string][]appliedEntry), - readMapIDsFn: func(string) ([7]ebpf.MapID, error) { return ids, nil }, + readMapIDsFn: func(string) ([8]ebpf.MapID, error) { return ids, nil }, } if err := programmer.Initialize(); err != nil { t.Fatalf("first Initialize: %v", err) @@ -316,9 +316,9 @@ func TestInitializeDoesNotDisableFinalizedPolicy(t *testing.T) { func TestInitializeRepairsRecreatedInterface(t *testing.T) { routes, access, grants := &recordingTable{}, &recordingTable{}, &recordingTable{} programmer := testRouteProgrammer(routes, access, grants) - programmer.mapIDs = [7]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} programmer.mapIDsValid = true - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return programmer.mapIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } consumerIndex := uint32(10) programmer.targetIndexFn = func(name string) (uint32, error) { switch name { @@ -354,9 +354,9 @@ func TestInitializeRepairsRecreatedInterface(t *testing.T) { func TestInitializeRepairsSharedRefsAsCompleteGeneration(t *testing.T) { routes, access, grants := &recordingTable{}, &recordingTable{}, &recordingTable{} programmer := testRouteProgrammer(routes, access, grants) - programmer.mapIDs = [7]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} programmer.mapIDsValid = true - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return programmer.mapIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } consumerIndex := uint32(10) programmer.targetIndexFn = func(name string) (uint32, error) { switch name { @@ -406,9 +406,9 @@ func TestInitializeRepairsSharedRefsAsCompleteGeneration(t *testing.T) { func TestInitializeRetriesIncompleteGenerationRebuild(t *testing.T) { routes, access, grants := &recordingTable{}, &recordingTable{}, &recordingTable{} programmer := testRouteProgrammer(routes, access, grants) - programmer.mapIDs = [7]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} programmer.mapIDsValid = true - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return programmer.mapIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } consumerIndex := uint32(10) programmer.targetIndexFn = func(name string) (uint32, error) { if name == testConsumerDeviceName { @@ -443,9 +443,9 @@ func TestInitializeRetriesIncompleteGenerationRebuild(t *testing.T) { func TestRemoveRetryClearsSyncFailure(t *testing.T) { routes := &recordingTable{deleteErr: errors.New("transient delete failure")} programmer := testRouteProgrammer(routes, &recordingTable{}, &recordingTable{}) - programmer.mapIDs = [7]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} programmer.mapIDsValid = true - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return programmer.mapIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } programmer.targetIndexFn = func(name string) (uint32, error) { if name == testConsumerDeviceName { return 10, nil @@ -473,12 +473,12 @@ func TestRemoveRetryClearsSyncFailure(t *testing.T) { func TestMapReplacementClearsStaleMutationFailure(t *testing.T) { routes := &recordingTable{deleteErr: errors.New("transient delete failure")} programmer := testRouteProgrammer(routes, &recordingTable{}, &recordingTable{}) - oldIDs := [7]ebpf.MapID{1, 2, 3, 4, 5, 6, 7} - newIDs := [7]ebpf.MapID{11, 12, 13, 14, 15, 16, 17} + oldIDs := [8]ebpf.MapID{1, 2, 3, 4, 5, 6, 7} + newIDs := [8]ebpf.MapID{11, 12, 13, 14, 15, 16, 17} currentIDs := oldIDs programmer.mapIDs = oldIDs programmer.mapIDsValid = true - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return currentIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return currentIDs, nil } programmer.targetIndexFn = func(name string) (uint32, error) { if name == testConsumerDeviceName { return 10, nil @@ -502,7 +502,7 @@ func TestMapReplacementClearsStaleMutationFailure(t *testing.T) { newRoutes := &recordingTable{} newPolicyState := &recordingTable{} currentIDs = newIDs - programmer.openMapSetFn = func(string) (*serviceroutemap.Tables, []*ebpf.Map, [7]ebpf.MapID, error) { + programmer.openMapSetFn = func(string) (*serviceroutemap.Tables, []*ebpf.Map, [8]ebpf.MapID, error) { return serviceroutemap.New(newRoutes, &recordingTable{}, &recordingTable{}, &recordingTable{}, &recordingTable{identityTokens: map[uint32]uint64{10: 100, 20: 200}}, newPolicyState, &recordingTable{}), nil, newIDs, nil @@ -525,10 +525,10 @@ func TestMapReplacementClearsStaleMutationFailure(t *testing.T) { func TestInitializeReopensAllMapsWhenGrantMapChanges(t *testing.T) { oldRoutes, oldAccess, oldGrants := &recordingTable{}, &recordingTable{}, &recordingTable{} programmer := testRouteProgrammer(oldRoutes, oldAccess, oldGrants) - programmer.mapIDs = [7]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} programmer.mapIDsValid = true currentIDs := programmer.mapIDs - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return currentIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return currentIDs, nil } programmer.targetIndexFn = func(name string) (uint32, error) { if name == testConsumerDeviceName { return 10, nil @@ -543,7 +543,7 @@ func TestInitializeReopensAllMapsWhenGrantMapChanges(t *testing.T) { newRoutes, newAccess, newGrants := &recordingTable{}, &recordingTable{}, &recordingTable{} openCalls := 0 currentIDs[3] = 99 - programmer.openMapSetFn = func(string) (*serviceroutemap.Tables, []*ebpf.Map, [7]ebpf.MapID, error) { + programmer.openMapSetFn = func(string) (*serviceroutemap.Tables, []*ebpf.Map, [8]ebpf.MapID, error) { openCalls++ return serviceroutemap.New( newRoutes, newAccess, &recordingTable{}, newGrants, @@ -682,9 +682,9 @@ func leasedTestProgrammer() (*EBPFRouteProgrammer, *persistedPolicyTable, *persi programmer := testRouteProgrammer(&recordingTable{}, &recordingTable{}, &recordingTable{}) programmer.tables = serviceroutemap.New(routes, &persistedPolicyTable{}, &persistedPolicyTable{}, grants, &recordingTable{identityTokens: map[uint32]uint64{10: 100, 20: 200, 42: 420}}, gate, &persistedPolicyTable{}) - programmer.mapIDs = [7]ebpf.MapID{1, 2, 3, 4, 5, 6, 7} + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6, 7} programmer.mapIDsValid = true - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return programmer.mapIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } programmer.targetIndexFn = func(name string) (uint32, error) { switch name { case testConsumerDeviceName: @@ -815,9 +815,9 @@ func TestMapReplayPrunesExpiredAuthorizationAndPreservesStartupGate(t *testing.T t.Fatal(err) } } - newIDs := [7]ebpf.MapID{11, 12, 13, 14, 15, 16, 17} - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return newIDs, nil } - programmer.openMapSetFn = func(string) (*serviceroutemap.Tables, []*ebpf.Map, [7]ebpf.MapID, error) { + newIDs := [8]ebpf.MapID{11, 12, 13, 14, 15, 16, 17} + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return newIDs, nil } + programmer.openMapSetFn = func(string) (*serviceroutemap.Tables, []*ebpf.Map, [8]ebpf.MapID, error) { return serviceroutemap.New(newRoutes, &persistedPolicyTable{}, &persistedPolicyTable{}, &persistedPolicyTable{}, &recordingTable{identityTokens: map[uint32]uint64{10: 100, 20: 200, 42: 420}}, newGate, &persistedPolicyTable{}), nil, newIDs, nil diff --git a/internal/serviceroute/planner.go b/internal/serviceroute/planner.go index 39caf274..98f24322 100644 --- a/internal/serviceroute/planner.go +++ b/internal/serviceroute/planner.go @@ -41,6 +41,8 @@ type RouteIntent struct { Frontend *net.IPNet ConsumerDevice string ServiceDevice string + ConsumerNextHop net.IP + ServiceNextHop net.IP ServiceSID net.IP ConsumerSID net.IP GrantID ServiceGrantID @@ -143,6 +145,7 @@ func compile(policy *networkv1alpha1.ServiceRoutePolicy, endpoint *networkv1alph Attachment: consumerKey, ProducerAttachment: producerKey, Kind: RouteIntentLocal, Service: cloneIPNet(service), Frontend: cloneIPNet(frontend), ConsumerDevice: consumer.Status.HostInterface, ServiceDevice: producer.Status.HostInterface, Ports: clonePorts(ports), + ConsumerNextHop: attachmentNextHop(consumer, service.IP), ServiceNextHop: attachmentNextHop(producer, service.IP), }) } continue @@ -164,6 +167,7 @@ func compile(policy *networkv1alpha1.ServiceRoutePolicy, endpoint *networkv1alph Attachment: consumerKey, ProducerAttachment: producerKey, Kind: RouteIntentRemoteConsumer, Service: cloneIPNet(service), Frontend: cloneIPNet(frontend), ConsumerDevice: consumer.Status.HostInterface, ServiceSID: cloneIP(serviceSID), GrantID: grantID, Ports: clonePorts(ports), + ConsumerNextHop: attachmentNextHop(consumer, service.IP), }) } if producer.Status.Node == localNode { @@ -176,6 +180,7 @@ func compile(policy *networkv1alpha1.ServiceRoutePolicy, endpoint *networkv1alph Attachment: consumerKey, ProducerAttachment: producerKey, Kind: RouteIntentRemoteProducer, Service: cloneIPNet(service), Frontend: cloneIPNet(frontend), ServiceDevice: producer.Status.HostInterface, ConsumerSID: cloneIP(consumerSID), GrantID: grantID, Ports: clonePorts(ports), + ServiceNextHop: attachmentNextHop(producer, service.IP), }) } } @@ -189,6 +194,15 @@ func compile(policy *networkv1alpha1.ServiceRoutePolicy, endpoint *networkv1alph return intents, nil } +// attachmentNextHop returns the guest address for a routed attachment subnet. +func attachmentNextHop(attachment *cloudv1alpha1.VPCAttachment, service net.IP) net.IP { + ip, _, err := net.ParseCIDR(attachment.Status.PodSubnet) + if err != nil || (ip.To4() == nil) != (service.To4() == nil) { + return nil + } + return ip +} + func validateProducerTopology(endpoint *networkv1alpha1.ServiceEndpoint, producers []*cloudv1alpha1.VPCAttachment, ) error { diff --git a/internal/serviceroute/planner_test.go b/internal/serviceroute/planner_test.go index 8ed6d711..8da1e8ed 100644 --- a/internal/serviceroute/planner_test.go +++ b/internal/serviceroute/planner_test.go @@ -458,3 +458,19 @@ func TestServiceGrantGenerationCompatibility(t *testing.T) { }) } } + +func TestAttachmentNextHopUsesGuestAddress(t *testing.T) { + attachment := &cloudv1alpha1.VPCAttachment{} + attachment.Status.PodSubnet = "fd20::4:0:0/96" + got := attachmentNextHop(attachment, net.ParseIP("fd70::53")) + if !got.Equal(net.ParseIP("fd20::4:0:0")) { + t.Fatalf("next hop = %v", got) + } + if got := attachmentNextHop(attachment, net.ParseIP("192.0.2.53")); got != nil { + t.Fatalf("wrong-family next hop = %v", got) + } + attachment.Status.PodSubnet = "" + if got := attachmentNextHop(attachment, net.ParseIP("fd70::53")); got != nil { + t.Fatalf("missing subnet next hop = %v", got) + } +} From 3acfe9134b5b6c56ffb3df069344844e274ffcd0 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 19:13:55 -0500 Subject: [PATCH 07/12] test: satisfy lint checks for staging network regressions --- internal/cni/netns_test.go | 27 +++++++++++++++------------ internal/hostgw/hostgw_test.go | 2 +- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/internal/cni/netns_test.go b/internal/cni/netns_test.go index f1f7a860..bdbbc22b 100644 --- a/internal/cni/netns_test.go +++ b/internal/cni/netns_test.go @@ -5,6 +5,7 @@ package cni import ( + "errors" "fmt" "net" "os" @@ -59,7 +60,7 @@ func createTestNetnsWithDummy(t *testing.T) (netnsPath string, cleanup func()) { if err != nil { return err } - defer handle.Close() //nolint:errcheck // best-effort cleanup + defer func() { _ = handle.Close() }() //nolint:errcheck // best-effort cleanup dummy := &netlink.Dummy{ LinkAttrs: netlink.LinkAttrs{Name: "test-dummy"}, @@ -84,7 +85,7 @@ func createTestNetnsWithDummy(t *testing.T) (netnsPath string, cleanup func()) { if err != nil { return err } - defer handle.Close() //nolint:errcheck // best-effort cleanup + defer func() { _ = handle.Close() }() //nolint:errcheck // best-effort cleanup link, err := handle.LinkByName("test-dummy") if err != nil { return nil // already gone @@ -162,7 +163,7 @@ func TestCleanupContainerNetnsVeth(t *testing.T) { if err != nil { return err } - defer handle.Close() //nolint:errcheck // best-effort cleanup + defer func() { _ = handle.Close() }() //nolint:errcheck // best-effort cleanup veth := &netlink.Veth{ LinkAttrs: netlink.LinkAttrs{Name: guestVethName}, @@ -193,7 +194,7 @@ func TestCleanupContainerNetnsVeth(t *testing.T) { if err != nil { return err } - defer handle.Close() //nolint:errcheck // best-effort cleanup + defer func() { _ = handle.Close() }() //nolint:errcheck // best-effort cleanup for _, name := range []string{guestVethName, peerVethName} { if _, err := handle.LinkByName(name); err == nil { return fmt.Errorf("interface %q still exists after cleanup", name) @@ -234,7 +235,7 @@ func defaultRouteVia(t *testing.T, netnsPath string) net.IP { if err != nil { return err } - defer handle.Close() //nolint:errcheck // netlink cleanup on teardown + defer func() { _ = handle.Close() }() //nolint:errcheck // netlink cleanup on teardown link, err := handle.LinkByName("test-dummy") if err != nil { @@ -318,7 +319,7 @@ func TestConfigureInterfaceInNetnsAddAfterDel(t *testing.T) { if err != nil { return err } - defer handle.Close() //nolint:errcheck // netlink cleanup on teardown + defer func() { _ = handle.Close() }() //nolint:errcheck // netlink cleanup on teardown link, err := handle.LinkByName("test-dummy") if err != nil { @@ -407,7 +408,7 @@ func TestFlushGuestNetnsConfigRemovesAddressAndRoute(t *testing.T) { if err != nil { return err } - defer handle.Close() //nolint:errcheck // netlink cleanup on teardown + defer func() { _ = handle.Close() }() //nolint:errcheck // netlink cleanup on teardown link, err := handle.LinkByName("test-dummy") if err != nil { @@ -484,14 +485,14 @@ func TestSecondaryInterfacePreservesPrimaryDefault(t *testing.T) { if err != nil { t.Fatal(err) } - defer netns.Close() + defer func() { _ = netns.Close() }() primaryGateway := net.ParseIP("fd00:99::1") err = netns.Do(func(_ ns.NetNS) error { handle, err := netlink.NewHandle() if err != nil { return err } - defer handle.Close() + defer func() { _ = handle.Close() }() primary := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "primary"}} if err := handle.LinkAdd(primary); err != nil { return err @@ -499,7 +500,9 @@ func TestSecondaryInterfacePreservesPrimaryDefault(t *testing.T) { if err := handle.LinkSetUp(primary); err != nil { return err } - return handle.RouteAdd(&netlink.Route{Gw: primaryGateway, LinkIndex: primary.Attrs().Index, Flags: int(netlink.FLAG_ONLINK)}) + return handle.RouteAdd(&netlink.Route{ + Gw: primaryGateway, LinkIndex: primary.Attrs().Index, Flags: int(netlink.FLAG_ONLINK), + }) }) if err != nil { t.Fatal(err) @@ -517,7 +520,7 @@ func TestSecondaryInterfacePreservesPrimaryDefault(t *testing.T) { if err != nil { return err } - defer handle.Close() + defer func() { _ = handle.Close() }() routes, err := handle.RouteList(nil, netlink.FAMILY_V6) if err != nil { return err @@ -527,7 +530,7 @@ func TestSecondaryInterfacePreservesPrimaryDefault(t *testing.T) { return nil } } - return fmt.Errorf("primary default route is missing") + return errors.New("primary default route is missing") }) if err != nil { t.Fatal(err) diff --git a/internal/hostgw/hostgw_test.go b/internal/hostgw/hostgw_test.go index 19249ff2..8fa5763b 100644 --- a/internal/hostgw/hostgw_test.go +++ b/internal/hostgw/hostgw_test.go @@ -319,7 +319,7 @@ func TestDelegatedVethSubnetUsesGuestNextHop(t *testing.T) { if err := netlink.RouteAdd(&netlink.Route{Dst: subnet, LinkIndex: link.Attrs().Index, Table: tableID}); err != nil { t.Fatal(err) } - for i := 0; i < 2; i++ { + for range 2 { if err := installPodSubnetRoute(link, subnet, netlink.FAMILY_V6, tableID, true); err != nil { t.Fatal(err) } From dd6142df18b997bc7aff3e0c99d12b4da52f83e0 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 19:15:33 -0500 Subject: [PATCH 08/12] test: include attachment next hops in map replacement checks --- internal/serviceroute/cleanup_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/serviceroute/cleanup_test.go b/internal/serviceroute/cleanup_test.go index dc70b703..48414c86 100644 --- a/internal/serviceroute/cleanup_test.go +++ b/internal/serviceroute/cleanup_test.go @@ -19,9 +19,9 @@ func TestCleanupDrainsFailedApplyRollbackBeforeFinalize(t *testing.T) { routes := &recordingTable{putErr: applyFailure} access := &recordingTable{deleteErr: rollbackFailure} programmer := testRouteProgrammer(routes, access, &recordingTable{}) - programmer.mapIDs = [7]ebpf.MapID{1, 2, 3, 4, 5, 6, 7} + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6, 7} programmer.mapIDsValid = true - programmer.readMapIDsFn = func(string) ([7]ebpf.MapID, error) { return programmer.mapIDs, nil } + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } programmer.targetIndexFn = func(name string) (uint32, error) { switch name { case testConsumerDeviceName: From d67c99754ddf8a93374a1a0cea58ea21e690794e Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 19:43:48 -0500 Subject: [PATCH 09/12] feat: alert on persistent private service routing failures --- config/monitoring/prometheusrule.yaml | 14 ++++++++++++ config/monitoring/tests/rules-test.yaml | 30 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/config/monitoring/prometheusrule.yaml b/config/monitoring/prometheusrule.yaml index 6df7d20b..cf94dc13 100644 --- a/config/monitoring/prometheusrule.yaml +++ b/config/monitoring/prometheusrule.yaml @@ -496,6 +496,20 @@ spec: {{ $labels.kind }} routes it holds a best path for, so this node cannot reach those prefixes. The router's "route install failed" log lines name each prefix and the error, usually a SID with no route over an SRv6 uplink or an unresolved neighbor. + - alert: GalacticServiceRouteProgrammingFailing + expr: sum without (operation, result) (increase(galactic_service_route_route_operations_total{job="galactic-router",result="error"}[10m])) > 0 + for: 15m + labels: + severity: warning + service: galactic + team: connect + annotations: + summary: "Private service routing on {{ $labels.node }} keeps failing" + runbook_url: "https://github.com/datum-cloud/infra/blob/main/docs/runbooks/galactic/README.md#galacticservicerouteprogrammingfailing" + description: >- + Private service routes have repeatedly failed to apply or remove on {{ $labels.node }}. + Consumers can lose access even when the router Pod is ready and policies are accepted. + Check the router's service policy errors, attachment host interfaces, and node programming acknowledgments. - alert: GalacticRouterMetricsDown expr: up{job=~"galactic-router|galactic-router-rr"} == 0 for: 5m diff --git a/config/monitoring/tests/rules-test.yaml b/config/monitoring/tests/rules-test.yaml index 8d1f43c5..2f6d4241 100644 --- a/config/monitoring/tests/rules-test.yaml +++ b/config/monitoring/tests/rules-test.yaml @@ -2289,3 +2289,33 @@ tests: galactic-nat on dfw-worker2 has failed to renew its nat slot's lease in the XDP dispatcher more than twice in every 5-minute window for 5 minutes. Once the lease expires the dispatcher skips the slot and tenant traffic leaves untranslated. The shard's log names the error. + + # Repeated failures alert; a rollout burst and idle or disabled routing do not. + - interval: 30s + input_series: + - series: 'galactic_service_route_route_operations_total{job="galactic-router",node="failing",operation="apply",result="error"}' + values: "0+1x60" + - series: 'galactic_service_route_route_operations_total{job="galactic-router",node="recovering",operation="apply",result="error"}' + values: "0+1x5 5x55" + - series: 'galactic_service_route_route_operations_total{job="galactic-router",node="idle",operation="apply",result="error"}' + values: "0x60" + alert_rule_test: + - eval_time: 10m + alertname: GalacticServiceRouteProgrammingFailing + exp_alerts: [] + - eval_time: 25m + alertname: GalacticServiceRouteProgrammingFailing + exp_alerts: + - exp_labels: + severity: warning + service: galactic + team: connect + job: galactic-router + node: failing + exp_annotations: + summary: "Private service routing on failing keeps failing" + runbook_url: "https://github.com/datum-cloud/infra/blob/main/docs/runbooks/galactic/README.md#galacticservicerouteprogrammingfailing" + description: >- + Private service routes have repeatedly failed to apply or remove on failing. + Consumers can lose access even when the router Pod is ready and policies are accepted. + Check the router's service policy errors, attachment host interfaces, and node programming acknowledgments. From 2fee30657e64d738560b206905aefb5ee0189a7d Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 19:52:46 -0500 Subject: [PATCH 10/12] fix: report withdrawn attachments and private route programming failures --- .../controller/bgpadvertisement_controller.go | 77 +++++++++++++++++++ .../bgpadvertisement_ownership_test.go | 56 ++++++++++++++ .../controller/serviceroute_controller.go | 2 +- .../controller/serviceroute_programming.go | 7 +- .../serviceroute_programming_test.go | 43 +++++++++++ 5 files changed, 181 insertions(+), 4 deletions(-) diff --git a/internal/cloud/controller/bgpadvertisement_controller.go b/internal/cloud/controller/bgpadvertisement_controller.go index 1e2086aa..d38f2bbf 100644 --- a/internal/cloud/controller/bgpadvertisement_controller.go +++ b/internal/cloud/controller/bgpadvertisement_controller.go @@ -64,6 +64,9 @@ type BGPAdvertisementReconciler struct { func (r *BGPAdvertisementReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { var advertisement bgpv1alpha1.BGPAdvertisement if err := r.Get(ctx, req.NamespacedName, &advertisement); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, r.clearWithdrawnAdvertisement(ctx, req.NamespacedName) + } return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -123,6 +126,72 @@ func (r *BGPAdvertisementReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } +// clearWithdrawnAdvertisement removes delivery status only when no tenant +// advertisement can certify the attachment, including after a node move. +func (r *BGPAdvertisementReconciler) clearWithdrawnAdvertisement(ctx context.Context, key types.NamespacedName) error { + vpc, attachmentID, ok := galactic.SplitAdvertisementName(key.Name) + if !ok || key.Namespace != galactic.SystemNamespace { + return nil + } + identity := galactic.AdvertisementName(vpc, attachmentID) + var advertisements bgpv1alpha1.BGPAdvertisementList + if err := r.List(ctx, &advertisements, client.InNamespace(key.Namespace), + client.MatchingFields{IndexBGPAdvertisementIdentity: identity}); err != nil { + return fmt.Errorf("list replacement advertisements: %w", err) + } + for i := range advertisements.Items { + advertisement := &advertisements.Items[i] + router, err := r.routerFor(ctx, advertisement.Namespace, advertisement.Spec.RouterRef.Name) + if err != nil { + return err + } + if router == nil || bgpv1alpha1.IsLegacyOwned(router) { + return nil + } + } + var attachments cloudv1alpha1.VPCAttachmentList + if err := r.List(ctx, &attachments, client.MatchingFields{IndexVPCAttachmentIdentity: identity}); err != nil { + return fmt.Errorf("list withdrawn attachments: %w", err) + } + for i := range attachments.Items { + attachment := &attachments.Items[i] + if attachment.Status.Node == "" || key.Name != identity+"-"+attachment.Status.Node { + continue + } + attachment.Status.Node = "" + attachment.Status.HostInterface = "" + attachment.Status.VRFInterface = "" + attachment.Status.PodSubnet = "" + programmed := metav1.Condition{ + Type: cloudv1alpha1.ConditionTypeProgrammed, Status: metav1.ConditionFalse, + Reason: "AdvertisementWithdrawn", Message: "the attachment has no network advertisement", + ObservedGeneration: attachment.Generation, + } + meta.SetStatusCondition(&attachment.Status.Conditions, programmed) + if err := r.projectOntoInterface(ctx, attachment, programmed); err != nil { + return err + } + if err := r.Status().Update(ctx, attachment); err != nil { + return fmt.Errorf("clear withdrawn attachment %s: %w", client.ObjectKeyFromObject(attachment), err) + } + } + return nil +} + +func advertisementForAttachment(_ context.Context, obj client.Object) []reconcile.Request { + attachment, ok := obj.(*cloudv1alpha1.VPCAttachment) + if !ok || attachment.Status.Node == "" { + return nil + } + identity := galactic.AdvertisementName(attachment.Status.VPC, attachment.Status.VPCAttachment) + if identity == "" { + return nil + } + return []reconcile.Request{{NamespacedName: types.NamespacedName{ + Namespace: galactic.SystemNamespace, Name: identity + "-" + attachment.Status.Node, + }}} +} + // projectOntoInterface closes the Programmed condition NSO deliberately leaves // for whoever realizes the interface. func (r *BGPAdvertisementReconciler) projectOntoInterface( @@ -279,6 +348,14 @@ func programmedCondition(advertisement *bgpv1alpha1.BGPAdvertisement) metav1.Con func (r *BGPAdvertisementReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&bgpv1alpha1.BGPAdvertisement{}). + Watches(&cloudv1alpha1.VPCAttachment{}, handler.EnqueueRequestsFromMapFunc(advertisementForAttachment), + builder.WithPredicates(predicate.Funcs{UpdateFunc: func(e event.UpdateEvent) bool { + oldAttachment, oldOK := e.ObjectOld.(*cloudv1alpha1.VPCAttachment) + newAttachment, newOK := e.ObjectNew.(*cloudv1alpha1.VPCAttachment) + return oldOK && newOK && (oldAttachment.Status.Node != newAttachment.Status.Node || + oldAttachment.Status.VPC != newAttachment.Status.VPC || + oldAttachment.Status.VPCAttachment != newAttachment.Status.VPCAttachment) + }})). Watches(&networkingv1alpha.NetworkInterface{}, handler.EnqueueRequestsFromMapFunc(r.advertisementForInterface), builder.WithPredicates(predicate.Funcs{ diff --git a/internal/cloud/controller/bgpadvertisement_ownership_test.go b/internal/cloud/controller/bgpadvertisement_ownership_test.go index bcb1a486..b53b333f 100644 --- a/internal/cloud/controller/bgpadvertisement_ownership_test.go +++ b/internal/cloud/controller/bgpadvertisement_ownership_test.go @@ -190,3 +190,59 @@ func TestReplayExcludesFabricAdvertisements(t *testing.T) { } } } + +func TestWithdrawnAdvertisementClearsDeliveryStatus(t *testing.T) { + f := newOwnershipFixture(t, false) + f.reconcile(t, f.tenantAdv) + if err := f.client.Delete(context.Background(), f.tenantAdv); err != nil { + t.Fatal(err) + } + // Replaying attachments also discovers withdrawals missed while the controller was offline. + var attachment cloudv1alpha1.VPCAttachment + if err := f.client.Get(context.Background(), client.ObjectKeyFromObject(f.attachment), &attachment); err != nil { + t.Fatal(err) + } + requests := advertisementForAttachment(context.Background(), &attachment) + if len(requests) != 1 || requests[0].Name != f.tenantAdv.Name { + t.Fatalf("withdrawal requests = %v", requests) + } + if _, err := f.reconciler.Reconcile(context.Background(), requests[0]); err != nil { + t.Fatal(err) + } + if err := f.client.Get(context.Background(), client.ObjectKeyFromObject(f.attachment), &attachment); err != nil { + t.Fatal(err) + } + condition := meta.FindStatusCondition(attachment.Status.Conditions, cloudv1alpha1.ConditionTypeProgrammed) + if condition == nil || condition.Status != metav1.ConditionFalse || attachment.Status.Node != "" || attachment.Status.HostInterface != "" { + t.Fatalf("withdrawn attachment status = %+v", attachment.Status) + } + var iface networkingv1alpha.NetworkInterface + if err := f.client.Get(context.Background(), client.ObjectKeyFromObject(f.iface), &iface); err != nil { + t.Fatal(err) + } + if !meta.IsStatusConditionFalse(iface.Status.Conditions, networkingv1alpha.NetworkInterfaceProgrammed) { + t.Fatalf("withdrawn interface status = %+v", iface.Status) + } +} + +func TestWithdrawalPreservesReplacementAdvertisement(t *testing.T) { + f := newOwnershipFixture(t, false) + f.reconcile(t, f.tenantAdv) + replacement := f.tenantAdv.DeepCopy() + replacement.Name += "-replacement" + replacement.ResourceVersion = "" + if err := f.client.Create(context.Background(), replacement); err != nil { + t.Fatal(err) + } + if err := f.client.Delete(context.Background(), f.tenantAdv); err != nil { + t.Fatal(err) + } + f.reconcile(t, f.tenantAdv) + var attachment cloudv1alpha1.VPCAttachment + if err := f.client.Get(context.Background(), client.ObjectKeyFromObject(f.attachment), &attachment); err != nil { + t.Fatal(err) + } + if !meta.IsStatusConditionTrue(attachment.Status.Conditions, cloudv1alpha1.ConditionTypeProgrammed) || attachment.Status.Node == "" { + t.Fatalf("replacement lost delivery status: %+v", attachment.Status) + } +} diff --git a/internal/controller/serviceroute_controller.go b/internal/controller/serviceroute_controller.go index 163c7d0d..0b3cd313 100644 --- a/internal/controller/serviceroute_controller.go +++ b/internal/controller/serviceroute_controller.go @@ -188,7 +188,7 @@ func (r *ServiceRoutePolicyReconciler) Reconcile(ctx context.Context, req ctrl.R all = append(all, &attachments.Items[i]) } if policy.Spec.Frontend != nil { - inputDigest = serviceProgrammingDigest(policy, endpoint, all, vpcIdentity) + inputDigest = serviceProgrammingDigest(policy, endpoint, all, vpcIdentity, r.NodeName) } conflicts, conflictErr := r.conflictingPolicies(ctx, all) if conflictErr != nil { diff --git a/internal/controller/serviceroute_programming.go b/internal/controller/serviceroute_programming.go index 61f53e57..8fc1440c 100644 --- a/internal/controller/serviceroute_programming.go +++ b/internal/controller/serviceroute_programming.go @@ -6,6 +6,7 @@ package controller import ( "context" "errors" + "slices" "sort" "time" @@ -52,7 +53,7 @@ func (r *ServiceRoutePolicyReconciler) setNodeProgramming( found = true } } - if !found && !ready { + if !found && !ready && digest == "" { return nil } if !found { @@ -66,10 +67,10 @@ func (r *ServiceRoutePolicyReconciler) setNodeProgramming( } func serviceProgrammingDigest(policy *networkv1alpha1.ServiceRoutePolicy, endpoint *networkv1alpha1.ServiceEndpoint, - attachments []*cloudv1alpha1.VPCAttachment, vpcIdentity string, + attachments []*cloudv1alpha1.VPCAttachment, vpcIdentity, nodeName string, ) string { plan, err := serviceroute.ProgrammingPlan(policy, endpoint, attachments, vpcIdentity) - if err != nil { + if err != nil || !slices.Contains(plan.Nodes, nodeName) { return "" } return plan.InputDigest diff --git a/internal/controller/serviceroute_programming_test.go b/internal/controller/serviceroute_programming_test.go index 98ce4ece..21c74585 100644 --- a/internal/controller/serviceroute_programming_test.go +++ b/internal/controller/serviceroute_programming_test.go @@ -159,3 +159,46 @@ func TestDisablingFrontendWithdrawsNodeAcknowledgment(t *testing.T) { t.Fatal("disabled node retained programming proof") } } + +func TestInitialProgrammingFailureReportsUnreadyNode(t *testing.T) { + r, p, _ := pscFixture(t) + r.Programmer = &retryRouteProgrammer{failApply: 1} + ctx := context.Background() + key := types.NamespacedName{Namespace: p.Namespace, Name: p.Name} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key}); err == nil { + t.Fatal("expected programming failure") + } + current := &networkv1alpha1.ServiceRoutePolicy{} + if err := r.Get(ctx, key, current); err != nil { + t.Fatal(err) + } + if len(current.Status.Nodes) != 1 || current.Status.Nodes[0].Ready || current.Status.Nodes[0].InputDigest == "" { + t.Fatalf("missing initial failure report: %+v", current.Status.Nodes) + } + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key}); err != nil { + t.Fatal(err) + } + if err := r.Get(ctx, key, current); err != nil { + t.Fatal(err) + } + if len(current.Status.Nodes) != 1 || !current.Status.Nodes[0].Ready { + t.Fatalf("missing recovery report: %+v", current.Status.Nodes) + } +} + +func TestUninvolvedNodeDoesNotPublishProgrammingReport(t *testing.T) { + r, p, _ := pscFixture(t) + r.NodeName = "uninvolved" + ctx := context.Background() + key := types.NamespacedName{Namespace: p.Namespace, Name: p.Name} + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key}); err != nil { + t.Fatal(err) + } + current := &networkv1alpha1.ServiceRoutePolicy{} + if err := r.Get(ctx, key, current); err != nil { + t.Fatal(err) + } + if len(current.Status.Nodes) != 0 { + t.Fatalf("uninvolved node published a report: %+v", current.Status.Nodes) + } +} From bf3bd0eadcdf08e1f4ee561783eeff514b265e93 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 20:32:32 -0500 Subject: [PATCH 11/12] fix: withdraw routes for missing interfaces --- internal/serviceroute/dataplane.go | 15 ++++- internal/serviceroute/dataplane_test.go | 90 +++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/internal/serviceroute/dataplane.go b/internal/serviceroute/dataplane.go index d7e65bae..39866830 100644 --- a/internal/serviceroute/dataplane.go +++ b/internal/serviceroute/dataplane.go @@ -464,10 +464,21 @@ func (p *EBPFRouteProgrammer) closeHandles() { p.mapIDsValid = false } +// Missing links withdraw their entries while preserving intent for recreation. +// Other lookup errors cannot prove that an interface has disappeared. +func (p *EBPFRouteProgrammer) recoverableEntries(intent RouteIntent) ([]appliedEntry, error) { + entries, err := p.entries(intent) + var missing netlink.LinkNotFoundError + if errors.As(err, &missing) { + return []appliedEntry{}, nil + } + return entries, err +} + func (p *EBPFRouteProgrammer) rebuildDesired() error { for _, key := range sortedIntentKeys(p.desiredIntents) { for _, intent := range p.desiredIntents[key] { - entries, err := p.entries(intent) + entries, err := p.recoverableEntries(intent) if err != nil { return fmt.Errorf("re-derive service route state %s: %w", key, err) } @@ -495,7 +506,7 @@ func (p *EBPFRouteProgrammer) repairDesired() error { continue } for index, intent := range intents { - fresh, err := p.entries(intent) + fresh, err := p.recoverableEntries(intent) if err != nil { return fmt.Errorf("re-derive service route state %s: %w", key, err) } diff --git a/internal/serviceroute/dataplane_test.go b/internal/serviceroute/dataplane_test.go index a4cdfcb1..df115c80 100644 --- a/internal/serviceroute/dataplane_test.go +++ b/internal/serviceroute/dataplane_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/cilium/ebpf" + "github.com/vishvananda/netlink" api "go.datum.net/galactic/api/network/v1alpha1" "go.datum.net/galactic/internal/plumbing/ebpf/serviceroutemap" @@ -856,3 +857,92 @@ func TestPersistentDataplaneHasNoExpiry(t *testing.T) { t.Fatal(err) } } + +func TestInitializeWithdrawsMissingInterfaceAndRecovers(t *testing.T) { + programmer := testRouteProgrammer(&recordingTable{}, &recordingTable{}, &recordingTable{}) + missing := false + programmer.targetIndexFn = func(name string) (uint32, error) { + if name == testConsumerDeviceName && missing { + return 0, netlink.LinkNotFoundError{} + } + if name == testConsumerDeviceName { + return 10, nil + } + return 20, nil + } + intent := localTestIntent() + if err := programmer.Apply(intent); err != nil { + t.Fatal(err) + } + missing = true + if err := programmer.Initialize(); err != nil { + t.Fatalf("withdraw missing link: %v", err) + } + if len(programmer.routeRefs) != 0 || len(programmer.accessRefs) != 0 { + t.Fatal("missing interface retained programmed routes or access") + } + if len(programmer.desiredIntents[intentKey(intent)]) != 1 { + t.Fatal("lost recoverable intent") + } + missing = false + if err := programmer.Initialize(); err != nil { + t.Fatalf("recover recreated link: %v", err) + } + if len(programmer.routeRefs) == 0 { + t.Fatal("recreated interface did not recover routes") + } + missing = true + if err := programmer.Remove(intent); err != nil { + t.Fatalf("remove missing link: %v", err) + } + if len(programmer.desiredIntents) != 0 { + t.Fatal("removed intent retained for replay") + } +} + +func TestMissingInterfaceDoesNotBlockOtherRoutes(t *testing.T) { + programmer := testRouteProgrammer(&recordingTable{}, &recordingTable{}, &recordingTable{}) + missing := false + programmer.targetIndexFn = func(name string) (uint32, error) { + switch name { + case testConsumerDeviceName: + if missing { + return 0, netlink.LinkNotFoundError{} + } + return 10, nil + case testOtherConsumerDevice: + return 11, nil + default: + return 20, nil + } + } + intent := localTestIntent() + if err := programmer.Apply(intent); err != nil { + t.Fatal(err) + } + missing = true + other := localTestIntent() + other.ConsumerDevice = testOtherConsumerDevice + if err := programmer.Apply(other); err != nil { + t.Fatalf("unrelated route blocked: %v", err) + } + if len(programmer.routeRefs) != 1 { + t.Fatalf("routes = %d, want only live route", len(programmer.routeRefs)) + } + if err := programmer.Remove(intent); err != nil { + t.Fatal(err) + } + if len(programmer.appliedRefs[intentKey(other)]) != 1 { + t.Fatal("unrelated route lost during cleanup") + } +} + +func TestRepairPropagatesInterfaceLookupErrors(t *testing.T) { + programmer := testRouteProgrammer(&recordingTable{}, &recordingTable{}, &recordingTable{}) + lookupErr := errors.New("netlink unavailable") + programmer.targetIndexFn = func(string) (uint32, error) { return 0, lookupErr } + programmer.desiredIntents["test"] = []RouteIntent{localTestIntent()} + if err := programmer.Initialize(); !errors.Is(err, lookupErr) { + t.Fatalf("Initialize = %v, want lookup error", err) + } +} From a28836b9306b48dec0e6235178c59d720cb237af Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 20:47:09 -0500 Subject: [PATCH 12/12] test: isolate route repair from kernel maps --- internal/serviceroute/dataplane_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/serviceroute/dataplane_test.go b/internal/serviceroute/dataplane_test.go index df115c80..318dd000 100644 --- a/internal/serviceroute/dataplane_test.go +++ b/internal/serviceroute/dataplane_test.go @@ -860,6 +860,9 @@ func TestPersistentDataplaneHasNoExpiry(t *testing.T) { func TestInitializeWithdrawsMissingInterfaceAndRecovers(t *testing.T) { programmer := testRouteProgrammer(&recordingTable{}, &recordingTable{}, &recordingTable{}) + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDsValid = true + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } missing := false programmer.targetIndexFn = func(name string) (uint32, error) { if name == testConsumerDeviceName && missing { @@ -902,6 +905,9 @@ func TestInitializeWithdrawsMissingInterfaceAndRecovers(t *testing.T) { func TestMissingInterfaceDoesNotBlockOtherRoutes(t *testing.T) { programmer := testRouteProgrammer(&recordingTable{}, &recordingTable{}, &recordingTable{}) + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDsValid = true + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } missing := false programmer.targetIndexFn = func(name string) (uint32, error) { switch name { @@ -939,6 +945,9 @@ func TestMissingInterfaceDoesNotBlockOtherRoutes(t *testing.T) { func TestRepairPropagatesInterfaceLookupErrors(t *testing.T) { programmer := testRouteProgrammer(&recordingTable{}, &recordingTable{}, &recordingTable{}) + programmer.mapIDs = [8]ebpf.MapID{1, 2, 3, 4, 5, 6} + programmer.mapIDsValid = true + programmer.readMapIDsFn = func(string) ([8]ebpf.MapID, error) { return programmer.mapIDs, nil } lookupErr := errors.New("netlink unavailable") programmer.targetIndexFn = func(string) (uint32, error) { return 0, lookupErr } programmer.desiredIntents["test"] = []RouteIntent{localTestIntent()}