Skip to content

feat: cancel stuck pending jobs #17803

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.

Sign up for GitHub

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 24 commits into from
May 20, 2025
Merged

feat: cancel stuck pending jobs #17803

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0f51f35
added queries for fetching NotStartedProvisionerJobs
ibetitsmike Apr 1, 2025
2f3d606
added detector handling of not started jobs
ibetitsmike May 9, 2025
4b252eb
filling out started_at when unhanging not started jobs
ibetitsmike May 9, 2025
ca49519
WIP
ibetitsmike May 13, 2025
af994c2
refactored to reaper & added tests
ibetitsmike May 13, 2025
3815727
Revert "filling out started_at when unhanging not started jobs"
ibetitsmike May 13, 2025
b65f620
created new ORM update to avoid forcing setting StartedAt on every Co…
ibetitsmike May 13, 2025
3c7c323
added missing dbauthz tests
ibetitsmike May 13, 2025
35df01f
added checks for StartedAt value in the updated jobs
ibetitsmike May 13, 2025
8aa1ee2
refactor from reaper to jobreaper
ibetitsmike May 14, 2025
4385933
WIP
ibetitsmike May 14, 2025
96fee51
WIP
ibetitsmike May 14, 2025
d8db119
WIP
ibetitsmike May 15, 2025
5120fb1
WIP
ibetitsmike May 15, 2025
8d4fa5a
fixed sql comments
ibetitsmike May 15, 2025
18b809c
taking a step back with RBAC
ibetitsmike May 16, 2025
0fe1404
WIP
ibetitsmike May 16, 2025
77be34e
WIP
ibetitsmike May 16, 2025
4351529
WIP
ibetitsmike May 16, 2025
c03bfa3
fixed InOrg check for provisionerjob resource
ibetitsmike May 19, 2025
a15bd1c
PR review; naming in the comments, added comments for SQL, less verbo…
ibetitsmike May 19, 2025
5b9348f
fixes to tests after lint remove rand
ibetitsmike May 19, 2025
91d2d32
readded rand to fix gen failing in CI
ibetitsmike May 19, 2025
767cb77
adjusted TODOs
ibetitsmike May 20, 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
Prev Previous commit
Next Next commit
WIP
  • Loading branch information
@ibetitsmike
ibetitsmike committed May 14, 2025
commit 96fee5183e7848f01970c30709d56d13bf2a0e11
61 changes: 28 additions & 33 deletions coderd/database/dbauthz/dbauthz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,10 @@ var (
Identifier: rbac.RoleIdentifier{Name: "provisionerd"},
DisplayName: "Provisioner Daemon",
Site: rbac.Permissions(map[string][]policy.Action{
// TODO: Add ProvisionerJob resource type.
rbac.ResourceFile.Type: {policy.ActionRead},
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
rbac.ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate},
rbac.ResourceFile.Type: {policy.ActionRead},
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate},
// Unsure why provisionerd needs update and read personal
rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop},
Expand Down Expand Up @@ -1093,11 +1093,10 @@ func (q *querier) AcquireNotificationMessages(ctx context.Context, arg database.
return q.db.AcquireNotificationMessages(ctx, arg)
}

// TODO: We need to create a ProvisionerJob resource type
func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.AcquireProvisionerJobParams) (database.ProvisionerJob, error) {
// if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
// return database.ProvisionerJob{}, err
// }
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil {
return database.ProvisionerJob{}, err
}
return q.db.AcquireProvisionerJob(ctx, arg)
}

Expand Down Expand Up @@ -2322,28 +2321,28 @@ func (q *querier) GetProvisionerJobTimingsByJobID(ctx context.Context, jobID uui
return q.db.GetProvisionerJobTimingsByJobID(ctx, jobID)
}

// TODO: We have a ProvisionerJobs resource, but it hasn't been checked for this use-case.
func (q *querier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.ProvisionerJob, error) {
// if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
// return nil, err
// }
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil {
return nil, err
}
return q.db.GetProvisionerJobsByIDs(ctx, ids)
}

