From fceb39ecd2a8d3d5182f780634ea3583e2a5b4bd Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 1 Nov 2024 11:42:58 +0200 Subject: [PATCH] kubeadm: ensure proper parsing of SSR username - Split the code that tries to get node name from SSR into a new function getNodeNameFromSSR(). Unit test the function. - Fix error that the "system:nodes:" prefix was not trimmed. - Fix mislearding errors around FetchInitConfigurationFromCluster. This function performs multiple actions, and the "get node" action can also be of type apierrors.NotFound(). This creates confusion in the returned error in enforceRequirement during upgrade. Fix this problem. --- cmd/kubeadm/app/cmd/upgrade/common.go | 7 -- cmd/kubeadm/app/util/config/cluster.go | 31 ++++++--- cmd/kubeadm/app/util/config/cluster_test.go | 73 +++++++++++++++++++++ 3 files changed, 96 insertions(+), 15 deletions(-) diff --git a/cmd/kubeadm/app/cmd/upgrade/common.go b/cmd/kubeadm/app/cmd/upgrade/common.go index d39401a9231..67a47c8cc0a 100644 --- a/cmd/kubeadm/app/cmd/upgrade/common.go +++ b/cmd/kubeadm/app/cmd/upgrade/common.go @@ -26,8 +26,6 @@ import ( "github.com/pkg/errors" "github.com/spf13/pflag" - apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" fakediscovery "k8s.io/client-go/discovery/fake" clientset "k8s.io/client-go/kubernetes" @@ -96,11 +94,6 @@ func enforceRequirements(flagSet *pflag.FlagSet, flags *applyPlanFlags, args []s initCfg, err := configutil.FetchInitConfigurationFromCluster(client, printer, "upgrade/config", false, false) if err != nil { - if apierrors.IsNotFound(err) { - _, _ = printer.Printf("[upgrade/config] In order to upgrade, a ConfigMap called %q in the %q namespace must exist.\n", constants.KubeadmConfigConfigMap, metav1.NamespaceSystem) - _, _ = printer.Printf("[upgrade/config] Use 'kubeadm init phase upload-config --config your-config.yaml' to re-upload it.\n") - err = errors.Errorf("the ConfigMap %q in the %q namespace was not found", constants.KubeadmConfigConfigMap, metav1.NamespaceSystem) - } return nil, nil, nil, nil, errors.Wrap(err, "[upgrade/init config] FATAL") } diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go index 6300ec95b3e..3071a11ca8b 100644 --- a/cmd/kubeadm/app/util/config/cluster.go +++ b/cmd/kubeadm/app/util/config/cluster.go @@ -53,8 +53,9 @@ func FetchInitConfigurationFromCluster(client clientset.Interface, printer outpu if printer == nil { printer = &output.TextPrinter{} } - printer.Printf("[%s] Reading configuration from the cluster...\n", logPrefix) - printer.Printf("[%s] FYI: You can look at this config file with 'kubectl -n %s get cm %s -o yaml'\n", logPrefix, metav1.NamespaceSystem, constants.KubeadmConfigConfigMap) + _, _ = printer.Printf("[%s] Reading configuration from the %q ConfigMap in namespace %q...\n", + logPrefix, constants.KubeadmConfigConfigMap, metav1.NamespaceSystem) + _, _ = printer.Printf("[%s] Use 'kubeadm init phase upload-config --config your-config.yaml' to re-upload it.\n", logPrefix) // Fetch the actual config from cluster cfg, err := getInitConfigurationFromCluster(constants.KubernetesDir, client, newControlPlane, skipComponentConfigs) @@ -139,11 +140,9 @@ func GetNodeName(kubeconfigFile string) (string, error) { if kubeconfigFile != "" { client, err := kubeconfig.ClientSetFromFile(kubeconfigFile) if err == nil { - ssr, err := client.AuthenticationV1().SelfSubjectReviews(). - Create(context.Background(), &authv1.SelfSubjectReview{}, metav1.CreateOptions{}) - - if err == nil && ssr.Status.UserInfo.Username != "" { - return ssr.Status.UserInfo.Username, nil + nodeName, err = getNodeNameFromSSR(client) + if err == nil { + return nodeName, nil } } nodeName, err = getNodeNameFromKubeletConfig(kubeconfigFile) @@ -167,7 +166,7 @@ func GetNodeRegistration(kubeconfigFile string, client clientset.Interface, node } // gets the corresponding node and retrieves attributes stored there. - node, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{}) + node, err := client.CoreV1().Nodes().Get(context.Background(), nodeName, metav1.GetOptions{}) if err != nil { return errors.Wrap(err, "failed to get corresponding node") } @@ -231,6 +230,22 @@ func getNodeNameFromKubeletConfig(fileName string) (string, error) { return strings.TrimPrefix(cert.Subject.CommonName, constants.NodesUserPrefix), nil } +// getNodeNameFromSSR reads the node name from the SelfSubjectReview for a given client. +// If the kubelet.conf is passed as fileName it can be used to retrieve the node name. +func getNodeNameFromSSR(client clientset.Interface) (string, error) { + ssr, err := client.AuthenticationV1().SelfSubjectReviews(). + Create(context.Background(), &authv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return "", err + } + user := ssr.Status.UserInfo.Username + if !strings.HasPrefix(user, constants.NodesUserPrefix) { + return "", errors.Errorf("%q is not a node client, must have %q prefix in the name", + user, constants.NodesUserPrefix) + } + return strings.TrimPrefix(user, constants.NodesUserPrefix), nil +} + func getAPIEndpoint(client clientset.Interface, nodeName string, apiEndpoint *kubeadmapi.APIEndpoint) error { return getAPIEndpointWithRetry(client, nodeName, apiEndpoint, constants.KubernetesAPICallRetryInterval, kubeadmapi.GetActiveTimeouts().KubernetesAPICall.Duration) diff --git a/cmd/kubeadm/app/util/config/cluster_test.go b/cmd/kubeadm/app/util/config/cluster_test.go index d5e44ae4bd7..9ea6ce2adef 100644 --- a/cmd/kubeadm/app/util/config/cluster_test.go +++ b/cmd/kubeadm/app/util/config/cluster_test.go @@ -29,6 +29,7 @@ import ( "github.com/pkg/errors" + authv1 "k8s.io/api/authentication/v1" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -860,3 +861,75 @@ func TestGetRawAPIEndpointFromPodAnnotationWithoutRetry(t *testing.T) { }) } } + +func TestGetNodeNameFromSSR(t *testing.T) { + var tests = []struct { + name string + clientSetup func(*clientsetfake.Clientset) + expectedNodeName string + expectedError bool + }{ + { + name: "valid node name", + clientSetup: func(clientset *clientsetfake.Clientset) { + clientset.PrependReactor("create", "selfsubjectreviews", + func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + obj := &authv1.SelfSubjectReview{ + Status: authv1.SelfSubjectReviewStatus{ + UserInfo: authv1.UserInfo{ + Username: kubeadmconstants.NodesUserPrefix + "foo", + }, + }, + } + return true, obj, nil + }) + }, + expectedNodeName: "foo", + expectedError: false, + }, + { + name: "SSR created but client is not a node", + clientSetup: func(clientset *clientsetfake.Clientset) { + clientset.PrependReactor("create", "selfsubjectreviews", + func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + obj := &authv1.SelfSubjectReview{ + Status: authv1.SelfSubjectReviewStatus{ + UserInfo: authv1.UserInfo{ + Username: "foo", + }, + }, + } + return true, obj, nil + }) + }, + expectedNodeName: "", + expectedError: true, + }, + { + name: "error creating SSR", + clientSetup: func(clientset *clientsetfake.Clientset) { + clientset.PrependReactor("create", "selfsubjectreviews", + func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.New("") + }) + }, + expectedNodeName: "", + expectedError: true, + }, + } + for _, rt := range tests { + t.Run(rt.name, func(t *testing.T) { + client := clientsetfake.NewSimpleClientset() + rt.clientSetup(client) + + nodeName, err := getNodeNameFromSSR(client) + + if (err != nil) != rt.expectedError { + t.Fatalf("expected error: %+v, got: %+v", rt.expectedError, err) + } + if rt.expectedNodeName != nodeName { + t.Fatalf("expected nodeName: %s, got: %s", rt.expectedNodeName, nodeName) + } + }) + } +}