From 68f82748c1bb80cfca10d5a6dfd113b3d50d742e Mon Sep 17 00:00:00 2001 From: poneding Date: Tue, 31 Dec 2024 23:03:27 +0800 Subject: [PATCH 1/3] fix: cluster and user name conflict on add kubeconfig with same context cluster and user --- cmd/add.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/add.go b/cmd/add.go index 8d9568de..7b19036c 100644 --- a/cmd/add.go +++ b/cmd/add.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" "golang.org/x/exp/slices" + "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -245,7 +246,7 @@ func (kc *KubeConfigOption) handleContext(oldConfig *clientcmdapi.Config, isClusterNameExist, isUserNameExist := checkClusterAndUserName(oldConfig, ctx.Cluster, ctx.AuthInfo) newConfig := clientcmdapi.NewConfig() - suffix := HashSufString(name) + suffix := rand.String(10) if isClusterNameExist { clusterNameSuffix = "-" + suffix @@ -278,7 +279,7 @@ func (kc *KubeConfigOption) handleContext(oldConfig *clientcmdapi.Config, func addExample() string { return ` # Merge test.yaml with $HOME/.kube/config -kubecm add -f test.yaml +kubecm add -f test.yaml # Merge test.yaml with $HOME/.kube/config and add a prefix before context name kubecm add -cf test.yaml --context-prefix test # Merge test.yaml with $HOME/.kube/config and define the attributes used for composing the context name From 050e806d7710746e637ac00d79fbec1a334c4ed4 Mon Sep 17 00:00:00 2001 From: poneding Date: Fri, 3 Jan 2025 16:30:25 +0800 Subject: [PATCH 2/3] fix: cluster and user name conflict on add kubeconfig with same context cluster and user --- cmd/add.go | 67 +++++++++++++++--- cmd/add_test.go | 175 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 208 insertions(+), 34 deletions(-) diff --git a/cmd/add.go b/cmd/add.go index 7b19036c..8745d28c 100644 --- a/cmd/add.go +++ b/cmd/add.go @@ -218,22 +218,44 @@ func checkContextName(name string, oldConfig *clientcmdapi.Config) bool { return false } -func checkClusterAndUserName(oldConfig *clientcmdapi.Config, newClusterName, newUserName string) (bool, bool) { +// checkClusterAndUserName check if the cluster and user exist in the same context, or just cluster or user exist +func checkClusterAndUserName(oldConfig *clientcmdapi.Config, newClusterName, newUserName string) (bool, bool, bool) { var ( - isClusterNameExist bool - isUserNameExist bool + clusterAndUserNameExistInSameContext bool + justClusterNameExist bool + justUserNameExist bool ) for _, ctx := range oldConfig.Contexts { + if ctx.Cluster == newClusterName && ctx.AuthInfo == newUserName { + clusterAndUserNameExistInSameContext = true + break + } if ctx.Cluster == newClusterName { - isClusterNameExist = true + justClusterNameExist = true } if ctx.AuthInfo == newUserName { - isUserNameExist = true + justUserNameExist = true } } - return isClusterNameExist, isUserNameExist + return clusterAndUserNameExistInSameContext, justClusterNameExist, justUserNameExist +} + +// isSameKubeConfigAlreadyExist return true if the same kubeconfig is already +// exist, assert by cluster, user name and corresponding cluster and user info +func (kc *KubeConfigOption) isSameKubeConfigAlreadyExist(oldConfig *clientcmdapi.Config, ctx *clientcmdapi.Context) bool { + oldCluster, ok := oldConfig.Clusters[ctx.Cluster] + if !ok { + return false + } + + oldUser, ok := oldConfig.AuthInfos[ctx.AuthInfo] + if !ok { + return false + } + + return reflect.DeepEqual(oldCluster, kc.config.Clusters[ctx.Cluster]) && reflect.DeepEqual(oldUser, kc.config.AuthInfos[ctx.AuthInfo]) } func (kc *KubeConfigOption) handleContext(oldConfig *clientcmdapi.Config, @@ -244,14 +266,20 @@ func (kc *KubeConfigOption) handleContext(oldConfig *clientcmdapi.Config, userNameSuffix string ) - isClusterNameExist, isUserNameExist := checkClusterAndUserName(oldConfig, ctx.Cluster, ctx.AuthInfo) newConfig := clientcmdapi.NewConfig() + bothExistInSameContext, justClusterNameExist, justUserNameExist := checkClusterAndUserName(oldConfig, ctx.Cluster, ctx.AuthInfo) + // if same kubeconfig is already exist, skip it + if bothExistInSameContext { + if kc.isSameKubeConfigAlreadyExist(oldConfig, ctx) { + return newConfig + } + } suffix := rand.String(10) - if isClusterNameExist { + if justClusterNameExist { clusterNameSuffix = "-" + suffix } - if isUserNameExist { + if justUserNameExist { userNameSuffix = "-" + suffix } @@ -267,8 +295,25 @@ func (kc *KubeConfigOption) handleContext(oldConfig *clientcmdapi.Config, cluster.CertificateAuthorityData = nil } - newConfig.AuthInfos[userName] = kc.config.AuthInfos[newCtx.AuthInfo] - newConfig.Clusters[clusterName] = cluster + var clusterInfoExist, userInfoExist bool + for _, oldCluster := range oldConfig.Clusters { + if reflect.DeepEqual(oldCluster, cluster) { + clusterInfoExist = true + } + } + if !clusterInfoExist { + newConfig.Clusters[clusterName] = cluster + } + + for _, oldUser := range oldConfig.AuthInfos { + if reflect.DeepEqual(oldUser, kc.config.AuthInfos[newCtx.AuthInfo]) { + userInfoExist = true + } + } + if !userInfoExist { + newConfig.AuthInfos[userName] = kc.config.AuthInfos[newCtx.AuthInfo] + } + newConfig.Contexts[name] = newCtx newConfig.Contexts[name].AuthInfo = userName newConfig.Contexts[name].Cluster = clusterName diff --git a/cmd/add_test.go b/cmd/add_test.go index b0b79053..31af35ce 100644 --- a/cmd/add_test.go +++ b/cmd/add_test.go @@ -5,7 +5,6 @@ import ( "testing" "k8s.io/client-go/tools/clientcmd" - clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -40,15 +39,6 @@ var ( "federal": {AuthInfo: "red-user", Cluster: "cow-cluster", Namespace: "hammer-ns"}, }, } - handleConfig = clientcmdapi.Config{ - AuthInfos: map[string]*clientcmdapi.AuthInfo{ - "red-user-cbc897d6ch": {Token: "red-token"}}, - Clusters: map[string]*clientcmdapi.Cluster{ - "cow-cluster-cbc897d6ch": {Server: "http://cow.org:8080"}}, - Contexts: map[string]*clientcmdapi.Context{ - "federal-context": {AuthInfo: "red-user-cbc897d6ch", Cluster: "cow-cluster-cbc897d6ch", Namespace: "hammer-ns"}, - }, - } handleNotExistConfig = clientcmdapi.Config{ AuthInfos: map[string]*clientcmdapi.AuthInfo{ "not-exist": {Token: "not-exist-token"}}, @@ -60,24 +50,18 @@ var ( } mergedConfig = clientcmdapi.Config{ AuthInfos: map[string]*clientcmdapi.AuthInfo{ - "black-user": {Token: "black-token"}, - "red-user": {Token: "red-token"}, - "red-user-cbc897d6ch": {Token: "red-token"}, - "black-user-d2m9fd8b7d": {Token: "black-token"}, - "not-exist": {Token: "not-exist-token"}, + "black-user": {Token: "black-token"}, + "red-user": {Token: "red-token"}, + "not-exist": {Token: "not-exist-token"}, }, Clusters: map[string]*clientcmdapi.Cluster{ - "pig-cluster": {Server: "http://pig.org:8080"}, - "cow-cluster": {Server: "http://cow.org:8080"}, - "cow-cluster-cbc897d6ch": {Server: "http://cow.org:8080"}, - "pig-cluster-d2m9fd8b7d": {Server: "http://pig.org:8080"}, - "not-exist": {Server: "http://not.exist:8080"}, + "pig-cluster": {Server: "http://pig.org:8080"}, + "cow-cluster": {Server: "http://cow.org:8080"}, + "not-exist": {Server: "http://not.exist:8080"}, }, Contexts: map[string]*clientcmdapi.Context{ "root": {AuthInfo: "black-user", Cluster: "pig-cluster", Namespace: "saw-ns"}, "federal": {AuthInfo: "red-user", Cluster: "cow-cluster", Namespace: "hammer-ns"}, - "root-context": {AuthInfo: "black-user-d2m9fd8b7d", Cluster: "pig-cluster-d2m9fd8b7d", Namespace: "saw-ns"}, - "federal-context": {AuthInfo: "red-user-cbc897d6ch", Cluster: "cow-cluster-cbc897d6ch", Namespace: "hammer-ns"}, "not-exist-context": {AuthInfo: "not-exist", Cluster: "not-exist", Namespace: "not-exist-ns"}, }, } @@ -250,7 +234,7 @@ func TestKubeConfig_handleContext(t *testing.T) { want *clientcmdapi.Config }{ // TODO: Add more test cases. - {"one", fields{config: newConfig}, args{"federal-context", &testCtx}, &handleConfig}, + {"one", fields{config: newConfig}, args{"federal-context", &testCtx}, clientcmdapi.NewConfig()}, {"two", fields{config: newConfig}, args{"not-exist-context", &testNotExistCtx}, &handleNotExistConfig}, } for _, tt := range tests { @@ -567,3 +551,148 @@ func TestAddToLocal_InsecureSkipTLSVerify(t *testing.T) { }) } } + +func TestIsSameKubeConfigAlreadyExist(t *testing.T) { + oldCfg := &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "test-cluster": { + Server: "https://test.example.org", + CertificateAuthority: "/fake/ca/path", + CertificateAuthorityData: []byte("fake-ca-data"), + InsecureSkipTLSVerify: false, + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "test-authinfo": {Token: "test-token"}, + }, + Contexts: map[string]*clientcmdapi.Context{ + "test-context": { + AuthInfo: "test-authinfo", + Cluster: "test-cluster", + Namespace: "test-namespace", + }, + }, + CurrentContext: "test-context", + } + + tests := []struct { + name string + kco *KubeConfigOption + exist bool + }{ + { + name: "same cluster and user info already exist", + kco: &KubeConfigOption{ + config: &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "test-cluster": { + Server: "https://test.example.org", + CertificateAuthority: "/fake/ca/path", + CertificateAuthorityData: []byte("fake-ca-data"), + InsecureSkipTLSVerify: false, + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "test-authinfo": {Token: "test-token"}, + }, + Contexts: map[string]*clientcmdapi.Context{ + "test-context": { + AuthInfo: "test-authinfo", + Cluster: "test-cluster", + Namespace: "test-namespace", + }, + }, + CurrentContext: "test-context", + }, + }, + exist: true, + }, + { + name: "different cluster and user info", + kco: &KubeConfigOption{ + config: &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "test-cluster": { + Server: "https://test1.example.org", + CertificateAuthority: "/fake/ca/path", + CertificateAuthorityData: []byte("fake-ca-data"), + InsecureSkipTLSVerify: false, + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "test-authinfo": {Token: "test1-token"}, + }, + Contexts: map[string]*clientcmdapi.Context{ + "test-context": { + AuthInfo: "test-authinfo", + Cluster: "test-cluster", + Namespace: "test-namespace", + }, + }, + CurrentContext: "test-context", + }, + }, + exist: false, + }, + { + name: "different cluster info but same user info", + kco: &KubeConfigOption{ + config: &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "test-cluster": { + Server: "https://test2.example.org", + CertificateAuthority: "/fake/ca/path", + CertificateAuthorityData: []byte("fake-ca-data"), + InsecureSkipTLSVerify: false, + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "test-authinfo": {Token: "test-token"}, + }, + Contexts: map[string]*clientcmdapi.Context{ + "test-context": { + AuthInfo: "test-authinfo", + Cluster: "test-cluster", + Namespace: "test-namespace", + }, + }, + CurrentContext: "test-context", + }, + }, + exist: false, + }, + { + name: "same cluster info but different user info", + kco: &KubeConfigOption{ + config: &clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "test-cluster": { + Server: "https://test.example.org", + CertificateAuthority: "/fake/ca/path", + CertificateAuthorityData: []byte("fake-ca-data"), + InsecureSkipTLSVerify: false, + }, + }, + AuthInfos: map[string]*clientcmdapi.AuthInfo{ + "test-authinfo": {Token: "test2-token"}, + }, + Contexts: map[string]*clientcmdapi.Context{ + "test-context": { + AuthInfo: "test-authinfo", + Cluster: "test-cluster", + Namespace: "test-namespace", + }, + }, + CurrentContext: "test-context", + }, + }, + exist: false, + }, + } + + for _, tt := range tests { + if got := tt.kco.isSameKubeConfigAlreadyExist(oldCfg, tt.kco.config.Contexts["test-context"]); got != tt.exist { + t.Errorf("IsSameKubeConfigAlreadyExist() = %v, want %v", got, tt.exist) + } + } +} From b214cd35022e918127b0739dee8b3ecd7b68a86e Mon Sep 17 00:00:00 2001 From: poneding Date: Fri, 24 Jan 2025 14:45:42 +0800 Subject: [PATCH 3/3] fix: allow rename equals to filename and loop check --- cmd/add.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cmd/add.go b/cmd/add.go index 8745d28c..5792ca51 100644 --- a/cmd/add.go +++ b/cmd/add.go @@ -1,7 +1,6 @@ package cmd import ( - "errors" "fmt" "io" "os" @@ -171,17 +170,20 @@ func (kc *KubeConfigOption) handleContexts(oldConfig *clientcmdapi.Config, conte } } - if checkContextName(newName, oldConfig) { + var quitNewName bool + for checkContextName(newName, oldConfig) { nameConfirm := BoolUI(fmt.Sprintf("「%s」 Name already exists, do you want to rename it? (If you select `False`, this context will not be merged)", newName)) if nameConfirm == "True" { newName = PromptUI("Rename", newName) - if newName == kc.fileName { - return nil, errors.New("need to rename") - } - } else { continue + } else { + quitNewName = true + break } } + if quitNewName { + continue + } itemConfig := kc.handleContext(oldConfig, newName, ctx) newConfig = appendConfig(newConfig, itemConfig) fmt.Printf("Add Context: %s \n", newName) @@ -229,7 +231,6 @@ func checkClusterAndUserName(oldConfig *clientcmdapi.Config, newClusterName, new for _, ctx := range oldConfig.Contexts { if ctx.Cluster == newClusterName && ctx.AuthInfo == newUserName { clusterAndUserNameExistInSameContext = true - break } if ctx.Cluster == newClusterName { justClusterNameExist = true