From 0613fdeccb5acf249bc281fc82a1889cfe51b729 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Sun, 19 Oct 2025 17:03:46 +0200 Subject: [PATCH] kubeadm: add missing cluster-info context validation When retrieving the cluster-info CM, ensure the cluster pointed out by the current context in the kubeconfig is validated. Add unit test for the above. Make the function GetClusterFromKubeConfig() to return various errors. Handle the errors on call sites. Add unit tests for the update. The above changes prevent panics when the users has manually edited and malformed the kubeconfig in the cluster-info CM. --- cmd/kubeadm/app/cmd/join.go | 5 +- cmd/kubeadm/app/cmd/util/join.go | 6 +- cmd/kubeadm/app/cmd/util/join_test.go | 2 +- cmd/kubeadm/app/discovery/discovery.go | 6 +- cmd/kubeadm/app/discovery/file/file.go | 11 ++-- cmd/kubeadm/app/discovery/token/token.go | 6 +- cmd/kubeadm/app/discovery/token/token_test.go | 34 ++++++++++- cmd/kubeadm/app/util/kubeconfig/kubeconfig.go | 12 ++-- .../app/util/kubeconfig/kubeconfig_test.go | 59 +++++++++++++------ 9 files changed, 104 insertions(+), 37 deletions(-) diff --git a/cmd/kubeadm/app/cmd/join.go b/cmd/kubeadm/app/cmd/join.go index f10b4a6a355..20920abe6e1 100644 --- a/cmd/kubeadm/app/cmd/join.go +++ b/cmd/kubeadm/app/cmd/join.go @@ -692,7 +692,10 @@ func fetchInitConfigurationFromJoinConfiguration(cfg *kubeadmapi.JoinConfigurati } // Create the final KubeConfig file with the cluster name discovered after fetching the cluster configuration - _, clusterinfo := kubeconfigutil.GetClusterFromKubeConfig(tlsBootstrapCfg) + _, clusterinfo, err := kubeconfigutil.GetClusterFromKubeConfig(tlsBootstrapCfg) + if err != nil { + return nil, errors.Wrap(err, "the TLS bootstrap kubeconfig is malformed") + } tlsBootstrapCfg.Clusters = map[string]*clientcmdapi.Cluster{ initConfiguration.ClusterName: clusterinfo, } diff --git a/cmd/kubeadm/app/cmd/util/join.go b/cmd/kubeadm/app/cmd/util/join.go index feded1d06d7..aad2e4dff0f 100644 --- a/cmd/kubeadm/app/cmd/util/join.go +++ b/cmd/kubeadm/app/cmd/util/join.go @@ -56,9 +56,9 @@ func getJoinCommand(kubeConfigFile, token, key string, controlPlane, skipTokenPr } // load the default cluster config - _, clusterConfig := kubeconfigutil.GetClusterFromKubeConfig(config) - if clusterConfig == nil { - return "", errors.New("failed to get default cluster config") + _, clusterConfig, err := kubeconfigutil.GetClusterFromKubeConfig(config) + if err != nil { + return "", errors.Wrapf(err, "malformed kubeconfig file: %s", kubeConfigFile) } // load CA certificates from the kubeconfig (either from PEM data or by file path) diff --git a/cmd/kubeadm/app/cmd/util/join_test.go b/cmd/kubeadm/app/cmd/util/join_test.go index c9d08f3b17a..d883ac0479d 100644 --- a/cmd/kubeadm/app/cmd/util/join_test.go +++ b/cmd/kubeadm/app/cmd/util/join_test.go @@ -133,7 +133,7 @@ func TestGetJoinCommand(t *testing.T) { kubeConfig: &clientcmdapi.Config{}, token: "test-token", expectError: true, - errorMessage: "failed to get default cluster config", + errorMessage: "the current context is invalid", }, { name: "Error when CA certificate is invalid", diff --git a/cmd/kubeadm/app/discovery/discovery.go b/cmd/kubeadm/app/discovery/discovery.go index dbb45a53890..ae51a902dc0 100644 --- a/cmd/kubeadm/app/discovery/discovery.go +++ b/cmd/kubeadm/app/discovery/discovery.go @@ -21,6 +21,7 @@ import ( clientset "k8s.io/client-go/kubernetes" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + bootstrapapi "k8s.io/cluster-bootstrap/token/api" "k8s.io/klog/v2" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" @@ -51,7 +52,10 @@ func For(client clientset.Interface, cfg *kubeadmapi.JoinConfiguration) (*client if len(cfg.Discovery.TLSBootstrapToken) != 0 { klog.V(1).Info("[discovery] Using provided TLSBootstrapToken as authentication credentials for the join process") - _, clusterinfo := kubeconfigutil.GetClusterFromKubeConfig(config) + _, clusterinfo, err := kubeconfigutil.GetClusterFromKubeConfig(config) + if err != nil { + return nil, errors.Wrapf(err, "malformed kubeconfig in the %s ConfigMap", bootstrapapi.ConfigMapClusterInfo) + } return kubeconfigutil.CreateWithToken( clusterinfo.Server, kubeadmapiv1.DefaultClusterName, diff --git a/cmd/kubeadm/app/discovery/file/file.go b/cmd/kubeadm/app/discovery/file/file.go index c3a8c176c55..536781b794b 100644 --- a/cmd/kubeadm/app/discovery/file/file.go +++ b/cmd/kubeadm/app/discovery/file/file.go @@ -52,9 +52,9 @@ func ValidateConfigInfo(config *clientcmdapi.Config, discoveryTimeout time.Durat if len(config.Clusters) < 1 { return nil, errors.New("the provided kubeconfig file must have at least one Cluster defined") } - currentClusterName, currentCluster := kubeconfigutil.GetClusterFromKubeConfig(config) - if currentCluster == nil { - return nil, errors.New("the provided kubeconfig file must have a unnamed Cluster or a CurrentContext that specifies a non-nil Cluster") + currentClusterName, currentCluster, err := kubeconfigutil.GetClusterFromKubeConfig(config) + if err != nil { + return nil, errors.Wrap(err, "the provided kubeconfig file is malformed") } if err := clientcmd.Validate(*config); err != nil { return nil, err @@ -124,7 +124,10 @@ func ValidateConfigInfo(config *clientcmdapi.Config, discoveryTimeout time.Durat return config, nil } - _, refreshedCluster := kubeconfigutil.GetClusterFromKubeConfig(refreshedBaseKubeConfig) + _, refreshedCluster, err := kubeconfigutil.GetClusterFromKubeConfig(refreshedBaseKubeConfig) + if err != nil { + return nil, errors.Wrapf(err, "malformed kubeconfig in the %s ConfigMap", bootstrapapi.ConfigMapClusterInfo) + } if currentCluster.Server != refreshedCluster.Server { klog.Warningf("[discovery] the API Server endpoint %q in use is different from the endpoint %q which defined in the %s ConfigMap", currentCluster.Server, refreshedCluster.Server, bootstrapapi.ConfigMapClusterInfo) } diff --git a/cmd/kubeadm/app/discovery/token/token.go b/cmd/kubeadm/app/discovery/token/token.go index c05be648896..195678b1430 100644 --- a/cmd/kubeadm/app/discovery/token/token.go +++ b/cmd/kubeadm/app/discovery/token/token.go @@ -103,9 +103,9 @@ func retrieveValidatedConfigInfo(client clientset.Interface, cfg *kubeadmapi.Dis return nil, errors.Wrapf(err, "couldn't parse the kubeconfig file in the %s ConfigMap", bootstrapapi.ConfigMapClusterInfo) } - // The ConfigMap should contain a single cluster - if len(insecureConfig.Clusters) != 1 { - return nil, errors.Errorf("expected the kubeconfig file in the %s ConfigMap to have a single cluster, but it had %d", bootstrapapi.ConfigMapClusterInfo, len(insecureConfig.Clusters)) + _, _, err = kubeconfigutil.GetClusterFromKubeConfig(insecureConfig) + if err != nil { + return nil, errors.Wrapf(err, "malformed kubeconfig in the %s ConfigMap", bootstrapapi.ConfigMapClusterInfo) } // If no TLS root CA pinning was specified, we're done diff --git a/cmd/kubeadm/app/discovery/token/token_test.go b/cmd/kubeadm/app/discovery/token/token_test.go index a823af66ccd..2614662dcea 100644 --- a/cmd/kubeadm/app/discovery/token/token_test.go +++ b/cmd/kubeadm/app/discovery/token/token_test.go @@ -77,10 +77,12 @@ users: null name string tokenID string tokenSecret string + currentContextCluster string cfg *kubeadmapi.Discovery configMap *fakeConfigMap delayedJWSSignaturePatch bool expectedError bool + expectedErrorString string }{ { // This is the default behavior. The JWS signature is patched after the cluster-info ConfigMap is created @@ -130,6 +132,24 @@ users: null data: nil, }, }, + { + name: "invalid: the kubeconfig in the configmap has the wrong current context", + tokenID: "123456", + tokenSecret: "abcdef1234567890", + cfg: &kubeadmapi.Discovery{ + BootstrapToken: &kubeadmapi.BootstrapTokenDiscovery{ + Token: "123456.abcdef1234567890", + CACertHashes: []string{caCertHash}, + }, + }, + configMap: &fakeConfigMap{ + name: bootstrapapi.ConfigMapClusterInfo, + data: nil, + }, + currentContextCluster: "foo", + expectedError: true, + expectedErrorString: `malformed kubeconfig in the cluster-info ConfigMap: no matching cluster for the current context: token-bootstrap-client@somecluster`, + }, { name: "invalid: token format is invalid", tokenID: "foo", @@ -216,6 +236,13 @@ users: null for _, test := range tests { t.Run(test.name, func(t *testing.T) { kubeconfig := buildSecureBootstrapKubeConfig("127.0.0.1", []byte(caCert), "somecluster") + if len(test.currentContextCluster) > 0 { + currentContext := kubeconfig.Contexts[kubeconfig.CurrentContext] + if currentContext == nil { + t.Fatal("unexpected nil current context") + } + currentContext.Cluster = test.currentContextCluster + } kubeconfigBytes, err := clientcmd.Write(*kubeconfig) if err != nil { t.Fatalf("cannot marshal kubeconfig %v", err) @@ -267,8 +294,13 @@ users: null t.Errorf("expected error %v, got %v, error: %v", test.expectedError, err != nil, err) } - // Return if an error is expected if err != nil { + if len(test.expectedErrorString) > 0 && test.expectedErrorString != err.Error() { + t.Fatalf("expected error string: %s, got: %s", + test.expectedErrorString, err.Error()) + } + + // Return if an error is expected return } diff --git a/cmd/kubeadm/app/util/kubeconfig/kubeconfig.go b/cmd/kubeadm/app/util/kubeconfig/kubeconfig.go index 58bb616f50c..1b943645720 100644 --- a/cmd/kubeadm/app/util/kubeconfig/kubeconfig.go +++ b/cmd/kubeadm/app/util/kubeconfig/kubeconfig.go @@ -104,17 +104,21 @@ func WriteToDisk(filename string, kubeconfig *clientcmdapi.Config) error { } // GetClusterFromKubeConfig returns the default Cluster of the specified KubeConfig -func GetClusterFromKubeConfig(config *clientcmdapi.Config) (string, *clientcmdapi.Cluster) { +func GetClusterFromKubeConfig(config *clientcmdapi.Config) (string, *clientcmdapi.Cluster, error) { // If there is an unnamed cluster object, use it if config.Clusters[""] != nil { - return "", config.Clusters[""] + return "", config.Clusters[""], nil } currentContext := config.Contexts[config.CurrentContext] if currentContext != nil { - return currentContext.Cluster, config.Clusters[currentContext.Cluster] + if config.Clusters[currentContext.Cluster] != nil { + return currentContext.Cluster, config.Clusters[currentContext.Cluster], nil + } + return "", nil, errors.Errorf("no matching cluster for the current context: %s", config.CurrentContext) } - return "", nil + + return "", nil, errors.Errorf("the current context is invalid: %s", config.CurrentContext) } // HasAuthenticationCredentials returns true if the current user has valid authentication credentials for diff --git a/cmd/kubeadm/app/util/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/util/kubeconfig/kubeconfig_test.go index 45934874b87..d108d1acc29 100644 --- a/cmd/kubeadm/app/util/kubeconfig/kubeconfig_test.go +++ b/cmd/kubeadm/app/util/kubeconfig/kubeconfig_test.go @@ -351,17 +351,49 @@ func TestGetClusterFromKubeConfig(t *testing.T) { config *clientcmdapi.Config expectedClusterName string expectedCluster *clientcmdapi.Cluster + expectedError bool }{ { - name: "cluster is empty", + name: "an existing cluster with an empty name is returned directly", config: &clientcmdapi.Config{ - CurrentContext: "kubernetes", + Clusters: map[string]*clientcmdapi.Cluster{ + "": {Server: "http://foo:8080"}, + }, + }, + expectedClusterName: "", + expectedCluster: &clientcmdapi.Cluster{ + Server: "http://foo:8080", + }, + }, + { + name: "the current context is invalid", + config: &clientcmdapi.Config{ + CurrentContext: "foo", + Contexts: map[string]*clientcmdapi.Context{ + "bar": {AuthInfo: "bar", Cluster: "bar"}, + }, }, expectedClusterName: "", expectedCluster: nil, + expectedError: true, }, { - name: "cluster and currentContext are not empty", + name: "no matching cluster for the current context", + config: &clientcmdapi.Config{ + CurrentContext: "foo", + Contexts: map[string]*clientcmdapi.Context{ + "foo": {AuthInfo: "bar", Cluster: "bar"}, + }, + Clusters: map[string]*clientcmdapi.Cluster{ + "baz": {Server: "https://bar:16443"}, + }, + }, + expectedClusterName: "", + expectedCluster: nil, + expectedError: true, + }, + { + name: "valid current context and cluster", config: &clientcmdapi.Config{ CurrentContext: "foo", Contexts: map[string]*clientcmdapi.Context{ @@ -378,31 +410,20 @@ func TestGetClusterFromKubeConfig(t *testing.T) { Server: "http://foo:8080", }, }, - { - name: "cluster is not empty and currentContext is not in contexts", - config: &clientcmdapi.Config{ - CurrentContext: "foo", - Contexts: map[string]*clientcmdapi.Context{ - "bar": {AuthInfo: "bar", Cluster: "bar"}, - }, - Clusters: map[string]*clientcmdapi.Cluster{ - "foo": {Server: "http://foo:8080"}, - "bar": {Server: "https://bar:16443"}, - }, - }, - expectedClusterName: "", - expectedCluster: nil, - }, } for _, rt := range tests { t.Run(rt.name, func(t *testing.T) { - clusterName, cluster := GetClusterFromKubeConfig(rt.config) + clusterName, cluster, err := GetClusterFromKubeConfig(rt.config) if clusterName != rt.expectedClusterName { t.Errorf("got cluster name = %s, expected %s", clusterName, rt.expectedClusterName) } if !reflect.DeepEqual(cluster, rt.expectedCluster) { t.Errorf("got cluster = %+v, expected %+v", cluster, rt.expectedCluster) } + if (err != nil) != rt.expectedError { + t.Errorf("expected error: %v, got: %v, error: %v", + rt.expectedError, err != nil, err) + } }) } }