Skip to content

Handle CA rotation for Catalogd web server trust #915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
trgeiger opened this issue Jun 7, 2024 · 2 comments · Fixed by #1062
Closed

Handle CA rotation for Catalogd web server trust #915

trgeiger opened this issue Jun 7, 2024 · 2 comments · Fixed by #1062

Comments

@trgeiger
Copy link
Contributor

trgeiger commented Jun 7, 2024

With the addition of TLS communication between operator-controller and catalogd, we now mount the CA certificate into operator-controller so it can trust connections to the catalogd web server.

It would be ideal if we could handle rotations of the mounted CA certificate without restarting the manager. As of now, the contents of the mounted CA certificate do change on disk when the certificate rotates, but the manager does not pick up any changes since Go does not have a mechanism for reloading rootCA.

There is the possibility to implement custom connection verification logic in tls.Config.VerifyConnection(), but for this to work you would also need to set InsecureSkipVerify to true which isn't ideal. The ideal solution would probably be to contribute something akin to GetCertificate but for the rootCA upstream into Go and then use that feature in operator-controller.

It might also be worth modifying the CertificateWatcher in controller-runtime so it can function without providing a key--currently it requires both the cert and key since it's built for managing rotation on servers. A GetRootCA-type functionality for re-loading the CA cert in tls.Config would utilize the modified CertificateWatcher in a similar manner to how the certificate rotation is handled in Catalogd.

@tmshort
Copy link
Contributor

tmshort commented Jul 22, 2024

See also: #1062

@tmshort
Copy link
Contributor

tmshort commented Jul 22, 2024

With #1062 merged, this is basically done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants