From fcd78be23c16b2909159bf7a6e4097599c894514 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 24 Sep 2024 16:20:53 -0300 Subject: [PATCH 1/7] feat: add create-admin action --- charmcraft.yaml | 43 ++++++++++++----------- src-docs/charm.py.md | 3 +- src/charm.py | 60 +++++++++++++++++++++++++-------- tests/integration/test_charm.py | 25 ++++++++++++++ tests/unit/conftest.py | 11 +++++- tests/unit/test_charm.py | 15 +++++++++ 6 files changed, 121 insertions(+), 36 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index df11fe8..2da182e 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -1,6 +1,7 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. +type: charm name: maubot title: maubot description: | @@ -12,28 +13,17 @@ links: source: https://github.com/canonical/maubot-operator contact: - https://launchpad.net/~canonical-is-devops - -resources: - maubot-image: - type: oci-image - description: OCI image for maubot - +# build properties +assumes: + - juju >= 3.4 +base: ubuntu@24.04 +build-base: ubuntu@24.04 containers: maubot: resource: maubot-image mounts: - storage: data location: /data -storage: - data: - type: filesystem - -type: charm -base: ubuntu@24.04 -build-base: ubuntu@24.04 -platforms: - amd64: - parts: charm: build-packages: @@ -42,13 +32,26 @@ parts: - libssl-dev - pkg-config - rustc - -assumes: - - juju >= 3.4 - +platforms: + amd64: requires: postgresql: interface: postgresql_client limit: 1 ingress: interface: ingress +resources: + maubot-image: + type: oci-image + description: OCI image for Maubot. +storage: + data: + type: filesystem +# actions +actions: + create-admin: + description: Create administrator user to Maubot. + params: + name: + type: string + description: The name of the administrator user. diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index 61eb9ab..71694c9 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -8,6 +8,7 @@ Maubot charm service. **Global Variables** --------------- - **MAUBOT_NAME** +- **MAUBOT_CONFIGURATION_PATH** --- @@ -15,7 +16,7 @@ Maubot charm service. ## class `MaubotCharm` Maubot charm. - + ### function `__init__` diff --git a/src/charm.py b/src/charm.py index c808662..afc6272 100755 --- a/src/charm.py +++ b/src/charm.py @@ -8,6 +8,7 @@ """Maubot charm service.""" import logging +import secrets import typing import ops @@ -27,6 +28,7 @@ logger = logging.getLogger(__name__) MAUBOT_NAME = "maubot" +MAUBOT_CONFIGURATION_PATH = "/data/config.yaml" class MissingPostgreSQLRelationDataError(Exception): @@ -43,49 +45,57 @@ def __init__(self, *args: typing.Any): args: Arguments passed to the CharmBase parent constructor. """ super().__init__(*args) + self.container = self.unit.get_container(MAUBOT_NAME) self.ingress = IngressPerAppRequirer(self, port=8080) self.postgresql = DatabaseRequires( self, relation_name="postgresql", database_name=self.app.name ) self.framework.observe(self.on.maubot_pebble_ready, self._on_maubot_pebble_ready) self.framework.observe(self.on.config_changed, self._on_config_changed) + # Actions events handlers + self.framework.observe(self.on.create_admin_action, self._on_create_admin_action) # Integrations events handlers self.framework.observe(self.postgresql.on.database_created, self._on_database_created) self.framework.observe(self.postgresql.on.endpoints_changed, self._on_endpoints_changed) self.framework.observe(self.ingress.on.ready, self._on_ingress_ready) self.framework.observe(self.ingress.on.revoked, self._on_ingress_revoked) - def _configure_maubot(self, container: ops.Container) -> None: - """Configure maubot. + def _get_configuration(self) -> dict: + """Get Maubot configuration content. - Args: - container: Container of the charm. + Returns: + Maubot configuration file as a dict. """ + config_content = str( + self.container.pull(MAUBOT_CONFIGURATION_PATH, encoding="utf-8").read() + ) + return yaml.safe_load(config_content) + + def _configure_maubot(self) -> None: + """Configure maubot.""" commands = [ - ["cp", "--update=none", "/example-config.yaml", "/data/config.yaml"], + ["cp", "--update=none", "/example-config.yaml", MAUBOT_CONFIGURATION_PATH], ["mkdir", "-p", "/data/plugins", "/data/trash", "/data/dbs"], ] for command in commands: - process = container.exec(command, combine_stderr=True) + process = self.container.exec(command, combine_stderr=True) process.wait() - config_content = str(container.pull("/data/config.yaml", encoding="utf-8").read()) - config = yaml.safe_load(config_content) + config = self._get_configuration() config["database"] = self._get_postgresql_credentials() - container.push("/data/config.yaml", yaml.safe_dump(config)) + self.container.push("/data/config.yaml", yaml.safe_dump(config)) def _reconcile(self) -> None: """Reconcile workload configuration.""" self.unit.status = ops.MaintenanceStatus() - container = self.unit.get_container(MAUBOT_NAME) - if not container.can_connect(): + if not self.container.can_connect(): return try: - self._configure_maubot(container) + self._configure_maubot() except MissingPostgreSQLRelationDataError: self.unit.status = ops.BlockedStatus("postgresql integration is required") return - container.add_layer(MAUBOT_NAME, self._pebble_layer, combine=True) - container.replan() + self.container.add_layer(MAUBOT_NAME, self._pebble_layer, combine=True) + self.container.replan() self.unit.status = ops.ActiveStatus() def _on_maubot_pebble_ready(self, _: ops.PebbleReadyEvent) -> None: @@ -104,6 +114,28 @@ def _on_ingress_revoked(self, _: IngressPerAppRevokedEvent) -> None: """Handle ingress revoked event.""" self._reconcile() + # Actions events handlers + def _on_create_admin_action(self, event: ops.ActionEvent) -> None: + """Handle delete-profile action. + + Args: + event: Action event. + """ + if ( + not self.container.can_connect() + or MAUBOT_NAME not in self.container.get_plan().services + or not self.container.get_service(MAUBOT_NAME).is_running() + ): + event.fail("maubot is not ready") + return + name = event.params["name"] + password = secrets.token_urlsafe(10) + config = self._get_configuration() + config["admins"][name] = password + self.container.push(MAUBOT_CONFIGURATION_PATH, yaml.safe_dump(config)) + self.container.restart(MAUBOT_NAME) + event.set_results({"password": password}) + # Integrations events handlers def _on_database_created(self, _: DatabaseCreatedEvent) -> None: """Handle database created event.""" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 00b2d1b..5d69d59 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -63,3 +63,28 @@ async def test_build_and_deploy( ) assert response.status_code == 200 assert "Maubot Manager" in response.text + + +async def test_create_admin_action(ops_test: OpsTest): + """ + arrange: Maubot charm integrated with PostgreSQL. + act: run the create-admin action. + assert: the action results contains a password. + """ + name = "test" + assert ops_test.model + unit = ops_test.model.applications["penpot"].units[0] + + action = await unit.run_action("create-admin", name=name) + await action.wait() + + assert "password" in action.results + password = action.results["password"] + response = requests.post( + "http://127.0.0.1/_matrix/maubot/v1/auth/login", + timeout=5, + headers={"Host": "maubot.local"}, + data=f'{{"username":"{name}","password":"{password}"}}', + ) + assert response.status_code == 200 + assert "token" in response.text diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 00b2c83..f04d6f9 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -24,6 +24,15 @@ def harness_fixture(): ) root = harness.get_filesystem_root("maubot") (root / "data").mkdir() - (root / "data" / "config.yaml").write_text("database: sqlite:maubot.db") + yaml_content = """\ +database: sqlite:maubot.db +server: + hostname: 0.0.0.0 + port: 29316 + public_url: https://example.com +admins: + root: +""" + (root / "data" / "config.yaml").write_text(yaml_content) yield harness harness.cleanup() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index edd468d..5ac54d0 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -98,3 +98,18 @@ def test_database_created(harness): harness.charm._get_postgresql_credentials() == "postgresql://someuser:somepasswd@dbhost:5432/maubot" ) + + +def test_create_admin_action(harness): + """ + arrange: initialize the testing harness and set up all required integration. + act: run create-admin charm action. + assert: ensure correct commands are executed. + """ + harness.set_leader() + harness.begin_with_initial_hooks() + set_postgresql_integration(harness) + + action = harness.run_action("create-admin", {"name": "test"}) + + assert "password" in action.results From 5bdd377b653ecbe42d87c3f68e26fa795cb42394 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 24 Sep 2024 16:25:59 -0300 Subject: [PATCH 2/7] fix: fix typos --- charmcraft.yaml | 1 - src/charm.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 2da182e..ab1ef21 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -47,7 +47,6 @@ resources: storage: data: type: filesystem -# actions actions: create-admin: description: Create administrator user to Maubot. diff --git a/src/charm.py b/src/charm.py index afc6272..e030c43 100755 --- a/src/charm.py +++ b/src/charm.py @@ -82,7 +82,7 @@ def _configure_maubot(self) -> None: process.wait() config = self._get_configuration() config["database"] = self._get_postgresql_credentials() - self.container.push("/data/config.yaml", yaml.safe_dump(config)) + self.container.push(MAUBOT_CONFIGURATION_PATH, yaml.safe_dump(config)) def _reconcile(self) -> None: """Reconcile workload configuration.""" From 637ec471671202877dee673f97dfe8c1e77a9ea6 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 24 Sep 2024 16:34:00 -0300 Subject: [PATCH 3/7] fix: actions fails if name is root --- src/charm.py | 5 ++++- tests/unit/test_charm.py | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index e030c43..133fc16 100755 --- a/src/charm.py +++ b/src/charm.py @@ -121,6 +121,10 @@ def _on_create_admin_action(self, event: ops.ActionEvent) -> None: Args: event: Action event. """ + name = event.params["name"] + if name == "root": + event.fail("root is reserved, please choose a different username") + return if ( not self.container.can_connect() or MAUBOT_NAME not in self.container.get_plan().services @@ -128,7 +132,6 @@ def _on_create_admin_action(self, event: ops.ActionEvent) -> None: ): event.fail("maubot is not ready") return - name = event.params["name"] password = secrets.token_urlsafe(10) config = self._get_configuration() config["admins"][name] = password diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 5ac54d0..34fcf30 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -100,11 +100,11 @@ def test_database_created(harness): ) -def test_create_admin_action(harness): +def test_create_admin_action_success(harness): """ arrange: initialize the testing harness and set up all required integration. act: run create-admin charm action. - assert: ensure correct commands are executed. + assert: ensure password is in the results. """ harness.set_leader() harness.begin_with_initial_hooks() @@ -113,3 +113,19 @@ def test_create_admin_action(harness): action = harness.run_action("create-admin", {"name": "test"}) assert "password" in action.results + + +def test_create_admin_action_failed(harness): + """ + arrange: initialize the testing harness and set up all required integration. + act: run create-admin charm action with reserved name root. + assert: ensure action fails. + """ + harness.set_leader() + harness.begin_with_initial_hooks() + set_postgresql_integration(harness) + + try: + harness.run_action("create-admin", {"name": "root"}) + except ops.testing.ActionFailed as e: + assert e.message == "root is reserved, please choose a different username" From 64cca02dbc15160b6812af05d7d99e4a72809940 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 24 Sep 2024 16:36:17 -0300 Subject: [PATCH 4/7] fix: change error message --- src/charm.py | 2 +- tests/unit/test_charm.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index 133fc16..38c4173 100755 --- a/src/charm.py +++ b/src/charm.py @@ -123,7 +123,7 @@ def _on_create_admin_action(self, event: ops.ActionEvent) -> None: """ name = event.params["name"] if name == "root": - event.fail("root is reserved, please choose a different username") + event.fail("root is reserved, please choose a different name") return if ( not self.container.can_connect() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 34fcf30..9622ba5 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -128,4 +128,4 @@ def test_create_admin_action_failed(harness): try: harness.run_action("create-admin", {"name": "root"}) except ops.testing.ActionFailed as e: - assert e.message == "root is reserved, please choose a different username" + assert e.message == "root is reserved, please choose a different name" From 194e0f36ad15d851cf272d8225ce444b5885d5a7 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 24 Sep 2024 17:03:16 -0300 Subject: [PATCH 5/7] fix: fix typo again --- tests/integration/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 5d69d59..2cf0898 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -73,7 +73,7 @@ async def test_create_admin_action(ops_test: OpsTest): """ name = "test" assert ops_test.model - unit = ops_test.model.applications["penpot"].units[0] + unit = ops_test.model.applications["maubot"].units[0] action = await unit.run_action("create-admin", name=name) await action.wait() From 133a78cfd52560e7d2381aa3c343225b5eb861fd Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 30 Sep 2024 18:26:53 -0300 Subject: [PATCH 6/7] chores: change _get_configuration return type --- src/charm.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 38c4173..42057cd 100755 --- a/src/charm.py +++ b/src/charm.py @@ -9,7 +9,7 @@ import logging import secrets -import typing +from typing import Any, Dict import ops import yaml @@ -38,7 +38,7 @@ class MissingPostgreSQLRelationDataError(Exception): class MaubotCharm(ops.CharmBase): """Maubot charm.""" - def __init__(self, *args: typing.Any): + def __init__(self, *args: Any): """Construct. Args: @@ -60,7 +60,7 @@ def __init__(self, *args: typing.Any): self.framework.observe(self.ingress.on.ready, self._on_ingress_ready) self.framework.observe(self.ingress.on.revoked, self._on_ingress_revoked) - def _get_configuration(self) -> dict: + def _get_configuration(self) -> Dict[str, Any]: """Get Maubot configuration content. Returns: From c019c2f472d7969b16a44b69f5e45cba5f9ddfcf Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 30 Sep 2024 19:14:23 -0300 Subject: [PATCH 7/7] fix: handle user exists and add integration test for failure --- src/charm.py | 23 ++++++++++++++++++++--- tests/integration/test_charm.py | 26 +++++++++++++++++++++++++- tests/unit/test_charm.py | 5 ++++- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/charm.py b/src/charm.py index 42057cd..6127271 100755 --- a/src/charm.py +++ b/src/charm.py @@ -115,6 +115,18 @@ def _on_ingress_revoked(self, _: IngressPerAppRevokedEvent) -> None: self._reconcile() # Actions events handlers + def _fail_event(self, event: ops.ActionEvent, results: Dict[str, str], message: str) -> None: + """Handle failure events. + + Args: + event: Action event. + results: Event results. + message: Error message. + """ + results["error"] = message + event.set_results(results) + event.fail(message) + def _on_create_admin_action(self, event: ops.ActionEvent) -> None: """Handle delete-profile action. @@ -122,22 +134,27 @@ def _on_create_admin_action(self, event: ops.ActionEvent) -> None: event: Action event. """ name = event.params["name"] + results = {"password": "", "error": ""} if name == "root": - event.fail("root is reserved, please choose a different name") + self._fail_event(event, results, "root is reserved, please choose a different name") return if ( not self.container.can_connect() or MAUBOT_NAME not in self.container.get_plan().services or not self.container.get_service(MAUBOT_NAME).is_running() ): - event.fail("maubot is not ready") + self._fail_event(event, results, "maubot is not ready") return password = secrets.token_urlsafe(10) config = self._get_configuration() + if name in config["admins"]: + self._fail_event(event, results, f"{name} already exists") + return config["admins"][name] = password self.container.push(MAUBOT_CONFIGURATION_PATH, yaml.safe_dump(config)) self.container.restart(MAUBOT_NAME) - event.set_results({"password": password}) + results["password"] = password + event.set_results(results) # Integrations events handlers def _on_database_created(self, _: DatabaseCreatedEvent) -> None: diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 2cf0898..483059a 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -65,7 +65,7 @@ async def test_build_and_deploy( assert "Maubot Manager" in response.text -async def test_create_admin_action(ops_test: OpsTest): +async def test_create_admin_action_success(ops_test: OpsTest): """ arrange: Maubot charm integrated with PostgreSQL. act: run the create-admin action. @@ -88,3 +88,27 @@ async def test_create_admin_action(ops_test: OpsTest): ) assert response.status_code == 200 assert "token" in response.text + + +@pytest.mark.parametrize( + "name,expected_message", + [ + pytest.param("root", "root is reserved, please choose a different name", id="root"), + pytest.param("test", "test already exists", id="user_exists"), + ], +) +async def test_create_admin_action_failed(name: str, expected_message: str, ops_test: OpsTest): + """ + arrange: Maubot charm integrated with PostgreSQL. + act: run the create-admin action. + assert: the action results fails. + """ + assert ops_test.model + unit = ops_test.model.applications["maubot"].units[0] + + action = await unit.run_action("create-admin", name=name) + await action.wait() + + assert "error" in action.results + error = action.results["error"] + assert error == expected_message diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9622ba5..3b92356 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -113,6 +113,7 @@ def test_create_admin_action_success(harness): action = harness.run_action("create-admin", {"name": "test"}) assert "password" in action.results + assert "error" in action.results and not action.results["error"] def test_create_admin_action_failed(harness): @@ -128,4 +129,6 @@ def test_create_admin_action_failed(harness): try: harness.run_action("create-admin", {"name": "root"}) except ops.testing.ActionFailed as e: - assert e.message == "root is reserved, please choose a different name" + message = "root is reserved, please choose a different name" + assert e.output.results["error"] == message + assert e.message == message