From 8162a4bd44cf0eb759d44914a5a06b0cc85bd0ad Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 15:36:54 +0000 Subject: [PATCH] feat: Only send load balancer traffic to verified backends Keep the original interface identity through projection so a serving location can confirm a backend still belongs to the right VPC before it sends it traffic. Backends without a verified identity are left out. Key changes: - Preserve the source interface UID on projected interfaces - Exclude route endpoints without a verified source identity - Document the serving-location contract --- api/v1alpha/networkinterface_types.go | 4 + .../network-load-balancer-cell-contract.md | 95 +++++++++++++++++++ .../controller/networkinterface_projection.go | 3 + .../networkinterface_projection_test.go | 20 +++- .../networkloadbalancerroute_controller.go | 31 +++--- ...etworkloadbalancerroute_controller_test.go | 17 +++- 6 files changed, 156 insertions(+), 14 deletions(-) create mode 100644 docs/plans/network-load-balancer-cell-contract.md diff --git a/api/v1alpha/networkinterface_types.go b/api/v1alpha/networkinterface_types.go index 7319a3a1..2f1fa994 100644 --- a/api/v1alpha/networkinterface_types.go +++ b/api/v1alpha/networkinterface_types.go @@ -446,4 +446,8 @@ const ( // published from, so a collector can find the source of a copy it is looking // at. NetworkInterfaceSourceNamespaceLabel = "networking.datumapis.com/source-namespace" + + // NetworkInterfaceSourceUIDLabel identifies the exact cell interface behind + // a published copy. A replacement may reuse the name, but never the UID. + NetworkInterfaceSourceUIDLabel = "networking.datumapis.com/source-uid" ) diff --git a/docs/plans/network-load-balancer-cell-contract.md b/docs/plans/network-load-balancer-cell-contract.md new file mode 100644 index 00000000..500e4f86 --- /dev/null +++ b/docs/plans/network-load-balancer-cell-contract.md @@ -0,0 +1,95 @@ +# Network load balancer cell contract + +## Identity available today + +A project's `NetworkInterface` is a copy of a cell interface. The copy has the +cell location, source namespace, and source name. This change also publishes the +source interface UID. These four values identify one source object, including +across deletion and recreation under the same name. The copy deliberately omits +`status.attachmentRef` and `status.vpc`; those values name cell resources and +are not authoritative in a project control plane. + +The route resolver counts only projected endpoints carrying this source UID. +Older copies without the label remain unprogrammable until the cell publishes +them again. The UID on a copy is a hint for the cell to check against its live +object, never authorization by itself. + +Inside a cell, Cloud creates a `VPCAttachment` with +`spec.interfaceRef.name` pointing to the source interface in the same +namespace. The source interface reports the attachment in +`status.attachmentRef` and the fabric VPC identifier in `status.vpc`. Galactic +reads attachment addresses, `status.vpc`, and `status.node`. Matching an address +alone is unsafe because different VPCs can use the same address. + +## Proposed intent + +The project route reconciler should publish a **versioned, location-scoped +intent** through the hub to the target cell. This resource does not exist yet. +The owner reference must identify the route by UID and generation so a deleted +or replaced route cannot keep a VIP active. Its minimum schema is: + +```yaml +apiVersion: networking.datumapis.com/v1alpha +kind: NetworkLoadBalancerCellIntent +spec: + source: + projectCluster: + namespace: + routeName: + routeUID: + routeGeneration: + parentName: + listenerName: + location: + networkName: + vip: + protocol: TCP # or UDP + listenerPort: 443 + backends: + - sourceNamespace: + sourceName: + sourceUID: + address: + port: 8443 + weight: 1 + healthCheck: +``` + +Only a platform controller may write this intent. The cell must treat the +projected source tuple as a lookup hint, not as proof. For every backend it +must read the cell `NetworkInterface` and require the UID, network, bound phase, +holder availability, location, and address to match. It must then read the +same-namespace `VPCAttachment` named by the interface's `status.attachmentRef` +and require its `spec.interfaceRef.name`, controller owner UID, `status.vpc`, +IPv6 address, and programmed generation to match the source. The attachment +must have a node and the source must identify the expected VPC. A missing or +changed field removes the backend from the effective set. The cell must also +verify that the source namespace belongs to the project recorded in the +intent, and that the attachment's `spec.vpc` resolves to a live VPC whose +fabric ID matches both statuses. A source UID change invalidates old intent +even when the name and IP are reused. + +The cell should report resolved attachment identities and probe results per +backend, with the intent generation it observed. A project controller may set +route `Programmed=True` only after the cell has acknowledged that generation +and every announcing gateway node confirms the same healthy backend set. VIP +announcement follows that acknowledgment; an empty healthy set withdraws the +VIP but retains the allocated address. + +## Galactic API dependencies + +The current `NetworkRule` requires a nonempty `backendSelector`. Cloud's +`VPCAttachment` writer does not provide a trusted label that selects all and +only this route's attachments. `backendAddresses` narrows a selector, so it +cannot be used alone today. The Network API should permit an omitted selector +**only when `backendAddresses` is explicitly set**. Galactic should then +select attachments by authoritative `status.vpc == spec.vpcRef` and intersect +their IPv6 addresses with `backendAddresses`. An explicit empty list means no +backends and withdrawal. The writer must verify authorization for `vpcRef`; +the existing Galactic admission contract does not itself prove it. + +Current DSR also cannot safely serve two VIPs sharing a backend +address/port/protocol, and does not implement weighted selection or active +application probes. Those capabilities and an all-gateway programming +acknowledgment are required before this intent can drive the full native API. +Until then, route and load balancer `Programmed` and `Announced` remain false. diff --git a/internal/controller/networkinterface_projection.go b/internal/controller/networkinterface_projection.go index 1cc761c2..8a5119f6 100644 --- a/internal/controller/networkinterface_projection.go +++ b/internal/controller/networkinterface_projection.go @@ -61,6 +61,9 @@ func projectionLabels(source *networkingv1alpha.NetworkInterface, location strin labels[networkingv1alpha.NetworkInterfaceProjectionLabel] = "true" labels[networkingv1alpha.NetworkInterfaceSourceNamespaceLabel] = source.Namespace + if source.UID != "" { + labels[networkingv1alpha.NetworkInterfaceSourceUIDLabel] = string(source.UID) + } if location != "" { labels[networkingv1alpha.NetworkInterfaceLocationLabel] = location diff --git a/internal/controller/networkinterface_projection_test.go b/internal/controller/networkinterface_projection_test.go index 8731e7dd..5d6037e9 100644 --- a/internal/controller/networkinterface_projection_test.go +++ b/internal/controller/networkinterface_projection_test.go @@ -345,7 +345,7 @@ func (v *visibility) projectCopy() (*networkingv1alpha.NetworkInterface, bool) { func TestInterfaceReachesTheProjectControlPlane(t *testing.T) { v := newVisibility(t) - v.interfaceOnCell() + source := v.interfaceOnCell() v.publish() v.handToProject() @@ -365,6 +365,24 @@ func TestInterfaceReachesTheProjectControlPlane(t *testing.T) { require.Equal(t, testLocationName, copied.Labels[networkingv1alpha.NetworkInterfaceLocationLabel]) require.Equal(t, boundInterfaceName, copied.Labels[networkingv1alpha.NetworkInterfaceHolderLabel]) + require.Equal(t, v.cellNamespace, copied.Labels[networkingv1alpha.NetworkInterfaceSourceNamespaceLabel]) + require.Equal(t, string(source.UID), copied.Labels[networkingv1alpha.NetworkInterfaceSourceUIDLabel]) +} + +func TestProjectedInterfaceSourceIdentityChangesWithUID(t *testing.T) { + source := &networkingv1alpha.NetworkInterface{ObjectMeta: metav1.ObjectMeta{ + Namespace: "cell", Name: "nic", UID: "first", + Labels: map[string]string{networkingv1alpha.NetworkInterfaceSourceUIDLabel: "forged"}, + }} + first := projectedInterface(source, "dfw") + require.Equal(t, "first", first.Labels[networkingv1alpha.NetworkInterfaceSourceUIDLabel]) + source.UID = "second" + second := projectedInterface(source, "dfw") + require.Equal(t, "second", second.Labels[networkingv1alpha.NetworkInterfaceSourceUIDLabel]) + require.Equal(t, "cell", second.Labels[networkingv1alpha.NetworkInterfaceSourceNamespaceLabel]) + source.UID = "" + third := projectedInterface(source, "dfw") + require.NotContains(t, third.Labels, networkingv1alpha.NetworkInterfaceSourceUIDLabel) } func TestProjectedInterfaceReportsDNSSettings(t *testing.T) { diff --git a/internal/controller/networkloadbalancerroute_controller.go b/internal/controller/networkloadbalancerroute_controller.go index bc95d517..5bb5f751 100644 --- a/internal/controller/networkloadbalancerroute_controller.go +++ b/internal/controller/networkloadbalancerroute_controller.go @@ -53,11 +53,13 @@ type NetworkLoadBalancerRouteReconciler struct { } type networkLoadBalancerRouteEndpoint struct { - NetworkService string - Interface string - Location string - Address string - Port int32 + NetworkService string + Interface string + SourceNamespace string + SourceUID string + Location string + Address string + Port int32 } type networkLoadBalancerRouteReferenceError struct { @@ -220,7 +222,7 @@ func reconcileRouteParent( fmt.Sprintf("All backend references resolved; %d holder-available IPv6 endpoints are eligible", len(endpoints))) if status.Effective && len(endpoints) == 0 { setRouteParentCondition(&status, route.Generation, networkingv1alpha.NetworkLoadBalancerRouteProgrammed, - metav1.ConditionFalse, nlbRouteReasonNoEligibleEndpoints, "No enabled backend has a holder-available IPv6 endpoint in a selected location") + metav1.ConditionFalse, nlbRouteReasonNoEligibleEndpoints, "No enabled backend has a holder-available IPv6 endpoint with source identity in a selected location") } return status, nil } @@ -493,7 +495,10 @@ func resolveNetworkLoadBalancerRouteBackends( for i := range members { member := &members[i] location := member.Labels[networkingv1alpha.NetworkInterfaceLocationLabel] - if location == "" || !routeLocationSelected(parent, listener, location) || !isServiceMemberHealthy(member) { + sourceNamespace := member.Labels[networkingv1alpha.NetworkInterfaceSourceNamespaceLabel] + sourceUID := member.Labels[networkingv1alpha.NetworkInterfaceSourceUIDLabel] + if !isProjection(member) || sourceNamespace == "" || sourceUID == "" || + location == "" || !routeLocationSelected(parent, listener, location) || !isServiceMemberHealthy(member) { continue } address, ok := addressOfType(interfaceBackhaulAddresses(member), discoveryv1.AddressTypeIPv6) @@ -501,11 +506,13 @@ func resolveNetworkLoadBalancerRouteBackends( continue } endpoints = append(endpoints, networkLoadBalancerRouteEndpoint{ - NetworkService: ref.Name, - Interface: member.Name, - Location: location, - Address: address, - Port: port.Port, + NetworkService: ref.Name, + Interface: member.Name, + SourceNamespace: sourceNamespace, + SourceUID: sourceUID, + Location: location, + Address: address, + Port: port.Port, }) } } diff --git a/internal/controller/networkloadbalancerroute_controller_test.go b/internal/controller/networkloadbalancerroute_controller_test.go index ab16bd8b..de0c8625 100644 --- a/internal/controller/networkloadbalancerroute_controller_test.go +++ b/internal/controller/networkloadbalancerroute_controller_test.go @@ -76,7 +76,13 @@ func testRouteMember(name string, available bool, address string) *networkingv1a return &networkingv1alpha.NetworkInterface{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", Name: name, - Labels: map[string]string{"app": "storefront", networkingv1alpha.NetworkInterfaceLocationLabel: "dfw"}, + Labels: map[string]string{ + "app": "storefront", + networkingv1alpha.NetworkInterfaceLocationLabel: "dfw", + networkingv1alpha.NetworkInterfaceProjectionLabel: "true", + networkingv1alpha.NetworkInterfaceSourceNamespaceLabel: "cell-test", + networkingv1alpha.NetworkInterfaceSourceUIDLabel: "source-" + name, + }, }, Spec: networkingv1alpha.NetworkInterfaceSpec{ Network: networkingv1alpha.LocalNetworkRef{Name: "default"}, @@ -508,6 +514,8 @@ func TestNetworkLoadBalancerRouteEndpointResolution(t *testing.T) { require.NoError(t, err) require.Len(t, endpoints, 1) assert.Equal(t, "fd00::1", endpoints[0].Address) + assert.Equal(t, "cell-test", endpoints[0].SourceNamespace) + assert.Equal(t, "source-ready-v6", endpoints[0].SourceUID) assert.Equal(t, "dfw", endpoints[0].Location) parent.Spec.Exposure.Addresses[0].Locations = &networkingv1alpha.NetworkLoadBalancerLocationSelection{ @@ -521,6 +529,13 @@ func TestNetworkLoadBalancerRouteEndpointResolution(t *testing.T) { endpoints, err = resolveNetworkLoadBalancerRouteBackends(ctx, cl, route, parent, &parent.Spec.Listeners[0]) require.NoError(t, err) require.Len(t, endpoints, 1, "omitted locations must follow backend membership") + + member := testRouteMember("missing-source-uid", true, "fd00::3/128") + delete(member.Labels, networkingv1alpha.NetworkInterfaceSourceUIDLabel) + cl = fake.NewClientBuilder().WithScheme(networkServiceTestScheme(t)).WithObjects(service, member).Build() + endpoints, err = resolveNetworkLoadBalancerRouteBackends(ctx, cl, route, parent, &parent.Spec.Listeners[0]) + require.NoError(t, err) + assert.Empty(t, endpoints) } func TestNetworkLoadBalancerRouteUDPRequiresTCPProbe(t *testing.T) {