refactor: Share a common helper (vhost builder) for sourcing domains (#2620)

* chore: Split vhost helper method and use filepath vars

- Helpers `accounts.sh` and `aliases.sh` can move their vhost code into this helper.
- They share duplicate code with `bin/open-dkim` which will also leverage this vhost helper going forward.

* chore: Sync vhost generation logic into helper

- Chunky commit, but mostly copy/paste of logic into a common method.
- `bin/open-dkim` additionally wrapped relevant logic in a function call and revised inline docs.

* chore: Include LDAP vhost support

- Revises notes for LDAP vhost support.
- This now ensures LDAP users get vhost rebuilt to match the startup script for when change detection support is enabled.
- `bin/open-dkim` will additionally be able to support the default `DOMAINNAME` var (set via `helpers/dns.sh`) for LDAP users instead of requiring them to provide one explicitly.

* chore(`bin/open-dkim`): Ensure `DOMAINNAME` is properly set

- This will ensure LDAP users insert the same `DOMAINNAME` value as used during container startup.
- The container itself should panic at startup (during `helpers/dns.sh`) if this isn't configured correctly already, thus it should not introduce any breaking change to users of this utility?

* chore: Set the 2nd value as blank `_`

Line is split by a delimiter such as white-space (or via IFS: `|`), the blank `_` var is to indicate we're not interested in that value, but still leverage how `read -r` works, instead of splitting the var ourselves first thing.

* chore: Remove shellcheck disable lines

No longer applicable with the switch to `_`

target/bin/open-dkim

@@ -98,43 +98,28 @@ do
   esac
 done
 
-DATABASE_ACCOUNTS='/tmp/docker-mailserver/postfix-accounts.cf'
+DATABASE_VHOST='/tmp/vhost.dkim'
-DATABASE_VIRTUAL='/tmp/docker-mailserver/postfix-virtual.cf'
+# Prepare a file with one domain per line:
-DATABASE_VHOST='/tmp/vhost'
+function _generate_domains_config
-TMP_VHOST='/tmp/vhost.dkim.tmp'
+{
-touch "${TMP_VHOST}"
+  local TMP_VHOST='/tmp/vhost.dkim.tmp'
+
+  # Generate the default vhost (equivalent to /etc/postfix/vhost),
+  # unless CLI arg DOMAINS provided an alternative list to use instead:
   if [[ -z ${DOMAINS} ]]
   then
-  # getting domains FROM mail accounts
+    _obtain_hostname_and_domainname
-  if [[ -f ${DATABASE_ACCOUNTS} ]]
+    # uses TMP_VHOST:
-  then
+    _vhost_collect_postfix_domains
-    # shellcheck disable=SC2034
-    while IFS=$'|' read -r LOGIN PASS
-    do
-      DOMAIN=$(echo "${LOGIN}" | cut -d @ -f2)
-      echo "${DOMAIN}" >>"${TMP_VHOST}"
-    done < <(_get_valid_lines_from_file "${DATABASE_ACCOUNTS}")
-  fi
-
-  # getting domains FROM mail aliases
-  if [[ -f ${DATABASE_VIRTUAL} ]]
-  then
-    # shellcheck disable=SC2034
-    while read -r FROM TO
-    do
-      UNAME=$(echo "${FROM}" | cut -d @ -f1)
-      DOMAIN=$(echo "${FROM}" | cut -d @ -f2)
-
-      [[ ${UNAME} != "${DOMAIN}" ]] && echo "${DOMAIN}" >>"${TMP_VHOST}"
-    done < <(_get_valid_lines_from_file "${DATABASE_VIRTUAL}")
-  fi
   else
     tr ',' '\n' <<< "${DOMAINS}" >"${TMP_VHOST}"
   fi
 
-sort < "${TMP_VHOST}" | uniq >"${DATABASE_VHOST}"
+  # uses DATABASE_VHOST + TMP_VHOST:
-rm "${TMP_VHOST}"
+  _create_vhost
+}
 
+_generate_domains_config
 if [[ ! -s ${DATABASE_VHOST} ]]
 then
   _log 'warn' 'No entries found, no keys to make'

target/scripts/helpers/accounts.sh

@@ -91,8 +91,6 @@ function _create_accounts
       then
         cp "/tmp/docker-mailserver/${LOGIN}.dovecot.sieve" "/var/mail/${DOMAIN}/${USER}/.dovecot.sieve"
       fi
-
-      echo "${DOMAIN}" >>/tmp/vhost.tmp
     done < <(_get_valid_lines_from_file "${DATABASE_ACCOUNTS}")
 
     _create_dovecot_alias_dummy_accounts

target/scripts/helpers/aliases.sh

@@ -22,17 +22,6 @@ function _handle_postfix_virtual_config
     fi
 
     cp -f "${DATABASE_VIRTUAL}" /etc/postfix/virtual
-
-    # the `to` is important, don't delete it
-    # shellcheck disable=SC2034
-    while read -r FROM TO
-    do
-      UNAME=$(echo "${FROM}" | cut -d @ -f1)
-      DOMAIN=$(echo "${FROM}" | cut -d @ -f2)
-
-      # if they are equal it means the line looks like: "user1     other@domain.tld"
-      [[ ${UNAME} != "${DOMAIN}" ]] && echo "${DOMAIN}" >>/tmp/vhost.tmp
-    done < <(_get_valid_lines_from_file "${DATABASE_VIRTUAL}")
   else
     _log 'debug' "'${DATABASE_VIRTUAL}' not provided - no mail alias/forward created"
   fi

target/scripts/helpers/postfix.sh

@@ -16,23 +16,73 @@
 # - `postmap` only seems relevant when the lookup type is one of these `file_type` values: http://www.postfix.org/postmap.1.html
 #   Should not be a concern for most types used by `docker-mailserver`: texthash, ldap, pcre, tcp, unionmap, unix.
 #   The only other type in use by `docker-mailserver` is the hash type for /etc/aliases, which `postalias` handles.
+
 function _create_postfix_vhost
 {
   # `main.cf` configures `virtual_mailbox_domains = /etc/postfix/vhost`
   # NOTE: Amavis also consumes this file.
-  : >/etc/postfix/vhost
+  local DATABASE_VHOST='/etc/postfix/vhost'
+  local TMP_VHOST='/tmp/vhost.postfix.tmp'
 
-  # Account and Alias generation will store values in `/tmp/vhost.tmp`.
+  _vhost_collect_postfix_domains
-  # Filter unique values to the proper config.
+  _create_vhost
-  # NOTE: LDAP stores the domain value set by `docker-mailserver`,
+}
-  # and correctly removes it from `mydestination` in `main.cf` in `setup-stack.sh`.
+
-  if [[ -f /tmp/vhost.tmp ]]
+# Filter unique values into a proper DATABASE_VHOST config:
+function _create_vhost
+{
+  : >"${DATABASE_VHOST}"
+
+  if [[ -f ${TMP_VHOST} ]]
   then
-    sort < /tmp/vhost.tmp | uniq >> /etc/postfix/vhost
+    sort < "${TMP_VHOST}" | uniq >>"${DATABASE_VHOST}"
-    rm /tmp/vhost.tmp
+    rm "${TMP_VHOST}"
   fi
 }
 
+# Collects domains from configs (DATABASE_) into TMP_VHOST
+function _vhost_collect_postfix_domains
+{
+  local DATABASE_ACCOUNTS='/tmp/docker-mailserver/postfix-accounts.cf'
+  local DATABASE_VIRTUAL='/tmp/docker-mailserver/postfix-virtual.cf'
+  local DOMAIN UNAME
+
+  # getting domains FROM mail accounts
+  if [[ -f ${DATABASE_ACCOUNTS} ]]
+  then
+    while IFS=$'|' read -r LOGIN _
+    do
+      DOMAIN=$(echo "${LOGIN}" | cut -d @ -f2)
+      echo "${DOMAIN}" >>"${TMP_VHOST}"
+    done < <(_get_valid_lines_from_file "${DATABASE_ACCOUNTS}")
+  fi
+
+  # getting domains FROM mail aliases
+  if [[ -f ${DATABASE_VIRTUAL} ]]
+  then
+    while read -r FROM _
+    do
+      UNAME=$(echo "${FROM}" | cut -d @ -f1)
+      DOMAIN=$(echo "${FROM}" | cut -d @ -f2)
+
+      # if they are equal it means the line looks like: "user1     other@domain.tld"
+      [[ ${UNAME} != "${DOMAIN}" ]] && echo "${DOMAIN}" >>"${TMP_VHOST}"
+    done < <(_get_valid_lines_from_file "${DATABASE_VIRTUAL}")
+  fi
+
+  _vhost_ldap_support
+}
+
+# Add DOMAINNAME (not an ENV, set by `helpers/dns.sh`) to vhost.
+# NOTE: `setup-stack.sh:_setup_ldap` has related logic:
+# - `main.cf:mydestination` setting removes `$mydestination` as an LDAP bugfix.
+# - `main.cf:virtual_mailbox_domains` uses `/etc/postfix/vhost`, but may
+#   conditionally include a 2nd table (ldap:/etc/postfix/ldap-domains.cf).
+function _vhost_ldap_support
+{
+  [[ ${ENABLE_LDAP} -eq 1 ]] && echo "${DOMAINNAME}" >>"${TMP_VHOST}"
+}
+
 # Docs - Postfix lookup table files:
 # http://www.postfix.org/DATABASE_README.html
 #

target/scripts/startup/setup-stack.sh

@@ -360,9 +360,6 @@ function _setup_ldap
 
   configomat.sh "DOVECOT_" "/etc/dovecot/dovecot-ldap.conf.ext"
 
-  # add domainname to vhost
-  echo "${DOMAINNAME}" >>/tmp/vhost.tmp
-
   _log 'trace' 'Enabling Dovecot LDAP authentication'
 
   sed -i -e '/\!include auth-ldap\.conf\.ext/s/^#//' /etc/dovecot/conf.d/10-auth.conf