[DPE-5318] charm manges only created users (#475)

## Issue:

1. Users created by clients get automatically deleted by MongoDB charm
2. Users for mongos K8s charm get automatically deleted by MongoDB charm

## Solution:

Keep track of which users the charm should manage and only remove/update
those users

## Testing

Lib tested for all charms that use it i.e. MongoDB VM+K8s and Mongos K8s

---------

Co-authored-by: Mehdi Bendriss <bendrissmehdi@gmail.com>

lib/charms/mongodb/v1/helpers.py

@@ -340,5 +340,5 @@ def safe_exec(
         logger.debug(f"{output=}")
         return output
     except subprocess.CalledProcessError as err:
-        logger.error(f"cmd failed - {err.cmd = }, {err.stdout = }, {err.stderr = }")
+        logger.error(f"cmd failed - {err.cmd}, {err.stdout}, {err.stderr}")
         raise

lib/charms/mongodb/v1/mongodb_provider.py

@@ -37,6 +37,7 @@
 REL_NAME = "database"
 MONGOS_RELATIONS = "cluster"
 MONGOS_CLIENT_RELATIONS = "mongos_proxy"
+MANAGED_USERS_KEY = "managed-users-key"
 
 # We expect the MongoDB container to use the default ports
 
@@ -164,12 +165,36 @@ def oversee_users(self, departed_relation_id: Optional[int], event):
         When the function is executed in relation departed event, the departed
         relation is still on the list of all relations. Therefore, for proper
         work of the function, we need to exclude departed relation from the list.
+
+        Raises:
+            PyMongoError
         """
         with MongoConnection(self.charm.mongo_config) as mongo:
             database_users = mongo.get_users()
-            relation_users = self._get_users_from_relations(departed_relation_id)
 
-            for username in database_users - relation_users:
+        users_being_managed = database_users.intersection(self._get_relational_users_to_manage())
+        expected_current_users = self._get_users_from_relations(departed_relation_id)
+
+        self.remove_users(users_being_managed, expected_current_users)
+        self.add_users(users_being_managed, expected_current_users)
+        self.update_users(event, users_being_managed, expected_current_users)
+        self.auto_delete_dbs(departed_relation_id)
+
+    def remove_users(
+        self, users_being_managed: Set[str], expected_current_users: Set[str]
+    ) -> None:
+        """Removes users from Charmed MongoDB.
+
+        Note this only removes users that this application of Charmed MongoDB is responsible for
+        managing. It won't remove:
+        1. users created from other applications
+        2. users created from other mongos routers.
+
+        Raises:
+            PyMongoError
+        """
+        with MongoConnection(self.charm.mongo_config) as mongo:
+            for username in users_being_managed - expected_current_users:
                 logger.info("Remove relation user: %s", username)
                 if (
                     self.charm.is_role(Config.Role.MONGOS)
@@ -178,25 +203,51 @@ def oversee_users(self, departed_relation_id: Optional[int], event):
                     continue
 
                 mongo.drop_user(username)
+                self._remove_from_relational_users_to_manage(username)
 
-            for username in relation_users - database_users:
-                config = self._get_config(username, None, event)
+    def add_users(self, users_being_managed: Set[str], expected_current_users: Set[str]) -> None:
+        """Adds users to Charmed MongoDB.
+
+        Raises:
+            PyMongoError
+        """
+        with MongoConnection(self.charm.mongo_config) as mongo:
+            for username in expected_current_users - users_being_managed:
+                config = self._get_config(username, None)
                 if config.database is None:
                     # We need to wait for the moment when the provider library
                     # set the database name into the relation.
                     continue
                 logger.info("Create relation user: %s on %s", config.username, config.database)
 
                 mongo.create_user(config)
+                self._add_to_relational_users_to_manage(username)
                 self._set_relation(config)
 
-            for username in relation_users.intersection(database_users):
+    def update_users(
+        self, event: EventBase, users_being_managed: Set[str], expected_current_users: Set[str]
+    ) -> None:
+        """Updates existing users in Charmed MongoDB.
+
+        Raises:
+            PyMongoError
+        """
+        with MongoConnection(self.charm.mongo_config) as mongo:
+            for username in expected_current_users.intersection(users_being_managed):
                 config = self._get_config(username, None)
                 logger.info("Update relation user: %s on %s", config.username, config.database)
                 mongo.update_user(config)
                 logger.info("Updating relation data according to diff")
                 self._diff(event)
 
+    def auto_delete_dbs(self, departed_relation_id):
+        """Delete's unused dbs if configured to do so.
+
+        Raises:
+            PyMongoError
+        """
+        with MongoConnection(self.charm.mongo_config) as mongo:
+
             if not self.charm.model.config["auto-delete"]:
                 return
 
@@ -408,6 +459,35 @@ def get_relation_name(self):
         else:
             return REL_NAME
 
+    def _get_relational_users_to_manage(self) -> Set[str]:
+        """Returns a set of the users to manage.
+
+        Note json cannot serialise sets. Convert from list.
+        """
+        return set(json.loads(self.charm.app_peer_data.get(MANAGED_USERS_KEY, "[]")))
+
+    def _update_relational_users_to_manage(self, new_users: Set[str]) -> None:
+        """Updates the set of the users to manage.
+
+        Note json cannot serialise sets. Convert from list.
+        """
+        if not self.charm.unit.is_leader():
+            raise Exception("Cannot update relational data on non-leader unit")
+
+        self.charm.app_peer_data[MANAGED_USERS_KEY] = json.dumps(list(new_users))
+
+    def _remove_from_relational_users_to_manage(self, user_to_remove: str) -> None:
+        """Removes the provided user from the set of the users to manage."""
+        current_users = self._get_relational_users_to_manage()
+        updated_users = current_users - {user_to_remove}
+        self._update_relational_users_to_manage(updated_users)
+
+    def _add_to_relational_users_to_manage(self, user_to_add: str) -> None:
+        """Adds the provided user to the set of the users to manage."""
+        current_users = self._get_relational_users_to_manage()
+        current_users.add(user_to_add)
+        self._update_relational_users_to_manage(current_users)
+
     @staticmethod
     def _get_database_from_relation(relation: Relation) -> Optional[str]:
         """Return database name from relation."""

tests/integration/relation_tests/new_relations/helpers.py

@@ -4,6 +4,8 @@
 
 import json
 
+from pymongo import MongoClient
+from pymongo.errors import OperationFailure
 from pytest_operator.plugin import OpsTest
 from tenacity import (
     RetryError,
@@ -16,6 +18,8 @@
 
 from ...helpers import get_application_relation_data
 
+AUTH_FAILED_CODE = 18
+
 
 async def verify_application_data(
     ops_test: OpsTest,
@@ -69,3 +73,27 @@ async def get_connection_string(
 
     first_relation_user_data = await get_secret_data(ops_test, secret_uri)
     return first_relation_user_data.get("uris")
+
+
+async def assert_created_user_can_connect(
+    ops_test: OpsTest, database_name: str, username: str, password: str
+):
+    """Verifies that the provided username can connect to the DB with the given password."""
+    hosts = [unit.public_address for unit in ops_test.model.applications[database_name].units]
+    hosts = ",".join(hosts)
+    connection_string = f"mongodb://{username}:{password}@{hosts}"
+    client = MongoClient(
+        connection_string,
+        directConnection=False,
+        connect=False,
+        serverSelectionTimeoutMS=1000,
+        connectTimeoutMS=2000,
+    )
+    try:
+        client.admin.command("ping")
+    except OperationFailure as e:
+        if e.code == AUTH_FAILED_CODE:
+            assert False, "user does not have access to MongoDB"
+            return
+
+        raise

tests/integration/relation_tests/new_relations/test_charm_relations.py

@@ -15,6 +15,7 @@
 from ...ha_tests.helpers import replica_set_primary
 from ...helpers import check_or_scale_app, get_app_name, is_relation_joined
 from .helpers import (
+    assert_created_user_can_connect,
     get_application_relation_data,
     get_connection_string,
     verify_application_data,
@@ -31,9 +32,14 @@
 MULTIPLE_DATABASE_CLUSTERS_RELATION_NAME = "multiple-database-clusters"
 ALIASED_MULTIPLE_DATABASE_CLUSTERS_RELATION_NAME = "aliased-multiple-database-clusters"
 ANOTHER_DATABASE_APP_NAME = "another-database"
+ANOTHER_APPLICATION_NAME = "another-application"
 APP_NAMES = [APPLICATION_APP_NAME, ANOTHER_DATABASE_APP_NAME]
 
 
+USER_CREATED_FROM_APP1 = "test_user_1"
+PW_CREATED_FROM_APP1 = "test_user_pass_1"
+
+
 @pytest.mark.runner(["self-hosted", "linux", "X64", "jammy", "large"])
 @pytest.mark.group(1)
 @pytest.mark.abort_on_fail
@@ -278,11 +284,22 @@ async def test_user_with_extra_roles(ops_test: OpsTest):
         connectTimeoutMS=2000,
     )
     client.admin.command(
-        "createUser", "newTestUser", pwd="Test123", roles=[{"role": "readWrite", "db": database}]
+        "createUser",
+        USER_CREATED_FROM_APP1,
+        pwd=PW_CREATED_FROM_APP1,
+        roles=[{"role": "readWrite", "db": database}],
     )
     client["new_database"]
     client.close()
 
+    db_app_name = (
+        await get_app_name(ops_test, test_deployments=[ANOTHER_DATABASE_APP_NAME])
+        or DATABASE_APP_NAME
+    )
+    await assert_created_user_can_connect(
+        ops_test, db_app_name, username=USER_CREATED_FROM_APP1, password=PW_CREATED_FROM_APP1
+    )
+
 
 @pytest.mark.runner(["self-hosted", "linux", "X64", "jammy", "large"])
 @pytest.mark.group(1)
@@ -291,14 +308,13 @@ async def test_two_applications_doesnt_share_the_same_relation_data(
 ):
     """Test that two different application connect to the database with different credentials."""
     # Set some variables to use in this test.
-    another_application_app_name = "another-application"
-    all_app_names = [another_application_app_name]
+    all_app_names = [ANOTHER_APPLICATION_NAME]
     all_app_names.extend(APP_NAMES)
 
     # Deploy another application.
     await ops_test.model.deploy(
         application_charm,
-        application_name=another_application_app_name,
+        application_name=ANOTHER_APPLICATION_NAME,
     )
     await ops_test.model.wait_for_idle(apps=all_app_names, status="active")
 
@@ -309,7 +325,7 @@ async def test_two_applications_doesnt_share_the_same_relation_data(
     # Relate the new application with the database
     # and wait for them exchanging some connection data.
     await ops_test.model.integrate(
-        f"{another_application_app_name}:{FIRST_DATABASE_RELATION_NAME}", db_app_name
+        f"{ANOTHER_APPLICATION_NAME}:{FIRST_DATABASE_RELATION_NAME}", db_app_name
     )
     await ops_test.model.wait_for_idle(apps=all_app_names, status="active")
 
@@ -319,7 +335,7 @@ async def test_two_applications_doesnt_share_the_same_relation_data(
     )
 
     another_application_connection_string = await get_connection_string(
-        ops_test, another_application_app_name, FIRST_DATABASE_RELATION_NAME
+        ops_test, ANOTHER_APPLICATION_NAME, FIRST_DATABASE_RELATION_NAME
     )
     assert application_connection_string != another_application_connection_string
 
@@ -470,3 +486,12 @@ async def test_removed_relation_no_longer_has_access(ops_test: OpsTest):
     assert (
         removed_access
     ), "application: {APPLICATION_APP_NAME} still has access to mongodb after relation removal."
+
+    # mongodb should not clean up users it does not manage.
+    db_app_name = (
+        await get_app_name(ops_test, test_deployments=[ANOTHER_DATABASE_APP_NAME])
+        or DATABASE_APP_NAME
+    )
+    await assert_created_user_can_connect(
+        ops_test, db_app_name, username=USER_CREATED_FROM_APP1, password=PW_CREATED_FROM_APP1
+    )

tests/unit/test_mongodb_provider.py

@@ -1,6 +1,7 @@
 # Copyright 2024 Canonical Ltd.
 # See LICENSE file for licensing details.
 
+import json
 import unittest
 from unittest import mock
 from unittest.mock import patch
@@ -148,6 +149,9 @@ def test_oversee_users_get_users_failure(self, connection):
     def test_oversee_users_drop_user_failure(self, connection, relation_users):
         """Verifies that when unable to drop users from mongod an exception is raised."""
         # presets, such that there is a need to drop users.
+        self.harness.charm.app_peer_data["managed-users-key"] = json.dumps(
+            ["relation-user1", "relation-user2"]
+        )
         relation_users.return_value = {"relation-user1"}
         connection.return_value.__enter__.return_value.get_users.return_value = {
             "relation-user1",
@@ -268,6 +272,7 @@ def test_oversee_users_update_get_config_failure(self, connection, relation_user
     def test_oversee_users_update_user_failure(self, connection, relation_users, get_config):
         """Verifies that when updating users fails an exception is raised."""
         # presets, such that the need to update user relations is triggered
+        self.harness.charm.app_peer_data["managed-users-key"] = json.dumps(["relation-user1"])
         relation_users.return_value = {"relation-user1"}
         connection.return_value.__enter__.return_value.get_users.return_value = {"relation-user1"}