kubeadm: optimize tests in pki_helpers_tests.go

Reduce the number of calls to algorithm.GenerateKey()
but try not to reduce coverage.

cmd/kubeadm/app/util/pkiutil/pki_helpers_test.go

@@ -21,8 +21,8 @@ import (
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
-	"crypto/rsa"
 	"crypto/x509"
+	"fmt"
 	"io/ioutil"
 	"net"
 	"os"
@@ -33,61 +33,63 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
 
-func TestNewCertificateAuthority(t *testing.T) {
+var (
-	cert, key, err := NewCertificateAuthority(&CertConfig{
+	// TestMain generates the bellow certs and keys so that
-		Config: certutil.Config{CommonName: "kubernetes"},
+	// they are reused in tests whenever possible
+
+	rootCACert, servCert *x509.Certificate
+	rootCAKey, servKey   crypto.Signer
+
+	ecdsaKey *ecdsa.PrivateKey
+)
+
+func TestMain(m *testing.M) {
+	var err error
+
+	rootCACert, rootCAKey, err = NewCertificateAuthority(&CertConfig{
+		Config: certutil.Config{
+			CommonName: "Root CA 1",
+		},
+		PublicKeyAlgorithm: x509.RSA,
 	})
-
-	if cert == nil {
-		t.Error("failed NewCertificateAuthority, cert == nil")
-	} else if !cert.IsCA {
-		t.Error("cert is not a valida CA")
-	}
-
-	if key == nil {
-		t.Error("failed NewCertificateAuthority, key == nil")
-	}
-
 	if err != nil {
-		t.Errorf("failed NewCertificateAuthority with an error: %+v", err)
+		panic(fmt.Sprintf("Failed generating Root CA: %v", err))
 	}
+	if !rootCACert.IsCA {
+		panic("rootCACert is not a valid CA")
+	}
+
+	servCert, servKey, err = NewCertAndKey(rootCACert, rootCAKey, &CertConfig{
+		Config: certutil.Config{
+			CommonName: "kubernetes",
+			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		},
+	})
+	if err != nil {
+		panic(fmt.Sprintf("Failed generating serving cert/key: %v", err))
+	}
+
+	ecdsaKey, err = ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		panic("Could not generate ECDSA key")
+	}
+
+	os.Exit(m.Run())
 }
 
 func TestNewCertAndKey(t *testing.T) {
 	var tests = []struct {
 		name string
-		keyGenFunc func() (crypto.Signer, error)
+		key  crypto.Signer
-		expected   bool
 	}{
-		{
-			name: "RSA key too small",
-			keyGenFunc: func() (crypto.Signer, error) {
-				return rsa.GenerateKey(rand.Reader, 128)
-			},
-			expected: false,
-		},
-		{
-			name: "RSA should succeed",
-			keyGenFunc: func() (crypto.Signer, error) {
-				return rsa.GenerateKey(rand.Reader, 2048)
-			},
-			expected: true,
-		},
 		{
 			name: "ECDSA should succeed",
-			keyGenFunc: func() (crypto.Signer, error) {
+			key:  ecdsaKey,
-				return ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
-			},
-			expected: true,
 		},
 	}
 
 	for _, rt := range tests {
 		t.Run(rt.name, func(t *testing.T) {
-			caKey, err := rt.keyGenFunc()
-			if err != nil {
-				t.Fatalf("Couldn't create Private Key")
-			}
 			caCert := &x509.Certificate{}
 			config := &CertConfig{
 				Config: certutil.Config{
@@ -96,20 +98,24 @@ func TestNewCertAndKey(t *testing.T) {
 					Usages:       []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
 				},
 			}
-			_, _, actual := NewCertAndKey(caCert, caKey, config)
+			_, _, err := NewCertAndKey(caCert, rt.key, config)
-			if (actual == nil) != rt.expected {
+			if err != nil {
-				t.Errorf(
+				t.Errorf("failed NewCertAndKey: %v", err)
-					"failed NewCertAndKey:\n\texpected: %t\n\t  actual: %t",
-					rt.expected,
-					(actual == nil),
-				)
 			}
 		})
 	}
 }
 
 func TestHasServerAuth(t *testing.T) {
-	caCert, caKey, _ := NewCertificateAuthority(&CertConfig{Config: certutil.Config{CommonName: "kubernetes"}})
+	// Override NewPrivateKey to reuse the same key for all certs
+	// since this test is only checking cert.ExtKeyUsage
+	privateKeyFunc := NewPrivateKey
+	NewPrivateKey = func(x509.PublicKeyAlgorithm) (crypto.Signer, error) {
+		return rootCAKey, nil
+	}
+	defer func() {
+		NewPrivateKey = privateKeyFunc
+	}()
 
 	var tests = []struct {
 		name     string
@@ -151,7 +157,7 @@ func TestHasServerAuth(t *testing.T) {
 
 	for _, rt := range tests {
 		t.Run(rt.name, func(t *testing.T) {
-			cert, _, err := NewCertAndKey(caCert, caKey, &rt.config)
+			cert, _, err := NewCertAndKey(rootCACert, rootCAKey, &rt.config)
 			if err != nil {
 				t.Fatalf("Couldn't create cert: %v", err)
 			}
@@ -174,12 +180,8 @@ func TestWriteCertAndKey(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	caKey, err := rsa.GenerateKey(rand.Reader, 2048)
-	if err != nil {
-		t.Fatalf("Couldn't create rsa Private Key")
-	}
 	caCert := &x509.Certificate{}
-	actual := WriteCertAndKey(tmpdir, "foo", caCert, caKey)
+	actual := WriteCertAndKey(tmpdir, "foo", caCert, rootCAKey)
 	if actual != nil {
 		t.Errorf(
 			"failed WriteCertAndKey with an error: %v",
@@ -227,11 +229,7 @@ func TestWriteKey(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	caKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	actual := WriteKey(tmpdir, "foo", rootCAKey)
-	if err != nil {
-		t.Fatalf("Couldn't create rsa Private Key")
-	}
-	actual := WriteKey(tmpdir, "foo", caKey)
 	if actual != nil {
 		t.Errorf(
 			"failed WriteCertAndKey with an error: %v",
@@ -247,11 +245,7 @@ func TestWritePublicKey(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	caKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	actual := WritePublicKey(tmpdir, "foo", rootCAKey.Public())
-	if err != nil {
-		t.Fatalf("Couldn't create rsa Private Key")
-	}
-	actual := WritePublicKey(tmpdir, "foo", &caKey.PublicKey)
 	if actual != nil {
 		t.Errorf(
 			"failed WriteCertAndKey with an error: %v",
@@ -267,12 +261,8 @@ func TestCertOrKeyExist(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	caKey, err := rsa.GenerateKey(rand.Reader, 2048)
-	if err != nil {
-		t.Fatalf("Couldn't create rsa Private Key")
-	}
 	caCert := &x509.Certificate{}
-	actual := WriteCertAndKey(tmpdir, "foo", caCert, caKey)
+	actual := WriteCertAndKey(tmpdir, "foo", caCert, rootCAKey)
 	if actual != nil {
 		t.Errorf(
 			"failed WriteCertAndKey with an error: %v",
@@ -320,16 +310,7 @@ func TestTryLoadCertAndKeyFromDisk(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	caCert, caKey, err := NewCertificateAuthority(&CertConfig{
+	err = WriteCertAndKey(tmpdir, "foo", rootCACert, rootCAKey)
-		Config: certutil.Config{CommonName: "kubernetes"},
-	})
-	if err != nil {
-		t.Fatalf(
-			"failed to create cert and key with an error: %v",
-			err,
-		)
-	}
-	err = WriteCertAndKey(tmpdir, "foo", caCert, caKey)
 	if err != nil {
 		t.Fatalf(
 			"failed to write cert and key with an error: %v",
@@ -377,16 +358,13 @@ func TestTryLoadCertFromDisk(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	caCert, _, err := NewCertificateAuthority(&CertConfig{
-		Config: certutil.Config{CommonName: "kubernetes"},
-	})
 	if err != nil {
 		t.Fatalf(
 			"failed to create cert and key with an error: %v",
 			err,
 		)
 	}
-	err = WriteCert(tmpdir, "foo", caCert)
+	err = WriteCert(tmpdir, "foo", rootCACert)
 	if err != nil {
 		t.Fatalf(
 			"failed to write cert and key with an error: %v",
@@ -434,29 +412,13 @@ func TestTryLoadCertChainFromDisk(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	caCert, caKey, err := NewCertificateAuthority(&CertConfig{
+	err = WriteCert(tmpdir, "leaf", servCert)
-		Config: certutil.Config{CommonName: "Intermediate CA"},
-	})
-	if err != nil {
-		t.Fatalf("failed to create intermediate CA cert and key with an error: %v", err)
-	}
-
-	cert, _, err := NewCertAndKey(caCert, caKey, &CertConfig{
-		Config: certutil.Config{
-			CommonName: "kubernetes",
-			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-		},
-	})
-	if err != nil {
-		t.Fatalf("failed to create leaf cert and key with an error: %v", err)
-	}
-
-	err = WriteCert(tmpdir, "leaf", cert)
 	if err != nil {
 		t.Fatalf("failed to write cert: %v", err)
 	}
 
-	bundle := []*x509.Certificate{cert, caCert}
+	// rootCACert is treated as an intermediate CA here
+	bundle := []*x509.Certificate{servCert, rootCACert}
 	err = WriteCertBundle(tmpdir, "bundle", bundle)
 	if err != nil {
 		t.Fatalf("failed to write cert bundle: %v", err)
@@ -513,39 +475,32 @@ func TestTryLoadCertChainFromDisk(t *testing.T) {
 }
 
 func TestTryLoadKeyFromDisk(t *testing.T) {
-
 	var tests = []struct {
 		desc       string
 		pathSuffix string
 		name       string
-		keyGenFunc func() (crypto.Signer, error)
+		caKey      crypto.Signer
 		expected   bool
 	}{
 		{
 			desc:       "empty path and name",
 			pathSuffix: "somegarbage",
 			name:       "",
-			keyGenFunc: func() (crypto.Signer, error) {
+			caKey:      rootCAKey,
-				return rsa.GenerateKey(rand.Reader, 2048)
-			},
 			expected:   false,
 		},
 		{
 			desc:       "RSA valid path and name",
 			pathSuffix: "",
 			name:       "foo",
-			keyGenFunc: func() (crypto.Signer, error) {
+			caKey:      rootCAKey,
-				return rsa.GenerateKey(rand.Reader, 2048)
-			},
 			expected:   true,
 		},
 		{
 			desc:       "ECDSA valid path and name",
 			pathSuffix: "",
 			name:       "foo",
-			keyGenFunc: func() (crypto.Signer, error) {
+			caKey:      ecdsaKey,
-				return ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
-			},
 			expected:   true,
 		},
 	}
@@ -557,15 +512,7 @@ func TestTryLoadKeyFromDisk(t *testing.T) {
 			}
 			defer os.RemoveAll(tmpdir)
 
-			caKey, err := rt.keyGenFunc()
+			err = WriteKey(tmpdir, "foo", rt.caKey)
-			if err != nil {
-				t.Errorf(
-					"failed to create key with an error: %v",
-					err,
-				)
-			}
-
-			err = WriteKey(tmpdir, "foo", caKey)
 			if err != nil {
 				t.Errorf(
 					"failed to write key with an error: %v",
@@ -912,23 +859,6 @@ func TestVerifyCertChain(t *testing.T) {
 	}
 	defer os.RemoveAll(tmpdir)
 
-	rootCert1, rootKey1, err := NewCertificateAuthority(&CertConfig{
-		Config: certutil.Config{CommonName: "Root CA 1"},
-	})
-	if err != nil {
-		t.Errorf("failed to create root CA cert and key with an error: %v", err)
-	}
-
-	leafCert1, _, err := NewCertAndKey(rootCert1, rootKey1, &CertConfig{
-		Config: certutil.Config{
-			CommonName: "Leaf Certificate 1",
-			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
-		},
-	})
-	if err != nil {
-		t.Errorf("failed to create leaf cert and key with an error: %v", err)
-	}
-
 	rootCert2, rootKey2, err := NewCertificateAuthority(&CertConfig{
 		Config: certutil.Config{CommonName: "Root CA 2"},
 	})
@@ -965,9 +895,9 @@ func TestVerifyCertChain(t *testing.T) {
 	}{
 		{
 			desc:          "without any intermediate CAs",
-			leaf:          leafCert1,
+			leaf:          servCert,
 			intermediates: []*x509.Certificate{},
-			root:          rootCert1,
+			root:          rootCACert,
 			expected:      true,
 		},
 		{