fix: Validation of XListenerSet certificateRefs by krishicks · Pull Request #8168 · envoyproxy/gateway

krishicks · 2026-02-03T17:53:14Z

What this PR does / why we need it:

Previously, validateTerminateModeAndGetTLSSecrets would always use the namespace of the listener's gateway when verifying a cross-namespace ref.

This meant that if the listener were from an XListenerSet, whether or not the Secret associated with the certificateRef was in the same namespace as the XListenerSet, it would not be permitted.

Additionally, and relatedly, this fixes an issue where an XListenerSet could reference a Secret in the gateway's namespace without a ReferenceGrant being present.

With this change we add a new GetNamespace() method to gatewayapi.ListenerContext which returns the listener's gateway's namespace for a listener added directly to the gateway, or the XListenerSet's namespace otherwise. This is similar to some of the other methods that were added to ListenerContext in support of XListenerSets.

The new method is used when creating the crossNamespaceFrom to determine if the certificateRef is permitted. If the Secret and XListenerSet are in the same namespace, it is permitted. If that is not the case a ReferenceGrant from the XListenerSet to the Secret will be properly searched for.

Release Notes: Yes

netlify · 2026-02-03T17:53:21Z

✅ Deploy Preview for cerulean-figolla-1f9435 canceled.

Name	Link
🔨 Latest commit	`8190fbb`
🔍 Latest deploy log	https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/698394c4bf70c5000867e340

