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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mafredri merged 6 commits intomainfrommafredri/feat-cli-provisioner-job-cancel
Jan 27, 2025

Conversation

mafredri
Copy link
Member

@mafredrimafredri commentedJan 24, 2025•
edited
Loading

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

@mafredrimafredri self-assigned thisJan 24, 2025
@mafredrimafredriforce-pushed themafredri/feat-cli-provisioner-job-cancel branch 2 times, most recently from20596bf tod50b500CompareJanuary 24, 2025 12:55
@mafredrimafredriforce-pushed themafredri/feat-cli-provisioner-job-cancel branch fromd50b500 toefdcbbaCompareJanuary 24, 2025 12:56
return
}

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

@mafredrimafredriJan 24, 2025•
edited
Loading

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 existingapi.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 reacted with thumbs up emoji
@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.

mafredri reacted with thumbs up emoji

Copy link
Member

@johnstcnjohnstcn 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.

@mafredrimafredrienabled auto-merge (squash)January 27, 2025 16:12
@mafredrimafredri merged commit75c899f intomainJan 27, 2025
33 of 34 checks passed
@mafredrimafredri deleted the mafredri/feat-cli-provisioner-job-cancel branchJanuary 27, 2025 16:26
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsJan 27, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@johnstcnjohnstcnjohnstcn approved these changes

@mtojekmtojekmtojek left review comments

Assignees

@mafredrimafredri

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add CLI command to cancel provisioner jobs

3 participants

@mafredri@johnstcn@mtojek
[8]ページ先頭
©2009-2025 Movatter.jp