xds/resolver: pass route's auto_host_rewrite to LB picker (gRFC A81) #8740

Pranjali-2501 · 2025-12-03T14:55:40Z

This PR implements the ConfigSelector changes required for gRFC A81.

It ensures that the auto_host_rewrite field from the xDS Route Configuration is correctly propagated through the resolver and made available to the Load Balancer picker via the RPC context.

Key Changes:

Pass the AutoHostRewrite field value from Route struct via RPC context.
Add helper functions for AutoHostRewrite in internal/xds/balancer/cluserimpl/picker.go.
Update ConfigSelector.SelectConfig to pass the AutoHostRewrite boolean in RPC context.

RELEASE NOTES: None

codecov · 2025-12-03T15:09:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.22%. Comparing base (4c27cc8) to head (afcc3fb).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8740      +/-   ##
==========================================
- Coverage   83.28%   83.22%   -0.07%     
==========================================
  Files         418      418              
  Lines       32367    32378      +11     
==========================================
- Hits        26958    26947      -11     
+ Misses       4034     4028       -6     
- Partials     1375     1403      +28

Files with missing lines	Coverage Δ
internal/xds/balancer/clusterimpl/picker.go	`97.40% <100.00%> (+0.25%)`	⬆️
internal/xds/resolver/serviceconfig.go	`67.64% <100.00%> (-20.33%)`	⬇️
internal/xds/resolver/xds_resolver.go	`85.79% <100.00%> (-2.78%)`	⬇️

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

easwars · 2025-12-04T00:22:47Z

Sorry for suggesting that we could possibly send the whole route instead of sending only the auto_host_rewrite field, as specified in A60. After reading A60, I realized that sending the whole route down is not as simple as just stashing a pointer to the route in an attribute. We need to take into account things like reference counts to the route and the selected cluster and the whole cluster map.

So, let's keep it simple for now and just add an attribute that carries the auto_host_rewrite just like how we pass the selected cluster name from the config selector to the LB policy. Thanks.

easwars

Also please remove any mentions to MatchedRoute from the PR description and from comments in the PR.

internal/xds/resolver/serviceconfig.go

internal/xds/resolver/xds_resolver_test.go

easwars · 2025-12-08T19:04:22Z

internal/xds/balancer/clusterimpl/picker.go

+	return autohostRewrite
+}
+
+// GetAutoHostRewriteForTesting returns the value of autoHostRewrite feild;


Nit: s/feild/field ?

easwars · 2025-12-08T19:05:23Z

internal/xds/balancer/clusterimpl/picker.go

+type autoHostRewriteKey struct{}
+
+// AutoHostRewrite retrieves the autoHostRewrite value from the provided context.
+func AutoHostRewrite(ctx context.Context) bool {


This need not be exported, right?

easwars · 2025-12-08T19:06:46Z

internal/xds/balancer/clusterimpl/picker.go

+	return autohostRewrite
+}
+
+// GetAutoHostRewriteForTesting returns the value of autoHostRewrite feild;


We need to still not have the Get prefix here: go/go-style/decisions#getters

easwars · 2025-12-08T19:13:17Z

internal/xds/resolver/xds_resolver_test.go

+// TestResolver_AutoHostRewrite verifies the propagation of the AutoHostRewrite
+// field from the xDS RouteConfiguration to the internal MatchedRoute.


We need to be clear here that we are only testing if the xDS resolver correctly propagates the value of the auto_host_rewrite field, and makes it available to the picker.

easwars · 2025-12-08T19:13:36Z

internal/xds/resolver/xds_resolver_test.go

+// TestResolver_AutoHostRewrite verifies the propagation of the AutoHostRewrite
+// field from the xDS RouteConfiguration to the internal MatchedRoute.


There is no more MatchedRoute struct. Please update.

easwars · 2025-12-08T19:15:05Z

internal/xds/resolver/xds_resolver_test.go

