coder · DanielleMaywood · Nov 12, 2024 · Nov 12, 2024 · Nov 12, 2024 · Nov 12, 2024
diff --git a/cli/server.go b/cli/server.go
@@ -897,31 +897,37 @@ 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 (
+				cfg                  = 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 cfg.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(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
 
-			// nolint:gocritic // We need to run the manager in a notifier context.
-			notificationsManager.Run(dbauthz.AsNotifier(ctx))
+				// 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()
+				// 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
@@ -1098,17 +1104,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
@@ -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: 
@@ -392,6 +392,9 @@ Configure how notifications are processed and delivered.
       --notifications-dispatch-timeout duration, $CODER_NOTIFICATIONS_DISPATCH_TIMEOUT (default: 1m0s)
           How long to wait while a notification is being sent before giving up.
 
+      --notifications-enabled bool, $CODER_NOTIFICATIONS_ENABLED (default: true)
+          Controls if notifications are enabled.
+
       --notifications-max-send-attempts int, $CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS (default: 5)
           The upper limit of attempts to send a notification.
 
@@ -409,11 +412,11 @@ Configure how email notifications are sent.
           The sender's address to use.
           DEPRECATED: Use --email-from instead.
 
-      --notifications-email-hello string, $CODER_NOTIFICATIONS_EMAIL_HELLO
+      --notifications-email-hello string, $CODER_NOTIFICATIONS_EMAIL_HELLO (default: localhost)
           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
@@ -518,53 +518,11 @@ userQuietHoursSchedule:
 # compatibility reasons, this will be removed in a future release.
 # (default: false, type: bool)
 allowWorkspaceRenames: false
-# Configure how emails are sent.
-email:
-  # The sender's address to use.
-  # (default: <unset>, type: string)
-  from: ""
-  # The intermediary SMTP host through which emails are sent.
-  # (default: localhost:587, type: host:port)
-  smarthost: localhost:587
-  # The hostname identifying the SMTP server.
-  # (default: localhost, type: string)
-  hello: localhost
-  # Force a TLS connection to the configured SMTP smarthost.
-  # (default: false, type: bool)
-  forceTLS: false
-  # Configure SMTP authentication options.
-  emailAuth:
-    # Identity to use with PLAIN authentication.
-    # (default: <unset>, type: string)
-    identity: ""
-    # Username to use with PLAIN/LOGIN authentication.
-    # (default: <unset>, type: string)
-    username: ""
-    # File from which to load password for use with PLAIN/LOGIN authentication.
-    # (default: <unset>, type: string)
-    passwordFile: ""
-  # Configure TLS for your SMTP server target.
-  emailTLS:
-    # Enable STARTTLS to upgrade insecure SMTP connections using TLS.
-    # (default: <unset>, type: bool)
-    startTLS: false
-    # Server name to verify against the target certificate.
-    # (default: <unset>, type: string)
-    serverName: ""
-    # Skip verification of the target server's certificate (insecure).
-    # (default: <unset>, type: bool)
-    insecureSkipVerify: false
-    # CA certificate file to use.
-    # (default: <unset>, type: string)
-    caCertFile: ""
-    # Certificate file to use.
-    # (default: <unset>, type: string)
-    certFile: ""
-    # Certificate key file to use.
-    # (default: <unset>, type: string)
-    certKeyFile: ""
 # Configure how notifications are processed and delivered.
 notifications:
+  # Controls if notifications are enabled.
+  # (default: true, type: bool)
+  enabled: true
   # Which delivery method to use (available options: 'smtp', 'webhook').
   # (default: smtp, type: string)
   method: smtp
@@ -577,10 +535,10 @@ notifications:
     # (default: <unset>, type: string)
     from: ""
     # The intermediary SMTP host through which emails are sent.
-    # (default: <unset>, type: host:port)
-    smarthost: localhost:587
-    # The hostname identifying the SMTP server.
     # (default: <unset>, type: string)
+    smarthost: ""
+    # The hostname identifying the SMTP server.
+    # (default: localhost, type: string)
     hello: localhost
     # Force a TLS connection to the configured SMTP smarthost.
     # (default: <unset>, type: bool)
@@ -654,3 +612,48 @@ notifications:
   # How often to query the database for queued notifications.
   # (default: 15s, type: duration)
   fetchInterval: 15s
+# Configure how emails are sent.
+email:
+  # The sender's address to use.
+  # (default: <unset>, type: string)
+  from: ""
+  # The intermediary SMTP host through which emails are sent.
+  # (default: <unset>, type: string)
+  smarthost: ""
+  # The hostname identifying the SMTP server.
+  # (default: localhost, type: string)
+  hello: localhost
+  # Force a TLS connection to the configured SMTP smarthost.
+  # (default: false, type: bool)
+  forceTLS: false
+  # Configure SMTP authentication options.
+  emailAuth:
+    # Identity to use with PLAIN authentication.
+    # (default: <unset>, type: string)
+    identity: ""
+    # Username to use with PLAIN/LOGIN authentication.
+    # (default: <unset>, type: string)
+    username: ""
+    # File from which to load password for use with PLAIN/LOGIN authentication.
+    # (default: <unset>, type: string)
+    passwordFile: ""
+  # Configure TLS for your SMTP server target.
+  emailTLS:
+    # Enable STARTTLS to upgrade insecure SMTP connections using TLS.
+    # (default: <unset>, type: bool)
+    startTLS: false
+    # Server name to verify against the target certificate.
+    # (default: <unset>, type: string)
+    serverName: ""
+    # Skip verification of the target server's certificate (insecure).
+    # (default: <unset>, type: bool)
+    insecureSkipVerify: false
+    # CA certificate file to use.
+    # (default: <unset>, type: string)
+    caCertFile: ""
+    # Certificate file to use.
+    # (default: <unset>, type: string)
+    certFile: ""
+    # Certificate key file to use.
+    # (default: <unset>, type: string)
+    certKeyFile: ""
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git 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' 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
+	hostport := strings.TrimSpace(s.cfg.Smarthost.String())
+	if hostport == "" {
+		return "", "", ValidationNoSmarthostErr
 	}
-	if port == "" {
-		return "", "", ValidationNoSmarthostPortErr
+
+	host, port, err := net.SplitHostPort(hostport)
+	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
@@ -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
@@ -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),