Skip to content

fix(gatewayapi): sort api key auth credentials#9042

Merged
zirain merged 7 commits into
envoyproxy:mainfrom
nobbs:fix-apikey-auth-credential-order
Jun 9, 2026
Merged

fix(gatewayapi): sort api key auth credentials#9042
zirain merged 7 commits into
envoyproxy:mainfrom
nobbs:fix-apikey-auth-credential-order

Conversation

@nobbs

@nobbs nobbs commented May 20, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

fix

What this PR does / why we need it:

API key auth credentials are currently built by iterating over Secret.Data directly. Since Secret.Data is a Go map, the credential order can vary between otherwise equivalent translations.

That nondeterministic order is preserved into the generated API key auth filter and per-route xDS config. In clusters with frequent Gateway API/xDS rebuilds, Envoy can receive byte-different listener or route resources even when the effective API key auth configuration did not change. For long-lived connections such as WebSockets, this can cause Envoy to replace the active filter chain and close existing connections with:

downstream_local_disconnect(filter_chain_is_being_removed)

This PR makes API key auth credential generation deterministic by sorting the secret data keys before appending credentials to the IR. It also adds a unit test that verifies credentials are emitted in stable client ID order.

With this change applied in a reproducing environment, the internal HTTPS listener stopped flipping due to API key credential reordering and the WebSocket connection stayed alive.

Validation:

  • make generate gen-check
  • go test ./internal/gatewayapi

Which issue(s) this PR fixes:

Fixes #8889

Release Notes: Yes

@nobbs nobbs requested a review from a team as a code owner May 20, 2026 09:06
@netlify

netlify Bot commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit b8c4b05
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a276cde373e8f00084116e8
😎 Deploy Preview https://deploy-preview-9042--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@nobbs nobbs force-pushed the fix-apikey-auth-credential-order branch from a9bfe1d to 46f9774 Compare May 20, 2026 09:07
@codecov

codecov Bot commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.88%. Comparing base (04aadb9) to head (b8c4b05).

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #9042      +/-   ##
==========================================
+ Coverage   74.87%   74.88%   +0.01%     
==========================================
  Files         252      252              
  Lines       40792    40797       +5     
==========================================
+ Hits        30542    30550       +8     
+ Misses       8168     8163       -5     
- Partials     2082     2084       +2

☔ View full report in Codecov by Harness.
📢 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.

@zirain zirain force-pushed the fix-apikey-auth-credential-order branch from 46f9774 to 6825ffc Compare May 20, 2026 22:02
@nobbs nobbs force-pushed the fix-apikey-auth-credential-order branch 2 times, most recently from 7cd801c to e7efd21 Compare May 21, 2026 10:43
zirain
zirain previously approved these changes May 22, 2026
View reviewed changes
@nobbs nobbs force-pushed the fix-apikey-auth-credential-order branch from e7efd21 to ffc44f2 Compare May 22, 2026 18:24
@nobbs nobbs requested a review from zirain May 25, 2026 18:24
zirain
zirain previously approved these changes May 25, 2026
View reviewed changes
@nobbs nobbs force-pushed the fix-apikey-auth-credential-order branch from 6b7a522 to 85d67cc Compare May 28, 2026 06:53
@nobbs

nobbs commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Any chance to get this merged? As far as I can tell the tests are randomly failing and not only for this PR, but others, too.

