From 5705be267f58f954730c16bb2fb00cb8804ff777 Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Fri, 19 Jul 2024 23:11:46 +0300 Subject: [PATCH] Fix the CEL definitions to allow policies that use target selectors without explicit targetRefs Signed-off-by: Lior Okman --- api/v1alpha1/backendtrafficpolicy_types.go | 2 +- api/v1alpha1/clienttrafficpolicy_types.go | 2 +- api/v1alpha1/envoyextensionypolicy_types.go | 2 +- api/v1alpha1/securitypolicy_types.go | 2 +- ....envoyproxy.io_backendtrafficpolicies.yaml | 5 +++-- ...y.envoyproxy.io_clienttrafficpolicies.yaml | 5 +++-- ....envoyproxy.io_envoyextensionpolicies.yaml | 5 +++-- ...ateway.envoyproxy.io_securitypolicies.yaml | 5 +++-- .../backendtrafficpolicy_test.go | 19 +++++++++++++++++++ .../clienttrafficpolicy_test.go | 19 +++++++++++++++++++ .../envoyextensionpolicy_test.go | 19 +++++++++++++++++++ test/cel-validation/securitypolicy_test.go | 19 +++++++++++++++++++ 12 files changed, 92 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/backendtrafficpolicy_types.go b/api/v1alpha1/backendtrafficpolicy_types.go index 8c23b133231..f484f44b409 100644 --- a/api/v1alpha1/backendtrafficpolicy_types.go +++ b/api/v1alpha1/backendtrafficpolicy_types.go @@ -34,7 +34,7 @@ type BackendTrafficPolicy struct { Status gwapiv1a2.PolicyStatus `json:"status,omitempty"` } -// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs))", message="either targetRef or targetRefs must be used" +// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() > 0) ", message="either targetRef or targetRefs must be used" // // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.group == 'gateway.networking.k8s.io' : true ", message="this policy can only have a targetRef.group of gateway.networking.k8s.io" // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'UDPRoute', 'TCPRoute', 'TLSRoute'] : true", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute" diff --git a/api/v1alpha1/clienttrafficpolicy_types.go b/api/v1alpha1/clienttrafficpolicy_types.go index 20953b1960b..397535ebf43 100644 --- a/api/v1alpha1/clienttrafficpolicy_types.go +++ b/api/v1alpha1/clienttrafficpolicy_types.go @@ -34,7 +34,7 @@ type ClientTrafficPolicy struct { Status gwapiv1a2.PolicyStatus `json:"status,omitempty"` } -// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs))", message="either targetRef or targetRefs must be used" +// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() > 0) ", message="either targetRef or targetRefs must be used" // // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.group == 'gateway.networking.k8s.io' : true", message="this policy can only have a targetRef.group of gateway.networking.k8s.io" // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.kind == 'Gateway' : true", message="this policy can only have a targetRef.kind of Gateway" diff --git a/api/v1alpha1/envoyextensionypolicy_types.go b/api/v1alpha1/envoyextensionypolicy_types.go index b88cdcf0d32..cbab194b24e 100644 --- a/api/v1alpha1/envoyextensionypolicy_types.go +++ b/api/v1alpha1/envoyextensionypolicy_types.go @@ -32,7 +32,7 @@ type EnvoyExtensionPolicy struct { Status gwapiv1a2.PolicyStatus `json:"status,omitempty"` } -// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs))", message="either targetRef or targetRefs must be used" +// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() > 0) ", message="either targetRef or targetRefs must be used" // // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.group == 'gateway.networking.k8s.io' : true", message="this policy can only have a targetRef.group of gateway.networking.k8s.io" // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'UDPRoute', 'TCPRoute', 'TLSRoute'] : true", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute/UDPRoute/TLSRoute" diff --git a/api/v1alpha1/securitypolicy_types.go b/api/v1alpha1/securitypolicy_types.go index c0cb7543a45..61277ba3734 100644 --- a/api/v1alpha1/securitypolicy_types.go +++ b/api/v1alpha1/securitypolicy_types.go @@ -33,7 +33,7 @@ type SecurityPolicy struct { Status gwapiv1a2.PolicyStatus `json:"status,omitempty"` } -// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs))", message="either targetRef or targetRefs must be used" +// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() > 0) ", message="either targetRef or targetRefs must be used" // // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.group == 'gateway.networking.k8s.io' : true", message="this policy can only have a targetRef.group of gateway.networking.k8s.io" // +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute'] : true", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute" diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml index 2c7ec871c64..20ffe833923 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml @@ -1124,8 +1124,9 @@ spec: type: object x-kubernetes-validations: - message: either targetRef or targetRefs must be used - rule: (has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) - && has(self.targetRefs)) + rule: '(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) + && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() + > 0) ' - message: this policy can only have a targetRef.group of gateway.networking.k8s.io rule: 'has(self.targetRef) ? self.targetRef.group == ''gateway.networking.k8s.io'' : true ' diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index 2e2224ecca3..48cfb9f3aad 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml @@ -700,8 +700,9 @@ spec: type: object x-kubernetes-validations: - message: either targetRef or targetRefs must be used - rule: (has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) - && has(self.targetRefs)) + rule: '(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) + && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() + > 0) ' - message: this policy can only have a targetRef.group of gateway.networking.k8s.io rule: 'has(self.targetRef) ? self.targetRef.group == ''gateway.networking.k8s.io'' : true' diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyextensionpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyextensionpolicies.yaml index 0b257603050..61827ee1205 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyextensionpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyextensionpolicies.yaml @@ -521,8 +521,9 @@ spec: type: object x-kubernetes-validations: - message: either targetRef or targetRefs must be used - rule: (has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) - && has(self.targetRefs)) + rule: '(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) + && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() + > 0) ' - message: this policy can only have a targetRef.group of gateway.networking.k8s.io rule: 'has(self.targetRef) ? self.targetRef.group == ''gateway.networking.k8s.io'' : true' diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index e5c0b7fc95a..3906b325b3d 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -1129,8 +1129,9 @@ spec: type: object x-kubernetes-validations: - message: either targetRef or targetRefs must be used - rule: (has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) - && has(self.targetRefs)) + rule: '(has(self.targetRef) && !has(self.targetRefs)) || (!has(self.targetRef) + && has(self.targetRefs)) || (has(self.targetSelectors) && self.targetSelectors.size() + > 0) ' - message: this policy can only have a targetRef.group of gateway.networking.k8s.io rule: 'has(self.targetRef) ? self.targetRef.group == ''gateway.networking.k8s.io'' : true' diff --git a/test/cel-validation/backendtrafficpolicy_test.go b/test/cel-validation/backendtrafficpolicy_test.go index e8418790d31..903d3fa19f1 100644 --- a/test/cel-validation/backendtrafficpolicy_test.go +++ b/test/cel-validation/backendtrafficpolicy_test.go @@ -1134,6 +1134,25 @@ func TestBackendTrafficPolicyTarget(t *testing.T) { " Invalid value: \"object\": either targetRef or targetRefs must be used", }, }, + { + desc: "target selectors without targetRefs or targetRef", + mutate: func(sp *egv1a1.BackendTrafficPolicy) { + sp.Spec = egv1a1.BackendTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetSelectors: []egv1a1.TargetSelector{ + { + Group: ptr.To(gwapiv1a2.Group("gateway.networking.k8s.io")), + Kind: "HTTPRoute", + MatchLabels: map[string]string{ + "eg/namespace": "reference-apps", + }, + }, + }, + }, + } + }, + wantErrors: []string{}, + }, } for _, tc := range cases { diff --git a/test/cel-validation/clienttrafficpolicy_test.go b/test/cel-validation/clienttrafficpolicy_test.go index e28ad6c83bf..4d98efd0aa2 100644 --- a/test/cel-validation/clienttrafficpolicy_test.go +++ b/test/cel-validation/clienttrafficpolicy_test.go @@ -445,6 +445,25 @@ func TestClientTrafficPolicyTarget(t *testing.T) { }, wantErrors: []string{}, }, + { + desc: "target selectors without targetRefs or targetRef", + mutate: func(sp *egv1a1.ClientTrafficPolicy) { + sp.Spec = egv1a1.ClientTrafficPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetSelectors: []egv1a1.TargetSelector{ + { + Group: ptr.To(gwapiv1a2.Group("gateway.networking.k8s.io")), + Kind: "HTTPRoute", + MatchLabels: map[string]string{ + "eg/namespace": "reference-apps", + }, + }, + }, + }, + } + }, + wantErrors: []string{}, + }, } for _, tc := range cases { diff --git a/test/cel-validation/envoyextensionpolicy_test.go b/test/cel-validation/envoyextensionpolicy_test.go index c16d7169dbc..b199ed49d72 100644 --- a/test/cel-validation/envoyextensionpolicy_test.go +++ b/test/cel-validation/envoyextensionpolicy_test.go @@ -407,6 +407,25 @@ func TestEnvoyExtensionPolicyTarget(t *testing.T) { "spec.extProc[0].processingMode.request.body: Unsupported value: \"not-a-body-mode\": supported values: \"Streamed\", \"Buffered\", \"BufferedPartial\"", }, }, + { + desc: "target selectors without targetRefs or targetRef", + mutate: func(sp *egv1a1.EnvoyExtensionPolicy) { + sp.Spec = egv1a1.EnvoyExtensionPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetSelectors: []egv1a1.TargetSelector{ + { + Group: ptr.To(gwapiv1a2.Group("gateway.networking.k8s.io")), + Kind: "HTTPRoute", + MatchLabels: map[string]string{ + "eg/namespace": "reference-apps", + }, + }, + }, + }, + } + }, + wantErrors: []string{}, + }, } for _, tc := range cases { diff --git a/test/cel-validation/securitypolicy_test.go b/test/cel-validation/securitypolicy_test.go index 4c24c8ed712..24f229dd6f1 100644 --- a/test/cel-validation/securitypolicy_test.go +++ b/test/cel-validation/securitypolicy_test.go @@ -899,6 +899,25 @@ func TestSecurityPolicyTarget(t *testing.T) { }, wantErrors: []string{}, }, + { + desc: "target selectors without targetRefs or targetRef", + mutate: func(sp *egv1a1.SecurityPolicy) { + sp.Spec = egv1a1.SecurityPolicySpec{ + PolicyTargetReferences: egv1a1.PolicyTargetReferences{ + TargetSelectors: []egv1a1.TargetSelector{ + { + Group: ptr.To(gwapiv1a2.Group("gateway.networking.k8s.io")), + Kind: "HTTPRoute", + MatchLabels: map[string]string{ + "eg/namespace": "reference-apps", + }, + }, + }, + }, + } + }, + wantErrors: []string{}, + }, } for _, tc := range cases {