From d1f03b3914a5bdd5936d7562b4614ad5309c34c0 Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Thu, 8 Oct 2026 21:31:17 -0400 Subject: [PATCH] refactor!: Drop vpcAttachmentRef from NetworkRule A NetworkRule's backends come from backendSelector, which picks any number of VPCAttachments in vpcRef's VPC. A single attachment reference no longer identifies anything about the rule, and no consumer reads it: galactic only copied it into a struct and an error message. Remove the field from NetworkRuleSpec, the CRD, the sample and the e2e test, and add an e2e step that a rule still setting it is rejected as an unknown field. Reword the shared Accepted reason comments, which also serve NetworkEgressPolicy, so they no longer name the field. NetworkEgressPolicy keeps its own vpcAttachmentRef. Co-Authored-By: Claude Opus 5.5 --- api/v1alpha1/egresspolicy_types.go | 10 ++--- api/v1alpha1/rule_types.go | 40 ++++++++---------- api/v1alpha1/rule_types_test.go | 12 ++---- ...k.datumapis.com_networkegresspolicies.yaml | 10 ++--- .../network.datumapis.com_networkrules.yaml | 27 ++++-------- ...atumapis.com_v1alpha1_networkrule_tcp.yaml | 1 - docs/api/gateway.md | 30 +++++++------- .../chainsaw-test.yaml | 41 +++++++++++++++++-- 8 files changed, 90 insertions(+), 81 deletions(-) diff --git a/api/v1alpha1/egresspolicy_types.go b/api/v1alpha1/egresspolicy_types.go index f084b5a..8629c5c 100644 --- a/api/v1alpha1/egresspolicy_types.go +++ b/api/v1alpha1/egresspolicy_types.go @@ -13,11 +13,11 @@ import ( // backend list. // // It is namespaced (deployed to galactic-system) and tenant-writable; like -// NetworkRule, vpcRef/vpcAttachmentRef are opaque string identifiers because -// the VPC API is owned by a separate companion operator, not this repo. An -// admission webhook (implemented by the consuming controller) must verify -// the requester is authorized for vpcRef/vpcAttachmentRef before a policy is -// accepted — see the Accepted condition. +// NetworkRule's vpcRef, vpcRef/vpcAttachmentRef are opaque string identifiers +// because the VPC API is owned by a separate companion operator, not this +// repo. An admission webhook (implemented by the consuming controller) must +// verify the requester is authorized for vpcRef/vpcAttachmentRef before a +// policy is accepted — see the Accepted condition. // // Presence of an accepted NetworkEgressPolicy resolves only *enablement* // (should this tenant's VRF get an egress route toward the shared diff --git a/api/v1alpha1/rule_types.go b/api/v1alpha1/rule_types.go index 0ff18a3..7b62eb2 100644 --- a/api/v1alpha1/rule_types.go +++ b/api/v1alpha1/rule_types.go @@ -18,29 +18,28 @@ const ( NetworkRuleProtocolUDP NetworkRuleProtocol = "udp" ) -// Accepted reasons — used as Accepted.Reason on NetworkRule, set by the -// admission webhook that verifies the requester is authorized for the -// vpcRef/vpcAttachmentRef named in the rule. +// Accepted reasons — used as Accepted.Reason on NetworkRule (and reused by +// NetworkEgressPolicy), set by the admission webhook that verifies the +// requester is authorized for the VPC resources the object references. const ( // AcceptedReasonOwnershipVerified indicates admission verified the - // requester is authorized for the target VPC/VPCAttachment. + // requester is authorized for the VPC resources the object references. AcceptedReasonOwnershipVerified string = "OwnershipVerified" - // AcceptedReasonOwnershipDenied indicates admission rejected the rule - // because the requester is not authorized for the target - // VPC/VPCAttachment named in vpcRef/vpcAttachmentRef. + // AcceptedReasonOwnershipDenied indicates admission rejected the object + // because the requester is not authorized for the VPC resources it + // references. AcceptedReasonOwnershipDenied string = "OwnershipDenied" ) -// NetworkRule defines ingress load-balancing for a single tenant -// VPC/VPCAttachment, served by every NetworkGateway node identically -// (anycast Direct Server Return — see NetworkGateway's doc comment). It is -// namespaced (deployed to galactic-system) and tenant-writable; the -// vpcRef/vpcAttachmentRef fields are opaque string identifiers because the -// VPC API is owned by a separate companion operator, not this repo. An -// admission webhook (implemented by the consuming controller) must verify -// the requester is authorized for vpcRef/vpcAttachmentRef before a rule is -// accepted — see the Accepted condition. +// NetworkRule defines ingress load-balancing for a single tenant VPC, served +// by every NetworkGateway node identically (anycast Direct Server Return — +// see NetworkGateway's doc comment). It is namespaced (deployed to +// galactic-system) and tenant-writable; vpcRef is an opaque string identifier +// because the VPC API is owned by a separate companion operator, not this +// repo. An admission webhook (implemented by the consuming controller) must +// verify the requester is authorized for vpcRef before a rule is accepted — +// see the Accepted condition. // // Unlike the earlier Full-NAT design this type originally described, there // is no primary/secondary gateway node for a rule: every NetworkGateway @@ -72,7 +71,7 @@ type NetworkRule struct { } // NetworkRuleSpec defines the desired ingress load-balancing state for a -// tenant VPC/VPCAttachment. +// tenant VPC. type NetworkRuleSpec struct { // VPCRef is the opaque identifier of the target VPC this rule applies // to. This repo does not own the VPC API and does not validate the @@ -83,13 +82,6 @@ type NetworkRuleSpec struct { // +kubebuilder:validation:MinLength=1 VPCRef string `json:"vpcRef"` - // VPCAttachmentRef is the opaque identifier of the target - // VPCAttachment this rule applies to. Like VPCRef, this is an opaque - // string reference validated by the admission webhook, not by this API. - // +kubebuilder:validation:Required - // +kubebuilder:validation:MinLength=1 - VPCAttachmentRef string `json:"vpcAttachmentRef"` - // VIPAddresses is the list of ingress VIP addresses (IPv4 and/or IPv6) // this rule provisions on the assigned gateway node(s). // +kubebuilder:validation:Required diff --git a/api/v1alpha1/rule_types_test.go b/api/v1alpha1/rule_types_test.go index 6c57f0e..f0fca19 100644 --- a/api/v1alpha1/rule_types_test.go +++ b/api/v1alpha1/rule_types_test.go @@ -15,11 +15,10 @@ func newTestRule() *NetworkRule { }, ObjectMeta: metav1.ObjectMeta{Name: "test-rule", Namespace: "galactic-system"}, Spec: NetworkRuleSpec{ - VPCRef: "vpc-a", - VPCAttachmentRef: "vpcattachment-a", - VIPAddresses: []string{"2001:db8:1::10"}, - Protocol: NetworkRuleProtocolTCP, - Port: 443, + VPCRef: "vpc-a", + VIPAddresses: []string{"2001:db8:1::10"}, + Protocol: NetworkRuleProtocolTCP, + Port: 443, BackendSelector: metav1.LabelSelector{ MatchLabels: map[string]string{"app": "web"}, }, @@ -77,9 +76,6 @@ func TestNetworkRuleJSONRoundTrip(t *testing.T) { if got.Spec.VPCRef != orig.Spec.VPCRef { t.Errorf("VPCRef: got %q, want %q", got.Spec.VPCRef, orig.Spec.VPCRef) } - if got.Spec.VPCAttachmentRef != orig.Spec.VPCAttachmentRef { - t.Errorf("VPCAttachmentRef: got %q, want %q", got.Spec.VPCAttachmentRef, orig.Spec.VPCAttachmentRef) - } if got.Spec.BackendSelector.MatchLabels["app"] != "web" || len(got.Spec.BackendSelector.MatchExpressions) != 1 { t.Errorf("BackendSelector: got %+v", got.Spec.BackendSelector) } diff --git a/config/crd/network.datumapis.com_networkegresspolicies.yaml b/config/crd/network.datumapis.com_networkegresspolicies.yaml index a20527b..4b6044b 100644 --- a/config/crd/network.datumapis.com_networkegresspolicies.yaml +++ b/config/crd/network.datumapis.com_networkegresspolicies.yaml @@ -39,11 +39,11 @@ spec: backend list. It is namespaced (deployed to galactic-system) and tenant-writable; like - NetworkRule, vpcRef/vpcAttachmentRef are opaque string identifiers because - the VPC API is owned by a separate companion operator, not this repo. An - admission webhook (implemented by the consuming controller) must verify - the requester is authorized for vpcRef/vpcAttachmentRef before a policy is - accepted — see the Accepted condition. + NetworkRule's vpcRef, vpcRef/vpcAttachmentRef are opaque string identifiers + because the VPC API is owned by a separate companion operator, not this + repo. An admission webhook (implemented by the consuming controller) must + verify the requester is authorized for vpcRef/vpcAttachmentRef before a + policy is accepted — see the Accepted condition. Presence of an accepted NetworkEgressPolicy resolves only *enablement* (should this tenant's VRF get an egress route toward the shared diff --git a/config/crd/network.datumapis.com_networkrules.yaml b/config/crd/network.datumapis.com_networkrules.yaml index f1cb406..2b5cf86 100644 --- a/config/crd/network.datumapis.com_networkrules.yaml +++ b/config/crd/network.datumapis.com_networkrules.yaml @@ -33,15 +33,14 @@ spec: schema: openAPIV3Schema: description: |- - NetworkRule defines ingress load-balancing for a single tenant - VPC/VPCAttachment, served by every NetworkGateway node identically - (anycast Direct Server Return — see NetworkGateway's doc comment). It is - namespaced (deployed to galactic-system) and tenant-writable; the - vpcRef/vpcAttachmentRef fields are opaque string identifiers because the - VPC API is owned by a separate companion operator, not this repo. An - admission webhook (implemented by the consuming controller) must verify - the requester is authorized for vpcRef/vpcAttachmentRef before a rule is - accepted — see the Accepted condition. + NetworkRule defines ingress load-balancing for a single tenant VPC, served + by every NetworkGateway node identically (anycast Direct Server Return — + see NetworkGateway's doc comment). It is namespaced (deployed to + galactic-system) and tenant-writable; vpcRef is an opaque string identifier + because the VPC API is owned by a separate companion operator, not this + repo. An admission webhook (implemented by the consuming controller) must + verify the requester is authorized for vpcRef before a rule is accepted — + see the Accepted condition. Unlike the earlier Full-NAT design this type originally described, there is no primary/secondary gateway node for a rule: every NetworkGateway @@ -77,7 +76,7 @@ spec: spec: description: |- NetworkRuleSpec defines the desired ingress load-balancing state for a - tenant VPC/VPCAttachment. + tenant VPC. properties: backendPort: description: BackendPort is the destination port on every selected @@ -173,13 +172,6 @@ spec: x-kubernetes-validations: - message: vipAddresses must all be valid IPv4 or IPv6 addresses rule: self.all(v, isIP(v)) - vpcAttachmentRef: - description: |- - VPCAttachmentRef is the opaque identifier of the target - VPCAttachment this rule applies to. Like VPCRef, this is an opaque - string reference validated by the admission webhook, not by this API. - minLength: 1 - type: string vpcRef: description: |- VPCRef is the opaque identifier of the target VPC this rule applies @@ -195,7 +187,6 @@ spec: - port - protocol - vipAddresses - - vpcAttachmentRef - vpcRef type: object status: diff --git a/config/samples/network.datumapis.com_v1alpha1_networkrule_tcp.yaml b/config/samples/network.datumapis.com_v1alpha1_networkrule_tcp.yaml index bea9747..14955d9 100644 --- a/config/samples/network.datumapis.com_v1alpha1_networkrule_tcp.yaml +++ b/config/samples/network.datumapis.com_v1alpha1_networkrule_tcp.yaml @@ -5,7 +5,6 @@ metadata: namespace: galactic-system spec: vpcRef: vpc-a - vpcAttachmentRef: vpc-a-iad vipAddresses: - 2001:db8:1::10 protocol: tcp diff --git a/docs/api/gateway.md b/docs/api/gateway.md index 12142b4..9a4c898 100644 --- a/docs/api/gateway.md +++ b/docs/api/gateway.md @@ -212,11 +212,11 @@ of an egress flow is an arbitrary internet address, not a pre-configured backend list. It is namespaced (deployed to galactic-system) and tenant-writable; like -NetworkRule, vpcRef/vpcAttachmentRef are opaque string identifiers because -the VPC API is owned by a separate companion operator, not this repo. An -admission webhook (implemented by the consuming controller) must verify -the requester is authorized for vpcRef/vpcAttachmentRef before a policy is -accepted — see the Accepted condition. +NetworkRule's vpcRef, vpcRef/vpcAttachmentRef are opaque string identifiers +because the VPC API is owned by a separate companion operator, not this +repo. An admission webhook (implemented by the consuming controller) must +verify the requester is authorized for vpcRef/vpcAttachmentRef before a +policy is accepted — see the Accepted condition. Presence of an accepted NetworkEgressPolicy resolves only *enablement* (should this tenant's VRF get an egress route toward the shared @@ -356,15 +356,14 @@ _Appears in:_ -NetworkRule defines ingress load-balancing for a single tenant -VPC/VPCAttachment, served by every NetworkGateway node identically -(anycast Direct Server Return — see NetworkGateway's doc comment). It is -namespaced (deployed to galactic-system) and tenant-writable; the -vpcRef/vpcAttachmentRef fields are opaque string identifiers because the -VPC API is owned by a separate companion operator, not this repo. An -admission webhook (implemented by the consuming controller) must verify -the requester is authorized for vpcRef/vpcAttachmentRef before a rule is -accepted — see the Accepted condition. +NetworkRule defines ingress load-balancing for a single tenant VPC, served +by every NetworkGateway node identically (anycast Direct Server Return — +see NetworkGateway's doc comment). It is namespaced (deployed to +galactic-system) and tenant-writable; vpcRef is an opaque string identifier +because the VPC API is owned by a separate companion operator, not this +repo. An admission webhook (implemented by the consuming controller) must +verify the requester is authorized for vpcRef before a rule is accepted — +see the Accepted condition. Unlike the earlier Full-NAT design this type originally described, there is no primary/secondary gateway node for a rule: every NetworkGateway @@ -422,7 +421,7 @@ _Appears in:_ NetworkRuleSpec defines the desired ingress load-balancing state for a -tenant VPC/VPCAttachment. +tenant VPC. @@ -432,7 +431,6 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `vpcRef` _string_ | VPCRef is the opaque identifier of the target VPC this rule applies
to. This repo does not own the VPC API and does not validate the
identifier beyond non-emptiness; the admission webhook of the
consuming controller is responsible for verifying the requester is
authorized for this VPC before the rule is accepted. | | MinLength: 1
Required: \{\}
| -| `vpcAttachmentRef` _string_ | VPCAttachmentRef is the opaque identifier of the target
VPCAttachment this rule applies to. Like VPCRef, this is an opaque
string reference validated by the admission webhook, not by this API. | | MinLength: 1
Required: \{\}
| | `vipAddresses` _string array_ | VIPAddresses is the list of ingress VIP addresses (IPv4 and/or IPv6)
this rule provisions on the assigned gateway node(s). | | MaxItems: 8
MinItems: 1
Required: \{\}
items:MaxLength: 45
| | `protocol` _[NetworkRuleProtocol](#networkruleprotocol)_ | Protocol is the transport protocol matched by VIPAddresses/Port. | | Enum: [tcp udp]
Required: \{\}
| | `port` _integer_ | Port is the ingress port on VIPAddresses that this rule load-balances. | | Maximum: 65535
Minimum: 1
Required: \{\}
| diff --git a/test/e2e/tests/networkrule-backend-selector/chainsaw-test.yaml b/test/e2e/tests/networkrule-backend-selector/chainsaw-test.yaml index ba79c65..7f04566 100644 --- a/test/e2e/tests/networkrule-backend-selector/chainsaw-test.yaml +++ b/test/e2e/tests/networkrule-backend-selector/chainsaw-test.yaml @@ -7,7 +7,8 @@ spec: Verify the schema rules for a NetworkRule's backend selection: a rule with a non-empty backendSelector and a backendPort is accepted, a rule with an empty backendSelector is rejected, and a rule that still lists static - backends is rejected, since that field no longer exists. Also verifies that a + backends or a vpcAttachmentRef is rejected, since neither field exists any + more. Also verifies that a ServiceVIPBinding requires vpcRef. Does not require any operator to be running; every rule under test is enforced by the API server alone. steps: @@ -21,7 +22,6 @@ spec: name: e2e-rule-selector spec: vpcRef: vpc-a - vpcAttachmentRef: vpc-a-attachment vipAddresses: - 2001:db8:1::10 protocol: tcp @@ -54,7 +54,6 @@ spec: name: e2e-rule-empty-selector spec: vpcRef: vpc-a - vpcAttachmentRef: vpc-a-attachment vipAddresses: - 2001:db8:1::11 protocol: tcp @@ -87,7 +86,6 @@ spec: name: e2e-rule-static-backends spec: vpcRef: vpc-a - vpcAttachmentRef: vpc-a-attachment vipAddresses: - 2001:db8:1::12 protocol: tcp @@ -109,6 +107,41 @@ spec: esac echo "OK: rule with static backends correctly rejected" + - name: reject-vpc-attachment-ref + try: + - script: + content: | + set +e + OUTPUT=$(kubectl apply -n "$NAMESPACE" -f - 2>&1 <<'YAML' + apiVersion: network.datumapis.com/v1alpha1 + kind: NetworkRule + metadata: + name: e2e-rule-vpc-attachment-ref + spec: + vpcRef: vpc-a + vpcAttachmentRef: vpc-a-attachment + vipAddresses: + - 2001:db8:1::13 + protocol: tcp + port: 443 + backendSelector: + matchLabels: + app: web + backendPort: 8443 + YAML + ) + EXIT=$? + set -e + if [ "$EXIT" -eq 0 ]; then + echo "ERROR: expected rejection of a rule with vpcAttachmentRef but the resource was accepted" + exit 1 + fi + case "$OUTPUT" in + *'unknown field "spec.vpcAttachmentRef"'*) ;; + *) echo "ERROR: rejected for the wrong reason: $OUTPUT"; exit 1 ;; + esac + echo "OK: rule with vpcAttachmentRef correctly rejected" + - name: reject-binding-without-vpcref try: - script: