Merge pull request #78 from seb-elico/7.0.1.0-fix-chown-volumes

[FIX][7.0.1.0] Ensure odoo user owns volumes only if they're not readonly

Dockerfile

@@ -79,8 +79,7 @@ USER odoo
 
 # If the folders are created with "RUN mkdir" command, they will belong to root
 # instead of odoo! Hence the "RUN /bin/bash -c" trick.
-RUN /bin/bash -c "mkdir -p /opt/odoo/{bin,etc,sources/odoo,additional_addons,data,ssh}"
-RUN /bin/bash -c "mkdir -p /opt/odoo/var/{run,log,egg-cache}"
+RUN /bin/bash -c "mkdir -p /opt/odoo/{etc,sources/odoo,additional_addons,data,ssh}"
 
 # Add Odoo OCB sources and remove .git folder in order to reduce image size
 WORKDIR /opt/odoo/sources
@@ -98,7 +97,6 @@ User 0
 RUN chmod -R 775 /opt/odoo && chown -R odoo:odoo /opt/odoo
 
 VOLUME [ \
-  "/opt/odoo/var", \
   "/opt/odoo/etc", \
   "/opt/odoo/additional_addons", \
   "/opt/odoo/data", \

bin/boot

@@ -20,12 +20,32 @@ function man {
     set -e
 }
 
-function _ensure_odoo_user_owns_folder_if_exists {
-    if [ -d $1 ]; then
-        chown $odoo_user:$odoo_user $1
+function _ensure_odoo_user_owns_volume {
+    # Make sure the folder exists
+    if [ -d "$1" ]; then
+        # Check if the volume has been mounted read-only
+        mount_type=$( cat /proc/mounts | grep "\s$1\s" | \
+            awk '{print tolower(substr($4,0,3))}' )
+
+        if [ "$mount_type" != 'ro' ]; then
+            # Set target user as owner
+            chown "$odoo_user":"$odoo_user" "$1"
+        else
+            echo $log_src[`date +%F.%H:%M:%S`]' Read-only volume:' "$1"
+        fi
     fi
 }
 
+function _ensure_odoo_user_owns_volumes {
+    _ensure_odoo_user_owns_volume /opt/odoo/etc
+    _ensure_odoo_user_owns_volume /opt/odoo/additional_addons
+    _ensure_odoo_user_owns_volume /opt/odoo/data
+    _ensure_odoo_user_owns_volume /opt/odoo/data/filestore
+    _ensure_odoo_user_owns_volume /opt/odoo/data/sessions
+    _ensure_odoo_user_owns_volume /opt/odoo/data/addons
+    _ensure_odoo_user_owns_volume /opt/odoo/ssh
+}
+
 function _update_odoo_conf_params {
     # Loop over all the "ODOO_" ENV variables (see `<<<` after `done`)
     while read -r env_var; do
@@ -35,38 +55,39 @@ function _update_odoo_conf_params {
 
         # Get the value of the corresponding ENV variable and escape slashes
         val=${!env_var}
-        val=$( echo $val | sed 's/\//\\\//g')
+        val=$( echo "$val" | sed 's/\//\\\//g')
 
         # FIXME Should not be an external script (see reason in script header)
-        bash /app/bin/update_odoo_param.sh $odoo_user $odoo_conf_file $odoo_param $val
+        bash /app/bin/update_odoo_param.sh "$odoo_user" "$odoo_conf_file" \
+            "$odoo_param" "$val"
 
         # Unset the environment variable for security purpose
-        unset $env_var
+        unset "$env_var"
     done <<< "$( printenv | grep '^ODOO_' | sed 's/=.*//g' )"
 }
 
 function _setup_ssh_key {
     # Create SSH config folder in $HOME folder of Odoo target user
-    ssh_folder=$( getent passwd $odoo_user | cut -d: -f6 )/.ssh
-    sudo -i -u $odoo_user mkdir $ssh_folder
+    ssh_folder=$( getent passwd "$odoo_user" | cut -d: -f6 )/.ssh
+    sudo -i -u "$odoo_user" mkdir "$ssh_folder"
 
     # Copy SSH private key from /opt/odoo/ssh
-    sudo -i -u $odoo_user cp /opt/odoo/ssh/id_rsa $ssh_folder
+    sudo -i -u "$odoo_user" cp /opt/odoo/ssh/id_rsa "$ssh_folder"
 
     echo $log_src[`date +%F.%H:%M:%S`]' Scanning GitHub key...'
     # Hide ssh-keyscan stderr output since it's actually log message
     ssh-keyscan github.com 2> /dev/null | \
-        sudo -i -u $odoo_user tee $ssh_folder/known_hosts > /dev/null
+        sudo -i -u "$odoo_user" tee "$ssh_folder/known_hosts" > /dev/null
 
     # Bind SSH key to GitHub host
     echo "host github.com
             HostName github.com
             User git
             IdentityFile $ssh_folder/id_rsa" | \
-        sudo -i -u $odoo_user tee $ssh_folder/config > /dev/null
+        sudo -i -u "$odoo_user" tee "$ssh_folder/config" > /dev/null
 
     # Secure SSH key
-    chmod 400 $ssh_folder/id_rsa
+    chmod 400 "$ssh_folder/id_rsa"
 }
 
 function _download_addons {
@@ -75,8 +96,8 @@ function _download_addons {
     #  2) There's a file called `oca_dependencies.txt` at the root of extra addons folder
     if [ "$ADDONS_REPO" -o -a /opt/odoo/additional_addons/oca_dependencies.txt ]; then
         # Git config for target user
-        sudo -i -u $odoo_user git config --global user.email "[email protected]"
-        sudo -i -u $odoo_user git config --global user.name "Elico Corp - Odoo Docker"
+        sudo -i -u "$odoo_user" git config --global user.email "[email protected]"
+        sudo -i -u "$odoo_user" git config --global user.name "Elico Corp - Odoo Docker"
 
         # Setup SSH key
         if [ -a /opt/odoo/ssh/id_rsa ]; then
@@ -89,10 +110,12 @@ function _download_addons {
         fi
 
         echo $log_src[`date +%F.%H:%M:%S`]' Downloading additional addons...'
-        sudo -i -u $odoo_user python /opt/odoo/auto_addons/addons.py $FETCH_OCA_DEPENDENCIES $ADDONS_REPO
+        sudo -i -u "$odoo_user" python /opt/odoo/auto_addons/addons.py \
+            "$FETCH_OCA_DEPENDENCIES" "$ADDONS_REPO"
     else
         # No additional addons to download
-        sudo -i -u $odoo_user bash /opt/odoo/auto_addons/no_addons.sh $odoo_conf_file
+        # FIXME Should not be an external script (see reason in script header)
+        bash /app/bin/no_addons.sh "$odoo_user" "$odoo_conf_file"
     fi
 }
 
@@ -106,13 +129,7 @@ function start {
     # If the folders mapped to the volumes didn't exist, Docker has created
     # them with root instead of the target Odoo user. Making sure to give back
     # the ownership to the corresponding host user.
-    chown $odoo_user:$odoo_user /opt/odoo/{etc,additional_addons,data,var} \
-                                /opt/odoo/var/{run,log,egg-cache}
-
-    # The following folders might not exist
-    _ensure_odoo_user_owns_folder_if_exists /opt/odoo/data/filestore
-    _ensure_odoo_user_owns_folder_if_exists /opt/odoo/data/sessions
-    _ensure_odoo_user_owns_folder_if_exists /opt/odoo/data/addons
+    _ensure_odoo_user_owns_volumes
 
     echo $log_src[`date +%F.%H:%M:%S`]' Checking special requirements...'
     bash /opt/scripts/startup.sh
@@ -126,29 +143,30 @@ function start {
 
     echo $log_src[`date +%F.%H:%M:%S`]' Running odoo...'
     set +e
-    if [ ! -e $1 ]; then
-        echo $log_src[`date +%F.%H:%M:%S`]' ...with additional args: ' $*
+    if [ ! -e "$1" ]; then
+        echo $log_src[`date +%F.%H:%M:%S`]' ...with additional args:' "$*"
     fi
-    sudo -i -u $odoo_user /usr/bin/python \
-                          /opt/odoo/sources/odoo/$BINARY_NAME \
-                          -c $odoo_conf_file \
-                          $*
+    sudo -i -u "$odoo_user" /usr/bin/python \
+                            "/opt/odoo/sources/odoo/$BINARY_NAME" \
+                            -c "$odoo_conf_file" \
+                            $*
 
-    SERVICE_PID=$!
+    SERVICE_PID="$!"
     set -e
 }
 
 # smart shutdown on SIGINT and SIGTERM
 function on_exit() {
-    kill -TERM $SERVICE_PID
-    wait $SERVICE_PID 2> /dev/null
+    kill -TERM "$SERVICE_PID"
+    wait "$SERVICE_PID" 2> /dev/null
     exit 0
 }
 trap on_exit INT TERM
 
 echo $log_src[`date +%F.%H:%M:%S`]' Running command...'
 for arg in "$*"
 do
+    # Not protected with `"` in order to pass the arguments
     $arg
 done
 

auto_addons/no_addons.sh → bin/no_addons.sh

@@ -9,7 +9,14 @@
 #
 log_src='['${0##*/}']'
 
-grep -P "^addons_path\s*=" $1
-if [ $? -ne 0 ]; then
-    echo "addons_path = /opt/odoo/sources/odoo/addons" >> $1
-fi
+odoo_user="$1"
+odoo_conf_file="$2"
+
+grep -q '^addons_path\s*=' "$odoo_conf_file"
+found="$?"
+
+if [ "$found" -ne 0 ]; then
+    # Append the parameter (hide tee output to stdout)
+    echo 'addons_path = /opt/odoo/sources/odoo/addons' | \
+        sudo -i -u "$odoo_user" tee -a "$odoo_conf_file" > /dev/null
+fi

bin/target_user.sh

@@ -12,31 +12,39 @@ odoo_user='odoo'
 
 # Check if there's a target user to run Odoo
 if [ "$TARGET_UID" ]; then
+    # Name of the target Odoo user
+    TARGET_USER_NAME='target-odoo-user'
+
     # Check whether target user exists or not
-    exists=$( getent passwd $TARGET_UID | wc -l )
+    exists=$( getent passwd "$TARGET_UID" | wc -l )
 
     # Create target user
-    if [ $exists == "0" ]; then
+    if [ "$exists" == "0" ]; then
         echo $log_src[`date +%F.%H:%M:%S`]' Creating target Odoo user...'
-        odoo_user='target-odoo-user'
-        adduser --uid $TARGET_UID --disabled-login --gecos "" --quiet $odoo_user
+        odoo_user="$TARGET_USER_NAME"
+        adduser --uid "$TARGET_UID" --disabled-login --gecos "" --quiet \
+            "$odoo_user"
 
         # Add target user to odoo group so that he can read/write the content
         # of /opt/odoo
-        usermod -a -G odoo $odoo_user
+        usermod -a -G odoo "$odoo_user"
     else
-        # Target user already exists, make sure it's odoo
-        odoo_user_id=$( id -u $odoo_user )
+        # Target user already exists in the following cases:
+        #  1) Mapping with the same UID as odoo, OK
+        #  2) Target user has already been created (e.g. container has been
+        #     restarted), OK
+        #  3) Mapping with another existing user (e.g. root, etc.), not OK
+        odoo_user_id=$( id -u "$odoo_user" )
+        target_uid_name=$( getent passwd "$TARGET_UID" | cut -d: -f1 )
 
-        # If the user already exists, check if it's the same as odoo
-        if [ $TARGET_UID -ne $odoo_user_id ]; then
-            echo $log_src[`date +%F.%H:%M:%S`]' ERROR: The UID of the target' \
-                'user already exists but it is not the same as the ID of' \
-                '`odoo` user'
+        if [ "$TARGET_UID" -ne "$odoo_user_id" ] && \
+                [ "$TARGET_USER_NAME" != "$target_uid_name" ]; then
+            echo $log_src[`date +%F.%H:%M:%S`]' ERROR: Cannot create target' \
+                'user as target UID already exists.'
             exit 1
         fi
     fi
 fi
 
 # Return target Odoo user to boot script
-echo $odoo_user > /tmp/odoo_user
+echo "$odoo_user" > /tmp/odoo_user

bin/update_odoo_param.sh

@@ -1,27 +1,28 @@
 #!/bin/bash
 #
 # FIXME the below code should be in boot but for an unknown reason, the
-# instruction `grep -q "^$param\s*=" $odoo_conf_file` fails to run in boot, when it
-# works fine here.
+# instruction `grep` fails to run in boot, when it works fine here.
 # Both are bash scripts, but:
 #  - boot is run by Docker as an entrypoint
 #  - target_user.sh is run using bash command
 #
 log_src='['${0##*/}']'
 
-odoo_user=$1
-odoo_conf_file=$2
-odoo_param=$3
-val=$4
+odoo_user="$1"
+odoo_conf_file="$2"
+odoo_param="$3"
+val="$4"
 
 # Check if the conf already contains that parameter
-grep -q "^$odoo_param\s*=" $odoo_conf_file
-found=$?
+grep -q "^$odoo_param\s*=" "$odoo_conf_file"
+found="$?"
 
-if [ $found -eq 0 ]; then
+if [ "$found" -eq 0 ]; then
     # Substitute the value
-    sudo -i -u $odoo_user sed -i "s/^$odoo_param\s*=.*/$odoo_param = $val/g" $odoo_conf_file
+    sudo -i -u "$odoo_user" sed -i \
+        "s/^$odoo_param\s*=.*/$odoo_param = $val/g" "$odoo_conf_file"
 else
     # Append the parameter (hide tee output to stdout)
-    echo "$odoo_param = $val" | sudo -i -u $odoo_user tee -a $odoo_conf_file > /dev/null
+    echo "$odoo_param = $val" | \
+        sudo -i -u "$odoo_user" tee -a "$odoo_conf_file" > /dev/null
 fi