+			wantAutoHostRewrite: false,
+		},
+		{
+			name:                "Disable_UntrustedServer",


Test name is wrong. The env var is enabled here.

easwars · 2025-12-08T19:15:53Z

internal/xds/resolver/xds_resolver_test.go

+			wantAutoHostRewrite: false,
+		},
+		{
+			name:                "Disable_Routeconfig",


easwars · 2025-12-08T19:24:58Z

internal/xds/resolver/xds_resolver_test.go

+		wantAutoHostRewrite bool
+	}{
+		{
+			name:                "Enabled",


I think I sort of understand the naming of these sub-tests now. But, I think it's quite confusing. How about the following approach:

EnvVarDisabled_NonTrustedServer_AutoHostRewriteOff

EnvVarDisabled_NonTrustedServer_AutoHostRewriteOn

EnvVarDisabled_TrustedServer_AutoHostRewriteOff

EnvVarDisabled_TrustedServer_AutoHostRewriteOn

EnvVarEnabled_NonTrustedServer_AutoHostRewriteOff

EnvVarEnabled_NonTrustedServer_AutoHostRewriteOn

EnvVarEnabled_TrustedServer_AutoHostRewriteOff

EnvVarEnabled_TrustedServer_AutoHostRewriteOn

easwars · 2025-12-08T19:29:14Z

internal/xds/resolver/xds_resolver_test.go

+			ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+			defer cancel()
+			nodeID := uuid.New().String()
+			mgmtServer, _, _, _ := setupManagementServerForTest(t, nodeID)


You probably don't need this helper function here, since you don't use any of the returned channels from that helper function. You could instead manually start the management server here with e2e.StartManagementServer.

easwars · 2025-12-08T19:30:48Z

internal/xds/resolver/xds_resolver_test.go

+					}},
+				}},
+			}}
+			configureAllResourcesOnManagementServer(ctx, t, mgmtServer, nodeID, listeners, routes, clusters, endpoints)


Clusters and endpoints are not required for the purposes of this test. So, you should directly call the Update method on the management server and just give it the listener and the route (you could also inline the route configuration), and set the SkipValidation field in the UpdateOptions field to true. That will make it possible to not specify all resources.

Pranjali-2501 added Type: Feature New features or improvements in behavior Area: xDS Includes everything xDS related, including LB policies used with xDS. labels Dec 3, 2025

Pranjali-2501 changed the title ~~xds/resolver: propagate route's auto_host_rewrite field in MatchedRoute (gRFC A81)~~ xds/resolver: pass route's auto_host_rewrite to LB picker (gRFC A81) Dec 3, 2025

Pranjali-2501 added this to the 1.78 Release milestone Dec 3, 2025

pass matched route to LB picker

a6618d6

Pranjali-2501 force-pushed the configSelector-changes branch from 2d08842 to a6618d6 Compare December 3, 2025 15:14

Pranjali-2501 requested a review from easwars December 3, 2025 15:29

Pranjali-2501 assigned easwars Dec 3, 2025

easwars assigned Pranjali-2501 and unassigned easwars Dec 4, 2025

pass autoHostRewrite in RPC ctx

b8ef6f5

Pranjali-2501 assigned easwars and unassigned Pranjali-2501 Dec 4, 2025

easwars reviewed Dec 4, 2025

View reviewed changes

internal/xds/resolver/serviceconfig.go Outdated Show resolved Hide resolved

internal/xds/resolver/serviceconfig.go Outdated Show resolved Hide resolved

internal/xds/resolver/xds_resolver_test.go Outdated Show resolved Hide resolved

easwars assigned Pranjali-2501 and unassigned easwars Dec 4, 2025

Pranjali-2501 added 2 commits December 7, 2025 17:39

resolving comments

aff1b32

rename helper functions

afcc3fb

Pranjali-2501 assigned easwars and unassigned Pranjali-2501 Dec 8, 2025

Pranjali-2501 requested a review from easwars December 8, 2025 06:31

easwars reviewed Dec 8, 2025

View reviewed changes

easwars assigned Pranjali-2501 and unassigned easwars Dec 8, 2025

		// TestResolver_AutoHostRewrite verifies the propagation of the AutoHostRewrite
		// field from the xDS RouteConfiguration to the internal MatchedRoute.

xds/resolver: pass route's auto_host_rewrite to LB picker (gRFC A81) #8740

Are you sure you want to change the base?

xds/resolver: pass route's auto_host_rewrite to LB picker (gRFC A81) #8740

Conversation

Pranjali-2501 commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes:

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

easwars commented Dec 4, 2025

Uh oh!

easwars left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pranjali-2501 commented Dec 3, 2025 •

edited

Loading

codecov bot commented Dec 3, 2025 •

edited

Loading