From 9e8228f14a95b61df8eee2fa39083e15635a562c Mon Sep 17 00:00:00 2001 From: liz Date: Mon, 19 Nov 2018 14:29:19 -0500 Subject: [PATCH 1/2] Move some test functions into test utils --- cmd/kubeadm/app/phases/certs/certs_test.go | 164 ++++----------------- cmd/kubeadm/test/certs/util.go | 114 ++++++++++++++ 2 files changed, 145 insertions(+), 133 deletions(-) diff --git a/cmd/kubeadm/app/phases/certs/certs_test.go b/cmd/kubeadm/app/phases/certs/certs_test.go index cd5561b0ef..d13416f317 100644 --- a/cmd/kubeadm/app/phases/certs/certs_test.go +++ b/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) - caCert, caKey := createCACert(t) + setupCert, setupKey := certstestutil.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) - setupCert, setupKey := createTestCert(t, caCert, caKey) - cert, key := createTestCert(t, caCert, caKey) + caCert, caKey := certstestutil.CreateCACert(t) + setupCert, setupKey := certstestutil.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) - anotherCert, anotherKey := createTestCert(t, anotherCaCert, anotherCaKey) + anotherCaCert, anotherCaKey := certstestutil.CreateCACert(t) + 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) - _, key := createTestCert(t, caCert, caKey) + caCert, caKey := certstestutil.CreateCACert(t) + _, 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) - fpCACert, fpCAKey := createCACert(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) { + for _, test := range certstestutil.GetSparseCertTestCases(t) { + 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) - cert, key := createTestCert(t, caCert, caKey) + caCert, caKey := certstestutil.CreateCACert(t) + 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 { diff --git a/cmd/kubeadm/test/certs/util.go b/cmd/kubeadm/test/certs/util.go index f22a3056a2..3a867d85d9 100644 --- a/cmd/kubeadm/test/certs/util.go +++ b/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) + } + } + } +} From 2f14e1801ea4ff9c6c5c2c7b78ae7406501f6bf7 Mon Sep 17 00:00:00 2001 From: liz Date: Mon, 19 Nov 2018 15:25:11 -0500 Subject: [PATCH 2/2] `kubeadm init` supports sparse certificates --- cmd/kubeadm/app/cmd/phases/BUILD | 3 +++ cmd/kubeadm/app/cmd/phases/certs.go | 26 +++++++++++++++----- cmd/kubeadm/app/cmd/phases/certs_test.go | 30 ++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/cmd/kubeadm/app/cmd/phases/BUILD b/cmd/kubeadm/app/cmd/phases/BUILD index 932f6b732f..08d6431656 100644 --- a/cmd/kubeadm/app/cmd/phases/BUILD +++ b/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", ], ) diff --git a/cmd/kubeadm/app/cmd/phases/certs.go b/cmd/kubeadm/app/cmd/phases/certs.go index 778cd6c368..cf82a5be68 100644 --- a/cmd/kubeadm/app/cmd/phases/certs.go +++ b/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 - if data.ExternalCA() { - fmt.Printf("[certs] External CA mode: Using existing %s certificate authority\n", ca.BaseName) + // TODO(EKF): can we avoid loading these certificates every time? + if _, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), ca.BaseName); err == nil { + 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 - if data.ExternalCA() { - fmt.Printf("[certs] External CA mode: Using existing %s certificate\n", cert.BaseName) + // TODO(EKF): can we avoid loading these certificates every time? + if certData, _, err := pkiutil.TryLoadCertAndKeyFromDisk(data.CertificateDir(), cert.BaseName); err == nil { + 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 } diff --git a/cmd/kubeadm/app/cmd/phases/certs_test.go b/cmd/kubeadm/app/cmd/phases/certs_test.go index 06fc61ce63..3e9c49d8d6 100644 --- a/cmd/kubeadm/app/cmd/phases/certs_test.go +++ b/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) + } + }) + } +}