feat(tls): add certificate hot-reload for gNMI target connections - #964
feat(tls): add certificate hot-reload for gNMI target connections#964bradrevans wants to merge 7 commits into
Conversation
|
Additionally updated |
|
Thanks for this, zero-downtime cert rotation is something large deployments have asked for, and the cert/key part of this is in good shape. 1) CA verification
If step 1 fails, the handshake aborts and neither callback runs. To make CA reload real, the built-in check has to be replaced: // only when the user did NOT set skip-verify
tlsConfig.InsecureSkipVerify = true
tlsConfig.VerifyConnection = func(cs tls.ConnectionState) error {
if len(cs.PeerCertificates) == 0 {
return errors.New("no peer certificate")
}
opts := x509.VerifyOptions{
Roots: caR.getPool(),
DNSName: cs.ServerName,
Intermediates: x509.NewCertPool(),
}
for _, c := range cs.PeerCertificates[1:] {
opts.Intermediates.AddCert(c)
}
_, err := cs.PeerCertificates[0].Verify(opts)
return err
}
If you would rather keep the scope small for a first PR, the alternative is to drop 2) Default should be off for nowThe flag defaults to |
Thanks so much for the incredibly detailed review - I really appreciate you taking the time and providing the I've pushed two new commits to address everything you mentioned:
Please let me know if there's anything else you'd like me to tweak! |
Motivation
In large deployments with thousands of unique targets, periodically rotating client TLS certificates or CA bundles currently requires a full restart of the
gnmiccollector. This forces an unnecessary connectivity drop across all targets, even if only a small subset of credentials needed updating.Solution
This PR introduces zero-downtime, mtime-based TLS hot-reloading for target connections. Because gNMI connections are long-lived, when a certificate expires or is rotated on disk, the new material is seamlessly picked up on the next TLS handshake/reconnect without needing to bounce the collector process.
Technical Implementation
tls.X509KeyPairand CA pool assignments inutils.NewTLSConfigwith Go's standard dynamic callbacks (GetClientCertificate,GetCertificate,VerifyPeerCertificate).certReloaderandcaReloadertopkg/api/utils/tls.go. During a handshake, these helpers checkos.Staton the cert/key/ca files. If themtimehasn't changed, they instantly return the in-memory parsed certificate (imposing zero disk I/O overhead on reconnects).Configuration Impacts
tls-reloadglobal config option and--tls-reloadCLI flag (enabled by default).tls-reloadtoTargetConfigso users can explicitly opt-out specific legacy targets if desired.utils.NewTLSConfig(e.g., outputs, inputs, loaders, apiserver) have been explicitly updated to passfalseto the newhotReloadparameter. This guarantees their static TLS behavior remains entirely unchanged, avoiding any unintended side effects outside of target connections.Testing & Documentation
tls_test.goto validate thread-safety under heavy concurrent reconnects, mtime rotation logic, and mid-write garbage file fallbacks.docs/global_flags.md,docs/user_guide/targets/targets.md, andtargets_session_sec.mdto document the new behavior.Side Notes
This is my first ever contribution to this project so my apologies if I've not followed the process properly and I would appreciate any feedback/recommendations in order to see this capability included.