Merge pull request #1201 from erik-wramner/fix_1197_setup_in_tests

Fix 1197 random test failures

Makefile

@@ -29,7 +29,7 @@ run:
 	# Run containers
 	docker run -d --name mail \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-v "`pwd`/test/onedir":/var/mail-state \
 		-e ENABLE_CLAMAV=1 \
 		-e SPOOF_PROTECTION=1 \
@@ -51,7 +51,7 @@ run:
 	sleep 15
 	docker run -d --name mail_privacy \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_CLAMAV=1 \
 		-e ENABLE_SPAMASSASSIN=1 \
 		-e SA_TAG=-5.0 \
@@ -68,7 +68,7 @@ run:
 	sleep 15
 	docker run -d --name mail_pop3 \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-v "`pwd`/test/config/letsencrypt":/etc/letsencrypt/live \
 		-e ENABLE_POP3=1 \
 		-e DMS_DEBUG=0 \
@@ -77,7 +77,7 @@ run:
 	sleep 15
 	docker run -d --name mail_smtponly \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e SMTP_ONLY=1 \
 		-e PERMIT_DOCKER=network \
 		-e DMS_DEBUG=0 \
@@ -93,7 +93,7 @@ run:
 	sleep 15
 	docker run -d --name mail_override_hostname \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e PERMIT_DOCKER=network \
 		-e DMS_DEBUG=0 \
 		-e ENABLE_SRS=1 \
@@ -103,7 +103,7 @@ run:
 	sleep 15
 	docker run -d --name mail_domainname \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e PERMIT_DOCKER=network \
 		-e DMS_DEBUG=0 \
 		-e ENABLE_SRS=1 \
@@ -113,7 +113,7 @@ run:
 	sleep 15
 	docker run -d --name mail_srs_domainname \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e PERMIT_DOCKER=network \
 		-e DMS_DEBUG=0 \
 		-e ENABLE_SRS=1 \
@@ -124,7 +124,7 @@ run:
 	sleep 15
 	docker run -d --name mail_fail2ban \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_FAIL2BAN=1 \
 		-e POSTSCREEN_ACTION=ignore \
 		--cap-add=NET_ADMIN \
@@ -132,7 +132,7 @@ run:
 	sleep 15
 	docker run -d --name mail_fetchmail \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_FETCHMAIL=1 \
 		--cap-add=NET_ADMIN \
 		-e DMS_DEBUG=0 \
@@ -140,7 +140,7 @@ run:
 	sleep 15
 	docker run -d --name mail_disabled_clamav_spamassassin \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_CLAMAV=0 \
 		-e ENABLE_SPAMASSASSIN=0 \
 		-e DMS_DEBUG=0 \
@@ -148,7 +148,7 @@ run:
 	sleep 15
 	docker run -d --name mail_manual_ssl \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e SSL_TYPE=manual \
 		-e SSL_CERT_PATH=/tmp/docker-mailserver/letsencrypt/mail.my-domain.com/fullchain.pem \
 		-e SSL_KEY_PATH=/tmp/docker-mailserver/letsencrypt/mail.my-domain.com/privkey.pem \
@@ -161,7 +161,7 @@ run:
 	sleep 15
 	docker run -d --name mail_with_ldap \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_LDAP=1 \
 		-e LDAP_SERVER_HOST=ldap \
 		-e LDAP_START_TLS=no \
@@ -190,7 +190,7 @@ run:
 	sleep 15
 	docker run -d --name mail_with_imap \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_SASLAUTHD=1 \
 		-e SASLAUTHD_MECHANISMS=rimap \
 		-e SASLAUTHD_MECH_OPTIONS=127.0.0.1 \
@@ -200,7 +200,7 @@ run:
 	sleep 15
 	docker run -d --name mail_postscreen \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e POSTSCREEN_ACTION=enforce \
 		--cap-add=NET_ADMIN \
 		-h mail.my-domain.com -t $(NAME)
@@ -208,7 +208,7 @@ run:
 	docker run -d --name mail_lmtp_ip \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
 		-v "`pwd`/test/config/dovecot-lmtp":/etc/dovecot \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_POSTFIX_VIRTUAL_TRANSPORT=1 \
 		-e POSTFIX_DAGENT=lmtp:127.0.0.1:24 \
 		-e DMS_DEBUG=0 \
