| *** evrardjp has quit IRC | 05:33 | |
| *** evrardjp has joined #openstack-placement | 05:33 | |
| *** martinkennelly has joined #openstack-placement | 09:26 | |
| *** sean-k-mooney1 has joined #openstack-placement | 09:42 | |
| *** sean-k-mooney has quit IRC | 09:43 | |
| *** zzzeek has quit IRC | 09:45 | |
| *** zzzeek has joined #openstack-placement | 09:46 | |
| *** zzzeek has quit IRC | 10:22 | |
| *** zzzeek has joined #openstack-placement | 10:25 | |
| *** cdent has joined #openstack-placement | 11:23 | |
| *** sean-k-mooney1 is now known as sean-k-mooney | 11:36 | |
| *** artom has joined #openstack-placement | 12:54 | |
| *** artom has quit IRC | 14:37 | |
| *** artom has joined #openstack-placement | 14:37 | |
| *** janice61 has quit IRC | 15:02 | |
| *** martinkennelly has quit IRC | 15:35 | |
| *** janice61 has joined #openstack-placement | 16:35 | |
| openstackgerrit | Lance Bragstad proposed openstack/placement master: WIP add protection testing for aggregates https://review.opendev.org/762557 | 17:52 |
|---|---|---|
| lbragstad | cdent quick question on the approach here ^ | 18:05 |
| cdent | oh hey hi | 18:05 |
| lbragstad | i was trying to figure out why https://review.opendev.org/#/c/762557/1/placement/tests/functional/gabbits/aggregate-system-admin-policy.yaml,unified@9 didn't work | 18:05 |
| lbragstad | and then i found this - https://opendev.org/openstack/placement/src/branch/master/placement/auth.py#L49-L52 | 18:06 |
| lbragstad | which is the default auth strategy for the APIFixture | 18:06 |
| lbragstad | and that makes sense | 18:06 |
| lbragstad | i took a stab at trying to use a new fixture https://review.opendev.org/#/c/762557/1/placement/tests/functional/fixtures/gabbits.py@758 | 18:06 |
| lbragstad | but that's going to require keystone, unless we mock it some how | 18:07 |
| lbragstad | i don't see a lot of mocks anywhere in the placements test, so i'm wondering what placement folks would do to fix this | 18:07 |
| cdent | The general rule of thumb was “if you think you need a mock, you probably want a unit test” or “if gabbi can’t do what you want, see if skipping the api layer will do it” | 18:09 |
| cdent | in this case you might want the latter | 18:09 |
| cdent | if I understand the existing test correctly you’re trying to make sure that “policy works” | 18:10 |
| cdent | not that keystone or the http requests do the right stuff | 18:10 |
| lbragstad | yeah - exactly | 18:11 |
| lbragstad | and to do that with gabbi i need to set the request headers to simulate what keystonemiddleware would set | 18:11 |
| lbragstad | o | 18:12 |
| lbragstad | i'm really testing these - https://opendev.org/openstack/placement/src/branch/master/placement/handlers/aggregate.py#L93 | 18:12 |
| cdent | what _might_ work, if you really want to do this with gabbi, is to create a non-keystone auth middleware thing like noauth2 that does the header setting for you | 18:12 |
| lbragstad | ok - i thought about that | 18:12 |
| lbragstad | and it could be exposed via a configuration option? | 18:13 |
| lbragstad | or just wired up internally? | 18:13 |
| cdent | you could also call get_aggregates directly with a pre-builed req object, in a functional test that sets the policies up front. one sec | 18:13 |
| cdent | if you grep for ‘webob.Request.blank’ in the tests that might give you some ideas | 18:15 |
| cdent | if you went the different auth middleware route, I’d intiially wire it up internally and see where it got you | 18:16 |
| cdent | the more I look the more I think you could “cover” this with a unit test | 18:16 |
| lbragstad | ok | 18:17 |
| cdent | you can create a placement.context with whatever you like | 18:17 |
| lbragstad | since the policy stuff is encapsulated in the api layer - my knee jerk reaction was to test it as a functional test, but a unit test is fine with me | 18:17 |
| lbragstad | so - something like this perhaps https://opendev.org/openstack/placement/src/branch/master/placement/tests/unit/test_util.py#L80-L82 | 18:19 |
| cdent | yes, something like that. as far as I can tell all you really care about is whether `can` succeeds or fails depending on various inputs | 18:21 |
| lbragstad | correct | 18:21 |
| lbragstad | so long as the context object represents a realistic token | 18:22 |
| cdent | that’s up to you and how you create it when putting on the req object | 18:24 |
| lbragstad | cool - that'll work then | 18:24 |
| cdent | huzzah! | 18:25 |
| lbragstad | i appreciate the help - i'll work on an alternative to 762557 | 18:25 |
| cdent | I’m glad I’m able to still be a bit useful; much of this stuff has fallen out of my brain. | 18:26 |
| lbragstad | :) | 18:28 |
| *** amodi has quit IRC | 19:11 | |
| *** cdent has quit IRC | 19:45 | |
| *** artom has quit IRC | 21:15 | |
| openstackgerrit | Lance Bragstad proposed openstack/placement master: Implement secure RBAC for aggregates https://review.opendev.org/760235 | 22:32 |
| *** zzzeek has quit IRC | 22:34 | |
| *** zzzeek has joined #openstack-placement | 22:34 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!