| opendevreview | Cyril Roelandt proposed openstack/glance_store stable/wallaby: Add cinder's new attachment support https://review.opendev.org/c/openstack/glance_store/+/805926 | 02:27 |
|---|---|---|
| opendevreview | Cyril Roelandt proposed openstack/glance_store stable/wallaby: Glance cinder nfs: Block creating qcow2 volumes https://review.opendev.org/c/openstack/glance_store/+/805927 | 02:27 |
| opendevreview | Merged openstack/glance master: Check policies for image import operation in API https://review.opendev.org/c/openstack/glance/+/804590 | 06:20 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef property policy checks in the API https://review.opendev.org/c/openstack/glance/+/799635 | 06:32 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef tag policy checks in the API https://review.opendev.org/c/openstack/glance/+/799636 | 06:32 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Check policies for Image Tags in API https://review.opendev.org/c/openstack/glance/+/804588 | 06:37 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Check policies for image tasks information in API https://review.opendev.org/c/openstack/glance/+/805590 | 06:37 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Check policies for Image Cache in API https://review.opendev.org/c/openstack/glance/+/805797 | 06:37 |
| *** akekane_ is now known as abhishekk | 08:19 | |
| opendevreview | Erno Kuvaja proposed openstack/python-glanceclient master: Add support for Cache API https://review.opendev.org/c/openstack/python-glanceclient/+/800172 | 08:37 |
| opendevreview | Rajat Dhasmana proposed openstack/glance stable/wallaby: Cinder Store: Update legacy images tests https://review.opendev.org/c/openstack/glance/+/805967 | 08:38 |
| opendevreview | Erno Kuvaja proposed openstack/glance master: Cache API endpoints https://review.opendev.org/c/openstack/glance/+/792022 | 08:39 |
| opendevreview | Rajat Dhasmana proposed openstack/glance master: Replace FakeObject with mock.MagicMock https://review.opendev.org/c/openstack/glance/+/805974 | 09:05 |
| opendevreview | Erno Kuvaja proposed openstack/glance master: Cache API endpoints https://review.opendev.org/c/openstack/glance/+/792022 | 09:14 |
| opendevreview | Merged openstack/python-glanceclient master: setup.cfg: Replace dashes with underscores https://review.opendev.org/c/openstack/python-glanceclient/+/789757 | 09:22 |
| opendevreview | Merged openstack/glance master: Get rid of deprecated xml.etree.cElementTree https://review.opendev.org/c/openstack/glance/+/802146 | 11:18 |
| opendevreview | Merged openstack/glance master: tests: Remove use of 'oslo_db.sqlalchemy.test_base' https://review.opendev.org/c/openstack/glance/+/802762 | 11:30 |
| *** lbragstad_ is now known as lbragstad | 14:39 | |
| *** abhishekk is now known as akekane|home | 15:08 | |
| *** akekane|home is now known as abhishekk | 15:08 | |
| opendevreview | Rajat Dhasmana proposed openstack/glance stable/wallaby: Cinder Store: Update legacy images tests https://review.opendev.org/c/openstack/glance/+/805967 | 15:08 |
| dansmith | lbragstad: replied here, which I think is what abhishekk would say: https://review.opendev.org/c/openstack/glance/+/804588 | 15:11 |
| abhishekk | ++ | 15:11 |
| lbragstad | dansmith abhishekk ack - thanks for following up | 15:16 |
| dansmith | abhishekk: question for you though: https://review.opendev.org/c/openstack/glance/+/804588 | 15:17 |
| abhishekk | added on patch ? | 15:17 |
| dansmith | just now | 15:17 |
| abhishekk | ack | 15:17 |
| abhishekk | In legacy behavior it is enforcing both policies, get_image and modidy_image | 15:18 |
| abhishekk | hmm, makes sense | 15:19 |
| dansmith | where? | 15:20 |
| dansmith | I didn't see it in auth or policy | 15:20 |
| abhishekk | line no #51 will enforce get_image and image.save will call modify_image I think | 15:21 |
| dansmith | oh I see you just mean because of the implicit db check | 15:22 |
| dansmith | but same for image, yet we're not doing that double check | 15:22 |
| abhishekk | yeah | 15:23 |
| dansmith | today if someone wants to grant someone tag ability, they have to grant them get and modify, | 15:23 |
| dansmith | after we fix that by making the db not override our checks, | 15:23 |
| dansmith | that person would be granted the same amount of privilege, but it will be more than they need, so nothing will break | 15:23 |
| dansmith | yet an operator *could* reduce their privilege after the change | 15:23 |
| dansmith | so I don't think that we're breaking anything by enforcing the policy we want here | 15:23 |
| abhishekk | yep, make sense | 15:24 |
| dansmith | I think the test will be the same since you're asserting the 404 behavior which we should still get | 15:24 |
| abhishekk | yes | 15:25 |
| abhishekk | dansmith, apart from current patches we will need a releasenote patch | 15:26 |
| dansmith | for this specific change? | 15:27 |
| abhishekk | no, no for all work we have done | 15:28 |
| dansmith | ...for all the policy work | 15:28 |
| dansmith | are you thinking we need to be super detailed about that or just say "we've moved to policy enforcement in the API so some details may have changed? | 15:29 |
| abhishekk | no, small one with additional note about legacy behavior is maintained if RBAC is not enabled | 15:30 |
| dansmith | okay, I think that I've also learned as part of this that we're not as close on rbac as I thought we were, because of all the other checks, | 15:31 |
| dansmith | so maybe just one that says "We're in the middle of a policy refactor. Some things are more flexible than they were, especially if RBAC is enabled, but not everything is possible that should be. And if rbac is disabled we're largely still enforcing the legacy admin-or-owner on most everything." ? | 15:32 |
| abhishekk | sounds good | 15:33 |
| dansmith | I'm assuming you're telling me this so I'll write it? :) | 15:34 |
| abhishekk | :D | 15:35 |
| abhishekk | I need to rebase all patches on image tags and then secure-rbac rebasing on metadef patches so that croelandt can look at those before going on leave | 15:36 |
| opendevreview | Dan Smith proposed openstack/glance master: Add release note about policy-refactor https://review.opendev.org/c/openstack/glance/+/806017 | 15:39 |
| dansmith | abhishekk: something like that? ^ | 15:39 |
| abhishekk | superdan | 15:39 |
| dansmith | for writing some words? more like clark dan. | 15:41 |
| abhishekk | nope, for perfection | 15:41 |
| abhishekk | looks perfect | 15:42 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Check policies for Image Tags in API https://review.opendev.org/c/openstack/glance/+/804588 | 15:52 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Check policies for image tasks information in API https://review.opendev.org/c/openstack/glance/+/805590 | 15:52 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Check policies for Image Cache in API https://review.opendev.org/c/openstack/glance/+/805797 | 15:52 |
| * abhishekk going for dinner | 15:52 | |
| abhishekk | are we going to give another crack at remaining reviews today | 17:33 |
| croelandt | I think I reviewed all the policy refactoring patches (except one) | 17:36 |
| croelandt | I can do another pass in a few hours before going to bed | 17:37 |
| abhishekk | ack | 17:40 |
| abhishekk | I will keep ready rbac patches for you by tomorrow | 17:41 |
| croelandt | I see 11 of them with a conflict right now :/ | 17:48 |
| abhishekk | ignore glance-tempest-plugin at the moment | 17:50 |
| abhishekk | I have 5 rebased in local just waiting for metadef policy related stuff to get merged | 17:51 |
| dansmith | abhishekk: I thought you were going to stack them all remaining so we could get them in one order? | 17:51 |
| abhishekk | image related are stacked | 17:52 |
| dansmith | okay, and lbragstad had a question on one but I see you answered | 17:52 |
| abhishekk | are you talking about metadef RBAC ? | 17:52 |
| dansmith | no | 17:52 |
| abhishekk | yes | 17:53 |
| dansmith | croelandt: you were +2 on this before, abhi removed one duplicate check for me, but probably an easy re-ack: https://review.opendev.org/c/openstack/glance/+/804588 | 17:56 |
| abhishekk | lbragstad, around ? | 17:56 |
| dansmith | abhishekk: the cache image policy refactor is going to conflict with the cache api patch itself | 18:06 |
| dansmith | I assume you want to merge the refactor now in case the api patch doesn't make it or something? | 18:07 |
| abhishekk | correct | 18:07 |
| dansmith | okay | 18:07 |
| lbragstad | abhishekk o/ | 18:32 |
| abhishekk | lbragstad, In glance-tempest-plugin I need to create namespace for two different projects | 18:33 |
| abhishekk | So can I use DynamicCredentialProvider to create two diffetent clients ? | 18:33 |
| lbragstad | abhishekk i think gmann baked that into tempest | 18:35 |
| lbragstad | so - you can use the credentials class attribute to have tempest create the clients for you | 18:35 |
| abhishekk | ok, need to look out for that as well | 18:36 |
| lbragstad | let me see if i can dig up the documentation now | 18:42 |
| lbragstad | https://docs.openstack.org/tempest/latest/keystone_scopes_and_roles_support.html | 18:43 |
| lbragstad | https://docs.openstack.org/tempest/latest/keystone_scopes_and_roles_support.html#project-scoped-personas | 18:43 |
| croelandt | dansmith: I'm not the biggest fan of gerrit, but being able to compare 2 specifc patchsets is a fantastic feature | 18:44 |
| dansmith | yup | 18:44 |
| lbragstad | abhishekk i think you could use that to wire up the clients for different projects so you can create different namespaces in each | 18:45 |
| abhishekk | lbragstad, ack, looking | 18:46 |
| lbragstad | https://paste.opendev.org/show/808319/ is a really rough example | 18:51 |
| abhishekk | looking | 18:51 |
| lbragstad | if you want two different project clients | 18:51 |
| lbragstad | to test tenancy | 18:51 |
| abhishekk | yes | 18:52 |
| abhishekk | this could be helpful | 18:52 |
| *** timburke_ is now known as timburke | 20:27 | |
| abhishekk | dansmith, we still have two patches remaining for metadef policies, when you get time please have a look | 20:37 |
| *** timburke_ is now known as timburke | 20:55 | |
| opendevreview | Abhishek Kekane proposed openstack/glance-tempest-plugin master: WIP Add protection testing for metadef namespaces https://review.opendev.org/c/openstack/glance-tempest-plugin/+/800902 | 20:55 |
| abhishekk | lbragstad, just wip, I guess this can be refactored aw well | 20:56 |
| abhishekk | Please have a look when you get time | 20:56 |
| dansmith | abhishekk: I know, I'm trying! | 21:03 |
| abhishekk | sorry for pushing so much | 21:03 |
| * dansmith cowers from abhishekk's whip | 21:04 | |
| abhishekk | lol | 21:04 |
| abhishekk | i definitely need to work on my looks :P | 21:05 |
| dansmith | lbragstad: https://review.opendev.org/c/openstack/glance/+/799635/19..21/glance/api/v2/metadef_properties.py | 21:21 |
| dansmith | lbragstad: did I misunderstand your intent on the previous set? | 21:21 |
| lbragstad | dansmith no - i dont' think so | 21:22 |
| lbragstad | i was concerned about running the same check N times when it wouldn't change | 21:22 |
| dansmith | yeah, same, okay | 21:22 |
| dansmith | abhishekk: was that a misunderstanding or did you remove it entirely for some other reason? | 21:23 |
| abhishekk | dansmith, I remove it entirely | 21:23 |
| abhishekk | earlier we were checking each property in a loop with .check('get_metadef_property') whereas now making single check outside loop does not sounds correct | 21:25 |
| dansmith | abhishekk: why? The way you have it, they can't show the individual things but they can see them in index right? that's not right | 21:26 |
| dansmith | like, I can do a list and see them all, but if I try to show one of those directly, I'll get a 403 | 21:26 |
| abhishekk | show is different call right? | 21:26 |
| abhishekk | for example namespace property index call returns 5 properties for namespace A | 21:27 |
| dansmith | yes, but aren't we including the full property in the index? | 21:27 |
| abhishekk | if I do a get_property check then it will return empty list to me | 21:28 |
| abhishekk | because it will fail for all, as it belongs to one namespace only | 21:29 |
| abhishekk | or may be I am confused now :/ | 21:29 |
| dansmith | abhishekk: L120 will cause show to return 403 if I'm not allowed, I won't see an empty list right? | 21:30 |
| dansmith | so if I'm not allowed to see them, I would expect the index to also not show them to me (i.e. advertise that I could show them, which I can't) | 21:30 |
| abhishekk | just a minute | 21:30 |
| dansmith | can I just remind everyone that I hate the metadef api? kthx. | 21:31 |
| abhishekk | dansmith, ack | 21:33 |
| abhishekk | these metadef policies are useless | 21:33 |
| abhishekk | But are we not changing behavior here? | 21:34 |
| abhishekk | earlier in index api call we were not checking for individual checks | 21:34 |
| abhishekk | in case of image we are excluding that from the list | 21:35 |
| abhishekk | but now in case of metadef we are going to raise 403 or just catch that and return empty list to user? | 21:35 |
| dansmith | idfk, I'm sick of caring about it | 21:40 |
| dansmith | you're saying that nowhere in policy or auth are we currently checking get_ policy yeah? | 21:41 |
| abhishekk | for index call, no | 21:42 |
| dansmith | aight | 21:42 |
| abhishekk | my brain will explode now | 21:47 |
| * dansmith schedules an extra beer for this evening | 21:47 | |
| * abhishekk cheers | 21:51 | |
| opendevreview | Abhishek Kekane proposed openstack/glance-tempest-plugin master: WIP Add protection testing for metadef namespaces https://review.opendev.org/c/openstack/glance-tempest-plugin/+/800902 | 22:02 |
| * abhishekk signing out for the day | 22:03 | |
| dansmith | you mean signing out for yesterday | 22:03 |
| abhishekk | :D | 22:04 |
| abhishekk | need to add recheck on tasks info patch, tempest failed on it after 3 hours :/ | 22:06 |
| dansmith | ugh | 22:07 |
| dansmith | I will try to check on those once more before my eod | 22:07 |
| abhishekk | cool, thank you | 22:08 |
| gmann | abhishekk: yes, as lbragstad mentioned. you can request different type of token clients within tests - https://github.com/openstack/tempest/blob/master/tempest/lib/common/cred_provider.py#L51-L107 | 22:38 |
| abhishekk | gmann, ack | 22:39 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!