Closed Bug 1392522 Opened 2 years ago Closed 2 years ago

prevent staging infrastructure grab mozilla-beta as repo

Categories

(Release Engineering :: Release Automation: Other, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mtabara, Assigned: rail)

Details

(Whiteboard: [releaseduty])

Attachments

(2 files)

We accidentally started staging releases off mozilla-beta several times. This is somewhat harmless as 

* staging release runner lies on bm83
* production release runner lie on bm85

Each of them has another set of credentials guarded by puppet.

But, if we start staging release based on mozilla-beta, we grab the production configs[1] instead of jamun project branches[2]. Hence, a bunch of stuff like balrog instance, beetmover buckets, bouncer and others are gonna point to production instance rather than the staging onces (currently staging releases are based on jamun[3]).

So the worse case scenario we try to push with staging credentials to production destinations and we fail. However, this may create a bit of confusion over in the emails and in the channel so it might be nice to guard ourselves in the future.

Solutions:
a) prevent ship-it to grab mozilla-beta as option for repository in Ship-it dev[4]
b) prevent staging release runner (bm83) to grab mozilla-beta the same way and fail fast
c) cleanest solution would be what Rail suggested, to "we can add a releaserunner sanity check and specify accepted branches in release-runner.yml"

Sounds like c) is the cleanest solution. Let's go with that.


[1]: http://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/config.py
[2]: https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/project_branches.py
[3]: https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/project_branches.py#l193
[4]: https://ship-it-dev.allizom.org/
Assignee: nobody → mtabara
Truth be told, another danger is that using the production configs means we're tagging and bumping the patcher configs off the real tools repo[1] instead of the fake ones we usually use in project branches. 

Fixing this bug via bug 1392522 comment 0 would also solve this problem.

[1]: http://hg.mozilla.org/build/tools/
[2]: https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/project_branches.py#l177
More on this for later context:

jlorenzo> the graph had the time to: repack, create partials, generate docker images, packageing the source and... submit that thing in bouncer https://tools.taskcluster.net/groups/3NLV9LrRSiyaS-zop5EdAw/tasks/3tglbGTrQr-AmXvkmk0beg/details 
09:49:25 <jlorenzo> that latter step is the only one that worries me 
09:51:37 <mtabara|focus> how the hell did that work
09:51:48 — mtabara|focus scratches his head and digs
09:52:49 <mtabara|focus> unless credentials are shared across staging/production bouncer which I doubt
09:53:51 <mtabara|focus> ah, right
09:53:58 <mtabara|focus> tuxedo credentials are coming from BB ...
09:56:42 <mtabara|focus> so given it used config.py instead of project_branches, it used the production bouncer 
09:57:37 — mtabara|focus dives in BB
09:59:42 <mtabara|focus> yep
09:59:46 <mtabara|focus> https://hg.mozilla.org/build/buildbotcustom/file/tip/process/release.py#l1848
10:00:17 <rail> :/
10:00:54 <mtabara|focus> how did we miss this?
10:01:07 <mtabara|focus> I mean, how did it work?
10:01:15 <rail> probably we have never gone that far
10:01:33 <mtabara|focus> bouncer submission is the *first* task that executes as its the fastest one
10:01:34 <mtabara|focus> but
10:01:54 <mtabara|focus> I think it's because we've always started staging releases for beta release X.bY before we actually shipped it 
10:02:13 <mtabara|focus> so ... I suppose when the real beta release arrived, it just did an overwritting there
10:03:02 <rail> the most important part is beetmover, and it should be fine, because it failed
10:03:02 <mtabara|focus> given that bouncer actually allows overwriting
10:03:10 <rail> just need to check the bouncer redirects
10:03:47 <mtabara|focus> yes
10:03:52 <rail> and balrog maybe
10:04:21 <rail> iirc it should rejects the staging URLs
10:05:29 <mtabara|focus> bouncer seems fine
10:05:30 <mtabara|focus> hm
10:06:03 <mtabara|focus> wait a second
10:06:18 <mtabara|focus> even if it succeded ... bouncer redirects are pointing to mirrors
10:06:41 <mtabara|focus> and mirrors couldn't have been overwritten because 1) beetmover failed anyway 2) artifacts were there already due to real 56.0b4-build5
10:06:53 <rail> yup
10:07:10 <rail> the bouncer submission was a noop I think
10:07:17 <rail> the same product, the same urls
10:08:02 <mtabara|focus> 1. bouncer should have complained in this or it does and our script silentily shuts up
10:08:25 <mtabara|focus> 2. the fact that BB doesnt have guarded logic for staging/production makes me want to get rid of it even faster
10:08:56 <rail> fix releaserunner! :)
10:09:24 <mtabara|focus> fail fast :)
I can take a look at this.
Assignee: mtabara → rail
Comment on attachment 8901796 [details]
Bug 1392522 - prevent staging infrastructure grab mozilla-beta as repo

https://reviewboard.mozilla.org/r/173230/#review178578

LGTM!

::: buildfarm/release/release-runner.py:92
(Diff revision 1)
> +    branch = release['branch']
> +    allowed_branches = releases_config[product]['allowed_branches']
> +    for pattern in allowed_branches:
> +        if re.match(pattern, branch):
> +            return
> +    raise RuntimeError("%s branch is not allowed: %s", branch, allowed_branches)

Minor: How about raising `ValueError`? That sounds closer than a generic RuntimeError
Attachment #8901796 - Flags: review?(jlorenzo) → review+
Comment on attachment 8901797 [details]
Bug 1392522 - prevent staging infrastructure grab mozilla-beta as repo

https://reviewboard.mozilla.org/r/173234/#review178588

::: modules/releaserunner/templates/release-runner.yml.erb:58
(Diff revision 1)
>      checks:
>        - long_revision
>        - l10n_changesets
>        - partial_updates
> +      - check_allowed_branches
> +    allowed_branches: <%= require "json"; JSON.generate(Array(@env_config["allowed_branches"])) %>

Nit: I think we can simplify this part with a ruby loop. What do you think?

```ruby
allowed_branches:
<%- @env_config["allowed_brances"].each do |branch| %>
  - <%= branch %>
<%- end %>
```
Comment on attachment 8901797 [details]
Bug 1392522 - prevent staging infrastructure grab mozilla-beta as repo

https://reviewboard.mozilla.org/r/173234/#review178590

Look great to me. Thank you for preventing us from footguns \o/
Attachment #8901797 - Flags: review?(jlorenzo) → review+
Comment on attachment 8901797 [details]
Bug 1392522 - prevent staging infrastructure grab mozilla-beta as repo

https://reviewboard.mozilla.org/r/173234/#review178596

::: modules/releaserunner/templates/release-runner.yml.erb:58
(Diff revision 1)
>      checks:
>        - long_revision
>        - l10n_changesets
>        - partial_updates
> +      - check_allowed_branches
> +    allowed_branches: <%= require "json"; JSON.generate(Array(@env_config["allowed_branches"])) %>

That was my first approach, but it looks a but noisy and relies on formatting. So I ended up with this one liner.
Comment on attachment 8901796 [details]
Bug 1392522 - prevent staging infrastructure grab mozilla-beta as repo

https://hg.mozilla.org/build/tools/rev/29217b3a253f167f99da5300927e8753aaf01ceb
Attachment #8901796 - Flags: checked-in+
Comment on attachment 8901796 [details]
Bug 1392522 - prevent staging infrastructure grab mozilla-beta as repo

Bustage fix: https://hg.mozilla.org/build/tools/rev/93a4ecc6954b436f4211e22567f84d75292e51f4. I misread the data structure.
Deployed on bm85/bm83
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.