improvement: get rid of subshell + exec in helper-functions.sh (#2401)

* get rid of subshell + exec

The new way of executing `sha512sum` should work as well as the old way
but without the clutter and possible problems the usage of subshells +
exec incurs.

Moreover, there was a misconception about array expansion. Using `""`
around an expanding array (`${ARRAY[@]}`) is quite fine (and actually
the preffered way), not because it makes the expansion _one_ string
(this would be `${ARRAY[*]}`), but it makes sure when elements are
expanded, each element has `""` around them so to speak, i.e. there is
no re-splitting of these elements.

* removed old concerns in comments

* increase test and check for changes sleep duration
This commit is contained in:
Georg Lauterbach 2022-02-08 23:21:45 +01:00 committed by GitHub
parent 7b21db77cc
commit ede2b2394a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 19 additions and 24 deletions

View file

@ -50,6 +50,7 @@ do
# get chksum and check it, no need to lock config yet # get chksum and check it, no need to lock config yet
_monitored_files_checksums >"${CHKSUM_FILE}.new" _monitored_files_checksums >"${CHKSUM_FILE}.new"
cmp --silent -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new" cmp --silent -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new"
# cmp return codes # cmp return codes
# 0 files are identical # 0 files are identical
# 1 files differ # 1 files differ
@ -60,13 +61,7 @@ do
create_lock # Shared config safety lock create_lock # Shared config safety lock
CHANGED=$(grep -Fxvf "${CHKSUM_FILE}" "${CHKSUM_FILE}.new" | sed 's/^[^ ]\+ //') CHANGED=$(grep -Fxvf "${CHKSUM_FILE}" "${CHKSUM_FILE}.new" | sed 's/^[^ ]\+ //')
# Bug alert! This overwrites the alias set by start-mailserver.sh # TODO Perform updates below conditionally too
# Take care that changes in one script are propagated to the other
# ! NEEDS FIX -----------------------------------------
# TODO FIX --------------------------------------------
# ! NEEDS EXTENSIONS ----------------------------------
# TODO Perform updates below conditionally too --------
# Also note that changes are performed in place and are not atomic # 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 # We should fix that and write to temporary files, stop, swap and start
@ -137,7 +132,7 @@ do
# mark changes as applied # mark changes as applied
mv "${CHKSUM_FILE}.new" "${CHKSUM_FILE}" mv "${CHKSUM_FILE}.new" "${CHKSUM_FILE}"
sleep 1 sleep 2
done done
exit 0 exit 0

View file

@ -225,33 +225,33 @@ export -f _notify
CHKSUM_FILE=/tmp/docker-mailserver-config-chksum CHKSUM_FILE=/tmp/docker-mailserver-config-chksum
# Compute checksums of monitored files, # Compute checksums of monitored files,
# returned output is lines of hashed content + filepath pairs. # returned output on `stdout`: hash + filepath tuple on each line
function _monitored_files_checksums function _monitored_files_checksums
{ {
# If a wildcard path pattern (or an empty ENV) would yield an invalid path # If a wildcard path pattern (or an empty ENV) would yield an invalid path
# or no results, `shopt -s nullglob` prevents it from being added. # or no results, `shopt -s nullglob` prevents it from being added.
shopt -s nullglob shopt -s nullglob
declare -a CERT_FILES
# React to any cert changes within the following letsencrypt locations: # React to any cert changes within the following letsencrypt locations:
local CERT_FILES=( CERT_FILES=(
/etc/letsencrypt/live/"${SSL_DOMAIN}"/*.pem /etc/letsencrypt/live/"${SSL_DOMAIN}"/*.pem
/etc/letsencrypt/live/"${HOSTNAME}"/*.pem /etc/letsencrypt/live/"${HOSTNAME}"/*.pem
/etc/letsencrypt/live/"${DOMAINNAME}"/*.pem /etc/letsencrypt/live/"${DOMAINNAME}"/*.pem
) )
# CERT_FILES should expand to separate paths, not a single string; if [[ ! -d /tmp/docker-mailserver ]]
# otherwise fails to generate checksums for these file paths. then
#shellcheck disable=SC2068 return 1
( fi
cd /tmp/docker-mailserver || exit 1
exec sha512sum 2>/dev/null -- \ sha512sum 2>/dev/null -- \
postfix-accounts.cf \ /tmp/docker-mailserver/postfix-accounts.cf \
postfix-virtual.cf \ /tmp/docker-mailserver/postfix-virtual.cf \
postfix-aliases.cf \ /tmp/docker-mailserver/postfix-aliases.cf \
dovecot-quotas.cf \ /tmp/docker-mailserver/dovecot-quotas.cf \
/etc/letsencrypt/acme.json \ /etc/letsencrypt/acme.json \
${CERT_FILES[@]} "${CERT_FILES[@]}"
)
} }
export -f _monitored_files_checksums export -f _monitored_files_checksums

View file

@ -48,7 +48,7 @@ function teardown_file() {
@test "checking changedetector: can detect changes & between two containers using same config" { @test "checking changedetector: can detect changes & between two containers using same config" {
echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf" echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf"
sleep 15 sleep 25
run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail -3000 changedetector" run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail -3000 changedetector"
assert_output --partial "postfix: stopped" assert_output --partial "postfix: stopped"
assert_output --partial "postfix: started" assert_output --partial "postfix: started"