Closed Bug 1317783 Opened 3 years ago Closed 3 years ago

enable chain of trust verification in pushapk

Categories

(Release Engineering :: Release Automation: Other, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: jlorenzo)

References

Details

Attachments

(8 files)

We'll have to make these changes:

a. scopes

For signing, we have 3 levels of permissions, guarded by scopes.
project:releng:signing:cert:release-signing [1], which we only allow on release-capable branches, project:releng:signing:cert:nightly-signing [2], which we only allow on nightly-capable branches, and project:releng:signing:cert:dep-signing, which can be used anywhere.

Scriptworker uses the above-linked data structures to verify that a privileged scope is only used on an appropriate branch.  Signingscript determines which level of access to grant based on those scopes [3].

We need to follow this model for the other *scripts.  For pushapk, I think this would be what accounts/products we're allowed to push to.  That would allow chain of trust verification to make sure we're not pushing to a privileged account/product from a non-privileged branch.

If we can't separate creds at first, let's file a followup bug to do so, and verify our upload bucket location matches release, nightly, or staging based on these scopes.

[1] https://github.com/mozilla-releng/scriptworker/blob/121c474f5b21084a4a3742f21c3f30c018e5c766/scriptworker/constants.py#L219
[2] https://github.com/mozilla-releng/scriptworker/blob/121c474f5b21084a4a3742f21c3f30c018e5c766/scriptworker/constants.py#L232
[3] https://github.com/mozilla-releng/signingscript/blob/master/signingscript/task.py#L19

b. downloads / upstreamArtifacts

For signing, we used to have task.payload.unsignedArtifacts, which was a list of URLs.  Now we have task.payload.upstreamArtifacts [4], which is a list of dictionaries that look like

    "upstreamArtifacts": [{
        "paths": [
            "public/build/target.tar.bz2",
            "public/build/target.checksums"
        ],
        "formats": ["gpg"],
        "taskId": "GFPKeLbAQN2fytGOXgatIg",
        "taskType": "build"

    }, {
        ...
    }]

The taskId is the taskId of the task we're downloading from.  The paths are the artifact paths we're downloading.  I don't know if you need to use the "formats" key or need to embed any additional information; we can play with this schema.  "taskType" is there for chain of trust verification.  Currently we only support "build", "l10n", "decision", "docker-image", but we can add more.

Scriptworker will pre-download these artifacts into $artifact_dir/public/cot/$task_id/$path , and verify their SHAs before calling the script.  Pushapk no longer needs to download these artifacts; it can and should use the pre-downloaded artifacts on disk.

I don't know how many artifacts we download, and how large this is going to get.  If we don't want to upload all of these upstreamArtifacts at the end of the pushapk task, we can move them to $work_dir or otherwise remove from $artifact_dir before the end of the task, or change where scriptworker downloads them to.

[4] https://queue.taskcluster.net/v1/task/M81unWcDQje2XEwhtmDXrw

c. scriptworker.cot.verify will need to support pushapk type workers

We'll also have to support any other new task types that we depend on.

d. upstream tasks will need to point at the right deps and have chain of trust generation enabled.

To enable chain of trust generation in a non-scriptworker task, set task.payload.features.ChainOfTrust to true.
When there are additional tasks we need to set as chain of trust dependencies in non-scriptworker tasks, we add them to task.extra.ChainOfTrust.inputs, which looks like

    "inputs": {
        "docker-image": "taskId",
        ...
    }


For upstream scriptworker tasks, we have sign_chain_of_trust [5] and upstreamArtifacts.  We can also follow the same task.extra.chainOfTrust.inputs model if that's easiest.

This may prevent us from fully enabling chain of trust verification on pushapk if we depend directly on other non-signing scriptworker tasks that don't yet have chain of trust enabled, but we have some prefs [6] we can use until it's all enabled end-to-end.

