Merge pull request #2061 from munnerz/http01-annotation-overrides

Add annotations to allow overriding HTTP01 ingress class and name

pkg/apis/certmanager/v1alpha1/types.go

@@ -29,6 +29,21 @@ const (
 // Annotation names for CertificateRequests
 const (
 	CRPrivateKeyAnnotationKey = "certmanager.k8s.io/private-key-secret-name"
+
+	// If this annotation is specified on a Certificate or Order resource when
+	// using the HTTP01 solver type, the ingress.name field of the HTTP01
+	// solver's configuration will be set to the value given here.
+	// This is especially useful for users of Ingress controllers that maintain
+	// a 1:1 mapping between endpoint IP and Ingress resource.
+	ACMECertificateHTTP01IngressNameOverride = "acme.certmanager.k8s.io/http01-override-ingress-name"
+
+	// If this annotation is specified on a Certificate or Order resource when
+	// using the HTTP01 solver type, the ingress.class field of the HTTP01
+	// solver's configuration will be set to the value given here.
+	// This is especially useful for users deploying many different ingress
+	// classes into a single cluster that want to be able to re-use a single
+	// solver for each ingress class.
+	ACMECertificateHTTP01IngressClassOverride = "acme.certmanager.k8s.io/http01-override-ingress-class"
 )
 
 // ConditionStatus represents a condition's status.

pkg/controller/acmeorders/BUILD.bazel

@@ -60,7 +60,10 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["sync_test.go"],
+    srcs = [
+        "sync_test.go",
+        "util_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//pkg/acme/client:go_default_library",
@@ -74,9 +77,10 @@ go_test(
         "//vendor/github.com/kr/pretty:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//vendor/k8s.io/client-go/testing:go_default_library",
         "//vendor/k8s.io/component-base/featuregate/testing:go_default_library",
         "//vendor/k8s.io/utils/clock/testing:go_default_library",
+        "//vendor/k8s.io/utils/diff:go_default_library",
+        "//vendor/k8s.io/utils/pointer:go_default_library",
     ],
 )

pkg/controller/acmeorders/util.go

@@ -280,7 +280,13 @@ func challengeSpecForAuthorization(ctx context.Context, cl acmecl.Interface, iss
 		return nil, err
 	}
 
-	// 4. construct Challenge resource with spec.solver field set
+	// 4. handle overriding the HTTP01 ingress class and name fields using the
+	//    ACMECertificateHTTP01IngressNameOverride & Class annotations
+	if err := applyIngressParameterAnnotationOverrides(o, selectedSolver); err != nil {
+		return nil, err
+	}
+
+	// 5. construct Challenge resource with spec.solver field set
 	return &cmapi.ChallengeSpec{
 		AuthzURL:  authz.URL,
 		Type:      selectedChallenge.Type,
@@ -294,6 +300,32 @@ func challengeSpecForAuthorization(ctx context.Context, cl acmecl.Interface, iss
 	}, nil
 }
 
+func applyIngressParameterAnnotationOverrides(o *cmapi.Order, s *cmapi.ACMEChallengeSolver) error {
+	if s.HTTP01 == nil || s.HTTP01.Ingress == nil || o.Annotations == nil {
+		return nil
+	}
+
+	manualIngressName, hasManualIngressName := o.Annotations[cmapi.ACMECertificateHTTP01IngressNameOverride]
+	manualIngressClass, hasManualIngressClass := o.Annotations[cmapi.ACMECertificateHTTP01IngressClassOverride]
+	// don't allow both override annotations to be specified at once
+	if hasManualIngressName && hasManualIngressClass {
+		return fmt.Errorf("both ingress name and ingress class overrides specified - only one may be specified at a time")
+	}
+	// if an override annotation is specified, clear out the existing solver
+	// config
+	if hasManualIngressClass || hasManualIngressName {
+		s.HTTP01.Ingress.Class = nil
+		s.HTTP01.Ingress.Name = ""
+	}
+	if hasManualIngressName {
+		s.HTTP01.Ingress.Name = manualIngressName
+	}
+	if hasManualIngressClass {
+		s.HTTP01.Ingress.Class = &manualIngressClass
+	}
+	return nil
+}
+
 func keyForChallenge(cl acmecl.Interface, challenge *cmapi.ACMEChallenge) (string, error) {
 	var err error
 	switch challenge.Type {

pkg/controller/ingress-shim/sync.go

@@ -326,7 +326,25 @@ func (c *controller) setIssuerSpecificConfig(crt *v1alpha1.Certificate, issuer v
 	if ingAnnotations == nil {
 		ingAnnotations = map[string]string{}
 	}
+
 	// for ACME issuers
+	editInPlaceVal, _ := ingAnnotations[editInPlaceAnnotation]
+	editInPlace := editInPlaceVal == "true"
+	if editInPlace {
+		if crt.Annotations == nil {
+			crt.Annotations = make(map[string]string)
+		}
+		crt.Annotations[v1alpha1.ACMECertificateHTTP01IngressNameOverride] = ing.Name
+	}
+
+	ingressClassVal, hasIngressClassVal := ingAnnotations[acmeIssuerHTTP01IngressClassAnnotation]
+	if hasIngressClassVal {
+		if crt.Annotations == nil {
+			crt.Annotations = make(map[string]string)
+		}
+		crt.Annotations[v1alpha1.ACMECertificateHTTP01IngressClassOverride] = ingressClassVal
+	}
+
 	if issuer.GetSpec().ACME != nil {
 		challengeType, ok := ingAnnotations[acmeIssuerChallengeTypeAnnotation]
 		if !ok {

pkg/controller/ingress-shim/sync_test.go

@@ -148,6 +148,9 @@ func TestSync(t *testing.T) {
 						Labels: map[string]string{
 							"my-test-label": "should be copied",
 						},
+						Annotations: map[string]string{
+							v1alpha1.ACMECertificateHTTP01IngressNameOverride: "ingress-name",
+						},
 						OwnerReferences: buildOwnerReferences("ingress-name", gen.DefaultTestNamespace),
 					},
 					Spec: v1alpha1.CertificateSpec{
@@ -173,6 +176,56 @@ func TestSync(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name:   "create a Certificate with the HTTP01 name override if the given ingress uses http01 annotations",
+			Issuer: gen.Issuer(acmeIssuer.Name),
+			Ingress: &extv1beta1.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "ingress-name",
+					Namespace: gen.DefaultTestNamespace,
+					Labels: map[string]string{
+						"my-test-label": "should be copied",
+					},
+					Annotations: map[string]string{
+						clusterIssuerNameAnnotation: "issuer-name",
+						editInPlaceAnnotation:       "true",
+					},
+					UID: types.UID("ingress-name"),
+				},
+				Spec: extv1beta1.IngressSpec{
+					TLS: []extv1beta1.IngressTLS{
+						{
+							Hosts:      []string{"example.com", "www.example.com"},
+							SecretName: "example-com-tls",
+						},
+					},
+				},
+			},
+			ClusterIssuerLister: []runtime.Object{acmeClusterIssuer},
+			ExpectedCreate: []*v1alpha1.Certificate{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "example-com-tls",
+						Namespace: gen.DefaultTestNamespace,
+						Labels: map[string]string{
+							"my-test-label": "should be copied",
+						},
+						Annotations: map[string]string{
+							v1alpha1.ACMECertificateHTTP01IngressNameOverride: "ingress-name",
+						},
+						OwnerReferences: buildOwnerReferences("ingress-name", gen.DefaultTestNamespace),
+					},
+					Spec: v1alpha1.CertificateSpec{
+						DNSNames:   []string{"example.com", "www.example.com"},
+						SecretName: "example-com-tls",
+						IssuerRef: v1alpha1.ObjectReference{
+							Name: "issuer-name",
+							Kind: "ClusterIssuer",
+						},
+					},
+				},
+			},
+		},
 		{
 			Name:   "return a single HTTP01 Certificate for an ingress with a single valid TLS entry and HTTP01 annotations with no ingress class set",
 			Issuer: acmeClusterIssuer,
@@ -309,6 +362,9 @@ func TestSync(t *testing.T) {
 						Name:            "example-com-tls",
 						Namespace:       gen.DefaultTestNamespace,
 						OwnerReferences: buildOwnerReferences("ingress-name", gen.DefaultTestNamespace),
+						Annotations: map[string]string{
+							v1alpha1.ACMECertificateHTTP01IngressClassOverride: "cert-ing",
+						},
 					},
 					Spec: v1alpha1.CertificateSpec{
 						DNSNames:   []string{"example.com", "www.example.com"},