Skip to content

feat(coderd): add webpush package #17091

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 40 commits into from
Mar 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a980379
wip
kylecarbs Mar 17, 2025
bc02200
fix migration number
johnstcn Mar 25, 2025
2ecae9d
make fmt
johnstcn Mar 25, 2025
84e3ace
fix tests
johnstcn Mar 25, 2025
377eaab
add cli command to regenerate vapid keypair and remove existing subsc…
johnstcn Mar 25, 2025
982acef
remove dbauthz system usage
johnstcn Mar 25, 2025
d82073e
add test endpoint for push notifications
johnstcn Mar 25, 2025
fc59c70
make linter happy
johnstcn Mar 25, 2025
cbff3ca
test push notifications endpoint
johnstcn Mar 25, 2025
9e7e1dc
add deployment config for push notifications
johnstcn Mar 25, 2025
1315a46
add test for push notifications being enabled
johnstcn Mar 25, 2025
85db78c
rename migration
johnstcn Mar 25, 2025
892388a
skip vapid keypair test on non-postgres;
johnstcn Mar 25, 2025
e600b7d
fix some tests
johnstcn Mar 25, 2025
eef2038
hide subscribe button
johnstcn Mar 25, 2025
46f7519
conditionally hide push notification button
johnstcn Mar 25, 2025
4408090
Revert "conditionally hide push notification button"
johnstcn Mar 25, 2025
204ab4a
fix data race caused by webpush-go mutating msg []byte
johnstcn Mar 25, 2025
0535ed6
Remove deployment config for push notifications in favour of Experime…
johnstcn Mar 26, 2025
9f1f4f9
push notification -> webpush
johnstcn Mar 26, 2025
29bba04
notification -> webpush
johnstcn Mar 26, 2025
46c2cd8
move to webpush package
johnstcn Mar 26, 2025
960c5db
webpush: refactor notification test and send logic
johnstcn Mar 26, 2025
aa22161
move endpoints under webpush beside notifications
johnstcn Mar 26, 2025
57d84a9
skip apidocgen for push notification endpoints
johnstcn Mar 26, 2025
ef22b35
make webpush endpoints 404 if experiment not enabled
johnstcn Mar 26, 2025
9a1a605
auth on test notification endpoint
johnstcn Mar 26, 2025
e8b6083
fix ts
johnstcn Mar 26, 2025
5331f9c
ui fixes
johnstcn Mar 26, 2025
449f882
fixup migrations
johnstcn Mar 26, 2025
e5fd00e
fix check_unstaged.sh again
johnstcn Mar 26, 2025
bcf108a
make bee jenn
johnstcn Mar 26, 2025
eb7b102
address comments
johnstcn Mar 26, 2025
9b5ed09
address more comments + renaming
johnstcn Mar 26, 2025
c06558e
move out check_unstaged.sh changes to separate PR
johnstcn Mar 26, 2025
10ac9fb
remove site changes
johnstcn Mar 26, 2025
a114ddb
fixup! remove site changes
johnstcn Mar 26, 2025
ca72676
nits
johnstcn Mar 27, 2025
3b1dc70
make gen
johnstcn Mar 27, 2025
12808f1
gen
johnstcn Mar 27, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import (
"github.com/coder/coder/v2/coderd/entitlements"
"github.com/coder/coder/v2/coderd/notifications/reports"
"github.com/coder/coder/v2/coderd/runtimeconfig"
"github.com/coder/coder/v2/coderd/webpush"

"github.com/coder/coder/v2/buildinfo"
"github.com/coder/coder/v2/cli/clilog"
Expand Down Expand Up @@ -775,6 +776,26 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
return xerrors.Errorf("set deployment id: %w", err)
}

// Manage push notifications.
experiments := coderd.ReadExperiments(options.Logger, options.DeploymentValues.Experiments.Value())
if experiments.Enabled(codersdk.ExperimentWebPush) {
webpusher, err := webpush.New(ctx, &options.Logger, options.Database)
if err != nil {
options.Logger.Error(ctx, "failed to create web push dispatcher", slog.Error(err))
options.Logger.Warn(ctx, "web push notifications will not work until the VAPID keys are regenerated")
webpusher = &webpush.NoopWebpusher{
Msg: "Web Push notifications are disabled due to a system error. Please contact your Coder administrator.",
}
}
options.WebPushDispatcher = webpusher
} else {
options.WebPushDispatcher = &webpush.NoopWebpusher{
// Users will likely not see this message as the endpoints return 404
// if not enabled. Just in case...
Msg: "Web Push notifications are an experimental feature and are disabled by default. Enable the 'web-push' experiment to use this feature.",
}
}

githubOAuth2ConfigParams, err := getGithubOAuth2ConfigParams(ctx, options.Database, vals)
if err != nil {
return xerrors.Errorf("get github oauth2 config params: %w", err)
Expand Down Expand Up @@ -1255,6 +1276,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
}