[5] https://github.com/mozilla-releng/scriptworker/blob/121c474f5b21084a4a3742f21c3f30c018e5c766/scriptworker/constants.py#L58
[6] https://github.com/mozilla-releng/scriptworker/blob/121c474f5b21084a4a3742f21c3f30c018e5c766/scriptworker/constants.py#L57-L60

e. puppet

With bug 1316702, we now have a shared scriptworker puppet module.  Let's use that.

* There are updated dependencies, all pushed to the python3.5 pypi location.
* We now use a scriptworker.yaml which is much larger than our previous config.json.  This is populated in the scriptworker module, using variables you pass [7].  I still have the supervisord settings in the signing scriptworker area, because the watch file list can be different per instance type.
* gpg keys - we'll need to create new gpg keys per scriptworker instance, and make sure they're signed by an appropriate key.  The trusted keys are in scriptworker/trusted and the worker keys go into scriptworker/valid in the cot-gpg-keys repo [8].

[7] https://hg.mozilla.org/build/puppet/file/tip/modules/signing_scriptworker/manifests/init.pp#l54
[8] https://github.com/mozilla-releng/cot-gpg-keys
:jlorenzo, I have some questions about pushapk.

a) I think these are rolled out.  Anything live?  Aurora?
b) If so, are these part of the Aurora nightly graph, or a separate graph?
c) I think these are a good candidate for enabling chain of trust verification, since it doesn't directly block nightly tier2.  A separate graph might make it harder, but it might be doable.  Should I start investigating this on my own, or should we wait til we're both back (Nov28?)
Flags: needinfo?(jlorenzo)
Blocks: 1317789
Nice! Thank for the documentation!

> a) I think these are rolled out.  Anything live?  Aurora?
PushApkWorker is on the edge of being live for Aurora (bug 1307826). I got the production machines/creds and "puppetized" it. It should be a matter of turning the machines on and test the whole chain. I'm currently in a work-week for ship-it, so I'm not sure if I'll be able to test it before next week.

Beta and release have not been started yet.

> b) If so, are these part of the Aurora nightly graph, or a separate graph?
PushApk is not officially part of the nightly graph. It creates its own tasks and report back to Treeherder[1]. Tasks are created based on Pulse events when multi-locales builds are done[2]. By the way, the treeherder jobs are still in tier3. I'd like to upgrade them once the production instance is stable enough.

[1] For example, staging instance: https://treeherder.mozilla.org/#/jobs?repo=mozilla-aurora&filter-tier=3&selectedJob=4195094
[2] https://github.com/mozilla-releng/fennec-aurora-task-creator/blob/7c6c5882877bd5bb9a48555698b81cdfcfa86a06/fennec_aurora_task_creator/config.py#L62 and https://github.com/mozilla-releng/fennec-aurora-task-creator/blob/06d773a024080f286d7d5b640bb7ab35d36947e1/fennec_aurora_task_creator/worker.py#L41
> c) I think these are a good candidate for enabling chain of trust verification, since
> it doesn't directly block nightly tier2.  A separate graph might make it harder, but
> it might be doable.  Should I start investigating this on my own, or should we wait
> til we're both back (Nov28?)
I agree, PushApk is a great candidate. If you don't mind, I'd like to finish up the current production instance and get it running for a couple of days. November 28th sounds great to enable COT there :)

More details in regard to comment 0
> a. scopes
The current PushApk is meant to push either nightly or release signed builds[3]. Hence, project:releng:signing:cert:dep-signing would be used.

For the record, Google Play Store has very restrictive checks about what APK can be uploaded. For example, it checks if the signing cert matches the one given in the previous APKs. The only problem would be if somebody tries to push aurora onto beta (which use both the nightly certificate). If I understand correctly, COT wouldn't prevent such a scenario. However, Google Play also checks the package name, which prevent this scenario.
To sum up, pushapk looks perfect to test out COT! We have Google Play which acts as a strong gatekeeper, so even if COT was broken, we won't be afraid using real data.

