Merge pull request kubernetes#81056 from neolit123/1.16-kubeadm-node-names

kubeadm: prevent bootstrap of nodes with known names

Author: k8s-ci-robot

cmd/kubeadm/app/cmd/phases/init/bootstraptoken.go

@@ -86,6 +86,10 @@ func runBootstrapToken(c workflow.RunData) error {
 	if err := nodebootstraptokenphase.UpdateOrCreateTokens(client, false, data.Cfg().BootstrapTokens); err != nil {
 		return errors.Wrap(err, "error updating or creating token")
 	}
+	// Create RBAC rules that makes the bootstrap tokens able to get nodes
+	if err := nodebootstraptokenphase.AllowBoostrapTokensToGetNodes(client); err != nil {
+		return errors.Wrap(err, "error allowing bootstrap tokens to get Nodes")
+	}
 	// Create RBAC rules that makes the bootstrap tokens able to post CSRs
 	if err := nodebootstraptokenphase.AllowBootstrapTokensToPostCSRs(client); err != nil {
 		return errors.Wrap(err, "error allowing bootstrap tokens to post CSRs")

cmd/kubeadm/app/cmd/phases/join/BUILD

@@ -30,6 +30,9 @@ go_library(
         "//cmd/kubeadm/app/preflight:go_default_library",
         "//cmd/kubeadm/app/util/apiclient:go_default_library",
         "//cmd/kubeadm/app/util/kubeconfig:go_default_library",
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",

cmd/kubeadm/app/cmd/phases/join/kubelet.go

@@ -22,6 +22,9 @@ import (
 
 	"github.com/lithammer/dedent"
 	"github.com/pkg/errors"
+	v1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/version"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
@@ -128,6 +131,28 @@ func runKubeletStartJoinPhase(c workflow.RunData) (returnErr error) {
 		return errors.Errorf("couldn't create client from kubeconfig file %q", bootstrapKubeConfigFile)
 	}
 
+	// Obtain the name of this Node.
+	nodeName, _, err := kubeletphase.GetNodeNameAndHostname(&cfg.NodeRegistration)
+	if err != nil {
+		klog.Warning(err)
+	}
+
+	// Make sure to exit before TLS bootstrap if a Node with the same name exist in the cluster
+	// and it has the "Ready" status.
+	// A new Node with the same name as an existing control-plane Node can cause undefined
+	// behavior and ultimately control-plane failure.
+	klog.V(1).Infof("[kubelet-start] Checking for an existing Node in the cluster with name %q and status %q", nodeName, v1.NodeReady)
+	node, err := bootstrapClient.CoreV1().Nodes().Get(nodeName, metav1.GetOptions{})
+	if err != nil && !apierrors.IsNotFound(err) {
+		return errors.Wrapf(err, "cannot get Node %q", nodeName)
+	}
+	for _, cond := range node.Status.Conditions {
+		if cond.Type == v1.NodeReady {
+			return errors.Errorf("a Node with name %q and status %q already exists in the cluster. "+
+				"You must delete the existing Node or change the name of this new joining Node", nodeName, v1.NodeReady)
+		}
+	}
+
 	// Configure the kubelet. In this short timeframe, kubeadm is trying to stop/restart the kubelet
 	// Try to stop the kubelet service so no race conditions occur when configuring it
 	klog.V(1).Infoln("[kubelet-start] Stopping the kubelet")

cmd/kubeadm/app/phases/bootstraptoken/node/tlsbootstrap.go

@@ -32,6 +32,8 @@ const (
 	NodeBootstrapperClusterRoleName = "system:node-bootstrapper"
 	// NodeKubeletBootstrap defines the name of the ClusterRoleBinding that lets kubelets post CSRs
 	NodeKubeletBootstrap = "kubeadm:kubelet-bootstrap"
+	// GetNodesClusterRoleName defines the name of the ClusterRole and ClusterRoleBinding to get nodes
+	GetNodesClusterRoleName = "kubeadm:get-nodes"
 
 	// CSRAutoApprovalClusterRoleName defines the name of the auto-bootstrapped ClusterRole for making the csrapprover controller auto-approve the CSR
 	// TODO: This value should be defined in an other, generic authz package instead of here
@@ -67,6 +69,45 @@ func AllowBootstrapTokensToPostCSRs(client clientset.Interface) error {
 	})
 }
 
+// AllowBoostrapTokensToGetNodes creates RBAC rules to allow Node Bootstrap Tokens to list nodes
+func AllowBoostrapTokensToGetNodes(client clientset.Interface) error {
+	fmt.Println("[bootstrap-token] configured RBAC rules to allow Node Bootstrap tokens to get nodes")
+
+	if err := apiclient.CreateOrUpdateClusterRole(client, &rbac.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      GetNodesClusterRoleName,
+			Namespace: metav1.NamespaceSystem,
+		},
+		Rules: []rbac.PolicyRule{
+			{
+				Verbs:     []string{"get"},
+				APIGroups: []string{""},
+				Resources: []string{"nodes"},
+			},
+		},
+	}); err != nil {
+		return err
+	}
+
+	return apiclient.CreateOrUpdateClusterRoleBinding(client, &rbac.ClusterRoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      GetNodesClusterRoleName,
+			Namespace: metav1.NamespaceSystem,
+		},
+		RoleRef: rbac.RoleRef{
+			APIGroup: rbac.GroupName,
+			Kind:     "ClusterRole",
+			Name:     GetNodesClusterRoleName,
+		},
+		Subjects: []rbac.Subject{
+			{
+				Kind: rbac.GroupKind,
+				Name: constants.NodeBootstrapTokenAuthGroup,
+			},
+		},
+	})
+}
+
 // AutoApproveNodeBootstrapTokens creates RBAC rules in a way that makes Node Bootstrap Tokens' CSR auto-approved by the csrapprover controller
 func AutoApproveNodeBootstrapTokens(client clientset.Interface) error {
 	fmt.Println("[bootstrap-token] configured RBAC rules to allow the csrapprover controller automatically approve CSRs from a Node Bootstrap Token")

cmd/kubeadm/app/phases/kubelet/flags.go

@@ -41,17 +41,29 @@ type kubeletFlagsOpts struct {
 	registerTaintsUsingFlags bool
 	execer                   utilsexec.Interface
 	isServiceActiveFunc      func(string) (bool, error)
-	defaultHostname          string
+}
+
+// GetNodeNameAndHostname obtains the name for this Node using the following precedence
+// (from lower to higher):
+// - actual hostname
+// - NodeRegistrationOptions.Name (same as "--node-name" passed to "kubeadm init/join")
+// - "hostname-overide" flag in NodeRegistrationOptions.KubeletExtraArgs
+// It also returns the hostname or an error if getting the hostname failed.
+func GetNodeNameAndHostname(cfg *kubeadmapi.NodeRegistrationOptions) (string, string, error) {
+	hostname, err := kubeadmutil.GetHostname("")
+	nodeName := hostname
+	if cfg.Name != "" {
+		nodeName = cfg.Name
+	}
+	if name, ok := cfg.KubeletExtraArgs["hostname-override"]; ok {
+		nodeName = name
+	}
+	return nodeName, hostname, err
 }
 
 // WriteKubeletDynamicEnvFile writes an environment file with dynamic flags to the kubelet.
 // Used at "kubeadm init" and "kubeadm join" time.
 func WriteKubeletDynamicEnvFile(cfg *kubeadmapi.ClusterConfiguration, nodeReg *kubeadmapi.NodeRegistrationOptions, registerTaintsUsingFlags bool, kubeletDir string) error {
-	hostName, err := kubeadmutil.GetHostname("")
-	if err != nil {
-		return err
-	}
-
 	flagOpts := kubeletFlagsOpts{
 		nodeRegOpts:              nodeReg,
 		featureGates:             cfg.FeatureGates,
@@ -65,7 +77,6 @@ func WriteKubeletDynamicEnvFile(cfg *kubeadmapi.ClusterConfiguration, nodeReg *k
 			}
 			return initSystem.ServiceIsActive(name), nil
 		},
-		defaultHostname: hostName,
 	}
 	stringMap := buildKubeletArgMap(flagOpts)
 	argList := kubeadmutil.BuildArgumentListFromMap(stringMap, nodeReg.KubeletExtraArgs)
@@ -113,15 +124,19 @@ func buildKubeletArgMap(opts kubeletFlagsOpts) map[string]string {
 		kubeletFlags["resolv-conf"] = "/run/systemd/resolve/resolv.conf"
 	}
 
-	// Make sure the node name we're passed will work with Kubelet
-	if opts.nodeRegOpts.Name != "" && opts.nodeRegOpts.Name != opts.defaultHostname {
-		klog.V(1).Infof("setting kubelet hostname-override to %q", opts.nodeRegOpts.Name)
-		kubeletFlags["hostname-override"] = opts.nodeRegOpts.Name
+	// Pass the "--hostname-override" flag to the kubelet only if it's different from the hostname
+	nodeName, hostname, err := GetNodeNameAndHostname(opts.nodeRegOpts)
+	if err != nil {
+		klog.Warning(err)
+	}
+	if nodeName != hostname {
+		klog.V(1).Infof("setting kubelet hostname-override to %q", nodeName)
+		kubeletFlags["hostname-override"] = nodeName
 	}
 
 	// TODO: Conditionally set `--cgroup-driver` to either `systemd` or `cgroupfs` for CRI other than Docker
 
-	// TODO: The following code should be remvoved after dual-stack is GA.
+	// TODO: The following code should be removed after dual-stack is GA.
 	// Note: The user still retains the ability to explicitly set feature-gates and that value will overwrite this base value.
 	if enabled, present := opts.featureGates[features.IPv6DualStack]; present {
 		kubeletFlags["feature-gates"] = fmt.Sprintf("%s=%t", features.IPv6DualStack, enabled)

cmd/kubeadm/app/phases/kubelet/flags_test.go

@@ -109,7 +109,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
-					Name:      "foo",
 					Taints: []v1.Taint{ // This should be ignored as registerTaintsUsingFlags is false
 						{
 							Key:    "foo",
@@ -120,22 +119,35 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				},
 				execer:              errCgroupExecer,
 				isServiceActiveFunc: serviceIsNotActiveFunc,
-				defaultHostname:     "foo",
 			},
 			expected: map[string]string{
 				"network-plugin": "cni",
 			},
 		},
 		{
-			name: "nodeRegOpts.Name != default hostname",
+			name: "hostname override from NodeRegistrationOptions.Name",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
 					Name:      "override-name",
 				},
 				execer:              errCgroupExecer,
 				isServiceActiveFunc: serviceIsNotActiveFunc,
-				defaultHostname:     "default",
+			},
+			expected: map[string]string{
+				"network-plugin":    "cni",
+				"hostname-override": "override-name",
+			},
+		},
+		{
+			name: "hostname override from NodeRegistrationOptions.KubeletExtraArgs",
+			opts: kubeletFlagsOpts{
+				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
+					CRISocket:        "/var/run/dockershim.sock",
+					KubeletExtraArgs: map[string]string{"hostname-override": "override-name"},
+				},
+				execer:              errCgroupExecer,
+				isServiceActiveFunc: serviceIsNotActiveFunc,
 			},
 			expected: map[string]string{
 				"network-plugin":    "cni",
@@ -147,11 +159,9 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
-					Name:      "foo",
 				},
 				execer:              systemdCgroupExecer,
 				isServiceActiveFunc: serviceIsNotActiveFunc,
