scripts: improve custom user-supplied Postfix configuration

[georglauterbach]

Description

The new way of providing your custom Postfix configuration is similar to
the old way, but now, the whole file is appended at once, and then
"filtered" so Postfix is okay with it.

Users can now override the default Postfix configuration and do not have
to worry about providing the config line-by-line.

The documentation was adjusted accordingly. It emphasises how the
configuration is applied to enable users to understand what they can do
and how they can achieve that.

Note: The new way of adjusting Postfix should in theory be backwards
compatible. (Nothing is ever certain with Postfix...) I had to adjust for
a read-write-conflict with postconf -nf though, but this should be fine.

I also moved the default compatibility level into the configuration file instead of having it set in the function (where it cannot be properly overridden by the user...). see #2597

---

This should be the last PR for v11.1.0 :)

Fixes #2594

Type of change

• Improvement (non-breaking change that does improve existing functionality)
• This change requires a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (README.md or the documentation under docs/)
• If necessary I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[georglauterbach]

This PR is expected to fail until #2597 is merged (because of compatibility_level).

[reneploetz]

Is there a specific reason we cannot use the same idea for master.cf?
At first glance postconf -Mf seems to show the contents of master.cf (loosing comments and formatting too) so we could concat too, can't we?
(Note: That change would break existing setups because the format for the master.cf would change)

[georglauterbach]

I guess I was not looking at the manpage of postconf close enough, but that is a good idea @reneploetz! @polarathene what‘s your take?

[polarathene]

Is there a specific reason we cannot use the same idea for master.cf?

Yes. I looked into that too when I did my earlier attempt. We process that differently.. Rather than appending config, it's using a different syntax to update which is intended for single line updates:

docker-mailserver/test/config/postfix-master.cf

Line 1 in 0d30b92

submission/inet/smtpd_sasl_security_options=noanonymous

This is from our tests. It updates an existing parameter for a service with that service/type/parameter=value syntax, rather than appending or overriding.

I have not looked if we can get the same append + merge behaviour. (EDIT: Does not work, appended an existing service line and updated one of the parameters (-o lines), postconf -Mf did not merge it. Same with updating chroot for a service line appended.. treated as separate service)

If we could, that would be a breaking change though to any users that use the current format supported. It may be better to wait until a user requests support for extending rather than modifying master.cf, or at least queue separately for a major version release. (EDIT: Didn't read the full message, you already mentioned it'd be a breaking change due to format change 😅 )

---

This update format is described as:

With -P, edit the master.cf configuration file, and add or update one or more service parameter settings (-o parameter=value settings) with new values as specified with service/type/parameter=value

That may not cover all desired changes/updates though (there are other modes for master.cf management with postconf too).

---

@polarathene what‘s your take?

Better to leave as-is.

It doesn't seem like postconf -Mf will provide same merge of duplicates like postconf -nf does. If there is demand for appending/replacing master.cf (or main.cf) instead of modifying the one we ship in /etc/postfix, we could look into that later. For now users can use user-patches.sh if they need that.

---

Oh, we can do postfix-master.cf changes all in one postconf -P command it seems. Docs mention in -eP that one or more changes can be provided, delimited by spaces. It presumably wouldn't make much noticeable difference to bother with implementing though?

[polarathene]

Nice improvements to the docs! 😀

The config update logic LGTM! 👍

[polarathene]

The other test failure I understand will be resolved by related PR dependency, but I didn't expect this one:

not ok 93 ssl(letsencrypt): Traefik 'acme.json' (*.example.test) in 42042ms
# (from function `assert_output' in file test/test_helper/bats-assert/src/assert.bash, line 239,
#  from function `_has_matching_line' in file test/mail_ssl_letsencrypt.bats, line 203,
#  from function `_should_have_valid_config' in file test/mail_ssl_letsencrypt.bats, line 194,
#  from function `_acme_wildcard' in file test/mail_ssl_letsencrypt.bats, line 157,
#  in test file test/mail_ssl_letsencrypt.bats, line 180)
#   `_acme_wildcard' failed
# [   INF   ]  mail.example.test is up and running
# [  DEBUG  ]  2022-05-18 10:41:27  Chagedetector is ready
# 
# -- output differs --
# expected : smtpd_tls_chain_files = /etc/letsencrypt/live/example.test/key.pem /etc/letsencrypt/live/example.test/fullchain.pem
# actual   : smtpd_tls_chain_files = /etc/letsencrypt/live/example.test/key.pem /etc/letsencrypt/live/example.test/fullchain.pem /etc/letsencrypt/live/mail.example.test/fullchain.pem
# --
# 
# mail_ssl_letsencrypt
ok 94 checking ssl: ENV vars provided are valid files in 387ms
not ok 95 checking ssl: manual configuration is correct in 132ms
# (from function `assert_output' in file test/test_helper/bats-assert/src/assert.bash, line 239,
#  in test file test/mail_ssl_manual.bats, line 53)
#   `assert_output "smtpd_tls_chain_files = ${PRIMARY_KEY} ${PRIMARY_CERT} ${FALLBACK_KEY} ${FALLBACK_CERT}"' failed
# 
# -- output differs --
# expected : smtpd_tls_chain_files = /etc/dms/tls/key /etc/dms/tls/cert /etc/dms/tls/fallback_key /etc/dms/tls/fallback_cert
# actual   : smtpd_tls_chain_files = /etc/dms/tls/key /etc/dms/tls/cert
# --
#

