Merge pull request #7100 from inteon/cleanup_test_helpers

Cleanup test helpers by increasing usage of gen package.

internal/apis/certmanager/validation/certificaterequest_test.go

@@ -17,11 +17,9 @@ limitations under the License.
 package validation
 
 import (
-	"bytes"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/asn1"
-	"encoding/pem"
 	"reflect"
 	"testing"
 
@@ -32,7 +30,6 @@ import (
 	cminternal "github.com/cert-manager/cert-manager/internal/apis/certmanager"
 	cminternalmeta "github.com/cert-manager/cert-manager/internal/apis/meta"
 	cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
-	"github.com/cert-manager/cert-manager/pkg/util/pki"
 	utilpki "github.com/cert-manager/cert-manager/pkg/util/pki"
 	"github.com/cert-manager/cert-manager/test/unit/gen"
 )
@@ -572,10 +569,12 @@ func TestValidateCertificateRequest(t *testing.T) {
 			cr: &cminternal.CertificateRequest{
 				Spec: cminternal.CertificateRequestSpec{
 					// mustGenerateCSR will set the default usages for us
-					Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) {
+					Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) error {
 						// manually remove extensions that encode default usages
 						cr.Extensions = nil
 						cr.ExtraExtensions = nil
+
+						return nil
 					}),
 					IssuerRef: validIssuerRef,
 					Usages:    []cminternal.KeyUsage{cminternal.UsageKeyEncipherment, cminternal.UsageDigitalSignature},
@@ -588,12 +587,12 @@ func TestValidateCertificateRequest(t *testing.T) {
 			cr: &cminternal.CertificateRequest{
 				Spec: cminternal.CertificateRequestSpec{
 					// mustGenerateCSR will set the default usages for us
-					Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) {
+					Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) error {
 						// manually remove extensions that encode default usages
 						cr.Extensions = nil
 						cr.ExtraExtensions = []pkix.Extension{
 							{
-								Id:       pki.OIDExtensionKeyUsage,
+								Id:       utilpki.OIDExtensionKeyUsage,
 								Critical: false,
 								Value: func(t *testing.T) []byte {
 									asn1KeyUsage, err := asn1.Marshal(asn1.BitString{Bytes: []byte{}, BitLength: 0})
@@ -605,6 +604,8 @@ func TestValidateCertificateRequest(t *testing.T) {
 								}(t),
 							},
 						}
+
+						return nil
 					}),
 					IssuerRef: validIssuerRef,
 					Usages:    []cminternal.KeyUsage{cminternal.UsageKeyEncipherment, cminternal.UsageDigitalSignature},
@@ -877,30 +878,10 @@ func TestValidateCertificateRequest(t *testing.T) {
 	}
 }
 
-func mustGenerateCSR(t *testing.T, crt *cmapi.Certificate, modifiers ...func(*x509.CertificateRequest)) []byte {
-	// Create a new private key
-	pk, err := utilpki.GenerateRSAPrivateKey(2048)
+func mustGenerateCSR(t *testing.T, crt *cmapi.Certificate, modifiers ...gen.CSRModifier) []byte {
+	csrPEM, _, err := gen.CSRForCertificate(crt, modifiers...)
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	x509CSR, err := utilpki.GenerateCSR(crt)
-	if err != nil {
-		t.Fatal(err)
-	}
-	for _, modifier := range modifiers {
-		modifier(x509CSR)
-	}
-	csrDER, err := utilpki.EncodeCSR(x509CSR, pk)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	csrPEM := bytes.NewBuffer([]byte{})
-	err = pem.Encode(csrPEM, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	return csrPEM.Bytes()
+	return csrPEM
 }

pkg/controller/certificates/requestmanager/util_test.go

@@ -19,7 +19,6 @@ package requestmanager
 import (
 	"crypto"
 	"crypto/x509"
-	"encoding/pem"
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -77,7 +76,7 @@ func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error)
 		return nil, err
 	}
 
-	privateKey, err := pki.GeneratePrivateKeyForCertificate(crt)
+	csrPEM, privateKey, err := gen.CSRForCertificate(crt)
 	if err != nil {
 		return nil, err
 	}
@@ -87,11 +86,6 @@ func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error)
 		return nil, err
 	}
 
-	csrPEM, err := generateCSRImpl(crt, privateKeyBytes)
-	if err != nil {
-		return nil, err
-	}
-
 	csr, err := pki.DecodeX509CertificateRequestBytes(csrPEM)
 	if err != nil {
 		return nil, err
@@ -173,26 +167,3 @@ func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error)
 		certBytes:                              certBytes,
 	}, nil
 }
