Fixing lint errors & minor tests

Signed-off-by: Danny Kopping <danny@coder.com>
coder · dannykopping · Jul 8, 2024 · Jun 11, 2024 · Jun 11, 2024 · Jun 11, 2024
commit 4856aed2c5ccd8fc91daa21ed8763022b1742e31
diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
@@ -326,6 +326,68 @@ can safely ignore these settings.
           Minimum supported version of TLS. Accepted values are "tls10",
           "tls11", "tls12" or "tls13".
 
+NOTIFICATIONS OPTIONS: 
+      --notifications-dispatch-timeout duration, $CODER_NOTIFICATIONS_DISPATCH_TIMEOUT (default: 1m0s)
+          How long to wait while a notification is being sent before giving up.
+
+      --notifications-fetch-interval duration, $CODER_NOTIFICATIONS_FETCH_INTERVAL (default: 15s)
+          How often to query the database for queued notifications.
+
+      --notifications-lease-count int, $CODER_NOTIFICATIONS_LEASE_COUNT (default: 10)
+          How many notifications a notifier should lease per fetch interval.
+
+      --notifications-lease-period duration, $CODER_NOTIFICATIONS_LEASE_PERIOD (default: 2m0s)
+          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.
+
+      --notifications-max-send-attempts int, $CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS (default: 5)
+          The upper limit of attempts to send a notification.
+
+      --notifications-method string, $CODER_NOTIFICATIONS_METHOD (default: smtp)
+          Which delivery method to use (available options: 'smtp', 'webhook').
+
+      --notifications-retry-interval duration, $CODER_NOTIFICATIONS_RETRY_INTERVAL (default: 5m0s)
+          The minimum time between retries.
+
+      --notifications-store-sync-buffer-size int, $CODER_NOTIFICATIONS_STORE_SYNC_BUFFER_SIZE (default: 50)
+          The notifications system buffers message updates in memory to ease
+          pressure on the database. This option controls how many updates are
+          kept in memory. The lower this value the lower the change of state
+          inconsistency in a non-graceful shutdown - but it also increases load
+          on the database. It is recommended to keep this option at its default
+          value.
+
+      --notifications-store-sync-interval duration, $CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL (default: 2s)
+          The notifications system buffers message updates in memory to ease
+          pressure on the database. This option controls how often it
+          synchronizes its state with the database. The shorter this value the
+          lower the change of state inconsistency in a non-graceful shutdown -
+          but it also increases load on the database. It is recommended to keep
+          this option at its default value.
+
+      --notifications-worker-count int, $CODER_NOTIFICATIONS_WORKER_COUNT (default: 2)
+          How many workers should be processing messages in the queue; increase
+          this count if notifications are not being processed fast enough.
+
+NOTIFICATIONS / EMAIL OPTIONS: 
+      --notifications-email-from string, $CODER_NOTIFICATIONS_EMAIL_FROM
+          The sender's address to use.
+
+      --notifications-email-hello string, $CODER_NOTIFICATIONS_EMAIL_HELLO (default: localhost)
+          The hostname identifying the SMTP server.
+
+      --notifications-email-smarthost host:port, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST (default: localhost:587)
+          The intermediary SMTP host through which emails are sent.
+
+NOTIFICATIONS / WEBHOOK OPTIONS: 
+      --notifications-webhook-endpoint url, $CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT
+          The endpoint to which to send webhooks.
+
 OAUTH2 / GITHUB OPTIONS: 
       --oauth2-github-allow-everyone bool, $CODER_OAUTH2_GITHUB_ALLOW_EVERYONE
           Allow all logins, setting this option means allowed orgs and teams

diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden
@@ -493,3 +493,62 @@ userQuietHoursSchedule:
 # compatibility reasons, this will be removed in a future release.
 # (default: false, type: bool)
 allowWorkspaceRenames: false
