fix: Workaround postconf write settling logic

[polarathene]

Description

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.

Fixes #2985

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Improvement (non-breaking change that does improve existing functionality)

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
• New and existing unit tests pass locally with my changes

[georglauterbach]

LGTM 👍🏼 Nice work!

[polarathene]

I'm thinking perhaps to add a check on the mtime being < 2 seconds ago into this util method. Do you think it'd be useful?

check-for-changes.sh could then call that whenever reloading Postfix (where the Postgrey test shows a 2 sec delay during reload if main.cf is modified).

[georglauterbach]

The pipeline Test Merge Requests / Test AMD64 Image / Test (serial) (pull_request) is now Successful in 13m. We're already pretty close to 12min with the tests. When we improve on this further, and we definitely can (and will :D), we will see times lower than 12mins, so the next logical step when it comes to improving the CI is to speed up the ARM64 build. @polarathene @casperklein @wernerfred do you have any ideas how to do that?

[polarathene]

I'm thinking perhaps to add a check on the mtime being < 2 seconds ago into this util method. Do you think it'd be useful?

• I've now added the conditional check for this, and added it into check-for-changes.sh which may benefit from it.
• Also identified a few other locations where it should be applied. Since it was needed across a few tests that manually modify main.cf, I've added a helper method into common.bash.

[casperklein]

• I've now added the conditional check for this

Just to be save or do you expect any problems?

[polarathene]

Just to be save or do you expect any problems?

From what I understand there shouldn't be any.

EDIT: I originally responded about the feature itself, then realized the question was only about the conditional check 😅

I added the conditional to not write to disk an mtime change everytime the method is called, since it may not be required to adjust if the file wasn't very recently updated. Calling the method at a higher frequency (such as with change detection) would no longer result in changing mtime pointlessly if no changes were made to main.cf.

The conditional handling probably isn't that important. The current change detection writes to the filesystem every interval (which isn't ideal):

docker-mailserver/target/scripts/check-for-changes.sh

Lines 39 to 41 in 0ecb647

	# get chksum and check it, no need to lock config yet
	_monitored_files_checksums >"${CHKSUM_FILE}.new"
	cmp --silent -- "${CHKSUM_FILE}" "${CHKSUM_FILE}.new"

---

Original response

In the associated issue I linked to the source code upstream in Postfix that is responsible for the delay.

It's a safety guard for reading main.cf by checking mtime isn't too recent so that it can be assumed safe to read (not in a partially updated state, all writes completed / settled). It's a valid condition to wait on to ensure correct config is read.

I believe it's safe to read after make changes, but perhaps depending on hardware or third-party software something may not play well (or try to abuse/exploit it?). Postfix will tend to ignore anything that it doesn't recognize as valid config keys, which could result in a default setting used instead, or the key is valid but the value is not the intended value? I think exploiting sensitive timing like that is a difficult attack and if that were an option the system is likely already vulnerable to much worse?

---

It's mostly tests that will benefit from this, or in actual deployments minimal downtime during change events (seems to be an actual concern, I think there are some similar issues reported). Additionally reduces container start-up time, but I doubt that matters much to most users.

I am inclined to say it should be fine to workaround. It's usually more of an issue when wanting to write to a file that other processes might also attempt, or reading while a file is actively being written to (what this protects against).