Refactor gcp.go methods for testability, add tests

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>

Kubernetes-commit: ad4fdc7d150a26e7b4df5bdbc7e9b45570cf78c9

plugin/pkg/client/auth/gcp/gcp.go

@@ -42,8 +42,18 @@ func init() {
 	}
 }
 
-// Stubbable for testing
+var (
-var execCommand = exec.Command
+	// Stubbable for testing
+	execCommand = exec.Command
+
+	// defaultScopes:
+	// - cloud-platform is the base scope to authenticate to GCP.
+	// - userinfo.email is used to authenticate to GKE APIs with gserviceaccount
+	//   email instead of numeric uniqueID.
+	defaultScopes = []string{
+		"https://www.googleapis.com/auth/cloud-platform",
+		"https://www.googleapis.com/auth/userinfo.email"}
+)
 
 // gcpAuthProvider is an auth provider plugin that uses GCP credentials to provide
 // tokens for kubectl to authenticate itself to the apiserver. A sample json config
@@ -104,9 +114,26 @@ type gcpAuthProvider struct {
 }
 
 func newGCPAuthProvider(_ string, gcpConfig map[string]string, persister restclient.AuthProviderConfigPersister) (restclient.AuthProvider, error) {
-	var ts oauth2.TokenSource
+	ts, err := tokenSource(isCmdTokenSource(gcpConfig), gcpConfig)
-	var err error
+	if err != nil {
-	if cmd, useCmd := gcpConfig["cmd-path"]; useCmd {
+		return nil, err
+	}
+	cts, err := newCachedTokenSource(gcpConfig["access-token"], gcpConfig["expiry"], persister, ts, gcpConfig)
+	if err != nil {
+		return nil, err
+	}
+	return &gcpAuthProvider{cts, persister}, nil
+}
+
+func isCmdTokenSource(gcpConfig map[string]string) bool {
+	_, ok := gcpConfig["cmd-path"]
+	return ok
+}
+
+func tokenSource(isCmd bool, gcpConfig map[string]string) (oauth2.TokenSource, error) {
+	// Command-based token source
+	if isCmd {
+		cmd := gcpConfig["cmd-path"]
 		if len(cmd) == 0 {
 			return nil, fmt.Errorf("missing access token cmd")
 		}
@@ -121,28 +148,29 @@ func newGCPAuthProvider(_ string, gcpConfig map[string]string, persister restcli
 			cmd = fields[0]
 			args = fields[1:]
 		}
-		ts = newCmdTokenSource(cmd, args, gcpConfig["token-key"], gcpConfig["expiry-key"], gcpConfig["time-fmt"])
+		return newCmdTokenSource(cmd, args, gcpConfig["token-key"], gcpConfig["expiry-key"], gcpConfig["time-fmt"]), nil
-	} else {
-		var scopes []string
-		if gcpConfig["scopes"] != "" {
-			scopes = strings.Split(gcpConfig["scopes"], ",")
-		} else {
-			// default scopes: userinfo.email is used to authenticate to
-			// GKE APIs with gserviceaccount email instead of numeric uniqueID.
-			scopes = []string{
-				"https://www.googleapis.com/auth/cloud-platform",
-				"https://www.googleapis.com/auth/userinfo.email"}
-		}
-		ts, err = google.DefaultTokenSource(context.Background(), scopes...)
 	}
+
+	// Google Application Credentials-based token source
+	scopes := parseScopes(gcpConfig)
+	ts, err := google.DefaultTokenSource(context.Background(), scopes...)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("cannot construct google default token source: %v", err)
 	}
-	cts, err := newCachedTokenSource(gcpConfig["access-token"], gcpConfig["expiry"], persister, ts, gcpConfig)
+	return ts, nil
-	if err != nil {
+}
-		return nil, err
+
+// parseScopes constructs a list of scopes that should be included in token source
+// from the config map.
+func parseScopes(gcpConfig map[string]string) []string {
+	scopes, ok := gcpConfig["scopes"]
+	if !ok {
+		return defaultScopes
 	}
-	return &gcpAuthProvider{cts, persister}, nil
+	if scopes == "" {
+		return []string{}
+	}
+	return strings.Split(gcpConfig["scopes"], ",")
 }
 
 func (g *gcpAuthProvider) WrapTransport(rt http.RoundTripper) http.RoundTripper {

plugin/pkg/client/auth/gcp/gcp_test.go

@@ -18,6 +18,7 @@ package gcp
 
 import (
 	"fmt"
+	"io/ioutil"
 	"net/http"
 	"os"
 	"os/exec"
@@ -116,6 +117,114 @@ func TestHelperProcess(t *testing.T) {
 	os.Exit(0)
 }
 
+func Test_isCmdTokenSource(t *testing.T) {
+	c1 := map[string]string{"cmd-path": "foo"}
+	if v := isCmdTokenSource(c1); !v {
+		t.Fatalf("cmd-path present in config (%+v), but got %v", c1, v)
+	}
+
+	c2 := map[string]string{"cmd-args": "foo bar"}
+	if v := isCmdTokenSource(c2); v {
+		t.Fatalf("cmd-path not present in config (%+v), but got %v", c2, v)
+	}
+}
+
+func Test_tokenSource_cmd(t *testing.T) {
+	if _, err := tokenSource(true, map[string]string{}); err == nil {
+		t.Fatalf("expected error, cmd-args not present in config")
+	}
+
+	c := map[string]string{
+		"cmd-path": "foo",
+		"cmd-args": "bar"}
+	ts, err := tokenSource(true, c)
+	if err != nil {
+		t.Fatalf("failed to return cmd token source: %+v", err)
+	}
+	if ts == nil {
+		t.Fatal("returned nil token source")
+	}
+	if _, ok := ts.(*commandTokenSource); !ok {
+		t.Fatalf("returned token source type:(%T) expected:(*commandTokenSource)", ts)
+	}
+}
+
+func Test_tokenSource_cmdCannotBeUsedWithScopes(t *testing.T) {
+	c := map[string]string{
+		"cmd-path": "foo",
+		"scopes":   "A,B"}
+	if _, err := tokenSource(true, c); err == nil {
+		t.Fatal("expected error when scopes is used with cmd-path")
+	}
+}
+
+func Test_tokenSource_applicationDefaultCredentials_fails(t *testing.T) {
+	// try to use empty ADC file
+	fakeTokenFile, err := ioutil.TempFile("", "adctoken")
+	if err != nil {
+		t.Fatalf("failed to create fake token file: +%v", err)
+	}
+	fakeTokenFile.Close()
+	defer os.Remove(fakeTokenFile.Name())
+
+	os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", fakeTokenFile.Name())
+	defer os.Unsetenv("GOOGLE_APPLICATION_CREDENTIALS")
+	if _, err := tokenSource(false, map[string]string{}); err == nil {
+		t.Fatalf("expected error because specified ADC token file is not a JSON")
+	}
+}
+
+func Test_tokenSource_applicationDefaultCredentials(t *testing.T) {
+	fakeTokenFile, err := ioutil.TempFile("", "adctoken")
+	if err != nil {
+		t.Fatalf("failed to create fake token file: +%v", err)
+	}
+	fakeTokenFile.Close()
+	defer os.Remove(fakeTokenFile.Name())
+	if err := ioutil.WriteFile(fakeTokenFile.Name(), []byte(`{"type":"service_account"}`), 0600); err != nil {
+		t.Fatalf("failed to write to fake token file: %+v", err)
+	}
+
+	os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", fakeTokenFile.Name())
+	defer os.Unsetenv("GOOGLE_APPLICATION_CREDENTIALS")
+	ts, err := tokenSource(false, map[string]string{})
+	if err != nil {
+		t.Fatalf("failed to get a token source: %+v", err)
+	}
+	if ts == nil {
+		t.Fatal("returned nil token soruce")
+	}
+}
+
+func Test_parseScopes(t *testing.T) {
+	cases := []struct {
+		in  map[string]string
+		out []string
+	}{
+		{
+			map[string]string{},
+			[]string{
+				"https://www.googleapis.com/auth/cloud-platform",
+				"https://www.googleapis.com/auth/userinfo.email"},
+		},
+		{
+			map[string]string{"scopes": ""},
+			[]string{},
+		},
+		{
+			map[string]string{"scopes": "A,B,C"},
+			[]string{"A", "B", "C"},
+		},
+	}
+
+	for _, c := range cases {
+		got := parseScopes(c.in)
+		if !reflect.DeepEqual(got, c.out) {
+			t.Errorf("expected=%v, got=%v", c.out, got)
+		}
+	}
+}
+
 func errEquiv(got, want error) bool {
 	if got == want {
 		return true