Update Certificates with Ingress annotations

This updates the annotations of a Certificate owned by an Ingress when
they are added or changed after the Certificate exists. Before they were
only updated when an unrelated change, like a changed label value,
triggered an update.

Also removing the call to `setIssuerSpecificConfig()` as this is already
done before the `existingCrt` check.

Fixes cert-manager#6065

Signed-off-by: Tobi Nehrlich <tobi.nehrlich@amazee.io>

pkg/controller/certificate-shim/sync.go

@@ -431,14 +431,12 @@ func buildCertificates(
 
 			updateCrt := existingCrt.DeepCopy()
 
-			updateCrt.Spec = crt.Spec
+			updateCrt.Annotations = crt.Annotations
 			updateCrt.Labels = crt.Labels
-
-			setIssuerSpecificConfig(crt, ingLike)
+			updateCrt.Spec = crt.Spec
 
 			updateCrts = append(updateCrts, updateCrt)
 		} else {
-
 			newCrts = append(newCrts, crt)
 		}
 	}
@@ -489,9 +487,13 @@ func certNeedsUpdate(a, b *cmapi.Certificate) bool {
 	}
 
 	// TODO: we may need to allow users to edit the managed Certificate resources
-	// to add their own labels directly.
-	// Right now, we'll reset/remove the label values back automatically.
+	// to add their own annotations and labels directly.
+	// Right now, we'll reset/remove the annotation and label values back automatically.
 	// Let's hope no other controllers do this automatically, else we'll start fighting...
+	if !reflect.DeepEqual(a.Annotations, b.Annotations) {
+		return true
+	}
+
 	if !reflect.DeepEqual(a.Labels, b.Labels) {
 		return true
 	}

pkg/controller/certificate-shim/sync_test.go

@@ -70,6 +70,79 @@ func Test_hasShimAnnotation(t *testing.T) {
 	})
 }
 
+func Test_certNeedsUpdate(t *testing.T) {
+	type testT struct {
+		CertA *cmapi.Certificate
+		CertB *cmapi.Certificate
+		Want  bool
+	}
+
+	t.Run("ingress", func(t *testing.T) {
+		tests := []testT{
+			{
+				CertA: &cmapi.Certificate{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example-com-tls",
+						Namespace: gen.DefaultTestNamespace,
+					},
+				},
+				CertB: &cmapi.Certificate{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example-com-tls",
+						Namespace: gen.DefaultTestNamespace,
+					},
+				},
+				Want: false,
+			},
+			{
+				CertA: &cmapi.Certificate{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example-com-tls",
+						Namespace: gen.DefaultTestNamespace,
+						Labels: map[string]string{
+							"changing-label": "will be updated",
+						},
+					},
+				},
+				CertB: &cmapi.Certificate{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example-com-tls",
+						Namespace: gen.DefaultTestNamespace,
+						Labels: map[string]string{
+							"changing-label": "is updated",
+						},
+					},
+				},
+				Want: true,
+			},
+			{
+				CertA: &cmapi.Certificate{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example-com-tls",
+						Namespace: gen.DefaultTestNamespace,
+					},
+				},
+				CertB: &cmapi.Certificate{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example-com-tls",
+						Namespace: gen.DefaultTestNamespace,
+						Annotations: map[string]string{
+							cmapi.IngressACMEIssuerHTTP01IngressClassAnnotationKey: "nginx",
+						},
+					},
+				},
+				Want: true,
+			},
+		}
+		for _, test := range tests {
+			certNeedsUpdate := certNeedsUpdate(test.CertA, test.CertB)
+			if certNeedsUpdate != test.Want {
+				t.Errorf("Expected certNeedsUpdate=%v for certificates \n%#v\nand\n%#v", test.Want, test.CertA, test.CertB)
+			}
+		}
+	})
+}
+
 func TestSync(t *testing.T) {
 	clusterIssuer := gen.ClusterIssuer("issuer-name")
 	acmeIssuerNewFormat := gen.Issuer("issuer-name",