From 38e9cdfda6081aa34e3988b277b43caf264debbb Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 7 Oct 2024 15:51:23 -0300 Subject: [PATCH 01/11] feat: add matrix-auth integration --- charmcraft.yaml | 3 + lib/charms/synapse/v0/matrix_auth.py | 476 +++++++++++++++++++++++++++ src-docs/charm.py.md | 23 +- src/charm.py | 72 +++- tests/unit/test_charm.py | 47 ++- 5 files changed, 607 insertions(+), 14 deletions(-) create mode 100644 lib/charms/synapse/v0/matrix_auth.py diff --git a/charmcraft.yaml b/charmcraft.yaml index 4c1df5b..1af5e50 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -55,6 +55,9 @@ assumes: - juju >= 3.4 requires: + matrix-auth: + interface: matrix-auth + limit: 1 postgresql: interface: postgresql_client limit: 1 diff --git a/lib/charms/synapse/v0/matrix_auth.py b/lib/charms/synapse/v0/matrix_auth.py new file mode 100644 index 0000000..2342b99 --- /dev/null +++ b/lib/charms/synapse/v0/matrix_auth.py @@ -0,0 +1,476 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache2.0. See LICENSE file in charm source for details. + +"""Library to manage the plugin integrations with the Synapse charm. + +This library contains the Requires and Provides classes for handling the integration +between an application and a charm providing the `matrix_plugin` integration. + +### Requirer Charm + +```python + +from charms.synapse.v0.matrix_auth import MatrixAuthRequires + +class MatrixAuthRequirerCharm(ops.CharmBase): + def __init__(self, *args): + super().__init__(*args) + self.plugin_auth = MatrixAuthRequires(self) + self.framework.observe(self.matrix_auth.on.matrix_auth_request_processed, self._handler) + ... + + def _handler(self, events: MatrixAuthRequestProcessed) -> None: + ... + +``` + +As shown above, the library provides a custom event to handle the scenario in +which a matrix authentication (homeserver and shared secret) has been added or updated. + +The MatrixAuthRequires provides an `update_relation_data` method to update the relation data by +passing a `MatrixAuthRequirerData` data object, requesting a new authentication. + +### Provider Charm + +Following the previous example, this is an example of the provider charm. + +```python +from charms.synapse.v0.matrix_auth import MatrixAuthProvides + +class MatrixAuthProviderCharm(ops.CharmBase): + def __init__(self, *args): + super().__init__(*args) + self.plugin_auth = MatrixAuthProvides(self) + ... + +``` +The MatrixAuthProvides object wraps the list of relations into a `relations` property +and provides an `update_relation_data` method to update the relation data by passing +a `MatrixAuthRelationData` data object. + +```python +class MatrixAuthProviderCharm(ops.CharmBase): + ... + + def _on_config_changed(self, _) -> None: + for relation in self.model.relations[self.plugin_auth.relation_name]: + self.plugin_auth.update_relation_data(relation, self._get_matrix_auth_data()) + +``` +""" + +# The unique Charmhub library identifier, never change it +LIBID = "ff6788c89b204448b3b62ba6f93e2768" + +# Increment this major API version when introducing breaking changes +LIBAPI = 0 + +# Increment this PATCH version before using `charmcraft publish-lib` or reset +# to 0 if you are raising the major API version +LIBPATCH = 3 + +# pylint: disable=wrong-import-position +import json +import logging +from typing import Dict, List, Optional, Tuple, cast + +import ops +from pydantic import BaseModel, Field, SecretStr + +logger = logging.getLogger(__name__) + +#### Constants #### +APP_REGISTRATION_LABEL = "app-registration" +APP_REGISTRATION_CONTENT_LABEL = "app-registration-content" +DEFAULT_RELATION_NAME = "matrix-auth" +SHARED_SECRET_LABEL = "shared-secret" +SHARED_SECRET_CONTENT_LABEL = "shared-secret-content" + + +#### Data models for Provider and Requirer #### +class MatrixAuthProviderData(BaseModel): + """Represent the MatrixAuth provider data. + + Attributes: + homeserver: the homeserver URL. + shared_secret: the Matrix shared secret. + shared_secret_id: the shared secret Juju secret ID. + """ + + homeserver: str + shared_secret: Optional[SecretStr] = Field(default=None, exclude=True) + shared_secret_id: Optional[SecretStr] = Field(default=None) + + def set_shared_secret_id(self, model: ops.Model, relation: ops.Relation) -> None: + """Store the Matrix shared secret as a Juju secret. + + Args: + model: the Juju model + relation: relation to grant access to the secrets to. + """ + # password is always defined since pydantic guarantees it + password = cast(SecretStr, self.shared_secret) + # pylint doesn't like get_secret_value + secret_value = password.get_secret_value() # pylint: disable=no-member + try: + secret = model.get_secret(label=SHARED_SECRET_LABEL) + secret.set_content({SHARED_SECRET_CONTENT_LABEL: secret_value}) + # secret.id is not None at this point + self.shared_secret_id = cast(str, secret.id) + except ops.SecretNotFoundError: + secret = relation.app.add_secret( + {SHARED_SECRET_CONTENT_LABEL: secret_value}, label=SHARED_SECRET_LABEL + ) + secret.grant(relation) + self.shared_secret_id = cast(str, secret.id) + + @classmethod + def get_shared_secret( + cls, model: ops.Model, shared_secret_id: Optional[str] + ) -> Optional[SecretStr]: + """Retrieve the shared secret corresponding to the shared_secret_id. + + Args: + model: the Juju model. + shared_secret_id: the secret ID for the shared secret. + + Returns: + the shared secret or None if not found. + """ + if not shared_secret_id: + return None + try: + secret = model.get_secret(id=shared_secret_id) + password = secret.get_content().get(SHARED_SECRET_CONTENT_LABEL) + if not password: + return None + return SecretStr(password) + except ops.SecretNotFoundError: + return None + + def to_relation_data(self, model: ops.Model, relation: ops.Relation) -> Dict[str, str]: + """Convert an instance of MatrixAuthProviderData to the relation representation. + + Args: + model: the Juju model. + relation: relation to grant access to the secrets to. + + Returns: + Dict containing the representation. + """ + self.set_shared_secret_id(model, relation) + return self.model_dump(exclude_unset=True) + + @classmethod + def from_relation(cls, model: ops.Model, relation: ops.Relation) -> "MatrixAuthProviderData": + """Initialize a new instance of the MatrixAuthProviderData class from the relation. + + Args: + relation: the relation. + + Returns: + A MatrixAuthProviderData instance. + + Raises: + ValueError: if the value is not parseable. + """ + app = cast(ops.Application, relation.app) + relation_data = relation.data[app] + shared_secret_id = ( + (relation_data["shared_secret_id"]) + if "shared_secret_id" in relation_data + else None + ) + shared_secret = MatrixAuthProviderData.get_shared_secret(model, shared_secret_id) + homeserver = relation_data.get("homeserver") + if shared_secret is None or homeserver is None: + raise ValueError("Invalid relation data") + return MatrixAuthProviderData( + homeserver=homeserver, + shared_secret=shared_secret, + ) + + +class MatrixAuthRequirerData(BaseModel): + """Represent the MatrixAuth requirer data. + + Attributes: + registration: a generated app registration file. + registration_id: the registration Juju secret ID. + """ + + registration: Optional[SecretStr] = Field(default=None, exclude=True) + registration_secret_id: Optional[SecretStr] = Field(default=None) + + def set_registration_id(self, model: ops.Model, relation: ops.Relation) -> None: + """Store the app registration as a Juju secret. + + Args: + model: the Juju model + relation: relation to grant access to the secrets to. + """ + # password is always defined since pydantic guarantees it + password = cast(SecretStr, self.registration) + # pylint doesn't like get_secret_value + secret_value = password.get_secret_value() # pylint: disable=no-member + try: + secret = model.get_secret(label=APP_REGISTRATION_LABEL) + secret.set_content({APP_REGISTRATION_CONTENT_LABEL: secret_value}) + # secret.id is not None at this point + self.registration_secret_id = cast(str, secret.id) + except ops.SecretNotFoundError: + secret = relation.app.add_secret( + {APP_REGISTRATION_CONTENT_LABEL: secret_value}, label=APP_REGISTRATION_LABEL + ) + secret.grant(relation) + self.registration_secret_id = cast(str, secret.id) + + @classmethod + def get_registration( + cls, model: ops.Model, registration_secret_id: Optional[str] + ) -> Optional[SecretStr]: + """Retrieve the registration corresponding to the registration_secret_id. + + Args: + model: the Juju model. + registration_secret_id: the secret ID for the registration. + + Returns: + the registration or None if not found. + """ + if not registration_secret_id: + return None + try: + secret = model.get_secret(id=registration_secret_id) + password = secret.get_content().get(APP_REGISTRATION_CONTENT_LABEL) + if not password: + return None + return SecretStr(password) + except ops.SecretNotFoundError: + return None + + def to_relation_data(self, model: ops.Model, relation: ops.Relation) -> Dict[str, str]: + """Convert an instance of MatrixAuthRequirerData to the relation representation. + + Args: + model: the Juju model. + relation: relation to grant access to the secrets to. + + Returns: + Dict containing the representation. + """ + self.set_registration_id(model, relation) + dumped_model = self.model_dump(exclude_unset=True) + dumped_data = { + "registration_secret_id": dumped_model["registration_secret_id"], + } + return dumped_data + + @classmethod + def from_relation(cls, model: ops.Model, relation: ops.Relation) -> "MatrixAuthRequirerData": + """Get a MatrixAuthRequirerData from the relation data. + + Args: + model: the Juju model. + relation: the relation. + + Returns: + the relation data and the processed entries for it. + + Raises: + ValueError: if the value is not parseable. + """ + app = cast(ops.Application, relation.app) + relation_data = relation.data[app] + registration_secret_id = relation_data.get("registration_secret_id") + registration = MatrixAuthRequirerData.get_registration(model, registration_secret_id) + return MatrixAuthRequirerData( + registration=registration, + ) + + +#### Events #### +class MatrixAuthRequestProcessed(ops.RelationEvent): + """MatrixAuth event emitted when a new request is processed.""" + + def get_matrix_auth_provider_relation_data(self) -> MatrixAuthProviderData: + """Get a MatrixAuthProviderData for the relation data. + + Returns: + the MatrixAuthProviderData for the relation data. + """ + return MatrixAuthProviderData.from_relation(self.framework.model, self.relation) + + +class MatrixAuthRequestReceived(ops.RelationEvent): + """MatrixAuth event emitted when a new request is made.""" + + +class MatrixAuthRequiresEvents(ops.CharmEvents): + """MatrixAuth requirer events. + + This class defines the events that a MatrixAuth requirer can emit. + + Attributes: + matrix_auth_request_processed: the MatrixAuthRequestProcessed. + """ + + matrix_auth_request_processed = ops.EventSource(MatrixAuthRequestProcessed) + + +class MatrixAuthProvidesEvents(ops.CharmEvents): + """MatrixAuth provider events. + + This class defines the events that a MatrixAuth provider can emit. + + Attributes: + matrix_auth_request_received: the MatrixAuthRequestReceived. + """ + + matrix_auth_request_received = ops.EventSource(MatrixAuthRequestReceived) + + +#### Provides and Requires #### +class MatrixAuthProvides(ops.Object): + """Provider side of the MatrixAuth relation. + + Attributes: + on: events the provider can emit. + """ + + on = MatrixAuthProvidesEvents() + + def __init__(self, charm: ops.CharmBase, relation_name: str = DEFAULT_RELATION_NAME) -> None: + """Construct. + + Args: + charm: the provider charm. + relation_name: the relation name. + """ + super().__init__(charm, relation_name) + self.relation_name = relation_name + self.framework.observe(charm.on[relation_name].relation_changed, self._on_relation_changed) + + def get_remote_relation_data(self) -> Optional[MatrixAuthRequirerData]: + """Retrieve the remote relation data. + + Returns: + MatrixAuthRequirerData: the relation data. + """ + relation = self.model.get_relation(self.relation_name) + return MatrixAuthRequirerData.from_relation(self.model, relation=relation) if relation else None + + def _is_remote_relation_data_valid(self, relation: ops.Relation) -> bool: + """Validate the relation data. + + Args: + relation: the relation to validate. + + Returns: + true: if the relation data is valid. + """ + try: + _ = MatrixAuthRequirerData.from_relation(self.model, relation=relation) + return True + except ValueError as ex: + logger.warning("Error validating the relation data %s", ex) + return False + + def _on_relation_changed(self, event: ops.RelationChangedEvent) -> None: + """Event emitted when the relation has changed. + + Args: + event: event triggering this handler. + """ + assert event.relation.app + relation_data = event.relation.data[event.relation.app] + if relation_data and self._is_remote_relation_data_valid(event.relation): + self.on.matrix_auth_request_received.emit( + event.relation, app=event.app, unit=event.unit + ) + + def update_relation_data( + self, relation: ops.Relation, matrix_auth_provider_data: MatrixAuthProviderData + ) -> None: + """Update the relation data. + + Args: + relation: the relation for which to update the data. + matrix_auth_provider_data: a MatrixAuthProviderData instance wrapping the data to be + updated. + """ + relation_data = matrix_auth_provider_data.to_relation_data(self.model, relation) + relation.data[self.model.app].update(relation_data) + + +class MatrixAuthRequires(ops.Object): + """Requirer side of the MatrixAuth requires relation. + + Attributes: + on: events the provider can emit. + """ + + on = MatrixAuthRequiresEvents() + + def __init__(self, charm: ops.CharmBase, relation_name: str = DEFAULT_RELATION_NAME) -> None: + """Construct. + + Args: + charm: the provider charm. + relation_name: the relation name. + """ + super().__init__(charm, relation_name) + self.relation_name = relation_name + self.framework.observe(charm.on[relation_name].relation_changed, self._on_relation_changed) + + def get_remote_relation_data(self) -> Optional[MatrixAuthProviderData]: + """Retrieve the remote relation data. + + Returns: + MatrixAuthProviderData: the relation data. + """ + relation = self.model.get_relation(self.relation_name) + return MatrixAuthProviderData.from_relation(self.model, relation=relation) if relation else None + + def _is_remote_relation_data_valid(self, relation: ops.Relation) -> bool: + """Validate the relation data. + + Args: + relation: the relation to validate. + + Returns: + true: if the relation data is valid. + """ + try: + _ = MatrixAuthProviderData.from_relation(self.model, relation=relation) + return True + except ValueError as ex: + logger.warning("Error validating the relation data %s", ex) + return False + + def _on_relation_changed(self, event: ops.RelationChangedEvent) -> None: + """Event emitted when the relation has changed. + + Args: + event: event triggering this handler. + """ + assert event.relation.app + relation_data = event.relation.data[event.relation.app] + if relation_data and self._is_remote_relation_data_valid(event.relation): + self.on.matrix_auth_request_processed.emit( + event.relation, app=event.app, unit=event.unit + ) + + def update_relation_data( + self, + relation: ops.Relation, + matrix_auth_requirer_data: MatrixAuthRequirerData, + ) -> None: + """Update the relation data. + + Args: + relation: the relation for which to update the data. + matrix_auth_requirer_data: MatrixAuthRequirerData wrapping the data to be updated. + """ + relation_data = matrix_auth_requirer_data.to_relation_data(self.model, relation) + relation.data[self.model.app].update(relation_data) diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index a920763..0a2db5d 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -16,7 +16,7 @@ Maubot charm service. ## class `MaubotCharm` Maubot charm. - + ### function `__init__` @@ -74,8 +74,25 @@ Unit that this execution is responsible for. --- -## class `MissingPostgreSQLRelationDataError` -Custom exception to be raised in case of malformed/missing Postgresql relation data. +## class `MissingRelationDataError` +Custom exception to be raised in case of malformed/missing relation data. + + + +### function `__init__` + +```python +__init__(message: str, relation_name: str = '') → None +``` + +Init custom exception. + + + +**Args:** + + - `message`: Exception message. + - `relation_name`: Relation name that raised the exception. diff --git a/src/charm.py b/src/charm.py index d876ef9..279c117 100755 --- a/src/charm.py +++ b/src/charm.py @@ -17,6 +17,7 @@ DatabaseEndpointsChangedEvent, DatabaseRequires, ) +from charms.synapse.v0.matrix_auth import MatrixAuthRequestProcessed, MatrixAuthRequires from charms.traefik_k8s.v2.ingress import ( IngressPerAppReadyEvent, IngressPerAppRequirer, @@ -30,8 +31,18 @@ NGINX_NAME = "nginx" -class MissingPostgreSQLRelationDataError(Exception): - """Custom exception to be raised in case of malformed/missing Postgresql relation data.""" +class MissingRelationDataError(Exception): + """Custom exception to be raised in case of malformed/missing relation data.""" + + def __init__(self, message: str, relation_name: str = "") -> None: + """Init custom exception. + + Args: + message: Exception message. + relation_name: Relation name that raised the exception. + """ + super().__init__(message) + self.relation_name = relation_name class MaubotCharm(ops.CharmBase): @@ -48,6 +59,7 @@ def __init__(self, *args: typing.Any): self.postgresql = DatabaseRequires( self, relation_name="postgresql", database_name=self.app.name ) + self.matrix_auth = MatrixAuthRequires(self) self.framework.observe(self.on.maubot_pebble_ready, self._on_maubot_pebble_ready) self.framework.observe(self.on.config_changed, self._on_config_changed) # Integrations events handlers @@ -55,6 +67,10 @@ def __init__(self, *args: typing.Any): 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) + self.framework.observe( + self.matrix_auth.on.matrix_auth_request_processed, + self._on_matrix_auth_request_processed, + ) def _configure_maubot(self, container: ops.Container) -> None: """Configure maubot. @@ -72,6 +88,7 @@ def _configure_maubot(self, container: ops.Container) -> None: config_content = str(container.pull("/data/config.yaml", encoding="utf-8").read()) config = yaml.safe_load(config_content) config["database"] = self._get_postgresql_credentials() + config["homeservers"] = self._get_matrix_credentials() config["server"]["public_url"] = self.config.get("public-url") container.push("/data/config.yaml", yaml.safe_dump(config)) @@ -83,8 +100,14 @@ def _reconcile(self) -> None: return try: self._configure_maubot(container) - except MissingPostgreSQLRelationDataError: - self.unit.status = ops.BlockedStatus("postgresql integration is required") + except MissingRelationDataError as e: + self.unit.status = ops.BlockedStatus(f"{e.relation_name} integration is required") + try: + container.stop(MAUBOT_NAME) + except RuntimeError: + logging.info("maubot is not running, no action taken") + except ops.pebble.ChangeError as pe: + logging.exception("failed to stop maubot", exc_info=pe) return container.add_layer(MAUBOT_NAME, self._pebble_layer, combine=True) container.restart(MAUBOT_NAME) @@ -99,6 +122,7 @@ def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: """Handle changed configuration.""" self._reconcile() + # Integrations events handlers def _on_ingress_ready(self, _: IngressPerAppReadyEvent) -> None: """Handle ingress ready event.""" self._reconcile() @@ -107,7 +131,6 @@ def _on_ingress_revoked(self, _: IngressPerAppRevokedEvent) -> None: """Handle ingress revoked event.""" self._reconcile() - # Integrations events handlers def _on_database_created(self, _: DatabaseCreatedEvent) -> None: """Handle database created event.""" self._reconcile() @@ -116,6 +139,10 @@ def _on_endpoints_changed(self, _: DatabaseEndpointsChangedEvent) -> None: """Handle endpoints changed event.""" self._reconcile() + def _on_matrix_auth_request_processed(self, _: MatrixAuthRequestProcessed) -> None: + """Handle matrix auth request processed event.""" + self._reconcile() + # Relation data handlers def _get_postgresql_credentials(self) -> str: """Get postgresql credentials from the postgresql integration. @@ -124,23 +151,50 @@ def _get_postgresql_credentials(self) -> str: postgresql credentials. Raises: - MissingPostgreSQLRelationDataError: if relation is not found. + MissingRelationDataError: if relation is not found. """ relation = self.model.get_relation("postgresql") if not relation or not relation.app: - raise MissingPostgreSQLRelationDataError("No postgresql relation data") + raise MissingRelationDataError( + "No postgresql relation data", relation_name="postgresql" + ) endpoints = self.postgresql.fetch_relation_field(relation.id, "endpoints") database = self.postgresql.fetch_relation_field(relation.id, "database") username = self.postgresql.fetch_relation_field(relation.id, "username") password = self.postgresql.fetch_relation_field(relation.id, "password") if not endpoints: - raise MissingPostgreSQLRelationDataError("Missing mandatory relation data") + raise MissingRelationDataError( + "Missing mandatory relation data", relation_name="postgresql" + ) primary_endpoint = endpoints.split(",")[0] if not all((primary_endpoint, database, username, password)): - raise MissingPostgreSQLRelationDataError("Missing mandatory relation data") + raise MissingRelationDataError( + "Missing mandatory relation data", relation_name="postgresql" + ) return f"postgresql://{username}:{password}@{primary_endpoint}/{database}" + def _get_matrix_credentials(self) -> dict[str, dict[str, str]]: + """Get Matrix credentials from the matrix-auth integration. + + Returns: + matrix credentials. + + Raises: + MissingRelationDataError: if relation is not found. + """ + relation = self.model.get_relation("matrix-auth") + if not relation or not relation.app: + raise MissingRelationDataError("No matrix relation data", relation_name="matrix-auth") + relation_data = self.matrix_auth.get_remote_relation_data() + homeserver = relation_data.homeserver + shared_secret_id = relation_data.shared_secret.get_secret_value() + if not all((homeserver, shared_secret_id)): + raise MissingRelationDataError( + "Missing mandatory relation data", relation_name="matrix-auth" + ) + return {"synapse": {"url": homeserver, "secret": shared_secret_id}} + # Properties @property def _pebble_layer(self) -> pebble.LayerDict: diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index ab6bf04..d138cc0 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -7,6 +7,7 @@ import ops import ops.testing +from charms.synapse.v0.matrix_auth import MatrixAuthProviderData def set_postgresql_integration(harness) -> None: @@ -32,6 +33,28 @@ def set_postgresql_integration(harness) -> None: ) +def set_matrix_auth_integration(harness, monkeypatch) -> None: + """Set matrix-auth integration. + + Args: + harness: harness instance. + monkeypatch: monkeypatch instance. + """ + monkeypatch.setattr( + MatrixAuthProviderData, "get_shared_secret", lambda *args: "test-shared-secret" + ) + relation_data = {"homeserver": "https://example.com", "shared_secret_id": "test-secret-id"} + matrix_relation_id = harness.add_relation( # pylint: disable=attribute-defined-outside-init + "matrix-auth", "synapse", app_data=relation_data + ) + harness.add_relation_unit(matrix_relation_id, "synapse/0") + harness.update_relation_data( + matrix_relation_id, + "synapse", + relation_data, + ) + + def test_maubot_pebble_ready_postgresql_required(harness): """ arrange: initialize the testing harness with handle_exec and @@ -46,7 +69,7 @@ def test_maubot_pebble_ready_postgresql_required(harness): assert harness.model.unit.status == ops.BlockedStatus("postgresql integration is required") -def test_maubot_pebble_ready(harness): +def test_maubot_pebble_ready(harness, monkeypatch): """ arrange: initialize the testing harness with handle_exec and config.yaml file. @@ -55,7 +78,11 @@ def test_maubot_pebble_ready(harness): the service is running and the charm is active. """ harness.begin() + harness.set_can_connect("maubot", True) set_postgresql_integration(harness) + assert harness.model.unit.status == ops.BlockedStatus("matrix-auth integration is required") + set_matrix_auth_integration(harness, monkeypatch) + expected_plan = { "services": { "maubot": { @@ -100,7 +127,7 @@ def test_database_created(harness): ) -def test_public_url_config_changed(harness): +def test_public_url_config_changed(harness, monkeypatch): """ arrange: initialize harness and set postgresql integration. act: change public-url config. @@ -108,9 +135,25 @@ def test_public_url_config_changed(harness): """ harness.begin_with_initial_hooks() set_postgresql_integration(harness) + set_matrix_auth_integration(harness, monkeypatch) harness.update_config({"public-url": "https://example1.com"}) service = harness.model.unit.get_container("maubot").get_service("maubot") assert service.is_running() assert harness.model.unit.status == ops.ActiveStatus() + + +def test_matrix_credentials_registered(harness, monkeypatch): + """ + arrange: initialize harness and verify that there is no credentials. + act: set matrix-auth integration. + assert: matrix credentials are set as expected. + """ + harness.begin_with_initial_hooks() + + set_matrix_auth_integration(harness, monkeypatch) + + assert harness.charm._get_matrix_credentials() == { + "synapse": {"secret": "test-shared-secret", "url": "https://example.com"} + } From f0561775471e94c57a519daf1d8cc05812f58823 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 7 Oct 2024 16:43:34 -0300 Subject: [PATCH 02/11] fix: remove matrix requirement for now --- src/charm.py | 3 ++- tests/unit/test_charm.py | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 279c117..5097ea5 100755 --- a/src/charm.py +++ b/src/charm.py @@ -185,7 +185,8 @@ def _get_matrix_credentials(self) -> dict[str, dict[str, str]]: """ relation = self.model.get_relation("matrix-auth") if not relation or not relation.app: - raise MissingRelationDataError("No matrix relation data", relation_name="matrix-auth") + logging.warning("no matrix-auth relation found, getting default matrix credentials") + return {"matrix": {"url": "https://matrix-client.matrix.org", "secret": "null"}} relation_data = self.matrix_auth.get_remote_relation_data() homeserver = relation_data.homeserver shared_secret_id = relation_data.shared_secret.get_secret_value() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index d138cc0..6a628cb 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -80,8 +80,6 @@ def test_maubot_pebble_ready(harness, monkeypatch): harness.begin() harness.set_can_connect("maubot", True) set_postgresql_integration(harness) - assert harness.model.unit.status == ops.BlockedStatus("matrix-auth integration is required") - set_matrix_auth_integration(harness, monkeypatch) expected_plan = { "services": { From 7a2279b969742d69d832d2df8bbe7e12e0a30c17 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Mon, 7 Oct 2024 17:20:58 -0300 Subject: [PATCH 03/11] fix: handle APIError --- 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 5097ea5..e453bc3 100755 --- a/src/charm.py +++ b/src/charm.py @@ -106,7 +106,7 @@ def _reconcile(self) -> None: container.stop(MAUBOT_NAME) except RuntimeError: logging.info("maubot is not running, no action taken") - except ops.pebble.ChangeError as pe: + except (ops.pebble.ChangeError, ops.pebble.APIError) as pe: logging.exception("failed to stop maubot", exc_info=pe) return container.add_layer(MAUBOT_NAME, self._pebble_layer, combine=True) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 6a628cb..e3e7cc2 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -69,7 +69,7 @@ def test_maubot_pebble_ready_postgresql_required(harness): assert harness.model.unit.status == ops.BlockedStatus("postgresql integration is required") -def test_maubot_pebble_ready(harness, monkeypatch): +def test_maubot_pebble_ready(harness): """ arrange: initialize the testing harness with handle_exec and config.yaml file. From fecc0560d53989a0b9b3eb6f8b8143e721ad6929 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 8 Oct 2024 09:59:25 -0300 Subject: [PATCH 04/11] chore: remove pylint comment --- tests/unit/test_charm.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e3e7cc2..bc74cf3 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -44,9 +44,7 @@ def set_matrix_auth_integration(harness, monkeypatch) -> None: MatrixAuthProviderData, "get_shared_secret", lambda *args: "test-shared-secret" ) relation_data = {"homeserver": "https://example.com", "shared_secret_id": "test-secret-id"} - matrix_relation_id = harness.add_relation( # pylint: disable=attribute-defined-outside-init - "matrix-auth", "synapse", app_data=relation_data - ) + matrix_relation_id = harness.add_relation("matrix-auth", "synapse", app_data=relation_data) harness.add_relation_unit(matrix_relation_id, "synapse/0") harness.update_relation_data( matrix_relation_id, From 5bfb4bee290b1d880b21bff772c84b381906451f Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 8 Oct 2024 10:00:52 -0300 Subject: [PATCH 05/11] chore: move set_matrix_auth_integration --- tests/unit/test_charm.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index bc74cf3..c16ccbc 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -33,26 +33,6 @@ def set_postgresql_integration(harness) -> None: ) -def set_matrix_auth_integration(harness, monkeypatch) -> None: - """Set matrix-auth integration. - - Args: - harness: harness instance. - monkeypatch: monkeypatch instance. - """ - monkeypatch.setattr( - MatrixAuthProviderData, "get_shared_secret", lambda *args: "test-shared-secret" - ) - relation_data = {"homeserver": "https://example.com", "shared_secret_id": "test-secret-id"} - matrix_relation_id = harness.add_relation("matrix-auth", "synapse", app_data=relation_data) - harness.add_relation_unit(matrix_relation_id, "synapse/0") - harness.update_relation_data( - matrix_relation_id, - "synapse", - relation_data, - ) - - def test_maubot_pebble_ready_postgresql_required(harness): """ arrange: initialize the testing harness with handle_exec and @@ -153,3 +133,23 @@ def test_matrix_credentials_registered(harness, monkeypatch): assert harness.charm._get_matrix_credentials() == { "synapse": {"secret": "test-shared-secret", "url": "https://example.com"} } + + +def set_matrix_auth_integration(harness, monkeypatch) -> None: + """Set matrix-auth integration. + + Args: + harness: harness instance. + monkeypatch: monkeypatch instance. + """ + monkeypatch.setattr( + MatrixAuthProviderData, "get_shared_secret", lambda *args: "test-shared-secret" + ) + relation_data = {"homeserver": "https://example.com", "shared_secret_id": "test-secret-id"} + matrix_relation_id = harness.add_relation("matrix-auth", "synapse", app_data=relation_data) + harness.add_relation_unit(matrix_relation_id, "synapse/0") + harness.update_relation_data( + matrix_relation_id, + "synapse", + relation_data, + ) From ba120c61969cfb5cbdaf4b3dafced54bc4aa4004 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 8 Oct 2024 10:08:32 -0300 Subject: [PATCH 06/11] chore: fix unit tests --- tests/unit/test_charm.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index c16ccbc..54afc89 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -7,8 +7,11 @@ import ops import ops.testing +import pytest from charms.synapse.v0.matrix_auth import MatrixAuthProviderData +from charm import MissingRelationDataError + def set_postgresql_integration(harness) -> None: """Set postgresql integration. @@ -94,6 +97,8 @@ def test_database_created(harness): assert: postgresql credentials are set as expected. """ harness.begin_with_initial_hooks() + with pytest.raises(MissingRelationDataError): + harness.charm._get_postgresql_credentials() set_postgresql_integration(harness) @@ -122,11 +127,14 @@ def test_public_url_config_changed(harness, monkeypatch): def test_matrix_credentials_registered(harness, monkeypatch): """ - arrange: initialize harness and verify that there is no credentials. + arrange: initialize harness and verify that the credentials are set with default values. act: set matrix-auth integration. assert: matrix credentials are set as expected. """ harness.begin_with_initial_hooks() + assert harness.charm._get_matrix_credentials() == { + "matrix": {"secret": "null", "url": "https://matrix-client.matrix.org"} + } set_matrix_auth_integration(harness, monkeypatch) From 982c802ea18fe8f28dcfa34565fcf18e5f4a0b6f Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 8 Oct 2024 12:11:10 -0300 Subject: [PATCH 07/11] fix: handle runtimeerror in unit tests only --- src/charm.py | 2 -- tests/unit/test_charm.py | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index e453bc3..a8dbff7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -104,8 +104,6 @@ def _reconcile(self) -> None: self.unit.status = ops.BlockedStatus(f"{e.relation_name} integration is required") try: container.stop(MAUBOT_NAME) - except RuntimeError: - logging.info("maubot is not running, no action taken") except (ops.pebble.ChangeError, ops.pebble.APIError) as pe: logging.exception("failed to stop maubot", exc_info=pe) return diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 54afc89..93858ea 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -5,6 +5,9 @@ """Unit tests.""" +from functools import wraps +from typing import Any, Callable + import ops import ops.testing import pytest @@ -13,6 +16,46 @@ from charm import MissingRelationDataError +def handle_runtime_exception(f: Callable[..., Any | None]) -> Callable[..., Any | None]: + """Handle RunTimeError raised by Harness when a service is not found. + + This behavior is not expected in real environment. + For more details, see this issue: + https://github.com/canonical/operator/issues/1310 + + Args: + f (Callable[..., Any | None]): the unit test. + + Returns: + Executed test. + """ + + @wraps(f) + def wrapper(*args, **kw) -> Any | None: + """Wrap the test function. + + Args: + args: test arguments. + kw: keyword arguments to pass to the decorated function. + + Raises: + e: Exception raised by the test. + + Returns: + Executed test. + """ + try: + return f(*args, **kw) + except RuntimeError as e: + if str(e) == '400 Bad Request: service "maubot" does not exist': + pass + else: + raise e + return None + + return wrapper + + def set_postgresql_integration(harness) -> None: """Set postgresql integration. @@ -36,6 +79,7 @@ def set_postgresql_integration(harness) -> None: ) +@handle_runtime_exception def test_maubot_pebble_ready_postgresql_required(harness): """ arrange: initialize the testing harness with handle_exec and @@ -90,6 +134,7 @@ def test_maubot_pebble_ready(harness): assert harness.model.unit.status == ops.ActiveStatus() +@handle_runtime_exception def test_database_created(harness): """ arrange: initialize harness and verify that there is no credentials. @@ -108,6 +153,7 @@ def test_database_created(harness): ) +@handle_runtime_exception def test_public_url_config_changed(harness, monkeypatch): """ arrange: initialize harness and set postgresql integration. @@ -125,6 +171,7 @@ def test_public_url_config_changed(harness, monkeypatch): assert harness.model.unit.status == ops.ActiveStatus() +@handle_runtime_exception def test_matrix_credentials_registered(harness, monkeypatch): """ arrange: initialize harness and verify that the credentials are set with default values. From dac8473940be8079e31089fedd45615e431527a7 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 8 Oct 2024 12:26:47 -0300 Subject: [PATCH 08/11] chore: handle RuntimeError in charm code again --- src/charm.py | 11 +++++++++- tests/unit/test_charm.py | 47 ---------------------------------------- 2 files changed, 10 insertions(+), 48 deletions(-) diff --git a/src/charm.py b/src/charm.py index a8dbff7..145ea32 100755 --- a/src/charm.py +++ b/src/charm.py @@ -93,7 +93,9 @@ def _configure_maubot(self, container: ops.Container) -> None: container.push("/data/config.yaml", yaml.safe_dump(config)) def _reconcile(self) -> None: - """Reconcile workload configuration.""" + # Ignoring DC050 for now since RuntimeError is handled/re-raised only + # because a Harness issue. + """Reconcile workload configuration.""" # noqa: DCO050 self.unit.status = ops.MaintenanceStatus() container = self.unit.get_container(MAUBOT_NAME) if not container.can_connect(): @@ -104,6 +106,13 @@ def _reconcile(self) -> None: self.unit.status = ops.BlockedStatus(f"{e.relation_name} integration is required") try: container.stop(MAUBOT_NAME) + except RuntimeError as re: + if str(re) == '400 Bad Request: service "maubot" does not exist': + # Remove this once Harness is fixed + # See https://github.com/canonical/operator/issues/1310 + pass + else: + raise e from re except (ops.pebble.ChangeError, ops.pebble.APIError) as pe: logging.exception("failed to stop maubot", exc_info=pe) return diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 93858ea..54afc89 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -5,9 +5,6 @@ """Unit tests.""" -from functools import wraps -from typing import Any, Callable - import ops import ops.testing import pytest @@ -16,46 +13,6 @@ from charm import MissingRelationDataError -def handle_runtime_exception(f: Callable[..., Any | None]) -> Callable[..., Any | None]: - """Handle RunTimeError raised by Harness when a service is not found. - - This behavior is not expected in real environment. - For more details, see this issue: - https://github.com/canonical/operator/issues/1310 - - Args: - f (Callable[..., Any | None]): the unit test. - - Returns: - Executed test. - """ - - @wraps(f) - def wrapper(*args, **kw) -> Any | None: - """Wrap the test function. - - Args: - args: test arguments. - kw: keyword arguments to pass to the decorated function. - - Raises: - e: Exception raised by the test. - - Returns: - Executed test. - """ - try: - return f(*args, **kw) - except RuntimeError as e: - if str(e) == '400 Bad Request: service "maubot" does not exist': - pass - else: - raise e - return None - - return wrapper - - def set_postgresql_integration(harness) -> None: """Set postgresql integration. @@ -79,7 +36,6 @@ def set_postgresql_integration(harness) -> None: ) -@handle_runtime_exception def test_maubot_pebble_ready_postgresql_required(harness): """ arrange: initialize the testing harness with handle_exec and @@ -134,7 +90,6 @@ def test_maubot_pebble_ready(harness): assert harness.model.unit.status == ops.ActiveStatus() -@handle_runtime_exception def test_database_created(harness): """ arrange: initialize harness and verify that there is no credentials. @@ -153,7 +108,6 @@ def test_database_created(harness): ) -@handle_runtime_exception def test_public_url_config_changed(harness, monkeypatch): """ arrange: initialize harness and set postgresql integration. @@ -171,7 +125,6 @@ def test_public_url_config_changed(harness, monkeypatch): assert harness.model.unit.status == ops.ActiveStatus() -@handle_runtime_exception def test_matrix_credentials_registered(harness, monkeypatch): """ arrange: initialize harness and verify that the credentials are set with default values. From 27b926a54ecb3028203813ec7630c1973d38dedb Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Tue, 8 Oct 2024 12:27:33 -0300 Subject: [PATCH 09/11] fix: small raise fix --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 145ea32..fd4a1b6 100755 --- a/src/charm.py +++ b/src/charm.py @@ -112,7 +112,7 @@ def _reconcile(self) -> None: # See https://github.com/canonical/operator/issues/1310 pass else: - raise e from re + raise re except (ops.pebble.ChangeError, ops.pebble.APIError) as pe: logging.exception("failed to stop maubot", exc_info=pe) return From add2809ea0f26130828ffa12aa64e95b3b14d5c3 Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Thu, 10 Oct 2024 10:09:10 -0300 Subject: [PATCH 10/11] chore: remove default relation_name --- src-docs/charm.py.md | 2 +- src/charm.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index 3ed39c4..e1fe295 100644 --- a/src-docs/charm.py.md +++ b/src-docs/charm.py.md @@ -92,7 +92,7 @@ Custom exception to be raised in case of malformed/missing relation data. ### function `__init__` ```python -__init__(message: str, relation_name: str = '') → None +__init__(message: str, relation_name: str) → None ``` Init custom exception. diff --git a/src/charm.py b/src/charm.py index d8ae62d..950aac9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -36,7 +36,7 @@ class MissingRelationDataError(Exception): """Custom exception to be raised in case of malformed/missing relation data.""" - def __init__(self, message: str, relation_name: str = "") -> None: + def __init__(self, message: str, relation_name: str) -> None: """Init custom exception. Args: From 52da8a6e7832155d6e6c68ed782cd0cfc001c4fd Mon Sep 17 00:00:00 2001 From: Amanda Hager Lopes de Andrade Katz Date: Fri, 11 Oct 2024 13:26:59 -0300 Subject: [PATCH 11/11] nit: replace - for _ --- charmcraft.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index cfc768e..23bfb83 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -46,7 +46,7 @@ platforms: amd64: requires: matrix-auth: - interface: matrix-auth + interface: matrix_auth limit: 1 postgresql: interface: postgresql_client