-				defaultHostname:     "foo",
 			},
 			expected: map[string]string{
 				"network-plugin": "cni",
@@ -163,11 +173,9 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
-					Name:      "foo",
 				},
 				execer:              cgroupfsCgroupExecer,
 				isServiceActiveFunc: serviceIsNotActiveFunc,
-				defaultHostname:     "foo",
 			},
 			expected: map[string]string{
 				"network-plugin": "cni",
@@ -179,11 +187,9 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/containerd.sock",
-					Name:      "foo",
 				},
 				execer:              cgroupfsCgroupExecer,
 				isServiceActiveFunc: serviceIsNotActiveFunc,
-				defaultHostname:     "foo",
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
@@ -195,7 +201,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/containerd.sock",
-					Name:      "foo",
 					Taints: []v1.Taint{
 						{
 							Key:    "foo",
@@ -212,7 +217,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				registerTaintsUsingFlags: true,
 				execer:                   cgroupfsCgroupExecer,
 				isServiceActiveFunc:      serviceIsNotActiveFunc,
-				defaultHostname:          "foo",
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
@@ -225,11 +229,9 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/containerd.sock",
-					Name:      "foo",
 				},
 				execer:              cgroupfsCgroupExecer,
 				isServiceActiveFunc: serviceIsActiveFunc,
-				defaultHostname:     "foo",
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
@@ -242,12 +244,10 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
-					Name:      "foo",
 				},
 				pauseImage:          "gcr.io/pause:3.1",
 				execer:              cgroupfsCgroupExecer,
 				isServiceActiveFunc: serviceIsNotActiveFunc,
-				defaultHostname:     "foo",
 			},
 			expected: map[string]string{
 				"network-plugin":            "cni",