1490832 - Show "Trigger {missing jobs, all talos jobs}" to all users, not just is_staff

Status: RESOLVED FIXED

(Tree Management :: Treeherder, enhancement, P1)

Description

[Ed Morley [:emorley]]

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?

Comment 1

[Ed Morley [:emorley]]

And I'm presuming the reason access was limited was to prevent people using too many resources?

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Always showing them seems a good solution to me.

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4029

Comment 4

[Ed Morley [:emorley]]

Ah ok so when these feature were added, they triggers tasks via Treeherder's API and then pulse actions, which

Comment 5

[Ed Morley [:emorley]]

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

[Treeherder GitHub Bugbot]

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.