[3] https://github.com/mozilla-releng/pushapkscript/blob/062afa6bf6395745d266dd5742954f6f0cc0cd76/config_example.json

> b. downloads / upstreamArtifacts
Nice! Right now PushApk needs to have the 2 APKs present on the disk. Our apk publishing script enforces it, so we don't have any version mismatch between architectures[4]. If I understand correctly, we will download 3 files per APK (the actual apk, the checksum file and the asc)

[4] https://github.com/mozilla-releng/mozapkpublisher/blob/05779f38fc2290e615c7acde820cd3070c97b1a4/mozapkpublisher/push_apk.py#L52

> e. puppet
Awesome! This'll will greatly simplify the configuration
Depends on: 1307826
Flags: needinfo?(jlorenzo)
I spoke with :catlee, and we thought:

* we likely won't be enabling chain of trust verification for pushapk until the nightly graph has merged to mozilla-aurora.  At that point, we can make pushapk part of the nightly graph, and enable chain of trust verification.
* until then, we can follow the funsize model, and add an embedded signature from the scheduler in the task definition.  If we verify that signature, we can proceed.  This verification can live in the pushapk script that scriptworker calls.  If the script isn't a good place, we could call a wrapper that verifies the signature before calling the existing pushapk script.

We can still get some of this work done ahead of time, but pushapk shouldn't block the nightly tier1 migration.
No longer blocks: 1317789
See Also: → 1321513
Priority: -- → P3
See Also: → 1345559
Blocks: 1346396
Assignee: nobody → jlorenzo
Status: NEW → ASSIGNED
This allows pushapkscript to downloads artifacts from 2 different signing tasks.

PR r+'d by :aki[1]. Landed on master at [2].

[1] https://github.com/mozilla-releng/scriptworker/pull/92#pullrequestreview-28009946
[2] https://github.com/mozilla-releng/scriptworker/commit/8f10612fd2856289d501f24c37ec4184402f85d6
Attachment #8850112 - Flags: review+
More details in PR.
Attachment #8850113 - Flags: review?(aki)
With the 4 patches above, I got to the point where:
* PushApk is part of the nightly graph (on date)
* My loaner verifies and downloads APKs from the arm and the x86 signing tasks
* pushapkscript knows which APKs is whether the x86 or the arm one. This is done by looking in the APK, under the lib/ folder.
* pushapkscript passes the correct APKs to mozapkpublisher
* Google Play refuses the APKs, because they don't have the right package name. Date has is called "org.mozilla.firefox_date" which is not accepted by Google Play.

Hence, here's what's missing:
* Release a new version of scriptworker containing both patches
* Write tests for the architecture detection in pushapkscript
* Release a new version of pushapkscript
* Add a new dummy decision task in tree. On aurora, this task will check whether trees are closed. Otherwise, it'll be a human decision task. 
* Generate production GPG keys for pushapk_scriptworker.
* Store them the secrets on puppet master
* Update puppet manifests to:
  * point to the above-released versions
  * add my personal public gpg key in the whitelisted bucket
  * turn on CoT on 
* Update [1] to tell releaseduty folks there will be 2 human decision tasks to update

[1] https://github.com/mozilla/releasewarrior/blob/master/how-tos/fennec-temp-relpro.md
Comment on attachment 8850113 [details] [review]
scriptworker PR - Expose absolute paths of upstream artifacts

r+ w/ comments in the PR.
Attachment #8850113 - Flags: review?(aki) → review+
Comment on attachment 8848039 [details]
Bug 1317783 - Put PushApk tasks in-tree

I fixed the problem on try[1]. Like said in comment 8, another patch is needed to add the decision task for pushapk. However, I'd like to know if I'm on the right track. One example of the task graph is at [2]. Try is not broken anymore per[3]

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=85402045&repo=try&lineNumber=166 
[2] https://tools.taskcluster.net/task-group-inspector/#/bm0aWGIdQI6hu-QjytckVg
[3] https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff8da7566e8fdbdc98a93cc00fad1bb6e3eed0fc&selectedJob=85926096
Attachment #8848039 - Flags: feedback?(bugspam.Callek)
Comment on attachment 8848039 [details]
Bug 1317783 - Put PushApk tasks in-tree

