Add default /etc/aliases for amavis, clamav. Add bsd-mailx. by MakerMatrix · Pull Request #1709 · docker-mailserver/docker-mailserver

MakerMatrix · 2020-12-08T23:33:42Z

Also fixed dovecot keygen so it doesn't generate an error msg about RANDFILE.

…ot keygen.

georglauterbach · 2020-12-09T08:41:23Z

I like these changes - thanks for creating this PR:)

I've added some comments I'd like to resolve first, and I've requested a review from @erik-wramner as weill, which I would like to see before merging.

PS: I find it very nice that the key generation error is fixed! :D

MakerMatrix · 2020-12-09T16:06:56Z

On reviewing your comments I think it would be much better if we wrapped the the clamav alias inside an if stmt, to check whether the user asked for that feature or not. I put it in because I noticed that cron job(s) get run as the clamav user. But I guess only if user asked for that feature.

georglauterbach · 2020-12-09T18:49:55Z

On reviewing your comments I think it would be much better if we wrapped the the clamav alias inside an if stmt, to check whether the user asked for that feature or not. I put it in because I noticed that cron job(s) get run as the clamav user. But I guess only if user asked for that feature.

I accidentally did not press the submit button. You can now see my comments, sorry:) These are the ones I was referring to.

EDIT: But your idea is worth a shot - backwards compatibility and stuff...

georglauterbach · 2020-12-09T18:55:48Z

In #1706 you mentioned that /etc/aliases is completely emptied on "check-for-changes.sh`-runs. I've already mentioned it in a comment - this causes some headache. I cannot confidently say whether this may break things.

MakerMatrix · 2020-12-09T19:07:37Z

In #1706 you mentioned that /etc/aliases is completely emptied on "check-for-changes.sh`-runs. I've already mentioned it in a comment - this causes some headache. I cannot confidently say whether this may break things.

Fair question, but if you look closely you see that line was already there - I only made the comment, and then added the two lines below to append amavis and clamav. It was always the case that /etc/aliases got recreated from scratch when these functions are called. There are some comments around this and a TODO about how the check-for-changes script overwrites things done by start-mailserver, and also about making these things atomic, here.

georglauterbach · 2020-12-09T19:12:16Z

Oh my... I totally missed that. I didn't look close enough. Resolved:)

georglauterbach · 2020-12-10T22:11:39Z

Looks somewhat fine to me now. As mentioned in #1706 I am not sure whether the change now adds the aliases for root, amavis and clam, but I really don't see why this would be case. So is this working?:)

georglauterbach · 2020-12-18T22:16:09Z

What's the status here? @erik-wramner what do you think?

EDIT: I think it's fine to merge this for the moment. Only when #1721 is done, we will release 7.3, so changes here will definitely be stable after we reworked the aliases system.

MakerMatrix · 2020-12-21T03:18:12Z

Looks somewhat fine to me now. As mentioned in #1706 I am not sure whether the change now adds the aliases for root, amavis and clam, but I really don't see why this would be case. So is this working?:)

Yeah it works. In 1706 I had made some faulty assumptions about the default testing container config, just learning how to build and test. :). It was a simple matter of getting a a couple things configured away from defaults which in any real-world deployment would be the first things somebody would set.

georglauterbach · 2020-12-29T13:31:03Z

@erik-wramner when you've reviewed this PR, feel free to either merge it or close it. SInce we are working on the alias code anyways, I will leave it up to you what to do with this PR.

erik-wramner · 2020-12-30T09:06:55Z

  chown -R clamav:root /var/log/mail/freshclam.log && \
+  mkdir -p /var/log/supervisor && \
+  touch /var/log/supervisor/supervisord.log && \
+  chown -R syslog:root /var/log/supervisor && \


Please explain the need for this as well and how it is related to the aliases. It looks like you are trying to solve something, but what?

It's not related to aliases, sorry. I believe I found errors in the logs where supervisord was throwing an error when trying to write to /var/log/supervisor/supervisord.log and it did not exist. I do apologize for not documenting this one better, I see I didn't even put a comment along with the PR and frankly even I cannot remember exactly where I found this one.

I'll amend the first sentence of the previous to say that it was tangentially related to aliases in the sense that I found it when trying to discover exactly what was going on with aliases when supervisord brings up the mail service the first time after container creation. There are some conditions where they don't get added, which appear to be based on how some of the environment variables are set. I never narrowed it down further than that but thought it important for supervisord to be able to write its logs "out of the box"

erik-wramner · 2020-12-30T09:09:23Z

+      fi
+
+      # Append external postifx aliases
      if [[ -f /tmp/docker-mailserver/postfix-aliases.cf ]]


This is probably fine, but we could check if postfix-aliases.cf has a mapping for amavis. Many existing installations may already have one (mine does). Not a big deal though, perhaps it is better to keep the code simple and write something about it in the README.

Hmmm, mine didn't. My /tmp/dockermailserver/postfix-aliases.cf is empty. If that's a better place to do these, then by all means.

