fix: validate SDS URL by zirain · Pull Request #9030 · envoyproxy/gateway

zirain · 2026-05-19T06:02:22Z

netlify · 2026-05-19T06:02:27Z

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

Name	Link
🔨 Latest commit	`d9b806e`
🔍 Latest deploy log	https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a5f7010dc600100083283fd
😎 Deploy Preview	https://deploy-preview-9030--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5fcdb4439

ℹ️ 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".

codecov · 2026-05-19T11:15:44Z

Codecov Report

❌ Patch coverage is 80.43478% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.58%. Comparing base (2874485) to head (d9b806e).
⚠️ Report is 27 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/xds/translator/sds.go	76.47%	2 Missing and 2 partials ⚠️
internal/xds/translator/listener.go	0.00%	3 Missing ⚠️
internal/ir/xds.go	77.77%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9030      +/-   ##
==========================================
- Coverage   75.59%   75.58%   -0.01%     
==========================================
  Files         252      253       +1     
  Lines       41713    41744      +31     
==========================================
+ Hits        31531    31552      +21     
- Misses       8057     8066       +9     
- Partials     2125     2126       +1

☔ 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.

zhaohuabing · 2026-05-27T04:04:06Z

-      # /var/run/secrets/workload-spiffe-uds/socket
-      url: L3Zhci9ydW4vc2VjcmV0cy93b3JrbG9hZC1zcGlmZmUtdWRzL3NvY2tldA==
+      # unix:///var/run/secrets/workload-spiffe-uds/socket
+      url: dW5peDovLy92YXIvcnVuL3NlY3JldHMvd29ya2xvYWQtc3BpZmZlLXVkcy9zb2NrZXQ=


This would break upgrades from v1.8.0 to v1.9.0. Would it make sense to default to UDS for url without scheme?

it's experimental, it should be fine.
I'm fine to add compatibility in v1.9, cc @envoyproxy/gateway-maintainers WDYT?

If this breaking change is acceptable, it's better to cp this into v1.8.1 to break early.

zhaohuabing · 2026-06-03T03:36:26Z

internal/utils/net/url.go (line 25) accepts unix://host/path values but silently drops the host by returning only u.Path. For example unix://var/run/app.sock becomes /run/app.sock, which can point Envoy at the wrong socket while still passing validation. The validator should reject non-empty u.Host for unix URLs, or normalize it intentionally with tests.

zirain · 2026-06-03T03:49:47Z

is unix://var/run/app.sock valid? should it be unix:///var/run/app.sock?

zhaohuabing · 2026-06-03T08:00:33Z

unix:///var/run/secrets/workload-spiffe-uds/socket

unix://var/run/app.sock shouldn't become /run/app.sock. Should we just reject unix://var/run/app.sock?

zirain · 2026-06-03T11:40:47Z

unix:///var/run/secrets/workload-spiffe-uds/socket

unix://var/run/app.sock shouldn't become /run/app.sock. Should we just reject unix://var/run/app.sock?

this should be rejected, the implement updated.

Signed-off-by: zirain <zirain2009@gmail.com>

zhaohuabing

LGTM. Thanks!

zhaohuabing · 2026-07-21T03:53:52Z

(High — test regression) sds-invalid no longer tests what it claims. internal/gatewayapi/testdata/sds-invalid.in.yaml was not updated, so its sds-ref-1/sds-ref-2 secrets still use bare-path URLs (/var/run/secrets/workload-spiffe-uds/socket, no scheme). With the new validation those now fail at ParseURL before the checks the test was built for. The golden diff confirms it: every case's message changed from Multiple SDS reference secrets are not supported. / Cannot mix SDS reference secrets with other CA certificate types. to the URL-scheme error. So all three labeled cases ("multiple SDS", "mix with ConfigMap", "mix with Secret") now short-circuit on URL validation and those validation paths are no longer exercised.
→ Update sds-invalid.in.yaml to give the SDS secrets valid unix://… URLs so the multiple/mixing logic is still covered, and add a separate case for the invalid-URL error.
(Medium — compat / release-note framing) This rejects a previously-accepted format. The valid test (sds.in.yaml) and the invalid test both used bare paths before, so bare-path SDS URLs were the prior norm — they're now hard-rejected. If SDS-via-secret shipped in any release before this, that's a breaking change for existing secrets, and the release note (filed under bug_fixes, "added validation … instead of silently producing an invalid socket address") doesn't mention that bare paths stop working. Please confirm the feature is unreleased (same release as chore: add e2e for SDS Ref #8983); if not, call out the migration or accept bare paths as unix://.

Signed-off-by: zirain <zirain2009@gmail.com>

arkodg

LGTM thanks
non blocking: does the IR validation failure error trickle into resource status to let the user know of the issue ?

