Fix: Try fixing the #147 issue and conver it by unit tests

scripts/common-run.sh

@@ -143,9 +143,8 @@ postfix_upgrade_conf() {
 	local entry
 	local filename
 	local OLD_IFS
-	local daemon_directory
-
-	# Check for any references to the old "hash:" and "btree:" databases and replae them with "lmdb:"
+	
+	# Check for any references to the old "hash:" and "btree:" databases and replace them with "lmdb:"
 	if cat "$maincf" | egrep -v "^#" | egrep -q "(hash|btree):"; then
 		warn "Detected old hash: and btree: references in the config file, which are not supported anymore. Upgrading to lmdb:"
 		sed -i -E 's/(hash|btree):/lmdb:/g' "$maincf"
@@ -169,44 +168,54 @@ postfix_upgrade_conf() {
 	else
 		debug "No upgrade of hashes needed needed."
 	fi
+}
+
+postfix_upgrade_daemon_directory() {
+	# Debian, Ubuntu
+	local dir_debian="/usr/lib/postfix/sbin"
+	# Alpine
+	local dir_alpine="/usr/libexec/postfix"
+
 
 	# Some people will keep the configuration of postfix on an external drive, although this is not strictly neccessary by this
 	# image. And when they switch between different distrubtions (Alpine -> Debian and vice versa), the image will fail with the
 	# old configuration. This is a quick and dirty check to solve this issue so we don't get issues like these:
 	# https://github.com/bokysan/docker-postfix/issues/147
-	if grep -q -E "^\s*daemon_directory\s*=" "$maincf"; then
-		# Get the directory
-		daemon_directory="$(grep -q -E "^\s*daemon_directory\s*=" "$maincf")"
-		daemon_directory="${daemon_directory#*=}"
-		daemon_directory="${daemon_directory#"${daemon_directory%%[![:space:]]*}"}" # remove leading whitespace characters
-		daemon_directory="${daemon_directory%"${daemon_directory##*[![:space:]]}"}" # remove trailing whitespace characters
+	local daemon_directory="$(get_postconf "daemon_directory")"
 
-		# Debian, Ubuntu
-		# daemon_directory = /usr/lib/postfix/sbin
-		# Alpine
-		# daemon_directory = /usr/libexec/postfix
-
-		if [[ daemon_directory == "/usr/lib/postfix/sbin" ]] && [[ ! -d "/usr/lib/postfix/sbin" ]] && [[ -d "/usr/libexec/postfix" ]]; then
-			warn "You're switching from Debian/Ubuntu distribution to Alpine. Changing ${emphasis}daemon_directory = /usr/libexec/postfix${reset}, otherwise this image will not run."
+	if [[ daemon_directory == "${dir_debian}" ]] && [[ ! -d "${dir_debian}" ]]; then
+		if [[ -d "${dir_alpine}" ]]; then
+			warn "You're switching from Debian/Ubuntu distribution to Alpine. Changing ${emphasis}daemon_directory = ${dir_alpine}${reset}, otherwise this image will not run."
 			notice "To avoid these warnings in the future, it is suggested ${emphasis}NOT${reset} to link ${emphasis}/etc/postfix${reset} to a volume and let this image manage it itself."
-			do_postconf -e daemon_directory=/usr/libexec/postfix
-			daemon_directory=/usr/libexec/postfix
-		elif [[ daemon_directory == "/usr/libexec/postfix" ]] && [[ ! -d "/usr/libexec/postfix" ]] && [[ -d "/usr/lib/postfix/sbin" ]]; then
-			warn "You're switching from Alpine to Debian/Ubuntu distribution. Changing ${emphasis}daemon_directory = /usr/lib/postfix/sbin${reset}, otherwise this image will not run."
+			do_postconf -e "daemon_directory=${dir_alpine}"
+			daemon_directory="${dir_alpine}"
+		else
+			warn "Running with default Debian/Ubuntu deamon directory ${emphasis}${dir_debian}${reset}, but I can't identify it as a dir."
+		fi
+	elif [[ daemon_directory == "${dir_alpine}" ]] && [[ ! -d "${dir_alpine}" ]]; then
+		if [[ -d "${dir_debian}" ]]; then
+			warn "You're switching from Alpine to Debian/Ubuntu distribution. Changing ${emphasis}daemon_directory = ${dir_debian}${reset}, otherwise this image will not run."
 			notice "To avoid these warnings in the future, it is suggested ${emphasis}NOT${reset} to link ${emphasis}/etc/postfix${reset} to a volume and let this image manage it itself."
-			do_postconf -e daemon_directory=/usr/lib/postfix/sbin
-			daemon_directory=/usr/lib/postfix/sbin
+			do_postconf -e "daemon_directory=${dir_debian}"
+			daemon_directory="${dir_debian}"
+		else
+			warn "Running with default Alpine deamon directory ${emphasis}${dir_debian}${reset}, but I can't identify it as a dir."
 		fi
+	fi
 
-		if [[ ! -d "${daemon_directory}" ]]; then
-			error "Your ${emphasis}daemon_directory${reset} is set to ${emphasis}${daemon_directory}${reset} but it does not exist. Postfix startup will most likely fail."
-		fi
+	if [[ ! -d "${daemon_directory}" ]]; then
+		error "Your ${emphasis}daemon_directory${reset} is set to ${emphasis}${daemon_directory}${reset} but it does not exist. Postfix startup will most likely fail."
+	else
+		# Ensure that daemon_directory is owned by root
+		chown root:root "${daemon_directory}"
 	fi
 }
 
 postfix_disable_utf8() {
 	if [[ -f /etc/alpine-release ]]; then
 		do_postconf -e smtputf8_enable=no
+	else 
+		do_postconf -e smtputf8_enable=yes
 	fi
 }
 
@@ -687,7 +696,7 @@ execute_post_init_scripts() {
 	fi
 }
 
-unset_sensible_variables() {
+unset_sensitive_variables() {
 	unset RELAYHOST_PASSWORD
 	unset XOAUTH2_CLIENT_ID
 	unset XOAUTH2_SECRET

scripts/common.sh

@@ -169,7 +169,32 @@ do_postconf() {
 		shift
 		postconf -e "$@"
 	fi
+}
 
+############################
+# Read a configuration from postfix configuration
+############################
+get_postconf() {
+	local name="${1}"
+	local result
+	local error
+
+	# This will throw a warning if the config option does not exist, e.g.
+	# postconf: warning: foo_bar: unknown parameter
+
+	# This is a bash magic to capture both out and error in the same line.
+	# We're just basically calling "postconf <name>"
+	. <({ error=$({ result="$(postconf "${name}")"; } 2>&1; declare -p result >&2); declare -p error; } 2>&1)
+
+	if [[ -n "${error}" ]]; then
+		error: "Could not read variable ${emphasis}${name}${reset}: ${error}"
+		return
+	fi
+
+	result="${result#*=}"
+	result="${result#"${result%%[![:space:]]*}"}" # remove leading whitespace characters
+	result="${result%"${result##*[![:space:]]}"}" # remove trailing whitespace characters
+	printf '%s' "${result}"
 }
 
 # usage: file_env VAR [DEFAULT]

scripts/run.sh

@@ -12,6 +12,7 @@ setup_conf                          # Copy over files from /etc/postfix.template
 reown_folders                       # Make and reown /var/spool/postfix/ folders
 postfix_enable_chroot               # Allow Postfix to run in chroot
 postfix_upgrade_conf                # Upgrade old coniguration, replace "hash:" and "btree:" databases to "lmdb:"
+postfix_upgrade_daemon_directory    # Change the 'daemon_directory' postfix configuration, if a change is detected from Alpine<->Debian/Ubuntu
 postfix_disable_utf8                # Disable SMTPUTF8, because libraries (ICU) are missing in alpine
 postfix_create_aliases              # Update aliases database. It's not used, but postfix complains if the .db file is missing
 postfix_disable_local_mail_delivery # Disable local mail delivery
@@ -35,7 +36,7 @@ postfix_custom_commands             # Apply custom postfix settings
 opendkim_custom_commands            # Apply custom OpenDKIM settings
 postfix_open_submission_port        # Enable the submission port
 execute_post_init_scripts           # Execute any scripts found in /docker-init.db/
-unset_sensible_variables            # Remove environment variables that contains sensible values (secrets) that are read from conf files
+unset_sensitive_variables            # Remove environment variables that contains sensitive values (secrets) that are read from conf files
 
 notice "Starting: ${emphasis}rsyslog${reset}, ${emphasis}postfix${reset}$DKIM_ENABLED"
 exec supervisord -c /etc/supervisord.conf

unit-tests/000_test-multi-comment.bats

@@ -3,6 +3,9 @@
 load /code/scripts/common.sh
 load /code/scripts/common-run.sh
 
+# 
+postconf daemon_directory=/usr/libexec/postfix
+
 if [[ ! -f /etc/postfix/main.test-multi-comment ]]; then
 	cp /etc/postfix/main.cf /etc/postfix/main.test-multi-comment
 fi

unit-tests/001_verify_read_write_properties.bats

@@ -0,0 +1,38 @@
+#!/usr/bin/env bats
+
+load /code/scripts/common.sh
+load /code/scripts/common-run.sh
+
+
+@test "verify reading and writting propreties" {
+    local value
+    local old_value
+    old_value="$(get_postconf "mydestination")"
+
+    do_postconf -# mydestination
+
+    value="$(get_postconf "mydestination")"
+    if [[ -n "${value}" ]]; then
+        echo "Expected '', got: '$value' for 'mydestination'" >&2
+        exit 1
+    fi
+
+    do_postconf -e 'mydestination=$myhostname, localhost.$mydomain $mydomain'
+
+    value="$(get_postconf "mydestination")"
+    if [[ "${value}" != 'mydestination=$myhostname, localhost.$mydomain $mydomain' ]]; then
+        echo "Expected 'mydestination=\$myhostname, localhost.\$mydomain \$mydomain', got: '$value' for mydestination" >&2
+        exit 1
+    fi
+
+    do_postconf -# mydestination
+    echo "           mydestination    =       localhost" >> /etc/postfix/main.cf
+
+    value="$(get_postconf "mydestination")"
+    if [[ "${value}" != "localhost" ]]; then
+        echo "Expected 'localhost', got: '$value' for mydestination" >&2
+        exit 1
+    fi
+
+    do_postconf -e "mydestination=${old_value}"
+}

unit-tests/002_postfix_upgrade_daemon_directory.bats

@@ -0,0 +1,76 @@
+#!/usr/bin/env bats
+
+load /code/scripts/common.sh
+load /code/scripts/common-run.sh
+
+
+@test "verify reading of daemon_directory" {
+    local daemon_directory
+    local old_daemon_directory
+    local dir_debian="/usr/lib/postfix/sbin"
+    local dir_alpine="/usr/libexec/postfix"
+    local dir_temp="/tmp/deamon_directory"
+    old_daemon_directory="$(get_postconf "daemon_directory")"
+
+    rm -rf "${dir_debian}.bak" "${dir_alpine}.bak"
+    if [[ -d "${dir_debian}" ]]; then
+        cp -r ${dir_debian} ${dir_temp}
+        mv ${dir_debian} ${dir_debian}.bak
+    fi
+
+    if [[ -d "${dir_alpine}" ]]; then
+        cp -r ${dir_alpine} ${dir_temp}
+        mv ${dir_alpine} ${dir_alpine}.bak
+    fi
+
+    # Test if Debian/Ubuntu directory remains the same when run on Debian/Ubuntu
+    cp -r ${dir_temp} ${dir_debian}
+    do_postconf -e "daemon_directory=${dir_debian}"
+    postfix_upgrade_daemon_directory
+    postfix check
+    rm -rf ${dir_debian} ${dir_alpine}
+    [ "$(get_postconf "daemon_directory")" == "${dir_debian}" ]
+
+    # Test if Debian/Ubuntu directory gets changes the same when run on Alpine
+    cp -r ${dir_temp} ${dir_alpine}
+    do_postconf -e "daemon_directory=${dir_debian}"
+    postfix_upgrade_daemon_directory
+    postfix check
+    rm -rf ${dir_debian} ${dir_alpine}
+    [ "$(get_postconf "daemon_directory")" == "${dir_alpine}" ]
+
+    # Test if Alpine directory remains the same when run on Alpine
+    cp -r ${dir_temp} ${dir_alpine}
+    do_postconf -e "daemon_directory=${dir_alpine}"
+    postfix_upgrade_daemon_directory
+    postfix check
+    rm -rf ${dir_debian} ${dir_alpine}
+    [ "$(get_postconf "daemon_directory")" == "${dir_alpine}" ]
+
+    # Test if Alpine directory gets changes the same when run on Debian/Ubuntu
+    cp -r ${dir_temp} ${dir_debian}
+    do_postconf -e "daemon_directory=${dir_alpine}"
+    postfix_upgrade_daemon_directory
+    postfix check
+    rm -rf ${dir_debian} ${dir_alpine}
+    [ "$(get_postconf "daemon_directory")" == "${dir_debian}" ]
+
+    # Test if things work with custom directory
+    do_postconf -e "daemon_directory=${dir_temp}"
+    postfix_upgrade_daemon_directory
+    postfix check
+    rm -rf ${dir_debian} ${dir_alpine}
+    [ "$(get_postconf "daemon_directory")" == "${dir_temp}" ]
+
+    rm -rf ${dir_temp}
+
+    if [[ -d "${dir_debian}.bak" ]]; then
+        mv ${dir_debian}.bak ${dir_debian}
+    fi
+
+    if [[ -d "${dir_alpine}.bak" ]]; then
+        mv ${dir_alpine}.bak ${dir_alpine}
+    fi
+
+    do_postconf -e "daemon_directory=${old_daemon_directory}"
+}

unit-tests/007_xoauth2_support.bats

@@ -10,7 +10,7 @@ load /code/scripts/common-run.sh
 	local XOAUTH2_INITIAL_ACCESS_TOKEN="access_token"
 	local XOAUTH2_INITIAL_REFRESH_TOKEN="refres_token"
 
-	unset_sensible_variables
+	unset_sensitive_variables
 
 	[ -z "$RELAYHOST_PASSWORD" ]
 	[ -z "$XOAUTH2_CLIENT_ID" ]

unit-tests/Dockerfile

@@ -2,15 +2,18 @@ ARG ALPINE_VERSION=latest
 FROM alpine:${ALPINE_VERSION} as build
 
 ARG SASL_XOAUTH2_REPO_URL=https://github.com/tarickb/sasl-xoauth2.git
-ARG SASL_XOAUTH2_GIT_REF=release-0.9
+ARG SASL_XOAUTH2_GIT_REF=release-0.24
+COPY *.patch /build-scripts/
 
 RUN        true && \
            apk add --no-cache --upgrade git && \
-           apk add --no-cache --upgrade cmake clang make gcc g++ libc-dev pkgconfig curl-dev jsoncpp-dev cyrus-sasl-dev && \
-           git clone --depth 1 --branch ${SASL_XOAUTH2_GIT_REF} ${SASL_XOAUTH2_REPO_URL} /sasl-xoauth2  && \
-           cd /sasl-xoauth2 && \
-           mkdir build && \
+           apk add --no-cache --upgrade cmake clang make gcc g++ libc-dev pkgconfig curl-dev jsoncpp-dev cyrus-sasl-dev patch
+RUN        git clone --depth 1 --branch ${SASL_XOAUTH2_GIT_REF} ${SASL_XOAUTH2_REPO_URL} /sasl-xoauth2
+WORKDIR    /sasl-xoauth2
+RUN        mkdir build && \
            cd build && \
+           patch -p1 -d .. < /build-scripts/sasl-xoauth2-01.patch && \
+           patch -p1 -d .. < /build-scripts/sasl-xoauth2-02.patch && \
            cmake -DCMAKE_INSTALL_PREFIX=/ .. && \
            make
 

unit-tests/docker-compose.yml

@@ -1,4 +1,4 @@
-version: '3.7'
+version: '3.8'
 services:
   tests:
     image: "boky/postfix-unit-test"

unit-tests/sasl-xoauth2-01.patch

@@ -0,0 +1,12 @@
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index da4dd69..3739e18 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -31,7 +31,6 @@ option(EnableTests "Enable tests." ON)
+ if(EnableTests)
+   enable_testing()
+ endif()
+-add_subdirectory(docs)
+ add_subdirectory(scripts)
+ add_subdirectory(src)
+ 

unit-tests/sasl-xoauth2-02.patch

@@ -0,0 +1,10 @@
+--- a/src/test_config.cc	2022-09-06 21:21:10.600553457 +0200
++++ b/src/test_config.cc	2022-09-06 21:21:42.736614599 +0200
+@@ -1,6 +1,7 @@
+ #include <getopt.h>
+ #include <sasl/sasl.h>
+ #include <string.h>
++#include <libgen.h>
+ 
+ #include "config.h"
+ #include "log.h"