Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

xds bootstrap: support insecure and make Creds required #3881

Merged
merged 1 commit into from Sep 17, 2020
Merged

xds bootstrap: support insecure and make Creds required #3881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 0 additions & 30 deletions xds/internal/balancer/edsbalancer/xds_client_wrapper.go
View file
Open in desktop
Expand Up @@ -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)}
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 11 additions & 4 deletions xds/internal/client/bootstrap/bootstrap.go
View file
Open in desktop
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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 {
easwars marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
73 changes: 39 additions & 34 deletions xds/internal/client/bootstrap/bootstrap_test.go
View file
Open in desktop
Expand Up @@ -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": `
Expand All @@ -52,7 +55,7 @@ var (
"xds_servers" : [{
"server_uri": "trafficdirector.googleapis.com:443",
"channel_creds": [
{ "type": "not-google-default" }
{ "type": "insecure" }
]
}],
"unknownField": "foobar"
Expand All @@ -67,7 +70,10 @@ var (
}
},
"xds_servers" : [{
"server_uri": "trafficdirector.googleapis.com:443"
"server_uri": "trafficdirector.googleapis.com:443",
"channel_creds": [
{ "type": "insecure" }
]
}]
}`,
"unknownFieldInXdsServer": `
Expand All @@ -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": {
Expand Down Expand Up @@ -222,7 +201,7 @@ var (
}
nilCredsConfigV2 = &Config{
BalancerName: "trafficdirector.googleapis.com:443",
Creds: nil,
Creds: grpc.WithInsecure(),
NodeProto: v2NodeProto,
}
nonNilCredsConfigV2 = &Config{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -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},
Expand Down
31 changes: 0 additions & 31 deletions xds/internal/resolver/xds_resolver.go
View file
Open in desktop
Expand Up @@ -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)}
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions xds/internal/resolver/xds_resolver_test.go
View file
Open in desktop
Expand Up @@ -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))
Expand Down Expand Up @@ -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",
Expand Down