diff --git a/docs/explanation/roles.md b/docs/explanation/roles.md index fe13887ef..a113a3a13 100644 --- a/docs/explanation/roles.md +++ b/docs/explanation/roles.md @@ -53,12 +53,14 @@ Charmed MySQL also introduces database level roles, with permissions tied to eac Example for a database named `test`: ```text -mysql> SELECT host, user FROM mysql.user WHERE user LIKE '%_test'; -+-----------+------------------+ -| host | user | -+-----------+------------------+ -| % | charmed_dba_test | -+-----------+------------------+ +mysql> SELECT host, user FROM mysql.user WHERE user LIKE '%_test_%'; ++-----------+---------------------+ +| host | user | ++-----------+---------------------+ +| % | charmed_dba_test_00 | ++-----------+---------------------+ ``` -The `charmed_dba_` role contains every data and schema related permission, scoped to the database it references. +The `charmed_dba__` role contains every data and schema related permission, scoped to the database it references. +The numeric part is introduced in order to differentiate across DBA roles whose first set of characters is the same, +given that database names will be pruned in order to fit into the MySQL role length limit (32 characters). diff --git a/lib/charms/mysql/v0/mysql.py b/lib/charms/mysql/v0/mysql.py index cd08144ba..688390dcf 100644 --- a/lib/charms/mysql/v0/mysql.py +++ b/lib/charms/mysql/v0/mysql.py @@ -127,7 +127,7 @@ def wait_until_mysql_connection(self) -> None: # Increment this major API version when introducing breaking changes LIBAPI = 0 -LIBPATCH = 94 +LIBPATCH = 95 UNIT_TEARDOWN_LOCKNAME = "unit-teardown" UNIT_ADD_LOCKNAME = "unit-add" @@ -1209,6 +1209,21 @@ def render_mysqld_configuration( # noqa: C901 config.write(string_io) return string_io.getvalue(), dict(config["mysqld"]) + def _build_mysql_database_dba_role(self, database: str) -> str: + """Builds the database-level DBA role, given length constraints.""" + role_prefix = "charmed_dba" + role_suffix = "XX" + + role_name_available = ROLE_MAX_LENGTH - len(role_prefix) - len(role_suffix) - 2 + role_name_description = database[:role_name_available] + role_name_collisions = self.list_mysql_roles(f"{role_prefix}_{role_name_description}_%") + + return "_".join(( + role_prefix, + role_name_description, + str(len(role_name_collisions)).zfill(len(role_suffix)), + )) + def configure_mysql_router_roles(self) -> None: """Configure the MySQL Router roles for the instance.""" for role in (LEGACY_ROLE_ROUTER, MODERN_ROLE_ROUTER): @@ -1513,29 +1528,35 @@ def configure_mysqlrouter_user( logger.error(f"Failed to configure mysqlrouter {username=}") raise MySQLConfigureRouterUserError from e + @retry( + reraise=True, + stop=stop_after_attempt(3), + retry=retry_if_exception_type(MySQLCreateApplicationDatabaseError), + ) def create_database(self, database: str) -> None: """Create an application database.""" - role_name = f"charmed_dba_{database}" + databases = self.get_non_system_databases() + if database in databases: + return - if len(role_name) >= ROLE_MAX_LENGTH: - logger.error(f"Failed to create application database {database}") - raise MySQLCreateApplicationDatabaseError("Role name longer than 32 characters") + role_name = self._build_mysql_database_dba_role(database) - create_database_commands = ( + create_commands = ( "shell.connect_to_primary()", - f'session.run_sql("CREATE DATABASE IF NOT EXISTS `{database}`;")', + f'session.run_sql("CREATE ROLE `{role_name}`;")', + f'session.run_sql("CREATE DATABASE `{database}`;")', + ) + grant_commands = ( f'session.run_sql("GRANT SELECT ON `{database}`.* TO {ROLE_READ};")', f'session.run_sql("GRANT SELECT, INSERT, DELETE, UPDATE ON `{database}`.* TO {ROLE_DML};")', - ) - create_dba_role_commands = ( - f'session.run_sql("CREATE ROLE IF NOT EXISTS `{role_name}`;")', f'session.run_sql("GRANT SELECT, INSERT, DELETE, UPDATE, EXECUTE ON `{database}`.* TO {role_name};")', f'session.run_sql("GRANT ALTER, ALTER ROUTINE, CREATE, CREATE ROUTINE, CREATE VIEW, DROP, INDEX, LOCK TABLES, REFERENCES, TRIGGER ON `{database}`.* TO {role_name};")', ) try: + logger.info(f"Creating application {database=} and DBA {role_name=}") self._run_mysqlsh_script( - "\n".join(create_database_commands + create_dba_role_commands), + "\n".join(create_commands + grant_commands), user=self.server_config_user, password=self.server_config_password, host=self.instance_def(self.server_config_user), @@ -1584,6 +1605,7 @@ def create_scoped_user( ) try: + logger.info(f"Creating application scoped user {username}@{hostname}") self._run_mysqlsh_script( "\n".join(create_scoped_user_commands + grant_scoped_user_commands), user=self.server_config_user, diff --git a/tests/integration/roles/test_database_dba_role.py b/tests/integration/roles/test_database_dba_role.py index 0933a379d..c79eadafc 100644 --- a/tests/integration/roles/test_database_dba_role.py +++ b/tests/integration/roles/test_database_dba_role.py @@ -74,7 +74,7 @@ async def test_charmed_dba_role(ops_test: OpsTest): await ops_test.model.applications[f"{INTEGRATOR_APP_NAME}2"].set_config({ "database-name": "throwaway", - "extra-user-roles": "charmed_dba_preserved", + "extra-user-roles": "charmed_dba_preserved_00", }) await ops_test.model.add_relation(f"{INTEGRATOR_APP_NAME}2", DATABASE_APP_NAME) await ops_test.model.wait_for_idle( @@ -143,4 +143,4 @@ async def test_charmed_dba_role(ops_test: OpsTest): assert sorted(rows) == sorted([ "test_data_1", "test_data_2", - ]), "Unexpected data in preserved with charmed_dba_preserved role" + ]), "Unexpected data in preserved with charmed_dba_preserved_00 role" diff --git a/tests/unit/test_mysql.py b/tests/unit/test_mysql.py index d7c0f1849..40da4d7b7 100644 --- a/tests/unit/test_mysql.py +++ b/tests/unit/test_mysql.py @@ -356,19 +356,33 @@ def test_configure_mysqlrouter_user_failure( "test_username", "test_password", "1.1.1.1", "app/0" ) + @patch("charms.mysql.v0.mysql.MySQLBase.get_non_system_databases") + @patch("charms.mysql.v0.mysql.MySQLBase.list_mysql_roles") @patch("charms.mysql.v0.mysql.MySQLBase._run_mysqlsh_script") - def test_create_application_database(self, _run_mysqlsh_script): + def test_create_application_database( + self, _run_mysqlsh_script, _list_mysql_roles, _get_non_system_databases + ): """Test the successful execution of create_application_database.""" + _get_non_system_databases.return_value = {"test_database"} + _list_mysql_roles.return_value = set() + _run_mysqlsh_script.return_value = "" + + self.mysql.create_database("test_database") + + self.assertEqual(_run_mysqlsh_script.call_count, 0) + + _get_non_system_databases.return_value = set() + _list_mysql_roles.return_value = set() _run_mysqlsh_script.return_value = "" _expected_create_scoped_user_commands = "\n".join(( "shell.connect_to_primary()", - 'session.run_sql("CREATE DATABASE IF NOT EXISTS `test_database`;")', + 'session.run_sql("CREATE ROLE `charmed_dba_test_database_00`;")', + 'session.run_sql("CREATE DATABASE `test_database`;")', 'session.run_sql("GRANT SELECT ON `test_database`.* TO charmed_read;")', 'session.run_sql("GRANT SELECT, INSERT, DELETE, UPDATE ON `test_database`.* TO charmed_dml;")', - 'session.run_sql("CREATE ROLE IF NOT EXISTS `charmed_dba_test_database`;")', - 'session.run_sql("GRANT SELECT, INSERT, DELETE, UPDATE, EXECUTE ON `test_database`.* TO charmed_dba_test_database;")', - 'session.run_sql("GRANT ALTER, ALTER ROUTINE, CREATE, CREATE ROUTINE, CREATE VIEW, DROP, INDEX, LOCK TABLES, REFERENCES, TRIGGER ON `test_database`.* TO charmed_dba_test_database;")', + 'session.run_sql("GRANT SELECT, INSERT, DELETE, UPDATE, EXECUTE ON `test_database`.* TO charmed_dba_test_database_00;")', + 'session.run_sql("GRANT ALTER, ALTER ROUTINE, CREATE, CREATE ROUTINE, CREATE VIEW, DROP, INDEX, LOCK TABLES, REFERENCES, TRIGGER ON `test_database`.* TO charmed_dba_test_database_00;")', )) self.mysql.create_database("test_database") @@ -386,20 +400,20 @@ def test_create_application_database(self, _run_mysqlsh_script): ], ) + @patch("charms.mysql.v0.mysql.MySQLBase.get_non_system_databases") + @patch("charms.mysql.v0.mysql.MySQLBase.list_mysql_roles") @patch("charms.mysql.v0.mysql.MySQLBase._run_mysqlsh_script") - def test_create_application_database_failure(self, _run_mysqlsh_script): + def test_create_application_database_failure( + self, _run_mysqlsh_script, _list_mysql_roles, _get_non_system_databases + ): """Test failure to create application database and scoped user.""" + _get_non_system_databases.return_value = set() + _list_mysql_roles.return_value = set() _run_mysqlsh_script.side_effect = MySQLClientError("Error on subprocess") with self.assertRaises(MySQLCreateApplicationDatabaseError): self.mysql.create_database("test_database") - @patch("charms.mysql.v0.mysql.MySQLBase._run_mysqlsh_script") - def test_create_application_database_invalid(self, _run_mysqlsh_script): - """Test failure to create an invalid application database.""" - with self.assertRaises(MySQLCreateApplicationDatabaseError): - self.mysql.create_database("extremely_extra_long_database_name") - @patch("charms.mysql.v0.mysql.MySQLBase._run_mysqlsh_script") def test_create_application_scoped_user(self, _run_mysqlsh_script): """Test the successful execution of create_application_scoped_user."""