Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(Amavis): only add configuration to Postfix when enabled #3046

Merged
merged 6 commits into from Feb 3, 2023

Conversation

georglauterbach
Copy link
Member

Description

Since I am running Rspamd nowadays, I noticed there still are ports open that belong to Amavis. This is because the Amavis configuration is a fixed part of Postfix's master.cf. I changed that. Now, the Amavis section is added when Amavis really is enabled.

I took the chance and added proper indentation to master.cf; hence the diff is a bit fuzzy. But, only the Amavis part was adjusted, the rest is just styling.

Type of change

  • 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
  • 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

Since I am running Rspamd nowadays, I noticed there still are ports open
that belong to Amavis. This is because the Amavis configuration is a
fixed part of Postfix's `master.cf`. I changed that. Now, the Amavis
section is added when Amavis really is enabled.

I took the chance and added proper indentation to `master.cf`; hence the
diff is a bit fuzzy. **But**, only the Amavis part was adjusted, the
rest is just styling.
@georglauterbach georglauterbach added kind/improvement Improve an existing feature, configuration file or the documentation area/configuration (file) service/security/amavis labels Jan 30, 2023
@georglauterbach georglauterbach added this to the v12.0.0 milestone Jan 30, 2023
@georglauterbach georglauterbach self-assigned this Jan 30, 2023
polarathene
polarathene previously approved these changes Jan 31, 2023
Copy link
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure about storing the config in /etc/amavis within the container, but otherwise no other concerns 👍

Might be worth doing similar for the other 3 additions we have, but these are all default and not configurable I think. pickup in particular is modifying the original pickup config.

If I get around to refactoring the relay feature support, it'd be taking a similar approach to this Amavis appending. So I'm a bit more swayed towards a common location in the container (/etc/dms/postfix/master.d?)

target/postfix/master.cf Outdated Show resolved Hide resolved
commit 6a5bc44
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Jan 30 21:12:16 2023 +0100

    chore(deps): Bump docker/setup-buildx-action from 2.2.1 to 2.4.0 (#3042)

    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@georglauterbach
Copy link
Member Author

I updated the file location inside the container (to /etc/dms/postfix/master.d/postfix-amavis.cf). Also adjusted the indentation.

@georglauterbach georglauterbach changed the title only add Amavis configuration to Postfix when enabled chore(Amavis): only add configuration to Postfix when enabled Jan 31, 2023
@georglauterbach georglauterbach merged commit 774a340 into master Feb 3, 2023
@georglauterbach georglauterbach deleted the amavis-in-postfix branch February 3, 2023 16:32
@polarathene
Copy link
Member

@georglauterbach I noticed this:

# Additional option for filtering
content_filter = smtp-amavis:[127.0.0.1]:10024

It's removed if ENABLE_AMAVIS=0, but probably should be added instead if ENABLE_AMAVIS=1?:

if [[ ${ENABLE_AMAVIS} -eq 1 ]]
then
_log 'debug' 'Setting up Amavis'
cat /etc/dms/postfix/master.d/postfix-amavis.cf >>/etc/postfix/master.cf
sed -i \
"s|^#\$myhostname = \"mail.example.com\";|\$myhostname = \"${HOSTNAME}\";|" \
/etc/amavis/conf.d/05-node_id
else
_log 'debug' "Removing Amavis from Postfix's configuration"
sed -i 's|content_filter =.*|content_filter =|' /etc/postfix/main.cf

georglauterbach added a commit that referenced this pull request Feb 9, 2023
georglauterbach added a commit that referenced this pull request Feb 9, 2023
georglauterbach added a commit that referenced this pull request Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration (file) kind/improvement Improve an existing feature, configuration file or the documentation service/security/amavis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants