Ingress-nginx: Change in Certificate behaviour after upgrade to Go 1.9

[rikatz]

Is this a request for help? (If yes, you should use our troubleshooting guide and community support channels, see https://kubernetes.io/docs/tasks/debug-application-cluster/troubleshooting/.):

What keywords did you search in NGINX Ingress controller issues before filing this one? (If you have found any duplicates, you should instead reply there.):

---

Is this a BUG REPORT or FEATURE REQUEST? (choose one):

NGINX Ingress controller version: 0.9.0-beta19

Kubernetes version (use kubectl version): 1.7

Environment: Linux / Baremetal

• Cloud provider or hardware configuration: Baremetal
• OS (e.g. from /etc/os-release): CoreOS
• Kernel (e.g. uname -a):
• Install tools:
• Others:

What happened:
After upgrading to 0.9.0-beta19, my certificates are not working anymore.

In fact, this line with the change to Go 1.9 (and the new x509 library) is not considering the Certificate Common Name in Certificates that have subjectAltName with thing other than DNSNames.

This issue is reported here, but it seems this is an expected behaviour in this new Golang version.

So this issue is more for a discussion: Should we change our error message to something other then ssl certificate %v does not contain a Common Name or Subject Alternative Name for host %v, including the error returned by the VerifyHostname Function?

Or at least inform that in this new Ingress Version, Certificates must have also a SAN with the used DNS Names?

[aledbf]

@rikatz I think we cannot break existing clusters or force user to change the certificates. Let me see how we can replace that validation to be backward compatible.

[aledbf]

@rikatz than you for the report and the issue in the go project.

[rikatz]

@aledbf I'll think in some alternative also, the thing is that all the 'major' CAs are already generating the certificate correctly, but this might brake anyway small users, or in our case poor CA (our internal one) that doesn't generate certificates with SAN yet :)

[aledbf]

@rikatz we can just copy the code from go with the old verification like here https://gist.github.com/aledbf/589d18832b659d5abd636bf6b89940df and add an additional check with a TODO comment to remove this in the future. WDYT?

[aledbf]

@rikatz please check #1771

[rikatz]

@aledbf Will test / compile tomorrow in Prod environment.

BTW, shouldn't a Warning message be added if we find a 'future' invalid certificate, as a deprecation warning? If we find a certificate with SAN but without DNSNames, let's warn that this is going to be deprecated in future releases :)

[aledbf]

@rikatz goog point. I will add a warning.