Skip to content

Commit ad4fdc7

Browse files
ahmetbahmetb
committed
Refactor gcp.go methods for testability, add tests
Signed-off-by: Ahmet Alp Balkan <[email protected]>
1 parent e19dc6a commit ad4fdc7

File tree

2 files changed

+160
-23
lines changed

2 files changed

+160
-23
lines changed

‎staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp.go

+51-23
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,18 @@ func init() {
4242
}
4343
}
4444

45-
// Stubbable for testing
46-
var execCommand = exec.Command
45+
var (
46+
// Stubbable for testing
47+
execCommand = exec.Command
48+
49+
// defaultScopes:
50+
// - cloud-platform is the base scope to authenticate to GCP.
51+
// - userinfo.email is used to authenticate to GKE APIs with gserviceaccount
52+
// email instead of numeric uniqueID.
53+
defaultScopes = []string{
54+
"https://www.googleapis.com/auth/cloud-platform",
55+
"https://www.googleapis.com/auth/userinfo.email"}
56+
)
4757

4858
// gcpAuthProvider is an auth provider plugin that uses GCP credentials to provide
4959
// tokens for kubectl to authenticate itself to the apiserver. A sample json config
@@ -104,9 +114,26 @@ type gcpAuthProvider struct {
104114
}
105115

