Convert release-promotion to an action hook

RESOLVED FIXED in Firefox -esr60

Status

task
RESOLVED FIXED
Last year
5 months ago

People

(Reporter: dustin, Assigned: aki)

Tracking

(Blocks 1 bug)

Trunk
mozilla68
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 fixed, firefox66+ fixed, firefox67 fixed, firefox68 fixed)

Details

Attachments

(5 attachments, 1 obsolete attachment)

The actual change to accomplish this is tiny, similar to
  https://reviewboard.mozilla.org/r/253236/diff/4

However, the implications are larger, and probably security sensitive, and also relate to scripts that start off releases.  So, I'd like to work with someone from the release team to get this right..  Rail, is that you?
Flags: needinfo?
Blocks: 1415868
Flags: needinfo?(rail)
Flags: needinfo?
Does converting to a hook disable non-hook action triggers? If so, we'd have to change and test our various processes more thoroughly before cutting over. If not, we could just do a quick staging release test on maple before landing.

Security-wise, I *think* this should be an overall improvement: we would be able to more easily restrict the release scopes to the hook, rather than to all level 3 roles. Do you have any specific concerns here? Am I missing anything?
Flags: needinfo?(dustin)
Flags: needinfo?(rail)
I think your first question is the concern -- actions.json would contain {kind: "hook", ..}, and not {kind: "task", ..}, so if there's automation looking for the latter, it will fail.

For treeherder and TC tools, we have implemented support for both kinds, meaning that the kind change can ride the trains (or be uplifted).  Maybe that's a good choice here, too?

And, yes, I think there's a good chance for this to be a big security improvement.  We can issue various scopes to *only* this action -- not crontasks or pushes to the repo.  And we can limit who has access to call triggerHook. And, we can write a schema for the hook payload to limit the allowed inputs.  I don't really have the bandwidth to figure out what those should be, but given that information I can definitely get them configured.
Flags: needinfo?(dustin)
(In reply to Dustin J. Mitchell [:dustin] pronoun: he from comment #2)
> I think your first question is the concern -- actions.json would contain
> {kind: "hook", ..}, and not {kind: "task", ..}, so if there's automation
> looking for the latter, it will fail.
> 
> For treeherder and TC tools, we have implemented support for both kinds,
> meaning that the kind change can ride the trains (or be uplifted).  Maybe
> that's a good choice here, too?

Makes sense. Looks like we should try this on maple, and either verify it works or add support for hooks where needed before rolling out.

> And, yes, I think there's a good chance for this to be a big security
> improvement.  We can issue various scopes to *only* this action -- not
> crontasks or pushes to the repo.  And we can limit who has access to call
> triggerHook. And, we can write a schema for the hook payload to limit the
> allowed inputs.  I don't really have the bandwidth to figure out what those
> should be, but given that information I can definitely get them configured.

+1, thanks for the offer! We'll need to compile that list.
Current status:

- enabled relpro-hook on the maple `relpro-hook` branch. [1]  This is visible on treeherder here [2].

- I was able to trigger a promotion graph off that revision. action task: [3] graph: [4].  The input.yml is here [5].

    a) this Just Works from treeherder (no scopes issues), so hopefully it's straightforward to trigger from ship-it.

    b) it looks like switching to the hook breaks CoT verification on the action task. This is because the treeherder symbol is currently set to "${input.release_promotion_flavor}"; we'll need to have that expand to its actual value during cot verification.

- Once I applied this patch [6] locally, `verify_cot --task-type action -- PV3gwVnWSgaGzUQuqVr41w` works.  This means we have a working solution.  We can find a better solution or go with this one.

Next:

