Skip to content

Commit aceedef

Browse files
EmyrkEmyrk
authored
chore: add template_with_user view to include user contextual data (#8568)
* chore: Refactor template sql queries to use new view * TemplateWithUser -> Template * Add unit test to enforce good view
1 parent cdbae29 commit aceedef

25 files changed

+453
-358
lines changed

‎coderd/database/dbauthz/dbauthz.go

+13-10
Original file line numberDiff line numberDiff line change
@@ -1808,9 +1808,12 @@ func (q *querier) InsertReplica(ctx context.Context, arg database.InsertReplicaP
18081808
return q.db.InsertReplica(ctx, arg)
18091809
}
18101810

1811-
func (q *querier) InsertTemplate(ctx context.Context, arg database.InsertTemplateParams) (database.Template, error) {
1811+
func (q *querier) InsertTemplate(ctx context.Context, arg database.InsertTemplateParams) error {
18121812
obj := rbac.ResourceTemplate.InOrg(arg.OrganizationID)
1813-
return insert(q.log, q.auth, obj, q.db.InsertTemplate)(ctx, arg)
1813+
if err := q.authorizeContext(ctx, rbac.ActionCreate, obj); err != nil {
1814+
return err
1815+
}
1816+
return q.db.InsertTemplate(ctx, arg)
18141817
}
18151818

18161819
func (q *querier) InsertTemplateVersion(ctx context.Context, arg database.InsertTemplateVersionParams) (database.TemplateVersion, error) {
@@ -2134,13 +2137,13 @@ func (q *querier) UpdateReplica(ctx context.Context, arg database.UpdateReplicaP
21342137
return q.db.UpdateReplica(ctx, arg)
21352138
}
21362139

2137-
func (q *querier) UpdateTemplateACLByID(ctx context.Context, arg database.UpdateTemplateACLByIDParams) (database.Template, error) {
2138-
// UpdateTemplateACL uses the ActionCreate action. Only users that can create the template
2139-
// may update the ACL.
2140+
func (q *querier) UpdateTemplateACLByID(ctx context.Context, arg database.UpdateTemplateACLByIDParams) error {
21402141
fetch := func(ctx context.Context, arg database.UpdateTemplateACLByIDParams) (database.Template, error) {
21412142
return q.db.GetTemplateByID(ctx, arg.ID)
21422143
}
2143-
return fetchAndQuery(q.log, q.auth, rbac.ActionCreate, fetch, q.db.UpdateTemplateACLByID)(ctx, arg)
2144+
// UpdateTemplateACL uses the ActionCreate action. Only users that can create the template
2145+
// may update the ACL.
2146+
return fetchAndExec(q.log, q.auth, rbac.ActionCreate, fetch, q.db.UpdateTemplateACLByID)(ctx, arg)
21442147
}
21452148

21462149
func (q *querier) UpdateTemplateActiveVersionByID(ctx context.Context, arg database.UpdateTemplateActiveVersionByIDParams) error {
@@ -2155,18 +2158,18 @@ func (q *querier) UpdateTemplateDeletedByID(ctx context.Context, arg database.Up
21552158
return q.SoftDeleteTemplateByID(ctx, arg.ID)
21562159
}
21572160

2158-
func (q *querier) UpdateTemplateMetaByID(ctx context.Context, arg database.UpdateTemplateMetaByIDParams) (database.Template, error) {
2161+
func (q *querier) UpdateTemplateMetaByID(ctx context.Context, arg database.UpdateTemplateMetaByIDParams) error {
21592162
fetch := func(ctx context.Context, arg database.UpdateTemplateMetaByIDParams) (database.Template, error) {
21602163
return q.db.GetTemplateByID(ctx, arg.ID)
21612164
}
2162-
return updateWithReturn(q.log, q.auth, fetch, q.db.UpdateTemplateMetaByID)(ctx, arg)
2165+
return update(q.log, q.auth, fetch, q.db.UpdateTemplateMetaByID)(ctx, arg)
21632166
}
21642167

2165-
func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.UpdateTemplateScheduleByIDParams) (database.Template, error) {
2168+
func (q *querier) UpdateTemplateScheduleByID(ctx context.Context, arg database.UpdateTemplateScheduleByIDParams) error {
21662169
fetch := func(ctx context.Context, arg database.UpdateTemplateScheduleByIDParams) (database.Template, error) {
21672170
return q.db.GetTemplateByID(ctx, arg.ID)
21682171
}
2169-
return updateWithReturn(q.log, q.auth, fetch, q.db.UpdateTemplateScheduleByID)(ctx, arg)
2172+
return update(q.log, q.auth, fetch, q.db.UpdateTemplateScheduleByID)(ctx, arg)
21702173
}
21712174

21722175
func (q *querier) UpdateTemplateVersionByID(ctx context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) {

‎coderd/database/dbauthz/dbauthz_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,7 @@ func (s *MethodTestSuite) TestTemplate() {
772772
t1 := dbgen.Template(s.T(), db, database.Template{})
773773
check.Args(database.UpdateTemplateACLByIDParams{
774774
ID: t1.ID,
775-
}).Asserts(t1, rbac.ActionCreate).Returns(t1)
775+
}).Asserts(t1, rbac.ActionCreate)
776776
}))
777777
s.Run("UpdateTemplateActiveVersionByID", s.Subtest(func(db database.Store, check *expects) {
778778
t1 := dbgen.Template(s.T(), db, database.Template{

‎coderd/database/dbfake/dbfake.go

+52-29
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func New() database.Store {
5858
workspaceResourceMetadata: make([]database.WorkspaceResourceMetadatum, 0),
5959
provisionerJobs: make([]database.ProvisionerJob, 0),
6060
templateVersions: make([]database.TemplateVersion, 0),
61-
templates: make([]database.Template, 0),
61+
templates: make([]database.TemplateTable, 0),
6262
workspaceAgentStats: make([]database.WorkspaceAgentStat, 0),
6363
workspaceAgentLogs: make([]database.WorkspaceAgentStartupLog, 0),
6464
workspaceBuilds: make([]database.WorkspaceBuild, 0),
@@ -130,7 +130,7 @@ type data struct {
130130
templateVersions []database.TemplateVersion
131131
templateVersionParameters []database.TemplateVersionParameter
132132
templateVersionVariables []database.TemplateVersionVariable
133-
templates []database.Template
133+
templates []database.TemplateTable
134134
workspaceAgents []database.WorkspaceAgent
135135
workspaceAgentMetadata []database.WorkspaceAgentMetadatum
136136
workspaceAgentLogs []database.WorkspaceAgentStartupLog
@@ -446,12 +446,37 @@ func (q *FakeQuerier) getLatestWorkspaceBuildByWorkspaceIDNoLock(_ context.Conte
446446
func (q *FakeQuerier) getTemplateByIDNoLock(_ context.Context, id uuid.UUID) (database.Template, error) {
447447
for _, template := range q.templates {
448448
if template.ID == id {
449-
return template.DeepCopy(), nil
449+
return q.templateWithUserNoLock(template), nil
450450
}
451451
}
452452
return database.Template{}, sql.ErrNoRows
453453
}
454454

455+
func (q *FakeQuerier) templatesWithUserNoLock(tpl []database.TemplateTable) []database.Template {
456+
cpy := make([]database.Template, 0, len(tpl))
457+
for _, t := range tpl {
458+
cpy = append(cpy, q.templateWithUserNoLock(t))
459+
}
460+
return cpy
461+
}
462+
463+
func (q *FakeQuerier) templateWithUserNoLock(tpl database.TemplateTable) database.Template {
464+
var user database.User
465+
for _, _user := range q.users {
466+
if _user.ID == tpl.CreatedBy {
467+
user = _user
468+
break
469+
}
470+
}
471+
var withUser database.Template
472+
// This is a cheeky way to copy the fields over without explicitly listing them all.
473+
d, _ := json.Marshal(tpl)
474+
_ = json.Unmarshal(d, &withUser)
475+
withUser.CreatedByUsername = user.Username
476+
withUser.CreatedByAvatarURL = user.AvatarURL.String
477+
return withUser
478+
}
479+
455480
func (q *FakeQuerier) getTemplateVersionByIDNoLock(_ context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) {
456481
for _, templateVersion := range q.templateVersions {
457482
if templateVersion.ID != templateVersionID {
@@ -1869,7 +1894,7 @@ func (q *FakeQuerier) GetTemplateByOrganizationAndName(_ context.Context, arg da
18691894
if template.Deleted != arg.Deleted {
18701895
continue
18711896
}
1872-
return template.DeepCopy(), nil
1897+
return q.templateWithUserNoLock(template), nil
18731898
}
18741899
return database.Template{}, sql.ErrNoRows
18751900
}
@@ -2092,17 +2117,14 @@ func (q *FakeQuerier) GetTemplates(_ context.Context) ([]database.Template, erro
20922117
defer q.mutex.RUnlock()
20932118

20942119
templates := slices.Clone(q.templates)
2095-
for i := range templates {
2096-
templates[i] = templates[i].DeepCopy()
2097-
}
2098-
slices.SortFunc(templates, func(i, j database.Template) bool {
2120+
slices.SortFunc(templates, func(i, j database.TemplateTable) bool {
20992121
if i.Name != j.Name {
21002122
return i.Name < j.Name
21012123
}
21022124
return i.ID.String() < j.ID.String()
21032125
})
21042126

2105-
return templates, nil
2127+
return q.templatesWithUserNoLock(templates), nil
21062128
}
21072129

21082130
func (q *FakeQuerier) GetTemplatesWithFilter(ctx context.Context, arg database.GetTemplatesWithFilterParams) ([]database.Template, error) {
@@ -3436,16 +3458,16 @@ func (q *FakeQuerier) InsertReplica(_ context.Context, arg database.InsertReplic
34363458
return replica, nil
34373459
}
34383460

3439-
func (q *FakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTemplateParams) (database.Template, error) {
3461+
func (q *FakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTemplateParams) error {
34403462
if err := validateDatabaseType(arg); err != nil {
3441-
return database.Template{}, err
3463+
return err
34423464
}
34433465

34443466
q.mutex.Lock()
34453467
defer q.mutex.Unlock()
34463468

34473469
//nolint:gosimple
3448-
template := database.Template{
3470+
template := database.TemplateTable{
34493471
ID: arg.ID,
34503472
CreatedAt: arg.CreatedAt,
34513473
UpdatedAt: arg.UpdatedAt,
@@ -3464,7 +3486,7 @@ func (q *FakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl
34643486
AllowUserAutostop: true,
34653487
}
34663488
q.templates = append(q.templates, template)
3467-
return template.DeepCopy(), nil
3489+
return nil
34683490
}
34693491

34703492
func (q *FakeQuerier) InsertTemplateVersion(_ context.Context, arg database.InsertTemplateVersionParams) (database.TemplateVersion, error) {
@@ -4172,9 +4194,9 @@ func (q *FakeQuerier) UpdateReplica(_ context.Context, arg database.UpdateReplic
41724194
return database.Replica{}, sql.ErrNoRows
41734195
}
41744196

4175-
func (q *FakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.UpdateTemplateACLByIDParams) (database.Template, error) {
4197+
func (q *FakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.UpdateTemplateACLByIDParams) error {
41764198
if err := validateDatabaseType(arg); err != nil {
4177-
return database.Template{}, err
4199+
return err
41784200
}
41794201

41804202
q.mutex.Lock()
@@ -4186,11 +4208,11 @@ func (q *FakeQuerier) UpdateTemplateACLByID(_ context.Context, arg database.Upda
41864208
template.UserACL = arg.UserACL
41874209

41884210
q.templates[i] = template
4189-
return template.DeepCopy(), nil
4211+
return nil
41904212
}
41914213
}
41924214

4193-
return database.Template{}, sql.ErrNoRows
4215+
return sql.ErrNoRows
41944216
}
41954217

41964218
func (q *FakeQuerier) UpdateTemplateActiveVersionByID(_ context.Context, arg database.UpdateTemplateActiveVersionByIDParams) error {
@@ -4233,9 +4255,9 @@ func (q *FakeQuerier) UpdateTemplateDeletedByID(_ context.Context, arg database.
42334255
return sql.ErrNoRows
42344256
}
42354257

4236-
func (q *FakeQuerier) UpdateTemplateMetaByID(_ context.Context, arg database.UpdateTemplateMetaByIDParams) (database.Template, error) {
4258+
func (q *FakeQuerier) UpdateTemplateMetaByID(_ context.Context, arg database.UpdateTemplateMetaByIDParams) error {
42374259
if err := validateDatabaseType(arg); err != nil {
4238-
return database.Template{}, err
4260+
return err
42394261
}
42404262

42414263
q.mutex.Lock()
@@ -4251,15 +4273,15 @@ func (q *FakeQuerier) UpdateTemplateMetaByID(_ context.Context, arg database.Upd
42514273
tpl.Description = arg.Description
42524274
tpl.Icon = arg.Icon
42534275
q.templates[idx] = tpl
4254-
return tpl.DeepCopy(), nil
4276+
return nil
42554277
}
42564278

4257-
return database.Template{}, sql.ErrNoRows
4279+
return sql.ErrNoRows
42584280
}
42594281

4260-
func (q *FakeQuerier) UpdateTemplateScheduleByID(_ context.Context, arg database.UpdateTemplateScheduleByIDParams) (database.Template, error) {
4282+
func (q *FakeQuerier) UpdateTemplateScheduleByID(_ context.Context, arg database.UpdateTemplateScheduleByIDParams) error {
42614283
if err := validateDatabaseType(arg); err != nil {
4262-
return database.Template{}, err
4284+
return err
42634285
}
42644286

42654287
q.mutex.Lock()
@@ -4278,10 +4300,10 @@ func (q *FakeQuerier) UpdateTemplateScheduleByID(_ context.Context, arg database
42784300
tpl.InactivityTTL = arg.InactivityTTL
42794301
tpl.LockedTTL = arg.LockedTTL
42804302
q.templates[idx] = tpl
4281-
return tpl.DeepCopy(), nil
4303+
return nil
42824304
}
42834305

4284-
return database.Template{}, sql.ErrNoRows
4306+
return sql.ErrNoRows
42854307
}
42864308

42874309
func (q *FakeQuerier) UpdateTemplateVersionByID(_ context.Context, arg database.UpdateTemplateVersionByIDParams) (database.TemplateVersion, error) {
@@ -4984,7 +5006,8 @@ func (q *FakeQuerier) GetAuthorizedTemplates(ctx context.Context, arg database.G
49845006
}
49855007

49865008
var templates []database.Template
4987-
for _, template := range q.templates {
5009+
for _, templateTable := range q.templates {
5010+
template := q.templateWithUserNoLock(templateTable)
49885011
if prepared != nil && prepared.Authorize(ctx, template.RBACObject()) != nil {
49895012
continue
49905013
}
@@ -5012,7 +5035,7 @@ func (q *FakeQuerier) GetAuthorizedTemplates(ctx context.Context, arg database.G
50125035
continue
50135036
}
50145037
}
5015-
templates = append(templates, template.DeepCopy())
5038+
templates = append(templates, template)
50165039
}
50175040
if len(templates) > 0 {
50185041
slices.SortFunc(templates, func(i, j database.Template) bool {
@@ -5031,7 +5054,7 @@ func (q *FakeQuerier) GetTemplateGroupRoles(_ context.Context, id uuid.UUID) ([]
50315054
q.mutex.RLock()
50325055
defer q.mutex.RUnlock()
50335056

5034-
var template database.Template
5057+
var template database.TemplateTable
50355058
for _, t := range q.templates {
50365059
if t.ID == id {
50375060
template = t
@@ -5068,7 +5091,7 @@ func (q *FakeQuerier) GetTemplateUserRoles(_ context.Context, id uuid.UUID) ([]d
50685091
q.mutex.RLock()
50695092
defer q.mutex.RUnlock()
50705093

5071-
var template database.Template
5094+
var template database.TemplateTable
50725095
for _, t := range q.templates {
50735096
if t.ID == id {
50745097
template = t

‎coderd/database/dbgen/dbgen.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ func AuditLog(t testing.TB, db database.Store, seed database.AuditLog) database.
6262
}
6363

6464
func Template(t testing.TB, db database.Store, seed database.Template) database.Template {
65-
template, err := db.InsertTemplate(genCtx, database.InsertTemplateParams{
66-
ID: takeFirst(seed.ID, uuid.New()),
65+
id := takeFirst(seed.ID, uuid.New())
66+
err := db.InsertTemplate(genCtx, database.InsertTemplateParams{
67+
ID: id,
6768
CreatedAt: takeFirst(seed.CreatedAt, database.Now()),
6869
UpdatedAt: takeFirst(seed.UpdatedAt, database.Now()),
6970
OrganizationID: takeFirst(seed.OrganizationID, uuid.New()),
@@ -79,6 +80,9 @@ func Template(t testing.TB, db database.Store, seed database.Template) database.
7980
AllowUserCancelWorkspaceJobs: seed.AllowUserCancelWorkspaceJobs,
8081
})
8182
require.NoError(t, err, "insert template")
83+
84+
template, err := db.GetTemplateByID(context.Background(), id)
85+
require.NoError(t, err, "get template")
8286
return template
8387
}
8488

‎coderd/database/dbmetrics/dbmetrics.go

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

0 commit comments

Comments
 (0)