search

mailserver2 / mailserver

Simple and full-featured mail server using Docker
https://store.docker.com/community/images/mailserver2/mailserver
MIT License
133 stars 28 forks source link

Update for bullseye debian-mail-overlay #37

Closed SaraSmiseth closed 2 years ago

SaraSmiseth commented 3 years ago

Description

This PR is for the updated bullseye debian-mail-overlay which is here https://github.com/mailserver2/debian-mail-overlay/pull/11. Once that is merged this PR will fix the failing tests and do other necessary changes.

I fixed a few tests that were broken, because logging messages changed. There are still 2 tests left that do not run successfully. I'm not sure why they fail at the moment.

The failing tests are:

https://github.com/mailserver2/mailserver/blob/ab9cac04334ae9397045fbb7a06a864fce1ec3c6/test/tests.bats#L1462 https://github.com/mailserver2/mailserver/blob/ab9cac04334ae9397045fbb7a06a864fce1ec3c6/test/tests.bats#L1468

Fixes

Type of change

Status

TODO List

How has this been tested?

I have updated my server to use an image based on the updated debian-mail-overlay for about 2 weeks.

SaraSmiseth commented 2 years ago

The 2 tests mentioned above still fail. I will look into them when I have time at the weekend. It looks like Travis does not run. I found an error message here: Could not authorize build request for mailserver2/mailserver.

AndrewSav commented 2 years ago

It looks like Travis does not run. I found an error message here: Could not authorize build request for mailserver2/mailserver.

I think I fixed this by selecting the free plan. However my build fails, due to EICAR, which I believe you already fixed?

Probably no worth fixing my PR here, so I will just close it, but feel free to pull the update of the travis link (unrelated) to your PR from mine as well.

SaraSmiseth commented 2 years ago

The reason for both checking dovecot: piped ham message with sieve tests failing is that the dovecot logging for sieve has changed. If I run the container with DEBUG_MODE=true the messages that the tests are expecting are there. It seems that they were changed from INFO to DEBUG level. I have to change the tests so that they check for the messages in a container that is started with DEBUG_MODE=true . I'm not exactly sure how. Maybe add a new container just for these 2 tests? I will have to check if that's possible and we don't run into problems having too much test containers running at the same time.

AndrewSav commented 2 years ago

Sorry, do not have anything useful to contribute, but everything you say makes sense to me. A separate container should work, but at the same time we already have quite a lot of them and they are quire heavy, so the concern about a new one is founded.

SaraSmiseth commented 2 years ago

Tests are all working now. I added a new container for the sieve tests. The tests require about 1.7GB more ram now.

We have a new travis problem:

Owner mailserver2 user license exceeded.

I'll try to use GitHub Actions instead.

SaraSmiseth commented 2 years ago

I have played around with github actions here and got it working.

The problem I had was that random rspamd tests failed because the runners do not have enough RAM. I got it working by splitting tests and running the new container and the tests for the new container after everything else. See this ugly commit.

I don't like that workaround. What do you think?

AndrewSav commented 2 years ago

@SaraSmiseth what worries you? If we do not have enough memory to run them all at the same time we have to run them in stages, and that's what you did. May be to get rid of the mental sense of ugliness we just need to accept that this is a multistage tests which require different set of containers running at each stage. Would that thinking help? Or is there anything else particularly ugly that does not sit well?

SaraSmiseth commented 2 years ago

May be to get rid of the mental sense of ugliness we just need to accept that this is a multistage tests which require different set of containers running at each stage. Would that thinking help?

Yes thanks.

I added a target for each mailserver-test-container to the Makefile. This allows to run for example only ldap tests with make ldap. Running only make will execute all tests. I split the test.bats file into multiple bats-test-files.

I have also added a github actions workflow for each mailserver-test-container. The workflows will build the docker image and then run the tests. For example: the workflow .github/workflows/default.yml will run make default to run tests for the default container. These workflows run only for pull requests into master.

EDIT: The workflows are not running here because they are not in master yet. I have tested them in my fork.

This should be all. Should we merge it?

AndrewSav commented 2 years ago

This should be all. Should we merge it?

How urgent do you feel? I can do my best to try and give at a spin next weekend. If you are fairly confident that this works, then by all means go ahead.

AndrewSav commented 2 years ago

Also can we then remove travis or something? (I'm referring to the "All check have failed" message) Because it does not look like it provides any value at this stage.

SaraSmiseth commented 2 years ago

How urgent do you feel? I can do my best to try and give at a spin next weekend. If you are fairly confident that this works, then by all means go ahead.

It's not that urgent. Take your time.

Also can we then remove travis or something? (I'm referring to the "All check have failed" message) Because it does not look like it provides any value at this stage.

Yes I'll remove it.

sknight80 commented 2 years ago

Sorry, I am a little bit late for the party. How can I help?

AndrewSav commented 2 years ago

@sknight80 by testing that the changes here are working fine for you. Thanks.

AndrewSav commented 2 years ago

I keep getting 

✗ checking postfix: milter-reject - clamav virus found
   (from function `assert_success' in file test/test_helper/bats-assert/src/assert.bash, line 114,
    in test file test/default.bats, line 491)
     `assert_success' failed

   -- command failed --
   status : 1
   output :
   --

docker exec mailserver_default cat /var/log/mail.log | grep EIC yields:

2021-09-18T02:00:25.064261+00:00 mail clamd[1770]: instream(127.0.0.1@35030): Win.Test.EICAR_HDB-1(697ef5516dc83129c71542c60777da9c:790) FOUND
2021-09-18T02:00:25.330080+00:00 mail postfix/cleanup[2179]: 087FE810: milter-reject: END-OF-MESSAGE from localhost[127.0.0.1]: 5.7.1 clamav: virus found: "Win.Test.EICAR_HDB-1"; from=<virus@gmail.com> to=<john.doe@domain.tld> proto=SMTP helo=<mx.gmail.com>

Are they keeping changing virus found messages?

AndrewSav commented 2 years ago

@SaraSmiseth what about sleeps? I think we should add them to make the test work on a bigger range of machines until such time someone has time to substitute them with proper waits for readiness?

SaraSmiseth commented 2 years ago

I have added the sleeps and updated EICAR tests. I agree that the sleeps should be changed later.

AndrewSav commented 2 years ago

@SaraSmiseth thank you for your work on this upgrade. A bit of unrelated question, I know you are not using rainloop, what mail client are you using with the mailserver?

SaraSmiseth commented 2 years ago

@SaraSmiseth thank you for your work on this upgrade. A bit of unrelated question, I know you are not using rainloop, what mail client are you using with the mailserver?

Thank you for the review. I use thunderbird and no webmail at all.