@@ -216,7 +216,7 @@ run:
 	sleep 30
 	docker run -d --name mail_with_postgrey \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_POSTGREY=1 \
 		-e POSTGREY_DELAY=15 \
 		-e POSTGREY_MAX_AGE=35 \
@@ -227,14 +227,14 @@ run:
 	sleep 20
 	docker run -d --name mail_undef_spam_subject \
 		-v "`pwd`/test/config":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e ENABLE_SPAMASSASSIN=1 \
 		-e SA_SPAM_SUBJECT="undef" \
 		-h mail.my-domain.com -t $(NAME)
 	sleep 15
 	docker run -d --name mail_with_relays \
 		-v "`pwd`/test/config/relay-hosts":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e RELAY_HOST=default.relay.com \
 		-e RELAY_PORT=2525 \
 		-e RELAY_USER=smtp_user \
@@ -246,7 +246,7 @@ run:
 	sleep 15
 	docker run -d --name mail_with_default_relay \
 		-v "`pwd`/test/config/relay-hosts":/tmp/docker-mailserver \
-		-v "`pwd`/test":/tmp/docker-mailserver-test \
+		-v "`pwd`/test/test-files":/tmp/docker-mailserver-test:ro \
 		-e DEFAULT_RELAY_HOST=default.relay.host.invalid:25 \
 		--cap-add=SYS_PTRACE \
 		-e PERMIT_DOCKER=host \

setup.sh

