From 770908fc6c58546a4f4679cfbe5977d4543b1355 Mon Sep 17 00:00:00 2001 From: Michael Yang Date: Wed, 16 Nov 2022 18:38:00 -0800 Subject: [PATCH] fix: create one kubeconfig context create only one kubeconfig context per destination regardless if there may be namespaces granted to the user. update `use` to set the `Namespace` field of the target context. --- internal/cmd/kubernetes.go | 88 +++++------ internal/cmd/kubernetes_test.go | 269 ++++++++++++++++++++------------ internal/cmd/use_test.go | 29 +--- 3 files changed, 223 insertions(+), 163 deletions(-) diff --git a/internal/cmd/kubernetes.go b/internal/cmd/kubernetes.go index 7c84e95122..506ddbf134 100644 --- a/internal/cmd/kubernetes.go +++ b/internal/cmd/kubernetes.go @@ -27,33 +27,33 @@ func clientConfig() clientcmd.ClientConfig { func kubernetesSetContext(cluster, namespace string) error { config := clientConfig() - kubeconfig, err := config.RawConfig() + kubeConfig, err := config.RawConfig() if err != nil { return err } name := strings.TrimPrefix(cluster, "infra:") - if namespace != "" { - name = fmt.Sprintf("%s:%s", name, namespace) - } - // set friendly name based on user input rather than internal format friendlyName := strings.ReplaceAll(name, ":", ".") - context := fmt.Sprintf("infra:%s", name) - if _, ok := kubeconfig.Contexts[context]; !ok { + contextName := fmt.Sprintf("infra:%s", name) + kubeContext, ok := kubeConfig.Contexts[contextName] + if !ok { return fmt.Errorf("context not found: %v", friendlyName) } - kubeconfig.CurrentContext = context + kubeContext.Namespace = namespace + + kubeConfig.CurrentContext = contextName + kubeConfig.Contexts[contextName] = kubeContext - if err := clientcmd.WriteToFile(kubeconfig, config.ConfigAccess().GetDefaultFilename()); err != nil { + configFile := config.ConfigAccess().GetDefaultFilename() + if err := safelyWriteConfigToFile(kubeConfig, configFile); err != nil { return err } fmt.Fprintf(os.Stderr, "Switched to context %q.\n", friendlyName) - return nil } @@ -85,11 +85,16 @@ func writeKubeconfig(user *api.User, destinations []api.Destination, grants []ap return err } - keep := make(map[string]bool) + type clusterContext struct { + Namespace string + URL string + CA []byte + } + + infraContexts := make(map[string]clusterContext) for _, g := range grants { parts := strings.Split(g.Resource, ".") - cluster := parts[0] var namespace string @@ -97,68 +102,65 @@ func writeKubeconfig(user *api.User, destinations []api.Destination, grants []ap namespace = parts[1] } - context := "infra:" + cluster - - if namespace != "" { - context += ":" + namespace + if namespace == "default" { + namespace = "" } - var ( - url string - ca []byte - exists bool - ) + contextName := "infra:" + cluster + if _, ok := infraContexts[contextName]; ok && namespace != "" { + continue + } + var infraContext clusterContext for _, d := range destinations { if !isResourceForDestination(g.Resource, d.Name) { continue } if isDestinationAvailable(d) { - url = d.Connection.URL - ca = []byte(d.Connection.CA) - exists = true + infraContext = clusterContext{ + URL: d.Connection.URL, + CA: []byte(d.Connection.CA), + } break } } - if !exists { + if infraContext.URL == "" { continue } - u, err := urlx.Parse(url) + infraContext.Namespace = namespace + infraContexts[contextName] = infraContext + } + + for contextName, infraContext := range infraContexts { + logging.Debugf("creating kubeconfig for %s", contextName) + + u, err := urlx.Parse(infraContext.URL) if err != nil { return err } u.Scheme = "https" - logging.Debugf("creating kubeconfig for %s", context) - - kubeConfig.Clusters[context] = &clientcmdapi.Cluster{ + kubeConfig.Clusters[contextName] = &clientcmdapi.Cluster{ Server: u.String(), - CertificateAuthorityData: ca, + CertificateAuthorityData: infraContext.CA, } // use existing kubeContext if possible which may contain // user-defined overrides. preserve them if possible - kubeContext, ok := kubeConfig.Contexts[context] + kubeContext, ok := kubeConfig.Contexts[contextName] if !ok { kubeContext = &clientcmdapi.Context{ - Cluster: context, + Cluster: contextName, AuthInfo: user.Name, - Namespace: namespace, + Namespace: infraContext.Namespace, } } - if namespace != "" { - // force the namespace if defined by Infra - if kubeContext.Namespace != namespace { - kubeContext.Namespace = namespace - } - } - - kubeConfig.Contexts[context] = kubeContext + kubeConfig.Contexts[contextName] = kubeContext executable, err := os.Executable() if err != nil { @@ -173,8 +175,6 @@ func writeKubeconfig(user *api.User, destinations []api.Destination, grants []ap InteractiveMode: clientcmdapi.IfAvailableExecInteractiveMode, }, } - - keep[context] = true } // cleanup others @@ -189,7 +189,7 @@ func writeKubeconfig(user *api.User, destinations []api.Destination, grants []ap continue } - if _, ok := keep[id]; !ok { + if _, ok := infraContexts[id]; !ok { delete(kubeConfig.AuthInfos, ctx.AuthInfo) delete(kubeConfig.Clusters, ctx.Cluster) delete(kubeConfig.Contexts, id) diff --git a/internal/cmd/kubernetes_test.go b/internal/cmd/kubernetes_test.go index b4e250d3ef..fc19503dc6 100644 --- a/internal/cmd/kubernetes_test.go +++ b/internal/cmd/kubernetes_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "gotest.tools/v3/assert" "k8s.io/client-go/tools/clientcmd" @@ -16,12 +17,6 @@ import ( ) func TestWriteKubeconfig(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - t.Setenv("USERPROFILE", home) - kubeConfigPath := filepath.Join(home, "nonexistent", "kubeconfig") - t.Setenv("KUBECONFIG", kubeConfigPath) - user := api.User{Name: "user"} destinations := []api.Destination{ { @@ -48,123 +43,205 @@ func TestWriteKubeconfig(t *testing.T) { }, }, } - grants := []api.Grant{ - { - Resource: "connected", + + run := func(t *testing.T, grants ...api.Grant) clientcmdapi.Config { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + kubeConfigPath := filepath.Join(home, "nonexistent", "kubeconfig") + t.Setenv("KUBECONFIG", kubeConfigPath) + + err := writeKubeconfig(&user, destinations, grants) + assert.NilError(t, err) + + configFileStat, err := os.Stat(kubeConfigPath) + assert.NilError(t, err) + assert.Equal(t, int(configFileStat.Mode().Perm()), 0o600) + + kubeConfig, err := clientConfig().RawConfig() + assert.NilError(t, err) + + return kubeConfig + } + + expectedClusters := map[string]*clientcmdapi.Cluster{ + "infra:connected": { + Server: "https://connected.example.com", + CertificateAuthorityData: []byte(destinationCA), }, } - err := writeKubeconfig(&user, destinations, grants) - assert.NilError(t, err) + expectedAuthInfos := map[string]*clientcmdapi.AuthInfo{ + "user": {}, + } - expected := clientcmdapi.Config{ - Clusters: map[string]*clientcmdapi.Cluster{ + kubeConfigCmpOpts := []cmp.Option{ + cmpopts.EquateEmpty(), + cmpopts.IgnoreFields(clientcmdapi.Context{}, "LocationOfOrigin"), + cmpopts.IgnoreFields(clientcmdapi.Cluster{}, "LocationOfOrigin"), + cmpopts.IgnoreFields(clientcmdapi.AuthInfo{}, "LocationOfOrigin"), + cmpopts.IgnoreFields(clientcmdapi.AuthInfo{}, "Exec"), + } + + t.Run("OneNamespace", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ "infra:connected": { - Server: "https://connected.example.com", - CertificateAuthorityData: []byte(destinationCA), + AuthInfo: "user", + Cluster: "infra:connected", + Namespace: "namespace", }, - }, - Contexts: map[string]*clientcmdapi.Context{ + } + + actual := run(t, api.Grant{Resource: "connected.namespace"}) + + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) + }) + + t.Run("MultipleNamespaces", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ + "infra:connected": { + AuthInfo: "user", + Cluster: "infra:connected", + Namespace: "namespace", + }, + } + + grants := []api.Grant{ + {Resource: "connected.namespace"}, + {Resource: "connected.namespace2"}, + {Resource: "connected.namespace3"}, + } + + actual := run(t, grants...) + + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) + }) + + t.Run("DefaultNamespace", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ "infra:connected": { AuthInfo: "user", Cluster: "infra:connected", }, - }, - AuthInfos: map[string]*clientcmdapi.AuthInfo{ - "user": {}, - }, - } - - configFileStats, err := os.Stat(kubeConfigPath) - assert.NilError(t, err) + } - permissions := configFileStats.Mode().Perm() - assert.Equal(t, int(permissions), 0o600) // kube config should not be world or group readable, only user read/write + grants := []api.Grant{ + {Resource: "connected.default"}, + } - actual, err := clientConfig().RawConfig() - assert.NilError(t, err) + actual := run(t, grants...) - assert.DeepEqual(t, expected, actual, - cmpopts.EquateEmpty(), - cmpopts.IgnoreFields(clientcmdapi.Cluster{}, "LocationOfOrigin"), - cmpopts.IgnoreFields(clientcmdapi.Context{}, "LocationOfOrigin"), - cmpopts.IgnoreFields(clientcmdapi.AuthInfo{}, "LocationOfOrigin"), - cmpopts.IgnoreFields(clientcmdapi.AuthInfo{}, "Exec"), - ) + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) + }) - t.Run("clearKubeconfig", func(t *testing.T) { - err := clearKubeconfig() - assert.NilError(t, err) + t.Run("DefaultAndMultipleNamespaces", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ + "infra:connected": { + AuthInfo: "user", + Cluster: "infra:connected", + }, + } - expected := clientcmdapi.Config{ - Clusters: map[string]*clientcmdapi.Cluster{}, - Contexts: map[string]*clientcmdapi.Context{}, - AuthInfos: map[string]*clientcmdapi.AuthInfo{}, + grants := []api.Grant{ + {Resource: "connected.namespace"}, + {Resource: "connected.namespace2"}, + {Resource: "connected.default"}, + {Resource: "connected.namespace3"}, } - actual, err := clientConfig().RawConfig() - assert.NilError(t, err) + actual := run(t, grants...) - assert.DeepEqual(t, expected, actual, cmpopts.EquateEmpty()) + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) }) -} -func TestWriteKubeconfig_UserNamespaceOverride(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - t.Setenv("USERPROFILE", home) + t.Run("Cluster", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ + "infra:connected": { + AuthInfo: "user", + Cluster: "infra:connected", + }, + } - kubeconfig := filepath.Join(home, "kubeconfig") - t.Setenv("KUBECONFIG", kubeconfig) + actual := run(t, api.Grant{Resource: "connected"}) - expected := clientcmdapi.Config{ - Clusters: map[string]*clientcmdapi.Cluster{ - "infra:cluster": { - Server: "https://cluster.example.com", - CertificateAuthorityData: []byte(destinationCA), + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) + }) + + t.Run("ClusterAndDefaultNamespace", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ + "infra:connected": { + AuthInfo: "user", + Cluster: "infra:connected", }, - }, - Contexts: map[string]*clientcmdapi.Context{ - "infra:cluster": { - AuthInfo: "user", - Cluster: "infra:cluster", - Namespace: "override", + } + + grants := []api.Grant{ + {Resource: "connected.default"}, + {Resource: "connected"}, + } + + actual := run(t, grants...) + + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) + }) + + t.Run("ClusterAndMultipleNamespaces", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ + "infra:connected": { + AuthInfo: "user", + Cluster: "infra:connected", }, - }, - AuthInfos: map[string]*clientcmdapi.AuthInfo{ - "user": {}, - }, - } + } - err := clientcmd.WriteToFile(expected, kubeconfig) - assert.NilError(t, err) + grants := []api.Grant{ + {Resource: "connected.namespace"}, + {Resource: "connected.namespace2"}, + {Resource: "connected.namespace3"}, + {Resource: "connected"}, + } - user := api.User{Name: "user"} - destinations := []api.Destination{ - { - Name: "cluster", - Connected: true, - Connection: api.DestinationConnection{ - URL: "cluster.example.com", - CA: destinationCA, + actual := run(t, grants...) + + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) + }) + + t.Run("OmitUnavailableClusters", func(t *testing.T) { + expectedContexts := map[string]*clientcmdapi.Context{ + "infra:connected": { + AuthInfo: "user", + Cluster: "infra:connected", }, - }, - } - grants := []api.Grant{ - { - Resource: "cluster", - }, - } + } - err = writeKubeconfig(&user, destinations, grants) - assert.NilError(t, err) + grants := []api.Grant{ + {Resource: "connected"}, + {Resource: "disconnected"}, + {Resource: "pending"}, + } - actual, err := clientConfig().RawConfig() - assert.NilError(t, err) - assert.Equal(t, actual.Contexts["infra:cluster"].Namespace, "override") + actual := run(t, grants...) + + assert.DeepEqual(t, actual.Contexts, expectedContexts, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.Clusters, expectedClusters, kubeConfigCmpOpts...) + assert.DeepEqual(t, actual.AuthInfos, expectedAuthInfos, kubeConfigCmpOpts...) + }) } -func TestWriteKubeconfig_UserNamespaceOverrideResetNamespacedContext(t *testing.T) { +func TestWriteKubeconfig_UserNamespaceOverride(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) t.Setenv("USERPROFILE", home) @@ -174,13 +251,13 @@ func TestWriteKubeconfig_UserNamespaceOverrideResetNamespacedContext(t *testing. expected := clientcmdapi.Config{ Clusters: map[string]*clientcmdapi.Cluster{ - "infra:cluster:default": { + "infra:cluster": { Server: "https://cluster.example.com", CertificateAuthorityData: []byte(destinationCA), }, }, Contexts: map[string]*clientcmdapi.Context{ - "infra:cluster:default": { + "infra:cluster": { AuthInfo: "user", Cluster: "infra:cluster", Namespace: "override", @@ -207,7 +284,7 @@ func TestWriteKubeconfig_UserNamespaceOverrideResetNamespacedContext(t *testing. } grants := []api.Grant{ { - Resource: "cluster.default", + Resource: "cluster", }, } @@ -216,7 +293,7 @@ func TestWriteKubeconfig_UserNamespaceOverrideResetNamespacedContext(t *testing. actual, err := clientConfig().RawConfig() assert.NilError(t, err) - assert.Equal(t, actual.Contexts["infra:cluster:default"].Namespace, "default") + assert.Equal(t, actual.Contexts["infra:cluster"].Namespace, "override") } func TestSafelyWriteConfigToFile(t *testing.T) { diff --git a/internal/cmd/use_test.go b/internal/cmd/use_test.go index 9834d0b007..2c7365c15e 100644 --- a/internal/cmd/use_test.go +++ b/internal/cmd/use_test.go @@ -120,8 +120,8 @@ func TestUse(t *testing.T) { kubeconfig, err := clientConfig().RawConfig() assert.NilError(t, err) - assert.Equal(t, len(kubeconfig.Clusters), 2) - assert.Equal(t, len(kubeconfig.Contexts), 2) + assert.Equal(t, len(kubeconfig.Clusters), 1) + assert.Equal(t, len(kubeconfig.Contexts), 1) assert.Equal(t, len(kubeconfig.AuthInfos), 1) assert.Equal(t, kubeconfig.CurrentContext, "infra:cluster") assert.Assert(t, is.Contains(kubeconfig.AuthInfos, "testuser@example.com")) @@ -136,29 +136,12 @@ func TestUse(t *testing.T) { kubeconfig, err := clientConfig().RawConfig() assert.NilError(t, err) - assert.Equal(t, len(kubeconfig.Clusters), 2) - assert.Equal(t, len(kubeconfig.Contexts), 2) + assert.Equal(t, len(kubeconfig.Clusters), 1) + assert.Equal(t, len(kubeconfig.Contexts), 1) assert.Equal(t, len(kubeconfig.AuthInfos), 1) - assert.Equal(t, kubeconfig.CurrentContext, "infra:cluster:namespace") - assert.Assert(t, is.Contains(kubeconfig.AuthInfos, "testuser@example.com")) - }) - - t.Run("InfraUse", func(t *testing.T) { - setup(t) - - err := Run(context.Background(), "use", "infra:cluster") - assert.NilError(t, err) - - kubeconfig, err := clientConfig().RawConfig() - assert.NilError(t, err) assert.Equal(t, kubeconfig.CurrentContext, "infra:cluster") - - err = Run(context.Background(), "use", "infra:cluster.namespace") - assert.NilError(t, err) - - kubeconfig, err = clientConfig().RawConfig() - assert.NilError(t, err) - assert.Equal(t, kubeconfig.CurrentContext, "infra:cluster:namespace") + assert.Equal(t, kubeconfig.Contexts[kubeconfig.CurrentContext].Namespace, "namespace") + assert.Assert(t, is.Contains(kubeconfig.AuthInfos, "testuser@example.com")) }) t.Run("UseUnknown", func(t *testing.T) {