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
could only be "added" by deleting the Certificate itself.

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",