[v1.40.x] creds/google: replace NewComputeEngineCredsWithOptions with…

… NewDefaultCredentialsWithOptions (grpc#4830)
menghanl · Nov 30, 2021 · a82a483 · a82a483
1 parent 50b588d
commit a82a483
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 52 deletions.
diff --git a/credentials/google/google.go b/credentials/google/google.go
@@ -35,75 +35,63 @@ const tokenRequestTimeout = 30 * time.Second
 
 var logger = grpclog.Component("credentials")
 
-// NewDefaultCredentials returns a credentials bundle that is configured to work
-// with google services.
+// DefaultCredentialsOptions constructs options to build DefaultCredentials.
+type DefaultCredentialsOptions struct {
+	// PerRPCCreds is a per RPC credentials that is passed to a bundle.
+	PerRPCCreds credentials.PerRPCCredentials
+}
+
+// NewDefaultCredentialsWithOptions returns a credentials bundle that is
+// configured to work with google services.
 //
 // This API is experimental.
-func NewDefaultCredentials() credentials.Bundle {
-	c := &creds{
-		newPerRPCCreds: func() credentials.PerRPCCredentials {
-			ctx, cancel := context.WithTimeout(context.Background(), tokenRequestTimeout)
-			defer cancel()
-			perRPCCreds, err := oauth.NewApplicationDefault(ctx)
-			if err != nil {
-				logger.Warningf("google default creds: failed to create application oauth: %v", err)
-			}
-			return perRPCCreds
-		},
+func NewDefaultCredentialsWithOptions(opts DefaultCredentialsOptions) credentials.Bundle {
+	if opts.PerRPCCreds == nil {
+		ctx, cancel := context.WithTimeout(context.Background(), tokenRequestTimeout)
+		defer cancel()
+		var err error
+		opts.PerRPCCreds, err = oauth.NewApplicationDefault(ctx)
+		if err != nil {
+			logger.Warningf("NewDefaultCredentialsWithOptions: failed to create application oauth: %v", err)
+		}
 	}
+	c := &creds{opts: opts}
 	bundle, err := c.NewWithMode(internal.CredsBundleModeFallback)
 	if err != nil {
-		logger.Warningf("google default creds: failed to create new creds: %v", err)
+		logger.Warningf("NewDefaultCredentialsWithOptions: failed to create new creds: %v", err)
 	}
 	return bundle
 }
 
-// NewComputeEngineCredentials returns a credentials bundle that is configured to work
-// with google services. This API must only be used when running on GCE. Authentication configured
-// by this API represents the GCE VM's default service account.
+// NewDefaultCredentials returns a credentials bundle that is configured to work
+// with google services.
 //
 // This API is experimental.
-func NewComputeEngineCredentials() credentials.Bundle {
-	return NewComputeEngineCredsWithOptions(ComputeEngineCredsOptions{})
-}
-
-// ComputeEngineCredsOptions constructs compite engine credentials with options.
-type ComputeEngineCredsOptions struct {
-	// PerRPCCreds is a  per RPC credentials that is passed to a bundle.
-	PerRPCCreds credentials.PerRPCCredentials
+func NewDefaultCredentials() credentials.Bundle {
+	return NewDefaultCredentialsWithOptions(DefaultCredentialsOptions{})
 }
 
-// NewComputeEngineCredsWithOptions returns a credentials bundle that is configured to work
-// with google services. This API must only be used when running on GCE.
+// NewComputeEngineCredentials returns a credentials bundle that is configured to work
+// with google services. This API must only be used when running on GCE. Authentication configured
+// by this API represents the GCE VM's default service account.
 //
 // This API is experimental.
-func NewComputeEngineCredsWithOptions(perRPCOpts ComputeEngineCredsOptions) credentials.Bundle {
-	perRPC := oauth.NewComputeEngine()
-	if perRPCOpts.PerRPCCreds != nil {
-		perRPC = perRPCOpts.PerRPCCreds
-	}
-	c := &creds{
-		newPerRPCCreds: func() credentials.PerRPCCredentials {
-			return perRPC
-		},
-	}
-	bundle, err := c.NewWithMode(internal.CredsBundleModeFallback)
-	if err != nil {
-		logger.Warningf("compute engine creds with per rpc: failed to create new creds: %v", err)
-	}
-	return bundle
+func NewComputeEngineCredentials() credentials.Bundle {
+	return NewDefaultCredentialsWithOptions(DefaultCredentialsOptions{
+		PerRPCCreds: oauth.NewComputeEngine(),
+	})
 }
 
 // creds implements credentials.Bundle.
 type creds struct {
+	opts DefaultCredentialsOptions
+
 	// Supported modes are defined in internal/internal.go.
 	mode string
-	// The transport credentials associated with this bundle.
+	// The active transport credentials associated with this bundle.
 	transportCreds credentials.TransportCredentials
-	// The per RPC credentials associated with this bundle.
+	// The active per RPC credentials associated with this bundle.
 	perRPCCreds credentials.PerRPCCredentials
-	// Creates new per RPC credentials
-	newPerRPCCreds func() credentials.PerRPCCredentials
 }
 
 func (c *creds) TransportCredentials() credentials.TransportCredentials {
@@ -130,8 +118,8 @@ var (
 // existing Bundle may cause races.
 func (c *creds) NewWithMode(mode string) (credentials.Bundle, error) {
 	newCreds := &creds{
-		mode:           mode,
-		newPerRPCCreds: c.newPerRPCCreds,
+		opts: c.opts,
+		mode: mode,
 	}
 
 	// Create transport credentials.
@@ -147,7 +135,7 @@ func (c *creds) NewWithMode(mode string) (credentials.Bundle, error) {
 	}
 
 	if mode == internal.CredsBundleModeFallback || mode == internal.CredsBundleModeBackendFromBalancer {
-		newCreds.perRPCCreds = newCreds.newPerRPCCreds()
+		newCreds.perRPCCreds = newCreds.opts.PerRPCCreds
 	}
 
 	return newCreds, nil

diff --git a/credentials/google/google_test.go b/credentials/google/google_test.go
@@ -76,9 +76,9 @@ func overrideNewCredsFuncs() func() {
 func TestClientHandshakeBasedOnClusterName(t *testing.T) {
 	defer overrideNewCredsFuncs()()
 	for bundleTyp, tc := range map[string]credentials.Bundle{
-		"defaultCreds":       NewDefaultCredentials(),
-		"computeCreds":       NewComputeEngineCredentials(),
-		"computeCredsPerRPC": NewComputeEngineCredsWithOptions(ComputeEngineCredsOptions{}),
+		"defaultCredsWithOptions": NewDefaultCredentialsWithOptions(DefaultCredentialsOptions{}),
+		"defaultCreds":            NewDefaultCredentials(),
+		"computeCreds":            NewComputeEngineCredentials(),
 	} {
 		tests := []struct {
 			name    string