test(issuing): new test: when req mismatches, cert can't be updated

This new unit test highlights an unexpected behavior of the issuing
controller: the issuing controller is updating the certificate's status
when the certificate request has a failure ("Reason = Failed"), but the
controller might have picked up an out-of-date certificate request.

The consequence is that the issuing controller would set the certificate
to "Issuing = False". That happens when a re-issuance is triggered with
an old failing certificate request.

Signed-off-by: Maël Valais <mael.valais@gmail.com>

pkg/controller/certificates/internal/test/test.go

@@ -72,14 +72,14 @@ type CryptoBundle struct {
 }
 
 func MustCreateCryptoBundle(t *testing.T, crt *cmapi.Certificate, fixedClock *fakeclock.FakeClock) CryptoBundle {
-	c, err := createCryptoBundle(crt, fixedClock)
+	c, err := CreateCryptoBundle(crt, fixedClock)
 	if err != nil {
 		t.Fatalf("error generating crypto bundle: %v", err)
 	}
 	return *c
 }
 
-func createCryptoBundle(originalCert *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{}

pkg/controller/certificates/issuing/issuing_controller_test.go

@@ -174,6 +174,38 @@ func TestIssuingController(t *testing.T) {
 			expectedErr: false,
 		},
 
+		"if certificate is in Issuing state, one CertificateRequest, but has failed and does not match the certificate spec, do nothing": {
+			certificate: exampleBundle.Certificate,
+			builder: &testpkg.Builder{
+				CertManagerObjects: []runtime.Object{
+					issuingCert.DeepCopy(),
+					gen.CertificateRequestFrom(createCertificateRequestOrPanic(gen.CertificateFrom(issuingCert,
+						gen.SetCertificateDNSNames("foo.com"), // Mismatch since the cert has "example.com"
+					)), gen.AddCertificateRequestAnnotations(map[string]string{
+						cmapi.CertificateRequestRevisionAnnotationKey: "2", // Current Certificate revision=1
+					}), gen.SetCertificateRequestStatusCondition(cmapi.CertificateRequestCondition{
+						Type:    cmapi.CertificateRequestConditionReady,
+						Status:  cmmeta.ConditionFalse,
+						Reason:  cmapi.CertificateRequestReasonFailed,
+						Message: "The certificate request failed because of reasons",
+					})),
+				},
+				KubeObjects: []runtime.Object{
+					&corev1.Secret{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      nextPrivateKeySecretName,
+							Namespace: exampleBundle.Certificate.Namespace,
+						},
+						Data: map[string][]byte{
+							corev1.TLSPrivateKeyKey: exampleBundle.PrivateKeyBytes,
+						},
+					},
+				},
+				ExpectedActions: []testpkg.Action{},
+			},
+			expectedErr: false,
+		},
+
 		"if certificate is in Issuing state, one CertificateRequest, but has failed, set failed state and log event": {
 			certificate: exampleBundle.Certificate,
 			builder: &testpkg.Builder{
@@ -932,3 +964,14 @@ func TestIssuingController(t *testing.T) {
 		})
 	}
 }
+
+// We don't need to full bundle, just a simple CertificateRequest. We don't
+// use MustCreateCryptoBundle since it requires *testing.T which we don't
+// want to bother with: a panic is fine here.
+func createCertificateRequestOrPanic(crt *cmapi.Certificate) *cmapi.CertificateRequest {
+	bundle, err := internaltest.CreateCryptoBundle(crt, fakeclock.NewFakeClock(time.Now()))
+	if err != nil {
+		panic(err)
+	}
+	return bundle.CertificateRequest
+}