Skip to content

feat(cli): add provisioner job cancel command #16252

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 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 6 commits into from
Jan 27, 2025

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Jan 24, 2025

This PR adds a command to cancel provisioner jobs.

I've re-used existing cancellation endpoints to ensure we keep current RBAC permissions. This revealed that template admins are only allowed to cancel TemplateVersionImport jobs, and not dry-run or workspace jobs.

This is due to:

rbac.ResourceWorkspace.InOrg(templateVersion.OrganizationID).WithOwner(job.ProvisionerJob.InitiatorID.String())) {

And en explanation for it exists here:

// We use the workspace RBAC check since we don't want to allow dry runs if
// the user can't create workspaces.

So I'm not sure what to make of this, is it a bug or not?

For now I've encoded the current behavior in the tests.

Fixes #16117
Updates #15084

@mafredri mafredri self-assigned this Jan 24, 2025
@mafredri mafredri force-pushed the mafredri/feat-cli-provisioner-job-cancel branch 2 times, most recently from 20596bf to d50b500 Compare January 24, 2025 12:55
@mafredri mafredri force-pushed the mafredri/feat-cli-provisioner-job-cancel branch from d50b500 to efdcbba Compare January 24, 2025 12:56
@mafredri mafredri requested review from johnstcn and mtojek January 24, 2025 12:57
return
}

jobs, ok := api.getProvisionerJobs(rw, r, []uuid.UUID{jobID})
Copy link
Member Author

@mafredri mafredri Jan 24, 2025

Choose a reason for hiding this comment

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

Review: Note that I'm sharing logic between this endpoint and the one for returning multiple. I mainly did this to ensure they use the same authorization code between the endpoints. That's because the existing api.Database.GetProvisionerJobByID is mainly used by provisioners, I did not want to use the same here until we can enforce proper RBAC for jobs (#16160)

@johnstcn
Copy link
Member

So I'm not sure what to make of this, is it a bug or not?

I think it's definitely worth an issue. I can definitely foresee a template admin trying to cancel some hanging workspace build jobs and being surprised that they are not allowed to do so.

Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

Nice work 👍 Some comments below, but nothing blocking.

@mafredri mafredri enabled auto-merge (squash) January 27, 2025 16:12
@mafredri mafredri merged commit 75c899f into main Jan 27, 2025
33 of 34 checks passed
@mafredri mafredri deleted the mafredri/feat-cli-provisioner-job-cancel branch January 27, 2025 16:26
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add CLI command to cancel provisioner jobs
3 participants