@@ -12,6 +12,7 @@ INFO=$(docker ps \
 IMAGE_NAME=$(echo $INFO | awk '{print $1}')
 CONTAINER_NAME=$(echo $INFO | awk '{print $2}')
 DEFAULT_CONFIG_PATH="$(pwd)/config"
+USE_CONTAINER=false
 
 _update_config_path() {
   VOLUME=$(docker inspect $CONTAINER_NAME \
@@ -98,14 +99,21 @@ _docker_image_exists() {
 }
 
 _docker_image() {
+  if [ "$USE_CONTAINER" = true ]; then
+    # Reuse existing container specified on command line
+    docker exec -ti "$CONTAINER_NAME" "$@"
+  else
+    # Start temporary container with specified image
     if ! _docker_image_exists "$IMAGE_NAME"; then
       echo "Image '$IMAGE_NAME' not found. Pulling ..."
       docker pull "$IMAGE_NAME"
     fi
+
     docker run \
       --rm \
       -v "$CONFIG_PATH":/tmp/docker-mailserver \
       -ti "$IMAGE_NAME" $@
+  fi
 }
 
 _docker_container() {
@@ -121,6 +129,7 @@ while getopts ":c:i:p:" OPT; do
   case $OPT in
     c)
       CONTAINER_NAME="$OPTARG"
+      USE_CONTAINER=true # Container specified, connect to running instance
       ;;
     i)
       IMAGE_NAME="$OPTARG"

target/bin/addmailuser

@@ -22,13 +22,20 @@ escape() {
 [ -z "$USER" ] && { usage; errex "no username specified"; }
 expr index "$USER" "@" >/dev/null || { usage; errex "username must include the domain"; }
 
-grep -qi "^$(escape "$USER")|" $DATABASE 2>/dev/null &&
+# Protect config file with lock to avoid race conditions
+touch $DATABASE
+( 
+  flock -e 200
+
+  grep -qi "^$(escape "$USER")|" $DATABASE 2>/dev/null &&
     errex "User \"$USER\" already exists"
 
-if [ -z "$PASSWD" ]; then
+  if [ -z "$PASSWD" ]; then
     read -s -p "Enter Password: " PASSWD
     echo
     [ -z "$PASSWD" ] && errex "Password must not be empty"
-fi
+  fi
-HASH="$(doveadm pw -s SHA512-CRYPT -u "$USER" -p "$PASSWD")"
+
-echo "$USER|$HASH" >> $DATABASE
+  HASH="$(doveadm pw -s SHA512-CRYPT -u "$USER" -p "$PASSWD")"
+  echo "$USER|$HASH" >> $DATABASE
+) 200<$DATABASE

target/bin/delmailuser

@@ -33,8 +33,12 @@ shift $((OPTIND-1))
 [ -z "$@" ] && { usage; errex "No user specifed"; }
 [ -s "$DATABASE" ] || exit 0
 
-for USER in "$@"; do
+# Protect config file with lock to avoid race conditions
-	#very simple plausibility check
+(
+  flock -e 200
+
+  for USER in "$@"; do
+    # very simple plausibility check
     [[ "$USER" != *"@"*"."* ]] && errex "No valid address: $USER"
     MAILARR=(${USER//@/ })
     # XXX $USER must not contain /s and other syntactic characters
@@ -54,4 +58,6 @@ for USER in "$@"; do
     [ "$MAILDEL" != "y" ] && errex "Leaving the maildir untouched. If you want to delete it at a later point use \"sudo docker exec mail rm -R /var/mail/${MAILARR[1]}/${MAILARR[0]}\""
     rm -r -f /var/mail/${MAILARR[1]}/${MAILARR[0]}
     [ $? = 0 ] && echo "Maildir deleted." || errex "Maildir couldn't be deleted: $?"
-done
+  done
+
+) 200<$DATABASE

target/bin/listmailuser

@@ -10,4 +10,9 @@ errex() {
 [ -f $DATABASE ] || errex "No postfix-accounts.cf file"
 [ -s $DATABASE ] || errex "Empty postfix-accounts.cf - no users have been added"
 
-awk -F '|' '{ print $1; }' $DATABASE
+# Lock database even though we are only reading
+(
+  flock -e 200
+  awk -F '|' '{ print $1; }' $DATABASE
+) 200<$DATABASE
+

target/bin/updatemailuser

@@ -27,7 +27,13 @@ if [ -z "$PASSWD" ]; then
 	[ -z "$PASSWD" ] && errex "Password must not be empty"
 fi
 HASH="$(doveadm pw -s SHA512-CRYPT -u "$USER" -p "$PASSWD")"
-grep -qi "^$(escape "$USER")|" $DATABASE 2>/dev/null ||
+
+# Protect config file with lock to avoid race conditions
+(
+  flock -e 200
+
+  grep -qi "^$(escape "$USER")|" $DATABASE 2>/dev/null ||
     errex "User \"$USER\" does not exist"
 
-sed -i "s ^"$USER"|.* "$USER"|"$HASH" " $DATABASE
+  sed -i "s ^"$USER"|.* "$USER"|"$HASH" " $DATABASE
+) 200<$DATABASE

target/check-for-changes.sh

@@ -1,9 +1,7 @@
-#! /bin/bash
+#!/bin/bash
 
 # create date for log output
 log_date=$(date +"%Y-%m-%d %H:%M:%S ")
-# Prevent a start too early
-sleep 5
 echo "${log_date} Start check-for-changes script."
 
 # change directory
@@ -15,6 +13,13 @@ if [ ! -f postfix-accounts.cf ]; then
    exit
 fi
 
+# Verify checksum file exists; must be prepared by start-mailserver.sh
+CHKSUM_FILE=/tmp/docker-mailserver-config-chksum
+if [ ! -f $CHKSUM_FILE ]; then
+   echo "${log_date} ${CHKSUM_FILE} is missing! Start script failed? Exit!"
+   exit
+fi
+
 # Determine postmaster address, duplicated from start-mailserver.sh
 # This script previously didn't work when POSTMASTER_ADDRESS was empty
 if [[ -n "${OVERRIDE_HOSTNAME}" ]]; then
@@ -25,15 +30,14 @@ fi
 PM_ADDRESS="${POSTMASTER_ADDRESS:=postmaster@${DOMAINNAME}}"
 echo "${log_date} Using postmaster address ${PM_ADDRESS}"
 
-# create an array of files to monitor (perhaps simple *.cf would be ok here)
+# Create an array of files to monitor, must be the same as in start-mailserver.sh
 declare -a cf_files=()
 for file in postfix-accounts.cf postfix-virtual.cf postfix-aliases.cf; do
   [ -f "$file" ] && cf_files+=("$file")
 done
 
-# Update / generate after start
+# Wait to make sure server is up before we start
-echo "${log_date} Makeing new checksum file."
+sleep 10
-sha512sum ${cf_files[@]/#/--tag } > chksum
 
 # Run forever
 while true; do
@@ -41,14 +45,21 @@ while true; do
 # recreate logdate
 log_date=$(date +"%Y-%m-%d %H:%M:%S ")
 
-# Get chksum and check it.
+# Get chksum and check it, no need to lock config yet
-chksum=$(sha512sum -c --ignore-missing chksum)
+chksum=$(sha512sum -c --ignore-missing $CHKSUM_FILE)
 
 if [[ $chksum == *"FAIL"* ]]; then
 	echo "${log_date} Change detected"
 
 	# Bug alert! This overwrites the alias set by start-mailserver.sh
 	# Take care that changes in one script are propagated to the other
+        # Also note that changes are performed in place and are not atomic
+        # We should fix that and write to temporary files, stop, swap and start
+
+        # Lock configuration while working
+        # Not fixing indentation yet to reduce diff (fix later in separate commit)
+        (
+          flock -e 200
 
 	#regen postix aliases.
 	echo "root: ${PM_ADDRESS}" > /etc/aliases
@@ -187,7 +198,9 @@ if [[ $chksum == *"FAIL"* ]]; then
 	fi 
 
 	echo "${log_date} Update checksum"
-	sha512sum ${cf_files[@]/#/--tag } > chksum
+	sha512sum ${cf_files[@]/#/--tag } >$CHKSUM_FILE
+
+        ) 200<postfix-accounts.cf # end lock
 fi
 
 sleep 1

target/start-mailserver.sh

@@ -49,6 +49,7 @@ DEFAULT_VARS["VIRUSMAILS_DELETE_DELAY"]="${VIRUSMAILS_DELETE_DELAY:="7"}"
 ##########################################################################
 HOSTNAME="$(hostname -f)"
 DOMAINNAME="$(hostname -d)"
+CHKSUM_FILE=/tmp/docker-mailserver-config-chksum
 ##########################################################################
 # << GLOBAL VARS
 ##########################################################################
@@ -165,6 +166,9 @@ function register_functions() {
   	_register_setup_function "_setup_mail_summary"
   fi
 
+        # Compute last as the config files are modified in-place
+        _register_setup_function "_setup_chksum_file"
+
 	################### << setup funcs
 
 	################### >> fix funcs
@@ -439,6 +443,30 @@ function _setup_default_vars() {
 	done
 }
 
+function _setup_chksum_file() {
+        notify 'task' "Setting up configuration checksum file"
+
+
+        if [ -d /tmp/docker-mailserver ]; then
+          pushd /tmp/docker-mailserver
+
+          declare -a cf_files=()
+          for file in postfix-accounts.cf postfix-virtual.cf postfix-aliases.cf; do
+            [ -f "$file" ] && cf_files+=("$file")
+          done
+
+          notify 'inf' "Creating $CHKSUM_FILE"
+          sha512sum ${cf_files[@]/#/--tag } >$CHKSUM_FILE
+
+          popd
+        else
+          # We could just skip the file, but perhaps config can be added later?
+          # If so it must be processed by the check for changes script
+          notify 'inf' "Creating empty $CHKSUM_FILE (no config)"
+          touch $CHKSUM_FILE
+        fi
+}
+
 function _setup_mailname() {
 	notify 'task' 'Setting up Mailname'
 

test/config/chksum

@@ -1,2 +0,0 @@
-SHA512 (postfix-accounts.cf) = 8eba4eedb76e8c3b740ca4eea5d7ad1c8695fd7fd39d3e5d161201101b5b63a1c1471ab85bd79d6ed4ba192c423275a3d1210a9596a06904434ab5707ad3753b
-SHA512 (postfix-virtual.cf) = c5bff9b5c50886a72a559ee52b1230b751c3820cd12de3ffe2a632f8d2a7845fa33ddac397c62889f0680d096be24418f22c1b81fb93ad56fd4937869f16954b

test/tests.bats

@@ -1,5 +1,28 @@
 load 'test_helper/bats-support/load'
 load 'test_helper/bats-assert/load'
+
+
+#
+# shared functions
+#
+
+function wait_for_service() {
+  containerName=$1
+  serviceName=$2
+  count=0
+  while ! (docker exec $containerName /usr/bin/supervisorctl status $serviceName | grep RUNNING >/dev/null)
+  do
+    ((count++)) && ((count==30)) && break
+    sleep 5
+  done
+  return $(docker exec $containerName /usr/bin/supervisorctl status $serviceName | grep RUNNING >/dev/null)
+}
+
+function count_processed_changes() {
+  containerName=$1
+  docker exec $containerName cat /var/log/supervisor/changedetector.log | grep "Update checksum" | wc -l
+}
+
 #
 # configuration checks
 #
@@ -880,7 +903,11 @@ load 'test_helper/bats-assert/load'
   MAIL_FAIL2BAN_IP=$(docker inspect --format '{{ .NetworkSettings.IPAddress }}' mail_fail2ban)
 
   # Create a container which will send wrong authentications and should get banned
-  docker run --name fail-auth-mailer -e MAIL_FAIL2BAN_IP=$MAIL_FAIL2BAN_IP -v "$(pwd)/test":/tmp/docker-mailserver-test -d $(docker inspect --format '{{ .Config.Image }}' mail) tail -f /var/log/faillog
+  docker run --name fail-auth-mailer \
+    -e MAIL_FAIL2BAN_IP=$MAIL_FAIL2BAN_IP \
+    -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test \
+    -d $(docker inspect --format '{{ .Config.Image }}' mail) \
+    tail -f /var/log/faillog
 
   # can't pipe the file as usual due to postscreen. (respecting postscreen_greet_wait time and talking in turn):
   for i in {1,2}; do
@@ -1234,14 +1261,32 @@ load 'test_helper/bats-assert/load'
 }
 
 # email
-@test "checking setup.sh: setup.sh email add" {
+@test "checking setup.sh: setup.sh email add and login" {
+  wait_for_service mail changedetector
+  assert_success
+
+  originalChangesProcessed=$(count_processed_changes mail)
+
   run ./setup.sh -c mail email add setup_email_add@example.com test_password
   assert_success
 
   value=$(cat ./test/config/postfix-accounts.cf | grep setup_email_add@example.com | awk -F '|' '{print $1}')
   [ "$value" = "setup_email_add@example.com" ]
+  assert_success
 
-  # we test the login of this user later to let the container digest the addition
+  # wait until change detector has processed the change
+  count=0
+  while [ "${originalChangesProcessed}" = "$(count_processed_changes mail)" ]
+  do
+    ((count++)) && ((count==60)) && break
+    sleep 1
+  done
+
+  [ "${originalChangesProcessed}" != "$(count_processed_changes mail)" ]
+  assert_success
+
+  result=$(docker exec mail doveadm auth test -x service=smtp setup_email_add@example.com 'test_password' | grep 'auth succeeded')
+  [ "$result" = "passdb: setup_email_add@example.com auth succeeded" ]
 }
 
 @test "checking setup.sh: setup.sh email list" {
@@ -1250,9 +1295,20 @@ load 'test_helper/bats-assert/load'
 }
 
 @test "checking setup.sh: setup.sh email update" {
-  ./setup.sh -c mail email add lorem@impsum.org test_test && initialpass=$(cat ./test/config/postfix-accounts.cf | grep lorem@impsum.org | awk -F '|' '{print $2}')
+  run ./setup.sh -c mail email add lorem@impsum.org test_test
+  assert_success
+
+  initialpass=$(cat ./test/config/postfix-accounts.cf | grep lorem@impsum.org | awk -F '|' '{print $2}')
+  [ "$initialpass" != "" ]
+  assert_success
+
   run ./setup.sh -c mail email update lorem@impsum.org my password
+  assert_success
+
   updatepass=$(cat ./test/config/postfix-accounts.cf | grep lorem@impsum.org | awk -F '|' '{print $2}')
+  [ "$updatepass" != "" ]
+  assert_success
+
   [ "$initialpass" != "$updatepass" ]
   assert_success
 
@@ -1421,12 +1477,6 @@ load 'test_helper/bats-assert/load'
   assert_success
 }
 
-@test "checking setup.sh: email add login validation" {
-  # validates that the user created previously with setup.sh can login
-  result=$(docker exec mail doveadm auth test -x service=smtp setup_email_add@example.com 'test_password' | grep 'auth succeeded')
-  [ "$result" = "passdb: setup_email_add@example.com auth succeeded" ]
-}
-
 #
 # LDAP
 #