https://reviewboard.mozilla.org/r/121006/#review125534

::: taskcluster/taskgraph/transforms/push_apk.py:78
(Diff revision 2)
> +@transforms.add
> +def concatenate_dependent_jobs(_, jobs):
> +    return {
> +        'dependent_tasks': [job.get('dependent-task') for job in jobs],
> +        'label': 'push-apk/opt',
> +    }

This (filter_out and concatenate) are one way you can do this.

Another is to use a custom loader function that does this concatenation before ever getting to the transforms.

The latter *might* be clearer, but I don't have a strong feeling either way (:dustin may have a stronger feeling than me)

::: taskcluster/taskgraph/transforms/push_apk.py:88
(Diff revision 2)
> +    check_every_architecture_is_present_in_dependent_tasks(job['dependent_tasks'])
> +    return job
> +
> +
> +def check_every_architecture_is_present_in_dependent_tasks(dependent_tasks):
> +    if len(dependent_tasks) != len(REQUIRED_ARCHITECTURES):

This code could be problematic if we ever sign normal CI jobs, because then it would be matching against multiple android versions to be ready to push.

It could also rear an ugly head if someone is looking to add a new android build+signing type, before being ready for it to be shipped to users, e.g. android-api20, or android-x86_64 type of potential variants.

(I wouldn't block on it though)

::: taskcluster/taskgraph/transforms/push_apk.py:138
(Diff revision 2)
> +            'platform': 'Android/opt',
> +            'tier': 2,
> +            'kind': 'other',
> +        },
> +        # Force this job to only run in a release-like context
> +        'run-on-projects': ['release', 'date', 'jamun'],

I'd argue the run-on-projects designation should be in the yml config file, somehow.

Date is also meant to be like 'nightly'.

::: taskcluster/taskgraph/transforms/push_apk.py:162
(Diff revision 2)
> +        'paths': ['public/build/target.apk'],
> +    } for task_kind in dependencies.keys()]
> +
> +
> +def generate_google_play_track(project):
> +    return _lookup_project_in_dict(project, GOOGLE_PLAY_TRACT_PER_PROJECT)

GOOGLE_PLAY_TRACT_PER_PROJECT.get(project, 'WRONG_PROJECT')