- Rail was going to look at implementing action hook handling in ship-it.  The implementation details here will guide how we roll out the changes to the release branches.  (E.g., if we can support both hooks and non-hooks simultaneously, we have the option of riding the trains; if it requires a hard cutover, we'll need to uplift everywhere.)
- We should roll out a CoT fix in scriptworker.
- Once we're ready, we can uplift the in-tree patch to the various release branches.
- At that point, we can look at restricting scopes to the hook.  We will likely need at least 3 sets of scopes: one for nightly cron, one for staging releases on level 1 trees, and one for real releases.  If we're ambitious, we can split that last one into esr vs beta vs release (vs nightly promotion, once shippable builds is a thing).


[1] https://hg.mozilla.org/projects/maple/rev/358b78517a800f02e025a9020ffbf0f8516c204c
[2] https://treeherder.mozilla.org/#/jobs?repo=maple&selectedJob=198189192
[3] https://tools.taskcluster.net/groups/E7w_SFN8TouLJXQ-xNVbhw/tasks/PV3gwVnWSgaGzUQuqVr41w/details
[4] https://tools.taskcluster.net/groups/PV3gwVnWSgaGzUQuqVr41w
[5] https://bugzilla.mozilla.org/attachment.cgi?id=9007401
[6] https://github.com/mozilla-releng/scriptworker/pull/256
Blocks: 1470625
make the relpro action a hook.
Posted file GitHub Pull Request (obsolete) —
Assignee: nobody → aki
Attachment #9008185 - Flags: review?(mozilla)
Comment on attachment 9008177 [details]
bug 1485680 - make the relpro action a hook.

Dustin J. Mitchell [:dustin] pronoun: he has approved the revision.
Attachment #9008177 - Flags: review+
Attachment #9008185 - Flags: review?(mozilla)
Depends on: 1491371
Blocks: 1492274
I think this has become higher priority since bug 1491186 landed -- release promotion is currently busted on Maple with:
[task 2018-09-25T00:18:01.880Z] + ./mach --log-no-times taskgraph action-callback
[task 2018-09-25T00:18:02.404Z] loading config from `taskcluster/ci/config.yml`
[task 2018-09-25T00:18:03.236Z] Traceback (most recent call last):
[task 2018-09-25T00:18:03.236Z]   File "/builds/worker/checkouts/gecko/taskcluster/mach_commands.py", line 252, in action_callback
[task 2018-09-25T00:18:03.236Z]     test=False)
[task 2018-09-25T00:18:03.236Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/registry.py", line 353, in trigger_action_callback
[task 2018-09-25T00:18:03.236Z]     sanity_check_task_scope(callback, parameters, graph_config)
[task 2018-09-25T00:18:03.236Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/actions/registry.py", line 332, in sanity_check_task_scope
[task 2018-09-25T00:18:03.236Z]     raise Exception('Expected task scope {} for this action'.format(expected_scope))
[task 2018-09-25T00:18:03.236Z] Exception: Expected task scope assume:repo:hg.mozilla.org/projects/maple:action:generic for this action

(From https://tools.taskcluster.net/groups/PC32PCpzQy2U_aK7vpxKpA/tasks/bE30WPeoT_i5me8Pp-5ZIw/details)
I have two patches that fix the CoT issue. One's close; we could defer the ownTaskId issue if we wanted it sooner. However, we don't currently have anyone working on the ship-it v2 side, so that may be the long pole.
Attachment #9008185 - Attachment is obsolete: true
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/24bb83f24599
add actionPerm to actions.json for hooks. r=dustin
Status:

X actionPerm:
  X review & land https://bugzilla.mozilla.org/attachment.cgi?id=9032313
_ relpro hook cot:
  _ review & land https://github.com/mozilla-releng/scriptworker/pull/286
  _ scriptworker release
  _ roll out via puppet
_ shipit v2 relpro hook support
_ make relpro a hook
  _ land https://bugzilla.mozilla.org/attachment.cgi?id=9008177
https://hg.mozilla.org/mozilla-central/rev/24bb83f24599
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
Status:

X actionPerm:
  X review & land https://bugzilla.mozilla.org/attachment.cgi?id=9032313
X relpro hook cot:
  X review & land https://github.com/mozilla-releng/scriptworker/pull/286
  X scriptworker release
  X roll out via puppet
_ add a comment in ci-admin to update scriptworker if action hook logic changes
_ shipit v2 relpro hook support
_ make relpro a hook
  _ land https://bugzilla.mozilla.org/attachment.cgi?id=9008177
_ fix up scopes
Blocks: 1525906
Blocks: 1529948
Keywords: leave-open
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f95100eef22
make the relpro action a hook. r=dustin
Status: REOPENED → RESOLVED
Closed: 8 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

I believe we need to uplift the final patch to esr60, beta, and release to make relpro a hook everywhere.
We may want bug 1491371 to land first, so we don't break TB.

(In reply to Aki Sasaki [:aki] from comment #22)

I believe we need to uplift the final patch to esr60, beta, and release to make relpro a hook everywhere.
We may want bug 1491371 to land first, so we don't break TB.

I concur with all of this.

\o/ \o/ \o/ \o/ \o/ \o/ \o/

You need to log in before you can comment on or make changes to this bug.