+notifications:
+  # The upper limit of attempts to send a notification.
+  # (default: 5, type: int)
+  max-send-attempts: 5
+  # The minimum time between retries.
+  # (default: 5m0s, type: duration)
+  retry-interval: 5m0s
+  # The notifications system buffers message updates in memory to ease pressure on
+  # the database. This option controls how often it synchronizes its state with the
+  # database. The shorter this value the lower the change of state inconsistency in
+  # a non-graceful shutdown - but it also increases load on the database. It is
+  # recommended to keep this option at its default value.
+  # (default: 2s, type: duration)
+  store-sync-interval: 2s
+  # The notifications system buffers message updates in memory to ease pressure on
+  # the database. This option controls how many updates are kept in memory. The
+  # lower this value the lower the change of state inconsistency in a non-graceful
+  # shutdown - but it also increases load on the database. It is recommended to keep
+  # this option at its default value.
+  # (default: 50, type: int)
+  store-sync-buffer-size: 50
+  # How many workers should be processing messages in the queue; increase this count
+  # if notifications are not being processed fast enough.
+  # (default: 2, type: int)
+  worker-count: 2
+  # 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.
+  # (default: 2m0s, type: duration)
+  lease-period: 2m0s
+  # How many notifications a notifier should lease per fetch interval.
+  # (default: 10, type: int)
+  lease-count: 10
+  # How often to query the database for queued notifications.
+  # (default: 15s, type: duration)
+  fetch-interval: 15s
+  # Which delivery method to use (available options: 'smtp', 'webhook').
+  # (default: smtp, type: string)
+  method: smtp
+  # How long to wait while a notification is being sent before giving up.
+  # (default: 1m0s, type: duration)
+  dispatch-timeout: 1m0s
+  email:
+    # The sender's address to use.
+    # (default: <unset>, type: string)
+    from: ""
+    # The intermediary SMTP host through which emails are sent.
+    # (default: localhost:587, type: host:port)
+    smarthost: localhost:587
+    # The hostname identifying the SMTP server.
+    # (default: localhost, type: string)
+    hello: localhost
+  webhook:
+    # The endpoint to which to send webhooks.
+    # (default: <unset>, type: url)
+    hello:
diff --git a/coderd/notifications/dispatch/webhook.go b/coderd/notifications/dispatch/webhook.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"io"
 	"net/http"
 
