2050 Configured sendmail in dev / ci environments to return immediate…

[osaerdna]

…ly instead of trying to send mail

I did notice that the user creation was somewhat slow in my dev environment, a profile of the request gave
...
800,815,612 (95.49%) 2,152 ( 0.09%) /var/www/html/src/inc/Util.php:Hashtopolis\inc\Util::sendMail
800,811,639 (95.49%) 880 ( 0.04%) php:internal:php::mail
21,322,474 ( 2.54%) 296 ( 0.01%) /var/www/html/src/inc/Encryption.php:Hashtopolis\inc\Encryption::passwordHash
21,319,117 ( 2.54%) 96 ( 0.00%) php:internal:php::password_hash
7,164,671 ( 0.85%) 402,800 (16.43%) /var/www/html/vendor/slim/slim/Slim/Middleware/RoutingMiddleware.php:Slim\Middleware\RoutingMiddleware->performRouting
...
Which I interpret as lots of time is spent in php:internal:php::mail. Since I don't have any sendmail config in my environment I guess it tries with default config and traps waiting for something.

I guess that we don't see this in many production environments since most of them probably have mail configured correctly and then there is not much time taken for the send to complete.

About the fix, I took what I got from copilot, now we configure the environments (devcontainers and ci) to use /bin/true to simulate successful mail sending. I guess there are cases where we really would like the email to be sent and verified but that might be the exception and then will require some specific mail configuration to be done.

Profiling after the fix the send mail function is not noticed as significant enough to report (my guess) and the integration tests in test_user.py is faster to run, and my local setup of server / web-ui is runs with expected latency.

[jessevz]

Very nice profiling to find out the bottleneck! But I do think it would be nice to check with logic wether mail is configured instead before sending an email. The thing is that it is not a requirement for a production setup to have mail configured, so then they will run into this bottleneck. Probably the easiest way would be to check the existence of the ssmtp.conf file. But if you can find another solution that would be good aswell

[osaerdna]

I have had a look at it and I cant seem to find a good way that satisfies all scenarios, the /etc/ssmtp/ssmtp.conf file is installed as a placeholder/default configuration when installing the ssmtp package (my guess, since I have it and have not manually configured it). So I guess all systems running in a containerized environment and using the projects compose files will have that file (configured or not).

One could check the actual configuration in the ssmtp.conf and see if it differs from the default, but the container user does not have read access to that file (probably for a good reason since there can be secrets and other information in that). I rather not change that.

Another potential check would be to look at our configuration values, the DConfig::EMAIL_SENDER and see if that is set or differ from the default value, but I guess, you could configure the ssmtp service and still use the default value (the 'From' line would probably differ though, it is not allowed to override by default). The result would be for existing systems (with the default config) that mail silently stops after upgrading, also it seems somewhat un-intuitive that the default value for sender address governs if (email) notifications are on or off.

Maybe an easy solution would be to add an email notification on and off switch in the settings, Enable email notifications that would translate to 'on' for existing systems that has a DConfig::EMAIL_SENDER != hashtopolis@example.com and off for those using the default value, then at least it would be more obvious for those systems sending mails with the default sender configured when mail suddenly stops.

Thoughts are welcome

[osaerdna]

I went with @jessevz s idea about checking the ssmtp.conf existence. For that to work the container image build Dockerfile do install the ssmtp program but removes the default configuration file, the assumption is that if you have mail configured you probably mounted the real config into /etc/ssmtp/ directory at container start.

The unit tests cover this change, for the TestMocks.php, it is what it is and probably not the best way to do it. Maybe we could wrap these system level functions into something that can be mocked properly and use that throughout the code instead, I which to put that into a separate ticket in that case that can be a little more focused on that depending on what changes would be necessary.

[osaerdna]

I am actually not sure about this change, I merged dev branch and had problems getting ci to build properly. Advice appreciated, it seems using the lock would prevent unstable builds?

[jessevz]

Ah yes I think we forgot to actually copy the .lock file before installing. the .lock file is so that we have reproducable builds and no change in dependencies until we update the .lock file