xds: Support :authority header rewriting for LOGICAL_DNS clusters#8822
xds: Support :authority header rewriting for LOGICAL_DNS clusters#8822easwars merged 1 commit intogrpc:masterfrom
:authority header rewriting for LOGICAL_DNS clusters#8822Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8822 +/- ##
==========================================
- Coverage 83.42% 83.32% -0.11%
==========================================
Files 414 414
Lines 32763 32763
==========================================
- Hits 27334 27299 -35
- Misses 4040 4062 +22
- Partials 1389 1402 +13
🚀 New features to boost your workflow:
|
|
@Pranjali-2501 : If it is true that we missed support for LOGICAL_DNS clusters, then it is a bug because A81 does mention it. |
|
@Pranjali-2501 : Can you please make a first pass and loop me in once the changes look good to you. |
Pranjali-2501
left a comment
Pranjali-2501
left a comment
There was a problem hiding this comment.
Thanks @110y for taking the initiative to fix this.
110y
force-pushed
the
xds-cds-hostname
branch
from
62079e5 to
0f1c58c
Compare
110y
force-pushed
the
xds-cds-hostname
branch
from
0f1c58c to
c744f86
Compare
|
Updated! Please take another look |
Pranjali-2501
left a comment
Pranjali-2501
left a comment
There was a problem hiding this comment.
LGTM.
Assigning it to @easwars for second pass.
| pName := fmt.Sprintf("priority-%v", g.prefix) | ||
| for i, e := range endpoints { | ||
| retEndpoints[i] = hierarchy.SetInEndpoint(e, []string{pName}) | ||
| // Set hostname for gRFC A81 authority rewriting. |
There was a problem hiding this comment.
Could you please replace the existing comment with something like this which adds more information:
// For Logical DNS clusters, the same hostname attribute is added
// to all endpoints. It is set to the name that is resolved for the
// Logical DNS cluster, including the port number.
| // configureXDSResources configures the management server with a route that | ||
| // enables auto_host_rewrite and an endpoint with the specified hostname. | ||
| func configureXDSResources(ctx context.Context, t *testing.T, mgmtServer *e2e.ManagementServer, nodeID string, serverAddr string, endpointHostname string, secLevel e2e.SecurityLevel) { | ||
| func configureXDSResources(ctx context.Context, t *testing.T, mgmtServer *e2e.ManagementServer, nodeID string, serverAddr string, endpointHostname string, secLevel e2e.SecurityLevel, clusterType e2e.ClusterType) { |
There was a problem hiding this comment.
Nit: While you are here, could you please shorten the parameter list a bit by doing this:
func configureXDSResources(ctx context.Context, t *testing.T, mgmtServer *e2e.ManagementServer, nodeID, serverAddr, endpointHostname string, secLevel e2e.SecurityLevel, clusterType e2e.ClusterType) { ... }Thanks
easwars
changed the title
:authority header rewriting for LOGICAL_DNS clusters
|
@eshitachandwani : FYI when you delete the |
110y
force-pushed
the
xds-cds-hostname
branch
from
c744f86 to
b08ad38
Compare
|
I resolved a conflict and update as you commented. Please take another look, thanks 🙏 |
|
@110y : Thank you for your contribution! |
|
Thank you for your reviews! |
|
@110y : Do let us know if you are interested in making more contributions. We are always looking for more contributions from the community and would be happy to review code/design etc. |
|
Yes, I'm interested as I'm a fan of gRPC ecosystem! If we have any tasks or issues for beginners, I would appreciate it if you could introduce me to them. Thanks! P.S. |
…rpc#8822) ## What This PR enables Auto Host Rewriting for Logical_DNS clusters by setting DNS Hostname as an Endpoint Attribute. ## Why With current grpc-go's gRFC A81 implementation, Auto Host Rewriting is not enabled even when a Route config enables AutoHostRewrite and GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE is set to be true when a Cluster config uses Logical DNS (current implementation seems supports only EDS). RELEASE NOTES: - xDS: Added support for :authority rewriting (gRFC A81) for LOGICAL_DNS clusters
3 participants
What
This PR enables Auto Host Rewriting for Logical_DNS clusters by setting DNS Hostname as an Endpoint Attribute.
Why
With current grpc-go's gRFC A81 implementation, Auto Host Rewriting is not enabled even when a Route config enables AutoHostRewrite and GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE is set to be true when a Cluster config uses Logical DNS (current implementation seems supports only EDS).
RELEASE NOTES: