From 00fa026b9d9c00076d1a64ed6c688355aac40615 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Fri, 18 Aug 2017 09:14:11 +0200 Subject: [PATCH] Main work -- cleanup certs CLI command --- cmd/kubeadm/app/cmd/init.go | 4 +- cmd/kubeadm/app/cmd/phases/certs.go | 251 ++--------------------- cmd/kubeadm/app/cmd/phases/certs_test.go | 106 +++++++--- 3 files changed, 92 insertions(+), 269 deletions(-) diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index f1f88127397..39f1c8b7adb 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -31,13 +31,13 @@ import ( kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/features" - cmdphases "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" dnsaddonphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns" proxyaddonphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy" apiconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/apiconfig" 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" "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil" controlplanephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane" etcdphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/etcd" @@ -234,7 +234,7 @@ func (i *Init) Run(out io.Writer) error { } // PHASE 1: Generate certificates - if err := cmdphases.CreatePKIAssets(i.cfg); err != nil { + if err := certsphase.CreatePKIAssets(i.cfg); err != nil { return err } diff --git a/cmd/kubeadm/app/cmd/phases/certs.go b/cmd/kubeadm/app/cmd/phases/certs.go index aa8c23ce1be..6e4344d74cd 100644 --- a/cmd/kubeadm/app/cmd/phases/certs.go +++ b/cmd/kubeadm/app/cmd/phases/certs.go @@ -17,17 +17,12 @@ limitations under the License. package phases import ( - "crypto/rsa" - "crypto/x509" - "fmt" - "github.com/spf13/cobra" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1" - kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" - certphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" - "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil" + "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" + certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config" "k8s.io/kubernetes/pkg/api" @@ -64,37 +59,37 @@ func getCertsSubCommands() []*cobra.Command { { use: "all", short: "Generate all PKI assets necessary to establish the control plane", - cmdFunc: CreatePKIAssets, + cmdFunc: certsphase.CreatePKIAssets, }, { use: "ca", short: "Generate CA certificate and key for a Kubernetes cluster.", - cmdFunc: createOrUseCACertAndKey, + cmdFunc: certsphase.CreateCACertAndKeyfiles, }, { use: "apiserver", short: "Generate API Server serving certificate and key.", - cmdFunc: createOrUseAPIServerCertAndKey, + cmdFunc: certsphase.CreateAPIServerCertAndKeyFiles, }, { use: "apiserver-kubelet-client", short: "Generate a client certificate for the API Server to connect to the kubelets securely.", - cmdFunc: createOrUseAPIServerKubeletClientCertAndKey, + cmdFunc: certsphase.CreateAPIServerKubeletClientCertAndKeyFiles, }, { use: "sa", short: "Generate a private key for signing service account tokens along with its public key.", - cmdFunc: createOrUseServiceAccountKeyAndPublicKey, + cmdFunc: certsphase.CreateServiceAccountKeyAndPublicKeyFiles, }, { use: "front-proxy-ca", short: "Generate front proxy CA certificate and key for a Kubernetes cluster.", - cmdFunc: createOrUseFrontProxyCACertAndKey, + cmdFunc: certsphase.CreateFrontProxyCACertAndKeyFiles, }, { use: "front-proxy-client", short: "Generate front proxy CA client certificate and key for a Kubernetes cluster.", - cmdFunc: createOrUseFrontProxyClientCertAndKey, + cmdFunc: certsphase.CreateFrontProxyClientCertAndKeyFiles, }, } @@ -131,6 +126,9 @@ func runCmdFunc(cmdFunc func(cfg *kubeadmapi.MasterConfiguration) error, cfgPath // are shared between sub commands and gets access to current value e.g. flags value. return func(cmd *cobra.Command, args []string) { + if err := validation.ValidateMixedArguments(cmd.Flags()); err != nil { + kubeadmutil.CheckErr(err) + } // This call returns the ready-to-use configuration based on the configuration file that might or might not exist and the default cfg populated by flags internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(*cfgPath, cfg) @@ -141,228 +139,3 @@ func runCmdFunc(cmdFunc func(cfg *kubeadmapi.MasterConfiguration) error, cfgPath kubeadmutil.CheckErr(err) } } - -// CreatePKIAssets will create and write to disk all PKI assets necessary to establish the control plane. -// Please note that this action is a bulk action calling all the atomic certphase actions -func CreatePKIAssets(cfg *kubeadmapi.MasterConfiguration) error { - - certActions := []func(cfg *kubeadmapi.MasterConfiguration) error{ - createOrUseCACertAndKey, - createOrUseAPIServerCertAndKey, - createOrUseAPIServerKubeletClientCertAndKey, - createOrUseServiceAccountKeyAndPublicKey, - createOrUseFrontProxyCACertAndKey, - createOrUseFrontProxyClientCertAndKey, - } - - for _, action := range certActions { - err := action(cfg) - if err != nil { - return err - } - } - - fmt.Printf("[certificates] Valid certificates and keys now exist in %q\n", cfg.CertificatesDir) - - return nil -} - -// createOrUseCACertAndKey create a new self signed CA, or use the existing one. -func createOrUseCACertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseCertificateAuthorithy( - cfg.CertificatesDir, - kubeadmconstants.CACertAndKeyBaseName, - "CA", - certphase.NewCACertAndKey, - ) -} - -// createOrUseAPIServerCertAndKey create a new CA certificate for apiserver, or use the existing one. -// It assumes the CA certificates should exists into the CertificatesDir -func createOrUseAPIServerCertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseSignedCertificate( - cfg.CertificatesDir, - kubeadmconstants.CACertAndKeyBaseName, - kubeadmconstants.APIServerCertAndKeyBaseName, - "API server", - func(caCert *x509.Certificate, caKey *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey, error) { - return certphase.NewAPIServerCertAndKey(cfg, caCert, caKey) - }, - ) -} - -// create a new CA certificate for kubelets calling apiserver, or use the existing one -// It assumes the CA certificates should exists into the CertificatesDir -func createOrUseAPIServerKubeletClientCertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseSignedCertificate( - cfg.CertificatesDir, - kubeadmconstants.CACertAndKeyBaseName, - kubeadmconstants.APIServerKubeletClientCertAndKeyBaseName, - "API server kubelet client", - certphase.NewAPIServerKubeletClientCertAndKey, - ) -} - -// createOrUseServiceAccountKeyAndPublicKey create a new public/private key pairs for signing service account user, or use the existing one. -func createOrUseServiceAccountKeyAndPublicKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseKeyAndPublicKey( - cfg.CertificatesDir, - kubeadmconstants.ServiceAccountKeyBaseName, - "service account", - certphase.NewServiceAccountSigningKey, - ) -} - -// createOrUseFrontProxyCACertAndKey create a new self signed front proxy CA, or use the existing one. -func createOrUseFrontProxyCACertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseCertificateAuthorithy( - cfg.CertificatesDir, - kubeadmconstants.FrontProxyCACertAndKeyBaseName, - "front-proxy CA", - certphase.NewFrontProxyCACertAndKey, - ) -} - -// createOrUseFrontProxyClientCertAndKey create a new certificate for proxy server client, or use the existing one. -// It assumes the front proxy CA certificates should exists into the CertificatesDir -func createOrUseFrontProxyClientCertAndKey(cfg *kubeadmapi.MasterConfiguration) error { - - return createOrUseSignedCertificate( - cfg.CertificatesDir, - kubeadmconstants.FrontProxyCACertAndKeyBaseName, - kubeadmconstants.FrontProxyClientCertAndKeyBaseName, - "front-proxy client", - certphase.NewFrontProxyClientCertAndKey, - ) -} - -// createOrUseCertificateAuthorithy is a generic function that will create a new certificate Authorithy using the given newFunc, -// assign file names according to the given baseName, or use the existing one already present in pkiDir. -func createOrUseCertificateAuthorithy(pkiDir string, baseName string, UXName string, newFunc func() (*x509.Certificate, *rsa.PrivateKey, error)) error { - - // If cert or key exists, we should try to load them - if pkiutil.CertOrKeyExist(pkiDir, baseName) { - - // Try to load .crt and .key from the PKI directory - caCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName) - if err != nil { - return fmt.Errorf("failure loading %s certificate: %v", UXName, err) - } - - // Check if the existing cert is a CA - if !caCert.IsCA { - return fmt.Errorf("certificate %s is not a CA", UXName) - } - - fmt.Printf("[certificates] Using the existing %s certificate and key.\n", UXName) - } else { - // The certificate and the key did NOT exist, let's generate them now - caCert, caKey, err := newFunc() - if err != nil { - return fmt.Errorf("failure while generating %s certificate and key: %v", UXName, err) - } - - // Write .crt and .key files to disk - if err = pkiutil.WriteCertAndKey(pkiDir, baseName, caCert, caKey); err != nil { - return fmt.Errorf("failure while saving %s certificate and key: %v", UXName, err) - } - - fmt.Printf("[certificates] Generated %s certificate and key.\n", UXName) - } - return nil -} - -// createOrUseSignedCertificate is a generic function that will create a new signed certificate using the given newFunc, -// assign file names according to the given baseName, or use the existing one already present in pkiDir. -func createOrUseSignedCertificate(pkiDir string, CABaseName string, baseName string, UXName string, newFunc func(*x509.Certificate, *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey, error)) error { - - // Checks if certificate authorithy exists in the PKI directory - if !pkiutil.CertOrKeyExist(pkiDir, CABaseName) { - return fmt.Errorf("couldn't load certificate authorithy for %s from certificate dir", UXName) - } - - // Try to load certificate authorithy .crt and .key from the PKI directory - caCert, caKey, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, CABaseName) - if err != nil { - return fmt.Errorf("failure loading certificate authorithy for %s: %v", UXName, err) - } - - // Make sure the loaded CA cert actually is a CA - if !caCert.IsCA { - return fmt.Errorf("certificate authorithy for %s is not a CA", UXName) - } - - // Checks if the signed certificate exists in the PKI directory - if pkiutil.CertOrKeyExist(pkiDir, baseName) { - // Try to load signed certificate .crt and .key from the PKI directory - signedCert, _, err := pkiutil.TryLoadCertAndKeyFromDisk(pkiDir, baseName) - if err != nil { - return fmt.Errorf("failure loading %s certificate: %v", UXName, err) - } - - // Check if the existing cert is signed by the given CA - if err := signedCert.CheckSignatureFrom(caCert); err != nil { - return fmt.Errorf("certificate %s is not signed by corresponding CA", UXName) - } - - fmt.Printf("[certificates] Using the existing %s certificate and key.\n", UXName) - } else { - // The certificate and the key did NOT exist, let's generate them now - signedCert, signedKey, err := newFunc(caCert, caKey) - if err != nil { - return fmt.Errorf("failure while generating %s key and certificate: %v", UXName, err) - } - - // Write .crt and .key files to disk - if err = pkiutil.WriteCertAndKey(pkiDir, baseName, signedCert, signedKey); err != nil { - return fmt.Errorf("failure while saving %s certificate and key: %v", UXName, err) - } - - fmt.Printf("[certificates] Generated %s certificate and key.\n", UXName) - if pkiutil.HasServerAuth(signedCert) { - fmt.Printf("[certificates] %s serving cert is signed for DNS names %v and IPs %v\n", UXName, signedCert.DNSNames, signedCert.IPAddresses) - } - } - - return nil -} - -// createOrUseKeyAndPublicKey is a generic function that will create a new public/private key pairs using the given newFunc, -// assign file names according to the given baseName, or use the existing one already present in pkiDir. -func createOrUseKeyAndPublicKey(pkiDir string, baseName string, UXName string, newFunc func() (*rsa.PrivateKey, error)) error { - - // Checks if the key exists in the PKI directory - if pkiutil.CertOrKeyExist(pkiDir, baseName) { - - // Try to load .key from the PKI directory - _, err := pkiutil.TryLoadKeyFromDisk(pkiDir, baseName) - if err != nil { - return fmt.Errorf("%s key existed but they could not be loaded properly: %v", UXName, err) - } - - fmt.Printf("[certificates] Using the existing %s key.\n", UXName) - } else { - // The key does NOT exist, let's generate it now - key, err := newFunc() - if err != nil { - return fmt.Errorf("failure while generating %s key: %v", UXName, err) - } - - // Write .key and .pub files to disk - if err = pkiutil.WriteKey(pkiDir, baseName, key); err != nil { - return fmt.Errorf("failure while saving %s key: %v", UXName, err) - } - - if err = pkiutil.WritePublicKey(pkiDir, baseName, &key.PublicKey); err != nil { - return fmt.Errorf("failure while saving %s public key: %v", UXName, err) - } - fmt.Printf("[certificates] Generated %s key and public key.\n", UXName) - } - - return nil -} diff --git a/cmd/kubeadm/app/cmd/phases/certs_test.go b/cmd/kubeadm/app/cmd/phases/certs_test.go index 8cb5e7414e2..abf67f69923 100644 --- a/cmd/kubeadm/app/cmd/phases/certs_test.go +++ b/cmd/kubeadm/app/cmd/phases/certs_test.go @@ -33,7 +33,61 @@ import ( cmdtestutil "k8s.io/kubernetes/cmd/kubeadm/test/cmd" ) -func TestSubCmdCertsCreateFiles(t *testing.T) { +func TestCertsSubCommandsHasFlags(t *testing.T) { + + subCmds := getCertsSubCommands() + + commonFlags := []string{ + "cert-dir", + "config", + } + + var tests = []struct { + command string + additionalFlags []string + }{ + { + command: "all", + additionalFlags: []string{ + "apiserver-advertise-address", + "apiserver-cert-extra-sans", + "service-cidr", + "service-dns-domain", + }, + }, + { + command: "ca", + }, + { + command: "apiserver", + additionalFlags: []string{ + "apiserver-advertise-address", + "apiserver-cert-extra-sans", + "service-cidr", + "service-dns-domain", + }, + }, + { + command: "apiserver-kubelet-client", + }, + { + command: "sa", + }, + { + command: "front-proxy-ca", + }, + { + command: "front-proxy-client", + }, + } + + for _, test := range tests { + expectedFlags := append(commonFlags, test.additionalFlags...) + cmdtestutil.AssertSubCommandHasFlags(t, subCmds, test.command, expectedFlags...) + } +} + +func TestSubCmdCertsCreateFilesWithFlags(t *testing.T) { subCmds := getCertsSubCommands() @@ -53,25 +107,13 @@ func TestSubCmdCertsCreateFiles(t *testing.T) { }, }, { - subCmds: []string{"ca"}, - expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName}, - }, - { - subCmds: []string{"ca", "apiserver"}, - expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName}, - }, - { - subCmds: []string{"ca", "apiserver-kubelet-client"}, - expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName}, + subCmds: []string{"ca", "apiserver", "apiserver-kubelet-client"}, + expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName}, }, { subCmds: []string{"sa"}, expectedFiles: []string{kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName}, }, - { - subCmds: []string{"front-proxy-ca"}, - expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName}, - }, { subCmds: []string{"front-proxy-ca", "front-proxy-client"}, expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName}, @@ -94,7 +136,7 @@ func TestSubCmdCertsCreateFiles(t *testing.T) { } } -func TestSubCmdApiServerFlags(t *testing.T) { +func TestSubCmdCertsApiServerForwardsFlags(t *testing.T) { subCmds := getCertsSubCommands() @@ -116,6 +158,7 @@ func TestSubCmdApiServerFlags(t *testing.T) { } cmdtestutil.RunSubCommand(t, subCmds, "apiserver", apiserverFlags...) + // asserts created cert has values from CLI flags APIserverCert, err := pkiutil.TryLoadCertFromDisk(tmpdir, kubeadmconstants.APIServerCertAndKeyBaseName) if err != nil { t.Fatalf("Error loading API server certificate: %v", err) @@ -135,29 +178,36 @@ func TestSubCmdApiServerFlags(t *testing.T) { } } -func TestSubCmdCertsReadsConfig(t *testing.T) { +func TestSubCmdCertsCreateFilesWithConfigFile(t *testing.T) { subCmds := getCertsSubCommands() var tests = []struct { - subCmds []string - expectedFileCount int + subCmds []string + expectedFiles []string }{ { - subCmds: []string{"sa"}, - expectedFileCount: 2, + subCmds: []string{"all"}, + expectedFiles: []string{ + kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, + kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, + kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName, + kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName, + kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, + kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName, + }, }, { - subCmds: []string{"front-proxy-ca", "front-proxy-client"}, - expectedFileCount: 4, + subCmds: []string{"ca", "apiserver", "apiserver-kubelet-client"}, + expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName}, }, { - subCmds: []string{"ca", "apiserver", "apiserver-kubelet-client"}, - expectedFileCount: 6, + subCmds: []string{"front-proxy-ca", "front-proxy-client"}, + expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName}, }, { - subCmds: []string{"all"}, - expectedFileCount: 12, + subCmds: []string{"sa"}, + expectedFiles: []string{kubeadmconstants.ServiceAccountPrivateKeyName, kubeadmconstants.ServiceAccountPublicKeyName}, }, } @@ -182,6 +232,6 @@ func TestSubCmdCertsReadsConfig(t *testing.T) { } // verify expected files are there - testutil.AssertFilesCount(t, tmpdir, test.expectedFileCount) + testutil.AssertFileExists(t, tmpdir, test.expectedFiles...) } }