106116
func newGCPAuthProvider(_ string, gcpConfig map[string]string, persister restclient.AuthProviderConfigPersister) (restclient.AuthProvider, error) {
107-
var ts oauth2.TokenSource
108-
var err error
109-
if cmd, useCmd := gcpConfig["cmd-path"]; useCmd {
117+
ts, err := tokenSource(isCmdTokenSource(gcpConfig), gcpConfig)
118+
if err != nil {
119+
return nil, err
120+
}
121+
cts, err := newCachedTokenSource(gcpConfig["access-token"], gcpConfig["expiry"], persister, ts, gcpConfig)
122+
if err != nil {
123+
return nil, err
124+
}
125+
return &gcpAuthProvider{cts, persister}, nil
126+
}
127+
128+
func isCmdTokenSource(gcpConfig map[string]string) bool {
129+
_, ok := gcpConfig["cmd-path"]
130+
return ok
131+
}
132+
133+
func tokenSource(isCmd bool, gcpConfig map[string]string) (oauth2.TokenSource, error) {
134+
// Command-based token source
135+
if isCmd {
136+
cmd := gcpConfig["cmd-path"]
110137
if len(cmd) == 0 {
111138
return nil, fmt.Errorf("missing access token cmd")
112139
}
@@ -121,28 +148,29 @@ func newGCPAuthProvider(_ string, gcpConfig map[string]string, persister restcli
121148
cmd = fields[0]
122149
args = fields[1:]
123150
}
124-
ts = newCmdTokenSource(cmd, args, gcpConfig["token-key"], gcpConfig["expiry-key"], gcpConfig["time-fmt"])
125-
} else {
126-
var scopes []string
127-
if gcpConfig["scopes"] != "" {
128-
scopes = strings.Split(gcpConfig["scopes"], ",")
129-
} else {
130-
// default scopes: userinfo.email is used to authenticate to
131-
// GKE APIs with gserviceaccount email instead of numeric uniqueID.
132-
scopes = []string{
133-
"https://www.googleapis.com/auth/cloud-platform",
134-
"https://www.googleapis.com/auth/userinfo.email"}
135-
}
136-
ts, err = google.DefaultTokenSource(context.Background(), scopes...)
151+
return newCmdTokenSource(cmd, args, gcpConfig["token-key"], gcpConfig["expiry-key"], gcpConfig["time-fmt"]), nil
137152
}
153+
154+
// Google Application Credentials-based token source
155+
scopes := parseScopes(gcpConfig)
156+
ts, err := google.DefaultTokenSource(context.Background(), scopes...)
138157
if err != nil {
139-
return nil, err
158+
return nil, fmt.Errorf("cannot construct google default token source: %v", err)
140159
}
141-
cts, err := newCachedTokenSource(gcpConfig["access-token"], gcpConfig["expiry"], persister, ts, gcpConfig)
142-
if err != nil {
143-
return nil, err
160+
return ts, nil
161+
}
162+
163+
// parseScopes constructs a list of scopes that should be included in token source
164+
// from the config map.
165+
func parseScopes(gcpConfig map[string]string) []string {
166+
scopes, ok := gcpConfig["scopes"]
167+
if !ok {
168+
return defaultScopes
144169
}
145-
return &gcpAuthProvider{cts, persister}, nil
170+
if scopes == "" {
171+
return []string{}
172+
}
173+
return strings.Split(gcpConfig["scopes"], ",")
146174
}
147175

148176
func (g *gcpAuthProvider) WrapTransport(rt http.RoundTripper) http.RoundTripper {

‎staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_test.go

+109
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package gcp
1818

1919
import (
2020
"fmt"
21+
"io/ioutil"
2122
"net/http"
2223
"os"
2324
"os/exec"
@@ -116,6 +117,114 @@ func TestHelperProcess(t *testing.T) {
116117
os.Exit(0)
117118
}
118119

120+
func Test_isCmdTokenSource(t *testing.T) {
121+
c1 := map[string]string{"cmd-path": "foo"}
122+
if v := isCmdTokenSource(c1); !v {
123+
t.Fatalf("cmd-path present in config (%+v), but got %v", c1, v)
124+
}
125+
126+
c2 := map[string]string{"cmd-args": "foo bar"}
127+
if v := isCmdTokenSource(c2); v {
128+
t.Fatalf("cmd-path not present in config (%+v), but got %v", c2, v)
129+
}
130+
}
131+
132+
func Test_tokenSource_cmd(t *testing.T) {
133+
if _, err := tokenSource(true, map[string]string{}); err == nil {
134+
t.Fatalf("expected error, cmd-args not present in config")
135+
}
136+
137+
c := map[string]string{
138+
"cmd-path": "foo",
139+
"cmd-args": "bar"}
140+
ts, err := tokenSource(true, c)
141+
if err != nil {
142+
t.Fatalf("failed to return cmd token source: %+v", err)
143+
}
144+
if ts == nil {
145+
t.Fatal("returned nil token source")
146+
}
147+
if _, ok := ts.(*commandTokenSource); !ok {
148+
t.Fatalf("returned token source type:(%T) expected:(*commandTokenSource)", ts)
149+
}
150+
}
151+
152+
func Test_tokenSource_cmdCannotBeUsedWithScopes(t *testing.T) {
153+
c := map[string]string{
154+
"cmd-path": "foo",
155+
"scopes": "A,B"}
156+
if _, err := tokenSource(true, c); err == nil {
157+
t.Fatal("expected error when scopes is used with cmd-path")
158+
}
159+
}
160+
161+
func Test_tokenSource_applicationDefaultCredentials_fails(t *testing.T) {
162+
// try to use empty ADC file
163+
fakeTokenFile, err := ioutil.TempFile("", "adctoken")
164+
if err != nil {
165+
t.Fatalf("failed to create fake token file: +%v", err)
166+
}
167+
fakeTokenFile.Close()
168+
defer os.Remove(fakeTokenFile.Name())
169+
170+
os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", fakeTokenFile.Name())
171+
defer os.Unsetenv("GOOGLE_APPLICATION_CREDENTIALS")
172+
if _, err := tokenSource(false, map[string]string{}); err == nil {
173+
t.Fatalf("expected error because specified ADC token file is not a JSON")
174+
}
175+
}
176+
177+
func Test_tokenSource_applicationDefaultCredentials(t *testing.T) {
178+
fakeTokenFile, err := ioutil.TempFile("", "adctoken")
179+
if err != nil {
180+
t.Fatalf("failed to create fake token file: +%v", err)
181+
}
182+
fakeTokenFile.Close()
183+
defer os.Remove(fakeTokenFile.Name())
184+
if err := ioutil.WriteFile(fakeTokenFile.Name(), []byte(`{"type":"service_account"}`), 0600); err != nil {
185+
t.Fatalf("failed to write to fake token file: %+v", err)
186+
}
187+
188+
os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", fakeTokenFile.Name())
189+
defer os.Unsetenv("GOOGLE_APPLICATION_CREDENTIALS")
190+
ts, err := tokenSource(false, map[string]string{})
191+
if err != nil {
192+
t.Fatalf("failed to get a token source: %+v", err)
193+
}
194+
if ts == nil {
195+
t.Fatal("returned nil token soruce")
196+
}
197+
}
198+
199+
func Test_parseScopes(t *testing.T) {
200+
cases := []struct {
201+
in map[string]string
202+
out []string
203+
}{
204+
{
205+
map[string]string{},
206+
[]string{
207+
"https://www.googleapis.com/auth/cloud-platform",
208+
"https://www.googleapis.com/auth/userinfo.email"},
209+
},
210+
{
211+
map[string]string{"scopes": ""},
212+
[]string{},
213+
},
214+
{
215+
map[string]string{"scopes": "A,B,C"},
216+
[]string{"A", "B", "C"},
217+
},
218+
}
219+
220+
for _, c := range cases {
221+
got := parseScopes(c.in)
222+
if !reflect.DeepEqual(got, c.out) {
223+
t.Errorf("expected=%v, got=%v", c.out, got)
224+
}
225+
}
226+
}
227+
119228
func errEquiv(got, want error) bool {
120229
if got == want {
121230
return true

0 commit comments

Comments
 (0)