From 8c8625afb9e19e664c7d924c7f8c13c4295d5321 Mon Sep 17 00:00:00 2001 From: Mia Altieri <32723809+MiaAltieri@users.noreply.github.com> Date: Thu, 29 Feb 2024 19:27:09 +0100 Subject: [PATCH] Fix password rotate + tests (#362) ## Issue Password Rotate was broken and so were the tests for it ## Solution Fix password rotate --- lib/charms/mongodb/v1/shards_interface.py | 58 ++++++++++++++++--- .../sharding_tests/test_sharding_backups.py | 19 +++++- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/lib/charms/mongodb/v1/shards_interface.py b/lib/charms/mongodb/v1/shards_interface.py index 370a2656a..824f56a18 100644 --- a/lib/charms/mongodb/v1/shards_interface.py +++ b/lib/charms/mongodb/v1/shards_interface.py @@ -363,10 +363,7 @@ def _update_relation_data(self, relation_id: int, data: dict) -> None: data: dict containing the key-value pairs that should be updated in the relation. """ - if self.charm.unit.is_leader(): - relation = self.charm.model.get_relation(self.relation_name, relation_id) - if relation: - relation.data[self.charm.model.app].update(data) + self.database_provides.update_relation_data(relation_id, data) def _get_shards_from_relations(self, departed_shard_id: Optional[int]): """Returns a list of the shards related to the config-server.""" @@ -487,6 +484,10 @@ def __init__( charm.on[self.relation_name].relation_changed, self._on_relation_changed ) + self.framework.observe( + getattr(self.charm.on, "secret_changed"), self._handle_changed_secrets + ) + self.framework.observe( charm.on[self.relation_name].relation_departed, self.charm.check_relation_broken_or_scale_down, @@ -496,6 +497,50 @@ def __init__( charm.on[self.relation_name].relation_broken, self._on_relation_broken ) + def _handle_changed_secrets(self, event) -> None: + """Update operator and backup user passwords when rotation occurs. + + Changes in secrets do not re-trigger a relation changed event, so it is necessary to listen + to secret changes events. + """ + if ( + not self.charm.unit.is_leader() + or not event.secret.label + or not self.model.get_relation(self.relation_name) + ): + return + + config_server_relation = self.model.get_relation(self.relation_name) + + # many secret changed events occur, only listen to those related to our interface with the + # config-server + secret_changing_label = event.secret.label + sharding_secretes_label = f"{self.relation_name}.{config_server_relation.id}.extra.secret" + if secret_changing_label != sharding_secretes_label: + logger.info( + "A secret unrelated to this sharding relation %s is changing, igorning secret changed event.", + str(config_server_relation.id), + ) + return + + operator_password = self.database_requires.fetch_relation_field( + config_server_relation.id, OPERATOR_PASSWORD_KEY + ) + backup_password = self.database_requires.fetch_relation_field( + config_server_relation.id, BACKUP_PASSWORD_KEY + ) + + try: + self.update_password( + username=OperatorUser.get_username(), new_password=operator_password + ) + self.update_password(BackupUser.get_username(), new_password=backup_password) + except RetryError: + self.charm.unit.status = BlockedStatus("Failed to rotate cluster secrets") + logger.error("Shard failed to rotate cluster secrets.") + event.defer() + return + def _on_relation_changed(self, event): """Retrieves secrets from config-server and updates them within the shard.""" if not self.pass_hook_checks(event): @@ -796,10 +841,7 @@ def _update_relation_data(self, relation_id: int, data: dict) -> None: data: dict containing the key-value pairs that should be updated in the relation. """ - if self.charm.unit.is_leader(): - relation = self.charm.model.get_relation(self.relation_name, relation_id) - if relation: - relation.data[self.charm.model.app].update(data) + self.database_requires.update_relation_data(relation_id, data) def _is_mongos_reachable(self) -> bool: """Returns True if mongos is reachable.""" diff --git a/tests/integration/sharding_tests/test_sharding_backups.py b/tests/integration/sharding_tests/test_sharding_backups.py index 9fcbb0af0..49c32bd95 100644 --- a/tests/integration/sharding_tests/test_sharding_backups.py +++ b/tests/integration/sharding_tests/test_sharding_backups.py @@ -134,7 +134,7 @@ async def test_create_and_list_backups_in_cluster(ops_test: OpsTest) -> None: # the action `create-backup` only confirms that the command was sent to the `pbm`. Creating a # backup can take a lot of time so this function returns once the command was successfully # sent to pbm. Therefore we should retry listing the backup several times - for attempt in Retrying(stop=stop_after_delay(20), wait=wait_fixed(3), reraise=True): + for attempt in Retrying(stop=stop_after_delay(TIMEOUT), wait=wait_fixed(3), reraise=True): with attempt: backups = await backup_helpers.count_logical_backups(leader_unit) assert backups == 1 @@ -161,6 +161,12 @@ async def test_shards_cannot_run_backup_actions(ops_test: OpsTest) -> None: @pytest.mark.abort_on_fail async def test_rotate_backup_password(ops_test: OpsTest) -> None: """Tests that sharded cluster can successfully create and list backups.""" + await ops_test.model.wait_for_idle( + apps=[CONFIG_SERVER_APP_NAME, SHARD_ONE_APP_NAME, SHARD_TWO_APP_NAME], + idle_period=20, + timeout=TIMEOUT, + status="active", + ) config_leader_id = await get_leader_id(ops_test, app_name=CONFIG_SERVER_APP_NAME) new_password = "new-password" @@ -185,17 +191,24 @@ async def test_rotate_backup_password(ops_test: OpsTest) -> None: apps=[CONFIG_SERVER_APP_NAME, SHARD_ONE_APP_NAME, SHARD_TWO_APP_NAME], idle_period=20, timeout=TIMEOUT, + status="active", ) + config_svr_backup_password = await get_password( + ops_test, username="backup", app_name=CONFIG_SERVER_APP_NAME + ) + assert ( + config_svr_backup_password == new_password + ), "Application config-srver did not rotate password" shard_backup_password = await get_password( ops_test, username="backup", app_name=SHARD_ONE_APP_NAME ) - assert shard_backup_password != new_password, "Application shard-one did not rotate password" + assert shard_backup_password == new_password, "Application shard-one did not rotate password" shard_backup_password = await get_password( ops_test, username="backup", app_name=SHARD_TWO_APP_NAME ) - assert shard_backup_password != new_password, "Application shard-two did not rotate password" + assert shard_backup_password == new_password, "Application shard-two did not rotate password" # verify backup actions work after password rotation leader_unit = await backup_helpers.get_leader_unit(