Skip to content

Commit

Permalink
DPE-4643 ensure username uniqueness (#464)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
paulomach authored Jul 5, 2024
1 parent 2d3059c commit 90bb1fb
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 11 deletions.
11 changes: 5 additions & 6 deletions lib/charms/mysql/v0/mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
30 changes: 27 additions & 3 deletions src/relations/mysql_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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
Expand Down Expand Up @@ -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}")

Expand Down
5 changes: 4 additions & 1 deletion tests/unit/test_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_mysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 90bb1fb

Please sign in to comment.