Either sign or bootstrap talos extensions and install them temporarily

RESOLVED FIXED in Firefox 46

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

unspecified
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed, firefox47 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

3 years ago
There are some talos extensions that are needed to run talos. There seem to be 4 in total:

1) https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/pageloader
2) https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/talos-powers
3) https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/startup_test/tresize/addon
4) https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/startup_test/sessionrestore/addon

For each of the above we'll either need to sign them and check in the signed versions of the addon, or else make them restartless and install them at runtime.
and we have:
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/devtools
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/tart
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/tabswitch

we have wanted to re-write pageloader in bug 1231626, but it isn't a high priority.  Is there a specific guide for what needs to be done here?  Knowing that this took ~3 months for mochitest/reftest/etc. should I plan on at least 1 month of dev time (probably 3 months for me to complete between other meetings/projects)?

:ahal, is this something you are going to do?
Flags: needinfo?(ahalberstadt)
Assignee

Comment 2

3 years ago
The easiest thing to do is to just sign them. The downside to that is that every time they need to get changed (even if just pushing to try for example) they'll need to get signed again. Signing isn't terribly difficult or anything, but it could be a big enough burden if they change frequently.

The other option is to install them at runtime. This means they can no longer be installed by dropping them into the test profile. There's a marionette API to install temporary addons, or if depending on marionette is too hard, there's a chrome JS api that can be used to do it as well. I assume this won't take as long as mochitest as there aren't thousands of tests to deal with. I don't mind looking into it, but keep in mind I've never written a talos patch before, so I could take a bit longer.
Flags: needinfo?(ahalberstadt)
if we sign them, and we want to debug something locally or on try, would that be possible?  if it is, these rarely change, so signing would be very realistic.  Are there docs for how to sign this?  Are there requirements?  

I recall trying marionette out a couple of years ago and the perf hit was pretty high and it created more variations into the results making them noisy.  is there docs for the chrome JS api?
Assignee

Comment 4

3 years ago
You can use jpm to sign addons, here's a tutorial:
https://blog.mozilla.org/addons/2015/12/18/signing-firefox-add-ons-with-jpm-sign/

Kmoir has already created an "automation account" on AMO, so she can e-mail us a key + secret pair to use (we can't expose them publicly though).

As for installing at runtime, I don't think there's any documentation, but we can copy what marionette does:
https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/addons.py#86
Assignee

Comment 5

3 years ago
Oh, and debugging is still possible, it's just a pain. Like, if you want to add a dump statement, the addon needs to be re-signed.

The plan is to streamline the signing process. Eventually maybe extensions will be automatically signed if mozbuild detects they change or something.. but for now, it'll be a manual process.
Assignee

Comment 6

3 years ago
The jpm sign tool started timing out so I couldn't sign either tart or tabswitch.. but I got the others. Here's a try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=de5956771c85
Assignee

Comment 7

3 years ago
Ok, that didn't work. Here's the relevant log:

> addons.xpi	WARN	Refusing to install staged add-on tresize@mozilla.org with signed state 1
> addons.xpi	WARN	Refusing to install staged add-on talos-powers@mozilla.org with signed state 0
> addons.xpi    WARN    Refusing to install staged add-on pageloader@mozilla.org with signed state 0

There are two things that jump out:

1) For some reason talos-powers and pageloader still don't seem to be signed. I verified this locally, for some reason `jpm sign` appears to succeed when they get signed, but then there are no key files inside of the returned .xpi files. I don't know why it worked for tresize, but not talos-powers or pageloader.

2) Even though tresize got signed, it's still not being installed. Notably, this WARN message isn't the normal error message I've seen when working on mochitest/reftest. So possibly talos is trying to install the addons differently somehow?

tl;dr - I have no idea what's going on.
Assignee

Comment 9

3 years ago
I already emailed this to Andy, but figured I'd post her for posterity. Here are some STR to demonstrate how `jpm sign` seems to be broken (substitute version for the one you bump install.rdf to and ping me for credentials if you want to try it out):


STR:
$ cd mozilla-central/testing/talos/talos/pageloader
$ <bump version in install.rdf>
$ zip -r pageloader.xpi .
$ jpm sign --api-key <key> --api-secret <secret> --xpi pageloader.xpi
JPM [info] Signing XPI: pageloader-signed.xpi
Validating add-on [..................................................................]
JPM [info] Validation results: https://addons.mozilla.org/en-US/developers/upload/acb9b8197057445f9ff20aa79285b200
Downloading signed files: 100%
JPM [info] Downloaded:
JPM [info]     ./pageloader_extension-1.0.1.xpi
JPM [info] SUCCESS

$ unzip -q pageloader_extension-1.0.1.xpi
$ ls META-INF
ls: cannot access META-INF: No such file or directory
can we assign this bug to whomever is in charge of the signing server?
Assignee

Comment 11

3 years ago
Hi Kumar, could you help us triage and find someone to look into this? See comment 7 on down. Should I file a bug (if so what component) or a github issue?
Flags: needinfo?(kumar.mcmillan)
Assignee

Comment 12

3 years ago
Andy figure out what was going on. Apparently AMO won't sign anything with an application-id of 'toolkit@mozilla.com'. He said he'll fix it on the AMO side.

Andy, do you have any idea about the second problem? Namely, see comment 7 and notice the first line:
> addons.xpi  WARN  Refusing to install staged add-on tresize@mozilla.org with signed state 1

