Prevent updating matrix-auth data more than once

Author: amandahla

lib/charms/synapse/v1/matrix_auth.py

@@ -67,7 +67,7 @@ def _on_config_changed(self, _) -> None:
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 4
+LIBPATCH = 5
 
 # pylint: disable=wrong-import-position
 import json
@@ -442,15 +442,24 @@ def _on_relation_changed(self, event: ops.RelationChangedEvent) -> None:
     def update_relation_data(
         self, relation: ops.Relation, matrix_auth_provider_data: MatrixAuthProviderData
     ) -> None:
-        """Update the relation data.
+        """Update the relation data. Since provider values should not be changed
+            while instance exists, this method updates relation data only if
+            invalid or empty.
 
         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)
+        try:
+            logger.warning(relation)
+            MatrixAuthProviderData.from_relation(self.model, relation=relation)
+            logger.warning("Matrix Provider relation data is already set, skipping")
+        except ValueError as e:
+            logger.warning(e)
+            logger.warning("Matrix Provider relation data is invalid or empty, updating")
+            relation_data = matrix_auth_provider_data.to_relation_data(self.model, relation)
+            relation.data[self.model.app].update(relation_data)
 
 
 class MatrixAuthRequires(ops.Object):

src/matrix_auth_observer.py

@@ -62,9 +62,6 @@ def update_matrix_auth_integration(self, charm_state: CharmState) -> None:
         logger.info("%d matrix-auth relations found", len(matrix_auth_relations))
         for relation in matrix_auth_relations:
             provider_data = self._get_matrix_auth_provider_data(charm_state)
-            if self._matrix_auth_relation_updated(relation, provider_data):
-                return
-            logger.info("updating matrix-auth relation %d", relation.id)
             self.matrix_auth.update_relation_data(relation, provider_data)
 
     def get_requirer_registration_secrets(self) -> Optional[List]:
@@ -121,30 +118,6 @@ def _get_matrix_auth_provider_data(
         shared_secret = synapse.get_registration_shared_secret(container=container)
         return MatrixAuthProviderData(homeserver=homeserver, shared_secret=shared_secret)
 
-    def _matrix_auth_relation_updated(
-        self, relation: ops.Relation, provider_data: MatrixAuthProviderData
-    ) -> bool:
-        """Compare current information with the one in the relation.
-
-        This check is done to prevent triggering relation-changed.
-
-        Args:
-            relation: The matrix-auth relation.
-            provider_data: current Synapse configuration as MatrixAuthProviderData.
-
-        Returns:
-            True if current configuration and relation data are the same.
-        """
-        relation_homeserver = relation.data[self._charm.app].get("homeserver", "")
-        relation_shared_secret = relation.data[self._charm.app].get("shared_secret", "")
-        if (
-            provider_data.homeserver != relation_homeserver
-            or provider_data.shared_secret != relation_shared_secret
-        ):
-            logger.info("matrix-auth relation ID %s is outdated and will be updated", relation.id)
-            return False
-        return True
-
     @validate_charm_state
     def _on_matrix_auth_relation_changed(self, _: ops.EventBase) -> None:
         """Handle matrix-auth request received event."""

tests/unit/test_matrix_auth_integration.py

@@ -5,11 +5,16 @@
 
 # pylint: disable=protected-access
 
+import logging
 from unittest.mock import ANY, MagicMock
 
 import pytest
 import yaml
-from charms.synapse.v1.matrix_auth import MatrixAuthRequirerData, encrypt_string
+from charms.synapse.v1.matrix_auth import (
+    MatrixAuthProviderData,
+    MatrixAuthRequirerData,
+    encrypt_string,
+)
 from ops.testing import Harness
 from pydantic import SecretStr
 
@@ -47,6 +52,40 @@ def test_matrix_auth_update_success(harness: Harness, monkeypatch: pytest.Monkey
     assert update_relation_data.call_args[0][1].homeserver == f"https://{TEST_SERVER_NAME}"
 
 
+def test_matrix_auth_update_only_once(harness: Harness, monkeypatch: pytest.MonkeyPatch, caplog):
+    """
+    arrange: start the Synapse charm.
+    act: integrate via matrix-auth.
+    assert: update_relation_data is called and homeserver has same value as
+        server_name.
+    """
+    harness.update_config({"server_name": TEST_SERVER_NAME})
+    harness.set_can_connect(synapse.SYNAPSE_CONTAINER_NAME, True)
+    harness.set_leader(True)
+    harness.begin()
+    monkeypatch.setattr(
+        MatrixAuthProviderData, "get_shared_secret", MagicMock(return_value="shared-secret")
+    )
+    monkeypatch.setattr(
+        synapse, "get_registration_shared_secret", MagicMock(return_value="shared_secret")
+    )
+    rel_id = harness.add_relation(
+        "matrix-auth",
+        "maubot",
+        app_data={
+            "homeserver": "example.com",
+            "shared_secret_id": "test-secret-id",
+            "encryption_key_secret_id": "test-secret-id",
+        },
+    )
+    harness.add_relation_unit(rel_id, "maubot/0")
+
+    with caplog.at_level(logging.WARNING):
+        harness.charm.on.config_changed.emit()
+
+    assert "Matrix Provider relation data is invalid or empty, updating" not in caplog.text
+
+
 def test_matrix_auth_update_public_baseurl_success(
     harness: Harness, monkeypatch: pytest.MonkeyPatch
 ):