chore: re-add default for smarthost

coder · DanielleMaywood · Nov 19, 2024 · Nov 12, 2024 · Nov 12, 2024 · Nov 12, 2024
commit d5aff283582d7a54754e44797a07bc8b084b1b14
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 string, $CODER_EMAIL_SMARTHOST
+      --email-smarthost host:port, $CODER_EMAIL_SMARTHOST (default: localhost:587)
           The intermediary SMTP host through which emails are sent.
 
 EMAIL / EMAIL AUTHENTICATION OPTIONS: 
@@ -416,7 +416,7 @@ Configure how email notifications are sent.
           The hostname identifying the SMTP server.
           DEPRECATED: Use --email-hello instead.
 
-      --notifications-email-smarthost string, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
+      --notifications-email-smarthost host:port, $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
@@ -535,8 +535,8 @@ notifications:
     # (default: <unset>, type: string)
     from: ""
     # The intermediary SMTP host through which emails are sent.
-    # (default: <unset>, type: string)
-    smarthost: ""
+    # (default: <unset>, type: host:port)
+    smarthost: localhost:587
     # The hostname identifying the SMTP server.
     # (default: localhost, type: string)
     hello: localhost
@@ -618,8 +618,8 @@ email:
   # (default: <unset>, type: string)
   from: ""
   # The intermediary SMTP host through which emails are sent.
-  # (default: <unset>, type: string)
-  smarthost: ""
+  # (default: localhost:587, type: host:port)
+  smarthost: localhost:587
   # The hostname identifying the SMTP server.
   # (default: localhost, type: string)
   hello: localhost

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,10 +34,11 @@ import (
 )
 
 var (
-	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")
+	ValidationNoFromAddressErr   = xerrors.New("'from' address not defined")
+	ValidationNoToAddressErr     = xerrors.New("'to' address(es) not 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")
 
 	//go:embed smtp/html.gotmpl
 	htmlTemplate string
@@ -520,14 +521,15 @@ func (s *SMTPHandler) validateToAddrs(to string) ([]string, error) {
 // Does not allow overriding.
 // nolint:revive // documented.
 func (s *SMTPHandler) smarthost() (string, string, error) {
-	hostport := strings.TrimSpace(s.cfg.Smarthost.String())
-	if hostport == "" {
-		return "", "", ValidationNoSmarthostErr
-	}
+	host := s.cfg.Smarthost.Host
+	port := s.cfg.Smarthost.Port
 
-	host, port, err := net.SplitHostPort(hostport)
-	if err != nil {
-		return "", "", xerrors.Errorf("split host port: %w", err)
+	// We don't validate the contents themselves; this will be done by the underlying SMTP library.
+	if host == "" {
+		return "", "", ValidationNoSmarthostHostErr
+	}
+	if port == "" {
+		return "", "", ValidationNoSmarthostPortErr
 	}
 
 	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 = serpent.String(hp.String())
+			tc.cfg.Smarthost = hp
 
 			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.String(fmt.Sprintf("localhost:%d", mockSMTPSrv.PortNumber())),
+		Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%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 = serpent.String(hp.String())
+				smtpConfig.Smarthost = hp
 
 				// 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.String(fmt.Sprintf("localhost:%d", mockSMTPSrv.PortNumber())),
+		Smarthost: serpent.HostPort{Host: "localhost", Port: fmt.Sprintf("%d", mockSMTPSrv.PortNumber())},
 	}
 	cfg.Webhook = codersdk.NotificationsWebhookConfig{
 		Endpoint: *serpent.URLOf(endpoint),

diff --git a/codersdk/deployment.go b/codersdk/deployment.go
@@ -692,7 +692,7 @@ 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.String `json:"smarthost" typescript:",notnull"`
+	Smarthost serpent.HostPort `json:"smarthost" typescript:",notnull"`
 	// The hostname identifying the SMTP server.
 	Hello serpent.String `json:"hello" typescript:",notnull"`
 
@@ -1030,6 +1030,7 @@ 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/docs/reference/api/general.md b/docs/reference/api/general.md
diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md
diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md
diff --git 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 string, $CODER_EMAIL_SMARTHOST
+      --email-smarthost host:port, $CODER_EMAIL_SMARTHOST (default: localhost:587)
           The intermediary SMTP host through which emails are sent.
 
 EMAIL / EMAIL AUTHENTICATION OPTIONS: 
@@ -417,7 +417,7 @@ Configure how email notifications are sent.
           The hostname identifying the SMTP server.
           DEPRECATED: Use --email-hello instead.
 
-      --notifications-email-smarthost string, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
+      --notifications-email-smarthost host:port, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST
           The intermediary SMTP host through which emails are sent.
           DEPRECATED: Use --email-smarthost instead.