Ingress v1 - TLS from Spec

[rickhlx]

Fixes #851
Fixes #1780

This will allow us to define TLS certs at the Ingress resource and dynamically respond with the correct cert based on the request.

[rickhlx]

@szuecs would love to receive some initial feedback on this proposal

[AlexanderYastrebov]

The current design sets certificate to the route which creates a problem for route update when tls secret changes and also loops over all valid routes which is completely unnecessary (e.g. only a handful of routes out of thousands may require certificate); it also couples certificate lookup to the routing. The proposal does not addess explicitly the problem of multiple ingresses defining different secrets for the same host. It also reloads secrets and re-creates certificates on every ingress poll cycle (3 seconds by default) which might be ok for the initial implementation but be suboptimal performance-wise as secrets would change less often.

I propose we think about a new component e.g. CertificateRegistry that would implement

• GetCertificate(helloInfo *tls.ClientHelloInfo) (*tls.Certificate, error) to be used by server listener
• RegisterCertificate(host string, *tls.Certificate) error to be used by Kubernetes data client (not sure if we need host here at all as certificate should contain Common Name already and the default certificate lookup for "static" certificates uses https://pkg.go.dev/crypto/tls#ClientHelloInfo.SupportsCertificate afaict, see code)

It is not clear what should be the strategy to cleanup of unused certificates and if we should decouple secret/certificate update from the ingress update at this point.

Also lets check how other ingress controllers implement the lookup logic.

[AlexanderYastrebov]

The proposal does not addess explicitly the problem of multiple ingresses defining different secrets for the same host.

There is a bit of chicken-and-egg problem - server listener has to choose the certificate before routing, i.e. it does not know yet which route the request will follow and hence can not determine the ingress and its tls config that defines route.

E.g. I do not see how and if it should be possible to define two ingresses that use different certificates for the same host and different paths:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: foo
spec:
  tls:
  - hosts:
      - https-example.example.com
    secretName: foo-secret
  rules:
  - host: https-example.example.com
    http:
      paths:
      - path: /foo
        pathType: Prefix
        backend:
          service:
            name: service1
            port:
              number: 80
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: bar
spec:
  tls:
  - hosts:
      - https-example.example.com
    secretName: bar-secret # different cert
  rules:
  - host: https-example.example.com
    http:
      paths:
      - path: /bar
        pathType: Prefix
        backend:
          service:
            name: service2
            port:
              number: 80

It is also not clear how certificate selection should work if one ingress defines the spec.tls but other does not while they use the same hostname:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: foo
spec:
  tls:
  - hosts:
      - https-example.example.com
    secretName: foo-secret
  rules:
  - host: https-example.example.com
    http:
      paths:
      - path: /foo
        pathType: Prefix
        backend:
          service:
            name: service1
            port:
              number: 80
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: bar
spec:
  # no tls defined for https-example.example.com
  rules:
  - host: https-example.example.com
    http:
      paths:
      - path: /bar
        pathType: Prefix
        backend:
          service:
            name: service2
            port:
              number: 80

It looks like the out best shot would be to have a global certificate registry that does not account for ingress/routing and let listener to pick first matching certificate. This way in example 1 different certs for the same host would be an undefined behaviour and in example 2 the second ingress would use certificate configured by the first.

[rickhlx]

It looks like the out best shot would be to have a global certificate registry that does not account for ingress/routing and let listener to pick first matching certificate

I like this idea. Terminating TLS should not care what the route is, should only care if a rule's host matches the tls host and add it to the registry.

It is not clear what should be the strategy to cleanup of unused certificates and if we should decouple secret/certificate update from the ingress update at this point.

Could it do something similar to how routing is updated? Get a list of current, updated and deleted TLS certs and send them over to the certificate registry?

[rickhlx]

Also lets check how other ingress controllers implement the lookup logic.

afaict the NGINX Ingress controller has a similar store component which implements an sslStore tracker.

Certificates are generated and stored in this tracker to later be read. The store has the logic to manage old/stale certificates.

[rickhlx]

@AlexanderYastrebov, implemented your suggestion of a Certificate Registry. Included the following proposals:

• Generate a default TLS certificate as a fallback
• Only update a certificate when it has changed
• Return the first TLS certificate that matches a configured host
  • As per the Ingress - TLS docs:

  hosts in the tls section need to explicitly match the host in the rules section.

• Initialize Certificate Registry, optionally enable TLS to use registry for GetCertificate using the flag tls-certificate-registry

If this something we could get your feedback on?

[AlexanderYastrebov]

@rickhlx Hello, thank you, I'll have a look once I have some spare capacity

[AlexanderYastrebov]

@rickhlx Thank you, great work!

[AlexanderYastrebov]

👍

[szuecs]

I think 4 small changes and we are good to merge. Thanks for your great work @rickhlx and thanks @AlexanderYastrebov for the great review!

[szuecs]

👍

[AlexanderYastrebov]

👍

[rickhlx]

@AlexanderYastrebov @szuecs thanks for the excellent communication and feedback, was a pleasure to work on this. Learned a lot!