DPE-4643 ensure username uniqueness (#464)

* ensure username uniqueness

* libs bump

* temporary branch for router

* test against temp branch revision of router

* support for legacy username format

* model uuid as suffix

* trunc at 26chars

* leftover

Author: paulomach

lib/charms/mysql/v0/mysql.py

@@ -1215,29 +1215,28 @@ def delete_users_for_unit(self, unit_name: str) -> None:
             logger.exception(f"Failed to query and delete users for unit {unit_name}")
             raise MySQLDeleteUsersForUnitError(e.message)
 
-    def delete_users_for_relation(self, relation_id: int) -> None:
+    def delete_users_for_relation(self, username: str) -> None:
         """Delete users for a relation.
 
         Args:
-            relation_id: The id of the relation for which to delete mysql users for
+            username: The username do drop
 
         Raises:
             MySQLDeleteUsersForRelationError if there is an error deleting users for the relation
         """
-        user = f"relation-{str(relation_id)}"
         drop_users_command = [
             f"shell.connect_to_primary('{self.server_config_user}:{self.server_config_password}@{self.instance_address}')",
-            f"session.run_sql(\"DROP USER IF EXISTS '{user}'@'%';\")",
+            f"session.run_sql(\"DROP USER IF EXISTS '{username}'@'%';\")",
         ]
         # If the relation is with a MySQL Router charm application, delete any users
         # created by that application.
         drop_users_command.extend(
-            self._get_statements_to_delete_users_with_attribute("created_by_user", f"'{user}'")
+            self._get_statements_to_delete_users_with_attribute("created_by_user", f"'{username}'")
         )
         try:
             self._run_mysqlsh_script("\n".join(drop_users_command))
         except MySQLClientError as e:
-            logger.exception(f"Failed to delete users for relation {relation_id}")
+            logger.exception(f"Failed to delete {username=}")
             raise MySQLDeleteUsersForRelationError(e.message)
 
     def delete_user(self, username: str) -> None:

src/relations/mysql_provider.py

@@ -194,6 +194,20 @@ def _get_or_set_password(self, relation) -> str:
         self.database.update_relation_data(relation.id, {"password": password})
         return password
 
+    def _get_username(self, relation_id: int, legacy: bool = False) -> str:
+        """Generate a unique username for the relation using the model uuid and the relation id.
+
+        Args:
+            relation_id (int): The relation id.
+            legacy (bool): If True, generate a username without the model uuid.
+
+        Returns:
+            str: A valid unique username (max 32 characters long)
+        """
+        if legacy:
+            return f"relation-{relation_id}"
+        return f"relation-{relation_id}_{self.model.uuid.replace('-', '')}"[:26]
+
     def _on_database_requested(self, event: DatabaseRequestedEvent):
         """Handle the `database-requested` event."""
         if not self.charm.unit.is_leader():
@@ -211,7 +225,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent):
         if event.extra_user_roles:
             extra_user_roles = event.extra_user_roles.split(",")
         # user name is derived from the relation id
-        db_user = f"relation-{relation_id}"
+        db_user = self._get_username(relation_id)
         db_pass = self._get_or_set_password(event.relation)
 
         remote_app = event.app.name
@@ -274,8 +288,18 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None:
 
         relation_id = event.relation.id
         try:
-            self.charm._mysql.delete_users_for_relation(relation_id)
-            logger.info(f"Removed user for relation {relation_id}")
+            if self.charm._mysql.does_mysql_user_exist(self._get_username(relation_id), "%"):
+                self.charm._mysql.delete_users_for_relation(self._get_username(relation_id))
+            elif self.charm._mysql.does_mysql_user_exist(
+                self._get_username(relation_id, legacy=True), "%"
+            ):
+                self.charm._mysql.delete_users_for_relation(
+                    self._get_username(relation_id, legacy=True)
+                )
+            else:
+                logger.warning(f"User(s) not found for relation {relation_id}")
+                return
+            logger.info(f"Removed user(s) for relation {relation_id}")
         except (MySQLDeleteUsersForRelationError, KeyError):
             logger.error(f"Failed to delete user for relation {relation_id}")
 

tests/unit/test_database.py

@@ -66,12 +66,15 @@ def test_database_requested(
             self.database_relation_id, "app", {"database": "test_db"}
         )
 
+        username = (
+            f"relation-{self.database_relation_id}_{self.harness.model.uuid.replace('-', '')}"
+        )[:26]
         self.assertEqual(
             database_relation_databag,
             {
                 "data": '{"database": "test_db"}',
                 "password": "super_secure_password",
-                "username": f"relation-{self.database_relation_id}",
+                "username": username,
                 "endpoints": "2.2.2.2:3306",
                 "version": "8.0.29-0ubuntu0.20.04.3",
                 "database": "test_db",

tests/unit/test_mysql.py

@@ -872,7 +872,7 @@ def test_delete_users_for_relation_failure(
         _run_mysqlsh_script.side_effect = MySQLClientError
 
         with self.assertRaises(MySQLDeleteUsersForRelationError):
-            self.mysql.delete_users_for_relation(40)
+            self.mysql.delete_users_for_relation("foouser")
 
     @patch("charms.mysql.v0.mysql.MySQLBase._run_mysqlsh_script")
     def test_delete_user(self, _run_mysqlsh_script):