createAdminUserCmd := r.newCreateAdminUserCommand()
regenerateVapidKeypairCmd := r.newRegenerateVapidKeypairCommand()

rawURLOpt := serpent.Option{
Flag: "raw-url",
Expand All @@ -1268,7 +1290,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.

serverCmd.Children = append(
serverCmd.Children,
createAdminUserCmd, postgresBuiltinURLCmd, postgresBuiltinServeCmd,
createAdminUserCmd, postgresBuiltinURLCmd, postgresBuiltinServeCmd, regenerateVapidKeypairCmd,
)

return serverCmd
Expand Down
112 changes: 112 additions & 0 deletions cli/server_regenerate_vapid_keypair.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
//go:build !slim

package cli

import (
"fmt"

"golang.org/x/xerrors"

"cdr.dev/slog"
"cdr.dev/slog/sloggers/sloghuman"

"github.com/coder/coder/v2/cli/cliui"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/awsiamrds"
"github.com/coder/coder/v2/coderd/webpush"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/serpent"
)

func (r *RootCmd) newRegenerateVapidKeypairCommand() *serpent.Command {
var (
regenVapidKeypairDBURL string
regenVapidKeypairPgAuth string
)
regenerateVapidKeypairCommand := &serpent.Command{
Use: "regenerate-vapid-keypair",
Short: "Regenerate the VAPID keypair used for web push notifications.",
Hidden: true, // Hide this command as it's an experimental feature
Handler: func(inv *serpent.Invocation) error {
var (
ctx, cancel = inv.SignalNotifyContext(inv.Context(), StopSignals...)
cfg = r.createConfig()
logger = inv.Logger.AppendSinks(sloghuman.Sink(inv.Stderr))
)
if r.verbose {
logger = logger.Leveled(slog.LevelDebug)
}

defer cancel()

if regenVapidKeypairDBURL == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm quite sure I have seen this pattern in other CLI commands:
startBuiltinPostgres - codersdk.PostgresAuth - ConnectToPostgres

we may consider some base pattern for all commands using the database

Copy link
Member Author

@johnstcn johnstcn Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep there are a few commands that do this. I'll create a follow-up. (EDIT: coder/internal#541)

cliui.Infof(inv.Stdout, "Using built-in PostgreSQL (%s)", cfg.PostgresPath())
url, closePg, err := startBuiltinPostgres(ctx, cfg, logger, "")
if err != nil {
return err
}
defer func() {
_ = closePg()
}()
regenVapidKeypairDBURL = url
}

sqlDriver := "postgres"
var err error
if codersdk.PostgresAuth(regenVapidKeypairPgAuth) == codersdk.PostgresAuthAWSIAMRDS {
sqlDriver, err = awsiamrds.Register(inv.Context(), sqlDriver)
if err != nil {
return xerrors.Errorf("register aws rds iam auth: %w", err)
}
}

sqlDB, err := ConnectToPostgres(ctx, logger, sqlDriver, regenVapidKeypairDBURL, nil)
if err != nil {
return xerrors.Errorf("connect to postgres: %w", err)
}
defer func() {
_ = sqlDB.Close()
}()
db := database.New(sqlDB)

// Confirm that the user really wants to regenerate the VAPID keypair.
cliui.Infof(inv.Stdout, "Regenerating VAPID keypair...")
cliui.Infof(inv.Stdout, "This will delete all existing webpush subscriptions.")
cliui.Infof(inv.Stdout, "Are you sure you want to continue? (y/N)")

if resp, err := cliui.Prompt(inv, cliui.PromptOptions{
IsConfirm: true,
Default: cliui.ConfirmNo,
}); err != nil || resp != cliui.ConfirmYes {
return xerrors.Errorf("VAPID keypair regeneration failed: %w", err)
}

if _, _, err := webpush.RegenerateVAPIDKeys(ctx, db); err != nil {
return xerrors.Errorf("regenerate vapid keypair: %w", err)
}

_, _ = fmt.Fprintln(inv.Stdout, "VAPID keypair regenerated successfully.")
return nil
},
}

regenerateVapidKeypairCommand.Options.Add(
cliui.SkipPromptOption(),
serpent.Option{
Env: "CODER_PG_CONNECTION_URL",
Flag: "postgres-url",
Description: "URL of a PostgreSQL database. If empty, the built-in PostgreSQL deployment will be used (Coder must not be already running in this case).",
Value: serpent.StringOf(&regenVapidKeypairDBURL),
},
serpent.Option{
Name: "Postgres Connection Auth",
Description: "Type of auth to use when connecting to postgres.",
Flag: "postgres-connection-auth",
Env: "CODER_PG_CONNECTION_AUTH",
Default: "password",
Value: serpent.EnumOf(&regenVapidKeypairPgAuth, codersdk.PostgresAuthDrivers...),
},
)

return regenerateVapidKeypairCommand
}
118 changes: 118 additions & 0 deletions cli/server_regenerate_vapid_keypair_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package cli_test

