1
0
mirror of https://github.com/tomav/docker-mailserver.git synced 2024-06-29 12:41:27 +02:00
Commit Graph

53 Commits

Author SHA1 Message Date
Georg Lauterbach
bf69ef248e
Postfix: add smtpd_data_restrictions = reject_unauth_pipelining (#3744)
* add `smtpd_data_restrictions = reject_unauth_pipelining`

* fix: Skip restriction if trusted

* add changelog entry

* revert change to `postfix-amavis.cf`

* Update CHANGELOG.md

---------

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2024-01-04 22:13:13 +01:00
Georg Lauterbach
9e81517fe3
tests: Use swaks instead of nc for sending mail (#3732)
See associated `CHANGELOG.md` entry for details.

---------

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2024-01-03 13:17:54 +13:00
Brennan Kinney
0889b0ff06
fix: supervisor-app.conf - Correct the log location for postgrey (#3724)
* fix: `supervisor-app.conf` - Correct `postgrey` log location

Looks like this should have been like every other service and reference a log file(s) based on program name in the supervisor log directory.

* tests: Adjust log location for `postgrey_enabled.bats`
2023-12-30 09:59:09 +13:00
Brennan Kinney
5908d9f060
tests(refactor): Dovecot quotas (#3068)
* chore: Extract out Dovecot Quota test cases into new test file

Test cases are just cut + paste, no logic changed there yet.

* chore: Rename test case descriptions

* chore: Use `setup ...` methods instead of direct calls

* chore: Adjust `_run_in_container_bash` to `_run_in_container`

Plus some additional bug fixes in the disabled test case

* tests(refactor): Revise ENV test cases for max mailbox and message sizes

* tests(refactor): Revise ENV test cases for mailbox and message limits v2

Removes the extra variables and filtering in favour of explicit values instead of matching for comparison.

- Easier at a glance to know what is actually expected.
- Additionally reworks the quota limit checks in other test cases. Using a different formatter for `doveadm` is easier to match the desired value (`Limit`).

* chore: Sync improvement from `tests.bats` master

---

NOTE: This PR has been merged to avoid additional maintenance burden without losing the improvements. It was not considered complete, but remaining tasks were not documented in the PR.
2023-12-19 14:33:38 +13:00
Georg Lauterbach
a11951e398
hotfix: solve #3665 (#3669)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2023-11-28 10:33:29 +01:00
Georg Lauterbach
f674232f71
misc: final Rspamd adjustments for v13 (#3599)
* outsource Rspamd ENVs into explicit helper

This will allow us to uniformly source the helper and get the values
from everywhere consistently. This is more than desirable since we will
be using these values not only for the Rspamd setup, but also for DKIM
management and during change-detection.

* integrate Rspamd into changedetection

We outsource one more function to reside in the helper script for Rspamd
so that we can call this function from the Rspamd setup and from the
changedetection functionality too.

* realize deprecation of old commands file for Rspamd

THIS IS A BREAKING CHANGE!

This change realizes the log message: "Using old file location now
(deprecated) - this will prevent startup in v13.0.0" Startup will now
fail.

* added '--force' option to Rspamd DKIM script

* use new helper to get ENVs for Rspamd in DKIM script

* remove the need for linking directories

This was unnecessary, as explained in
https://github.com/docker-mailserver/docker-mailserver/pull/3597#discussion_r1369413599

* Apply suggestions from code review

review by @polarathene

* apply more review feedback from @polarathene

- <https://github.com/docker-mailserver/docker-mailserver/pull/3599#discussion_r1370885519>
- <https://github.com/docker-mailserver/docker-mailserver/pull/3599#discussion_r1370904201>

* update documentation

---------

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2023-10-30 10:20:37 +01:00
Georg Lauterbach
cb62ce20e6
bugfix: change Rspamd DKIM default config location (#3597)
Instead of using `etc/rspamd/override.d/dkim_signing.conf`, we will now
be using `/tmp/docker-mailserver/rspamd/override.d/dkim_signing.conf`.
The new location is persisted (and linked again during startup) and
hence better suited.
2023-10-24 10:31:22 +02:00
Brennan Kinney
aae42fae9b
ci(fix): Normalize for .gitattributes + improve eclint coverage (#3566) 2023-10-04 12:53:32 +02:00
Brennan Kinney
e9f04cf8a7
chore: Change setup config dkim default key size to 2048 (open-dkim) (#3508)
* chore: Adjust default DKIM size (`open-dkim`) from 4096-bit to 2048-bit

4096-bit is excessive in size for DKIM key. 2048-bit is plenty.

* chore: Additional revisions to `open-dkim` command help output

- The examples use `keysize 2048`, but as that's the new default it makes sense to change that.
- Other help text was also revised.
- Last example for domains did not need to demonstrate the other options. Changed example domains to more appropriate values.

* docs: Revise DKIM docs

Primarily for the change in default key size, but does revise some text to better communicate to the user.
- While the referenced RFC advises 512-bit to 2048-bit key size, we now explicitly discourage `512-bit` as it's not secure. `1024-bit` is still likely safe for most, but `2048-bit` is a good default for those not rotating their keys.
- Adjusted the domains example to match the new `setup config dkim domain` domains example.
- Tip for changing default key size changed to "info" with added clarity of lowering security or increasing it (excessively).
- Rspamd section is minor formatting changes, with the exception of clarifying the "main domain" for the mail accounts is assumed as the DMS FQDN with any subdomain (like `mail.`) stripped away. This is not great, but a legacy issue that needs to be addressed in future.
- `docs-rspamd-override-d` ref removed, and usage replaced with equivalent ref `docs-rspamd-config-dropin`, while `docs-rspamd-config-declarative` ref was not in use and also removed.
- Revised the `<selector>.txt` DNS formatting info section to better communicate with the reader. Additionally it had mixed usage of default `mail` and custom `dkim-rsa` selectors (_file content and output_).

* docs: Sync DKIM commands help messages and update DKIM docs for LDAP

- Adopt the help options format style from the `rspamd-dkim` into `open-dkim` command. And convert `./setup.sh` to `setup`. `selector` option has been implemented. for a while now.
- Update `rspamd-dkim` examples help output to align with `open-dkim` command examples.
- Give both DKIM command tools a consistent description. The two tools differ in support for the `domain` option (_implicit domain sourcing for default account provisioner, and support for multiple domains as input_).
- DKIM docs for LDAP domain support revised to better communicate when explicit domain config is necessary.

* tests: Adjust test-cases for `setup config dkim` change

`rspamd_dkim.bats`:
- Update assert for command help output.
- Don't bother creating a DKIM key at 512-bit size.

`setup_cli.bats`:
- Update assert for command help output of the `setup config dkim` (OpenDKIM) command.

* docs: Update DKIM section for large keys to newer RFC

The linked discussion from 2021 does mention this updated RFC over the original. That removes outdated advice about `512-bit` key length support.

The discussion link is still kept to reference a comment for the reader to better understand the security strength of 2048-bit RSA keys and why larger keys are not worthwhile, especially for DKIM.

* docs: Extract out common DKIM generation command from content tabs

Should be fine to be DRY here, not specific to `open-dkim` or `rspamd` generation/support. Previously rspamd lacked support of an equivalent command in DMS.

* docs: DKIM refactoring

- Shifted out the info admonition on key size advice out of the content tabs as it's now generic information.
- Indented the 4096-bit warning into this, which is less of a concern as the default for our DKIM generation tools is consistently 2048-bit now.
- Reworked the LDAP and Rspamd multi-domain advice. To avoid causing a bad diff, these sections haven't been moved/merged yet.

* docs: Revise DKIM docs

Advice for managing domains individually with LDAP and Rspamd extracted out of the content tabs. Default domain behaviour explained with extra info about OpenDKIM + FILE provisioner sourcing extra domains implicitly.
2023-08-29 09:40:02 +12:00
H4R0
bb2038e8c6
feat: Allow marking spam as read via a sieve filter (ENV MARK_SPAM_AS_READ=1) (#3489)
* add MARK_SPAM_AS_READ environment variable

* review changes

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

* update unit test

---------

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2023-08-21 10:32:26 +12:00
Georg Lauterbach
f28fce9cc4
rspamd: disable checks for authenticated users (#3440)
Co-authored-by: Casper <casperklein@users.noreply.github.com>
Co-authored-by: William Desportes <williamdes@wdes.fr>
2023-08-08 10:43:21 +02:00
Casper
8bfe8424fc
Change 'for' style (#3368) 2023-05-26 14:00:40 +02:00
Casper
cf74127f78
change if style (#3361) 2023-05-24 09:06:59 +02:00
LucidityCrash
7af7546d88
feature: adding getmail as an alternative to fetchmail (#2803)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
Co-authored-by: Casper <casperklein@users.noreply.github.com>
2023-05-23 17:25:08 +02:00
Georg Lauterbach
abd72b6f10
ci: fix ShellCheck linting for BATS tests (#3347)
* updated `lint.sh` to lint BATS (again)
* fix linting errors
2023-05-23 14:33:58 +00:00
Georg Lauterbach
7453bc096b
Dovecot: make home dir distinct from mail dir (#3335)
* add new home dir for Dovecot

I tried changing the mail dir, but this is a _very_ disruptive change,
so I took approach 3 on
<https://doc.dovecot.org/configuration_manual/home_directories_for_virtual_users/>,
whereby the home directory is now inside the mail directory.

The MDBOX/SDBOX formats are not touched by this change. The change
itself could be considered breaking though.

* adjust Sieve tests accordingly

* Update target/dovecot/10-mail.conf

* Update target/dovecot/auth-passwdfile.inc

---------

Co-authored-by: Casper <casperklein@users.noreply.github.com>
2023-05-15 20:10:29 +02:00
Georg Lauterbach
9fd00bd6ad
Rspamd: adjust learning of ham (#3334)
* adjust learning of ham

See #3333

When moving a mail from the Junk folder to the Trash folder, the mail
previously classified as ham due to the wildcard match of `*`. Because
the syntax does not allow for negation, we can only change the behavior
in a way that mails are learned as ham when they are moved into `INBOX`
from `Junk`. This is reasonable though.

* adjust tests accordingly

* adjust docs accordingly
2023-05-13 13:59:16 +02:00
Georg Lauterbach
78b7f0cbea
scripts: improve CLAMAV_MESSAGE_SIZE_LIMIT usage (#3332)
* add sanity check for Clam size & adjusted MaxScanSize

The second part is of special importance! See
<https://askubuntu.com/a/1448525>, which explains that the maximum scan
size is important as well. We previously just set the maximum file size,
which actually is pretty insecure as we silently not scan mile bigger
than `MaxScanSize`. This is corrected now.

* add SlamAV size configuration to Rspamd
2023-05-12 16:04:41 +02:00
Georg Lauterbach
bba72daedf
scripts: add DKIM helper script for Rspamd (#3286)
Co-authored-by: Casper <casperklein@users.noreply.github.com>
2023-05-03 08:30:49 +02:00
Georg Lauterbach
cd1721334c
scripts: Rspamd stabilization pt. 2 (#3282)
* move modules adjustment file to new location

Because we link `/tmp/docker-mailserver/rspamd/override.d` to
`/etc/rspamd/override.d`, I think it makes sense to move the modules
adjustment file into `/tmp/docker-mailserver/rspamd/` as well.

I write the code in a way that it is backwards compatible for now, so
this is NOT a breaking change.

* minor improvement to `__rspamd__handle_user_modules_adjustments`

The expansion of `ARGUMENT3` is now done in a way that only adds the
whitespace in case the variable is set and not null.

* move test file structure to respect latest changes

Because we're now linking `rspamd/override.d/`, we can simplify the
setup a bit. But this requires a change in directory structure.

The current Rspamd test will be renamed to `rspamd_full.bats`, because I
plan on adding more tests in different files for different feature sets.
This is done to make this feature well-tested!

* improved and added tests to Rspamd-full

FYI: The line

```bats
_run_in_container grep 'sieve_global_extensions.*\+vnd\.dovecot\.pipe'
"${SIEVE_CONFIG_FILE}"
```

was testing a condition that should actually not be met, but when I
started working on this feature, I thought this was the correct
configuration. Adding the `assert_success` statements revealed this
wrong line.

I also added tests to check whether `override.d` is linked correctly.

* renamed: `rspamd.bats` => `rspamd_full.bats`

* added new tests for incomplete Rspamd feature set

We now test that warnings are emitted & features are disabled correctly.

* update documentation
2023-04-23 14:02:56 +02:00
Georg Lauterbach
638975922e
scripts: Rspamd stabilization pt. 1 (#3261)
* added checks whether OpenDKIM/OpenDMARC/policyd-spf are enabled
* added functions to check if VAR is 0/0 or an int

and also added tests.

I also adjusted the test file to not run in a container, because there
is no need. This also decreases test time, which, in turn, increases
maintainers' happiness.

* added more checks to Rspamd setup

I added the helpers from the previous commit to the Rspamd setup to make
the whole setup more robust, and indicate to the user that an ENV
variable's value is incorrect.

While we did not issues for this in the past, I believe it to be
worthwhile for the future.

* added canonical directory for users to place files in

This dir is canonical with DMS's optional configuration dirs, as it
lives in well-known volume mounts. Hence, users will not need to adjust
`/etc/rspamd/override.d` manually anymore, or mount a volume to this
place.

The docs explain this now, but the DKIM page needs a slight update on
this too I guess. I will follow-up here.

* misc minor improvements
* use variables for common directories
2023-04-23 12:22:54 +02:00
Georg Lauterbach
9a284150b2
Rspamd: replace reject_unknown_client_hostname with Rspamd HFILTER_HOSTNAME_UNKNOWN and make it configurable (#3248) 2023-04-11 18:51:23 +02:00
Georg Lauterbach
806d3efef9
Rspamd: add greylisting option & code refactoring (#3206) 2023-04-11 09:16:57 +02:00
Casper
f4fe5bf527
Update SA_KILL values; follow up to #3058 (#3204) 2023-03-27 01:59:43 +02:00
Georg Lauterbach
e58dd1b95b
Rspamd: more features (#3159) 2023-03-18 23:32:48 +08:00
Georg Lauterbach
5e9849d94f
rspamd: rename ENABLE_REDIS & add persistence for Redis (#3143) 2023-03-04 10:45:43 +01:00
Georg Lauterbach
f35b60042f
scripts: split setup-stack.sh (#3115) 2023-02-26 11:42:14 +01:00
Georg Lauterbach
ae05e6a7c3
tests: improve _send_email (#3105) 2023-02-24 10:44:18 +01:00
Dmitry R
199e3c7721
config: disable SMTP authentication on port 25 (#3006)
* 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>
2023-02-23 15:19:39 +01:00
Georg Lauterbach
35692a9111
ci: refactored sedfile & used _send_mail where possible (#3103)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2023-02-22 10:26:04 +01:00
Georg Lauterbach
bee9e3627d
rspamd: add feature for adjusting options with a file parsed by DMS (#3059)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2023-02-19 12:36:43 +01:00
Georg Lauterbach
40e10d755d
setup: improve Amavis setup routine (#3079)
* improve Amavis setup routine

see https://github.com/docker-mailserver/docker-mailserver/pull/3046#issuecomment-1423718811
2023-02-18 19:22:20 +01:00
Georg Lauterbach
ac1df91181
chore(Postfix): disable DNSBLs (#3069) 2023-02-11 00:24:09 +01:00
Brennan Kinney
88767f7cc8
tests(refactor): open_dkim.bats (#3060)
* tests(refactor): Make test cases for opendkim keysizes DRY

- These all do roughly the same logic that can be split into two separate methods.
- `_should_generate_dkim_key()` covers a bit more logic as it can be leveraged to handle other test cases that also perform the same logic.
- The `config/opendkim/` doesn't seem necessary for tests. Only the first few test cases here are testing against it, so we can conditionally make that available. `process_check_restart.bats` also depended on it to run OpenDKIM successfully, but this was due to the `setup-stack.sh` config defaults failing to find an "empty" file forcing `supervisord` to constantly restart the process..
- With this, there we inverse the default opendkim config, so we don't have to mount unique / empty subfolders for each test case, followed by copying over the two extra configs.

* tests(refactor): DRY up more test cases

All the remaining test cases but the last one were refactored here for a clean commit diff. The last test case will be refactored in the following commit.

Plenty of repeated logic spread across these test cases, now condensed into shared methods.

* tests(refactor): Make final test case DRY

* chore: Migrate to new testing helpers

* chore: Revise test case descriptions

* tests(refactor): Improve and simplify assertions

* tests(refactor): Use common container setup instead of `docker run`

- As the majority of test cases are only running `open-dkim` helper, we don't actually have to wait for a full container setup. So an alternative container start is called.
- Also improves assertions a bit more instead of just counting lines.
- Some test cases don't bind mount all of `/tmp/docker-mailserver` contents, thus don't raise permission errors on subsequent test runs.
- Instead of `rm -f` on some config files, have opted to mount them read-only instead, or alternatively mount an anonymous empty volume instead.
- Collapsed the first three test cases into one, thus no `setup_file()` necessary.
- Shift the `_wait_for_finished_setup_in_container()` method into `_common_container_setup()` instead since nothing else is using `_common_container_start()` yet, this allows for avoiding the wait.

* tests(refactor): Collapse dkim key size test cases into single test case

This makes these tests a bit more DRY, and enhances the raised quality issue with these tests. Now not only is the domain checked in the generated DNS dkim record, but we also verify the key size is corrected in the public and private keys via openssl.

* chore: Revise container names

* chore: Swap order of test case 1 and 2

* tests(refactor): Assert generated log output

- `__should_have_tables_trustedhosts_for_domain` shifted in each test case to just after generating the domains keys.
- Asserts `open-dkim` logs instead of just counting them.
- Added checks for domains that should not be present in a test case.
- Additional coverage and notes about the alias from vhost `@localdomain.com`
- Single assert statement with switch statement as all are using common args.

* chore: Minor changes

* tests(refactor):  Share `find` logic in helpers and tests

* tests(fix): Listing file content does not need to match line order

The order printed from local system vs CI differed causing the CI to fail. The order of lines is irrelevant so `--index` is not required.

Additionally correct the prefix of the called method to be only one `_` now that it's a `common.bash` helper method.

* chore: Collapse custom DKIM selector test into custom DKIM domain test

These cover the same test logic for the most part, the first domain could also be testing the custom selector.

`special_use_folders.bats` + `mailbox_format_dbox` can assert lines instead, removing the need for `--partial`.

* Apply suggestions from code review

Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>

* chore: Split switch statement method into wrapper methods

---------

Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
2023-02-10 00:18:06 +13:00
Georg Lauterbach
2caec2cdf6
tests: refactor POP3, IMAP (actually SASLauthd + RIMAP) & relay (#3062)
* refactor `mail_pop3.bats`
* refactor `mail_with_imap.bats`
* refactor `mail_with_relays.bats`
* moved test that that did not belong into POP3 test
* slightly clean up `no_container.bats`

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2023-02-06 23:59:01 +01:00
Brennan Kinney
7a61214aa5
tests: Migrate and combine ENV tests for *_INET_PROTOCOLS (#3052)
* tests: Migrate and combine ENV tests for `*_INET_PROTOCOLS`

These two features + tests were introduced years apart but serve the same purpose for both Postfix and Dovecot.
2023-02-02 15:31:37 +13:00
Brennan Kinney
ed6917ff22
tests: Migrate Dovecot DBox tests (sdbox + mdbox) (#3051)
* tests: Migrate Dovecot DBox tests to new testing format
* tests: Group Dovecot related tests into subfolder
2023-02-02 02:30:16 +00:00
Georg Lauterbach
e6dee0f2f1
ci: move tests than can be run in parallel now (#3038) 2023-01-30 09:19:47 +01:00
Georg Lauterbach
1a0c2a351a
rspamd: follow-up of #3016 (#3039) 2023-01-30 08:23:58 +01:00
Georg Lauterbach
f496897b09
test helpers: add functionality for sending emails (#3026)
* add functionality for filtering mail log by ID

This was not planned, but as @polarthene mentioned in
https://github.com/docker-mailserver/docker-mailserver/pull/3033#issuecomment-1407169569
, filtering the mail log by email ID would be (the only) correct
approach for the Rspamd test (to eliminate race conditions).

I asserted the currect state, and came to the conclusion that this might
(or actually is) something we want in more than one place. So I went
ahead and implemented a solution.

The solution for acquiring the ID is a bit slower because it ensures the
mail queue is empty _before_ and _after_ the mail is sent. This is the
tradeoff one has to make if they want to send multiple emails in one
test file and get their IDs.

I hope you like this approach. I will provide another PR that adjusts
our current tests to use these new functions.

* added note about our helper functions in the docs

I think our work for our custom test framework should be noted in the
docs for newcomers to better understand what they should do.

* adjust Rspamd test to use new helpers for sending

* improve filter helpers further

* add sanity check when acquiring mail ID

* re-add `refute_output` to test which should now work well
2023-01-29 14:52:38 +01:00
Brennan Kinney
ed63a6f90a
tests(refactor): Amavis spam_junk_folder.bats + spam_bounced.bats (#3036)
* tests(fix): `spam_junk_folder.bats` Wait on Amavis port to be ready

Postfix can potentially be ready before Amavis is. This caused test failures as mail was sent before Amavis was ready to process it.

Both test cases shared the same test logic, except for the expected location to deliver the spam to. Extracted into a shared test method, and moved the port conditions into there.

* tests(chore): `spam_junk_folder.bats` minor revisions

Test case descriptions, container names and test prefix are now more descriptive of what is under test here (an ENV for Amavis).

* tests(chore): Move Amavis bounce test into `spam_junk_folder.bats`

These two tests seem to be related to the same feature. Grouping them into a single test file instead.

* tests(refactor): Split shared method into smaller methods

Now it can be better shared with the bounce test case.

* tests(chore): Shift test cases to match their CONTAINER_NAME order

No changes to code, just cut + paste of the `CONTAINER3_NAME` test case to shift it to the last test case position.
2023-01-29 12:29:25 +00:00
Georg Lauterbach
555fbb78c4
feature: provide better rspamd suppport (#3016)
* added options to toggle OpenDKIM & OpenDMARC

rspamd can provide DKIM signing and DMARC checking itself, so users
should be able to disable OpenDKIM & OpenDMARC. The default is left at
1, so users have to to opt-in when the want to disable the features.

* misc small enhancements

* adjusted start of rspamd

The order of starting redis + rspamd was reversed (now correct) and
rspamd now starts with the correct user.

* adjusted rspamd core configuration

The main configuration was revised. This includes AV configuration as
well as worker/proxy/controller configuration used to control the main
rspamd processes.

The configuration is not tested extensively, but well enough that I am
confident to go forward with it until we declare rspamd support as
stable.

* update & improve the documentation

* add tests

These are some initial tests which test the most basic functionality.

* tests(refactor): Improve consistency and documentation for test helpers (#3012)

* added `ALWAYS_RUN` target `Makefile` recipies (#3013)

This ensures the recipies are always run.

Co-authored-by: georglauterbach <44545919+georglauterbach@users.noreply.github.com>

* adjusted rspamd test to refactored test helper functions

* improve documentation

* apply suggestions from code review (no. 1 by @polarthene)

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

* streamline heredoc (EOM -> EOF)

* adjust rspamd test (remove unnecessary run arguments)

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
2023-01-25 10:28:59 +01:00
Georg Lauterbach
e3c4ef76c6
tests(refactor): Improve consistency and documentation for test helpers (#3012) 2023-01-22 00:05:28 +01:00
Brennan Kinney
fb82082cf1
tests(refactor): mail_fetchmail.bats + co-locate test cases for processes (#3010)
* 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.
2023-01-18 14:42:55 +13:00
Brennan Kinney
a7e6439a39
fix: Workaround postconf write settling logic (#2998)
* fix: Workaround `postconf` write settle logic

After updating `main.cf`, to avoid an enforced delay from reading the config by postfix tools, we can ensure the modified time is at least 2 seconds in the past as a workaround. This should be ok with our usage AFAIK.

Shaves off 2+ seconds roughly off each container startup, reduces roughly 2+ minutes off tests.

* chore: Only modify `mtime` if less than 2 seconds ago

- Slight improvement by avoiding unnecessary writes with a conditional check on the util method.
- Can more comfortably call this during `postfix reload` in the change detection cycle now.
- Identified other tests that'd benefit from this, created a helper method to call instead of copy/paste.
- The `setup email restrict` command also did a modification and reload. Added util method here too.

* tests(fix): `mail_smtponly.bats` should wait for Postfix

- `postfix reload` fails if the service is not ready yet.
- `service postfix reload` and `/etc/init.d/postfix reload` presumably wait until it is ready? (as these work regardless)

* chore: Review feedback - Move reload method into utilities
2023-01-13 10:10:58 +13:00
Georg Lauterbach
41c44cb91d
update BATS & helper + minor updates to BATS variables (#2988) 2023-01-09 08:54:04 +01:00
Brennan Kinney
1024e0ccf2
tests: Extract some test cases out from tests.bats (#2980)
While working on tests, I noticed that some of the configs being mounted were adding a few seconds to the start-up time of each container. Notably `postfix-*` and `dovecot.conf` config files, which have been extracted out into their own tests with those files moved into a separate config folder.

`tests.bats` has been adapted to the common setup helper, and removed ENV no longer required to run those tests. Future PRs will extract out more tests.

Review may be easier via individual commit diffs and their associated commit messages describing relevant changes.

<details>
<summary>Commit message history for reference</summary>

```markdown
tests(chore): `tests.bats` - Remove redundant config
===
- ONEDIR volume support no longer relevant, this should have been dropped.
- ClamAV ENV no longer relevant as related tests have been extracted already.
- Same with the some of the SpamAssassin ENV config.
- `VIRUSMAILS_DELETE_DELAY` is tested in the file, but doesn't use this ENV at all? (runs a separate instance to test the ENV instead)
- Hostname updated in preparation for migrating to new test helpers. Relevant test lines referencing the hostname have likewise been updated.
```

```markdown
tests(chore): `tests.bats` - Convert to common setup
===
ENV remains the same, but required adding `ENABLE_AMAVIS=1` to bring that back, while the following became redundant as they're now defaulting to explicitly disabled in the helper method:

- `ENABLE_CLAMAV=0`
- `LOG_LEVEL=debug`
- `ENABLE_UPDATE_CHECK=0`
- `--hostname` + `--tty` + standard `--volume` lines
- `-e` option expanded to long-name `--env`, and all `\` dropped as no longer necessary.

`wait_for_finished_setup_in_container` is now redundant thanks to `common_container_setup`.
```

```markdown
tests(refactor): `tests.bats` - Extract out Dovecot Sieve tests
===
Sieve test files relocated into `test/config/dovecot-sieve/` for better isolation.

`dovecot.sieve` was not using the `reject` import, and we should not encourage it? (docs still do):
https://support.tigertech.net/sieve#the-sieve-reject-jmp
```

```markdown
tests: `tests.bats` - Extract out `checking smtp` tests
===
Migrated to the standard template and copied over the original test cases with `_run_in_container` adjustment only.

Identified minimum required ENV along with which mail is required for each test case.
```

```markdown
tests(refactor): `smtp-delivery.bats`
===
- Disabled `ENABLE_SRS=1`, not necessary for these tests.
- Added a SpamAssassin related test (X-SPAM headers) which requires `SA_TAG` to properly pass (or `ENABLE_SRS=1` to deliver into inbox).
- Many lines with double quotes changed to single quote wrapping, and moving out `grep` filters into `assert_output --partial` lines instead.
- Instead of `wc -l` making failures less helpful, switch to the helper method `_should_output_number_of_lines`
- x2 `assert_output` with different EOF style of usage was not actually failing on tests when it should. Changed to assert partial output of each expected line, and count the number of lines instead.
- Added additional comments related to the test cases with a `TODO` note about `SPAMASSASSIN_SPAM_TO_INBOX=1`.
- Revised test case names, including using the common prefix var.
- `tests.bats` no longer needs to send all these emails, no other test cases require them. This affects a test checking a `/mail` folder exists which has been corrected, and a quotas test case adjusted to expect an empty quota size output.
```

```markdown
tests: `tests.bats` - Extract out test cases for config overrides
===
Slight improvement by additionally matching `postconf` output to verify the setting is properly applied.
```

```markdown
tests: `tests.bats` - Extract out Amavis SpamAssassin test case
===
Removes the need for SpamAssassin ENV in `tests.bats`.
```

</details>
2023-01-07 11:36:20 +13:00
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
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