codecov · 2026-02-03T18:33:59Z

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.79%. Comparing base (bbd7cde) to head (8190fbb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/gatewayapi/validate.go	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8168      +/-   ##
==========================================
+ Coverage   73.68%   73.79%   +0.10%     
==========================================
  Files         241      241              
  Lines       36569    36579      +10     
==========================================
+ Hits        26946    26992      +46     
+ Misses       7713     7681      -32     
+ Partials     1910     1906       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

krishicks · 2026-02-03T21:17:46Z

It's possible this solution (ed: the solution was updated based on this comment) isn't the right one given ReferenceGrant Semantics, which seems to imply the ReferenceGrant is not necessary when the ListenerSet references a secret in its own namespace:

ReferenceGrant Semantics: - ReferenceGrants applied to a Gateway are not inherited by child ListenerSets - ReferenceGrants applied to a ListenerSet do not grant permission to the parent Gateway's listeners - A ListenerSet can reference secrets/backends in its own namespace without a ReferenceGrant

If that's the case, I think the appropriate fix would be to skip the processSecretRef call entirely iff the secret namespace matches the XListenerSet namespace. However, it is possible for an XListenerSet to refer to a Secret in a different namespace than itself, in which the original code would possibly be valid. I say possibly because even now when I do add the ReferenceGrant for the XListenerSet as the code prior to this PR expects, the Gateway fails to resolve the secret refs as it expects a ReferenceGrant from it (meaning the Gateway) to the Secret.

krishicks · 2026-02-03T21:45:24Z

There was also a similar issue in kgateway. That was fixed by using the listener's parent's namespace rather than the gateway's namespace.

I think the same fix would apply here given this line in internal/gatewayapi/validate.go:

https://github.com/envoyproxy/gateway/blob/main/internal/gatewayapi/validate.go#L399

if certificateRef.Namespace != nil && string(*certificateRef.Namespace) != "" && string(*certificateRef.Namespace) != listener.gateway.Namespace {

If the listener is an XListenerSet it should use the XListenerSet's namespace, not the gateway's namespace. That would allow an XListenerSet to refer to a secret in the same namespace as it.

I think updating this code would also fix another issue I discovered, which is that an XListenerSet in one namespace can presently reference a secret that exists the Gateway's different namespace without a ReferenceGrant present, but it should not. This is because the listener's Gateway's namespace is used in the comparison instead of the namespace of the XListenerSet.

krishicks · 2026-02-04T02:17:16Z

I updated the PR based on my comments above, and am marking it as ready for review.

zhaohuabing · 2026-02-04T03:16:23Z

@krishicks Thanks for taking care of this. This PR looks good!

Could we also move the secrets to another namespace in these two tests and add ReferenceGrants to cover the cross-ns secret references use case?

https://github.com/envoyproxy/gateway/blob/main/internal%2Fgatewayapi%2Ftestdata%2Fxlistenerset-tlsroute.in.yaml

https://github.com/envoyproxy/gateway/blob/main/test%2Fe2e%2Ftestdata%2Fxlistenerset-tls.yaml

krishicks · 2026-02-04T04:18:14Z

@krishicks Thanks for taking care of this. This PR looks good!

Could we also move the secrets to another namespace in these two tests and add ReferenceGrants to cover the cross-ns secret references use case?

https://github.com/envoyproxy/gateway/blob/main/internal%2Fgatewayapi%2Ftestdata%2Fxlistenerset-tlsroute.in.yaml

https://github.com/envoyproxy/gateway/blob/main/test%2Fe2e%2Ftestdata%2Fxlistenerset-tls.yaml

Yes, of course!

I'm assuming you meant e2e/testdata/xlistenerset-base.yaml, as e2e/testdata/xlistenerset-tls.yaml doesn't use TLS termination (it uses passthrough).

I pushed new changes. Please take a look at the e2e test for me; I think it's right but I'm not able to run it locally.

zirain · 2026-02-04T09:28:24Z

@krishicks can you make CI happy?

krishicks · 2026-02-04T16:23:08Z

I understand the e2e failures and am working on an update. I also see how to run them locally, so I'll make sure they're green as well.

kkk777-7 · 2026-02-04T17:20:29Z

@krishicks thanks for working this, your code looks good to me!

e2e failure root cause is below

=== RUN   TestE2E/XListenerSetHTTPS
    conformance.go:70: 2026-02-04T15:22:39.701304814Z: Applying testdata/xlistenerset-base.yaml
    apply.go:275: 2026-02-04T15:22:39.718643104Z: Creating xlistener-gateway Gateway
    apply.go:294: 2026-02-04T15:22:39.738690269Z: Updating xlistener-base Namespace
    apply.go:275: 2026-02-04T15:22:39.750423435Z: Creating xlistener-https-certificate Secret
    apply.go:277: 
        	Error Trace:	/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/conformance/utils/kubernetes/apply.go:277
        	            				/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/conformance/utils/suite/conformance.go:71
        	            				/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/conformance/utils/suite/suite.go:487
        	Error:      	Received unexpected error:
        	            	secrets "xlistener-https-certificate" is forbidden: unable to create new content in namespace xlistener-base because it is being terminated
        	Test:       	TestE2E/XListenerSetHTTPS
        	Messages:   	error creating resource

During cleanup for each XListenerSet test, we delete xlistenerset-base.yaml, which ends up deleting namespace as well. The problem seems to be that the next test creates Secret in the same namespace before it has been completely removed.

so, we should move namespace and secret and XListenerSet (for HTTPS) into xlistenerset-https.yaml, then may resolve e2e failure.

krishicks · 2026-02-04T18:25:19Z

Thanks @kkk777-7 , that was my understanding of the failure. I chose to rename the existing XListenerSetTLS test to XListenerSetTLSPassthrough and add a new one, XListenerSetTLSTermination, which has its own Secret/ReferenceGrant/Namespace.

I was able to run both of those e2e tests locally and see them pass, but the whole suite failed when I ran it in some other, non-XListenerSet area.

Previously, validateTerminateModeAndGetTLSSecrets would always use the namespace of the listener's gateway when verifying a cross-namespace ref. This meant that if the listener were from an XListenerSet, whether or not the Secret associated with the certificateRef was in the same namespace as the XListenerSet, it would not be permitted. Additionally, and relatedly, this fixes an issue where an XListenerSet could reference a Secret in the gateway's namespace without a ReferenceGrant being present. With this change we add a new GetNamespace() method to gatewayapi.ListenerContext which returns the listener's gateway's namespace for a listener added directly to the gateway, or the XListenerSet's namespace otherwise. This is similar to some of the other methods that were added to ListenerContext in support of XListenerSets. The new method is used when creating the `crossNamespaceFrom` to determine if the certificateRef is permitted. If the Secret and XListenerSet are in the same namespace, it is permitted. If that is not the case a ReferenceGrant from the XListenerSet to the Secret will be properly searched for. Signed-off-by: Kris Hicks <[email protected]>

krishicks · 2026-02-04T19:31:13Z

This is ready for another round through CI.

krishicks · 2026-02-04T22:58:57Z

The test failures all seem unrelated to these changes. 🤔

zirain · 2026-02-04T23:28:43Z

/retest

kkk777-7 · 2026-02-05T01:44:03Z

LGTM, thanks!

Previously, validateTerminateModeAndGetTLSSecrets would always use the namespace of the listener's gateway when verifying a cross-namespace ref. This meant that if the listener were from an XListenerSet, whether or not the Secret associated with the certificateRef was in the same namespace as the XListenerSet, it would not be permitted. Additionally, and relatedly, this fixes an issue where an XListenerSet could reference a Secret in the gateway's namespace without a ReferenceGrant being present. With this change we add a new GetNamespace() method to gatewayapi.ListenerContext which returns the listener's gateway's namespace for a listener added directly to the gateway, or the XListenerSet's namespace otherwise. This is similar to some of the other methods that were added to ListenerContext in support of XListenerSets. The new method is used when creating the `crossNamespaceFrom` to determine if the certificateRef is permitted. If the Secret and XListenerSet are in the same namespace, it is permitted. If that is not the case a ReferenceGrant from the XListenerSet to the Secret will be properly searched for. Signed-off-by: Kris Hicks <[email protected]>

Previously, validateTerminateModeAndGetTLSSecrets would always use the namespace of the listener's gateway when verifying a cross-namespace ref. This meant that if the listener were from an XListenerSet, whether or not the Secret associated with the certificateRef was in the same namespace as the XListenerSet, it would not be permitted. Additionally, and relatedly, this fixes an issue where an XListenerSet could reference a Secret in the gateway's namespace without a ReferenceGrant being present. With this change we add a new GetNamespace() method to gatewayapi.ListenerContext which returns the listener's gateway's namespace for a listener added directly to the gateway, or the XListenerSet's namespace otherwise. This is similar to some of the other methods that were added to ListenerContext in support of XListenerSets. The new method is used when creating the `crossNamespaceFrom` to determine if the certificateRef is permitted. If the Secret and XListenerSet are in the same namespace, it is permitted. If that is not the case a ReferenceGrant from the XListenerSet to the Secret will be properly searched for. Signed-off-by: krishicks <[email protected]> Signed-off-by: Karol Szwaj <[email protected]>

* chore(docs): Update Azure Entra link in OIDC guide (#8167) Update Azure Entra link in OIDC guide Signed-off-by: Guy Daich <[email protected]> * fix: continue processing the remaining xDS with invalid EnvoyPatchPolicies (#8153) continue processing the remaining xDS with invalid EnvoyPatchPolicies Signed-off-by: Huabing (Robin) Zhao <[email protected]> Signed-off-by: Karol Szwaj <[email protected]> * build(deps): bump the actions group across 1 directory with 2 updates (#8178) Bumps the actions group with 2 updates in the / directory: [docker/login-action](https://github.com/docker/login-action) and [github/codeql-action](https://github.com/github/codeql-action). Updates `docker/login-action` from 3.6.0 to 3.7.0 - [Release notes](https://github.com/docker/login-action/releases) - [Commits](docker/login-action@5e57cd1...c94ce9f) Updates `github/codeql-action` from 4.32.0 to 4.32.1 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@b20883b...6bc82e0) --- updated-dependencies: - dependency-name: docker/login-action dependency-version: 3.7.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions - dependency-name: github/codeql-action dependency-version: 4.32.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Isaac Wilson <[email protected]> Signed-off-by: Karol Szwaj <[email protected]> * fix: skip provision when IR Infra is invalid (#7754) * fix: do not trigger IR deletion when EnvoyProxy is invalid Signed-off-by: zirain <[email protected]> * add Invalid to ir.Infra Signed-off-by: zirain <[email protected]> * fix gen Signed-off-by: zirain <[email protected]> * add e2e Signed-off-by: zirain <[email protected]> * remove invalid Signed-off-by: zirain <[email protected]> * add comments Signed-off-by: zirain <[email protected]> * update Signed-off-by: zirain <[email protected]> * merge loop Signed-off-by: zirain <[email protected]> * move back Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> Signed-off-by: Karol Szwaj <[email protected]> * docs: add HTTP header and method based authentication task (#7990) * docs: add HTTP header and method based authentication task Signed-off-by: Aditya Sanskar Srivastav <[email protected]> * docs: replace api-key examples with user header Signed-off-by: Aditya Sanskar Srivastav <[email protected]> * docs: format header and method authentication examples Signed-off-by: Aditya Sanskar Srivastav <[email protected]> * docs: add header and method based authorization examples Signed-off-by: Aditya Sanskar Srivastav <[email protected]> --------- Signed-off-by: Aditya Sanskar Srivastav <[email protected]> Signed-off-by: Karol Szwaj <[email protected]> * fix: Validation of XListenerSet certificateRefs (#8168) Previously, validateTerminateModeAndGetTLSSecrets would always use the namespace of the listener's gateway when verifying a cross-namespace ref. This meant that if the listener were from an XListenerSet, whether or not the Secret associated with the certificateRef was in the same namespace as the XListenerSet, it would not be permitted. Additionally, and relatedly, this fixes an issue where an XListenerSet could reference a Secret in the gateway's namespace without a ReferenceGrant being present. With this change we add a new GetNamespace() method to gatewayapi.ListenerContext which returns the listener's gateway's namespace for a listener added directly to the gateway, or the XListenerSet's namespace otherwise. This is similar to some of the other methods that were added to ListenerContext in support of XListenerSets. The new method is used when creating the `crossNamespaceFrom` to determine if the certificateRef is permitted. If the Secret and XListenerSet are in the same namespace, it is permitted. If that is not the case a ReferenceGrant from the XListenerSet to the Secret will be properly searched for. Signed-off-by: krishicks <[email protected]> Signed-off-by: Karol Szwaj <[email protected]> * fix: Remove whitespace for nodeSelector in deployment YAML - helm chart change (#8185) Remove whitespace for nodeSelector in deployment YAML Signed-off-by: Jess Belliveau <[email protected]> Signed-off-by: Karol Szwaj <[email protected]> * [release/v1.7.0] release notes (#8188) Signed-off-by: Karol Szwaj <[email protected]> --------- Signed-off-by: Guy Daich <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Signed-off-by: Karol Szwaj <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: zirain <[email protected]> Signed-off-by: Aditya Sanskar Srivastav <[email protected]> Signed-off-by: krishicks <[email protected]> Signed-off-by: Jess Belliveau <[email protected]> Co-authored-by: Guy Daich <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Isaac Wilson <[email protected]> Co-authored-by: zirain <[email protected]> Co-authored-by: Aditya Sanskar Srivastav <[email protected]> Co-authored-by: krishicks <[email protected]> Co-authored-by: Jess Belliveau <[email protected]>

Previously, validateTerminateModeAndGetTLSSecrets would always use the namespace of the listener's gateway when verifying a cross-namespace ref. This meant that if the listener were from an XListenerSet, whether or not the Secret associated with the certificateRef was in the same namespace as the XListenerSet, it would not be permitted. Additionally, and relatedly, this fixes an issue where an XListenerSet could reference a Secret in the gateway's namespace without a ReferenceGrant being present. With this change we add a new GetNamespace() method to gatewayapi.ListenerContext which returns the listener's gateway's namespace for a listener added directly to the gateway, or the XListenerSet's namespace otherwise. This is similar to some of the other methods that were added to ListenerContext in support of XListenerSets. The new method is used when creating the `crossNamespaceFrom` to determine if the certificateRef is permitted. If the Secret and XListenerSet are in the same namespace, it is permitted. If that is not the case a ReferenceGrant from the XListenerSet to the Secret will be properly searched for. Signed-off-by: Kris Hicks <[email protected]>

krishicks requested a review from a team as a code owner February 3, 2026 17:53

krishicks force-pushed the main branch from efe83f5 to 7b89beb Compare February 3, 2026 17:54

arkodg added this to the v1.7.0 Release milestone Feb 3, 2026

arkodg requested a review from zhaohuabing February 3, 2026 18:38

krishicks marked this pull request as draft February 4, 2026 00:57

krishicks force-pushed the main branch from 7b89beb to 71f6969 Compare February 4, 2026 01:46

krishicks changed the title ~~fix: Discovery of ReferenceGrants for XListenerSets~~ fix: Validation of XListenerSet certificateRefs Feb 4, 2026

krishicks force-pushed the main branch 4 times, most recently from 7a1ab25 to 26e5a76 Compare February 4, 2026 02:16

krishicks marked this pull request as ready for review February 4, 2026 02:17

krishicks force-pushed the main branch 2 times, most recently from b3473e8 to 877a659 Compare February 4, 2026 02:26

krishicks force-pushed the main branch from 877a659 to 5a3e716 Compare February 4, 2026 04:14

zirain mentioned this pull request Feb 4, 2026

XListenerSet TLS Secret reference fails even in same namespace (ReferenceGrant required and ineffective) #8181

Closed

krishicks force-pushed the main branch from 5a3e716 to 382e496 Compare February 4, 2026 14:13

krishicks force-pushed the main branch from 382e496 to 531f0f1 Compare February 4, 2026 18:23

krishicks force-pushed the main branch from 531f0f1 to 0ce41af Compare February 4, 2026 18:46

krishicks force-pushed the main branch from 0ce41af to 8190fbb Compare February 4, 2026 18:49

kkk777-7 approved these changes Feb 5, 2026

View reviewed changes

zirain approved these changes Feb 5, 2026

View reviewed changes

zirain merged commit d85a78e into envoyproxy:main Feb 5, 2026
55 of 59 checks passed

Conversation

krishicks commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for cerulean-figolla-1f9435 canceled.

Uh oh!

codecov bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

krishicks commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krishicks commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krishicks commented Feb 4, 2026

Uh oh!

zhaohuabing commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krishicks commented Feb 4, 2026

Uh oh!

zirain commented Feb 4, 2026

Uh oh!

krishicks commented Feb 4, 2026

Uh oh!

kkk777-7 commented Feb 4, 2026

Uh oh!

krishicks commented Feb 4, 2026

Uh oh!

krishicks commented Feb 4, 2026

Uh oh!

krishicks commented Feb 4, 2026

Uh oh!

zirain commented Feb 4, 2026

Uh oh!

kkk777-7 commented Feb 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

krishicks commented Feb 3, 2026 •

edited

Loading

netlify bot commented Feb 3, 2026 •

edited

Loading

codecov bot commented Feb 3, 2026 •

edited

Loading

krishicks commented Feb 3, 2026 •

edited

Loading

krishicks commented Feb 3, 2026 •

edited

Loading

zhaohuabing commented Feb 4, 2026 •

edited

Loading