* chore: Drop management of `SASLAUTHD_*` ENV
- `variables-stack.sh` does not need to manage all these extra ENV or store them. They're not used anywhere else.
- `saslauthd.sh` is the only consumer of these ENV which are effectively direct key/value mappings, with some defaults provided / inherited.
Instead of trying to conditionally support key/value pairs when ENV is set, we could instead use `sed` to delete lines with empty values.
* chore: Drop fallbacks + update configs to match docs
- Drop deprecated support:
- `DOVECOT_HOSTS` is an ENV deprecated since v10.
- Fallback for missing URI scheme introduced for Dovecot and SASLAuthd in v10.
- Adding error log message when no LDAP URI scheme is detected for the supported ENV (when set).
- Docs updated for ENV to reflect the mandatory requirement. `mailserver.env` partially synced equivalent sections.
- Provided base LDAP configs (for overriding) likewise updated from `domain.com` to `example.com`.
- LDAP test updated for required `ldap://` URI scheme. Common ENV shared across LDAP configs hoisted out of the Postfix group.
* chore: Remove unset lines in generated `saslauthd.conf`
* chore: Use white-space in query filters to improve readability
* tests: LDAP ENV query filters documented
- These filters remain roughly the same as they were before. The conditions are the same, but restructured to make the complimentary constraints more separated from the actual target attribtues.
- The DOMAIN query additionally includes the `mailAlias` from `PostfixBookMailAccount` class as well.
- Heavy inline documentation breaking down how the filters work for any maintainer to reference. This will likely be migrated after revision into our user docs for LDAP. Some quirks have also been noted with advice for future changes.
* tests: LDAP - Support test-case specific containers
A bit more complicated than other test files due to the larger ENV config array that most containers may need to share.
Example introduced with the test-case checking custom config file support.
* tests: Adjust LDAP test configs
- Paths for `.ldif` files used with volumes shortened
- Postfix LDAP `.cf` files adjusted to conventions used in LDAP tests.
- The `uniqueIdentifier` attribute is not appropriate and was relying on `objectClass: extensibleObject` as a workaround to allow it. A more appropriate attribute to use instead is `userID` (_short name: `uid`_).
- Removing `extensibleObject` now requires switching the user accounts to use `inetOrgPerson` class (_which inherits from `organizationalPerson`_). which allows the attributes `givenName`, `userID` and `mail` (_also provided via the `PostfixBookMailAccount` class_).
- The LDAP root object now uses `dc` attributes for `example.test` instead of `localhost.localdomain`. This has nothing to do with DMS or LDAP containers networking config, nor the users mail addresses.
- Users are now grouped under the organizational unit of `users` instead of `people`. Purely a naming change out of preference, no functional difference.
The LDAP test ENV has been updated to accommodate the above changes. An additional ENV override was required for SASLAuthd to switch an attribute set for `ldap_filter` in `/etc/saslauthd.conf` from the implicit default of `uniqueIdentifier` (_that we set during startup as an ENV default for fallback_) to the `userID` attribute.
**TL;DR:**
- New image is actively maintained vs existing one that is over 5 years old.
- Slight improvement to LDAP tree config via `.ldif` files.
- No more `Dockerfile` required to build, we can just rely on `docker run`.
`osixia/openldap` has not seen any activity since Feb 2021, while our `Dockerfile` was fixed to v1.1.6` (Feb 2018).
Startup time for this new image is around 5 seconds? (_The LDAP test uses a standard 20 second timeout check to wait until the server is ready before continuing with starting the DMS image_).
This commit migrates to `bitnami/openldap` which required modifying the `01_mail-tree.ldif` to also include adding the root object to start successfully. This image is actively maintained and one of the most popular OpenLDAP images on DockerHub.
The user account `.ldif` files have minimal changes:
- Lines moved around for better organization
- Additional comments for context
- Removal of inherited `objectClass` attributes (`person`, `top`) from the `orgnizationalPerson` class. Attribute `sn` changed to long form `surname` and values corrected with `givenName`. `changetype: add` was also not necessary.
Additionally the image does not support the `.schema` format, they must be converted to `.ldif` which has been done for `postfix-book.schema`.
See PR for more details.
* tests: Switch to setup helper and conventions
* tests: Adapt run command to new conventions
- We have two helper methods with implicit `CONTAINER_NAME` reference, which is a bit more DRY and improves readability.
- `wc -l` + `assert_output 1` converted to use helper `_should_output_number_of_lines 1`
- `DOMAIN` var changed from `my-domain.com` to local testing domain `example.test`.
* tests: Refactor `setup_file()`
- Test wide ENV defined at the top
- OpenLDAP build and run logic grouped together. Added notes, network alias and tty not required.
- Extracted out special LDAP Postfix/Dovecot ENV into separate array. LDAP specific provisioning / auth ENV also included, with comments + linebreak to better group related ENV.
- Likewise additional ENV to support test cases has been extracted to a separate array with additional comments for context.
- Those two arrays are expanded back into the main `CUSTOM_SETUP_ARGUMENTS` that configure hostname and network for the DMS container.
* tests: Refactor the LDAP account table query testcase
- Covers 3 accounts to test from LDAP.
- 2 are the same query against users/aliases/groups tables in Postfix, only differing by account queried (and expected as returned result).
- 1 separate test to ensure a difference in config is supported correctly.
- Extracted repeated test logic into a helper method.
- Added additional context in comments about the creation source of these LDAP accounts and their related Postfix config / interaction. Direct reference to special case PR (since `git blame` will be less useful).
* tests: Use iteration for `grep` setting checks
More DRY approach. With a bit more helpful failure context via `assert_output` (_and only grepping the key_). Simpler to grok what's being covered.
* tests: DRY test email delivery
A bit more verbose with the new helper method. `test-email.txt` template is only used by the LDAP test, as is the `sendmail` command.
Helper will take two args to support the testcases, but at a later date should be refactored to be consistent with the `_send_email()` helper (_which presently uses `nc` that is required for plain-text mail/auth, otherwise we'd have used `openssl`, bigger refactor required_).
* tests: Slight revisions and relocating testcases
- Dovecot quota plugin testcase revised to check files exist instead of rely on `ls` failure.
- Moved Postfix quota plugin testcase into prior dovecot testcase for quota plugin check. Better error output by only querying the `smtpd_recipient_restrictions` setting (_which should be the only one configured for the service_).
- Moved the saslauthd and pflogsumm testcases (_no changes beyond revised comments_) above the `ATTENTION` comment, and one testcase below the comment that belonged to that group.
* tests: Simplify openldap `docker build` command
- `--no-cache` was creating a new image on the Docker host each time the test is run. Shouldn't be any need to build without cache.
- No need to use `pushd` + `popd`, can just provide the path context directly, and the `./Dockerfile` is an implicit default thus `-f` not required either.
Additionally removed the old `checking` prefix from testcase names.
* tests: Move LDAP specific config into `test/config/ldap/`
- No changes to any of these config files, just better isolation as not relevant to any other tests.
- Section heading in `setup_file()` added to distinguish the remainder of the function is dedicated to the DMS container setup.
- Comment providing some context about the `mv` to maintainers, this should be done after defaults are initialized but before starting up the container.
* chore: Appease the lint gods
* Apply suggestions from code review
When a new version of docker-mailserver is available the account used in this
tests also gets the postmaster notification for the new version. The mailbox
then may contain 2 mails but only one with 'This is a test mail.'.
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* postfix: remove smtpd_sasl_auth_enable global setting
* tests: disable auth on 25 port
* tests: revert ldap-smtp-auth-spoofed-sender-with-filter-exception.txt
* Skip failing test
The test seems to have been broken from the beginning.
Sadly, no LDAP maintainers can verify. Added a TODO item if ever a LDAP maintainer comes around.
* Apply PR feedback
---------
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* chore: Co-locate process checking and process restart verification
Extract the test cases for checking a process is running and properly restarts from various test files into a single one:
Core (always running):
opendkim, opendmarc, master (postfix)
ENV dependent:
amavi (amavisd-new), clamd, dovecot, fail2ban-server (fail2ban), fetchmail, postgrey, postsrsd, saslauthd
These now run off a single container with the required ENV and call a common function (the revised version in parallel test cases).
* fix(saslauthd): Quote wrap supervisor config vars
`saslauth.conf` calls `-O` option for most commands defined with an ENV that may be empty/null. This would cause the process to silently fail / die.
This doesn't happen if quote wrapping the ENV, which calls `-O` with an empty string.
Not necessary, but since one of `postgrey` ENV were quote wrapped in `supervisor-app.conf`, I've also done the same there.
* fix(postsrsd): Change supervisor `autorestart` policy to `true`
The PR that introduced the config switched from `true` to `unexpected` without any context. That prevents restart working when the process is killed. Setting to `true` instead will correctly restart the service.
* chore: Remove disabled postgrey test file
`mail_with_postgrey_disabled_by_default.bats` only checked the migrated test cases, removed as no longer serving a purpose.
* tests(refactor): Make `_should_restart_when_killed()` more reliable
The previous version did not ensure that the last checks process was actually restarted, only that it was running.
It turns out that `pkill` is only sending the signal, there can be some delay before the original process is actually killed and restarted.
This can be identified with `pgrep --older <seconds>`. First ensure the process is at a specified age, then after killing check that the process is not running that is at least that old, finally check that there is a younger process actually running.. (_could fail if a process doesn't restart, or there is a delay such as imposed by `sleep` in wrapper scripts for postfix and fail2ban_)
The helper method is not used anywhere else now, move it into this test instead. It has been refactored to accomodate the needs for `--older`, and `--list-full` provides some output that can be matched (similar for `pkill --echo`).
* test(docs): Add inline notes about processes
* chore: Compress test cases into single case with loop
Moves the list of processes into array vars to iterate through instead.
If a failure occurs, the process name is visible along with line number in `_should_restart_when_killed()` to identify what went wrong.
* chore: Handle `FETCHMAIL_PARALLEL=1` process checks as well
* tests: Add test case for disabled ENV
Additional coverage to match what other test files were doing before, ensuring that these ENV can prevent their respective service from running.
* chore: Move `clamd` enabled check to it's own test case
Not sure about this.
It reduces the time of CPU activity (sustained full load on a thread) and increase in memory usage (1GB+ loading signatures database), but as a separate test case it also adds 10 seconds without reducing the time of the test case it was extracted from.
* chore: Make `disabled` variant the 1st test case
* fix: Adjust test cases to pass when using slower wrapper scripts
* tests(refactor): `mail_fetchmail.bats` updated to new format
Additionally merges in the parallel test file.
* chore: Move `config/fetchmail.cf` into separate sub-directory
Keep out of the default base config for tests.
* chore: Change `fetchmail.cf` FQDNs to `.test` TLD
Changed the first configs remote and local user values to more clearly document what their values should represent (_and that they don't need to be a full mail address, that's just what our Dovecot is configured with for login_).
Shifted the `here` to the end of the `is` line. It's optional syntax, only intended to contrast with the remote `there` for readability.
Additionally configured imap protocol. Not tested or verified if that's correct configuration for usage with imap protocol instead. The fetchmail feature tests are currently lacking.
Added an inline doc into the fetchmail test to reference a PR about the importance of the trailing `.` in the config. Updated the partial matching to ensure it matches for that in the value as well.
* chore: Finalize `process-check-restart.bats`
Few minor adjustments. The other ENV for clamd doesn't seem to provide any benefit, trim out the noise. Added a note about why it's been split out.
Fetchmail parallel configs are matching the config file path in the process command that is returned. The `.rc` suffix is just to add further clarity to that.
Allows for using `load` with an absolute path instead of a relative one, which makes it possible to group tests into different directories.
Parallel tests differ slightly, loading the newer `helper/common.bash` and `helper/setup.bash` files instead of the older `test_helper/common.bash` which serial tests continue to use.
- `test_helper.bats` needs more work than this PR provides to be compatible with parallel tests, so must remain as a serial test for now.
- `spam_bounced.bats` had failures as a serial test, but works well converted to a parallel test in a future commit.
