fix: do not return 500 for all requests when part of BackendRefs are invalid by zhaohuabing · Pull Request #7488 · envoyproxy/gateway

zhaohuabing · 2025-11-11T11:42:16Z

This issue was introduced in #4363 and caused EG return 5XX responses to all requests when part of BackendRefs are invalid.

This PR fixes it by creating a DestinationSetting with no endpoints to represent the invalid backendRef.
This ensures requests are routed correctly between valid backends and the synthetic invalid-backend-cluster(which return 500) based on their respective weights.

Fixes: #7483

Release note: yes.

This issue also exists on these releases: v1.5.x, v1.4.x, v1.3.x. I'll raise follow-up on these branches since cherry-pick may be tricky.

codecov · 2025-11-11T11:51:15Z

Codecov Report

❌ Patch coverage is 86.20690% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.28%. Comparing base (2637771) to head (2df9fdb).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/gatewayapi/route.go	84.00%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7488      +/-   ##
==========================================
- Coverage   72.37%   72.28%   -0.09%     
==========================================
  Files         231      231              
  Lines       34071    34074       +3     
==========================================
- Hits        24658    24632      -26     
- Misses       7643     7669      +26     
- Partials     1770     1773       +3

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

...httproute-attaching-to-listener-with-multiple-backend-backendrefs-diff-address-type.out.yaml

internal/gatewayapi/testdata/httproute-with-some-invalid-backend-refs-no-service.out.yaml

internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.in.yaml

zhaohuabing · 2025-11-11T15:19:46Z

internal/gatewayapi/route.go

 			}

