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: