Skip to content

Commit a1b4e2b

Browse files
dannykoppingdannykopping
committed
Fixing lint errors & minor tests
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent d57c59b commit a1b4e2b

File tree

12 files changed

+221
-29
lines changed

12 files changed

+221
-29
lines changed

‎cli/testdata/coder_server_--help.golden

+62
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,68 @@ can safely ignore these settings.
326326
Minimum supported version of TLS. Accepted values are "tls10",
327327
"tls11", "tls12" or "tls13".
328328

329+
NOTIFICATIONS OPTIONS:
330+
--notifications-dispatch-timeout duration, $CODER_NOTIFICATIONS_DISPATCH_TIMEOUT (default: 1m0s)
331+
How long to wait while a notification is being sent before giving up.
332+
333+
--notifications-fetch-interval duration, $CODER_NOTIFICATIONS_FETCH_INTERVAL (default: 15s)
334+
How often to query the database for queued notifications.
335+
336+
--notifications-lease-count int, $CODER_NOTIFICATIONS_LEASE_COUNT (default: 10)
337+
How many notifications a notifier should lease per fetch interval.
338+
339+
--notifications-lease-period duration, $CODER_NOTIFICATIONS_LEASE_PERIOD (default: 2m0s)
340+
How long a notifier should lease a message. This is effectively how
341+
long a notification is 'owned' by a notifier, and once this period
342+
expires it will be available for lease by another notifier. Leasing is
343+
important in order for multiple running notifiers to not pick the same
344+
messages to deliver concurrently. This lease period will only expire
345+
if a notifier shuts down ungracefully; a dispatch of the notification
346+
releases the lease.
347+
348+
--notifications-max-send-attempts int, $CODER_NOTIFICATIONS_MAX_SEND_ATTEMPTS (default: 5)
349+
The upper limit of attempts to send a notification.
350+
351+
--notifications-method string, $CODER_NOTIFICATIONS_METHOD (default: smtp)
352+
Which delivery method to use (available options: 'smtp', 'webhook').
353+
354+
--notifications-retry-interval duration, $CODER_NOTIFICATIONS_RETRY_INTERVAL (default: 5m0s)
355+
The minimum time between retries.
356+
357+
--notifications-store-sync-buffer-size int, $CODER_NOTIFICATIONS_STORE_SYNC_BUFFER_SIZE (default: 50)
358+
The notifications system buffers message updates in memory to ease
359+
pressure on the database. This option controls how many updates are
360+
kept in memory. The lower this value the lower the change of state
361+
inconsistency in a non-graceful shutdown - but it also increases load
362+
on the database. It is recommended to keep this option at its default
363+
value.
364+
365+
--notifications-store-sync-interval duration, $CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL (default: 2s)
366+
The notifications system buffers message updates in memory to ease
367+
pressure on the database. This option controls how often it
368+
synchronizes its state with the database. The shorter this value the
369+
lower the change of state inconsistency in a non-graceful shutdown -
370+
but it also increases load on the database. It is recommended to keep
371+
this option at its default value.
372+
373+
--notifications-worker-count int, $CODER_NOTIFICATIONS_WORKER_COUNT (default: 2)
374+
How many workers should be processing messages in the queue; increase
375+
this count if notifications are not being processed fast enough.
376+
377+
NOTIFICATIONS / EMAIL OPTIONS:
378+
--notifications-email-from string, $CODER_NOTIFICATIONS_EMAIL_FROM
379+
The sender's address to use.
380+
381+
--notifications-email-hello string, $CODER_NOTIFICATIONS_EMAIL_HELLO (default: localhost)
382+
The hostname identifying the SMTP server.
383+
384+
--notifications-email-smarthost host:port, $CODER_NOTIFICATIONS_EMAIL_SMARTHOST (default: localhost:587)
385+
The intermediary SMTP host through which emails are sent.
386+
387+
NOTIFICATIONS / WEBHOOK OPTIONS:
388+
--notifications-webhook-endpoint url, $CODER_NOTIFICATIONS_WEBHOOK_ENDPOINT
389+
The endpoint to which to send webhooks.
390+
329391
OAUTH2 / GITHUB OPTIONS:
330392
--oauth2-github-allow-everyone bool, $CODER_OAUTH2_GITHUB_ALLOW_EVERYONE
331393
Allow all logins, setting this option means allowed orgs and teams

