api: add HTTP/3 advertised port to ClientTrafficPolicy - #9855
Conversation
Signed-off-by: Haseeb Nazir <36381672+iamhaseebn@users.noreply.github.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![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: 95b1ce7bf3
ℹ️ 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".
| if http3Settings.AdvertisedPort != nil { | ||
| advertisedPort = *http3Settings.AdvertisedPort |
There was a problem hiding this comment.
When HTTPS listeners share an address and internal port but have section-targeted ClientTrafficPolicies with different advertisedPort values, http3EnabledListeners retains only the last listener's settings for that address/port. Reading the override from that shared value therefore emits the same alt-svc port for every listener's routes, directing clients for at least one listener to the wrong external port. Keep enablement shared, but select the override from the current httpListener.HTTP3 settings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in ef6758f. Shared address/port state now tracks only HTTP/3 enablement, while the alt-svc override comes from the current listener. The xDS golden fixture now covers distinct advertised ports on listeners sharing the same address and port.
Signed-off-by: Haseeb Nazir <36381672+iamhaseebn@users.noreply.github.com>
Signed-off-by: Haseeb Nazir <36381672+iamhaseebn@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef6758f205
ℹ️ 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".
|
LGTM thanks |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9855 +/- ##
==========================================
- Coverage 81.31% 81.28% -0.03%
==========================================
Files 263 263
Lines 40870 40874 +4
==========================================
- Hits 33233 33226 -7
- Misses 7637 7648 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Haseeb Nazir <36381672+iamhaseebn@users.noreply.github.com>
Signed-off-by: Haseeb Nazir <36381672+iamhaseebn@users.noreply.github.com>
Signed-off-by: Haseeb Nazir <36381672+iamhaseebn@users.noreply.github.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
3 participants
What this PR does / why we need it:
When Envoy Gateway runs behind an external load balancer, the client-facing HTTP/3 port can differ from the Gateway listener port. The generated
alt-svcheader currently always uses the listener's external port, so clients can be directed to the wrong port.This adds an optional
spec.http3.advertisedPortfield toClientTrafficPolicy. When set, it overrides only the port advertised in the generatedalt-svcheader. The listener and Service ports remain unchanged. When omitted, the existing behavior is preserved.The field uses
gatewayv1.PortNumberwith admission validation for ports 1 through 65535. The value is carried through the infrastructure and xDS IR. The change also updates the generated CRDs and API reference, HTTP/3 documentation, release notes, and regression coverage for both the override and fallback paths.Verified with:
make gen-checkmake lintgo test -p=2 -race ./...make go.test.coverage GO_TEST_COVERAGE_ARGS='--tags=integration,conformance_unit_test -race -p=2'make go.test.celmake lint.markdown licensecheckWhich issue(s) this PR fixes:
Fixes #9721
PR Checklist
git commit -s). See DCO: Sign your work./api), the API was discussed and agreed before the implementation. The API change can be in a separate PR, or in the same PR, but the API must be agreed before implementation. N/A if this PR does not contain API changes.make generate gen-check,make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, butgen-check,lint, and coverage MUST pass.)release-notes/current/<section>/<pr-number>-<slug>.md(seerelease-notes/current/README.mdfor sections and naming). N/A if this PR does not contain non-trivial changes.make gen-checkand committed the result if API/helm charts/modules changed.release-notes/current/breaking_changes/.