// TODO: We have a ProvisionerJobs resource, but it hasn't been checked for this use-case.
func (q *querier) GetProvisionerJobsByIDsWithQueuePosition(ctx context.Context, ids []uuid.UUID) ([]database.GetProvisionerJobsByIDsWithQueuePositionRow, error) {
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil {
return nil, err
}
return q.db.GetProvisionerJobsByIDsWithQueuePosition(ctx, ids)
Copy link
Member

Choose a reason for hiding this comment

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

These should really be scoped. If users are inside orgs, even if they are org-admins they should not be able to read across org boundaries.

The organization boundaries have to be kept.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now it's impossible to check this without extending the database that would link ProvisionerJobs to owners.

I'll do that in a separate PR.

}

func (q *querier) GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx context.Context, arg database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams) ([]database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerRow, error) {
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner)(ctx, arg)
}

// TODO: We have a ProvisionerJobs resource, but it hasn't been checked for this use-case.
func (q *querier) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]database.ProvisionerJob, error) {
// if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
// return nil, err
// }
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil {
return nil, err
}
return q.db.GetProvisionerJobsCreatedAfter(ctx, createdAt)
}

Expand Down Expand Up @@ -3531,27 +3530,24 @@ func (q *querier) InsertPresetParameters(ctx context.Context, arg database.Inser
return q.db.InsertPresetParameters(ctx, arg)
}

// TODO: We need to create a ProvisionerJob resource type
func (q *querier) InsertProvisionerJob(ctx context.Context, arg database.InsertProvisionerJobParams) (database.ProvisionerJob, error) {
// if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
// return database.ProvisionerJob{}, err
// }
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceProvisionerJobs); err != nil {
return database.ProvisionerJob{}, err
}
return q.db.InsertProvisionerJob(ctx, arg)
}

// TODO: We need to create a ProvisionerJob resource type
func (q *querier) InsertProvisionerJobLogs(ctx context.Context, arg database.InsertProvisionerJobLogsParams) ([]database.ProvisionerJobLog, error) {
// if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
// return nil, err
// }
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceProvisionerJobs); err != nil {
return nil, err
}
return q.db.InsertProvisionerJobLogs(ctx, arg)
}

// TODO: We need to create a ProvisionerJob resource type
func (q *querier) InsertProvisionerJobTimings(ctx context.Context, arg database.InsertProvisionerJobTimingsParams) ([]database.ProvisionerJobTiming, error) {
// if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
// return nil, err
// }
if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceProvisionerJobs); err != nil {
return nil, err
}
return q.db.InsertProvisionerJobTimings(ctx, arg)
}

Expand Down Expand Up @@ -4174,11 +4170,10 @@ func (q *querier) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg dat
return q.db.UpdateProvisionerDaemonLastSeenAt(ctx, arg)
}

// TODO: We need to create a ProvisionerJob resource type
func (q *querier) UpdateProvisionerJobByID(ctx context.Context, arg database.UpdateProvisionerJobByIDParams) error {
// if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
// return err
// }
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil {
return err
}
return q.db.UpdateProvisionerJobByID(ctx, arg)
}

Expand Down
2 changes: 1 addition & 1 deletion coderd/rbac/roles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
// the ability to create templates and provisioners has
// a lot of overlap.
ResourceProvisionerDaemon.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
ResourceProvisionerJobs.Type: {policy.ActionRead},
ResourceProvisionerJobs.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionCreate},
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't regular members need this permission too? To create the jobs for their workspaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They would, but I'm not sure I want to polute the RBAC with assigning members access to all provisioner jobs.

We currently don't check this at all. I'll introduce another PR that separetly focuses on solving this issue.

}),
},
User: []Permission{},
Expand Down
2 changes: 1 addition & 1 deletion coderd/rbac/roles_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ func TestRolePermissions(t *testing.T) {
},
{
Name: "ProvisionerJobs",
Actions: []policy.Action{policy.ActionRead},
Actions: []policy.Action{policy.ActionRead, policy.ActionUpdate},
Resource: rbac.ResourceProvisionerJobs.InOrg(orgID),
AuthorizeMap: map[bool][]hasAuthSubjects{
true: {owner, orgTemplateAdmin, orgAdmin},
Expand Down
2 changes: 1 addition & 1 deletion codersdk/rbacresources_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading