scripts: restructure container restart behavior (#4323)

Signed-off-by: georglauterbach <44545919+georglauterbach@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -19,8 +19,6 @@ All notable changes to this project will be documented in this file. The format
   - DMS v14 mistakenly relocated the _getmail state directory_ to the _DMS Config Volume_ as a `getmail/` subdirectory.
     - This has been corrected to `/var/lib/getmail` (_if you have mounted a DMS State Volume to `/var/mail-state`, `/var/lib/getmail` will be symlinked to `/var/mail-state/lib-getmail`_).
     - To preserve this state when upgrading to DMS v15, **you must manually migrate `getmail/` from the _DMS Config Volume_ to `lib-getmail/` in the _DMS State Volume_.**
-  - Added missing `debug getmail` subcommand to `setup` ([#4346](https://github.com/docker-mailserver/docker-mailserver/pull/4346))
-- **removed `VERSION`** file that was used for checking version updates ([#3677](https://github.com/docker-mailserver/docker-mailserver/issues/3677),[#4321](https://github.com/docker-mailserver/docker-mailserver/pull/4321))
 
 ### Security
 
@@ -31,9 +29,11 @@ All notable changes to this project will be documented in this file. The format
 
 - **Internal:**
   - Add password confirmation to several `setup` CLI subcommands ([#4072](https://github.com/docker-mailserver/docker-mailserver/pull/4072))
+  - Added a `debug getmail` subcommand to `setup` ([#4346](https://github.com/docker-mailserver/docker-mailserver/pull/4346))
 
 ### Updates
 
+- **Removed `VERSION` file** from the repo that releases of DMS prior to v13 (Nov 2023) would check to detect new releases ([#3677](https://github.com/docker-mailserver/docker-mailserver/issues/3677), [#4321](https://github.com/docker-mailserver/docker-mailserver/pull/4321))
 - **Fail2ban:**
   - Updated to version [`1.1.0`](https://github.com/fail2ban/fail2ban/releases/tag/1.1.0) ([#4045](https://github.com/docker-mailserver/docker-mailserver/pull/4045))
 - **Documentation:**
@@ -54,11 +54,12 @@ All notable changes to this project will be documented in this file. The format
   - Correctly apply a compatibility fix for OAuth2 introduced in DMS v13.3.1 which had not been applied to the actual LDAP config changes ([#4175](https://github.com/docker-mailserver/docker-mailserver/pull/4175))
 - **Internal:**
   - The main `mail.log` (_which is piped to stdout via `tail`_) now correctly begins from the first log line of the active container run. Previously some daemon logs and potential warnings/errors were omitted ([#4146](https://github.com/docker-mailserver/docker-mailserver/pull/4146))
-  - Fixed a regression introduced in v14 where `postfix-main.cf` appended `stderr` output into `/etc/postfix/main.cf`, causing Postfix startup to fail ([#4147](https://github.com/docker-mailserver/docker-mailserver/pull/4147))
   - `start-mailserver.sh` removed unused `shopt -s inherit_errexit` ([#4161](https://github.com/docker-mailserver/docker-mailserver/pull/4161))
+  - Fixed a regression introduced in v14 where `postfix-main.cf` appended `stderr` output into `/etc/postfix/main.cf`, causing Postfix startup to fail ([#4147](https://github.com/docker-mailserver/docker-mailserver/pull/4147))
+  - Fixed a regression introduced in v14 to better support running `start-mailserver.sh` with container restarts, which now only skip calling `_setup()` ([#4323](https://github.com/docker-mailserver/docker-mailserver/pull/4323#issuecomment-2629559254))
+  - The command `swaks --help` is now functional ([#4282](https://github.com/docker-mailserver/docker-mailserver/pull/4282))
 - **Rspamd:**
   - DKIM private key path checking is now performed only on paths that do not contain `$` ([#4201](https://github.com/docker-mailserver/docker-mailserver/pull/4201))
-- The command `swaks --help` is now functional ([#4282](https://github.com/docker-mailserver/docker-mailserver/pull/4282))
 
 ### CI
 

target/scripts/start-mailserver.sh

@@ -38,7 +38,6 @@ function _register_functions() {
   # ? >> Checks
 
   _register_check_function '_check_hostname'
-  _register_check_function '_check_log_level'
   _register_check_function '_check_spam_prefix'
 
   # ? >> Setup
@@ -63,7 +62,6 @@ function _register_functions() {
       ;;
 
     ( 'LDAP' )
-      _environment_variables_ldap
       _register_setup_function '_setup_ldap'
       ;;
 
@@ -76,15 +74,8 @@ function _register_functions() {
       ;;
   esac
 
-  if [[ ${ENABLE_OAUTH2} -eq 1 ]]; then
+  [[ ${ENABLE_OAUTH2} -eq 1 ]] && _register_setup_function '_setup_oauth2'
-      _environment_variables_oauth2
+  [[ ${ENABLE_SASLAUTHD} -eq 1 ]] && _register_setup_function '_setup_saslauthd'
-      _register_setup_function '_setup_oauth2'
-  fi
-
-  if [[ ${ENABLE_SASLAUTHD} -eq 1 ]]; then
-    _environment_variables_saslauthd
-    _register_setup_function '_setup_saslauthd'
-  fi
 
   _register_setup_function '_setup_dovecot_inet_protocols'
 
@@ -122,14 +113,17 @@ function _register_functions() {
   _register_setup_function '_setup_logwatch'
 
   _register_setup_function '_setup_save_states'
-  _register_setup_function '_setup_apply_fixes_after_configuration'
+  _register_setup_function '_setup_adjust_state_permissions'
-  _register_setup_function '_environment_variables_export'
 
   if [[ ${ENABLE_MTA_STS} -eq 1 ]]; then
     _register_setup_function '_setup_mta_sts'
     _register_start_daemon '_start_daemon_mta_sts_daemon'
   fi
 
+  # ! The following functions must be executed after all other setup functions
+  _register_setup_function '_setup_directory_and_file_permissions'
+  _register_setup_function '_setup_run_user_patches'
+
   # ? >> Daemons
 
   _register_start_daemon '_start_daemon_cron'
@@ -174,9 +168,6 @@ function _register_functions() {
 # ? >> Executing all stacks / actual start of DMS
 # ------------------------------------------------------------
 
-# Ensure DMS only adjusts config files for a new container.
-# Container restarts should skip as they retain the modified config.
-if [[ ! -f /CONTAINER_START ]]; then
 _early_supervisor_setup
 _early_variables_setup
 
@@ -184,16 +175,17 @@ if [[ ! -f /CONTAINER_START ]]; then
 
 _register_functions
 _check
-  _setup
+
-  _run_user_patches
+# Ensure DMS only adjusts config files for a new container.
+# Container restarts should skip as they retain the modified config.
+if [[ -f /CONTAINER_START ]]; then
+  _log 'info' 'Container was restarted. Skipping most setup routines.'
+  # We cannot skip all setup routines because some need to run _after_
+  # the initial setup (and hence, they cannot be moved to the check stack).
+  _setup_directory_and_file_permissions
+  _setup_adjust_state_permissions
 else
-  # container was restarted
+  _setup
-  _early_variables_setup
-
-  _log 'info' 'Container was restarted. Skipping setup routines.'
-  _log 'info' "Welcome to docker-mailserver ${DMS_RELEASE}"
-
-  _register_functions
 fi
 
 # marker to check if container was restarted

target/scripts/startup/check-stack.sh

@@ -26,24 +26,6 @@ function _check_hostname() {
   fi
 }
 
-function _check_log_level() {
-  if [[ ${LOG_LEVEL} == 'trace' ]] \
-  || [[ ${LOG_LEVEL} == 'debug' ]] \
-  || [[ ${LOG_LEVEL} == 'info' ]]  \
-  || [[ ${LOG_LEVEL} == 'warn' ]]  \
-  || [[ ${LOG_LEVEL} == 'error' ]]
-  then
-    return 0
-  else
-    local DEFAULT_LOG_LEVEL='info'
-    _log 'warn' "Log level '${LOG_LEVEL}' is invalid (falling back to default '${DEFAULT_LOG_LEVEL}')"
-
-    # shellcheck disable=SC2034
-    VARS[LOG_LEVEL]="${DEFAULT_LOG_LEVEL}"
-    LOG_LEVEL="${DEFAULT_LOG_LEVEL}"
-  fi
-}
-
 function _check_spam_prefix() {
   # This check should be independent of ENABLE_POP3 and ENABLE_IMAP
   if [[ ${MOVE_SPAM_TO_JUNK} -eq 0 ]] \

target/scripts/startup/setup-stack.sh

@@ -82,7 +82,7 @@ function _setup_timezone() {
   fi
 }
 
-function _setup_apply_fixes_after_configuration() {
+function _setup_directory_and_file_permissions() {
   _log 'trace' 'Removing leftover PID files from a stop/start'
   find /var/run/ -not -name 'supervisord.pid' -name '*.pid' -delete
   touch /dev/shm/supervisor.sock
@@ -103,7 +103,7 @@ function _setup_apply_fixes_after_configuration() {
   fi
 }
 
-function _run_user_patches() {
+function _setup_run_user_patches() {
   local USER_PATCHES='/tmp/docker-mailserver/user-patches.sh'
 
   if [[ -f ${USER_PATCHES} ]]; then

target/scripts/startup/setup.d/mail_state.sh

@@ -3,18 +3,20 @@
 # Consolidate all states into a single directory
 # (/var/mail-state) to allow persistence using docker volumes
 function _setup_save_states() {
-  local DEST DESTDIR STATEDIR SERVICEDIR SERVICEDIRS SERVICEFILE SERVICEFILES
+  if [[ ! -d ${DMS_STATE_DIR:?DMS_STATE_DIR is not set} ]]; then
+    _log 'debug' "'${DMS_STATE_DIR}' is not present - not consolidating state"
+    return 0
+  fi
 
-  STATEDIR='/var/mail-state'
+  _log 'debug' "Consolidating all state onto ${DMS_STATE_DIR}"
 
-  if [[ -d ${STATEDIR} ]]; then
+  local DEST SERVICEDIR SERVICEDIRS SERVICEFILE SERVICEFILES
-    _log 'debug' "Consolidating all state onto ${STATEDIR}"
 
   # Always enabled features:
   SERVICEDIRS=(
-      lib/logrotate
+    'lib/logrotate'
-      lib/postfix
+    'lib/postfix'
-      spool/postfix
+    'spool/postfix'
   )
 
   # Only consolidate state for services that are enabled
@@ -36,10 +38,10 @@ function _setup_save_states() {
   [[ ${ENABLE_SRS}          -eq 1 ]] && SERVICEFILES+=('/etc/postsrsd.secret')
 
   for SERVICEFILE in "${SERVICEFILES[@]}"; do
-      DEST="${STATEDIR}/${SERVICEFILE}"
+    DEST="${DMS_STATE_DIR}/${SERVICEFILE}"
-      DESTDIR="${DEST%/*}"
 
-      mkdir -p "${DESTDIR}"
+    # Append service parent dir(s) path to the state dir and ensure it exists:
+    mkdir -p "${DEST%/*}"
     if [[ -f ${DEST} ]]; then
       _log 'trace' "Destination ${DEST} exists, linking ${SERVICEFILE} to it"
       # Original content from image no longer relevant, remove it:
@@ -50,7 +52,7 @@ function _setup_save_states() {
       mv "${SERVICEFILE}" "${DEST}"
       # Apply SELinux security context to match the state directory, so access
       # is not restricted to the current running container:
-        chcon -R --reference="${STATEDIR}" "${DEST}" 2>/dev/null || true
+      chcon -R --reference="${DMS_STATE_DIR}" "${DEST}" 2>/dev/null || true
     fi
 
     # Symlink the original file in the container ($SERVICEFILE) to be
@@ -59,7 +61,7 @@ function _setup_save_states() {
   done
 
   for SERVICEDIR in "${SERVICEDIRS[@]}"; do
-      DEST="${STATEDIR}/${SERVICEDIR//\//-}"
+    DEST="${DMS_STATE_DIR}/${SERVICEDIR//\//-}"
     SERVICEDIR="/var/${SERVICEDIR}"
 
     # If relevant content is found in /var/mail-state (presumably a volume mount),
@@ -74,33 +76,40 @@ function _setup_save_states() {
       mv "${SERVICEDIR}" "${DEST}"
       # Apply SELinux security context to match the state directory, so access
       # is not restricted to the current running container:
-        chcon -R --reference="${STATEDIR}" "${DEST}" 2>/dev/null || true
+      # https://github.com/docker-mailserver/docker-mailserver/pull/3890
+      chcon -R --reference="${DMS_STATE_DIR}" "${DEST}" 2>/dev/null || true
     else
       _log 'error' "${SERVICEDIR} should exist but is missing"
     fi
 
     # Symlink the original path in the container ($SERVICEDIR) to be
-      # sourced from assocaiated path in /var/mail-state/ ($DEST):
+    # sourced from associated path in /var/mail-state/ ($DEST):
     ln -s "${DEST}" "${SERVICEDIR}"
   done
+}
+
+# These corrections are to fix changes to UID/GID values between upgrades,
+# or when ownership/permissions were altered externally on the host (eg: migration or system scripts)
+function _setup_adjust_state_permissions() {
+  [[ ! -d ${DMS_STATE_DIR:?DMS_STATE_DIR is not set} ]] && return 0
 
   # This ensures the user and group of the files from the external mount have their
   # numeric ID values in sync. New releases where the installed packages order changes
   # can change the values in the Docker image, causing an ownership mismatch.
   # NOTE: More details about users and groups added during image builds are documented here:
   # https://github.com/docker-mailserver/docker-mailserver/pull/3011#issuecomment-1399120252
-    _log 'trace' "Fixing ${STATEDIR}/* permissions"
+  _log 'trace' "Ensuring correct ownership + permissions for DMS state dir: '${DMS_STATE_DIR}'"
-    [[ ${ENABLE_AMAVIS}       -eq 1 ]] && chown -R amavis:amavis             "${STATEDIR}/lib-amavis"
+  [[ ${ENABLE_AMAVIS}       -eq 1 ]] && chown -R amavis:amavis             "${DMS_STATE_DIR}/lib-amavis"
-    [[ ${ENABLE_CLAMAV}       -eq 1 ]] && chown -R clamav:clamav             "${STATEDIR}/lib-clamav"
+  [[ ${ENABLE_CLAMAV}       -eq 1 ]] && chown -R clamav:clamav             "${DMS_STATE_DIR}/lib-clamav"
-    [[ ${ENABLE_FETCHMAIL}    -eq 1 ]] && chown -R fetchmail:nogroup         "${STATEDIR}/lib-fetchmail"
+  [[ ${ENABLE_FETCHMAIL}    -eq 1 ]] && chown -R fetchmail:nogroup         "${DMS_STATE_DIR}/lib-fetchmail"
-    [[ ${ENABLE_MTA_STS}      -eq 1 ]] && chown -R _mta-sts:_mta-sts         "${STATEDIR}/lib-mta-sts"
+  [[ ${ENABLE_MTA_STS}      -eq 1 ]] && chown -R _mta-sts:_mta-sts         "${DMS_STATE_DIR}/lib-mta-sts"
-    [[ ${ENABLE_POSTGREY}     -eq 1 ]] && chown -R postgrey:postgrey         "${STATEDIR}/lib-postgrey"
+  [[ ${ENABLE_POSTGREY}     -eq 1 ]] && chown -R postgrey:postgrey         "${DMS_STATE_DIR}/lib-postgrey"
-    [[ ${ENABLE_RSPAMD}       -eq 1 ]] && chown -R _rspamd:_rspamd           "${STATEDIR}/lib-rspamd"
+  [[ ${ENABLE_RSPAMD}       -eq 1 ]] && chown -R _rspamd:_rspamd           "${DMS_STATE_DIR}/lib-rspamd"
-    [[ ${ENABLE_RSPAMD_REDIS} -eq 1 ]] && chown -R redis:redis               "${STATEDIR}/lib-redis"
+  [[ ${ENABLE_RSPAMD_REDIS} -eq 1 ]] && chown -R redis:redis               "${DMS_STATE_DIR}/lib-redis"
-    [[ ${ENABLE_SPAMASSASSIN} -eq 1 ]] && chown -R debian-spamd:debian-spamd "${STATEDIR}/lib-spamassassin"
+  [[ ${ENABLE_SPAMASSASSIN} -eq 1 ]] && chown -R debian-spamd:debian-spamd "${DMS_STATE_DIR}/lib-spamassassin"
 
-    chown -R root:root "${STATEDIR}/lib-logrotate"
+  chown -R root:root "${DMS_STATE_DIR}/lib-logrotate"
-    chown -R postfix:postfix "${STATEDIR}/lib-postfix"
+  chown -R postfix:postfix "${DMS_STATE_DIR}/lib-postfix"
 
   # NOTE: The Postfix spool location has mixed owner/groups to take into account:
   # UID = postfix(101): active, bounce, corrupt, defer, deferred, flush, hold, incoming, maildrop, private, public, saved, trace
@@ -109,17 +118,14 @@ function _setup_save_states() {
   # GID for all other directories is root(0)
   # NOTE: `spool-postfix/private/` will be set to `postfix:postfix` when Postfix starts / restarts
   # Set most common ownership:
-    chown -R postfix:root "${STATEDIR}/spool-postfix"
+  chown -R postfix:root "${DMS_STATE_DIR}/spool-postfix"
-    chown root:root "${STATEDIR}/spool-postfix"
+  chown root:root "${DMS_STATE_DIR}/spool-postfix"
 
   # These two require the postdrop(103) group:
-    chgrp -R postdrop "${STATEDIR}"/spool-postfix/{maildrop,public}
+  chgrp -R postdrop "${DMS_STATE_DIR}"/spool-postfix/{maildrop,public}
 
   # These permissions rely on the `postdrop` binary having the SGID bit set.
   # Ref: https://github.com/docker-mailserver/docker-mailserver/pull/3625
-    chmod 730 "${STATEDIR}/spool-postfix/maildrop"
+  chmod 730 "${DMS_STATE_DIR}/spool-postfix/maildrop"
-    chmod 710 "${STATEDIR}/spool-postfix/public"
+  chmod 710 "${DMS_STATE_DIR}/spool-postfix/public"
-  else
-    _log 'debug' "'${STATEDIR}' is not present; Not consolidating state"
-  fi
 }

target/scripts/startup/variables-stack.sh

@@ -4,9 +4,27 @@
 declare -A VARS
 
 function _early_variables_setup() {
+  __environment_variables_log_level
   _obtain_hostname_and_domainname
   __environment_variables_backwards_compatibility
   __environment_variables_general_setup
+
+  [[ ${ACCOUNT_PROVISIONER} == 'LDAP' ]] && __environment_variables_ldap
+  [[ ${ENABLE_OAUTH2} -eq 1 ]]           && __environment_variables_oauth2
+  [[ ${ENABLE_SASLAUTHD} -eq 1 ]]        && __environment_variables_saslauthd
+
+  __environment_variables_export
+}
+
+# Declare a variable as readonly if it is not already set.
+function __declare_readonly() {
+  local VARIABLE_NAME=${1:?Variable name required when declaring a variable as readonly}
+  local VARIABLE_VALUE=${2:?Variable value required when declaring a variable as readonly}
+
+  if [[ ! -v ${VARIABLE_NAME} ]]; then
+    readonly "${VARIABLE_NAME}=${VARIABLE_VALUE}"
+    VARS[${VARIABLE_NAME}]="${VARIABLE_VALUE}"
+  fi
 }
 
 # This function handles variables that are deprecated. This allows a
@@ -55,6 +73,12 @@ function __environment_variables_general_setup() {
   VARS[DMS_VMAIL_UID]="${DMS_VMAIL_UID:=5000}"
   VARS[DMS_VMAIL_GID]="${DMS_VMAIL_GID:=5000}"
 
+  # internal variables are next
+
+  __declare_readonly 'DMS_STATE_DIR' '/var/mail-state'
+
+  # user-customizable are last
+
   _log 'trace' 'Setting anti-spam & anti-virus environment variables'
 
   VARS[AMAVIS_LOGLEVEL]="${AMAVIS_LOGLEVEL:=0}"
@@ -159,15 +183,27 @@ function __environment_variables_general_setup() {
   VARS[UPDATE_CHECK_INTERVAL]="${UPDATE_CHECK_INTERVAL:=1d}"
 }
 
-function _environment_variables_oauth2() {
+function __environment_variables_log_level() {
-  _log 'debug' 'Setting OAUTH2-related environment variables now'
+  if [[ ${LOG_LEVEL} == 'trace' ]] \
+  || [[ ${LOG_LEVEL} == 'debug' ]] \
+  || [[ ${LOG_LEVEL} == 'info' ]]  \
+  || [[ ${LOG_LEVEL} == 'warn' ]]  \
+  || [[ ${LOG_LEVEL} == 'error' ]]
+  then
+    return 0
+  else
+    local DEFAULT_LOG_LEVEL='info'
+    _log 'warn' "Log level '${LOG_LEVEL}' is invalid (falling back to default '${DEFAULT_LOG_LEVEL}')"
 
-  VARS[OAUTH2_INTROSPECTION_URL]="${OAUTH2_INTROSPECTION_URL:=}"
+    # shellcheck disable=SC2034
+    VARS[LOG_LEVEL]="${DEFAULT_LOG_LEVEL}"
+    LOG_LEVEL="${DEFAULT_LOG_LEVEL}"
+  fi
 }
 
 # This function handles environment variables related to LDAP.
 # NOTE: SASLAuthd and Dovecot LDAP support inherit these common ENV.
-function _environment_variables_ldap() {
+function __environment_variables_ldap() {
   _log 'debug' 'Setting LDAP-related environment variables now'
 
   VARS[LDAP_BIND_DN]="${LDAP_BIND_DN:=}"
@@ -177,9 +213,15 @@ function _environment_variables_ldap() {
   VARS[LDAP_START_TLS]="${LDAP_START_TLS:=no}"
 }
 
+function __environment_variables_oauth2() {
+  _log 'debug' 'Setting OAUTH2-related environment variables now'
+
+  VARS[OAUTH2_INTROSPECTION_URL]="${OAUTH2_INTROSPECTION_URL:=}"
+}
+
 # This function handles environment variables related to SASLAUTHD
 # LDAP specific ENV handled in: `startup/setup.d/saslauthd.sh:_setup_saslauthd()`
-function _environment_variables_saslauthd() {
+function __environment_variables_saslauthd() {
   _log 'debug' 'Setting SASLAUTHD-related environment variables now'
 
   # This ENV is only used by the supervisor service config `saslauth.conf`:
@@ -190,7 +232,7 @@ function _environment_variables_saslauthd() {
 # This function Writes the contents of the `VARS` map (associative array)
 # to locations where they can be sourced from (e.g. `/etc/dms-settings`)
 # or where they can be used by Bash directly (e.g. `/root/.bashrc`).
-function _environment_variables_export() {
+function __environment_variables_export() {
   _log 'debug' "Exporting environment variables now (creating '/etc/dms-settings')"
 
   : >/root/.bashrc     # make DMS variables available in login shells and their subprocesses