1275370 - make it harder to return <updates><addons></addons></updates> from balrog

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (backend), defect)

Description

[[e-mail] (:bhearsum)]

According to rhelmer, <updates><addons></addons></updates> instructions Firefox to disable _all_ system addons. Right now, an "empty" GMP blob (which SystemAddons uses) returns this response. We should probably return <updates></updates> for these cases instead, and only return the former if some special attribute is set in the blob.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Is this a regression from the GMP multifile landing ?

Comment 2

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #1)
> Is this a regression from the GMP multifile landing ?

Even though we don't use multifile for System Addons yet, it looks like it probably is. We used to check for inner contents before adding the <addons> bit, per https://github.com/mozilla/balrog/commit/93a6d3d5f30d9db4ccccb441e958953b6eb77c5e#diff-646fe03b04415266d51566958c0a2ed5L1.

Good catch!

Comment 3

[[e-mail] (:bhearsum)]

Varun is working on this in https://github.com/mozilla/balrog/pull/95

Comment 4

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum (:bhearsum) from comment #0)
> According to rhelmer, <updates><addons></addons></updates> instructions
> Firefox to disable _all_ system addons. Right now, an "empty" GMP blob
> (which SystemAddons uses) returns this response. We should probably return
> <updates></updates> for these cases instead, and only return the former if
> some special attribute is set in the blob.

Varun pointed out that this is not what http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-manager/SystemAddons.html says. According to it, <updates></updates> disables everything, and <updates><addons></addons></addons> does nothing. Can you clarify, Rob? I'm guessing that I got things mixed up when I first filed this bug.

Comment 5

[Robert Helmer [:rhelmer]]

(In reply to Ben Hearsum (:bhearsum) from comment #4)
> (In reply to Ben Hearsum (:bhearsum) from comment #0)
> > According to rhelmer, <updates><addons></addons></updates> instructions
> > Firefox to disable _all_ system addons. Right now, an "empty" GMP blob
> > (which SystemAddons uses) returns this response. We should probably return
> > <updates></updates> for these cases instead, and only return the former if
> > some special attribute is set in the blob.
> 
> Varun pointed out that this is not what
> http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-
> manager/SystemAddons.html says. According to it, <updates></updates>
> disables everything, and <updates><addons></addons></addons> does nothing.
> Can you clarify, Rob? I'm guessing that I got things mixed up when I first
> filed this bug.

Hm, can you point me to where in the doc it says this? I see in http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-manager/SystemAddons.html#disable-all-system-add-ons 

```
A response from AUS with an empty add-on set will disable all system add-ons:

<updates>
  <addons></addons>
</updates>

```

In the Rollout section http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-manager/SystemAddons.html#rollout there's an example of the empty response:


```
<updates></updates>

With the empty response, Firefox will not make any changes.
```

Comment 6

[[e-mail] (:bhearsum)]

(In reply to Robert Helmer [:rhelmer] from comment #5)
> (In reply to Ben Hearsum (:bhearsum) from comment #4)
> > (In reply to Ben Hearsum (:bhearsum) from comment #0)
> > > According to rhelmer, <updates><addons></addons></updates> instructions
> > > Firefox to disable _all_ system addons. Right now, an "empty" GMP blob
> > > (which SystemAddons uses) returns this response. We should probably return
> > > <updates></updates> for these cases instead, and only return the former if
> > > some special attribute is set in the blob.
> > 
> > Varun pointed out that this is not what
> > http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-
> > manager/SystemAddons.html says. According to it, <updates></updates>
> > disables everything, and <updates><addons></addons></addons> does nothing.
> > Can you clarify, Rob? I'm guessing that I got things mixed up when I first
> > filed this bug.
> 
> Hm, can you point me to where in the doc it says this? I see in
> http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-
> manager/SystemAddons.html#disable-all-system-add-ons 
> 
> ```
> A response from AUS with an empty add-on set will disable all system add-ons:
> 
> <updates>
>   <addons></addons>
> </updates>
> 
> ```
> 
> In the Rollout section
> http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-
> manager/SystemAddons.html#rollout there's an example of the empty response:
> 
> 
> ```
> <updates></updates>
> 
> With the empty response, Firefox will not make any changes.
> ```

I think we talked about this on IRC yesterday, and discovered that the "Update Behavior" section doesn't match the examples (http://gecko.readthedocs.io/en/latest/toolkit/mozapps/extensions/addon-manager/SystemAddons.html#update-behavior). It says: "If the <addons> tag is missing in the response, disable all system add-ons, including both the update and default sets."

Comment 7

[[github robot]]

Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/504dedf87722476246b4b6186cc7167b4468b8af
bug 1275370: Added blob for SystemAddons and fixed response for no updates (#95). r=bhearsum

Comment 8

[[e-mail] (:bhearsum)]

This is working fine in stage. It should go to production later this week or early next.

Comment 9

[[e-mail] (:bhearsum)]

This is in production, and I've asked that new System Addon releases use this blob instead of the GMP one.