…envoyproxy#9184/envoyproxy#9030 SDS changes main's new TLSRoute dynamic-resolver support referenced a destSettings variable that doesn't exist on this branch, which renamed it to allDs/backendClusterRefs earlier in this branch's own history. git's 3-way merge combined the two without a textual conflict, silently producing code that failed to compile. Adapt the check to use allDs (a dynamic resolver is never merge-eligible, so it can only appear there) while counting both allDs and backendClusterRefs for the single-destination requirement. Signed-off-by: Muhammad Waqar <mwaqar@confluent.io>

* fix: add validation for SDS URL Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> * lint Signed-off-by: zirain <zirain2009@gmail.com> * fix test Signed-off-by: zirain <zirain2009@gmail.com> * reject UDS with host Signed-off-by: zirain <zirain2009@gmail.com> * fix lint Signed-off-by: zirain <zirain2009@gmail.com> * fix related path check Signed-off-by: zirain <zirain2009@gmail.com> * release notes Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com>

zirain requested a review from a team as a code owner May 19, 2026 06:02

zirain mentioned this pull request May 19, 2026

chore: add e2e for SDS Ref #8983

Open

chatgpt-codex-connector Bot reviewed May 19, 2026

View reviewed changes

Comment thread internal/utils/net/url.go

zhaohuabing reviewed May 27, 2026

View reviewed changes

zirain force-pushed the fix/sds-uds-scheme branch from e413ba5 to 30e5b8d Compare June 3, 2026 04:32

zirain force-pushed the fix/sds-uds-scheme branch 2 times, most recently from c8fd664 to 29387ac Compare June 8, 2026 22:56

zhaohuabing requested a review from a team June 11, 2026 13:33

arkodg previously approved these changes Jun 11, 2026

View reviewed changes

zirain dismissed arkodg’s stale review via 42a3495 June 12, 2026 02:16

zirain requested review from arkodg and zhaohuabing June 12, 2026 02:16

zirain added this to the v1.9.0-rc.1 Release milestone Jul 10, 2026

zirain added 8 commits July 21, 2026 09:46

fix: add validation for SDS URL

5c1b3a3

Signed-off-by: zirain <zirain2009@gmail.com>

fix

6db9f74

Signed-off-by: zirain <zirain2009@gmail.com>

lint

8309c17

Signed-off-by: zirain <zirain2009@gmail.com>

fix test

fbf86dd

Signed-off-by: zirain <zirain2009@gmail.com>

reject UDS with host

709509d

Signed-off-by: zirain <zirain2009@gmail.com>

fix lint

184833b

Signed-off-by: zirain <zirain2009@gmail.com>

fix related path check

2435dbe

Signed-off-by: zirain <zirain2009@gmail.com>

release notes

e69cfad

Signed-off-by: zirain <zirain2009@gmail.com>

zirain force-pushed the fix/sds-uds-scheme branch from 42a3495 to e69cfad Compare July 21, 2026 01:47

zirain mentioned this pull request Jul 21, 2026

feat(gatewayapi): support SDS listener certificate references #9525

Merged

11 tasks

zhaohuabing previously approved these changes Jul 21, 2026

View reviewed changes

zhaohuabing requested a review from a team July 21, 2026 03:51

fix

d9b806e

Signed-off-by: zirain <zirain2009@gmail.com>

zirain dismissed zhaohuabing’s stale review via d9b806e July 21, 2026 13:11

zirain requested a review from zhaohuabing July 21, 2026 13:17

zhaohuabing approved these changes Jul 22, 2026

View reviewed changes

zhaohuabing requested a review from a team July 24, 2026 07:54

arkodg approved these changes Jul 26, 2026

View reviewed changes

arkodg added the cherrypick/release-v1.8.4 label Jul 26, 2026

arkodg modified the milestones: v1.9.0-rc.1 Release, v1.9.0 Release Jul 27, 2026

zirain merged commit 7f980e6 into envoyproxy:main Jul 27, 2026
63 of 65 checks passed

zirain deleted the fix/sds-uds-scheme branch July 27, 2026 12:27

zirain mentioned this pull request Jul 27, 2026

SDS: does the IR validation failure error trickle into resource status to let the user know of the issue #9585

Open

Uh oh!

Conversation

zirain commented May 19, 2026

Uh oh!

netlify Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

codecov Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zirain May 27, 2026

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing commented Jun 3, 2026

Uh oh!

zirain commented Jun 3, 2026

Uh oh!

zhaohuabing commented Jun 3, 2026

Uh oh!

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

Uh oh!

Uh oh!

zhaohuabing left a comment

Choose a reason for hiding this comment

Uh oh!

zhaohuabing commented Jul 21, 2026

Uh oh!

arkodg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify Bot commented May 19, 2026 •

edited

Loading

codecov Bot commented May 19, 2026 •

edited

Loading

zhaohuabing May 27, 2026 •

edited

Loading

zhaohuabing May 28, 2026 •

edited

Loading

zirain commented Jun 3, 2026 •

edited

Loading