1156333 - Don't review a file if its signing failed

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect)

Description

[Mathieu Agopian [:magopian]]

We should not sign a file if the signing failed, for whatever reason (eg: the signing server is down).

We should halt the flow at that point.

Comment 1

[Mathieu Agopian [:magopian]]

I meant "we should not review a file if the signing failed". The signing is done when the review is made, so if a file is reviewed, so we should rather fail the review entirely.

Comment 2

[Mathieu Agopian [:magopian]]

PR: https://github.com/mozilla/olympia/pull/524

The STR are simple... but I'm not sure if they're doable: I think it'll need an ops help to either take the signing endpoint down, or simply configure wrong ones so the post to the signing endpoint fails:
1/ submit a new addon, a new version, or a new file
2/ make sure the signing endpoints are not working
3/ review the addon: it should try signing the file, and fail (and show a "oops" page, at least that's what's done in the current iteration)
4/ if you go back to the review queue, the addon should still be there to be reviewed, and the the signing endpoint is "fixed", it should be reviewable (and will sign it this time)

If it's too complicated to QA, I can simply mark the bug as [qa-]

Comment 3

[Mathieu Agopian [:magopian]]

Fixed in https://github.com/mozilla/olympia/commit/8d6f0ba353a5eabe236a16c88b1371e8d6bd0389

Comment 4

[Madalin Cotetiu]

The steps are not complicated but we would need help with the step 2. Who should we talk to to help us with the step 2?

Comment 5

[Mathieu Agopian [:magopian]]

Maybe :jason can help with that? Or I can mark the bug as [qa-]

Comment 6

[Jason Thomas [:jason]]

(In reply to Madalin Cotetiu from comment #4)
> The steps are not complicated but we would need help with the step 2. Who
> should we talk to to help us with the step 2?

Ping me on IRC and I can block/break the signing service on -dev for testing.

Comment 7

[Madalin Cotetiu]

First part seems to work, i get an oops error after trying to approve, the issue is that the add-on is really approved
Screencast: http://screencast.com/t/Qx9tjHISU
Reopening.

Comment 8

[Mathieu Agopian [:magopian]]

New PR: https://github.com/mozilla/olympia/pull/562

Comment 9

[Mathieu Agopian [:magopian]]

Fixed in https://github.com/mozilla/olympia/pull/562

Comment 10

[Madalin Cotetiu]

Verified as fixed in FF38(Win7) in addons-dev.allizom.org
Postfix screencast with the services blocked: http://screencast.com/t/vSc0xSeGFcN
After turning them on: http://screencast.com/t/7sOWfP8X3qBL
Also the file is signed after the approval 
Closing bug.