Regression: Default certificate used if oldest ingress does not contain TLS secret

[zeeZ]

NGINX Ingress controller version: 0.27.0

What happened:

When the oldest Ingress that specifies a TLS host does not reference a secret, the default certificate is always served, even if another Ingress references a valid secret.

What you expected to happen:

If multiple Ingress specify a TLS host, but only one references a valid secret, the certificate from that secret should be used, not the default certificate

How to reproduce it:

1. create a tls secret "mysecret" with a valid certificate for "foo.bar.com"

2. create Ingress without secret reference

   echo "apiVersion: extensions/v1beta1
   kind: Ingress
   metadata:
     name: default-cert
   spec:
     rules:
     - host: foo.bar.com
       http:
         paths:
         - path: /foo
           backend:
             serviceName: foobar
             servicePort: 80
     tls:
     - hosts:
       - foo.bar.com" | kubectl apply -f-
   

3. create Ingress with secret reference

   echo "apiVersion: extensions/v1beta1
   kind: Ingress
   metadata:
     name: custom-cert
   spec:
     rules:
     - host: foo.bar.com
       http:
         paths:
         - path: /bar
           backend:
             serviceName: foobar
             servicePort: 80
     tls:
     - hosts:
       - foo.bar.com
       secretName: mysecret" | kubectl apply -f-
   

4. check certificate returned

   openssl s_client -connect $INGRESS_ADDR -servername foo.bar.com
   

Anything else we need to know:

Applying steps 2 and 3 in reverse order will serve the correct certificate. Restart the controller after removing Ingresses when testing because certificates may linger.

See #2279 and #4563 which asked for this behaviour.

I suspect this was broken by #4816.

/kind bug

[aledbf]

@zeeZ this works as expected. Please check https://kubernetes.github.io/ingress-nginx/how-it-works/
The rule is the oldest ingress "wins".

[zeeZ]

You could easily add an "oldest eligible certificate/secret wins" rule, but if this is indeed expected behaviour supposed to stay this way I will have no choice but to migrate to another controller.

[aledbf]

You could easily add an "oldest eligible certificate/secret wins" rule

The first rule is invalid, you should not have an ingress with a tls section without secretName. Without that, the ingress controller cannot configure SSL. The ingress controller cannot guess what is the intention here, that's why we cannot mix definitions from different ingresses.

but if this is indeed expected behaviour supposed to stay this way I will have no choice but to migrate to another controller.

This is the behavior since the inception of the controller and we have no plans to change it.

[zeeZ]

The ingress controller cannot guess what is the intention here, that's why we cannot mix definitions from different ingresses.

But definitions from different ingresses are already mixed, for example with server-snippet. It doesn't have to be the oldest one that defines this annotation, no?

I don't see the need for the controller to guess. There is a valid secret defined for a host, albeit not the oldest one, but instead it uses the default certificate, which is invalid for that host. It's just a matter of choosing the default certificate after checking the first ingress vs after all configuration is merged.

This is the behavior since the inception of the controller and we have no plans to change it.

Not for the 0.26 versions, but that was a bug then.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.