-			if ds == nil {
+			// skip backendRefs with weight 0 as they do not affect the traffic distribution
+			if ds.Weight != nil && *ds.Weight == 0 {


This doesn't look great - the ir ds.weight should be a value, not a pointer. I'm leaving it as is to avoid touching too many files in this PR and keep focus on the bug fix.

We can clean this up in a follow-up PR.

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

internal/gatewayapi/route.go

release-notes/current.yaml

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

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

internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.in.yaml

arkodg · 2025-11-12T03:53:22Z

thanks @zhaohuabing, changes LGTM, can we add a e2e to avoid a regression

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

zhaohuabing · 2025-11-12T11:35:16Z

thanks @zhaohuabing, changes LGTM, can we add a e2e to avoid a regression

Glad we added the e2e test, otherwise I wouldn't have realized that NeedsClusterPerSetting also needed updating. :-)

zirain · 2025-11-13T00:13:35Z

/retest

…invalid (envoyproxy#7488) * do not return 500 for all requests when part of BackendRefs are invalid Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]>

…art of BackendRefs are … (#7503) fix: do not return 500 for all requests when part of BackendRefs are invalid (#7488) * do not return 500 for all requests when part of BackendRefs are invalid Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]>

…invalid (envoyproxy#7488) * do not return 500 for all requests when part of BackendRefs are invalid Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]>

…art of BackendRefs are invalid (#7504) fix: do not return 500 for all requests when part of BackendRefs are invalid (#7488) * do not return 500 for all requests when part of BackendRefs are invalid Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]>

…invalid (envoyproxy#7488) * do not return 500 for all requests when part of BackendRefs are invalid Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> (cherry picked from commit 2899416) Signed-off-by: Huabing Zhao <[email protected]>

* fix: oidc authentication endpoint was overwritten by discovered value (#7460) fix: oid authentication endpoint was overriden by discovered value Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> (cherry picked from commit 50dcb15) Signed-off-by: Huabing Zhao <[email protected]> * fix: do not return 500 for all requests when part of BackendRefs are invalid (#7488) * do not return 500 for all requests when part of BackendRefs are invalid Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> (cherry picked from commit 2899416) Signed-off-by: Huabing Zhao <[email protected]> * fix: prevent skeleton route status entries for unmanaged GatewayClasses (#7536) * fix: prevent skeleton route status entries for unmanaged GatewayClasses When processing policies (EnvoyExtensionPolicy, SecurityPolicy), the translator was calling GetRouteParentContext for ALL parentRefs in a route, even those referencing gateways with different GatewayClasses not managed by this translator. GetRouteParentContext creates a skeleton RouteParentStatus entry with just the controllerName when called on a parentRef that hasn't been processed yet. Since all GatewayClass instances share the same controller name, these skeleton entries persisted in status without conditions. The fix checks if a parentRef context already exists before attempting to apply policy configuration to it. If the context doesn't exist, it means this parentRef wasn't processed by this translator and should be skipped. Signed-off-by: Raj Singh <[email protected]> * fix: also prevent skeleton entries in BackendTrafficPolicy processing The same issue exists in BackendTrafficPolicy route processing - calling GetRouteParentContext for all parentRefs creates skeleton status entries. Apply the same fix: check if parentRef context exists before adding to list. Signed-off-by: Raj Singh <[email protected]> --------- Signed-off-by: Raj Singh <[email protected]> (cherry picked from commit ff13742) Signed-off-by: Huabing Zhao <[email protected]> * treat too many addresses as programmed (#7542) Signed-off-by: cong <[email protected]> (cherry picked from commit 7cb5f72) Signed-off-by: Huabing Zhao <[email protected]> * bechmark: fix cpu sampling (#7581) use fixed duration for cpu rate Signed-off-by: Huabing Zhao <[email protected]> (cherry picked from commit 536486f) Signed-off-by: Huabing Zhao <[email protected]> * chore: bump golang.org/x/crypto (#7588) * chore: bump golang.org/x/crypto Signed-off-by: zirain <[email protected]> * fix gen Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> (cherry picked from commit 70fa59a) Signed-off-by: Huabing Zhao <[email protected]> * findOwningGateway should return controller based on linked GatewayClass (#7611) * fix: filter Gateway by controller in findOwningGateway Prevent cross-controller Gateway mutations by validating GatewayClass Signed-off-by: Sudipto Baral <[email protected]> (cherry picked from commit ba8e0e2) Signed-off-by: Huabing Zhao <[email protected]> * fix: use default when namespace is unset (#7612) * fix: use default when namespace is unset Signed-off-by: zirain <[email protected]> * fix Signed-off-by: zirain <[email protected]> * fix test Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> (cherry picked from commit be2cc73) Signed-off-by: Huabing Zhao <[email protected]> * bump Gateway API v1.4.1 (#7653) Signed-off-by: zirain <[email protected]> (cherry picked from commit 0fa26d7) Signed-off-by: Huabing Zhao <[email protected]> * update release note Signed-off-by: Huabing Zhao <[email protected]> * fix gen check Signed-off-by: Huabing Zhao <[email protected]> * ci: add script to free disk space (#7534) * feat: free disk space Signed-off-by: Shreemaan Abhishek <[email protected]> * lint Signed-off-by: Shreemaan Abhishek <[email protected]> * cleanup Signed-off-by: Shreemaan Abhishek <[email protected]> * make target and tools/hack Signed-off-by: Shreemaan Abhishek <[email protected]> * lint Signed-off-by: Shreemaan Abhishek <[email protected]> * modular action Signed-off-by: Shreemaan Abhishek <[email protected]> --------- Signed-off-by: Shreemaan Abhishek <[email protected]> (cherry picked from commit 4312f38) Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Signed-off-by: Raj Singh <[email protected]> Signed-off-by: cong <[email protected]> Signed-off-by: zirain <[email protected]> Signed-off-by: Sudipto Baral <[email protected]> Signed-off-by: Shreemaan Abhishek <[email protected]> Co-authored-by: Raj Singh <[email protected]> Co-authored-by: 聪 <[email protected]> Co-authored-by: zirain <[email protected]> Co-authored-by: Sudipto Baral <[email protected]> Co-authored-by: shreealt <[email protected]>

zhaohuabing requested a review from a team as a code owner November 11, 2025 11:42

zhaohuabing marked this pull request as draft November 11, 2025 11:42

zhaohuabing force-pushed the fix-7483 branch from cc5be5c to 973b8dc Compare November 11, 2025 12:11