From 9d981b0eb01a1ccd61f16593461277e95e83a34b Mon Sep 17 00:00:00 2001 From: Arvind Bright Date: Mon, 4 Mar 2024 13:23:19 -0800 Subject: [PATCH] cherry-pick #6997 to 1.62.x release branch (#6979) (#7018) --- clientconn.go | 2 + internal/testutils/xds/e2e/clientresources.go | 4 +- resolver/resolver.go | 3 + resolver_wrapper.go | 1 + test/xds/xds_client_federation_test.go | 85 +++++++++++++++++++ xds/internal/resolver/helpers_test.go | 5 +- xds/internal/resolver/xds_resolver.go | 19 +++-- 7 files changed, 107 insertions(+), 12 deletions(-) diff --git a/clientconn.go b/clientconn.go index f6e815e6bfc..f0b7f3200f1 100644 --- a/clientconn.go +++ b/clientconn.go @@ -1772,6 +1772,8 @@ func parseTarget(target string) (resolver.Target, error) { return resolver.Target{URL: *u}, nil } +// encodeAuthority escapes the authority string based on valid chars defined in +// https://datatracker.ietf.org/doc/html/rfc3986#section-3.2. func encodeAuthority(authority string) string { const upperhex = "0123456789ABCDEF" diff --git a/internal/testutils/xds/e2e/clientresources.go b/internal/testutils/xds/e2e/clientresources.go index 347ea804231..175490e91cf 100644 --- a/internal/testutils/xds/e2e/clientresources.go +++ b/internal/testutils/xds/e2e/clientresources.go @@ -365,11 +365,11 @@ func HTTPFilter(name string, config proto.Message) *v3httppb.HttpFilter { } // DefaultRouteConfig returns a basic xds RouteConfig resource. -func DefaultRouteConfig(routeName, ldsTarget, clusterName string) *v3routepb.RouteConfiguration { +func DefaultRouteConfig(routeName, vhDomain, clusterName string) *v3routepb.RouteConfiguration { return &v3routepb.RouteConfiguration{ Name: routeName, VirtualHosts: []*v3routepb.VirtualHost{{ - Domains: []string{ldsTarget}, + Domains: []string{vhDomain}, Routes: []*v3routepb.Route{{ Match: &v3routepb.RouteMatch{PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/"}}, Action: &v3routepb.Route_Route{Route: &v3routepb.RouteAction{ diff --git a/resolver/resolver.go b/resolver/resolver.go index adf89dd9cfe..d72f21c1b32 100644 --- a/resolver/resolver.go +++ b/resolver/resolver.go @@ -168,6 +168,9 @@ type BuildOptions struct { // field. In most cases though, it is not appropriate, and this field may // be ignored. Dialer func(context.Context, string) (net.Conn, error) + // Authority is the effective authority of the clientconn for which the + // resolver is built. + Authority string } // An Endpoint is one network endpoint, or server, which may have multiple diff --git a/resolver_wrapper.go b/resolver_wrapper.go index c79bab12149..f845ac95893 100644 --- a/resolver_wrapper.go +++ b/resolver_wrapper.go @@ -75,6 +75,7 @@ func (ccr *ccResolverWrapper) start() error { DialCreds: ccr.cc.dopts.copts.TransportCredentials, CredsBundle: ccr.cc.dopts.copts.CredsBundle, Dialer: ccr.cc.dopts.copts.Dialer, + Authority: ccr.cc.authority, } var err error ccr.resolver, err = ccr.cc.resolverBuilder.Build(ccr.cc.parsedTarget, ccr, opts) diff --git a/test/xds/xds_client_federation_test.go b/test/xds/xds_client_federation_test.go index 75dd77ba195..e826d443b97 100644 --- a/test/xds/xds_client_federation_test.go +++ b/test/xds/xds_client_federation_test.go @@ -140,6 +140,91 @@ func (s) TestClientSideFederation(t *testing.T) { } } +// TestClientSideFederationWithOnlyXDSTPStyleLDS tests that federation is +// supported with new xdstp style names for LDS only while using the old style +// for other resources. This test in addition also checks that when service name +// contains escapable characters, we "fully" encode it for looking up +// VirtualHosts in xDS RouteConfigurtion. +func (s) TestClientSideFederationWithOnlyXDSTPStyleLDS(t *testing.T) { + // Start a management server as a sophisticated authority. + const authority = "traffic-manager.xds.notgoogleapis.com" + mgmtServer, err := e2e.StartManagementServer(e2e.ManagementServerOptions{}) + if err != nil { + t.Fatalf("Failed to spin up the xDS management server: %v", err) + } + t.Cleanup(mgmtServer.Stop) + + // Create a bootstrap file in a temporary directory. + nodeID := uuid.New().String() + bootstrapContents, err := bootstrap.Contents(bootstrap.Options{ + NodeID: nodeID, + ServerURI: mgmtServer.Address, + ClientDefaultListenerResourceNameTemplate: fmt.Sprintf("xdstp://%s/envoy.config.listener.v3.Listener/%%s", authority), + Authorities: map[string]string{authority: mgmtServer.Address}, + }) + if err != nil { + t.Fatalf("Failed to create bootstrap file: %v", err) + } + + resolverBuilder := internal.NewXDSResolverWithConfigForTesting.(func([]byte) (resolver.Builder, error)) + resolver, err := resolverBuilder(bootstrapContents) + if err != nil { + t.Fatalf("Failed to create xDS resolver for testing: %v", err) + } + server := stubserver.StartTestService(t, nil) + defer server.Stop() + + // serviceName with escapable characters - ' ', and '/'. + const serviceName = "my-service-client-side-xds/2nd component" + + // All other resources are with old style name. + const rdsName = "route-" + serviceName + const cdsName = "cluster-" + serviceName + const edsName = "endpoints-" + serviceName + + // Resource update sent to go-control-plane mgmt server. + resourceUpdate := e2e.UpdateOptions{ + NodeID: nodeID, + Listeners: func() []*v3listenerpb.Listener { + // LDS is new style xdstp name. Since the LDS resource name is prefixed + // with xdstp, the string will be %-encoded excluding '/'s. See + // bootstrap.PopulateResourceTemplate(). + const specialEscapedServiceName = "my-service-client-side-xds/2nd%20component" // same as bootstrap.percentEncode(serviceName) + ldsName := fmt.Sprintf("xdstp://%s/envoy.config.listener.v3.Listener/%s", authority, specialEscapedServiceName) + return []*v3listenerpb.Listener{e2e.DefaultClientListener(ldsName, rdsName)} + }(), + Routes: func() []*v3routepb.RouteConfiguration { + // RouteConfiguration will has one entry in []VirutalHosts that contains the + // "fully" escaped service name in []Domains. This is to assert that gRPC + // uses the escaped service name to lookup VirtualHosts. RDS is also with + // old style name. + const fullyEscapedServiceName = "my-service-client-side-xds%2F2nd%20component" // same as url.PathEscape(serviceName) + return []*v3routepb.RouteConfiguration{e2e.DefaultRouteConfig(rdsName, fullyEscapedServiceName, cdsName)} + }(), + Clusters: []*v3clusterpb.Cluster{e2e.DefaultCluster(cdsName, edsName, e2e.SecurityLevelNone)}, + Endpoints: []*v3endpointpb.ClusterLoadAssignment{e2e.DefaultEndpoint(edsName, "localhost", []uint32{testutils.ParsePort(t, server.Address)})}, + SkipValidation: true, + } + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := mgmtServer.Update(ctx, resourceUpdate); err != nil { + t.Fatal(err) + } + + // Create a ClientConn and make a successful RPC. + cc, err := grpc.Dial(fmt.Sprintf("xds:///%s", serviceName), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(resolver)) + if err != nil { + t.Fatalf("failed to dial local test server: %v", err) + } + defer cc.Close() + + client := testgrpc.NewTestServiceClient(cc) + if _, err := client.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true)); err != nil { + t.Fatalf("rpc EmptyCall() failed: %v", err) + } +} + // TestFederation_UnknownAuthorityInDialTarget tests the case where a ClientConn // is created with a dial target containing an authority which is not specified // in the bootstrap configuration. The test verifies that RPCs on the ClientConn diff --git a/xds/internal/resolver/helpers_test.go b/xds/internal/resolver/helpers_test.go index d30ea10995a..e15982a46eb 100644 --- a/xds/internal/resolver/helpers_test.go +++ b/xds/internal/resolver/helpers_test.go @@ -21,6 +21,7 @@ package resolver_test import ( "context" "fmt" + "net/url" "strings" "testing" "time" @@ -104,7 +105,9 @@ func buildResolverForTarget(t *testing.T, target resolver.Target) (chan resolver } } tcc := &testutils.ResolverClientConn{Logger: t, UpdateStateF: updateStateF, ReportErrorF: reportErrorF} - r, err := builder.Build(target, tcc, resolver.BuildOptions{}) + r, err := builder.Build(target, tcc, resolver.BuildOptions{ + Authority: url.PathEscape(target.Endpoint()), + }) if err != nil { t.Fatalf("Failed to build xDS resolver for target %q: %v", target, err) } diff --git a/xds/internal/resolver/xds_resolver.go b/xds/internal/resolver/xds_resolver.go index d1b3195dd1f..6d1966230c6 100644 --- a/xds/internal/resolver/xds_resolver.go +++ b/xds/internal/resolver/xds_resolver.go @@ -22,7 +22,6 @@ package resolver import ( "context" "fmt" - "strings" "sync/atomic" "google.golang.org/grpc/internal" @@ -114,12 +113,8 @@ func (b *xdsResolverBuilder) Build(target resolver.Target, cc resolver.ClientCon if err != nil { return nil, err } - endpoint := target.URL.Path - if endpoint == "" { - endpoint = target.URL.Opaque - } - endpoint = strings.TrimPrefix(endpoint, "/") - r.ldsResourceName = bootstrap.PopulateResourceTemplate(template, endpoint) + r.dataplaneAuthority = opts.Authority + r.ldsResourceName = bootstrap.PopulateResourceTemplate(template, target.Endpoint()) r.listenerWatcher = newListenerWatcher(r.ldsResourceName, r) return r, nil } @@ -190,6 +185,12 @@ type xdsResolver struct { serializer *grpcsync.CallbackSerializer serializerCancel context.CancelFunc + // dataplaneAuthority is the authority used for the data plane connections, + // which is also used to select the VirtualHost within the xDS + // RouteConfiguration. This is %-encoded to match with VirtualHost Domain + // in xDS RouteConfiguration. + dataplaneAuthority string + ldsResourceName string listenerWatcher *listenerWatcher listenerUpdateRecvd bool @@ -413,9 +414,9 @@ func (r *xdsResolver) onResolutionComplete() { } func (r *xdsResolver) applyRouteConfigUpdate(update xdsresource.RouteConfigUpdate) { - matchVh := xdsresource.FindBestMatchingVirtualHost(r.ldsResourceName, update.VirtualHosts) + matchVh := xdsresource.FindBestMatchingVirtualHost(r.dataplaneAuthority, update.VirtualHosts) if matchVh == nil { - r.onError(fmt.Errorf("no matching virtual host found for %q", r.ldsResourceName)) + r.onError(fmt.Errorf("no matching virtual host found for %q", r.dataplaneAuthority)) return } r.currentRouteConfig = update