From 2ae57605e8e4258a41fa12850e5a6d621cc9dfcb Mon Sep 17 00:00:00 2001
From: Mykola Marzhan <303592+delgod@users.noreply.github.com>
Date: Thu, 23 Mar 2023 17:27:54 +0100
Subject: [PATCH] Fix data directory permissions with chown (#120)

* Fix data directory permissions with chown

Co-authored-by: Mia Altieri <32723809+MiaAltieri@users.noreply.github.com>
---
 src/charm.py             | 41 ++++++++++++++++++++++++++++------------
 tests/unit/test_charm.py |  8 +++++---
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/src/charm.py b/src/charm.py
index 457b00798..9cbf0be34 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -15,6 +15,7 @@
 
 from charms.mongodb.v0.helpers import (
     CONF_DIR,
+    DATA_DIR,
     KEY_FILE,
     TLS_EXT_CA_FILE,
     TLS_EXT_PEM_FILE,
@@ -46,6 +47,8 @@
     "resource": {"db": "", "collection": ""},
     "actions": ["listIndexes", "listCollections", "dbStats", "dbHash", "collStats", "find"],
 }
+UNIX_USER = "mongodb"
+UNIX_GROUP = "mongodb"
 
 
 class MongoDBCharm(CharmBase):
@@ -91,6 +94,8 @@ def on_mongod_pebble_ready(self, event) -> None:
         try:
             self._push_certificate_to_workload(container)
             self._push_keyfile_to_workload(container)
+            self._fix_data_dir(container)
+
         except (PathError, ProtocolError) as e:
             logger.error("Cannot put keyFile: %r", e)
             event.defer()
@@ -240,8 +245,8 @@ def _mongod_layer(self) -> Layer:
                     "summary": "mongod",
                     "command": "mongod " + get_mongod_args(self.mongodb_config),
                     "startup": "enabled",
-                    "user": "mongodb",
-                    "group": "mongodb",
+                    "user": UNIX_USER,
+                    "group": UNIX_GROUP,
                 }
             },
         }
@@ -323,8 +328,8 @@ def _push_keyfile_to_workload(self, container: Container) -> None:
             self.get_secret("app", "keyfile"),
             make_dirs=True,
             permissions=0o400,
-            user="mongodb",
-            group="mongodb",
+            user=UNIX_USER,
+            group=UNIX_GROUP,
         )
 
     def _push_certificate_to_workload(self, container: Container) -> None:
@@ -337,8 +342,8 @@ def _push_certificate_to_workload(self, container: Container) -> None:
                 external_ca,
                 make_dirs=True,
                 permissions=0o400,
-                user="mongodb",
-                group="mongodb",
+                user=UNIX_USER,
+                group=UNIX_GROUP,
             )
         if external_pem is not None:
             logger.debug("Uploading external pem to workload container")
@@ -347,8 +352,8 @@ def _push_certificate_to_workload(self, container: Container) -> None:
                 external_pem,
                 make_dirs=True,
                 permissions=0o400,
-                user="mongodb",
-                group="mongodb",
+                user=UNIX_USER,
+                group=UNIX_GROUP,
             )
 
         internal_ca, internal_pem = self.tls.get_tls_files("app")
@@ -359,8 +364,8 @@ def _push_certificate_to_workload(self, container: Container) -> None:
                 internal_ca,
                 make_dirs=True,
                 permissions=0o400,
-                user="mongodb",
-                group="mongodb",
+                user=UNIX_USER,
+                group=UNIX_GROUP,
             )
         if internal_pem is not None:
             logger.debug("Uploading internal pem to workload container")
@@ -369,10 +374,22 @@ def _push_certificate_to_workload(self, container: Container) -> None:
                 internal_pem,
                 make_dirs=True,
                 permissions=0o400,
-                user="mongodb",
-                group="mongodb",
+                user=UNIX_USER,
+                group=UNIX_GROUP,
             )
 
+    def _fix_data_dir(self, container: Container) -> None:
+        """Ensure the data directory for mongodb is writable for the "mongodb" user.
+
+        Until the ability to set fsGroup and fsGroupChangePolicy via Pod securityContext
+        is available we fix permissions incorrectly with chown.
+        """
+        paths = container.list_files(DATA_DIR, itself=True)
+        assert len(paths) == 1, "list_files doesn't return only directory itself"
+        logger.debug(f"Data directory ownership: {paths[0].user}:{paths[0].group}")
+        if paths[0].user != UNIX_USER or paths[0].group != UNIX_GROUP:
+            container.exec(f"chown -o {UNIX_USER} -g {UNIX_GROUP} -R {DATA_DIR}".split(" "))
+
     def get_hostname_by_unit(self, unit_name: str) -> str:
         """Create a DNS name for a MongoDB unit.
 
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 300b46cea..05a8459ec 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -5,6 +5,7 @@
 from unittest import mock
 from unittest.mock import patch
 
+from charms.mongodb.v0.helpers import CONF_DIR, DATA_DIR, KEY_FILE
 from ops.model import ActiveStatus, ModelError
 from ops.pebble import APIError, ExecError, PathError, ProtocolError
 from ops.testing import Harness
@@ -44,7 +45,8 @@ def setUp(self):
         self.addCleanup(self.harness.cleanup)
 
     @patch("ops.framework.EventBase.defer")
-    def test_mongod_pebble_ready(self, defer):
+    @patch("charm.MongoDBCharm._fix_data_dir")
+    def test_mongod_pebble_ready(self, fix_data_dir, defer):
         # Expected plan after Pebble ready with default config
         expected_plan = {
             "services": {
@@ -56,9 +58,9 @@ def test_mongod_pebble_ready(self, defer):
                     "command": (
                         "mongod --bind_ip_all "
                         "--replSet=mongodb-k8s "
-                        "--dbpath=/var/lib/mongodb --auth "
+                        f"--dbpath={DATA_DIR} --auth "
                         "--clusterAuthMode=keyFile "
-                        "--keyFile=/etc/mongod/keyFile \n"
+                        f"--keyFile={CONF_DIR}/{KEY_FILE} \n"
                     ),
                     "startup": "enabled",
                 }