Skip to content

Commit 8d8402d

Browse files
johnstcnjohnstcn
authored
fix(coderd/database): avoid clobbering workspace build state (coder#9826)
Fixes coder#9823. - Decomposes UpdateWorkspaceBuildByID into UpdateWorkspaceBuildProvisionerStateByID and UpdateWorkspaceBuildDeadlineByID. - Replaces existing invocations of UpdateWorkspaceBuildByID with the newer queries where applicable. - Modifies GetActiveWorkspaceBuildsByTemplateID to not return incomplete workspace builds.
1 parent a1f3a6b commit 8d8402d

File tree

14 files changed

+237
-114
lines changed

14 files changed

+237
-114
lines changed

‎coderd/activitybump_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,11 @@ func TestWorkspaceActivityBump(t *testing.T) {
7777
dbBuild, err := db.GetWorkspaceBuildByID(ctx, workspace.LatestBuild.ID)
7878
require.NoError(t, err)
7979

80-
err = db.UpdateWorkspaceBuildByID(ctx, database.UpdateWorkspaceBuildByIDParams{
81-
ID: workspace.LatestBuild.ID,
82-
UpdatedAt: dbtime.Now(),
83-
ProvisionerState: dbBuild.ProvisionerState,
84-
Deadline: dbBuild.Deadline,
85-
MaxDeadline: dbtime.Now().Add(maxTTL),
80+
err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
81+
ID: workspace.LatestBuild.ID,
82+
UpdatedAt: dbtime.Now(),
83+
Deadline: dbBuild.Deadline,
84+
MaxDeadline: dbtime.Now().Add(maxTTL),
8685
})
8786
require.NoError(t, err)
8887
}

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2675,7 +2675,15 @@ func (q *querier) UpdateWorkspaceAutostart(ctx context.Context, arg database.Upd
26752675
return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceAutostart)(ctx, arg)
26762676
}
26772677

