From 5c36c93ee603a11867659031bca1530e6387aca5 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 12 Nov 2024 12:06:23 +0000 Subject: [PATCH 01/10] fix: remove defaults for CODER_EMAIL_ options --- cli/testdata/coder_server_--help.golden | 8 ++++---- cli/testdata/server-config.yaml.golden | 16 ++++++++-------- coderd/notifications/dispatch/smtp.go | 12 +++--------- coderd/notifications/notifications_test.go | 6 +++--- codersdk/deployment.go | 5 +---- .../cli/testdata/coder_server_--help.golden | 8 ++++---- 6 files changed, 23 insertions(+), 32 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 7e4088ccb6212..fb34a8d285533 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -109,16 +109,16 @@ Use a YAML configuration file when your server launch become unwieldy. EMAIL OPTIONS: Configure how emails are sent. - --email-force-tls bool, $CODER_EMAIL_FORCE_TLS (default: false) + --email-force-tls bool, $CODER_EMAIL_FORCE_TLS Force a TLS connection to the configured SMTP smarthost. --email-from string, $CODER_EMAIL_FROM The sender's address to use. - --email-hello string, $CODER_EMAIL_HELLO (default: localhost) + --email-hello string, $CODER_EMAIL_HELLO 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..8ab9f4da33549 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -524,13 +524,13 @@ 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 + # (default: , type: string) + hello: "" # Force a TLS connection to the configured SMTP smarthost. - # (default: false, type: bool) + # (default: , type: bool) forceTLS: false # Configure SMTP authentication options. emailAuth: @@ -577,11 +577,11 @@ 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 + hello: "" # Force a TLS connection to the configured SMTP smarthost. # (default: , type: bool) forceTLS: false diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index dfb628b62eb86..9944a128e46a2 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -521,15 +521,9 @@ 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 - } - if port == "" { - return "", "", ValidationNoSmarthostPortErr + host, port, err := net.SplitHostPort(s.cfg.Smarthost.String()) + if err != nil { + return "", "", fmt.Errorf("split host port: %w", err) } return host, port, nil 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 5959c35fa9d95..cc1a9be683039 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -690,7 +690,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.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 +1028,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", @@ -1038,7 +1037,6 @@ when required by your organization's security policy.`, Description: "The hostname identifying the SMTP server.", Flag: "email-hello", Env: "CODER_EMAIL_HELLO", - Default: "localhost", Value: &c.Notifications.SMTP.Hello, Group: &deploymentGroupEmail, YAML: "hello", @@ -1048,7 +1046,6 @@ when required by your organization's security policy.`, Description: "Force a TLS connection to the configured SMTP smarthost.", Flag: "email-force-tls", Env: "CODER_EMAIL_FORCE_TLS", - Default: "false", Value: &c.Notifications.SMTP.ForceTLS, Group: &deploymentGroupEmail, YAML: "forceTLS", diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index aaa4725c65181..1cf58a9578a8b 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -110,16 +110,16 @@ Use a YAML configuration file when your server launch become unwieldy. EMAIL OPTIONS: Configure how emails are sent. - --email-force-tls bool, $CODER_EMAIL_FORCE_TLS (default: false) + --email-force-tls bool, $CODER_EMAIL_FORCE_TLS Force a TLS connection to the configured SMTP smarthost. --email-from string, $CODER_EMAIL_FROM The sender's address to use. - --email-hello string, $CODER_EMAIL_HELLO (default: localhost) + --email-hello string, $CODER_EMAIL_HELLO 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. From 196f53831348987f4cee4dc2581eafdf0f9eb8fe Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 12 Nov 2024 12:23:08 +0000 Subject: [PATCH 02/10] fix: failing tests --- coderd/apidoc/docs.go | 6 +----- coderd/apidoc/swagger.json | 6 +----- coderd/notifications/dispatch/smtp.go | 13 ++++++++----- docs/reference/api/general.md | 5 +---- docs/reference/api/schemas.md | 22 +++++----------------- docs/reference/cli/server.md | 7 ++----- 6 files changed, 18 insertions(+), 41 deletions(-) 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 9944a128e46a2..57804a851c628 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("no 'from' address defined") + ValidationNoToAddressErr = xerrors.New("no 'to' address(es) defined") + ValidationNoSmarthostErr = xerrors.New("no 'smarthost' defined") + ValidationNoHelloErr = xerrors.New("'hello' not defined") //go:embed smtp/html.gotmpl htmlTemplate string @@ -521,6 +520,10 @@ func (s *SMTPHandler) validateToAddrs(to string) ([]string, error) { // Does not allow overriding. // nolint:revive // documented. func (s *SMTPHandler) smarthost() (string, string, error) { + if s.cfg.Smarthost.String() == "" { + return "", "", ValidationNoSmarthostErr + } + host, port, err := net.SplitHostPort(s.cfg.Smarthost.String()) if err != nil { return "", "", fmt.Errorf("split host port: %w", err) 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..7a18d67c01069 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. @@ -1245,7 +1244,6 @@ The intermediary SMTP host through which emails are sent. | Type | string | | Environment | $CODER_EMAIL_HELLO | | YAML | email.hello | -| Default | localhost | The hostname identifying the SMTP server. @@ -1256,7 +1254,6 @@ The hostname identifying the SMTP server. | Type | bool | | Environment | $CODER_EMAIL_FORCE_TLS | | YAML | email.forceTLS | -| Default | false | Force a TLS connection to the configured SMTP smarthost. @@ -1395,7 +1392,7 @@ The sender's address to use. | | | | ----------- | ------------------------------------------------- | -| Type | host:port | +| Type | string | | Environment | $CODER_NOTIFICATIONS_EMAIL_SMARTHOST | | YAML | notifications.email.smarthost | From b6700098bc9067c25232e9b9b2ff0d10ec1ac143 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 12 Nov 2024 12:57:19 +0000 Subject: [PATCH 03/10] fix: tests (again) --- cli/testdata/server-config.yaml.golden | 90 +++++++++++----------- coderd/notifications/dispatch/smtp.go | 2 +- coderd/notifications/dispatch/smtp_test.go | 2 +- codersdk/deployment.go | 30 ++++---- codersdk/deployment_test.go | 88 +++++++++++++++++++++ 5 files changed, 150 insertions(+), 62 deletions(-) diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 8ab9f4da33549..a1fd70a9581db 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -518,51 +518,6 @@ 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: , type: string) - from: "" - # The intermediary SMTP host through which emails are sent. - # (default: , type: string) - smarthost: "" - # The hostname identifying the SMTP server. - # (default: , type: string) - hello: "" - # Force a TLS connection to the configured SMTP smarthost. - # (default: , type: bool) - forceTLS: false - # Configure SMTP authentication options. - emailAuth: - # Identity to use with PLAIN authentication. - # (default: , type: string) - identity: "" - # Username to use with PLAIN/LOGIN authentication. - # (default: , type: string) - username: "" - # File from which to load password for use with PLAIN/LOGIN authentication. - # (default: , type: string) - passwordFile: "" - # Configure TLS for your SMTP server target. - emailTLS: - # Enable STARTTLS to upgrade insecure SMTP connections using TLS. - # (default: , type: bool) - startTLS: false - # Server name to verify against the target certificate. - # (default: , type: string) - serverName: "" - # Skip verification of the target server's certificate (insecure). - # (default: , type: bool) - insecureSkipVerify: false - # CA certificate file to use. - # (default: , type: string) - caCertFile: "" - # Certificate file to use. - # (default: , type: string) - certFile: "" - # Certificate key file to use. - # (default: , type: string) - certKeyFile: "" # Configure how notifications are processed and delivered. notifications: # Which delivery method to use (available options: 'smtp', 'webhook'). @@ -654,3 +609,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: , type: string) + from: "" + # The intermediary SMTP host through which emails are sent. + # (default: , type: string) + smarthost: "" + # The hostname identifying the SMTP server. + # (default: , type: string) + hello: "" + # Force a TLS connection to the configured SMTP smarthost. + # (default: , type: bool) + forceTLS: false + # Configure SMTP authentication options. + emailAuth: + # Identity to use with PLAIN authentication. + # (default: , type: string) + identity: "" + # Username to use with PLAIN/LOGIN authentication. + # (default: , type: string) + username: "" + # File from which to load password for use with PLAIN/LOGIN authentication. + # (default: , type: string) + passwordFile: "" + # Configure TLS for your SMTP server target. + emailTLS: + # Enable STARTTLS to upgrade insecure SMTP connections using TLS. + # (default: , type: bool) + startTLS: false + # Server name to verify against the target certificate. + # (default: , type: string) + serverName: "" + # Skip verification of the target server's certificate (insecure). + # (default: , type: bool) + insecureSkipVerify: false + # CA certificate file to use. + # (default: , type: string) + caCertFile: "" + # Certificate file to use. + # (default: , type: string) + certFile: "" + # Certificate key file to use. + # (default: , type: string) + certKeyFile: "" diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 57804a851c628..0badc604371f5 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -526,7 +526,7 @@ func (s *SMTPHandler) smarthost() (string, string, error) { host, port, err := net.SplitHostPort(s.cfg.Smarthost.String()) if err != nil { - return "", "", fmt.Errorf("split host port: %w", err) + 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/codersdk/deployment.go b/codersdk/deployment.go index cc1a9be683039..39cc9ad1c4d83 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2578,21 +2578,6 @@ Write out the current server config as YAML to stdout.`, YAML: "thresholdDatabase", Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), }, - // Email options - emailFrom, - emailSmarthost, - emailHello, - emailForceTLS, - emailAuthIdentity, - emailAuthUsername, - emailAuthPassword, - emailAuthPasswordFile, - emailTLSStartTLS, - emailTLSServerName, - emailTLSSkipCertVerify, - emailTLSCertAuthorityFile, - emailTLSCertFile, - emailTLSCertKeyFile, // Notifications Options { Name: "Notifications: Method", @@ -2854,6 +2839,21 @@ Write out the current server config as YAML to stdout.`, Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), Hidden: true, // Hidden because most operators should not need to modify this. }, + // Email options + emailFrom, + emailSmarthost, + emailHello, + emailForceTLS, + emailAuthIdentity, + emailAuthUsername, + emailAuthPassword, + emailAuthPasswordFile, + emailTLSStartTLS, + emailTLSServerName, + emailTLSSkipCertVerify, + emailTLSCertAuthorityFile, + emailTLSCertFile, + emailTLSCertKeyFile, } return opts diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 61474a3b77ea1..6ab3f859f0474 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -568,3 +568,91 @@ 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 TestEmailValuesTakeCorrectPrecedent(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + envs []serpent.EnvVar + oldEnv string + newEnv string + expectedValue string + }{ + { + name: "CODER_NOTIFICATIONS_EMAIL_SMARTHOST is not discarded", + envs: []serpent.EnvVar{ + { + Name: "CODER_NOTIFICATIONS_EMAIL_SMARTHOST", + Value: "localhost:999", + }, + }, + oldEnv: "CODER_NOTIFICATIONS_EMAIL_SMARTHOST", + newEnv: "CODER_EMAIL_SMARTHOST", + expectedValue: "localhost:999", + }, + { + name: "CODER_EMAIL_SMARTHOST is not discarded", + envs: []serpent.EnvVar{ + { + Name: "CODER_EMAIL_SMARTHOST", + Value: "localhost:999", + }, + }, + oldEnv: "CODER_NOTIFICATIONS_EMAIL_SMARTHOST", + newEnv: "CODER_EMAIL_SMARTHOST", + expectedValue: "localhost:999", + }, + { + name: "CODER_EMAIL_SMARTHOST is prioritized", + envs: []serpent.EnvVar{ + { + Name: "CODER_NOTIFICATIONS_EMAIL_SMARTHOST", + Value: "localhost:999", + }, + { + Name: "CODER_EMAIL_SMARTHOST", + Value: "localhost:1000", + }, + }, + oldEnv: "CODER_NOTIFICATIONS_EMAIL_SMARTHOST", + newEnv: "CODER_EMAIL_SMARTHOST", + expectedValue: "localhost:1000", + }, + } + + 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.envs) + require.NoError(t, err) + + err = opts.SetDefaults() + require.NoError(t, err) + + var oldEnvValue string + var newEnvValue string + + for _, opt := range opts { + switch { + case opt.Env == tt.oldEnv: + oldEnvValue = opt.Value.String() + case opt.Env == tt.newEnv: + newEnvValue = opt.Value.String() + } + + if oldEnvValue != "" && newEnvValue != "" { + break + } + } + + require.Equal(t, tt.expectedValue, oldEnvValue) + require.Equal(t, tt.expectedValue, newEnvValue) + }) + } +} From 19398cce26355d0373587d2a3ed49b68c1bf698d Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 12 Nov 2024 13:02:26 +0000 Subject: [PATCH 04/10] fix: run 'make gen' --- docs/reference/cli/server.md | 278 +++++++++++++++++------------------ 1 file changed, 139 insertions(+), 139 deletions(-) diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 7a18d67c01069..c0f6f172299d3 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1217,145 +1217,6 @@ Refresh interval for healthchecks. The threshold for the database health check. If the median latency of the database exceeds this threshold over 5 attempts, the database is considered unhealthy. The default value is 15ms. -### --email-from - -| | | -| ----------- | ------------------------------ | -| Type | string | -| Environment | $CODER_EMAIL_FROM | -| YAML | email.from | - -The sender's address to use. - -### --email-smarthost - -| | | -| ----------- | ----------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_SMARTHOST | -| YAML | email.smarthost | - -The intermediary SMTP host through which emails are sent. - -### --email-hello - -| | | -| ----------- | ------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_HELLO | -| YAML | email.hello | - -The hostname identifying the SMTP server. - -### --email-force-tls - -| | | -| ----------- | ----------------------------------- | -| Type | bool | -| Environment | $CODER_EMAIL_FORCE_TLS | -| YAML | email.forceTLS | - -Force a TLS connection to the configured SMTP smarthost. - -### --email-auth-identity - -| | | -| ----------- | --------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_AUTH_IDENTITY | -| YAML | email.emailAuth.identity | - -Identity to use with PLAIN authentication. - -### --email-auth-username - -| | | -| ----------- | --------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_AUTH_USERNAME | -| YAML | email.emailAuth.username | - -Username to use with PLAIN/LOGIN authentication. - -### --email-auth-password - -| | | -| ----------- | --------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_AUTH_PASSWORD | - -Password to use with PLAIN/LOGIN authentication. - -### --email-auth-password-file - -| | | -| ----------- | -------------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_AUTH_PASSWORD_FILE | -| YAML | email.emailAuth.passwordFile | - -File from which to load password for use with PLAIN/LOGIN authentication. - -### --email-tls-starttls - -| | | -| ----------- | -------------------------------------- | -| Type | bool | -| Environment | $CODER_EMAIL_TLS_STARTTLS | -| YAML | email.emailTLS.startTLS | - -Enable STARTTLS to upgrade insecure SMTP connections using TLS. - -### --email-tls-server-name - -| | | -| ----------- | ---------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_TLS_SERVERNAME | -| YAML | email.emailTLS.serverName | - -Server name to verify against the target certificate. - -### --email-tls-skip-verify - -| | | -| ----------- | ---------------------------------------------- | -| Type | bool | -| Environment | $CODER_EMAIL_TLS_SKIPVERIFY | -| YAML | email.emailTLS.insecureSkipVerify | - -Skip verification of the target server's certificate (insecure). - -### --email-tls-ca-cert-file - -| | | -| ----------- | ---------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_TLS_CACERTFILE | -| YAML | email.emailTLS.caCertFile | - -CA certificate file to use. - -### --email-tls-cert-file - -| | | -| ----------- | -------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_TLS_CERTFILE | -| YAML | email.emailTLS.certFile | - -Certificate file to use. - -### --email-tls-cert-key-file - -| | | -| ----------- | ----------------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_TLS_CERTKEYFILE | -| YAML | email.emailTLS.certKeyFile | - -Certificate key file to use. - ### --notifications-method | | | @@ -1537,3 +1398,142 @@ The endpoint to which to send webhooks. | Default | 5 | The upper limit of attempts to send a notification. + +### --email-from + +| | | +| ----------- | ------------------------------ | +| Type | string | +| Environment | $CODER_EMAIL_FROM | +| YAML | email.from | + +The sender's address to use. + +### --email-smarthost + +| | | +| ----------- | ----------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_SMARTHOST | +| YAML | email.smarthost | + +The intermediary SMTP host through which emails are sent. + +### --email-hello + +| | | +| ----------- | ------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_HELLO | +| YAML | email.hello | + +The hostname identifying the SMTP server. + +### --email-force-tls + +| | | +| ----------- | ----------------------------------- | +| Type | bool | +| Environment | $CODER_EMAIL_FORCE_TLS | +| YAML | email.forceTLS | + +Force a TLS connection to the configured SMTP smarthost. + +### --email-auth-identity + +| | | +| ----------- | --------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_AUTH_IDENTITY | +| YAML | email.emailAuth.identity | + +Identity to use with PLAIN authentication. + +### --email-auth-username + +| | | +| ----------- | --------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_AUTH_USERNAME | +| YAML | email.emailAuth.username | + +Username to use with PLAIN/LOGIN authentication. + +### --email-auth-password + +| | | +| ----------- | --------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_AUTH_PASSWORD | + +Password to use with PLAIN/LOGIN authentication. + +### --email-auth-password-file + +| | | +| ----------- | -------------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_AUTH_PASSWORD_FILE | +| YAML | email.emailAuth.passwordFile | + +File from which to load password for use with PLAIN/LOGIN authentication. + +### --email-tls-starttls + +| | | +| ----------- | -------------------------------------- | +| Type | bool | +| Environment | $CODER_EMAIL_TLS_STARTTLS | +| YAML | email.emailTLS.startTLS | + +Enable STARTTLS to upgrade insecure SMTP connections using TLS. + +### --email-tls-server-name + +| | | +| ----------- | ---------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_TLS_SERVERNAME | +| YAML | email.emailTLS.serverName | + +Server name to verify against the target certificate. + +### --email-tls-skip-verify + +| | | +| ----------- | ---------------------------------------------- | +| Type | bool | +| Environment | $CODER_EMAIL_TLS_SKIPVERIFY | +| YAML | email.emailTLS.insecureSkipVerify | + +Skip verification of the target server's certificate (insecure). + +### --email-tls-ca-cert-file + +| | | +| ----------- | ---------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_TLS_CACERTFILE | +| YAML | email.emailTLS.caCertFile | + +CA certificate file to use. + +### --email-tls-cert-file + +| | | +| ----------- | -------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_TLS_CERTFILE | +| YAML | email.emailTLS.certFile | + +Certificate file to use. + +### --email-tls-cert-key-file + +| | | +| ----------- | ----------------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_TLS_CERTKEYFILE | +| YAML | email.emailTLS.certKeyFile | + +Certificate key file to use. From 4175bc8d562c5bfbe2e16835aca59a1faf3989af Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 13 Nov 2024 10:35:33 +0000 Subject: [PATCH 05/10] fix: consistency Co-authored-by: Danny Kopping --- coderd/notifications/dispatch/smtp.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 0badc604371f5..628c658b6ecf5 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -34,9 +34,9 @@ import ( ) var ( - ValidationNoFromAddressErr = xerrors.New("no 'from' address defined") - ValidationNoToAddressErr = xerrors.New("no 'to' address(es) defined") - ValidationNoSmarthostErr = xerrors.New("no 'smarthost' 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 From 66d1a6943d6e17f7b4627d2d4cafd1b2edf8fb01 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 13 Nov 2024 10:44:56 +0000 Subject: [PATCH 06/10] nit: trim smarthost --- coderd/notifications/dispatch/smtp.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 628c658b6ecf5..d2dc7cdf20872 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -520,11 +520,12 @@ func (s *SMTPHandler) validateToAddrs(to string) ([]string, error) { // Does not allow overriding. // nolint:revive // documented. func (s *SMTPHandler) smarthost() (string, string, error) { - if s.cfg.Smarthost.String() == "" { + hostport := strings.TrimSpace(s.cfg.Smarthost.String()) + if hostport == "" { return "", "", ValidationNoSmarthostErr } - host, port, err := net.SplitHostPort(s.cfg.Smarthost.String()) + host, port, err := net.SplitHostPort(hostport) if err != nil { return "", "", xerrors.Errorf("split host port: %w", err) } From 4689e5bb4c14a953002a9dbbb5879c35d54be058 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 13 Nov 2024 14:04:27 +0000 Subject: [PATCH 07/10] nit: re-add default for CODER_EMAIL_HELLO temporarily using new branch from coder/serpent for this --- codersdk/deployment.go | 2 ++ codersdk/deployment_test.go | 24 ++++++++++++++++++++++++ go.mod | 2 +- go.sum | 2 ++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 39cc9ad1c4d83..88a2b2fbf1a05 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1037,6 +1037,7 @@ when required by your organization's security policy.`, Description: "The hostname identifying the SMTP server.", Flag: "email-hello", Env: "CODER_EMAIL_HELLO", + Default: "localhost", Value: &c.Notifications.SMTP.Hello, Group: &deploymentGroupEmail, YAML: "hello", @@ -2625,6 +2626,7 @@ Write out the current server config as YAML to stdout.`, Description: "The hostname identifying the SMTP server.", Flag: "notifications-email-hello", Env: "CODER_NOTIFICATIONS_EMAIL_HELLO", + Default: "localhost", Value: &c.Notifications.SMTP.Hello, Group: &deploymentGroupNotificationsEmail, YAML: "hello", diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 6ab3f859f0474..4878c6ac691b7 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -619,6 +619,30 @@ func TestEmailValuesTakeCorrectPrecedent(t *testing.T) { newEnv: "CODER_EMAIL_SMARTHOST", expectedValue: "localhost:1000", }, + { + name: "CODER_NOTIFICATIONS_EMAIL_HELLO is not discarded", + envs: []serpent.EnvVar{ + { + Name: "CODER_NOTIFICATIONS_EMAIL_HELLO", + Value: "not-localhost", + }, + }, + oldEnv: "CODER_NOTIFICATIONS_EMAIL_HELLO", + newEnv: "CODER_EMAIL_HELLO", + expectedValue: "not-localhost", + }, + { + name: "CODER_EMAIL_HELLO is not discarded", + envs: []serpent.EnvVar{ + { + Name: "CODER_EMAIL_HELLO", + Value: "not-localhost", + }, + }, + oldEnv: "CODER_NOTIFICATIONS_EMAIL_HELLO", + newEnv: "CODER_EMAIL_HELLO", + expectedValue: "not-localhost", + }, } for _, tt := range tests { diff --git a/go.mod b/go.mod index fc1a1c2c35cfd..87a8e885ea610 100644 --- a/go.mod +++ b/go.mod @@ -204,7 +204,7 @@ require ( github.com/charmbracelet/bubbles v0.20.0 github.com/charmbracelet/bubbletea v1.2.1 github.com/charmbracelet/lipgloss v1.0.0 - github.com/coder/serpent v0.8.0 + github.com/coder/serpent v0.8.1-0.20241113113009-ad8fe148f0d8 github.com/emersion/go-sasl v0.0.0-20200509203442-7bfe0ed36a21 github.com/emersion/go-smtp v0.21.2 github.com/go-jose/go-jose/v4 v4.0.2 diff --git a/go.sum b/go.sum index 9429161c83051..b6eea453a0f1b 100644 --- a/go.sum +++ b/go.sum @@ -230,6 +230,8 @@ github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= github.com/coder/retry v1.5.1/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY= github.com/coder/serpent v0.8.0 h1:6OR+k6fekhSeEDmwwzBgnSjaa7FfGGrMlc3GoAEH9dg= github.com/coder/serpent v0.8.0/go.mod h1:cZFW6/fP+kE9nd/oRkEHJpG6sXCtQ+AX7WMMEHv0Y3Q= +github.com/coder/serpent v0.8.1-0.20241113113009-ad8fe148f0d8 h1:xBsCXOg2KBd5eq2uJFjXjVEJCZn6QaflS7qgzzipqZE= +github.com/coder/serpent v0.8.1-0.20241113113009-ad8fe148f0d8/go.mod h1:cZFW6/fP+kE9nd/oRkEHJpG6sXCtQ+AX7WMMEHv0Y3Q= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuOD6a/zVP3rcxezNsoDseTUw= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788/go.mod h1:aGQbuCLyhRLMzZF067xc84Lh7JDs1FKwCmF1Crl9dxQ= github.com/coder/tailscale v1.1.1-0.20241003034647-02286e537fc2 h1:mBbPFyJ2i9o490IwWGvWgtG0qmvIk45R7GWJpoaXotI= From 01d4957c50cb6d51a9673a4115d742501e6f996d Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 13 Nov 2024 14:10:59 +0000 Subject: [PATCH 08/10] fix: run 'make {update-golden-files,gen}' --- cli/testdata/coder_server_--help.golden | 4 ++-- cli/testdata/server-config.yaml.golden | 8 ++++---- docs/reference/cli/server.md | 2 ++ enterprise/cli/testdata/coder_server_--help.golden | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index fb34a8d285533..097a8fdeabf04 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -115,7 +115,7 @@ Configure how emails are sent. --email-from string, $CODER_EMAIL_FROM The sender's address to use. - --email-hello string, $CODER_EMAIL_HELLO + --email-hello string, $CODER_EMAIL_HELLO (default: localhost) The hostname identifying the SMTP server. --email-smarthost string, $CODER_EMAIL_SMARTHOST @@ -409,7 +409,7 @@ 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. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index a1fd70a9581db..400a1f8a8d82e 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -535,8 +535,8 @@ notifications: # (default: , type: string) smarthost: "" # The hostname identifying the SMTP server. - # (default: , type: string) - hello: "" + # (default: localhost, type: string) + hello: localhost # Force a TLS connection to the configured SMTP smarthost. # (default: , type: bool) forceTLS: false @@ -618,8 +618,8 @@ email: # (default: , type: string) smarthost: "" # The hostname identifying the SMTP server. - # (default: , type: string) - hello: "" + # (default: localhost, type: string) + hello: localhost # Force a TLS connection to the configured SMTP smarthost. # (default: , type: bool) forceTLS: false diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index c0f6f172299d3..9ff167d7752a0 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1266,6 +1266,7 @@ The intermediary SMTP host through which emails are sent. | Type | string | | Environment | $CODER_NOTIFICATIONS_EMAIL_HELLO | | YAML | notifications.email.hello | +| Default | localhost | The hostname identifying the SMTP server. @@ -1426,6 +1427,7 @@ The intermediary SMTP host through which emails are sent. | Type | string | | Environment | $CODER_EMAIL_HELLO | | YAML | email.hello | +| Default | localhost | The hostname identifying the SMTP server. diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 1cf58a9578a8b..114539b1bbecc 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -116,7 +116,7 @@ Configure how emails are sent. --email-from string, $CODER_EMAIL_FROM The sender's address to use. - --email-hello string, $CODER_EMAIL_HELLO + --email-hello string, $CODER_EMAIL_HELLO (default: localhost) The hostname identifying the SMTP server. --email-smarthost string, $CODER_EMAIL_SMARTHOST @@ -410,7 +410,7 @@ 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. From 27fa5bf25f9768d1e8b4caffc4cf68536a2bfd33 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 13 Nov 2024 14:22:39 +0000 Subject: [PATCH 09/10] fix: run 'make fmt' --- go.sum | 2 -- 1 file changed, 2 deletions(-) diff --git a/go.sum b/go.sum index b6eea453a0f1b..0909e18b33b6b 100644 --- a/go.sum +++ b/go.sum @@ -228,8 +228,6 @@ github.com/coder/quartz v0.1.2 h1:PVhc9sJimTdKd3VbygXtS4826EOCpB1fXoRlLnCrE+s= github.com/coder/quartz v0.1.2/go.mod h1:vsiCc+AHViMKH2CQpGIpFgdHIEQsxwm8yCscqKmzbRA= github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= github.com/coder/retry v1.5.1/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY= -github.com/coder/serpent v0.8.0 h1:6OR+k6fekhSeEDmwwzBgnSjaa7FfGGrMlc3GoAEH9dg= -github.com/coder/serpent v0.8.0/go.mod h1:cZFW6/fP+kE9nd/oRkEHJpG6sXCtQ+AX7WMMEHv0Y3Q= github.com/coder/serpent v0.8.1-0.20241113113009-ad8fe148f0d8 h1:xBsCXOg2KBd5eq2uJFjXjVEJCZn6QaflS7qgzzipqZE= github.com/coder/serpent v0.8.1-0.20241113113009-ad8fe148f0d8/go.mod h1:cZFW6/fP+kE9nd/oRkEHJpG6sXCtQ+AX7WMMEHv0Y3Q= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuOD6a/zVP3rcxezNsoDseTUw= From 6ca784459361471aeaf7567be411c9a7d70026f2 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 13 Nov 2024 16:38:05 +0000 Subject: [PATCH 10/10] feat: allow disabling notifications entirely --- cli/server.go | 72 ++++++++++--------- cli/testdata/coder_server_--help.golden | 5 +- cli/testdata/server-config.yaml.golden | 5 +- coderd/apidoc/docs.go | 3 + coderd/apidoc/swagger.json | 3 + codersdk/deployment.go | 13 ++++ docs/reference/api/general.md | 1 + docs/reference/api/schemas.md | 4 ++ docs/reference/cli/server.md | 12 ++++ .../cli/testdata/coder_server_--help.golden | 5 +- site/src/api/typesGenerated.ts | 1 + 11 files changed, 89 insertions(+), 35 deletions(-) diff --git a/cli/server.go b/cli/server.go index aa0a010eb0aa4..7c9d467461bfd 100644 --- 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 index 097a8fdeabf04..571299dbc6097 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -109,7 +109,7 @@ Use a YAML configuration file when your server launch become unwieldy. EMAIL OPTIONS: Configure how emails are sent. - --email-force-tls bool, $CODER_EMAIL_FORCE_TLS + --email-force-tls bool, $CODER_EMAIL_FORCE_TLS (default: false) Force a TLS connection to the configured SMTP smarthost. --email-from string, $CODER_EMAIL_FROM @@ -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. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 400a1f8a8d82e..63764c06449f0 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -520,6 +520,9 @@ userQuietHoursSchedule: allowWorkspaceRenames: false # 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 @@ -621,7 +624,7 @@ email: # (default: localhost, type: string) hello: localhost # Force a TLS connection to the configured SMTP smarthost. - # (default: , type: bool) + # (default: false, type: bool) forceTLS: false # Configure SMTP authentication options. emailAuth: diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 80d4a82914150..1955e41747e12 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11383,6 +11383,9 @@ const docTemplate = `{ } ] }, + "enabled": { + "type": "boolean" + }, "fetch_interval": { "description": "How often to query the database for queued notifications.", "type": "integer" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 9abb0d3551bc1..f5e44b46eada7 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10208,6 +10208,9 @@ } ] }, + "enabled": { + "type": "boolean" + }, "fetch_interval": { "description": "How often to query the database for queued notifications.", "type": "integer" diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 88a2b2fbf1a05..8e7864b3b6332 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -649,6 +649,8 @@ type HealthcheckConfig struct { } type NotificationsConfig struct { + Enabled serpent.Bool `json:"enabled" typescript:",notnull"` + // The upper limit of attempts to send a notification. MaxSendAttempts serpent.Int64 `json:"max_send_attempts" typescript:",notnull"` // The minimum time between retries. @@ -1047,6 +1049,7 @@ when required by your organization's security policy.`, Description: "Force a TLS connection to the configured SMTP smarthost.", Flag: "email-force-tls", Env: "CODER_EMAIL_FORCE_TLS", + Default: "false", Value: &c.Notifications.SMTP.ForceTLS, Group: &deploymentGroupEmail, YAML: "forceTLS", @@ -2580,6 +2583,16 @@ Write out the current server config as YAML to stdout.`, Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), }, // Notifications Options + { + Name: "Notifications: Enabled", + Description: "Controls if notifications are enabled.", + Flag: "notifications-enabled", + Env: "CODER_NOTIFICATIONS_ENABLED", + Default: "true", + Value: &c.Notifications.Enabled, + Group: &deploymentGroupNotifications, + YAML: "enabled", + }, { Name: "Notifications: Method", Description: "Which delivery method to use (available options: 'smtp', 'webhook').", diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index c65d9ec4175b9..227e99d4d3964 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -276,6 +276,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "start_tls": true } }, + "enabled": true, "fetch_interval": 0, "lease_count": 0, "lease_period": 0, diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index d40fe8e240005..63b8ca7a314c2 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1894,6 +1894,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "start_tls": true } }, + "enabled": true, "fetch_interval": 0, "lease_count": 0, "lease_period": 0, @@ -2318,6 +2319,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "start_tls": true } }, + "enabled": true, "fetch_interval": 0, "lease_count": 0, "lease_period": 0, @@ -3421,6 +3423,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "start_tls": true } }, + "enabled": true, "fetch_interval": 0, "lease_count": 0, "lease_period": 0, @@ -3453,6 +3456,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | ------------------- | -------------------------------------------------------------------------- | -------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `dispatch_timeout` | integer | false | | How long to wait while a notification is being sent before giving up. | | `email` | [codersdk.NotificationsEmailConfig](#codersdknotificationsemailconfig) | false | | Email settings. | +| `enabled` | boolean | false | | | | `fetch_interval` | integer | false | | How often to query the database for queued notifications. | | `lease_count` | integer | false | | How many notifications a notifier should lease per fetch interval. | | `lease_period` | integer | false | | How long a notifier should lease a message. This is effectively how long a notification is 'owned' by a notifier, and once this period expires it will be available for lease by another notifier. Leasing is important in order for multiple running notifiers to not pick the same messages to deliver concurrently. This lease period will only expire if a notifier shuts down ungracefully; a dispatch of the notification releases the lease. | diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 9ff167d7752a0..d655547e63faa 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1217,6 +1217,17 @@ Refresh interval for healthchecks. The threshold for the database health check. If the median latency of the database exceeds this threshold over 5 attempts, the database is considered unhealthy. The default value is 15ms. +### --notifications-enabled + +| | | +| ----------- | ----------------------------------------- | +| Type | bool | +| Environment | $CODER_NOTIFICATIONS_ENABLED | +| YAML | notifications.enabled | +| Default | true | + +Controls if notifications are enabled. + ### --notifications-method | | | @@ -1438,6 +1449,7 @@ The hostname identifying the SMTP server. | Type | bool | | Environment | $CODER_EMAIL_FORCE_TLS | | YAML | email.forceTLS | +| Default | false | Force a TLS connection to the configured SMTP smarthost. diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 114539b1bbecc..be4a86e32d323 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -110,7 +110,7 @@ Use a YAML configuration file when your server launch become unwieldy. EMAIL OPTIONS: Configure how emails are sent. - --email-force-tls bool, $CODER_EMAIL_FORCE_TLS + --email-force-tls bool, $CODER_EMAIL_FORCE_TLS (default: false) Force a TLS connection to the configured SMTP smarthost. --email-from string, $CODER_EMAIL_FROM @@ -393,6 +393,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. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index bd00dbda353c3..36cb7f389cae5 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -800,6 +800,7 @@ export interface NotificationTemplate { // From codersdk/deployment.go export interface NotificationsConfig { + readonly enabled: boolean; readonly max_send_attempts: number; readonly retry_interval: number; readonly sync_interval: number;