785636 - Automated Blocklist Update (for tree checkin) failing on all approval-required trees

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Justin Wood (:Callek)]

We currently have the automated blocklist update script failing on all our approval required trees, that would be beta and aurora. For both TB and Firefox.

Consequences of this being that end-users who install Firefox/TB may be vulnerable to crashy/malicious plugins for the time it takes the application to automatically contact AMO and download the up to date blocklist version.

e.g.

hg -R blocklist commit -u ffxbld -m Automated blocklist update from host linux-ix-slave13
hg -R blocklist push -e ssh -l ffxbld -i ~cltbld/.ssh/ffxbld_dsa ssh://hg.mozilla.org/releases/mozilla-beta
pushing to ssh://hg.mozilla.org/releases/mozilla-beta
searching for changes
remote: Warning: Permanently added the RSA host key for IP address '63.245.215.25' to the list of known hosts.
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: Pushing to an APPROVAL REQUIRED tree requires your top changeset comment to include: a=... (or, more accurately, a\S*=...)
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.a_treeclosure hook failed
ERROR hg push exited with exit code: 1, probably raced another changeset
hg -R blocklist rollback
rolling back to revision 100558 (undo commit)

Comment 1

[Phil Ringnalda (:philor)]

That'd be because bug 637444 stopped doing CLOSED TREE, and sync-hg-blocklist.sh has a single flag for adding "CLOSED TREE a=blocklist-update".

Assuming that was accidental rather than intentional (aurora and beta and release didn't actually get mentioned there), there's three problems:

* approval and closed need to be separate flags

* release would need both, except

* release really needs to stop being CLOSED at all times, because that makes it impossible to close it - there are times when it should be closed not only to accidental pushes to it, but also to things which are explicitly approved for it (and to blocklist updates), which is impossible with the way it misuses CLOSED to mean APPROVAL REALLY REALLY REQUIRED

Comment 2

[Alex Keybl [:akeybl]]

Can you all give us an idea for how long this has been happening (since bug 637444?), and whether this also affected the FF15 release build?

In the meantime, can we easily update the blocklist on beta/release manually prior to building?

(In reply to Justin Wood (:Callek) from comment #0)
> Consequences of this being that end-users who install Firefox/TB may be
> vulnerable to crashy/malicious plugins for the time it takes the application
> to automatically contact AMO and download the up to date blocklist version.

My biggest concern from the release standpoint is not those ~24 hours till the first blocklist ping (leaving the user vulnerable), but rather add-ons/plugins that we've blocklisted because they cause the user to startup crash after installation.

(In reply to Phil Ringnalda (:philor) from comment #1)
> * release really needs to stop being CLOSED at all times, because that makes
> it impossible to close it - there are times when it should be closed not
> only to accidental pushes to it, but also to things which are explicitly
> approved for it (and to blocklist updates), which is impossible with the way
> it misuses CLOSED to mean APPROVAL REALLY REALLY REQUIRED

If changing the mozilla-release branch to APPROVAL REQUIRED helps in any way, I have no issue with doing that.

Comment 3

[Justin Wood (:Callek)]

(In reply to Alex Keybl [:akeybl] from comment #2)
> Can you all give us an idea for how long this has been happening (since bug
> 637444?), and whether this also affected the FF15 release build?

Lets see:

Last automated checkin for this on what is now m-release (Firefox 15) was |2012-06-02 03:12 -0700| 

There was an automated landing on m-c for blocklist on |2012-06-09 03:12 -0700| (which was backed out for orange, as well as other m-c landings since then)

Last automated checkin for this on what is now m-aurora (Firefox 16) was |2012-07-14 03:11 -0700|

(next on central after that was |2012-08-04 03:24 -0700|)

So it seems like quite a while. I believe philor was right re: broken since Bug 637444

> In the meantime, can we easily update the blocklist on beta/release manually
> prior to building?

It should be quite easy for someone to update the blocklist manually, until we properly fix this bug.

> (In reply to Justin Wood (:Callek) from comment #0)
> > Consequences of this being that end-users who install Firefox/TB may be
> > vulnerable to crashy/malicious plugins for the time it takes the application
> > to automatically contact AMO and download the up to date blocklist version.
> 
> My biggest concern from the release standpoint is not those ~24 hours till
> the first blocklist ping (leaving the user vulnerable), but rather
> add-ons/plugins that we've blocklisted because they cause the user to
> startup crash after installation.

Good point, I didn't think of that situation.

> (In reply to Phil Ringnalda (:philor) from comment #1)
> > * release really needs to stop being CLOSED at all times, because that makes
> > it impossible to close it - there are times when it should be closed not
> > only to accidental pushes to it, but also to things which are explicitly
> > approved for it (and to blocklist updates), which is impossible with the way
> > it misuses CLOSED to mean APPROVAL REALLY REALLY REQUIRED
> 
> If changing the mozilla-release branch to APPROVAL REQUIRED helps in any
> way, I have no issue with doing that.

It won't fix this bug, since we never automatically [ever tried to] checkin blocklist updates to m-release anyway, and this has been failing on aurora/beta.

Comment 4

[Justin Wood (:Callek)]

I just manually updated Firefox and Thunderbird on aurora with a local run of the script

hg -R blocklist push -e ssh -l [e-mail] -i ~/.ssh/id_dsa ssh://hg.mozilla.org/releases/mozilla-aurora
pushing to ssh://hg.mozilla.org/releases/mozilla-aurora
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: You can view your change at the following URL:
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/2ec0163a9aa9
remote: Trying to insert into pushlog.
remote: Please do not interrupt...
remote: Inserted into the pushlog db successfully.

AND:

hg -R blocklist push -e ssh -l [e-mail] -i ~/.ssh/id_dsa ssh://hg.mozilla.org/releases/comm-aurora
pushing to ssh://hg.mozilla.org/releases/comm-aurora
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: You can view your change at the following URL:
remote:   https://hg.mozilla.org/releases/comm-aurora/rev/f0f70426d010
remote: Trying to insert into pushlog.
remote: Please do not interrupt...
remote: Inserted into the pushlog db successfully.

Comment 5

[Phil Ringnalda (:philor)]

We don't try to update esr10, either, but particularly in light of the "we blocklist things that cause startup crashes" thing, I'm not sure bug 663820 comment 8 is a persuasive reason to not update them both.

Though given the Friday release builds, they might be better off with either manual pushes, or better yet Thursday automatic pushes - along with minimizing the chances of getting into a push race, Saturday updates also minimize the chances of anyone noticing bustage, which thanks to our permasheriff in GMT a late Thursday/early Friday push would not.

Comment 6

[Alex Keybl [:akeybl]]

We discussed this today in crash-kill, a respin of 15.0 won't be necessary given a review of recent blocklist additions.

Comment 7

[Alex Keybl [:akeybl]]

catlee - who's in the best position to look at this for the next release? Thanks!

Comment 8

[Chris AtLee [:catlee]]

coop touched the blocklist stuff last

Comment 9

[Chris Cooper [:coop] (he/him)]

Damn blocklist cooties.

I can add separate flags for closed tree and approval. Can I get a ruling as to what the default state of those flags should be for each tree where this would be running?

mozilla-central
mozilla-aurora
mozilla-beta
mozilla-release
mozilla-esr10

Comment 10

[Ed Morley [:emorley]]

I've just changed *-release to approval required (from closed), since as philor correctly said, it gives us no way to close the tree otherwise.

As such, all trees apart from mozilla-central just need "a=foo", but do not need to (& must not use) CLOSED TREE. 

mozilla-central may optionally use a=foo as well, since it won't hurt anything and I'm presuming makes your life easier :-)

Comment 11

[Chris Cooper [:coop] (he/him)]

Attachment: Add distinct -a option for setting blocklist update approval

Comment 12

[Chris Cooper [:coop] (he/him)]

Attachment: Set blocklist_update approval by default (buildbot-configs)

Comment 13

[Chris Cooper [:coop] (he/him)]

Attachment: Pass -a option when blocklist_update_set_approval is set (buildbotcustom)

Comment 14

[Chris Cooper [:coop] (he/him)]

Attachment: Add distinct -a option for setting blocklist update approval

Comment 15

[Justin Wood (:Callek)]

Comment on attachment 660138 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval

Review of attachment 660138 [details] [diff] [review]:
-----------------------------------------------------------------

coop++ for escaping the quotes in the echo while you're here

Comment 16

[Chris Cooper [:coop] (he/him)]

Comment on attachment 660138 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval

https://hg.mozilla.org/build/tools/rev/c364b05afc33

Comment 17

[Chris Cooper [:coop] (he/him)]

Comment on attachment 660136 [details] [diff] [review]
Set blocklist_update approval by default (buildbot-configs)

https://hg.mozilla.org/build/buildbot-configs/rev/9b5a483c09d6

Comment 18

[Chris Cooper [:coop] (he/him)]

Comment on attachment 660137 [details] [diff] [review]
Pass -a option when blocklist_update_set_approval is set (buildbotcustom)

https://hg.mozilla.org/build/buildbotcustom/rev/edbc4a9f7b5d

Comment 19

[Kim Moir [:kmoir] ET]

In production.