2678-
func (q *querier) UpdateWorkspaceBuildByID(ctx context.Context, arg database.UpdateWorkspaceBuildByIDParams) error {
2678+
// UpdateWorkspaceBuildCostByID is used by the provisioning system to update the cost of a workspace build.
2679+
func (q *querier) UpdateWorkspaceBuildCostByID(ctx context.Context, arg database.UpdateWorkspaceBuildCostByIDParams) error {
2680+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil {
2681+
return err
2682+
}
2683+
return q.db.UpdateWorkspaceBuildCostByID(ctx, arg)
2684+
}
2685+
2686+
func (q *querier) UpdateWorkspaceBuildDeadlineByID(ctx context.Context, arg database.UpdateWorkspaceBuildDeadlineByIDParams) error {
26792687
build, err := q.db.GetWorkspaceBuildByID(ctx, arg.ID)
26802688
if err != nil {
26812689
return err
@@ -2685,20 +2693,19 @@ func (q *querier) UpdateWorkspaceBuildByID(ctx context.Context, arg database.Upd
26852693
if err != nil {
26862694
return err
26872695
}
2696+
26882697
err = q.authorizeContext(ctx, rbac.ActionUpdate, workspace.RBACObject())
26892698
if err != nil {
26902699
return err
26912700
}
2692-
2693-
return q.db.UpdateWorkspaceBuildByID(ctx, arg)
2701+
return q.db.UpdateWorkspaceBuildDeadlineByID(ctx, arg)
26942702
}
26952703

2696-
// UpdateWorkspaceBuildCostByID is used by the provisioning system to update the cost of a workspace build.
2697-
func (q *querier) UpdateWorkspaceBuildCostByID(ctx context.Context, arg database.UpdateWorkspaceBuildCostByIDParams) error {
2704+
func (q *querier) UpdateWorkspaceBuildProvisionerStateByID(ctx context.Context, arg database.UpdateWorkspaceBuildProvisionerStateByIDParams) error {
26982705
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil {
26992706
return err
27002707
}
2701-
return q.db.UpdateWorkspaceBuildCostByID(ctx, arg)
2708+
return q.db.UpdateWorkspaceBuildProvisionerStateByID(ctx, arg)
27022709
}
27032710

27042711
// Deprecated: Use SoftDeleteWorkspaceByID

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,14 +1232,13 @@ func (s *MethodTestSuite) TestWorkspace() {
12321232
ID: ws.ID,
12331233
}).Asserts(ws, rbac.ActionUpdate).Returns()
12341234
}))
1235-
s.Run("UpdateWorkspaceBuildByID", s.Subtest(func(db database.Store, check *expects) {
1235+
s.Run("UpdateWorkspaceBuildDeadlineByID", s.Subtest(func(db database.Store, check *expects) {
12361236
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
12371237
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1238-
check.Args(database.UpdateWorkspaceBuildByIDParams{
1239-
ID: build.ID,
1240-
UpdatedAt: build.UpdatedAt,
1241-
Deadline: build.Deadline,
1242-
ProvisionerState: []byte{},
1238+
check.Args(database.UpdateWorkspaceBuildDeadlineByIDParams{
1239+
ID: build.ID,
1240+
UpdatedAt: build.UpdatedAt,
1241+
Deadline: build.Deadline,
12431242
}).Asserts(ws, rbac.ActionUpdate)
12441243
}))
12451244
s.Run("SoftDeleteWorkspaceByID", s.Subtest(func(db database.Store, check *expects) {
@@ -1378,6 +1377,14 @@ func (s *MethodTestSuite) TestSystemFunctions() {
13781377
DailyCost: 10,
13791378
}).Asserts(rbac.ResourceSystem, rbac.ActionUpdate)
13801379
}))
1380+
s.Run("UpdateWorkspaceBuildProvisionerStateByID", s.Subtest(func(db database.Store, check *expects) {
1381+
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1382+
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1383+
check.Args(database.UpdateWorkspaceBuildProvisionerStateByIDParams{
1384+
ID: build.ID,
1385+
ProvisionerState: []byte("testing"),
1386+
}).Asserts(rbac.ResourceSystem, rbac.ActionUpdate)
1387+
}))
13811388
s.Run("UpsertLastUpdateCheck", s.Subtest(func(db database.Store, check *expects) {
13821389
check.Args("value").Asserts(rbac.ResourceSystem, rbac.ActionUpdate)
13831390
}))

‎coderd/database/dbfake/dbfake.go

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5854,7 +5854,7 @@ func (q *FakeQuerier) UpdateWorkspaceAutostart(_ context.Context, arg database.U
58545854
return sql.ErrNoRows
58555855
}
58565856

5857-
func (q *FakeQuerier) UpdateWorkspaceBuildByID(_ context.Context, arg database.UpdateWorkspaceBuildByIDParams) error {
5857+
func (q *FakeQuerier) UpdateWorkspaceBuildCostByID(_ context.Context, arg database.UpdateWorkspaceBuildCostByIDParams) error {
58585858
if err := validateDatabaseType(arg); err != nil {
58595859
return err
58605860
}
@@ -5866,32 +5866,55 @@ func (q *FakeQuerier) UpdateWorkspaceBuildByID(_ context.Context, arg database.U
58665866
if workspaceBuild.ID != arg.ID {
58675867
continue
58685868
}
5869-
workspaceBuild.UpdatedAt = arg.UpdatedAt
5870-
workspaceBuild.ProvisionerState = arg.ProvisionerState
5871-
workspaceBuild.Deadline = arg.Deadline
5872-
workspaceBuild.MaxDeadline = arg.MaxDeadline
5869+
workspaceBuild.DailyCost = arg.DailyCost
58735870
q.workspaceBuilds[index] = workspaceBuild
58745871
return nil
58755872
}
58765873
return sql.ErrNoRows
58775874
}
58785875

5879-
func (q *FakeQuerier) UpdateWorkspaceBuildCostByID(_ context.Context, arg database.UpdateWorkspaceBuildCostByIDParams) error {
5880-
if err := validateDatabaseType(arg); err != nil {
5876+
func (q *FakeQuerier) UpdateWorkspaceBuildDeadlineByID(_ context.Context, arg database.UpdateWorkspaceBuildDeadlineByIDParams) error {
5877+
err := validateDatabaseType(arg)
5878+
if err != nil {
58815879
return err
58825880
}
58835881

58845882
q.mutex.Lock()
58855883
defer q.mutex.Unlock()
58865884

5887-
for index, workspaceBuild := range q.workspaceBuilds {
5888-
if workspaceBuild.ID != arg.ID {
5885+
for idx, build := range q.workspaceBuilds {
5886+
if build.ID != arg.ID {
58895887
continue
58905888
}
5891-
workspaceBuild.DailyCost = arg.DailyCost
5892-
q.workspaceBuilds[index] = workspaceBuild
5889+
build.Deadline = arg.Deadline
5890+
build.MaxDeadline = arg.MaxDeadline
5891+
build.UpdatedAt = arg.UpdatedAt
5892+
q.workspaceBuilds[idx] = build
58935893
return nil
58945894
}
5895+
5896+
return sql.ErrNoRows
5897+
}
5898+
5899+
func (q *FakeQuerier) UpdateWorkspaceBuildProvisionerStateByID(_ context.Context, arg database.UpdateWorkspaceBuildProvisionerStateByIDParams) error {
5900+
err := validateDatabaseType(arg)
5901+
if err != nil {
5902+
return err
5903+
}
5904+
5905+
q.mutex.Lock()
5906+
defer q.mutex.Unlock()
5907+
5908+
for idx, build := range q.workspaceBuilds {
5909+
if build.ID != arg.ID {
5910+
continue
5911+
}
5912+
build.ProvisionerState = arg.ProvisionerState
5913+
build.UpdatedAt = arg.UpdatedAt
5914+
q.workspaceBuilds[idx] = build
5915+
return nil
5916+
}
5917+
58955918
return sql.ErrNoRows
58965919
}
58975920

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 14 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 26 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 46 additions & 25 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)