Merge ~jsimpso/prometheus-snap:master into prometheus-snap:master

[James Simpson]

Improve file ownership logic (fixes lp:1992713)

[Loïc Gomez (kotodama)]

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 7ff4c5fc6e6674784c27cd04540fffe081bc3f27

diff --git a/snap_config_wrapper b/snap_config_wrapper
index a3a85ae..3392e57 100755
--- a/snap_config_wrapper
+++ b/snap_config_wrapper
@@ -9,6 +9,10 @@ SNAP_DB_DIR="${SNAP_COMMON}/tsdb"
 OWNER_USER="snap_daemon"
 OWNER_GROUP="root"
 
+# Drop privileges with setpriv and run as snap_daemon
+# https://snapcraft.io/docs/system-usernames
+DROP_PRIVS="${SNAP}/usr/bin/setpriv --clear-groups --reuid snap_daemon --regid snap_daemon --"
+
 # Snaps can't manage ownership of SNAP_COMMON directly, so we'll move the DB
 # to a subdir of SNAP_COMMON -- https://forum.snapcraft.io/t/system-usernames/13386/12
 if [ ! -d "${SNAP_DB_DIR}" ]; then
@@ -17,16 +21,20 @@ if [ ! -d "${SNAP_DB_DIR}" ]; then
     find "${SNAP_COMMON}" -maxdepth 1 -mindepth 1 -not -name tsdb -exec mv -t "${SNAP_DB_DIR}" {} +
 fi
 
-find "${SNAP_DB_DIR}" -type d \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec chmod 775 {} \; -exec chown "${OWNER_USER}:${OWNER_GROUP}" {} \;
-find "${SNAP_DB_DIR}" -type f \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec chmod 664 {} \; -exec chown "${OWNER_USER}:${OWNER_GROUP}" {} \;
+# Ensure ownership is as expected
+find "${SNAP_DB_DIR}" -type d \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec chown "${OWNER_USER}:${OWNER_GROUP}" {} \;
+
 
 for DIR in ${SNAP_DATA_DIRS}
 do
     if [ ! -d "${SNAP_DATA}/${DIR}" ]; then
         mkdir "${SNAP_DATA}/${DIR}"
     fi
-    find "${SNAP_DATA}/${DIR}" -type d \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec chmod 775 {} \; -exec chown "${OWNER_USER}:${OWNER_GROUP}" {} \;
-    find "${SNAP_DATA}/${DIR}" -type f \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec chmod 664 {} \; -exec chown "${OWNER_USER}:${OWNER_GROUP}" {} \;
+    # Ensure ownership is as expected
+    find "${SNAP_DATA}/${DIR}" -type d \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec chown "${OWNER_USER}:${OWNER_GROUP}" {} \;
+    # Ensure permissions are as expexted
+    find "${SNAP_DATA}/${DIR}" -type d \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec ${DROP_PRIVS} chmod 775 {} \;
+    find "${SNAP_DATA}/${DIR}" -type f \( ! -user "${OWNER_USER}" -o ! -group "${OWNER_GROUP}" \) -exec ${DROP_PRIVS} chmod 664 {} \;
 done
 
 for FILE in ${SNAP_DATA_FILES}
@@ -36,8 +44,14 @@ do
     fi
         file_user=$(stat "${SNAP_DATA}/${FILE}" --format '%U')
         file_group=$(stat "${SNAP_DATA}/${FILE}" --format '%G')
+        file_mode=$(stat "%{SNAP_DATA}/${FILE}" --format '%a')
+        # Ensure ownership is correct
         if [ "${file_user}" != "${OWNER_USER}" ] || [ "${file_group}" != "${OWNER_GROUP}" ]; then
-           chmod 664 "${SNAP_DATA}/${FILE}" && chown "${OWNER_USER}:${OWNER_GROUP}" "${SNAP_DATA}/${FILE}"
+            chown "${OWNER_USER}:${OWNER_GROUP}" "${SNAP_DATA}/${FILE}"
+        fi
+        # Ensure permissions are correct
+        if [ "${file_mode}" != '664' ]; then
+            ${DROP_PRIVS} chmod 664 "${SNAP_DATA}/${FILE}"
         fi
 
 done
@@ -46,8 +60,6 @@ ulimit -n 65536
 . "${SNAP_DATA}"/daemon_arguments
 
 COMMAND="${SNAP}/bin/prometheus ${ARGS} --config.file ${SNAP_DATA}/prometheus.yml --storage.tsdb.path=${SNAP_COMMON}/tsdb"
-# Drop privileges with setpriv and run as snap_daemon
-# https://snapcraft.io/docs/system-usernames
-DROP_PRIVS="${SNAP}/usr/bin/setpriv --clear-groups --reuid snap_daemon --regid snap_daemon --"
+
 
 exec ${DROP_PRIVS} ${COMMAND}
 \ No newline at end of file