DPE-3264 refactor for in lib secrets (#385)

* (wip) migrating

* base charm testing module

* secrets tests

* lint fixes

* remove fallack key

* using correct secret label

* bump lib and test fix

* bump agent versions

* current 2.9.x
* current 3.1.x (secret fix in lib

.github/workflows/ci.yaml

@@ -65,9 +65,9 @@ jobs:
       fail-fast: false
       matrix:
         juju:
-          - agent: 2.9.45
+          - agent: 2.9.46
             libjuju: ^2
-          - agent: 3.1.6
+          - agent: 3.1.7
     name: Integration test charm | ${{ matrix.juju.agent }}
     needs:
       - lint

lib/charms/mysql/v0/mysql.py

@@ -75,16 +75,11 @@ def wait_until_mysql_connection(self) -> None:
 import socket
 import time
 from abc import ABC, abstractmethod
-from typing import Any, Dict, Iterable, List, Optional, Tuple, Union
+from typing import Any, Dict, Iterable, List, Optional, Tuple, Union, get_args
 
 import ops
-from charms.data_platform_libs.v0.data_secrets import (
-    APP_SCOPE,
-    UNIT_SCOPE,
-    Scopes,
-    SecretCache,
-    generate_secret_label,
-)
+from charms.data_platform_libs.v0.data_interfaces import DataPeer, DataPeerUnit
+from charms.data_platform_libs.v0.data_secrets import APP_SCOPE, UNIT_SCOPE, Scopes, SecretCache
 from ops.charm import ActionEvent, CharmBase, RelationBrokenEvent
 from ops.model import Unit
 from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed, wait_random
@@ -101,7 +96,6 @@ def wait_until_mysql_connection(self) -> None:
     PEER,
     ROOT_PASSWORD_KEY,
     ROOT_USERNAME,
-    SECRET_ID_KEY,
     SERVER_CONFIG_PASSWORD_KEY,
     SERVER_CONFIG_USERNAME,
 )
@@ -117,7 +111,7 @@ def wait_until_mysql_connection(self) -> None:
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 53
+LIBPATCH = 54
 
 UNIT_TEARDOWN_LOCKNAME = "unit-teardown"
 UNIT_ADD_LOCKNAME = "unit-add"
@@ -129,6 +123,9 @@ def wait_until_mysql_connection(self) -> None:
 RECOVERY_CHECK_TIME = 10  # seconds
 GET_MEMBER_STATE_TIME = 10  # seconds
 
+SECRET_INTERNAL_LABEL = "secret-id"
+SECRET_DELETED_LABEL = "None"
+
 
 class Error(Exception):
     """Base class for exceptions in this module."""
@@ -379,6 +376,32 @@ def __init__(self, *args):
         super().__init__(*args)
 
         self.secrets = SecretCache(self)
+        self.peer_relation_app = DataPeer(
+            self,
+            relation_name=PEER,
+            additional_secret_fields=[
+                ROOT_PASSWORD_KEY,
+                SERVER_CONFIG_PASSWORD_KEY,
+                MONITORING_PASSWORD_KEY,
+                CLUSTER_ADMIN_PASSWORD_KEY,
+                BACKUPS_PASSWORD_KEY,
+            ],
+            secret_field_name=SECRET_INTERNAL_LABEL,
+            deleted_label=SECRET_DELETED_LABEL,
+        )
+        self.peer_relation_unit = DataPeerUnit(
+            self,
+            relation_name=PEER,
+            additional_secret_fields=[
+                "key",
+                "csr",
+                "cert",
+                "cauth",
+                "chain",
+            ],
+            secret_field_name=SECRET_INTERNAL_LABEL,
+            deleted_label=SECRET_DELETED_LABEL,
+        )
 
         self.framework.observe(self.on.get_cluster_status_action, self._get_cluster_status)
         self.framework.observe(self.on.get_password_action, self._on_get_password)
@@ -394,6 +417,11 @@ def _mysql(self) -> "MySQLBase":
         """Return the MySQL instance."""
         raise NotImplementedError
 
+    @abstractmethod
+    def get_unit_hostname(self):
+        """Return unit hostname."""
+        raise NotImplementedError
+
     def _on_get_password(self, event: ActionEvent) -> None:
         """Action used to retrieve the system user's password."""
         username = event.params.get("username") or ROOT_USERNAME
@@ -536,142 +564,51 @@ def has_cos_relation(self) -> bool:
 
         return len(active_cos_relations) > 0
 
-    def _scope_obj(self, scope: Scopes):
-        if scope == APP_SCOPE:
-            return self.app
-        if scope == UNIT_SCOPE:
-            return self.unit
-
-    def _peer_data(self, scope: Scopes) -> Dict:
-        """Return corresponding databag for app/unit."""
-        if self.peers is None:
-            return {}
-        return self.peers.data[self._scope_obj(scope)]
-
-    def _safe_get_secret(self, scope: Scopes, label: str) -> SecretCache:
-        """Safety measure, for upgrades between versions.
-
-        Based on secret URI usage to others with labels usage.
-        If the secret can't be retrieved by label, we search for the uri -- and
-        if found, we "stick" the label on the secret for further usage.
-        """
-        secret_uri = self._peer_data(scope).get(SECRET_ID_KEY, None)
-        secret = self.secrets.get(label, secret_uri)
-
-        # Since now we switched to labels, the databag reference can be removed
-        if secret_uri and secret and scope == APP_SCOPE and self.unit.is_leader():
-            self._peer_data(scope).pop(SECRET_ID_KEY, None)
-        return secret
-
-    def _get_secret_from_juju(self, scope: Scopes, key: str) -> Optional[str]:
-        """Retrieve and return the secret from the juju secret storage."""
-        label = generate_secret_label(self, scope)
-        secret = self._safe_get_secret(scope, label)
-
-        if not secret:
-            logger.debug("Getting a secret when secret is not added in juju")
-            return
-
-        value = secret.get_content().get(key)
-        return value
-
-    def _get_secret_from_databag(self, scope: str, key: str) -> Optional[str]:
-        """Retrieve and return the secret from the peer relation databag."""
-        if scope == "unit":
-            return self.unit_peer_data.get(key)
-
-        return self.app_peer_data.get(key)
-
     def get_secret(
-        self, scope: Scopes, key: str, fallback_key: Optional[str] = None
+        self,
+        scope: Scopes,
+        key: str,
     ) -> Optional[str]:
         """Get secret from the secret storage.
 
         Retrieve secret from juju secrets backend if secret exists there.
-        Else retrieve from peer databag (with key or fallback_key). This is to
-        account for cases where secrets are stored in peer databag but the charm
-        is then refreshed to a newer revision.
+        Else retrieve from peer databag. This is to account for cases where secrets are stored in
+        peer databag but the charm is then refreshed to a newer revision.
         """
-        if scope not in ["app", "unit"]:
-            raise MySQLSecretError(f"Invalid secret scope: {scope}")
-
-        if ops.jujuversion.JujuVersion.from_environ().has_secrets:
-            secret = self._get_secret_from_juju(scope, key)
-            if secret:
-                return secret
-
-        return self._get_secret_from_databag(scope, key) or self._get_secret_from_databag(
-            scope, fallback_key
-        )
-
-    def _set_secret_in_databag(self, scope: Scopes, key: str, value: Optional[str]) -> None:
-        """Set secret in the peer relation databag."""
-        if not value:
-            try:
-                self._peer_data(scope).pop(key)
-                return
-            except KeyError:
-                logger.error(f"Non-existing secret {scope}:{key} was attempted to be removed.")
-                return
-
-        self._peer_data(scope)[key] = value
-
-    def _set_secret_in_juju(self, scope: Scopes, key: str, value: Optional[str]) -> None:
-        """Set the secret in the juju secret storage."""
-        # Charm could have been upgraded since last run
-        # We make an attempt to remove potential traces from the databag
-        self._peer_data(scope).pop(key, None)
-
-        label = generate_secret_label(self, scope)
-        secret = self._safe_get_secret(scope, label)
-        if not secret and value:
-            self.secrets.add(label, {key: value}, scope)
-            return
-
-        content = secret.get_content() if secret else None
-
-        if not value:
-            if content and key in content:
-                content.pop(key, None)
-            else:
-                logger.error(f"Non-existing secret {scope}:{key} was attempted to be removed.")
-                return
-        else:
-            content.update({key: value})
-
-        # Temporary solution: this should come from the shared lib
-        # Improved after https://warthogs.atlassian.net/browse/DPE-3056 is resolved
-        if content:
-            secret.set_content(content)
+        peers = self.model.get_relation(PEER)
+        if scope == APP_SCOPE:
+            value = self.peer_relation_app.fetch_my_relation_field(peers.id, key)
         else:
-            secret.meta.remove_all_revisions()
+            value = self.peer_relation_unit.fetch_my_relation_field(peers.id, key)
+        return value
 
-    def set_secret(
-        self, scope: Scopes, key: str, value: Optional[str], fallback_key: Optional[str] = None
-    ) -> None:
+    def set_secret(self, scope: Scopes, key: str, value: Optional[str]) -> None:
         """Set a secret in the secret storage."""
-        if scope not in ["app", "unit"]:
-            raise MySQLSecretError(f"Invalid secret scope: {scope}")
+        if scope not in get_args(Scopes):
+            raise MySQLSecretError(f"Invalid secret {scope=}")
 
-        if scope == "app" and not self.unit.is_leader():
+        if scope == APP_SCOPE and not self.unit.is_leader():
             raise MySQLSecretError("Can only set app secrets on the leader unit")
 
-        if ops.jujuversion.JujuVersion.from_environ().has_secrets:
-            self._set_secret_in_juju(scope, key, value)
-
-            # for refresh from juju <= 3.1.4 to >= 3.1.5, we need to clear out
-            # secrets from the databag as well
-            if self._get_secret_from_databag(scope, key):
-                self._set_secret_in_databag(scope, key, None)
+        if not value:
+            return self.remove_secret(scope, key)
 
-            if fallback_key and self._get_secret_from_databag(scope, fallback_key):
-                self._set_secret_in_databag(scope, key, None)
+        peers = self.model.get_relation(PEER)
+        if scope == APP_SCOPE:
+            self.peer_relation_app.update_relation_data(peers.id, {key: value})
+        elif scope == UNIT_SCOPE:
+            self.peer_relation_unit.update_relation_data(peers.id, {key: value})
 
-            return
+    def remove_secret(self, scope: Scopes, key: str) -> None:
+        """Removing a secret."""
+        if scope not in get_args(Scopes):
+            raise RuntimeError("Unknown secret scope.")
 
-        self._set_secret_in_databag(scope, key, value)
-        if fallback_key:
-            self._set_secret_in_databag(scope, fallback_key, None)
+        peers = self.model.get_relation(PEER)
+        if scope == APP_SCOPE:
+            self.peer_relation_app.delete_relation_data(peers.id, [key])
+        else:
+            self.peer_relation_unit.delete_relation_data(peers.id, [key])
 
 
 class MySQLMemberState(str, enum.Enum):
@@ -2555,6 +2492,7 @@ def check_mysqlsh_connection(self) -> bool:
             self._run_mysqlsh_script("\n".join(connect_commands))
             return True
         except MySQLClientError:
+            logger.exception("Failed to connect to MySQL with mysqlsh")
             return False
 
     def get_pid_of_port_3306(self) -> Optional[str]:

lib/charms/mysql/v0/tls.py

@@ -24,6 +24,7 @@
 import logging
 import re
 import socket
+import typing
 from typing import List, Optional, Tuple
 
 import ops
@@ -35,7 +36,7 @@
     generate_csr,
     generate_private_key,
 )
