Skip to content

Commit b6d6ede

Browse files
authored
xds: use google default creds (#3673)
- use google default creds, so the client works not only on GCE (e.g. it also reads env variable for creds). - Change google default creds to use jwt directly if scope is not set. - Leak check is disabled temporarily due to googleapis/google-cloud-go#2417
1 parent eb11ffd commit b6d6ede

File tree

3 files changed

+47
-16
lines changed

3 files changed

+47
-16
lines changed

credentials/oauth/oauth.go

+38-2
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ func NewJWTAccessFromKey(jsonKey []byte) (credentials.PerRPCCredentials, error)
7474
}
7575

7676
func (j jwtAccess) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) {
77+
// TODO: the returned TokenSource is reusable. Store it in a sync.Map, with
78+
// uri as the key, to avoid recreating for every RPC.
7779
ts, err := google.JWTAccessTokenSourceFromJSON(j.jsonKey, uri[0])
7880
if err != nil {
7981
return nil, err
@@ -177,9 +179,43 @@ func NewServiceAccountFromFile(keyFile string, scope ...string) (credentials.Per
177179
// NewApplicationDefault returns "Application Default Credentials". For more
178180
// detail, see https://developers.google.com/accounts/docs/application-default-credentials.
179181
func NewApplicationDefault(ctx context.Context, scope ...string) (credentials.PerRPCCredentials, error) {
180-
t, err := google.DefaultTokenSource(ctx, scope...)
182+
creds, err := google.FindDefaultCredentials(ctx, scope...)
181183
if err != nil {
182184
return nil, err
183185
}
184-
return TokenSource{t}, nil
186+
187+
// If JSON is nil, the authentication is provided by the environment and not
188+
// with a credentials file, e.g. when code is running on Google Cloud
189+
// Platform. Use the returned token source.
190+
if creds.JSON == nil {
191+
return TokenSource{creds.TokenSource}, nil
192+
}
193+
194+
// If auth is provided by env variable or creds file, the behavior will be
195+
// different based on whether scope is set. Because the returned
196+
// creds.TokenSource does oauth with jwt by default, and it requires scope.
197+
// We can only use it if scope is not empty, otherwise it will fail with
198+
// missing scope error.
199+
//
200+
// If scope is set, use it, it should just work.
201+
//
202+
// If scope is not set, we try to use jwt directly without oauth (this only
203+
// works if it's a service account).
204+
205+
if len(scope) != 0 {
206+
return TokenSource{creds.TokenSource}, nil
207+
}
208+
209+
// Try to convert JSON to a jwt config without setting the optional scope
210+
// parameter to check if it's a service account (the function errors if it's
211+
// not). This is necessary because the returned config doesn't show the type
212+
// of the account.
213+
if _, err := google.JWTConfigFromJSON(creds.JSON); err != nil {
214+
// If this fails, it's not a service account, return the original
215+
// TokenSource from above.
216+
return TokenSource{creds.TokenSource}, nil
217+
}
218+
219+
// If it's a service account, create a JWT only access with the key.
220+
return NewJWTAccessFromKey(creds.JSON)
185221
}

xds/internal/client/bootstrap/bootstrap.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func NewConfig() (*Config, error) {
139139
config.BalancerName = xs.ServerURI
140140
for _, cc := range xs.ChannelCreds {
141141
if cc.Type == googleDefaultCreds {
142-
config.Creds = grpc.WithCredentialsBundle(google.NewComputeEngineCredentials())
142+
config.Creds = grpc.WithCredentialsBundle(google.NewDefaultCredentials())
143143
// We stop at the first credential type that we support.
144144
break
145145
}

xds/internal/client/bootstrap/bootstrap_test.go

+8-13
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// +build !appengine
2+
13
/*
24
*
35
* Copyright 2019 gRPC authors.
@@ -22,13 +24,11 @@ import (
2224
"os"
2325
"testing"
2426

27+
corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
2528
"github.com/golang/protobuf/proto"
29+
structpb "github.com/golang/protobuf/ptypes/struct"
2630
"google.golang.org/grpc"
2731
"google.golang.org/grpc/credentials/google"
28-
"google.golang.org/grpc/internal/grpctest"
29-
30-
corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
31-
structpb "github.com/golang/protobuf/ptypes/struct"
3232
)
3333

3434
var (
@@ -58,19 +58,14 @@ var (
5858
}
5959
)
6060

61-
type s struct {
62-
grpctest.Tester
63-
}
64-
65-
func Test(t *testing.T) {
66-
grpctest.RunSubTests(t, s{})
67-
}
61+
// TODO: enable leak check for this package when
62+
// https://github.com/googleapis/google-cloud-go/issues/2417 is fixed.
6863

6964
// TestNewConfig exercises the functionality in NewConfig with different
7065
// bootstrap file contents. It overrides the fileReadFunc by returning
7166
// bootstrap file contents defined in this test, instead of reading from a
7267
// file.
73-
func (s) TestNewConfig(t *testing.T) {
68+
func TestNewConfig(t *testing.T) {
7469
bootstrapFileMap := map[string]string{
7570
"empty": "",
7671
"badJSON": `["test": 123]`,
@@ -283,7 +278,7 @@ func (s) TestNewConfig(t *testing.T) {
283278
}
284279
}
285280

286-
func (s) TestNewConfigEnvNotSet(t *testing.T) {
281+
func TestNewConfigEnvNotSet(t *testing.T) {
287282
os.Unsetenv(fileEnv)
288283
config, err := NewConfig()
289284
if err == nil {

0 commit comments

Comments
 (0)