canonical / prometheus-k8s-operator

This charmed operator automates the operational procedures of running Prometheus, an open-source metrics backend.
https://charmhub.io/prometheus-k8s
Apache License 2.0
21 stars 34 forks source link

fix: only add TLS config if cert file has been written to disk #509

Closed lucabello closed 1 year ago

lucabello commented 1 year ago

Issue

Fixes #506.

This likely happens when _configure() is called by something other than _on_server_cert_changed (e.g., a perfectly timed update-status), but when the certificate is already in relation data (or Prometheus has already been related through the certificates interface but the certificate is not ready yet).

Solution

Only add the tls_server_config for Prometheus if the certificate exists on the file sytstem.

The _configure() method is also called by _on_server_cert_changed(), which (sequentially) writes the cert to disk before calling _configure(). This means we don't risk skip adding the TLS configuration with this extra check.

Testing Instructions

It's pretty much a race condition, so it's hard to add a test for it. I guess trying to replicate the issue is something, though of course it doesn't guarantee it.

Release Notes

Only add the tls_server_config for Prometheus if the certificate exists on the file sytstem.

lucabello commented 1 year ago

I don't think it's necessarily in the scope of this issue, which is the failed replan; this behavior when TLS is enabled has been implemented in the TLS PR :)

I'll add it anyway!

lucabello commented 1 year ago

After noticing the itests were passing locally, rerunning them fixed things :D