mirror/docker-mailserver.docker-mailserver
1
0
Fork 0
mirror of https://github.com/docker-mailserver/docker-mailserver.git synced 2025-08-04 01:55:29 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

chore: addmailuser - Remove delaying completion until /var/mail is ready (#2729)

Browse source 
## Quick Summary

Resolves a `TODO` task with `addmailuser`.

## Overview

The main change is adding three new methods in `common.bash`, which replace the completion delay in `addmailuser` / `setup email add` command.

Other than that:

- I swapped `sh -c 'addmailuser ...'` to `setup email add ...`.
- Improved three tests in `setup-cli.bats` for `setup email add|update|del` (_logic remains effectively the same still_).
- Rewrote the `TODO` comment for `setup-cli.bats` test on `setup email del` to better clarify the concern, but the test itself was no longer affected due to changes prior to this PR, so I enabled the commented out assertion.
- Removed unnecessary waits. The two `skip` tests in `test/tests.bats` could be enabled again after this PR.
- Additional fixes to tests were made during the PR (see discussion comments for details), resolving race conditions.

Individual commit messages of the PR provide additional details if helpful.

---

## Relevant commit messages

* chore: Remove creation delay in `addmailuser`

This was apparently only for supporting tests that need to wait on account creation being ready to test against.

As per the removed inline docs, it should be fine to remove once tests are updated to work correctly without it.

* tests(feat): Add two new common helper methods

`wait_until_account_maildir_exists()` provides the same logic `addmailuser` command was carrying, to wait upon the account dir creation in `/var/mail`.

As this was specifically to support tests, it makes more sense as a test method.

`add_mail_account_then_wait_until_ready()` was added to handle the common pattern of creating account and waiting on it. An internal assert will ensure the account was successfully created first during the test before attempting to wait.

* tests(feat): Add common helper for waiting on change event to be processed

The current helper is more complicated for no real benefit, it only detects when a change is made that would trigger a change event in the `changedetector` service. Our usage of this in tests however is only interested in waiting out the completion of the change event.

Remove unnecessary change event waits. These waits should not be necessary if handled correctly.

* tests: `addmailuser` to `add_mail_account_then_wait_until_ready mail()`

This helper method is used where appropriate.
- A password is not relevant (optional).
- We need to wait on the creation on the account (Dovecot and `/var/mail` directory).

* tests: `setup-cli` revise `add`, `update`, `del` tests

The delete test was failing as the `/var/mail` directory did not yet exist.

There is now a proper delay imposed in the `add` test now shares the same account for both `update` and `del` tests resolving that failure.

Additionally tests use better asserts where appropriate and the wait + sleep logic in `add` has been improved (now takes 10 seconds to complete, approx half the time than before).

The `del` test TODO while not technically addressed is no longer relevant due to the tests being switched to `-c` option (there is a separate `no container` test file, but it doesn't provide a `del` test).

* tests(fix): Ensure Postfix is reachable after waiting on ClamAV

There is not much reason to check before waiting on ClamAV.

It is more helpful to debug failures from `nc` mail send commands if we know that nothing went wrong inbetween the ClamAV wait time.

Additionally added an assertion which should provide more information if this part of the test setup fails again.

* tests(fix): Move health check to the top

This test is a bit fragile. It relies on defaults for the healthcheck with intervals of 30 seconds.

If the check occurs while Postfix is down due a change event from earlier tests and the healthcheck kicks in at that point, then if there is not enough time to refresh the health status from `unhealthy`, the test will fail with a false-positive as Postfix is actually working and up again..

* tests(fix): Wait on directory to be removed

Workaround that tries not to introduce heavier delays by waiting on a full change event to complete in the previous `email update` if possible.

There is a chance that the account has the folder deleted, but restored from an active change event (for password update, then the account delete).
This commit is contained in:
Brennan Kinney 2022-08-23 10:15:06 +12:00 • committed by GitHub
parent 8a4329ae9f
commit 75a75bfae6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 182 additions and 109 deletions
Download patch file Download diff file Expand all files Collapse all files

76
test/tests.bats
View file

@ -10,6 +10,7 @@ setup_file() {
PRIVATE_CONFIG=$(duplicate_config_for_container . mail)
mv "${PRIVATE_CONFIG}/user-patches/user-patches.sh" "${PRIVATE_CONFIG}/user-patches.sh"
# `LOG_LEVEL=debug` required for using `wait_until_change_detection_event_completes()`
docker run --rm -d --name mail \
-v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
-v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \
@ -22,6 +23,7 @@ setup_file() {
-e ENABLE_SPAMASSASSIN=1 \
-e ENABLE_SRS=1 \
-e ENABLE_UPDATE_CHECK=0 \
-e LOG_LEVEL='debug' \
-e PERMIT_DOCKER=container \
-e PERMIT_DOCKER=host \
-e PFLOGSUMM_TRIGGER=logrotate \
@ -49,16 +51,18 @@ setup_file() {
# setup sieve
docker cp "${PRIVATE_CONFIG}/sieve/dovecot.sieve" mail:/var/mail/localhost.localdomain/user1/.dovecot.sieve
# this relies on the checksum file beeing updated after all changes have been applied
wait_for_changes_to_be_detected_in_container mail
wait_for_smtp_port_in_container mail
# this relies on the checksum file being updated after all changes have been applied
wait_until_change_detection_event_completes mail
# wait for ClamAV to be fully setup or we will get errors on the log
repeat_in_container_until_success_or_timeout 60 mail test -e /var/run/clamav/clamd.ctl
# sending test mails
docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-spam.txt"
wait_for_smtp_port_in_container mail
# The first mail sent leverages an assert for better error output if a failure occurs:
run docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-spam.txt"
assert_success
docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-virus.txt"
docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/existing-alias-external.txt"
docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/existing-alias-local.txt"
@ -97,6 +101,21 @@ teardown_file() {
assert_success
}
#
# healthcheck
#
# NOTE: Healthcheck defaults an interval of 30 seconds
# If Postfix is temporarily down (eg: restart triggered by `check-for-changes.sh`),
# it may result in a false-positive `unhealthy` state.
# Be careful with re-locating this test if earlier tests could potentially fail it by
# triggering the `changedetector` service.
@test "checking container healthcheck" {
run bash -c "docker inspect mail | jq -r '.[].State.Health.Status'"
assert_output "healthy"
assert_success
}
#
# processes
#
@ -627,6 +646,8 @@ EOF
}
@test "checking accounts: user3 should have been removed from /tmp/docker-mailserver/postfix-accounts.cf but not auser3" {
wait_until_account_maildir_exists mail 'user3@domain.tld'
docker exec mail /bin/sh -c "delmailuser -y user3@domain.tld"
run docker exec mail /bin/sh -c "grep '^user3@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf"
@ -639,7 +660,7 @@ EOF
}
@test "checking user updating password for user in /tmp/docker-mailserver/postfix-accounts.cf" {
docker exec mail /bin/sh -c "addmailuser user4@domain.tld mypassword"
add_mail_account_then_wait_until_ready mail 'user4@domain.tld'
initialpass=$(docker exec mail /bin/sh -c "grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf")
sleep 2
@ -689,8 +710,7 @@ EOF
@test "checking quota: setquota user must be existing" {
run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword"
assert_success
add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld'
run docker exec mail /bin/sh -c "setquota quota_user 50M"
assert_failure
@ -703,9 +723,9 @@ EOF
run docker exec mail /bin/sh -c "delmailuser -y quota_user@domain.tld"
assert_success
}
@test "checking quota: setquota <quota> must be well formatted" {
run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword"
assert_success
add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld'
run docker exec mail /bin/sh -c "setquota quota_user@domain.tld 26GIGOTS"
assert_failure
@ -733,10 +753,8 @@ EOF
assert_success
}
@test "checking quota: delquota user must be existing" {
run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword"
assert_success
add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld'
run docker exec mail /bin/sh -c "delquota uota_user@domain.tld"
assert_failure
@ -755,9 +773,9 @@ EOF
run docker exec mail /bin/sh -c "delmailuser -y quota_user@domain.tld"
assert_success
}
@test "checking quota: delquota allow when no quota for existing user" {
run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword"
assert_success
add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld'
run docker exec mail /bin/sh -c "grep -i 'quota_user@domain.tld' /tmp/docker-mailserver/dovecot-quotas.cf"
assert_failure
@ -811,8 +829,7 @@ EOF
}
@test "checking quota: quota directive is removed when mailbox is removed" {
run docker exec mail /bin/sh -c "addmailuser quserremoved@domain.tld mypassword"
assert_success
add_mail_account_then_wait_until_ready mail 'quserremoved@domain.tld'
run docker exec mail /bin/sh -c "setquota quserremoved@domain.tld 12M"
assert_success
@ -828,16 +845,12 @@ EOF
}
@test "checking quota: dovecot applies user quota" {
wait_for_changes_to_be_detected_in_container mail
run docker exec mail /bin/sh -c "doveadm quota get -u 'user1@localhost.localdomain' | grep 'User quota STORAGE'"
assert_output --partial "- 0"
run docker exec mail /bin/sh -c "setquota user1@localhost.localdomain 50M"
assert_success
wait_for_changes_to_be_detected_in_container mail
# wait until quota has been updated
run repeat_until_success_or_timeout 20 sh -c "docker exec mail sh -c 'doveadm quota get -u user1@localhost.localdomain | grep -oP \"(User quota STORAGE\s+[0-9]+\s+)51200(.*)\"'"
assert_success
@ -845,8 +858,6 @@ EOF
run docker exec mail /bin/sh -c "delquota user1@localhost.localdomain"
assert_success
wait_for_changes_to_be_detected_in_container mail
# wait until quota has been updated
run repeat_until_success_or_timeout 20 sh -c "docker exec mail sh -c 'doveadm quota get -u user1@localhost.localdomain | grep -oP \"(User quota STORAGE\s+[0-9]+\s+)-(.*)\"'"
assert_success
@ -855,14 +866,11 @@ EOF
@test "checking quota: warn message received when quota exceeded" {
skip 'disabled as it fails randomly: https://github.com/docker-mailserver/docker-mailserver/pull/2511'
wait_for_changes_to_be_detected_in_container mail
# create user
run docker exec mail /bin/sh -c "addmailuser quotauser@otherdomain.tld mypassword && setquota quotauser@otherdomain.tld 10k"
add_mail_account_then_wait_until_ready mail 'quotauser@otherdomain.tld'
run docker exec mail /bin/sh -c 'setquota quotauser@otherdomain.tld 10k'
assert_success
wait_for_changes_to_be_detected_in_container mail
# wait until quota has been updated
run repeat_until_success_or_timeout 20 sh -c "docker exec mail sh -c 'doveadm quota get -u quotauser@otherdomain.tld | grep -oP \"(User quota STORAGE\s+[0-9]+\s+)10(.*)\"'"
assert_success
@ -967,16 +975,6 @@ EOF
assert_failure
}
#
# healthcheck
#
@test "checking container healthcheck" {
run bash -c "docker inspect mail | jq -r '.[].State.Health.Status'"
assert_output "healthy"
assert_success
}
#
# supervisor
#