From b6420bfdc5ad7e8e5ca4e47699446feddd94ff72 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Mon, 21 Apr 2025 17:09:05 +0300 Subject: [PATCH 1/2] Make username mandatory --- src/charm.py | 8 ++++++-- tests/unit/test_charm.py | 22 +++++++++++++++------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/charm.py b/src/charm.py index b3f11cd0bb..102b9b6368 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1240,7 +1240,9 @@ def _on_get_password(self, event: ActionEvent) -> None: If no user is provided, the password of the operator user is returned. """ - username = event.params.get("username", USER) + if not (username := event.params.get("username")): + event.fail("The action requires a username") + return if username not in PASSWORD_USERS and self.is_ldap_enabled: event.fail("The action can be run only for system users when LDAP is enabled") return @@ -1260,7 +1262,9 @@ def _on_set_password(self, event: ActionEvent) -> None: # noqa: C901 event.fail("The action can be run only on leader unit") return - username = event.params.get("username", USER) + if not (username := event.params.get("username")): + event.fail("The action requires a username") + return if username not in SYSTEM_USERS and self.is_ldap_enabled: event.fail("The action can be run only for system users when LDAP is enabled") return diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index c169047fd0..208c8c760d 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -376,7 +376,8 @@ def test_on_get_password(harness): mock_event.reset_mock() del mock_event.params["username"] harness.charm._on_get_password(mock_event) - mock_event.set_results.assert_called_once_with({"password": "test-password"}) + mock_event.fail.assert_called_once() + mock_event.set_results.assert_not_called() # Also test providing the username option. mock_event.reset_mock() @@ -393,14 +394,16 @@ def test_on_set_password(harness): patch("charm.PostgresqlOperatorCharm.postgresql") as _postgresql, patch("charm.Patroni.are_all_members_ready") as _are_all_members_ready, patch("charm.PostgresqlOperatorCharm._on_leader_elected"), + patch("charm.new_password", return_value="newpass"), ): # Create a mock event. mock_event = MagicMock(params={}) # Set some values for the other mocks. - _are_all_members_ready.side_effect = [False, True, True, True, True] - _postgresql.update_user_password = PropertyMock( - side_effect=[PostgreSQLUpdateUserPasswordError, None, None, None] + _are_all_members_ready.return_value = False + _postgresql.update_user_password = PropertyMock() + _postgresql.update_user_password.return_value.side_effect = ( + PostgreSQLUpdateUserPasswordError ) # Test trying to set a password through a non leader unit. @@ -424,20 +427,25 @@ def test_on_set_password(harness): _set_secret.assert_not_called() # Test for an error updating when updating the user password in the database. + _are_all_members_ready.return_value = True mock_event.reset_mock() harness.charm._on_set_password(mock_event) mock_event.fail.assert_called_once() _set_secret.assert_not_called() # Test without providing the username option. + _postgresql.update_user_password.return_value.side_effect = None + mock_event.reset_mock() harness.charm._on_set_password(mock_event) - assert _set_secret.call_args_list[0][0][1] == "operator-password" + mock_event.fail.assert_called_once() + _set_secret.assert_not_called() # Also test providing the username option. + mock_event.reset_mock() _set_secret.reset_mock() mock_event.params["username"] = "replication" harness.charm._on_set_password(mock_event) - assert _set_secret.call_args_list[0][0][1] == "replication-password" + _set_secret.assert_called_once_with("app", "replication-password", "newpass") # And test providing both the username and password options. _set_secret.reset_mock() @@ -1209,7 +1217,7 @@ def test_on_get_password_secrets(harness): mock_event.reset_mock() del mock_event.params["username"] harness.charm._on_get_password(mock_event) - mock_event.set_results.assert_called_once_with({"password": "test-password"}) + mock_event.fail.assert_called_once_with("The action requires a username") # Also test providing the username option. mock_event.reset_mock() From 8f76d3257fc097fdfd192ccdf5ada2b70b7d8453 Mon Sep 17 00:00:00 2001 From: Dragomir Penev Date: Thu, 24 Apr 2025 18:45:45 +0300 Subject: [PATCH 2/2] Default in get_password --- src/charm.py | 4 +--- tests/unit/test_charm.py | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/charm.py b/src/charm.py index 558da4b18c..ba46ee1e3b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1240,9 +1240,7 @@ def _on_get_password(self, event: ActionEvent) -> None: If no user is provided, the password of the operator user is returned. """ - if not (username := event.params.get("username")): - event.fail("The action requires a username") - return + username = event.params.get("username", USER) if username not in PASSWORD_USERS and self.is_ldap_enabled: event.fail("The action can be run only for system users when LDAP is enabled") return diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 208c8c760d..fb85d337bf 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -376,8 +376,7 @@ def test_on_get_password(harness): mock_event.reset_mock() del mock_event.params["username"] harness.charm._on_get_password(mock_event) - mock_event.fail.assert_called_once() - mock_event.set_results.assert_not_called() + mock_event.set_results.assert_called_once_with({"password": "test-password"}) # Also test providing the username option. mock_event.reset_mock() @@ -1217,7 +1216,7 @@ def test_on_get_password_secrets(harness): mock_event.reset_mock() del mock_event.params["username"] harness.charm._on_get_password(mock_event) - mock_event.fail.assert_called_once_with("The action requires a username") + mock_event.set_results.assert_called_once_with({"password": "test-password"}) # Also test providing the username option. mock_event.reset_mock()