My understanding is that if an SMTP server is not set, the Coder server will throw warning logs, right?

Perhaps we should also disable notifications if the STMP server is not set. That way, a default Coder install doesn't have warning logs. I could be misunderstanding this though.

The issue is that when we introduced notifications, defaults were set on the cli for configuring SMTP. So we can either retroactively drop these defaults and risk breaking existing customers installs or add this flag to allow people to disable the functionality entirely.

What are the defaults that are set that would break customers? Also, does the defaults always throw an error, or is it only in some environments?

We currently have a default for CODER_EMAIL_SMARTHOST, CODER_EMAIL_HELLO and CODER_EMAIL_FORCE_TLS.

Dropping the default for CODER_EMAIL_FORCE_TLS doesn't change anything as it will default to false anyways (due to go's zero value). Dropping the defaults for CODER_EMAIL_SMARTHOST and CODER_EMAIL_HELLO will cause an issue if any customers rely on these existing defaults.

If we drop the default for CODER_EMAIL_HELLO, it will cause the SMTP client creation to fail.

If we drop the default for CODER_EMAIL_SMARTHOST, it will cause an issue because serpent.HostPort cannot be an invalid host port. But assuming that can be resolved, it will cause an error when attempting to dispatch a notification.

Currently, we will always try to dispatch notifications. The only way for these defaults to not error is for there to be a local SMTP at localhost:587.

Currently, we will always try to dispatch notifications. The only way for these defaults to not error is for there to be a local SMTP at localhost:587.

So perhaps we should change this default to "", which is essentially the equivalent of disabling SMTP notifications. I imagine this will break practically zero deployments, and we can call it out in the changelog.

Our default install creating warnings is a pretty bad UX, so I'm are definitely most concerned about fixing that versus requiring an additional config flag to surpass the warnings.

If we go with that approach, what would the expected behaviour be if someone supplied CODER_EMAIL_SMARTHOST but not CODER_EMAIL_HELLO (or vice versa)? Would we keep the notifications disabled with only a log message indicating an issue or would we cause startup to fail and present an error? The context being that they're both required for us to send notifications.

We should also keep in mind that webhook is also a delivery method.

I agree that implicitly disabling notifications by checking if all the required configs are unset more ergonomic, but as Danielle correctly identified we'd have to enumerate which fields are required to make each delivery method work, which makes things complicated and less predictable for the operator.

Perhaps we can start by 1) adding the killswitch option and 2) only displaying the warning about misconfigured SMTP/webhook settings when the notification manager is instantiated (i.e. _once, rather than on every dispatch)?

IIRC I made it warn on each dispatch to make the likelihood of discovery by operators more likely, thinking that everyone would want to enable notifications, but given that we've had at least 1 customer now asking to disable them altogether it feels like we need an overall killswitch which is very clear in its usage.

If we go with that approach, what would the expected behaviour be if someone supplied CODER_EMAIL_SMARTHOST but not CODER_EMAIL_HELLO (or vice versa)? Would we keep the notifications disabled with only a log message indicating an issue or would we cause startup to fail and present an error? The context being that they're both required for us to send notifications.

Good point!

Perhaps we can start by 1) adding the killswitch option and 2) only displaying the warning about misconfigured SMTP/webhook settings when the notification manager is instantiated (i.e. _once, rather than on every dispatch)?

This is certainly better! It still feels uncomfortable for the default Coder install to show a warning. There are plenty of capabilities we expect people to opt-in to for a better experience. However, if unspecified we disable or provide sensible defaults so that the install works out-of-the-box:

• access URL: we use Coder's SaaS tunnel service so folks can have a HTTPS access URL and wildcards
• database: if unspecified, we download postgres and start a basic database
• provisioners: if unspecified, Coder starts 3 built-in provisioners
• terraform: if not on the machine, Coder will download the Terraform binary

So, given all of those attempts at sensible defaults, it definitely feels strange for Coder to suddenly have a new required dependency that people must manually opt-out of or manually configure to suppress warnings.

It seems like there are two options:

1. Change the default install to somehow use a cloud or local SMTP server so that email notifications can work out-of-the-box, except for in network-gapped or Kubernetes deployments, which already have extended docs to use the default features.
2. Change the default install to show an "info" statement, not a warning, that SMTP notifications are disabled due to missing configuration. This also would involve creating a breaking change that disables notifications SMTP by default. It will be OK if this means that we have to ship a breaking change where certain deployments must re-configure a flag to re-enable notifications. Better now than never. This could be changing the defaults of 1-2 existing flags OR adding a new one that disables SMTP entirely by default. I'll defer this to y'all, particularly what feels like the easiest for folks to understand via our docs.

I'm mostly just against us displaying "warnings" on our default install, given all of the work we have done to get Coder to work out of the box with practically zero config. This also gives us the opportunity to display in the UI (where most people look anyways) a message that the SMTP server disabled and link to docs on how to configure it, versus people assuming it is working and relying on server logs to identify issues.

Of course, if the user manually configures a SMTP server and Coder can't reach it, then we should show a warning

I hear you about not showing warnings on a default install, and how we provide defaults so the installation works out-of-the-box. However, in those cases the functionality is non-optional whereas this very much is. Open SMTP servers exist but will largely be abused by spammers, so I don't think we can make this work out-of-the-box, at least for SMTP. We could default the installation to webhooks and have a loopback webhook receiver with some basic UI to display the received notifications in our dashboard, but that seems meh to me.

I think option 2 above is the most sensible. We'd have to call it out specifically in the release notes, but I doubt anyone is actually using the defaults as they are now to integrate with a real SMTP server.

As an aside, I think I recall how notifications got released without a killswitch. We had notifications behind an experiment (which effectively was the killswitch) and when we removed the experiment it became enabled by default; this was an oversight on my part, apologies!

I'm in favour of option 2 tbh. While disabling notifications by default is technically a breaking change, it's easily resolved and can be called out in the release notes.

All good, we'll do option 2 and call it out in the release notes!

FWIW, I'm also in favor of changing the default for SMTP to be unconfigured & disabled. Same goes for webhooks if that's not already the case.