Merge pull request #86216 from neolit123/1.18-fix-panic-get-node-name

kubeadm: add basic validation around kubelet.conf parsing
kubernetes · Dec 13, 2019 · e622579 · e622579
2 parents c34d140 + effe299
commit e622579
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 6 deletions.
diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go
@@ -147,7 +147,14 @@ func getNodeNameFromKubeletConfig(kubeconfigDir string) (string, error) {
 	}
 
 	// gets the info about the current user
-	authInfo := config.AuthInfos[config.Contexts[config.CurrentContext].AuthInfo]
+	currentContext, exists := config.Contexts[config.CurrentContext]
+	if !exists {
+		return "", errors.Errorf("invalid kubeconfig file %s: missing context %s", fileName, config.CurrentContext)
+	}
+	authInfo, exists := config.AuthInfos[currentContext.AuthInfo]
+	if !exists {
+		return "", errors.Errorf("invalid kubeconfig file %s: missing AuthInfo %s", fileName, currentContext.AuthInfo)
+	}
 
 	// gets the X509 certificate with current user credentials
 	var certs []*x509.Certificate
@@ -162,7 +169,7 @@ func getNodeNameFromKubeletConfig(kubeconfigDir string) (string, error) {
 			return "", err
 		}
 	} else {
-		return "", errors.New("invalid kubelet.conf. X509 certificate expected")
+		return "", errors.Errorf("invalid kubeconfig file %s. x509 certificate expected", fileName)
 	}
 
 	// We are only putting one certificate in the certificate pem file, so it's safe to just pick the first one

diff --git a/cmd/kubeadm/app/util/config/cluster_test.go b/cmd/kubeadm/app/util/config/cluster_test.go
@@ -50,8 +50,8 @@ kubernetesVersion: ` + k8sVersionString + `
 	"ClusterStatus_v1beta1": []byte(`
 apiVersion: kubeadm.k8s.io/v1beta1
 kind: ClusterStatus
-apiEndpoints: 
-  ` + nodeName + `: 
+apiEndpoints:
+  ` + nodeName + `:
     advertiseAddress: 1.2.3.4
     bindPort: 1234
 `),
@@ -71,8 +71,8 @@ kubernetesVersion: ` + k8sVersionString + `
 	"ClusterStatus_v1beta2": []byte(`
 apiVersion: kubeadm.k8s.io/v1beta2
 kind: ClusterStatus
-apiEndpoints: 
-  ` + nodeName + `: 
+apiEndpoints:
+  ` + nodeName + `:
     advertiseAddress: 1.2.3.4
     bindPort: 1234
 `),
@@ -143,6 +143,44 @@ current-context: system:node:mynode@kubernetes
 kind: Config
 preferences: {}
 users:
+- name: system:node:mynode
+  user:
+      client-certificate: kubelet.pem
+`),
+	"configWithInvalidContext": []byte(`
+apiVersion: v1
+clusters:
+- cluster:
+    server: https://10.0.2.15:6443
+  name: kubernetes
+contexts:
+- context:
+    cluster: kubernetes
+    user: system:node:mynode
+  name: system:node:mynode@kubernetes
+current-context: invalidContext
+kind: Config
+preferences: {}
+users:
+- name: system:node:mynode
+  user:
+      client-certificate: kubelet.pem
+`),
+	"configWithInvalidUser": []byte(`
+apiVersion: v1
+clusters:
+- cluster:
+    server: https://10.0.2.15:6443
+  name: kubernetes
+contexts:
+- context:
+    cluster: kubernetes
+    user: invalidUser
+  name: system:node:mynode@kubernetes
+current-context: system:node:mynode@kubernetes
+kind: Config
+preferences: {}
+users:
 - name: system:node:mynode
   user:
       client-certificate: kubelet.pem
@@ -204,6 +242,16 @@ func TestGetNodeNameFromKubeletConfig(t *testing.T) {
 			kubeconfigContent: kubeletConfFiles["withoutX509Cert"],
 			expectedError:     true,
 		},
+		{
+			name:              "invalid - the current context is invalid",
+			kubeconfigContent: kubeletConfFiles["configWithInvalidContext"],
+			expectedError:     true,
+		},
+		{
+			name:              "invalid - the user of the current context is invalid",
+			kubeconfigContent: kubeletConfFiles["configWithInvalidUser"],
+			expectedError:     true,
+		},
 	}
 
 	for _, rt := range tests {