Merge pull request kubernetes#128487 from neolit123/1.32-fix-get-node-from-ssr-bug

kubeadm: ensure proper parsing of SSR username

Author: k8s-ci-robot

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")
 	}
 

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)

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)
+			}
+		})
+	}
+}