Project

General

Profile

Actions

Bug #22703

open

Default configuration should not try to send email

Added by Brett Smith 7 days ago. Updated 6 days ago.

Status:
New
Priority:
Normal
Assigned To:
-
Category:
Deployment
Target version:
-
Story points:
-

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.


Related issues 1 (1 open0 closed)

Related to Arvados - Bug #21622: Mail delivery failure should not cause API calls to failNewActions
Actions #1

Updated by Brett Smith 7 days ago

  • Related to Bug #21622: Mail delivery failure should not cause API calls to fail added
Actions #2

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?

Actions #3

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 to false.
  • I still think the structure of some of our more involved configuration fields is underdocumented and we should consider how to improve it.
Actions

Also available in: Atom PDF