diff --git a/src/charm.py b/src/charm.py index 8000cfdea..ae5a470ef 100755 --- a/src/charm.py +++ b/src/charm.py @@ -17,12 +17,20 @@ from charms.operator_libs_linux.v2 import snap from charms.pgbouncer_k8s.v0 import pgb from jinja2 import Template +from ops import JujuVersion from ops.charm import CharmBase from ops.framework import StoredState from ops.main import main -from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus +from ops.model import ( + ActiveStatus, + BlockedStatus, + MaintenanceStatus, + SecretNotFoundError, + WaitingStatus, +) from constants import ( + APP_SCOPE, AUTH_FILE_NAME, CLIENT_RELATION_NAME, EXTENSIONS_BLOCKING_MESSAGE, @@ -35,8 +43,14 @@ PGB_LOG_DIR, PGBOUNCER_EXECUTABLE, POSTGRESQL_SNAP_NAME, + SECRET_CACHE_LABEL, + SECRET_DELETED_LABEL, + SECRET_INTERNAL_LABEL, + SECRET_KEY_OVERRIDES, + SECRET_LABEL, SNAP_PACKAGES, SNAP_TMP_DIR, + UNIT_SCOPE, ) from relations.backend_database import BackendDatabaseRequires from relations.db import DbProvides @@ -56,6 +70,8 @@ class PgBouncerCharm(CharmBase): def __init__(self, *args): super().__init__(*args) + self.secrets = {APP_SCOPE: {}, UNIT_SCOPE: {}} + self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.remove, self._on_remove) self.framework.observe(self.on.start, self._on_start) @@ -197,6 +213,176 @@ def version(self) -> str: logger.exception("Unable to get Pgbouncer version") return "" + def _normalize_secret_key(self, key: str) -> str: + new_key = key.replace("_", "-") + new_key = new_key.strip("-") + + return new_key + + def _scope_obj(self, scope: str): + if scope == APP_SCOPE: + return self.framework.model.app + if scope == UNIT_SCOPE: + return self.framework.model.unit + + def _juju_secrets_get(self, scope: str) -> Optional[bool]: + """Helper function to get Juju secret.""" + if scope == UNIT_SCOPE: + peer_data = self.peers.unit_databag + else: + peer_data = self.peers.app_databag + + if not peer_data.get(SECRET_INTERNAL_LABEL): + return + + if SECRET_CACHE_LABEL not in self.secrets[scope]: + try: + # NOTE: Secret contents are not yet available! + secret = self.model.get_secret(id=peer_data[SECRET_INTERNAL_LABEL]) + except SecretNotFoundError as e: + logging.debug(f"No secret found for ID {peer_data[SECRET_INTERNAL_LABEL]}, {e}") + return + + logging.debug(f"Secret {peer_data[SECRET_INTERNAL_LABEL]} downloaded") + + # We keep the secret object around -- needed when applying modifications + self.secrets[scope][SECRET_LABEL] = secret + + # We retrieve and cache actual secret data for the lifetime of the event scope + self.secrets[scope][SECRET_CACHE_LABEL] = secret.get_content() + + return bool(self.secrets[scope].get(SECRET_CACHE_LABEL)) + + def _juju_secret_get_key(self, scope: str, key: str) -> Optional[str]: + if not key: + return + + key = SECRET_KEY_OVERRIDES.get(key, self._normalize_secret_key(key)) + + if self._juju_secrets_get(scope): + secret_cache = self.secrets[scope].get(SECRET_CACHE_LABEL) + if secret_cache: + secret_data = secret_cache.get(key) + if secret_data and secret_data != SECRET_DELETED_LABEL: + logging.debug(f"Getting secret {scope}:{key}") + return secret_data + logging.debug(f"No value found for secret {scope}:{key}") + + def get_secret(self, scope: str, key: str) -> Optional[str]: + """Get secret from the secret storage.""" + if scope not in [APP_SCOPE, UNIT_SCOPE]: + raise RuntimeError("Unknown secret scope.") + + if scope == UNIT_SCOPE: + result = self.peers.unit_databag.get(key, None) + else: + result = self.peers.app_databag.get(key, None) + + # TODO change upgrade to switch to secrets once minor version upgrades is done + if result: + return result + + juju_version = JujuVersion.from_environ() + if juju_version.has_secrets: + return self._juju_secret_get_key(scope, key) + + def _juju_secret_set(self, scope: str, key: str, value: str) -> Optional[str]: + """Helper function setting Juju secret.""" + if scope == UNIT_SCOPE: + peer_data = self.peers.unit_databag + else: + peer_data = self.peers.app_databag + self._juju_secrets_get(scope) + + key = SECRET_KEY_OVERRIDES.get(key, self._normalize_secret_key(key)) + + secret = self.secrets[scope].get(SECRET_LABEL) + + # It's not the first secret for the scope, we can re-use the existing one + # that was fetched in the previous call + if secret: + secret_cache = self.secrets[scope][SECRET_CACHE_LABEL] + + if secret_cache.get(key) == value: + logging.debug(f"Key {scope}:{key} has this value defined already") + else: + secret_cache[key] = value + try: + secret.set_content(secret_cache) + except OSError as error: + logging.error( + f"Error in attempt to set {scope}:{key}. " + f"Existing keys were: {list(secret_cache.keys())}. {error}" + ) + return + logging.debug(f"Secret {scope}:{key} was {key} set") + + # We need to create a brand-new secret for this scope + else: + scope_obj = self._scope_obj(scope) + + secret = scope_obj.add_secret({key: value}) + if not secret: + raise RuntimeError(f"Couldn't set secret {scope}:{key}") + + self.secrets[scope][SECRET_LABEL] = secret + self.secrets[scope][SECRET_CACHE_LABEL] = {key: value} + logging.debug(f"Secret {scope}:{key} published (as first). ID: {secret.id}") + peer_data.update({SECRET_INTERNAL_LABEL: secret.id}) + + return self.secrets[scope][SECRET_LABEL].id + + def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str]: + """Set secret from the secret storage.""" + if scope not in [APP_SCOPE, UNIT_SCOPE]: + raise RuntimeError("Unknown secret scope.") + + if not value: + return self.remove_secret(scope, key) + + juju_version = JujuVersion.from_environ() + + if juju_version.has_secrets: + self._juju_secret_set(scope, key, value) + return + if scope == UNIT_SCOPE: + self.peers.unit_databag.update({key: value}) + else: + self.peers.app_databag.update({key: value}) + + def _juju_secret_remove(self, scope: str, key: str) -> None: + """Remove a Juju 3.x secret.""" + self._juju_secrets_get(scope) + + key = SECRET_KEY_OVERRIDES.get(key, self._normalize_secret_key(key)) + + secret = self.secrets[scope].get(SECRET_LABEL) + if not secret: + logging.error(f"Secret {scope}:{key} wasn't deleted: no secrets are available") + return + + secret_cache = self.secrets[scope].get(SECRET_CACHE_LABEL) + if not secret_cache or key not in secret_cache: + logging.error(f"No secret {scope}:{key}") + return + + secret_cache[key] = SECRET_DELETED_LABEL + secret.set_content(secret_cache) + logging.debug(f"Secret {scope}:{key}") + + def remove_secret(self, scope: str, key: str) -> None: + """Removing a secret.""" + if scope not in [APP_SCOPE, UNIT_SCOPE]: + raise RuntimeError("Unknown secret scope.") + + juju_version = JujuVersion.from_environ() + if juju_version.has_secrets: + return self._juju_secret_remove(scope, key) + if scope == UNIT_SCOPE: + del self.peers.unit_databag[key] + else: + del self.peers.app_databag[key] + def _on_start(self, _) -> None: """On Start hook. @@ -401,7 +587,7 @@ def render_prometheus_service(self): rendered = template.render( stats_user=self.backend.stats_user, pgb_service=f"{PGB}-{self.app.name}", - stats_password=self.peers.get_secret("app", MONITORING_PASSWORD_KEY), + stats_password=self.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY), listen_port=self.config["listen_port"], metrics_port=self.config["metrics_port"], ) diff --git a/src/constants.py b/src/constants.py index 668154c42..135abdeee 100644 --- a/src/constants.py +++ b/src/constants.py @@ -35,3 +35,17 @@ MONITORING_PASSWORD_KEY = "monitoring_password" EXTENSIONS_BLOCKING_MESSAGE = "bad relation request - remote app requested extensions, which are unsupported. Please remove this relation." + +SECRET_LABEL = "secret" +SECRET_CACHE_LABEL = "cache" +SECRET_INTERNAL_LABEL = "internal-secret" +SECRET_DELETED_LABEL = "None" + +APP_SCOPE = "app" +UNIT_SCOPE = "unit" + +SECRET_KEY_OVERRIDES = { + "cfg_file": "cfg-file", + "monitoring_password": "monitoring-password", + "auth_file": "auth-file", +} diff --git a/src/relations/backend_database.py b/src/relations/backend_database.py index 058e4f28f..74955ad3e 100644 --- a/src/relations/backend_database.py +++ b/src/relations/backend_database.py @@ -67,6 +67,7 @@ ) from constants import ( + APP_SCOPE, AUTH_FILE_NAME, BACKEND_RELATION_NAME, MONITORING_PASSWORD_KEY, @@ -139,11 +140,9 @@ def _on_database_created(self, event: DatabaseCreatedEvent) -> None: self.initialise_auth_function([self.database.database, PG]) # Add the monitoring user. - if not ( - monitoring_password := self.charm.peers.get_secret("app", MONITORING_PASSWORD_KEY) - ): + if not (monitoring_password := self.charm.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY)): monitoring_password = pgb.generate_password() - self.charm.peers.set_secret("app", MONITORING_PASSWORD_KEY, monitoring_password) + self.charm.set_secret("app", MONITORING_PASSWORD_KEY, monitoring_password) hashed_monitoring_password = pgb.get_hashed_password(self.stats_user, monitoring_password) self.charm.render_auth_file( diff --git a/src/relations/db.py b/src/relations/db.py index fa5c844ff..27cb3ee00 100644 --- a/src/relations/db.py +++ b/src/relations/db.py @@ -111,7 +111,7 @@ WaitingStatus, ) -from constants import EXTENSIONS_BLOCKING_MESSAGE, PG +from constants import APP_SCOPE, EXTENSIONS_BLOCKING_MESSAGE, PG logger = logging.getLogger(__name__) @@ -228,7 +228,7 @@ def _on_relation_joined(self, join_event: RelationJoinedEvent): if self.charm.unit.is_leader(): password = pgb.generate_password() else: - if not (password := self.charm.peers.app_databag.get(user, None)): + if not (password := self.charm.get_secret(APP_SCOPE, user)): join_event.defer() self.update_databags( diff --git a/src/relations/peers.py b/src/relations/peers.py index b7278b361..a54144cd9 100644 --- a/src/relations/peers.py +++ b/src/relations/peers.py @@ -60,7 +60,7 @@ from ops.model import Relation, Unit from ops.pebble import ConnectionError -from constants import PEER_RELATION_NAME +from constants import APP_SCOPE, PEER_RELATION_NAME CFG_FILE_DATABAG_KEY = "cfg_file" AUTH_FILE_DATABAG_KEY = "auth_file" @@ -90,6 +90,8 @@ def __init__(self, charm: CharmBase): self.framework.observe(charm.on[PEER_RELATION_NAME].relation_created, self._on_created) self.framework.observe(charm.on[PEER_RELATION_NAME].relation_joined, self._on_changed) self.framework.observe(charm.on[PEER_RELATION_NAME].relation_changed, self._on_changed) + self.framework.observe(charm.on.secret_changed, self._on_changed) + self.framework.observe(charm.on.secret_remove, self._on_changed) self.framework.observe(charm.on[PEER_RELATION_NAME].relation_departed, self._on_departed) @property @@ -162,7 +164,7 @@ def _on_created(self, event: RelationCreatedEvent): self.update_auth_file(self.charm.read_auth_file()) except FileNotFoundError: # Subordinate leader got recreated - if auth_file := self.get_secret("app", AUTH_FILE_DATABAG_KEY): + if auth_file := self.charm.get_secret(APP_SCOPE, AUTH_FILE_DATABAG_KEY): self.charm.render_auth_file(auth_file) def _on_changed(self, event: RelationChangedEvent): @@ -183,10 +185,10 @@ def _on_changed(self, event: RelationChangedEvent): self.app_databag[LEADER_ADDRESS_KEY] = self.charm.unit_ip return - if cfg := self.get_secret("app", CFG_FILE_DATABAG_KEY): + if cfg := self.charm.get_secret(APP_SCOPE, CFG_FILE_DATABAG_KEY): self.charm.render_pgb_config(PgbConfig(cfg)) - if auth_file := self.get_secret("app", AUTH_FILE_DATABAG_KEY): + if auth_file := self.charm.get_secret(APP_SCOPE, AUTH_FILE_DATABAG_KEY): self.charm.render_auth_file(auth_file) if self.charm.backend.postgres: @@ -208,61 +210,17 @@ def update_connection(self): if self.charm.unit.is_leader(): self.app_databag[LEADER_ADDRESS_KEY] = self.charm.unit_ip - def set_secret(self, scope: str, key: str, value: str): - """Sets secret value. - - Pass in "None" to the value to delete the secret. - - Placeholder method for Juju Secrets interface. - - Args: - scope: scope for data. Can be "unit" or "app". - key: key to set value to - value: value to be set - """ - if scope == "unit": - if not value: - self.unit_databag.pop(key, None) - return - self.unit_databag.update({key: value}) - elif scope == "app": - if not value: - self.app_databag.pop(key, None) - return - self.app_databag.update({key: value}) - else: - raise RuntimeError("Unknown secret scope.") - - def get_secret(self, scope: str, key: str) -> Optional[str]: - """Gets secret value. - - Placeholder method for Juju Secrets interface. - - Args: - scope: scope for data. Can be "unit" or "app". - key: key to access data - - Returns: - value at `key` in `scope` databag. - """ - if scope == "unit": - return self.unit_databag.get(key, None) - elif scope == "app": - return self.app_databag.get(key, None) - else: - raise RuntimeError("Unknown secret scope.") - def update_cfg(self, cfg: PgbConfig) -> None: """Writes cfg to app databag if leader.""" if not self.charm.unit.is_leader() or not self.relation: return - self.set_secret("app", CFG_FILE_DATABAG_KEY, cfg.render()) + self.charm.set_secret(APP_SCOPE, CFG_FILE_DATABAG_KEY, cfg.render()) logger.debug("updated config file in peer databag") def get_cfg(self) -> PgbConfig: """Retrieves the pgbouncer config from the peer databag.""" - if cfg := self.get_secret("app", CFG_FILE_DATABAG_KEY): + if cfg := self.charm.get_secret(APP_SCOPE, CFG_FILE_DATABAG_KEY): return PgbConfig(cfg) else: return None @@ -272,7 +230,7 @@ def update_auth_file(self, auth_file: str) -> None: if not self.charm.unit.is_leader() or not self.relation: return - self.set_secret("app", AUTH_FILE_DATABAG_KEY, auth_file) + self.charm.set_secret(APP_SCOPE, AUTH_FILE_DATABAG_KEY, auth_file) logger.debug("updated auth file in peer databag") def add_user(self, username: str, password: str): @@ -280,11 +238,11 @@ def add_user(self, username: str, password: str): if not self.charm.unit.is_leader(): return - self.set_secret("app", username, password) + self.charm.set_secret(APP_SCOPE, username, password) def remove_user(self, username: str): """Removes user from app databag.""" if not self.charm.unit.is_leader(): return - self.set_secret("app", username, None) + self.charm.set_secret(APP_SCOPE, username, None) diff --git a/tests/unit/relations/test_backend_database.py b/tests/unit/relations/test_backend_database.py index 000beb187..6ed68715c 100644 --- a/tests/unit/relations/test_backend_database.py +++ b/tests/unit/relations/test_backend_database.py @@ -32,7 +32,7 @@ def setUp(self): self.peers_rel_id = self.harness.add_relation(PEER_RELATION_NAME, "pgbouncer/0") self.harness.add_relation_unit(self.peers_rel_id, self.unit) - @patch("charm.Peers.get_secret", return_value=None) + @patch("charm.PgBouncerCharm.get_secret", return_value=None) @patch("charm.PgBouncerCharm.render_prometheus_service") @patch( "relations.backend_database.BackendDatabaseRequires.auth_user", diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 8902cee09..b11443f55 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -4,7 +4,7 @@ import os import unittest from copy import deepcopy -from unittest.mock import MagicMock, PropertyMock, call, patch +from unittest.mock import MagicMock, Mock, PropertyMock, call, patch import ops.testing from charms.operator_libs_linux.v1 import systemd @@ -17,8 +17,13 @@ from constants import ( BACKEND_RELATION_NAME, INI_NAME, + PEER_RELATION_NAME, PGB_CONF_DIR, PGB_LOG_DIR, + SECRET_CACHE_LABEL, + SECRET_DELETED_LABEL, + SECRET_INTERNAL_LABEL, + SECRET_LABEL, SNAP_PACKAGES, ) from tests.helpers import patch_network_get @@ -34,11 +39,14 @@ class TestCharm(unittest.TestCase): def setUp(self): self.harness = Harness(PgBouncerCharm) self.addCleanup(self.harness.cleanup) - self.harness.begin() + with patch("charm.systemd"): + self.harness.begin_with_initial_hooks() self.charm = self.harness.charm self.unit = self.harness.charm.unit + self.rel_id = self.harness.model.relations[PEER_RELATION_NAME][0].id + @patch("builtins.open", unittest.mock.mock_open()) @patch("charm.PgBouncerCharm._install_snap_packages") @patch("charms.operator_libs_linux.v1.systemd.service_stop") @@ -304,3 +312,102 @@ def test_render_pgb_config(self, _render, _reload): self.charm.render_pgb_config(cfg, reload_pgbouncer=True) _reload.assert_called_once() + + @patch_network_get(private_address="1.1.1.1") + def test_get_secret(self): + # Test application scope. + assert self.charm.get_secret("app", "password") is None + self.harness.update_relation_data( + self.rel_id, self.charm.app.name, {"password": "test-password"} + ) + assert self.charm.get_secret("app", "password") == "test-password" + + # Test unit scope. + assert self.charm.get_secret("unit", "password") is None + self.harness.update_relation_data( + self.rel_id, self.charm.unit.name, {"password": "test-password"} + ) + assert self.charm.get_secret("unit", "password") == "test-password" + + @patch("ops.charm.model.Model.get_secret") + @patch("charm.JujuVersion.has_secrets", new_callable=PropertyMock, return_value=True) + @patch_network_get(private_address="1.1.1.1") + def test_get_secret_juju(self, _, _get_secret): + _get_secret.return_value.get_content.return_value = {"password": "test-password"} + + # clean the caches + if SECRET_INTERNAL_LABEL in self.charm.peers.app_databag: + del self.charm.app_peer_data[SECRET_INTERNAL_LABEL] + self.charm.secrets["app"] = {} + + # Test application scope. + assert self.charm.get_secret("app", "password") is None + self.harness.update_relation_data( + self.rel_id, self.charm.app.name, {SECRET_INTERNAL_LABEL: "secret_key"} + ) + assert self.charm.get_secret("app", "password") == "test-password" + _get_secret.assert_called_once_with(id="secret_key") + + _get_secret.reset_mock() + + # Test unit scope. + assert self.charm.get_secret("unit", "password") is None + self.harness.update_relation_data( + self.rel_id, self.charm.unit.name, {SECRET_INTERNAL_LABEL: "secret_key"} + ) + assert self.charm.get_secret("unit", "password") == "test-password" + _get_secret.assert_called_once_with(id="secret_key") + + def test_set_secret(self): + # Test application scope. + assert "password" not in self.harness.get_relation_data(self.rel_id, self.charm.app.name) + self.charm.set_secret("app", "password", "test-password") + assert ( + self.harness.get_relation_data(self.rel_id, self.charm.app.name)["password"] + == "test-password" + ) + self.charm.set_secret("app", "password", None) + assert "password" not in self.harness.get_relation_data(self.rel_id, self.charm.app.name) + + # Test unit scope. + assert "password" not in self.harness.get_relation_data(self.rel_id, self.charm.unit.name) + self.charm.set_secret("unit", "password", "test-password") + assert ( + self.harness.get_relation_data(self.rel_id, self.charm.unit.name)["password"] + == "test-password" + ) + self.charm.set_secret("unit", "password", None) + assert "password" not in self.harness.get_relation_data(self.rel_id, self.charm.unit.name) + + @patch("charm.JujuVersion.has_secrets", new_callable=PropertyMock, return_value=True) + def test_set_secret_juju(self, _): + secret_mock = Mock() + self.charm.secrets["app"][SECRET_LABEL] = secret_mock + self.charm.secrets["app"][SECRET_CACHE_LABEL] = {} + self.charm.secrets["unit"][SECRET_LABEL] = secret_mock + self.charm.secrets["unit"][SECRET_CACHE_LABEL] = {} + + # Test application scope. + assert "password" not in self.charm.secrets["app"].get(SECRET_CACHE_LABEL, {}) + self.charm.set_secret("app", "password", "test-password") + assert self.charm.secrets["app"][SECRET_CACHE_LABEL]["password"] == "test-password" + secret_mock.set_content.assert_called_once_with( + self.charm.secrets["app"][SECRET_CACHE_LABEL] + ) + secret_mock.reset_mock() + + self.charm.set_secret("app", "password", None) + assert self.charm.secrets["app"][SECRET_CACHE_LABEL]["password"] == SECRET_DELETED_LABEL + secret_mock.set_content.assert_called_once_with( + self.charm.secrets["app"][SECRET_CACHE_LABEL] + ) + secret_mock.reset_mock() + + # Test unit scope. + assert "password" not in self.charm.secrets["unit"].get(SECRET_CACHE_LABEL, {}) + self.charm.set_secret("unit", "password", "test-password") + assert self.charm.secrets["unit"][SECRET_CACHE_LABEL]["password"] == "test-password" + secret_mock.set_content.assert_called_once_with( + self.charm.secrets["unit"][SECRET_CACHE_LABEL] + ) + secret_mock.reset_mock()