Merge pull request #41653 from jlowdermilk/gcp-auth-plugin

Automatic merge from submit-queue (batch tested with PRs 42080, 41653, 42598, 42555)

Support whitespace in command path for gcp auth plugin

```
External command option on gcp client auth plugin supports whitespace in command path.
```

Splitting on whitespace to get cmd+args breaks when the path the executable contains spaces. Resolve by adding a new "cmd-args" field to config to allow the full string of "cmd-path" to be interpreted as path to executable.

This change is backwards compatible with existing behavior.
This commit is contained in:
Kubernetes Submit Queue 2017-03-06 17:16:13 -08:00 committed by GitHub
commit 73c5d6cd2f
2 changed files with 208 additions and 82 deletions

View File

@ -41,6 +41,9 @@ func init() {
}
}
// Stubbable for testing
var execCommand = exec.Command
// 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
// is provided below with all recognized options described.
@ -62,10 +65,13 @@ func init() {
// # These options direct the plugin to execute a specified command and parse
// # token and expiry time from the output of the command.
//
// # Command to execute for access token. String is split on whitespace
// # with first field treated as the executable, remaining fields as args.
// # Command output will be parsed as JSON.
// "cmd-path": "/usr/bin/gcloud config config-helper --output=json",
// # Command to execute for access token. Command output will be parsed as JSON.
// # If "cmd-args" is not present, this value will be split on whitespace, with
// # the first element interpreted as the command, remaining elements as args.
// "cmd-path": "/usr/bin/gcloud",
//
// # Arguments to pass to command to execute for access token.
// "cmd-args": "config config-helper --output=json"
//
// # JSONPath to the string field that represents the access token in
// # command output. If omitted, defaults to "{.access_token}".
@ -89,11 +95,21 @@ type gcpAuthProvider struct {
}
func newGCPAuthProvider(_ string, gcpConfig map[string]string, persister restclient.AuthProviderConfigPersister) (restclient.AuthProvider, error) {
cmd, useCmd := gcpConfig["cmd-path"]
var ts oauth2.TokenSource
var err error
if useCmd {
ts, err = newCmdTokenSource(cmd, gcpConfig["token-key"], gcpConfig["expiry-key"], gcpConfig["time-fmt"])
if cmd, useCmd := gcpConfig["cmd-path"]; useCmd {
if len(cmd) == 0 {
return nil, fmt.Errorf("missing access token cmd")
}
var args []string
if cmdArgs, ok := gcpConfig["cmd-args"]; ok {
args = strings.Fields(cmdArgs)
} else {
fields := strings.Fields(cmd)
cmd = fields[0]
args = fields[1:]
}
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")
}
@ -192,7 +208,7 @@ type commandTokenSource struct {
timeFmt string
}
func newCmdTokenSource(cmd, tokenKey, expiryKey, timeFmt string) (*commandTokenSource, error) {
func newCmdTokenSource(cmd string, args []string, tokenKey, expiryKey, timeFmt string) *commandTokenSource {
if len(timeFmt) == 0 {
timeFmt = time.RFC3339Nano
}
@ -202,25 +218,21 @@ func newCmdTokenSource(cmd, tokenKey, expiryKey, timeFmt string) (*commandTokenS
if len(expiryKey) == 0 {
expiryKey = "{.token_expiry}"
}
fields := strings.Fields(cmd)
if len(fields) == 0 {
return nil, fmt.Errorf("missing access token cmd")
}
return &commandTokenSource{
cmd: fields[0],
args: fields[1:],
cmd: cmd,
args: args,
tokenKey: tokenKey,
expiryKey: expiryKey,
timeFmt: timeFmt,
}, nil
}
}
func (c *commandTokenSource) Token() (*oauth2.Token, error) {
fullCmd := fmt.Sprintf("%s %s", c.cmd, strings.Join(c.args, " "))
cmd := exec.Command(c.cmd, c.args...)
fullCmd := strings.Join(append([]string{c.cmd}, c.args...), " ")
cmd := execCommand(c.cmd, c.args...)
output, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("error executing access token command %q: %v", fullCmd, err)
return nil, fmt.Errorf("error executing access token command %q: err=%v output=%s", fullCmd, err, output)
}
token, err := c.parseTokenCmdOutput(output)
if err != nil {
@ -241,11 +253,11 @@ func (c *commandTokenSource) parseTokenCmdOutput(output []byte) (*oauth2.Token,
accessToken, err := parseJSONPath(data, "token-key", c.tokenKey)
if err != nil {
return nil, fmt.Errorf("error parsing token-key %q: %v", c.tokenKey, err)
return nil, fmt.Errorf("error parsing token-key %q from %q: %v", c.tokenKey, string(output), err)
}
expiryStr, err := parseJSONPath(data, "expiry-key", c.expiryKey)
if err != nil {
return nil, fmt.Errorf("error parsing expiry-key %q: %v", c.expiryKey, err)
return nil, fmt.Errorf("error parsing expiry-key %q from %q: %v", c.expiryKey, string(output), err)
}
var expiry time.Time
if t, err := time.Parse(c.timeFmt, expiryStr); err != nil {

View File

@ -18,6 +18,8 @@ package gcp
import (
"fmt"
"os"
"os/exec"
"reflect"
"strings"
"sync"
@ -27,118 +29,229 @@ import (
"golang.org/x/oauth2"
)
func TestCmdTokenSource(t *testing.T) {
fakeExpiry := time.Date(2016, 10, 31, 22, 31, 9, 123000000, time.UTC)
customFmt := "2006-01-02 15:04:05.999999999"
type fakeOutput struct {
args []string
output string
}
tests := []struct {
name string
output []byte
cmd, tokenKey, expiryKey, timeFmt string
tok *oauth2.Token
expectErr error
}{
{
"defaults",
[]byte(`{
var (
wantCmd []string
// Output for fakeExec, keyed by command
execOutputs = map[string]fakeOutput{
"/default/no/args": {
args: []string{},
output: `{
"access_token": "faketoken",
"token_expiry": "2016-10-31T22:31:09.123000000Z"
}`),
"/fake/cmd/path", "", "", "",
&oauth2.Token{
AccessToken: "faketoken",
TokenType: "Bearer",
Expiry: fakeExpiry,
},
nil,
},
{
"custom keys",
[]byte(`{
}`},
"/default/legacy/args": {
args: []string{"arg1", "arg2", "arg3"},
output: `{
"access_token": "faketoken",
"token_expiry": "2016-10-31T22:31:09.123000000Z"
}`},
"/space in path/customkeys": {
args: []string{"can", "haz", "auth"},
output: `{
"token": "faketoken",
"token_expiry": {
"datetime": "2016-10-31 22:31:09.123"
}
}`),
"/fake/cmd/path", "{.token}", "{.token_expiry.datetime}", customFmt,
}`},
"missing/tokenkey/noargs": {
args: []string{},
output: `{
"broken": "faketoken",
"token_expiry": {
"datetime": "2016-10-31 22:31:09.123000000Z"
}
}`},
"missing/expirykey/legacyargs": {
args: []string{"split", "on", "whitespace"},
output: `{
"access_token": "faketoken",
"expires": "2016-10-31T22:31:09.123000000Z"
}`},
"invalid expiry/timestamp": {
args: []string{"foo", "--bar", "--baz=abc,def"},
output: `{
"access_token": "faketoken",
"token_expiry": "sometime soon, idk"
}`},
"badjson": {
args: []string{},
output: `{
"access_token": "faketoken",
"token_expiry": "sometime soon, idk"
------
`},
}
)
func fakeExec(command string, args ...string) *exec.Cmd {
cs := []string{"-test.run=TestHelperProcess", "--", command}
cs = append(cs, args...)
cmd := exec.Command(os.Args[0], cs...)
cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"}
return cmd
}
func TestHelperProcess(t *testing.T) {
if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
return
}
// Strip out the leading args used to exec into this function.
gotCmd := os.Args[3]
gotArgs := os.Args[4:]
output, ok := execOutputs[gotCmd]
if !ok {
fmt.Fprintf(os.Stdout, "unexpected call cmd=%q args=%v\n", gotCmd, gotArgs)
os.Exit(1)
} else if !reflect.DeepEqual(output.args, gotArgs) {
fmt.Fprintf(os.Stdout, "call cmd=%q got args %v, want: %v\n", gotCmd, gotArgs, output.args)
os.Exit(1)
}
fmt.Fprintf(os.Stdout, output.output)
os.Exit(0)
}
func errEquiv(got, want error) bool {
if got == want {
return true
}
if got != nil && want != nil {
return strings.Contains(got.Error(), want.Error())
}
return false
}
func TestCmdTokenSource(t *testing.T) {
execCommand = fakeExec
fakeExpiry := time.Date(2016, 10, 31, 22, 31, 9, 123000000, time.UTC)
customFmt := "2006-01-02 15:04:05.999999999"
tests := []struct {
name string
gcpConfig map[string]string
tok *oauth2.Token
newErr, tokenErr error
}{
{
"default",
map[string]string{
"cmd-path": "/default/no/args",
},
&oauth2.Token{
AccessToken: "faketoken",
TokenType: "Bearer",
Expiry: fakeExpiry,
},
nil,
nil,
},
{
"default legacy args",
map[string]string{
"cmd-path": "/default/legacy/args arg1 arg2 arg3",
},
&oauth2.Token{
AccessToken: "faketoken",
TokenType: "Bearer",
Expiry: fakeExpiry,
},
nil,
nil,
},
{
"custom keys",
map[string]string{
"cmd-path": "/space in path/customkeys",
"cmd-args": "can haz auth",
"token-key": "{.token}",
"expiry-key": "{.token_expiry.datetime}",
"time-fmt": customFmt,
},
&oauth2.Token{
AccessToken: "faketoken",
TokenType: "Bearer",
Expiry: fakeExpiry,
},
nil,
nil,
},
{
"missing cmd",
nil,
"", "", "", "",
map[string]string{
"cmd-path": "",
},
nil,
fmt.Errorf("missing access token cmd"),
nil,
},
{
"missing token-key",
[]byte(`{
"broken": "faketoken",
"token_expiry": {
"datetime": "2016-10-31 22:31:09.123000000Z"
}
}`),
"/fake/cmd/path", "{.token}", "", "",
map[string]string{
"cmd-path": "missing/tokenkey/noargs",
"token-key": "{.token}",
},
nil,
nil,
fmt.Errorf("error parsing token-key %q", "{.token}"),
},
{
"missing expiry-key",
[]byte(`{
"access_token": "faketoken",
"expires": "2016-10-31T22:31:09.123000000Z"
}`),
"/fake/cmd/path", "", "{.expiry}", "",
map[string]string{
"cmd-path": "missing/expirykey/legacyargs split on whitespace",
"expiry-key": "{.expiry}",
},
nil,
nil,
fmt.Errorf("error parsing expiry-key %q", "{.expiry}"),
},
{
"invalid expiry timestamp",
[]byte(`{
"access_token": "faketoken",
"token_expiry": "sometime soon, idk"
}`),
"/fake/cmd/path", "", "", "",
map[string]string{
"cmd-path": "invalid expiry/timestamp",
"cmd-args": "foo --bar --baz=abc,def",
},
&oauth2.Token{
AccessToken: "faketoken",
TokenType: "Bearer",
Expiry: time.Time{},
},
nil,
nil,
},
{
"bad JSON",
[]byte(`{
"access_token": "faketoken",
"token_expiry": "sometime soon, idk"
------
`),
"/fake/cmd", "", "", "",
map[string]string{
"cmd-path": "badjson",
},
nil,
nil,
fmt.Errorf("invalid character '-' after object key:value pair"),
},
}
for _, tc := range tests {
ts, err := newCmdTokenSource(tc.cmd, tc.tokenKey, tc.expiryKey, tc.timeFmt)
if err != nil {
if !strings.Contains(err.Error(), tc.expectErr.Error()) {
t.Errorf("%s newCmdTokenSource error: %v, want %v", tc.name, err, tc.expectErr)
}
provider, err := newGCPAuthProvider("", tc.gcpConfig, nil /* persister */)
if !errEquiv(err, tc.newErr) {
t.Errorf("%q newGCPAuthProvider error: got %v, want %v", tc.name, err, tc.newErr)
continue
}
tok, err := ts.parseTokenCmdOutput(tc.output)
if err != tc.expectErr && !strings.Contains(err.Error(), tc.expectErr.Error()) {
t.Errorf("%s parseCmdTokenSource error: %v, want %v", tc.name, err, tc.expectErr)
if err != nil {
continue
}
ts := provider.(*gcpAuthProvider).tokenSource.(*cachedTokenSource).source.(*commandTokenSource)
wantCmd = append([]string{ts.cmd}, ts.args...)
tok, err := ts.Token()
if !errEquiv(err, tc.tokenErr) {
t.Errorf("%q Token() error: got %v, want %v", tc.name, err, tc.tokenErr)
}
if !reflect.DeepEqual(tok, tc.tok) {
t.Errorf("%s got token %v, want %v", tc.name, tok, tc.tok)
t.Errorf("%q Token() got %v, want %v", tc.name, tok, tc.tok)
}
}
}
@ -161,6 +274,7 @@ func (f *fakePersister) Persist(cache map[string]string) error {
func (f *fakePersister) read() map[string]string {
ret := map[string]string{}
f.lk.Lock()
defer f.lk.Unlock()
for k, v := range f.cache {
ret[k] = v
}