[DPE-2447] Secret labels (#200)

* Labels (+SecretCache) in Mongo Charm

* Minor unittest changes required

* Integration tests changes required

* Secrets in separate lib

* Changes following PR comments

lib/charms/mongodb/v0/mongodb_secrets.py

@@ -0,0 +1,137 @@
+"""Secrets related helper classes/functions."""
+# Copyright 2023 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+from typing import Dict, Optional
+
+from ops import Secret, SecretInfo
+from ops.charm import CharmBase
+from ops.model import SecretNotFoundError
+
+from config import Config
+from exceptions import SecretAlreadyExistsError
+
+# The unique Charmhub library identifier, never change it
+
+# The unique Charmhub library identifier, never change it
+LIBID = "87456e41c7594240b92b783a648592b5"
+
+# Increment this major API version when introducing breaking changes
+LIBAPI = 0
+
+# Increment this PATCH version before using `charmcraft publish-lib` or reset
+# to 0 if you are raising the major API version
+LIBPATCH = 1
+
+APP_SCOPE = Config.Relations.APP_SCOPE
+UNIT_SCOPE = Config.Relations.UNIT_SCOPE
+Scopes = Config.Relations.Scopes
+
+
+def generate_secret_label(charm: CharmBase, scope: Scopes) -> str:
+    """Generate unique group_mappings for secrets within a relation context.
+
+    Defined as a standalone function, as the choice on secret labels definition belongs to the
+    Application Logic. To be kept separate from classes below, which are simply to provide a
+    (smart) abstraction layer above Juju Secrets.
+    """
+    members = [charm.app.name, scope]
+    return f"{'.'.join(members)}"
+
+
+# Secret cache
+
+
+class CachedSecret:
+    """Abstraction layer above direct Juju access with caching.
+
+    The data structure is precisely re-using/simulating Juju Secrets behavior, while
+    also making sure not to fetch a secret multiple times within the same event scope.
+    """
+
+    def __init__(self, charm: CharmBase, label: str, secret_uri: Optional[str] = None):
+        self._secret_meta = None
+        self._secret_content = {}
+        self._secret_uri = secret_uri
+        self.label = label
+        self.charm = charm
+
+    def add_secret(self, content: Dict[str, str], scope: Scopes) -> Secret:
+        """Create a new secret."""
+        if self._secret_uri:
+            raise SecretAlreadyExistsError(
+                "Secret is already defined with uri %s", self._secret_uri
+            )
+
+        if scope == Config.APP_SCOPE:
+            secret = self.charm.app.add_secret(content, label=self.label)
+        else:
+            secret = self.charm.unit.add_secret(content, label=self.label)
+        self._secret_uri = secret.id
+        self._secret_meta = secret
+        return self._secret_meta
+
+    @property
+    def meta(self) -> Optional[Secret]:
+        """Getting cached secret meta-information."""
+        if self._secret_meta:
+            return self._secret_meta
+
+        if not (self._secret_uri or self.label):
+            return
+
+        try:
+            self._secret_meta = self.charm.model.get_secret(label=self.label)
+        except SecretNotFoundError:
+            if self._secret_uri:
+                self._secret_meta = self.charm.model.get_secret(
+                    id=self._secret_uri, label=self.label
+                )
+        return self._secret_meta
+
+    def get_content(self) -> Dict[str, str]:
+        """Getting cached secret content."""
+        if not self._secret_content:
+            if self.meta:
+                self._secret_content = self.meta.get_content()
+        return self._secret_content
+
+    def set_content(self, content: Dict[str, str]) -> None:
+        """Setting cached secret content."""
+        if self.meta:
+            self.meta.set_content(content)
+            self._secret_content = content
+
+    def get_info(self) -> Optional[SecretInfo]:
+        """Wrapper function for get the corresponding call on the Secret object if any."""
+        if self.meta:
+            return self.meta.get_info()
+
+
+class SecretCache:
+    """A data structure storing CachedSecret objects."""
+
+    def __init__(self, charm):
+        self.charm = charm
+        self._secrets: Dict[str, CachedSecret] = {}
+
+    def get(self, label: str, uri: Optional[str] = None) -> Optional[CachedSecret]:
+        """Getting a secret from Juju Secret store or cache."""
+        if not self._secrets.get(label):
+            secret = CachedSecret(self.charm, label, uri)
+            if secret.meta:
+                self._secrets[label] = secret
+        return self._secrets.get(label)
+
+    def add(self, label: str, content: Dict[str, str], scope: Scopes) -> CachedSecret:
+        """Adding a secret to Juju Secret."""
+        if self._secrets.get(label):
+            raise SecretAlreadyExistsError(f"Secret {label} already exists")
+
+        secret = CachedSecret(self.charm, label)
+        secret.add_secret(content, scope)
+        self._secrets[label] = secret
+        return self._secrets[label]
+
+
+# END: Secret cache

src/charm.py

@@ -25,6 +25,7 @@
 )
 from charms.mongodb.v0.mongodb_backups import S3_RELATION, MongoDBBackups
 from charms.mongodb.v0.mongodb_provider import MongoDBProvider
