fix(translator): Fix panic with request mirror + grpcroute by AndyMoreland · Pull Request #6875 · envoyproxy/gateway

AndyMoreland · 2025-08-30T04:50:06Z

What this PR does / why we need it:

This fixes an unconditional panic when the RequestMirror filter is used with GRPCRoutes

Which issue(s) this PR fixes:

Release Notes: No

codecov · 2025-08-30T06:15:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.08%. Comparing base (c181d47) to head (c502048).
⚠️ Report is 200 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6875      +/-   ##
==========================================
- Coverage   71.10%   71.08%   -0.03%     
==========================================
  Files         225      225              
  Lines       39859    39868       +9     
==========================================
- Hits        28343    28340       -3     
- Misses       9848     9857       +9     
- Partials     1668     1671       +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.

arkodg · 2025-08-30T18:14:11Z

internal/gatewayapi/filters.go

can we eliminate this ?
validateBackendRef already accepts a backend interface

gateway/internal/gatewayapi/validate.go

Line 29 in c181d47

func (t *Translator) validateBackendRef(backendRefContext BackendRefContext, route RouteContext,

@arkodg I think we need some kind of split in the concrete type of the iface approximately here, because validateBackendRefFilters needs GRPCRoutes to have a GRPCRouteFilter-containing-backendref, and HTTPRoutes to have an HTTPRouteFilter-containing-backendref -- the panic was caused by the existing unification causing the downstream cast to fail.

(I'm not a go expert or familiar with the types in play, but this was the smallest code diff way I found to avoid the unchecked cast panic)

kkk777-7 · 2025-09-01T15:40:25Z

@AndyMoreland
thanks for raising the issue and working on the fix!
If we just want to switch the type of BackendRef, how about retrieving the RouteType from filterContext and converting it to the appropriate type?

e.g.

	var mirrorBackendRef BackendRefContext
	routeType := GetRouteType(filterContext.Route)
	if routeType == resource.KindGRPCRoute {
		mirrorBackendRef = gwapiv1.GRPCBackendRef{
			BackendRef: gwapiv1.BackendRef{
				BackendObjectReference: mirrorBackend,
				Weight:                 &weight,
			},
		}
	} else {
		mirrorBackendRef = gwapiv1.HTTPBackendRef{
			BackendRef: gwapiv1.BackendRef{
				BackendObjectReference: mirrorBackend,
				Weight:                 &weight,
			},
		}
	}

	// This sets the status on the HTTPRoute, should the usage be changed so that the status message reflects that the backendRef is from the filter?
	filterNs := filterContext.Route.GetNamespace()
	serviceNamespace := NamespaceDerefOr(mirrorBackend.Namespace, filterNs)
	err = t.validateBackendRef(mirrorBackendRef, filterContext.Route,
		resources, serviceNamespace, routeType)

arkodg · 2025-09-02T17:40:24Z

+1 to #6875 (comment)

Signed-off-by: Andrew Moreland <[email protected]>

The validateBackendRefFilters function casts the Filters field to the specific filter type (HTTPRouteFilter vs GRPCRouteFilter) based on the routeKind. Using the wrong wrapper type causes a type assertion panic, so we must keep the typed wrappers despite the duplication. Signed-off-by: Andrew Moreland <[email protected]>

Signed-off-by: Andrew Moreland <[email protected]>

Simplified the RequestMirror filter processing by: - Getting RouteType directly from filterContext - Creating appropriate BackendRef type (HTTP or gRPC) based on RouteType - Consolidating logic into single processRequestMirrorFilter function - Keeping processGRPCRequestMirrorFilter as a delegate for compatibility This approach follows the review feedback and maintains the fix for GRPCRoute panic while simplifying the code structure. Signed-off-by: Andrew Moreland <[email protected]>

AndyMoreland · 2025-09-02T18:53:01Z

I've applied the suggested change

ProcessGRPCFilters now calls processRequestMirrorFilter directly since it already handles both HTTP and gRPC routes based on RouteType. Signed-off-by: Andrew Moreland <[email protected]>

Signed-off-by: Andrew Moreland <[email protected]>

internal/gatewayapi/filters.go

arkodg · 2025-09-02T20:08:25Z

thanks for addressing the comments @AndyMoreland, added one minor comment

Co-authored-by: Arko Dasgupta <[email protected]> Signed-off-by: Andrew Moreland <[email protected]>

AndyMoreland · 2025-09-02T21:25:40Z

I've merged your suggestion

arkodg

LGTM thanks

…y#6875) Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: shawnh2 <[email protected]>

* fix(xds-server): clear snapshot on stream close (#6618) * fix(xds-server): clear snapshot on stream close Signed-off-by: Zachary Vacura <[email protected]> * check if there are other active connections before clearning the snapshot Signed-off-by: Zachary Vacura <[email protected]> Signed-off-by: shawnh2 <[email protected]> * bug: disable x-envoy-ratelimited by default (#7110) * bug: disable x-envoy-ratelimited by default * can be enabled with `enableEnvoyHeaders` in CTP Relates to #7034 Signed-off-by: Arko Dasgupta <[email protected]> * fix tests and release note Signed-off-by: Arko Dasgupta <[email protected]> * fix testdata Signed-off-by: Arko Dasgupta <[email protected]> --------- Signed-off-by: Arko Dasgupta <[email protected]> Signed-off-by: shawnh2 <[email protected]> * fix(translator): Fix panic with request mirror + grpcroute (#6875) Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: shawnh2 <[email protected]> * fix: bug in overlap detection of cert SANs (#7234) Signed-off-by: shawnh2 <[email protected]> * bump golang for crypto/x509 reggression (#7236) Signed-off-by: zirain <[email protected]> Signed-off-by: shawnh2 <[email protected]> * fix gen-check Signed-off-by: shawnh2 <[email protected]> --------- Signed-off-by: Zachary Vacura <[email protected]> Signed-off-by: shawnh2 <[email protected]> Signed-off-by: Arko Dasgupta <[email protected]> Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: zirain <[email protected]> Co-authored-by: Zach Vacura <[email protected]> Co-authored-by: Arko Dasgupta <[email protected]> Co-authored-by: Andrew Moreland <[email protected]> Co-authored-by: Rudrakh Panigrahi <[email protected]> Co-authored-by: zirain <[email protected]>

…y#6875) Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: zirain <[email protected]>

* fix: bug in overlap detection of cert SANs (#7234) Signed-off-by: zirain <[email protected]> * fix(translator): Fix panic with request mirror + grpcroute (#6875) Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: zirain <[email protected]> * fix: watch change for the ca cert in the Backend (#7294) * watch change for the ca cert in the Backend Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: zirain <[email protected]> * fix ipFamily not set in UDPListener (#7313) fix: set ipfamily in udpistener (#7312) Signed-off-by: cong <[email protected]> Signed-off-by: zirain <[email protected]> * coalesce updates to reduce intermediate updates (#7328) * coalesce updates to reduce redundant processing in subscription handler Signed-off-by: Huabing Zhao <[email protected]> * retain order Signed-off-by: Huabing Zhao <[email protected]> * keep intermediate delete updates Signed-off-by: Huabing Zhao <[email protected]> * minor wording Signed-off-by: Huabing Zhao <[email protected]> * treat delete as normal operations Signed-off-by: Huabing Zhao <[email protected]> * retain the original order of the last updates for each key Signed-off-by: Huabing Zhao <[email protected]> * address comments Signed-off-by: Huabing Zhao <[email protected]> * fix test Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: zirain <[email protected]> * fix: port typo (#7397) Signed-off-by: cong <[email protected]> Signed-off-by: zirain <[email protected]> * fix: validate EnvoyGateway configuration before reload (#7412) Signed-off-by: zirain <[email protected]> * fix: missing jwt provider when jwt is configured on multiple listeners sharing the same port (#7337) * fix jwt provider missing when jwt is configured at multiple ir listeners Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: zirain <[email protected]> * fix: memory leak (#7429) Fix memory leak. Two watchable.Maps were never closed when shutting down the provider: - GatewayClassStatuses.Close() - missing in GatewayAPIStatuses.Close() - BackendTrafficPolicyStatuses.Close() - missing in PolicyStatuses.Close() Each unclosed map leaked 3 goroutines: 1. Internal watchable.Map.coalesce goroutine 2. HandleSubscription goroutine blocked on channel read 3. Error handler goroutine blocked on channel read Signed-off-by: Gonzalo Serrano <[email protected]> Signed-off-by: zirain <[email protected]> * fix gen after cherry-pick Signed-off-by: zirain <[email protected]> * fix watchutil test Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: zirain <[email protected]> Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: cong <[email protected]> Signed-off-by: Gonzalo Serrano <[email protected]> Co-authored-by: Rudrakh Panigrahi <[email protected]> Co-authored-by: Andrew Moreland <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: 聪 <[email protected]> Co-authored-by: Gonzalo Serrano <[email protected]>

AndyMoreland requested a review from a team as a code owner August 30, 2025 04:50

AndyMoreland force-pushed the fix/6874-fix-request-mirror branch 2 times, most recently from 0261526 to da5dfd6 Compare August 30, 2025 05:00

AndyMoreland mentioned this pull request Aug 30, 2025

GRPCRoutes panic the controller translator when RequestMirror filter is used #6874

Closed

AndyMoreland force-pushed the fix/6874-fix-request-mirror branch from c2e5f76 to fea450c Compare August 30, 2025 17:47

arkodg reviewed Aug 30, 2025

View reviewed changes

arkodg requested a review from kkk777-7 August 30, 2025 21:38

AndyMoreland added 10 commits September 2, 2025 11:52

Reproduce the issue; no expected output yet

ad6ec94

Signed-off-by: Andrew Moreland <[email protected]>

Work towards reference output

a09db5e

Signed-off-by: Andrew Moreland <[email protected]>

Fix crash

987a2eb

Signed-off-by: Andrew Moreland <[email protected]>

Test passes

76b4505

Signed-off-by: Andrew Moreland <[email protected]>

Update comment

4112496

Signed-off-by: Andrew Moreland <[email protected]>

Refactor

fbea6b3

Signed-off-by: Andrew Moreland <[email protected]>

Update grpcroute-with-mirror testdata formatting

05e670c

Signed-off-by: Andrew Moreland <[email protected]>

Fix lint

57ce28e

Signed-off-by: Andrew Moreland <[email protected]>

AndyMoreland force-pushed the fix/6874-fix-request-mirror branch from 521204e to 4740589 Compare September 2, 2025 18:52

AndyMoreland added 2 commits September 2, 2025 11:56

Remove unnecessary processGRPCRequestMirrorFilter stub function

8df852f

ProcessGRPCFilters now calls processRequestMirrorFilter directly since it already handles both HTTP and gRPC routes based on RouteType. Signed-off-by: Andrew Moreland <[email protected]>

Fix lint

629d6f9

Signed-off-by: Andrew Moreland <[email protected]>

arkodg reviewed Sep 2, 2025

View reviewed changes

internal/gatewayapi/filters.go Outdated Show resolved Hide resolved

Update internal/gatewayapi/filters.go

c502048

Co-authored-by: Arko Dasgupta <[email protected]> Signed-off-by: Andrew Moreland <[email protected]>

arkodg approved these changes Sep 2, 2025

View reviewed changes

arkodg requested a review from a team September 2, 2025 22:08

zirain approved these changes Sep 2, 2025

View reviewed changes

arkodg merged commit a8c8487 into envoyproxy:main Sep 2, 2025
49 of 51 checks passed

kkk777-7 mentioned this pull request Sep 10, 2025

feat: support ir metadata to tcproute/udproute #6815

Merged

arkodg added cherrypick/release-v1.4.5 cherrypick/release-v1.5.4 labels Oct 14, 2025

shawnh2 pushed a commit to shawnh2/gateway that referenced this pull request Oct 15, 2025

fix(translator): Fix panic with request mirror + grpcroute (envoyprox…

c7d229c

…y#6875) Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: shawnh2 <[email protected]>

arkodg added cherrypick/release-v1.5.5 and removed cherrypick/release-v1.5.4 labels Oct 20, 2025

zirain pushed a commit to zirain/gateway that referenced this pull request Nov 14, 2025

fix(translator): Fix panic with request mirror + grpcroute (envoyprox…

6dd3d0d

…y#6875) Signed-off-by: Andrew Moreland <[email protected]> Signed-off-by: zirain <[email protected]>

aburanrbx mentioned this pull request Mar 17, 2026

fix bug with grpcroute mirror filter #8541

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(translator): Fix panic with request mirror + grpcroute#6875

fix(translator): Fix panic with request mirror + grpcroute#6875
arkodg merged 13 commits intoenvoyproxy:mainfrom
AndyMoreland:fix/6874-fix-request-mirror

AndyMoreland commented Aug 30, 2025

Uh oh!

codecov bot commented Aug 30, 2025 •

edited

Loading

Uh oh!

arkodg Aug 30, 2025

Uh oh!

AndyMoreland Aug 30, 2025

Uh oh!

AndyMoreland Aug 30, 2025

Uh oh!

kkk777-7 commented Sep 1, 2025

Uh oh!

arkodg commented Sep 2, 2025

Uh oh!

AndyMoreland commented Sep 2, 2025

Uh oh!

Uh oh!

arkodg commented Sep 2, 2025 •

edited

Loading

Uh oh!

AndyMoreland commented Sep 2, 2025

Uh oh!

arkodg left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

AndyMoreland commented Aug 30, 2025

Uh oh!

codecov bot commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arkodg Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

AndyMoreland Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

AndyMoreland Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

kkk777-7 commented Sep 1, 2025

Uh oh!

arkodg commented Sep 2, 2025

Uh oh!

AndyMoreland commented Sep 2, 2025

Uh oh!

Uh oh!

arkodg commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndyMoreland commented Sep 2, 2025

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!

4 participants

codecov bot commented Aug 30, 2025 •

edited

Loading

arkodg commented Sep 2, 2025 •

edited

Loading