fix private key generation (#78)

* fix private key generation

* add missing _on_upgrade

* linting

* fix race condition in privkey generation (#76)

* fix race condition in privkey generation

* fix static checks in other libs

* tox fmt

* update LIBPATCH

---------

Co-authored-by: Luca Bello <[email protected]>

lib/charms/observability_libs/v1/cert_handler.py

@@ -66,7 +66,7 @@
 
 LIBID = "b5cd5cd580f3428fa5f59a8876dcbe6a"
 LIBAPI = 1
-LIBPATCH = 2
+LIBPATCH = 3
 
 
 def is_ip_address(value: str) -> bool:
@@ -192,14 +192,11 @@ def _on_certificates_relation_joined(self, _) -> None:
     def _generate_privkey(self):
         # Generate priv key unless done already
         # TODO figure out how to go about key rotation.
-        relation = self.charm.model.get_relation(self.certificates_relation_name)
 
-        if not relation:
+        if not (relation := self.charm.model.get_relation(self.certificates_relation_name)):
             return
 
-        private_key = relation.data[self.charm.unit].get("private-key", None)
-
-        if not private_key:
+        if not self.private_key:
             private_key = generate_private_key()
             secret = self.charm.unit.add_secret({"private-key": private_key.decode()})
             secret.grant(relation)

tests/integration/helpers.py

@@ -0,0 +1,24 @@
+# Copyright 2024 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Helper functions for writing tests."""
+
+import subprocess
+
+from pytest_operator.plugin import OpsTest
+
+
+def get_secret(ops_test: OpsTest, app_name: str, path: str) -> str:
+    return subprocess.check_output(
+        [
+            "juju",
+            "ssh",
+            "--model",
+            ops_test.model_full_name,
+            "--container",
+            "httpbin",
+            f"{app_name}/0",
+            "cat",
+            path,
+        ]
+    ).decode()

tests/integration/test_cert_handler_v1.py

@@ -8,13 +8,18 @@
 
 import pytest
 import yaml
+from helpers import get_secret
 from pytest_operator.plugin import OpsTest
 
 logger = logging.getLogger(__name__)
 
 METADATA = yaml.safe_load(Path("./tests/integration/tester-charm/metadata.yaml").read_text())
 APP_NAME = METADATA["name"]
 
+KEY_PATH = "/home/ubuntu/secrets/server.key"
+CERT_PATH = "/home/ubuntu/secrets/server.cert"
+CA_CERT_PATH = "/home/ubuntu/secrets/ca.cert"
+
 
 @pytest.mark.abort_on_fail
 async def test_cert_handler_v1(
@@ -52,7 +57,7 @@ async def test_cert_handler_v1(
     await ops_test.model.wait_for_idle(apps=apps, status="active", wait_for_exact_units=1)
 
     # Check the certs files are in the filesystem
-    for path in ["/tmp/server.key", "/tmp/server.cert", "/tmp/ca.cert"]:
+    for path in [KEY_PATH, CERT_PATH, CA_CERT_PATH]:
         assert 0 == subprocess.check_call(
             [
                 "juju",
@@ -65,3 +70,20 @@ async def test_cert_handler_v1(
                 f"ls {path}",
             ]
         )
+
+
+@pytest.mark.abort_on_fail
+async def test_secrets_does_not_change_after_refresh(ops_test: OpsTest, tester_charm: Path):
+    paths = [KEY_PATH, CERT_PATH, CA_CERT_PATH]
+    secrets = {paths[0]: "", paths[1]: "", paths[2]: ""}
+
+    for path in paths:
+        secrets[path] = get_secret(ops_test, APP_NAME, path)
+
+    await ops_test.model.applications[APP_NAME].refresh(path=tester_charm)
+    await ops_test.model.wait_for_idle(
+        status="active", raise_on_error=False, timeout=600, idle_period=30
+    )
+
+    for path in paths:
+        assert secrets[path] == get_secret(ops_test, APP_NAME, path)

tests/integration/tester-charm/src/charm.py

@@ -14,9 +14,9 @@
 
 VALID_LOG_LEVELS = ["info", "debug", "warning", "error", "critical"]
 
-KEY_PATH = "/tmp/server.key"
-CERT_PATH = "/tmp/server.cert"
-CA_CERT_PATH = "/tmp/ca.cert"
+KEY_PATH = "/home/ubuntu/secrets/server.key"
+CERT_PATH = "/home/ubuntu/secrets/server.cert"
+CA_CERT_PATH = "/home/ubuntu/secrets/ca.cert"
 
 
 class TesterCharm(ops.CharmBase):
@@ -34,6 +34,10 @@ def __init__(self, *args):
         self.framework.observe(self.cert_handler.on.cert_changed, self._on_server_cert_changed)
         self.framework.observe(self.on["httpbin"].pebble_ready, self._on_httpbin_pebble_ready)
         self.framework.observe(self.on.config_changed, self._on_config_changed)
+        self.framework.observe(self.on.upgrade_charm, self._on_upgrade_charm)
+
+    def _on_upgrade_charm(self, _):
+        self._update_cert()
 
     def _on_server_cert_changed(self, _):
         self._update_cert()