MacOS linting & testing support + docs (#2001)

.github/workflows/linting.yml

@@ -6,7 +6,6 @@ on:
   push:
     branches:
       - master
-
 jobs:
   lint:
     runs-on: ubuntu-20.04
@@ -17,25 +16,16 @@ jobs:
           submodules: recursive
       - name: Hadolint
         run: |
-          sudo curl -S -L https://github.com/hadolint/hadolint/releases/download/v${HADOLINT_VERSION}/hadolint-$(uname -s)-$(uname -m) -o /usr/local/bin/hadolint
-          sudo chmod +rx /usr/local/bin/hadolint
           make hadolint
         env:
           HADOLINT_VERSION: 2.4.1
       - name: ShellCheck
         run: |
-          sudo curl -S -L "https://github.com/koalaman/shellcheck/releases/download/v${SHELLCHECK_VERSION}/shellcheck-v${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar -xJ
-          sudo mv "shellcheck-v${SHELLCHECK_VERSION}/shellcheck" /usr/bin/
-          sudo rm -rf "shellcheck-v${SHELLCHECK_VERSION}"
           make shellcheck
         env:
           SHELLCHECK_VERSION: 0.7.2
       - name: ECLint
         run: |
-          sudo curl -S -L "https://github.com/editorconfig-checker/editorconfig-checker/releases/download/${ECLINT_VERSION}/ec-linux-amd64.tar.gz" | tar -xaz
-          sudo mv bin/ec-linux-amd64 /usr/bin/eclint
-          sudo rm -rf bin
-          sudo chmod +x /usr/bin/eclint
           make eclint
         env:
           ECLINT_VERSION: 2.3.5

.gitignore

@@ -43,6 +43,9 @@ test/config/without-virtual/
 test/config/with-domain/
 test/onedir
 test/duplicate_configs
+test/alias
+test/quota
+test/relay
 
 config.bak
 testconfig.bak

Makefile

@@ -4,10 +4,6 @@ NAME   ?= mailserver-testing:ci
 VCS_REF = $(shell git rev-parse --short HEAD)
 VCS_VER = $(shell git describe --tags --contains --always)
 
-HADOLINT_VERSION   = 2.4.1
-SHELLCHECK_VERSION = 0.7.2
-ECLINT_VERSION     = 2.3.5
-
 export CDIR = $(shell pwd)
 
 # –––––––––––––––––––––––––––––––––––––––––––––––
@@ -29,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 ; } || :
-	-@ docker ps -a | grep -E "mail|ldap_for_mail|mail_overri.*" | cut -f 1-1 -d ' ' | xargs --no-run-if-empty docker rm -f
+	-@ for container in $$(docker ps -a | grep -E "mail|ldap_for_mail|mail_overri.*|hadolint|eclint|shellcheck" | 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/
 
 # –––––––––––––––––––––––––––––––––––––––––––––––
@@ -60,13 +56,3 @@ shellcheck:
 
 eclint:
 	@ ./test/linting/lint.sh eclint
-
-install_linters:
-	@ mkdir -p tools
-	@ curl -S -L \
-		"https://github.com/hadolint/hadolint/releases/download/v$(HADOLINT_VERSION)/hadolint-$(shell uname -s)-$(shell uname -m)" -o tools/hadolint
-	@ curl -S -L \
-		"https://github.com/koalaman/shellcheck/releases/download/v$(SHELLCHECK_VERSION)/shellcheck-v$(SHELLCHECK_VERSION).linux.x86_64.tar.xz" | tar -Jx shellcheck-v$(SHELLCHECK_VERSION)/shellcheck -O > tools/shellcheck
-	@ curl -S -L \
-		"https://github.com/editorconfig-checker/editorconfig-checker/releases/download/$(ECLINT_VERSION)/ec-linux-amd64.tar.gz" | tar -zx bin/ec-linux-amd64 -O > tools/eclint
-	@ chmod u+rx tools/*

docs/content/contributing/issues-and-pull-requests.md

@@ -35,10 +35,9 @@ The development workflow is the following:
    2. Run `git submodule update --init --recursive`
 2. Write the code that is needed :D
 3. Add integration tests if necessary
-4. Get the linters with `make install_linters` and install `jq` with the package manager of your OS
+4. [Prepare your environment and run linting and tests][docs-tests]
-5. Use `make clean all` to build image locally and run tests (note that tests work on Linux **only**)
+5. Document your improvements if necessary (e.g. if you introduced new environment variables, describe those in the [ENV documentation][docs-environment])
-6. Document your improvements if necessary (e.g. if you introduced new environment variables, describe those in the [ENV documentation][docs-environment])
+6. [Commit][commit] and [sign your commit][gpg], push and create a pull-request to merge into `master`. Please **use the pull-request template** to provide a minimum of contextual information and make sure to meet the requirements of the checklist. 
-7. [Commit][commit] and [sign your commit][gpg], push and create a pull-request to merge into `master`. Please **use the pull-request template** to provide a minimum of contextual information and make sure to meet the requirements of the checklist. 
    1. Pull requests are automatically tested against the CI and will be reviewed when tests pass
    2. When your changes are validated, your branch is merged
    3. CI builds the new `:edge` image immediately and your changes will be includes in the next version release.
@@ -46,5 +45,6 @@ The development workflow is the following:
 [docs]: https://docker-mailserver.github.io/docker-mailserver/edge
 [github-file-readme]: https://github.com/docker-mailserver/docker-mailserver/blob/master/README.md
 [docs-environment]: ../config/environment.md
+[docs-tests]: ./tests.md
 [commit]: https://help.github.com/articles/closing-issues-via-commit-messages/
 [gpg]: https://docs.github.com/en/github/authenticating-to-github/generating-a-new-gpg-key

docs/content/contributing/tests.md

@@ -2,5 +2,16 @@
 title: 'Contributing | Tests'
 ---
 
-!!! todo
+1. Install docker
-    This section should provide a detailed step by step guide on how to write tests
+2. Execute `git submodule update --init --recursive`
+3. Install jq
+
+    !!! info "MacOS Specific (needed for tests)"
+
+        ```bash
+        brew install coreutils
+        # bash >= 4.0 for associative arrays
+        brew install bash
+        ```
+
+4. Execute `make clean all`

setup.sh

@@ -36,6 +36,12 @@ ly and use ./setup.sh help and read the VERSION section.\n" >&2
 
 function _get_absolute_script_directory
 {
+  if [[ "$(uname)" == "Darwin" ]]
+  then
+    readlink() {
+      greadlink "${@:+$@}" # Requires coreutils
+    }
+  fi
   if dirname "$(readlink -f "${0}")" &>/dev/null
   then
     DIR="$(dirname "$(readlink -f "${0}")")"
@@ -225,7 +231,7 @@ function _docker_image
   if ${USE_CONTAINER}
   then
     # reuse existing container specified on command line
-    ${CRI} exec "${USE_TTY}" "${CONTAINER_NAME}" "${@}"
+    ${CRI} exec "${USE_TTY}" "${CONTAINER_NAME}" "${@:+$@}"
   else
     # start temporary container with specified image
     if ! _docker_image_exists "${IMAGE_NAME}"
@@ -236,7 +242,7 @@ function _docker_image
 
     ${CRI} run --rm \
       -v "${CONFIG_PATH}:/tmp/docker-mailserver${USE_SELINUX}" \
-      "${USE_TTY}" "${IMAGE_NAME}" "${@}"
+      "${USE_TTY}" "${IMAGE_NAME}" "${@:+$@}"
   fi
 }
 
@@ -244,7 +250,7 @@ function _docker_container
 {
   if [[ -n ${CONTAINER_NAME} ]]
   then
-    ${CRI} exec "${USE_TTY}" "${CONTAINER_NAME}" "${@}"
+    ${CRI} exec "${USE_TTY}" "${CONTAINER_NAME}" "${@:+$@}"
   else
     echo "The mailserver is not running!"
     exit 1
@@ -340,10 +346,10 @@ function _main
 
     email )
       case ${2:-} in
-        add      ) shift 2 ; _docker_image addmailuser "${@}" ;;
+        add      ) shift 2 ; _docker_image addmailuser "${@:+$@}" ;;
-        update   ) shift 2 ; _docker_image updatemailuser "${@}" ;;
+        update   ) shift 2 ; _docker_image updatemailuser "${@:+$@}" ;;
-        del      ) shift 2 ; _docker_container delmailuser "${@}" ;;
+        del      ) shift 2 ; _docker_container delmailuser "${@:+$@}" ;;
-        restrict ) shift 2 ; _docker_container restrict-access "${@}" ;;
+        restrict ) shift 2 ; _docker_container restrict-access "${@:+$@}" ;;
         list     ) _docker_container listmailuser ;;
         *        ) _usage ;;
       esac
@@ -360,24 +366,24 @@ function _main
 
     quota )
       case ${2:-} in
-        set      ) shift 2 ; _docker_image setquota "${@}" ;;
+        set      ) shift 2 ; _docker_image setquota "${@:+$@}" ;;
-        del      ) shift 2 ; _docker_image delquota "${@}" ;;
+        del      ) shift 2 ; _docker_image delquota "${@:+$@}" ;;
         *        ) _usage ;;
       esac
       ;;
 
     config )
       case ${2:-} in
-        dkim     ) shift 2 ; _docker_image open-dkim "${@}" ;;
+        dkim     ) shift 2 ; _docker_image open-dkim "${@:+$@}" ;;
         *        ) _usage ;;
       esac
       ;;
 
     relay )
       case ${2:-} in
-        add-domain     ) shift 2 ; _docker_image addrelayhost "${@}" ;;
+        add-domain     ) shift 2 ; _docker_image addrelayhost "${@:+$@}" ;;
-        add-auth       ) shift 2 ; _docker_image addsaslpassword "${@}" ;;
+        add-auth       ) shift 2 ; _docker_image addsaslpassword "${@:+$@}" ;;
-        exclude-domain ) shift 2 ; _docker_image excluderelaydomain "${@}" ;;
+        exclude-domain ) shift 2 ; _docker_image excluderelaydomain "${@:+$@}" ;;
         *              ) _usage ;;
       esac
       ;;
@@ -385,7 +391,7 @@ function _main
     debug )
       case ${2:-} in
         fetchmail      ) _docker_image debug-fetchmail ;;
-        fail2ban       ) shift 2 ; _docker_container fail2ban "${@}" ;;
+        fail2ban       ) shift 2 ; _docker_container fail2ban "${@:+$@}" ;;
         show-mail-logs ) _docker_container cat /var/log/mail/mail.log ;;
         inspect        ) _inspect ;;
         login          )
@@ -394,7 +400,7 @@ function _main
           then
             _docker_container /bin/bash
           else
-            _docker_container /bin/bash -c "${@}"
+            _docker_container /bin/bash -c "${@:+$@}"
           fi
           ;;
         * ) _usage ; exit 1 ;;
@@ -406,4 +412,4 @@ function _main
   esac
 }
 
-_main "${@}"
+_main "${@:+$@}"

target/scripts/helper-functions.sh

@@ -163,6 +163,7 @@ function _populate_relayhost_map
     if ! grep -q -e "^@${DOMAIN}\b" /etc/postfix/relayhost_map && ! grep -qs -e "^\s*@${DOMAIN}\s*$" /tmp/docker-mailserver/postfix-relaymap.cf
     then
       _notify 'inf' "Adding relay mapping for ${DOMAIN}"
+      # shellcheck disable=SC2153
       echo "@${DOMAIN}    [${RELAY_HOST}]:${RELAY_PORT}" >> /etc/postfix/relayhost_map
     fi
   done

test/linting/.ecrc.json

@@ -1,11 +1,18 @@
 {
-  "Version": "2.3.5",
   "Verbose": false,
   "Debug": false,
   "IgnoreDefaults": false,
   "SpacesAftertabs": true,
   "NoColor": false,
-  "Exclude": [],
+  "Exclude": [
+    "^test/",
+    "\\.git.*",
+    "\\.bats$",
+    "\\.cf$",
+    "\\.conf$",
+    "\\.init$",
+    "\\.md$"
+  ],
   "AllowedContentTypes": [],
   "PassedFiles": [],
   "Disable": {

test/linting/.hadolint.yaml

@@ -2,6 +2,7 @@ ignored:
   - DL3005
   - DL3008
   - DL3015
+  - DL3005
 
 trustedRegistries:
   - docker.io

test/linting/lint.sh

@@ -6,6 +6,13 @@
 
 SCRIPT="lint.sh"
 
+SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+REPO_ROOT="$(realpath "${SCRIPT_DIR}"/../../)"
+
+HADOLINT_VERSION=2.4.1
+ECLINT_VERSION=2.3.5
+SHELLCHECK_VERSION=0.7.2
+
 set -eEuo pipefail
 trap '__log_err "${FUNCNAME[0]:-?}" "${BASH_COMMAND:-?}" ${LINENO:-?} ${?:-?}' ERR
 
@@ -51,26 +58,11 @@ function _eclint
 {
   local SCRIPT='EDITORCONFIG LINTER'
 
-  local IGNORE='.*\.git.*|.*\.md$|\.bats$|\.cf$|'
+  if docker run --rm --tty \
-  IGNORE+='\.conf$|\.init$|.*test/.*|.*tools/.*'
+      --volume "${REPO_ROOT}:/ci:ro" \
-
+      --workdir "/ci" \
-  local LINT=(
+      --name eclint \
-    eclint
+      "mstruebing/editorconfig-checker:${ECLINT_VERSION}" ec -config "/ci/test/linting/.ecrc.json"
-    -config
-    "${CDIR}/test/linting/.ecrc.json"
-    -exclude
-    "(${IGNORE})"
-  )
-
-  if ! __in_path "${LINT[0]}"
-  then
-    __log_failure 'linter not in PATH'
-    return 2
-  fi
-
-  __log_info "$(${LINT[0]} --version)"
-
-  if "${LINT[@]}"
   then
     __log_success
   else
@@ -82,17 +74,11 @@ function _eclint
 function _hadolint
 {
   local SCRIPT='HADOLINT'
-  local LINT=(hadolint -c "${CDIR}/test/linting/.hadolint.yaml")
 
-  if ! __in_path "${LINT[0]}"
+  if docker run --rm --tty \
-  then
+      --volume "${REPO_ROOT}:/ci:ro" \
-    __log_failure 'linter not in PATH'
+      --workdir "/ci" \
-    return 2
+      "hadolint/hadolint:v${HADOLINT_VERSION}-alpine" hadolint --config "/ci/test/linting/.hadolint.yaml" Dockerfile
-  fi
-
-  __log_info "$(${LINT[0]} --version | grep -E -o "[0-9\.]*")"
-
-  if "${LINT[@]}" Dockerfile
   then
     __log_success
   else
@@ -104,73 +90,42 @@ function _hadolint
 function _shellcheck
 {
   local SCRIPT='SHELLCHECK'
-  local ERR=0
+  
-  local LINT=(shellcheck -x -S style -Cauto -o all -e SC2154 -W 50)
+  # File paths for shellcheck:
+  F_SH="$(find . -type f -iname '*.sh' \
+    -not -path './test/bats/*' \
+    -not -path './test/test_helper/*' \
+    -not -path './target/docker-configomat/*'
+  )"
+  # macOS lacks parity for `-executable` but presently produces the same results: https://stackoverflow.com/a/4458361
+  [[ "$(uname)" == "Darwin" ]] && FIND_EXEC="-perm +111 -type l -or" || FIND_EXEC="-executable"
+  # shellcheck disable=SC2248
+  F_BIN="$(find 'target/bin' ${FIND_EXEC} -type f)"
+  F_BATS="$(find 'test' -maxdepth 1 -type f -iname '*.bats')"
 
-  if ! __in_path "${LINT[0]}"
+  # This command is a bit easier to grok as multi-line. There is a `.shellcheckrc` file, but it's only supports half of the options below, thus kept as CLI:
+  CMD_SHELLCHECK=(shellcheck 
+    --external-sources 
+    --check-sourced
+    --severity=style
+    --color=auto
+    --wiki-link-count=50
+    --enable=all
+    --exclude=SC2154
+    --source-path=SCRIPTDIR
+    "${F_SH} ${F_BIN} ${F_BATS}"
+  )
+  
+  # shellcheck disable=SC2068
+  if docker run --rm --tty \
+      --volume "${REPO_ROOT}:/ci:ro" \
+      --workdir "/ci" \
+      "koalaman/shellcheck-alpine:v${SHELLCHECK_VERSION}" ${CMD_SHELLCHECK[@]}
   then
-    __log_failure 'linter not in PATH'
-    return 2
-  fi
-
-  __log_info "$(${LINT[0]} --version | grep -m 2 -o "[0-9.]*")"
-
-  # an overengineered solution to allow shellcheck -x to
-  # properly follow `source=<SOURCE FILE>` when sourcing
-  # files with `. <FILE>` in shell scripts.
-  while read -r FILE
-  do
-    if ! (
-      cd "$(realpath "$(dirname "$(readlink -f "${FILE}")")")"
-      if ! "${LINT[@]}" "$(basename -- "${FILE}")"
-      then
-        exit 1
-      fi
-    )
-    then
-      ERR=1
-    fi
-  done < <(find . -type f -iname "*.sh" \
-    -not -path "./test/bats/*" \
-    -not -path "./test/test_helper/*" \
-    -not -path "./target/docker-configomat/*")
-
-  # the same for executables in target/bin/
-  while read -r FILE
-  do
-    if ! (
-      cd "$(realpath "$(dirname "$(readlink -f "${FILE}")")")"
-      if ! "${LINT[@]}" "$(basename -- "${FILE}")"
-      then
-        exit 1
-      fi
-    )
-    then
-      ERR=1
-    fi
-  done < <(find target/bin -executable -type f)
-
-  # the same for all test files
-  while read -r FILE
-  do
-    if ! (
-      cd "$(realpath "$(dirname "$(readlink -f "${FILE}")")")"
-      if ! "${LINT[@]}" "$(basename -- "${FILE}")"
-      then
-        exit 1
-      fi
-    )
-    then
-      ERR=1
-    fi
-  done < <(find test/ -maxdepth 1 -type f -iname "*.bats")
-
-  if [[ ${ERR} -eq 1 ]]
-  then
-    __log_failure 'errors encountered'
-    return 1
-  else
     __log_success
+  else
+    __log_failure
+    return 1
   fi
 }
 
@@ -187,8 +142,4 @@ function __main
   esac
 }
 
-# prefer linters installed in tools
-PATH="${CDIR}/tools:${PATH}"
-export PATH
-
 __main "${@}" || exit ${?}

test/mail_with_relays.bats

@@ -12,7 +12,7 @@ function setup_file() {
     # We use a temporary config directory since we'll be dynamically editing
     # it with setup.sh.
     tmp_confdir=$(mktemp -d /tmp/docker-mailserver-config-relay-hosts-XXXXX)
-    cp -aT test/config/relay-hosts "${tmp_confdir}"
+    cp -a test/config/relay-hosts/* "${tmp_confdir}/"
 
     docker run -d --name mail_with_relays \
             -v "${tmp_confdir}":/tmp/docker-mailserver \
@@ -39,12 +39,12 @@ function teardown_file() {
 
 @test "checking relay hosts: default mapping is added from env vars" {
   run docker exec mail_with_relays grep -e domainone.tld /etc/postfix/relayhost_map
-  assert_output -e '^@domainone.tld\s+\[default.relay.com\]:2525$'
+  assert_output -e '^@domainone.tld[[:space:]]+\[default.relay.com\]:2525$'
 }
 
 @test "checking relay hosts: default mapping is added from env vars for virtual user entry" {
   run docker exec mail_with_relays grep -e domain1.tld /etc/postfix/relayhost_map
-  assert_output -e '^@domain1.tld\s+\[default.relay.com\]:2525$'
+  assert_output -e '^@domain1.tld[[:space:]]+\[default.relay.com\]:2525$'
 }
 
 @test "checking relay hosts: default mapping is added from env vars for new user entry" {
@@ -52,7 +52,7 @@ function teardown_file() {
   assert_output ''
   run ./setup.sh -c mail_with_relays email add user0@domainzero.tld password123
   run_until_success_or_timeout 10 docker exec mail_with_relays grep -e domainzero.tld /etc/postfix/relayhost_map
-  assert_output -e '^@domainzero.tld\s+\[default.relay.com\]:2525$'
+  assert_output -e '^@domainzero.tld[[:space:]]+\[default.relay.com\]:2525$'
 }
 
 @test "checking relay hosts: default mapping is added from env vars for new virtual user entry" {
@@ -60,12 +60,12 @@ function teardown_file() {
   assert_output ''
   run ./setup.sh -c mail_with_relays alias add user2@domain2.tld user2@domaintwo.tld
   run_until_success_or_timeout 10 docker exec mail_with_relays grep -e domain2.tld /etc/postfix/relayhost_map
-  assert_output -e '^@domain2.tld\s+\[default.relay.com\]:2525$'
+  assert_output -e '^@domain2.tld[[:space:]]+\[default.relay.com\]:2525$'
 }
 
 @test "checking relay hosts: custom mapping is added from file" {
   run docker exec mail_with_relays grep -e domaintwo.tld /etc/postfix/relayhost_map
-  assert_output -e '^@domaintwo.tld\s+\[other.relay.com\]:587$'
+  assert_output -e '^@domaintwo.tld[[:space:]]+\[other.relay.com\]:587$'
 }
 
 @test "checking relay hosts: ignored domain is not added" {