(I'm also more a fan of 'invalid' or some such)
Comment on attachment 8848039 [details]
Bug 1317783 - Put PushApk tasks in-tree

(Commenting here to drop the feedback? flag...)

10:01 AM <Callek> jlorenzo: https://treeherder.mozilla.org/logviewer.html#?job_id=86514801&repo=date&lineNumber=1051 fyi -- looks like we *need* to not let the human decision stuff be scheduled on `date` and similar trunk-related branch
10:02 AM <Callek> jlorenzo: fails due to lack of `"queue:create-task:null-provisioner/human-decision"`
10:04 AM <jlorenzo> Callek: I added that scope the dev hook on Friday, but probably not to the nightly hook. In any case, you're right, that human decision task will only be on release-like branches. I'll define another type of decision task for nightly-likes (a task which checks the status of Balrog channels)
Attachment #8848039 - Flags: feedback?(bugspam.Callek)
Blocks: 1351664
Attachment #8848039 - Flags: review?(aki)
Comment on attachment 8848039 [details]
Bug 1317783 - Put PushApk tasks in-tree

https://reviewboard.mozilla.org/r/121006/#review127184

::: taskcluster/taskgraph/loader/push_apk.py:23
(Diff revision 3)
> +        yield job
> +
> +
> +def get_dependent_loaded_tasks(config, loaded_tasks):
> +    nightly_tasks = (
> +        task for task in loaded_tasks if task.attributes.get('nightly')

One problem with that filter. It shows up in regular beta graph. For instance, with these params: 
https://queue.taskcluster.net/v1/task/DVZgCCnxQKa4EgbxkUXm1w/runs/0/artifacts/public/parameters.yml 

I see beetmover does the same, but doesn't run on regular beta. :aki, would you know what behaves differently?
(In reply to Johan Lorenzo [:jlorenzo] from comment #15)
> One problem with that filter. It shows up in regular beta graph. For
> instance, with these params: 
> https://queue.taskcluster.net/v1/task/DVZgCCnxQKa4EgbxkUXm1w/runs/0/
> artifacts/public/parameters.yml 
> 
> I see beetmover does the same, but doesn't run on regular beta. :aki, would
> you know what behaves differently?

It's filtered here: https://hg.mozilla.org/releases/mozilla-beta/file/tip/taskcluster/taskgraph/target_tasks.py#l167

We probably want to add the pushapk kind in that list.
(In reply to Aki Sasaki [:aki] from comment #16)
> (In reply to Johan Lorenzo [:jlorenzo] from comment #15)
> > One problem with that filter. It shows up in regular beta graph. For
> > instance, with these params: 
> > https://queue.taskcluster.net/v1/task/DVZgCCnxQKa4EgbxkUXm1w/runs/0/
> > artifacts/public/parameters.yml 
> > 
> > I see beetmover does the same, but doesn't run on regular beta. :aki, would
> > you know what behaves differently?
> 
> It's filtered here:
> https://hg.mozilla.org/releases/mozilla-beta/file/tip/taskcluster/taskgraph/
> target_tasks.py#l167
> 
> We probably want to add the pushapk kind in that list.

Also, https://hg.mozilla.org/releases/mozilla-beta/file/tip/taskcluster/taskgraph/target_tasks.py#l159 filters android out by build_platform.  That's another way to filter pushapk out.
Attachment #8850115 - Flags: review?(aki)
Comment on attachment 8848039 [details]
Bug 1317783 - Put PushApk tasks in-tree

https://reviewboard.mozilla.org/r/121006/#review127322

Overall looks good, with nits.

::: taskcluster/ci/push-apk-breakpoint/kind.yml:28
(Diff revision 4)
> +        treeherder:
> +            symbol: pub(Br)
> +            platform: Android/opt
> +            tier: 2
> +            kind: other
> +        run-on-projects: ['release', 'date', 'jamun']

My approach has been:

- put the correct, restricted list on m-c, m-a, and m-b
- add a staging patch onto jamun and date to allow for them to run staging stuff as well

So here, we'd set `run-on-projects` to ['release'], and then add jamun and date on jamun and date, respectively.  That way, when we stop using jamun and date, we don't have to remember to clean up as many places.

::: taskcluster/ci/push-apk/kind.yml:26
(Diff revision 4)
> +        worker-type: scriptworker-prov-v1/pushapk-v1-dev
> +        worker:
> +            upstream-artifacts: # see transforms
> +            google-play-track: # see transforms
> +            implementation: push-apk
> +            # TODO unhardcode that line

Ah, we're running with `dry-run: true`? Makes sense, til we know it looks good when scheduled in-tree.

::: taskcluster/ci/push-apk/kind.yml:34
(Diff revision 4)
> +        treeherder:
> +            symbol: pub(gp)
> +            platform: Android/opt
> +            tier: 2
> +            kind: other
> +        run-on-projects: ['release', 'date', 'jamun']

Another place where we can remove staging branches from the release-branch config.

::: taskcluster/taskgraph/transforms/task.py:366
(Diff revision 4)
> +
> +            # Paths to the artifacts to sign
> +            Required('paths'): [basestring],
> +        }],
> +
> +        Required('google-play-track'): Any('production', 'beta', 'alpha', 'invalid'),

I'm not entirely clear why we have an 'invalid' here.  If we don't have a task on non-release branches, should we ever have 'invalid'?

If this is a way to support try or other branches, but force noop if we're invalid, I approve.

::: taskcluster/taskgraph/util/scriptworker.py:155
(Diff revision 4)
> +        'date',
> +    ])
> +], [
> +    'beta', set([
> +        'mozilla-beta',
> +        'jamun'

Here too for jamun+date.

::: taskcluster/taskgraph/util/scriptworker.py:182
(Diff revision 4)
> +}
> +
> +PUSH_APK_BREAKPOINT_WORKER_TYPE = {
> +    'aurora': 'aws-provisioner-v1/taskcluster-generic',
> +    'beta': 'null-provisioner/human-decision',
> +    'release': 'null-provisioner/human-decision',

We could call this breakpoint instead of human-decision, but doesn't matter.
Attachment #8848039 - Flags: review?(aki) → review+
Attachment #8850115 - Flags: review?(aki) → review+
Comment on attachment 8848039 [details]
Bug 1317783 - Put PushApk tasks in-tree

https://reviewboard.mozilla.org/r/121006/#review127322

> My approach has been:
> 
> - put the correct, restricted list on m-c, m-a, and m-b
> - add a staging patch onto jamun and date to allow for them to run staging stuff as well
> 
> So here, we'd set `run-on-projects` to ['release'], and then add jamun and date on jamun and date, respectively.  That way, when we stop using jamun and date, we don't have to remember to clean up as many places.

Fixed. I explicitely called out m-a, m-b and m-r (instead of "release") because we don't publish nightly on Google Play (yet).

> Ah, we're running with `dry-run: true`? Makes sense, til we know it looks good when scheduled in-tree.

Correct. I'll land a follow patch once things look good.

> I'm not entirely clear why we have an 'invalid' here.  If we don't have a task on non-release branches, should we ever have 'invalid'?
> 
> If this is a way to support try or other branches, but force noop if we're invalid, I approve.

That's a way to noop (I should say, not break) on try and other non-supported branches. "Invalid" is a suggestion that I took from called. I agree with him, it makes things clearer if a task is accidentally created. 

In regard to this part of the code, I added a comment.

> We could call this breakpoint instead of human-decision, but doesn't matter.

`human-breakpoint` it is :) (`human-decision` is the worker type for desktop release promotion. I agree keeping `decision` here is confusing).
Attached file cot-gpg-keys PR
Attachment #8852950 - Flags: review?(aki)
Attachment #8852958 - Attachment description: Bug 1317783 - enable chain of trust verification in pushapk → Puppet patch
Comment on attachment 8852958 [details]
Bug 1317783 - Turn on CoT on pushapk_scriptworker

https://reviewboard.mozilla.org/r/119734/#review127682
Attachment #8852958 - Flags: review?(aki) → review+
Comment on attachment 8852950 [details] [review]
cot-gpg-keys PR

r+ed and merged.
Attachment #8852950 - Flags: review?(aki) → review+
Attachment #8852950 - Flags: checked-in+
Attachment #8850115 - Flags: checked-in+
Attachment #8850113 - Flags: checked-in+
Attachment #8850112 - Flags: checked-in+
Keywords: leave-open
Attachment #8852958 - Attachment description: Bug 1317783 - part 1: scriptworker, add jlorenzo pubkey → Puppet patch part 1: add jlorenzo pubkey
> Mar 30 11:28:31 signing-linux-1.srv.releng.use1.mozilla.com puppet-agent: (/Stage[main]/Signing_scriptworker/Scriptworker::Instance[/builds/scriptworker]/Scriptworker::Chain_of_trust[signing_scriptworker]/Exec[rebuild_gpg_homedirs]/returns) scriptworker.exceptions.ScriptWorkerGPGException: Found a revocation marker rev in DB7DD96BE5D86A036B1DEB6CD544400EB6233422!

This is the subkey revocation in your gpg key that we hit a while back.

We can either
1. support subkey revocation (difficult to do right in my estimation)
2. have someone else generate the scriptworker keys and signatures
3. generate a new key for you with no revocation markers

both (2) and (3) will involve new cot-gpg-keys patches for the pushapk pubkey, and backing out your [old] pubkey.
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e91496c7153e
Backed out changeset 0edd9de2ca10 for duplicate checkin to autoland
This is attachment 8848039 [details] rebased on top of the aurora branch. Decision task has been tested locally with the parameters of: aurora-nightly-android, aurora-nightly-desktop, aurora, beta-candidate, beta, central, inbound, try

Even though attachment 8848039 [details] hasn't made central yet. I plan to land it in aurora. The reasons are the following: 
* attachment 8848039 [details] has (and must have) no effect on central. It's only relevant to mozilla-aurora, mozilla-beta, mozilla-release
* it should land before an aurora build (as in the build we ship daily) starts
* I warned release management and sheriffs on IRC about the potential breakage (notably the Nightly Android decision task being broken because of some Taskcluster missing scopes). 

If anything goes wrong, I'll remain ready to fix aurora nightlies today. I'll remain available to backout this patch as well.
Attachment #8853298 - Flags: checked-in+
Attachment #8852958 - Flags: checked-in+
Blocks: 1352367
Blocks: 1352477
So we are backing thing out now for predicted future bustage now?
In case you don't get it I am referring to the check-in comment for the backout that implies this has been backed out because it might break nightly in the future.
Plus we have this odd needinfo request which asks for no information. If there was other dialog on IRC about this, that fact should have been documented in the bug.
I Hate to be a stickler about things but we should really be up to independent security audits which means everything needs to follow our documented policies, which means everything needs to be documented in the bug.  offline stuff wont really cut it or stand up to any kind of security audit.
Thank you for backing this patch out :KWierso :) . For reference, this was my request to #sheriff on Friday March, 31st (UTC):
> 18:01:11  <jlorenzo> KWierso: hey! would it possible to back out https://hg.mozilla.org/mozilla-central/rev/0edd9de2ca10 ? This is gonna break next nightly :| 
> 18:01:40  <KWierso> jlorenzo: sure thing
> 18:01:45  <jlorenzo> that's because of a side-effect described at https://bugzilla.mozilla.org/show_bug.cgi?id=1352477
> 18:01:48  <firebot> Bug 1352477 — NEW, jlorenzo@mozilla.com — taskgraph ignores run-on-project for fennec-nightlies


For reference, this patch now lives in Aurora and in Beta. That's needed to release Fennec in both channels and doesn't break anything there. Bug 1352477 only affects central. I'll fix that bug first.
No longer blocks: 1352477
Depends on: 1352477
Flags: needinfo?(jlorenzo)
No longer blocks: 1346396
Duplicate of this bug: 1346396
Blocks: 1353303
Blocks: 1353333
I rebased attachment 8848039 [details] on top of bug 1352477. Because that bug only impacted central, there is no need to uplift the changes in aurora and beta.

For the record, the uplift to beta (attachment 8853379 [details] [diff] [review]) led to a correct task graph: https://tools.taskcluster.net/task-group-inspector/#/ehxz2GL0RU2IpOy5aOgN6Q. We just need the greenlight to publish Fennec 53.0b9 on Google Play.

Once attachment 8848039 [details] reaches central and the task is verified on beta, we'll be able to resolve this bug.
Landed in central + task graph passed on beta[1]. Let's close this bug.

[1] https://tools.taskcluster.net/task-group-inspector/#/ehxz2GL0RU2IpOy5aOgN6Q/GNUsvLvzTa2e-GXxMm0pHQ?_k=u6p2rz
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Blocks: 1286297
Duplicate of this bug: 1306311
No longer blocks: 1428444
Depends on: 1432817
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.