xds bootstrap: support insecure and make Creds required (#3881)
diff --git a/xds/internal/balancer/edsbalancer/xds_client_wrapper.go b/xds/internal/balancer/edsbalancer/xds_client_wrapper.go
index 06b4584..dd66a41 100644
--- a/xds/internal/balancer/edsbalancer/xds_client_wrapper.go
+++ b/xds/internal/balancer/edsbalancer/xds_client_wrapper.go
@@ -141,14 +141,6 @@
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 @@
}
}
-// 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 b2805bf..e3f2ce5 100644
--- a/xds/internal/client/bootstrap/bootstrap.go
+++ b/xds/internal/client/bootstrap/bootstrap.go
@@ -47,7 +47,8 @@
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 @@
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 @@
}
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 18c28db..a7734f0 100644
--- a/xds/internal/client/bootstrap/bootstrap_test.go
+++ b/xds/internal/client/bootstrap/bootstrap_test.go
@@ -38,7 +38,10 @@
"emptyNodeProto": `
{
"xds_servers" : [{
- "server_uri": "trafficdirector.googleapis.com:443"
+ "server_uri": "trafficdirector.googleapis.com:443",
+ "channel_creds": [
+ { "type": "insecure" }
+ ]
}]
}`,
"unknownTopLevelFieldInFile": `
@@ -52,7 +55,7 @@
"xds_servers" : [{
"server_uri": "trafficdirector.googleapis.com:443",
"channel_creds": [
- { "type": "not-google-default" }
+ { "type": "insecure" }
]
}],
"unknownField": "foobar"
@@ -67,7 +70,10 @@
}
},
"xds_servers" : [{
- "server_uri": "trafficdirector.googleapis.com:443"
+ "server_uri": "trafficdirector.googleapis.com:443",
+ "channel_creds": [
+ { "type": "insecure" }
+ ]
}]
}`,
"unknownFieldInXdsServer": `
@@ -81,38 +87,11 @@
"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 @@
}
nilCredsConfigV2 = &Config{
BalancerName: "trafficdirector.googleapis.com:443",
- Creds: nil,
+ Creds: grpc.WithInsecure(),
NodeProto: v2NodeProto,
}
nonNilCredsConfigV2 = &Config{
@@ -290,6 +269,33 @@
}
}
}`,
+ "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 @@
{
"emptyNodeProto", &Config{
BalancerName: "trafficdirector.googleapis.com:443",
+ Creds: grpc.WithInsecure(),
NodeProto: &v2corepb.Node{
BuildVersion: gRPCVersion,
UserAgentName: gRPCUserAgentName,
@@ -342,8 +349,6 @@
{"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 02bf19c..359ee12 100644
--- a/xds/internal/resolver/xds_resolver.go
+++ b/xds/internal/resolver/xds_resolver.go
@@ -68,15 +68,6 @@
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)}
@@ -100,28 +91,6 @@
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 9d04e3c..7eb0629 100644
--- a/xds/internal/resolver/xds_resolver_test.go
+++ b/xds/internal/resolver/xds_resolver_test.go
@@ -113,7 +113,7 @@
// 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))
@@ -159,7 +159,7 @@
NodeProto: xdstestutils.EmptyNodeProtoV2,
},
xdsClientFunc: getXDSClientMakerFunc(xdsclient.Options{Config: validConfig}),
- wantErr: false,
+ wantErr: true,
},
{
name: "error-dialer-in-rbo",
