scripts: make policyd-spf configurable#3246
Conversation
georglauterbach
added
kind/new feature
georglauterbach
force-pushed
the
spf-configurable
branch
from
49fa2cb to
f607f95
Compare
|
Documentation preview for this PR is ready! 🎉 Built with commit: f607f95 |
If you are defaulting to Rspamd that soon, please get it into a state that it's more standardized for configuration like we support with other features already. Last I saw the docs were being very vague on local storage and saying to mount content directly into There is still advice about manual config management (and placement, including no strict file name, such as dkim signing module). Along with a DSL of sorts config file you've designed, which I've not looked too much into. I get the impression of the support being rushed / pushed a bit eagerly, with potential for burdening yourself with more maintenance as rspamd is largely your area of the project. |
polarathene
left a comment
polarathene
left a comment
There was a problem hiding this comment.
I'm not sure about the env/docs to communicate rspamd relevance, but will approve on the basis it'll no longer be relevant 6 months later.
|
|
||
| ##### ENABLE_POLICYD_SPF | ||
|
|
||
| Enabled `policyd-spf` in Postfix's configuration. You will likely want to set this to `0` in case you're using Rspamd ([`ENABLE_RSPAMD=1`](#enable_rspamd)). |
There was a problem hiding this comment.
Is the rspamd context necessary?
Shouldn't this detection just be handled within the scripts as a conflict like we usually do? Emit a warning in the logs instead?
Eventually, if it makes no sense to have the alternatives possible to use while Rspamd is active, throw an error on startup, all of which will be temporary with the plan to drop support for our non-rspamd SPF/DKIM/DMARC validators?
There was a problem hiding this comment.
Fair point; I will be adding these checks soon. I think the Rspamd context does not hurt at all, and the only relevant setup where this setting really should be used as of now is ENABLE_RSPAMD=1.
| else | ||
| _log 'debug' 'Disabling policyd-spf' | ||
| sedfile -i -E 's|check_policy_service unix:private/policyd-spf, ||g' /etc/postfix/main.cf | ||
| fi |
There was a problem hiding this comment.
Wouldn't it make sense to include this in the enable logic above?
# From (but only for smtpd_recipient_restrictions):
# https://github.com/docker-mailserver/docker-mailserver/blob/bbe1d2da3117264c484a87c75813b0d3da71161a/target/postfix/main.cf#L51
reject_unauth_destination, reject_unauth_pipelining
# To (sed value inbetween):
reject_unauth_destination, check_policy_service unix:private/policyd-spf, reject_unauth_pipelining
Bit more work to support, I'm not going to block since this snippet will be removed before the end of the year I assume.
There was a problem hiding this comment.
I actually thought the same, but then I think it is easier with my initial way. You're right though: this should be removed by the end of the year :)
Basically #3246 (comment) Looking at https://github.com/docker-mailserver/docker-mailserver/actions/runs/4699505235/jobs/8333061595?pr=3261, we can see that `postconf` is complaining: ```txt postconf: warning: /etc/postfix/main.cf: unused parameter: policyd-spf_time_limit=3600 ``` This PR resolves the matter and puts all the code that integrates policyd-spf in one place.
3 participants
Description
Users of Rspamd do not need
policyd-spf, hence it should be configurable. Withv13.0.0, the default should change to0becauseENABLE_RSPAMDwill default to1.Fixes #
Type of change
Checklist:
docs/)