docker-mailserver/target/scripts/check-for-changes.sh

215 lines
8.2 KiB
Bash
Raw Normal View History

#!/bin/bash
fix: `check-for-changes.sh` should not fall out of sync with shared logic (#2260) Removes duplicate logic from `check-for-changes.sh` that is used/maintained elsewhere to avoid risk of problems, as this code is already starting to diverge / rot. --- Previously the change detection support has had code added for rebuilding config upon change detection which is the same as code run during startup scripts. Unfortunately over time this has fallen out of sync. Mostly the startup scripts would get maintenance and the contributor and reviewers may not have been aware of the duplicate code handled by `check-for-changes.sh`. That code was starting to diverge in addition to some changes in structure (_eg: relay host logic seems interleaved here vs separated out in startup scripts_). I wanted to address this before it risks becoming a much bigger headache. Rather than bloat `helper-functions.sh` further, I've added a `helpers/` folder extracting relevant common logic between startup scripts and `changedetector`. If you want to follow that process I've kept scoped commits to make those diffs easier. Some minor changes/improvements were added but nothing significant. --- - chore: Extract relay host logic to new `relay.sh` helper - chore: Extract `/etc/postfix/sasl_passwd` logic to new `sasl.sh` helper - chore: Extract `postfix-accounts.cf` logic to new `accounts.sh` helper - chore: Extract `/etc/aliases` logic to new `aliases.sh` helper - chore: Extract `/etc/postfix/vhost` logic to new `postfix.sh` helper - chore: Add inline docs for Postfix configs > These are possibly more verbose than needed and can be reduced at a later stage. > They are helpful during this refactor process while investigating that everything is handled correctly. `accounts.sh`: - Add note regarding potential bug for bare domain setups with `/etc/postfix/vhost` and `mydestination` sharing same domain value. `relay.sh`: - Remove the tabs for a single space delimiter, revised associated comment. - Add PR reference for original `_populate_relayhost_map` implementation which has some useful details. Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> Co-authored-by: Casper <casperklein@users.noreply.github.com>
2021-11-20 20:33:49 +00:00
# TODO: Adapt for compatibility with LDAP
# Only the cert renewal change detection may be relevant for LDAP?
refactor: letsencrypt implicit location discovery (#2525) * chore: Extract letsencrypt logic into methods This allows other scripts to share the functionality to discover the correct letsencrypt folder from the 3 possible locations (where specific order is important). As these methods should now return a string value, the `return 1` after a panic is now dropped. * chore: Update comments The todo is resolved with this PR, `_setup_ssl` will be called by both cert conditional statements with purpose for each better documented to maintainers at the start of the logic block. * refactor: Defer most logic to helper/ssl.sh The loop is no longer required, extraction is delegated to `_setup_ssl` now. For the change event prevention, we retrieve the relevant FQDN via the new helper method, beyond that it's just indentation diff. `check-for-changes.sh` adjusted to allow locally scoped var declarations by wrapping a function. Presently no loop control flow is needed so this seems fine. Made it clear that `CHANGED` is local and `CHKSUM_FILE` is not. Panic scope doesn't require `SSL_TYPE` for context, it's clearly`letsencrypt`. * fix: Correctly match wildcard results Now that the service configs are properly updated, when the services restart they will return a cert with the SAN `DNS:*.example.test`, which is valid for `mail.example.test`, however the test function did not properly account for this in the regexp query. Resolved by truncating the left-most DNS label from FQDN and adding a third check to match a returned wildcard DNS result. Extracted out the common logic to create the regexp query and renamed the methods to communicate more clearly that they check the FQDN is supported, not necessarily explicitly listed by the cert. * tests(letsencrypt): Enable remaining tests These will now pass. Adjusted comments accordingly. Added an additional test on a fake FQDN that should still be valid to a wildcard cert (SNI validation in a proper setup would reject the connection afterwards). Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2022-04-18 10:52:50 +00:00
# CHKSUM_FILE global is imported from this file:
# shellcheck source=./helpers/index.sh
source /usr/local/bin/helpers/index.sh
refactor: letsencrypt implicit location discovery (#2525) * chore: Extract letsencrypt logic into methods This allows other scripts to share the functionality to discover the correct letsencrypt folder from the 3 possible locations (where specific order is important). As these methods should now return a string value, the `return 1` after a panic is now dropped. * chore: Update comments The todo is resolved with this PR, `_setup_ssl` will be called by both cert conditional statements with purpose for each better documented to maintainers at the start of the logic block. * refactor: Defer most logic to helper/ssl.sh The loop is no longer required, extraction is delegated to `_setup_ssl` now. For the change event prevention, we retrieve the relevant FQDN via the new helper method, beyond that it's just indentation diff. `check-for-changes.sh` adjusted to allow locally scoped var declarations by wrapping a function. Presently no loop control flow is needed so this seems fine. Made it clear that `CHANGED` is local and `CHKSUM_FILE` is not. Panic scope doesn't require `SSL_TYPE` for context, it's clearly`letsencrypt`. * fix: Correctly match wildcard results Now that the service configs are properly updated, when the services restart they will return a cert with the SAN `DNS:*.example.test`, which is valid for `mail.example.test`, however the test function did not properly account for this in the regexp query. Resolved by truncating the left-most DNS label from FQDN and adding a third check to match a returned wildcard DNS result. Extracted out the common logic to create the regexp query and renamed the methods to communicate more clearly that they check the FQDN is supported, not necessarily explicitly listed by the cert. * tests(letsencrypt): Enable remaining tests These will now pass. Adjusted comments accordingly. Added an additional test on a fake FQDN that should still be valid to a wildcard cert (SNI validation in a proper setup would reject the connection afterwards). Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2022-04-18 10:52:50 +00:00
_log_with_date 'debug' 'Starting changedetector'
# ATTENTION: Do not remove!
# This script requires some environment variables to be properly set.
# POSTMASTER_ADDRESS (for helpers/alias.sh) is read from /etc/dms-settings
# shellcheck source=/dev/null
source /etc/dms-settings
# HOSTNAME and DOMAINNAME are used by helpers/ssl.sh and _monitored_files_checksums
# These are not stored in /etc/dms-settings
# TODO: It is planned to stop overriding HOSTNAME and replace that
# usage with DMS_HOSTNAME, which should remove the need to call this:
_obtain_hostname_and_domainname
# This is a helper to properly set all Rspamd-related environment variables
# correctly and in one place.
_rspamd_get_envs
Final Migration Step (#6) * first migration steps * altered issue templates * altered README * removed .travis.yml * adjusting registry & repository, Dockerfile and compose.env * Close stale issues automatically * Integrated CI with Github Actions (#3) * feat: integrated ci with github actions * fix: use secrets for docker org and update image * docs: clarify why we use -t if no tty exists * fix: correct remaining references to old repo chore: prettier automatically updated markdown as well * fix: hardcode docker org * change testing image to just testing * ci: add armv7 as a supported platform * finished migration steps * corrected linting in build-push action * corrected linting in build-push action (2) * minor preps for PR * correcting push on pull request and minor details * adjusted workflows to adhere closer to @wernerfred's diagram * minor patches * adjusting Dockerfile's installation of base packages * adjusting schedule for stale issue action * reverting license text * improving CONTRIBUTING.md PR text * Update CONTRIBUTING.md * a bigger patch at the end * moved all scripts into one directory under target/scripts/ * moved the quota-warning.sh script into target/scripts/ and removed empty directory /target/dovecot/scripts * minor fixes here and there * adjusted workflows for use a fully qualified name (i.e. docker.io/...) * improved on the Dockerfile layer count * corrected local tests - now they (actually) work (fine)! * corrected start-mailserver.sh to make use of defaults consistently * removed very old, deprecated variables (actually only one) * various smaller improvements in the end * last commit before merging #6 * rearranging variables to use alphabetic order Co-authored-by: casperklein <casperklein@users.noreply.github.com> Co-authored-by: Nick Pappas <radicand@users.noreply.github.com> Co-authored-by: William Desportes <williamdes@wdes.fr>
2021-01-16 09:16:05 +00:00
# verify checksum file exists; must be prepared by start-mailserver.sh
2023-05-24 07:06:59 +00:00
if [[ ! -f ${CHKSUM_FILE} ]]; then
_exit_with_error "'${CHKSUM_FILE}' is missing" 0
fi
_log_with_date 'trace' "Using postmaster address '${POSTMASTER_ADDRESS}'"
_log_with_date 'debug' "Changedetector is ready"
2023-05-25 23:01:41 +00:00
function _check_for_changes() {
# get chksum and check it, no need to lock config yet
_monitored_files_checksums >"${CHKSUM_FILE}.new"
cmp --silent -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new"
# cmp return codes
# 0 files are identical
# 1 files differ
# 2 inaccessible or missing argument
2023-05-24 07:06:59 +00:00
if [[ ${?} -eq 1 ]]; then
_log_with_date 'info' 'Change detected'
_create_lock # Shared config safety lock
refactor: letsencrypt implicit location discovery (#2525) * chore: Extract letsencrypt logic into methods This allows other scripts to share the functionality to discover the correct letsencrypt folder from the 3 possible locations (where specific order is important). As these methods should now return a string value, the `return 1` after a panic is now dropped. * chore: Update comments The todo is resolved with this PR, `_setup_ssl` will be called by both cert conditional statements with purpose for each better documented to maintainers at the start of the logic block. * refactor: Defer most logic to helper/ssl.sh The loop is no longer required, extraction is delegated to `_setup_ssl` now. For the change event prevention, we retrieve the relevant FQDN via the new helper method, beyond that it's just indentation diff. `check-for-changes.sh` adjusted to allow locally scoped var declarations by wrapping a function. Presently no loop control flow is needed so this seems fine. Made it clear that `CHANGED` is local and `CHKSUM_FILE` is not. Panic scope doesn't require `SSL_TYPE` for context, it's clearly`letsencrypt`. * fix: Correctly match wildcard results Now that the service configs are properly updated, when the services restart they will return a cert with the SAN `DNS:*.example.test`, which is valid for `mail.example.test`, however the test function did not properly account for this in the regexp query. Resolved by truncating the left-most DNS label from FQDN and adding a third check to match a returned wildcard DNS result. Extracted out the common logic to create the regexp query and renamed the methods to communicate more clearly that they check the FQDN is supported, not necessarily explicitly listed by the cert. * tests(letsencrypt): Enable remaining tests These will now pass. Adjusted comments accordingly. Added an additional test on a fake FQDN that should still be valid to a wildcard cert (SNI validation in a proper setup would reject the connection afterwards). Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2022-04-18 10:52:50 +00:00
local CHANGED
CHANGED=$(_get_changed_files "${CHKSUM_FILE}" "${CHKSUM_FILE}.new")
# Handle any changes
_ssl_changes
_postfix_dovecot_changes
_rspamd_changes
fix(changedetector): Use service `reload` commands instead of `supervisorctl restart <service>` (#2947) With `reload` a change detection event during local testing can be processed in less than a second according to logs. Previously this was 5+ seconds (_plus additional downtime for Postfix/Dovecot to become available again_). In the past it was apparently an issue to use `<service> reload` due to a concern with the PID for wrapper scripts that `supervisorctl` managed, thus `supervisorctl <service> restart` had been used. Past discussions with maintainers suggest this is not likely an issue anymore, and `reload` should be fine to switch to now :+1: --- **NOTE:** It may not be an issue in the CI, but on _**local systems running tests may risk failure in `setup-cli.bats` from a false positive**_ due to 1 second polling window of the test helper method, and a change event being possible to occur entirely between the two checks undetected by the current approach. If this is a problem, we may need to think of a better way to catch the change. The `letsencrypt` test counts how many change events are expected to have been processed, and this could technically be leveraged by the test helper too. --- **NOTE:** These two lines (_with regex pattern for postfix_) are output in the terminal when using the services respective `reload` commands: ``` postfix/master.*: reload -- version .*, configuration /etc/postfix dovecot: master: Warning: SIGHUP received - reloading configuration ``` I wasn't sure how to match them as they did not appear in the `changedetector` log (_**EDIT:** they appear in the main log output, eg `docker logs <container name>`_). Instead I've just monitored the `changedetector` log messages, which should be ok for logic that previously needed to ensure Dovecot / Postfix was back up after the `restart` was issued. --- Commit history: * chore: Change events `reload` Dovecot and Postfix instead of `restart` Reloading is faster than restarting the processes. Restarting is a bit heavy handed here and may no longer be necessary for general usage? * tests: Adapt tests to support service `reload` instead of `restart` * chore: Additional logging for debugging change event logs * fix: Wait on change detection, then verify directory created Change detection is too fast now (0-1 seconds vs 5+). Directory being waited on here was created near the end of a change event, reducing that time to detect a change by the utility method further. We can instead check that the directory exists after the change detection event is completed. * chore: Keep using the maildir polling check We don't presently use remote storage in tests, but it might be relevant in future when testing NFS. This at least avoids any confusing failure happening when that scenario is tested.
2022-12-23 12:57:24 +00:00
_log_with_date 'debug' 'Reloading services due to detected changes'
[[ ${ENABLE_AMAVIS} -eq 1 ]] && _reload_amavis
_reload_postfix
fix(changedetector): Use service `reload` commands instead of `supervisorctl restart <service>` (#2947) With `reload` a change detection event during local testing can be processed in less than a second according to logs. Previously this was 5+ seconds (_plus additional downtime for Postfix/Dovecot to become available again_). In the past it was apparently an issue to use `<service> reload` due to a concern with the PID for wrapper scripts that `supervisorctl` managed, thus `supervisorctl <service> restart` had been used. Past discussions with maintainers suggest this is not likely an issue anymore, and `reload` should be fine to switch to now :+1: --- **NOTE:** It may not be an issue in the CI, but on _**local systems running tests may risk failure in `setup-cli.bats` from a false positive**_ due to 1 second polling window of the test helper method, and a change event being possible to occur entirely between the two checks undetected by the current approach. If this is a problem, we may need to think of a better way to catch the change. The `letsencrypt` test counts how many change events are expected to have been processed, and this could technically be leveraged by the test helper too. --- **NOTE:** These two lines (_with regex pattern for postfix_) are output in the terminal when using the services respective `reload` commands: ``` postfix/master.*: reload -- version .*, configuration /etc/postfix dovecot: master: Warning: SIGHUP received - reloading configuration ``` I wasn't sure how to match them as they did not appear in the `changedetector` log (_**EDIT:** they appear in the main log output, eg `docker logs <container name>`_). Instead I've just monitored the `changedetector` log messages, which should be ok for logic that previously needed to ensure Dovecot / Postfix was back up after the `restart` was issued. --- Commit history: * chore: Change events `reload` Dovecot and Postfix instead of `restart` Reloading is faster than restarting the processes. Restarting is a bit heavy handed here and may no longer be necessary for general usage? * tests: Adapt tests to support service `reload` instead of `restart` * chore: Additional logging for debugging change event logs * fix: Wait on change detection, then verify directory created Change detection is too fast now (0-1 seconds vs 5+). Directory being waited on here was created near the end of a change event, reducing that time to detect a change by the utility method further. We can instead check that the directory exists after the change detection event is completed. * chore: Keep using the maildir polling check We don't presently use remote storage in tests, but it might be relevant in future when testing NFS. This at least avoids any confusing failure happening when that scenario is tested.
2022-12-23 12:57:24 +00:00
[[ ${SMTP_ONLY} -ne 1 ]] && dovecot reload
_remove_lock
_log_with_date 'debug' 'Completed handling of detected change'
refactor: letsencrypt implicit location discovery (#2525) * chore: Extract letsencrypt logic into methods This allows other scripts to share the functionality to discover the correct letsencrypt folder from the 3 possible locations (where specific order is important). As these methods should now return a string value, the `return 1` after a panic is now dropped. * chore: Update comments The todo is resolved with this PR, `_setup_ssl` will be called by both cert conditional statements with purpose for each better documented to maintainers at the start of the logic block. * refactor: Defer most logic to helper/ssl.sh The loop is no longer required, extraction is delegated to `_setup_ssl` now. For the change event prevention, we retrieve the relevant FQDN via the new helper method, beyond that it's just indentation diff. `check-for-changes.sh` adjusted to allow locally scoped var declarations by wrapping a function. Presently no loop control flow is needed so this seems fine. Made it clear that `CHANGED` is local and `CHKSUM_FILE` is not. Panic scope doesn't require `SSL_TYPE` for context, it's clearly`letsencrypt`. * fix: Correctly match wildcard results Now that the service configs are properly updated, when the services restart they will return a cert with the SAN `DNS:*.example.test`, which is valid for `mail.example.test`, however the test function did not properly account for this in the regexp query. Resolved by truncating the left-most DNS label from FQDN and adding a third check to match a returned wildcard DNS result. Extracted out the common logic to create the regexp query and renamed the methods to communicate more clearly that they check the FQDN is supported, not necessarily explicitly listed by the cert. * tests(letsencrypt): Enable remaining tests These will now pass. Adjusted comments accordingly. Added an additional test on a fake FQDN that should still be valid to a wildcard cert (SNI validation in a proper setup would reject the connection afterwards). Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2022-04-18 10:52:50 +00:00
# mark changes as applied
mv "${CHKSUM_FILE}.new" "${CHKSUM_FILE}"
fi
}
2023-05-25 23:01:41 +00:00
function _get_changed_files() {
local CHKSUM_CURRENT=${1}
local CHKSUM_NEW=${2}
# Diff the two files for lines that don't match or differ from lines in CHKSUM_FILE
# grep -Fxvf
# -f use CHKSUM_FILE lines as input patterns to match for
# -F The patterns to match are treated as strings only, not treated as regex syntax
# -x (match whole lines only)
# -v invert the matching so only non-matches are output
# Extract file paths by truncating the matched content hash and white-space from lines:
# sed -r 's/^\S+[[:space:]]+//'
grep -Fxvf "${CHKSUM_CURRENT}" "${CHKSUM_NEW}" | sed -r 's/^\S+[[:space:]]+//'
}
2023-05-25 23:01:41 +00:00
function _reload_amavis() {
2023-05-24 07:06:59 +00:00
if [[ ${CHANGED} =~ ${DMS_DIR}/postfix-accounts.cf ]] || [[ ${CHANGED} =~ ${DMS_DIR}/postfix-virtual.cf ]]; then
# /etc/postfix/vhost was updated, amavis must refresh it's config by
# reading this file again in case of new domains, otherwise they will be ignored.
amavisd-new reload
fi
}
# 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
2023-05-25 23:01:41 +00:00
function _postfix_dovecot_changes() {
local DMS_DIR=/tmp/docker-mailserver
# Regenerate accounts via `helpers/accounts.sh`:
# - dovecot-quotas.cf used by _create_accounts + _create_dovecot_alias_dummy_accounts
# - postfix-virtual.cf used by _create_dovecot_alias_dummy_accounts (only when ENABLE_QUOTAS=1)
if [[ ${CHANGED} =~ ${DMS_DIR}/postfix-accounts.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/postfix-virtual.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/postfix-aliases.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/dovecot-quotas.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/dovecot-masters.cf ]]
then
fix(changedetector): Use service `reload` commands instead of `supervisorctl restart <service>` (#2947) With `reload` a change detection event during local testing can be processed in less than a second according to logs. Previously this was 5+ seconds (_plus additional downtime for Postfix/Dovecot to become available again_). In the past it was apparently an issue to use `<service> reload` due to a concern with the PID for wrapper scripts that `supervisorctl` managed, thus `supervisorctl <service> restart` had been used. Past discussions with maintainers suggest this is not likely an issue anymore, and `reload` should be fine to switch to now :+1: --- **NOTE:** It may not be an issue in the CI, but on _**local systems running tests may risk failure in `setup-cli.bats` from a false positive**_ due to 1 second polling window of the test helper method, and a change event being possible to occur entirely between the two checks undetected by the current approach. If this is a problem, we may need to think of a better way to catch the change. The `letsencrypt` test counts how many change events are expected to have been processed, and this could technically be leveraged by the test helper too. --- **NOTE:** These two lines (_with regex pattern for postfix_) are output in the terminal when using the services respective `reload` commands: ``` postfix/master.*: reload -- version .*, configuration /etc/postfix dovecot: master: Warning: SIGHUP received - reloading configuration ``` I wasn't sure how to match them as they did not appear in the `changedetector` log (_**EDIT:** they appear in the main log output, eg `docker logs <container name>`_). Instead I've just monitored the `changedetector` log messages, which should be ok for logic that previously needed to ensure Dovecot / Postfix was back up after the `restart` was issued. --- Commit history: * chore: Change events `reload` Dovecot and Postfix instead of `restart` Reloading is faster than restarting the processes. Restarting is a bit heavy handed here and may no longer be necessary for general usage? * tests: Adapt tests to support service `reload` instead of `restart` * chore: Additional logging for debugging change event logs * fix: Wait on change detection, then verify directory created Change detection is too fast now (0-1 seconds vs 5+). Directory being waited on here was created near the end of a change event, reducing that time to detect a change by the utility method further. We can instead check that the directory exists after the change detection event is completed. * chore: Keep using the maildir polling check We don't presently use remote storage in tests, but it might be relevant in future when testing NFS. This at least avoids any confusing failure happening when that scenario is tested.
2022-12-23 12:57:24 +00:00
_log_with_date 'trace' 'Regenerating accounts (Dovecot + Postfix)'
fix: `check-for-changes.sh` should not fall out of sync with shared logic (#2260) Removes duplicate logic from `check-for-changes.sh` that is used/maintained elsewhere to avoid risk of problems, as this code is already starting to diverge / rot. --- Previously the change detection support has had code added for rebuilding config upon change detection which is the same as code run during startup scripts. Unfortunately over time this has fallen out of sync. Mostly the startup scripts would get maintenance and the contributor and reviewers may not have been aware of the duplicate code handled by `check-for-changes.sh`. That code was starting to diverge in addition to some changes in structure (_eg: relay host logic seems interleaved here vs separated out in startup scripts_). I wanted to address this before it risks becoming a much bigger headache. Rather than bloat `helper-functions.sh` further, I've added a `helpers/` folder extracting relevant common logic between startup scripts and `changedetector`. If you want to follow that process I've kept scoped commits to make those diffs easier. Some minor changes/improvements were added but nothing significant. --- - chore: Extract relay host logic to new `relay.sh` helper - chore: Extract `/etc/postfix/sasl_passwd` logic to new `sasl.sh` helper - chore: Extract `postfix-accounts.cf` logic to new `accounts.sh` helper - chore: Extract `/etc/aliases` logic to new `aliases.sh` helper - chore: Extract `/etc/postfix/vhost` logic to new `postfix.sh` helper - chore: Add inline docs for Postfix configs > These are possibly more verbose than needed and can be reduced at a later stage. > They are helpful during this refactor process while investigating that everything is handled correctly. `accounts.sh`: - Add note regarding potential bug for bare domain setups with `/etc/postfix/vhost` and `mydestination` sharing same domain value. `relay.sh`: - Remove the tabs for a single space delimiter, revised associated comment. - Add PR reference for original `_populate_relayhost_map` implementation which has some useful details. Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> Co-authored-by: Casper <casperklein@users.noreply.github.com>
2021-11-20 20:33:49 +00:00
[[ ${SMTP_ONLY} -ne 1 ]] && _create_accounts
fi
# Regenerate relay config via `helpers/relay.sh`:
# - postfix-sasl-password.cf used by _relayhost_sasl
# - _populate_relayhost_map relies on:
# - postfix-relaymap.cf
# - postfix-accounts.cf + postfix-virtual.cf (both will be dropped in future)
if [[ ${CHANGED} =~ ${DMS_DIR}/postfix-accounts.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/postfix-virtual.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/postfix-relaymap.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/postfix-sasl-password.cf ]]
then
fix(changedetector): Use service `reload` commands instead of `supervisorctl restart <service>` (#2947) With `reload` a change detection event during local testing can be processed in less than a second according to logs. Previously this was 5+ seconds (_plus additional downtime for Postfix/Dovecot to become available again_). In the past it was apparently an issue to use `<service> reload` due to a concern with the PID for wrapper scripts that `supervisorctl` managed, thus `supervisorctl <service> restart` had been used. Past discussions with maintainers suggest this is not likely an issue anymore, and `reload` should be fine to switch to now :+1: --- **NOTE:** It may not be an issue in the CI, but on _**local systems running tests may risk failure in `setup-cli.bats` from a false positive**_ due to 1 second polling window of the test helper method, and a change event being possible to occur entirely between the two checks undetected by the current approach. If this is a problem, we may need to think of a better way to catch the change. The `letsencrypt` test counts how many change events are expected to have been processed, and this could technically be leveraged by the test helper too. --- **NOTE:** These two lines (_with regex pattern for postfix_) are output in the terminal when using the services respective `reload` commands: ``` postfix/master.*: reload -- version .*, configuration /etc/postfix dovecot: master: Warning: SIGHUP received - reloading configuration ``` I wasn't sure how to match them as they did not appear in the `changedetector` log (_**EDIT:** they appear in the main log output, eg `docker logs <container name>`_). Instead I've just monitored the `changedetector` log messages, which should be ok for logic that previously needed to ensure Dovecot / Postfix was back up after the `restart` was issued. --- Commit history: * chore: Change events `reload` Dovecot and Postfix instead of `restart` Reloading is faster than restarting the processes. Restarting is a bit heavy handed here and may no longer be necessary for general usage? * tests: Adapt tests to support service `reload` instead of `restart` * chore: Additional logging for debugging change event logs * fix: Wait on change detection, then verify directory created Change detection is too fast now (0-1 seconds vs 5+). Directory being waited on here was created near the end of a change event, reducing that time to detect a change by the utility method further. We can instead check that the directory exists after the change detection event is completed. * chore: Keep using the maildir polling check We don't presently use remote storage in tests, but it might be relevant in future when testing NFS. This at least avoids any confusing failure happening when that scenario is tested.
2022-12-23 12:57:24 +00:00
_log_with_date 'trace' 'Regenerating relay config (Postfix)'
fix: `check-for-changes.sh` should not fall out of sync with shared logic (#2260) Removes duplicate logic from `check-for-changes.sh` that is used/maintained elsewhere to avoid risk of problems, as this code is already starting to diverge / rot. --- Previously the change detection support has had code added for rebuilding config upon change detection which is the same as code run during startup scripts. Unfortunately over time this has fallen out of sync. Mostly the startup scripts would get maintenance and the contributor and reviewers may not have been aware of the duplicate code handled by `check-for-changes.sh`. That code was starting to diverge in addition to some changes in structure (_eg: relay host logic seems interleaved here vs separated out in startup scripts_). I wanted to address this before it risks becoming a much bigger headache. Rather than bloat `helper-functions.sh` further, I've added a `helpers/` folder extracting relevant common logic between startup scripts and `changedetector`. If you want to follow that process I've kept scoped commits to make those diffs easier. Some minor changes/improvements were added but nothing significant. --- - chore: Extract relay host logic to new `relay.sh` helper - chore: Extract `/etc/postfix/sasl_passwd` logic to new `sasl.sh` helper - chore: Extract `postfix-accounts.cf` logic to new `accounts.sh` helper - chore: Extract `/etc/aliases` logic to new `aliases.sh` helper - chore: Extract `/etc/postfix/vhost` logic to new `postfix.sh` helper - chore: Add inline docs for Postfix configs > These are possibly more verbose than needed and can be reduced at a later stage. > They are helpful during this refactor process while investigating that everything is handled correctly. `accounts.sh`: - Add note regarding potential bug for bare domain setups with `/etc/postfix/vhost` and `mydestination` sharing same domain value. `relay.sh`: - Remove the tabs for a single space delimiter, revised associated comment. - Add PR reference for original `_populate_relayhost_map` implementation which has some useful details. Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> Co-authored-by: Casper <casperklein@users.noreply.github.com>
2021-11-20 20:33:49 +00:00
_rebuild_relayhost
fi
# Regenerate system + virtual account aliases via `helpers/aliases.sh`:
[[ ${CHANGED} =~ ${DMS_DIR}/postfix-virtual.cf ]] && _handle_postfix_virtual_config
[[ ${CHANGED} =~ ${DMS_DIR}/postfix-regexp.cf ]] && _handle_postfix_regexp_config
[[ ${CHANGED} =~ ${DMS_DIR}/postfix-aliases.cf ]] && _handle_postfix_aliases_config
2019-08-01 17:39:25 +00:00
# Regenerate `/etc/postfix/vhost` (managed mail domains) via `helpers/postfix.sh`:
if [[ ${CHANGED} =~ ${DMS_DIR}/postfix-accounts.cf ]] \
|| [[ ${CHANGED} =~ ${DMS_DIR}/postfix-virtual.cf ]]
then
fix(changedetector): Use service `reload` commands instead of `supervisorctl restart <service>` (#2947) With `reload` a change detection event during local testing can be processed in less than a second according to logs. Previously this was 5+ seconds (_plus additional downtime for Postfix/Dovecot to become available again_). In the past it was apparently an issue to use `<service> reload` due to a concern with the PID for wrapper scripts that `supervisorctl` managed, thus `supervisorctl <service> restart` had been used. Past discussions with maintainers suggest this is not likely an issue anymore, and `reload` should be fine to switch to now :+1: --- **NOTE:** It may not be an issue in the CI, but on _**local systems running tests may risk failure in `setup-cli.bats` from a false positive**_ due to 1 second polling window of the test helper method, and a change event being possible to occur entirely between the two checks undetected by the current approach. If this is a problem, we may need to think of a better way to catch the change. The `letsencrypt` test counts how many change events are expected to have been processed, and this could technically be leveraged by the test helper too. --- **NOTE:** These two lines (_with regex pattern for postfix_) are output in the terminal when using the services respective `reload` commands: ``` postfix/master.*: reload -- version .*, configuration /etc/postfix dovecot: master: Warning: SIGHUP received - reloading configuration ``` I wasn't sure how to match them as they did not appear in the `changedetector` log (_**EDIT:** they appear in the main log output, eg `docker logs <container name>`_). Instead I've just monitored the `changedetector` log messages, which should be ok for logic that previously needed to ensure Dovecot / Postfix was back up after the `restart` was issued. --- Commit history: * chore: Change events `reload` Dovecot and Postfix instead of `restart` Reloading is faster than restarting the processes. Restarting is a bit heavy handed here and may no longer be necessary for general usage? * tests: Adapt tests to support service `reload` instead of `restart` * chore: Additional logging for debugging change event logs * fix: Wait on change detection, then verify directory created Change detection is too fast now (0-1 seconds vs 5+). Directory being waited on here was created near the end of a change event, reducing that time to detect a change by the utility method further. We can instead check that the directory exists after the change detection event is completed. * chore: Keep using the maildir polling check We don't presently use remote storage in tests, but it might be relevant in future when testing NFS. This at least avoids any confusing failure happening when that scenario is tested.
2022-12-23 12:57:24 +00:00
_log_with_date 'trace' 'Regenerating vhosts (Postfix)'
fix: `check-for-changes.sh` should not fall out of sync with shared logic (#2260) Removes duplicate logic from `check-for-changes.sh` that is used/maintained elsewhere to avoid risk of problems, as this code is already starting to diverge / rot. --- Previously the change detection support has had code added for rebuilding config upon change detection which is the same as code run during startup scripts. Unfortunately over time this has fallen out of sync. Mostly the startup scripts would get maintenance and the contributor and reviewers may not have been aware of the duplicate code handled by `check-for-changes.sh`. That code was starting to diverge in addition to some changes in structure (_eg: relay host logic seems interleaved here vs separated out in startup scripts_). I wanted to address this before it risks becoming a much bigger headache. Rather than bloat `helper-functions.sh` further, I've added a `helpers/` folder extracting relevant common logic between startup scripts and `changedetector`. If you want to follow that process I've kept scoped commits to make those diffs easier. Some minor changes/improvements were added but nothing significant. --- - chore: Extract relay host logic to new `relay.sh` helper - chore: Extract `/etc/postfix/sasl_passwd` logic to new `sasl.sh` helper - chore: Extract `postfix-accounts.cf` logic to new `accounts.sh` helper - chore: Extract `/etc/aliases` logic to new `aliases.sh` helper - chore: Extract `/etc/postfix/vhost` logic to new `postfix.sh` helper - chore: Add inline docs for Postfix configs > These are possibly more verbose than needed and can be reduced at a later stage. > They are helpful during this refactor process while investigating that everything is handled correctly. `accounts.sh`: - Add note regarding potential bug for bare domain setups with `/etc/postfix/vhost` and `mydestination` sharing same domain value. `relay.sh`: - Remove the tabs for a single space delimiter, revised associated comment. - Add PR reference for original `_populate_relayhost_map` implementation which has some useful details. Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> Co-authored-by: Casper <casperklein@users.noreply.github.com>
2021-11-20 20:33:49 +00:00
_create_postfix_vhost
fi
# Legacy workaround handled here, only seems necessary for _create_accounts:
# - `helpers/accounts.sh` logic creates folders/files with wrong ownership.
_chown_var_mail_if_necessary
}
2023-05-25 23:01:41 +00:00
function _ssl_changes() {
local REGEX_NEVER_MATCH='(?\!)'
# _setup_ssl is required for:
# manual - copy to internal DMS_TLS_PATH (/etc/dms/tls) that Postfix and Dovecot are configured to use.
# acme.json - presently uses /etc/letsencrypt/live/<FQDN> instead of DMS_TLS_PATH,
# path may change requiring Postfix/Dovecot config update.
2023-05-24 07:06:59 +00:00
if [[ ${SSL_TYPE} == 'manual' ]]; then
# only run the SSL setup again if certificates have really changed.
if [[ ${CHANGED} =~ ${SSL_CERT_PATH:-${REGEX_NEVER_MATCH}} ]] \
|| [[ ${CHANGED} =~ ${SSL_KEY_PATH:-${REGEX_NEVER_MATCH}} ]] \
|| [[ ${CHANGED} =~ ${SSL_ALT_CERT_PATH:-${REGEX_NEVER_MATCH}} ]] \
|| [[ ${CHANGED} =~ ${SSL_ALT_KEY_PATH:-${REGEX_NEVER_MATCH}} ]]
then
_log_with_date 'debug' 'Manual certificates have changed - extracting certificates'
_setup_ssl
fi
# `acme.json` is only relevant to Traefik, and is where it stores the certificates it manages.
# When a change is detected it's assumed to be a possible cert renewal that needs to be
# extracted for `docker-mailserver` services to adjust to.
2023-05-24 07:06:59 +00:00
elif [[ ${CHANGED} =~ /etc/letsencrypt/acme.json ]]; then
_log_with_date 'debug' "'/etc/letsencrypt/acme.json' has changed - extracting certificates"
_setup_ssl
# Prevent an unnecessary change detection from the newly extracted cert files by updating their hashes in advance:
local CERT_DOMAIN ACME_CERT_DIR
CERT_DOMAIN=$(_find_letsencrypt_domain)
ACME_CERT_DIR="/etc/letsencrypt/live/${CERT_DOMAIN}"
sed -i "\|${ACME_CERT_DIR}|d" "${CHKSUM_FILE}.new"
sha512sum "${ACME_CERT_DIR}"/*.pem >> "${CHKSUM_FILE}.new"
fi
# If monitored certificate files in /etc/letsencrypt/live have changed and no `acme.json` is in use,
# They presently have no special handling other than to trigger a change that will restart Postfix/Dovecot.
refactor: letsencrypt implicit location discovery (#2525) * chore: Extract letsencrypt logic into methods This allows other scripts to share the functionality to discover the correct letsencrypt folder from the 3 possible locations (where specific order is important). As these methods should now return a string value, the `return 1` after a panic is now dropped. * chore: Update comments The todo is resolved with this PR, `_setup_ssl` will be called by both cert conditional statements with purpose for each better documented to maintainers at the start of the logic block. * refactor: Defer most logic to helper/ssl.sh The loop is no longer required, extraction is delegated to `_setup_ssl` now. For the change event prevention, we retrieve the relevant FQDN via the new helper method, beyond that it's just indentation diff. `check-for-changes.sh` adjusted to allow locally scoped var declarations by wrapping a function. Presently no loop control flow is needed so this seems fine. Made it clear that `CHANGED` is local and `CHKSUM_FILE` is not. Panic scope doesn't require `SSL_TYPE` for context, it's clearly`letsencrypt`. * fix: Correctly match wildcard results Now that the service configs are properly updated, when the services restart they will return a cert with the SAN `DNS:*.example.test`, which is valid for `mail.example.test`, however the test function did not properly account for this in the regexp query. Resolved by truncating the left-most DNS label from FQDN and adding a third check to match a returned wildcard DNS result. Extracted out the common logic to create the regexp query and renamed the methods to communicate more clearly that they check the FQDN is supported, not necessarily explicitly listed by the cert. * tests(letsencrypt): Enable remaining tests These will now pass. Adjusted comments accordingly. Added an additional test on a fake FQDN that should still be valid to a wildcard cert (SNI validation in a proper setup would reject the connection afterwards). Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2022-04-18 10:52:50 +00:00
}
function _rspamd_changes() {
# RSPAMD_DMS_D='/tmp/docker-mailserver/rspamd'
if [[ ${CHANGED} =~ ${RSPAMD_DMS_D}/.* ]]; then
# "${RSPAMD_DMS_D}/override.d"
if [[ ${CHANGED} =~ ${RSPAMD_DMS_OVERRIDE_D}/.* ]]; then
_log_with_date 'trace' 'Rspamd - Copying configuration overrides'
rm "${RSPAMD_OVERRIDE_D}"/*
cp "${RSPAMD_DMS_OVERRIDE_D}"/* "${RSPAMD_OVERRIDE_D}"
fi
# "${RSPAMD_DMS_D}/custom-commands.conf"
if [[ ${CHANGED} =~ ${RSPAMD_DMS_CUSTOM_COMMANDS_F} ]]; then
_log_with_date 'trace' 'Rspamd - Generating new configuration from custom commands'
_rspamd_handle_user_modules_adjustments
fi
# "${RSPAMD_DMS_D}/dkim"
if [[ ${CHANGED} =~ ${RSPAMD_DMS_DKIM_D} ]]; then
_log_with_date 'trace' 'Rspamd - DKIM files updated'
fi
_log_with_date 'debug' 'Rspamd configuration has changed - restarting service'
supervisorctl restart rspamd
fi
}
2023-05-25 23:39:39 +00:00
while true; do
refactor: letsencrypt implicit location discovery (#2525) * chore: Extract letsencrypt logic into methods This allows other scripts to share the functionality to discover the correct letsencrypt folder from the 3 possible locations (where specific order is important). As these methods should now return a string value, the `return 1` after a panic is now dropped. * chore: Update comments The todo is resolved with this PR, `_setup_ssl` will be called by both cert conditional statements with purpose for each better documented to maintainers at the start of the logic block. * refactor: Defer most logic to helper/ssl.sh The loop is no longer required, extraction is delegated to `_setup_ssl` now. For the change event prevention, we retrieve the relevant FQDN via the new helper method, beyond that it's just indentation diff. `check-for-changes.sh` adjusted to allow locally scoped var declarations by wrapping a function. Presently no loop control flow is needed so this seems fine. Made it clear that `CHANGED` is local and `CHKSUM_FILE` is not. Panic scope doesn't require `SSL_TYPE` for context, it's clearly`letsencrypt`. * fix: Correctly match wildcard results Now that the service configs are properly updated, when the services restart they will return a cert with the SAN `DNS:*.example.test`, which is valid for `mail.example.test`, however the test function did not properly account for this in the regexp query. Resolved by truncating the left-most DNS label from FQDN and adding a third check to match a returned wildcard DNS result. Extracted out the common logic to create the regexp query and renamed the methods to communicate more clearly that they check the FQDN is supported, not necessarily explicitly listed by the cert. * tests(letsencrypt): Enable remaining tests These will now pass. Adjusted comments accordingly. Added an additional test on a fake FQDN that should still be valid to a wildcard cert (SNI validation in a proper setup would reject the connection afterwards). Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2022-04-18 10:52:50 +00:00
_check_for_changes
sleep 2
done
2021-02-24 16:28:59 +00:00
exit 0