Merge pull request #71232 from liztio/sparse-cert-creation

Sparse cert creation for `kubeadm init`

cmd/kubeadm/app/cmd/phases/BUILD

@@ -44,6 +44,7 @@ go_library(
         "//cmd/kubeadm/app/util/apiclient:go_default_library",
         "//cmd/kubeadm/app/util/config:go_default_library",
         "//cmd/kubeadm/app/util/dryrun:go_default_library",
+        "//cmd/kubeadm/app/util/pkiutil:go_default_library",
         "//pkg/util/normalizer:go_default_library",
         "//pkg/version:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
@@ -72,7 +73,9 @@ go_test(
         "//cmd/kubeadm/app/phases/certs:go_default_library",
         "//cmd/kubeadm/app/util/pkiutil:go_default_library",
         "//cmd/kubeadm/test:go_default_library",
+        "//cmd/kubeadm/test/certs:go_default_library",
         "//pkg/version:go_default_library",
+        "//vendor/github.com/spf13/cobra:go_default_library",
     ],
 )
 

cmd/kubeadm/app/cmd/phases/certs.go

@@ -31,6 +31,7 @@ import (
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
+	"k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
 	"k8s.io/kubernetes/pkg/util/normalizer"
 )
 
@@ -228,9 +229,13 @@ func runCAPhase(ca *certsphase.KubeadmCert) func(c workflow.RunData) error {
 			return errors.New("certs phase invoked with an invalid data struct")
 		}
 
-		// if external CA mode, skips certificate authority generation
+		// TODO(EKF): can we avoid loading these certificates every time?
-		if data.ExternalCA() {
+		if _, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), ca.BaseName); err == nil {
-			fmt.Printf("[certs] External CA mode: Using existing %s certificate authority\n", ca.BaseName)
+			if _, err := pkiutil.TryLoadKeyFromDisk(data.CertificateDir(), ca.BaseName); err == nil {
+				fmt.Printf("[certs] Using existing %s certificate authority\n", ca.BaseName)
+				return nil
+			}
+			fmt.Printf("[certs] Using existing %s keyless certificate authority", ca.BaseName)
 			return nil
 		}
 
@@ -257,9 +262,18 @@ func runCertPhase(cert *certsphase.KubeadmCert, caCert *certsphase.KubeadmCert)
 			return errors.New("certs phase invoked with an invalid data struct")
 		}
 
-		// if external CA mode, skip certificate generation
+		// TODO(EKF): can we avoid loading these certificates every time?
-		if data.ExternalCA() {
+		if certData, _, err := pkiutil.TryLoadCertAndKeyFromDisk(data.CertificateDir(), cert.BaseName); err == nil {
-			fmt.Printf("[certs] External CA mode: Using existing %s certificate\n", cert.BaseName)
+			caCertData, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), caCert.BaseName)
+			if err != nil {
+				return errors.Wrapf(err, "couldn't load CA certificate %s", caCert.Name)
+			}
+
+			if err := certData.CheckSignatureFrom(caCertData); err != nil {
+				return errors.Wrapf(err, "[certs] certificate %s not signed by CA certificate %s", cert.BaseName, caCert.BaseName)
+			}
+
+			fmt.Printf("[certs] Using existing %s certificate and key on disk\n", cert.BaseName)
 			return nil
 		}
 

cmd/kubeadm/app/cmd/phases/certs_test.go

