diff --git a/cli/server.go b/cli/server.go index aa0a010eb0aa4..968ba66122075 100644 --- a/cli/server.go +++ b/cli/server.go @@ -897,31 +897,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) - } - options.NotificationsEnqueuer = enqueuer + if notificationsCfg.Enabled() { + metrics := notifications.NewMetrics(options.PrometheusRegistry) + helpers := templateHelpers(options) - // 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) - } + // 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)) + // 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() + // 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.") + } // Since errCh only has one buffered slot, all routines // sending on it must be wrapped in a select/default to @@ -1098,17 +1106,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 diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 7e4088ccb6212..6f524dff9a25a 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -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. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 38b2b68c24de1..72f39997b5734 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -524,8 +524,8 @@ email: # (default: , type: string) from: "" # The intermediary SMTP host through which emails are sent. - # (default: localhost:587, type: host:port) - smarthost: localhost:587 + # (default: , type: string) + smarthost: "" # The hostname identifying the SMTP server. # (default: localhost, type: string) hello: localhost @@ -577,8 +577,8 @@ notifications: # (default: , type: string) from: "" # The intermediary SMTP host through which emails are sent. - # (default: , type: host:port) - smarthost: localhost:587 + # (default: , type: string) + smarthost: "" # The hostname identifying the SMTP server. # (default: , type: string) hello: localhost diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 983abb61169c9..80d4a82914150 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11471,11 +11471,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.", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 67cc92c71331d..9abb0d3551bc1 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10296,11 +10296,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.", diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index dfb628b62eb86..14ce6b63b4e33 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -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 diff --git a/coderd/notifications/dispatch/smtp_test.go b/coderd/notifications/dispatch/smtp_test.go index c9a60b426ae70..b448dd2582e67 100644 --- a/coderd/notifications/dispatch/smtp_test.go +++ b/coderd/notifications/dispatch/smtp_test.go @@ -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")) diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index 763046bc20cb0..6ba8f614bd38d 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -155,7 +155,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"))) @@ -1113,7 +1113,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 @@ -1524,7 +1524,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), diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 0a97da7d3958f..7de961ec869f8 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -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", diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 61474a3b77ea1..7a84fcbbd831b 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -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()) + }) + } +} diff --git a/docs/admin/monitoring/notifications/index.md b/docs/admin/monitoring/notifications/index.md index eabc09438d7b9..a9e6a87d78139 100644 --- a/docs/admin/monitoring/notifications/index.md +++ b/docs/admin/monitoring/notifications/index.md @@ -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:** diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index b6452545842f7..c65d9ec4175b9 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -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", diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index fe8db822aafb5..d40fe8e240005 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -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 diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 3b3d2376c9aab..bdb338076c1a9 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1231,10 +1231,9 @@ The sender's address to use. | | | | ----------- | ----------------------------------- | -| Type | host:port | +| Type | string | | Environment | $CODER_EMAIL_SMARTHOST | | YAML | email.smarthost | -| Default | localhost:587 | The intermediary SMTP host through which emails are sent. @@ -1395,7 +1394,7 @@ The sender's address to use. | | | | ----------- | ------------------------------------------------- | -| Type | host:port | +| Type | string | | Environment | $CODER_NOTIFICATIONS_EMAIL_SMARTHOST | | YAML | notifications.email.smarthost | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index aaa4725c65181..8c12249a8bdd7 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -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.