ofn-install
ofn-install copied to clipboard
openfoodfoundation
Simplify email configuration
Description
We store our email configuration in the ofn-secrets repository. When provisioning a server, we create /etc/defaults/openfoodnetwork to store the email configuration in environment variables for the user. Those are only loaded in a login session though and not available in the unicorn process.
During deployment, we run rake db:seed which reads the environment variables and writes them to the database via Spree::Config. The application (unicorn and delayed_job) reads these values through Spree::Config. These values are cached though so that the retrieval depends on memcached to be running, otherwise defaults are provided. A stale cache can also lead to different processes seeing different configurations and not looking at the database.
This process is complex and brittle and led to #6529.
Expected Behavior
The application reads the email configuration from a file or the environment without depending on the database or a cache.
Actual Behaviour
Email fails if the cache is out of sync or down. We also have multiple places storing the config (on disk and in the database).
Steps to Reproduce
- As ofn-admin:
sudo systemctl stop memcached.service -
bundle exec rails console - Verify output of
ActionMailer::Base.smtp_settings
Severity
tech-debt
Possible Fix
Since Rails 4.1 we can store configuration like this in config/secrets.yml and configure ActionMailer with that.
https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#config-secrets-yml https://guides.rubyonrails.org/4_1_release_notes.html#config-secrets-yml
@andrewpbrett hit this again while fixing Canada's Gmail credentials issue today. What we didn't like was the fact that we had to redeploy for the ENV var change to be applied. This slowed us down a lot.
Wouldn't it be as simple as copying the ENV vars values to ActionMailer::Base.smtp_settings and forget about anything else, including Spree::Config?
Yes, since we configure mail settings via ofn-install, we shouldn't need Spree::Config at all for this.
it sounds like we could get rid of quite some code and end up with a plain simple configuration :heart_eyes:
Great, this is long overdue!
We set the mail configs in environment variables here when provisioning: https://github.com/openfoodfoundation/ofn-install/blob/807e6fd12dd1960eae61303ba154bd86f4090d90/roles/unicorn_user/templates/defaults.j2
We apply those configs via db/seed.rb during deployment (which is a bit crazy): https://github.com/openfoodfoundation/openfoodnetwork/blob/55215bb59a5b2695b3d6838654a1f078d7ff23f6/db/seeds.rb#L5-L18
...which calls this service to persist them in the (problematic) Spree Configs here: https://github.com/openfoodfoundation/openfoodnetwork/blob/55215bb59a5b2695b3d6838654a1f078d7ff23f6/app/services/mail_configuration.rb#L6-L10
Lets clean it up :muscle:
I think we can leave the environment variables as they are, move the setup out of db/seed.rb, and update that service to avoid Spree::Config.
There's another piece here. It's https://github.com/openfoodfoundation/openfoodnetwork/blob/master/lib/spree/core/mail_settings.rb#L16 the line that ends up configuring Rails' SMTP settings. In a vanilla Rails app that's the only line you need to make it work.
Also, we can't forget about the couple of form fields we still have in /admin/mail_methods/edit. They may ruin the party...
Why not make something like this, as you know, this is also a RoR application? Mail settings don't change that often. We can set it up once and forget about it. Today, there are regularly some problems with the mail settings, especially after updates.
Yes, that's what we are aiming for. The problem was that Spree stores this configuration in the database and we have been moving away from that. At the moment, we are in an awkward in-between phase but we want a simple config file as you described.