From 2079349d6ae1328c056a154415482b2f2e1f102d Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 18:10:07 -0500 Subject: [PATCH 1/2] feat(gateway): select NetworkService backends by address within VPC --- .../networkservice_protocol_envtest_test.go | 27 ++++++++++++++ api/network/v1alpha1/networkservice_types.go | 28 +++++++++------ .../v1alpha1/networkservice_types_test.go | 24 +++++++++++++ api/network/v1alpha1/zz_generated.deepcopy.go | 9 +++++ ...alactic.datumapis.com_networkservices.yaml | 35 ++++++++++++------- docs/agents/ARCHITECTURE-GATEWAY.md | 1 + docs/api/network-gateway.md | 11 +++--- .../controller/networkgateway_controller.go | 4 +++ internal/controller/rulebackends.go | 31 +++++++++++++--- internal/controller/rulebackends_test.go | 29 +++++++++++++++ 10 files changed, 167 insertions(+), 32 deletions(-) diff --git a/api/network/v1alpha1/networkservice_protocol_envtest_test.go b/api/network/v1alpha1/networkservice_protocol_envtest_test.go index 0fafac11..6df48aba 100644 --- a/api/network/v1alpha1/networkservice_protocol_envtest_test.go +++ b/api/network/v1alpha1/networkservice_protocol_envtest_test.go @@ -108,6 +108,33 @@ func protocolService(name string, protocol networkv1alpha1.NetworkServiceProtoco } } +func TestNetworkServiceBackendAddressAdmission(t *testing.T) { + c, _ := startProtocolAPI(t) + ctx := context.Background() + svc := protocolService("address-only", networkv1alpha1.NetworkServiceProtocolTCP, 443, 8443) + svc.Spec.BackendSelector = metav1.LabelSelector{} + addresses := []string{"fd00:10::1"} + svc.Spec.BackendAddresses = &addresses + if err := c.Create(ctx, svc); err != nil { + t.Fatalf("address-only service must be admitted: %v", err) + } + addresses = []string{} + if err := c.Update(ctx, svc); err != nil { + t.Fatalf("explicit empty address list must permit withdrawal: %v", err) + } + svc.Spec.BackendAddresses = nil + if err := c.Update(ctx, svc); err == nil { + t.Fatal("service with no selector or address list must be refused") + } + bad := protocolService("ipv4-backend-address", networkv1alpha1.NetworkServiceProtocolTCP, 443, 8443) + bad.Spec.BackendSelector = metav1.LabelSelector{} + v4 := []string{"192.0.2.1"} + bad.Spec.BackendAddresses = &v4 + if err := c.Create(ctx, bad); err == nil { + t.Fatal("IPv4 backend address must be refused") + } +} + // requireRefusal fails unless err is a schema rejection containing every one // of want and none of unwanted. func requireRefusal(t *testing.T, err error, want, unwanted []string) { diff --git a/api/network/v1alpha1/networkservice_types.go b/api/network/v1alpha1/networkservice_types.go index 396850b5..a7d306c8 100644 --- a/api/network/v1alpha1/networkservice_types.go +++ b/api/network/v1alpha1/networkservice_types.go @@ -70,11 +70,9 @@ const ( // backend selection never needs a single "owning" node the way Full-NAT's // SNAT-source model did. // -// Backends are not listed. BackendSelector picks the VPCAttachments that -// serve the service, so the backend set follows attachments as they are -// created, deleted or moved to another node, and galactic-router on each -// node generates the ServiceVIPBinding its backends need from the same -// selection. +// BackendSelector and optional BackendAddresses select VPCAttachments in the +// service's observed VPC. Address-only selection does not require a label. +// galactic-router generates ServiceVIPBindings from the same selection. // // +kubebuilder:object:root=true // +kubebuilder:subresource:status @@ -98,6 +96,7 @@ type NetworkService struct { // +kubebuilder:validation:XValidation:rule="!(self.protocol in ['tcp', 'udp', 'sctp']) || has(self.backendPort)",message="backendPort is required for tcp, udp and sctp" // +kubebuilder:validation:XValidation:rule="!(self.protocol in ['esp', 'gre', 'icmp']) || !has(self.port)",message="port must not be set for esp, gre and icmp, which have no ports" // +kubebuilder:validation:XValidation:rule="!(self.protocol in ['esp', 'gre', 'icmp']) || !has(self.backendPort)",message="backendPort must not be set for esp, gre and icmp, which have no ports" +// +kubebuilder:validation:XValidation:rule="has(self.backendAddresses) || (has(self.backendSelector) && ((has(self.backendSelector.matchLabels) && size(self.backendSelector.matchLabels) > 0) || (has(self.backendSelector.matchExpressions) && size(self.backendSelector.matchExpressions) > 0)))",message="a nonempty backendSelector or backendAddresses is required" type NetworkServiceSpec struct { // VPCRef is the opaque identifier of the target VPC this service applies // to. This repo does not own the VPC API and does not validate the @@ -177,11 +176,20 @@ type NetworkServiceSpec struct { // BackendPort, for both VIPs: its node translates the IPv4 VIP's flows to // and from IPv6, and its IPv4 addresses are never backends. An attachment // with no status.node, or no IPv6 address, is not a backend until it has - // both. An empty selector is rejected rather than read as "every - // attachment in the VPC". - // +kubebuilder:validation:Required - // +kubebuilder:validation:XValidation:rule="(has(self.matchLabels) && size(self.matchLabels) > 0) || (has(self.matchExpressions) && size(self.matchExpressions) > 0)",message="backendSelector must not be empty" - BackendSelector metav1.LabelSelector `json:"backendSelector"` + // both. An empty selector is accepted only with BackendAddresses and then + // matches all attachments in this service's observed VPC. + // +optional + BackendSelector metav1.LabelSelector `json:"backendSelector,omitempty"` + + // BackendAddresses restricts selected attachments to these IPv6 backend + // addresses. When set without a selector, it matches attachments by address + // only, still requiring status.vpc to equal VPCRef. An explicit empty list + // withdraws all backends and their bindings without broadening selection. + // +optional + // +kubebuilder:validation:MaxItems=64 + // +kubebuilder:validation:XValidation:rule="self.all(a, isIP(a) && ip(a).family() == 6)",message="backendAddresses must contain IPv6 addresses" + // +listType=set + BackendAddresses *[]string `json:"backendAddresses,omitempty"` // BackendPort is the destination port on every selected backend, 1-65535. // Like Port, it is required for tcp, udp and sctp and must be unset for diff --git a/api/network/v1alpha1/networkservice_types_test.go b/api/network/v1alpha1/networkservice_types_test.go index 6ce42a91..eae86d05 100644 --- a/api/network/v1alpha1/networkservice_types_test.go +++ b/api/network/v1alpha1/networkservice_types_test.go @@ -32,10 +32,13 @@ func newTestService() *NetworkService { // copy: mutations to slices in the copy must not affect the original. func TestNetworkServiceDeepCopy(t *testing.T) { orig := newTestService() + addresses := []string{"fd00:10::1"} + orig.Spec.BackendAddresses = &addresses dup := orig.DeepCopy() dup.Spec.VIPAddresses[0] = "2001:db8:1::20" dup.Spec.BackendSelector.MatchLabels["app"] = "api" + (*dup.Spec.BackendAddresses)[0] = "fd00:10::2" if orig.Spec.VIPAddresses[0] != "2001:db8:1::10" { t.Errorf("VIPAddresses[0] mutated: got %q", orig.Spec.VIPAddresses[0]) @@ -43,6 +46,9 @@ func TestNetworkServiceDeepCopy(t *testing.T) { if orig.Spec.BackendSelector.MatchLabels["app"] != "web" { t.Errorf("BackendSelector.MatchLabels mutated: got %v", orig.Spec.BackendSelector.MatchLabels) } + if (*orig.Spec.BackendAddresses)[0] != "fd00:10::1" { + t.Errorf("BackendAddresses mutated: got %v", *orig.Spec.BackendAddresses) + } } // TestNetworkServiceDeepCopyNil verifies DeepCopy on a nil pointer returns nil. @@ -148,6 +154,24 @@ func TestNetworkServiceBackendFieldNames(t *testing.T) { } } +func TestNetworkServiceExplicitEmptyBackendAddresses(t *testing.T) { + svc := newTestService() + svc.Spec.BackendSelector = metav1.LabelSelector{} + addresses := []string{} + svc.Spec.BackendAddresses = &addresses + data, err := json.Marshal(svc.Spec) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), `"backendAddresses":[]`) { + t.Fatalf("explicit empty list lost during encoding: %s", data) + } + var got NetworkServiceSpec + if err := json.Unmarshal(data, &got); err != nil || got.BackendAddresses == nil || len(*got.BackendAddresses) != 0 { + t.Fatalf("explicit empty list lost during decoding: %+v, %v", got.BackendAddresses, err) + } +} + // TestNetworkServiceProtocolValues pins the NetworkServiceProtocol values, // which are part of the published API. func TestNetworkServiceProtocolValues(t *testing.T) { diff --git a/api/network/v1alpha1/zz_generated.deepcopy.go b/api/network/v1alpha1/zz_generated.deepcopy.go index f647e6b1..d6955060 100644 --- a/api/network/v1alpha1/zz_generated.deepcopy.go +++ b/api/network/v1alpha1/zz_generated.deepcopy.go @@ -3971,6 +3971,15 @@ func (in *NetworkServiceSpec) DeepCopyInto(out *NetworkServiceSpec) { copy(*out, *in) } in.BackendSelector.DeepCopyInto(&out.BackendSelector) + if in.BackendAddresses != nil { + in, out := &in.BackendAddresses, &out.BackendAddresses + *out = new([]string) + if **in != nil { + in, out := *in, *out + *out = make([]string, len(*in)) + copy(*out, *in) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkServiceSpec. diff --git a/config/crd/galactic.datumapis.com_networkservices.yaml b/config/crd/galactic.datumapis.com_networkservices.yaml index b07d7422..244c2e74 100644 --- a/config/crd/galactic.datumapis.com_networkservices.yaml +++ b/config/crd/galactic.datumapis.com_networkservices.yaml @@ -50,11 +50,9 @@ spec: backend selection never needs a single "owning" node the way Full-NAT's SNAT-source model did. - Backends are not listed. BackendSelector picks the VPCAttachments that - serve the service, so the backend set follows attachments as they are - created, deleted or moved to another node, and galactic-router on each - node generates the ServiceVIPBinding its backends need from the same - selection. + BackendSelector and optional BackendAddresses select VPCAttachments in the + service's observed VPC. Address-only selection does not require a label. + galactic-router generates ServiceVIPBindings from the same selection. properties: apiVersion: description: |- @@ -78,6 +76,20 @@ spec: NetworkServiceSpec defines the desired ingress load-balancing state for a tenant VPC. properties: + backendAddresses: + description: |- + BackendAddresses restricts selected attachments to these IPv6 backend + addresses. When set without a selector, it matches attachments by address + only, still requiring status.vpc to equal VPCRef. An explicit empty list + withdraws all backends and their bindings without broadening selection. + items: + type: string + maxItems: 64 + type: array + x-kubernetes-list-type: set + x-kubernetes-validations: + - message: backendAddresses must contain IPv6 addresses + rule: self.all(a, isIP(a) && ip(a).family() == 6) backendPort: description: |- BackendPort is the destination port on every selected backend, 1-65535. @@ -99,8 +111,8 @@ spec: BackendPort, for both VIPs: its node translates the IPv4 VIP's flows to and from IPv6, and its IPv4 addresses are never backends. An attachment with no status.node, or no IPv6 address, is not a backend until it has - both. An empty selector is rejected rather than read as "every - attachment in the VPC". + both. An empty selector is accepted only with BackendAddresses and then + matches all attachments in this service's observed VPC. properties: matchExpressions: description: matchExpressions is a list of label selector requirements. @@ -145,10 +157,6 @@ spec: type: object type: object x-kubernetes-map-type: atomic - x-kubernetes-validations: - - message: backendSelector must not be empty - rule: (has(self.matchLabels) && size(self.matchLabels) > 0) || (has(self.matchExpressions) - && size(self.matchExpressions) > 0) port: description: |- Port is the ingress port on VIPAddresses that this service load-balances, @@ -240,7 +248,6 @@ spec: minLength: 1 type: string required: - - backendSelector - protocol - vipAddresses - vpcRef @@ -255,6 +262,10 @@ spec: - message: backendPort must not be set for esp, gre and icmp, which have no ports rule: '!(self.protocol in [''esp'', ''gre'', ''icmp'']) || !has(self.backendPort)' + - message: a nonempty backendSelector or backendAddresses is required + rule: has(self.backendAddresses) || (has(self.backendSelector) && ((has(self.backendSelector.matchLabels) + && size(self.backendSelector.matchLabels) > 0) || (has(self.backendSelector.matchExpressions) + && size(self.backendSelector.matchExpressions) > 0))) status: description: NetworkServiceStatus defines the observed state of a NetworkService. properties: diff --git a/docs/agents/ARCHITECTURE-GATEWAY.md b/docs/agents/ARCHITECTURE-GATEWAY.md index 2a98d6fa..98946052 100644 --- a/docs/agents/ARCHITECTURE-GATEWAY.md +++ b/docs/agents/ARCHITECTURE-GATEWAY.md @@ -1036,6 +1036,7 @@ that tag into `config/galactic-gateway/base`. ## Known Constraints +- **A backend socket cannot currently be shared by two NLB services with different VIPs.** The backend node's `vip_xlat_table` egress key contains VPC, backend address, port, protocol, and VIP family, but no incoming VIP within that family. Its one value selects the source VIP for replies. Address-only selection does not change that return path. `ruleBackendOwners` and `ServiceVIPBindingReconciler` keep the newer conflicting service from serving; removing those guards would send its replies with the wrong VIP. Reuse needs a return-path design that retains the chosen VIP for each connection or flow, with matching teardown and failover behavior. - **E2E coverage is lab-only.** `task verify:gateway-ingress` and `task verify:gateway-ingress-ipv4` in `deploy/containerlab/` send real traffic through every lab gateway node (see [Testing](#testing)). Before it, the `gatewayDatapathKeepAlive` incident described in [Entry Points](#cmd-galactic-gatewaygateway-go--datapath-setup) above was only discovered because ingress traffic silently stopped being intercepted. Nothing runs that check in CI, since it needs the containerlab lab. - **Galactic does not check VPC ownership on `NetworkService`.** `NetworkService` is internal: the platform API that serves tenants verifies the tenant owns `vpcRef` before it writes the rule, and RBAC on `galactic-system` decides who else can write one. `updateAcceptedCondition` sets `Accepted=True` once gateway nodes exist for the namespace; it is not an ownership result. galactic-router used to carry an allow-all admission webhook for this, never deployed, removed in #366. The `api/network/v1alpha1` doc comments still call the CRD tenant-writable and define `OwnershipVerified`/`OwnershipDenied` reasons nothing sets. - **The uSID backend resolver's tenant-ownership check depends on `BGPVRFInstance` naming staying deterministic.** `verifyTenantOwnership` closes the ambiguous-match gap an earlier version of this resolver had, but it is only as strong as `crdnames.BGPVRFInstanceName(vpc, nodeName)` staying the exact name `galactic-bgp` writes — a divergence between the two would fail closed (a real backend never resolving) rather than open (a wrong-tenant resolve), which is the safer failure direction but still worth knowing about when either side of that naming contract changes. diff --git a/docs/api/network-gateway.md b/docs/api/network-gateway.md index 94caab46..aa92b622 100644 --- a/docs/api/network-gateway.md +++ b/docs/api/network-gateway.md @@ -382,11 +382,9 @@ flow (internal/maglev), and forwards without rewriting anything — backend selection never needs a single "owning" node the way Full-NAT's SNAT-source model did. -Backends are not listed. BackendSelector picks the VPCAttachments that -serve the service, so the backend set follows attachments as they are -created, deleted or moved to another node, and galactic-router on each -node generates the ServiceVIPBinding its backends need from the same -selection. +BackendSelector and optional BackendAddresses select VPCAttachments in the +service's observed VPC. Address-only selection does not require a label. +galactic-router generates ServiceVIPBindings from the same selection. @@ -451,7 +449,8 @@ _Appears in:_ | `vipAddresses` _string array_ | VIPAddresses is the list of ingress VIP addresses this service provisions
on the gateway nodes: at most one IPv6 and at most one IPv4 address.
A backend node rewrites a reply's source from the backend's address, port
and protocol back to a single VIP of the client's family, so a backend
cannot answer for a second VIP of one family. Another VIP of a family
needs its own service whose backends listen on a different BackendPort.
An IPv4 VIP is written in dotted form; the IPv4-mapped IPv6 form is
refused.
Workloads are IPv6 only, so both VIPs are served by the selected
attachments' IPv6 addresses. A backend node translates the IPv4 VIP's
flows statelessly: the backend sees an IPv4 client a.b.c.d as
64:ff9b::a.b.c.d (RFC 6052) and replies to that address, and its node
sends the reply back to the client as IPv4 from the VIP. A gateway node
serves the IPv4 VIP only while it has IPv4 VIPs turned on, and
load-balances it only over the backends whose ServiceVIPBinding for it
reports Bound. It advertises the IPv4 VIP while at least one such
backend exists, and reports the outcome in its "/IPv4Served"
condition. The IPv6 VIP is served independently of the IPv4 one.
An existing service whose list breaks these limits keeps validating while
the list is unchanged, since the API server ratchets validation of an
unchanged field on Kubernetes 1.30 and later; any edit to the list must
bring it within them, and an older API server refuses any update to such
a service. | | MaxItems: 8
MinItems: 1
Required: \{\}
items:MaxLength: 45
| | `protocol` _[NetworkServiceProtocol](#networkserviceprotocol)_ | Protocol is the IP protocol this service load-balances on VIPAddresses.
The API admits tcp, udp, sctp, esp, gre and icmp, and the gateway
datapath serves tcp and udp. A service with any other admitted value is
refused with a message saying its protocol is not served yet. sctp has
ports and needs Port and BackendPort. esp, gre and icmp have no ports
and must set neither; a backend for them is chosen from the client and
VIP addresses alone. The set of values is open-ended, so a client must
accept and preserve a value it does not recognise, as
NetworkServiceProtocol describes. | | Enum: [tcp udp sctp esp gre icmp]
Required: \{\}
| | `port` _integer_ | Port is the ingress port on VIPAddresses that this service load-balances,
1-65535. It is required for a protocol with ports (tcp, udp and sctp)
and must be unset for one without (esp, gre and icmp). Zero means unset
and is omitted when encoding, and an explicit 0 is refused. | | Maximum: 65535
Minimum: 1
| -| `backendSelector` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#labelselector-v1-meta)_ | BackendSelector selects the VPCAttachments (galactic.datumapis.com/v1alpha1,
in any namespace) that serve this service, matched against each
VPCAttachment object's own labels. Only attachments whose status.vpc
equals VPCRef are candidates, whatever their labels, so a selector cannot
reach into another tenant's VPC. Each selected attachment contributes the
IPv6 addresses in its spec.interface.addresses as backends, on
BackendPort, for both VIPs: its node translates the IPv4 VIP's flows to
and from IPv6, and its IPv4 addresses are never backends. An attachment
with no status.node, or no IPv6 address, is not a backend until it has
both. An empty selector is rejected rather than read as "every
attachment in the VPC". | | Required: \{\}
| +| `backendSelector` _[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#labelselector-v1-meta)_ | BackendSelector selects the VPCAttachments (galactic.datumapis.com/v1alpha1,
in any namespace) that serve this service, matched against each
VPCAttachment object's own labels. Only attachments whose status.vpc
equals VPCRef are candidates, whatever their labels, so a selector cannot
reach into another tenant's VPC. Each selected attachment contributes the
IPv6 addresses in its spec.interface.addresses as backends, on
BackendPort, for both VIPs: its node translates the IPv4 VIP's flows to
and from IPv6, and its IPv4 addresses are never backends. An attachment
with no status.node, or no IPv6 address, is not a backend until it has
both. An empty selector is accepted only with BackendAddresses and then
matches all attachments in this service's observed VPC. | | | +| `backendAddresses` _string_ | BackendAddresses restricts selected attachments to these IPv6 backend
addresses. When set without a selector, it matches attachments by address
only, still requiring status.vpc to equal VPCRef. An explicit empty list
withdraws all backends and their bindings without broadening selection. | | MaxItems: 64
| | `backendPort` _integer_ | BackendPort is the destination port on every selected backend, 1-65535.
Like Port, it is required for tcp, udp and sctp and must be unset for
esp, gre and icmp. Zero means unset and is omitted when encoding, and an
explicit 0 is refused. | | Maximum: 65535
Minimum: 1
| diff --git a/internal/controller/networkgateway_controller.go b/internal/controller/networkgateway_controller.go index dae16c0a..7d750689 100644 --- a/internal/controller/networkgateway_controller.go +++ b/internal/controller/networkgateway_controller.go @@ -1509,6 +1509,10 @@ func resolveFamilyBackends( err = fmt.Errorf("every selected backend is served by an older rule: %s", cappedList(unresolved)) case len(unresolved) > 0: err = fmt.Errorf("no selected attachment is a backend yet: %s", cappedList(unresolved)) + case rule.Spec.BackendAddresses != nil && len(*rule.Spec.BackendAddresses) == 0: + err = errors.New("backendAddresses is empty") + case rule.Spec.BackendAddresses != nil: + err = fmt.Errorf("backendAddresses match no selected VPCAttachment in VPC %s", rule.Spec.VPCRef) default: err = fmt.Errorf("backendSelector matches no VPCAttachment in VPC %s", rule.Spec.VPCRef) } diff --git a/internal/controller/rulebackends.go b/internal/controller/rulebackends.go index d2abf2b7..c5fdc80d 100644 --- a/internal/controller/rulebackends.go +++ b/internal/controller/rulebackends.go @@ -219,17 +219,37 @@ 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{}{} + } + } for _, attachment := range attachments { if attachment.Status.VPC != rule.Spec.VPCRef || !selector.Matches(labels.Set(attachment.Labels)) { continue } + addrs := attachmentIPv6Addresses(attachment) + if allowed != nil { + addrs = slices.DeleteFunc(addrs, func(addr netip.Addr) bool { + _, ok := allowed[addr] + return !ok + }) + if len(addrs) == 0 { + continue + } + } key := types.NamespacedName{Namespace: attachment.Namespace, Name: attachment.Name} if attachment.Status.Node == "" { pending = append(pending, key.String()+": status.node not set by datum-cloud/cloud") continue } - addrs := attachmentIPv6Addresses(attachment) if len(addrs) == 0 { pending = append(pending, key.String()+": no IPv6 address") continue @@ -484,12 +504,15 @@ func errTooManyVIPs(family addrFamily, n int) error { } // ruleBackendSelector converts rule's BackendSelector to a labels.Selector. -// The CRD rejects an empty selector; this rejects one anyway, since an empty -// selector would otherwise match every attachment in the VPC. +// An empty selector matches attachments by address only when an explicit +// BackendAddresses list is present; status.vpc still limits the candidate VPC. func ruleBackendSelector(rule *bgpv1alpha1.NetworkService) (labels.Selector, error) { sel := rule.Spec.BackendSelector if len(sel.MatchLabels) == 0 && len(sel.MatchExpressions) == 0 { - return nil, errors.New("backendSelector is empty") + if rule.Spec.BackendAddresses == nil { + return nil, errors.New("backendSelector is empty and backendAddresses is unset") + } + return labels.Everything(), nil } selector, err := metav1.LabelSelectorAsSelector(&sel) if err != nil { diff --git a/internal/controller/rulebackends_test.go b/internal/controller/rulebackends_test.go index 486a3672..8c357a78 100644 --- a/internal/controller/rulebackends_test.go +++ b/internal/controller/rulebackends_test.go @@ -121,6 +121,35 @@ func TestSelectRuleBackends_DropsConflictingBackends(t *testing.T) { } } +func TestRuleCandidateBackends_AddressOnlyWithinObservedVPC(t *testing.T) { + rule := newTestRule(testRuleName, testVPCRef, testVIP) + rule.Spec.BackendSelector = metav1.LabelSelector{} + addresses := []string{"fd00:10::1"} + rule.Spec.BackendAddresses = &addresses + + inVPC := newBackendAttachment(testVPCRef, "fd00:10::1") + inVPC.Labels = nil + otherVPC := newBackendAttachment("other-vpc", "fd00:10::1") + otherVPC.Name = "other-vpc" + unlisted := newBackendAttachment(testVPCRef, "fd00:10::2") + unlisted.Name = "unlisted" + unlisted.Status.Node = "" + + got, pending, err := ruleCandidateBackends(rule, []*cloudv1alpha1.VPCAttachment{otherVPC, unlisted, inVPC}) + if err != nil || len(got) != 1 || got[0].attachment.Name != inVPC.Name || len(pending) != 0 { + t.Fatalf("address-only selection = %+v, pending %v, %v; want only the attachment in the observed VPC", got, pending, err) + } + addresses = []string{} + got, _, err = ruleCandidateBackends(rule, []*cloudv1alpha1.VPCAttachment{inVPC}) + if err != nil || len(got) != 0 { + t.Fatalf("explicit empty list = %+v, %v; want no backends", got, err) + } + rule.Spec.BackendAddresses = nil + if _, _, err = ruleCandidateBackends(rule, []*cloudv1alpha1.VPCAttachment{inVPC}); err == nil { + t.Fatal("empty selector without backendAddresses must fail closed") + } +} + // 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.NetworkService { From 201a25ace884b6da6f3a9fc0f5e9a9fe5c2117a6 Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Wed, 21 Oct 2026 18:17:22 -0500 Subject: [PATCH 2/2] Bound backend address validation cost and fix lint --- api/network/v1alpha1/networkservice_types.go | 1 + config/crd/galactic.datumapis.com_networkservices.yaml | 1 + internal/controller/rulebackends_test.go | 6 +++--- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/api/network/v1alpha1/networkservice_types.go b/api/network/v1alpha1/networkservice_types.go index a7d306c8..9a68a851 100644 --- a/api/network/v1alpha1/networkservice_types.go +++ b/api/network/v1alpha1/networkservice_types.go @@ -187,6 +187,7 @@ type NetworkServiceSpec struct { // withdraws all backends and their bindings without broadening selection. // +optional // +kubebuilder:validation:MaxItems=64 + // +kubebuilder:validation:items:MaxLength=45 // +kubebuilder:validation:XValidation:rule="self.all(a, isIP(a) && ip(a).family() == 6)",message="backendAddresses must contain IPv6 addresses" // +listType=set BackendAddresses *[]string `json:"backendAddresses,omitempty"` diff --git a/config/crd/galactic.datumapis.com_networkservices.yaml b/config/crd/galactic.datumapis.com_networkservices.yaml index 244c2e74..966dd819 100644 --- a/config/crd/galactic.datumapis.com_networkservices.yaml +++ b/config/crd/galactic.datumapis.com_networkservices.yaml @@ -83,6 +83,7 @@ spec: only, still requiring status.vpc to equal VPCRef. An explicit empty list withdraws all backends and their bindings without broadening selection. items: + maxLength: 45 type: string maxItems: 64 type: array diff --git a/internal/controller/rulebackends_test.go b/internal/controller/rulebackends_test.go index 8c357a78..dd868b82 100644 --- a/internal/controller/rulebackends_test.go +++ b/internal/controller/rulebackends_test.go @@ -129,15 +129,15 @@ func TestRuleCandidateBackends_AddressOnlyWithinObservedVPC(t *testing.T) { inVPC := newBackendAttachment(testVPCRef, "fd00:10::1") inVPC.Labels = nil - otherVPC := newBackendAttachment("other-vpc", "fd00:10::1") - otherVPC.Name = "other-vpc" + otherVPC := newBackendAttachment(testOtherVPCRef, "fd00:10::1") + otherVPC.Name = "outside" unlisted := newBackendAttachment(testVPCRef, "fd00:10::2") unlisted.Name = "unlisted" unlisted.Status.Node = "" got, pending, err := ruleCandidateBackends(rule, []*cloudv1alpha1.VPCAttachment{otherVPC, unlisted, inVPC}) if err != nil || len(got) != 1 || got[0].attachment.Name != inVPC.Name || len(pending) != 0 { - t.Fatalf("address-only selection = %+v, pending %v, %v; want only the attachment in the observed VPC", got, pending, err) + t.Fatalf("address-only selection = %+v, pending %v, %v; want only the observed VPC", got, pending, err) } addresses = []string{} got, _, err = ruleCandidateBackends(rule, []*cloudv1alpha1.VPCAttachment{inVPC})