Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit3e3de05

Browse files
fix: send workspace create/update notifications to template admins only (#16071)
Relates to#15845Rather than sending the notification to the user, we send it to thetemplate admins. We also do not send it to the person that created therequest.
1 parentcd62e39 commit3e3de05

File tree

4 files changed

+103
-33
lines changed

4 files changed

+103
-33
lines changed

‎coderd/workspacebuilds.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,20 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
446446
// If this workspace build has a different template version ID to the previous build
447447
// we can assume it has just been updated.
448448
ifcreateBuild.TemplateVersionID!=uuid.Nil&&createBuild.TemplateVersionID!=previousWorkspaceBuild.TemplateVersionID {
449-
api.notifyWorkspaceUpdated(ctx,apiKey.UserID,workspace,createBuild.RichParameterValues)
449+
// nolint:gocritic // Need system context to fetch admins
450+
admins,err:=findTemplateAdmins(dbauthz.AsSystemRestricted(ctx),api.Database)
451+
iferr!=nil {
452+
api.Logger.Error(ctx,"find template admins",slog.Error(err))
453+
}else {
454+
for_,admin:=rangeadmins {
455+
// Don't send notifications to user which initiated the event.
456+
ifadmin.ID==apiKey.UserID {
457+
continue
458+
}
459+
460+
api.notifyWorkspaceUpdated(ctx,apiKey.UserID,admin.ID,workspace,createBuild.RichParameterValues)
461+
}
462+
}
450463
}
451464

452465
api.publishWorkspaceUpdate(ctx,workspace.OwnerID, wspubsub.WorkspaceEvent{
@@ -460,6 +473,7 @@ func (api *API) postWorkspaceBuilds(rw http.ResponseWriter, r *http.Request) {
460473
func (api*API)notifyWorkspaceUpdated(
461474
ctx context.Context,
462475
initiatorID uuid.UUID,
476+
receiverID uuid.UUID,
463477
workspace database.Workspace,
464478
parameters []codersdk.WorkspaceBuildParameter,
465479
) {
@@ -500,7 +514,7 @@ func (api *API) notifyWorkspaceUpdated(
500514
if_,err:=api.NotificationsEnqueuer.EnqueueWithData(
501515
// nolint:gocritic // Need notifier actor to enqueue notifications
502516
dbauthz.AsNotifier(ctx),
503-
workspace.OwnerID,
517+
receiverID,
504518
notifications.TemplateWorkspaceManuallyUpdated,
505519
map[string]string{
506520
"organization":template.OrganizationName,

‎coderd/workspacebuilds_test.go

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -565,30 +565,31 @@ func TestWorkspaceBuildResources(t *testing.T) {
565565
funcTestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t*testing.T) {
566566
t.Parallel()
567567

568-
t.Run("OnlyOneNotification",func(t*testing.T) {
568+
t.Run("NoRepeatedNotifications",func(t*testing.T) {
569569
t.Parallel()
570570

571571
notify:=&notificationstest.FakeEnqueuer{}
572572

573573
client:=coderdtest.New(t,&coderdtest.Options{IncludeProvisionerDaemon:true,NotificationsEnqueuer:notify})
574574
first:=coderdtest.CreateFirstUser(t,client)
575+
templateAdminClient,templateAdmin:=coderdtest.CreateAnotherUser(t,client,first.OrganizationID,rbac.RoleTemplateAdmin())
575576
userClient,user:=coderdtest.CreateAnotherUser(t,client,first.OrganizationID)
576577

577578
// Create a template with an initial version
578-
version:=coderdtest.CreateTemplateVersion(t,client,first.OrganizationID,nil)
579-
coderdtest.AwaitTemplateVersionJobCompleted(t,client,version.ID)
580-
template:=coderdtest.CreateTemplate(t,client,first.OrganizationID,version.ID)
579+
version:=coderdtest.CreateTemplateVersion(t,templateAdminClient,first.OrganizationID,nil)
580+
coderdtest.AwaitTemplateVersionJobCompleted(t,templateAdminClient,version.ID)
581+
template:=coderdtest.CreateTemplate(t,templateAdminClient,first.OrganizationID,version.ID)
581582

582583
// Create a workspace using this template
583584
workspace:=coderdtest.CreateWorkspace(t,userClient,template.ID)
584585
coderdtest.AwaitWorkspaceBuildJobCompleted(t,userClient,workspace.LatestBuild.ID)
585586
coderdtest.MustTransitionWorkspace(t,userClient,workspace.ID,database.WorkspaceTransitionStart,database.WorkspaceTransitionStop)
586587

587588
// Create a new version of the template
588-
newVersion:=coderdtest.CreateTemplateVersion(t,client,first.OrganizationID,nil,func(ctvr*codersdk.CreateTemplateVersionRequest) {
589+
newVersion:=coderdtest.CreateTemplateVersion(t,templateAdminClient,first.OrganizationID,nil,func(ctvr*codersdk.CreateTemplateVersionRequest) {
589590
ctvr.TemplateID=template.ID
590591
})
591-
coderdtest.AwaitTemplateVersionJobCompleted(t,client,newVersion.ID)
592+
coderdtest.AwaitTemplateVersionJobCompleted(t,templateAdminClient,newVersion.ID)
592593

593594
// Create a workspace build using this new template version
594595
build:=coderdtest.CreateWorkspaceBuild(t,userClient,workspace,database.WorkspaceTransitionStart,func(cwbr*codersdk.CreateWorkspaceBuildRequest) {
@@ -597,21 +598,45 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
597598
coderdtest.AwaitWorkspaceBuildJobCompleted(t,userClient,build.ID)
598599
coderdtest.MustTransitionWorkspace(t,userClient,workspace.ID,database.WorkspaceTransitionStart,database.WorkspaceTransitionStop)
599600

600-
// Create the workspace build _again_. We are doing this to ensure we only create 1 notification.
601+
// Create the workspace build _again_. We are doing this to
602+
// ensure we do not create _another_ notification. This is
603+
// separate to the notifications subsystem dedupe mechanism
604+
// as this build shouldn't create a notification. It shouldn't
605+
// create another notification as this new build isn't changing
606+
// the template version.
601607
build=coderdtest.CreateWorkspaceBuild(t,userClient,workspace,database.WorkspaceTransitionStart,func(cwbr*codersdk.CreateWorkspaceBuildRequest) {
602608
cwbr.TemplateVersionID=newVersion.ID
603609
})
604610
coderdtest.AwaitWorkspaceBuildJobCompleted(t,userClient,build.ID)
605611
coderdtest.MustTransitionWorkspace(t,userClient,workspace.ID,database.WorkspaceTransitionStart,database.WorkspaceTransitionStop)
606612

607-
// Ensure we receive only 1 workspace manually updated notification
613+
// We're going to have two notifications (one for the first user and one for the template admin)
614+
// By ensuring we only have these two, we are sure the second build didn't trigger more
615+
// notifications.
608616
sent:=notify.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceManuallyUpdated))
609-
require.Len(t,sent,1)
610-
require.Equal(t,user.ID,sent[0].UserID)
617+
require.Len(t,sent,2)
618+
619+
receivers:=make([]uuid.UUID,len(sent))
620+
foridx,notif:=rangesent {
621+
receivers[idx]=notif.UserID
622+
}
623+
624+
// Check the notification was sent to the first user and template admin
625+
// (both of whom have the "template admin" role), and explicitly not the
626+
// workspace owner (since they initiated the workspace build).
627+
require.Contains(t,receivers,templateAdmin.ID)
628+
require.Contains(t,receivers,first.UserID)
629+
require.NotContains(t,receivers,user.ID)
630+
611631
require.Contains(t,sent[0].Targets,template.ID)
612632
require.Contains(t,sent[0].Targets,workspace.ID)
613633
require.Contains(t,sent[0].Targets,workspace.OrganizationID)
614634
require.Contains(t,sent[0].Targets,workspace.OwnerID)
635+
636+
require.Contains(t,sent[1].Targets,template.ID)
637+
require.Contains(t,sent[1].Targets,workspace.ID)
638+
require.Contains(t,sent[1].Targets,workspace.OrganizationID)
639+
require.Contains(t,sent[1].Targets,workspace.OwnerID)
615640
})
616641

617642
t.Run("ToCorrectUser",func(t*testing.T) {
@@ -621,23 +646,24 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
621646

622647
client:=coderdtest.New(t,&coderdtest.Options{IncludeProvisionerDaemon:true,NotificationsEnqueuer:notify})
623648
first:=coderdtest.CreateFirstUser(t,client)
624-
userClient,user:=coderdtest.CreateAnotherUser(t,client,first.OrganizationID)
649+
templateAdminClient,templateAdmin:=coderdtest.CreateAnotherUser(t,client,first.OrganizationID,rbac.RoleTemplateAdmin())
650+
userClient,_:=coderdtest.CreateAnotherUser(t,client,first.OrganizationID)
625651

626652
// Create a template with an initial version
627-
version:=coderdtest.CreateTemplateVersion(t,client,first.OrganizationID,nil)
628-
coderdtest.AwaitTemplateVersionJobCompleted(t,client,version.ID)
629-
template:=coderdtest.CreateTemplate(t,client,first.OrganizationID,version.ID)
653+
version:=coderdtest.CreateTemplateVersion(t,templateAdminClient,first.OrganizationID,nil)
654+
coderdtest.AwaitTemplateVersionJobCompleted(t,templateAdminClient,version.ID)
655+
template:=coderdtest.CreateTemplate(t,templateAdminClient,first.OrganizationID,version.ID)
630656

631657
// Create a workspace using this template
632658
workspace:=coderdtest.CreateWorkspace(t,userClient,template.ID)
633659
coderdtest.AwaitWorkspaceBuildJobCompleted(t,userClient,workspace.LatestBuild.ID)
634660
coderdtest.MustTransitionWorkspace(t,userClient,workspace.ID,database.WorkspaceTransitionStart,database.WorkspaceTransitionStop)
635661

636662
// Create a new version of the template
637-
newVersion:=coderdtest.CreateTemplateVersion(t,client,first.OrganizationID,nil,func(ctvr*codersdk.CreateTemplateVersionRequest) {
663+
newVersion:=coderdtest.CreateTemplateVersion(t,templateAdminClient,first.OrganizationID,nil,func(ctvr*codersdk.CreateTemplateVersionRequest) {
638664
ctvr.TemplateID=template.ID
639665
})
640-
coderdtest.AwaitTemplateVersionJobCompleted(t,client,newVersion.ID)
666+
coderdtest.AwaitTemplateVersionJobCompleted(t,templateAdminClient,newVersion.ID)
641667

642668
// Create a workspace build using this new template version from a different user
643669
ctx:=testutil.Context(t,testutil.WaitShort)
@@ -652,7 +678,7 @@ func TestWorkspaceBuildWithUpdatedTemplateVersionSendsNotification(t *testing.T)
652678
// Ensure we receive only 1 workspace manually updated notification and to the right user
653679
sent:=notify.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceManuallyUpdated))
654680
require.Len(t,sent,1)
655-
require.Equal(t,user.ID,sent[0].UserID)
681+
require.Equal(t,templateAdmin.ID,sent[0].UserID)
656682
require.Contains(t,sent[0].Targets,template.ID)
657683
require.Contains(t,sent[0].Targets,workspace.ID)
658684
require.Contains(t,sent[0].Targets,workspace.OrganizationID)

‎coderd/workspaces.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -665,9 +665,6 @@ func createWorkspace(
665665
)
666666
returnerr
667667
},nil)
668-
669-
api.notifyWorkspaceCreated(ctx,workspace,req.RichParameterValues)
670-
671668
varbldErr wsbuilder.BuildError
672669
ifxerrors.As(err,&bldErr) {
673670
httpapi.Write(ctx,rw,bldErr.Status, codersdk.Response{
@@ -689,6 +686,21 @@ func createWorkspace(
689686
api.Logger.Error(ctx,"failed to post provisioner job to pubsub",slog.Error(err))
690687
}
691688

689+
// nolint:gocritic // Need system context to fetch admins
690+
admins,err:=findTemplateAdmins(dbauthz.AsSystemRestricted(ctx),api.Database)
691+
iferr!=nil {
692+
api.Logger.Error(ctx,"find template admins",slog.Error(err))
693+
}else {
694+
for_,admin:=rangeadmins {
695+
// Don't send notifications to user which initiated the event.
696+
ifadmin.ID==initiatorID {
697+
continue
698+
}
699+
700+
api.notifyWorkspaceCreated(ctx,admin.ID,workspace,req.RichParameterValues)
701+
}
702+
}
703+
692704
auditReq.New=workspace.WorkspaceTable()
693705

694706
api.Telemetry.Report(&telemetry.Snapshot{
@@ -739,6 +751,7 @@ func createWorkspace(
739751

740752
func (api*API)notifyWorkspaceCreated(
741753
ctx context.Context,
754+
receiverID uuid.UUID,
742755
workspace database.Workspace,
743756
parameters []codersdk.WorkspaceBuildParameter,
744757
) {
@@ -773,7 +786,7 @@ func (api *API) notifyWorkspaceCreated(
773786
if_,err:=api.NotificationsEnqueuer.EnqueueWithData(
774787
// nolint:gocritic // Need notifier actor to enqueue notifications
775788
dbauthz.AsNotifier(ctx),
776-
workspace.OwnerID,
789+
receiverID,
777790
notifications.TemplateWorkspaceCreated,
778791
map[string]string{
779792
"workspace":workspace.Name,

‎coderd/workspaces_test.go

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -577,22 +577,38 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
577577
enqueuer:= notificationstest.FakeEnqueuer{}
578578
client:=coderdtest.New(t,&coderdtest.Options{IncludeProvisionerDaemon:true,NotificationsEnqueuer:&enqueuer})
579579
user:=coderdtest.CreateFirstUser(t,client)
580+
templateAdminClient,templateAdmin:=coderdtest.CreateAnotherUser(t,client,user.OrganizationID,rbac.RoleTemplateAdmin())
580581
memberClient,memberUser:=coderdtest.CreateAnotherUser(t,client,user.OrganizationID)
581582

582-
version:=coderdtest.CreateTemplateVersion(t,client,user.OrganizationID,nil)
583-
template:=coderdtest.CreateTemplate(t,client,user.OrganizationID,version.ID)
584-
coderdtest.AwaitTemplateVersionJobCompleted(t,client,version.ID)
583+
version:=coderdtest.CreateTemplateVersion(t,templateAdminClient,user.OrganizationID,nil)
584+
template:=coderdtest.CreateTemplate(t,templateAdminClient,user.OrganizationID,version.ID)
585+
coderdtest.AwaitTemplateVersionJobCompleted(t,templateAdminClient,version.ID)
585586

586587
workspace:=coderdtest.CreateWorkspace(t,memberClient,template.ID)
587588
coderdtest.AwaitWorkspaceBuildJobCompleted(t,memberClient,workspace.LatestBuild.ID)
588589

589590
sent:=enqueuer.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceCreated))
590-
require.Len(t,sent,1)
591-
require.Equal(t,memberUser.ID,sent[0].UserID)
591+
require.Len(t,sent,2)
592+
593+
receivers:=make([]uuid.UUID,len(sent))
594+
foridx,notif:=rangesent {
595+
receivers[idx]=notif.UserID
596+
}
597+
598+
// Check the notification was sent to the first user and template admin
599+
require.Contains(t,receivers,templateAdmin.ID)
600+
require.Contains(t,receivers,user.UserID)
601+
require.NotContains(t,receivers,memberUser.ID)
602+
592603
require.Contains(t,sent[0].Targets,template.ID)
593604
require.Contains(t,sent[0].Targets,workspace.ID)
594605
require.Contains(t,sent[0].Targets,workspace.OrganizationID)
595606
require.Contains(t,sent[0].Targets,workspace.OwnerID)
607+
608+
require.Contains(t,sent[1].Targets,template.ID)
609+
require.Contains(t,sent[1].Targets,workspace.ID)
610+
require.Contains(t,sent[1].Targets,workspace.OrganizationID)
611+
require.Contains(t,sent[1].Targets,workspace.OwnerID)
596612
})
597613

598614
t.Run("CreateSendsNotificationToCorrectUser",func(t*testing.T) {
@@ -601,14 +617,15 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
601617
enqueuer:= notificationstest.FakeEnqueuer{}
602618
client:=coderdtest.New(t,&coderdtest.Options{IncludeProvisionerDaemon:true,NotificationsEnqueuer:&enqueuer})
603619
user:=coderdtest.CreateFirstUser(t,client)
620+
templateAdminClient,_:=coderdtest.CreateAnotherUser(t,client,user.OrganizationID,rbac.RoleTemplateAdmin(),rbac.RoleOwner())
604621
_,memberUser:=coderdtest.CreateAnotherUser(t,client,user.OrganizationID)
605622

606-
version:=coderdtest.CreateTemplateVersion(t,client,user.OrganizationID,nil)
607-
template:=coderdtest.CreateTemplate(t,client,user.OrganizationID,version.ID)
608-
coderdtest.AwaitTemplateVersionJobCompleted(t,client,version.ID)
623+
version:=coderdtest.CreateTemplateVersion(t,templateAdminClient,user.OrganizationID,nil)
624+
template:=coderdtest.CreateTemplate(t,templateAdminClient,user.OrganizationID,version.ID)
625+
coderdtest.AwaitTemplateVersionJobCompleted(t,templateAdminClient,version.ID)
609626

610627
ctx:=testutil.Context(t,testutil.WaitShort)
611-
workspace,err:=client.CreateUserWorkspace(ctx,memberUser.Username, codersdk.CreateWorkspaceRequest{
628+
workspace,err:=templateAdminClient.CreateUserWorkspace(ctx,memberUser.Username, codersdk.CreateWorkspaceRequest{
612629
TemplateID:template.ID,
613630
Name:coderdtest.RandomUsername(t),
614631
})
@@ -617,7 +634,7 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
617634

618635
sent:=enqueuer.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceCreated))
619636
require.Len(t,sent,1)
620-
require.Equal(t,memberUser.ID,sent[0].UserID)
637+
require.Equal(t,user.UserID,sent[0].UserID)
621638
require.Contains(t,sent[0].Targets,template.ID)
622639
require.Contains(t,sent[0].Targets,workspace.ID)
623640
require.Contains(t,sent[0].Targets,workspace.OrganizationID)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp