fix(listenerset): preserve routes on hostname conflict winner - #9768
Merged
kkk777-7 merged 1 commit intoAug 27, 2026
Merged
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
TheisFerre
force-pushed
the
fix/listenerset-conflict-route-attachment
branch
from
August 17, 2026 11:35
b937209 to
5e976e9
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b93720932d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Theis Ferré Hjortkjær <daetfh@egmont.com>
TheisFerre
force-pushed
the
fix/listenerset-conflict-route-attachment
branch
from
August 26, 2026 13:13
5e976e9 to
e62f892
Compare
zirain
approved these changes
Aug 26, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9768 +/- ##
==========================================
+ Coverage 76.29% 76.32% +0.02%
==========================================
Files 261 261
Lines 44588 44591 +3
==========================================
+ Hits 34019 34032 +13
+ Misses 8312 8305 -7
+ Partials 2257 2254 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
/retest |
Member
|
/retest |
kkk777-7
pushed a commit
to kkk777-7/gateway
that referenced
this pull request
Aug 27, 2026
…roxy#9768) fix(listenerset): preserve routes on conflict winner Signed-off-by: Theis Ferré Hjortkjær <daetfh@egmont.com>
zirain
added a commit
that referenced
this pull request
Aug 28, 2026
* Revert 9532 (#9747) * Revert "chore: regen xds translator testdata for initialFetchTimeout (#9643)" This reverts commit 4898608. Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * Revert "fix: initial fetch timed out for type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret (#9532)" This reverts commit 4ce3a07. Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * regen: testdata output Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * fix gen Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> * fix: rate limiting cannot scale or enforce correctly (#9814) * change RateLimit cluster to EDS type Signed-off-by: zirain <zirain2009@gmail.com> * update test Signed-off-by: zirain <zirain2009@gmail.com> * release notes Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> * fix: udp consistent hash (#9826) * feat: udp consistent hash Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * add release note Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * fix lint Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> --------- Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * fix: honor access log text format when format.type is unset (#9817) * fix: honor access log text format when format.type is unset ProxyAccessLogFormat.Type is optional: the CEL rule on the type only requires that text or json is set when the type is omitted, so `format: {text: "..."}` is accepted by the API server for any sink. The OpenTelemetry sink honors it, File and ALS did not — both tested `format.Type != nil && *format.Type == Text` and fell through to the JSON branch, where a nil format.JSON produced an empty field list and the translator substituted the default JSON fields. The configured text format was dropped with no error, no status condition and no log line. Resolve the effective format type once, before the sink loop: an explicit type wins, otherwise a text-only format resolves to Text and everything else to JSON. The File and ALS sinks now branch on that. The no-sink default path, which built a JSON access log on /dev/stdout regardless of format, has the same root cause and is fixed with it. OpenTelemetry is deliberately left alone: it can carry text and attributes at the same time and already handles the unset type itself. The existing unit case "nil format type with text only uses text for file sink" from #7720 asserted the JSON fallback its own name argues against; its expectation is updated to the text access log. Fixes #9719 Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com> * fix: keep JSON when an unset format type carries both text and json The File and ALS sinks and the no-sink default path have always rendered JSON for format: {text, json} with no type, and the API accepts that input. Inferring Text from a non-nil text alone would silently switch those setups from structured JSON to text and break downstream parsing, so infer Text only when json is absent. Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com> --------- Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com> * security: enable AES-256-GCM OAuth2 cookie encryption by default (#9831) * security: enable AES-256-GCM OAuth2 cookie encryption by default Set envoy.reloadable_features.oauth2_use_gcm_encryption=true and envoy.reloadable_features.oauth2_legacy_cbc_decrypt_compat=false in the proxy bootstrap's global_config static layer, closing the padding oracle in CVE-2026-47775 for OIDC users by default. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * docs: note that Replace bootstraps do not get the OAuth2 GCM guards The guards live in the rendered default bootstrap, which spec.bootstrap type Replace discards. Call that out in the release note so OIDC users with a replacement bootstrap know to set them themselves. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> * fix: watch namespace changes for listener (#9649) * watch namespace changes Signed-off-by: zirain <zirain2009@gmail.com> * fix lint Signed-off-by: zirain <zirain2009@gmail.com> * fix watch and update Signed-off-by: zirain <zirain2009@gmail.com> * remove log Signed-off-by: zirain <zirain2009@gmail.com> * fix lint Signed-off-by: zirain <zirain2009@gmail.com> * fix e2e Signed-off-by: zirain <zirain2009@gmail.com> * release notes Signed-off-by: zirain <zirain2009@gmail.com> * Distinguish namespace events before forcing translation Signed-off-by: zirain <zirain2009@gmail.com> * fix flaky Signed-off-by: zirain <zirain2009@gmail.com> * remove Force Signed-off-by: zirain <zirain2009@gmail.com> * fix lint Signed-off-by: zirain <zirain2009@gmail.com> * remvoe unless namespace check Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> * fix: list extension manager resources once per reconcile (#9764) feat: list extension manager resources once per reconcile Collect ext-GVK in populateExtensionResources and call it once per reconcile from processGateways. Signed-off-by: stillya <st.ilya.101@gmail.com> * fix(metrics): widen watchable_subscribe_duration_seconds bucket spacing (#9812) * fix(metrics): widen watchable_subscribe_duration_seconds bucket spacing Motivation: watchable_subscribe_duration_seconds used buckets {0.001, 0.01, 0.1, 1, 5, 10}, with a 10x gap between 0.1 and 1 and a 5x gap between 1 and 5. Because histogram_quantile interpolates within a bucket, a control plane whose translations take ~2s puts every observation in (1, 5], so p50/p95/p99 all read as flat somewhere in that range regardless of whether the control plane is healthy or steadily degrading, until it crosses the next boundary. A sub-second control plane has the same problem in (0.1, 1]. Approach: Adopt the bucket spacing already used and reviewed in this repo for the k8s rest client latency histogram (rest_client_request_duration_seconds, internal/metrics/restclient/metrics.go), extended with a trailing 120.0 bucket to keep the slow tail visible: {0.005, 0.025, 0.1, 0.25, 0.5, 1.0, 2.0, 4.0, 8.0, 15.0, 30.0, 60.0, 120.0}. Reusing an already-reviewed spacing avoids picking new numbers from scratch. Bucket boundaries are a breaking change for any dashboard/alert referencing the old le values (0.001, 1, 5 no longer exist), so this adds a release note. The shipped Grafana panel for this metric (charts/gateway-addons-helm/dashboards/envoy-gateway-global.json) queries by le dynamically (legendFormat "{{le}}"), so it keeps working unchanged with the new boundaries. Out of scope: the issue also describes a watchable_depth gauge that's always 0 (reads len() of an unbuffered channel). That part needs a staged plan across the shipped Grafana dashboard (repoint a panel, migrate two label_values queries, then decide deprecate vs. redefine) and is left for a separate change. Two other histograms in this repo (internal/provider/kubernetes/metrics.go's status_update_duration_seconds and internal/infrastructure/kubernetes/metrics.go's resource_apply_duration_seconds / resource_delete_duration_seconds) have the same coarse buckets and were left untouched to keep this change scoped to the exact metric named in the report. Validation: - go build ./internal/message/... ./internal/metrics/... (passed) - go test ./internal/message/... ./internal/metrics/... (passed; no test in the repo pins specific bucket values for this histogram) - golangci-lint run --config=tools/linter/golangci-lint/.golangci.yml ./internal/message/... ./internal/metrics/... (clean) - bash tools/hack/check-release-notes-filenames.sh (passed) - gh run list --repo envoyproxy/gateway --branch main --event push --limit 20: lint, gen-check, coverage-test, and build jobs are green on main; only some e2e/conformance jobs and OSV-Scanner are currently red on main itself, pre-existing and unrelated to this change. Report: #9776 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code) * docs(release-notes): correct removed histogram boundaries list The note previously listed (0.001, 1, 5) as removed le boundaries, but 1.0 is still present in the new buckets while 0.01 and 10 were also dropped. Update the note to list the actual removed set: 0.001, 0.01, 5, 10. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> * docs(release-notes): rework wording per reviewer suggestion Adopt the clearer phrasing hsubramanianaks suggested: explicitly call out that _sum/_count are unchanged, use le="..." notation for the removed/retained bucket boundaries, and note that histogram_quantile() queries don't need changes since they aggregate by le dynamically. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --------- Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Co-authored-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> * fix: skip backendResource watch when CRD is missing instead of crashing controller (#9809) * fix: skip backendResource watch when CRD is missing instead of crashing controller When an extension manager declares backendResources pointing at a custom CRD that is absent (or the controller SA lacks list/watch RBAC), the informer's initial LIST fails, WaitForCacheSync times out, and the controller crash-loops — taking down the entire gateway, not just the routes using the custom backend. This adds a checkCRD call before registering each backendResource watch, following the same pattern already used for ServiceImport, Backend, HTTPRouteFilter, and other optional CRDs. If the CRD is missing, the watch is skipped with a warning log instead of failing cache sync. Fixes #9807 Signed-off-by: Lucas Siedlarczyk <lucas.siedlarczyk@gmail.com> * feat: make control plane trace more readable (#9672) * fix: make control plane trace more readable Signed-off-by: zirain <zirain2009@gmail.com> * update Signed-off-by: zirain <zirain2009@gmail.com> * make sure that span end on panic Signed-off-by: zirain <zirain2009@gmail.com> * record queue wait ms Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> * [observability] Add a dedicated span for EnvoyPatchPolicy JSON patch processing JSON patch processing can dominate the xDS translation time when a cluster has many EnvoyPatchPolicies or expensive patches, but today it is hidden inside the Translator.Translate span, so there is no way to tell how much of a multi-second translation is spent applying patches. Start a Translator.processJSONPatches child span, recording the number of policies and how many patches were applied, targeted a missing resource, or failed. The span is only started when the xDS IR actually carries EnvoyPatchPolicies. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Signed-off-by: zirain <zirain2009@gmail.com> * fix context Signed-off-by: zirain <zirain2009@gmail.com> * fix review comments Signed-off-by: zirain <zirain2009@gmail.com> * add trace link Signed-off-by: zirain <zirain2009@gmail.com> * nit Signed-off-by: zirain <zirain2009@gmail.com> * apply huabing's change Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * fix: keep orphaned OIDC PKCE cookies off every request (#9644) * fix: keep orphaned OIDC PKCE cookies off every request Envoy mints a nonce (CSRF) and a PKCE code verifier cookie for every authorization flow it starts, keyed by a random per-flow id so that several logins can be in flight at once. On a successful callback it deletes only the pair belonging to the flow that completed, which is deliberate - sweeping every flow would strip the cookies of the other tabs still mid-login. A flow that is started and abandoned therefore leaves its pair behind until it expires, and at the default path "/" those orphans are sent on every request. Combined with a provider that issues large id and access tokens they overflow the inbound request header limit, at which point the callback itself fails and the browser loops back through the flow, minting yet another pair. Envoy only needs the value of these two cookies when it validates the callback, so scope them to the OIDC redirect path. Orphans then stay off ordinary application requests instead of counting against the header limit on each one. This bounds the problem rather than eliminating it - they are still sent to the callback endpoint, and they still expire on their own within csrfTokenTTL. Also name the code verifier cookie CodeVerifier-<suffix>. It was the only one of the seven OAuth2 cookies left at Envoy's default name, so SecurityPolicies sharing a cookie domain all wrote one shared cookie, and logging out of one policy deleted the in-flight flow cookies of the others. The redirect path is only applied when it satisfies the pattern Envoy enforces on a cookie path, which is stricter than a URL path - "," and ";" for example are legal RFC 3986 sub-delims but are rejected. An unrepresentable path falls back to leaving the cookie path unset, which Envoy defaults to "/", rather than failing xDS validation and dropping the route. Fixes #9632 Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * test: make the OIDC e2e client path-aware The e2e cookie tracker replayed every stored cookie on every request, keyed by name alone. That made it blind to the cookie path, so the scoping this change relies on could regress without any test noticing, and cookies with the same name at two different paths overwrote each other. Track cookies by name and path, honour the RFC 6265 path-match rules when deciding what to send, and drop cookies the server expires. It is deliberately still not a net/http/cookiejar: the OAuth2 filter marks every cookie "secure" and these tests run over plain HTTP, so a spec-compliant jar would store the cookies and then never send them back. With that in place, assert that an authorization flow which has been started but not completed - the state an abandoned flow leaves behind - keeps its nonce and code verifier cookies off ordinary application requests. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * fix(listenerset): preserve routes on hostname conflict winner (#9768) fix(listenerset): preserve routes on conflict winner Signed-off-by: Theis Ferré Hjortkjær <daetfh@egmont.com> * update generate files Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * remove: each pr's release notes Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> --------- Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Andrey Maltsev <maltsev.andrey@gmail.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io> Signed-off-by: stillya <st.ilya.101@gmail.com> Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Signed-off-by: Lucas Siedlarczyk <lucas.siedlarczyk@gmail.com> Signed-off-by: Theis Ferré Hjortkjær <daetfh@egmont.com> Co-authored-by: zirain <zirain2009@gmail.com> Co-authored-by: Andrey Maltsev <maltsev.andrey@gmail.com> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: stillya <st.ilya.101@gmail.com> Co-authored-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Co-authored-by: Lucas Siedlarczyk <lucas.siedlarczyk@gmail.com> Co-authored-by: Theis Ferré Hjortkjær <daetfh@egmont.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Fixes HTTPRoutes becoming
Accepted=FalsewithNoMatchingListenerHostnamewhen their parent listener wins a hostname conflict.After #9192, ListenerSet precedence is reported correctly, but the losing listener can still remove the winner's hostname during route intersection. This causes the winning listener to report
attachedRoutes: 0and omits the route from xDS.This change tracks listeners that explicitly lose hostname conflict precedence and excludes only those listeners from route hostname filtering. Listeners that are unprogrammed for unrelated reasons retain their existing behavior.
This is my initial approach to the fix. I am open to alternatives if you prefer another way to represent hostname ownership after conflict resolution.
Tests cover both Gateway and ListenerSet winners, route status, attached route counts, conflict conditions, and xDS output.
Which issue(s) this PR fixes:
Fixes #9767
PR Checklist
git commit -s).make generate gen-checkandmake lintpass locally. The unit-test/coverage build will be verified by CI.gen-checkpasses.