-
Notifications
You must be signed in to change notification settings - Fork 103
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
Add Run Tasks permissions for Org. and Workspace #383
Conversation
ENABLE_BETA=1 go test -run TestTeamAccess -v ./... -tags=integration
|
ENABLE_BETA=1 go test -run TestOrganizations -v ./... -tags=integration
|
ENABLE_TEST=1 go test -run TestTeams -v ./... -tags=integration
|
ENABLE_BETA=1 go test -run TestWorkspacesRunTasksPermission -v ./... -tags=integration
|
I've extracted just the permissions pieces from #381 |
fb2d82c
to
9df7695
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does what it says on the tin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks 🔥
9df7695
to
c552d8b
Compare
Missed the |
c552d8b
to
7cf1ee7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things look good. One minor point for discussion below 👍
@@ -98,6 +98,7 @@ type TeamAccess struct { | |||
StateVersions StateVersionsPermissionType `jsonapi:"attr,state-versions"` | |||
SentinelMocks SentinelMocksPermissionType `jsonapi:"attr,sentinel-mocks"` | |||
WorkspaceLocking bool `jsonapi:"attr,workspace-locking"` | |||
RunTasks bool `jsonapi:"attr,run-tasks"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this a little more, should we document that this field is in beta and not widely available to everyone? Given I see this causing some manner of confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason why all these permissions have to be required?
Run Tasks is due out of beta in a week or so, so no. I don't think it's required.
This commit adds the beta API changes for the Run Tasks permissions as per RFC TF-474: * Permission to modify Organization level Run Tasks * Permission to modify Workspace level Run Tasks This commit also adds tests for these beta features.
7cf1ee7
to
94b9c8d
Compare
Had to rebase for the flaky tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-approving. Everything looks good 👍
Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes. |
Description
This commit adds the beta API changes for the Run Tasks permissions
as per RFC TF-474:
This commit also adds tests for these beta features.
Testing plan
External links