-from ops.charm import ActionEvent, CharmBase
+from ops.charm import ActionEvent
 from ops.framework import Object
 from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
 
@@ -50,18 +51,19 @@
 logger = logging.getLogger(__name__)
 
 LIBID = "eb73947deedd4380a3a90d527e0878eb"
-
 LIBAPI = 0
-
-LIBPATCH = 3
+LIBPATCH = 4
 
 SCOPE = "unit"
 
+if typing.TYPE_CHECKING:
+    from .mysql import MySQLCharmBase
+
 
 class MySQLTLS(Object):
     """MySQL TLS Provider class."""
 
-    def __init__(self, charm: CharmBase):
+    def __init__(self, charm: "MySQLCharmBase"):
         super().__init__(charm, "certificates")
         self.charm = charm
 
@@ -119,8 +121,8 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None:
         self.charm.set_secret(
             SCOPE, "chain", "\n".join(event.chain) if event.chain is not None else None
         )
-        self.charm.set_secret(SCOPE, "certificate", event.certificate, fallback_key="cert")
-        self.charm.set_secret(SCOPE, "certificate-authority", event.ca, fallback_key="ca")
+        self.charm.set_secret(SCOPE, "certificate", event.certificate)
+        self.charm.set_secret(SCOPE, "certificate-authority", event.ca)
 
         self.push_tls_files_to_workload()
         try:
