Main work -- cleanup certs CLI command

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
 	}
 

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"
+	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
-	certphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
+	certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
-	"k8s.io/kubernetes/cmd/kubeadm/app/phases/certs/pkiutil"
 	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
-}

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"},
+			subCmds:       []string{"ca", "apiserver", "apiserver-kubelet-client"},
-			expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName},
+			expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName},
-		},
-		{
-			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{"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
+		subCmds       []string
-		expectedFileCount int
+		expectedFiles []string
 	}{
 		{
-			subCmds:           []string{"sa"},
+			subCmds: []string{"all"},
-			expectedFileCount: 2,
+			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"},
+			subCmds:       []string{"ca", "apiserver", "apiserver-kubelet-client"},
-			expectedFileCount: 4,
+			expectedFiles: []string{kubeadmconstants.CACertName, kubeadmconstants.CAKeyName, kubeadmconstants.APIServerCertName, kubeadmconstants.APIServerKeyName, kubeadmconstants.APIServerKubeletClientCertName, kubeadmconstants.APIServerKubeletClientKeyName},
 		},
 		{
-			subCmds:           []string{"ca", "apiserver", "apiserver-kubelet-client"},
+			subCmds:       []string{"front-proxy-ca", "front-proxy-client"},
-			expectedFileCount: 6,
+			expectedFiles: []string{kubeadmconstants.FrontProxyCACertName, kubeadmconstants.FrontProxyCAKeyName, kubeadmconstants.FrontProxyClientCertName, kubeadmconstants.FrontProxyClientKeyName},
 		},
 		{
-			subCmds:           []string{"all"},
+			subCmds:       []string{"sa"},
-			expectedFileCount: 12,
+			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...)
 	}
 }