I will re-run the tests to double-check that the changes in this PR are actually causing that.

[georglauterbach]

The other test failure I understand will be resolved by related PR dependency, but I didn't expect this one:

not ok 93 ssl(letsencrypt): Traefik 'acme.json' (*.example.test) in 42042ms
# (from function `assert_output' in file test/test_helper/bats-assert/src/assert.bash, line 239,
#  from function `_has_matching_line' in file test/mail_ssl_letsencrypt.bats, line 203,
#  from function `_should_have_valid_config' in file test/mail_ssl_letsencrypt.bats, line 194,
#  from function `_acme_wildcard' in file test/mail_ssl_letsencrypt.bats, line 157,
#  in test file test/mail_ssl_letsencrypt.bats, line 180)
#   `_acme_wildcard' failed
# [   INF   ]  mail.example.test is up and running
# [  DEBUG  ]  2022-05-18 10:41:27  Chagedetector is ready
# 
# -- output differs --
# expected : smtpd_tls_chain_files = /etc/letsencrypt/live/example.test/key.pem /etc/letsencrypt/live/example.test/fullchain.pem
# actual   : smtpd_tls_chain_files = /etc/letsencrypt/live/example.test/key.pem /etc/letsencrypt/live/example.test/fullchain.pem /etc/letsencrypt/live/mail.example.test/fullchain.pem
# --
# 
# mail_ssl_letsencrypt

I will re-run the tests to double-check that the changes in this PR are actually causing that.

The Traefik test is still failing :O

[polarathene]

The Traefik test is still failing :O

I've looked into this and identified the problem :)

The tests are not matching the expected values, which are in fact there but postconf -nf is folding at a certain value length, so missing values have been split into a separate line(s).

This occurs for quite a few settings with long values, and may be at risk of scripts that use grep/sed or similar tailored with an expectation of the value on a single line, instead of delegating read/write via postconf.

Likewise, if any scripts rely on toggling a setting or similar logic that checks for # + <setting name>, this approach may cause breakage there.. I have not looked through scripts to see where we are doing these sort of operations though.

---

Fix for now would be to not fold values for storage. But this does not rule out risk with scripts built with # in mind.

[polarathene]

I've noticed with this change that the logs get a bit of spam with postconf -nf line. This would only happen with the first occurrence and I could not repeat from manually running the commands. I could when I chained these two, so that postconf was run immediately afterwards:

cat /tmp/docker-mailserver/postfix-main.cf >>/etc/postfix/main.cf && \
  postconf -n >/tmp/postfix-main-new.cf

You'll get logs / output such as the following (blank lines added to show grouping):

[  TRACE  ]  Setting up Postfix Override configuration

postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

postconf: warning: /etc/postfix/main.cf, line 6: overriding earlier entry: readme_directory=/tmp
postconf: warning: /etc/postfix/main.cf, line 16: overriding earlier entry: recipient_delimiter=~

postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

postconf: warning: /etc/postfix/main.cf, line 6: overriding earlier entry: readme_directory=/tmp
postconf: warning: /etc/postfix/main.cf, line 16: overriding earlier entry: recipient_delimiter=~
postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

postconf: warning: /etc/postfix/main.cf, line 6: overriding earlier entry: readme_directory=/tmp
postconf: warning: /etc/postfix/main.cf, line 16: overriding earlier entry: recipient_delimiter=~
postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

postconf: warning: /etc/postfix/main.cf, line 6: overriding earlier entry: readme_directory=/tmp
postconf: warning: /etc/postfix/main.cf, line 16: overriding earlier entry: recipient_delimiter=~
postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

postconf: warning: /etc/postfix/main.cf, line 6: overriding earlier entry: readme_directory=/tmp
postconf: warning: /etc/postfix/main.cf, line 16: overriding earlier entry: recipient_delimiter=~
postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