‎cli/testdata/server-config.yaml.golden

+59
Original file line numberDiff line numberDiff line change
@@ -493,3 +493,62 @@ userQuietHoursSchedule:
493493
# compatibility reasons, this will be removed in a future release.
494494
# (default: false, type: bool)
495495
allowWorkspaceRenames: false
496+
notifications:
497+
# The upper limit of attempts to send a notification.
498+
# (default: 5, type: int)
499+
max-send-attempts: 5
500+
# The minimum time between retries.
501+
# (default: 5m0s, type: duration)
502+
retry-interval: 5m0s
503+
# The notifications system buffers message updates in memory to ease pressure on
504+
# the database. This option controls how often it synchronizes its state with the
505+
# database. The shorter this value the lower the change of state inconsistency in
506+
# a non-graceful shutdown - but it also increases load on the database. It is
507+
# recommended to keep this option at its default value.
508+
# (default: 2s, type: duration)
509+
store-sync-interval: 2s
510+
# The notifications system buffers message updates in memory to ease pressure on
511+
# the database. This option controls how many updates are kept in memory. The
512+
# lower this value the lower the change of state inconsistency in a non-graceful
513+
# shutdown - but it also increases load on the database. It is recommended to keep
514+
# this option at its default value.
515+
# (default: 50, type: int)
516+
store-sync-buffer-size: 50
517+
# How many workers should be processing messages in the queue; increase this count
518+
# if notifications are not being processed fast enough.
519+
# (default: 2, type: int)
520+
worker-count: 2
521+
# How long a notifier should lease a message. This is effectively how long a
522+
# notification is 'owned' by a notifier, and once this period expires it will be
523+
# available for lease by another notifier. Leasing is important in order for
524+
# multiple running notifiers to not pick the same messages to deliver
525+
# concurrently. This lease period will only expire if a notifier shuts down
526+
# ungracefully; a dispatch of the notification releases the lease.
527+
# (default: 2m0s, type: duration)
528+
lease-period: 2m0s
529+
# How many notifications a notifier should lease per fetch interval.
530+
# (default: 10, type: int)
531+
lease-count: 10
532+
# How often to query the database for queued notifications.
533+
# (default: 15s, type: duration)
534+
fetch-interval: 15s
535+
# Which delivery method to use (available options: 'smtp', 'webhook').
536+
# (default: smtp, type: string)
537+
method: smtp
538+
# How long to wait while a notification is being sent before giving up.
539+
# (default: 1m0s, type: duration)
540+
dispatch-timeout: 1m0s
541+
email:
542+
# The sender's address to use.
543+
# (default: <unset>, type: string)
544+
from: ""
545+
# The intermediary SMTP host through which emails are sent.
546+
# (default: localhost:587, type: host:port)
547+
smarthost: localhost:587
548+
# The hostname identifying the SMTP server.
549+
# (default: localhost, type: string)
550+
hello: localhost
551+
webhook:
552+
# The endpoint to which to send webhooks.
553+
# (default: <unset>, type: url)
554+
hello:

‎coderd/database/dbmem/dbmem.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1218,7 +1218,7 @@ func (*FakeQuerier) BulkMarkNotificationMessagesFailed(_ context.Context, arg da
12181218
return 0, err
12191219
}
12201220

1221-
panic("not implemented")
1221+
return -1, nil
12221222
}
12231223

12241224
func (*FakeQuerier) BulkMarkNotificationMessagesSent(_ context.Context, arg database.BulkMarkNotificationMessagesSentParams) (int64, error) {
@@ -1227,7 +1227,7 @@ func (*FakeQuerier) BulkMarkNotificationMessagesSent(_ context.Context, arg data
12271227
return 0, err
12281228
}
12291229

1230-
panic("not implemented")
1230+
return -1, nil
12311231
}
12321232

