Closed reneradoi closed 3 months ago
Hey Judit, thanks for your review!
- I may have overlooed: we are not testing for the notification to be raised? But for the cert expiry/renewal (only)?
No the goal is not to test for the notification to be raised (aka the secret to expire). This is part of what the tls-operator
and juju
do for us, and we only use these mechanisms. What we really want to test is that our reaction to that even is as expected, and the expiring certificates are renewed by our operator, with Opensearch being still functional in the meantime. That's why this test is focussed on the certs to be updated, when this event occurs.
- Could we change the title of the PR? It's not only introducing tests but a behavior change for the Opensearch charm (namely to get notifications ahead of a cert expiry), which is a "product-level" change :-)
The notification is actually not a new feature that we are inventing. It is already there, and we would already get notified. I only added the config to make this notification a useful one by adjusting this time to a week. This will result in the secret expiry to be set to one week in advance of the certificate expiry instead of just some hours. I don't consider this a big change tbh.
This PR provides integration test coverage for the workflow of expiring TLS certificates and their renewal.
It also includes a bugfix for renewing the expiring certificates, or better creating the certificate signing requests: When initially creating them, they do not include
unique-id
in the subject. But on renewal this is included, which causes thesubject
of the renewed certificate to be different from the initial one.In addition to that, there's a config change to
TLSCertificatesRequiresV3
in order to get notification for expiring certificates with a reasonable lead time.Please also note the documentation added for the TLS certificates workflow here: https://github.com/canonical/opensearch-operator/wiki/TLS-certificates-workflow