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

RESOLVED FIXED

Status

Release Engineering
General Automation
P2
major
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Callek, Assigned: coop)

Tracking

other
All
Windows 7
Dependency tree / graph

Firefox Tracking Flags

(firefox15-, firefox16+ fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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)
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
Blocks: 637444

Comment 2

5 years ago
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.
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
(Reporter)

Comment 3

5 years ago
(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.
(Reporter)

Comment 4

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

hg -R blocklist push -e ssh -l Callek@gmail.com -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 Callek@gmail.com -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.
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

5 years ago
We discussed this today in crash-kill, a respin of 15.0 won't be necessary given a review of recent blocklist additions.
tracking-firefox15: ? → -
tracking-firefox16: ? → +

Comment 7

5 years ago
catlee - who's in the best position to look at this for the next release? Thanks!
Assignee: nobody → catlee
coop touched the blocklist stuff last
Assignee: catlee → coop
(Assignee)

Comment 9

5 years ago
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
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 :-)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Priority: -- → P2
(Assignee)

Comment 11

5 years ago
Created attachment 660135 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval
Attachment #660135 - Flags: review?(bugspam.Callek)
(Assignee)

Comment 12

5 years ago
Created attachment 660136 [details] [diff] [review]
Set blocklist_update approval by default (buildbot-configs)
Attachment #660136 - Flags: review?(bugspam.Callek)
(Assignee)

Comment 13

5 years ago
Created attachment 660137 [details] [diff] [review]
Pass -a option when blocklist_update_set_approval is set (buildbotcustom)
Attachment #660137 - Flags: review?(bugspam.Callek)
(Assignee)

Comment 14

5 years ago
Created attachment 660138 [details] [diff] [review]
Add distinct -a option for setting blocklist update approval
Attachment #660135 - Attachment is obsolete: true
Attachment #660135 - Flags: review?(bugspam.Callek)
Attachment #660138 - Flags: review?(bugspam.Callek)
(Reporter)

Comment 15

5 years ago
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
Attachment #660138 - Flags: review?(bugspam.Callek) → review+
(Reporter)

Updated

5 years ago
Attachment #660137 - Flags: review?(bugspam.Callek) → review+
(Reporter)

Updated

5 years ago
Attachment #660136 - Flags: review?(bugspam.Callek) → review+
(Assignee)

Comment 16

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

https://hg.mozilla.org/build/tools/rev/c364b05afc33
Attachment #660138 - Flags: checked-in+
(Assignee)

Comment 17

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

https://hg.mozilla.org/build/buildbot-configs/rev/9b5a483c09d6
Attachment #660136 - Flags: checked-in+
(Assignee)

Comment 18

5 years ago
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
Attachment #660137 - Flags: checked-in+
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
status-firefox16: --- → fixed

Comment 19

5 years ago
In production.
Depends on: 791492
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.