Closed Bug 1392522 Opened 2 years ago Closed 2 years ago
prevent staging infrastructure grab mozilla-beta as repo
59 bytes, text/x-review-board-request
59 bytes, text/x-review-board-request
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 instead of jamun project branches. 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). 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 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. : http://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/config.py : https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/project_branches.py : https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/project_branches.py#l193 : https://ship-it-dev.allizom.org/
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 instead of the fake ones we usually use in project branches. Fixing this bug via bug 1392522 comment 0 would also solve this problem. : http://hg.mozilla.org/build/tools/ : 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 8901797 [details] Bug 1392522 - prevent staging infrastructure grab mozilla-beta as repo remote: https://hg.mozilla.org/build/puppet/rev/e57fb2900edc709fd093e5e8abcbbed25c62d495 remote: https://hg.mozilla.org/build/puppet/rev/1fb890e981b433509a048cea1394f9f66519a483
Attachment #8901797 - Flags: checked-in+
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.