From b4b6f76c27375650c59314cce12fcb2325c7c65b Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Mon, 12 Oct 2026 11:22:27 -0400 Subject: [PATCH] feat: honor NetworkRule backend address filter --- docs/agents/ARCHITECTURE-GATEWAY.md | 12 +++++- go.mod | 2 +- go.sum | 2 + .../controller/networkgateway_controller.go | 5 +++ .../networkgateway_controller_test.go | 42 +++++++++++++++++++ .../networkrule_binding_controller_test.go | 31 ++++++++++++++ internal/controller/rulebackends.go | 16 +++++++ internal/controller/rulebackends_test.go | 33 +++++++++++++++ 8 files changed, 141 insertions(+), 2 deletions(-) diff --git a/docs/agents/ARCHITECTURE-GATEWAY.md b/docs/agents/ARCHITECTURE-GATEWAY.md index ded873e4..d14323b1 100644 --- a/docs/agents/ARCHITECTURE-GATEWAY.md +++ b/docs/agents/ARCHITECTURE-GATEWAY.md @@ -106,7 +106,17 @@ per-VPC gateway design to colocate with the workload's own VRF. | CRD | Scope | Written by | Purpose | | ---------------- | -------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `NetworkGateway` | Namespaced, one per gateway node (`spec.targetRef.name` = node name) | Operator, once per gateway node (see [worked example](#worked-containerlab-example) below) | Node-scoped root object, mirroring `BGPRouter`'s pattern. Identifies which nodes participate in the anycast mesh and surfaces each node's engine health via `status.conditions`. Carries **no** self-address field — DSR rewrites nothing, so there is no translation source to publish. | -| `NetworkRule` | Namespaced, internal | The platform API that serves tenants, after it verifies the tenant owns `vpcRef` (galactic does not repeat the check — see Known Constraints) | Ingress load-balancing spec: `vpcRef` (opaque tenant identifier), `vipAddresses` (1–8, IPv4/IPv6), `protocol` (`tcp`/`udp`), `port`, `backendSelector` (the VPCAttachments in `vpcRef` that serve it), `backendPort`. Served by every `NetworkGateway` in the namespace identically. | +| `NetworkRule` | Namespaced, internal | The platform API that serves tenants, after it verifies the tenant owns `vpcRef` (galactic does not repeat the check — see Known Constraints) | Ingress load-balancing spec: `vpcRef` (opaque tenant identifier), `vipAddresses` (1–8, IPv4/IPv6), `protocol` (`tcp`/`udp`), `port`, `backendSelector` (the VPCAttachments in `vpcRef` that serve it), optional `backendAddresses` filter, `backendPort`. Served by every `NetworkGateway` in the namespace identically. | + +`backendAddresses` lets a control plane supply an explicit healthy IPv6 +backend set while retaining the VPC-scoped attachment selector as an ownership +boundary. Omission keeps every selected address; an explicit empty list loads +no backends and withdraws the rule. Both the gateway and backend nodes filter +the same attachment addresses, so a removed address loses its ingress Maglev +slot and its `ServiceVIPBinding`. This does not change the DSR restriction: +one backend address, port and protocol can reply for only one IPv6 VIP. A +consumer must also map its service endpoint to the corresponding +`VPCAttachment` and its authoritative labels before writing a rule. Both are defined in `go.datum.net/network`'s `api/v1alpha1` package (`gateway_types.go`, `rule_types.go`) — the same external CRD module the diff --git a/go.mod b/go.mod index 4aa7bb3d..c7c9df82 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.0 + go.datum.net/network v0.4.1-0.20261012151636-f8e095afd3df 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 edf9cd50..1f2bf10c 100644 --- a/go.sum +++ b/go.sum @@ -248,6 +248,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.0 h1:JeNcJD7Ln9QFnGdbr2sjSWOy4p96gHoUXr3S6tEaqJM= go.datum.net/network v0.4.0/go.mod h1:LawI34k+ddtv3kRbmjiss0Lb6QKpSr60YP3fX1IG5RU= +go.datum.net/network v0.4.1-0.20261012151636-f8e095afd3df h1:C4Tly3CH9ZUr4AhFJMH+SkLk9Omkg2uvmRh2i50/FCw= +go.datum.net/network v0.4.1-0.20261012151636-f8e095afd3df/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.69.0 h1:2yEATaop1/a1I4psnSLgWVPLWwCzkqWakgJy7xTDVy0= diff --git a/internal/controller/networkgateway_controller.go b/internal/controller/networkgateway_controller.go index 8ffdd558..4a6ab7ec 100644 --- a/internal/controller/networkgateway_controller.go +++ b/internal/controller/networkgateway_controller.go @@ -805,6 +805,11 @@ func buildDesiredRule( case len(unresolved) > 0: return gateway.DesiredRule{}, nil, fmt.Errorf("no selected attachment is a backend yet: %s", cappedList(unresolved)) + case rule.Spec.BackendAddresses != nil && len(*rule.Spec.BackendAddresses) == 0: + return gateway.DesiredRule{}, nil, fmt.Errorf("backendAddresses is empty") + case rule.Spec.BackendAddresses != nil: + return gateway.DesiredRule{}, nil, fmt.Errorf("backendAddresses match no selected VPCAttachment in VPC %s", + rule.Spec.VPCRef) default: return gateway.DesiredRule{}, nil, fmt.Errorf("backendSelector matches no VPCAttachment in VPC %s", rule.Spec.VPCRef) diff --git a/internal/controller/networkgateway_controller_test.go b/internal/controller/networkgateway_controller_test.go index 96d02614..e8575405 100644 --- a/internal/controller/networkgateway_controller_test.go +++ b/internal/controller/networkgateway_controller_test.go @@ -980,6 +980,48 @@ func TestNetworkGatewayReconciler_WithdrawsRuleThatStopsBuilding(t *testing.T) { assertRuleProgrammed(t, fakeClient, metav1.ConditionFalse, reasonInvalidRule) } +func TestNetworkGatewayReconciler_EmptyBackendAddressesWithdrawsRule(t *testing.T) { + scheme := newRuleTestScheme(t) + backendRouter, backendAdv, backendVRF := newBackendFixtures("vpc-1") + rule := newTestRule(testRuleName, "vpc-1", testVIP) + acceptRule(rule) + c := newIndexedClientBuilder(scheme). + WithStatusSubresource(&bgpv1alpha1.NetworkGateway{}, &bgpv1alpha1.NetworkRule{}). + WithObjects(newTestGateway(testNodeGWA), newTestRouter(), backendRouter, backendAdv, backendVRF, + newBackendAttachment("vpc-1"), rule). + Build() + engine := newFakeGatewayEngine() + r := newGatewayReconciler(c, scheme, engine, testNodeGWA) + req := ctrl.Request{NamespacedName: testRuleKey(testNodeGWA)} + if _, err := r.Reconcile(context.Background(), req); err != nil { + t.Fatalf("initial Reconcile: %v", err) + } + adv := &bgpv1alpha1.BGPAdvertisement{} + if err := c.Get(context.Background(), testRuleKey(testRuleAdvV4), adv); err != nil { + t.Fatalf("get initial advertisement: %v", err) + } + + current := &bgpv1alpha1.NetworkRule{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(rule), current); err != nil { + t.Fatalf("get rule: %v", err) + } + empty := []string{} + current.Spec.BackendAddresses = &empty + if err := c.Update(context.Background(), current); err != nil { + t.Fatalf("update rule: %v", err) + } + if _, err := r.Reconcile(context.Background(), req); err != nil { + t.Fatalf("Reconcile with empty backendAddresses: %v", err) + } + if len(engine.lastDesired.Rules) != 0 { + t.Errorf("desired rules = %+v, want none", engine.lastDesired.Rules) + } + if err := c.Get(context.Background(), testRuleKey(testRuleAdvV4), adv); !apierrors.IsNotFound(err) { + t.Errorf("advertisement after empty backendAddresses: err = %v, want NotFound", err) + } + assertRuleProgrammed(t, c, metav1.ConditionFalse, reasonInvalidRule) +} + // TestNetworkGatewayReconciler_ReturnsOrphanSweepFailure covers the third // swallowed error from #365. The sweep is crash recovery, so its failure // must be retried -- and the status write that precedes it still has to diff --git a/internal/controller/networkrule_binding_controller_test.go b/internal/controller/networkrule_binding_controller_test.go index 9424428b..7d0bb709 100644 --- a/internal/controller/networkrule_binding_controller_test.go +++ b/internal/controller/networkrule_binding_controller_test.go @@ -161,6 +161,37 @@ func TestNetworkRuleBindingReconciler_SameNodeBackendsGetOwnBindings(t *testing. } } +func TestNetworkRuleBindingReconciler_RemovesFilteredBackend(t *testing.T) { + rule := newBindingTestRule() + r, c := newBindingWriter(t, rule, newBackendAttachment(testVPCRef, testBackendAddr, "fd00:10::2")) + reconcileBindings(t, r) + if got := backendAddresses(listBindings(t, c)); strings.Join(got, ",") != testBackendAddr+",fd00:10::2" { + t.Fatalf("initial bindings = %v, want both backends", got) + } + + current := getBindingRule(t, c) + allowed := []string{"fd00:10::2"} + current.Spec.BackendAddresses = &allowed + if err := c.Update(context.Background(), current); err != nil { + t.Fatalf("update rule: %v", err) + } + reconcileBindings(t, r) + if got := backendAddresses(listBindings(t, c)); strings.Join(got, ",") != "fd00:10::2" { + t.Errorf("bindings after removing an address = %v, want only fd00:10::2", got) + } + + current = getBindingRule(t, c) + empty := []string{} + current.Spec.BackendAddresses = &empty + if err := c.Update(context.Background(), current); err != nil { + t.Fatalf("update rule with empty allowlist: %v", err) + } + reconcileBindings(t, r) + if got := listBindings(t, c); len(got) != 0 { + t.Errorf("bindings after empty allowlist = %d, want none", len(got)) + } +} + // TestNetworkRuleBindingReconciler_OnlyThisNodesBackends covers a backend on // another node and one in another VPC carrying the selected label: neither // gets a binding here. diff --git a/internal/controller/rulebackends.go b/internal/controller/rulebackends.go index 50b98a0d..6a66e243 100644 --- a/internal/controller/rulebackends.go +++ b/internal/controller/rulebackends.go @@ -185,6 +185,17 @@ func ruleCandidateBackends( if err != nil { return nil, nil, err } + var allowed map[netip.Addr]struct{} + if rule.Spec.BackendAddresses != nil { + allowed = make(map[netip.Addr]struct{}, len(*rule.Spec.BackendAddresses)) + for _, raw := range *rule.Spec.BackendAddresses { + addr, parseErr := netip.ParseAddr(raw) + if parseErr != nil || !addr.Is6() || addr.Is4In6() { + return nil, nil, fmt.Errorf("invalid IPv6 backend address %q", raw) + } + allowed[addr] = struct{}{} + } + } //nolint:gosec // BackendPort is CRD-validated to [1,65535] (Minimum/Maximum markers on NetworkRuleSpec.BackendPort) port := uint16(rule.Spec.BackendPort) @@ -204,6 +215,11 @@ func ruleCandidateBackends( } kind := attachmentEgressKind(attachment.Spec.Interface.Mode) for _, addr := range addrs { + if allowed != nil { + if _, ok := allowed[addr]; !ok { + continue + } + } backends = append(backends, ruleBackend{ attachment: key, node: attachment.Status.Node, diff --git a/internal/controller/rulebackends_test.go b/internal/controller/rulebackends_test.go index e9a85d6f..a8bd7af2 100644 --- a/internal/controller/rulebackends_test.go +++ b/internal/controller/rulebackends_test.go @@ -121,6 +121,39 @@ func TestSelectRuleBackends_DropsConflictingBackends(t *testing.T) { } } +func TestRuleCandidateBackends_BackendAddresses(t *testing.T) { + rule := newTestRule(testRuleName, testVPCRef, testVIP) + selected := newBackendAttachment(testVPCRef, "fd00:10::1", "fd00:10::2") + otherVPC := newBackendAttachment("other-vpc", "fd00:10::3") + attachments := []*cloudv1alpha1.VPCAttachment{selected, otherVPC} + + check := func(want ...string) { + t.Helper() + backends, _, err := ruleCandidateBackends(rule, attachments) + if err != nil { + t.Fatalf("ruleCandidateBackends: %v", err) + } + got := make([]string, 0, len(backends)) + for _, b := range backends { + got = append(got, b.addr.String()) + } + if !slices.Equal(got, want) { + t.Errorf("backends = %v, want %v", got, want) + } + } + + check("fd00:10::1", "fd00:10::2") // omitted: selector-only behavior + allowed := []string{"fd00:10::2", "fd00:10::3"} + rule.Spec.BackendAddresses = &allowed + check("fd00:10::2") // no cross-VPC selection + allowed = []string{} + check() // explicit empty: no backends + allowed = []string{"not-an-ip"} + if _, _, err := ruleCandidateBackends(rule, attachments); err == nil { + t.Error("invalid address: want an error") + } +} + // newSharingRule returns a rule named name with the IPv6 VIP vip, created at // second created, selecting newBackendAttachment's attachments in testVPCRef. func newSharingRule(name, vip string, created int64) *bgpv1alpha1.NetworkRule {