Closed
Bug 1490832
Opened 6 years ago
Closed 6 years ago
Show "Trigger {missing jobs, all talos jobs}" to all users, not just is_staff
Categories
(Tree Management :: Treeherder, enhancement, P1)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
There was some confusion earlier on IRC because one of the new sheriffs was unable to use the "Trigger missing jobs" feature. It turned out they did not have the `is_staff` bit set in their Django user, causing Treeherder to hide the UI elements:
https://github.com/mozilla/treeherder/blob/829402cf45df3063b477a7472ef84edf7cf40d77/ui/job-view/pushes/PushActionMenu.jsx#L144-L155
However I don't think this is ideal, since:
* having the feature be hidden completely (rather than say greyed out) causes a confusing user experience
* the feature uses taskcluster actions to trigger the jobs, so (a) treeherder's is_staff check can be easily circumvented, (b) taskcluster scopes linked to LDAP groups seem like a much more appropriate place for this control (and would reduce the number of places permissions need updating for sheriff onboarding)
As such, should we just always show those links, and leave it to taskcluster to control who can use the feature?
Thoughts?
Assignee | ||
Updated•6 years ago
|
Component: Treeherder: Frontend → Treeherder: Job Triggering & Cancellation
Assignee | ||
Comment 1•6 years ago
|
||
And I'm presuming the reason access was limited was to prevent people using too many resources?
Updated•6 years ago
|
Comment 2•6 years ago
|
||
Always showing them seems a good solution to me.
Comment 3•6 years ago
|
||
Comment hidden (typo) |
Assignee | ||
Updated•6 years ago
|
Attachment #9009071 -
Flags: review?(helfi92)
Assignee | ||
Comment 5•6 years ago
|
||
Oops forgot I hadn't finished that message before pressing submit.
It was supposed to say something about the initial implementation of these features using pulse_actions via Treeherder's API, which (a) didn't have fine-grained permissions controls, (b) meant the requests weren't being made client side. Whereas now taskcluster actions does have fine-grained permissions via scopes, and the requests are made client side, so the Treeherder check is purely aesthetic, so less useful.
Comment 6•6 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/6101c7f3f6ab39bcb7dd16d9fe0b8ffd91830999
Bug 1490832 - Show trigger missing/talos jobs UI to all users (#4029)
...rather than just to users with the `is_staff` Django permission
(which mostly maps to sheriffs). This prevents the confusing UX of
UI elements being unknowingly hidden to users who are missing the
`is_staff` permission.
The check existed since these features used to use pulse_actions via
Treeherder's API (which had coarser permissions controls), whereas
now they use Taskcluster's API client side - so this check is purely
aesthetic, since people could just use Taskcluster's API/UIs directly.
If certain groups of users should be prevented from accidentally
scheduling too many jobs, access should instead be controlled via
Taskcluster scopes at the task configuration level.
Assignee | ||
Updated•6 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Attachment #9009071 -
Flags: review?(helfi92)
Updated•3 years ago
|
Component: Treeherder: Job Triggering & Cancellation → TreeHerder
You need to log in
before you can comment on or make changes to this bug.
Description
•