In bug 1445672 we updated the wrong bouncer alias' when shipping Firefox 60.0b3. We changed the release channel alias' instead of the Beta ones. Catlee points out that we should actually verify bouncer alias changes when we make them. This will need to go in the ship phase, and depend on the bouncer alias' task

I can work on this. I suppose we can add a release-bouncer-aliases-verification in-tree task and have the same bouncerworker test? I'll sketch some idea soon.

mtabara> bhearsum: moving the conversationa about bouncer tests to avoid disrupting a.ki and jlore.nzo talking about in-tree fix 17:25:54 <mtabara> re: 1445684 17:26:00 <mtabara> should that be a separate task altogether? 17:26:08 <mtabara> like release-bouncer-aliases-verification 17:26:34 <mtabara> kind of like the same in-tree definition + a new behavior in bouncerscript 17:26:36 <aki> either we'd have to have hardcodes somewhere, or the test's task definition would also be passed in through the graph, no? 17:27:00 <aki> if we have tests in bouncerscript, it could have checks before it submits the aliases 17:27:04 <aki> and it would be the same task 17:27:14 <mtabara> yeah, that's what I was thinking too 17:27:23 <aki> :) gmta 17:27:24 <mtabara> not sure why we need a follow-up task, since we can do it all in the same task 17:27:36 <mtabara> but I don't get one thjing 17:27:44 <aki> ? 17:28:06 <mtabara> what aliases we'd check? all of them basically? making sure that the one we're adding (product X) is not among any of the other aliases except the current payload 17:28:07 <mtabara> ? 17:28:18 <aki> i think so 17:28:33 <aki> all of them. somehow we'd have to know that beta products shouldn't go in non-beta aliases 17:28:53 <aki> probably esr too 17:30:57 <mtabara> should we hardcode that list in bouncerscript constants or have it in-tree and pass it along bouncer aliases task definition? 17:31:46 <bhearsum> not sure if it's useful to us, but mbrandt pointed me at the tests that caught this issue: https://github.com/mozilla-services/go-bouncer/tree/master/tests/e2e 17:33:08 <aki> our current approach is hardcoding in bouncerscript or the puppet config template. if we passed it in from in-tree, i'd want the rules to be in a flat file rather than dynamically created, or we could make the test go false-green with the same bad in-tree patch that changes the task 17:33:51 <aki> i suppose it would still be subject to transforms, so we could still break the test in-tree 17:34:06 <aki> so i'm leaning towards puppet config template 17:34:14 <mtabara> hm, I like the puppet config template idea 17:34:26 <mtabara> the aliases are not changing as often, do they? 17:34:36 <aki> not sure 17:34:49 <mtabara> except "sha1" cases and dawn, not sure the set suffered any modifications 17:35:24 <aki> i'm leaning towards regexes. someone should probably stop me 17:35:34 <mtabara> bhearsum: ah, thanks for poiting me out to that. I'll glance at that and come up with some approach in the bug. so far I'm leaning towards puppet config templates 17:35:35 <mtabara> hahah 17:35:45 <mtabara> come on, it's regexes, what can go wrong? 17:35:52 <mtabara> (TM) 17:36:38 <aki> it could be something like "firefox-latest-ssl": "release", "firefox-latest-beta-ssl": "beta", and something on the other end makes sure that the product looks like a release or beta product 17:37:09 <aki> up to us how we behave if there's a new alias. we can either inspect it to see if it looks like esr or beta, or we can break 17:37:30 <aki> the downside of the former is the test may be wrong. the downside of the latter is manually fixing up the bouncer aliases 17:37:38 <aki> i suppose puppet patch and rerun 17:39:00 <aki> i suppose "firefox-latest-ssl": "Firefox-[\d.]+-SSL" isn't so bad 17:39:11 <aki> assuming that regex is right =\ 17:42:16 <aki> i wonder if `"latest-beta": "[\d.]+b\d+"` would work. we'd have to order the string replacement, and then compare the alias versus the string-replaced product.lower() 17:42:18 <mtabara> puppet patch + rerun or manual is far better than in-tree patch anyway 17:42:32 <mtabara> ino 17:42:37 <mtabara> imo New messages since you tabbed out 17:42:50 <aki> yeah

* add check for bouncer aliases job (via puppet config) * some simplifying in the tests

