feat!: allow disabling notifications (#15509)

Resolves https://github.com/coder/coder/issues/15513

Disables notifications when both `$CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT` and `$CODER_EMAIL_SMARTHOST` are unset.

Breaking change: `$CODER_EMAIL_SMARTHOST` is no longer set by default as `localhost:587`, meaning any deployments that make use of this default value will need to add it back.

---------

Co-authored-by: Danny Kopping <danny@coder.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
This commit is contained in:
Danielle Maywood
2024-11-19 15:05:12 +00:00
committed by GitHub
co-authored by Danny Kopping Mathias Fredriksson
parent 1c08580f23
commit 576e1f48fe
15 changed files with 155 additions and 101 deletions
+44 -34
View File
@@ -876,31 +876,39 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
}
// Manage notifications.
cfg := options.DeploymentValues.Notifications
metrics := notifications.NewMetrics(options.PrometheusRegistry)
helpers := templateHelpers(options)
var (
notificationsCfg = options.DeploymentValues.Notifications
notificationsManager *notifications.Manager
)
// The enqueuer is responsible for enqueueing notifications to the given store.
enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, helpers, logger.Named("notifications.enqueuer"), quartz.NewReal())
if err != nil {
return xerrors.Errorf("failed to instantiate notification store enqueuer: %w", err)
if notificationsCfg.Enabled() {
metrics := notifications.NewMetrics(options.PrometheusRegistry)
helpers := templateHelpers(options)
// The enqueuer is responsible for enqueueing notifications to the given store.
enqueuer, err := notifications.NewStoreEnqueuer(notificationsCfg, options.Database, helpers, logger.Named("notifications.enqueuer"), quartz.NewReal())
if err != nil {
return xerrors.Errorf("failed to instantiate notification store enqueuer: %w", err)
}
options.NotificationsEnqueuer = enqueuer
// The notification manager is responsible for:
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
// - keeping the store updated with status updates
notificationsManager, err = notifications.NewManager(notificationsCfg, options.Database, helpers, metrics, logger.Named("notifications.manager"))
if err != nil {
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
}
// nolint:gocritic // We need to run the manager in a notifier context.
notificationsManager.Run(dbauthz.AsNotifier(ctx))
// Run report generator to distribute periodic reports.
notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal())
defer notificationReportGenerator.Close()
} else {
cliui.Info(inv.Stdout, "Notifications are currently disabled as there are no configured delivery methods. See https://coder.com/docs/admin/monitoring/notifications#delivery-methods for more details.")
}
options.NotificationsEnqueuer = enqueuer
// The notification manager is responsible for:
// - creating notifiers and managing their lifecycles (notifiers are responsible for dequeueing/sending notifications)
// - keeping the store updated with status updates
notificationsManager, err := notifications.NewManager(cfg, options.Database, helpers, metrics, logger.Named("notifications.manager"))
if err != nil {
return xerrors.Errorf("failed to instantiate notification manager: %w", err)
}
// nolint:gocritic // We need to run the manager in a notifier context.
notificationsManager.Run(dbauthz.AsNotifier(ctx))
// Run report generator to distribute periodic reports.
notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal())
defer notificationReportGenerator.Close()
// Since errCh only has one buffered slot, all routines
// sending on it must be wrapped in a select/default to
@@ -1077,17 +1085,19 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
// Cancel any remaining in-flight requests.
shutdownConns()
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
// their leases expire after a period of time and will be re-queued for sending.
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
cliui.Info(inv.Stdout, "Shutting down notifications manager..."+"\n")
err = shutdownWithTimeout(notificationsManager.Stop, 5*time.Second)
if err != nil {
cliui.Warnf(inv.Stderr, "Notifications manager shutdown took longer than 5s, "+
"this may result in duplicate notifications being sent: %s\n", err)
} else {
cliui.Info(inv.Stdout, "Gracefully shut down notifications manager\n")
if notificationsManager != nil {
// Stop the notification manager, which will cause any buffered updates to the store to be flushed.
// If the Stop() call times out, messages that were sent but not reflected as such in the store will have
// their leases expire after a period of time and will be re-queued for sending.
// See CODER_NOTIFICATIONS_LEASE_PERIOD.
cliui.Info(inv.Stdout, "Shutting down notifications manager..."+"\n")
err = shutdownWithTimeout(notificationsManager.Stop, 5*time.Second)
if err != nil {
cliui.Warnf(inv.Stderr, "Notifications manager shutdown took longer than 5s, "+
"this may result in duplicate notifications being sent: %s\n", err)
} else {
cliui.Info(inv.Stdout, "Gracefully shut down notifications manager\n")
}
}
// Shut down provisioners before waiting for WebSockets
+2 -2
View File
@@ -118,7 +118,7 @@ Configure how emails are sent.
--email-hello string, $CODER_EMAIL_HELLO (default: localhost)
The hostname identifying the SMTP server.
--email-smarthost host:port, $CODER_EMAIL_SMARTHOST (default: localhost:587)
--email-smarthost string, $CODER_EMAIL_SMARTHOST
The intermediary SMTP host through which emails are sent.
EMAIL / EMAIL AUTHENTICATION OPTIONS:
@@ -413,7 +413,7 @@ Configure how email notifications are sent.
The hostname identifying the SMTP server.
DEPRECATED: Use --email-hello instead.
--notifications-email-smarthost host:port, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
--notifications-email-smarthost string, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
The intermediary SMTP host through which emails are sent.
DEPRECATED: Use --email-smarthost instead.
+4 -4
View File
@@ -524,8 +524,8 @@ email:
# (default: <unset>, type: string)
from: ""
# The intermediary SMTP host through which emails are sent.
# (default: localhost:587, type: host:port)
smarthost: localhost:587
# (default: <unset>, type: string)
smarthost: ""
# The hostname identifying the SMTP server.
# (default: localhost, type: string)
hello: localhost
@@ -577,8 +577,8 @@ notifications:
# (default: <unset>, type: string)
from: ""
# The intermediary SMTP host through which emails are sent.
# (default: <unset>, type: host:port)
smarthost: localhost:587
# (default: <unset>, type: string)
smarthost: ""
# The hostname identifying the SMTP server.
# (default: <unset>, type: string)
hello: localhost
+1 -5
View File
@@ -11553,11 +11553,7 @@ const docTemplate = `{
},
"smarthost": {
"description": "The intermediary SMTP host through which emails are sent (host:port).",
"allOf": [
{
"$ref": "#/definitions/serpent.HostPort"
}
]
"type": "string"
},
"tls": {
"description": "TLS details.",
+1 -5
View File
@@ -10370,11 +10370,7 @@
},
"smarthost": {
"description": "The intermediary SMTP host through which emails are sent (host:port).",
"allOf": [
{
"$ref": "#/definitions/serpent.HostPort"
}
]
"type": "string"
},
"tls": {
"description": "TLS details.",
+11 -13
View File
@@ -34,11 +34,10 @@ import (
)
var (
ValidationNoFromAddressErr = xerrors.New("no 'from' address defined")
ValidationNoToAddressErr = xerrors.New("no 'to' address(es) defined")
ValidationNoSmarthostHostErr = xerrors.New("smarthost 'host' is not defined, or is invalid")
ValidationNoSmarthostPortErr = xerrors.New("smarthost 'port' is not defined, or is invalid")
ValidationNoHelloErr = xerrors.New("'hello' not defined")
ValidationNoFromAddressErr = xerrors.New("'from' address not defined")
ValidationNoToAddressErr = xerrors.New("'to' address(es) not defined")
ValidationNoSmarthostErr = xerrors.New("'smarthost' address not defined")
ValidationNoHelloErr = xerrors.New("'hello' not defined")
//go:embed smtp/html.gotmpl
htmlTemplate string
@@ -521,15 +520,14 @@ func (s *SMTPHandler) validateToAddrs(to string) ([]string, error) {
// Does not allow overriding.
// nolint:revive // documented.
func (s *SMTPHandler) smarthost() (string, string, error) {
host := s.cfg.Smarthost.Host
port := s.cfg.Smarthost.Port
// We don't validate the contents themselves; this will be done by the underlying SMTP library.
if host == "" {
return "", "", ValidationNoSmarthostHostErr
smarthost := strings.TrimSpace(string(s.cfg.Smarthost))
if smarthost == "" {
return "", "", ValidationNoSmarthostErr
}
if port == "" {
return "", "", ValidationNoSmarthostPortErr
host, port, err := net.SplitHostPort(string(s.cfg.Smarthost))
if err != nil {
return "", "", xerrors.Errorf("split host port: %w", err)
}
return host, port, nil
+1 -1
View File
@@ -440,7 +440,7 @@ func TestSMTP(t *testing.T) {
var hp serpent.HostPort
require.NoError(t, hp.Set(listen.Addr().String()))
tc.cfg.Smarthost = hp
tc.cfg.Smarthost = serpent.String(hp.String())
handler := dispatch.NewSMTPHandler(tc.cfg, logger.Named("smtp"))
+3 -3
View File
@@ -154,7 +154,7 @@ func TestSMTPDispatch(t *testing.T) {
cfg := defaultNotificationsConfig(method)
cfg.SMTP = codersdk.NotificationsEmailConfig{
From: from,
Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())},
Smarthost: serpent.String(fmt.Sprintf("localhost:%d", mockSMTPSrv.PortNumber())),
Hello: "localhost",
}
handler := newDispatchInterceptor(dispatch.NewSMTPHandler(cfg.SMTP, logger.Named("smtp")))
@@ -1112,7 +1112,7 @@ func TestNotificationTemplates_Golden(t *testing.T) {
var hp serpent.HostPort
require.NoError(t, hp.Set(listen.Addr().String()))
smtpConfig.Smarthost = hp
smtpConfig.Smarthost = serpent.String(hp.String())
// Start mock SMTP server in the background.
var wg sync.WaitGroup
@@ -1523,7 +1523,7 @@ func TestCustomNotificationMethod(t *testing.T) {
cfg.SMTP = codersdk.NotificationsEmailConfig{
From: "danny@coder.com",
Hello: "localhost",
Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())},
Smarthost: serpent.String(fmt.Sprintf("localhost:%d", mockSMTPSrv.PortNumber())),
}
cfg.Webhook = codersdk.NotificationsWebhookConfig{
Endpoint: *serpent.URLOf(endpoint),
+5 -2
View File
@@ -686,11 +686,15 @@ type NotificationsConfig struct {
Webhook NotificationsWebhookConfig `json:"webhook" typescript:",notnull"`
}
func (n *NotificationsConfig) Enabled() bool {
return n.SMTP.Smarthost != "" || n.Webhook.Endpoint != serpent.URL{}
}
type NotificationsEmailConfig struct {
// The sender's address.
From serpent.String `json:"from" typescript:",notnull"`
// The intermediary SMTP host through which emails are sent (host:port).
Smarthost serpent.HostPort `json:"smarthost" typescript:",notnull"`
Smarthost serpent.String `json:"smarthost" typescript:",notnull"`
// The hostname identifying the SMTP server.
Hello serpent.String `json:"hello" typescript:",notnull"`
@@ -1028,7 +1032,6 @@ when required by your organization's security policy.`,
Description: "The intermediary SMTP host through which emails are sent.",
Flag: "email-smarthost",
Env: "CODER_EMAIL_SMARTHOST",
Default: "localhost:587", // To pass validation.
Value: &c.Notifications.SMTP.Smarthost,
Group: &deploymentGroupEmail,
YAML: "smarthost",
+66
View File
@@ -568,3 +568,69 @@ func TestPremiumSuperSet(t *testing.T) {
require.NotContains(t, enterprise.Features(), "", "enterprise should not contain empty string")
require.NotContains(t, premium.Features(), "", "premium should not contain empty string")
}
func TestNotificationsCanBeDisabled(t *testing.T) {
t.Parallel()
tests := []struct {
name string
expectNotificationsEnabled bool
environment []serpent.EnvVar
}{
{
name: "NoDeliveryMethodSet",
environment: []serpent.EnvVar{},
expectNotificationsEnabled: false,
},
{
name: "SMTP_DeliveryMethodSet",
environment: []serpent.EnvVar{
{
Name: "CODER_EMAIL_SMARTHOST",
Value: "localhost:587",
},
},
expectNotificationsEnabled: true,
},
{
name: "Webhook_DeliveryMethodSet",
environment: []serpent.EnvVar{
{
Name: "CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT",
Value: "https://example.com/webhook",
},
},
expectNotificationsEnabled: true,
},
{
name: "WebhookAndSMTP_DeliveryMethodSet",
environment: []serpent.EnvVar{
{
Name: "CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT",
Value: "https://example.com/webhook",
},
{
Name: "CODER_EMAIL_SMARTHOST",
Value: "localhost:587",
},
},
expectNotificationsEnabled: true,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
dv := codersdk.DeploymentValues{}
opts := dv.Options()
err := opts.ParseEnv(tt.environment)
require.NoError(t, err)
require.Equal(t, tt.expectNotificationsEnabled, dv.Notifications.Enabled())
})
}
}
+7 -6
View File
@@ -74,7 +74,8 @@ flags.
Notifications can currently be delivered by either SMTP or webhook. Each message
can only be delivered to one method, and this method is configured globally with
[`CODER_NOTIFICATIONS_METHOD`](../../../reference/cli/server.md#--notifications-method)
(default: `smtp`).
(default: `smtp`). When there are no delivery methods configured, notifications
will be disabled.
Premium customers can configure which method to use for each of the supported
[Events](#workspace-events); see the
@@ -89,11 +90,11 @@ existing one.
**Server Settings:**
| Required | CLI | Env | Type | Description | Default |
| :------: | ------------------- | ----------------------- | ----------- | ----------------------------------------- | ------------- |
| ✔️ | `--email-from` | `CODER_EMAIL_FROM` | `string` | The sender's address to use. | |
| ✔️ | `--email-smarthost` | `CODER_EMAIL_SMARTHOST` | `host:port` | The SMTP relay to send messages through. | localhost:587 |
| ✔️ | `--email-hello` | `CODER_EMAIL_HELLO` | `string` | The hostname identifying the SMTP server. | localhost |
| Required | CLI | Env | Type | Description | Default |
| :------: | ------------------- | ----------------------- | -------- | ----------------------------------------- | --------- |
| ✔️ | `--email-from` | `CODER_EMAIL_FROM` | `string` | The sender's address to use. | |
| ✔️ | `--email-smarthost` | `CODER_EMAIL_SMARTHOST` | `string` | The SMTP relay to send messages |
| ✔️ | `--email-hello` | `CODER_EMAIL_HELLO` | `string` | The hostname identifying the SMTP server. | localhost |
**Authentication Settings:**
+1 -4
View File
@@ -266,10 +266,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \
"force_tls": true,
"from": "string",
"hello": "string",
"smarthost": {
"host": "string",
"port": "string"
},
"smarthost": "string",
"tls": {
"ca_file": "string",
"cert_file": "string",
+5 -17
View File
@@ -1884,10 +1884,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
"force_tls": true,
"from": "string",
"hello": "string",
"smarthost": {
"host": "string",
"port": "string"
},
"smarthost": "string",
"tls": {
"ca_file": "string",
"cert_file": "string",
@@ -2311,10 +2308,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
"force_tls": true,
"from": "string",
"hello": "string",
"smarthost": {
"host": "string",
"port": "string"
},
"smarthost": "string",
"tls": {
"ca_file": "string",
"cert_file": "string",
@@ -3417,10 +3411,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
"force_tls": true,
"from": "string",
"hello": "string",
"smarthost": {
"host": "string",
"port": "string"
},
"smarthost": "string",
"tls": {
"ca_file": "string",
"cert_file": "string",
@@ -3505,10 +3496,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
"force_tls": true,
"from": "string",
"hello": "string",
"smarthost": {
"host": "string",
"port": "string"
},
"smarthost": "string",
"tls": {
"ca_file": "string",
"cert_file": "string",
@@ -3528,7 +3516,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
| `force_tls` | boolean | false | | Force tls causes a TLS connection to be attempted. |
| `from` | string | false | | The sender's address. |
| `hello` | string | false | | The hostname identifying the SMTP server. |
| `smarthost` | [serpent.HostPort](#serpenthostport) | false | | The intermediary SMTP host through which emails are sent (host:port). |
| `smarthost` | string | false | | The intermediary SMTP host through which emails are sent (host:port). |
| `tls` | [codersdk.NotificationsEmailTLSConfig](#codersdknotificationsemailtlsconfig) | false | | Tls details. |
## codersdk.NotificationsEmailTLSConfig
+2 -3
View File
@@ -1231,10 +1231,9 @@ The sender's address to use.
| | |
| ----------- | ----------------------------------- |
| Type | <code>host:port</code> |
| Type | <code>string</code> |
| Environment | <code>$CODER_EMAIL_SMARTHOST</code> |
| YAML | <code>email.smarthost</code> |
| Default | <code>localhost:587</code> |
The intermediary SMTP host through which emails are sent.
@@ -1395,7 +1394,7 @@ The sender's address to use.
| | |
| ----------- | ------------------------------------------------- |
| Type | <code>host:port</code> |
| Type | <code>string</code> |
| Environment | <code>$CODER_NOTIFICATIONS_EMAIL_SMARTHOST</code> |
| YAML | <code>notifications.email.smarthost</code> |
+2 -2
View File
@@ -119,7 +119,7 @@ Configure how emails are sent.
--email-hello string, $CODER_EMAIL_HELLO (default: localhost)
The hostname identifying the SMTP server.
--email-smarthost host:port, $CODER_EMAIL_SMARTHOST (default: localhost:587)
--email-smarthost string, $CODER_EMAIL_SMARTHOST
The intermediary SMTP host through which emails are sent.
EMAIL / EMAIL AUTHENTICATION OPTIONS:
@@ -414,7 +414,7 @@ Configure how email notifications are sent.
The hostname identifying the SMTP server.
DEPRECATED: Use --email-hello instead.
--notifications-email-smarthost host:port, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
--notifications-email-smarthost string, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
The intermediary SMTP host through which emails are sent.
DEPRECATED: Use --email-smarthost instead.