diff --git a/xds/internal/balancer/edsbalancer/xds_client_wrapper.go b/xds/internal/balancer/edsbalancer/xds_client_wrapper.go index 06b45847e0b..dd66a4160a7 100644 --- a/xds/internal/balancer/edsbalancer/xds_client_wrapper.go +++ b/xds/internal/balancer/edsbalancer/xds_client_wrapper.go @@ -141,14 +141,6 @@ func (c *xdsclientWrapper) updateXDSClient(config *EDSConfig, attr *attributes.A return false } - if clientConfig.Creds == nil { - // TODO: Once we start supporting a mechanism to register credential - // types, a failure to find the credential type mentioned in the - // bootstrap file should result in a failure, and not in using - // credentials from the parent channel (passed through the - // resolver.BuildOptions). - clientConfig.Creds = c.defaultDialCreds(clientConfig.BalancerName) - } var dopts []grpc.DialOption if dialer := c.bbo.Dialer; dialer != nil { dopts = []grpc.DialOption{grpc.WithContextDialer(dialer)} @@ -281,28 +273,6 @@ func (c *xdsclientWrapper) close() { } } -// defaultDialCreds builds a DialOption containing the credentials to be used -// while talking to the xDS server (this is done only if the xds bootstrap -// process does not return any credentials to use). If the parent channel -// contains DialCreds, we use it as is. If it contains a CredsBundle, we use -// just the transport credentials from the bundle. If we don't find any -// credentials on the parent channel, we resort to using an insecure channel. -func (c *xdsclientWrapper) defaultDialCreds(balancerName string) grpc.DialOption { - switch { - case c.bbo.DialCreds != nil: - if err := c.bbo.DialCreds.OverrideServerName(balancerName); err != nil { - c.logger.Warningf("xds: failed to override server name in credentials: %v, using Insecure", err) - return grpc.WithInsecure() - } - return grpc.WithTransportCredentials(c.bbo.DialCreds) - case c.bbo.CredsBundle != nil: - return grpc.WithTransportCredentials(c.bbo.CredsBundle.TransportCredentials()) - default: - c.logger.Warningf("xds: no credentials available, using Insecure") - return grpc.WithInsecure() - } -} - // equalStringPointers returns true if // - a and b are both nil OR // - *a == *b (and a and b are both non-nil) diff --git a/xds/internal/client/bootstrap/bootstrap.go b/xds/internal/client/bootstrap/bootstrap.go index b2805bf7372..e3f2ce59dfb 100644 --- a/xds/internal/client/bootstrap/bootstrap.go +++ b/xds/internal/client/bootstrap/bootstrap.go @@ -47,7 +47,8 @@ const ( serverFeaturesV3 = "xds_v3" // Type name for Google default credentials. - googleDefaultCreds = "google_default" + credsGoogleDefault = "google_default" + credsInsecure = "insecure" gRPCUserAgentName = "gRPC Go" clientFeatureNoOverprovisioning = "envoy.lb.does_not_support_overprovisioning" ) @@ -161,9 +162,12 @@ func NewConfig() (*Config, error) { xs := servers[0] config.BalancerName = xs.ServerURI for _, cc := range xs.ChannelCreds { - if cc.Type == googleDefaultCreds { + // We stop at the first credential type that we support. + if cc.Type == credsGoogleDefault { config.Creds = grpc.WithCredentialsBundle(google.NewDefaultCredentials()) - // We stop at the first credential type that we support. + break + } else if cc.Type == credsInsecure { + config.Creds = grpc.WithInsecure() break } } @@ -185,7 +189,10 @@ func NewConfig() (*Config, error) { } if config.BalancerName == "" { - return nil, fmt.Errorf("xds: Required field %q not found in bootstrap", "xds_servers.server_uri") + return nil, fmt.Errorf("xds: Required field %q not found in bootstrap %s", "xds_servers.server_uri", jsonData["xds_servers"]) + } + if config.Creds == nil { + return nil, fmt.Errorf("xds: Required field %q doesn't contain valid value in bootstrap %s", "xds_servers.channel_creds", jsonData["xds_servers"]) } // We end up using v3 transport protocol version only if the following diff --git a/xds/internal/client/bootstrap/bootstrap_test.go b/xds/internal/client/bootstrap/bootstrap_test.go index 18c28db346b..a7734f03436 100644 --- a/xds/internal/client/bootstrap/bootstrap_test.go +++ b/xds/internal/client/bootstrap/bootstrap_test.go @@ -38,7 +38,10 @@ var ( "emptyNodeProto": ` { "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443" + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ + { "type": "insecure" } + ] }] }`, "unknownTopLevelFieldInFile": ` @@ -52,7 +55,7 @@ var ( "xds_servers" : [{ "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ - { "type": "not-google-default" } + { "type": "insecure" } ] }], "unknownField": "foobar" @@ -67,7 +70,10 @@ var ( } }, "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443" + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ + { "type": "insecure" } + ] }] }`, "unknownFieldInXdsServer": ` @@ -81,38 +87,11 @@ var ( "xds_servers" : [{ "server_uri": "trafficdirector.googleapis.com:443", "channel_creds": [ - { "type": "not-google-default" } + { "type": "insecure" } ], "unknownField": "foobar" }] }`, - "emptyChannelCreds": ` - { - "node": { - "id": "ENVOY_NODE_ID", - "metadata": { - "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" - } - }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443" - }] - }`, - "nonGoogleDefaultCreds": ` - { - "node": { - "id": "ENVOY_NODE_ID", - "metadata": { - "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" - } - }, - "xds_servers" : [{ - "server_uri": "trafficdirector.googleapis.com:443", - "channel_creds": [ - { "type": "not-google-default" } - ] - }] - }`, "multipleChannelCreds": ` { "node": { @@ -222,7 +201,7 @@ var ( } nilCredsConfigV2 = &Config{ BalancerName: "trafficdirector.googleapis.com:443", - Creds: nil, + Creds: grpc.WithInsecure(), NodeProto: v2NodeProto, } nonNilCredsConfigV2 = &Config{ @@ -290,6 +269,33 @@ func TestNewConfigV2ProtoFailure(t *testing.T) { } } }`, + "emptyChannelCreds": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443" + }] + }`, + "nonGoogleDefaultCreds": ` + { + "node": { + "id": "ENVOY_NODE_ID", + "metadata": { + "TRAFFICDIRECTOR_GRPC_HOSTNAME": "trafficdirector" + } + }, + "xds_servers" : [{ + "server_uri": "trafficdirector.googleapis.com:443", + "channel_creds": [ + { "type": "not-google-default" } + ] + }] + }`, } cancel := setupBootstrapOverride(bootstrapFileMap) defer cancel() @@ -331,6 +337,7 @@ func TestNewConfigV2ProtoSuccess(t *testing.T) { { "emptyNodeProto", &Config{ BalancerName: "trafficdirector.googleapis.com:443", + Creds: grpc.WithInsecure(), NodeProto: &v2corepb.Node{ BuildVersion: gRPCVersion, UserAgentName: gRPCUserAgentName, @@ -342,8 +349,6 @@ func TestNewConfigV2ProtoSuccess(t *testing.T) { {"unknownTopLevelFieldInFile", nilCredsConfigV2}, {"unknownFieldInNodeProto", nilCredsConfigV2}, {"unknownFieldInXdsServer", nilCredsConfigV2}, - {"emptyChannelCreds", nilCredsConfigV2}, - {"nonGoogleDefaultCreds", nilCredsConfigV2}, {"multipleChannelCreds", nonNilCredsConfigV2}, {"goodBootstrap", nonNilCredsConfigV2}, {"multipleXDSServers", nonNilCredsConfigV2}, diff --git a/xds/internal/resolver/xds_resolver.go b/xds/internal/resolver/xds_resolver.go index cdd103ef7dc..e94baafcf17 100644 --- a/xds/internal/resolver/xds_resolver.go +++ b/xds/internal/resolver/xds_resolver.go @@ -67,15 +67,6 @@ func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, rb r.logger = prefixLogger((r)) r.logger.Infof("Creating resolver for target: %+v", t) - if config.Creds == nil { - // TODO: Once we start supporting a mechanism to register credential - // types, a failure to find the credential type mentioned in the - // bootstrap file should result in a failure, and not in using - // credentials from the parent channel (passed through the - // resolver.BuildOptions). - config.Creds = r.defaultDialCreds(config.BalancerName, rbo) - } - var dopts []grpc.DialOption if rbo.Dialer != nil { dopts = []grpc.DialOption{grpc.WithContextDialer(rbo.Dialer)} @@ -98,28 +89,6 @@ func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, rb return r, nil } -// defaultDialCreds builds a DialOption containing the credentials to be used -// while talking to the xDS server (this is done only if the xds bootstrap -// process does not return any credentials to use). If the parent channel -// contains DialCreds, we use it as is. If it contains a CredsBundle, we use -// just the transport credentials from the bundle. If we don't find any -// credentials on the parent channel, we resort to using an insecure channel. -func (r *xdsResolver) defaultDialCreds(balancerName string, rbo resolver.BuildOptions) grpc.DialOption { - switch { - case rbo.DialCreds != nil: - if err := rbo.DialCreds.OverrideServerName(balancerName); err != nil { - r.logger.Errorf("Failed to override server name in credentials: %v, using Insecure", err) - return grpc.WithInsecure() - } - return grpc.WithTransportCredentials(rbo.DialCreds) - case rbo.CredsBundle != nil: - return grpc.WithTransportCredentials(rbo.CredsBundle.TransportCredentials()) - default: - r.logger.Warningf("No credentials available, using Insecure") - return grpc.WithInsecure() - } -} - // Name helps implement the resolver.Builder interface. func (*xdsResolverBuilder) Scheme() string { return xdsScheme diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index 98c6d6a2e5b..cdd7a553a2d 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -104,7 +104,7 @@ func getXDSClientMakerFunc(wantOpts xdsclient.Options) func(xdsclient.Options) ( // what the want option is. We should be able to do extensive // credential testing in e2e tests. if (gotOpts.Config.Creds != nil) != (wantOpts.Config.Creds != nil) { - return nil, fmt.Errorf("got len(creds): %s, want: %s", gotOpts.Config.Creds, wantOpts.Config.Creds) + return nil, fmt.Errorf("got len(creds): %v, want: %v", gotOpts.Config.Creds, wantOpts.Config.Creds) } if len(gotOpts.DialOpts) != len(wantOpts.DialOpts) { return nil, fmt.Errorf("got len(DialOpts): %v, want: %v", len(gotOpts.DialOpts), len(wantOpts.DialOpts)) @@ -150,7 +150,7 @@ func TestResolverBuilder(t *testing.T) { NodeProto: xdstestutils.EmptyNodeProtoV2, }, xdsClientFunc: getXDSClientMakerFunc(xdsclient.Options{Config: validConfig}), - wantErr: false, + wantErr: true, }, { name: "error-dialer-in-rbo",