arkodg
arkodg previously approved these changes Jun 8, 2026
View reviewed changes
@arkodg arkodg requested review from a team and zirain June 8, 2026 06:33
@arkodg arkodg added this to the v1.9.0-rc.1 Release milestone Jun 8, 2026
@zirain zirain requested a review from arkodg June 9, 2026 01:31
@zirain zirain merged commit b96a2d3 into envoyproxy:main Jun 9, 2026
39 checks passed
cleman95 pushed a commit to cleman95/gateway that referenced this pull request Jun 25, 2026
@nobbs @arkodg
@zirain @cleman95
fix(gatewayapi): sort api key auth credentials (envoyproxy#9042)
ca79ee4 
* fix(gatewayapi): sort api key auth credentials

Signed-off-by: Alexej Disterhoft <[email protected]>

* docs(release-notes): add api key auth fix note

Signed-off-by: Alexej Disterhoft <[email protected]>

---------

Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: zirain <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
Co-authored-by: zirain <[email protected]>
jukie added a commit that referenced this pull request Jul 1, 2026
@jukie @beckcl
@rissson @zhaohuabing @nobbs @arkodg @zirain @apkatsikas @jvlxz @MengjiaLiang @nimishamehta5 @cilindrox @gianniskt
[release-1.8] cherry-pick for v1.8.2 (#9376)
6c2e80d 
* fix rate limit validation on K8s 1.36 (#9166)

fixi: rate limit validation on K8s 1.36

Starting with K8s 1.36 the int range and maximum are validated
against each other, which breaks the BTP tests.

This can be fixed validating the field as uint32.

Signed-off-by: Clemens Beck <[email protected]>
Co-authored-by: Clemens Beck <[email protected]>
(cherry picked from commit a21fd29)
Signed-off-by: jukie <[email protected]>

* fix(validation): allow IPv6 to be specific in loadBalancerSourceRanges (#9050)

* fix(validation): allow IPv6 to be specific in loadBalancerSourceRanges and loadBalancerIP

Signed-off-by: Marc 'risson' Schmitt <[email protected]>
(cherry picked from commit 3c1d7b9)
Signed-off-by: jukie <[email protected]>

* fix: allow Backend TLS  to disable ALPN with an empty list (#9162)

* Fixed Backend TLS  to disable upstream ALPN instead of inheriting EnvoyProxy BackendTLS defaults.

Signed-off-by: Huabing Zhao <[email protected]>

* add test case for envoyproxy without tls

Signed-off-by: Huabing Zhao <[email protected]>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* address comments

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
(cherry picked from commit 21eb101)
Signed-off-by: jukie <[email protected]>

* fix(gatewayapi): sort api key auth credentials (#9042)

* fix(gatewayapi): sort api key auth credentials

Signed-off-by: Alexej Disterhoft <[email protected]>

* docs(release-notes): add api key auth fix note

Signed-off-by: Alexej Disterhoft <[email protected]>

---------

Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: zirain <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
Co-authored-by: zirain <[email protected]>
(cherry picked from commit b96a2d3)
Signed-off-by: jukie <[email protected]>

* fix(status): separate listener programmed state from HTTPRoute Accepted condition (#9129)

* fix(status): separate listener programmed state from HTTPRoute Accepted condition
  Remove the NoReadyListeners block that set Accepted: False on routes when
  no listeners were programmed. Route acceptance reflects binding validity;
  listener programmed state is already tracked by the listener's own conditions.

Signed-off-by: apkatsikas <[email protected]>

* fix(status): separate listener programmed state from TCPRoute/UDPRoute Accepted condition
  Routes with a valid binding to an unprogrammed listener now correctly reach
  Accepted=True regardless of listener readiness. Moves accepted=true and
  IncrementAttachedRoutes before the IsReady check so the Accepted condition
  reflects binding validity only, consistent with the HTTPRoute fix and the
  Gateway API spec.

Signed-off-by: apkatsikas <[email protected]>
(cherry picked from commit e9c3ac5)
Signed-off-by: jukie <[email protected]>

* fix: reject  ExternalName Service as a route backend (#9190)

fix: reject  Service as a route backend

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
(cherry picked from commit ee16b3f)
Signed-off-by: jukie <[email protected]>

* fix(status): fall back to service externalIPs for Gateway addresses (#9209)

When the Envoy service is of type LoadBalancer but no load balancer
controller assigns an ingress address (e.g. bare-metal clusters), the
Gateway status had no addresses and stayed Programmed=False with reason
AddressNotAssigned, even when reachable addresses were configured via
spec.externalIPs (e.g. through an EnvoyProxy service patch).

Use spec.externalIPs as a fallback when the load balancer ingress list
is empty. Ingress addresses keep priority when present, so behavior is
unchanged for clusters with a working load balancer controller.

Fixes #8987

Signed-off-by: Jules Dutel <[email protected]>
(cherry picked from commit f2c0921)
Signed-off-by: jukie <[email protected]>

* fix: don't duplicate ValidatingAdmissionPolicy in install.yaml (#9182)

* Fix duplicate VAP in install.yaml

Signed-off-by: jukie <[email protected]>

* Add regression check and release notes

Signed-off-by: jukie <[email protected]>

---------

Signed-off-by: jukie <[email protected]>
(cherry picked from commit 332081c)
Signed-off-by: jukie <[email protected]>

* fix(listenerset): implement hostname conflict listener precedence (#9192)

* fix(listenerset): implement hostname conflict listener precedence

- In Gateway, multiple listeners with the same hostname all get marked
  Conflicted (no winner). In ListenerSet, a Gateway-owned listener
  wins over ListenerSet listeners; among ListenerSet listeners the first
  in processing order wins.

- Conflicted ListenerSet listeners now get Accepted=False and
  Programmed=False with the conflict reason (HostnameConflict or
  ProtocolConflict), matching the Gateway API conformance expectation.

- AttachedListenerSets on the Gateway status is now incremented only for
  ListenerSets that have at least one accepted listener, aligning with
  the spec definition of "successfully attached".

Signed-off-by: zirain <[email protected]>

* ListenerSet conflict

Signed-off-by: zirain <[email protected]>

* fix and improve message

Signed-off-by: zirain <[email protected]>

* better display gateway listener

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 9271654)
Signed-off-by: jukie <[email protected]>

* fix: correct the config hot reload validation order (#9214)

Co-authored-by: Nimisha Mehta <[email protected]>
(cherry picked from commit 7e4492d)
Signed-off-by: jukie <[email protected]>

* feat(helm): add CRDs dependency toggle (#8850)

* feat(helm): add CRDs dependency toggle

Allow skipping installation of the `crds` dependency on the gateway-helm
chart by providing a conditional `crds.enabled` variable. This boolean
defaults to `true` and toggles the inclusion of the dependency on the
parent chart.

ref: #8560
Signed-off-by: Gaston Festari <[email protected]>

* docs: update Helm installation steps

Use the `crds.enabled` variable in the examples instead of the
`--skip-crds` flag.

Update crds.enabled variable description.

Co-authored-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Gaston Festari <[email protected]>

---------

Signed-off-by: Gaston Festari <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Co-authored-by: Huabing (Robin) Zhao <[email protected]>
(cherry picked from commit 62e01ce)
Signed-off-by: jukie <[email protected]>

* fix: validate API Key auth ExtractFrom (#9250)

* fix: validate API Key auth ExtractFrom

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* add test

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* update

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* update

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* update

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

---------

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
(cherry picked from commit 50b1339)
Signed-off-by: jukie <[email protected]>

* fix: XRateLimitHeadersOptionDisabled constant value must match CRD enum (#9224)

* fix: XRateLimitHeadersOptionDisabled constant value must match CRD enum
The constant held "Disabled" but the CRD enum specifies "Off", causing
xRateLimitHeaders: "Off" to silently fall through the translator switch
and always emit X-RateLimit headers.

Fixes #9223

Signed-off-by: gianniskt <[email protected]>

* fix: XRateLimitHeadersOptionDisabled constant value must match CRD enum
The constant held "Disabled" but the CRD enum specifies "Off", causing
xRateLimitHeaders: "Off" to silently fall through the translator switch
and always emit X-RateLimit headers.

Fixes #9223

Signed-off-by: gianniskt <[email protected]>

* fix: add release note for XRateLimitHeadersOptionDisabled fix

Signed-off-by: gianniskt <[email protected]>

---------

Signed-off-by: gianniskt <[email protected]>
Signed-off-by: Ioannis Koutroumpis <[email protected]>
(cherry picked from commit 43c9d8e)
Signed-off-by: jukie <[email protected]>

* fix(ratelimit): shared global ratelimit with cost not working (#9245)

* fix(ratelimit): shared global ratelimit with cost not working

Signed-off-by: zirain <[email protected]>

* fix

Signed-off-by: zirain <[email protected]>

* fix mixed shared rule

Signed-off-by: zirain <[email protected]>

* add e2e

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 7997791)
Signed-off-by: jukie <[email protected]>

* fix(gatewayapi): http retry without backoff (#9238)

* fix http retry without backoff

Signed-off-by: zirain <[email protected]>

* release notes

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 4872e53)
Signed-off-by: jukie <[email protected]>

* fix: panic with empty deploy (#9365)

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 42a0c2c)
Signed-off-by: jukie <[email protected]>

* chore: restore current.yaml after cherry-picks

The cherry-picked bug-fix commits appended their release-note entries to
release-notes/current.yaml. Those notes are already finalized in
release-notes/v1.8.2.yaml, so restore current.yaml to its prior state to
avoid duplicating them in the next release cycle.

Signed-off-by: jukie <[email protected]>

* [release-1.8] bump ratelimit image to 1e50889b

Bump the Envoy Ratelimit image from ff287602 to 1e50889b in source and
the gateway-helm chart, and note the bump under security updates in the
v1.8.2 release notes.

Signed-off-by: jukie <[email protected]>

* testdata

Signed-off-by: jukie <[email protected]>

* fix: use int64 format for ratelimit Requests field (#9377)

Signed-off-by: jukie <[email protected]>

---------

Signed-off-by: Clemens Beck <[email protected]>
Signed-off-by: jukie <[email protected]>
Signed-off-by: Marc 'risson' Schmitt <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: zirain <[email protected]>
Signed-off-by: apkatsikas <[email protected]>
Signed-off-by: Jules Dutel <[email protected]>
Signed-off-by: Gaston Festari <[email protected]>
Signed-off-by: gianniskt <[email protected]>
Signed-off-by: Ioannis Koutroumpis <[email protected]>
Co-authored-by: Clemens Beck <[email protected]>
Co-authored-by: Clemens Beck <[email protected]>
Co-authored-by: Marc 'risson' Schmitt <[email protected]>
Co-authored-by: Huabing (Robin) Zhao <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Alexej Disterhoft <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Andrew Katsikas <[email protected]>
Co-authored-by: jvlxz <[email protected]>
Co-authored-by: Mengjia Liang <[email protected]>
Co-authored-by: Nimisha Mehta <[email protected]>
Co-authored-by: Gaston Festari <[email protected]>
Co-authored-by: Ioannis Koutroumpis <[email protected]>
cnvergence added a commit that referenced this pull request Jul 7, 2026
@cnvergence @rissson
@zhaohuabing @nobbs @arkodg @zirain @immanuwell @jvlxz @MengjiaLiang @nimishamehta5 @jukie
[release/v1.7] cherry-pick for v1.7.5 (#9429)
50eb8f1 
* fix(validation): allow IPv6 to be specific in loadBalancerSourceRanges (#9050)

* fix(validation): allow IPv6 to be specific in loadBalancerSourceRanges and loadBalancerIP

Signed-off-by: Marc 'risson' Schmitt <[email protected]>
(cherry picked from commit 3c1d7b9)

* fix: allow Backend TLS  to disable ALPN with an empty list (#9162)

* Fixed Backend TLS  to disable upstream ALPN instead of inheriting EnvoyProxy BackendTLS defaults.

Signed-off-by: Huabing Zhao <[email protected]>

* add test case for envoyproxy without tls

Signed-off-by: Huabing Zhao <[email protected]>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* address comments

Signed-off-by: Huabing Zhao <[email protected]>

---------

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
(cherry picked from commit 21eb101)

* fix(gatewayapi): sort api key auth credentials (#9042)

* fix(gatewayapi): sort api key auth credentials

Signed-off-by: Alexej Disterhoft <[email protected]>

* docs(release-notes): add api key auth fix note

Signed-off-by: Alexej Disterhoft <[email protected]>

---------

Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: zirain <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
Co-authored-by: zirain <[email protected]>
(cherry picked from commit b96a2d3)

* fix(egctl): correct user-facing grammar in help text (#9217)

Signed-off-by: immanuwell <[email protected]>
(cherry picked from commit ef1d118)
Signed-off-by: Immanuel Tikhonov <[email protected]>

* fix: reject  ExternalName Service as a route backend (#9190)

fix: reject  Service as a route backend

Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
(cherry picked from commit ee16b3f)

* fix(status): fall back to service externalIPs for Gateway addresses (#9209)

When the Envoy service is of type LoadBalancer but no load balancer
controller assigns an ingress address (e.g. bare-metal clusters), the
Gateway status had no addresses and stayed Programmed=False with reason
AddressNotAssigned, even when reachable addresses were configured via
spec.externalIPs (e.g. through an EnvoyProxy service patch).

Use spec.externalIPs as a fallback when the load balancer ingress list
is empty. Ingress addresses keep priority when present, so behavior is
unchanged for clusters with a working load balancer controller.

Fixes #8987

Signed-off-by: Jules Dutel <[email protected]>
(cherry picked from commit f2c0921)
Signed-off-by: jvlxz <[email protected]>

* fix: correct the config hot reload validation order (#9214)

Co-authored-by: Nimisha Mehta <[email protected]>
(cherry picked from commit 7e4492d)
Signed-off-by: Mengjia Liang <[email protected]>

* fix: panic with empty deploy (#9365)

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 42a0c2c)

* fix: DaemonSet pod not using the configured custom ServiceAccount name (#9347)

* add test case

Signed-off-by: zirain <[email protected]>

* fix daemonset pod serviceaccount name

Signed-off-by: zirain <[email protected]>

* release notes

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
(cherry picked from commit 59a0cab)

* fix: use int64 format for ratelimit Requests field (#9377)

Signed-off-by: jukie <[email protected]>
(cherry picked from commit 6916245)
Signed-off-by: Isaac Wilson <[email protected]>

* [release/v1.7] bump envoy to 1.37.5 and ratelimit to f2fb1577

Signed-off-by: Karol Szwaj <[email protected]>

* [release/v1.7] add release notes for v1.7.5

Signed-off-by: Karol Szwaj <[email protected]>

* fix gen

Signed-off-by: Karol Szwaj <[email protected]>

* fix rn

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Marc 'risson' Schmitt <[email protected]>
Signed-off-by: Huabing Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: Alexej Disterhoft <[email protected]>
Signed-off-by: zirain <[email protected]>
Signed-off-by: immanuwell <[email protected]>
Signed-off-by: Immanuel Tikhonov <[email protected]>
Signed-off-by: Jules Dutel <[email protected]>
Signed-off-by: jvlxz <[email protected]>
Signed-off-by: Mengjia Liang <[email protected]>
Signed-off-by: jukie <[email protected]>
Signed-off-by: Isaac Wilson <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Co-authored-by: Marc 'risson' Schmitt <[email protected]>
Co-authored-by: Huabing (Robin) Zhao <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Alexej Disterhoft <[email protected]>
Co-authored-by: Arko Dasgupta <[email protected]>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Immanuel Tikhonov <[email protected]>
Co-authored-by: jvlxz <[email protected]>
Co-authored-by: Mengjia Liang <[email protected]>
Co-authored-by: Nimisha Mehta <[email protected]>
Co-authored-by: Isaac Wilson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhaohuabing zhaohuabing zhaohuabing approved these changes
@zirain zirain zirain approved these changes
@arkodg arkodg Awaiting requested review from arkodg

Assignees

No one assigned

Labels

cherrypick/release-v1.7.5 cherrypick/release-v1.8.2

Projects

None yet

Milestone

v1.9.0-rc.1 Release

Development

Successfully merging this pull request may close these issues.

bug: periodic equivalent xDS updates drain active websocket connections

4 participants

@nobbs @zhaohuabing @zirain @arkodg