From 01b5d0fa88b6afd224cbcc7f71a4bdcd7ce6856f Mon Sep 17 00:00:00 2001 From: Richard Wall Date: Wed, 19 Aug 2020 19:22:27 +0100 Subject: [PATCH] Fix tests in ./pkg/controller/certificates/... Signed-off-by: Richard Wall --- pkg/controller/certificates/internal/test/test.go | 8 ++++++-- .../certificates/issuing/issuing_controller.go | 7 +++++-- pkg/controller/certificates/issuing/temporary.go | 5 +++++ .../certificates/requestmanager/util_test.go | 8 ++++++-- pkg/controller/certificates/util.go | 5 +++++ pkg/util/pki/csr.go | 11 +++++++++-- pkg/util/pki/generate.go | 7 ++++++- test/unit/gen/certificate.go | 5 ++++- 8 files changed, 46 insertions(+), 10 deletions(-) diff --git a/pkg/controller/certificates/internal/test/test.go b/pkg/controller/certificates/internal/test/test.go index 57ffcaeb5..3561ec703 100644 --- a/pkg/controller/certificates/internal/test/test.go +++ b/pkg/controller/certificates/internal/test/test.go @@ -79,7 +79,11 @@ func MustCreateCryptoBundle(t *testing.T, crt *cmapi.Certificate, fixedClock *fa return *c } -func createCryptoBundle(crt *cmapi.Certificate, fixedClock *fakeclock.FakeClock) (*CryptoBundle, error) { +func createCryptoBundle(originalCert *cmapi.Certificate, fixedClock *fakeclock.FakeClock) (*CryptoBundle, error) { + crt := originalCert.DeepCopy() + if crt.Spec.PrivateKey == nil { + crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{} + } reqName, err := apiutil.ComputeCertificateRequestName(crt) if err != nil { return nil, err @@ -179,7 +183,7 @@ func createCryptoBundle(crt *cmapi.Certificate, fixedClock *fakeclock.FakeClock) } return &CryptoBundle{ - Certificate: crt, + Certificate: originalCert, ExpectedRequestName: reqName, PrivateKey: privateKey, PrivateKeyBytes: privateKeyBytes, diff --git a/pkg/controller/certificates/issuing/issuing_controller.go b/pkg/controller/certificates/issuing/issuing_controller.go index c6a05827d..a97277f35 100644 --- a/pkg/controller/certificates/issuing/issuing_controller.go +++ b/pkg/controller/certificates/issuing/issuing_controller.go @@ -311,6 +311,11 @@ func (c *controller) failIssueCertificate(ctx context.Context, log logr.Logger, // certificate, and then store the certificate, CA and private key into the // Secret in the appropriate format type. func (c *controller) issueCertificate(ctx context.Context, nextRevision int, crt *cmapi.Certificate, req *cmapi.CertificateRequest, pk crypto.Signer) error { + crt = crt.DeepCopy() + if crt.Spec.PrivateKey == nil { + crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{} + } + pkData, err := utilpki.EncodePrivateKey(pk, crt.Spec.PrivateKey.Encoding) if err != nil { return err @@ -326,8 +331,6 @@ func (c *controller) issueCertificate(ctx context.Context, nextRevision int, crt return err } - crt = crt.DeepCopy() - //Set status.revision to revision of the CertificateRequest crt.Status.Revision = &nextRevision diff --git a/pkg/controller/certificates/issuing/temporary.go b/pkg/controller/certificates/issuing/temporary.go index 9f48d312c..e9b4293fe 100644 --- a/pkg/controller/certificates/issuing/temporary.go +++ b/pkg/controller/certificates/issuing/temporary.go @@ -42,6 +42,11 @@ var temporaryCertificatePolicyChain = policies.Chain{ // - If the Certificate/Key pair does not match the 'NextPrivateKey' // Returns true is a temporary certificate was issued func (c *controller) ensureTemporaryCertificate(ctx context.Context, crt *cmapi.Certificate, pk crypto.Signer) (bool, error) { + crt = crt.DeepCopy() + if crt.Spec.PrivateKey == nil { + crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{} + } + // If certificate does not have temporary certificate annotation, do nothing if !certificateHasTemporaryCertificateAnnotation(crt) { return false, nil diff --git a/pkg/controller/certificates/requestmanager/util_test.go b/pkg/controller/certificates/requestmanager/util_test.go index c72c28997..7b7e7a1ed 100644 --- a/pkg/controller/certificates/requestmanager/util_test.go +++ b/pkg/controller/certificates/requestmanager/util_test.go @@ -68,7 +68,11 @@ func mustCreateCryptoBundle(t *testing.T, crt *cmapi.Certificate) cryptoBundle { return *c } -func createCryptoBundle(crt *cmapi.Certificate) (*cryptoBundle, error) { +func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error) { + crt := originalCert.DeepCopy() + if crt.Spec.PrivateKey == nil { + crt.Spec.PrivateKey = &cmapi.CertificatePrivateKey{} + } reqName, err := apiutil.ComputeCertificateRequestName(crt) if err != nil { return nil, err @@ -160,7 +164,7 @@ func createCryptoBundle(crt *cmapi.Certificate) (*cryptoBundle, error) { ) return &cryptoBundle{ - certificate: crt, + certificate: originalCert, expectedRequestName: reqName, privateKey: privateKey, privateKeyBytes: privateKeyBytes, diff --git a/pkg/controller/certificates/util.go b/pkg/controller/certificates/util.go index 9ceaf4717..078060db4 100644 --- a/pkg/controller/certificates/util.go +++ b/pkg/controller/certificates/util.go @@ -29,11 +29,16 @@ import ( "k8s.io/apimachinery/pkg/util/sets" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + v1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" "github.com/jetstack/cert-manager/pkg/util" "github.com/jetstack/cert-manager/pkg/util/pki" ) func PrivateKeyMatchesSpec(pk crypto.PrivateKey, spec cmapi.CertificateSpec) ([]string, error) { + spec = *spec.DeepCopy() + if spec.PrivateKey == nil { + spec.PrivateKey = &v1.CertificatePrivateKey{} + } switch spec.PrivateKey.Algorithm { case "", cmapi.RSAKeyAlgorithm: return rsaPrivateKeyMatchesSpec(pk, spec) diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index f1d8833f4..dc653cbec 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -32,7 +32,7 @@ import ( "time" apiutil "github.com/jetstack/cert-manager/pkg/api/util" - "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + v1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" ) func IPAddressesForCertificate(crt *v1.Certificate) []net.IP { @@ -127,6 +127,9 @@ const defaultOrganization = "cert-manager" // Certificate resource. // If an Organization is not specifically set, a default will be used. func OrganizationForCertificate(crt *v1.Certificate) []string { + if crt.Spec.Subject == nil { + return nil + } return crt.Spec.Subject.Organizations } @@ -445,7 +448,11 @@ func EncodeX509Chain(certs []*x509.Certificate) ([]byte, error) { func SignatureAlgorithm(crt *v1.Certificate) (x509.PublicKeyAlgorithm, x509.SignatureAlgorithm, error) { var sigAlgo x509.SignatureAlgorithm var pubKeyAlgo x509.PublicKeyAlgorithm - switch crt.Spec.PrivateKey.Algorithm { + var specAlgorithm v1.PrivateKeyAlgorithm + if crt.Spec.PrivateKey != nil { + specAlgorithm = crt.Spec.PrivateKey.Algorithm + } + switch specAlgorithm { case v1.PrivateKeyAlgorithm(""): // If keyAlgorithm is not specified, we default to rsa with keysize 2048 pubKeyAlgo = x509.RSA diff --git a/pkg/util/pki/generate.go b/pkg/util/pki/generate.go index 0728721eb..3654483c3 100644 --- a/pkg/util/pki/generate.go +++ b/pkg/util/pki/generate.go @@ -25,7 +25,8 @@ import ( "crypto/x509" "encoding/pem" "fmt" - "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + + v1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" ) const ( @@ -50,6 +51,10 @@ const ( // parameters on the provided resource. // The returned key will either be RSA or ECDSA. func GeneratePrivateKeyForCertificate(crt *v1.Certificate) (crypto.Signer, error) { + crt = crt.DeepCopy() + if crt.Spec.PrivateKey == nil { + crt.Spec.PrivateKey = &v1.CertificatePrivateKey{} + } switch crt.Spec.PrivateKey.Algorithm { case v1.PrivateKeyAlgorithm(""), v1.RSAKeyAlgorithm: keySize := MinRSAKeySize diff --git a/test/unit/gen/certificate.go b/test/unit/gen/certificate.go index baf65f0d1..3ece33649 100644 --- a/test/unit/gen/certificate.go +++ b/test/unit/gen/certificate.go @@ -22,7 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" + v1 "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" ) @@ -31,6 +31,9 @@ type CertificateModifier func(*v1.Certificate) func Certificate(name string, mods ...CertificateModifier) *v1.Certificate { c := &v1.Certificate{ ObjectMeta: ObjectMeta(name), + Spec: v1.CertificateSpec{ + PrivateKey: &v1.CertificatePrivateKey{}, + }, } for _, mod := range mods { mod(c)