Skip to content

Commit

Permalink
Infinite default timeout for WhoAmI API call & add --timeout CLI flag
Browse files Browse the repository at this point in the history
  • Loading branch information
cfryanr committed Nov 13, 2023
1 parent 4f79457 commit c0950d4
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
20 changes: 18 additions & 2 deletions cmd/pinniped/cmd/whoami.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func init() {

type whoamiFlags struct {
outputFormat string // e.g., yaml, json, text
timeout time.Duration

kubeconfigPath string
kubeconfigContextOverride string
Expand Down Expand Up @@ -58,6 +59,7 @@ func newWhoamiCommand(getClientset getConciergeClientsetFunc) *cobra.Command {
f.StringVar(&flags.kubeconfigPath, "kubeconfig", os.Getenv("KUBECONFIG"), "Path to kubeconfig file")
f.StringVar(&flags.kubeconfigContextOverride, "kubeconfig-context", "", "Kubeconfig context name (default: current active context)")
f.StringVar(&flags.apiGroupSuffix, "api-group-suffix", groupsuffix.PinnipedDefaultSuffix, "Concierge API group suffix")
f.DurationVar(&flags.timeout, "timeout", 0, "Timeout for the WhoAmI API request (default: 0, meaning no timeout)")

cmd.RunE = func(cmd *cobra.Command, _ []string) error {
return runWhoami(cmd.OutOrStdout(), getClientset, flags)
Expand All @@ -78,8 +80,22 @@ func runWhoami(output io.Writer, getClientset getConciergeClientsetFunc, flags *
return fmt.Errorf("could not get current cluster info: %w", err)
}

ctx, cancelFunc := context.WithTimeout(context.Background(), time.Second*20)
defer cancelFunc()
// Making the WhoAmI request may cause client-go to invoke the credential plugin, which may
// ask the user to interactively authenticate. The time that the user takes to authenticate
// is included in the timeout time, but their authentication is not cancelled by exceeding
// this timeout. Only the subsequent whoami API request is cancelled. Using a short timeout
// causes the odd behavior of a successful login immediately followed by a whoami request failure
// due to timeout. For comparison, kubectl uses an infinite timeout by default on API requests
// but also allows the user to adjust this timeout with the `--request-timeout` CLI option,
// so we will take a similar approach. Note that kubectl has the same behavior when a client-go
// credential plugin is invoked and the user takes longer then the timeout to authenticate.
ctx := context.Background()
if flags.timeout > 0 {
var cancelFunc context.CancelFunc
ctx, cancelFunc = context.WithTimeout(ctx, flags.timeout)
defer cancelFunc()
}

whoAmI, err := clientset.IdentityV1alpha1().WhoAmIRequests().Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{})
if err != nil {
hint := ""
Expand Down
12 changes: 9 additions & 3 deletions cmd/pinniped/cmd/whoami_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2021 the Pinniped contributors. All Rights Reserved.
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package cmd
Expand Down Expand Up @@ -45,6 +45,7 @@ func TestWhoami(t *testing.T) {
--kubeconfig string Path to kubeconfig file
--kubeconfig-context string Kubeconfig context name (default: current active context)
-o, --output string Output format (e.g., 'yaml', 'json', 'text') (default "text")
--timeout duration Timeout for the WhoAmI API request (default: 0, meaning no timeout)
`),
},
{
Expand Down Expand Up @@ -106,7 +107,7 @@ func TestWhoami(t *testing.T) {
Current user info:
Username: some-username
Groups:
Groups:
`),
},
{
Expand Down Expand Up @@ -312,7 +313,12 @@ func TestWhoami(t *testing.T) {
stdout, stderr := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{})
cmd.SetOut(stdout)
cmd.SetErr(stderr)
cmd.SetArgs(test.args)
if test.args == nil {
// cobra uses os.Args[1:] when SetArgs is called with nil, so avoid using nil for tests.
cmd.SetArgs([]string{})
} else {
cmd.SetArgs(test.args)
}

err := cmd.Execute()
if test.wantError {
Expand Down

0 comments on commit c0950d4

Please sign in to comment.