@@ -20,11 +20,13 @@ import (
 	"os"
 	"testing"
 
+	"github.com/spf13/cobra"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
 	testutil "k8s.io/kubernetes/cmd/kubeadm/test"
+	certstestutil "k8s.io/kubernetes/cmd/kubeadm/test/certs"
 )
 
 type testCertsData struct {
@@ -51,6 +53,9 @@ func TestCertsWithCSRs(t *testing.T) {
 	// global vars
 	csrOnly = true
 	csrDir = certDir
+	defer func() {
+		csrOnly = false
+	}()
 
 	phase := NewCertsPhase()
 	// find the api cert phase
@@ -75,3 +80,28 @@ func TestCertsWithCSRs(t *testing.T) {
 		t.Fatalf("couldn't load certificate %q: %v", cert.BaseName, err)
 	}
 }
+
+func TestCreateSparseCerts(t *testing.T) {
+	for _, test := range certstestutil.GetSparseCertTestCases(t) {
+		t.Run(test.Name, func(t *testing.T) {
+			tmpdir := testutil.SetupTempDir(t)
+			defer os.RemoveAll(tmpdir)
+
+			certstestutil.WritePKIFiles(t, tmpdir, test.Files)
+
+			r := workflow.NewRunner()
+			r.AppendPhase(NewCertsPhase())
+			r.SetDataInitializer(func(*cobra.Command) (workflow.RunData, error) {
+				certsData := &testCertsData{
+					cfg: testutil.GetDefaultInternalConfig(t),
+				}
+				certsData.cfg.CertificatesDir = tmpdir
+				return certsData, nil
+			})
+
+			if err := r.Run(); (err != nil) != test.ExpectError {
+				t.Fatalf("expected error to be %t, got %t (%v)", test.ExpectError, (err != nil), err)
+			}
+		})
+	}
+}

cmd/kubeadm/app/phases/certs/certs_test.go

@@ -37,27 +37,6 @@ import (
 	certstestutil "k8s.io/kubernetes/cmd/kubeadm/test/certs"
 )
 
-func createCACert(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) {
-	certCfg := &certutil.Config{CommonName: "kubernetes"}
-	cert, key, err := NewCACertAndKey(certCfg)
-	if err != nil {
-		t.Fatalf("couldn't create CA: %v", err)
-	}
-	return cert, key
-}
-
-func createTestCert(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey) {
-	cert, key, err := pkiutil.NewCertAndKey(caCert, caKey,
-		&certutil.Config{
-			CommonName: "testCert",
-			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
-		})
-	if err != nil {
-		t.Fatalf("couldn't create test cert: %v", err)
-	}
-	return cert, key
-}
-
 func createTestCSR(t *testing.T) (*x509.CertificateRequest, *rsa.PrivateKey) {
 	csr, key, err := pkiutil.NewCSRAndKey(
 		&certutil.Config{
@@ -71,8 +50,8 @@ func createTestCSR(t *testing.T) (*x509.CertificateRequest, *rsa.PrivateKey) {
 }
 
 func TestWriteCertificateAuthorithyFilesIfNotExist(t *testing.T) {
-	setupCert, setupKey := createCACert(t)
+	setupCert, setupKey := certstestutil.CreateCACert(t)
-	caCert, caKey := createCACert(t)
+	caCert, caKey := certstestutil.CreateCACert(t)
 
 	var tests = []struct {
 		setupFunc     func(pkiDir string) error
@@ -97,7 +76,7 @@ func TestWriteCertificateAuthorithyFilesIfNotExist(t *testing.T) {
 		},
 		{ // cert exists, but it is not a ca > err
 			setupFunc: func(pkiDir string) error {
-				cert, key := createTestCert(t, setupCert, setupKey)
+				cert, key := certstestutil.CreateTestCert(t, setupCert, setupKey)
 				return writeCertificateFilesIfNotExist(pkiDir, "dummy", setupCert, cert, key)
 			},
 			expectedError: true,
@@ -147,9 +126,9 @@ func TestWriteCertificateAuthorithyFilesIfNotExist(t *testing.T) {
 
 func TestWriteCertificateFilesIfNotExist(t *testing.T) {
 
-	caCert, caKey := createCACert(t)
+	caCert, caKey := certstestutil.CreateCACert(t)
-	setupCert, setupKey := createTestCert(t, caCert, caKey)
+	setupCert, setupKey := certstestutil.CreateTestCert(t, caCert, caKey)
-	cert, key := createTestCert(t, caCert, caKey)
+	cert, key := certstestutil.CreateTestCert(t, caCert, caKey)
 
 	var tests = []struct {
 		setupFunc     func(pkiDir string) error
@@ -174,8 +153,8 @@ func TestWriteCertificateFilesIfNotExist(t *testing.T) {
 		},
 		{ // cert exists, is signed by another ca > err
 			setupFunc: func(pkiDir string) error {
-				anotherCaCert, anotherCaKey := createCACert(t)
+				anotherCaCert, anotherCaKey := certstestutil.CreateCACert(t)
-				anotherCert, anotherKey := createTestCert(t, anotherCaCert, anotherCaKey)
+				anotherCert, anotherKey := certstestutil.CreateTestCert(t, anotherCaCert, anotherCaKey)
 
 				return writeCertificateFilesIfNotExist(pkiDir, "dummy", anotherCaCert, anotherCert, anotherKey)
 			},
@@ -375,18 +354,18 @@ func TestNewCACertAndKey(t *testing.T) {
 }
 
 func TestSharedCertificateExists(t *testing.T) {
-	caCert, caKey := createCACert(t)
+	caCert, caKey := certstestutil.CreateCACert(t)
-	_, key := createTestCert(t, caCert, caKey)
+	_, key := certstestutil.CreateTestCert(t, caCert, caKey)
 	publicKey := &key.PublicKey
 
 	var tests = []struct {
 		name          string
-		files         pkiFiles
+		files         certstestutil.PKIFiles
 		expectedError bool
 	}{
 		{
 			name: "success",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt":             caCert,
 				"ca.key":             caKey,
 				"front-proxy-ca.crt": caCert,
@@ -399,7 +378,7 @@ func TestSharedCertificateExists(t *testing.T) {
 		},
 		{
 			name: "missing ca.crt",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.key":             caKey,
 				"front-proxy-ca.crt": caCert,
 				"front-proxy-ca.key": caKey,
@@ -412,7 +391,7 @@ func TestSharedCertificateExists(t *testing.T) {
 		},
 		{
 			name: "missing sa.key",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt":             caCert,
 				"ca.key":             caKey,
 				"front-proxy-ca.crt": caCert,
@@ -425,7 +404,7 @@ func TestSharedCertificateExists(t *testing.T) {
 		},
 		{
 			name: "missing front-proxy.crt",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt":             caCert,
 				"ca.key":             caKey,
 				"front-proxy-ca.key": caKey,
@@ -438,7 +417,7 @@ func TestSharedCertificateExists(t *testing.T) {
 		},
 		{
 			name: "missing etcd/ca.crt",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt":             caCert,
 				"ca.key":             caKey,
 				"front-proxy-ca.key": caKey,
@@ -464,7 +443,7 @@ func TestSharedCertificateExists(t *testing.T) {
 			}
 
 			// created expected keys
-			writePKIFiles(t, tmpdir, test.files)
+			certstestutil.WritePKIFiles(t, tmpdir, test.files)
 
 			// executes create func
 			ret, err := SharedCertificateExists(cfg)
@@ -482,80 +461,24 @@ func TestSharedCertificateExists(t *testing.T) {
 }
 
 func TestCreatePKIAssetsWithSparseCerts(t *testing.T) {
-	caCert, caKey := createCACert(t)
+	for _, test := range certstestutil.GetSparseCertTestCases(t) {
-	fpCACert, fpCAKey := createCACert(t)
+		t.Run(test.Name, func(t *testing.T) {
-	etcdCACert, etcdCAKey := createCACert(t)
-
-	fpCert, fpKey := createTestCert(t, fpCACert, fpCAKey)
-
-	tests := []struct {
-		name        string
-		files       pkiFiles
-		expectError bool
-	}{
-		{
-			name: "nothing present",
-		},
-		{
-			name: "CAs already exist",
-			files: pkiFiles{
-				"ca.crt":             caCert,
-				"ca.key":             caKey,
-				"front-proxy-ca.crt": fpCACert,
-				"front-proxy-ca.key": fpCAKey,
-				"etcd/ca.crt":        etcdCACert,
-				"etcd/ca.key":        etcdCAKey,
-			},
-		},
-		{
-			name: "CA certs only",
-			files: pkiFiles{
-				"ca.crt":             caCert,
-				"front-proxy-ca.crt": fpCACert,
-				"etcd/ca.crt":        etcdCACert,
-			},
-			expectError: true,
-		},
-		{
-			name: "FrontProxyCA with certs",
-			files: pkiFiles{
-				"ca.crt":                 caCert,
-				"ca.key":                 caKey,
-				"front-proxy-ca.crt":     fpCACert,
-				"front-proxy-client.crt": fpCert,
-				"front-proxy-client.key": fpKey,
-				"etcd/ca.crt":            etcdCACert,
-				"etcd/ca.key":            etcdCAKey,
-			},
-		},
-		{
-			name: "FrontProxy certs missing CA",
-			files: pkiFiles{
-				"front-proxy-client.crt": fpCert,
-				"front-proxy-client.key": fpKey,
-			},
-			expectError: true,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
 			tmpdir := testutil.SetupTempDir(t)
 			defer os.RemoveAll(tmpdir)
 
 			cfg := testutil.GetDefaultInternalConfig(t)
 			cfg.ClusterConfiguration.CertificatesDir = tmpdir
 
-			writePKIFiles(t, tmpdir, test.files)
+			certstestutil.WritePKIFiles(t, tmpdir, test.Files)
 
 			err := CreatePKIAssets(cfg)
 			if err != nil {
-				if test.expectError {
+				if test.ExpectError {
 					return
 				}
 				t.Fatalf("Unexpected error: %v", err)
 			}
-			if test.expectError {
+			if test.ExpectError {
 				t.Fatal("Expected error from CreatePKIAssets, got none")
 			}
 			assertCertsExist(t, tmpdir)
@@ -612,19 +535,19 @@ func TestUsingExternalCA(t *testing.T) {
 
 func TestValidateMethods(t *testing.T) {
 
-	caCert, caKey := createCACert(t)
+	caCert, caKey := certstestutil.CreateCACert(t)
-	cert, key := createTestCert(t, caCert, caKey)
+	cert, key := certstestutil.CreateTestCert(t, caCert, caKey)
 
 	tests := []struct {
 		name            string
-		files           pkiFiles
+		files           certstestutil.PKIFiles
 		validateFunc    func(l certKeyLocation) error
 		loc             certKeyLocation
 		expectedSuccess bool
 	}{
 		{
 			name: "validateCACert",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt": caCert,
 			},
 			validateFunc:    validateCACert,
@@ -633,7 +556,7 @@ func TestValidateMethods(t *testing.T) {
 		},
 		{
 			name: "validateCACertAndKey (files present)",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt": caCert,
 				"ca.key": caKey,
 			},
@@ -642,7 +565,7 @@ func TestValidateMethods(t *testing.T) {
 			expectedSuccess: true,
 		},
 		{
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt": caCert,
 			},
 			name:            "validateCACertAndKey (key missing)",
@@ -652,7 +575,7 @@ func TestValidateMethods(t *testing.T) {
 		},
 		{
 			name: "validateSignedCert",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"ca.crt":        caCert,
 				"ca.key":        caKey,
 				"apiserver.crt": cert,
@@ -664,7 +587,7 @@ func TestValidateMethods(t *testing.T) {
 		},
 		{
 			name: "validatePrivatePublicKey",
-			files: pkiFiles{
+			files: certstestutil.PKIFiles{
 				"sa.pub": &key.PublicKey,
 				"sa.key": key,
 			},
@@ -679,7 +602,7 @@ func TestValidateMethods(t *testing.T) {
 		defer os.RemoveAll(dir)
 		test.loc.pkiDir = dir
 
-		writePKIFiles(t, dir, test.files)
+		certstestutil.WritePKIFiles(t, dir, test.files)
 
 		err := test.validateFunc(test.loc)
 		if test.expectedSuccess && err != nil {
@@ -722,31 +645,6 @@ func TestNewCSR(t *testing.T) {
 	}
 }
 
-type pkiFiles map[string]interface{}
-
-func writePKIFiles(t *testing.T, dir string, files pkiFiles) {
-	for filename, body := range files {
-		switch body := body.(type) {
-		case *x509.Certificate:
-			if err := certutil.WriteCert(path.Join(dir, filename), certutil.EncodeCertPEM(body)); err != nil {
-				t.Errorf("unable to write certificate to file %q: [%v]", dir, err)
-			}
-		case *rsa.PublicKey:
-			publicKeyBytes, err := certutil.EncodePublicKeyPEM(body)
-			if err != nil {
-				t.Errorf("unable to write public key to file %q: [%v]", filename, err)
-			}
-			if err := certutil.WriteKey(path.Join(dir, filename), publicKeyBytes); err != nil {
-				t.Errorf("unable to write public key to file %q: [%v]", filename, err)
-			}
-		case *rsa.PrivateKey:
-			if err := certutil.WriteKey(path.Join(dir, filename), certutil.EncodePrivateKeyPEM(body)); err != nil {
-				t.Errorf("unable to write private key to file %q: [%v]", filename, err)
-			}
-		}
-	}
-}
-
 func TestCreateCertificateFilesMethods(t *testing.T) {
 
 	var tests = []struct {

cmd/kubeadm/test/certs/util.go

@@ -20,6 +20,7 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"net"
+	"path"
 	"testing"
 
 	certutil "k8s.io/client-go/util/cert"
@@ -133,3 +134,116 @@ func AssertCertificateHasIPAddresses(t *testing.T, cert *x509.Certificate, IPAdd
 		}
 	}
 }
+
+// CreateCACert creates a generic CA cert.
+func CreateCACert(t *testing.T) (*x509.Certificate, *rsa.PrivateKey) {
+	certCfg := &certutil.Config{CommonName: "kubernetes"}
+	cert, key, err := pkiutil.NewCertificateAuthority(certCfg)
+	if err != nil {
+		t.Fatalf("couldn't create CA: %v", err)
+	}
+	return cert, key
+}
+
+// CreateTestCert makes a generic certficate with the given CA.
+func CreateTestCert(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey) (*x509.Certificate, *rsa.PrivateKey) {
+	cert, key, err := pkiutil.NewCertAndKey(caCert, caKey,
+		&certutil.Config{
+			CommonName: "testCert",
+			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
+		})
+	if err != nil {
+		t.Fatalf("couldn't create test cert: %v", err)
+	}
+	return cert, key
+}
+
+// CertTestCase is a configuration of certificates and whether it's expected to work.
+type CertTestCase struct {
+	Name        string
+	Files       PKIFiles
+	ExpectError bool
+}
+
+// GetSparseCertTestCases produces a series of cert configurations and their intended outcomes.
+func GetSparseCertTestCases(t *testing.T) []CertTestCase {
+
+	caCert, caKey := CreateCACert(t)
+	fpCACert, fpCAKey := CreateCACert(t)
+	etcdCACert, etcdCAKey := CreateCACert(t)
+
+	fpCert, fpKey := CreateTestCert(t, fpCACert, fpCAKey)
+
+	return []CertTestCase{
+		{
+			Name: "nothing present",
+		},
+		{
+			Name: "CAs already exist",
+			Files: PKIFiles{
+				"ca.crt":             caCert,
+				"ca.key":             caKey,
+				"front-proxy-ca.crt": fpCACert,
+				"front-proxy-ca.key": fpCAKey,
+				"etcd/ca.crt":        etcdCACert,
+				"etcd/ca.key":        etcdCAKey,
+			},
+		},
+		{
+			Name: "CA certs only",
+			Files: PKIFiles{
+				"ca.crt":             caCert,
+				"front-proxy-ca.crt": fpCACert,
+				"etcd/ca.crt":        etcdCACert,
+			},
+			ExpectError: true,
+		},
+		{
+			Name: "FrontProxyCA with certs",
+			Files: PKIFiles{
+				"ca.crt":                 caCert,
+				"ca.key":                 caKey,
+				"front-proxy-ca.crt":     fpCACert,
+				"front-proxy-client.crt": fpCert,
+				"front-proxy-client.key": fpKey,
+				"etcd/ca.crt":            etcdCACert,
+				"etcd/ca.key":            etcdCAKey,
+			},
+		},
+		{
+			Name: "FrontProxy certs missing CA",
+			Files: PKIFiles{
+				"front-proxy-client.crt": fpCert,
+				"front-proxy-client.key": fpKey,
+			},
+			ExpectError: true,
+		},
+	}
+}
+
+// PKIFiles are a list of files that should be created for a test case
+type PKIFiles map[string]interface{}
+
+// WritePKIFiles writes the given files out to the given directory
+func WritePKIFiles(t *testing.T, dir string, files PKIFiles) {
+	for filename, body := range files {
+		switch body := body.(type) {
+		case *x509.Certificate:
+			if err := certutil.WriteCert(path.Join(dir, filename), certutil.EncodeCertPEM(body)); err != nil {
+				t.Errorf("unable to write certificate to file %q: [%v]", dir, err)
+			}
+		case *rsa.PublicKey:
+			publicKeyBytes, err := certutil.EncodePublicKeyPEM(body)
+			if err != nil {
+				t.Errorf("unable to write public key to file %q: [%v]", filename, err)
+			}
+			if err := certutil.WriteKey(path.Join(dir, filename), publicKeyBytes); err != nil {
+				t.Errorf("unable to write public key to file %q: [%v]", filename, err)
+			}
+		case *rsa.PrivateKey:
+			if err := certutil.WriteKey(path.Join(dir, filename), certutil.EncodePrivateKeyPEM(body)); err != nil {
+				t.Errorf("unable to write private key to file %q: [%v]", filename, err)
+			}
+		}
+	}
+}