It seems to fail to install even though it's signed. Do you know why that might be?
Flags: needinfo?(kumar.mcmillan) → needinfo?(amckay)

Comment 13

3 years ago
AMO change being tracked here: https://github.com/mozilla/addons-server/issues/1740
Flags: needinfo?(amckay)

Comment 14

3 years ago
Signing toolkit@mozilla.org add-ons on AMO will mean we sign all add-ons with that application and that will hit Thunderbird, Seamonkey and other add-ons. It would mean that we would strip out any third party certs on those add-ons.

Would it be possible just to add in Firefox as a supported application on those add-ons? Then AMO will sign them and the Firefox tests which depend upon signing will get them signed.
Flags: needinfo?(ahalberstadt)
Assignee

Comment 15

3 years ago
Yeah, I don't think talos is even run on any of those other applications, so we can probably just change it from toolkit to firefox.
Flags: needinfo?(ahalberstadt)
we only run on firefox/fennec.  I assume changing from toolkit@mozilla.org will work just fine.
Assignee

Comment 17

3 years ago
So I've successfully signed everything, but I'm still hitting that second problem. I get these errors in the log:

> addons.xpi	WARN	Refusing to install staged add-on tresize@mozilla.org with signed state 1
> addons.xpi	WARN	Refusing to install staged add-on talos-powers@mozilla.org with signed state 1
> addons.xpi	WARN	Refusing to install staged add-on pageloader@mozilla.org with signed state 1

I did some digging and it's because of this condition [1]. So the addons I've signed are foreignInstall (whatever that means) and SIGNEDSTATE_PRELIMINARY [2] which is less than SIGNEDSTATE_SIGNED. So we either need to:

A. Make the signed addons non-foreign (not sure what this means)
B. Make them SIGNEDSTATE_SIGNED (I'm guessing this means an AMO review? If so that's not feasible)
C. Relax the condition and install foreign addons with SIGNEDSTATE_PRELIMINARY

Andy, any idea what this is about? How can I get these installed?

[1] https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#3258
[2] https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm#3121
Flags: needinfo?(amckay)

Comment 18

3 years ago
Good news, the difference between sideloaded and not is being removed in bug 1245956. So let's wait for that to land and then let's try this again.
Depends on: 1245956
Flags: needinfo?(amckay)
Assignee

Comment 19

3 years ago
Though bug 1245956 landed, the patch seems to have omitted the change we need for this bug, see bug 1245956#c18.
Duplicate of this bug: 1220593
Assignee

Comment 21

3 years ago
The various harness addons and test related addons used by talos all need
to be signed before we can enforce addon signing. For now, signing will be
a manual process. See the following guide for more details:
https://wiki.mozilla.org/EngineeringProductivity/HowTo/SignExtensions

Review commit: https://reviewboard.mozilla.org/r/37171/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37171/
Attachment #8724832 - Flags: review?(jmaher)
Comment on attachment 8724832 [details]
MozReview Request: Bug 1249733 - Sign talos harness and test extensions, r?jmaher

https://reviewboard.mozilla.org/r/37171/#review33711

assuming try ends up good, lets get moving forward.
Attachment #8724832 - Flags: review?(jmaher) → review+
Assignee

Comment 23

3 years ago
So looks like the 's' job is permafail:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a07f09c2d4fa

Jmaher noticed this line in the log:
> PROCESS | 10433 | 1456765284261	addons.productaddons	ERROR	Request failed certificate checks: [Exception... "SSL is required and URI scheme is not https."  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: resource://gre/modules/CertUtils.jsm :: checkCert :: line 145"  data: no]

Though, it doesn't happen immediately before the error, so I'm not sure it's related. The other jobs all seem to pass though.
we seem to fail on the cart/tart test- both the same addon:
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/tests/tart/addon

quite possibly there is something odd there.  Given this fails, running locally with |mach talos-test -a tart| should yield any failure.
Assignee

Comment 25

3 years ago
I can't reproduce this locally. I get return code 0.
if you did -a tart, lets try a couple other things locally:
mach talos-test -a cart
mach talos-test -a tart --e10s
mach talos-test -a cart --e10s

outside of that, I would be baffled why we couldn't reproduce this locally.  we seem to be failing on all 's' jobs- lets also determine if another test is somehow causing problems.

This is the list of tests we run:
https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos.json?from=talos.json#69

possibly push to try with no cart/tart in the list and see if 's' is green.  We could narrow it down that way.  Also push to try with only cart in the list, only tart in the list.  I assume a single platform like linux64 would suffice for fixing this.
Assignee

Comment 27

3 years ago
Aha!
./mach talos-test -a tart
./mach talos-test -a tart --e10s 

both passed, but -a cart fails. And it was much easier to pick the error out of my local logs than it was on treeherder:
> 1456766423108	addons.xpi	WARN	Refusing to install staged add-on bug848358@mozilla.org">bug848358@mozilla.org with signed state 0

Looks like I missed one!
Assignee

Comment 28

3 years ago
Fixed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bafd8ed86689

I don't think this requires a re-review, so I'll upload the new patch and carry the r+ forward. I also verified that no talos addons were modified recently on any of the integration branches.
Assignee

Updated

3 years ago
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Assignee

Comment 29

3 years ago
Comment on attachment 8724832 [details]
MozReview Request: Bug 1249733 - Sign talos harness and test extensions, r?jmaher

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37171/diff/1-2/

Comment 31

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/651ae113dc0d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.