import (
"context"
"database/sql"
"testing"

"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/cli/clitest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/pty/ptytest"
"github.com/coder/coder/v2/testutil"
)

func TestRegenerateVapidKeypair(t *testing.T) {
t.Parallel()
if !dbtestutil.WillUsePostgres() {
t.Skip("this test is only supported on postgres")
}

t.Run("NoExistingVAPIDKeys", func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
t.Cleanup(cancel)

connectionURL, err := dbtestutil.Open(t)
require.NoError(t, err)

sqlDB, err := sql.Open("postgres", connectionURL)
require.NoError(t, err)
defer sqlDB.Close()

db := database.New(sqlDB)
// Ensure there is no existing VAPID keypair.
rows, err := db.GetWebpushVAPIDKeys(ctx)
require.NoError(t, err)
require.Empty(t, rows)

inv, _ := clitest.New(t, "server", "regenerate-vapid-keypair", "--postgres-url", connectionURL, "--yes")

pty := ptytest.New(t)
inv.Stdout = pty.Output()
inv.Stderr = pty.Output()
clitest.Start(t, inv)

pty.ExpectMatchContext(ctx, "Regenerating VAPID keypair...")
pty.ExpectMatchContext(ctx, "This will delete all existing webpush subscriptions.")
pty.ExpectMatchContext(ctx, "Are you sure you want to continue? (y/N)")
pty.WriteLine("y")
pty.ExpectMatchContext(ctx, "VAPID keypair regenerated successfully.")

// Ensure the VAPID keypair was created.
keys, err := db.GetWebpushVAPIDKeys(ctx)
require.NoError(t, err)
require.NotEmpty(t, keys.VapidPublicKey)
require.NotEmpty(t, keys.VapidPrivateKey)
})

t.Run("ExistingVAPIDKeys", func(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
t.Cleanup(cancel)

connectionURL, err := dbtestutil.Open(t)
require.NoError(t, err)

sqlDB, err := sql.Open("postgres", connectionURL)
require.NoError(t, err)
defer sqlDB.Close()

db := database.New(sqlDB)
for i := 0; i < 10; i++ {
// Insert a few fake users.
u := dbgen.User(t, db, database.User{})
// Insert a few fake push subscriptions for each user.
for j := 0; j < 10; j++ {
_ = dbgen.WebpushSubscription(t, db, database.InsertWebpushSubscriptionParams{
UserID: u.ID,
})
}
}

inv, _ := clitest.New(t, "server", "regenerate-vapid-keypair", "--postgres-url", connectionURL, "--yes")

pty := ptytest.New(t)
inv.Stdout = pty.Output()
inv.Stderr = pty.Output()
clitest.Start(t, inv)

pty.ExpectMatchContext(ctx, "Regenerating VAPID keypair...")
pty.ExpectMatchContext(ctx, "This will delete all existing webpush subscriptions.")
pty.ExpectMatchContext(ctx, "Are you sure you want to continue? (y/N)")
pty.WriteLine("y")
pty.ExpectMatchContext(ctx, "VAPID keypair regenerated successfully.")

// Ensure the VAPID keypair was created.
keys, err := db.GetWebpushVAPIDKeys(ctx)
require.NoError(t, err)
require.NotEmpty(t, keys.VapidPublicKey)
require.NotEmpty(t, keys.VapidPrivateKey)

// Ensure the push subscriptions were deleted.
var count int64
rows, err := sqlDB.QueryContext(ctx, "SELECT COUNT(*) FROM webpush_subscriptions")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just confirming - is there a chance that there will be another testrun operating on this table (flakiness risk?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be. It operates entirely in its own transaction, and each test should be operating on its own database.

require.NoError(t, err)
t.Cleanup(func() {
_ = rows.Close()
})
require.True(t, rows.Next())
require.NoError(t, rows.Scan(&count))
require.Equal(t, int64(0), count)
})
}
12 changes: 6 additions & 6 deletions cli/testdata/coder_server_--help.golden
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ USAGE:
Start a Coder server

SUBCOMMANDS:
create-admin-user Create a new admin user with the given username,
email and password and adds it to every
organization.
postgres-builtin-serve Run the built-in PostgreSQL deployment.
postgres-builtin-url Output the connection URL for the built-in
PostgreSQL deployment.
create-admin-user Create a new admin user with the given username,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unrelated change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually not sure what's causing these changes, but the command to update these golden files is convinced that this is how it should be now.

email and password and adds it to every
organization.
postgres-builtin-serve Run the built-in PostgreSQL deployment.
postgres-builtin-url Output the connection URL for the built-in
PostgreSQL deployment.

OPTIONS:
--allow-workspace-renames bool, $CODER_ALLOW_WORKSPACE_RENAMES (default: false)
Expand Down
Loading
Loading