From e1320bba365a943fd2ca8125fbaf432759725136 Mon Sep 17 00:00:00 2001 From: Yago Nobre Date: Thu, 1 Nov 2018 10:24:10 -0300 Subject: [PATCH 1/2] Validate kubeconfig files in case of external CA mode --- cmd/kubeadm/app/cmd/init.go | 12 +- cmd/kubeadm/app/cmd/phases/kubeconfig.go | 3 +- .../app/phases/kubeconfig/kubeconfig.go | 75 +++++++-- .../app/phases/kubeconfig/kubeconfig_test.go | 149 +++++++++++++++++- 4 files changed, 220 insertions(+), 19 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index 4d82e9d3c96..fefb8d44f4e 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/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" @@ -316,6 +317,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, diff --git a/cmd/kubeadm/app/cmd/phases/kubeconfig.go b/cmd/kubeadm/app/cmd/phases/kubeconfig.go index a905d3967f8..cd0a9b6ecad 100644 --- a/cmd/kubeadm/app/cmd/phases/kubeconfig.go +++ b/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. @@ -116,7 +116,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 } diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go index 865d9d2d7a5..b18baf4ca90 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go +++ b/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. -// 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 { +// validateKubeConfig check if the kubeconfig file exist and has the expected CA and server URL +func validateKubeConfig(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); os.IsNotExist(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 + if _, err := os.Stat(kubeConfigFilePath); err != nil { + return err } // 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 +} diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go index 8ef9b2f7b88..9fdbc81ee56 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go +++ b/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{ From 52ef8ebd974c1f52f3c31ea2f6fc6a03a92d0a0c Mon Sep 17 00:00:00 2001 From: Yago Nobre Date: Sun, 4 Nov 2018 19:02:12 -0200 Subject: [PATCH 2/2] Update bazel --- cmd/kubeadm/app/phases/kubeconfig/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/kubeadm/app/phases/kubeconfig/BUILD b/cmd/kubeadm/app/phases/kubeconfig/BUILD index 59cb56519ec..74e70c2bab7 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/BUILD +++ b/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", ], )