diff --git a/Dockerfile b/Dockerfile index b2647000..3f5472ff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -61,7 +61,7 @@ RUN \ pax pflogsumm postgrey p7zip-full postfix-ldap postfix-pcre \ postfix-policyd-spf-python postsrsd pyzor \ razor rpm2cpio rsyslog sasl2-bin spamassassin supervisor \ - unrar-free unzip whois xz-utils && \ + unrar-free unzip uuid whois xz-utils && \ # Fail2Ban gpg --keyserver ${FAIL2BAN_GPG_PUBLIC_KEY_SERVER} \ --recv-keys ${FAIL2BAN_GPG_PUBLIC_KEY_ID} 2>&1 && \ diff --git a/Makefile b/Makefile index c38731d4..f8636644 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ clean: # remove running and stopped test containers -@ [[ -d config.bak ]] && { rm -rf config ; mv config.bak config ; } || : -@ [[ -d testconfig.bak ]] && { sudo rm -rf test/config ; mv testconfig.bak test/config ; } || : - -@ for container in $$(docker ps -a --filter name='^/mail$$|^ldap_for_mail$$|^mail_override_hostname$$|^mail_non_subdomain_hostname$$|^open-dkim$$|^hadolint$$|^eclint$$|^shellcheck$$' | sed 1d | cut -f 1-1 -d ' '); do docker rm -f $$container; done + -@ for container in $$(docker ps -a --filter name='^/mail$$|^ldap_for_mail$$|^mail_override_hostname$$|^mail_non_subdomain_hostname$$|^open-dkim$$|^hadolint$$|^eclint$$|^shellcheck$$|mail_changedetector.*' | sed 1d | cut -f 1-1 -d ' '); do docker rm -f $$container; done -@ sudo rm -rf test/onedir test/alias test/quota test/relay test/config/dovecot-lmtp/userdb test/config/key* test/config/opendkim/keys/domain.tld/ test/config/opendkim/keys/example.com/ test/config/opendkim/keys/localdomain2.com/ test/config/postfix-aliases.cf test/config/postfix-receive-access.cf test/config/postfix-receive-access.cfe test/config/dovecot-quotas.cf test/config/postfix-send-access.cf test/config/postfix-send-access.cfe test/config/relay-hosts/chksum test/config/relay-hosts/postfix-aliases.cf test/config/dhparams.pem test/config/dovecot-lmtp/dh.pem test/config/relay-hosts/dovecot-quotas.cf test/config/user-patches.sh test/alias/config/postfix-virtual.cf test/quota/config/dovecot-quotas.cf test/quota/config/postfix-accounts.cf test/relay/config/postfix-relaymap.cf test/relay/config/postfix-sasl-password.cf test/duplicate_configs/ # ----------------------------------------------- diff --git a/target/bin/addmailuser b/target/bin/addmailuser index 0a0a7f76..1d5355bd 100755 --- a/target/bin/addmailuser +++ b/target/bin/addmailuser @@ -42,9 +42,8 @@ PASSWD="${*}" [[ -z ${FULL_EMAIL} ]] && { __usage ; errex 'No username specified' ; } [[ "${FULL_EMAIL}" =~ .*\@.* ]] || { __usage ; errex 'Username must include the domain' ; } -# Protect config file with lock to avoid race conditions touch "${DATABASE}" -create_lock "$(basename "$0")" +create_lock # Protect config file with lock to avoid race conditions if grep -qi "^$(escape "${FULL_EMAIL}")|" "${DATABASE}" then echo "User '${FULL_EMAIL}' already exists." diff --git a/target/bin/delmailuser b/target/bin/delmailuser index fbb33195..b3740432 100755 --- a/target/bin/delmailuser +++ b/target/bin/delmailuser @@ -86,7 +86,8 @@ then fi fi -create_lock "$(basename "$0")" +create_lock # Protect config file with lock to avoid race conditions + for EMAIL in "${@}" do ERROR=false diff --git a/target/bin/setquota b/target/bin/setquota index 0c6db6bd..1bd7e671 100755 --- a/target/bin/setquota +++ b/target/bin/setquota @@ -32,8 +32,7 @@ then errex "invalid quota format. e.g. 302M (B (byte), k (kilobyte), M (megabyte), G (gigabyte) or T (terabyte))" fi -# Protect config file with lock to avoid race conditions -create_lock "$(basename "$0")" +create_lock # Protect config file with lock to avoid race conditions touch "${DATABASE}" if [ -z "${QUOTA}" ]; then diff --git a/target/bin/updatemailuser b/target/bin/updatemailuser index bd9efabd..b4d8a495 100755 --- a/target/bin/updatemailuser +++ b/target/bin/updatemailuser @@ -27,8 +27,7 @@ fi HASH="$(doveadm pw -s SHA512-CRYPT -u "${USER}" -p "${PASSWD}")" -# Protect config file with lock to avoid race conditions touch "${DATABASE}" -create_lock "$(basename "$0")" +create_lock # Protect config file with lock to avoid race conditions grep -qi "^$(escape "${USER}")|" "${DATABASE}" 2>/dev/null || errex "User \"${USER}\" does not exist" sed -i "s ^""${USER}""|.* ""${USER}""|""${HASH}"" " "${DATABASE}" diff --git a/target/scripts/check-for-changes.sh b/target/scripts/check-for-changes.sh index 7223d729..193db744 100755 --- a/target/scripts/check-for-changes.sh +++ b/target/scripts/check-for-changes.sh @@ -6,8 +6,6 @@ LOG_DATE=$(date +"%Y-%m-%d %H:%M:%S ") _notify 'task' "${LOG_DATE} Start check-for-changes script." -SCRIPT_NAME="$(basename "$0")" - # ? ––––––––––––––––––––––––––––––––––––––––––––– Checks cd /tmp/docker-mailserver || exit 1 @@ -40,9 +38,6 @@ while true do LOG_DATE=$(date +"%Y-%m-%d %H:%M:%S ") - # Lock configuration while working - create_lock "${SCRIPT_NAME}" - # get chksum and check it, no need to lock config yet _monitored_files_checksums >"${CHKSUM_FILE}.new" cmp --silent -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new" @@ -53,6 +48,7 @@ do if [ $? -eq 1 ] then _notify 'inf' "${LOG_DATE} Change detected" + create_lock # Shared config safety lock CHANGED=$(grep -Fxvf "${CHKSUM_FILE}" "${CHKSUM_FILE}.new" | sed 's/^[^ ]\+ //') # Bug alert! This overwrites the alias set by start-mailserver.sh @@ -227,11 +223,12 @@ s/$/ regexp:\/etc\/postfix\/regexp/ # prevent restart of dovecot when smtp_only=1 [[ ${SMTP_ONLY} -ne 1 ]] && supervisorctl restart dovecot + + remove_lock fi # mark changes as applied mv "${CHKSUM_FILE}.new" "${CHKSUM_FILE}" - remove_lock "${SCRIPT_NAME}" sleep 1 done diff --git a/target/scripts/helper-functions.sh b/target/scripts/helper-functions.sh index 5967e82a..9f9df0c1 100755 --- a/target/scripts/helper-functions.sh +++ b/target/scripts/helper-functions.sh @@ -1,6 +1,8 @@ #! /bin/bash DMS_DEBUG="${DMS_DEBUG:=0}" +SCRIPT_NAME="$(basename "$0")" # This becomes the sourcing script name (Example: check-for-changes.sh) +LOCK_ID="$(uuid)" # Used inside of lock files to identify them and prevent removal by other instances of docker-mailserver # ? --------------------------------------------- BIN HELPER @@ -17,17 +19,33 @@ function escape function create_lock { - SCRIPT_NAME="$1" - LOCK_FILE="/tmp/docker-mailserver/${SCRIPT_NAME}.lock" - [[ -e "${LOCK_FILE}" ]] && errex "Lock file ${LOCK_FILE} exists. Another $1 execution is happening. Try again later." - trap remove_lock EXIT # This won't work if the script is, for example, check-for-changes.sh which uses a while loop to stay running; you'll need to include a remove_lock call at the end of your logic - touch "${LOCK_FILE}" + LOCK_FILE="/tmp/docker-mailserver/${SCRIPT_NAME}.lock" + while [[ -e "${LOCK_FILE}" ]] + do + _notify 'warn' "Lock file ${LOCK_FILE} exists. Another ${SCRIPT_NAME} execution is happening. Trying again shortly..." + # Handle stale lock files left behind on crashes + # or premature/non-graceful exits of containers while they're making changes + if [[ -n "$(find "${LOCK_FILE}" -mmin +1 2>/dev/null)" ]] + then + _notify 'warn' "Lock file older than 1 minute. Removing stale lock file." + rm -f "${LOCK_FILE}" + _notify 'inf' "Removed stale lock ${LOCK_FILE}." + fi + sleep 5 + done + trap remove_lock EXIT + echo "${LOCK_ID}" > "${LOCK_FILE}" } function remove_lock { - SCRIPT_NAME=${SCRIPT_NAME:-$1} - rm -f "/tmp/docker-mailserver/${SCRIPT_NAME}.lock" + LOCK_FILE="${LOCK_FILE:-"/tmp/docker-mailserver/${SCRIPT_NAME}.lock"}" + [[ -z "${LOCK_ID}" ]] && errex "Cannot remove ${LOCK_FILE} as there is no LOCK_ID set" + if [[ -e "${LOCK_FILE}" && $(grep -c "${LOCK_ID}" "${LOCK_FILE}") -gt 0 ]] # Ensure we don't delete a lock that's not ours + then + rm -f "${LOCK_FILE}" + _notify 'inf' "Removed lock ${LOCK_FILE}." + fi } # ? ––––––––––––––––––––––––––––––––––––––––––––– IP & CIDR diff --git a/test/helper-functions.bats b/test/helper-functions.bats new file mode 100644 index 00000000..67a1e4cd --- /dev/null +++ b/test/helper-functions.bats @@ -0,0 +1,43 @@ +load 'test_helper/common' + +function setup() { + run_setup_file_if_necessary +} + +function teardown() { + run_teardown_file_if_necessary +} + +function setup_file() { + local PRIVATE_CONFIG + PRIVATE_CONFIG="$(duplicate_config_for_container .)" + docker run -d --name mail_helper_functions \ + --cap-add=NET_ADMIN \ + -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \ + -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \ + -e ENABLE_FETCHMAIL=1 \ + -e DMS_DEBUG=0 \ + -h mail.my-domain.com -t "${NAME}" + wait_for_finished_setup_in_container mail_helper_functions +} + +function teardown_file() { + docker rm -f mail_helper_functions +} + +@test "first" { + skip 'this test must come first to reliably identify when to run setup_file' +} + +@test "check helper-functions.sh: _sanitize_ipv4_to_subnet_cidr" { + run docker exec mail_helper_functions bash -c ". /usr/local/bin/helper-functions.sh; _sanitize_ipv4_to_subnet_cidr 255.255.255.255/0" + assert_output "0.0.0.0/0" + run docker exec mail_helper_functions bash -c ". /usr/local/bin/helper-functions.sh; _sanitize_ipv4_to_subnet_cidr 192.168.255.14/20" + assert_output "192.168.240.0/20" + run docker exec mail_helper_functions bash -c ". /usr/local/bin/helper-functions.sh; _sanitize_ipv4_to_subnet_cidr 192.168.255.14/32" + assert_output "192.168.255.14/32" +} + +@test "last" { + skip 'this test is only there to reliably mark the end for the teardown_file' +} diff --git a/test/helper_functions.bats b/test/helper_functions.bats deleted file mode 100644 index bac6827e..00000000 --- a/test/helper_functions.bats +++ /dev/null @@ -1,15 +0,0 @@ -load 'test_helper/bats-support/load' -load 'test_helper/bats-assert/load' - -# load the helper function into current context -# shellcheck source=../target/scripts/helper-functions.sh -. ./target/scripts/helper-functions.sh - -@test "check helper function: _sanitize_ipv4_to_subnet_cidr" { - output=$(_sanitize_ipv4_to_subnet_cidr 255.255.255.255/0) - assert_output "0.0.0.0/0" - output=$(_sanitize_ipv4_to_subnet_cidr 192.168.255.14/20) - assert_output "192.168.240.0/20" - output=$(_sanitize_ipv4_to_subnet_cidr 192.168.255.14/32) - assert_output "192.168.255.14/32" -} diff --git a/test/mail_changedetector.bats b/test/mail_changedetector.bats new file mode 100644 index 00000000..31c7f45b --- /dev/null +++ b/test/mail_changedetector.bats @@ -0,0 +1,94 @@ +load 'test_helper/common' + +function setup() { + run_setup_file_if_necessary +} + +function teardown() { + run_teardown_file_if_necessary +} + +function setup_file() { + local PRIVATE_CONFIG + PRIVATE_CONFIG="$(duplicate_config_for_container . mail_changedetector_one)" + + docker run -d --name mail_changedetector_one \ + -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \ + -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \ + -e DMS_DEBUG=1 \ + -h mail.my-domain.com -t "${NAME}" + wait_for_finished_setup_in_container mail_changedetector_one + + docker run -d --name mail_changedetector_two \ + -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \ + -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \ + -e DMS_DEBUG=1 \ + -h mail.my-domain.com -t "${NAME}" + wait_for_finished_setup_in_container mail_changedetector_two +} + +function teardown_file() { + docker rm -f mail_changedetector_one + docker rm -f mail_changedetector_two +} + +# this test must come first to reliably identify when to run setup_file +@test "first" { + skip 'Starting testing of changedetector' +} + +@test "checking changedetector: servers are ready" { + wait_for_service mail_changedetector_one changedetector + wait_for_service mail_changedetector_two changedetector +} + +@test "checking changedetector: can detect changes & between two containers using same config" { + echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf" + sleep 15 + run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail changedetector" + assert_output --partial "postfix: stopped" + assert_output --partial "postfix: started" + assert_output --partial "Change detected" + assert_output --partial "Removed lock" + run docker exec mail_changedetector_two /bin/bash -c "supervisorctl tail changedetector" + assert_output --partial "postfix: stopped" + assert_output --partial "postfix: started" + assert_output --partial "Change detected" + assert_output --partial "Removed lock" +} + +@test "checking changedetector: lock file found, blocks, and doesn't get prematurely removed" { + run docker exec mail_changedetector_two /bin/bash -c "supervisorctl stop changedetector" + docker exec mail_changedetector_one /bin/bash -c "touch /tmp/docker-mailserver/check-for-changes.sh.lock" + echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf" + run docker exec mail_changedetector_two /bin/bash -c "supervisorctl start changedetector" + sleep 15 + run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail changedetector" + assert_output --partial "check-for-changes.sh.lock exists" + run docker exec mail_changedetector_two /bin/bash -c "supervisorctl tail changedetector" + assert_output --partial "check-for-changes.sh.lock exists" + # Ensure starting a new check-for-changes.sh instance (restarting here) doesn't delete the lock + docker exec mail_changedetector_two /bin/bash -c "rm -f /var/log/supervisor/changedetector.log" + run docker exec mail_changedetector_two /bin/bash -c "supervisorctl restart changedetector" + sleep 5 + run docker exec mail_changedetector_two /bin/bash -c "supervisorctl tail changedetector" + refute_output --partial "check-for-changes.sh.lock exists" + refute_output --partial "Removed lock" +} + +@test "checking changedetector: lock stale and cleaned up" { + docker rm -f mail_changedetector_two + docker exec mail_changedetector_one /bin/bash -c "touch /tmp/docker-mailserver/check-for-changes.sh.lock" + echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf" + sleep 15 + run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail changedetector" + assert_output --partial "check-for-changes.sh.lock exists" + sleep 65 + run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail changedetector" + assert_output --partial "Removed stale lock" +} + +# this test is only there to reliably mark the end for the teardown_file +@test "last" { + skip 'Finished testing of changedetector' +}