@@ -95,7 +96,7 @@ func (w *WebhookHandler) dispatch(msgPayload types.MessagePayload, title, body,
 			// Body could be quite long here, let's grab the first 500B and hope it contains useful debug info.
 			var respBody []byte
 			respBody, err = abbreviatedRead(resp.Body, 500)
-			if err != nil && err != io.EOF {
+			if err != nil && !errors.Is(err, io.EOF) {
 				return true, xerrors.Errorf("non-200 response (%d), read body: %w", resp.StatusCode, err)
 			}
 			w.log.Warn(ctx, "unsuccessful delivery", slog.F("status_code", resp.StatusCode),

diff --git a/coderd/notifications/events.go b/coderd/notifications/events.go
@@ -2,8 +2,5 @@ package notifications
 
 import "github.com/google/uuid"
 
-var (
-	// Workspaces.
-	TemplateWorkspaceDeleted = uuid.MustParse("'f517da0b-cdc9-410f-ab89-a86107c420ed'")
-	// ...
-)
+// Workspaces.
+var TemplateWorkspaceDeleted = uuid.MustParse("'f517da0b-cdc9-410f-ab89-a86107c420ed'") // ...
diff --git a/coderd/notifications/manager_test.go b/coderd/notifications/manager_test.go
@@ -86,6 +86,8 @@ func TestBufferedUpdates(t *testing.T) {
 }
 
 func TestBuildPayload(t *testing.T) {
+	t.Parallel()
+
 	// given
 	const label = "Click here!"
 	const url = "http://xyz.com/"
@@ -198,6 +200,6 @@ func (e *enqueueInterceptor) EnqueueNotificationMessage(_ context.Context, arg d
 	return database.NotificationMessage{}, err
 }
 
-func (e *enqueueInterceptor) FetchNewMessageMetadata(_ context.Context, arg database.FetchNewMessageMetadataParams) (database.FetchNewMessageMetadataRow, error) {
+func (e *enqueueInterceptor) FetchNewMessageMetadata(_ context.Context, _ database.FetchNewMessageMetadataParams) (database.FetchNewMessageMetadataRow, error) {
 	return e.metadataFn(), nil
 }
diff --git a/coderd/notifications/notifier.go b/coderd/notifications/notifier.go
@@ -170,9 +170,7 @@ func (n *notifier) prepare(ctx context.Context, msg database.AcquireNotification
 		return nil, xerrors.Errorf("resolve handler: %w", err)
 	}
 
-	var (
-		title, body string
-	)
+	var title, body string
 	if title, err = render.GoTemplate(msg.TitleTemplate, payload, nil); err != nil {
 		return nil, xerrors.Errorf("render title: %w", err)
 	}

diff --git a/coderd/notifications/system/system_test.go b/coderd/notifications/system/system_test.go
@@ -5,17 +5,20 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+
 	"github.com/coder/coder/v2/coderd/notifications"
 	"github.com/coder/coder/v2/coderd/notifications/system"
 	"github.com/coder/coder/v2/coderd/notifications/types"
 	"github.com/coder/coder/v2/testutil"
-	"github.com/google/uuid"
-	"github.com/stretchr/testify/require"
 )
 
 // TestNotifyWorkspaceDeleted tests the "public" interface for enqueueing notifications.
 // Calling system.NotifyWorkspaceDeleted uses the Enqueuer singleton to enqueue the notification.
 func TestNotifyWorkspaceDeleted(t *testing.T) {
+	t.Parallel()
+
 	// given
 	manager := newFakeEnqueuer()
 	notifications.RegisterInstance(manager)
@@ -42,5 +45,6 @@ func newFakeEnqueuer() *fakeEnqueuer {
 
 func (f *fakeEnqueuer) Enqueue(context.Context, uuid.UUID, uuid.UUID, types.Labels, string, ...uuid.UUID) (*uuid.UUID, error) {
 	f.enqueued <- true
+	// nolint:nilnil // Irrelevant.
 	return nil, nil
 }
diff --git a/codersdk/deployment.go b/codersdk/deployment.go
@@ -2104,19 +2104,21 @@ Write out the current server config as YAML to stdout.`,
 			Default:     (time.Minute * 5).String(),
 			Group:       &deploymentGroupNotifications,
 			YAML:        "retry-interval",
+			Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
 		},
 		{
 			Name: "Notifications: Store Sync Interval",
 			Description: "The notifications system buffers message updates in memory to ease pressure on the database. " +
 				"This option controls how often it synchronizes its state with the database. The shorter this value the " +
 				"lower the change of state inconsistency in a non-graceful shutdown - but it also increases load on the " +
 				"database. It is recommended to keep this option at its default value.",
-			Flag:    "notifications-store-sync-interval",
-			Env:     "CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL",
-			Value:   &c.Notifications.StoreSyncInterval,
-			Default: (time.Second * 2).String(),
-			Group:   &deploymentGroupNotifications,
-			YAML:    "store-sync-interval",
+			Flag:        "notifications-store-sync-interval",
+			Env:         "CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL",
+			Value:       &c.Notifications.StoreSyncInterval,
+			Default:     (time.Second * 2).String(),
+			Group:       &deploymentGroupNotifications,
+			YAML:        "store-sync-interval",
+			Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
 		},
 		{
 			Name: "Notifications: Store Sync Buffer Size",
@@ -2149,12 +2151,13 @@ Write out the current server config as YAML to stdout.`,
 				"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.",
-			Flag:    "notifications-lease-period",
-			Env:     "CODER_NOTIFICATIONS_LEASE_PERIOD",
-			Value:   &c.Notifications.LeasePeriod,
-			Default: (time.Minute * 2).String(),
-			Group:   &deploymentGroupNotifications,
-			YAML:    "lease-period",
+			Flag:        "notifications-lease-period",
+			Env:         "CODER_NOTIFICATIONS_LEASE_PERIOD",
+			Value:       &c.Notifications.LeasePeriod,
+			Default:     (time.Minute * 2).String(),
+			Group:       &deploymentGroupNotifications,
+			YAML:        "lease-period",
+			Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
 		},
 		{
 			Name:        "Notifications: Lease Count",
@@ -2175,6 +2178,7 @@ Write out the current server config as YAML to stdout.`,
 			Default:     (time.Second * 15).String(),
 			Group:       &deploymentGroupNotifications,
 			YAML:        "fetch-interval",
+			Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
 		},
 		{
 			Name:        "Notifications: Method",
@@ -2195,6 +2199,7 @@ Write out the current server config as YAML to stdout.`,
 			Default:     time.Minute.String(),
 			Group:       &deploymentGroupNotifications,
 			YAML:        "dispatch-timeout",
+			Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
 		},
 		{
 			Name:        "Notifications: Email: From Address",
@@ -2210,6 +2215,7 @@ Write out the current server config as YAML to stdout.`,
 			Description: "The intermediary SMTP host through which emails are sent.",
 			Flag:        "notifications-email-smarthost",
 			Env:         "CODER_NOTIFICATIONS_EMAIL_SMARTHOST",
+			Default:     "localhost:587", // To pass validation.
 			Value:       &c.Notifications.SMTP.Smarthost,
 			Group:       &deploymentGroupNotificationsEmail,
 			YAML:        "smarthost",

diff --git a/docs/cli/server.md b/docs/cli/server.md
diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden
@@ -327,6 +327,68 @@ can safely ignore these settings.
           Minimum supported version of TLS. Accepted values are "tls10",
           "tls11", "tls12" or "tls13".
 
+NOTIFICATIONS OPTIONS: 
+      --notifications-dispatch-timeout duration, $CODER_NOTIFICATIONS_DISPATCH_TIMEOUT (default: 1m0s)
+          How long to wait while a notification is being sent before giving up.
+
+      --notifications-fetch-interval duration, $CODER_NOTIFICATIONS_FETCH_INTERVAL (default: 15s)
+          How often to query the database for queued notifications.
+
+      --notifications-lease-count int, $CODER_NOTIFICATIONS_LEASE_COUNT (default: 10)
+          How many notifications a notifier should lease per fetch interval.
+
+      --notifications-lease-period duration, $CODER_NOTIFICATIONS_LEASE_PERIOD (default: 2m0s)
+          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.
+
+      --notifications-max-send-attempts int, $CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS (default: 5)
+          The upper limit of attempts to send a notification.
+
+      --notifications-method string, $CODER_NOTIFICATIONS_METHOD (default: smtp)
+          Which delivery method to use (available options: 'smtp', 'webhook').
+
+      --notifications-retry-interval duration, $CODER_NOTIFICATIONS_RETRY_INTERVAL (default: 5m0s)
+          The minimum time between retries.
+
+      --notifications-store-sync-buffer-size int, $CODER_NOTIFICATIONS_STORE_SYNC_BUFFER_SIZE (default: 50)
+          The notifications system buffers message updates in memory to ease
+          pressure on the database. This option controls how many updates are
+          kept in memory. The lower this value the lower the change of state
+          inconsistency in a non-graceful shutdown - but it also increases load
+          on the database. It is recommended to keep this option at its default
+          value.
+
+      --notifications-store-sync-interval duration, $CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL (default: 2s)
+          The notifications system buffers message updates in memory to ease
+          pressure on the database. This option controls how often it
+          synchronizes its state with the database. The shorter this value the
+          lower the change of state inconsistency in a non-graceful shutdown -
+          but it also increases load on the database. It is recommended to keep
+          this option at its default value.
+
+      --notifications-worker-count int, $CODER_NOTIFICATIONS_WORKER_COUNT (default: 2)
+          How many workers should be processing messages in the queue; increase
+          this count if notifications are not being processed fast enough.
+
+NOTIFICATIONS / EMAIL OPTIONS: 
+      --notifications-email-from string, $CODER_NOTIFICATIONS_EMAIL_FROM
+          The sender's address to use.
+
+      --notifications-email-hello string, $CODER_NOTIFICATIONS_EMAIL_HELLO (default: localhost)
+          The hostname identifying the SMTP server.
+
+      --notifications-email-smarthost host:port, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST (default: localhost:587)
+          The intermediary SMTP host through which emails are sent.
+
+NOTIFICATIONS / WEBHOOK OPTIONS: 
+      --notifications-webhook-endpoint url, $CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT
+          The endpoint to which to send webhooks.
+
 OAUTH2 / GITHUB OPTIONS: 
       --oauth2-github-allow-everyone bool, $CODER_OAUTH2_GITHUB_ALLOW_EVERYONE
           Allow all logins, setting this option means allowed orgs and teams