12331233
func (*FakeQuerier) CleanTailnetCoordinators(_ context.Context) error {
@@ -1566,7 +1566,7 @@ func (q *FakeQuerier) DeleteOAuth2ProviderAppTokensByAppAndUserID(_ context.Cont
15661566
}
15671567

15681568
func (*FakeQuerier) DeleteOldNotificationMessages(context.Context) error {
1569-
panic("not implemented")
1569+
return nil
15701570
}
15711571

15721572
func (q *FakeQuerier) DeleteOldProvisionerDaemons(_ context.Context) error {
@@ -6189,7 +6189,7 @@ func (*FakeQuerier) InsertNotificationTemplate(_ context.Context, arg database.I
61896189
return database.NotificationTemplate{}, err
61906190
}
61916191

6192-
panic("not implemented")
6192+
return database.NotificationTemplate{}, nil
61936193
}
61946194

61956195
func (q *FakeQuerier) InsertOAuth2ProviderApp(_ context.Context, arg database.InsertOAuth2ProviderAppParams) (database.OAuth2ProviderApp, error) {

‎coderd/notifications/dispatch/webhook.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"encoding/json"
7+
"errors"
78
"io"
89
"net/http"
910

@@ -95,7 +96,7 @@ func (w *WebhookHandler) dispatch(msgPayload types.MessagePayload, title, body,
9596
// Body could be quite long here, let's grab the first 500B and hope it contains useful debug info.
9697
var respBody []byte
9798
respBody, err = abbreviatedRead(resp.Body, 500)
98-
if err != nil && err != io.EOF {
99+
if err != nil && !errors.Is(err, io.EOF) {
99100
return true, xerrors.Errorf("non-200 response (%d), read body: %w", resp.StatusCode, err)
100101
}
101102
w.log.Warn(ctx, "unsuccessful delivery", slog.F("status_code", resp.StatusCode),

‎coderd/notifications/events.go

+2-5
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,5 @@ package notifications
22

33
import "github.com/google/uuid"
44

5-
var (
6-
// Workspaces.
7-
TemplateWorkspaceDeleted = uuid.MustParse("'f517da0b-cdc9-410f-ab89-a86107c420ed'")
8-
// ...
9-
)
5+
// Workspaces.
6+
var TemplateWorkspaceDeleted = uuid.MustParse("'f517da0b-cdc9-410f-ab89-a86107c420ed'") // ...

‎coderd/notifications/manager_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ func TestBufferedUpdates(t *testing.T) {
8686
}
8787

8888
func TestBuildPayload(t *testing.T) {
89+
t.Parallel()
90+
8991
// given
9092
const label = "Click here!"
9193
const url = "http://xyz.com/"
@@ -198,6 +200,6 @@ func (e *enqueueInterceptor) EnqueueNotificationMessage(_ context.Context, arg d
198200
return database.NotificationMessage{}, err
199201
}
200202

201-
func (e *enqueueInterceptor) FetchNewMessageMetadata(_ context.Context, arg database.FetchNewMessageMetadataParams) (database.FetchNewMessageMetadataRow, error) {
203+
func (e *enqueueInterceptor) FetchNewMessageMetadata(_ context.Context, _ database.FetchNewMessageMetadataParams) (database.FetchNewMessageMetadataRow, error) {
202204
return e.metadataFn(), nil
203205
}

‎coderd/notifications/notifier.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,7 @@ func (n *notifier) prepare(ctx context.Context, msg database.AcquireNotification
170170
return nil, xerrors.Errorf("resolve handler: %w", err)
171171
}
172172

173-
var (
174-
title, body string
175-
)
173+
var title, body string
176174
if title, err = render.GoTemplate(msg.TitleTemplate, payload, nil); err != nil {
177175
return nil, xerrors.Errorf("render title: %w", err)
178176
}

‎coderd/notifications/system/system_test.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,20 @@ import (
55
"testing"
66
"time"
77

8+
"github.com/google/uuid"
9+
"github.com/stretchr/testify/require"
10+
811
"github.com/coder/coder/v2/coderd/notifications"
912
"github.com/coder/coder/v2/coderd/notifications/system"
1013
"github.com/coder/coder/v2/coderd/notifications/types"
1114
"github.com/coder/coder/v2/testutil"
12-
"github.com/google/uuid"
13-
"github.com/stretchr/testify/require"
1415
)
1516

1617
// TestNotifyWorkspaceDeleted tests the "public" interface for enqueueing notifications.
1718
// Calling system.NotifyWorkspaceDeleted uses the Enqueuer singleton to enqueue the notification.
1819
func TestNotifyWorkspaceDeleted(t *testing.T) {
20+
t.Parallel()
21+
1922
// given
2023
manager := newFakeEnqueuer()
2124
notifications.RegisterInstance(manager)
@@ -42,5 +45,6 @@ func newFakeEnqueuer() *fakeEnqueuer {
4245

4346
func (f *fakeEnqueuer) Enqueue(context.Context, uuid.UUID, uuid.UUID, types.Labels, string, ...uuid.UUID) (*uuid.UUID, error) {
4447
f.enqueued <- true
48+
// nolint:nilnil // Irrelevant.
4549
return nil, nil
4650
}

‎codersdk/deployment.go

+18-12
Original file line numberDiff line numberDiff line change
@@ -2104,19 +2104,21 @@ Write out the current server config as YAML to stdout.`,
21042104
Default: (time.Minute * 5).String(),
21052105
Group: &deploymentGroupNotifications,
21062106
YAML: "retry-interval",
2107+
Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
21072108
},
21082109
{
21092110
Name: "Notifications: Store Sync Interval",
21102111
Description: "The notifications system buffers message updates in memory to ease pressure on the database. " +
21112112
"This option controls how often it synchronizes its state with the database. The shorter this value the " +
21122113
"lower the change of state inconsistency in a non-graceful shutdown - but it also increases load on the " +
21132114
"database. It is recommended to keep this option at its default value.",
2114-
Flag: "notifications-store-sync-interval",
2115-
Env: "CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL",
2116-
Value: &c.Notifications.StoreSyncInterval,
2117-
Default: (time.Second * 2).String(),
2118-
Group: &deploymentGroupNotifications,
2119-
YAML: "store-sync-interval",
2115+
Flag: "notifications-store-sync-interval",
2116+
Env: "CODER_NOTIFICATIONS_STORE_SYNC_INTERVAL",
2117+
Value: &c.Notifications.StoreSyncInterval,
2118+
Default: (time.Second * 2).String(),
2119+
Group: &deploymentGroupNotifications,
2120+
YAML: "store-sync-interval",
2121+
Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
21202122
},
21212123
{
21222124
Name: "Notifications: Store Sync Buffer Size",
@@ -2149,12 +2151,13 @@ Write out the current server config as YAML to stdout.`,
21492151
"is important in order for multiple running notifiers to not pick the same messages to deliver concurrently. " +
21502152
"This lease period will only expire if a notifier shuts down ungracefully; a dispatch of the notification " +
21512153
"releases the lease.",
2152-
Flag: "notifications-lease-period",
2153-
Env: "CODER_NOTIFICATIONS_LEASE_PERIOD",
2154-
Value: &c.Notifications.LeasePeriod,
2155-
Default: (time.Minute * 2).String(),
2156-
Group: &deploymentGroupNotifications,
2157-
YAML: "lease-period",
2154+
Flag: "notifications-lease-period",
2155+
Env: "CODER_NOTIFICATIONS_LEASE_PERIOD",
2156+
Value: &c.Notifications.LeasePeriod,
2157+
Default: (time.Minute * 2).String(),
2158+
Group: &deploymentGroupNotifications,
2159+
YAML: "lease-period",
2160+
Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
21582161
},
21592162
{
21602163
Name: "Notifications: Lease Count",
@@ -2175,6 +2178,7 @@ Write out the current server config as YAML to stdout.`,
21752178
Default: (time.Second * 15).String(),
21762179
Group: &deploymentGroupNotifications,
21772180
YAML: "fetch-interval",
2181+
Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
21782182
},
21792183
{
21802184
Name: "Notifications: Method",
@@ -2195,6 +2199,7 @@ Write out the current server config as YAML to stdout.`,
21952199
Default: time.Minute.String(),
21962200
Group: &deploymentGroupNotifications,
21972201
YAML: "dispatch-timeout",
2202+
Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
21982203
},
21992204
{
22002205
Name: "Notifications: Email: From Address",
@@ -2210,6 +2215,7 @@ Write out the current server config as YAML to stdout.`,
22102215
Description: "The intermediary SMTP host through which emails are sent.",
22112216
Flag: "notifications-email-smarthost",
22122217
Env: "CODER_NOTIFICATIONS_EMAIL_SMARTHOST",
2218+
Default: "localhost:587", // To pass validation.
22132219
Value: &c.Notifications.SMTP.Smarthost,
22142220
Group: &deploymentGroupNotificationsEmail,
22152221
YAML: "smarthost",

‎docs/cli/server.md

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)