From ff072aa639614551a23e9d843dab0a7243b3d919 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Sun, 17 Mar 2019 13:03:40 +0100 Subject: [PATCH] fix-external-ca --- cmd/kubeadm/app/cmd/init.go | 27 +++- cmd/kubeadm/app/cmd/phases/init/certs.go | 4 +- cmd/kubeadm/app/phases/certs/certs.go | 23 ++-- cmd/kubeadm/app/phases/certs/certs_test.go | 126 +++++++++++++----- .../app/phases/kubeconfig/kubeconfig.go | 26 ++-- .../app/phases/kubeconfig/kubeconfig_test.go | 25 +++- 6 files changed, 165 insertions(+), 66 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index b55aa9e87c..fbb5fa6613 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -332,9 +332,17 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io } } - // Checks if an external CA is provided by the user. - externalCA, _ := certsphase.UsingExternalCA(&cfg.ClusterConfiguration) + // Checks if an external CA is provided by the user (when the CA Cert is present but the CA Key is not) + externalCA, err := certsphase.UsingExternalCA(&cfg.ClusterConfiguration) if externalCA { + // In case the certificates signed by CA (that should be provided by the user) are missing or invalid, + // returns, because kubeadm can't regenerate them without the CA Key + if err != nil { + return nil, errors.Wrapf(err, "invalid or incomplete external CA") + } + + // Validate that also the required kubeconfig files exists and are invalid, because + // kubeadm can't regenerate them without the CA Key kubeconfigDir := options.kubeconfigDir if options.dryRun { kubeconfigDir = dryRunDir @@ -342,11 +350,22 @@ func newInitData(cmd *cobra.Command, args []string, options *initOptions, out io if err := kubeconfigphase.ValidateKubeconfigsForExternalCA(kubeconfigDir, cfg); err != nil { return nil, err } - if options.uploadCerts { - return nil, errors.New("can't use externalCA mode and upload-certs") + } + + // Checks if an external Front-Proxy CA is provided by the user (when the Front-Proxy CA Cert is present but the Front-Proxy CA Key is not) + externalFrontProxyCA, err := certsphase.UsingExternalFrontProxyCA(&cfg.ClusterConfiguration) + if externalFrontProxyCA { + // In case the certificates signed by Front-Proxy CA (that should be provided by the user) are missing or invalid, + // returns, because kubeadm can't regenerate them without the Front-Proxy CA Key + if err != nil { + return nil, errors.Wrapf(err, "invalid or incomplete external front-proxy CA") } } + if options.uploadCerts && (externalCA || externalFrontProxyCA) { + return nil, errors.New("can't use upload-certs with an external CA or an external front-proxy CA") + } + return &initData{ cfg: cfg, certificatesDir: cfg.CertificatesDir, diff --git a/cmd/kubeadm/app/cmd/phases/init/certs.go b/cmd/kubeadm/app/cmd/phases/init/certs.go index cfdf7bacbb..8d2fe25a4e 100644 --- a/cmd/kubeadm/app/cmd/phases/init/certs.go +++ b/cmd/kubeadm/app/cmd/phases/init/certs.go @@ -190,7 +190,7 @@ func runCertsSa(c workflow.RunData) error { // if external CA mode, skip service account key generation if data.ExternalCA() { - fmt.Printf("[certs] External CA mode: Using existing sa keys\n") + fmt.Printf("[certs] Using existing sa keys\n") return nil } @@ -220,7 +220,7 @@ func runCAPhase(ca *certsphase.KubeadmCert) func(c workflow.RunData) error { fmt.Printf("[certs] Using existing %s certificate authority\n", ca.BaseName) return nil } - fmt.Printf("[certs] Using existing %s keyless certificate authority", ca.BaseName) + fmt.Printf("[certs] Using existing %s keyless certificate authority\n", ca.BaseName) return nil } diff --git a/cmd/kubeadm/app/phases/certs/certs.go b/cmd/kubeadm/app/phases/certs/certs.go index be9fd770e8..a79fb9cf2d 100644 --- a/cmd/kubeadm/app/phases/certs/certs.go +++ b/cmd/kubeadm/app/phases/certs/certs.go @@ -354,8 +354,9 @@ func SharedCertificateExists(cfg *kubeadmapi.ClusterConfiguration) (bool, error) } // UsingExternalCA determines whether the user is relying on an external CA. We currently implicitly determine this is the case -// when both the CA Cert and the front proxy CA Cert are present but the CA Key and front proxy CA Key are not. +// when the CA Cert is present but the CA Key is not. // This allows us to, e.g., skip generating certs or not start the csr signing controller. +// In case we are using an external front-proxy CA, the function validates the certificates signed by front-proxy CA that should be provided by the user. func UsingExternalCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) { if err := validateCACert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName, "", "CA"}); err != nil { @@ -364,20 +365,24 @@ func UsingExternalCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) { caKeyPath := filepath.Join(cfg.CertificatesDir, kubeadmconstants.CAKeyName) if _, err := os.Stat(caKeyPath); !os.IsNotExist(err) { - return false, errors.Errorf("%s exists", kubeadmconstants.CAKeyName) + return false, nil } if err := validateSignedCert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName, kubeadmconstants.APIServerCertAndKeyBaseName, "API server"}); err != nil { - return false, err + return true, err } if err := validateSignedCert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName, kubeadmconstants.APIServerKubeletClientCertAndKeyBaseName, "API server kubelet client"}); err != nil { - return false, err + return true, err } - if err := validatePrivatePublicKey(certKeyLocation{cfg.CertificatesDir, "", kubeadmconstants.ServiceAccountKeyBaseName, "service account"}); err != nil { - return false, err - } + return true, nil +} + +// UsingExternalFrontProxyCA determines whether the user is relying on an external front-proxy CA. We currently implicitly determine this is the case +// when the front proxy CA Cert is present but the front proxy CA Key is not. +// In case we are using an external front-proxy CA, the function validates the certificates signed by front-proxy CA that should be provided by the user. +func UsingExternalFrontProxyCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) { if err := validateCACert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.FrontProxyCACertAndKeyBaseName, "", "front-proxy CA"}); err != nil { return false, err @@ -385,11 +390,11 @@ func UsingExternalCA(cfg *kubeadmapi.ClusterConfiguration) (bool, error) { frontProxyCAKeyPath := filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyCAKeyName) if _, err := os.Stat(frontProxyCAKeyPath); !os.IsNotExist(err) { - return false, errors.Errorf("%s exists", kubeadmconstants.FrontProxyCAKeyName) + return false, nil } if err := validateSignedCert(certKeyLocation{cfg.CertificatesDir, kubeadmconstants.FrontProxyCACertAndKeyBaseName, kubeadmconstants.FrontProxyClientCertAndKeyBaseName, "front-proxy client"}); err != nil { - return false, err + return true, err } return true, nil diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index 874195fade..20bf5d4a74 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/cmd/kubeadm/app/phases/certs/certs_test.go @@ -514,47 +514,108 @@ func TestCreatePKIAssetsWithSparseCerts(t *testing.T) { func TestUsingExternalCA(t *testing.T) { tests := []struct { - setupFuncs []func(cfg *kubeadmapi.InitConfiguration) error - expected bool + name string + setupFuncs []func(cfg *kubeadmapi.InitConfiguration) error + externalCAFunc func(*kubeadmapi.ClusterConfiguration) (bool, error) + expected bool + expectedErr bool }{ { + name: "Test External CA, when complete PKI exists", setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{ CreatePKIAssets, }, - expected: false, + externalCAFunc: UsingExternalCA, + expected: false, }, { + name: "Test External CA, when ca.key missing", setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{ CreatePKIAssets, - deleteCAKey, - deleteFrontProxyCAKey, + deleteCertOrKey(kubeadmconstants.CAKeyName), }, - expected: true, + externalCAFunc: UsingExternalCA, + expected: true, + }, + { + name: "Test External CA, when ca.key missing and signed certs are missing", + setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{ + CreatePKIAssets, + deleteCertOrKey(kubeadmconstants.CAKeyName), + deleteCertOrKey(kubeadmconstants.APIServerCertName), + }, + externalCAFunc: UsingExternalCA, + expected: true, + expectedErr: true, + }, + { + name: "Test External CA, when ca.key missing", + setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{ + CreatePKIAssets, + deleteCertOrKey(kubeadmconstants.CAKeyName), + }, + externalCAFunc: UsingExternalCA, + expected: true, + }, + { + name: "Test External Front Proxy CA, when complete PKI exists", + setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{ + CreatePKIAssets, + }, + externalCAFunc: UsingExternalFrontProxyCA, + expected: false, + }, + { + name: "Test External Front Proxy CA, when front-proxy-ca.key missing", + setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{ + CreatePKIAssets, + deleteCertOrKey(kubeadmconstants.FrontProxyCAKeyName), + }, + externalCAFunc: UsingExternalFrontProxyCA, + expected: true, + }, + { + name: "Test External Front Proxy CA, when front-proxy-.key missing and signed certs are missing", + setupFuncs: []func(cfg *kubeadmapi.InitConfiguration) error{ + CreatePKIAssets, + deleteCertOrKey(kubeadmconstants.FrontProxyCAKeyName), + deleteCertOrKey(kubeadmconstants.FrontProxyClientCertName), + }, + externalCAFunc: UsingExternalFrontProxyCA, + expected: true, + expectedErr: true, }, } for _, test := range tests { - dir := testutil.SetupTempDir(t) - defer os.RemoveAll(dir) + t.Run(test.name, func(t *testing.T) { + dir := testutil.SetupTempDir(t) + defer os.RemoveAll(dir) - cfg := &kubeadmapi.InitConfiguration{ - LocalAPIEndpoint: kubeadmapi.APIEndpoint{AdvertiseAddress: "1.2.3.4"}, - ClusterConfiguration: kubeadmapi.ClusterConfiguration{ - Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"}, - CertificatesDir: dir, - }, - NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: "valid-hostname"}, - } - - for _, f := range test.setupFuncs { - if err := f(cfg); err != nil { - t.Errorf("error executing setup function: %v", err) + cfg := &kubeadmapi.InitConfiguration{ + LocalAPIEndpoint: kubeadmapi.APIEndpoint{AdvertiseAddress: "1.2.3.4"}, + ClusterConfiguration: kubeadmapi.ClusterConfiguration{ + Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"}, + CertificatesDir: dir, + }, + NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: "valid-hostname"}, } - } - if val, _ := UsingExternalCA(&cfg.ClusterConfiguration); val != test.expected { - t.Errorf("UsingExternalCA did not match expected: %v", test.expected) - } + for _, f := range test.setupFuncs { + if err := f(cfg); err != nil { + t.Errorf("error executing setup function: %v", err) + } + } + + val, err := test.externalCAFunc(&cfg.ClusterConfiguration) + if val != test.expected { + t.Errorf("UsingExternalCA did not match expected: %v", test.expected) + } + + if (err != nil) != test.expectedErr { + t.Errorf("UsingExternalCA returned un expected err: %v", err) + } + }) } } @@ -742,18 +803,13 @@ func TestCreateCertificateFilesMethods(t *testing.T) { } } -func deleteCAKey(cfg *kubeadmapi.InitConfiguration) error { - if err := os.Remove(filepath.Join(cfg.CertificatesDir, kubeadmconstants.CAKeyName)); err != nil { - return errors.Wrapf(err, "failed removing %s", kubeadmconstants.CAKeyName) +func deleteCertOrKey(name string) func(*kubeadmapi.InitConfiguration) error { + return func(cfg *kubeadmapi.InitConfiguration) error { + if err := os.Remove(filepath.Join(cfg.CertificatesDir, name)); err != nil { + return errors.Wrapf(err, "failed removing %s", name) + } + return nil } - return nil -} - -func deleteFrontProxyCAKey(cfg *kubeadmapi.InitConfiguration) error { - if err := os.Remove(filepath.Join(cfg.CertificatesDir, kubeadmconstants.FrontProxyCAKeyName)); err != nil { - return errors.Wrapf(err, "failed removing %s", kubeadmconstants.FrontProxyCAKeyName) - } - return nil } func assertCertsExist(t *testing.T, dir string) { diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go index de58ba2824..3931b2ea05 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go @@ -362,24 +362,24 @@ func ValidateKubeconfigsForExternalCA(outDir string, cfg *kubeadmapi.InitConfigu kubeadmconstants.SchedulerKubeConfigFileName, } - specs, err := getKubeConfigSpecs(cfg) + // Creates a kubeconfig file with the target CA and server URL + // to be used as a input for validating user provided kubeconfig files + caCert, err := pkiutil.TryLoadCertFromDisk(cfg.CertificatesDir, kubeadmconstants.CACertAndKeyBaseName) + if err != nil { + return errors.Wrapf(err, "the CA file couldn't be loaded") + } + + controlPlaneEndpoint, err := kubeadmutil.GetControlPlaneEndpoint(cfg.ControlPlaneEndpoint, &cfg.LocalAPIEndpoint) if err != nil { return err } + validationConfig := kubeconfigutil.CreateBasic(controlPlaneEndpoint, "dummy", "dummy", pkiutil.EncodeCertPEM(caCert)) + + // validate user provided kubeconfig files 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 + if err = validateKubeConfig(outDir, kubeConfigFileName, validationConfig); err != nil { + return errors.Wrapf(err, "the %s file does not exists or it is not valid", kubeConfigFileName) } } return nil diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go index 112df6e03a..a34ad20cbc 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go @@ -512,14 +512,23 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { }, } + // creates CA, write to pkiDir and remove ca.key to get into external CA condition 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) } + 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") + + // create a config with another CA + anotherCaCert, anotherCaKey := certstestutil.SetupCertificateAuthorithy(t) configWithAnotherClusterCa := setupdKubeConfigWithClientAuth(t, anotherCaCert, anotherCaKey, "https://1.2.3.4:1234", "test-cluster", "myOrg1") + + // create a config with another server URL configWithAnotherServerURL := setupdKubeConfigWithClientAuth(t, caCert, caKey, "https://4.3.2.1:4321", "test-cluster", "myOrg1") tests := map[string]struct { @@ -539,11 +548,21 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { initConfig: initConfig, expectedError: true, }, - "some files are invalid": { + "some files have invalid CA": { filesToWrite: map[string]*clientcmdapi.Config{ kubeadmconstants.AdminKubeConfigFileName: config, kubeadmconstants.KubeletKubeConfigFileName: config, kubeadmconstants.ControllerManagerKubeConfigFileName: configWithAnotherClusterCa, + kubeadmconstants.SchedulerKubeConfigFileName: config, + }, + initConfig: initConfig, + expectedError: true, + }, + "some files have invalid Server Url": { + filesToWrite: map[string]*clientcmdapi.Config{ + kubeadmconstants.AdminKubeConfigFileName: config, + kubeadmconstants.KubeletKubeConfigFileName: config, + kubeadmconstants.ControllerManagerKubeConfigFileName: config, kubeadmconstants.SchedulerKubeConfigFileName: configWithAnotherServerURL, }, initConfig: initConfig, @@ -567,7 +586,7 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { for name, config := range test.filesToWrite { if err := createKubeConfigFileIfNotExists(tmpdir, name, config); err != nil { - t.Errorf("createKubeConfigFileIfNotExists failed") + t.Errorf("createKubeConfigFileIfNotExists failed: %v", err) } }