+from charms.mongodb.v0.mongodb_secrets import SecretCache, generate_secret_label
 from charms.mongodb.v0.mongodb_tls import MongoDBTLS
 from charms.mongodb.v0.users import (
     CHARM_USERS,
@@ -50,7 +51,6 @@
     ModelError,
     Relation,
     RelationDataContent,
-    SecretNotFoundError,
     Unit,
     WaitingStatus,
 )
@@ -66,7 +66,7 @@
 from tenacity import before_log, retry, stop_after_attempt, wait_fixed
 
 from config import Config
-from exceptions import AdminUserCreationError, MissingSecretError, SecretNotAddedError
+from exceptions import AdminUserCreationError, MissingSecretError
 
 logger = logging.getLogger(__name__)
 
@@ -122,7 +122,7 @@ def __init__(self, *args):
             relation_name=Config.Relations.LOGGING,
             container_name=Config.CONTAINER_NAME,
         )
-        self.secrets = {APP_SCOPE: {}, UNIT_SCOPE: {}}
+        self.secrets = SecretCache(self)
 
     # BEGIN: properties
 
@@ -571,20 +571,19 @@ def _on_secret_changed(self, event):
         for backup tool on non-leader units to keep them working with MongoDB. The same workflow
         occurs on TLS certs change.
         """
-        if self._compare_secret_ids(
-            event.secret.id, self.app_peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL)
-        ):
+        label = None
+        if generate_secret_label(self, Config.APP_SCOPE) == event.secret.label:
+            label = generate_secret_label(self, Config.APP_SCOPE)
             scope = APP_SCOPE
-        elif self._compare_secret_ids(
-            event.secret.id, self.unit_peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL)
-        ):
+        elif generate_secret_label(self, Config.UNIT_SCOPE) == event.secret.label:
+            label = generate_secret_label(self, Config.UNIT_SCOPE)
             scope = UNIT_SCOPE
         else:
             logging.debug("Secret %s changed, but it's unknown", event.secret.id)
             return
         logging.debug("Secret %s for scope %s changed, refreshing", event.secret.id, scope)
 
-        self._juju_secrets_get(scope)
+        self.secrets.get(label)
         self._connect_mongodb_exporter()
         self._connect_pbm_agent()
 
@@ -804,120 +803,47 @@ def _set_leader_unit_active_if_needed(self):
         ):
             self.unit.status = ActiveStatus()
 
-    def _juju_secrets_get(self, scope: Scopes) -> Optional[bool]:
-        """Helper function to get Juju secret."""
-        peer_data = self._peer_data(scope)
-
-        if not peer_data.get(Config.Secrets.SECRET_INTERNAL_LABEL):
-            return
-
-        if Config.Secrets.SECRET_CACHE_LABEL not in self.secrets[scope]:
-            try:
-                # NOTE: Secret contents are not yet available!
-                secret = self.model.get_secret(id=peer_data[Config.Secrets.SECRET_INTERNAL_LABEL])
-            except SecretNotFoundError as e:
-                logging.debug(
-                    f"No secret found for ID {peer_data[Config.Secrets.SECRET_INTERNAL_LABEL]}, {e}"
-                )
-                return
-
-            logging.debug(f"Secret {peer_data[Config.Secrets.SECRET_INTERNAL_LABEL]} downloaded")
-
-            # We keep the secret object around -- needed when applying modifications
-            self.secrets[scope][Config.Secrets.SECRET_LABEL] = secret
-
-            # We retrieve and cache actual secret data for the lifetime of the event scope
-            self.secrets[scope][Config.Secrets.SECRET_CACHE_LABEL] = secret.get_content()
-
-        if self.secrets[scope].get(Config.Secrets.SECRET_CACHE_LABEL):
-            return True
-        return False
-
-    def _juju_secret_get_key(self, scope: Scopes, key: str) -> Optional[str]:
-        if not key:
-            return
-
-        if self._juju_secrets_get(scope):
-            secret_cache = self.secrets[scope].get(Config.Secrets.SECRET_CACHE_LABEL)
-            if secret_cache:
-                secret_data = secret_cache.get(key)
-                if secret_data and secret_data != Config.Secrets.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: Scopes, key: str) -> Optional[str]:
         """Getting a secret."""
-        return self._juju_secret_get_key(scope, key)
-
-    def _juju_secret_set(self, scope: Scopes, key: str, value: str) -> str:
-        """Helper function setting Juju secret."""
-        peer_data = self._peer_data(scope)
-        self._juju_secrets_get(scope)
-
-        secret = self.secrets[scope].get(Config.Secrets.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][Config.Secrets.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}"
-                    )
-                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_opj(scope)
-
-            secret = scope_obj.add_secret({key: value})
-            if not secret:
-                raise SecretNotAddedError(f"Couldn't set secret {scope}:{key}")
-
-            self.secrets[scope][Config.Secrets.SECRET_LABEL] = secret
-            self.secrets[scope][Config.Secrets.SECRET_CACHE_LABEL] = {key: value}
-            logging.debug(f"Secret {scope}:{key} published (as first). ID: {secret.id}")
-            peer_data.update({Config.Secrets.SECRET_INTERNAL_LABEL: secret.id})
+        label = generate_secret_label(self, scope)
+        secret = self.secrets.get(label)
+        if not secret:
+            return
 
-        return self.secrets[scope][Config.Secrets.SECRET_LABEL].id
+        value = secret.get_content().get(key)
+        if value != Config.Secrets.SECRET_DELETED_LABEL:
+            return value
 
     def set_secret(self, scope: Scopes, key: str, value: Optional[str]) -> Optional[str]:
         """(Re)defining a secret."""
         if not value:
             return self.remove_secret(scope, key)
 
-        return self._juju_secret_set(scope, key, value)
-
-    def _juju_secret_remove(self, scope: Scopes, key: str) -> None:
-        """Remove a Juju 3.x secret."""
-        self._juju_secrets_get(scope)
+        label = generate_secret_label(self, scope)
+        secret = self.secrets.get(label)
+        if not secret:
+            self.secrets.add(label, {key: value}, scope)
+        else:
+            content = secret.get_content()
+            content.update({key: value})
+            secret.set_content(content)
+        return label
 
-        secret = self.secrets[scope].get(Config.Secrets.SECRET_LABEL)
+    def remove_secret(self, scope, key) -> None:
+        """Removing a secret."""
+        label = generate_secret_label(self, scope)
+        secret = self.secrets.get(label)
         if not secret:
-            logging.error(f"Secret {scope}:{key} wasn't deleted: no secrets are available")
             return
 
-        secret_cache = self.secrets[scope].get(Config.Secrets.SECRET_CACHE_LABEL)
-        if not secret_cache or key not in secret_cache:
-            logging.error(f"No secret {scope}:{key}")
-            return
+        content = secret.get_content()
 
-        secret_cache[key] = Config.Secrets.SECRET_DELETED_LABEL
-        secret.set_content(secret_cache)
-        logging.debug(f"Secret {scope}:{key}")
+        if not content.get(key) or content[key] == Config.Secrets.SECRET_DELETED_LABEL:
+            logger.error(f"Non-existing secret {scope}:{key} was attempted to be removed.")
+            return
 
-    def remove_secret(self, scope, key) -> None:
-        """Removing a secret."""
-        return self._juju_secret_remove(scope, key)
+        content[key] = Config.Secrets.SECRET_DELETED_LABEL
+        secret.set_content(content)
 
     def restart_mongod_service(self):
         """Restart mongod service."""

src/exceptions.py

@@ -38,3 +38,9 @@ class MissingSecretError(MongoSecretError):
     """Could be raised when a Juju 3 mandatory secret couldn't be found."""
 
     pass
+
+
+class SecretAlreadyExistsError(MongoSecretError):
+    """A secret that we want to create already exists."""
+
+    pass

tests/integration/test_charm.py

@@ -149,6 +149,16 @@ async def test_reset_and_get_password_secret_same_as_cli(ops_test: OpsTest) -> N
     # Getting back the pw programmatically
     password = await get_password(ops_test, unit_id=leader_id, username="monitor")
 
+    #
+    # No way to retrieve a secet by label for now (https://bugs.launchpad.net/juju/+bug/2037104)
+    # Therefore we take advantage of the fact, that we only have ONE single secret a this point
+    # So we take the single member of the list
+    # NOTE: This would BREAK if for instance units had secrets at the start...
+    #
+    complete_command = "list-secrets"
+    _, stdout, _ = await ops_test.juju(*complete_command.split())
+    secret_id = stdout.split("\n")[1].split(" ")[0]
+
     # Getting back the pw from juju CLI
     complete_command = f"show-secret {secret_id} --reveal --format=json"
     _, stdout, _ = await ops_test.juju(*complete_command.split())