Don't use order URL as a pod label. Cleanup existing resources if multiple exist.

pkg/issuer/acme/http/http.go

@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/url"
-	"strings"
 	"time"
 
 	"github.com/golang/glog"
@@ -27,8 +26,9 @@ const (
 	acmeSolverListenPort = 8089
 	// orderURLLabelKey is the key used for the order URL label on resources
 	// created by the HTTP01 solver
-	orderURLLabelKey = "certmanager.k8s.io/acme-order-url"
-	domainLabelKey   = "certmanager.k8s.io/acme-http-domain"
+	certNameLabelKey      = "certmanager.k8s.io/certificate"
+	orderURLAnnotationKey = "certmanager.k8s.io/acme-order-url"
+	domainLabelKey        = "certmanager.k8s.io/acme-http-domain"
 )
 
 // Solver is an implementation of the acme http-01 challenge solver protocol
@@ -101,10 +101,6 @@ func (s *Solver) CleanUp(ctx context.Context, crt *v1alpha1.Certificate, domain,
 	return utilerrors.NewAggregate(errs)
 }
 
-func dns1035(s string) string {
-	return strings.Replace(s, ".", "-", -1)
-}
-
 // testReachability will attempt to connect to the 'domain' with 'path' and
 // check if the returned body equals 'key'
 func testReachability(ctx context.Context, domain, path, key string) (bool, error) {

pkg/issuer/acme/http/ingress.go

@@ -66,8 +66,26 @@ func (s *Solver) ensureIngress(crt *v1alpha1.Certificate, svcName, domain, token
 	}
 	if httpDomainCfg != nil &&
 		httpDomainCfg.Ingress != "" {
+
 		return s.addChallengePathToIngress(crt, svcName, domain, token, *httpDomainCfg)
 	}
+	existingIngresses, err := s.getIngressesForCertificate(crt, domain)
+	if err != nil {
+		return nil, err
+	}
+	if len(existingIngresses) > 0 {
+		errMsg := fmt.Sprintf("multiple challenge solver pods found for certificate '%s/%s'. Cleaning up existing pods.", crt.Namespace, crt.Name)
+		glog.Infof(errMsg)
+		err := s.cleanupPods(crt, domain)
+		if err != nil {
+			return nil, err
+		}
+		return nil, fmt.Errorf(errMsg)
+	}
+	if len(existingIngresses) == 1 {
+		return existingIngresses[0], nil
+	}
+	glog.Infof("No existing HTTP01 challenge solver pod found for Certificate %q. One will be created.")
 	return s.createIngress(crt, svcName, domain, token, *httpDomainCfg)
 }
 
@@ -131,6 +149,11 @@ func (s *Solver) addChallengePathToIngress(crt *v1alpha1.Certificate, svcName, d
 				// if an existing path exists on this rule for the challenge path,
 				// we overwrite it else we'll confuse ingress controllers
 				if p.Path == ingPathToAdd.Path {
+					// ingress resource is already up to date
+					if p.Backend.ServiceName == ingPathToAdd.Backend.ServiceName &&
+						p.Backend.ServicePort == ingPathToAdd.Backend.ServicePort {
+						return ing, nil
+					}
 					rule.HTTP.Paths[i] = ingPathToAdd
 					return s.client.ExtensionsV1beta1().Ingresses(ing.Namespace).Update(ing)
 				}

pkg/issuer/acme/http/pod.go

@@ -16,8 +16,8 @@ import (
 
 func podLabels(crt *v1alpha1.Certificate, domain string) map[string]string {
 	return map[string]string{
+		certNameLabelKey: crt.Name,
 		domainLabelKey:   domain,
-		orderURLLabelKey: crt.Status.ACME.Order.URL,
 	}
 }
 
@@ -28,10 +28,13 @@ func (s *Solver) ensurePod(crt *v1alpha1.Certificate, domain, token, key string)
 	}
 	var pod *corev1.Pod
 	if len(existingPods) > 0 {
-		// we will only care about the first pod if there are multiple returned
-		// here. The others should be cleaned up after a call to CleanUp is
-		// complete.
-		pod = existingPods[0]
+		errMsg := fmt.Sprintf("multiple challenge solver pods found for certificate '%s/%s'. Cleaning up existing pods.", crt.Namespace, crt.Name)
+		glog.Infof(errMsg)
+		err := s.cleanupPods(crt, domain)
+		if err != nil {
+			return nil, err
+		}
+		return nil, fmt.Errorf(errMsg)
 	}
 	if len(existingPods) == 0 {
 		glog.Infof("No existing HTTP01 challenge solver pod found for Certificate %q. One will be created.")

pkg/issuer/acme/http/service.go

@@ -1,6 +1,8 @@
 package http
 
 import (
+	"fmt"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -19,10 +21,13 @@ func (s *Solver) ensureService(crt *v1alpha1.Certificate, domain, token, key str
 	}
 	var service *corev1.Service
 	if len(existingServices) > 0 {
-		// we will only care about the first service if there are multiple returned
-		// here. The others should be cleaned up after a call to CleanUp is
-		// complete.
-		service = existingServices[0]
+		errMsg := fmt.Sprintf("multiple challenge solver services found for certificate '%s/%s'. Cleaning up existing services.", crt.Namespace, crt.Name)
+		glog.Infof(errMsg)
+		err := s.cleanupServices(crt, domain)
+		if err != nil {
+			return nil, err
+		}
+		return nil, fmt.Errorf(errMsg)
 	}
 	if len(existingServices) == 0 {
 		glog.Infof("No existing HTTP01 challenge solver service found for Certificate %q. One will be created.")
@@ -62,10 +67,6 @@ func (s *Solver) getServicesForCertificate(crt *v1alpha1.Certificate, domain str
 				"but it is not owned by the Certificate resource, so skipping it.", service.Name, crt.Name)
 			continue
 		}
-		if service.Labels == nil ||
-			service.Labels[domainLabelKey] != domain {
-			continue
-		}
 		relevantServices = append(relevantServices, service)
 	}
 

pkg/issuer/acme/prepare.go

@@ -102,7 +102,16 @@ func (a *Acme) Prepare(ctx context.Context, crt *v1alpha1.Certificate) error {
 	}
 
 	if len(failed) > 0 {
+		// clear the order url to trigger a new order to be created
 		crt.Status.ACMEStatus().Order.URL = ""
+		// clean up pending authorizations
+		for _, auth := range pending {
+			err := a.cleanupAuthorization(ctx, cl, crt, auth)
+			if err != nil {
+				// TODO: handle error properly
+				return err
+			}
+		}
 		// TODO: pretty-print the list of failed authorizations
 		s := fmt.Sprintf("Error obtaining validations for domains %v", failed)
 		crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorCheckAuthorization, s)
@@ -193,8 +202,7 @@ func (a *Acme) presentAuthorization(ctx context.Context, cl client.Interface, cr
 func (a *Acme) cleanupAuthorization(ctx context.Context, cl client.Interface, crt *v1alpha1.Certificate, auth *acme.Authorization) error {
 	challenge, err := a.challengeForAuthorization(cl, crt, auth)
 	if err != nil {
-		// TODO: handle error properly
-		return nil
+		return err
 	}
 	domain := auth.Identifier.Value
 	token := challenge.Token