From ec03f83ac6654079a6ae63707144af950560b58c Mon Sep 17 00:00:00 2001 From: reneradoi Date: Fri, 6 Dec 2024 14:40:46 +0000 Subject: [PATCH] remove implementation for `get-password` and `set-password` user actions --- actions.yaml | 21 --------- src/charm.py | 2 - src/events/actions.py | 75 ------------------------------- tests/integration/helpers.py | 13 ------ tests/integration/test_charm.py | 21 --------- tests/unit/test_actions.py | 78 --------------------------------- 6 files changed, 210 deletions(-) delete mode 100644 actions.yaml delete mode 100644 src/events/actions.py delete mode 100644 tests/unit/test_actions.py diff --git a/actions.yaml b/actions.yaml deleted file mode 100644 index b1295ad..0000000 --- a/actions.yaml +++ /dev/null @@ -1,21 +0,0 @@ -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -set-password: - description: Change the admin user's password, which is used by charm. It is for internal charm users and SHOULD NOT be used by applications. - params: - username: - type: string - description: The username, the default value 'root'. Possible values - 'root'. - default: root - password: - type: string - description: The password will be auto-generated if this option is not set. - -get-password: - description: Fetch the admin user's password and CA chain, which is used by charm. It is for internal charm users and SHOULD NOT be used by applications. - params: - username: - type: string - description: The username, the default value 'root'. Possible values - 'root'. - default: root \ No newline at end of file diff --git a/src/charm.py b/src/charm.py index 0e36abf..3c463b2 100755 --- a/src/charm.py +++ b/src/charm.py @@ -10,7 +10,6 @@ from ops import StatusBase from core.cluster import ClusterState -from events.actions import ActionEvents from events.etcd import EtcdEvents from literals import SUBSTRATE, DebugLevel, Status from managers.cluster import ClusterManager @@ -36,7 +35,6 @@ def __init__(self, *args): # --- EVENT HANDLERS --- self.etcd_events = EtcdEvents(self) - self.action_events = ActionEvents(self) def set_status(self, key: Status) -> None: """Set charm status.""" diff --git a/src/events/actions.py b/src/events/actions.py deleted file mode 100644 index f0616ba..0000000 --- a/src/events/actions.py +++ /dev/null @@ -1,75 +0,0 @@ -#!/usr/bin/env python3 -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Event handlers for Juju Actions.""" - -import logging -from typing import TYPE_CHECKING - -from ops.charm import ActionEvent -from ops.framework import Object - -from common.exceptions import EtcdUserManagementError -from literals import INTERNAL_USER - -if TYPE_CHECKING: - from charm import EtcdOperatorCharm - -logger = logging.getLogger(__name__) - - -class ActionEvents(Object): - """Handle all events for user-related Juju Actions.""" - - def __init__(self, charm: "EtcdOperatorCharm"): - super().__init__(charm, key="action_events") - self.charm = charm - - self.framework.observe(self.charm.on.set_password_action, self._on_set_password) - self.framework.observe(self.charm.on.get_password_action, self._on_get_password) - - def _on_get_password(self, event: ActionEvent) -> None: - """Return the password and certificate chain for the internal admin user.""" - username = event.params.get("username") - if username != INTERNAL_USER: - event.fail(f"Action only allowed for user {INTERNAL_USER}.") - return - - if not self.charm.state.cluster.internal_user_credentials: - event.fail("User credentials not created yet.") - return - - # todo: add the TLS CA chain here once TLS is implemented - event.set_results( - { - "username": username, - "password": self.charm.state.cluster.internal_user_credentials[INTERNAL_USER], - "ca-chain": "...", - } - ) - - def _on_set_password(self, event: ActionEvent) -> None: - """Handle the `set-password` action for the internal admin user. - - If no password is provided, generate one. - """ - username = event.params.get("username") - if username != INTERNAL_USER: - event.fail(f"Action only allowed for user {INTERNAL_USER}.") - return - - if not self.charm.unit.is_leader(): - event.fail("Action can only be run on the leader unit.") - return - - new_password = event.params.get("password") or self.charm.workload.generate_password() - - try: - self.charm.cluster_manager.update_credentials(username=username, password=new_password) - self.charm.state.cluster.update({f"{INTERNAL_USER}-password": new_password}) - except EtcdUserManagementError as e: - logger.error(e) - event.fail(e) - - event.set_results({f"{username}-password": new_password}) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index a3a152f..e5d672a 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -83,16 +83,3 @@ async def get_juju_leader_unit_name(ops_test: OpsTest, app_name: str = APP_NAME) for unit in ops_test.model.applications[app_name].units: if await unit.is_leader_from_status(): return unit.name - - -async def get_user_password(ops_test: OpsTest, user: str, unit: str) -> str: - """Use the action to retrieve the password for a user. - - Return: - String with the password stored on the peer relation databag. - """ - action = await ops_test.model.units.get(unit).run_action( - action_name="get-password", params={"username": user} - ) - credentials = await action.wait() - return credentials.results["password"] diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index edd18a4..0333869 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -15,7 +15,6 @@ get_cluster_members, get_juju_leader_unit_name, get_key, - get_user_password, put_key, ) @@ -83,27 +82,7 @@ async def test_authentication(ops_test: OpsTest) -> None: leader_unit = await get_juju_leader_unit_name(ops_test, APP_NAME) test_key = "test_key" test_value = "42" - new_password = "my_new_pwd" # check that reading/writing data without credentials fails assert get_key(model, leader_unit, endpoints, key=test_key) != test_value assert put_key(model, leader_unit, endpoints, key=test_key, value=test_value) != "OK" - - # run set-password action - action = await ops_test.model.units.get(leader_unit).run_action( - action_name="set-password", password=new_password - ) - result = await action.wait() - assert result.results.get(f"{INTERNAL_USER}-password") == new_password - - # run get-password action - updated_password = await get_user_password(ops_test, user=INTERNAL_USER, unit=leader_unit) - assert updated_password == new_password - - # use updated password to read data - assert ( - get_key( - model, leader_unit, endpoints, user=INTERNAL_USER, password=new_password, key=test_key - ) - == test_value - ) diff --git a/tests/unit/test_actions.py b/tests/unit/test_actions.py deleted file mode 100644 index bfeacce..0000000 --- a/tests/unit/test_actions.py +++ /dev/null @@ -1,78 +0,0 @@ -#!/usr/bin/env python3 -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -from pathlib import Path -from subprocess import CompletedProcess -from unittest.mock import patch - -import yaml -from ops import testing -from ops.testing import ActionFailed -from pytest import raises - -from charm import EtcdOperatorCharm -from literals import INTERNAL_USER, PEER_RELATION - -METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) -APP_NAME = METADATA["name"] - - -def test_get_password(): - ctx = testing.Context(EtcdOperatorCharm) - relation = testing.PeerRelation(id=1, endpoint=PEER_RELATION) - - # make sure admin credentials are created initially - state_in = testing.State(relations={relation}, leader=True) - state_out = ctx.run(ctx.on.leader_elected(), state_in) - - ctx.run(ctx.on.action("get-password", params={"username": f"{INTERNAL_USER}"}), state_out) - assert ctx.action_results.get("username") == INTERNAL_USER - assert ctx.action_results.get("password") - assert ctx.action_results.get("ca-chain") - - with raises(ActionFailed) as error: - ctx.run( - ctx.on.action("get-password", params={"username": "my_user"}), - state_in, - ) - assert error.value.message == f"Action only allowed for user {INTERNAL_USER}." - - -def test_set_password(): - ctx = testing.Context(EtcdOperatorCharm) - relation = testing.PeerRelation(id=1, endpoint=PEER_RELATION) - password = "test_pwd" - - # this action is not allowed on non-leader units - state_in = testing.State(relations={relation}, leader=False) - with raises(ActionFailed) as error: - ctx.run( - ctx.on.action( - "set-password", params={"username": INTERNAL_USER, "password": password} - ), - state_in, - ) - assert error.value.message == "Action can only be run on the leader unit." - - # make sure a password cannot set for any other than the admin user - state_in = testing.State(relations={relation}, leader=True) - with raises(ActionFailed) as error: - ctx.run( - ctx.on.action("set-password", params={"username": "my_user", "password": password}), - state_in, - ) - assert error.value.message == f"Action only allowed for user {INTERNAL_USER}." - - # update the admin user's password - state_in = testing.State(relations={relation}, leader=True) - with patch( - "subprocess.run", return_value=CompletedProcess(returncode=0, args=[], stdout="OK") - ): - ctx.run( - ctx.on.action( - "set-password", params={"username": INTERNAL_USER, "password": password} - ), - state_in, - ) - assert ctx.action_results.get(f"{INTERNAL_USER}-password") == password