Optional vhosts directory ownership setup

sknight80 commented 3 years ago

Description

I added a new configuration option to avoid long restart (boot) when you have lots of mailboxes with the right ownership setup.

Fixes # (issue)

What is the current behavior (you can also link to an open issue here) ? diroots from the Discord channel mentioned that when he restarted the mail server, it took lots of time to boot up due to the high volume of mailboxes and the find command that set the ownership for a given folder.
What is the new behavior (if this is a feature change)? When we turn on the DISABLE_VHOSTS_OWNERSHIP_SET environment variable, the setup.sh will skip the find /var/mail/vhosts line the image will boot up faster.

Type of change

Please delete options that are not relevant.

[ ] Bug fix (non-breaking change which fixes an issue)
[X] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality not to work as expected)
[ ] Documentation only
[ ] Performance improvement
[ ] Refactoring (a change that neither fixes a bug nor adds a feature)
[ ] Test (adding missing tests or correcting existing ones)
[ ] Other...

Status

[X] Ready
[ ] In development
[ ] Hold

Todo List

[ ] Implementation
[X] Tests
[ ] Documentation
[ ] ...

How has this been tested?

Describe the tests that you ran to verify your changes. Provide instructions so we can reproduce, and please also list any relevant details of your test configuration.

[x] Startup the mail server without the new configuration option => The system is up and running, and the startup log does not contain [INFO] VHOSTS directories permission set is disabled.

[x] Startup the mail server with the new configuration option set to true => The system is up and running, and the startup log does contain [INFO] VHOSTS directories permission set is disabled.

mailserver_1    | [INFO] VHOSTS directories permission set is disabled
mailserver_1    | [WARNING] If you manually created a directory under the vhost folder, you will not receive any emails on that email address!

diroots commented 3 years ago

many thanks for this @sknight80

sknight80 commented 3 years ago

@diroots I have to test my branch then I will submit this for review

AndrewSav commented 3 years ago

@sknight80 are you sure it was on discord? Cannot find any record of it ;(

sknight80 commented 3 years ago

@AndrewSav I updated the description with the link. ;)

AndrewSav commented 3 years ago

I had a look at this, was trying to figure out if there is an "automatic" way to avoid the delay without the needs to specify the option, but could not come up with one, otherwise looks good. Default set - check, documentation updated check. I was a bit unsure if the default should be in run.sh or setup.sh but it looks like we have them (for other vars) in both, so I guess does not matter?

sknight80 commented 3 years ago

I could think only for sending that one line into the background and logging back when it is done. We could have a couple of downtime minutes to store new mails on the read-only mailboxes until the command finished it. But honestly, when you are using the image and create a new mailbox via postfix, it should set up the right access level. So I don't think we should run the find command when a new container is created.

Let us merge it and iterate on it later.

diroots commented 3 years ago

hello @sknight80

i saw u did push this change to master, good to know. i also saw it was in the unreleased features, with some other. that means not yet available on docker hub.

Do you know when u plan to do a new release with this feature (and the others listed) ? still some work before it goes live?

sknight80 commented 3 years ago

Let me sync with @AndrewSav and @SaraSmiseth when do we want to release a new version

SaraSmiseth commented 3 years ago

I think its fine to release a new version.

What rules do we follow on when to release new versions?

sknight80 commented 3 years ago

I think this one: https://github.com/mailserver2/mailserver/blob/master/VERSIONING.md

SaraSmiseth commented 3 years ago

Well it does not mention when a new release will be created. What do you think about this: The person who merged something into the master branch should create a new release after that?

sknight80 commented 3 years ago

Sounds good to me.

On Mon, Oct 26, 2020 at 12:22 PM Sara Aimée Smiseth < notifications@github.com> wrote:

Well it does not mention when a new release will be created. What do you think about this: The person who merged something into the master branch should create a new release after that?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mailserver2/mailserver/pull/25#issuecomment-716661047, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABB5S2BE2JOZAJN2FXL5QWDSMWO2TANCNFSM4SLZ7YIA .

AndrewSav commented 3 years ago

Note that not all changes require a release. This one does though. So @sknight80, are you going to be doing this, in this case?

sknight80 commented 3 years ago

Yes, today I will do it.

sknight80 commented 3 years ago

I pushed the new version to the docker hub and added a new tag and release to GitHub. Going to create a new PR for updating the changelog.

mailserver2 / mailserver