1343002 - Treeherder should stop using pulse_actions for anything to do with taskcluster

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Brian Stack [:bstack]]

Now that tc credentials work from the th ui directly (barring any new bugs we discover), we have no reason for pulse_actions to be handling any tc specific tasks. It is still needed for bb jobs, but those aren't particularly a problem. The issue with pulse_actions is that it is a deputy for users and we end up with weird/wrong permissions checks in there to use pulse_action's very powerful tc creds. This creates two issues:

1. (very bad) if the hacky permission check is too loose, this is a major privilege escalation
2. (normal bad) if the checks are too strict, people are prevented from doing things they should be allowed to do.

As it stands, issue 2 is happening. There is a check in pulse_actions that only allows jobs to be added to pushes in try pushes due to security concerns. The fix for this is to move add-new-job functionality into th ui directly.

Afaict, the only two remaining things to port are add-new-jobs and retrigger.

Comment 1

[Brian Stack [:bstack]]

Specifically, the porting from pulse_actions to treeherder ui isn't a terribly hard thing to do, particularly if you have proper scopes that allow you to try the action out. An example of doing this is where I've done so with add-talos-jobs https://github.com/mozilla/treeherder/pull/2092/files#diff-1dcf92fc8473c8a5e650f4b7a8220c65L196

The diff shows a move from logic that only fired off a pulse message for pulse_actions to process, to one that creates a tc client and creates an action task in tc directly, and also fires off a pulse action so that it can do its thing with bb jobs as well.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

I am not sure if there are treeherder fixes needed here, but I added camd and rwood to this bug so they could chime in or help get to the right solution.

Comment 3

[GitHub Autolander Bot]

Attachment: [treeherder] imbstack:bug-1343002 > mozilla:master

Comment 4

[Brian Stack [:bstack]]

Comment on attachment 8841813 [details] [review]
[treeherder] imbstack:bug-1343002 > mozilla:master

Note: I just used the old action.yml instead of the new-cool action.json because it was already written for add-tasks and I just wanted to get this out of the way asap.

Comment 5

[William Lachance (:wlach)]

Comment on attachment 8841813 [details] [review]
[treeherder] imbstack:bug-1343002 > mozilla:master

This pretty much looks fine to me. A few small nits. Let me know when you've addressed those and I'll land it!

Comment 6

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/2e80b9d6d86896f100c3d9417482e04f7ecf6e6a
Bug 1343002 - Make add-jobs go direct to tc for tc jobs (#2208)

Comment 7

[Ed Morley [:emorley]]

Can this be marked as fixed, or is there more to do? :-)

Comment 8

[Brian Stack [:bstack]]

I believe this is fixed now. A couple of things are still using mozilla-tc instead of directly posting, but we can fix that in another bug.