other: open TODOs

[georglauterbach]

Subject

I would like to contribute to the project

Description

I'd like v13.0.0 to be a release that barely adds new functionality, because I think DMS is already packed with functionality. For the far future, I'd like to remove unnecessary features as much as possible (i.e. OpenDKIM, OpenDMARC, Amavis (plus codecs), Spamassassin, Postgrey, Razor, Pyzor, fts-xapian and the likes). With v13.0.0, the default will change to Rspamd. Later, with upcoming major releases after v13.0.0, we can remove functionality.

The script are already high-class, but they lack Bash's integrity and error checks - this should changes as well. Then there are many TODO tickets in the issue tracker that should be resolved.

---

• Make Rspamd the default, and change default of 1 to 0 for OpenDKIM, OpenDMARC, policyd-spf, Amavis & SpamAssassin
• clean up the issue tracker
• clean up stale PRs (and force-close those that have stalled for a very long time)
• introduce set -u and set -eE to our scripts to make them more robust (EDIT: could be done, but I lack the time)
• introduce set -o pipefail and shopt -s inherit_errexit to our scripts to make them more robust
• deprecate OVERRIDE_HOSTNAME and introduce DMS_FQDN

[georglauterbach]

Rationale for closing stale PRs: It may be a personal thing, but I like the PR list to be clean and up-to-date with non-stale PRs. I hope this does not offend anyone, but maintaining DMS for me is easier when I see all relevant PRs as open (or non-stale WIP). Maintaining the PR and issue list is a lot of work, especially for me, so I'd like to keep it in a way that is comfortable for me.

This does not mean your PR contains bad changes / additions! It's just that it has stalled. If you find the time, apply your changes to the most recent version of master and we can go about them again in a new PR.

---

I spent a lot of time on DMS, and I mean a lot. And I think DMS resonates well with users. But what many people don't see is the amount of work that goes into maintenance. After having maintained DMS for a long time, and keeping track and cleaning the issue tracker, I can confidently say that I feel like this is the right think, even if not everyone agrees.

[polarathene]

Additional TODO to consider

Misc

• /var/lib/clamav has an ownership all the way back to /var with clamav. Either from the Dockerfile chown, or potentially the mail-state startup script. I've not investigated, just noticed it.
• /build dir is persisted into the final image. No major issues, but it's only relevant to building the image.

---

Docs TODO

• Docs may need to communicate better that the DMS FQDN is for the mail-server, which has no relation to the email addresses / accounts it manages (even if they share the same domain). The DMS FQDN is what is relevant for TLS certs, while email domains that DMS manages need the appropriate DNS records. Partly related FAQ entry.

Example - Local Mail Relay

The example section itself is a candidate for removal, but probably still a worthwhile use-case to document somewhere? (common enough use-case that users run into with various workarounds / misconfigurations)

If not removed entirely, these two parts may no longer be relevant:

• Demonstrating SPOOF_PROTECTION feature (admonitions in step 6) incorrectly uses setup alias add (example of user attempting similar). v12.1 now prevents this and the workaround in docs via alias chaining is not the correct way (I don't believe we document how to allow additional accounts though).
• The earlier Firewall tip (admonition end of step 1) is not that useful AFAIK. Docker should port publishing should bypass it (both Firewalld and UFW), while Firewalld has some integration with Docker via it's zones. Similar integration is lacking for UFW, with some concerns to LANs exploiting routing when Docker daemon adds some rules via iptables.

Alias to redirect mail to external MTA like gmail is fine instead of POP3 AFAIK, but conflicts with using DMS to send mail from an account of the same alias address.

• Without SPOOF_PROTECTION, any logged in account can send as whomever, no local/virtual mailbox for the sender address required.
• With SPOOF_PROTECTION enabled, we need to document how to authorize an existing account.

Related (but may not be a v13 goal), is the SPOOF_PROTECTION feature itself getting some issues resolved, which would allow for it to become default. An overview summary was more recently provided.

FAQ Entries

• Docs FAQ entry on mail-state data volume could link to config reference page (presently only about config volume, but both linked docs content could be refactored to accomodate).
• Docs FAQ entry on discarding any mail received for an account could have a better title, but also probably a better solution. Workaround is due to current (v12.1 and prior) mydestination / myorigin values with full hostname, bare-domain not affected. transport_maps solution could be considered.
• Docs FAQ entry on Rancher support probably isn't that useful to keep and could be dropped.

Update for Compose V2 (V1 format deprecated from July 2023):

• Switching any usage of docker-compose commands to docker compose.
• docker-compose.yaml to compose.yaml

That would align with the changes in upstream Docker from June 2023. As referenced from their current docs below.

This Docker compose docs page has the current notification present:

Important

From the end of June 2023 Compose V1 won’t be supported anymore and will be removed from all Docker Desktop versions.
Make sure you switch to Compose V2 with the docker compose CLI plugin or by activating the Use Docker Compose V2 setting in Docker Desktop.
For more information, see the Evolution of Compose.

Final link mentions:

These three key file format versions and releases prior to v1.29.2 are collectively referred to as Compose V1.

In mid-2020 Compose V2 was released. It merged Compose file format V2 and V3 and was written in Go.
The file format is defined by the Compose specification. Compose V2 is the latest and recommended version of Compose and is compatible with Docker Engine version 19.03.0 and later.

[georglauterbach]

@polarathene could you give me a very rough (short) estimation of what it would take (also time wise) to update our CI so that we can take advantage of the Docker build cache for the ARM64 image as well?

[polarathene]

Sorry wasn't exactly short 😅

Time wise, I am not taking on any new tasks right now (presently on IPv6 docs task with userland-proxy research). From the info below, it probably wouldn't take you too long I think (assuming it's valid).

