search

go-resty / resty

Simple HTTP and REST client library for Go
MIT License
10.12k stars 710 forks source link

Feature request : Implement ability to dynamically reload TLS certificates #856

Closed cdoucy closed 1 week ago

cdoucy commented 1 month ago

Description

client.SetRootCertificate and client.SetClientRootCertificate allow to configure certificates for TLS communication. It would be great if it was possible to dynamically reload certificates when they expire to automatically handle certificate rotation.

Use-case

Assume there are multiple HTTPS services in a Kubernetes cluster. These HTTPS services are using Certificates issued by cert-manager. Therefore, pods who host HTTPS client need to be mounted with the root certificate. When cert-manager rotate the Certificates, the running HTTPS clients need to reload the mounted certificate.

To work around it, it's possible to either panic on TLS errors to restart the pod and reload the cert, or to create a fresh new resty Client for each HTTPS call, but that's not very efficient.

Possible implementations

I see two ways to implement this feature:

  1. Watch for file system events and reload the certificate when the file changes

fsnotify is a cross-platform library that allows to watch file system events. I believe it would be a good fit for the use-case. However, I'm not sure if fsnotify can be used in resty due to legal reasons. (fsnotify is BSD licensed while resty is MIT) Implementing a cross-platform module to watch FS system would require some engineering efforts.

  1. Poll periodically certificate file mod time and reload it when it changes

A routine could periodically call os.Stat() on the file to check the certificate file modification time. This approach would be straightforward to implement but is not as efficient as the previous option.

cdoucy commented 1 month ago

Hello @jeevatkm ! I would happy to start contributing to resty by addressing this issue.

jeevatkm commented 1 month ago

Hello @cdoucy - Thanks for reaching out and proposing a feature request. I agree; it is a helpful feature for Resty users.

I suggest a change in the implementation. What do you think of the following approach?

Add functionality with user-provided time.Duration and certs file paths as input, and Resty will reload the certificates when the timer hits by checking os.Stat instead of polling.

Please feel free to create a PR.

PS: Resty v2 Client settings modification is not set thread-safe 100%. It should be all right if there are no concurrent certificate modifications; it is just not immune to it yet. The upcoming Resty v3 Client comes with sync.RWMutex to manage settings on client instances, which is 100% thread-safe.

jeevatkm commented 1 month ago

@cdoucy I edited the above suggestion with input parameter details.

cdoucy commented 1 month ago

Hi @jeevatkm ! Thanks for your feedback!

I suggest a change in the implementation. What do you think of the following approach?

Add functionality with user-provided time.Duration and certs file paths as input, and Resty will reload the certificates when the timer hits by checking os.Stat instead of polling.

Agree, actually that's pretty much what I had in mind as well!

Please feel free to create a PR.

Great, will create a draft PR by Sunday.

jeevatkm commented 1 week ago

PR has been merged.