postconf: warning: /etc/postfix/main.cf, line 6: overriding earlier entry: readme_directory=/tmp
postconf: warning: /etc/postfix/main.cf, line 16: overriding earlier entry: recipient_delimiter=~
postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

[  TRACE  ]  Loaded '/tmp/docker-mailserver/postfix-main.cf'

I'm not entirely familiar with what's going on, but I guess the cat append operation has not finished writing by the time the postconf command begins? I didn't expect that personally :|

This is running in a local VM with an XFS filesystem (virtual disk file on host internal SATA SSD) on a host OS also with XFS that is booted from a raw disk image (originally used for a VM) which is stored on XFS filesystem for an external USB SSD.

I don't know if something weird is going on I/O wise there with my setup if you're unable to reproduce. I used time on the reproduction code and it takes 1.5 seconds while logging the warnings (staggered). Individually timed with delay between both commands they take less than 50ms and only two warnings like expected are output:

postconf: warning: /etc/postfix/main.cf, line 103: overriding earlier entry: recipient_delimiter=+
postconf: warning: /etc/postfix/main.cf, line 107: overriding earlier entry: readme_directory=no

---

Issue above aside, I assume we'd want to silence those warnings being emitted regardless?

[georglauterbach]

Very interesting observation @polarathene. I am not sure what the cause is, but I can add a small delay and silence the warnings, as they are indeed not in any way useful :)

[polarathene]

Whoops that commit was meant to be a fix not a chore 😅

Hope you don't mind, it should fix the failing tests. Which perhaps should be using postconf in future to interact with main.cf?

[georglauterbach]

Whoops that commit was meant to be a fix not a chore sweat_smile

Hope you don't mind, it should fix the failing tests.

All is well :)

Which perhaps should be using postconf in future to interact with main.cf?

Yes, that may really be a good idea :)

---

But still, we have this error:

not ok 256 checking system: /var/log/mail/mail.log is error free in 803ms
# (from function `assert_failure' in file test/test_helper/bats-assert/src/assert.bash, line 140,
#  in test file test/tests.bats, line 514)
#   `assert_failure' failed
# 
# -- command succeeded, but it was expected to fail --
# output (3 lines):
#   May 30 13:06:59 mail postfix/master[1432]: /etc/postfix/master.cf: line 71: using backwards-compatible default setting chroot=y
#   May 30 13:07:12 mail postfix/master[2485]: /etc/postfix/master.cf: line 71: using backwards-compatible default setting chroot=y
#   May 30 13:07:20 mail postfix/master[3549]: /etc/postfix/master.cf: line 71: using backwards-compatible default setting chroot=y
# --
#

This is still expected, right?

[polarathene]

This is still expected, right?

Yep, that's due to depending on the other PR.

[polarathene]

Creating a two line main.cf and just using postconf to read the setting override in this same way also reproduces the problem:

$ echo -e "max_idle = 300s\nmax_idle = 600s" >/etc/postfix/main.cf && postconf max_idle

postconf: warning: /etc/postfix/main.cf, line 2: overriding earlier entry: max_idle=300s
postconf: warning: /etc/postfix/main.cf, line 1: overriding earlier entry: max_idle=600s
postconf: warning: /etc/postfix/main.cf, line 2: overriding earlier entry: max_idle=300s
postconf: warning: /etc/postfix/main.cf, line 1: overriding earlier entry: max_idle=600s
postconf: warning: /etc/postfix/main.cf, line 2: overriding earlier entry: max_idle=300s
postconf: warning: /etc/postfix/main.cf, line 1: overriding earlier entry: max_idle=600s
postconf: warning: /etc/postfix/main.cf, line 2: overriding earlier entry: max_idle=300s
postconf: warning: /etc/postfix/main.cf, line 1: overriding earlier entry: max_idle=600s
postconf: warning: /etc/postfix/main.cf, line 2: overriding earlier entry: max_idle=300s
postconf: warning: /etc/postfix/main.cf, line 1: overriding earlier entry: max_idle=600s
postconf: warning: /etc/postfix/main.cf, line 2: overriding earlier entry: max_idle=300s
max_idle = 600s

Adding sleep will reduce the warning spam, but likely isn't contributing anything useful. I would need to test in a different environment to see if that occurs outside of this VM, it may just be disk I/O perf issue and postconf detecting something that causes it to restart multiple times (roughly every 250ms, for total 1.5 seconds).

---

In comparison, there is no issue with grep instead of postconf, which is very quick:

$ echo -e "max_idle = 300s\nmax_idle = 600s" >/etc/postfix/main.cf && grep max_idle /etc/postfix/main.cf

max_idle = 300s
max_idle = 600s

[github-actions]

Documentation preview for this PR is ready! 🎉

Built with commit: f1304e4