I haven't refreshed on GHA cache driver, so if that looks easier for you, maybe consider that.

---

@georglauterbach it's been a while since I thought about it. I recall two options:

• Switching to the beta / experimental GHA cache driver. If you look up the PRs where we introduced caching workflow or improvements, I think I provided some additional info/insight there at the time.
• Using the current local cache driver with separate job and build cache key (probably prefix with arm64 to not overlap/mix with amd64 cache?. Then build via separate job.

While only one build cache can be exported, multiple can be imported. If you perform these builds locally, or in your own fork on Github you should be able to inspect the cache location (I think the artifact can be downloaded manually via Actions cache management page). Depending on how it exports the cache, you might be able to upload the amd64 / arm64 build cache to separate keys.

I think if we are sure we build the image for amd64 / arm64, that earlier stage has uploaded the artifact of that platform build cache, and for publishing where it's relevant, you can just use the cache-from of both platform locations?

build-push-action docs seems to support multiple --cache-from via cache-from list value. Looks like it should be fine as multi-line yaml.

You might want to subscribe to this issue to be notified of ARM64 runner support if it ever arrives. But hopefully the caching in CI will be good enough 👍

---

For current cache approach, I think you could use (and using a platform subdir for the path key):

• cache-buildx-amd64-${{ steps.derive-image-cache-key.outputs.digest }}
• cache-buildx-arm64-${{ steps.derive-image-cache-key.outputs.digest }}

docker-mailserver/.github/workflows/generic_publish.yml

Lines 51 to 52 in bba72da

	path: /tmp/.buildx-cache
	key: cache-buildx-${{ inputs.cache-key }}

Then:

docker-mailserver/.github/workflows/generic_publish.yml

Line 84 in bba72da

cache-from: type=local,src=/tmp/.buildx-cache

cache-from: |
  type=local,src=/tmp/.buildx-cache/amd64
  type=local,src=/tmp/.buildx-cache/arm64

Just note that other workflows are using the derived key from the build workflow to pull in a cache too. You need to make sure you're properly handling the amd64/arm64 separate build caches for build workflow in other workflows such as the test PR one.

The build workflow should be fine if it's not doing mixed platform build, then it only needs to update the cache key and path can be left as-is. Other jobs that use generic-build.yml can then just have the build-push-action step load in the caches? I think only the publish workflow needs both AMD64 and ARM64 for build-push-action to have been built prior for cache use, while the testing workflow only needs generic-build.yml cache key to be adjusted to be platform specific (could use ${{ inputs.platforms }} if the input was always only amd64 or arm64 and not both?).

[georglauterbach]

Thank you very much! ❤️ I will, after much of the above work is done, consider the GHA build cache to finally bring down overall PR test time.

[casperklein]

tl;dr;

Providing alternatives 👍
Changing DMS defaults 👍
Removing anything soon: 👎

---

Later, with upcoming major releases after v13.0.0, we can remove functionality.

Just for the record. We should do this IMO very conservative. I am fine with providing alternatives like fetchmail --> getmail.
The same goes for rspamd which could supersede more than one service.

My reasoning is, at the moment, there are a lot of setups (mine included), that just works. For example, I've been using fetchmail for some time and didn't run in any issue ever. So from a rational point, there is absolutely no reason, why I should switch to getmail (as long as fetchmail is officially supported and receives security updates if required). Switching could only introduce new issues, that haven't been any before. Never change a running system 😎
Rspamd is even more critical to switch to, because it not only replaces SA, but also offers additional things like SPF/DKIM verification. It reminds me a bit of systemd..
Personally I really like the approach, 1 service for 1 thing. Instead of 1 service (+ modules) trying to do many things. I like it more isolated.
At the end, I may migrate to rspamd, that is not my point. But it will introduce additional work for me, with (at first glance) no benefit and potential to encounter issues, that I haven't before. My mailserver is critical to me and must work. Migration must be carefully planed, tested and monitored. At the moment, I don't have much time / willing to spend it for a migration.

[georglauterbach]

Agreed 👍🏼 the whole thing was more about switching. Before we actually remove functionality, there are going to be a lot of releases in between I suspect. I wouldn't change a running system either.

[polarathene]

@georglauterbach did you want this issue closed? Or should it be opened and drop the version specific target?

[georglauterbach]

@georglauterbach did you want this issue closed? Or should it be opened and drop the version specific target?

Good catch. But I'm thinking about opening a new one and transferring the leftover TODOs to have a new clean state. Nevermind, reopening this, not wasting time.

[georglauterbach]

@polarathene before we integrate new features like Vector, I'd like to see this issue being cleared. I'd provide the PR for making Rspamd the default in v14. I'd be very nice of we can also take care of the issues that are not yet completed mentioned in your comments :)

[polarathene]

before we integrate new features like Vector, I'd like to see this issue being cleared.

Yes absolutely I agree 👍

I only raised that issue as it was something I had been meaning to better document as a TODO that I could delegate. I don't expect it to be a priority any time soon.

I'd be very nice of we can also take care of the issues that are not yet completed mentioned in your comments :)

I'll see what I can do, current priorities are:

• Finish LDAP rework + docs.
• Find my local repo with Dovecot test PR and get that over the finish line (this fixes the Dovecot quota skipped tests IIRC).
• Get the encryption PR pushed (I've found this from an old VM).
• chown -R with /var/mail issue to be addressed. I've documented this with a TODO recently, and I need that encryption PR sorted before this as I recall it being affected by this logic too. Ideally after the Dovecot PR is handled, I can quickly convert the remaining test suites and look into how much effort it would be to PR the next rework of the test suite to leverage compose.yaml features for matrix tests.
• Ideally address the SPOOF_PROTECTION concerns issue. I had looked into that earlier but had to detour. I identified the issue with LDAP test, which requires a breaking change when using SASLAUTHD to use -r for the ldap mechanism, like rimap mechanism does. The logic for Postfix config also needs some changes, IIRC since introducing the dms_ config var into main.cf for submission(s) ports to use, the protection was no longer applicable to port 25 which had some relevance 😅
• After addressing the SPOOF_PROTECTION issue, following up with docs related to that. At this point, we may want to consider removing the setup alias guard, as we have an open issue where it seemed valid to support a scenario (deliver to local inbox of same address and forward via alias), at least I was not aware of any other way to support it (except with ldap and a postfix-book.schema).

If there's any other pressing TODO items beyond that, let me know. I'd like to tackle the DMS_FQDN one, along with the follow-up refactor of TLS startup scripts support (plus docs page refactoring). Probably delaying to v14 with current workload however. I've made sure to raise issues for these at least in the event I become unavailable.

The LDAP rework will close several long-standing issues, along with the chown -R issue being resolved. It seems best to address all of these LDAP related concerns for v13 since they're breaking changes. It should also become possible to better integrate with check-for-changes.sh for certificate updates.

[polarathene]

@zdenbe I don't know what you're doing with this DMS fork, but you recently contributed a PR there with no context that seems to be commits since 10.5 to 12.1: https://github.com/evymo/docker-mailserver/pull/1

That created a reference to many issues / PRs due to your merge commit message using our merge commits and their messages. Please don't do that.

[georglauterbach]

Perfect! Let me know if I can assist you somewhere (preferably not LDAP 😆)

[polarathene]

Let me know if I can assist you somewhere

If you've got time and any of these interest you, they'd probably be good to tackle:

• bug report: Dovecot sieve error logged when timestamp of .svbin is shared with .sieve source file #3558
• [TODO]: Postfix master.cf should avoid duplicating Amavis config #3280
• todo: Refactor PERMIT_DOCKER support #3478

I didn't get around to checking, but for PERMIT_DOCKER I noticed rspamd had some IP subnets being configured, might be related to what PERMIT_DOCKER is meant to control? (there's similar for Dovecot too I think)

The sieve and amavis issues are probably a good fit for you. The PERMIT_DOCKER one is probably a hassle to tackle, but it'd be good to have your input regarding if rspamd network trust should be managed by it like OpenDKIM / OpenDMARC is.

[zdenbe]

@zdenbe I don't know what you're doing with this DMS fork, but you recently contributed a PR there with no context that seems to be commits since 10.5 to 12.1: evymo#1 That created a reference to many issues / PRs due to your merge commit message using our merge commits and their messages. Please don't do that.

I am sorry. I don't know how it happened… My fault. Nothing to contribute to origin. I tried just update fork…