From a8754b56580f4cf02eb13066bb5ebc13a65dd68f Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Fri, 30 Apr 2021 15:00:54 -0700 Subject: [PATCH] Refactor: extract helper func from `runGetKubeconfig()` - Reduces the cyclomatic complexity of the function Signed-off-by: Ryan Richard --- cmd/pinniped/cmd/kubeconfig.go | 79 ++++++++++++++++------------- cmd/pinniped/cmd/kubeconfig_test.go | 40 +++++++++++---- 2 files changed, 75 insertions(+), 44 deletions(-) diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index c1b2e4f7..3f4afaf5 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -180,19 +180,6 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f return fmt.Errorf("invalid API group suffix: %w", err) } - execConfig := clientcmdapi.ExecConfig{ - APIVersion: clientauthenticationv1beta1.SchemeGroupVersion.String(), - Args: []string{}, - Env: []clientcmdapi.ExecEnvVar{}, - } - - var err error - execConfig.Command, err = deps.getPathToSelf() - if err != nil { - return fmt.Errorf("could not determine the Pinniped executable path: %w", err) - } - execConfig.ProvideClusterInfo = true - clientConfig := newClientConfig(flags.kubeconfigPath, flags.kubeconfigContextOverride) currentKubeConfig, err := clientConfig.RawConfig() if err != nil { @@ -236,15 +223,6 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f if err := discoverAuthenticatorParams(authenticator, &flags, deps.log); err != nil { return err } - // Append the flags to configure the Concierge credential exchange at runtime. - execConfig.Args = append(execConfig.Args, - "--enable-concierge", - "--concierge-api-group-suffix="+flags.concierge.apiGroupSuffix, - "--concierge-authenticator-name="+flags.concierge.authenticatorName, - "--concierge-authenticator-type="+flags.concierge.authenticatorType, - "--concierge-endpoint="+flags.concierge.endpoint, - "--concierge-ca-bundle-data="+base64.StdEncoding.EncodeToString(flags.concierge.caBundle), - ) // Point kubectl at the concierge endpoint. cluster.Server = flags.concierge.endpoint @@ -258,6 +236,45 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f } } + execConfig, err := newExecConfig(deps, flags) + if err != nil { + return err + } + + kubeconfig := newExecKubeconfig(cluster, execConfig, newKubeconfigNames) + if err := validateKubeconfig(ctx, flags, kubeconfig, deps.log); err != nil { + return err + } + + return writeConfigAsYAML(out, kubeconfig) +} + +func newExecConfig(deps kubeconfigDeps, flags getKubeconfigParams) (*clientcmdapi.ExecConfig, error) { + execConfig := &clientcmdapi.ExecConfig{ + APIVersion: clientauthenticationv1beta1.SchemeGroupVersion.String(), + Args: []string{}, + Env: []clientcmdapi.ExecEnvVar{}, + ProvideClusterInfo: true, + } + + var err error + execConfig.Command, err = deps.getPathToSelf() + if err != nil { + return nil, fmt.Errorf("could not determine the Pinniped executable path: %w", err) + } + + if !flags.concierge.disabled { + // Append the flags to configure the Concierge credential exchange at runtime. + execConfig.Args = append(execConfig.Args, + "--enable-concierge", + "--concierge-api-group-suffix="+flags.concierge.apiGroupSuffix, + "--concierge-authenticator-name="+flags.concierge.authenticatorName, + "--concierge-authenticator-type="+flags.concierge.authenticatorType, + "--concierge-endpoint="+flags.concierge.endpoint, + "--concierge-ca-bundle-data="+base64.StdEncoding.EncodeToString(flags.concierge.caBundle), + ) + } + // If --credential-cache is set, pass it through. if flags.credentialCachePathSet { execConfig.Args = append(execConfig.Args, "--credential-cache="+flags.credentialCachePath) @@ -266,7 +283,7 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f // If one of the --static-* flags was passed, output a config that runs `pinniped login static`. if flags.staticToken != "" || flags.staticTokenEnvName != "" { if flags.staticToken != "" && flags.staticTokenEnvName != "" { - return fmt.Errorf("only one of --static-token and --static-token-env can be specified") + return nil, fmt.Errorf("only one of --static-token and --static-token-env can be specified") } execConfig.Args = append([]string{"login", "static"}, execConfig.Args...) if flags.staticToken != "" { @@ -275,18 +292,13 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f if flags.staticTokenEnvName != "" { execConfig.Args = append(execConfig.Args, "--token-env="+flags.staticTokenEnvName) } - - kubeconfig := newExecKubeconfig(cluster, &execConfig, newKubeconfigNames) - if err := validateKubeconfig(ctx, flags, kubeconfig, deps.log); err != nil { - return err - } - return writeConfigAsYAML(out, kubeconfig) + return execConfig, nil } // Otherwise continue to parse the OIDC-related flags and output a config that runs `pinniped login oidc`. execConfig.Args = append([]string{"login", "oidc"}, execConfig.Args...) if flags.oidc.issuer == "" { - return fmt.Errorf("could not autodiscover --oidc-issuer and none was provided") + return nil, fmt.Errorf("could not autodiscover --oidc-issuer and none was provided") } execConfig.Args = append(execConfig.Args, "--issuer="+flags.oidc.issuer, @@ -317,11 +329,8 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f if flags.oidc.upstreamIDPType != "" { execConfig.Args = append(execConfig.Args, "--upstream-identity-provider-type="+flags.oidc.upstreamIDPType) } - kubeconfig := newExecKubeconfig(cluster, &execConfig, newKubeconfigNames) - if err := validateKubeconfig(ctx, flags, kubeconfig, deps.log); err != nil { - return err - } - return writeConfigAsYAML(out, kubeconfig) + + return execConfig, nil } type kubeconfigNames struct{ ContextName, UserName, ClusterName string } diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index 2efa13d1..51d9e169 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -134,15 +134,6 @@ func TestGetKubeconfig(t *testing.T) { `) }, }, - { - name: "fail to get self-path", - args: func(issuerCABundle string, issuerURL string) []string { return []string{} }, - getPathToSelfErr: fmt.Errorf("some OS error"), - wantError: true, - wantStderr: func(issuerCABundle string, issuerURL string) string { - return `Error: could not determine the Pinniped executable path: some OS error` + "\n" - }, - }, { name: "invalid OIDC CA bundle path", args: func(issuerCABundle string, issuerURL string) []string { @@ -626,6 +617,37 @@ func TestGetKubeconfig(t *testing.T) { return `Error: tried to autodiscover --oidc-ca-bundle, but JWTAuthenticator test-authenticator has invalid spec.tls.certificateAuthorityData: illegal base64 data at input byte 7` + "\n" }, }, + { + name: "fail to get self-path", + args: func(issuerCABundle string, issuerURL string) []string { + return []string{ + "--kubeconfig", "./testdata/kubeconfig.yaml", + } + }, + getPathToSelfErr: fmt.Errorf("some OS error"), + conciergeObjects: func(issuerCABundle string, issuerURL string) []runtime.Object { + return []runtime.Object{ + credentialIssuer(), + jwtAuthenticator(issuerCABundle, issuerURL), + } + }, + wantLogs: func(issuerCABundle string, issuerURL string) []string { + return []string{ + `"level"=0 "msg"="discovered CredentialIssuer" "name"="test-credential-issuer"`, + `"level"=0 "msg"="discovered Concierge operating in TokenCredentialRequest API mode"`, + `"level"=0 "msg"="discovered Concierge endpoint" "endpoint"="https://fake-server-url-value"`, + `"level"=0 "msg"="discovered Concierge certificate authority bundle" "roots"=0`, + `"level"=0 "msg"="discovered JWTAuthenticator" "name"="test-authenticator"`, + fmt.Sprintf(`"level"=0 "msg"="discovered OIDC issuer" "issuer"="%s"`, issuerURL), + `"level"=0 "msg"="discovered OIDC audience" "audience"="test-audience"`, + `"level"=0 "msg"="discovered OIDC CA bundle" "roots"=1`, + } + }, + wantError: true, + wantStderr: func(issuerCABundle string, issuerURL string) string { + return `Error: could not determine the Pinniped executable path: some OS error` + "\n" + }, + }, { name: "invalid static token flags", args: func(issuerCABundle string, issuerURL string) []string {