https://tools.taskcluster.net/groups/RVZZfhFbQNOIi2BM2DoOTA/tasks/VB7Bq0HfTrKzLsp1FV5ICQ/details was the initial bouncer aliases job that was green but wrongly updated the aliases in staging. It fails now in sanity check See more https://tools.taskcluster.net/groups/RVZZfhFbQNOIi2BM2DoOTA/tasks/WKW4OvfAQTuIa0D1pOnY0w/runs/0/logs/public%2Flogs%2Flive_backing.log automation >> human attention span. automation FTW!

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #4) > https://tools.taskcluster.net/groups/RVZZfhFbQNOIi2BM2DoOTA/tasks/ > VB7Bq0HfTrKzLsp1FV5ICQ/details was the initial bouncer aliases job that was > green but wrongly updated the aliases in staging. It fails now in sanity > check > See more > https://tools.taskcluster.net/groups/RVZZfhFbQNOIi2BM2DoOTA/tasks/ > WKW4OvfAQTuIa0D1pOnY0w/runs/0/logs/public%2Flogs%2Flive_backing.log > automation >> human attention span. automation FTW! \o/

Comment on attachment 8960808 [details] Bug 1445684 - add bouncer aliases preflight check regexes. https://reviewboard.mozilla.org/r/229536/#review235286 ::: modules/bouncer_scriptworker/manifests/settings.pp:107 (Diff revision 1) > submission => "${root}/lib/python3.5/site-packages/bouncerscript/data/bouncer_submission_task_schema.json", > aliases => "${root}/lib/python3.5/site-packages/bouncerscript/data/bouncer_aliases_task_schema.json", > }, > verbose => $verbose_logging, > bouncer_config => $_env_config['bouncer_instances'], > + aliases_regexes => $all_regexes We probably need to make sure this creates the file properly (with appropriate escaping). Thanks!

(In reply to Aki Sasaki [:aki] from comment #7) > Comment on attachment 8960808 [details] > Bug 1445684 - add bouncer aliases preflight check regexes. > > https://reviewboard.mozilla.org/r/229536/#review235286 > > ::: modules/bouncer_scriptworker/manifests/settings.pp:107 > (Diff revision 1) > > submission => "${root}/lib/python3.5/site-packages/bouncerscript/data/bouncer_submission_task_schema.json", > > aliases => "${root}/lib/python3.5/site-packages/bouncerscript/data/bouncer_aliases_task_schema.json", > > }, > > verbose => $verbose_logging, > > bouncer_config => $_env_config['bouncer_instances'], > > + aliases_regexes => $all_regexes > > We probably need to make sure this creates the file properly (with > appropriate escaping). > > Thanks! I initially added the strings as they were in the bouncerscript PR[1] but then I noticed puppet was double-escaping everything "\\\\d". So I switched back to the initial form of them, seems like `pp` files are doing these for us when they transition the content of the `script_config.json`. [1]: https://github.com/mozilla-releng/bouncerscript/pull/16/files#diff-3d6e28e1693286ec7fe31e724b9b9cf0R19

Works smoothly in puppet, I pinned the bouncer-dev to my environment and tested both the puppet patch + bouncerscript and work good. I'll try to successfully ship a staging release tomorrow to make sure things work properly and then I'll roll-out to production.

Awesome, thanks you! Let me know if you need a hand.

Comment on attachment 8960725 [details] Add preflight checks for bouncer aliases job Thank you for these checks. It's great to be safeguarded by them! I think we can make the overall config less error-prone. I left more details at: https://github.com/mozilla-releng/bouncerscript/pull/16#pullrequestreview-105654320

Comment on attachment 8960725 [details] Add preflight checks for bouncer aliases job https://github.com/mozilla-releng/bouncerscript/commit/4c78f3ff13d679b562efeeb707bbe29b7f20f4cd Rolled-out 1.2.1 to master branch.

Comment on attachment 8960808 [details] Bug 1445684 - add bouncer aliases preflight check regexes. Trimmed the regexes part and kept solely the version bump (1.2.1) https://hg.mozilla.org/build/puppet/rev/9dee0cf03e462b34c91382f51092cc86a9d98812

Rolled-out 1.2.1 to production and dev bouncer workers. This should increase our checks and security for upcoming bouncer aliases jobs. Closing this now.

https://taskcluster-artifacts.net/OgCFskaKSaGl5BwnJTfBBA/0/public/logs/live_backing.log https://taskcluster-artifacts.net/Pu2JDGByTLKihDXwwOK0eA/0/public/logs/live_backing.log