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.

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

xds: use google default creds #3673

Merged
merged 10 commits into from
Jun 11, 2020
40 changes: 38 additions & 2 deletions credentials/oauth/oauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion xds/internal/client/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
21 changes: 8 additions & 13 deletions xds/internal/client/bootstrap/bootstrap_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build !appengine
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is disabled for appengine, because of https://travis-ci.org/github/grpc/grpc-go/jobs/697000659#L525


/*
*
* Copyright 2019 gRPC authors.
Expand All @@ -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 (
Expand Down Expand Up @@ -58,19 +58,14 @@ var (
}
)

type s struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we please add a TODO here to re-add the leak check once the leak is fixed upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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]`,
Expand Down Expand Up @@ -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 {
Expand Down