ingress with multiple hosts will break ingress when new hosts are added

[schmitch]

Describe the bug:
well it's hard to describe and my title is probably not good, feel free to edit.

currently if my ingress is big:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/configuration-snippet: |
      if ($host !~* www\.(.*)) {
        set $www O;
      }
      
      if ($request_uri ~ ^/.well-known/acme-challenge/(.*)$) {
          set $www "${www}FALSE";
      }

      if ($host !~* (.*)\.MAINDOMAIN\.de) {
          set $www "${www}K";
      }

      if ($www = OK) {
        set $host_with_www www.$host;
        rewrite ^(.*)$ $scheme://$host_with_www$request_uri permanent; #1
      }
    nginx.ingress.kubernetes.io/ssl-redirect: "true"
    certmanager.k8s.io/acme-http01-edit-in-place: "true"
    certmanager.k8s.io/cluster-issuer: letsencrypt-prod
    kubernetes.io/tls-acme: "true"
    kubernetes.io/ingress.class: nginx
  name: main-cms-ingress
  namespace: main-cms
spec:
  rules:
  - host: cms2.maindomain.de
    http: &http_rules
      paths:
      - backend:
          serviceName: cms
          servicePort: http
        path: /
  - host: tld1.maindomain.de
    http: *http_rules
  - host: tld1.de
    http: *http_rules
  - host: www.tld1.de
    http: *http_rules
  - host: tld2.de
    http: *http_rules
  - host: www.tld2.de
    http: *http_rules
  - host: tld3.de
    http: *http_rules
  - host: www.tld3.de
    http: *http_rules    
  tls:
    - secretName: maindomain-cms-tls
      hosts: 
        - cms2.maindomain.de
        - tld1.maindomain.de
        - tld1.de
        - www.tld1.de
        - tld2.de
        - www.tld2.de
        - tld3.de
        - www.tld3.de

cert-manager will now break the ingress if I would add a new host/tld, i.e. I add tld4.de and www.tld4.de to tls rules and host rules.
i.e. it will remove all OTHER hosts from the rules section and only add the two ones that need a new certificate, after that all other ingress rules are lost forever, however the tls section will be kept.
i.e. the ingress after adding the new host will be changed by cert-manager to:

 rules:
  - host: tld4.de
    http: &http_rules
      paths:
      - backend:
          serviceName: cms
          servicePort: http
        path: /
        - backend:
            serviceName: cm-acme-http-solver-zrvzb
            servicePort: 8089
          path: /.well-known/acme-challenge/NUMBER
  - host: www.tld4.de
    http: &http_rules
      paths:
      - backend:
          serviceName: cms
          servicePort: http
        path: /
        - backend:
            serviceName: cm-acme-http-solver-zrvzb
            servicePort: 8089
          path: /.well-known/acme-challenge/NUMBER

instead of keeping my good old host list

Expected behaviour:
A concise description of what you expected to happen.
only updating values and not discaring correct entries. never actually remove sections from the ingress

Steps to reproduce the bug:
Steps to reproduce the bug should be clear and easily reproducible to help people
gain an understanding of the problem.
create a ingress like me and add hosts after you already have a valid certificate

Anything else we need to know?:
I'm like 60% sure that this behavior worked fine in 0.4

Environment details::

• Kubernetes version (e.g. v1.10.2): 1.10.x
• Cloud-provider/provisioner (e.g. GKE, kops AWS, etc): gke (nginx-ingress-0.24.0)
• cert-manager version (e.g. v0.4.0): cert-manager-v0.5.0
• Install method (e.g. helm or static manifests): helm

/kind bug

[wapa5pow]

I run into the same problem. I checked with v0.5.0 and v0.4.1.
v0.5.0 has the problem but v0.4.1 does not have the problem. This #831 might be the problem.

[pkdetlefsen]

I think I might be experiencing a similar issue, except my issue happens when any of the certificates are automatically renewed when the expiry date is within 30 days. When a certificate is renewed then all hosts are deleted from the ingress, leaving only the default backend.

Here are my annotations:

kubernetes.io/tls-acme: "true"
kubernetes.io/ingress.class: "gce"
certmanager.k8s.io/cluster-issuer: "letsencrypt-prod"
certmanager.k8s.io/acme-challenge-type: "http01"
certmanager.k8s.io/acme-http01-edit-in-place: "true"

Environment:

• Kubernetes: 1.10.7-gke.2.
• Provisioner: GKE
• cert-manager ver.: cert-manager-v0.5.0
• Install method: helm

I'm wondering if this issue can be avoided by not using the acme-http01-edit-in-place option, but I haven't tested it yet.

Is anyone else experiencing this?

[wapa5pow]

@pkdetlefsen My config was exactly same as yours. If I remove "certmanager.k8s.io/acme-http01-edit-in-place", cert-manager create another ingress than current ingress. That new ingress can't accept "challenge" because ip address is different than the domain's one.

If I use v0.4.1 with the config, it works well.

[pkdetlefsen]

Ah, that's right. Just remembered that I started using the option to get around that issue.

[pkdetlefsen]

@wapa5pow I took a look at #831, which you mentioned.

As I understand the code it loops through the rules of your Ingress to find the domain matching your certificate. Once it finds the matching host it checks if it contains the temporary path used for the ACME challenge and deletes that path if it exists. If there are more paths remaining then that rule is saved.

However, it seems like all the rules that doesn't match the certificate domain are simply dropped because none of them are added to the ingRules array? Before the change, the ing object was modified directly but now that ing.Spec.Rules is replaced with ingRules we lose all the hosts which aren't part of the new certificate.

Perhaps we need an else clause that adds all rules that doesn't match the certificate domain to ingRules? @munnerz

[schmitch]

btw. I have certmanager.k8s.io/acme-http01-edit-in-place: "true" and it does still break.

[dm3ch]

@pkdetlefsen How are you comparing rules? Are you just compare strings (what about wildcards in pathes)?

I just have same issue and I'm not sure what caused it. But I have locations '/*'

[pkdetlefsen]

I think the cleanup process uses string comparison between the host in the rules section and the domain in the certificate.

I would need someone familiar with the code to confirm/deny whether the issue is actually caused by #831 or not.

[munnerz]

@pkdetlefsen

Perhaps we need an else clause that adds all rules that doesn't match the certificate domain to ingRules? @munnerz

Yes, agreed. I've opened #1082 which should fix this.