From ccb8b62b4442cd524008bec233ad62c5c0195709 Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 10 Apr 2026 14:05:38 +0800 Subject: [PATCH 1/9] api: EnvoyPatchPolicy support targetRefs Signed-off-by: zirain --- api/v1alpha1/envoypatchpolicy_types.go | 17 +- api/v1alpha1/zz_generated.deepcopy.go | 11 +- ...eway.envoyproxy.io_envoypatchpolicies.yaml | 47 ++++- ...eway.envoyproxy.io_envoypatchpolicies.yaml | 47 ++++- site/content/en/latest/api/extension_types.md | 3 +- test/cel-validation/envoypatchpolicy_test.go | 172 ++++++++++++++++++ test/helm/gateway-crds-helm/all.out.yaml | 46 ++++- test/helm/gateway-crds-helm/e2e.out.yaml | 46 ++++- .../envoy-gateway-crds.out.yaml | 46 ++++- 9 files changed, 427 insertions(+), 8 deletions(-) create mode 100644 test/cel-validation/envoypatchpolicy_test.go diff --git a/api/v1alpha1/envoypatchpolicy_types.go b/api/v1alpha1/envoypatchpolicy_types.go index b131b7a46a..f3396a9964 100644 --- a/api/v1alpha1/envoypatchpolicy_types.go +++ b/api/v1alpha1/envoypatchpolicy_types.go @@ -40,6 +40,7 @@ type EnvoyPatchPolicy struct { // EnvoyPatchPolicySpec defines the desired state of EnvoyPatchPolicy. // +union +// +kubebuilder:validation:XValidation:rule="(has(self.targetRef) && (!has(self.targetRefs) || size(self.targetRefs) == 0)) || (!has(self.targetRef) && has(self.targetRefs) && size(self.targetRefs) > 0)",message="exactly one of targetRef or targetRefs must be set" type EnvoyPatchPolicySpec struct { // Type decides the type of patch. // Valid EnvoyPatchType values are "JSONPatch". @@ -57,7 +58,21 @@ type EnvoyPatchPolicySpec struct { // This Policy and the TargetRef MUST be in the same namespace // for this Policy to have effect and be applied to the Gateway // TargetRef - TargetRef gwapiv1.LocalPolicyTargetReference `json:"targetRef"` + // + // Deprecated: TargetRef is deprecated in favor of TargetRefs to support attaching to multiple resources. + // + // +optional + TargetRef *gwapiv1.LocalPolicyTargetReference `json:"targetRef"` + // TargetRefs is the names of the Gateway API resources this policy + // is being attached to. + // By default, attaching to Gateway is supported and + // when mergeGateways is enabled it should attach to GatewayClass. + // This Policy and the TargetRef MUST be in the same namespace + // for this Policy to have effect and be applied to the Gateway + // TargetRefs. + // + // +optional + TargetRefs []gwapiv1.LocalPolicyTargetReference `json:"targetRefs,omitempty"` // Priority of the EnvoyPatchPolicy. // If multiple EnvoyPatchPolicies are applied to the same // TargetRef, they will be applied in the ascending order of diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index fead3c5008..0e09a933ab 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -2788,7 +2788,16 @@ func (in *EnvoyPatchPolicySpec) DeepCopyInto(out *EnvoyPatchPolicySpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - out.TargetRef = in.TargetRef + if in.TargetRef != nil { + in, out := &in.TargetRef, &out.TargetRef + *out = new(v1.LocalPolicyTargetReference) + **out = **in + } + if in.TargetRefs != nil { + in, out := &in.TargetRefs, &out.TargetRefs + *out = make([]v1.LocalPolicyTargetReference, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnvoyPatchPolicySpec. diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml index 6c3579fee1..70deb57f8c 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml @@ -141,6 +141,8 @@ spec: This Policy and the TargetRef MUST be in the same namespace for this Policy to have effect and be applied to the Gateway TargetRef + + Deprecated: TargetRef is deprecated in favor of TargetRefs to support attaching to multiple resources. properties: group: description: Group is the group of the target resource. @@ -163,6 +165,45 @@ spec: - kind - name type: object + targetRefs: + description: |- + TargetRefs is the names of the Gateway API resources this policy + is being attached to. + By default, attaching to Gateway is supported and + when mergeGateways is enabled it should attach to GatewayClass. + This Policy and the TargetRef MUST be in the same namespace + for this Policy to have effect and be applied to the Gateway + TargetRefs. + items: + description: |- + LocalPolicyTargetReference identifies an API object to apply a direct or + inherited policy to. This should be used as part of Policy resources + that can target Gateway API resources. For more information on how this + policy attachment model works, and a sample Policy resource, refer to + the policy attachment documentation for Gateway API. + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + required: + - group + - kind + - name + type: object + type: array type: description: |- Type decides the type of patch. @@ -171,9 +212,13 @@ spec: - JSONPatch type: string required: - - targetRef - type type: object + x-kubernetes-validations: + - message: exactly one of targetRef or targetRefs must be set + rule: (has(self.targetRef) && (!has(self.targetRefs) || size(self.targetRefs) + == 0)) || (!has(self.targetRef) && has(self.targetRefs) && size(self.targetRefs) + > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml index c3f38eead5..1c691bb396 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoypatchpolicies.yaml @@ -140,6 +140,8 @@ spec: This Policy and the TargetRef MUST be in the same namespace for this Policy to have effect and be applied to the Gateway TargetRef + + Deprecated: TargetRef is deprecated in favor of TargetRefs to support attaching to multiple resources. properties: group: description: Group is the group of the target resource. @@ -162,6 +164,45 @@ spec: - kind - name type: object + targetRefs: + description: |- + TargetRefs is the names of the Gateway API resources this policy + is being attached to. + By default, attaching to Gateway is supported and + when mergeGateways is enabled it should attach to GatewayClass. + This Policy and the TargetRef MUST be in the same namespace + for this Policy to have effect and be applied to the Gateway + TargetRefs. + items: + description: |- + LocalPolicyTargetReference identifies an API object to apply a direct or + inherited policy to. This should be used as part of Policy resources + that can target Gateway API resources. For more information on how this + policy attachment model works, and a sample Policy resource, refer to + the policy attachment documentation for Gateway API. + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + required: + - group + - kind + - name + type: object + type: array type: description: |- Type decides the type of patch. @@ -170,9 +211,13 @@ spec: - JSONPatch type: string required: - - targetRef - type type: object + x-kubernetes-validations: + - message: exactly one of targetRef or targetRefs must be set + rule: (has(self.targetRef) && (!has(self.targetRefs) || size(self.targetRefs) + == 0)) || (!has(self.targetRef) && has(self.targetRefs) && size(self.targetRefs) + > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 7b40befba0..455d902704 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1927,7 +1927,8 @@ _Appears in:_ | --- | --- | --- | --- | --- | | `type` | _[EnvoyPatchType](#envoypatchtype)_ | true | | Type decides the type of patch.
Valid EnvoyPatchType values are "JSONPatch". | | `jsonPatches` | _[EnvoyJSONPatchConfig](#envoyjsonpatchconfig) array_ | false | | JSONPatch defines the JSONPatch configuration. | -| `targetRef` | _[LocalPolicyTargetReference](https://gateway-api.sigs.k8s.io/reference/1.5/spec/#localpolicytargetreference)_ | true | | TargetRef is the name of the Gateway API resource this policy
is being attached to.
By default, attaching to Gateway is supported and
when mergeGateways is enabled it should attach to GatewayClass.
This Policy and the TargetRef MUST be in the same namespace
for this Policy to have effect and be applied to the Gateway
TargetRef | +| `targetRef` | _[LocalPolicyTargetReference](https://gateway-api.sigs.k8s.io/reference/1.5/spec/#localpolicytargetreference)_ | false | | TargetRef is the name of the Gateway API resource this policy
is being attached to.
By default, attaching to Gateway is supported and
when mergeGateways is enabled it should attach to GatewayClass.
This Policy and the TargetRef MUST be in the same namespace
for this Policy to have effect and be applied to the Gateway
TargetRef
Deprecated: TargetRef is deprecated in favor of TargetRefs to support attaching to multiple resources. | +| `targetRefs` | _[LocalPolicyTargetReference](https://gateway-api.sigs.k8s.io/reference/1.5/spec/#localpolicytargetreference) array_ | false | | TargetRefs is the names of the Gateway API resources this policy
is being attached to.
By default, attaching to Gateway is supported and
when mergeGateways is enabled it should attach to GatewayClass.
This Policy and the TargetRef MUST be in the same namespace
for this Policy to have effect and be applied to the Gateway
TargetRefs. | | `priority` | _integer_ | true | | Priority of the EnvoyPatchPolicy.
If multiple EnvoyPatchPolicies are applied to the same
TargetRef, they will be applied in the ascending order of
the priority i.e. int32.min has the highest priority and
int32.max has the lowest priority.
Defaults to 0. | diff --git a/test/cel-validation/envoypatchpolicy_test.go b/test/cel-validation/envoypatchpolicy_test.go new file mode 100644 index 0000000000..7d568fe802 --- /dev/null +++ b/test/cel-validation/envoypatchpolicy_test.go @@ -0,0 +1,172 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +//go:build celvalidation + +package celvalidation + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" +) + +func TestEnvoyPatchPolicyTarget(t *testing.T) { + ctx := context.Background() + baseEPP := egv1a1.EnvoyPatchPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "epp", + Namespace: metav1.NamespaceDefault, + }, + Spec: egv1a1.EnvoyPatchPolicySpec{ + Type: egv1a1.JSONPatchEnvoyPatchType, + JSONPatches: []egv1a1.EnvoyJSONPatchConfig{ + { + Type: egv1a1.ListenerEnvoyResourceType, + Name: "test-listener", + Operation: egv1a1.JSONPatchOperation{ + Op: "add", + Path: ptr.To("/foo"), + Value: &apiextensionsv1.JSON{Raw: []byte(`"bar"`)}, + }, + }, + }, + }, + } + + cases := []struct { + desc string + mutate func(epp *egv1a1.EnvoyPatchPolicy) + mutateStatus func(epp *egv1a1.EnvoyPatchPolicy) + wantErrors []string + }{ + { + desc: "valid gateway targetRef", + mutate: func(epp *egv1a1.EnvoyPatchPolicy) { + epp.Spec.TargetRef = &gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + } + }, + wantErrors: []string{}, + }, + { + desc: "valid gateway targetRefs", + mutate: func(epp *egv1a1.EnvoyPatchPolicy) { + epp.Spec.TargetRefs = []gwapiv1.LocalPolicyTargetReference{ + { + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + }, + } + }, + wantErrors: []string{}, + }, + { + desc: "both targetRef and targetRefs", + mutate: func(epp *egv1a1.EnvoyPatchPolicy) { + epp.Spec.TargetRef = &gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + } + epp.Spec.TargetRefs = []gwapiv1.LocalPolicyTargetReference{ + { + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + }, + } + }, + wantErrors: []string{ + "spec: Invalid value:", + "exactly one of targetRef or targetRefs must be set", + }, + }, + { + desc: "no targetRef or targetRefs", + mutate: func(epp *egv1a1.EnvoyPatchPolicy) { + // Don't set either field + }, + wantErrors: []string{ + "spec: Invalid value:", + "exactly one of targetRef or targetRefs must be set", + }, + }, + { + desc: "targetRef set but targetRefs empty array", + mutate: func(epp *egv1a1.EnvoyPatchPolicy) { + epp.Spec.TargetRef = &gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg"), + } + epp.Spec.TargetRefs = []gwapiv1.LocalPolicyTargetReference{} + }, + wantErrors: []string{}, + }, + { + desc: "targetRefs with multiple gateways", + mutate: func(epp *egv1a1.EnvoyPatchPolicy) { + epp.Spec.TargetRefs = []gwapiv1.LocalPolicyTargetReference{ + { + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg1"), + }, + { + Group: gwapiv1.Group("gateway.networking.k8s.io"), + Kind: gwapiv1.Kind("Gateway"), + Name: gwapiv1.ObjectName("eg2"), + }, + } + }, + wantErrors: []string{}, + }, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + epp := baseEPP.DeepCopy() + epp.Name = fmt.Sprintf("epp-%v", time.Now().UnixNano()) + + if tc.mutate != nil { + tc.mutate(epp) + } + err := c.Create(ctx, epp) + + if tc.mutateStatus != nil { + tc.mutateStatus(epp) + err = c.Status().Update(ctx, epp) + } + + if (len(tc.wantErrors) != 0) != (err != nil) { + t.Fatalf("Unexpected response while creating EnvoyPatchPolicy; got err=\n%v\n;want error=%v", err, tc.wantErrors) + } + + var missingErrorStrings []string + for _, wantError := range tc.wantErrors { + if !strings.Contains(strings.ToLower(err.Error()), strings.ToLower(wantError)) { + missingErrorStrings = append(missingErrorStrings, wantError) + } + } + + if len(missingErrorStrings) != 0 { + t.Errorf("Unexpected response while creating EnvoyPatchPolicy; got err=\n%v\n;missing strings within error=%q", err, missingErrorStrings) + } + }) + } +} diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index 70cadd8674..8448da8199 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -30468,6 +30468,8 @@ spec: This Policy and the TargetRef MUST be in the same namespace for this Policy to have effect and be applied to the Gateway TargetRef + + Deprecated: TargetRef is deprecated in favor of TargetRefs to support attaching to multiple resources. properties: group: description: Group is the group of the target resource. @@ -30490,6 +30492,45 @@ spec: - kind - name type: object + targetRefs: + description: |- + TargetRefs is the names of the Gateway API resources this policy + is being attached to. + By default, attaching to Gateway is supported and + when mergeGateways is enabled it should attach to GatewayClass. + This Policy and the TargetRef MUST be in the same namespace + for this Policy to have effect and be applied to the Gateway + TargetRefs. + items: + description: |- + LocalPolicyTargetReference identifies an API object to apply a direct or + inherited policy to. This should be used as part of Policy resources + that can target Gateway API resources. For more information on how this + policy attachment model works, and a sample Policy resource, refer to + the policy attachment documentation for Gateway API. + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + required: + - group + - kind + - name + type: object + type: array type: description: |- Type decides the type of patch. @@ -30498,9 +30539,12 @@ spec: - JSONPatch type: string required: - - targetRef - type type: object + x-kubernetes-validations: + - message: exactly one of targetRef or targetRefs must be set + rule: (has(self.targetRef) && size(self.targetRefs) == 0) || (!has(self.targetRef) + && size(self.targetRefs) > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index 8fb22aa6fc..029869568a 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -8441,6 +8441,8 @@ spec: This Policy and the TargetRef MUST be in the same namespace for this Policy to have effect and be applied to the Gateway TargetRef + + Deprecated: TargetRef is deprecated in favor of TargetRefs to support attaching to multiple resources. properties: group: description: Group is the group of the target resource. @@ -8463,6 +8465,45 @@ spec: - kind - name type: object + targetRefs: + description: |- + TargetRefs is the names of the Gateway API resources this policy + is being attached to. + By default, attaching to Gateway is supported and + when mergeGateways is enabled it should attach to GatewayClass. + This Policy and the TargetRef MUST be in the same namespace + for this Policy to have effect and be applied to the Gateway + TargetRefs. + items: + description: |- + LocalPolicyTargetReference identifies an API object to apply a direct or + inherited policy to. This should be used as part of Policy resources + that can target Gateway API resources. For more information on how this + policy attachment model works, and a sample Policy resource, refer to + the policy attachment documentation for Gateway API. + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + required: + - group + - kind + - name + type: object + type: array type: description: |- Type decides the type of patch. @@ -8471,9 +8512,12 @@ spec: - JSONPatch type: string required: - - targetRef - type type: object + x-kubernetes-validations: + - message: exactly one of targetRef or targetRefs must be set + rule: (has(self.targetRef) && size(self.targetRefs) == 0) || (!has(self.targetRef) + && size(self.targetRefs) > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: diff --git a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml index c3c76d601a..791eef8d0d 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -8441,6 +8441,8 @@ spec: This Policy and the TargetRef MUST be in the same namespace for this Policy to have effect and be applied to the Gateway TargetRef + + Deprecated: TargetRef is deprecated in favor of TargetRefs to support attaching to multiple resources. properties: group: description: Group is the group of the target resource. @@ -8463,6 +8465,45 @@ spec: - kind - name type: object + targetRefs: + description: |- + TargetRefs is the names of the Gateway API resources this policy + is being attached to. + By default, attaching to Gateway is supported and + when mergeGateways is enabled it should attach to GatewayClass. + This Policy and the TargetRef MUST be in the same namespace + for this Policy to have effect and be applied to the Gateway + TargetRefs. + items: + description: |- + LocalPolicyTargetReference identifies an API object to apply a direct or + inherited policy to. This should be used as part of Policy resources + that can target Gateway API resources. For more information on how this + policy attachment model works, and a sample Policy resource, refer to + the policy attachment documentation for Gateway API. + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + required: + - group + - kind + - name + type: object + type: array type: description: |- Type decides the type of patch. @@ -8471,9 +8512,12 @@ spec: - JSONPatch type: string required: - - targetRef - type type: object + x-kubernetes-validations: + - message: exactly one of targetRef or targetRefs must be set + rule: (has(self.targetRef) && size(self.targetRefs) == 0) || (!has(self.targetRef) + && size(self.targetRefs) > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: From 63d20b8b646efb2c5f2d4923b85c694ada23fc03 Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 10 Apr 2026 15:46:07 +0800 Subject: [PATCH 2/9] impl targetRefs for EnvoyPatchPolicy Signed-off-by: zirain --- internal/gatewayapi/envoypatchpolicy.go | 197 ++++++++++-------- .../envoypatchpolicy-cross-ns-target.out.yaml | 14 +- ...chpolicy-invalid-feature-disabled.out.yaml | 16 -- ...atchpolicy-invalid-merge-gateways.out.yaml | 15 +- ...nvalid-target-kind-merge-gateways.out.yaml | 5 +- ...oypatchpolicy-invalid-target-kind.out.yaml | 22 +- ...oypatchpolicy-invalid-target-name.out.yaml | 27 ++- ...icy-no-status-for-unknown-gateway.out.yaml | 14 +- ...oypatchpolicy-valid-merge-gateways.in.yaml | 8 +- ...ypatchpolicy-valid-merge-gateways.out.yaml | 10 +- .../testdata/envoypatchpolicy-valid.in.yaml | 23 +- .../testdata/envoypatchpolicy-valid.out.yaml | 192 +++++++++++++++-- test/cel-validation/envoypatchpolicy_test.go | 2 +- 13 files changed, 382 insertions(+), 163 deletions(-) diff --git a/internal/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go index 2858973944..41130e595d 100644 --- a/internal/gatewayapi/envoypatchpolicy.go +++ b/internal/gatewayapi/envoypatchpolicy.go @@ -19,35 +19,59 @@ import ( func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.EnvoyPatchPolicy, xdsIR resource.XdsIRMap) { // EnvoyPatchPolicies are already sorted by the provider layer (priority, then timestamp, then name) - for _, policy := range envoyPatchPolicies { - var ( - ancestorRef gwapiv1.ParentReference - resolveErr *status.PolicyResolveError - targetKind string - irKey string - gwXdsIR *ir.Xds - ok bool - ) - - refGroup, refKind, refName := policy.Spec.TargetRef.Group, policy.Spec.TargetRef.Kind, policy.Spec.TargetRef.Name - if t.MergeGateways { - targetKind = resource.KindGatewayClass - // if ref GatewayClass name is not same as t.GatewayClassName, it will be skipped in L74. - irKey = string(refName) - ancestorRef = gwapiv1.ParentReference{ - Group: GroupPtr(gwapiv1.GroupName), - Kind: KindPtr(targetKind), - Name: refName, + targetRefs := getTargetRefsForEPP(policy) + for _, targetRef := range targetRefs { + var ( + ancestorRef gwapiv1.ParentReference + resolveErr *status.PolicyResolveError + targetKind string + irKey string + gwXdsIR *ir.Xds + ok bool + ) + + refKind, refName := targetRef.Kind, targetRef.Name + if t.MergeGateways { + targetKind = resource.KindGatewayClass + // if ref GatewayClass name is not same as t.GatewayClassName, it will be skipped when checking XDS IR. + irKey = string(refName) + ancestorRef = gwapiv1.ParentReference{ + Group: GroupPtr(gwapiv1.GroupName), + Kind: KindPtr(targetKind), + Name: refName, + } + } else { + targetKind = resource.KindGateway + gatewayNN := types.NamespacedName{ + Namespace: policy.Namespace, + Name: string(refName), + } + irKey = t.IRKey(gatewayNN) + ancestorRef = getAncestorRefForPolicy(gatewayNN, nil) } - gwXdsIR, ok = xdsIR[irKey] - if !ok { - // The TargetRef GatewayClass is not an accepted GatewayClass, then skip processing. - message := fmt.Sprintf( - "TargetRef.Group:%s TargetRef.Kind:%s TargetRef.Namespace:%s TargetRef.Name:%s not found or not accepted (MergeGateways=%t).", - refGroup, refKind, policy.Namespace, string(refName), t.MergeGateways, + // Ensure EnvoyPatchPolicy is enabled + if !t.EnvoyPatchPolicyEnabled { + resolveErr = &status.PolicyResolveError{ + Reason: egv1a1.PolicyReasonDisabled, + Message: "EnvoyPatchPolicy is disabled in the EnvoyGateway configuration", + } + status.SetResolveErrorForPolicyAncestor(&policy.Status, + &ancestorRef, + t.GatewayControllerName, + policy.Generation, + resolveErr, ) + + continue + } + + // Ensure EnvoyPatchPolicy is targeting to a support type + if targetRef.Group != gwapiv1.GroupName || string(refKind) != targetKind { + message := fmt.Sprintf("Target to %s/%s, only %s/%s is supported.", + targetRef.Group, targetRef.Kind, gwapiv1.GroupName, targetKind) + resolveErr = &status.PolicyResolveError{ Reason: gwapiv1.PolicyReasonInvalid, Message: message, @@ -58,85 +82,74 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo policy.Generation, resolveErr, ) - continue - } - } else { - targetKind = resource.KindGateway - gatewayNN := types.NamespacedName{ - Namespace: policy.Namespace, - Name: string(refName), + continue } - irKey = t.IRKey(gatewayNN) - ancestorRef = getAncestorRefForPolicy(gatewayNN, nil) gwXdsIR, ok = xdsIR[irKey] if !ok { - // The TargetRef Gateway is not an accepted Gateway, then skip processing. + var message string + message = fmt.Sprintf("Target to %s %s/%s does not exist.", targetKind, policy.Namespace, refName) + // if mergeGateways is enabled, the TargetRef should be GatewayClass, otherwise it should be Gateway. + if string(refKind) != targetKind { + message = fmt.Sprintf("Target to %s, only %s is supported when MergeGateways is %t.", refKind, targetKind, t.MergeGateways) + } + + resolveErr = &status.PolicyResolveError{ + Reason: egv1a1.PolicyReasonInvalid, + Message: message, + } + status.SetResolveErrorForPolicyAncestor(&policy.Status, + &ancestorRef, + t.GatewayControllerName, + policy.Generation, + resolveErr, + ) continue } - } - // Create the IR with the context need to publish the status later - policyIR := ir.EnvoyPatchPolicy{} - policyIR.Name = policy.Name - policyIR.Namespace = policy.Namespace - policyIR.Generation = policy.Generation - policyIR.Status = &policy.Status - - // Append the IR - gwXdsIR.EnvoyPatchPolicies = append(gwXdsIR.EnvoyPatchPolicies, &policyIR) - - // Ensure EnvoyPatchPolicy is enabled - if !t.EnvoyPatchPolicyEnabled { - resolveErr = &status.PolicyResolveError{ - Reason: egv1a1.PolicyReasonDisabled, - Message: "EnvoyPatchPolicy is disabled in the EnvoyGateway configuration", + // Create the IR with the context need to publish the status later + policyIR := ir.EnvoyPatchPolicy{} + policyIR.Name = policy.Name + policyIR.Namespace = policy.Namespace + policyIR.Generation = policy.Generation + policyIR.Status = &policy.Status + + // Append the IR + gwXdsIR.EnvoyPatchPolicies = append(gwXdsIR.EnvoyPatchPolicies, &policyIR) + + // Save the patch + for _, patch := range policy.Spec.JSONPatches { + irPatch := ir.JSONPatchConfig{} + irPatch.Type = string(patch.Type) + irPatch.Name = patch.Name + irPatch.Operation.Op = ir.JSONPatchOp(patch.Operation.Op) + irPatch.Operation.Path = patch.Operation.Path + irPatch.Operation.JSONPath = patch.Operation.JSONPath + irPatch.Operation.From = patch.Operation.From + irPatch.Operation.Value = patch.Operation.Value + + policyIR.JSONPatches = append(policyIR.JSONPatches, &irPatch) } - status.SetResolveErrorForPolicyAncestor(&policy.Status, - &ancestorRef, - t.GatewayControllerName, - policy.Generation, - resolveErr, - ) - - continue - } - - // Ensure EnvoyPatchPolicy is targeting to a support type - if refGroup != gwapiv1.GroupName || string(refKind) != targetKind { - message := fmt.Sprintf("TargetRef.Group:%s TargetRef.Kind:%s, only TargetRef.Group:%s and TargetRef.Kind:%s is supported.", - refGroup, policy.Spec.TargetRef.Kind, gwapiv1.GroupName, targetKind) - resolveErr = &status.PolicyResolveError{ - Reason: gwapiv1.PolicyReasonInvalid, - Message: message, + // Set Accepted=True + status.SetAcceptedForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, policy.Generation) + // If there are no valid TargetRefs, add a warning condition to the policy status. + if len(policy.Spec.TargetRefs) == 0 { + status.SetDeprecatedFieldsWarningForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, policy.Generation, + map[string]string{ + "spec.targetRef": "spec.targetRefs", + }) } - status.SetResolveErrorForPolicyAncestor(&policy.Status, - &ancestorRef, - t.GatewayControllerName, - policy.Generation, - resolveErr, - ) - - continue - } - - // Save the patch - for _, patch := range policy.Spec.JSONPatches { - irPatch := ir.JSONPatchConfig{} - irPatch.Type = string(patch.Type) - irPatch.Name = patch.Name - irPatch.Operation.Op = ir.JSONPatchOp(patch.Operation.Op) - irPatch.Operation.Path = patch.Operation.Path - irPatch.Operation.JSONPath = patch.Operation.JSONPath - irPatch.Operation.From = patch.Operation.From - irPatch.Operation.Value = patch.Operation.Value - - policyIR.JSONPatches = append(policyIR.JSONPatches, &irPatch) } + } +} - // Set Accepted=True - status.SetAcceptedForPolicyAncestor(&policy.Status, &ancestorRef, t.GatewayControllerName, policy.Generation) +// getTargetRefsForEPP returns the target refs for the given EnvoyPatchPolicy, handling both the deprecated TargetRef and the new TargetRefs fields. +// There's CEL validation to ensure that only one of TargetRef or TargetRefs is set, so we can safely return the non-nil field. +func getTargetRefsForEPP(policy *egv1a1.EnvoyPatchPolicy) []gwapiv1.LocalPolicyTargetReference { + if policy.Spec.TargetRef != nil { + return []gwapiv1.LocalPolicyTargetReference{*policy.Spec.TargetRef} } + return policy.Spec.TargetRefs } diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml index 9060e7ec66..c8d82ce45a 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml @@ -18,7 +18,19 @@ envoyPatchPolicies: name: gateway-1 type: JSONPatch status: - ancestors: null + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway-2 + conditions: + - lastTransitionTime: null + message: Target to Gateway envoy-gateway-2/gateway-1 does not exist. + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller gateways: - apiVersion: gateway.networking.k8s.io/v1 kind: Gateway diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml index b12d409eaa..f7ad6349a4 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml @@ -103,22 +103,6 @@ xdsIR: accessLog: json: - path: /dev/stdout - envoyPatchPolicies: - - name: edit-conn-buffer-bytes - namespace: envoy-gateway - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: GatewayClass - name: envoy-gateway-class - conditions: - - lastTransitionTime: null - message: EnvoyPatchPolicy is disabled in the EnvoyGateway configuration - reason: Disabled - status: "False" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: metadata: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml index 0822c1734b..74536156b1 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml @@ -25,9 +25,8 @@ envoyPatchPolicies: name: gateway-class-not-exists conditions: - lastTransitionTime: null - message: TargetRef.Group:gateway.networking.k8s.io TargetRef.Kind:GatewayClass - TargetRef.Namespace:envoy-gateway TargetRef.Name:gateway-class-not-exists - not found or not accepted (MergeGateways=true). + message: Target to GatewayClass envoy-gateway/gateway-class-not-exists does + not exist. reason: Invalid status: "False" type: Accepted @@ -63,6 +62,11 @@ envoyPatchPolicies: reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + reason: DeprecatedField + status: "True" + type: Warning controllerName: gateway.envoyproxy.io/gatewayclass-controller gateways: - apiVersion: gateway.networking.k8s.io/v1 @@ -159,6 +163,11 @@ xdsIR: reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + reason: DeprecatedField + status: "True" + type: Warning controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml index 56512ad219..ed90e18644 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml @@ -25,9 +25,8 @@ envoyPatchPolicies: name: gateway-1 conditions: - lastTransitionTime: null - message: TargetRef.Group:gateway.networking.k8s.io TargetRef.Kind:Gateway - TargetRef.Namespace:envoy-gateway TargetRef.Name:gateway-1 not found or - not accepted (MergeGateways=true). + message: Target to gateway.networking.k8s.io/Gateway, only gateway.networking.k8s.io/GatewayClass + is supported. reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml index 56ff1ce50f..fca78f02e7 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml @@ -26,8 +26,7 @@ envoyPatchPolicies: namespace: envoy-gateway conditions: - lastTransitionTime: null - message: TargetRef.Group:gateway.networking.k8s.io TargetRef.Kind:MyGateway, - only TargetRef.Group:gateway.networking.k8s.io and TargetRef.Kind:Gateway + message: Target to gateway.networking.k8s.io/MyGateway, only gateway.networking.k8s.io/Gateway is supported. reason: Invalid status: "False" @@ -97,25 +96,6 @@ xdsIR: accessLog: json: - path: /dev/stdout - envoyPatchPolicies: - - name: edit-conn-buffer-bytes - namespace: envoy-gateway - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: TargetRef.Group:gateway.networking.k8s.io TargetRef.Kind:MyGateway, - only TargetRef.Group:gateway.networking.k8s.io and TargetRef.Kind:Gateway - is supported. - reason: Invalid - status: "False" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: metadata: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml index 7944bff175..9909fcb8b4 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml @@ -19,7 +19,20 @@ envoyPatchPolicies: name: gateway-not-exists type: JSONPatch status: - ancestors: null + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-not-exists + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Target to Gateway envoy-gateway/gateway-not-exists does not exist. + observedGeneration: 10 + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: EnvoyPatchPolicy metadata: @@ -54,6 +67,12 @@ envoyPatchPolicies: reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + observedGeneration: 10 + reason: DeprecatedField + status: "True" + type: Warning controllerName: gateway.envoyproxy.io/gatewayclass-controller gateways: - apiVersion: gateway.networking.k8s.io/v1 @@ -144,6 +163,12 @@ xdsIR: reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + observedGeneration: 10 + reason: DeprecatedField + status: "True" + type: Warning controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-no-status-for-unknown-gateway.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-no-status-for-unknown-gateway.out.yaml index 2d295a28a9..f35e4f9f36 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-no-status-for-unknown-gateway.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-no-status-for-unknown-gateway.out.yaml @@ -11,6 +11,18 @@ envoyPatchPolicies: name: unknown-gateway type: JSONPatch status: - ancestors: null + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: unknown-gateway + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Target to Gateway envoy-gateway/unknown-gateway does not exist. + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller infraIR: {} xdsIR: {} diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.in.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.in.yaml index 5a1d480305..d616ec6682 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.in.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.in.yaml @@ -14,8 +14,8 @@ envoyPatchPolicies: name: edit-conn-buffer-bytes spec: type: "JSONPatch" - targetRef: - group: gateway.networking.k8s.io + targetRefs: + - group: gateway.networking.k8s.io kind: GatewayClass name: envoy-gateway-class jsonPatches: @@ -32,8 +32,8 @@ envoyPatchPolicies: name: edit-ignore-global-limit spec: type: "JSONPatch" - targetRef: - group: gateway.networking.k8s.io + targetRefs: + - group: gateway.networking.k8s.io kind: GatewayClass name: envoy-gateway-class # Higher priority diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml index b66fc89770..7a6af1e067 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml @@ -12,8 +12,9 @@ envoyPatchPolicies: path: /per_connection_buffer_limit_bytes value: "1024" type: type.googleapis.com/envoy.config.listener.v3.Listener - targetRef: - group: gateway.networking.k8s.io + targetRef: null + targetRefs: + - group: gateway.networking.k8s.io kind: GatewayClass name: envoy-gateway-class type: JSONPatch @@ -44,8 +45,9 @@ envoyPatchPolicies: value: "true" type: type.googleapis.com/envoy.config.listener.v3.Listener priority: -1 - targetRef: - group: gateway.networking.k8s.io + targetRef: null + targetRefs: + - group: gateway.networking.k8s.io kind: GatewayClass name: envoy-gateway-class type: JSONPatch diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-valid.in.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-valid.in.yaml index c03b69d15c..ee3ff0cb5b 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-valid.in.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-valid.in.yaml @@ -4,7 +4,6 @@ envoyPatchPolicies: metadata: namespace: envoy-gateway name: edit-conn-buffer-bytes - generation: 10 spec: type: "JSONPatch" targetRef: @@ -23,13 +22,15 @@ envoyPatchPolicies: metadata: namespace: envoy-gateway name: edit-ignore-global-limit - generation: 10 spec: type: "JSONPatch" - targetRef: - group: gateway.networking.k8s.io + targetRefs: + - group: gateway.networking.k8s.io kind: Gateway name: gateway-1 + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 # Higher priority priority: -1 jsonPatches: @@ -54,3 +55,17 @@ gateways: allowedRoutes: namespaces: from: Same +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-2 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: Same diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml index 6f3f290ea2..1be40100b6 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml @@ -2,7 +2,6 @@ envoyPatchPolicies: - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: EnvoyPatchPolicy metadata: - generation: 10 name: edit-conn-buffer-bytes namespace: envoy-gateway spec: @@ -28,15 +27,18 @@ envoyPatchPolicies: conditions: - lastTransitionTime: null message: Policy has been accepted. - observedGeneration: 10 reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + reason: DeprecatedField + status: "True" + type: Warning controllerName: gateway.envoyproxy.io/gatewayclass-controller - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: EnvoyPatchPolicy metadata: - generation: 10 name: edit-ignore-global-limit namespace: envoy-gateway spec: @@ -48,10 +50,14 @@ envoyPatchPolicies: value: "true" type: type.googleapis.com/envoy.config.listener.v3.Listener priority: -1 - targetRef: - group: gateway.networking.k8s.io + targetRef: null + targetRefs: + - group: gateway.networking.k8s.io kind: Gateway name: gateway-1 + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 type: JSONPatch status: ancestors: @@ -63,7 +69,18 @@ envoyPatchPolicies: conditions: - lastTransitionTime: null message: Policy has been accepted. - observedGeneration: 10 + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. reason: Accepted status: "True" type: Accepted @@ -108,6 +125,45 @@ gateways: kind: HTTPRoute - group: gateway.networking.k8s.io kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-2 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: Same + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 0 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute infraIR: envoy-gateway/gateway-1: proxy: @@ -127,14 +183,31 @@ infraIR: name: envoy-gateway-class name: envoy-gateway/gateway-1 namespace: envoy-gateway-system + envoy-gateway/gateway-2: + proxy: + listeners: + - name: envoy-gateway/gateway-2/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-2 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-2 + namespace: envoy-gateway-system xdsIR: envoy-gateway/gateway-1: accessLog: json: - path: /dev/stdout envoyPatchPolicies: - - generation: 10 - jsonPatches: + - jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace @@ -153,13 +226,16 @@ xdsIR: conditions: - lastTransitionTime: null message: Policy has been accepted. - observedGeneration: 10 reason: Accepted status: "True" type: Accepted + - lastTransitionTime: null + message: spec.targetRef is deprecated, use spec.targetRefs instead + reason: DeprecatedField + status: "True" + type: Warning controllerName: gateway.envoyproxy.io/gatewayclass-controller - - generation: 10 - jsonPatches: + - jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace @@ -178,7 +254,18 @@ xdsIR: conditions: - lastTransitionTime: null message: Policy has been accepted. - observedGeneration: 10 + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. reason: Accepted status: "True" type: Accepted @@ -224,3 +311,84 @@ xdsIR: ipFamily: IPv4 path: /ready port: 19003 + envoy-gateway/gateway-2: + accessLog: + json: + - path: /dev/stdout + envoyPatchPolicies: + - jsonPatches: + - name: envoy-gateway-gateway-1-http + operation: + op: replace + path: /ignore_global_conn_limit + value: "true" + type: type.googleapis.com/envoy.config.listener.v3.Listener + name: edit-ignore-global-limit + namespace: envoy-gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-2-4a0e4eb9 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-2 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-2-4a0e4eb9 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-2 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-2/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/test/cel-validation/envoypatchpolicy_test.go b/test/cel-validation/envoypatchpolicy_test.go index 7d568fe802..bf0fafb921 100644 --- a/test/cel-validation/envoypatchpolicy_test.go +++ b/test/cel-validation/envoypatchpolicy_test.go @@ -98,7 +98,7 @@ func TestEnvoyPatchPolicyTarget(t *testing.T) { }, { desc: "no targetRef or targetRefs", - mutate: func(epp *egv1a1.EnvoyPatchPolicy) { + mutate: func(_ *egv1a1.EnvoyPatchPolicy) { // Don't set either field }, wantErrors: []string{ From 79d5b8a0e54b0114d058a127967a78395d7ea443 Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 10 Apr 2026 19:56:37 +0800 Subject: [PATCH 3/9] fix gen Signed-off-by: zirain --- test/helm/gateway-crds-helm/all.out.yaml | 5 +++-- test/helm/gateway-crds-helm/e2e.out.yaml | 5 +++-- test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index 8448da8199..e90f84039b 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -30543,8 +30543,9 @@ spec: type: object x-kubernetes-validations: - message: exactly one of targetRef or targetRefs must be set - rule: (has(self.targetRef) && size(self.targetRefs) == 0) || (!has(self.targetRef) - && size(self.targetRefs) > 0) + rule: (has(self.targetRef) && (!has(self.targetRefs) || size(self.targetRefs) + == 0)) || (!has(self.targetRef) && has(self.targetRefs) && size(self.targetRefs) + > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index 029869568a..034d9265e9 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -8516,8 +8516,9 @@ spec: type: object x-kubernetes-validations: - message: exactly one of targetRef or targetRefs must be set - rule: (has(self.targetRef) && size(self.targetRefs) == 0) || (!has(self.targetRef) - && size(self.targetRefs) > 0) + rule: (has(self.targetRef) && (!has(self.targetRefs) || size(self.targetRefs) + == 0)) || (!has(self.targetRef) && has(self.targetRefs) && size(self.targetRefs) + > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: diff --git a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml index 791eef8d0d..006ba18ebf 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -8516,8 +8516,9 @@ spec: type: object x-kubernetes-validations: - message: exactly one of targetRef or targetRefs must be set - rule: (has(self.targetRef) && size(self.targetRefs) == 0) || (!has(self.targetRef) - && size(self.targetRefs) > 0) + rule: (has(self.targetRef) && (!has(self.targetRefs) || size(self.targetRefs) + == 0)) || (!has(self.targetRef) && has(self.targetRefs) && size(self.targetRefs) + > 0) status: description: Status defines the current status of EnvoyPatchPolicy. properties: From 61fca991ee8b80d6e018cfb79617ec362396aa68 Mon Sep 17 00:00:00 2001 From: zirain Date: Fri, 10 Apr 2026 21:41:21 +0800 Subject: [PATCH 4/9] update e2e Signed-off-by: zirain --- test/e2e/testdata/envoy-patch-policy.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/testdata/envoy-patch-policy.yaml b/test/e2e/testdata/envoy-patch-policy.yaml index ede3800d45..89fb6934d8 100644 --- a/test/e2e/testdata/envoy-patch-policy.yaml +++ b/test/e2e/testdata/envoy-patch-policy.yaml @@ -22,8 +22,8 @@ metadata: name: custom-response-patch-policy namespace: gateway-conformance-infra spec: - targetRef: - group: gateway.networking.k8s.io + targetRefs: + - group: gateway.networking.k8s.io kind: Gateway name: same-namespace type: JSONPatch From d59d36b88ac2f16a0bcf159eaf1ddd26f2956725 Mon Sep 17 00:00:00 2001 From: zirain Date: Mon, 13 Apr 2026 15:04:33 +0800 Subject: [PATCH 5/9] fix EPP status Signed-off-by: zirain --- internal/gatewayapi/envoypatchpolicy.go | 22 +++++++++ .../gatewayapi/status/envoypatchpolicy.go | 45 +++++++++++++++++++ .../envoypatchpolicy-cross-ns-target.out.yaml | 17 +++++++ ...chpolicy-invalid-feature-disabled.out.yaml | 16 +++++++ ...atchpolicy-invalid-merge-gateways.out.yaml | 22 ++++++++- ...nvalid-target-kind-merge-gateways.out.yaml | 17 +++++++ ...oypatchpolicy-invalid-target-kind.out.yaml | 18 ++++++++ ...oypatchpolicy-invalid-target-name.out.yaml | 23 ++++++++++ ...ypatchpolicy-valid-merge-gateways.out.yaml | 12 ++++- .../testdata/envoypatchpolicy-valid.out.yaml | 21 +++++++-- internal/ir/xds.go | 3 ++ internal/ir/zz_generated.deepcopy.go | 5 +++ internal/xds/runner/runner.go | 3 ++ internal/xds/translator/jsonpatch.go | 18 ++++++-- .../jsonpatch-add-op-empty-jsonpath.yaml | 7 ++- .../jsonpatch-add-op-without-value.yaml | 7 ++- .../in/xds-ir/jsonpatch-invalid-listener.yaml | 7 ++- .../in/xds-ir/jsonpatch-invalid-patch.yaml | 7 ++- .../in/xds-ir/jsonpatch-missing-resource.yaml | 7 ++- .../xds-ir/jsonpatch-move-op-with-value.yaml | 7 ++- .../jsonpatch-with-jsonpath-invalid.yaml | 7 ++- .../in/xds-ir/jsonpatch-with-jsonpath.yaml | 7 ++- .../testdata/in/xds-ir/jsonpatch.yaml | 13 +++++- ...-op-empty-jsonpath.envoypatchpolicies.yaml | 7 ++- ...d-op-without-value.envoypatchpolicies.yaml | 7 ++- ...atch-invalid-patch.envoypatchpolicies.yaml | 7 ++- ...h-missing-resource.envoypatchpolicies.yaml | 7 ++- ...move-op-with-value.envoypatchpolicies.yaml | 7 ++- ...h-jsonpath-invalid.envoypatchpolicies.yaml | 7 ++- ...atch-with-jsonpath.envoypatchpolicies.yaml | 7 ++- .../xds-ir/jsonpatch.envoypatchpolicies.yaml | 13 +++++- 31 files changed, 347 insertions(+), 26 deletions(-) diff --git a/internal/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go index 41130e595d..ee109bd41f 100644 --- a/internal/gatewayapi/envoypatchpolicy.go +++ b/internal/gatewayapi/envoypatchpolicy.go @@ -21,6 +21,8 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo // EnvoyPatchPolicies are already sorted by the provider layer (priority, then timestamp, then name) for _, policy := range envoyPatchPolicies { targetRefs := getTargetRefsForEPP(policy) + createdAnyPolicyIR := false + for _, targetRef := range targetRefs { var ( ancestorRef gwapiv1.ParentReference @@ -114,9 +116,11 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo policyIR.Namespace = policy.Namespace policyIR.Generation = policy.Generation policyIR.Status = &policy.Status + policyIR.AncestorRef = &ancestorRef // Append the IR gwXdsIR.EnvoyPatchPolicies = append(gwXdsIR.EnvoyPatchPolicies, &policyIR) + createdAnyPolicyIR = true // Save the patch for _, patch := range policy.Spec.JSONPatches { @@ -142,6 +146,24 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo }) } } + + // If no policyIR was created (all targets were rejected), but the policy has status + // (from rejected targets), create a status-only policyIR to ensure status gets published. + // Attach it to the first available XdsIR. + if !createdAnyPolicyIR && len(policy.Status.Ancestors) > 0 { + for _, gwXdsIR := range xdsIR { + policyIR := ir.EnvoyPatchPolicy{} + policyIR.Name = policy.Name + policyIR.Namespace = policy.Namespace + policyIR.Generation = policy.Generation + policyIR.Status = &policy.Status + // No AncestorRef since this is a status-only entry for all rejected targets + policyIR.AncestorRef = nil + + gwXdsIR.EnvoyPatchPolicies = append(gwXdsIR.EnvoyPatchPolicies, &policyIR) + break + } + } } } diff --git a/internal/gatewayapi/status/envoypatchpolicy.go b/internal/gatewayapi/status/envoypatchpolicy.go index 585ef9567b..4e96354e3d 100644 --- a/internal/gatewayapi/status/envoypatchpolicy.go +++ b/internal/gatewayapi/status/envoypatchpolicy.go @@ -35,6 +35,28 @@ func SetProgrammedForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus, generation int64) } } +// SetProgrammedForEnvoyPatchPolicyAncestor sets programmed condition for a specific ancestor reference if it is unset. +func SetProgrammedForEnvoyPatchPolicyAncestor(s *gwapiv1.PolicyStatus, ancestorRef *gwapiv1.ParentReference, generation int64) { + // Return early if Programmed condition is already set for this ancestor + for i, ancestor := range s.Ancestors { + if ancestorRefsEqual(&ancestor.AncestorRef, ancestorRef) { + for _, c := range ancestor.Conditions { + if c.Type == string(egv1a1.PolicyConditionProgrammed) { + return + } + if c.Type == string(gwapiv1.PolicyConditionAccepted) && c.Status == metav1.ConditionFalse { + return + } + } + + message := "Patches have been successfully applied." + cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionTrue, string(egv1a1.PolicyReasonProgrammed), message, generation) + s.Ancestors[i].Conditions = MergeConditions(s.Ancestors[i].Conditions, cond) + return + } + } +} + func SetTranslationErrorForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus, errMsg string, generation int64) { cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonInvalid), errMsg, generation) for i := range s.Ancestors { @@ -42,6 +64,17 @@ func SetTranslationErrorForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus, errMsg stri } } +// SetTranslationErrorForEnvoyPatchPolicyAncestor sets translation error for a specific ancestor reference. +func SetTranslationErrorForEnvoyPatchPolicyAncestor(s *gwapiv1.PolicyStatus, ancestorRef *gwapiv1.ParentReference, errMsg string, generation int64) { + cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonInvalid), errMsg, generation) + for i := range s.Ancestors { + if ancestorRefsEqual(&s.Ancestors[i].AncestorRef, ancestorRef) { + s.Ancestors[i].Conditions = MergeConditions(s.Ancestors[i].Conditions, cond) + return + } + } +} + func SetResourceNotFoundErrorForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus, notFoundResources []string, generation int64) { message := "Unable to find xds resources: " + strings.Join(notFoundResources, ",") cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonResourceNotFound), message, generation) @@ -49,3 +82,15 @@ func SetResourceNotFoundErrorForEnvoyPatchPolicy(s *gwapiv1.PolicyStatus, notFou s.Ancestors[i].Conditions = MergeConditions(s.Ancestors[i].Conditions, cond) } } + +// SetResourceNotFoundErrorForEnvoyPatchPolicyAncestor sets resource not found error for a specific ancestor reference. +func SetResourceNotFoundErrorForEnvoyPatchPolicyAncestor(s *gwapiv1.PolicyStatus, ancestorRef *gwapiv1.ParentReference, notFoundResources []string, generation int64) { + message := "Unable to find xds resources: " + strings.Join(notFoundResources, ",") + cond := newCondition(string(egv1a1.PolicyConditionProgrammed), metav1.ConditionFalse, string(egv1a1.PolicyReasonResourceNotFound), message, generation) + for i := range s.Ancestors { + if ancestorRefsEqual(&s.Ancestors[i].AncestorRef, ancestorRef) { + s.Ancestors[i].Conditions = MergeConditions(s.Ancestors[i].Conditions, cond) + return + } + } +} diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml index c8d82ce45a..f2569ce597 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-cross-ns-target.out.yaml @@ -95,6 +95,23 @@ xdsIR: accessLog: json: - path: /dev/stdout + envoyPatchPolicies: + - name: edit-conn-buffer-bytes + namespace: envoy-gateway-2 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway-2 + conditions: + - lastTransitionTime: null + message: Target to Gateway envoy-gateway-2/gateway-1 does not exist. + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: metadata: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml index f7ad6349a4..b12d409eaa 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-feature-disabled.out.yaml @@ -103,6 +103,22 @@ xdsIR: accessLog: json: - path: /dev/stdout + envoyPatchPolicies: + - name: edit-conn-buffer-bytes + namespace: envoy-gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: envoy-gateway-class + conditions: + - lastTransitionTime: null + message: EnvoyPatchPolicy is disabled in the EnvoyGateway configuration + reason: Disabled + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: metadata: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml index 74536156b1..005f5fd739 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-merge-gateways.out.yaml @@ -142,7 +142,27 @@ xdsIR: json: - path: /dev/stdout envoyPatchPolicies: - - jsonPatches: + - name: gateway-class-not-exists + namespace: envoy-gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: gateway-class-not-exists + conditions: + - lastTransitionTime: null + message: Target to GatewayClass envoy-gateway/gateway-class-not-exists + does not exist. + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: envoy-gateway-class + jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml index ed90e18644..ed3010434d 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml @@ -104,6 +104,23 @@ xdsIR: accessLog: json: - path: /dev/stdout + envoyPatchPolicies: + - name: edit-conn-buffer-bytes + namespace: envoy-gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: gateway-1 + conditions: + - lastTransitionTime: null + message: Target to gateway.networking.k8s.io/Gateway, only gateway.networking.k8s.io/GatewayClass + is supported. + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: metadata: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml index fca78f02e7..e093b70754 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml @@ -96,6 +96,24 @@ xdsIR: accessLog: json: - path: /dev/stdout + envoyPatchPolicies: + - name: edit-conn-buffer-bytes + namespace: envoy-gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Target to gateway.networking.k8s.io/MyGateway, only gateway.networking.k8s.io/Gateway + is supported. + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller globalResources: proxyServiceCluster: metadata: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml index 9909fcb8b4..3e93fd9f49 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-name.out.yaml @@ -140,6 +140,29 @@ xdsIR: - path: /dev/stdout envoyPatchPolicies: - generation: 10 + name: gateway-not-exists + namespace: envoy-gateway + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-not-exists + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Target to Gateway envoy-gateway/gateway-not-exists does not exist. + observedGeneration: 10 + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + generation: 10 jsonPatches: - name: envoy-gateway-gateway-1-http operation: diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml index 7a6af1e067..9f5fe1ff11 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-valid-merge-gateways.out.yaml @@ -138,7 +138,11 @@ xdsIR: json: - path: /dev/stdout envoyPatchPolicies: - - jsonPatches: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: envoy-gateway-class + jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace @@ -160,7 +164,11 @@ xdsIR: status: "True" type: Accepted controllerName: gateway.envoyproxy.io/gatewayclass-controller - - jsonPatches: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: envoy-gateway-class + jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml index 1be40100b6..5d2137ca62 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-valid.out.yaml @@ -207,7 +207,12 @@ xdsIR: json: - path: /dev/stdout envoyPatchPolicies: - - jsonPatches: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace @@ -235,7 +240,12 @@ xdsIR: status: "True" type: Warning controllerName: gateway.envoyproxy.io/gatewayclass-controller - - jsonPatches: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace @@ -316,7 +326,12 @@ xdsIR: json: - path: /dev/stdout envoyPatchPolicies: - - jsonPatches: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + jsonPatches: - name: envoy-gateway-gateway-1-http operation: op: replace diff --git a/internal/ir/xds.go b/internal/ir/xds.go index f45d3c0cb0..60566d2cad 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -2688,6 +2688,9 @@ type EnvoyPatchPolicyStatus struct { Generation int64 `json:"generation,omitempty" yaml:"generation"` // Status of the EnvoyPatchPolicy Status *gwapiv1.PolicyStatus `json:"status,omitempty" yaml:"status,omitempty"` + // AncestorRef is the specific ancestor that this policyIR entry corresponds to. + // This is used to ensure that xDS translation errors only affect the status of this specific target. + AncestorRef *gwapiv1.ParentReference `json:"ancestorRef,omitempty" yaml:"ancestorRef,omitempty"` } // JSONPatchConfig defines the configuration for patching a Envoy xDS Resource diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 5beedef87e..070038baa2 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -1387,6 +1387,11 @@ func (in *EnvoyPatchPolicyStatus) DeepCopyInto(out *EnvoyPatchPolicyStatus) { *out = new(v1.PolicyStatus) (*in).DeepCopyInto(*out) } + if in.AncestorRef != nil { + in, out := &in.AncestorRef, &out.AncestorRef + *out = new(v1.ParentReference) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EnvoyPatchPolicyStatus. diff --git a/internal/xds/runner/runner.go b/internal/xds/runner/runner.go index e1ffc4d42d..0a0ee0bfb8 100644 --- a/internal/xds/runner/runner.go +++ b/internal/xds/runner/runner.go @@ -35,6 +35,7 @@ import ( "github.com/envoyproxy/gateway/internal/crypto" "github.com/envoyproxy/gateway/internal/envoygateway/config" extension "github.com/envoyproxy/gateway/internal/extension/types" + "github.com/envoyproxy/gateway/internal/gatewayapi/status" "github.com/envoyproxy/gateway/internal/infrastructure/host" "github.com/envoyproxy/gateway/internal/infrastructure/kubernetes/ratelimit" "github.com/envoyproxy/gateway/internal/message" @@ -371,6 +372,8 @@ func (r *Runner) translateFromSubscription(sub <-chan watchable.Snapshot[string, // They may have been skipped in this translation because // their target is not found (not relevant) if len(e.Status.Ancestors) > 0 { + // Truncate status.ancestors to max 16 entries before storing + status.TruncatePolicyAncestors(e.Status, r.EnvoyGateway.Gateway.ControllerName, e.Generation) r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status) } delete(statusesToDelete, key) diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index 9848c1e012..6aac341b6a 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -156,17 +156,29 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, envoyPatchPolicies []* // Set translation errors for every policy ancestor references if tErrs != nil { - status.SetTranslationErrorForEnvoyPatchPolicy(e.Status, status.Error2ConditionMsg(tErrs), e.Generation) + if e.AncestorRef != nil { + status.SetTranslationErrorForEnvoyPatchPolicyAncestor(e.Status, e.AncestorRef, status.Error2ConditionMsg(tErrs), e.Generation) + } else { + status.SetTranslationErrorForEnvoyPatchPolicy(e.Status, status.Error2ConditionMsg(tErrs), e.Generation) + } errs = errors.Join(errs, tErrs) } // Set resources not found errors for every policy ancestor references if len(notFoundResources) > 0 { - status.SetResourceNotFoundErrorForEnvoyPatchPolicy(e.Status, notFoundResources, e.Generation) + if e.AncestorRef != nil { + status.SetResourceNotFoundErrorForEnvoyPatchPolicyAncestor(e.Status, e.AncestorRef, notFoundResources, e.Generation) + } else { + status.SetResourceNotFoundErrorForEnvoyPatchPolicy(e.Status, notFoundResources, e.Generation) + } } // Set Programmed condition if not yet set - status.SetProgrammedForEnvoyPatchPolicy(e.Status, e.Generation) + if e.AncestorRef != nil { + status.SetProgrammedForEnvoyPatchPolicyAncestor(e.Status, e.AncestorRef, e.Generation) + } else { + status.SetProgrammedForEnvoyPatchPolicy(e.Status, e.Generation) + } // Set output context tCtx.EnvoyPatchPolicyStatuses = append(tCtx.EnvoyPatchPolicyStatuses, &e.EnvoyPatchPolicyStatus) diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-empty-jsonpath.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-empty-jsonpath.yaml index 0a94f0a630..d8a52ffc89 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-empty-jsonpath.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-empty-jsonpath.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-without-value.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-without-value.yaml index 43eeaa082c..e578e1f81e 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-without-value.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-add-op-without-value.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-listener.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-listener.yaml index f4d967332a..c3936d292f 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-listener.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-listener.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-patch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-patch.yaml index 13a0929f10..30a70f9ff9 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-patch.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-invalid-patch.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-missing-resource.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-missing-resource.yaml index 26f4e802f3..6d3e029741 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-missing-resource.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-missing-resource.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-move-op-with-value.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-move-op-with-value.yaml index fb49a07ec2..5ab663f5a0 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-move-op-with-value.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-move-op-with-value.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath-invalid.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath-invalid.yaml index e0abd3ef66..a07b7e13f7 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath-invalid.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath-invalid.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath.yaml index 91b20a57c8..874089e796 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch-with-jsonpath.yaml @@ -1,5 +1,10 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml index 21f2cdd59d..d1bb451821 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml @@ -1,11 +1,22 @@ envoyPatchPolicies: -- status: +- ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "foobar" + status: ancestors: - ancestorRef: group: "gateway.networking.k8s.io" kind: "Gateway" namespace: "default" name: "foobar" + # This ancestor will be ignored since it's not the current ancestorRef. + - ancestorRef: + group: "gateway.networking.k8s.io" + kind: "Gateway" + namespace: "default" + name: "another-gateway" name: "first-policy" namespace: "default" generation: 1 diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-empty-jsonpath.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-empty-jsonpath.envoypatchpolicies.yaml index 89bb973c44..250061a15a 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-empty-jsonpath.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-empty-jsonpath.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-without-value.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-without-value.envoypatchpolicies.yaml index e5bfbcb5c8..a4923fe955 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-without-value.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-add-op-without-value.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-invalid-patch.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-invalid-patch.envoypatchpolicies.yaml index e63b3f6329..9dcbc4d5a1 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-invalid-patch.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-invalid-patch.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-missing-resource.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-missing-resource.envoypatchpolicies.yaml index 1020b285eb..37e0d84a22 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-missing-resource.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-missing-resource.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-move-op-with-value.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-move-op-with-value.envoypatchpolicies.yaml index 6274de31a5..c59d7bea2c 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-move-op-with-value.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-move-op-with-value.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath-invalid.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath-invalid.envoypatchpolicies.yaml index 2efc2d9c52..8f56dcd740 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath-invalid.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath-invalid.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath.envoypatchpolicies.yaml index bb7f32c91b..51409a6119 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch-with-jsonpath.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.envoypatchpolicies.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.envoypatchpolicies.yaml index bb7f32c91b..14695b39b8 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.envoypatchpolicies.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.envoypatchpolicies.yaml @@ -1,4 +1,9 @@ -- generation: 1 +- ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: foobar + namespace: default + generation: 1 name: first-policy namespace: default status: @@ -16,3 +21,9 @@ status: "True" type: Programmed controllerName: "" + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: another-gateway + namespace: default + controllerName: "" From f7983f50a444a0dfdfeabb639269bb43f3e6de31 Mon Sep 17 00:00:00 2001 From: zirain Date: Tue, 14 Apr 2026 19:42:21 +0800 Subject: [PATCH 6/9] fix codex review comments Signed-off-by: zirain --- internal/gatewayapi/envoypatchpolicy.go | 36 ++++++++- internal/xds/runner/runner.go | 80 +++++++++++++++++++- test/cel-validation/envoypatchpolicy_test.go | 3 +- 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/internal/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go index ee109bd41f..5c57a27653 100644 --- a/internal/gatewayapi/envoypatchpolicy.go +++ b/internal/gatewayapi/envoypatchpolicy.go @@ -169,9 +169,41 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo // getTargetRefsForEPP returns the target refs for the given EnvoyPatchPolicy, handling both the deprecated TargetRef and the new TargetRefs fields. // There's CEL validation to ensure that only one of TargetRef or TargetRefs is set, so we can safely return the non-nil field. +// Duplicates are removed to prevent creating multiple IR entries for the same target. func getTargetRefsForEPP(policy *egv1a1.EnvoyPatchPolicy) []gwapiv1.LocalPolicyTargetReference { + var refs []gwapiv1.LocalPolicyTargetReference if policy.Spec.TargetRef != nil { - return []gwapiv1.LocalPolicyTargetReference{*policy.Spec.TargetRef} + refs = []gwapiv1.LocalPolicyTargetReference{*policy.Spec.TargetRef} + } else { + refs = policy.Spec.TargetRefs } - return policy.Spec.TargetRefs + return deduplicateTargetRefs(refs) +} + +// deduplicateTargetRefs removes duplicate LocalPolicyTargetReference entries based on Group, Kind, and Name. +func deduplicateTargetRefs(refs []gwapiv1.LocalPolicyTargetReference) []gwapiv1.LocalPolicyTargetReference { + if len(refs) <= 1 { + return refs + } + + seen := make(map[string]bool) + deduplicated := make([]gwapiv1.LocalPolicyTargetReference, 0, len(refs)) + + for _, ref := range refs { + // Create a unique key from the reference fields + group := "" + if ref.Group != "" { + group = string(ref.Group) + } + kind := string(ref.Kind) + name := string(ref.Name) + key := fmt.Sprintf("%s/%s/%s", group, kind, name) + + if !seen[key] { + seen[key] = true + deduplicated = append(deduplicated, ref) + } + } + + return deduplicated } diff --git a/internal/xds/runner/runner.go b/internal/xds/runner/runner.go index 0a0ee0bfb8..ce2cbbecf7 100644 --- a/internal/xds/runner/runner.go +++ b/internal/xds/runner/runner.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/keepalive" ktypes "k8s.io/apimachinery/pkg/types" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/crypto" @@ -258,6 +259,36 @@ func registerServer(srv serverv3.Server, g *grpc.Server) { runtimev3.RegisterRuntimeDiscoveryServiceServer(g, srv) } +// ancestorRefKey generates a unique key for an ancestor reference. +// This is used to identify and merge ancestor status updates. +func ancestorRefKey(ref *gwapiv1.ParentReference) string { + if ref == nil { + return "" + } + + group := "" + if ref.Group != nil { + group = string(*ref.Group) + } + + kind := "" + if ref.Kind != nil { + kind = string(*ref.Kind) + } + + namespace := "" + if ref.Namespace != nil { + namespace = string(*ref.Namespace) + } + + sectionName := "" + if ref.SectionName != nil { + sectionName = string(*ref.SectionName) + } + + return fmt.Sprintf("%s/%s/%s/%s/%s", group, kind, namespace, ref.Name, sectionName) +} + func (r *Runner) translateFromSubscription(sub <-chan watchable.Snapshot[string, *message.XdsIRWithContext]) { // Subscribe to resources message.HandleSubscription( @@ -363,6 +394,7 @@ func (r *Runner) translateFromSubscription(sub <-chan watchable.Snapshot[string, } // Publish EnvoyPatchPolicyStatus + // Merge ancestor conditions from multiple translation runs for the same policy for _, e := range result.EnvoyPatchPolicyStatuses { key := ktypes.NamespacedName{ Name: e.Name, @@ -372,9 +404,51 @@ func (r *Runner) translateFromSubscription(sub <-chan watchable.Snapshot[string, // They may have been skipped in this translation because // their target is not found (not relevant) if len(e.Status.Ancestors) > 0 { - // Truncate status.ancestors to max 16 entries before storing - status.TruncatePolicyAncestors(e.Status, r.EnvoyGateway.Gateway.ControllerName, e.Generation) - r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status) + // Load existing status if it exists + existingStatus, exists := r.ProviderResources.EnvoyPatchPolicyStatuses.Load(key) + + if exists && existingStatus != nil { + // Merge ancestors: update existing ancestors or add new ones + mergedAncestors := make([]gwapiv1.PolicyAncestorStatus, 0, len(existingStatus.Ancestors)+len(e.Status.Ancestors)) + + // Create a map of new ancestors by reference for quick lookup + newAncestorMap := make(map[string]*gwapiv1.PolicyAncestorStatus) + for i := range e.Status.Ancestors { + refKey := ancestorRefKey(&e.Status.Ancestors[i].AncestorRef) + newAncestorMap[refKey] = &e.Status.Ancestors[i] + } + + // Update existing ancestors or keep them if not in new status + for i := range existingStatus.Ancestors { + refKey := ancestorRefKey(&existingStatus.Ancestors[i].AncestorRef) + if newAncestor, found := newAncestorMap[refKey]; found { + // Replace with updated ancestor + mergedAncestors = append(mergedAncestors, *newAncestor) + delete(newAncestorMap, refKey) + } else { + // Keep existing ancestor that wasn't updated + mergedAncestors = append(mergedAncestors, existingStatus.Ancestors[i]) + } + } + + // Add remaining new ancestors that weren't in existing status + for _, newAncestor := range newAncestorMap { + mergedAncestors = append(mergedAncestors, *newAncestor) + } + + // Create merged status + mergedStatus := &gwapiv1.PolicyStatus{ + Ancestors: mergedAncestors, + } + // Truncate merged status.ancestors to max 16 entries before storing + status.TruncatePolicyAncestors(mergedStatus, r.EnvoyGateway.Gateway.ControllerName, e.Generation) + r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, mergedStatus) + } else { + // No existing status, store the new one + // Truncate status.ancestors to max 16 entries before storing + status.TruncatePolicyAncestors(e.Status, r.EnvoyGateway.Gateway.ControllerName, e.Generation) + r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status) + } } delete(statusesToDelete, key) } diff --git a/test/cel-validation/envoypatchpolicy_test.go b/test/cel-validation/envoypatchpolicy_test.go index bf0fafb921..19bfa011e6 100644 --- a/test/cel-validation/envoypatchpolicy_test.go +++ b/test/cel-validation/envoypatchpolicy_test.go @@ -16,7 +16,6 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -37,7 +36,7 @@ func TestEnvoyPatchPolicyTarget(t *testing.T) { Name: "test-listener", Operation: egv1a1.JSONPatchOperation{ Op: "add", - Path: ptr.To("/foo"), + Path: new("/foo"), Value: &apiextensionsv1.JSON{Raw: []byte(`"bar"`)}, }, }, From ad7571a5395d470cb01ef7c8c246c4da52d12f47 Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 16 Apr 2026 11:58:01 +0800 Subject: [PATCH 7/9] update e2e Signed-off-by: zirain --- test/e2e/testdata/envoy-patch-policy.yaml | 21 +++++++++++++++++++ test/e2e/tests/envoy_patch_policy.go | 12 ++++++----- .../envoy_patch_policy_xds_name_scheme_v2.go | 2 +- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/test/e2e/testdata/envoy-patch-policy.yaml b/test/e2e/testdata/envoy-patch-policy.yaml index 89fb6934d8..2edb34b7f0 100644 --- a/test/e2e/testdata/envoy-patch-policy.yaml +++ b/test/e2e/testdata/envoy-patch-policy.yaml @@ -7,6 +7,7 @@ metadata: spec: parentRefs: - name: same-namespace + - name: all-namespaces rules: - backendRefs: - name: infra-backend-v1 @@ -26,8 +27,28 @@ spec: - group: gateway.networking.k8s.io kind: Gateway name: same-namespace + - group: gateway.networking.k8s.io + kind: Gateway + name: all-namespaces type: JSONPatch jsonPatches: + - type: "type.googleapis.com/envoy.config.listener.v3.Listener" + name: "gateway-conformance-infra/all-namespaces/http" + operation: + op: add + path: "/default_filter_chain/filters/0/typed_config/local_reply_config" + value: + mappers: + - filter: + status_code_filter: + comparison: + op: EQ + value: + default_value: 404 + runtime_key: key_b + status_code: 406 + body: + inline_string: "not acceptable" - type: "type.googleapis.com/envoy.config.listener.v3.Listener" name: "gateway-conformance-infra/same-namespace/http" operation: diff --git a/test/e2e/tests/envoy_patch_policy.go b/test/e2e/tests/envoy_patch_policy.go index 1d8affb1b5..2350068528 100644 --- a/test/e2e/tests/envoy_patch_policy.go +++ b/test/e2e/tests/envoy_patch_policy.go @@ -26,16 +26,18 @@ var EnvoyPatchPolicyTest = suite.ConformanceTest{ Description: "update xds using EnvoyPatchPolicy", Manifests: []string{"testdata/envoy-patch-policy.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { - t.Run("envoy patch policy", func(t *testing.T) { - testEnvoyPatchPolicy(t, suite) - }) + for _, gtw := range []string{"same-namespace", "all-namespaces"} { + t.Run(gtw, func(t *testing.T) { + testEnvoyPatchPolicy(t, suite, gtw) + }) + } }, } -func testEnvoyPatchPolicy(t *testing.T, suite *suite.ConformanceTestSuite) { +func testEnvoyPatchPolicy(t *testing.T, suite *suite.ConformanceTestSuite, gtwName string) { ns := "gateway-conformance-infra" routeNN := types.NamespacedName{Name: "http-envoy-patch-policy", Namespace: ns} - gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns} + gwNN := types.NamespacedName{Name: gtwName, Namespace: ns} gwAddr := kubernetes.GatewayAndRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), &gwapiv1.HTTPRoute{}, false, routeNN) OkResp := http.ExpectedResponse{ Request: http.Request{ diff --git a/test/e2e/tests/envoy_patch_policy_xds_name_scheme_v2.go b/test/e2e/tests/envoy_patch_policy_xds_name_scheme_v2.go index b7c8ce47d5..41eb35cd0c 100644 --- a/test/e2e/tests/envoy_patch_policy_xds_name_scheme_v2.go +++ b/test/e2e/tests/envoy_patch_policy_xds_name_scheme_v2.go @@ -23,7 +23,7 @@ var EnvoyPatchPolicyXDSNameSchemeV2Test = suite.ConformanceTest{ Manifests: []string{"testdata/envoy-patch-policy-xds-name-scheme-v2.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { t.Run("envoy patch policy", func(t *testing.T) { - testEnvoyPatchPolicy(t, suite) + testEnvoyPatchPolicy(t, suite, "same-namespace") }) }, } From 5c2d13cbb51470f5aedc7885fcc62405d345035c Mon Sep 17 00:00:00 2001 From: zirain Date: Sun, 19 Apr 2026 21:17:56 +0800 Subject: [PATCH 8/9] fix Signed-off-by: zirain --- internal/gatewayapi/envoypatchpolicy.go | 33 +++++-- internal/gatewayapi/envoypatchpolicy_test.go | 96 +++++++++++++++++++ ...nvalid-target-kind-merge-gateways.out.yaml | 4 +- ...oypatchpolicy-invalid-target-kind.out.yaml | 6 +- internal/xds/runner/runner.go | 84 ++++++++++------ internal/xds/runner/runner_test.go | 63 ++++++++++++ 6 files changed, 246 insertions(+), 40 deletions(-) create mode 100644 internal/gatewayapi/envoypatchpolicy_test.go diff --git a/internal/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go index 5c57a27653..554f7b9ec3 100644 --- a/internal/gatewayapi/envoypatchpolicy.go +++ b/internal/gatewayapi/envoypatchpolicy.go @@ -31,26 +31,22 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo irKey string gwXdsIR *ir.Xds ok bool + gatewayNN types.NamespacedName ) refKind, refName := targetRef.Kind, targetRef.Name + ancestorRef = getAncestorRefForEnvoyPatchPolicyTargetRef(targetRef) if t.MergeGateways { targetKind = resource.KindGatewayClass // if ref GatewayClass name is not same as t.GatewayClassName, it will be skipped when checking XDS IR. irKey = string(refName) - ancestorRef = gwapiv1.ParentReference{ - Group: GroupPtr(gwapiv1.GroupName), - Kind: KindPtr(targetKind), - Name: refName, - } } else { targetKind = resource.KindGateway - gatewayNN := types.NamespacedName{ + gatewayNN = types.NamespacedName{ Namespace: policy.Namespace, Name: string(refName), } irKey = t.IRKey(gatewayNN) - ancestorRef = getAncestorRefForPolicy(gatewayNN, nil) } // Ensure EnvoyPatchPolicy is enabled @@ -88,6 +84,16 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo continue } + if t.MergeGateways { + ancestorRef = gwapiv1.ParentReference{ + Group: GroupPtr(gwapiv1.GroupName), + Kind: KindPtr(targetKind), + Name: refName, + } + } else { + ancestorRef = getAncestorRefForPolicy(gatewayNN, nil) + } + gwXdsIR, ok = xdsIR[irKey] if !ok { var message string @@ -167,6 +173,19 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo } } +func getAncestorRefForEnvoyPatchPolicyTargetRef(targetRef gwapiv1.LocalPolicyTargetReference) gwapiv1.ParentReference { + ancestorRef := gwapiv1.ParentReference{ + Kind: KindPtr(string(targetRef.Kind)), + Name: targetRef.Name, + } + + if targetRef.Group != "" { + ancestorRef.Group = GroupPtr(string(targetRef.Group)) + } + + return ancestorRef +} + // getTargetRefsForEPP returns the target refs for the given EnvoyPatchPolicy, handling both the deprecated TargetRef and the new TargetRefs fields. // There's CEL validation to ensure that only one of TargetRef or TargetRefs is set, so we can safely return the non-nil field. // Duplicates are removed to prevent creating multiple IR entries for the same target. diff --git a/internal/gatewayapi/envoypatchpolicy_test.go b/internal/gatewayapi/envoypatchpolicy_test.go new file mode 100644 index 0000000000..339a390c7c --- /dev/null +++ b/internal/gatewayapi/envoypatchpolicy_test.go @@ -0,0 +1,96 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package gatewayapi + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/gatewayapi/resource" + "github.com/envoyproxy/gateway/internal/ir" +) + +func TestProcessEnvoyPatchPolicies_NonMergedInvalidAndValidSameNameDoNotCollide(t *testing.T) { + translator := &Translator{ + MergeGateways: false, + EnvoyPatchPolicyEnabled: true, + GatewayControllerName: "envoy-gateway-test-controller", + } + + gatewayNN := types.NamespacedName{Namespace: "default", Name: "shared-gw"} + xdsIR := resource.XdsIRMap{ + translator.IRKey(gatewayNN): &ir.Xds{}, + } + + policy := &egv1a1.EnvoyPatchPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "epp", + Namespace: "default", + Generation: 1, + }, + Spec: egv1a1.EnvoyPatchPolicySpec{ + Type: egv1a1.JSONPatchEnvoyPatchType, + TargetRefs: []gwapiv1.LocalPolicyTargetReference{ + // Invalid kind with the same name as the valid Gateway target. + {Group: gwapiv1.GroupName, Kind: gwapiv1.Kind(resource.KindGatewayClass), Name: gwapiv1.ObjectName("shared-gw")}, + {Group: gwapiv1.GroupName, Kind: gwapiv1.Kind(resource.KindGateway), Name: gwapiv1.ObjectName("shared-gw")}, + }, + }, + } + + translator.ProcessEnvoyPatchPolicies([]*egv1a1.EnvoyPatchPolicy{policy}, xdsIR) + + require.Len(t, policy.Status.Ancestors, 2) + // Only the valid gateway target should produce an attachable IR entry. + require.Len(t, xdsIR[translator.IRKey(gatewayNN)].EnvoyPatchPolicies, 1) + + var invalidAncestor, validAncestor *gwapiv1.PolicyAncestorStatus + for i := range policy.Status.Ancestors { + ancestor := &policy.Status.Ancestors[i] + switch { + case ancestor.AncestorRef.Kind != nil && *ancestor.AncestorRef.Kind == resource.KindGatewayClass: + invalidAncestor = ancestor + case ancestor.AncestorRef.Kind != nil && *ancestor.AncestorRef.Kind == resource.KindGateway: + validAncestor = ancestor + } + } + + require.NotNil(t, invalidAncestor) + require.NotNil(t, validAncestor) + + invalidAccepted := meta.FindStatusCondition(invalidAncestor.Conditions, string(gwapiv1.PolicyConditionAccepted)) + validAccepted := meta.FindStatusCondition(validAncestor.Conditions, string(gwapiv1.PolicyConditionAccepted)) + require.NotNil(t, invalidAccepted) + require.NotNil(t, validAccepted) + + assert.Equal(t, metav1.ConditionFalse, invalidAccepted.Status) + assert.Equal(t, metav1.ConditionTrue, validAccepted.Status) + // Valid Gateway ancestor should remain normalized with namespace in non-merged mode. + require.NotNil(t, validAncestor.AncestorRef.Namespace) + assert.Equal(t, gwapiv1.Namespace("default"), *validAncestor.AncestorRef.Namespace) +} + +func TestGetAncestorRefForEnvoyPatchPolicyTargetRef_UsesTargetFields(t *testing.T) { + ref := gwapiv1.LocalPolicyTargetReference{ + Group: gwapiv1.Group("example.io"), + Kind: gwapiv1.Kind("ExampleKind"), + Name: gwapiv1.ObjectName("example-name"), + } + + ancestor := getAncestorRefForEnvoyPatchPolicyTargetRef(ref) + require.NotNil(t, ancestor.Group) + require.NotNil(t, ancestor.Kind) + assert.Equal(t, gwapiv1.Group("example.io"), *ancestor.Group) + assert.Equal(t, gwapiv1.Kind("ExampleKind"), *ancestor.Kind) + assert.Equal(t, gwapiv1.ObjectName("example-name"), ancestor.Name) +} diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml index ed3010434d..a00e316b0c 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind-merge-gateways.out.yaml @@ -21,7 +21,7 @@ envoyPatchPolicies: ancestors: - ancestorRef: group: gateway.networking.k8s.io - kind: GatewayClass + kind: Gateway name: gateway-1 conditions: - lastTransitionTime: null @@ -111,7 +111,7 @@ xdsIR: ancestors: - ancestorRef: group: gateway.networking.k8s.io - kind: GatewayClass + kind: Gateway name: gateway-1 conditions: - lastTransitionTime: null diff --git a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml index e093b70754..111d03fb0d 100644 --- a/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml +++ b/internal/gatewayapi/testdata/envoypatchpolicy-invalid-target-kind.out.yaml @@ -21,9 +21,8 @@ envoyPatchPolicies: ancestors: - ancestorRef: group: gateway.networking.k8s.io - kind: Gateway + kind: MyGateway name: gateway-1 - namespace: envoy-gateway conditions: - lastTransitionTime: null message: Target to gateway.networking.k8s.io/MyGateway, only gateway.networking.k8s.io/Gateway @@ -103,9 +102,8 @@ xdsIR: ancestors: - ancestorRef: group: gateway.networking.k8s.io - kind: Gateway + kind: MyGateway name: gateway-1 - namespace: envoy-gateway conditions: - lastTransitionTime: null message: Target to gateway.networking.k8s.io/MyGateway, only gateway.networking.k8s.io/Gateway diff --git a/internal/xds/runner/runner.go b/internal/xds/runner/runner.go index ce2cbbecf7..2a3d489b67 100644 --- a/internal/xds/runner/runner.go +++ b/internal/xds/runner/runner.go @@ -289,6 +289,62 @@ func ancestorRefKey(ref *gwapiv1.ParentReference) string { return fmt.Sprintf("%s/%s/%s/%s/%s", group, kind, namespace, ref.Name, sectionName) } +func ancestorObservedGeneration(ancestor *gwapiv1.PolicyAncestorStatus) int64 { + if ancestor == nil { + return 0 + } + + var maxObservedGeneration int64 + for _, condition := range ancestor.Conditions { + if condition.ObservedGeneration > maxObservedGeneration { + maxObservedGeneration = condition.ObservedGeneration + } + } + + return maxObservedGeneration +} + +func mergePolicyAncestors(existing, translated []gwapiv1.PolicyAncestorStatus, generation int64) []gwapiv1.PolicyAncestorStatus { + if len(existing) == 0 { + return append([]gwapiv1.PolicyAncestorStatus(nil), translated...) + } + + mergedAncestors := make([]gwapiv1.PolicyAncestorStatus, 0, len(existing)+len(translated)) + + // Index translated ancestors by reference to update existing entries in place. + translatedByRef := make(map[string]gwapiv1.PolicyAncestorStatus, len(translated)) + for i := range translated { + refKey := ancestorRefKey(&translated[i].AncestorRef) + translatedByRef[refKey] = translated[i] + } + + // Keep translated replacements first; only retain missing existing entries when + // they are from the same generation to support aggregation across translation runs. + for i := range existing { + refKey := ancestorRefKey(&existing[i].AncestorRef) + if translatedAncestor, found := translatedByRef[refKey]; found { + mergedAncestors = append(mergedAncestors, translatedAncestor) + delete(translatedByRef, refKey) + continue + } + + if ancestorObservedGeneration(&existing[i]) == generation { + mergedAncestors = append(mergedAncestors, existing[i]) + } + } + + // Preserve translated ordering for newly introduced ancestors. + for i := range translated { + refKey := ancestorRefKey(&translated[i].AncestorRef) + if translatedAncestor, found := translatedByRef[refKey]; found { + mergedAncestors = append(mergedAncestors, translatedAncestor) + delete(translatedByRef, refKey) + } + } + + return mergedAncestors +} + func (r *Runner) translateFromSubscription(sub <-chan watchable.Snapshot[string, *message.XdsIRWithContext]) { // Subscribe to resources message.HandleSubscription( @@ -408,33 +464,7 @@ func (r *Runner) translateFromSubscription(sub <-chan watchable.Snapshot[string, existingStatus, exists := r.ProviderResources.EnvoyPatchPolicyStatuses.Load(key) if exists && existingStatus != nil { - // Merge ancestors: update existing ancestors or add new ones - mergedAncestors := make([]gwapiv1.PolicyAncestorStatus, 0, len(existingStatus.Ancestors)+len(e.Status.Ancestors)) - - // Create a map of new ancestors by reference for quick lookup - newAncestorMap := make(map[string]*gwapiv1.PolicyAncestorStatus) - for i := range e.Status.Ancestors { - refKey := ancestorRefKey(&e.Status.Ancestors[i].AncestorRef) - newAncestorMap[refKey] = &e.Status.Ancestors[i] - } - - // Update existing ancestors or keep them if not in new status - for i := range existingStatus.Ancestors { - refKey := ancestorRefKey(&existingStatus.Ancestors[i].AncestorRef) - if newAncestor, found := newAncestorMap[refKey]; found { - // Replace with updated ancestor - mergedAncestors = append(mergedAncestors, *newAncestor) - delete(newAncestorMap, refKey) - } else { - // Keep existing ancestor that wasn't updated - mergedAncestors = append(mergedAncestors, existingStatus.Ancestors[i]) - } - } - - // Add remaining new ancestors that weren't in existing status - for _, newAncestor := range newAncestorMap { - mergedAncestors = append(mergedAncestors, *newAncestor) - } + mergedAncestors := mergePolicyAncestors(existingStatus.Ancestors, e.Status.Ancestors, e.Generation) // Create merged status mergedStatus := &gwapiv1.PolicyStatus{ diff --git a/internal/xds/runner/runner_test.go b/internal/xds/runner/runner_test.go index a3afb438ee..625c37602b 100644 --- a/internal/xds/runner/runner_test.go +++ b/internal/xds/runner/runner_test.go @@ -26,7 +26,9 @@ import ( "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/crypto" @@ -626,3 +628,64 @@ func TestLoadTLSConfig_HostMode(t *testing.T) { require.Equal(t, tls.RequireAndVerifyClientCert, tlsConfig.ClientAuth) require.Equal(t, uint16(tls.VersionTLS13), tlsConfig.MinVersion) } + +func TestMergePolicyAncestors_PrunesStaleAncestorsOnGenerationChange(t *testing.T) { + ancestor := func(name string, generation int64) gwapiv1.PolicyAncestorStatus { + return gwapiv1.PolicyAncestorStatus{ + AncestorRef: gwapiv1.ParentReference{Name: gwapiv1.ObjectName(name)}, + Conditions: []metav1.Condition{{ + Type: string(gwapiv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + Reason: string(gwapiv1.PolicyReasonAccepted), + Message: "ok", + }}, + } + } + + existing := []gwapiv1.PolicyAncestorStatus{ + ancestor("gw-a", 1), + ancestor("gw-b", 1), + } + translated := []gwapiv1.PolicyAncestorStatus{ + ancestor("gw-a", 2), + } + + merged := mergePolicyAncestors(existing, translated, 2) + require.Len(t, merged, 1) + assert.Equal(t, gwapiv1.ObjectName("gw-a"), merged[0].AncestorRef.Name) + assert.Equal(t, int64(2), merged[0].Conditions[0].ObservedGeneration) +} + +func TestMergePolicyAncestors_RetainsSameGenerationForAggregation(t *testing.T) { + ancestor := func(name string, generation int64) gwapiv1.PolicyAncestorStatus { + return gwapiv1.PolicyAncestorStatus{ + AncestorRef: gwapiv1.ParentReference{Name: gwapiv1.ObjectName(name)}, + Conditions: []metav1.Condition{{ + Type: string(gwapiv1.PolicyConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + Reason: string(gwapiv1.PolicyReasonAccepted), + Message: "ok", + }}, + } + } + + existing := []gwapiv1.PolicyAncestorStatus{ + ancestor("gw-b", 2), + } + translated := []gwapiv1.PolicyAncestorStatus{ + ancestor("gw-a", 2), + } + + merged := mergePolicyAncestors(existing, translated, 2) + require.Len(t, merged, 2) + + actual := map[gwapiv1.ObjectName]int64{} + for _, ancestor := range merged { + actual[ancestor.AncestorRef.Name] = ancestor.Conditions[0].ObservedGeneration + } + + assert.Equal(t, int64(2), actual[gwapiv1.ObjectName("gw-a")]) + assert.Equal(t, int64(2), actual[gwapiv1.ObjectName("gw-b")]) +} From 30364329a658b964e1d3ef23da69092a3c4d4acc Mon Sep 17 00:00:00 2001 From: zirain Date: Mon, 20 Apr 2026 10:41:01 +0800 Subject: [PATCH 9/9] add e2e for stale status cleanup Signed-off-by: zirain --- test/e2e/tests/envoy_patch_policy.go | 59 ++++++++++++++++++++++++++++ test/e2e/tests/utils.go | 42 ++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/test/e2e/tests/envoy_patch_policy.go b/test/e2e/tests/envoy_patch_policy.go index 2350068528..5bdfecfd23 100644 --- a/test/e2e/tests/envoy_patch_policy.go +++ b/test/e2e/tests/envoy_patch_policy.go @@ -8,9 +8,13 @@ package tests import ( + "context" "testing" + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/conformance/utils/http" "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" @@ -31,6 +35,61 @@ var EnvoyPatchPolicyTest = suite.ConformanceTest{ testEnvoyPatchPolicy(t, suite, gtw) }) } + + t.Run("Stale status must be cleaned up", func(t *testing.T) { + eppNN := types.NamespacedName{Name: "custom-response-patch-policy", Namespace: "gateway-conformance-infra"} + for _, gtw := range []string{"same-namespace", "all-namespaces"} { + EnvoyPatchPolicyMustBeAccepted(t, suite, + eppNN, + suite.ControllerName, gwapiv1.ParentReference{ + Name: gwapiv1.ObjectName(gtw), + Namespace: new(gwapiv1.Namespace("gateway-conformance-infra")), + Kind: new(gwapiv1.Kind("Gateway")), + Group: new(gwapiv1.Group("gateway.networking.k8s.io")), + }) + } + + // let's remove same-namespace from targetRefs and check if the status is updated accordingly + err := wait.PollUntilContextTimeout(t.Context(), suite.TimeoutConfig.DefaultPollInterval, suite.TimeoutConfig.MaxTimeToConsistency, true, func(_ context.Context) (done bool, err error) { + epp := &egv1a1.EnvoyPatchPolicy{} + if err := suite.Client.Get(t.Context(), eppNN, epp); err != nil { + return false, err + } + + epp.Spec.TargetRefs = []gwapiv1.LocalPolicyTargetReference{ + { + Group: "gateway.networking.k8s.io", + Kind: "Gateway", + Name: "same-namespace", + }, + } + + if err := suite.Client.Update(t.Context(), epp); err != nil { + return false, err + } + return true, nil + }) + require.NoErrorf(t, err, "failed to update EnvoyPatchPolicy to remove all-namespaces from targetRefs: %v", err) + + // check if the status is updated and there is no stale status for all-namespaces + EnvoyPatchPolicyMustBeAccepted(t, suite, + eppNN, + suite.ControllerName, gwapiv1.ParentReference{ + Name: "same-namespace", + Namespace: new(gwapiv1.Namespace("gateway-conformance-infra")), + Kind: new(gwapiv1.Kind("Gateway")), + Group: new(gwapiv1.Group("gateway.networking.k8s.io")), + }) + + EnvoyPatchPolicyMustNotHaveAncestor(t, suite, + eppNN, + suite.ControllerName, gwapiv1.ParentReference{ + Name: "all-namespaces", + Namespace: new(gwapiv1.Namespace("gateway-conformance-infra")), + Kind: new(gwapiv1.Kind("Gateway")), + Group: new(gwapiv1.Group("gateway.networking.k8s.io")), + }) + }) }, } diff --git a/test/e2e/tests/utils.go b/test/e2e/tests/utils.go index 834a160ccf..a0adccb143 100644 --- a/test/e2e/tests/utils.go +++ b/test/e2e/tests/utils.go @@ -224,6 +224,48 @@ func SecurityPolicyMustBeMerged(t *testing.T, client client.Client, policyName t require.NoErrorf(t, waitErr, "error waiting for SecurityPolicy to have Merged condition") } +func EnvoyPatchPolicyMustBeAccepted(t *testing.T, suite *suite.ConformanceTestSuite, policyName types.NamespacedName, controllerName string, ancestorRef gwapiv1.ParentReference) { + t.Helper() + waitErr := wait.PollUntilContextTimeout(t.Context(), suite.TimeoutConfig.DefaultPollInterval, suite.TimeoutConfig.MaxTimeToConsistency, true, func(ctx context.Context) (bool, error) { + policy := &egv1a1.EnvoyPatchPolicy{} + err := suite.Client.Get(ctx, policyName, policy) + if err != nil { + return false, fmt.Errorf("error fetching EnvoyPatchPolicy: %w", err) + } + if policyAcceptedByAncestor(policy.Status.Ancestors, controllerName, ancestorRef) { + tlog.Logf(t, "EnvoyPatchPolicy has been accepted: %v", policy) + return true, nil + } + + return false, nil + }) + require.NoErrorf(t, waitErr, "error waiting for EnvoyPatchPolicy to be accepted") +} + +// EnvoyPatchPolicyMustNotHaveAncestor waits for the specified EnvoyPatchPolicy to not be accepted by the ancestor. +func EnvoyPatchPolicyMustNotHaveAncestor(t *testing.T, suite *suite.ConformanceTestSuite, policyName types.NamespacedName, controllerName string, ancestorRef gwapiv1.ParentReference) { + t.Helper() + waitErr := wait.PollUntilContextTimeout(t.Context(), suite.TimeoutConfig.DefaultPollInterval, suite.TimeoutConfig.MaxTimeToConsistency, true, func(ctx context.Context) (bool, error) { + policy := &egv1a1.EnvoyPatchPolicy{} + err := suite.Client.Get(ctx, policyName, policy) + if err != nil { + return false, fmt.Errorf("error fetching EnvoyPatchPolicy: %w", err) + } + for _, ancestor := range policy.Status.Ancestors { + if string(ancestor.ControllerName) != controllerName { + continue + } + + if cmp.Equal(ancestor.AncestorRef, ancestorRef) { + return false, nil + } + } + tlog.Logf(t, "EnvoyPatchPolicy is not accepted by the ancestor yet: %v", policy) + return true, nil + }) + require.NoErrorf(t, waitErr, "error waiting for EnvoyPatchPolicy to not be accepted") +} + // BackendTrafficPolicyMustBeAccepted waits for the specified BackendTrafficPolicy to be accepted. func BackendTrafficPolicyMustBeAccepted(t *testing.T, client client.Client, policyName types.NamespacedName, controllerName string, ancestorRef gwapiv1.ParentReference) { t.Helper()