diff --git a/credentials/oauth/oauth.go b/credentials/oauth/oauth.go index 899e3372ce3c..6657055d6609 100644 --- a/credentials/oauth/oauth.go +++ b/credentials/oauth/oauth.go @@ -74,6 +74,8 @@ func NewJWTAccessFromKey(jsonKey []byte) (credentials.PerRPCCredentials, error) } func (j jwtAccess) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { + // TODO: the returned TokenSource is reusable. Store it in a sync.Map, with + // uri as the key, to avoid recreating for every RPC. ts, err := google.JWTAccessTokenSourceFromJSON(j.jsonKey, uri[0]) if err != nil { return nil, err @@ -177,9 +179,43 @@ func NewServiceAccountFromFile(keyFile string, scope ...string) (credentials.Per // NewApplicationDefault returns "Application Default Credentials". For more // detail, see https://developers.google.com/accounts/docs/application-default-credentials. func NewApplicationDefault(ctx context.Context, scope ...string) (credentials.PerRPCCredentials, error) { - t, err := google.DefaultTokenSource(ctx, scope...) + creds, err := google.FindDefaultCredentials(ctx, scope...) if err != nil { return nil, err } - return TokenSource{t}, nil + + // If JSON is nil, the authentication is provided by the environment and not + // with a credentials file, e.g. when code is running on Google Cloud + // Platform. Use the returned token source. + if creds.JSON == nil { + return TokenSource{creds.TokenSource}, nil + } + + // If auth is provided by env variable or creds file, the behavior will be + // different based on whether scope is set. Because the returned + // creds.TokenSource does oauth with jwt by default, and it requires scope. + // We can only use it if scope is not empty, otherwise it will fail with + // missing scope error. + // + // If scope is set, use it, it should just work. + // + // If scope is not set, we try to use jwt directly without oauth (this only + // works if it's a service account). + + if len(scope) != 0 { + return TokenSource{creds.TokenSource}, nil + } + + // Try to convert JSON to a jwt config without setting the optional scope + // parameter to check if it's a service account (the function errors if it's + // not). This is necessary because the returned config doesn't show the type + // of the account. + if _, err := google.JWTConfigFromJSON(creds.JSON); err != nil { + // If this fails, it's not a service account, return the original + // TokenSource from above. + return TokenSource{creds.TokenSource}, nil + } + + // If it's a service account, create a JWT only access with the key. + return NewJWTAccessFromKey(creds.JSON) } diff --git a/xds/internal/client/bootstrap/bootstrap.go b/xds/internal/client/bootstrap/bootstrap.go index a9db93b15461..1ee0f1d47c5e 100644 --- a/xds/internal/client/bootstrap/bootstrap.go +++ b/xds/internal/client/bootstrap/bootstrap.go @@ -139,7 +139,7 @@ func NewConfig() (*Config, error) { config.BalancerName = xs.ServerURI for _, cc := range xs.ChannelCreds { if cc.Type == googleDefaultCreds { - config.Creds = grpc.WithCredentialsBundle(google.NewComputeEngineCredentials()) + config.Creds = grpc.WithCredentialsBundle(google.NewDefaultCredentials()) // We stop at the first credential type that we support. break } diff --git a/xds/internal/client/bootstrap/bootstrap_test.go b/xds/internal/client/bootstrap/bootstrap_test.go index d3dedb0454d4..de0ef3954e72 100644 --- a/xds/internal/client/bootstrap/bootstrap_test.go +++ b/xds/internal/client/bootstrap/bootstrap_test.go @@ -1,3 +1,5 @@ +// +build !appengine + /* * * Copyright 2019 gRPC authors. @@ -22,13 +24,11 @@ import ( "os" "testing" + corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" "github.com/golang/protobuf/proto" + structpb "github.com/golang/protobuf/ptypes/struct" "google.golang.org/grpc" "google.golang.org/grpc/credentials/google" - "google.golang.org/grpc/internal/grpctest" - - corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" - structpb "github.com/golang/protobuf/ptypes/struct" ) var ( @@ -58,19 +58,14 @@ var ( } ) -type s struct { - grpctest.Tester -} - -func Test(t *testing.T) { - grpctest.RunSubTests(t, s{}) -} +// TODO: enable leak check for this package when +// https://github.com/googleapis/google-cloud-go/issues/2417 is fixed. // TestNewConfig exercises the functionality in NewConfig with different // bootstrap file contents. It overrides the fileReadFunc by returning // bootstrap file contents defined in this test, instead of reading from a // file. -func (s) TestNewConfig(t *testing.T) { +func TestNewConfig(t *testing.T) { bootstrapFileMap := map[string]string{ "empty": "", "badJSON": `["test": 123]`, @@ -283,7 +278,7 @@ func (s) TestNewConfig(t *testing.T) { } } -func (s) TestNewConfigEnvNotSet(t *testing.T) { +func TestNewConfigEnvNotSet(t *testing.T) { os.Unsetenv(fileEnv) config, err := NewConfig() if err == nil {