From effe29908244e85d442de1b95345d00624ce428f Mon Sep 17 00:00:00 2001
From: "Lubomir I. Ivanov" <lubomirivanov@vmware.com>
Date: Thu, 12 Dec 2019 17:30:53 +0200
Subject: [PATCH] kubeadm: add basic validation around kubelet.conf parsing

If the user has modified the kubelet.conf post TLS bootstrap
to become invalid, the function getNodeNameFromKubeletConfig() can
panic. This was observed to trigger in "kubeadm reset" use cases.

Add basic validation and unit tests around parsing the kubelet.conf
with the aforementioned function.
---
 cmd/kubeadm/app/util/config/cluster.go      | 11 +++-
 cmd/kubeadm/app/util/config/cluster_test.go | 56 +++++++++++++++++++--
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go
index 48bf7ce1b486..3d6533c6712e 100644
--- 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
index eaf972e64742..6ac57b6b3715 100644
--- 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 {