From 7f749abb08c5e4d6dbbeb5f46a532c0a672dbce2 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Mon, 22 Jan 2018 11:21:17 -0800 Subject: [PATCH] Refactor gcp.go methods for testability, add tests Signed-off-by: Ahmet Alp Balkan Kubernetes-commit: ad4fdc7d150a26e7b4df5bdbc7e9b45570cf78c9 --- plugin/pkg/client/auth/gcp/gcp.go | 74 +++++++++++------ plugin/pkg/client/auth/gcp/gcp_test.go | 109 +++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 23 deletions(-) diff --git a/plugin/pkg/client/auth/gcp/gcp.go b/plugin/pkg/client/auth/gcp/gcp.go index 5844dc83..3a4f8677 100644 --- a/plugin/pkg/client/auth/gcp/gcp.go +++ b/plugin/pkg/client/auth/gcp/gcp.go @@ -42,8 +42,18 @@ func init() { } } -// Stubbable for testing -var execCommand = exec.Command +var ( + // 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 - var err error - if cmd, useCmd := gcpConfig["cmd-path"]; useCmd { + ts, err := tokenSource(isCmdTokenSource(gcpConfig), gcpConfig) + if err != nil { + 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"]) - } 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...) + return newCmdTokenSource(cmd, args, gcpConfig["token-key"], gcpConfig["expiry-key"], gcpConfig["time-fmt"]), nil } + + // 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) - if err != nil { - return nil, err + return ts, nil +} + +// 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 { diff --git a/plugin/pkg/client/auth/gcp/gcp_test.go b/plugin/pkg/client/auth/gcp/gcp_test.go index 662d38b8..fc7f774b 100644 --- a/plugin/pkg/client/auth/gcp/gcp_test.go +++ b/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