From 5c36c93ee603a11867659031bca1530e6387aca5 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 12 Nov 2024 12:06:23 +0000 Subject: [PATCH 01/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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; From d5aff283582d7a54754e44797a07bc8b084b1b14 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 10:16:49 +0000 Subject: [PATCH 11/22] chore: re-add default for smarthost --- cli/testdata/coder_server_--help.golden | 4 ++-- cli/testdata/server-config.yaml.golden | 8 +++---- coderd/apidoc/docs.go | 6 ++++- coderd/apidoc/swagger.json | 6 ++++- coderd/notifications/dispatch/smtp.go | 24 ++++++++++--------- coderd/notifications/dispatch/smtp_test.go | 2 +- coderd/notifications/notifications_test.go | 6 ++--- codersdk/deployment.go | 3 ++- docs/reference/api/general.md | 5 +++- docs/reference/api/schemas.md | 22 +++++++++++++---- docs/reference/cli/server.md | 5 ++-- .../cli/testdata/coder_server_--help.golden | 4 ++-- 12 files changed, 61 insertions(+), 34 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 571299dbc6097..cad6c7961319d 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 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 index 63764c06449f0..b049d7c463165 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -535,8 +535,8 @@ notifications: # (default: , type: string) from: "" # The intermediary SMTP host through which emails are sent. - # (default: , type: string) - smarthost: "" + # (default: , type: host:port) + smarthost: localhost:587 # The hostname identifying the SMTP server. # (default: localhost, type: string) hello: localhost @@ -618,8 +618,8 @@ email: # (default: , type: string) from: "" # The intermediary SMTP host through which emails are sent. - # (default: , 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 index 1955e41747e12..714ccb48136b0 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11474,7 +11474,11 @@ const docTemplate = `{ }, "smarthost": { "description": "The intermediary SMTP host through which emails are sent (host:port).", - "type": "string" + "allOf": [ + { + "$ref": "#/definitions/serpent.HostPort" + } + ] }, "tls": { "description": "TLS details.", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index f5e44b46eada7..b5332ec57223e 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10299,7 +10299,11 @@ }, "smarthost": { "description": "The intermediary SMTP host through which emails are sent (host:port).", - "type": "string" + "allOf": [ + { + "$ref": "#/definitions/serpent.HostPort" + } + ] }, "tls": { "description": "TLS details.", diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index d2dc7cdf20872..194cf7999d3ef 100644 --- 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 index b448dd2582e67..c9a60b426ae70 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 = 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 index 6ba8f614bd38d..763046bc20cb0 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.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 index 835f0656ccec4..991f9cce9d5a2 100644 --- 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 index 227e99d4d3964..34016516fe799 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -266,7 +266,10 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "force_tls": true, "from": "string", "hello": "string", - "smarthost": "string", + "smarthost": { + "host": "string", + "port": "string" + }, "tls": { "ca_file": "string", "cert_file": "string", diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 63b8ca7a314c2..6cca0b80b12e0 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1884,7 +1884,10 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "force_tls": true, "from": "string", "hello": "string", - "smarthost": "string", + "smarthost": { + "host": "string", + "port": "string" + }, "tls": { "ca_file": "string", "cert_file": "string", @@ -2309,7 +2312,10 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "force_tls": true, "from": "string", "hello": "string", - "smarthost": "string", + "smarthost": { + "host": "string", + "port": "string" + }, "tls": { "ca_file": "string", "cert_file": "string", @@ -3413,7 +3419,10 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "force_tls": true, "from": "string", "hello": "string", - "smarthost": "string", + "smarthost": { + "host": "string", + "port": "string" + }, "tls": { "ca_file": "string", "cert_file": "string", @@ -3500,7 +3509,10 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "force_tls": true, "from": "string", "hello": "string", - "smarthost": "string", + "smarthost": { + "host": "string", + "port": "string" + }, "tls": { "ca_file": "string", "cert_file": "string", @@ -3520,7 +3532,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` | string | false | | The intermediary SMTP host through which emails are sent (host:port). | +| `smarthost` | [serpent.HostPort](#serpenthostport) | 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 d655547e63faa..a3aa79b2761c9 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1264,7 +1264,7 @@ The sender's address to use. | | | | ----------- | ------------------------------------------------- | -| Type | string | +| Type | host:port | | Environment | $CODER_NOTIFICATIONS_EMAIL_SMARTHOST | | YAML | notifications.email.smarthost | @@ -1425,9 +1425,10 @@ The sender's address to use. | | | | ----------- | ----------------------------------- | -| Type | string | +| Type | host:port | | Environment | $CODER_EMAIL_SMARTHOST | | YAML | email.smarthost | +| Default | localhost:587 | The intermediary SMTP host through which emails are sent. diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index be4a86e32d323..a00e0bcd2541d 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 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. From 01cb99946088500685a6ba2509b92e4642db9b60 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 10:22:58 +0000 Subject: [PATCH 12/22] fix: remove default for deprecated option --- cli/testdata/coder_server_--help.golden | 2 +- cli/testdata/server-config.yaml.golden | 2 +- coderd/notifications/dispatch/smtp.go | 4 ++-- codersdk/deployment.go | 1 - docs/reference/cli/server.md | 1 - enterprise/cli/testdata/coder_server_--help.golden | 2 +- 6 files changed, 5 insertions(+), 7 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index cad6c7961319d..d63c3ed95fe7d 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -412,7 +412,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 (default: localhost) + --notifications-email-hello string, $CODER_NOTIFICATIONS_EMAIL_HELLO 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 b049d7c463165..d7c128fdc5802 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -538,7 +538,7 @@ notifications: # (default: , type: host:port) smarthost: localhost:587 # The hostname identifying the SMTP server. - # (default: localhost, type: string) + # (default: , type: string) hello: localhost # Force a TLS connection to the configured SMTP smarthost. # (default: , type: bool) diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index 194cf7999d3ef..fddff3e162171 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -34,8 +34,8 @@ import ( ) var ( - ValidationNoFromAddressErr = xerrors.New("'from' address not defined") - ValidationNoToAddressErr = xerrors.New("'to' address(es) not defined") + ValidationNoFromAddressErr = xerrors.New("no 'from' address defined") + ValidationNoToAddressErr = xerrors.New("no '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") diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 991f9cce9d5a2..f3ce8626a4814 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2654,7 +2654,6 @@ 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/docs/reference/cli/server.md b/docs/reference/cli/server.md index a3aa79b2761c9..a1a649affa082 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1277,7 +1277,6 @@ 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. diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index a00e0bcd2541d..338dfa0f55d04 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -413,7 +413,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 (default: localhost) + --notifications-email-hello string, $CODER_NOTIFICATIONS_EMAIL_HELLO The hostname identifying the SMTP server. DEPRECATED: Use --email-hello instead. From c51311310575c9be8894367b4bc0b8e91e764034 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 11:57:53 +0000 Subject: [PATCH 13/22] chore: revert change --- coderd/notifications/dispatch/smtp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/notifications/dispatch/smtp.go b/coderd/notifications/dispatch/smtp.go index fddff3e162171..dfb628b62eb86 100644 --- a/coderd/notifications/dispatch/smtp.go +++ b/coderd/notifications/dispatch/smtp.go @@ -35,7 +35,7 @@ import ( var ( ValidationNoFromAddressErr = xerrors.New("no 'from' address defined") - ValidationNoToAddressErr = xerrors.New("no 'to' address(es) not 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") From a8b5a610d3e2fc4b4cc88d3bdbe8b00530619b57 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 13:42:17 +0000 Subject: [PATCH 14/22] chore: remove unrelated test changes --- codersdk/deployment_test.go | 112 ------------------------------------ 1 file changed, 112 deletions(-) diff --git a/codersdk/deployment_test.go b/codersdk/deployment_test.go index 4878c6ac691b7..61474a3b77ea1 100644 --- a/codersdk/deployment_test.go +++ b/codersdk/deployment_test.go @@ -568,115 +568,3 @@ 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", - }, - { - 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 { - 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 1711e68bfbd7cef91f766b5175a1cbd2e48c1a60 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 14:11:43 +0000 Subject: [PATCH 15/22] revert: more changes --- cli/testdata/server-config.yaml.golden | 90 ++++---- codersdk/deployment.go | 30 +-- docs/reference/cli/server.md | 284 ++++++++++++------------- 3 files changed, 202 insertions(+), 202 deletions(-) diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index d7c128fdc5802..e9ab989a2c1e2 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -518,6 +518,51 @@ 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: 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: , 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: # Controls if notifications are enabled. @@ -612,48 +657,3 @@ 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: 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: , 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/codersdk/deployment.go b/codersdk/deployment.go index f3ce8626a4814..61a16babdc3c7 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2597,6 +2597,21 @@ 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: Enabled", @@ -2868,21 +2883,6 @@ 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/docs/reference/cli/server.md b/docs/reference/cli/server.md index a1a649affa082..02b99b13eb58a 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1217,6 +1217,148 @@ 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 | host:port | +| Environment | $CODER_EMAIL_SMARTHOST | +| YAML | email.smarthost | +| Default | localhost:587 | + +The intermediary SMTP host through which emails are sent. + +### --email-hello + +| | | +| ----------- | ------------------------------- | +| Type | string | +| Environment | $CODER_EMAIL_HELLO | +| YAML | email.hello | +| Default | localhost | + +The hostname identifying the SMTP server. + +### --email-force-tls + +| | | +| ----------- | ----------------------------------- | +| Type | bool | +| Environment | $CODER_EMAIL_FORCE_TLS | +| YAML | email.forceTLS | +| Default | false | + +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-enabled | | | @@ -1409,145 +1551,3 @@ 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 | host:port | -| Environment | $CODER_EMAIL_SMARTHOST | -| YAML | email.smarthost | -| Default | localhost:587 | - -The intermediary SMTP host through which emails are sent. - -### --email-hello - -| | | -| ----------- | ------------------------------- | -| Type | string | -| Environment | $CODER_EMAIL_HELLO | -| YAML | email.hello | -| Default | localhost | - -The hostname identifying the SMTP server. - -### --email-force-tls - -| | | -| ----------- | ----------------------------------- | -| Type | bool | -| Environment | $CODER_EMAIL_FORCE_TLS | -| YAML | email.forceTLS | -| Default | false | - -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 2e03ed1ffc7326a47303d8c8d79e70ec0e137c19 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 15:28:26 +0000 Subject: [PATCH 16/22] chore: simplify description Co-authored-by: Mathias Fredriksson --- codersdk/deployment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 61a16babdc3c7..60663dde0a7f6 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2615,7 +2615,7 @@ Write out the current server config as YAML to stdout.`, // Notifications Options { Name: "Notifications: Enabled", - Description: "Controls if notifications are enabled.", + Description: "Enable or disable notifications.", Flag: "notifications-enabled", Env: "CODER_NOTIFICATIONS_ENABLED", Default: "true", From 7ba3a6ce12a0411b8ace3a0a29020f19a478161e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 15:32:08 +0000 Subject: [PATCH 17/22] chore: make {gen,update-golden-files} --- cli/testdata/coder_server_--help.golden | 2 +- cli/testdata/server-config.yaml.golden | 2 +- docs/reference/cli/server.md | 2 +- enterprise/cli/testdata/coder_server_--help.golden | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index d63c3ed95fe7d..0454b9dffa40b 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -393,7 +393,7 @@ Configure how notifications are processed and delivered. 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. + Enable or disable notifications. --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 e9ab989a2c1e2..5afe4815167fd 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -565,7 +565,7 @@ email: certKeyFile: "" # Configure how notifications are processed and delivered. notifications: - # Controls if notifications are enabled. + # Enable or disable notifications. # (default: true, type: bool) enabled: true # Which delivery method to use (available options: 'smtp', 'webhook'). diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 02b99b13eb58a..75d7cc11b4e79 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1368,7 +1368,7 @@ Certificate key file to use. | YAML | notifications.enabled | | Default | true | -Controls if notifications are enabled. +Enable or disable notifications. ### --notifications-method diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 338dfa0f55d04..8260d02c1cc4d 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -394,7 +394,7 @@ Configure how notifications are processed and delivered. 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. + Enable or disable notifications. --notifications-max-send-attempts int, $CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS (default: 5) The upper limit of attempts to send a notification. From 4a72e4f5f66cd9022c177f2a710a04cba10de0cd Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 14 Nov 2024 15:33:01 +0000 Subject: [PATCH 18/22] chore: rename cfg to notificationsCfg --- cli/server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/server.go b/cli/server.go index 7c9d467461bfd..ddd9f559060c3 100644 --- a/cli/server.go +++ b/cli/server.go @@ -898,16 +898,16 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. // Manage notifications. var ( - cfg = options.DeploymentValues.Notifications + notificationsCfg = options.DeploymentValues.Notifications notificationsManager *notifications.Manager ) - if cfg.Enabled { + if notificationsCfg.Enabled { metrics := notifications.NewMetrics(options.PrometheusRegistry) helpers := templateHelpers(options) // The enqueuer is responsible for enqueueing notifications to the given store. - enqueuer, err := notifications.NewStoreEnqueuer(cfg, options.Database, helpers, logger.Named("notifications.enqueuer"), quartz.NewReal()) + 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) } @@ -916,7 +916,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. // 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")) + 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) } From 9c02842c17bb59c167c71766e6e1f92fece19856 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 18 Nov 2024 16:22:41 +0000 Subject: [PATCH 19/22] chore: drop enabled flag --- cli/server.go | 4 ++- cli/testdata/coder_server_--help.golden | 7 ++--- cli/testdata/server-config.yaml.golden | 11 +++----- coderd/apidoc/docs.go | 9 +------ coderd/apidoc/swagger.json | 9 +------ coderd/notifications/dispatch/smtp.go | 24 ++++++++--------- coderd/notifications/notifications_test.go | 6 ++--- codersdk/deployment.go | 19 ++++---------- docs/reference/api/general.md | 6 +---- docs/reference/api/schemas.md | 26 ++++--------------- docs/reference/cli/server.md | 16 ++---------- .../cli/testdata/coder_server_--help.golden | 7 ++--- site/src/api/typesGenerated.ts | 1 - 13 files changed, 40 insertions(+), 105 deletions(-) diff --git a/cli/server.go b/cli/server.go index ddd9f559060c3..10b58a6e5760c 100644 --- a/cli/server.go +++ b/cli/server.go @@ -902,7 +902,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. notificationsManager *notifications.Manager ) - if notificationsCfg.Enabled { + if notificationsCfg.Enabled() { metrics := notifications.NewMetrics(options.PrometheusRegistry) helpers := templateHelpers(options) @@ -927,6 +927,8 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. // 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 { + logger.Info(ctx, "notifications disabled as there are no notification methods configured") } // Since errCh only has one buffered slot, all routines diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 0454b9dffa40b..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: @@ -392,9 +392,6 @@ 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) - Enable or disable notifications. - --notifications-max-send-attempts int, $CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS (default: 5) The upper limit of attempts to send a notification. @@ -416,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 5afe4815167fd..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 @@ -565,9 +565,6 @@ email: certKeyFile: "" # Configure how notifications are processed and delivered. notifications: - # Enable or disable notifications. - # (default: true, type: bool) - enabled: true # Which delivery method to use (available options: 'smtp', 'webhook'). # (default: smtp, type: string) method: smtp @@ -580,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 714ccb48136b0..80d4a82914150 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11383,9 +11383,6 @@ const docTemplate = `{ } ] }, - "enabled": { - "type": "boolean" - }, "fetch_interval": { "description": "How often to query the database for queued notifications.", "type": "integer" @@ -11474,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 b5332ec57223e..9abb0d3551bc1 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10208,9 +10208,6 @@ } ] }, - "enabled": { - "type": "boolean" - }, "fetch_interval": { "description": "How often to query the database for queued notifications.", "type": "integer" @@ -10299,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/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 60663dde0a7f6..7de961ec869f8 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -649,8 +649,6 @@ 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. @@ -688,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"` @@ -1030,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", @@ -2613,16 +2614,6 @@ Write out the current server config as YAML to stdout.`, emailTLSCertFile, emailTLSCertKeyFile, // Notifications Options - { - Name: "Notifications: Enabled", - Description: "Enable or disable notifications.", - 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 34016516fe799..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", @@ -279,7 +276,6 @@ 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 6cca0b80b12e0..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", @@ -1897,7 +1894,6 @@ 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, @@ -2312,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", @@ -2325,7 +2318,6 @@ 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, @@ -3419,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", @@ -3432,7 +3421,6 @@ 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, @@ -3465,7 +3453,6 @@ 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. | @@ -3509,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", @@ -3532,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 75d7cc11b4e79..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. @@ -1359,17 +1358,6 @@ Certificate file to use. Certificate key file to use. -### --notifications-enabled - -| | | -| ----------- | ----------------------------------------- | -| Type | bool | -| Environment | $CODER_NOTIFICATIONS_ENABLED | -| YAML | notifications.enabled | -| Default | true | - -Enable or disable notifications. - ### --notifications-method | | | @@ -1406,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 8260d02c1cc4d..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: @@ -393,9 +393,6 @@ 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) - Enable or disable notifications. - --notifications-max-send-attempts int, $CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS (default: 5) The upper limit of attempts to send a notification. @@ -417,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. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 36cb7f389cae5..bd00dbda353c3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -800,7 +800,6 @@ 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; From dd527f9ad1ce83e957696578ad53d3d1044b8d07 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 18 Nov 2024 16:31:59 +0000 Subject: [PATCH 20/22] fix: invalid test --- coderd/notifications/dispatch/smtp_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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")) From 77d1803ba9e42e7d25dbcf946d7b0e2d97fa309e Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 19 Nov 2024 10:24:55 +0000 Subject: [PATCH 21/22] chore: improve info message and add to docs --- cli/server.go | 2 +- docs/admin/monitoring/notifications/index.md | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cli/server.go b/cli/server.go index 10b58a6e5760c..968ba66122075 100644 --- a/cli/server.go +++ b/cli/server.go @@ -928,7 +928,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. notificationReportGenerator := reports.NewReportGenerator(ctx, logger.Named("notifications.report_generator"), options.Database, options.NotificationsEnqueuer, quartz.NewReal()) defer notificationReportGenerator.Close() } else { - logger.Info(ctx, "notifications disabled as there are no notification methods configured") + 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 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:** From 5fd03b3a2b6075dd247516a2c98d051538407408 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 19 Nov 2024 11:26:37 +0000 Subject: [PATCH 22/22] test: notifications.Enabled() --- codersdk/deployment_test.go | 66 +++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) 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()) + }) + } +}