fix

Signed-off-by: zirain <zirain2009@gmail.com>

Author: zirain

api/v1alpha1/envoypatchpolicy_types.go

@@ -81,6 +81,7 @@ const (
 // using JSONPatch semantics.
 //
 // +kubebuilder:validation:XValidation:rule="!(has(self.name) && has(self.nameSelector))",message="only one of name and nameSelector can be specified"
+// +kubebuilder:validation:XValidation:rule="has(self.name) || has(self.nameSelector)",message="either name or nameSelector must be specified"
 type EnvoyJSONPatchConfig struct {
 	// Type is the typed URL of the Envoy xDS Resource
 	Type EnvoyResourceType `json:"type"`

charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml

@@ -145,6 +145,8 @@ spec:
                   x-kubernetes-validations:
                   - message: only one of name and nameSelector can be specified
                     rule: '!(has(self.name) && has(self.nameSelector))'
+                  - message: either name or nameSelector must be specified
+                    rule: has(self.name) || has(self.nameSelector)
                 type: array
               priority:
                 description: |-

charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml

@@ -144,6 +144,8 @@ spec:
                   x-kubernetes-validations:
                   - message: only one of name and nameSelector can be specified
                     rule: '!(has(self.name) && has(self.nameSelector))'
+                  - message: either name or nameSelector must be specified
+                    rule: has(self.name) || has(self.nameSelector)
                 type: array
               priority:
                 description: |-

internal/gatewayapi/envoypatchpolicy.go

@@ -129,6 +129,7 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
 		}
 
 		// Save the patch
+		policyRejected := false
 		for _, patch := range policy.Spec.JSONPatches {
 			irPatch := ir.JSONPatchConfig{}
 			irPatch.Type = string(patch.Type)
@@ -154,7 +155,8 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
 						resolveErr,
 					)
 
-					continue
+					policyRejected = true
+					break
 				}
 			}
 			irPatch.Operation.Op = ir.JSONPatchOp(patch.Operation.Op)
@@ -166,8 +168,14 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo
 			policyIR.JSONPatches = append(policyIR.JSONPatches, &irPatch)
 		}
 
-		// Set Accepted=True
-		status.SetAcceptedForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, policy.Generation)
+		// Only set Accepted=True if policy was not rejected due to validation errors
+		if !policyRejected {
+			// Set Accepted=True
+			status.SetAcceptedForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, policy.Generation)
+		} else {
+			// Clear any partial patches that were added before validation failed
+			policyIR.JSONPatches = nil
+		}
 	}
 
 	return res

test/cel-validation/envoypatchpolicy_test.go

@@ -125,6 +125,32 @@ func TestEnvoyPatchPolicy(t *testing.T) {
 			},
 			wantErrors: []string{"only one of name and nameSelector can be specified"},
 		},
+		{
+			desc: "invalid json patch with neither name nor nameSelector",
+			mutate: func(epp *egv1a1.EnvoyPatchPolicy) {
+				epp.Spec = egv1a1.EnvoyPatchPolicySpec{
+					Type: egv1a1.JSONPatchEnvoyPatchType,
+					JSONPatches: []egv1a1.EnvoyJSONPatchConfig{
+						{
+							Type: egv1a1.ListenerEnvoyResourceType,
+							Operation: egv1a1.JSONPatchOperation{
+								Op:   egv1a1.JSONPatchOperationType("test"),
+								Path: &patchPath,
+								Value: &apiextensionsv1.JSON{
+									Raw: []byte(`"listener-1"`),
+								},
+							},
+						},
+					},
+					TargetRef: gwapiv1.LocalPolicyTargetReference{
+						Group: gwapiv1.Group(gwapiv1.GroupName),
+						Kind:  gwapiv1.Kind("Gateway"),
+						Name:  gwapiv1.ObjectName("eg"),
+					},
+				}
+			},
+			wantErrors: []string{"either name or nameSelector must be specified"},
+		},
 	}
 
 	for _, tc := range cases {

test/helm/gateway-crds-helm/all.out.yaml

@@ -30517,6 +30517,8 @@ spec:
                   x-kubernetes-validations:
                   - message: only one of name and nameSelector can be specified
                     rule: '!(has(self.name) && has(self.nameSelector))'
+                  - message: either name or nameSelector must be specified
+                    rule: has(self.name) || has(self.nameSelector)
                 type: array
               priority:
                 description: |-

test/helm/gateway-crds-helm/e2e.out.yaml

@@ -8490,6 +8490,8 @@ spec:
                   x-kubernetes-validations:
                   - message: only one of name and nameSelector can be specified
                     rule: '!(has(self.name) && has(self.nameSelector))'
+                  - message: either name or nameSelector must be specified
+                    rule: has(self.name) || has(self.nameSelector)
                 type: array
               priority:
                 description: |-

test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml

@@ -8490,6 +8490,8 @@ spec:
                   x-kubernetes-validations:
                   - message: only one of name and nameSelector can be specified
                     rule: '!(has(self.name) && has(self.nameSelector))'
+                  - message: either name or nameSelector must be specified
+                    rule: has(self.name) || has(self.nameSelector)
                 type: array
               priority:
                 description: |-