fix: fix the CEL definitions to allow policies that use target select…

…ors without explicit targetRefs (envoyproxy#3904) Fix the CEL definitions to allow policies that use target selectors without explicit targetRefs Signed-off-by: Lior Okman <[email protected]>
guydc · Jul 22, 2024 · 1d98585 · 1d98585
1 parent a2849c7
commit 1d98585
Show file tree

Hide file tree

Showing 12 changed files with 92 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {