Commit graph

457 commits

Author SHA1 Message Date
Brennan Kinney 0bbec09529 refactor: Parallel Tests
- `disabled_clamav_spamassassin`:
  - Just shuffling the test order around, and removing the restart test at the end which doesn't make sense.

- `postscreen`:
  - Now uses common helper for getting container IP
  - Does not appear to need the `NET_ADMIN` capability?
  - Reduced startup time for the 2nd container + additional context about it's relevance.
  - Test cases are largely the same, but refactored the `nc` alternative that properly waits it's turn. This only needs to run once. Added additional commentary and made into a generic method if needed in other tests.

- `fail2ban`:
  - Use the common container IP helper method.
  - Postscreen isn't affecting this test, it's not required to do the much slower exchange with the mail server when sending a login failure.
  - IP being passed into ENV is no longer necessary.
  - `sleep 5` in the related test cases doesn't seem necessary, can better rely on polling with timeout.
  - `sleep 10` for `setup.sh` also doesn't appear to be necessary.

- `postgrey`:
  - Reduced POSTGREY_DELAY to 3, which shaves a fair amount of wasted time while still verifying the delay works.
  - One of the checks in `main.cf` doesn't seem to need to know about the earlier spamhaus portion of the line to work, removed.
  - Better test case descriptions.
  - Improved log matching via standard method that better documents the expected triplet under test.
  - Removed a redundant whitelist file and test that didn't seem to have any relevance. Added a TODO with additional notes about a concern with these tests.
  - Reduced test time as 8 second timeouts from `-w 8` don't appear to be required, better to poll with grep instead.
  - Replaced `wc -l` commands with a new method to assert expected line count, better enabling assertions on the actual output.

- `undef_spam_subject`:
  - Split to two separate test cases, and initialize each container in their case instead of `setup_file()`, allowing for using the default `teardown()` method (and slight benefit if running in parallel).

- `permit_docker`:
  - Not a parallel test, but I realized that the repeat helper methods don't necessarily play well with `run` as the command (can cause false positive of what was successful).
2023-01-06 16:50:09 +13:00
Brennan Kinney 2ec6c4abc0 tests: Adjust parallel tests
- The usual serial to parallel test conversion to utilize the `setup.bash` common setup structure, and adding a `TEST_PREFIX` var for each test case to leverage.
- Standardize on parallel test naming conventions for variables / values.
- More consistent use of `bash -c` instead of `/bin/bash -c` or `/bin/sh -c`.
- Using the `_run_in_container` helper instead of `run docker exec ${CONTAINER_NAME}`.
- Updates tests to use the `check_if_process_is_running` helper.

---

chore: Revise inline docs for the `ssl_letsencrypt` test

- Moves the override to be in closer proximity to the `initial_setup` call, and better communicates the intent to override.
- Removes top comment block that is no longer providing value or correct information to maintainers.
- Revised `acme.json` test case inline doc comments.
2023-01-06 16:50:09 +13:00
Brennan Kinney 306592fcad tests: Adjusted files not directly related to tests
`tls.bash` helper was adapted to the new helper scripts location. The `setup.bash` helper saw a bugfix (expanding the array properly) and updates the container default config to configure for IPv4 explicitly.

The IPv4 default was added after recent Docker pushes and I saw weird IPv6 related errors in the logs.. now we're sure IPv4 is the default during tests.

Added functionality to check if a process is running:
- This change adds a helper function to check whether a program is running inside a container or not.
- This added the need for a function like `_run_in_container` but allowing for providing an explicit container name.
- Future PRs can use this helper function now to check whether a process is running or not. This was done for the tests of Fail2Ban, but can be used for other tests in the future as well.

---

chore: Restructured BATS flags in `Makefile`

The `Makefile` has seen a bit of a restructuring when it comes to flags:

1. The `MAKEFLAGS` variables is used by `make`, and allows for adding additional flags that can be used within in recursive calls (via `$(MAKE)`) too,  thus DRY approach.
2. The flags for calling BATS were adjusted. `--no-parallelize-within-files` has been added as well to ensure tests  _inside_ a single file are run sequentially.

`dms-test` prefix matching changed to expect a `_` suffix as a delimiter.

---

