taskcluster icon indicating copy to clipboard operation
taskcluster copied to clipboard

[github] Assume a separate role for Github pre-releases

Open ahal opened this issue 2 years ago • 8 comments

With Github releases, you can specify that a release is a "pre-release". The intent here is that QA can then validate the release prior to it being ready to publish. Once validated, the release can be updated to a regular release.

For release automation, it might be desirable to run parts ofthe pipeline at the "pre-release" phase (e.g signing tasks), but other parts at the "release" phase (e.g actually deploying). Currently the Taskcluster Github service simply assumes a single <org>/<repo>:release role for all release events. So it's not possible to block deploy tasks via scopes on pre-releases. This means a project could inadvertently ship a "pre-release" if they don't have their tasks crafted carefully.

To resolve this, I propose the Github service assumes a separate role (e.g <org>/<repo>:pre-release if a "pre-release" is detected. This change would be backwards incompatible and possibly break existing pipelines without correlated ci-config changes. Though in practice I'd be surprised if it impacted anyone.

ahal avatar Jun 16 '22 15:06 ahal

As a workaround, we could possibly trigger an initial phase (build or promote) on pre-release (all github-release, and require additional actions to [promote/]push/ship.

We could ignore the github-release when githubscript moves it out of draft, by looking at the user; we do this in android-components. https://github.com/mozilla-mobile/android-components/blob/main/.taskcluster.yml#L111

escapewindow avatar Jun 16 '22 15:06 escapewindow

At the moment github service ignores action value of the release webhook.

So far we only processed published action, so we can extend it to assume different scope for prereleased action. Handling itself should stay the same, only scope will be changed, right?

lotas avatar Jun 16 '22 16:06 lotas

Correct, just a separate scope.

However reading your link clarified some things for me.. and I think we might not actually support VPN's use case after all. From the link:

published: a release, pre-release, or draft of a release is published prereleased: a pre-release is created released: a release is published, or a pre-release is changed to a release

So if we're only generating graphs for published, that implies we don't send any event when a pre-release is changed to a release. If I'm understanding things correctly, I think instead of handling the published event, we should be handling the prereleased and released events. Or we could continue handling published + released, though then we'd have to be careful not to generate duplicate graphs in .taskcluster.yml (since it looks like publishing a release sends both the published and released events).

@JohanLorenzo does my assessment here sound accurate to you?

ahal avatar Jun 16 '22 16:06 ahal

I'll try crafting some dummy releases on a staging repo to see what events get fired when and report back. We'll want separate scopes for sure, we just may also need to support upgrading a pre-release to a release.

ahal avatar Jun 16 '22 16:06 ahal

I set up a local server to listen to the webhook and created a pre-release and then "promoted" it. Indeed, it only seends the released action and not the published action. So we'll need to listen to the former if we're going to support VPN's intented process here.

ahal avatar Jun 16 '22 21:06 ahal

So far we only processed published action

TIL! I hadn't realized there are other types of event we could handle.

Indeed, it only seends the released action and not the published action. So we'll need to listen to the former if we're going to support VPN's intented process here.

Thanks for checking the actual behavior! Just to make sure I understand fully, would the process for the VPN be:

  • action == "published" && release.prerelease == False: taskgraph generates a promote graph for QA.
  • action == "release"|| (action == "published" && release.prerelease == True): taskgraph generates a ship graph (which includes tasks of the promote graph)

?

JohanLorenzo avatar Jun 17 '22 08:06 JohanLorenzo

I think you have the prerelease values mixed up, but if you swap them yes that could be one approach. Another approach could be:

action = preleased -> promote graph action = released -> ship graph (and possibly also promote if it doesn't exist)

Note that making new a release sends 3 events:

  • Published
  • Created
  • Released or prereleased

(Draft releases do not send that last one but I think we can ignore those)

It might be easier to switch the planned release model. Maybe this is a good reason to push to shipit faster. We have some time to figure this out as we only have signing tasks at the moment. Plus we can trigger the ship phase manually in the interim when we do have those tasks.

Though supporting prereleases would be a good general feature for Taskcluster to have.

ahal avatar Jun 17 '22 11:06 ahal

I think you have the prerelease values mixed up

Oops, you're right!

Maybe this is a good reason to push to shipit faster. [...] Though supporting prereleases would be a good general feature for Taskcluster to have.

I agree porting the VPN onto shipit makes sense and would solve this problem. Although, I wouldn't be surprised if a new team just wants to release their new product as fast as possible. Github Release is the most immediate way to go. So I concur: supporting preleases would be a good general feature 👍

JohanLorenzo avatar Jun 17 '22 12:06 JohanLorenzo

as discussed in PR and matrix we would change the role name to match action

assume:<repo>:release:published
assume:<repo>:release:prereleased
...

lotas avatar Jan 04 '23 15:01 lotas