Add TLS Client reload

[simonswine]

Cortex added TLS support for its GRPC/HTTP client connections as part of #2350.

To allow for a seamless roll-over of client certificates, it would be great to support live reloading of certificates without restarting Cortex itself.

Looking at other projects implementations:

• Kubernetes TLS Client: https://github.com/kubernetes/kubernetes/pull/79083/files
• Prometheus/Common: https://github.com/prometheus/common/pull/173/files

I would suggest following with the Kubernetes approach:

• Time limit reads from disk (e.g. at most once a minute)
• Hook into the dialer

I will investigate how a sensible implementation could look like and will update the issue

[pracucci]

Time limit reads from disk (e.g. at most once a minute)

Periodically re-read the certificate from disk is easy and effective. Not even required to make it configurable. A sane hardcoded default should be just fine (we're trying to simplify our config).

Hook into the dialer

Could you elaborate on this, please?

[simonswine]

If we want to be able to terminate long running connections established on the old client certificate, we would need to track those and that is the approach with using a custom dialer, as per the Kubenetes PR. It is using this Dial implementation: https://pkg.go.dev/k8s.io/client-go/util/connrotation.

If this is not a worry to us, I think your suggestion is simpler and more straightforward to implement.

[pracucci]

If we want to be able to terminate long running connections established on the old client certificate

Is it possible to configure a max connection age in the gRPC/HTTP connections so that we've the guarantee a connection would live longer then X time?

[simonswine]

I am not too sure about how long our HTTP calls are, but I assume we are not having long running HTTP calls at all? If we do I think the we could do it client (https://golang.org/pkg/net/#Dialer.Timeout) and server side (WriteTimeout https://golang.org/pkg/net/http/#Server).

For GRPC there seems to be this server side parameter MaxConnectionAge: https://godoc.org/google.golang.org/grpc/keepalive#ServerParameters

[pracucci]

I am not too sure about how long our HTTP calls are, but I assume we are not having long running HTTP calls at all?

I would suggest you to focus your investigation on the TCP connection (which could have a long life due to the HTTP keep-alive) and not the single HTTP request.

[simonswine]

You a absolutely right, I was somehow forgetting about keep-alive. I still think the Dialer.Timeout and the server options are the correct place to set an upper limit to the length of the http.Client's underlying TCP connection.

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

[simonswine]

This is still a valid feature and I am planning to work on this, was just too slow to respond.

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.

[milesbxf]

@simonswine hi 👋 have you managed to make any headway on this? We're quite keen to implement this for the etcd K/V client and I suppose it'd be good to try and re-use as much of your implementation as possible

[simonswine]

Hi @milesbxf 👋 . I am afraid this is has not been a priority recently and I think it will be a bit till I am able to look into this.

On the etcd client specifically, I have found that upstream PR, which might be a good start. (etcd-io/etcd#7829). I am more than happy to help on a contribution, feel free to ping me 🙂

[milesbxf]

Hi @milesbxf 👋 . I am afraid this is has not been a priority recently and I think it will be a bit till I am able to look into this.

On the etcd client specifically, I have found that upstream PR, which might be a good start. (etcd-io/etcd#7829). I am more than happy to help on a contribution, feel free to ping me 🙂

No worries at all. I had a bit of a dig around in the etcd codebase from the link you provided, and it turns out that my assumption that Cortex/etcd-client doesn't dynamically reload certs was completely wrong - the etcd transport actually sets GetCertificate on the tls.Config, and I've verified through some experimentation that the client definitely loads certs from disk on every TLS handshake 🙌 thanks for your help!

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 60 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.