@@ -145,7 +147,7 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None:
 
     def _on_certificate_expiring(self, event: CertificateExpiringEvent) -> None:
         """Request the new certificate when old certificate is expiring."""
-        if event.certificate != self.charm.get_secret(SCOPE, "certificate", fallback_key="cert"):
+        if event.certificate != self.charm.get_secret(SCOPE, "certificate"):
             logger.error("An unknown certificate expiring.")
             return
 
@@ -167,8 +169,8 @@ def _on_tls_relation_broken(self, _) -> None:
         """Disable TLS when TLS relation broken."""
         try:
             if not ops.jujuversion.JujuVersion.from_environ().has_secrets:
-                self.charm.set_secret(SCOPE, "certificate-authority", None, fallback_key="ca")
-                self.charm.set_secret(SCOPE, "certificate", None, fallback_key="cert")
+                self.charm.set_secret(SCOPE, "certificate-authority", None)
+                self.charm.set_secret(SCOPE, "certificate", None)
                 self.charm.set_secret(SCOPE, "chain", None)
         except KeyError:
             # ignore key error for unit teardown
@@ -237,12 +239,12 @@ def get_tls_content(self) -> Tuple[Optional[str], Optional[str], Optional[str]]:
         Returns:
             A tuple of strings with the content of server-key, ca and server-cert
         """
-        ca = self.charm.get_secret(SCOPE, "certificate-authority", fallback_key="ca")
+        ca = self.charm.get_secret(SCOPE, "certificate-authority")
         chain = self.charm.get_secret(SCOPE, "chain")
         ca_file = chain or ca
 
         key = self.charm.get_secret(SCOPE, "key")
-        cert = self.charm.get_secret(SCOPE, "certificate", fallback_key="cert")
+        cert = self.charm.get_secret(SCOPE, "certificate")
         return key, ca_file, cert
 
     def push_tls_files_to_workload(self) -> None:

src/constants.py

@@ -46,5 +46,4 @@
 ROOT_SYSTEM_USER = "root"
 GR_MAX_MEMBERS = 9
 HOSTNAME_DETAILS = "hostname-details"
-SECRET_ID_KEY = "secret-id"
 COS_AGENT_RELATION_NAME = "cos-agent"

src/mysql_vm_helpers.py

@@ -162,7 +162,9 @@ def install_and_configure_mysql_dependencies() -> None:
 
         try:
             # install the charmed-mysql snap
-            logger.debug("Installing charmed-mysql snap")
+            logger.debug(
+                f"Installing {CHARMED_MYSQL_SNAP_NAME} revision {CHARMED_MYSQL_SNAP_REVISION}"
+            )
             charmed_mysql.ensure(snap.SnapState.Present, revision=CHARMED_MYSQL_SNAP_REVISION)
             if not charmed_mysql.held:
                 # hold the snap in charm determined revision