Closed Bug 873600 Opened 11 years ago Closed 11 years ago

Update pre-existing packages ids.json to match device naming

Categories

(Marketplace Graveyard :: General, defect, P1)

Product:
Marketplace Graveyard
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
2013-05-30

People

(Reporter: robhudson, Assigned: ashort)

References

Details

(Whiteboard: p=2, A4A) 

See bug 870920.

We need a way to update all existing packages and update their META-INF/ids.json file to match the keys used on device side.

One thought I had was to remove the code that adds this file upon submission and do it pre-signing (assuming signing takes this file into account, otherwise we can do it post-signing while we're already writing META-INF files). Then we can remove all reviewer signed packages and re-sign all publicly signed packages (which we have a script for).
Ryan: Since ids.json is inside META-INF/ does it get used as part of the file's signature? IOW, can I add that file after signing and not break verification?
Flags: needinfo?(rtilder)
The META-INF/ids.json file is inside the manifest.mf which I'm pretty certain means it affects the package signature. So we need to include the ids.json prior to signing.
Flags: needinfo?(rtilder)
Alan: Can you take this on?

My thought was to move the `inject_ids` call, which currently happens during packaged app submission, to the pre-signing step. Once done we can re-sign all the public apps and not need an extra script, which may also come in handy later.

Note: This needs to handle cases where there is no META-INF/ids.json file (new packages) and cases where that file already exists (old packages).
Assignee: nobody → ashort
Priority: -- → P1
Whiteboard: p=2
Target Milestone: --- → 2013-05-23
https://github.com/washort/zamboni/commit/6db3f07
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
For QA on -dev:

Try to install this app:
https://marketplace-dev.allizom.org/app/simple-calculator

It should fail with an INVALID_IDS_JSON error (iirc).

Run this on -dev:
./manage.py --settings=settings_local_mkt sign_apps --webapps=428684

Try to install again to see if the ids.json file was fixed properly.
I keep hitting reinstall_forbidden for packaged apps on dev even after a fresh install. So, i'm blocked from verifying this bug on -dev
Whiteboard: p=2 → p=2, A4A
We found an issue with the script after further testing. We'll need to fix before running on prod. Unfortunately this misses our push for today.

The issue is the re-signing script mistakenly adds multiple META-INF/ids.json files. I believe this then triggers a signing error when we pass it to the signing server. Regardless this is the wrong behavior and we'll fix before running on any packaged apps on production machines.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 830876
Are you sure this isn't running on prod now?

I'm getting signing errors (INVALID_SIGNATURE) and the zips I've downloaded have multiple ids.json in the META-INF folder.
(In reply to Andrew Williamson [:eviljeff] from comment #8)
> Are you sure this isn't running on prod now?
> 
> I'm getting signing errors (INVALID_SIGNATURE) and the zips I've downloaded
> have multiple ids.json in the META-INF folder.

You're right, it is. I was only considering if we run it as part of our "re-signing" script. We're planning a 2 stage fix...
(1) to sign and re-sign packages regardless of if it already has ids.json file(s), and 
(2) a clean up script to strip ids.json files from existing uploaded zip packages. 

(2) isn't needed for (1) to work correctly, but we don't want those showing up if a dev re-downloads their original zip or showing up in the file viewer.
Reverted the prior patch (in comment 4) since we've discovered problems in prod:
https://github.com/mozilla/zamboni/commit/46a0b76
Target Milestone: 2013-05-23 → ---
https://github.com/washort/zamboni/commit/417d95d
https://github.com/washort/zamboni/commit/f606174
https://github.com/washort/zamboni/commit/ceeabbf
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2013-05-30
(In reply to Rob Hudson [:robhudson] from comment #5)
> For QA on -dev:
> 
> Try to install this app:
> https://marketplace-dev.allizom.org/app/simple-calculator
> 
> It should fail with an INVALID_IDS_JSON error (iirc).
> 
> Run this on -dev:
> ./manage.py --settings=settings_local_mkt sign_apps --webapps=428684
> 
> Try to install again to see if the ids.json file was fixed properly.

Installing simple calculator on -dev works just fine. Did we run the script on -dev already?
Rob did run this on -dev already.
I will mark this bug as verified after testing in prod tomorrow.
What's the status ?
Target Milestone: 2013-05-30 → 2013-05-23
Target Milestone: 2013-05-23 → 2013-05-30
I'm not seeing this is fixed - packaged apps still aren't installing - and the zip files contain duplicate ids.json files - seemingly one from the upload and one from when it was signed.
e.g. 
https://marketplace.firefox.com/reviewers/apps/review/foot-pong
https://marketplace.firefox.com/reviewers/apps/review/productivity-generator
https://marketplace.firefox.com/reviewers/apps/review/germanhelp
Severity: normal → major
Flags: needinfo?(ashort)
This is all fixed now, right?
I was able to install successfully the following packaged apps:
KitchenSink, Poppit, Terra and Calculator

Test device info:
Leo
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/4f318822e72c
Gaia   e1c59baed29e4665d1da9392f86239272073f07a
Build  2013-05-31-070205
(In reply to Lisa Brewster [:adora] from comment #20)
> But not able to install:
> https://marketplace.firefox.com/reviewers/apps/review/poppit
> https://marketplace.firefox.com/reviewers/apps/review/notesplus
> https://marketplace.firefox.com/reviewers/apps/review/kitchensink

App updates are still broken.  The script only fixed public apps and pending app submissions unfortunately.  

Rob, is it feasible for the script to be run on pending updates also or are we going to have to contact the developers and ask them to submit again (there were 6 updates last time I Looked)
Flags: needinfo?(ashort) → needinfo?(robhudson.mozbugs)
(In reply to Andrew Williamson [:eviljeff] from comment #21)
> Rob, is it feasible for the script to be run on pending updates also or are
> we going to have to contact the developers and ask them to submit again
> (there were 6 updates last time I Looked)

Yep, I could come up with a query to find those. I'll try to do that today and have IT run it.
Flags: needinfo?(robhudson.mozbugs)
(In reply to Rob Hudson [:robhudson] from comment #22)
> (In reply to Andrew Williamson [:eviljeff] from comment #21)
> > Rob, is it feasible for the script to be run on pending updates also or are
> > we going to have to contact the developers and ask them to submit again
> > (there were 6 updates last time I Looked)
> 
> Yep, I could come up with a query to find those. I'll try to do that today
> and have IT run it.

It was run and updated 8 packaged app updates.
QA - WHen you verify this will you confirm that 859688
 is also fixed?
See Also: → 859688
bug 859688 is completely unrelated to this bug.
See Also: 859688 → 
Worked for me for Distant Orbit, thanks !
You need to log in before you can comment on or make changes to this bug.