-
-func generateCSRImpl(crt *cmapi.Certificate, pk []byte) ([]byte, error) {
-	csr, err := pki.GenerateCSR(crt)
-	if err != nil {
-		return nil, err
-	}
-
-	signer, err := pki.DecodePrivateKeyBytes(pk)
-	if err != nil {
-		return nil, err
-	}
-
-	csrDER, err := pki.EncodeCSR(csr, signer)
-	if err != nil {
-		return nil, err
-	}
-
-	csrPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrDER,
-	})
-
-	return csrPEM, nil
-}

pkg/util/pki/match_test.go

@@ -14,14 +14,12 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package pki
+package pki_test
 
 import (
-	"bytes"
 	"crypto"
 	"crypto/x509"
 	"encoding/asn1"
-	"encoding/pem"
 	"reflect"
 	"testing"
 
@@ -29,10 +27,12 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
+	"github.com/cert-manager/cert-manager/pkg/util/pki"
+	"github.com/cert-manager/cert-manager/test/unit/gen"
 )
 
 func mustGenerateRSA(t *testing.T, keySize int) crypto.PrivateKey {
-	pk, err := GenerateRSAPrivateKey(keySize)
+	pk, err := pki.GenerateRSAPrivateKey(keySize)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -40,7 +40,7 @@ func mustGenerateRSA(t *testing.T, keySize int) crypto.PrivateKey {
 }
 
 func mustGenerateECDSA(t *testing.T, keySize int) crypto.PrivateKey {
-	pk, err := GenerateECPrivateKey(keySize)
+	pk, err := pki.GenerateECPrivateKey(keySize)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -48,7 +48,7 @@ func mustGenerateECDSA(t *testing.T, keySize int) crypto.PrivateKey {
 }
 
 func mustGenerateEd25519(t *testing.T) crypto.PrivateKey {
-	pk, err := GenerateEd25519PrivateKey()
+	pk, err := pki.GenerateEd25519PrivateKey()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -75,18 +75,18 @@ func TestPrivateKeyMatchesSpec(t *testing.T) {
 			violations:   []string{"spec.privateKey.size"},
 		},
 		"should match if keySize and algorithm are correct (ECDSA)": {
-			key:          mustGenerateECDSA(t, ECCurve256),
+			key:          mustGenerateECDSA(t, pki.ECCurve256),
 			expectedAlgo: cmapi.ECDSAKeyAlgorithm,
 			expectedSize: 256,
 		},
 		"should not match if ECDSA keySize is incorrect": {
-			key:          mustGenerateECDSA(t, ECCurve256),
+			key:          mustGenerateECDSA(t, pki.ECCurve256),
 			expectedAlgo: cmapi.ECDSAKeyAlgorithm,
-			expectedSize: ECCurve521,
+			expectedSize: pki.ECCurve521,
 			violations:   []string{"spec.privateKey.size"},
 		},
 		"should not match if keyAlgorithm is incorrect": {
-			key:          mustGenerateECDSA(t, ECCurve256),
+			key:          mustGenerateECDSA(t, pki.ECCurve256),
 			expectedAlgo: cmapi.RSAKeyAlgorithm,
 			expectedSize: 2048,
 			violations:   []string{"spec.privateKey.algorithm"},
@@ -98,7 +98,7 @@ func TestPrivateKeyMatchesSpec(t *testing.T) {
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			violations, err := PrivateKeyMatchesSpec(
+			violations, err := pki.PrivateKeyMatchesSpec(
 				test.key,
 				cmapi.CertificateSpec{
 					PrivateKey: &cmapi.CertificatePrivateKey{
@@ -132,7 +132,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 		violations []string
 	}{
 		"should not report any violation if Certificate otherName(s) match the CertificateRequest's": {
-			crSpec: MustBuildCertificateRequest(&cmapi.Certificate{Spec: cmapi.CertificateSpec{
+			crSpec: mustBuildCertificateRequest(t, &cmapi.Certificate{Spec: cmapi.CertificateSpec{
 				CommonName: "cn",
 				OtherNames: []cmapi.OtherName{
 					{
@@ -140,7 +140,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 						UTF8Value: "upn@testdomain.local",
 					},
 				},
-			}}, t),
+			}}),
 			certSpec: cmapi.CertificateSpec{
 				CommonName: "cn",
 				OtherNames: []cmapi.OtherName{
@@ -153,7 +153,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 			err: "",
 		},
 		"should report violation if Certificate otherName(s) mismatch the CertificateRequest's": {
-			crSpec: MustBuildCertificateRequest(&cmapi.Certificate{Spec: cmapi.CertificateSpec{
+			crSpec: mustBuildCertificateRequest(t, &cmapi.Certificate{Spec: cmapi.CertificateSpec{
 				CommonName: "cn",
 				OtherNames: []cmapi.OtherName{
 					{
@@ -161,7 +161,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 						UTF8Value: "upn@testdomain.local",
 					},
 				},
-			}}, t),
+			}}),
 			certSpec: cmapi.CertificateSpec{
 				CommonName: "cn",
 				OtherNames: []cmapi.OtherName{
@@ -177,7 +177,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 			},
 		},
 		"should not report violation if Certificate otherName(s) match the CertificateRequest's (with different order)": {
-			crSpec: MustBuildCertificateRequest(&cmapi.Certificate{Spec: cmapi.CertificateSpec{
+			crSpec: mustBuildCertificateRequest(t, &cmapi.Certificate{Spec: cmapi.CertificateSpec{
 				CommonName: "cn",
 				OtherNames: []cmapi.OtherName{
 					{
@@ -189,7 +189,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 						UTF8Value: "upn@testdomain.local",
 					},
 				},
-			}}, t),
+			}}),
 			certSpec: cmapi.CertificateSpec{
 				CommonName: "cn",
 				OtherNames: []cmapi.OtherName{
@@ -208,7 +208,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			violations, err := RequestMatchesSpec(test.crSpec, test.certSpec)
+			violations, err := pki.RequestMatchesSpec(test.crSpec, test.certSpec)
 			if err != nil {
 				if test.err == "" {
 					t.Errorf("Unexpected error: %s", err.Error())
@@ -226,12 +226,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) {
 
 func TestRequestMatchesSpecSubject(t *testing.T) {
 	createCSRBlob := func(literalSubject string) []byte {
-		pk, err := GenerateRSAPrivateKey(2048)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		seq, err := UnmarshalSubjectStringToRDNSequence(literalSubject)
+		seq, err := pki.UnmarshalSubjectStringToRDNSequence(literalSubject)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -241,16 +236,15 @@ func TestRequestMatchesSpecSubject(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		csr := &x509.CertificateRequest{
-			RawSubject: asn1Seq,
-		}
-
-		csrBytes, err := x509.CreateCertificateRequest(bytes.NewBuffer(nil), csr, pk)
+		pemBytes, _, err := gen.CSR(x509.Ed25519, func(cr *x509.CertificateRequest) error {
+			cr.RawSubject = asn1Seq
+			return nil
+		})
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes})
+		return pemBytes
 	}
 
 	tests := []struct {
@@ -282,7 +276,7 @@ func TestRequestMatchesSpecSubject(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			violations, err := RequestMatchesSpec(
+			violations, err := pki.RequestMatchesSpec(
 				&cmapi.CertificateRequest{
 					Spec: cmapi.CertificateRequestSpec{
 						Request: test.x509CSR,
@@ -442,7 +436,7 @@ func TestSecretDataAltNamesMatchSpec(t *testing.T) {
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			violations, err := SecretDataAltNamesMatchSpec(&corev1.Secret{Data: map[string][]byte{corev1.TLSCertKey: test.data}}, test.spec)
+			violations, err := pki.SecretDataAltNamesMatchSpec(&corev1.Secret{Data: map[string][]byte{corev1.TLSCertKey: test.data}}, test.spec)
 			switch {
 			case err != nil:
 				if test.err != err.Error() {
@@ -461,17 +455,17 @@ func TestSecretDataAltNamesMatchSpec(t *testing.T) {
 }
 
 func selfSignCertificate(t *testing.T, spec cmapi.CertificateSpec) []byte {
-	pk, err := GenerateRSAPrivateKey(2048)
+	pk, err := pki.GenerateRSAPrivateKey(2048)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	template, err := CertificateTemplateFromCertificate(&cmapi.Certificate{Spec: spec})
+	template, err := pki.CertificateTemplateFromCertificate(&cmapi.Certificate{Spec: spec})
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	pemData, _, err := SignCertificate(template, template, pk.Public(), pk)
+	pemData, _, err := pki.SignCertificate(template, template, pk.Public(), pk)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -479,23 +473,12 @@ func selfSignCertificate(t *testing.T, spec cmapi.CertificateSpec) []byte {
 	return pemData
 }
 
-func MustBuildCertificateRequest(crt *cmapi.Certificate, t *testing.T) *cmapi.CertificateRequest {
-	pk, err := GenerateRSAPrivateKey(2048)
+func mustBuildCertificateRequest(t *testing.T, crt *cmapi.Certificate) *cmapi.CertificateRequest {
+	pemData, _, err := gen.CSRForCertificate(crt)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	csrTemplate, err := GenerateCSR(crt, WithOtherNames(true))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	var buffer bytes.Buffer
-	csr, err := x509.CreateCertificateRequest(&buffer, csrTemplate, pk)
-	if err != nil {
-		t.Fatal(err)
-	}
-	pemData := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csr})
 	cr := &cmapi.CertificateRequest{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        t.Name(),

test/e2e/suite/issuers/acme/certificaterequest/dns01.go

@@ -116,7 +116,7 @@ func testRFC2136DNSProvider() bool {
 
 			crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
-			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 				[]string{dnsDomain}, nil, nil, x509.RSA)
 			Expect(err).NotTo(HaveOccurred())
 
@@ -129,7 +129,7 @@ func testRFC2136DNSProvider() bool {
 		It("should obtain a signed certificate for a wildcard domain", func() {
 			By("Creating a CertificateRequest")
 
-			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 				[]string{"*." + dnsDomain}, nil, nil, x509.RSA)
 			Expect(err).NotTo(HaveOccurred())
 
@@ -142,7 +142,7 @@ func testRFC2136DNSProvider() bool {
 		It("should obtain a signed certificate for a wildcard and apex domain", func() {
 			By("Creating a CertificateRequest")
 
-			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 				[]string{"*." + dnsDomain, dnsDomain}, nil, nil, x509.RSA)
 			Expect(err).NotTo(HaveOccurred())
 

test/e2e/suite/issuers/acme/certificaterequest/http01.go

@@ -125,7 +125,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func()
 		crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
 		By("Creating a CertificateRequest")
-		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 			[]string{acmeIngressDomain}, nil, nil, x509.RSA)
 		Expect(err).NotTo(HaveOccurred())
 
@@ -141,7 +141,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func()
 		crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
 		By("Creating a CertificateRequest")
-		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 			[]string{acmeIngressDomain}, nil, nil, x509.ECDSA)
 		Expect(err).NotTo(HaveOccurred())
 
@@ -158,7 +158,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func()
 		// the maximum length of a single segment of the domain being requested
 		const maxLengthOfDomainSegment = 63
 		By("Creating a CertificateRequest")
-		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 			[]string{
 				acmeIngressDomain,
 				e2eutil.RandomSubdomainLength(acmeIngressDomain, maxLengthOfDomainSegment),
@@ -176,7 +176,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func()
 		crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
 		By("Creating a CertificateRequest")
-		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 			[]string{e2eutil.RandomSubdomain(acmeIngressDomain)},
 			nil, nil, x509.RSA)
 		Expect(err).NotTo(HaveOccurred())
@@ -191,7 +191,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func()
 	It("should fail to obtain a certificate for an invalid ACME dns name", func() {
 		// create test fixture
 		By("Creating a CertificateRequest")
-		cr, _, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+		cr, _, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 			[]string{"google.com"}, nil, nil, x509.RSA)
 		Expect(err).NotTo(HaveOccurred())
 
@@ -210,7 +210,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func()
 		crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
 		By("Creating a CertificateRequest")
-		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil,
+		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil,
 			[]string{acmeIngressDomain}, nil, nil, x509.RSA)
 		Expect(err).NotTo(HaveOccurred())
 

test/e2e/suite/issuers/ca/certificaterequest.go

@@ -92,7 +92,7 @@ var _ = framework.CertManagerDescribe("CA CertificateRequest", func() {
 			certRequestClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
 			By("Creating a CertificateRequest")
-			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind,
+			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind,
 				&metav1.Duration{
 					Duration: time.Hour * 24 * 90,
 				},
@@ -109,7 +109,7 @@ var _ = framework.CertManagerDescribe("CA CertificateRequest", func() {
 			certRequestClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
 			By("Creating a CertificateRequest")
-			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind,
+			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind,
 				&metav1.Duration{
 					Duration: time.Hour * 24 * 90,
 				},
@@ -126,7 +126,7 @@ var _ = framework.CertManagerDescribe("CA CertificateRequest", func() {
 			certRequestClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
 			By("Creating a CertificateRequest")
-			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind,
+			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind,
 				&metav1.Duration{
 					Duration: time.Hour * 24 * 90,
 				},

test/e2e/suite/issuers/vault/certificaterequest/approle.go

@@ -153,7 +153,7 @@ func runVaultAppRoleTests(issuerKind string) {
 		Expect(err).NotTo(HaveOccurred())
 
 		By("Creating a CertificateRequest")
-		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, vaultIssuerName, issuerKind,
+		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, vaultIssuerName, issuerKind,
 			&metav1.Duration{
 				Duration: time.Hour * 24 * 90,
 			},
@@ -245,7 +245,7 @@ func runVaultAppRoleTests(issuerKind string) {
 			By("Creating a CertificateRequest")
 			crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name)
 
-			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, vaultIssuerName,
+			cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, vaultIssuerName,
 				issuerKind, v.inputDuration, crDNSNames, crIPAddresses, nil, x509.RSA)
 			Expect(err).NotTo(HaveOccurred())
 			_, err = crClient.Create(ctx, cr, metav1.CreateOptions{})

test/e2e/suite/issuers/venafi/tpp/certificaterequest.go

@@ -83,7 +83,7 @@ var _ = TPPDescribe("CertificateRequest with a properly configured Issuer", func
 
 		dnsNames := []string{rand.String(10) + ".venafi-e2e.example"}
 
-		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuer.Name, cmapi.IssuerKind, nil, dnsNames, nil, nil, x509.RSA)
+		cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuer.Name, cmapi.IssuerKind, nil, dnsNames, nil, nil, x509.RSA)
 		Expect(err).NotTo(HaveOccurred())
 
 		By("Creating a CertificateRequest")

test/e2e/util/util.go

@@ -22,8 +22,6 @@ import (
 	"context"
 	"crypto"
 	"crypto/x509"
-	"crypto/x509/pkix"
-	"encoding/pem"
 	"fmt"
 	"net"
 	"net/url"
@@ -46,7 +44,7 @@ import (
 	cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	clientset "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/typed/certmanager/v1"
 	"github.com/cert-manager/cert-manager/pkg/util"
-	"github.com/cert-manager/cert-manager/pkg/util/pki"
+	"github.com/cert-manager/cert-manager/test/unit/gen"
 )
 
 func CertificateOnlyValidForDomains(cert *x509.Certificate, commonName string, dnsNames ...string) bool {
@@ -163,8 +161,13 @@ func WaitForCRDToNotExist(ctx context.Context, client apiextensionsv1.CustomReso
 }
 
 // Deprecated: use test/unit/gen/CertificateRequest in future
-func NewCertManagerBasicCertificateRequest(name, issuerName string, issuerKind string, duration *metav1.Duration,
-	dnsNames []string, ips []net.IP, uris []string, keyAlgorithm x509.PublicKeyAlgorithm) (*v1.CertificateRequest, crypto.Signer, error) {
+func NewCertManagerBasicCertificateRequest(
+	name, namespace string,
+	issuerName, issuerKind string,
+	duration *metav1.Duration,
+	dnsNames []string, ips []net.IP, uris []string,
+	keyAlgorithm x509.PublicKeyAlgorithm,
+) (*v1.CertificateRequest, crypto.Signer, error) {
 	cn := "test.domain.com"
 	if len(dnsNames) > 0 {
 		cn = dnsNames[0]
@@ -179,68 +182,25 @@ func NewCertManagerBasicCertificateRequest(name, issuerName string, issuerKind s
 		parsedURIs = append(parsedURIs, parsed)
 	}
 
-	var sk crypto.Signer
-	var signatureAlgorithm x509.SignatureAlgorithm
-	var err error
-
-	switch keyAlgorithm {
-	case x509.RSA:
-		sk, err = pki.GenerateRSAPrivateKey(2048)
-		if err != nil {
-			return nil, nil, err
-		}
-		signatureAlgorithm = x509.SHA256WithRSA
-	case x509.ECDSA:
-		sk, err = pki.GenerateECPrivateKey(pki.ECCurve256)
-		if err != nil {
-			return nil, nil, err
-		}
-		signatureAlgorithm = x509.ECDSAWithSHA256
-	case x509.Ed25519:
-		sk, err = pki.GenerateEd25519PrivateKey()
-		if err != nil {
-			return nil, nil, err
-		}
-		signatureAlgorithm = x509.PureEd25519
-	default:
-		return nil, nil, fmt.Errorf("unrecognised key algorithm: %s", err)
-	}
-
-	csr := &x509.CertificateRequest{
-		Version:            0,
-		SignatureAlgorithm: signatureAlgorithm,
-		PublicKeyAlgorithm: keyAlgorithm,
-		PublicKey:          sk.Public(),
-		Subject: pkix.Name{
-			CommonName: cn,
-		},
-		DNSNames:    dnsNames,
-		IPAddresses: ips,
-		URIs:        parsedURIs,
-	}
-
-	csrBytes, err := pki.EncodeCSR(csr, sk)
+	csrPEM, sk, err := gen.CSR(keyAlgorithm,
+		gen.SetCSRCommonName(cn),
+		gen.SetCSRDNSNames(dnsNames...),
+		gen.SetCSRIPAddresses(ips...),
+		gen.SetCSRURIs(parsedURIs...),
+	)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	csrPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrBytes,
-	})
-
-	return &v1.CertificateRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
-		},
-		Spec: v1.CertificateRequestSpec{
-			Duration: duration,
-			Request:  csrPEM,
-			IssuerRef: cmmeta.ObjectReference{
-				Name: issuerName,
-				Kind: issuerKind,
-			},
-		},
-	}, sk, nil
+	return gen.CertificateRequest(name,
+		gen.SetCertificateRequestNamespace(namespace),
+		gen.SetCertificateRequestDuration(duration),
+		gen.SetCertificateRequestCSR(csrPEM),
+		gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{
+			Name: issuerName,
+			Kind: issuerKind,
+		}),
+	), sk, nil
 }
 
 func NewCertManagerVaultCertificate(name, secretName, issuerName string, issuerKind string, duration, renewBefore *metav1.Duration) *v1.Certificate {

test/integration/certificates/issuing_controller_test.go

@@ -148,22 +148,11 @@ func TestIssuingController(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Create x509 CSR from Certificate
-	csr, err := utilpki.GenerateCSR(crt)
+	csrPEM, err := gen.CSRWithSignerForCertificate(crt, sk)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// Encode CSR
-	csrDER, err := utilpki.EncodeCSR(csr, sk)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	csrPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrDER,
-	})
-
 	// Sign Certificate
 	certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt)
 	if err != nil {
@@ -371,22 +360,11 @@ func TestIssuingController_PKCS8_PrivateKey(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Create x509 CSR from Certificate
-	csr, err := utilpki.GenerateCSR(crt)
+	csrPEM, err := gen.CSRWithSignerForCertificate(crt, sk)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// Encode CSR
-	csrDER, err := utilpki.EncodeCSR(csr, sk)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	csrPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrDER,
-	})
-
 	// Sign Certificate
 	certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt)
 	if err != nil {
@@ -589,22 +567,11 @@ func Test_IssuingController_SecretTemplate(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Create x509 CSR from Certificate
-	csr, err := utilpki.GenerateCSR(crt)
+	csrPEM, err := gen.CSRWithSignerForCertificate(crt, sk)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// Encode CSR
-	csrDER, err := utilpki.EncodeCSR(csr, sk)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	csrPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrDER,
-	})
-
 	// Sign Certificate
 	certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt)
 	if err != nil {
@@ -836,22 +803,11 @@ func Test_IssuingController_AdditionalOutputFormats(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Create x509 CSR from Certificate
-	csr, err := utilpki.GenerateCSR(crt)
+	csrPEM, err := gen.CSRWithSignerForCertificate(crt, pk)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// Encode CSR
-	csrDER, err := utilpki.EncodeCSR(csr, pk)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	csrPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrDER,
-	})
-
 	// Sign Certificate
 	certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt)
 	if err != nil {

test/integration/certificates/revisionmanager_controller_test.go

@@ -18,7 +18,6 @@ package certificates
 
 import (
 	"context"
-	"encoding/pem"
 	"strconv"
 	"testing"
 	"time"
@@ -36,7 +35,6 @@ import (
 	"github.com/cert-manager/cert-manager/pkg/controller/certificates/revisionmanager"
 	logf "github.com/cert-manager/cert-manager/pkg/logs"
 	"github.com/cert-manager/cert-manager/pkg/metrics"
-	utilpki "github.com/cert-manager/cert-manager/pkg/util/pki"
 	"github.com/cert-manager/cert-manager/test/unit/gen"
 )
 
@@ -107,27 +105,11 @@ func TestRevisionManagerController(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Create a new private key
-	sk, err := utilpki.GenerateRSAPrivateKey(2048)
+	csrPEM, _, err := gen.CSRForCertificate(crt)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	csr, err := utilpki.GenerateCSR(crt)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// Encode CSR
-	csrDER, err := utilpki.EncodeCSR(csr, sk)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	csrPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrDER,
-	})
-
 	// Create 6 CertificateRequests which are owned by this Certificate
 	for i := 0; i < 6; i++ {
 		_, err = cmCl.CertmanagerV1().CertificateRequests(namespace).Create(ctx, &cmapi.CertificateRequest{

test/integration/validation/certificaterequest_test.go

@@ -18,7 +18,6 @@ package validation
 
 import (
 	"context"
-	"encoding/pem"
 	"strings"
 	"testing"
 	"time"
@@ -33,7 +32,7 @@ import (
 	"github.com/cert-manager/cert-manager/pkg/api"
 	cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
-	"github.com/cert-manager/cert-manager/pkg/util/pki"
+	"github.com/cert-manager/cert-manager/test/unit/gen"
 )
 
 var certGVK = schema.GroupVersionKind{
@@ -200,22 +199,9 @@ func TestValidationCertificateRequests(t *testing.T) {
 }
 
 func mustGenerateCSR(t *testing.T, cert *cmapi.Certificate) []byte {
-	request, err := pki.GenerateCSR(cert)
+	csr, _, err := gen.CSRForCertificate(cert)
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	sk, err := pki.GenerateRSAPrivateKey(2048)
-	if err != nil {
-		t.Fatal(err)
-	}
-	csrBytes, err := pki.EncodeCSR(request, sk)
-	if err != nil {
-		t.Fatal(err)
-	}
-	csr := pem.EncodeToMemory(&pem.Block{
-		Type: "CERTIFICATE REQUEST", Bytes: csrBytes,
-	})
-
 	return csr
 }

test/unit/gen/csr.go

@@ -28,11 +28,57 @@ import (
 	"net"
 	"net/url"
 
+	v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	"github.com/cert-manager/cert-manager/pkg/util/pki"
 )
 
 type CSRModifier func(*x509.CertificateRequest) error
 
+var defaultGenerateCSROptions = []pki.GenerateCSROption{
+	pki.WithEncodeBasicConstraintsInRequest(true),
+	pki.WithNameConstraints(true),
+	pki.WithOtherNames(true),
+	pki.WithUseLiteralSubject(true),
+}
+
+func CSRForCertificate(crt *v1.Certificate, mods ...CSRModifier) (csr []byte, sk crypto.Signer, err error) {
+	cr, err := pki.GenerateCSR(crt, defaultGenerateCSROptions...)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	modifiers := []CSRModifier{}
+	modifiers = append(modifiers, func(c *x509.CertificateRequest) error {
+		*c = *cr
+		return nil
+	})
+	modifiers = append(modifiers, mods...)
+
+	return CSR(cr.PublicKeyAlgorithm, modifiers...)
+}
+
+func CSRWithSignerForCertificate(crt *v1.Certificate, sk crypto.Signer, mods ...CSRModifier) (csr []byte, err error) {
+	cr, err := pki.GenerateCSR(crt, defaultGenerateCSROptions...)
+	if err != nil {
+		return nil, err
+	}
+
+	modifiers := []CSRModifier{}
+	modifiers = append(modifiers, func(c *x509.CertificateRequest) error {
+		if c.PublicKeyAlgorithm != cr.PublicKeyAlgorithm {
+			return fmt.Errorf("public key algorithm mismatch: %s != %s", c.PublicKeyAlgorithm, cr.PublicKeyAlgorithm)
+		}
+		if c.SignatureAlgorithm != cr.SignatureAlgorithm {
+			return fmt.Errorf("signature algorithm mismatch: %s != %s", c.SignatureAlgorithm, cr.SignatureAlgorithm)
+		}
+		*c = *cr
+		return nil
+	})
+	modifiers = append(modifiers, mods...)
+
+	return CSRWithSigner(sk, modifiers...)
+}
+
 func CSR(keyAlgorithm x509.PublicKeyAlgorithm, mods ...CSRModifier) (csr []byte, sk crypto.Signer, err error) {
 	switch keyAlgorithm {
 	case x509.RSA: