Merge pull request #70537 from yagonobre/validate-kubeconfig

Validate kubeconfig files in case of external CA mode

cmd/kubeadm/app/cmd/init.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 The Kubernetes Authors.
+Copyright 2018 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -47,6 +47,7 @@ import (
 	clusterinfophase "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo"
 	nodebootstraptokenphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/node"
 	certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
+	kubeconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubeconfig"
 	kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet"
 	markmasterphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/markmaster"
 	patchnodephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/patchnode"
@@ -331,6 +332,15 @@ func newInitData(cmd *cobra.Command, options *initOptions, out io.Writer) (initD
 
 	// Checks if an external CA is provided by the user.
 	externalCA, _ := certsphase.UsingExternalCA(cfg)
+	if externalCA {
+		kubeconfigDir := kubeadmconstants.KubernetesDir
+		if options.dryRun {
+			kubeconfigDir = dryRunDir
+		}
+		if err := kubeconfigphase.ValidateKubeconfigsForExternalCA(kubeconfigDir, cfg); err != nil {
+			return initData{}, err
+		}
+	}
 
 	return initData{
 		cfg:                   cfg,

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

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 The Kubernetes Authors.
+Copyright 2018 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -135,7 +135,6 @@ func runKubeConfigFile(kubeConfigFileName string) func(workflow.RunData) error {
 
 		// if external CA mode, skip certificate authority generation
 		if data.ExternalCA() {
-			//TODO: implement validation of existing kubeconfig files
 			fmt.Printf("[kubeconfig] External CA mode: Using user provided %s\n", kubeConfigFileName)
 			return nil
 		}

cmd/kubeadm/app/phases/kubeconfig/BUILD

@@ -54,5 +54,6 @@ go_test(
         "//cmd/kubeadm/test/kubeconfig:go_default_library",
         "//staging/src/k8s.io/client-go/tools/clientcmd:go_default_library",
         "//staging/src/k8s.io/client-go/tools/clientcmd/api:go_default_library",
+        "//vendor/github.com/renstrom/dedent:go_default_library",
     ],
 )

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

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 The Kubernetes Authors.
+Copyright 2018 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -216,22 +216,12 @@ func buildKubeConfigFromSpec(spec *kubeConfigSpec, clustername string) (*clientc
 	), nil
 }
 
-// createKubeConfigFileIfNotExists saves the KubeConfig object into a file if there isn't any file at the given path.
+// validateKubeConfig check if the kubeconfig file exist and has the expected CA and server URL
-// If there already is a kubeconfig file at the given path; kubeadm tries to load it and check if the values in the
+func validateKubeConfig(outDir, filename string, config *clientcmdapi.Config) error {
-// existing and the expected config equals. If they do; kubeadm will just skip writing the file as it's up-to-date,
-// but if a file exists but has old content or isn't a kubeconfig file, this function returns an error.
-func createKubeConfigFileIfNotExists(outDir, filename string, config *clientcmdapi.Config) error {
 	kubeConfigFilePath := filepath.Join(outDir, filename)
 
-	// Check if the file exist, and if it doesn't, just write it to disk
+	if _, err := os.Stat(kubeConfigFilePath); err != nil {
-	if _, err := os.Stat(kubeConfigFilePath); os.IsNotExist(err) {
+		return err
-		fmt.Printf("[kubeconfig] Writing %q kubeconfig file\n", filename)
-
-		err = kubeconfigutil.WriteToDisk(kubeConfigFilePath, config)
-		if err != nil {
-			return errors.Wrapf(err, "failed to save kubeconfig file %s on disk", kubeConfigFilePath)
-		}
-		return nil
 	}
 
 	// The kubeconfig already exists, let's check if it has got the same CA and server URL
@@ -254,6 +244,29 @@ func createKubeConfigFileIfNotExists(outDir, filename string, config *clientcmda
 		return errors.Errorf("a kubeconfig file %q exists already but has got the wrong API Server URL", kubeConfigFilePath)
 	}
 
+	return nil
+}
+
+// createKubeConfigFileIfNotExists saves the KubeConfig object into a file if there isn't any file at the given path.
+// If there already is a kubeconfig file at the given path; kubeadm tries to load it and check if the values in the
+// existing and the expected config equals. If they do; kubeadm will just skip writing the file as it's up-to-date,
+// but if a file exists but has old content or isn't a kubeconfig file, this function returns an error.
+func createKubeConfigFileIfNotExists(outDir, filename string, config *clientcmdapi.Config) error {
+	kubeConfigFilePath := filepath.Join(outDir, filename)
+
+	err := validateKubeConfig(outDir, filename, config)
+	if err != nil {
+		// Check if the file exist, and if it doesn't, just write it to disk
+		if !os.IsNotExist(err) {
+			return err
+		}
+		fmt.Printf("[kubeconfig] Writing %q kubeconfig file\n", filename)
+		err = kubeconfigutil.WriteToDisk(kubeConfigFilePath, config)
+		if err != nil {
+			return errors.Wrapf(err, "failed to save kubeconfig file %q on disk", kubeConfigFilePath)
+		}
+		return nil
+	}
 	// kubeadm doesn't validate the existing kubeconfig file more than this (kubeadm trusts the client certs to be valid)
 	// Basically, if we find a kubeconfig file with the same path; the same CA cert and the same server URL;
 	// kubeadm thinks those files are equal and doesn't bother writing a new file
@@ -333,3 +346,35 @@ func writeKubeConfigFromSpec(out io.Writer, spec *kubeConfigSpec, clustername st
 	fmt.Fprintln(out, string(configBytes))
 	return nil
 }
+
+// ValidateKubeconfigsForExternalCA check if the kubeconfig file exist and has the expected CA and server URL using kubeadmapi.InitConfiguration.
+func ValidateKubeconfigsForExternalCA(outDir string, cfg *kubeadmapi.InitConfiguration) error {
+	kubeConfigFileNames := []string{
+		kubeadmconstants.AdminKubeConfigFileName,
+		kubeadmconstants.KubeletKubeConfigFileName,
+		kubeadmconstants.ControllerManagerKubeConfigFileName,
+		kubeadmconstants.SchedulerKubeConfigFileName,
+	}
+
+	specs, err := getKubeConfigSpecs(cfg)
+	if err != nil {
+		return err
+	}
+
+	for _, kubeConfigFileName := range kubeConfigFileNames {
+		spec, exists := specs[kubeConfigFileName]
+		if !exists {
+			return errors.Errorf("couldn't retrive KubeConfigSpec for %s", kubeConfigFileName)
+		}
+
+		kubeconfig, err := buildKubeConfigFromSpec(spec, cfg.ClusterName)
+		if err != nil {
+			return err
+		}
+
+		if err = validateKubeConfig(outDir, kubeConfigFileName, kubeconfig); err != nil {
+			return err
+		}
+	}
+	return nil
+}

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

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 The Kubernetes Authors.
+Copyright 2018 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -23,9 +23,12 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
+	"github.com/renstrom/dedent"
+
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -437,6 +440,150 @@ func TestWriteKubeConfig(t *testing.T) {
 	}
 }
 
+func TestValidateKubeConfig(t *testing.T) {
+	caCert, caKey := certstestutil.SetupCertificateAuthorithy(t)
+	anotherCaCert, anotherCaKey := certstestutil.SetupCertificateAuthorithy(t)
+
+	config := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1")
+	configWithAnotherClusterCa := setupdKubeConfigWithClientAuth(t, anotherCaCert, anotherCaKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1")
+	configWithAnotherServerURL := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://4.3.2.1:4321", "test-cluster", "myOrg1")
+
+	tests := map[string]struct {
+		existingKubeConfig *clientcmdapi.Config
+		kubeConfig         *clientcmdapi.Config
+		expectedError      bool
+	}{
+		"kubeconfig don't exist": {
+			kubeConfig:    config,
+			expectedError: true,
+		},
+		"kubeconfig exist and has invalid ca": {
+			existingKubeConfig: configWithAnotherClusterCa,
+			kubeConfig:         config,
+			expectedError:      true,
+		},
+		"kubeconfig exist and has invalid server url": {
+			existingKubeConfig: configWithAnotherServerURL,
+			kubeConfig:         config,
+			expectedError:      true,
+		},
+		"kubeconfig exist and is valid": {
+			existingKubeConfig: config,
+			kubeConfig:         config,
+			expectedError:      false,
+		},
+	}
+
+	for name, test := range tests {
+		tmpdir := testutil.SetupTempDir(t)
+		defer os.RemoveAll(tmpdir)
+
+		if test.existingKubeConfig != nil {
+			if err := createKubeConfigFileIfNotExists(tmpdir, "test.conf", test.existingKubeConfig); err != nil {
+				t.Errorf("createKubeConfigFileIfNotExists failed")
+			}
+		}
+
+		err := validateKubeConfig(tmpdir, "test.conf", test.kubeConfig)
+		if (err != nil) != test.expectedError {
+			t.Fatalf(dedent.Dedent(
+				"validateKubeConfig failed\n%s\nexpected error: %t\n\tgot: %t\nerror: %v"),
+				name,
+				test.expectedError,
+				(err != nil),
+				err,
+			)
+		}
+	}
+}
+
+func TestValidateKubeconfigsForExternalCA(t *testing.T) {
+	tmpDir := testutil.SetupTempDir(t)
+	defer os.RemoveAll(tmpDir)
+	pkiDir := filepath.Join(tmpDir, "pki")
+
+	initConfig := &kubeadmapi.InitConfiguration{
+		ClusterConfiguration: kubeadmapi.ClusterConfiguration{
+			CertificatesDir: pkiDir,
+		},
+		APIEndpoint: kubeadmapi.APIEndpoint{
+			BindPort:         1234,
+			AdvertiseAddress: "1.2.3.4",
+		},
+	}
+
+	caCert, caKey := certstestutil.SetupCertificateAuthorithy(t)
+	anotherCaCert, anotherCaKey := certstestutil.SetupCertificateAuthorithy(t)
+	if err := pkiutil.WriteCertAndKey(pkiDir, kubeadmconstants.CACertAndKeyBaseName, caCert, caKey); err != nil {
+		t.Fatalf("failure while saving CA certificate and key: %v", err)
+	}
+
+	config := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1")
+	configWithAnotherClusterCa := setupdKubeConfigWithClientAuth(t, anotherCaCert, anotherCaKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1")
+	configWithAnotherServerURL := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://4.3.2.1:4321", "test-cluster", "myOrg1")
+
+	tests := map[string]struct {
+		filesToWrite  map[string]*clientcmdapi.Config
+		initConfig    *kubeadmapi.InitConfiguration
+		expectedError bool
+	}{
+		"files don't exist": {
+			initConfig:    initConfig,
+			expectedError: true,
+		},
+		"some files don't exist": {
+			filesToWrite: map[string]*clientcmdapi.Config{
+				kubeadmconstants.AdminKubeConfigFileName:   config,
+				kubeadmconstants.KubeletKubeConfigFileName: config,
+			},
+			initConfig:    initConfig,
+			expectedError: true,
+		},
+		"some files are invalid": {
+			filesToWrite: map[string]*clientcmdapi.Config{
+				kubeadmconstants.AdminKubeConfigFileName:             config,
+				kubeadmconstants.KubeletKubeConfigFileName:           config,
+				kubeadmconstants.ControllerManagerKubeConfigFileName: configWithAnotherClusterCa,
+				kubeadmconstants.SchedulerKubeConfigFileName:         configWithAnotherServerURL,
+			},
+			initConfig:    initConfig,
+			expectedError: true,
+		},
+		"all files are valid": {
+			filesToWrite: map[string]*clientcmdapi.Config{
+				kubeadmconstants.AdminKubeConfigFileName:             config,
+				kubeadmconstants.KubeletKubeConfigFileName:           config,
+				kubeadmconstants.ControllerManagerKubeConfigFileName: config,
+				kubeadmconstants.SchedulerKubeConfigFileName:         config,
+			},
+			initConfig:    initConfig,
+			expectedError: false,
+		},
+	}
+
+	for name, test := range tests {
+		tmpdir := testutil.SetupTempDir(t)
+		defer os.RemoveAll(tmpdir)
+
+		for name, config := range test.filesToWrite {
+			if err := createKubeConfigFileIfNotExists(tmpdir, name, config); err != nil {
+				t.Errorf("createKubeConfigFileIfNotExists failed")
+			}
+		}
+
+		err := ValidateKubeconfigsForExternalCA(tmpdir, test.initConfig)
+		if (err != nil) != test.expectedError {
+			t.Fatalf(dedent.Dedent(
+				"ValidateKubeconfigsForExternalCA failed\n%s\nexpected error: %t\n\tgot: %t\nerror: %v"),
+				name,
+				test.expectedError,
+				(err != nil),
+				err,
+			)
+		}
+	}
+}
+
 // setupdKubeConfigWithClientAuth is a test utility function that wraps buildKubeConfigFromSpec for building a KubeConfig object With ClientAuth
 func setupdKubeConfigWithClientAuth(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey, APIServer, clientName, clustername string, organizations ...string) *clientcmdapi.Config {
 	spec := &kubeConfigSpec{