From e19dc6a86830107bcb7a4a50c997bc8a396277b1 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Tue, 19 Dec 2017 00:14:57 -0800 Subject: [PATCH 1/2] configurable scopes for gcp default credentials - add config.scopes field comma-separated scope URLs, to be used with Google Application Default Credentials (i.e. GOOGLE_APPLICATION_CREDENTIALS env) - default scopes now include userinfo.email scope so the headless app with gserviceaccount keys can have RoleBindings with email instead of account ID. Signed-off-by: Ahmet Alp Balkan --- .../plugin/pkg/client/auth/gcp/gcp.go | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go index 5ed1203b2a6..5844dc837af 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go @@ -55,6 +55,14 @@ var execCommand = exec.Command // "name": "gcp", // // 'config': { +// # Authentication options +// # These options are used while getting a token. +// +// # comma-separated list of GCP API scopes. default value of this field +// # is "https://www.googleapis.com/auth/cloud-platform,https://www.googleapis.com/auth/userinfo.email". +// # to override the API scopes, specify this field explicitly. +// "scopes": "https://www.googleapis.com/auth/cloud-platform" +// // # Caching options // // # Raw string data representing cached access token. @@ -102,6 +110,9 @@ func newGCPAuthProvider(_ string, gcpConfig map[string]string, persister restcli if len(cmd) == 0 { return nil, fmt.Errorf("missing access token cmd") } + if gcpConfig["scopes"] != "" { + return nil, fmt.Errorf("scopes can only be used when kubectl is using a gcp service account key") + } var args []string if cmdArgs, ok := gcpConfig["cmd-args"]; ok { args = strings.Fields(cmdArgs) @@ -112,7 +123,17 @@ func newGCPAuthProvider(_ string, gcpConfig map[string]string, persister restcli } ts = newCmdTokenSource(cmd, args, gcpConfig["token-key"], gcpConfig["expiry-key"], gcpConfig["time-fmt"]) } else { - ts, err = google.DefaultTokenSource(context.Background(), "https://www.googleapis.com/auth/cloud-platform") + 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...) } if err != nil { return nil, err From ad4fdc7d150a26e7b4df5bdbc7e9b45570cf78c9 Mon Sep 17 00:00:00 2001 From: Ahmet Alp Balkan Date: Mon, 22 Jan 2018 11:21:17 -0800 Subject: [PATCH 2/2] Refactor gcp.go methods for testability, add tests Signed-off-by: Ahmet Alp Balkan --- .../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/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go index 5844dc837af..3a4f86777bd 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go +++ b/staging/src/k8s.io/client-go/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/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_test.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_test.go index 662d38b8f19..fc7f774b672 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_test.go +++ b/staging/src/k8s.io/client-go/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