feat(libs): Correct use of data_interfaces (#472)

Co-authored-by: Mia Altieri <[email protected]>

lib/charms/mongodb/v0/config_server_interface.py

@@ -11,10 +11,11 @@
 
 from charms.data_platform_libs.v0.data_interfaces import (
     DatabaseProvides,
+    DatabaseRequestedEvent,
     DatabaseRequires,
 )
 from charms.mongodb.v1.mongos import MongosConnection
-from ops.charm import CharmBase, EventBase, RelationBrokenEvent
+from ops.charm import CharmBase, EventBase, RelationBrokenEvent, RelationChangedEvent
 from ops.framework import Object
 from ops.model import (
     ActiveStatus,
@@ -42,7 +43,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 11
+LIBPATCH = 12
 
 
 class ClusterProvider(Object):
@@ -57,6 +58,9 @@ def __init__(
         self.database_provides = DatabaseProvides(self.charm, relation_name=self.relation_name)
 
         super().__init__(charm, self.relation_name)
+        self.framework.observe(
+            self.database_provides.on.database_requested, self._on_database_requested
+        )
         self.framework.observe(
             charm.on[self.relation_name].relation_changed, self._on_relation_changed
         )
@@ -105,8 +109,14 @@ def is_valid_mongos_integration(self) -> bool:
 
         return True
 
-    def _on_relation_changed(self, event) -> None:
-        """Handles providing mongos with KeyFile and hosts."""
+    def _on_database_requested(self, event: DatabaseRequestedEvent | RelationChangedEvent) -> None:
+        """Handles the database requested event.
+
+        The first time secrets are written to relations should be on this event.
+
+        Note: If secrets are written for the first time on other events we risk
+        the chance of writing secrets in plain sight.
+        """
         if not self.pass_hook_checks(event):
             if not self.is_valid_mongos_integration():
                 self.charm.status.set_and_share_status(
@@ -116,12 +126,9 @@ def _on_relation_changed(self, event) -> None:
                 )
             logger.info("Skipping relation joined event: hook checks did not pass")
             return
-
         config_server_db = self.generate_config_server_db()
-
         # create user and set secrets for mongos relation
         self.charm.client_relations.oversee_users(None, None)
-
         relation_data = {
             KEYFILE_KEY: self.charm.get_secret(
                 Config.Relations.APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME
@@ -135,9 +142,20 @@ def _on_relation_changed(self, event) -> None:
         )
         if int_tls_ca:
             relation_data[INT_TLS_CA_KEY] = int_tls_ca
-
         self.database_provides.update_relation_data(event.relation.id, relation_data)
 
+    def _on_relation_changed(self, event: RelationChangedEvent) -> None:
+        """Handles providing mongos with KeyFile and hosts."""
+        # First we need to ensure that the database requested event has run
+        # otherwise we risk the chance of writing secrets in plain sight.
+        if not self.database_provides.fetch_relation_field(event.relation.id, "database"):
+            logger.info("Database Requested has not run yet, skipping.")
+            event.defer()
+            return
+
+        # TODO : This workflow is a fix until we have time for a better and complete fix (DPE-5513)
+        self._on_database_requested(event)
+
     def _on_relation_broken(self, event) -> None:
         if self.charm.upgrade_in_progress:
             logger.warning(

tests/unit/test_config_server_lib.py

@@ -56,6 +56,39 @@ def is_config_mock_call(*args):
         self.harness.add_relation_unit(relation_id, "mongos/2")
         self.harness.charm.cluster.database_provides.update_relation_data.assert_not_called()
 
+    @mock.patch("charms.mongodb.v1.mongodb_provider.MongoDBProvider.oversee_users")
+    @mock.patch("data_platform_helpers.version_check.CrossAppVersionChecker.is_local_charm")
+    @mock.patch(
+        "data_platform_helpers.version_check.CrossAppVersionChecker.is_integrated_to_locally_built_charm"
+    )
+    @mock.patch("charm.get_charm_revision")
+    @mock.patch("ops.framework.EventBase.defer")
+    def test_relation_changed_fail_if_no_database_field(
+        self, defer, get_charm_rev, is_integrated, is_local, oversee
+    ):
+
+        def is_config_mock_call(*args):
+            assert args == ("config-server",)
+            return True
+
+        self.harness.charm.app_peer_data["db_initialised"] = "True"
+        self.harness.set_leader(True)
+        self.harness.charm.is_role = is_config_mock_call
+        self.harness.charm.cluster.database_provides.update_relation_data = mock.Mock()
+
+        relation_id = self.harness.add_relation("cluster", "mongos")
+        self.harness.add_relation_unit(relation_id, "mongos/0")
+
+        # Fails because there is no `database` field in the relation data
+        relation = self.harness.charm.model.get_relation("cluster")
+        self.harness.charm.on["cluster"].relation_changed.emit(relation=relation)
+        defer.assert_called()
+
+        # Success, we have the for the database field.
+        # Note: updating the relation data here triggers a relation_changed event.
+        self.harness.update_relation_data(relation_id, "mongos", {"database": "test-database"})
+        self.harness.charm.cluster.database_provides.update_relation_data.assert_called()
+
     def test_update_rel_data_failed_hook_checks(self):
         """Tests that no relation data is set when the cluster is not ready."""