Bug #22703
openDefault configuration should not try to send email
Description
source:lib/config/config.default.yml has the following (with some options skipped to highlight the important bits):
# Outgoing email configuration:
#
# In order to send mail, Arvados expects a default SMTP server
# on localhost:25. It cannot require authentication on
# connections from localhost. That server should be configured
# to relay mail to a "real" SMTP server that is able to send
# email on behalf of your domain.
# The welcome email sent to new users will be blind copied to
# these addresses.
UserNotifierEmailBcc:
SAMPLE: {}
# Recipients for notification email sent out when a user account
# is created and already set up to be able to log in
NewUserNotificationRecipients:
SAMPLE: {}
# Recipients for notification email sent out when a user account
# has been created but the user cannot log in until they are
# set up by an admin.
NewInactiveUserNotificationRecipients:
SAMPLE: {}
# Send an email to each user when their account has been set up
# (meaning they are able to log in).
SendUserSetupNotificationEmail: true
If you try to install arvados-api-server
with this configuration, the postinst script is likely to fail when it tries to seed the database.
If you're not running a mail server at all, you'll get "Errno::ECONNREFUSED: Connection refused - connect(2) for "localhost" port 25"
If you're running a stock mail server using Debian's snakeoil cert, you'll get "OpenSSL::SSL::SSLError: SSL_connect returned=1 errno=0 state=error: certificate verify failed (unspecified certificate verification error)"
Even if you disable that, you'll get "Net::SMTPFatalError: 550 5.1.1 <SAMPLE>: Recipient address rejected: User unknown in local recipient table" - because RailsAPI is trying to send mail to "SAMPLE" per this configuration above and that's bogus.
IMO the default configuration should be written in such a way that it never sends mail. This assumption that there's a smart relay on localhost:25 is a huge one and difficult for us to orchestrate in the installer. If you get this configuration wrong it leads to confusing errors at weird times like database setup, when you wouldn't think to be checking the mail settings.
More generally: almost every time "SAMPLE" appears in the default configuration, personally I tend to find it more confusing than helpful. I think it's intended to help illustrate the structure of the setting, but IMO it's not succeeding at that job. Usually I find myself wondering what valid keys are, and "SAMPLE" tends to obscure rather than illustrate that. And then when it gets picked up as intentional configuration like this, that leads to all kinds of surprising results. IMO we would be better served by better comments documenting the structure of each setting, and maybe a more realistic example in comments too.
Updated by Brett Smith 7 days ago
- Related to Bug #21622: Mail delivery failure should not cause API calls to fail added
Updated by Tom Clegg 6 days ago
Yes, SAMPLE
is a placeholder key to illustrate the structure of the config section below it without actually adding a default. The config loader ignores SAMPLE
entries.
arvados-server config-dump -config <(echo 'Clusters: {zzzzz: {}}') | grep NewUserNotificationRecipients
NewUserNotificationRecipients: {}
If you try to install arvados-api-server with this configuration ...
Are you talking about using the entire config.default.yml
file as a starting point for an actual site config file? The intended pattern is that the site config file only contains site-specific config, not a copy of all the defaults and comments. Is this something we want to support?
Updated by Brett Smith 6 days ago
Tom Clegg wrote in #note-2:
Are you talking about using the entire
config.default.yml
file as a starting point for an actual site config file? The intended pattern is that the site config file only contains site-specific config, not a copy of all the defaults and comments. Is this something we want to support?
Well, as a practical matter, that's what I did, because starting from config.default.yml
means I have all the possible values and comments in a single editor window instead of having to bounce between that and the documentation page or something.
I think we should consider the ergonomics here. But even if we decide no, it's not worth supporting this, then at least I think:
- The comment at the top of the file should be more explicit about this.
SendUserSetupNotificationEmail
should default tofalse
.- I still think the structure of some of our more involved configuration fields is underdocumented and we should consider how to improve it.