docs: Add a note regarding output from running tests in parallel
2023-01-06 16:50:09 +13:00
Brennan Kinney a81de22819 tests(chore): Move some serial tests into parallel sets
Additionally with the `tls.bash` helper for the `letsencrypt` tests.
2023-01-06 16:50:09 +13:00
Brennan Kinney 304747fad9
tests: Use mail.example.test as common container hostname (#2975) 2023-01-04 16:24:08 +01:00
Brennan Kinney b7bad82e07
tests(fix): wait_until_change_detection_event_completes to count (#2974) 2023-01-04 13:29:10 +01:00
Brennan Kinney 4dda5f8b1f
chore: Drop support for deprecated TLS versions (#2945)
* chore: Set `TLS_INTERMEDIATE_SUITE` to only use TLS 1.2 ciphersuites

Removes support of the following cipher suites that are only valid for TLS 1.0 + 1.1:

- `ECDHE-ECDSA-AES128-SHA`
- `ECDHE-RSA-AES128-SHA`
- `ECDHE-ECDSA-AES256-SHA`
- `ECDHE-RSA-AES256-SHA`
- `DHE-RSA-AES128-SHA`
- `DHE-RSA-AES256-SHA`

* chore: Update TLS version min and ignore settings

These are now the same as modern settings.

* fix: Remove min TLS support workaround

No longer required now that outdated TLS versions have been dropped.

* tests: Remove support for TLS 1.0 and 1.1 ciphersuites

* tests: Remove support for TLS 1.0 and 1.1 ciphersuites (Port 25)

The removed SHA1 cipher suites are still supported in TLS 1.2, thus they've been excluded for port 25 via the `SHA1` exclusion pattern in `main.cf`.
2022-12-24 02:30:43 +13:00
Brennan Kinney b58165762a
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 👍 

---

**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-24 01:57:24 +13:00
Brennan Kinney fe21fe78e2
chore: Remove legacy ENV SASL_PASSWD (#2946)
As per deprecation notice from v11.3 release notes, and a related prior PR; this ENV is to be removed.

It's no longer considered useful, and none of the tests that configured it were actually using it for relaying anything.
2022-12-23 15:30:40 +13:00
Brennan Kinney 75ee0c1145 tests(refactor): Conversion to parallel tests and use revised helpers
- Introduced `CONTAINER_NAME` and `TEST_NAME_PREFIX` as new vars for better managing test consistency (DRY).
- `CONTAINER_NAME` replaces any repeated container name with the variable. The value will differ slightly as the prior prefix (`mail_`) has been changed to `dms-test-`.
- `TEST_NAME_PREFIX` provides a prefix value for each `@test` description string.

---

chore: Add a reference template for tests
2022-11-26 14:52:42 +13:00
Brennan Kinney 32cc9d30e5 tests(refactor): common.bash helper split into two files
The current `test/test_helper/common.bash` was getting large. Setup logic has been extracted out into a new file.

`common.bash` resides in a directory  named `test_helper/`, the `test_` prefix is redundant. 

As an interim solution this provides a new approach for the updated tests, while the "old" tests can use the "old" `common.bash`. Eventually all tests should migrate to the new approach in `helper/` instead of the older `test_helper/`.

The new helper files are located under `test/helper/` (_which drops the `test_` prefix_). The new and updated helpers apply the new naming convention for ENV variables (_such as `CONTAINER_NAME` or `IMAGE_NAME`_).

---

Some refactoring occurred, including new methods like `_run_in_container()` and `_default_teardown()`.

---

I encountered a situation before in which the updated tests would fail because there were collisions of ENV names in the tests (_for example with `CONTAINER_NAME`_).
2022-11-26 14:52:42 +13:00
Brennan Kinney 835056d707 tests(chore): Use REPOSITORY_ROOT export var from Makefile
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.
2022-11-26 14:52:42 +13:00
Georg Lauterbach 59127e2b25 tests(chore): Rename test files to serial and parallel types
- `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.
2022-11-26 14:52:42 +13:00
Georg Lauterbach 0ef8d5ae2b
fix: adjust DNSBL return code interpretation (#2890) 2022-11-07 09:28:47 +01:00
Casper a930aeb18a
Remove unusual space from shebang line (#2834) 2022-10-17 10:40:09 +02:00
Georg Lauterbach 284c44faa9
build: cleaned up Makefile (#2833) 2022-10-17 08:08:04 +13:00
Casper 032ed00a17
fail2ban: enable network bans (#2818) 2022-10-15 12:01:59 +02:00
Georg Lauterbach 488b256e95
ci: misc test enhancements (#2815)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2022-10-14 09:48:28 +02:00
Brennan Kinney 70ad765a30
tests(fix): Avoid introducing CRLF into postfix-accounts.cf during setup (#2820)
Currently a change detection would be triggered and during processing, a CRLF is converted to LF, which updates the `postfix-accounts.cf` file and triggers another change event.

No need for the first approach to add an account, and it is the culprit for causing the CRLF to appear.
2022-10-09 19:43:39 +02:00
Casper ff969509f8
ci: fix/improve Fail2Ban tests (#2808) 2022-10-05 12:19:49 +02:00
Casper c52211b950
tests: Adding three more seconds to wait for health check (#2810) 2022-10-02 23:25:09 +02:00
Georg Lauterbach 32c508aa2a
ci: enhance build process (#2755)
This new script is a clean way of handling the installation of packages.
I think the huge `RUN` command in `Dockerfile` was hard to read and
maintain.

Using a script is a non-issue, as the image is rebuilt whenever the
script is touched.

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2022-09-21 09:31:28 +02:00
Casper b906d1ba6b
tests: Wait at least 30 seconds before checking the health state of the container (#2777) 2022-09-19 12:54:33 +02:00
Casper 1f137f3eca
Only calculate checksums, when there are files to monitor. (#2776) 2022-09-19 10:45:40 +02:00
Casper efed7d9e44
Remove unnecessary postconf switch '-e' and use single quotes where possible (#2746) 2022-08-29 13:26:44 +02:00
Brennan Kinney 672e9cf19a
tests: Ensure excessive FD limits are avoided (#2730)
* tests: Ensure excessive FD limits are avoided

Processes that run as daemons (`postsrsd` and `fail2ban-server`) initialize by closing all FDs (File Descriptors).

This behaviour queries that maximum limit and iterates through the entire range even if only a few FDs are open. In some environments (Docker, limit configured by distro) this can be a range exceeding 1 billion (from kernel default of 1024 soft, 4096 hard), causing an 8 minute delay with heavy CPU activity.

`postsrsd` has since been updated to use `close_range()` syscall, and `fail2ban` will now iterate through `/proc/self/fd` (open FDs) which should resolve the performance hit. Until those updates reach our Docker image, we need to workaround it with `--ulimit` option.

NOTE: If `docker.service` on a distro sets `LimitNOFILE=` to approx 1 million or lower, it should not be an issue. On distros such as Fedora 36, it is `LimitNOFILE=infinity` (approx 1 billion) that causes excessive delays.

* chore: Use Docker host limits instead

Typically on modern distros with systemd, this should equate to 1024 (soft) and 512K (hard) limits. A distro may override the built-in global defaults systemd sets via setting `DefaultLimitNOFILE=` in `/etc/systemd/user.conf` and `/etc/systemd/system.conf`.

* tests(fix): Better prevent non-deterministic failures

- `no_containers.bats` tests the external script `setup.sh` (without `-c`). It's expected that no existing DMS container is running  - otherwise it may attempt to use that container and fail. Detect this and fail early via `setup_file()` step.

- `mail_hostname.bats` had a odd timing failure with teardown due to the last tests bringing the containers down earlier (`docker stop` paired with the `docker run --rm`). Adding a moment of delay via `sleep` helps avoid that false positive scenario.
2022-08-23 11:24:23 +12:00
Brennan Kinney 75a75bfae6
chore: addmailuser - Remove delaying completion until /var/mail is ready (#2729)
## 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).
2022-08-23 10:15:06 +12:00
Georg Lauterbach 8a4329ae9f
scripts: revised linting script (#2737)
The new version uses our `log.sh` helper to simplify logging
significantly. Moreover, the script was adjusted to the current style
and the GitHub workflow was streamlined. The workflow is ot providing
the version anymore (which was useless anyway), and has been compacted.
2022-08-22 16:22:46 +02:00
Georg Lauterbach ab55343d8e
scripts: rework environment variables setup (#2716)
* outsourcing env variable setup

This commit contains major parts of the work of refactoring the setup
and usage of environment variables. It outsources the setup into its own
script and provides dedicated functions to be executed at a later point in time.

A **new** env variable was added: `USER_PROVISIONG` which provides a
better way of defining which method / protocol to use when it comes to
setting up users. This way, the `ENABLE_LDAP` variable is deprecated,
but all of this is backwards compatible due to a "compatibility layer", a function provided by the new variables script.

This is not a breaking change. It mostly refators internal scripts. The
only change facing the user-side is the deprecation of `ENABLE_LDAP`. We
can prolong the period of deprecation for this variable as long as we
want, because the new function that ensures backwards compatibility
provides a clean interface for the future.

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
2022-08-22 08:31:32 +02:00
Brennan Kinney 0d5f550bdf
tests: Update submodules for bats (#2715)
* tests: Update testing submodules (bats-assert, bats-support)
These two submodules were migrated to the `bats-core` organization, where they continued to receive updates.

* tests: Use tagged release of `bats-core/bats-support`
This is technically one commit backwards, but no relevant difference has been made since, other than moving the submodule to the `bats-core` organization.

* tests: Bump `bats-assert` to August 2022 (master)
No official release tag since Nov 2018, but a fair amount of changes since then.

* tests: Bump `bats-core` to `v1.7.0` release

* tests(fix): Correctly use assertions
Some tests were updated as the upgrade of bats submodules had `assert` methods raise awareness of incorrect usage.
This additionally revealed some existing tests that weren't meant to be using `run`, which swallowed failures from surfacing.
2022-08-12 11:09:17 +12:00
Brennan Kinney 57aeb6db2a
refactor: CLI commands for database management (#2654)
See the associated PR for more detailed commentary on specific changes.

### Commands refactored:
- User (**All:** add / list / update / del + _dovecot-master variants_)
- Quota (**All:** set / del)
- Virtual Alias (**All:** add / list /del)
- Relay (**All:** add-relayhost / add-sasl / exclude-domain)

### Overall changes involve:
- **Fairly common structure:**
  - `_main` method at the top provides an overview of logical steps:
    - After all methods are declared beneath it (_and imported from the new `helpers/database/db.sh`_), the `_main` is called at the bottom of the file.
    - `delmailuser` additionally processes option support for `-y` prior to calling `_main`.
  - `__usage` is now consistent with each of these commands, along with the `help` command.
  - Most logic delegated to new helper scripts. Some duplicate content remains on the basis that it's low-risk to maintenance and avoids less hassle to jump between files to check a single line, usually this is arg validation.
  - Error handling should be more consistent, along with var names (_no more `USER`/`EMAIL`/`FULL_EMAIL` to refer to the same expected value_).
- **Three new management scripts** (in `helpers/database/manage/`) using a common structure for managing changes to their respective "Database" config file.
  - `postfix-accounts.sh` unified not only add and update commands, but also all the dovecot-master versions, a single password call for all 4 of them, with a 5th consumer of the password prompt from the relay command `addsaslpassword`.
  - These scripts delegate actual writes to `helpers/database/db.sh` which provides a common API to support the changes made.
     - This is more verbose/complex vs the current inline operations each command currently has, as it provides generic support instead of slightly different variations being maintained, along with handling some edge cases that existed and would lead to bugs (notably substring matches).
     - Centralizing changes here seems wiser than scattered about. I've tried to make it easy to grok, hopefully it's not worse than the current situation.
     - List operations were kept in their respective commands, `db.sh` is only really managing writes. I didn't see a nice way for removing the code duplication for list commands as the duplication was fairly minimal, especially for `listalias` and `listdovecotmasteruser` which were quite simple in their differences in the loop body.
     - `listmailuser` and `delmailuser` also retain methods exclusive to respective commands, I wasn't sure if there was any benefit to move those, but they were refactored.
2022-07-29 12:10:23 +12:00
Ikko Ashimine b671329de5
tests: Fix typo in common.bash (#2665)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2022-06-28 13:36:57 +02:00
Andreas Perhab 68477e9047
fix: typo in changedetector ready message (#2663) 2022-06-28 11:02:43 +02:00
Brennan Kinney 4b37cb5383
tests: Extract setup.sh cases to their own test file (#2629)
* chore: Create bare new test file `setup-cli.bats`

Bare minimum to setup a new test.

* chore: Transfer over relevant tests

* chore: `mail` container name to dynamic `${TEST_NAME}`

Only applied where it's relevant. Next commit will handle the config path correction.

* chore: Use `TEST_TMP_CONFIG` for referencing local config directory

Could technically use the existing function call. Some paths were using a hard-coded config location.

Both have been converted to `TEST_TMP_CONFIG` and related `grep` calls normalizing the quote mark usage, escaping doesn't seem necessary.

* tests(fix): Create container without providing extra args reference var

If a variable name (of an array) was not provided to reference, this would fail trying to reference `'`.
2022-06-07 22:07:09 +00:00
Casper 72650d4dc3
Add basic container healthcheck (#2625) 2022-06-07 11:54:58 +02:00
Brennan Kinney 62fdcb05f5
chore: Remove redundant capability SYS_PTRACE (#2624)
* chore: Remove `SYS_PTRACE` capability from docs and configs
* chore: Remove `SYS_PTRACE` capability from tests

Doesn't seem to be required. It was originally added when the original change detection feature PR apparently needed it to function.
2022-06-07 01:20:13 +12:00
Brennan Kinney 40e2d88482
chore: Merge helpers/sasl.sh into helpers/relay.sh (#2605)
This helper was to support an earlier ENV for SASL auth support. When extracting logic into individual helpers, it was assumed this was separate from relay support, which it appears was not the case.

---

The `SASL_PASSWD` ENV is specified in tests but no longer used. There is no `external-domain.com` relay configured or tested against anywhere in the project.

The ENV was likely used in tests prior to improved relay support that allowed for adding more than a single set of relay credentials.

---

It likewise has no real relevance anywhere else outside of `relay.sh` as it's the only portion of code to operate with it.

It's only relevant for SASL auth as an SMTP client, not the SMTP server (`smtpd`) SASL support that is delegated to Dovecot. Functionality has been completely migrated into `relay.sh` as a result.

Documentation is poor for this ENV, it is unlikely in wide use? Should consider for removal.

---

The ENV has been dependent upon `RELAY_HOST` to actually enable postfix to use `/etc/postfix/sasl_passwd`, thus not likely relevant in existing setups?

---

Migrate `/etc/postfix/sasl_passwd` check from `tests.bats` as it belongs to relay tests.
2022-06-06 10:59:42 +12:00
Brennan Kinney 3d6e7a7bb8
service(postfix): Better handling of the compatibility_level setting (#2597)
* chore: Fix typo

* chore: Apply explicit chroot default for `sender-cleanup`

The implicit default is set to `y` as a compatibility fallback, but otherwise it is [advised to set to `n` going forward](http://www.postfix.org/COMPATIBILITY_README.html#chroot).

Test was changed to catch any backwards-compatibility logs, not just those for `chroot=y`. `using` added as a prefix to avoid catching log message whenever a setting is changed that the default compatibility level is active.

* chore: Set `compatibility_level` in `main.cf`

We retain the level`2` value previously set via scripts. This avoids log noise that isn't helpful.

Applied review feedback to give maintainers some context with this setting and why we have it presently set to `2`.
2022-06-05 12:10:20 +12:00
Brennan Kinney c862e1451d
chore(housekeeping): Create helpers/change-detection.sh (#2610)
* chore: Extract change-detection method to it's own helper

This doesn't really belong in `helpers/ssl.sh`. Moving to it's own helper script.

* chore: Co-locate related change-detection method from container startup

It seems relevant to migrate the related support during startup for the change detection feature into this helper.

I opted to move the call from `start-mailserver.sh` into the `_setup` call at the end for a more explicit/visible location.

* chore: Move `CHKSUM_FILE` into `helpers/change-detection.sh`

It belongs there, not in `helpers/index.sh`.

* chore: Revise inline documentation

* tests(fix): Ensure correct functionality

Presently `test/test_helper.bats` is using it's own  `CHKSUM_FILE` instead of sourcing the var for the filepath.

`test_helper/common.bash` was calling a method to check for changes, but this helper may not correctly detect letsencrypt related changes as these are not ENV rely on, but global vars handled by `helpers/dns.sh`, so that should be run first like it is for `check-for-changes.sh`.

* tests(chore): Use `CHKSUM_FILE` var from helper

* chore: `addmailuser` should use `CHKSUM_FILE` var

* chore: Update `check-for-changes.sh` log message with correct path
2022-06-05 11:59:54 +12:00
Brennan Kinney 3b4f44e837
tests(fix): Adjust for local testing conditions (#2606)
* tests(fix): Increase some timeouts

Running tests locally via a VM these tests would fail sometimes due to the time from being queued and Amavis actually processing being roughly around 30 seconds.

There should be no harm in raising this to 60 seconds, other than delaying a failure case which will ripple through other time sensitive tests.

It's better to pass when functionality is actually correct but just needs a bit longer to complete.



* tests(fix): Don't setup an invalid hostname

During container startup `helpers/dns.sh` would panic with `hostname -f` failing.

Dropping `--domainname` for this container is fine and does not affect the point of it's test.

---

It's unclear why this does not occur in CI. Possibly changes within the docker daemon since as CI runs docker on Ubuntu 20.04? (2020).

For clarity, this may be equivalent to setting a hostname of `domain.com.domain.com`, or `--hostname` value truncated the NIS domain (`--domainname`) of the same value.

IIRC, it would still fail with both options using different values if `--hostname` was multi-label. I believe I've documented how non-deterministic these options can be across different environments.

`--hostname` should be preferred. There doesn't seem to be any reason to actually need `--domainname` (which is NIS domain name, unrelated to the DNS domain name). We still need to properly investigate reworking our ENV support that `dns.sh` manages.

---

Containers were also not removing themselves after failures either (missing teardown). Which would cause problems when running tests again.



* chore: Normalize white-space

Sets a consistent indent size of 2 spaces. Previously this varied a fair bit, sometimes with tabs or mixed tabs and spaces.

Some formatting with blank lines.

Easier to review with white-space in diff ignored. Some minor edits besides blank lines, but no change in functionality.



* fix: `setup.sh` target container under test

Some of the `setup.sh` commands did not specify the container which was problematic if another `docker-mailserver` container was running, causing test failures.

This probably doesn't help with `test/no_container.bats`, but at least prevents `test/tests.bats` failing at this point.
2022-05-30 12:53:30 +12:00
Andrew Cornford 2f3cbfc144
feat: Support for Dovecot master accounts (#2535)
Dovecot master accounts can now be configured in DMS via `setup.sh`.
A master account is useful for administration purposes, or to perform mailbox backups of every user account over IMAP.
Upstream Docs: https://doc.dovecot.org/configuration_manual/authentication/master_users/

Co-authored-by: Casper <casperklein@users.noreply.github.com>
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2022-05-08 00:28:32 +02:00
Casper 2977cb6962
Streamline 'printf' usage and consequently use a format string (#2564) 2022-05-05 17:48:21 +02:00
Casper 18acd7860b
Run sedfile tests in container (#2569) 2022-05-05 12:58:00 +02:00
Casper 628e902233
Remove unnecessary quotes from command substitutions (#2561) 2022-05-05 10:28:38 +02:00
Casper de61d42e68
Add ban feature to fail2ban script (#2538) 2022-04-19 08:44:51 +00:00
Casper ee0c088b1f
setup.sh/setup: show usage when no argument is given (#2540) 2022-04-18 15:28:12 +02:00
Brennan Kinney 1b1877f025
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 22:52:50 +12:00
Georg Lauterbach a1726dc45a
scripts: added TZ environment variable to set timezone (#2530) 2022-04-06 16:48:41 +02:00
Georg Lauterbach a9305a073f
firewall: replace iptables with nftables (#2505)
* first adjustments to use Fail2Ban with nftables

* replace `iptables` -> `nftables` and adjust tests

nftables lists IPs a bit differently , so the order was adjusted for the
tests to be more flexible.

* line correction in mailserver.env

* change from `.conf` -> `.local` and remove redundant config

* revert HEREDOC to `echo`

Co-authored-by: Casper <casperklein@users.noreply.github.com>
2022-04-05 15:13:59 +02:00
Georg Lauterbach 21c218ac68
scripts: remove DMS_DEBUG (#2523)
* remove DMS_DEBUG from tests
* remove DMS_DEBUG from doc and scripts
* updated issue template
* re-add description about removal of DMS_DEBUG
2022-04-03 13:29:10 +02:00