feat(gatewayapi): support SDS listener certificate references - #9525
feat(gatewayapi): support SDS listener certificate references#9525Sharvash wants to merge 9 commits into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
6b20f25 to
f8a6e4d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9525 +/- ##
==========================================
+ Coverage 76.06% 76.27% +0.21%
==========================================
Files 260 261 +1
Lines 43449 43631 +182
==========================================
+ Hits 33048 33279 +231
+ Misses 8196 8155 -41
+ Partials 2205 2197 -8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
f8a6e4d to
9a20e02
Compare
|
Hi @Sharvash could you please fix the conflicts? |
Allow SDS-backed certificates in listener IR validation when both the Unix socket URL and secret name are set. Keep inline certificate validation unchanged and cover invalid SDS configurations. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
Accept Secrets with the gateway.envoyproxy.io/sds type when enableSDSSecretRef is enabled. Preserve ReferenceGrant checks, partial-invalid listener status, and certificate reference ordering while continuing to normalize inline TLS Secrets. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
Translate listener SDS references into canonical static UDS clusters and wire them into Envoy TLS certificate SDS configs. Deduplicate clusters by socket URL and reject name collisions without exposing raw socket paths in errors. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
Document the SDS Secret format, feature-gate behavior, cross-namespace references, and certificate-overlap limitation. Add the release note for issue envoyproxy#8915. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
Add a UDS-backed SDS test server and an HTTPS Gateway scenario that verifies backend traffic and the exact certificate served by Envoy. Enable SDS Secret references in each E2E profile. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
9a20e02 to
ecd9b22
Compare
|
Hi @zhaohuabing, done |
|
/retest |
SDS cluster names are an xDS translation detail and have no IR consumers. Keep the helper private to the translator and colocate its collision and UTF-8 tests without changing generated names. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
SDS listener references are validated before IR construction, and invalid references already surface as InvalidCertificateRef. Rely on that boundary to remove duplicate conversion error propagation and listener condition handling. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
Report opaque SDS certificate names through the Gateway API standard OverlappingTLSConfig condition while retaining the SDSCertificateOpaque reason. Remove the obsolete custom condition type and update status tests. Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com>
|
/retest |
|
/retest |
|
@codex review |
![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: b6171ac179
ℹ️ 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".
| for _, dnsName := range listener.tls.certDNSNames { | ||
| if areOverlappingHostnames(sdsListener.Hostname, new(gwapiv1.Hostname(dnsName))) { |
There was a problem hiding this comment.
When the SDS listener has an explicit hostname, this loop only downgrades non-SDS peers whose known SAN overlaps that hostname. However the SDS certificate's SANs are the opaque part, and EG does not require the served cert to contain only the listener hostname; an SDS server for sds.example.com can return a cert that also covers foo.example.com. In that configuration a client can open an h2 connection to the SDS listener and coalesce foo.example.com requests onto it, while the foo.example.com listener remains on h2 because its SANs do not match sds.example.com. Any same-port peer of a valid SDS-backed HTTPS listener should be treated as overlapping unless ALPN is explicitly configured.
Useful? React with 👍 / 👎.
4 participants
What this PR does / why we need it:
Allow Gateway listeners to use
gateway.envoyproxy.io/sdsSecrets intls.certificateRefswhenenableSDSSecretRefis enabled.This change:
The Unix socket cluster naming change affects existing generated xDS. EnvoyPatchPolicies and extension servers that refer to the previous cluster names must be updated. This is documented in the breaking-change release note.
Which issue(s) this PR fixes:
Fixes #8915
Local validation:
make generatemake gen-checkmake lintmake buildgo test ./internal/gatewayapi/... ./internal/ir/... ./internal/xds/translator/... -count=1The live Kubernetes E2E scenario was not run locally.
PR Checklist
git commit -s)./api.make gen-checkpasses.