From 4e25bcd4b230401df28b0ddf0d945a3ecf15312f Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 5 Apr 2021 11:41:06 -0500 Subject: [PATCH] Generate more helpful context/cluster/user names in `pinniped get kubeconfig` Before this change, the "context", "cluster", and "user" fields in generated kubeconfig YAML were always hardcoded to "pinniped". This could be confusing if you generated many kubeconfigs for different clusters. After this change, the fields will be copied from their names in the original kubeconfig, suffixed with "-pinniped". This suffix can be overridden by setting the new `--generated-name-suffix` CLI flag. The goal of this change is that you can distinguish between kubeconfigs generated for different clusters, as well as being able to distinguish between the Pinniped and original (admin) kubeconfigs for a cluster. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/kubeconfig.go | 59 +++++++++----- cmd/pinniped/cmd/kubeconfig_test.go | 98 ++++++++++++++--------- cmd/pinniped/cmd/testdata/kubeconfig.yaml | 20 +++-- cmd/pinniped/cmd/whoami_test.go | 12 +-- 4 files changed, 119 insertions(+), 70 deletions(-) diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index 151e04df..8e0051b6 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -86,6 +86,7 @@ type getKubeconfigParams struct { staticTokenEnvName string oidc getKubeconfigOIDCParams concierge getKubeconfigConciergeParams + generatedNameSuffix string } func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command { @@ -130,6 +131,7 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command { f.BoolVar(&flags.skipValidate, "skip-validation", false, "Skip final validation of the kubeconfig (default: false)") f.DurationVar(&flags.timeout, "timeout", 10*time.Minute, "Timeout for autodiscovery and validation") f.StringVarP(&flags.outputPath, "output", "o", "", "Output file path (default: stdout)") + f.StringVar(&flags.generatedNameSuffix, "generated-name-suffix", "-pinniped", "Suffix to append to generated cluster, context, user kubeconfig entries") mustMarkHidden(cmd, "oidc-debug-session-cache") @@ -178,15 +180,23 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f if err != nil { return fmt.Errorf("could not load --kubeconfig: %w", err) } - cluster, err := copyCurrentClusterFromExistingKubeConfig(currentKubeConfig, flags.kubeconfigContextOverride) + currentKubeconfigNames, err := getCurrentContext(currentKubeConfig, flags) if err != nil { return fmt.Errorf("could not load --kubeconfig/--kubeconfig-context: %w", err) } + cluster := currentKubeConfig.Clusters[currentKubeconfigNames.ClusterName] clientset, err := deps.getClientset(clientConfig, flags.concierge.apiGroupSuffix) if err != nil { return fmt.Errorf("could not configure Kubernetes client: %w", err) } + // Generate the new context/cluster/user names by appending the --generated-name-suffix to the original values. + newKubeconfigNames := &kubeconfigNames{ + ContextName: currentKubeconfigNames.ContextName + flags.generatedNameSuffix, + UserName: currentKubeconfigNames.UserName + flags.generatedNameSuffix, + ClusterName: currentKubeconfigNames.ClusterName + flags.generatedNameSuffix, + } + if !flags.concierge.disabled { credentialIssuer, err := waitForCredentialIssuer(ctx, clientset, flags, deps) if err != nil { @@ -236,7 +246,7 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f execConfig.Args = append(execConfig.Args, "--token-env="+flags.staticTokenEnvName) } - kubeconfig := newExecKubeconfig(cluster, &execConfig) + kubeconfig := newExecKubeconfig(cluster, &execConfig, newKubeconfigNames) if err := validateKubeconfig(ctx, flags, kubeconfig, deps.log); err != nil { return err } @@ -271,13 +281,33 @@ func runGetKubeconfig(ctx context.Context, out io.Writer, deps kubeconfigDeps, f if flags.oidc.requestAudience != "" { execConfig.Args = append(execConfig.Args, "--request-audience="+flags.oidc.requestAudience) } - kubeconfig := newExecKubeconfig(cluster, &execConfig) + kubeconfig := newExecKubeconfig(cluster, &execConfig, newKubeconfigNames) if err := validateKubeconfig(ctx, flags, kubeconfig, deps.log); err != nil { return err } return writeConfigAsYAML(out, kubeconfig) } +type kubeconfigNames struct{ ContextName, UserName, ClusterName string } + +func getCurrentContext(currentKubeConfig clientcmdapi.Config, flags getKubeconfigParams) (*kubeconfigNames, error) { + contextName := currentKubeConfig.CurrentContext + if flags.kubeconfigContextOverride != "" { + contextName = flags.kubeconfigContextOverride + } + ctx := currentKubeConfig.Contexts[contextName] + if ctx == nil { + return nil, fmt.Errorf("no such context %q", contextName) + } + if _, exists := currentKubeConfig.Clusters[ctx.Cluster]; !exists { + return nil, fmt.Errorf("no such cluster %q", ctx.Cluster) + } + if _, exists := currentKubeConfig.AuthInfos[ctx.AuthInfo]; !exists { + return nil, fmt.Errorf("no such user %q", ctx.AuthInfo) + } + return &kubeconfigNames{ContextName: contextName, UserName: ctx.AuthInfo, ClusterName: ctx.Cluster}, nil +} + func waitForCredentialIssuer(ctx context.Context, clientset conciergeclientset.Interface, flags getKubeconfigParams, deps kubeconfigDeps) (*configv1alpha1.CredentialIssuer, error) { credentialIssuer, err := lookupCredentialIssuer(clientset, flags.concierge.credentialIssuer, deps.log) if err != nil { @@ -461,15 +491,14 @@ func getConciergeFrontend(credentialIssuer *configv1alpha1.CredentialIssuer, mod return nil, fmt.Errorf("could not find successful Concierge strategy matching --concierge-mode=%s", mode.String()) } -func newExecKubeconfig(cluster *clientcmdapi.Cluster, execConfig *clientcmdapi.ExecConfig) clientcmdapi.Config { - const name = "pinniped" +func newExecKubeconfig(cluster *clientcmdapi.Cluster, execConfig *clientcmdapi.ExecConfig, newNames *kubeconfigNames) clientcmdapi.Config { return clientcmdapi.Config{ Kind: "Config", APIVersion: clientcmdapi.SchemeGroupVersion.Version, - Clusters: map[string]*clientcmdapi.Cluster{name: cluster}, - AuthInfos: map[string]*clientcmdapi.AuthInfo{name: {Exec: execConfig}}, - Contexts: map[string]*clientcmdapi.Context{name: {Cluster: name, AuthInfo: name}}, - CurrentContext: name, + Clusters: map[string]*clientcmdapi.Cluster{newNames.ClusterName: cluster}, + AuthInfos: map[string]*clientcmdapi.AuthInfo{newNames.UserName: {Exec: execConfig}}, + Contexts: map[string]*clientcmdapi.Context{newNames.ContextName: {Cluster: newNames.ClusterName, AuthInfo: newNames.UserName}}, + CurrentContext: newNames.ContextName, } } @@ -560,18 +589,6 @@ func writeConfigAsYAML(out io.Writer, config clientcmdapi.Config) error { return nil } -func copyCurrentClusterFromExistingKubeConfig(currentKubeConfig clientcmdapi.Config, currentContextNameOverride string) (*clientcmdapi.Cluster, error) { - contextName := currentKubeConfig.CurrentContext - if currentContextNameOverride != "" { - contextName = currentContextNameOverride - } - ctx := currentKubeConfig.Contexts[contextName] - if ctx == nil { - return nil, fmt.Errorf("no such context %q", contextName) - } - return currentKubeConfig.Clusters[ctx.Cluster], nil -} - func validateKubeconfig(ctx context.Context, flags getKubeconfigParams, kubeconfig clientcmdapi.Config, log logr.Logger) error { if flags.skipValidate { return nil diff --git a/cmd/pinniped/cmd/kubeconfig_test.go b/cmd/pinniped/cmd/kubeconfig_test.go index 879e2d71..c7342a54 100644 --- a/cmd/pinniped/cmd/kubeconfig_test.go +++ b/cmd/pinniped/cmd/kubeconfig_test.go @@ -73,6 +73,7 @@ func TestGetKubeconfig(t *testing.T) { --concierge-endpoint string API base for the Concierge endpoint --concierge-mode mode Concierge mode of operation (default TokenCredentialRequestAPI) --concierge-skip-wait Skip waiting for any pending Concierge strategies to become ready (default: false) + --generated-name-suffix string Suffix to append to generated cluster, context, user kubeconfig entries (default "-pinniped") -h, --help help for kubeconfig --kubeconfig string Path to kubeconfig file --kubeconfig-context string Kubeconfig context name (default: current active context) @@ -133,7 +134,7 @@ func TestGetKubeconfig(t *testing.T) { `), }, { - name: "invalid kubeconfig context", + name: "invalid kubeconfig context, missing", args: []string{ "--kubeconfig", "./testdata/kubeconfig.yaml", "--kubeconfig-context", "invalid", @@ -143,6 +144,28 @@ func TestGetKubeconfig(t *testing.T) { Error: could not load --kubeconfig/--kubeconfig-context: no such context "invalid" `), }, + { + name: "invalid kubeconfig context, missing cluster", + args: []string{ + "--kubeconfig", "./testdata/kubeconfig.yaml", + "--kubeconfig-context", "invalid-context-no-such-cluster", + }, + wantError: true, + wantStderr: here.Doc(` + Error: could not load --kubeconfig/--kubeconfig-context: no such cluster "invalid-cluster" + `), + }, + { + name: "invalid kubeconfig context, missing user", + args: []string{ + "--kubeconfig", "./testdata/kubeconfig.yaml", + "--kubeconfig-context", "invalid-context-no-such-user", + }, + wantError: true, + wantStderr: here.Doc(` + Error: could not load --kubeconfig/--kubeconfig-context: no such user "invalid-user" + `), + }, { name: "clientset creation failure", args: []string{ @@ -584,17 +607,17 @@ func TestGetKubeconfig(t *testing.T) { - cluster: certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== server: https://fake-server-url-value - name: pinniped + name: kind-cluster-pinniped contexts: - context: - cluster: pinniped - user: pinniped - name: pinniped - current-context: pinniped + cluster: kind-cluster-pinniped + user: kind-user-pinniped + name: kind-context-pinniped + current-context: kind-context-pinniped kind: Config preferences: {} users: - - name: pinniped + - name: kind-user-pinniped user: exec: apiVersion: client.authentication.k8s.io/v1beta1 @@ -653,17 +676,17 @@ func TestGetKubeconfig(t *testing.T) { - cluster: certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== server: https://fake-server-url-value - name: pinniped + name: kind-cluster-pinniped contexts: - context: - cluster: pinniped - user: pinniped - name: pinniped - current-context: pinniped + cluster: kind-cluster-pinniped + user: kind-user-pinniped + name: kind-context-pinniped + current-context: kind-context-pinniped kind: Config preferences: {} users: - - name: pinniped + - name: kind-user-pinniped user: exec: apiVersion: client.authentication.k8s.io/v1beta1 @@ -733,17 +756,17 @@ func TestGetKubeconfig(t *testing.T) { - cluster: certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== server: https://fake-server-url-value - name: pinniped + name: kind-cluster-pinniped contexts: - context: - cluster: pinniped - user: pinniped - name: pinniped - current-context: pinniped + cluster: kind-cluster-pinniped + user: kind-user-pinniped + name: kind-context-pinniped + current-context: kind-context-pinniped kind: Config preferences: {} users: - - name: pinniped + - name: kind-user-pinniped user: exec: apiVersion: client.authentication.k8s.io/v1beta1 @@ -785,6 +808,7 @@ func TestGetKubeconfig(t *testing.T) { "--oidc-debug-session-cache", "--oidc-request-audience", "test-audience", "--skip-validation", + "--generated-name-suffix", "-sso", }, conciergeObjects: []runtime.Object{ &configv1alpha1.CredentialIssuer{ @@ -815,17 +839,17 @@ func TestGetKubeconfig(t *testing.T) { - cluster: certificate-authority-data: %s server: https://explicit-concierge-endpoint.example.com - name: pinniped + name: kind-cluster-sso contexts: - context: - cluster: pinniped - user: pinniped - name: pinniped - current-context: pinniped + cluster: kind-cluster-sso + user: kind-user-sso + name: kind-context-sso + current-context: kind-context-sso kind: Config preferences: {} users: - - name: pinniped + - name: kind-user-sso user: exec: apiVersion: client.authentication.k8s.io/v1beta1 @@ -929,17 +953,17 @@ func TestGetKubeconfig(t *testing.T) { - cluster: certificate-authority-data: %s server: https://impersonation-proxy-endpoint.test - name: pinniped + name: kind-cluster-pinniped contexts: - context: - cluster: pinniped - user: pinniped - name: pinniped - current-context: pinniped + cluster: kind-cluster-pinniped + user: kind-user-pinniped + name: kind-context-pinniped + current-context: kind-context-pinniped kind: Config preferences: {} users: - - name: pinniped + - name: kind-user-pinniped user: exec: apiVersion: client.authentication.k8s.io/v1beta1 @@ -1035,17 +1059,17 @@ func TestGetKubeconfig(t *testing.T) { - cluster: certificate-authority-data: dGVzdC1jb25jaWVyZ2UtY2E= server: https://impersonation-proxy-endpoint.test - name: pinniped + name: kind-cluster-pinniped contexts: - context: - cluster: pinniped - user: pinniped - name: pinniped - current-context: pinniped + cluster: kind-cluster-pinniped + user: kind-user-pinniped + name: kind-context-pinniped + current-context: kind-context-pinniped kind: Config preferences: {} users: - - name: pinniped + - name: kind-user-pinniped user: exec: apiVersion: client.authentication.k8s.io/v1beta1 diff --git a/cmd/pinniped/cmd/testdata/kubeconfig.yaml b/cmd/pinniped/cmd/testdata/kubeconfig.yaml index c89a226e..be627bef 100644 --- a/cmd/pinniped/cmd/testdata/kubeconfig.yaml +++ b/cmd/pinniped/cmd/testdata/kubeconfig.yaml @@ -3,25 +3,33 @@ clusters: - cluster: certificate-authority-data: ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ== # fake-certificate-authority-data-value server: https://fake-server-url-value - name: kind-kind + name: kind-cluster - cluster: certificate-authority-data: c29tZS1vdGhlci1mYWtlLWNlcnRpZmljYXRlLWF1dGhvcml0eS1kYXRhLXZhbHVl # some-other-fake-certificate-authority-data-value server: https://some-other-fake-server-url-value name: some-other-cluster contexts: - context: - cluster: kind-kind - user: kind-kind - name: kind-kind + cluster: kind-cluster + user: kind-user + name: kind-context - context: cluster: some-other-cluster user: some-other-user name: some-other-context -current-context: kind-kind + - context: + cluster: invalid-cluster + user: some-other-user + name: invalid-context-no-such-cluster + - context: + cluster: some-other-cluster + user: invalid-user + name: invalid-context-no-such-user +current-context: kind-context kind: Config preferences: {} users: - - name: kind-kind + - name: kind-user user: client-certificate-data: ZmFrZS1jbGllbnQtY2VydGlmaWNhdGUtZGF0YS12YWx1ZQ== # fake-client-certificate-data-value client-key-data: ZmFrZS1jbGllbnQta2V5LWRhdGEtdmFsdWU= # fake-client-key-data-value diff --git a/cmd/pinniped/cmd/whoami_test.go b/cmd/pinniped/cmd/whoami_test.go index c5480036..cfd811f5 100644 --- a/cmd/pinniped/cmd/whoami_test.go +++ b/cmd/pinniped/cmd/whoami_test.go @@ -53,7 +53,7 @@ func TestWhoami(t *testing.T) { wantStdout: here.Doc(` Current cluster info: - Name: kind-kind + Name: kind-cluster URL: https://fake-server-url-value Current user info: @@ -68,7 +68,7 @@ func TestWhoami(t *testing.T) { wantStdout: here.Doc(` Current cluster info: - Name: kind-kind + Name: kind-cluster URL: https://fake-server-url-value Current user info: @@ -84,7 +84,7 @@ func TestWhoami(t *testing.T) { wantStdout: here.Doc(` Current cluster info: - Name: kind-kind + Name: kind-cluster URL: https://fake-server-url-value Current user info: @@ -100,7 +100,7 @@ func TestWhoami(t *testing.T) { wantStdout: here.Doc(` Current cluster info: - Name: kind-kind + Name: kind-cluster URL: https://fake-server-url-value Current user info: @@ -209,12 +209,12 @@ func TestWhoami(t *testing.T) { name: "different kubeconfig context, but same as current", args: []string{ "--kubeconfig", "./testdata/kubeconfig.yaml", - "--kubeconfig-context", "kind-kind", + "--kubeconfig-context", "kind-context", }, wantStdout: here.Doc(` Current cluster info: - Name: kind-kind + Name: kind-cluster URL: https://fake-server-url-value Current user info: