[feature] Reload TLSConfig.CAFile on SIGHUP

[SoMuchToGrok]

A very much appreciated feature was recently delivered to support graceful reloads of the TLS configuration.

https://github.com/hashicorp/nomad/pull/3479/files

I think this needs to be extended to also refresh the CA file (if defined). I run very short TTLs on both server certificates and root CAs. As it's currently implemented, this is still a huge pain point for me.

I might be able to take a stab at this myself, but want to get a conversation going first. Thoughts or concerns?

Thanks!

[schmichael]

Seems reasonable to me! Hopefully as easy as giving TLSConfig.CAFile the same treatment as the CertFile and KeyFile fields got in that PR.

[tmessi]

I think you will also want to refresh the VaultConfig.TLSCaFile and ConsulConfig.CAFile and then reload the vault/consul clients to use the new config, since it is likely that these would be using the same CA, or at least the CAs for vault and consul would also be short lived.

Edit: It looks like the vault client is being reloaded, not sure about consul.

[tmessi]

It looks like #3492 would almost resolve this issue. The only problem is that this function would potentially prevent reloads when they might be desired. In my case, the TLSConfig itself does not change, but the contents of the files referenced in the TLSConfig do change when a new CA is issued. If this reload would happen every time a SIGHUP is sent, it would address the TLSConfig.CAFile issue (the ConsulConfig.CAFile would still need to be addressed).

[schmichael]

Just a heads up this feature will not land in #3492 as it's already a large PR that's close to merging. We'll definitely be using what's in #3492 to add this feature though.

[SoMuchToGrok]

Sounds great @schmichael! Thanks for keeping us updated.

[SoMuchToGrok]

#4025

I think this might be the enhancement we're looking for?

[Xopherus]

I believe that PR does fix this issue @SoMuchToGrok. Though the fix does not reload the CA or cert on VaultConfig - that issue is captured in #6052

Seems like we can close this @schmichael?

[schmichael]

Yes, thanks @Xopherus! Closing.

[github-actions]

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.