diff --git a/plugin/pkg/client/auth/exec/exec.go b/plugin/pkg/client/auth/exec/exec.go index 6d3544bd..a56abd29 100644 --- a/plugin/pkg/client/auth/exec/exec.go +++ b/plugin/pkg/client/auth/exec/exec.go @@ -29,6 +29,7 @@ import ( "os" "os/exec" "reflect" + "strings" "sync" "time" @@ -38,6 +39,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/util/clock" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/pkg/apis/clientauthentication" "k8s.io/client-go/pkg/apis/clientauthentication/v1alpha1" @@ -51,6 +53,12 @@ import ( const execInfoEnv = "KUBERNETES_EXEC_INFO" const onRotateListWarningLength = 1000 +const installHintVerboseHelp = ` + +It looks like you are trying to use a client-go credential plugin that is not installed. + +To learn more about this feature, consult the documentation available at: + https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins` var scheme = runtime.NewScheme() var codecs = serializer.NewCodecFactory(scheme) @@ -108,6 +116,44 @@ func (c *cache) put(s string, a *Authenticator) *Authenticator { return a } +// sometimes rate limits how often a function f() is called. Specifically, Do() +// will run the provided function f() up to threshold times every interval +// duration. +type sometimes struct { + threshold int + interval time.Duration + + clock clock.Clock + mu sync.Mutex + + count int // times we have called f() in this window + window time.Time // beginning of current window of length interval +} + +func (s *sometimes) Do(f func()) { + s.mu.Lock() + defer s.mu.Unlock() + + now := s.clock.Now() + if s.window.IsZero() { + s.window = now + } + + // If we are no longer in our saved time window, then we get to reset our run + // count back to 0 and start increasing towards the threshold again. + if inWindow := now.Sub(s.window) < s.interval; !inWindow { + s.window = now + s.count = 0 + } + + // If we have not run the function more than threshold times in this current + // time window, we get to run it now! + if underThreshold := s.count < s.threshold; underThreshold { + s.count++ + f() + } +} + // GetAuthenticator returns an exec-based plugin for providing client credentials. func GetAuthenticator(config *api.ExecConfig) (*Authenticator, error) { return newAuthenticator(globalCache, config) @@ -129,6 +175,13 @@ func newAuthenticator(c *cache, config *api.ExecConfig) (*Authenticator, error) args: config.Args, group: gv, + installHint: config.InstallHint, + sometimes: &sometimes{ + threshold: 10, + interval: time.Hour, + clock: clock.RealClock{}, + }, + stdin: os.Stdin, stderr: os.Stderr, interactive: terminal.IsTerminal(int(os.Stdout.Fd())), @@ -152,6 +205,12 @@ type Authenticator struct { group schema.GroupVersion env []string + // Used to avoid log spew by rate limiting install hint printing. We didn't do + // this by interval based rate limiting alone since that way may have prevented + // the install hint from showing up for kubectl users. + sometimes *sometimes + installHint string + // Stubbable for testing stdin io.Reader stderr io.Writer @@ -323,7 +382,7 @@ func (a *Authenticator) refreshCredsLocked(r *clientauthentication.Response) err } if err := cmd.Run(); err != nil { - return fmt.Errorf("exec: %v", err) + return a.wrapCmdRunErrorLocked(err) } _, gvk, err := codecs.UniversalDecoder(a.group).Decode(stdout.Bytes(), nil, cred) @@ -394,3 +453,35 @@ func (a *Authenticator) refreshCredsLocked(r *clientauthentication.Response) err expirationMetrics.set(a, expiry) return nil } + +// wrapCmdRunErrorLocked pulls out the code to construct a helpful error message +// for when the exec plugin's binary fails to Run(). +// +// It must be called while holding the Authenticator's mutex. +func (a *Authenticator) wrapCmdRunErrorLocked(err error) error { + switch err.(type) { + case *exec.Error: // Binary does not exist (see exec.Error). + builder := strings.Builder{} + fmt.Fprintf(&builder, "exec: executable %s not found", a.cmd) + + a.sometimes.Do(func() { + fmt.Fprint(&builder, installHintVerboseHelp) + if a.installHint != "" { + fmt.Fprintf(&builder, "\n\n%s", a.installHint) + } + }) + + return errors.New(builder.String()) + + case *exec.ExitError: // Binary execution failed (see exec.Cmd.Run()). + e := err.(*exec.ExitError) + return fmt.Errorf( + "exec: executable %s failed with exit code %d", + a.cmd, + e.ProcessState.ExitCode(), + ) + + default: + return fmt.Errorf("exec: %v", err) + } +} diff --git a/plugin/pkg/client/auth/exec/exec_test.go b/plugin/pkg/client/auth/exec/exec_test.go index 9bef7090..8f5f6e60 100644 --- a/plugin/pkg/client/auth/exec/exec_test.go +++ b/plugin/pkg/client/auth/exec/exec_test.go @@ -32,12 +32,14 @@ import ( "net/http" "net/http/httptest" "reflect" + "strconv" "strings" "testing" "time" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/clock" "k8s.io/client-go/pkg/apis/clientauthentication" "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/transport" @@ -168,15 +170,17 @@ func compJSON(t *testing.T, got, want []byte) { func TestRefreshCreds(t *testing.T) { tests := []struct { - name string - config api.ExecConfig - output string - interactive bool - response *clientauthentication.Response - wantInput string - wantCreds credentials - wantExpiry time.Time - wantErr bool + name string + config api.ExecConfig + exitCode int + output string + interactive bool + response *clientauthentication.Response + wantInput string + wantCreds credentials + wantExpiry time.Time + wantErr bool + wantErrSubstr string }{ { name: "basic-request", @@ -450,17 +454,42 @@ func TestRefreshCreds(t *testing.T) { }`, wantErr: true, }, + { + name: "unknown-binary", + config: api.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1beta1", + Command: "does not exist", + InstallHint: "some install hint", + }, + wantErr: true, + wantErrSubstr: "some install hint", + }, + { + name: "binary-fails", + config: api.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1beta1", + }, + exitCode: 73, + wantErr: true, + wantErrSubstr: "73", + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { c := test.config - c.Command = "./testdata/test-plugin.sh" - c.Env = append(c.Env, api.ExecEnvVar{ - Name: "TEST_OUTPUT", - Value: test.output, - }) + if c.Command == "" { + c.Command = "./testdata/test-plugin.sh" + c.Env = append(c.Env, api.ExecEnvVar{ + Name: "TEST_OUTPUT", + Value: test.output, + }) + c.Env = append(c.Env, api.ExecEnvVar{ + Name: "TEST_EXIT_CODE", + Value: strconv.Itoa(test.exitCode), + }) + } a, err := newAuthenticator(newCache(), &c) if err != nil { @@ -475,6 +504,8 @@ func TestRefreshCreds(t *testing.T) { if err := a.refreshCredsLocked(test.response); err != nil { if !test.wantErr { t.Errorf("get token %v", err) + } else if !strings.Contains(err.Error(), test.wantErrSubstr) { + t.Errorf("expected error with substring '%v' got '%v'", test.wantErrSubstr, err.Error()) } return } @@ -763,6 +794,75 @@ func TestConcurrentUpdateTransportConfig(t *testing.T) { time.Sleep(2 * time.Second) } +func TestInstallHintRateLimit(t *testing.T) { + tests := []struct { + name string + + threshold int + interval time.Duration + + calls int + perCallAdvance time.Duration + + wantInstallHint int + }{ + { + name: "print-up-to-threshold", + threshold: 2, + interval: time.Second, + calls: 10, + wantInstallHint: 2, + }, + { + name: "after-interval-threshold-resets", + threshold: 2, + interval: time.Second * 5, + calls: 10, + perCallAdvance: time.Second, + wantInstallHint: 4, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := api.ExecConfig{ + Command: "does not exist", + APIVersion: "client.authentication.k8s.io/v1alpha1", + InstallHint: "some install hint", + } + a, err := newAuthenticator(newCache(), &c) + if err != nil { + t.Fatal(err) + } + + a.sometimes.threshold = test.threshold + a.sometimes.interval = test.interval + + clock := clock.NewFakeClock(time.Now()) + a.sometimes.clock = clock + + count := 0 + for i := 0; i < test.calls; i++ { + err := a.refreshCredsLocked(&clientauthentication.Response{}) + if strings.Contains(err.Error(), c.InstallHint) { + count++ + } + + clock.SetTime(clock.Now().Add(test.perCallAdvance)) + } + + if test.wantInstallHint != count { + t.Errorf( + "%s: expected install hint %d times got %d", + test.name, + test.wantInstallHint, + count, + ) + } + }) + } +} + // genClientCert generates an x509 certificate for testing. Certificate and key // are returned in PEM encoding. The generated cert expires in 24 hours. func genClientCert(t *testing.T) ([]byte, []byte) { diff --git a/plugin/pkg/client/auth/exec/testdata/test-plugin.sh b/plugin/pkg/client/auth/exec/testdata/test-plugin.sh index aa7daad5..b5071471 100755 --- a/plugin/pkg/client/auth/exec/testdata/test-plugin.sh +++ b/plugin/pkg/client/auth/exec/testdata/test-plugin.sh @@ -16,3 +16,4 @@ >&2 echo "$KUBERNETES_EXEC_INFO" echo "$TEST_OUTPUT" +exit "${TEST_EXIT_CODE:-0}" diff --git a/tools/clientcmd/api/types.go b/tools/clientcmd/api/types.go index 57acb3db..58f88a51 100644 --- a/tools/clientcmd/api/types.go +++ b/tools/clientcmd/api/types.go @@ -210,6 +210,11 @@ type ExecConfig struct { // Preferred input version of the ExecInfo. The returned ExecCredentials MUST use // the same encoding version as the input. APIVersion string `json:"apiVersion,omitempty"` + + // This text is shown to the user when the executable doesn't seem to be + // present. For example, `brew install foo-cli` might be a good InstallHint for + // foo-cli on Mac OS systems. + InstallHint string `json:"installHint,omitempty"` } var _ fmt.Stringer = new(ExecConfig) diff --git a/tools/clientcmd/api/v1/types.go b/tools/clientcmd/api/v1/types.go index c6880f43..d09c7858 100644 --- a/tools/clientcmd/api/v1/types.go +++ b/tools/clientcmd/api/v1/types.go @@ -209,6 +209,11 @@ type ExecConfig struct { // Preferred input version of the ExecInfo. The returned ExecCredentials MUST use // the same encoding version as the input. APIVersion string `json:"apiVersion,omitempty"` + + // This text is shown to the user when the executable doesn't seem to be + // present. For example, `brew install foo-cli` might be a good InstallHint for + // foo-cli on Mac OS systems. + InstallHint string `json:"installHint,omitempty"` } // ExecEnvVar is used for setting environment variables when executing an exec-based diff --git a/tools/clientcmd/api/v1/zz_generated.conversion.go b/tools/clientcmd/api/v1/zz_generated.conversion.go index 5c12551e..bf9eaeca 100644 --- a/tools/clientcmd/api/v1/zz_generated.conversion.go +++ b/tools/clientcmd/api/v1/zz_generated.conversion.go @@ -358,6 +358,7 @@ func autoConvert_v1_ExecConfig_To_api_ExecConfig(in *ExecConfig, out *api.ExecCo out.Args = *(*[]string)(unsafe.Pointer(&in.Args)) out.Env = *(*[]api.ExecEnvVar)(unsafe.Pointer(&in.Env)) out.APIVersion = in.APIVersion + out.InstallHint = in.InstallHint return nil } @@ -371,6 +372,7 @@ func autoConvert_api_ExecConfig_To_v1_ExecConfig(in *api.ExecConfig, out *ExecCo out.Args = *(*[]string)(unsafe.Pointer(&in.Args)) out.Env = *(*[]ExecEnvVar)(unsafe.Pointer(&in.Env)) out.APIVersion = in.APIVersion + out.InstallHint = in.InstallHint return nil } diff --git a/tools/clientcmd/client_config.go b/tools/clientcmd/client_config.go index 25845581..690afce0 100644 --- a/tools/clientcmd/client_config.go +++ b/tools/clientcmd/client_config.go @@ -24,6 +24,7 @@ import ( "net/url" "os" "strings" + "unicode" restclient "k8s.io/client-go/rest" clientauth "k8s.io/client-go/tools/auth" @@ -269,6 +270,7 @@ func (config *DirectClientConfig) getUserIdentificationPartialConfig(configAuthI } if configAuthInfo.Exec != nil { mergedConfig.ExecProvider = configAuthInfo.Exec + mergedConfig.ExecProvider.InstallHint = cleanANSIEscapeCodes(mergedConfig.ExecProvider.InstallHint) } // if there still isn't enough information to authenticate the user, try prompting @@ -314,6 +316,41 @@ func canIdentifyUser(config restclient.Config) bool { config.ExecProvider != nil } +// cleanANSIEscapeCodes takes an arbitrary string and ensures that there are no +// ANSI escape sequences that could put the terminal in a weird state (e.g., +// "\e[1m" bolds text) +func cleanANSIEscapeCodes(s string) string { + // spaceControlCharacters includes tab, new line, vertical tab, new page, and + // carriage return. These are in the unicode.Cc category, but that category also + // contains ESC (U+001B) which we don't want. + spaceControlCharacters := unicode.RangeTable{ + R16: []unicode.Range16{ + {Lo: 0x0009, Hi: 0x000D, Stride: 1}, + }, + } + + // Why not make this deny-only (instead of allow-only)? Because unicode.C + // contains newline and tab characters that we want. + allowedRanges := []*unicode.RangeTable{ + unicode.L, + unicode.M, + unicode.N, + unicode.P, + unicode.S, + unicode.Z, + &spaceControlCharacters, + } + builder := strings.Builder{} + for _, roon := range s { + if unicode.IsOneOf(allowedRanges, roon) { + builder.WriteRune(roon) // returns nil error, per go doc + } else { + fmt.Fprintf(&builder, "%U", roon) + } + } + return builder.String() +} + // Namespace implements ClientConfig func (config *DirectClientConfig) Namespace() (string, bool, error) { if config.overrides != nil && config.overrides.Context.Namespace != "" { diff --git a/tools/clientcmd/client_config_test.go b/tools/clientcmd/client_config_test.go index 550fa91d..0819ed53 100644 --- a/tools/clientcmd/client_config_test.go +++ b/tools/clientcmd/client_config_test.go @@ -624,6 +624,26 @@ func TestCreateMissingContext(t *testing.T) { } } +func TestCreateAuthConfigExecInstallHintCleanup(t *testing.T) { + config := createValidTestConfig() + clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{ + AuthInfo: clientcmdapi.AuthInfo{ + Exec: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1alpha1", + Command: "some-command", + InstallHint: "some install hint with \x1b[1mcontrol chars\x1b[0m\nand a newline", + }, + }, + }, nil) + cleanedInstallHint := "some install hint with U+001B[1mcontrol charsU+001B[0m\nand a newline" + + clientConfig, err := clientBuilder.ClientConfig() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + matchStringArg(cleanedInstallHint, clientConfig.ExecProvider.InstallHint, t) +} + func TestInClusterClientConfigPrecedence(t *testing.T) { tt := []struct { overrides *ConfigOverrides @@ -850,3 +870,70 @@ users: } } + +func TestCleanANSIEscapeCodes(t *testing.T) { + tests := []struct { + name string + in, out string + }{ + { + name: "DenyBoldCharacters", + in: "\x1b[1mbold tuna\x1b[0m, fish, \x1b[1mbold marlin\x1b[0m", + out: "U+001B[1mbold tunaU+001B[0m, fish, U+001B[1mbold marlinU+001B[0m", + }, + { + name: "DenyCursorNavigation", + in: "\x1b[2Aup up, \x1b[2Cright right", + out: "U+001B[2Aup up, U+001B[2Cright right", + }, + { + name: "DenyClearScreen", + in: "clear: \x1b[2J", + out: "clear: U+001B[2J", + }, + { + name: "AllowSpaceCharactersUnchanged", + in: "tuna\nfish\r\nmarlin\t\r\ntuna\vfish\fmarlin", + }, + { + name: "AllowLetters", + in: "alpha: \u03b1, beta: \u03b2, gamma: \u03b3", + }, + { + name: "AllowMarks", + in: "tu\u0301na with a mark over the u, fi\u0302sh with a mark over the i," + + " ma\u030Arlin with a mark over the a", + }, + { + name: "AllowNumbers", + in: "t1na, f2sh, m3rlin, t12a, f34h, m56lin, t123, f456, m567n", + }, + { + name: "AllowPunctuation", + in: "\"here's a sentence; with! some...punctuation ;)\"", + }, + { + name: "AllowSymbols", + in: "the integral of f(x) from 0 to n approximately equals the sum of f(x)" + + " from a = 0 to n, where a and n are natural numbers:" + + "\u222b\u2081\u207F f(x) dx \u2248 \u2211\u2090\u208C\u2081\u207F f(x)," + + " a \u2208 \u2115, n \u2208 \u2115", + }, + { + name: "AllowSepatators", + in: "here is a paragraph separator\u2029and here\u2003are\u2003some" + + "\u2003em\u2003spaces", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if len(test.out) == 0 { + test.out = test.in + } + + if actualOut := cleanANSIEscapeCodes(test.in); test.out != actualOut { + t.Errorf("expected %q, actual %q", test.out, actualOut) + } + }) + } +}