Add unit test, address comments

cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go

@@ -265,25 +265,25 @@ func validateKubeConfig(outDir, filename string, config *clientcmdapi.Config) er
 	caExpected := bytes.TrimSpace(config.Clusters[expectedCluster].CertificateAuthorityData)
 
 	// Parse the current certificate authority data
-	currentCaCerts, err := certutil.ParseCertsPEM(caCurrent)
+	currentCACerts, err := certutil.ParseCertsPEM(caCurrent)
 	if err != nil {
-		return errors.Errorf("the kubeconfig file %q contains an invalid ca cert", kubeConfigFilePath)
+		return errors.Errorf("the kubeconfig file %q contains an invalid CA cert", kubeConfigFilePath)
 	}
-	// only fetch the first certificate in the cacert
-	currentCaCert := currentCaCerts[0]
+	// Only use the first certificate in the CA cert list
+	currentCACert := currentCACerts[0]
 
 	// Parse the expected certificate authority data
-	expectedCaCerts, err := certutil.ParseCertsPEM(caExpected)
+	expectedCACerts, err := certutil.ParseCertsPEM(caExpected)
 	if err != nil {
-		return errors.Errorf("the expected base64 encoded ca cert %q could not be parsed as a pem", caExpected)
+		return errors.Errorf("the expected base64 encoded CA cert %q could not be parsed as a PEM", caExpected)
 	}
 
-	// only fetch the first certificate in the cacert. When this is read from file, only the first entry is considered
-	expectedCaCert := expectedCaCerts[0]
+	// Only use the first certificate in the expected CA cert list
+	expectedCACert := expectedCACerts[0]
 
-	// Compare the current CA cert to the expected CA cert (which is only 1 entry).
+	// Compare the current CA cert to the expected CA cert.
 	// If the contents of this certificate do not match then the file is stale.
-	if !bytes.Equal(currentCaCert.Raw, expectedCaCert.Raw) {
+	if !currentCACert.Equal(expectedCACert) {
 		return errors.Errorf("a kubeconfig file %q exists already but has got the wrong CA cert", kubeConfigFilePath)
 	}
 	// If the current API Server location on disk doesn't match the expected API server, show a warning

cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go

@@ -603,12 +603,10 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) {
 
 	// creates CA, write to pkiDir and remove ca.key to get into external CA condition
 	caCert, caKey := certstestutil.SetupCertificateAuthority(t)
-	if err := pkiutil.WriteCertAndKey(pkiDir, kubeadmconstants.CACertAndKeyBaseName, caCert, caKey); err != nil {
+
+	if err := pkiutil.WriteCertBundle(pkiDir, kubeadmconstants.CACertAndKeyBaseName, []*x509.Certificate{caCert}); err != nil {
 		t.Fatalf("failure while saving CA certificate and key: %v", err)
 	}
-	if err := os.Remove(filepath.Join(pkiDir, kubeadmconstants.CAKeyName)); err != nil {
-		t.Fatalf("failure while deleting ca.key: %v", err)
-	}
 
 	// create a valid config
 	config := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1")
@@ -620,6 +618,18 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) {
 	// create a config with another server URL
 	configWithAnotherServerURL := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://4.3.2.1:4321", "test-cluster", "myOrg1")
 
+	// create a config with a ca cert containing multiple certificates
+	intermediateCaCert1, intermediateCaKey1 := certstestutil.SetupIntermediteCertificateAuthority(t, caCert, caKey)
+	intermediateCaCert2, intermediateCaKey2 := certstestutil.SetupIntermediteCertificateAuthority(t, intermediateCaCert1, intermediateCaKey1)
+
+	var caCertBundle []*x509.Certificate
+	caCertBundle = append(caCertBundle, caCert, intermediateCaCert1, intermediateCaCert2)
+
+	clusterName := "myOrg1"
+	multipleCaConfig := setupdKubeConfigWithClientAuth(t, intermediateCaCert2, intermediateCaKey2, "https://1.2.3.4:1234", "test-cluster", clusterName)
+	caCertBytes, _ := pkiutil.EncodeCertBundlePEM(caCertBundle)
+	multipleCaConfig.Clusters[clusterName].CertificateAuthorityData = caCertBytes
+
 	tests := map[string]struct {
 		filesToWrite  map[string]*clientcmdapi.Config
 		initConfig    *kubeadmapi.InitConfiguration
@@ -670,6 +680,17 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) {
 			initConfig:    initConfig,
 			expectedError: false,
 		},
+		"all files are valid for CA with multiple certificates": {
+			filesToWrite: map[string]*clientcmdapi.Config{
+				kubeadmconstants.AdminKubeConfigFileName:             multipleCaConfig,
+				kubeadmconstants.SuperAdminKubeConfigFileName:        multipleCaConfig,
+				kubeadmconstants.KubeletKubeConfigFileName:           multipleCaConfig,
+				kubeadmconstants.ControllerManagerKubeConfigFileName: multipleCaConfig,
+				kubeadmconstants.SchedulerKubeConfigFileName:         multipleCaConfig,
+			},
+			initConfig:    initConfig,
+			expectedError: false,
+		},
 	}
 
 	for name, test := range tests {

cmd/kubeadm/app/util/certs/util.go

@@ -43,6 +43,19 @@ func SetupCertificateAuthority(t *testing.T) (*x509.Certificate, crypto.Signer)
 	return caCert, caKey
 }
 
+// SetupIntermediteCertificateAuthority is a utility function for kubeadm testing that creates a
+// Intermediate CertificateAuthority cert/key pair
+func SetupIntermediteCertificateAuthority(t *testing.T, parentCert *x509.Certificate, parentKey crypto.Signer) (*x509.Certificate, crypto.Signer) {
+	caCert, caKey, err := pkiutil.NewIntermediateCertificateAuthority(parentCert, parentKey, &pkiutil.CertConfig{
+		Config: certutil.Config{CommonName: "kubernetes Intermediate CA"},
+	})
+	if err != nil {
+		t.Fatalf("failure while generating intermediate CA certificate and key: %v", err)
+	}
+
+	return caCert, caKey
+}
+
 // AssertCertificateIsSignedByCa is a utility function for kubeadm testing that asserts if a given certificate is signed
 // by the expected CA
 func AssertCertificateIsSignedByCa(t *testing.T, cert *x509.Certificate, signingCa *x509.Certificate) {