From 543bd8b16bd076b49fd646ac9c2d7c28116e22ba Mon Sep 17 00:00:00 2001 From: Nathan Pierce Date: Mon, 7 Jun 2021 08:58:34 -0400 Subject: [PATCH] MacOS linting & testing support + docs (#2001) --- .github/workflows/linting.yml | 10 -- .gitignore | 3 + Makefile | 16 +- .../contributing/issues-and-pull-requests.md | 8 +- docs/content/contributing/tests.md | 15 +- setup.sh | 38 +++-- target/scripts/helper-functions.sh | 1 + test/linting/.ecrc.json | 11 +- test/linting/.hadolint.yaml | 1 + test/linting/lint.sh | 147 ++++++------------ test/mail_with_relays.bats | 12 +- 11 files changed, 109 insertions(+), 153 deletions(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index ec1a1e32..6fc50491 100644 --- a/.github/workflows/linting.yml +++ b/.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 diff --git a/.gitignore b/.gitignore index 0b897101..1999e03d 100644 --- a/.gitignore +++ b/.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 diff --git a/Makefile b/Makefile index a0eb90cc..fce1d93a 100644 --- a/Makefile +++ b/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/* diff --git a/docs/content/contributing/issues-and-pull-requests.md b/docs/content/contributing/issues-and-pull-requests.md index 38fe3184..a874bf4b 100644 --- a/docs/content/contributing/issues-and-pull-requests.md +++ b/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 -5. Use `make clean all` to build image locally and run tests (note that tests work on Linux **only**) -6. Document your improvements if necessary (e.g. if you introduced new environment variables, describe those in the [ENV documentation][docs-environment]) -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. +4. [Prepare your environment and run linting and tests][docs-tests] +5. 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. 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 diff --git a/docs/content/contributing/tests.md b/docs/content/contributing/tests.md index f41eeba3..aa4456cf 100644 --- a/docs/content/contributing/tests.md +++ b/docs/content/contributing/tests.md @@ -2,5 +2,16 @@ title: 'Contributing | Tests' --- -!!! todo - This section should provide a detailed step by step guide on how to write tests \ No newline at end of file +1. Install docker +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` diff --git a/setup.sh b/setup.sh index 6410ab91..3b8752d9 100755 --- a/setup.sh +++ b/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 "${@}" ;; - update ) shift 2 ; _docker_image updatemailuser "${@}" ;; - del ) shift 2 ; _docker_container delmailuser "${@}" ;; - restrict ) shift 2 ; _docker_container restrict-access "${@}" ;; + add ) shift 2 ; _docker_image addmailuser "${@:+$@}" ;; + update ) shift 2 ; _docker_image updatemailuser "${@:+$@}" ;; + del ) shift 2 ; _docker_container delmailuser "${@:+$@}" ;; + 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 "${@}" ;; - del ) shift 2 ; _docker_image delquota "${@}" ;; + set ) shift 2 ; _docker_image setquota "${@:+$@}" ;; + 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-auth ) shift 2 ; _docker_image addsaslpassword "${@}" ;; - exclude-domain ) shift 2 ; _docker_image excluderelaydomain "${@}" ;; + add-domain ) shift 2 ; _docker_image addrelayhost "${@:+$@}" ;; + add-auth ) shift 2 ; _docker_image addsaslpassword "${@:+$@}" ;; + 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 "${@:+$@}" diff --git a/target/scripts/helper-functions.sh b/target/scripts/helper-functions.sh index 399856c3..7b05870f 100755 --- a/target/scripts/helper-functions.sh +++ b/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 diff --git a/test/linting/.ecrc.json b/test/linting/.ecrc.json index 27ac948d..ac50331a 100644 --- a/test/linting/.ecrc.json +++ b/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": { diff --git a/test/linting/.hadolint.yaml b/test/linting/.hadolint.yaml index 74c65ac6..caa8f1cf 100644 --- a/test/linting/.hadolint.yaml +++ b/test/linting/.hadolint.yaml @@ -2,6 +2,7 @@ ignored: - DL3005 - DL3008 - DL3015 + - DL3005 trustedRegistries: - docker.io diff --git a/test/linting/lint.sh b/test/linting/lint.sh index bb2d53e7..aa0162f8 100755 --- a/test/linting/lint.sh +++ b/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$|' - IGNORE+='\.conf$|\.init$|.*test/.*|.*tools/.*' - - local LINT=( - eclint - -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[@]}" + if docker run --rm --tty \ + --volume "${REPO_ROOT}:/ci:ro" \ + --workdir "/ci" \ + --name eclint \ + "mstruebing/editorconfig-checker:${ECLINT_VERSION}" ec -config "/ci/test/linting/.ecrc.json" 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]}" - then - __log_failure 'linter not in PATH' - return 2 - fi - - __log_info "$(${LINT[0]} --version | grep -E -o "[0-9\.]*")" - - if "${LINT[@]}" Dockerfile + if docker run --rm --tty \ + --volume "${REPO_ROOT}:/ci:ro" \ + --workdir "/ci" \ + "hadolint/hadolint:v${HADOLINT_VERSION}-alpine" hadolint --config "/ci/test/linting/.hadolint.yaml" 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=` when sourcing - # files with `. ` 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 ${?} diff --git a/test/mail_with_relays.bats b/test/mail_with_relays.bats index 27525fd7..1c7c3e70 100644 --- a/test/mail_with_relays.bats +++ b/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" {