erik-wramner · 2020-12-30T09:10:22Z

+  then
+    echo "clamav: ${POSTMASTER_ADDRESS}" >> /etc/aliases
+  fi



See previous comment.

erik-wramner · 2020-12-30T09:13:12Z

@aendeavor @MakerMatrix I don't understand what several of the changes are for. Perhaps the important fixes are done elsewhere? If so let's close this. If not feel free to motivate and perhaps we can merge instead. It feels as if the changes are trying to fix several things that are not related to the alias issue at all?

georglauterbach · 2020-12-30T09:46:23Z

Very true. There are some hotfixes in here that could probably go in another PR, you're right. @MakerMatrix can explain them and then we can decide if we need another PR.

MakerMatrix · 2021-01-08T21:23:13Z

At this point I think this can be closed out without merging. I will try to recreate the supervisord.log issue and open that as a separate PR after the migration to the docker-mailserver github account (will I know when that happens?). I think everything else here including the additional default aliases is being addressed in different fixes.

georglauterbach · 2021-01-08T21:57:45Z

At this point I think this can be closed out without merging. I will try to recreate the supervisord.log issue and open that as a separate PR after the migration to the docker-mailserver github account (will I know when that happens?). I think everything else here including the additional default aliases is being addressed in different fixes.

A good decision. You will see when the repository migration is ready in the README and when the other PRs are all closed - just look out for that.

Add default /etc/aliases for amavis, clamav. Add bsd-mailx. Fix dovec…

69bebd5

…ot keygen.

MakerMatrix mentioned this pull request Dec 8, 2020

amavis uid does not match ownership of /var/mail/amavis #1701

Closed

georglauterbach mentioned this pull request Dec 9, 2020

Default mail aliases - what mechanism makes the most sense? #1706

Closed

1 task

georglauterbach requested review from erik-wramner and georglauterbach December 9, 2020 08:39

georglauterbach added enhancement labels Dec 9, 2020

georglauterbach linked an issue Dec 9, 2020 that may be closed by this pull request

amavis uid does not match ownership of /var/mail/amavis #1701

Closed

georglauterbach reviewed Dec 9, 2020

View reviewed changes

Comment thread Dockerfile Outdated

Comment thread target/start-mailserver.sh

Additional related fixes to default aliases based on review.

5922f30

georglauterbach reviewed Dec 10, 2020

View reviewed changes

Comment thread target/start-mailserver.sh Outdated

georglauterbach reviewed Dec 10, 2020

View reviewed changes

Comment thread target/check-for-changes.sh Outdated

MakerMatrix added 2 commits December 10, 2020 16:11

Fixed stupid typo

a20a184

Put _register_setup_function back in its original order

181c30d

georglauterbach mentioned this pull request Dec 13, 2020

Is this project mailgun/sendgrid DIY replacement? #1707

Closed

MakerMatrix mentioned this pull request Dec 13, 2020

Get rid of syslogd warnings about kernel logging. #1711

Merged

casperklein reviewed Dec 19, 2020

View reviewed changes

Comment thread Dockerfile

MakerMatrix mentioned this pull request Dec 21, 2020

[FR] Streamline alias code #1721

Open

erik-wramner reviewed Dec 30, 2020

View reviewed changes

MakerMatrix closed this Jan 8, 2021

polarathene mentioned this pull request Apr 23, 2023

Unset mydestination in postfix/main.cf #3264

Closed

8 tasks

Uh oh!

Conversation

MakerMatrix commented Dec 8, 2020

Uh oh!

georglauterbach commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MakerMatrix commented Dec 9, 2020

Uh oh!

Uh oh!

Uh oh!

georglauterbach commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georglauterbach commented Dec 9, 2020

Uh oh!

MakerMatrix commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georglauterbach commented Dec 9, 2020

Uh oh!

Uh oh!

Uh oh!

georglauterbach commented Dec 10, 2020

Uh oh!

georglauterbach commented Dec 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

MakerMatrix commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georglauterbach commented Dec 29, 2020

Uh oh!

Uh oh!

erik-wramner Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

MakerMatrix Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

MakerMatrix Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

erik-wramner Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

MakerMatrix Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

erik-wramner Dec 30, 2020

Choose a reason for hiding this comment

Uh oh!

MakerMatrix Jan 4, 2021

Choose a reason for hiding this comment

Uh oh!

erik-wramner commented Dec 30, 2020

Uh oh!

georglauterbach commented Dec 30, 2020

Uh oh!

MakerMatrix commented Jan 8, 2021

Uh oh!

georglauterbach commented Jan 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

georglauterbach commented Dec 9, 2020 •

edited

Loading

georglauterbach commented Dec 9, 2020 •

edited

Loading

MakerMatrix commented Dec 9, 2020 •

edited

Loading

georglauterbach commented Dec 18, 2020 •

edited

Loading

MakerMatrix commented Dec 21, 2020 •

edited

Loading