diff --git a/cli/server.go b/cli/server.go index 3fefc51357..717613b6c6 100644 --- a/cli/server.go +++ b/cli/server.go @@ -920,34 +920,30 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. notificationsManager *notifications.Manager ) - if notificationsCfg.Enabled() { - metrics := notifications.NewMetrics(options.PrometheusRegistry) - helpers := templateHelpers(options) + metrics := notifications.NewMetrics(options.PrometheusRegistry) + helpers := templateHelpers(options) - // The enqueuer is responsible for enqueueing notifications to the given store. - 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) - } - options.NotificationsEnqueuer = enqueuer - - // 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(notificationsCfg, options.Database, options.Pubsub, 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() - } else { - logger.Debug(ctx, "notifications are currently disabled as there are no configured delivery methods. See https://coder.com/docs/admin/monitoring/notifications#delivery-methods for more details") + // The enqueuer is responsible for enqueueing notifications to the given store. + 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) } + options.NotificationsEnqueuer = enqueuer + + // 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(notificationsCfg, options.Database, options.Pubsub, 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() // Since errCh only has one buffered slot, all routines // sending on it must be wrapped in a select/default to diff --git a/cli/server_test.go b/cli/server_test.go index d901939111..0dee317e27 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -298,7 +298,7 @@ func TestServer(t *testing.T) { out := pty.ReadAll() numLines := countLines(string(out)) t.Logf("numLines: %d", numLines) - require.Less(t, numLines, 12, "expected less than 12 lines of output (terminal width 80), got %d", numLines) + require.Less(t, numLines, 20, "expected less than 20 lines of output (terminal width 80), got %d", numLines) }) t.Run("OAuth2GitHubDefaultProvider", func(t *testing.T) { diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index df1f982bc5..174b25eae1 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -473,6 +473,10 @@ Configure TLS for your SMTP server target. Enable STARTTLS to upgrade insecure SMTP connections using TLS. DEPRECATED: Use --email-tls-starttls instead. +NOTIFICATIONS / INBOX OPTIONS: + --notifications-inbox-enabled bool, $CODER_NOTIFICATIONS_INBOX_ENABLED (default: true) + Enable Coder Inbox. + NOTIFICATIONS / WEBHOOK OPTIONS: --notifications-webhook-endpoint url, $CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT The endpoint to which to send webhooks. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index cffaf65cd3..39ed5eb2c0 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -643,6 +643,10 @@ notifications: # The endpoint to which to send webhooks. # (default: , type: url) endpoint: + inbox: + # Enable Coder Inbox. + # (default: true, type: bool) + enabled: true # The upper limit of attempts to send a notification. # (default: 5, type: int) maxSendAttempts: 5 diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index fe6aacf84d..c4915c16c6 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -12658,6 +12658,14 @@ const docTemplate = `{ "description": "How often to query the database for queued notifications.", "type": "integer" }, + "inbox": { + "description": "Inbox settings.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.NotificationsInboxConfig" + } + ] + }, "lease_count": { "description": "How many notifications a notifier should lease per fetch interval.", "type": "integer" @@ -12783,6 +12791,14 @@ const docTemplate = `{ } } }, + "codersdk.NotificationsInboxConfig": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + } + } + }, "codersdk.NotificationsSettings": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 7a399a0e04..0b45305e1c 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -11369,6 +11369,14 @@ "description": "How often to query the database for queued notifications.", "type": "integer" }, + "inbox": { + "description": "Inbox settings.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.NotificationsInboxConfig" + } + ] + }, "lease_count": { "description": "How many notifications a notifier should lease per fetch interval.", "type": "integer" @@ -11494,6 +11502,14 @@ } } }, + "codersdk.NotificationsInboxConfig": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + } + } + }, "codersdk.NotificationsSettings": { "type": "object", "properties": { diff --git a/coderd/notifications/enqueuer.go b/coderd/notifications/enqueuer.go index 84d3025a8e..b93a05aa96 100644 --- a/coderd/notifications/enqueuer.go +++ b/coderd/notifications/enqueuer.go @@ -3,6 +3,7 @@ package notifications import ( "context" "encoding/json" + "slices" "strings" "text/template" @@ -28,7 +29,10 @@ type StoreEnqueuer struct { store Store log slog.Logger - defaultMethod database.NotificationMethod + defaultMethod database.NotificationMethod + defaultEnabled bool + inboxEnabled bool + // helpers holds a map of template funcs which are used when rendering templates. These need to be passed in because // the template funcs will return values which are inappropriately encapsulated in this struct. helpers template.FuncMap @@ -44,11 +48,13 @@ func NewStoreEnqueuer(cfg codersdk.NotificationsConfig, store Store, helpers tem } return &StoreEnqueuer{ - store: store, - log: log, - defaultMethod: method, - helpers: helpers, - clock: clock, + store: store, + log: log, + defaultMethod: method, + defaultEnabled: cfg.Enabled(), + inboxEnabled: cfg.Inbox.Enabled.Value(), + helpers: helpers, + clock: clock, }, nil } @@ -69,11 +75,6 @@ func (s *StoreEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID return nil, xerrors.Errorf("new message metadata: %w", err) } - dispatchMethod := s.defaultMethod - if metadata.CustomMethod.Valid { - dispatchMethod = metadata.CustomMethod.NotificationMethod - } - payload, err := s.buildPayload(metadata, labels, data, targets) if err != nil { s.log.Warn(ctx, "failed to build payload", slog.F("template_id", templateID), slog.F("user_id", userID), slog.Error(err)) @@ -85,11 +86,22 @@ func (s *StoreEnqueuer) EnqueueWithData(ctx context.Context, userID, templateID return nil, xerrors.Errorf("failed encoding input labels: %w", err) } - uuids := make([]uuid.UUID, 0, 2) + methods := []database.NotificationMethod{} + if metadata.CustomMethod.Valid { + methods = append(methods, metadata.CustomMethod.NotificationMethod) + } else if s.defaultEnabled { + methods = append(methods, s.defaultMethod) + } + // All the enqueued messages are enqueued both on the dispatch method set by the user (or default one) and the inbox. // As the inbox is not configurable per the user and is always enabled, we always enqueue the message on the inbox. // The logic is done here in order to have two completely separated processing and retries are handled separately. - for _, method := range []database.NotificationMethod{dispatchMethod, database.NotificationMethodInbox} { + if !slices.Contains(methods, database.NotificationMethodInbox) && s.inboxEnabled { + methods = append(methods, database.NotificationMethodInbox) + } + + uuids := make([]uuid.UUID, 0, 2) + for _, method := range methods { id := uuid.New() err = s.store.EnqueueNotificationMessage(ctx, database.EnqueueNotificationMessageParams{ ID: id, diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index 57618ceb89..348185ef51 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -1856,6 +1856,90 @@ func TestNotificationDuplicates(t *testing.T) { require.NoError(t, err) } +func TestNotificationTargetMatrix(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + defaultMethod database.NotificationMethod + defaultEnabled bool + inboxEnabled bool + expectedEnqueued int + }{ + { + name: "NoDefaultAndNoInbox", + defaultMethod: database.NotificationMethodSmtp, + defaultEnabled: false, + inboxEnabled: false, + expectedEnqueued: 0, + }, + { + name: "DefaultAndNoInbox", + defaultMethod: database.NotificationMethodSmtp, + defaultEnabled: true, + inboxEnabled: false, + expectedEnqueued: 1, + }, + { + name: "NoDefaultAndInbox", + defaultMethod: database.NotificationMethodSmtp, + defaultEnabled: false, + inboxEnabled: true, + expectedEnqueued: 1, + }, + { + name: "DefaultAndInbox", + defaultMethod: database.NotificationMethodSmtp, + defaultEnabled: true, + inboxEnabled: true, + expectedEnqueued: 2, + }, + } + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // nolint:gocritic // Unit test. + ctx := dbauthz.AsNotifier(testutil.Context(t, testutil.WaitSuperLong)) + store, pubsub := dbtestutil.NewDB(t) + logger := testutil.Logger(t) + + cfg := defaultNotificationsConfig(tt.defaultMethod) + cfg.Inbox.Enabled = serpent.Bool(tt.inboxEnabled) + + // If the default method is not enabled, we want to ensure the config + // is wiped out. + if !tt.defaultEnabled { + cfg.SMTP = codersdk.NotificationsEmailConfig{} + cfg.Webhook = codersdk.NotificationsWebhookConfig{} + } + + mgr, err := notifications.NewManager(cfg, store, pubsub, defaultHelpers(), createMetrics(), logger.Named("manager")) + require.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, mgr.Stop(ctx)) + }) + + // Set the time to a known value. + mClock := quartz.NewMock(t) + mClock.Set(time.Date(2024, 1, 15, 9, 0, 0, 0, time.UTC)) + + enq, err := notifications.NewStoreEnqueuer(cfg, store, defaultHelpers(), logger.Named("enqueuer"), mClock) + require.NoError(t, err) + user := createSampleUser(t, store) + + // When: A notification is enqueued, it enqueues the correct amount of notifications. + enqueued, err := enq.Enqueue(ctx, user.ID, notifications.TemplateWorkspaceDeleted, + map[string]string{"initiator": "danny"}, "test", user.ID) + require.NoError(t, err) + require.Len(t, enqueued, tt.expectedEnqueued) + }) + } +} + type fakeHandler struct { mu sync.RWMutex succeeded, failed []string diff --git a/coderd/notifications/utils_test.go b/coderd/notifications/utils_test.go index 95155ea39c..d27093fb63 100644 --- a/coderd/notifications/utils_test.go +++ b/coderd/notifications/utils_test.go @@ -2,6 +2,7 @@ package notifications_test import ( "context" + "net/url" "sync/atomic" "testing" "text/template" @@ -21,6 +22,18 @@ import ( ) func defaultNotificationsConfig(method database.NotificationMethod) codersdk.NotificationsConfig { + var ( + smtp codersdk.NotificationsEmailConfig + webhook codersdk.NotificationsWebhookConfig + ) + + switch method { + case database.NotificationMethodSmtp: + smtp.Smarthost = serpent.String("localhost:1337") + case database.NotificationMethodWebhook: + webhook.Endpoint = serpent.URL(url.URL{Host: "localhost"}) + } + return codersdk.NotificationsConfig{ Method: serpent.String(method), MaxSendAttempts: 5, @@ -31,8 +44,11 @@ func defaultNotificationsConfig(method database.NotificationMethod) codersdk.Not RetryInterval: serpent.Duration(time.Millisecond * 50), LeaseCount: 10, StoreSyncBufferSize: 50, - SMTP: codersdk.NotificationsEmailConfig{}, - Webhook: codersdk.NotificationsWebhookConfig{}, + SMTP: smtp, + Webhook: webhook, + Inbox: codersdk.NotificationsInboxConfig{ + Enabled: serpent.Bool(true), + }, } } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 428ebac494..299ab90b96 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -698,12 +698,19 @@ type NotificationsConfig struct { SMTP NotificationsEmailConfig `json:"email" typescript:",notnull"` // Webhook settings. Webhook NotificationsWebhookConfig `json:"webhook" typescript:",notnull"` + // Inbox settings. + Inbox NotificationsInboxConfig `json:"inbox" typescript:",notnull"` } +// Are either of the notification methods enabled? func (n *NotificationsConfig) Enabled() bool { return n.SMTP.Smarthost != "" || n.Webhook.Endpoint != serpent.URL{} } +type NotificationsInboxConfig struct { + Enabled serpent.Bool `json:"enabled" typescript:",notnull"` +} + type NotificationsEmailConfig struct { // The sender's address. From serpent.String `json:"from" typescript:",notnull"` @@ -989,6 +996,11 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Parent: &deploymentGroupNotifications, YAML: "webhook", } + deploymentGroupInbox = serpent.Group{ + Name: "Inbox", + Parent: &deploymentGroupNotifications, + YAML: "inbox", + } ) httpAddress := serpent.Option{ @@ -2856,6 +2868,16 @@ Write out the current server config as YAML to stdout.`, Group: &deploymentGroupNotificationsWebhook, YAML: "endpoint", }, + { + Name: "Notifications: Inbox: Enabled", + Description: "Enable Coder Inbox.", + Flag: "notifications-inbox-enabled", + Env: "CODER_NOTIFICATIONS_INBOX_ENABLED", + Value: &c.Notifications.Inbox.Enabled, + Default: "true", + Group: &deploymentGroupInbox, + YAML: "enabled", + }, { Name: "Notifications: Max Send Attempts", Description: "The upper limit of attempts to send a notification.", diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index 2b4a1e36c2..25ecf30311 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -293,6 +293,9 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ } }, "fetch_interval": 0, + "inbox": { + "enabled": true + }, "lease_count": 0, "lease_period": 0, "max_send_attempts": 0, diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index a7e5e1421e..3de353851d 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1943,6 +1943,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o } }, "fetch_interval": 0, + "inbox": { + "enabled": true + }, "lease_count": 0, "lease_period": 0, "max_send_attempts": 0, @@ -2416,6 +2419,9 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o } }, "fetch_interval": 0, + "inbox": { + "enabled": true + }, "lease_count": 0, "lease_period": 0, "max_send_attempts": 0, @@ -3757,6 +3763,9 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith } }, "fetch_interval": 0, + "inbox": { + "enabled": true + }, "lease_count": 0, "lease_period": 0, "max_send_attempts": 0, @@ -3789,6 +3798,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith | `dispatch_timeout` | integer | false | | How long to wait while a notification is being sent before giving up. | | `email` | [codersdk.NotificationsEmailConfig](#codersdknotificationsemailconfig) | false | | Email settings. | | `fetch_interval` | integer | false | | How often to query the database for queued notifications. | +| `inbox` | [codersdk.NotificationsInboxConfig](#codersdknotificationsinboxconfig) | false | | Inbox settings. | | `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. | | `max_send_attempts` | integer | false | | The upper limit of attempts to send a notification. | @@ -3878,6 +3888,20 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith | `server_name` | string | false | | Server name to verify the hostname for the targets. | | `start_tls` | boolean | false | | Start tls attempts to upgrade plain connections to TLS. | +## codersdk.NotificationsInboxConfig + +```json +{ + "enabled": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|-----------|---------|----------|--------------|-------------| +| `enabled` | boolean | false | | | + ## codersdk.NotificationsSettings ```json diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index 91d565952d..888e569f9d 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -1560,6 +1560,17 @@ Certificate key file to use. The endpoint to which to send webhooks. +### --notifications-inbox-enabled + +| | | +|-------------|-------------------------------------------------| +| Type | bool | +| Environment | $CODER_NOTIFICATIONS_INBOX_ENABLED | +| YAML | notifications.inbox.enabled | +| Default | true | + +Enable Coder Inbox. + ### --notifications-max-send-attempts | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index f0b3e4b0aa..e8f71dcd78 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -474,6 +474,10 @@ Configure TLS for your SMTP server target. Enable STARTTLS to upgrade insecure SMTP connections using TLS. DEPRECATED: Use --email-tls-starttls instead. +NOTIFICATIONS / INBOX OPTIONS: + --notifications-inbox-enabled bool, $CODER_NOTIFICATIONS_INBOX_ENABLED (default: true) + Enable Coder Inbox. + NOTIFICATIONS / WEBHOOK OPTIONS: --notifications-webhook-endpoint url, $CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT The endpoint to which to send webhooks. diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 01ed0c919a..61f7bd77c4 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1313,6 +1313,7 @@ export interface NotificationsConfig { readonly dispatch_timeout: number; readonly email: NotificationsEmailConfig; readonly webhook: NotificationsWebhookConfig; + readonly inbox: NotificationsInboxConfig; } // From codersdk/deployment.go @@ -1343,6 +1344,11 @@ export interface NotificationsEmailTLSConfig { readonly key_file: string; } +// From codersdk/deployment.go +export interface NotificationsInboxConfig { + readonly enabled: boolean; +} + // From codersdk/notifications.go export interface NotificationsSettings { readonly notifier_paused: boolean;