- Notifications
You must be signed in to change notification settings - Fork1k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes fromall commits
Commits
Show all changes
40 commits Select commitHold shift + click to select a range
a980379 wip
kylecarbsbc02200 fix migration number
johnstcn2ecae9d make fmt
johnstcn84e3ace fix tests
johnstcn377eaab add cli command to regenerate vapid keypair and remove existing subsc…
johnstcn982acef remove dbauthz system usage
johnstcnd82073e add test endpoint for push notifications
johnstcnfc59c70 make linter happy
johnstcncbff3ca test push notifications endpoint
johnstcn9e7e1dc add deployment config for push notifications
johnstcn1315a46 add test for push notifications being enabled
johnstcn85db78c rename migration
johnstcn892388a skip vapid keypair test on non-postgres;
johnstcne600b7d fix some tests
johnstcneef2038 hide subscribe button
johnstcn46f7519 conditionally hide push notification button
johnstcn4408090 Revert "conditionally hide push notification button"
johnstcn204ab4a fix data race caused by webpush-go mutating msg []byte
johnstcn0535ed6 Remove deployment config for push notifications in favour of Experime…
johnstcn9f1f4f9 push notification -> webpush
johnstcn29bba04 notification -> webpush
johnstcn46c2cd8 move to webpush package
johnstcn960c5db webpush: refactor notification test and send logic
johnstcnaa22161 move endpoints under webpush beside notifications
johnstcn57d84a9 skip apidocgen for push notification endpoints
johnstcnef22b35 make webpush endpoints 404 if experiment not enabled
johnstcn9a1a605 auth on test notification endpoint
johnstcne8b6083 fix ts
johnstcn5331f9c ui fixes
johnstcn449f882 fixup migrations
johnstcne5fd00e fix check_unstaged.sh again
johnstcnbcf108a make bee jenn
johnstcneb7b102 address comments
johnstcn9b5ed09 address more comments + renaming
johnstcnc06558e move out check_unstaged.sh changes to separate PR
johnstcn10ac9fb remove site changes
johnstcna114ddb fixup! remove site changes
johnstcnca72676 nits
johnstcn3b1dc70 make gen
johnstcn12808f1 gen
johnstcnFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
24 changes: 23 additions & 1 deletioncli/server.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" | ||
| @@ -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) | ||
| @@ -1255,6 +1276,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. | ||
| } | ||
| createAdminUserCmd := r.newCreateAdminUserCommand() | ||
| regenerateVapidKeypairCmd := r.newRegenerateVapidKeypairCommand() | ||
johnstcn marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| rawURLOpt := serpent.Option{ | ||
| Flag: "raw-url", | ||
| @@ -1268,7 +1290,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. | ||
| serverCmd.Children = append( | ||
| serverCmd.Children, | ||
| createAdminUserCmd, postgresBuiltinURLCmd, postgresBuiltinServeCmd, regenerateVapidKeypairCmd, | ||
| ) | ||
| return serverCmd | ||
112 changes: 112 additions & 0 deletionscli/server_regenerate_vapid_keypair.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| 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 ( | ||
| regenVapidKeypairDBURLstring | ||
| regenVapidKeypairPgAuthstring | ||
| ) | ||
| 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)) | ||
| ) | ||
| ifr.verbose { | ||
| logger=logger.Leveled(slog.LevelDebug) | ||
| } | ||
| defercancel() | ||
| ifregenVapidKeypairDBURL=="" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: we may consider some base pattern for all commands using the database MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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,"") | ||
| iferr!=nil { | ||
| returnerr | ||
| } | ||
| deferfunc() { | ||
| _=closePg() | ||
| }() | ||
| regenVapidKeypairDBURL=url | ||
| } | ||
| sqlDriver:="postgres" | ||
| varerrerror | ||
| ifcodersdk.PostgresAuth(regenVapidKeypairPgAuth)==codersdk.PostgresAuthAWSIAMRDS { | ||
| sqlDriver,err=awsiamrds.Register(inv.Context(),sqlDriver) | ||
| iferr!=nil { | ||
| returnxerrors.Errorf("register aws rds iam auth: %w",err) | ||
| } | ||
| } | ||
| sqlDB,err:=ConnectToPostgres(ctx,logger,sqlDriver,regenVapidKeypairDBURL,nil) | ||
| iferr!=nil { | ||
| returnxerrors.Errorf("connect to postgres: %w",err) | ||
| } | ||
| deferfunc() { | ||
| _=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)") | ||
| ifresp,err:=cliui.Prompt(inv, cliui.PromptOptions{ | ||
| IsConfirm:true, | ||
| Default:cliui.ConfirmNo, | ||
| });err!=nil||resp!=cliui.ConfirmYes { | ||
| returnxerrors.Errorf("VAPID keypair regeneration failed: %w",err) | ||
| } | ||
| if_,_,err:=webpush.RegenerateVAPIDKeys(ctx,db);err!=nil { | ||
| returnxerrors.Errorf("regenerate vapid keypair: %w",err) | ||
| } | ||
| _,_=fmt.Fprintln(inv.Stdout,"VAPID keypair regenerated successfully.") | ||
| returnnil | ||
| }, | ||
| } | ||
| 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(®enVapidKeypairDBURL), | ||
| }, | ||
| 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(®enVapidKeypairPgAuth,codersdk.PostgresAuthDrivers...), | ||
| }, | ||
| ) | ||
| returnregenerateVapidKeypairCommand | ||
| } | ||
118 changes: 118 additions & 0 deletionscli/server_regenerate_vapid_keypair_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| 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" | ||
| ) | ||
| funcTestRegenerateVapidKeypair(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) | ||
| defersqlDB.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) | ||
| defersqlDB.Close() | ||
| db:=database.New(sqlDB) | ||
| fori:=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. | ||
| forj:=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. | ||
| varcountint64 | ||
| rows,err:=sqlDB.QueryContext(ctx,"SELECT COUNT(*) FROM webpush_subscriptions") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 deletionscli/testdata/coder_server_--help.golden
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,12 +6,12 @@ USAGE: | ||
| Start a Coder server | ||
| SUBCOMMANDS: | ||
| create-admin-userCreate a new admin user with the given username, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. nit: unrelated change? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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-serveRun the built-in PostgreSQL deployment. | ||
| postgres-builtin-urlOutput the connection URL for the built-in | ||
| PostgreSQL deployment. | ||
| OPTIONS: | ||
| --allow-workspace-renames bool, $CODER_ALLOW_WORKSPACE_RENAMES (default: false) | ||
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.