Fix issue causing existing ingresses to not be cleaned up properly

pkg/issuer/acme/http/ingress.go

@@ -223,6 +223,7 @@ func (s *Solver) cleanupIngresses(crt *v1alpha1.Certificate, ch v1alpha1.ACMEOrd
 	}
 
 	// otherwise, we need to remove any cert-manager added rules from the ingress resource
+	// TODO: switch to using ingress lister?
 	ing, err := s.Client.ExtensionsV1beta1().Ingresses(crt.Namespace).Get(existingIngressName, metav1.GetOptions{})
 	if k8sErrors.IsNotFound(err) {
 		glog.Infof("attempt to cleanup Ingress %q of ACME challenge path failed: %v", crt.Namespace+"/"+existingIngressName, err)
@@ -233,21 +234,32 @@ func (s *Solver) cleanupIngresses(crt *v1alpha1.Certificate, ch v1alpha1.ACMEOrd
 	}
 
 	ingPathToDel := solverPathFn(ch.Token)
-Outer:
+	var ingRules []extv1beta1.IngressRule
+
 	for _, rule := range ing.Spec.Rules {
 		if rule.Host == ch.Domain {
+			// if the 'http' stanza is not set, we will retain the rule as it is
+			// not managed by cert-manager
 			if rule.HTTP == nil {
-				return nil
+				ingRules = append(ingRules, rule)
+				continue
 			}
+			// check the rule for paths. If we find the ingress path we need to
+			// delete here, delete it
 			for i, path := range rule.HTTP.Paths {
 				if path.Path == ingPathToDel {
 					rule.HTTP.Paths = append(rule.HTTP.Paths[:i], rule.HTTP.Paths[i+1:]...)
-					break Outer
 				}
 			}
+			// if there are still paths level on this rule, we should retain it
+			if len(rule.HTTP.Paths) > 0 {
+				ingRules = append(ingRules, rule)
+			}
 		}
 	}
 
+	ing.Spec.Rules = ingRules
+
 	_, err = s.Client.ExtensionsV1beta1().Ingresses(ing.Namespace).Update(ing)
 	if err != nil {
 		return err

pkg/issuer/acme/http/ingress_test.go

@@ -25,9 +25,12 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	coretesting "k8s.io/client-go/testing"
 
 	"github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1"
+	"github.com/jetstack/cert-manager/pkg/controller/test"
 	"github.com/jetstack/cert-manager/test/util/generate"
 )
 
@@ -190,6 +193,80 @@ func TestCleanupIngresses(t *testing.T) {
 				}
 			},
 		},
+		"should clean up an ingress with a single challenge path inserted": {
+			Builder: &test.Builder{
+				KubeObjects: []runtime.Object{
+					&v1beta1.Ingress{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "testingress",
+							Namespace: defaultTestNamespace,
+						},
+						Spec: v1beta1.IngressSpec{
+							Backend: &v1beta1.IngressBackend{
+								ServiceName: "testsvc",
+								ServicePort: intstr.FromInt(8080),
+							},
+							Rules: []v1beta1.IngressRule{
+								{
+									Host: "example.com",
+									IngressRuleValue: v1beta1.IngressRuleValue{
+										HTTP: &v1beta1.HTTPIngressRuleValue{
+											Paths: []v1beta1.HTTPIngressPath{
+												{
+													Path: "/.well-known/acme-challenge/abcd",
+													Backend: v1beta1.IngressBackend{
+														ServiceName: "solversvc",
+														ServicePort: intstr.FromInt(8081),
+													},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			Certificate: generate.Certificate(generate.CertificateConfig{
+				Name:         "test",
+				Namespace:    defaultTestNamespace,
+				DNSNames:     []string{"example.com"},
+				ACMEOrderURL: "testurl",
+				SolverConfig: v1alpha1.SolverConfig{
+					HTTP01: &v1alpha1.HTTP01SolverConfig{
+						Ingress: "testingress",
+					},
+				},
+			}),
+			Challenge: v1alpha1.ACMEOrderChallenge{
+				Domain: "example.com",
+				Token:  "abcd",
+				SolverConfig: v1alpha1.SolverConfig{
+					HTTP01: &v1alpha1.HTTP01SolverConfig{
+						Ingress: "testingress",
+					},
+				},
+			},
+			PreFn: func(t *testing.T, s *solverFixture) {
+			},
+			CheckFn: func(t *testing.T, s *solverFixture, args ...interface{}) {
+				expectedIng := s.KubeObjects[0].(*v1beta1.Ingress).DeepCopy()
+				expectedIng.Spec.Rules = nil
+
+				actualIng, err := s.Builder.FakeKubeClient().ExtensionsV1beta1().Ingresses(s.Certificate.Namespace).Get(expectedIng.Name, metav1.GetOptions{})
+				if apierrors.IsNotFound(err) {
+					t.Errorf("expected ingress resource %q to not be deleted, but it was deleted", expectedIng.Name)
+				}
+				if err != nil {
+					t.Errorf("error getting ingress resource: %v", err)
+				}
+
+				if !reflect.DeepEqual(expectedIng, actualIng) {
+					t.Errorf("expected did not match actual: %v", diff.ObjectDiff(expectedIng, actualIng))
+				}
+			},
+		},
 		"should return an error if a delete fails": {
 			Certificate: generate.Certificate(generate.CertificateConfig{
 				Name:         "test",