Skip to content

Commit

Permalink
Fix password rotate + tests (#362)
Browse files Browse the repository at this point in the history
## Issue
Password Rotate was broken and so were the tests for it

## Solution
Fix password rotate
  • Loading branch information
MiaAltieri authored Feb 29, 2024
1 parent 55b000e commit 8c8625a
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 11 deletions.
58 changes: 50 additions & 8 deletions lib/charms/mongodb/v1/shards_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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,
Expand All @@ -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):
Expand Down Expand Up @@ -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."""
Expand Down
19 changes: 16 additions & 3 deletions tests/integration/sharding_tests/test_sharding_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"

Expand All @@ -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(
Expand Down

0 comments on commit 8c8625a

Please sign in to comment.