| opendevreview | Abhishek Kekane proposed openstack/glance master: Move member policy checks to API layer https://review.opendev.org/c/openstack/glance/+/802996 | 05:54 |
|---|---|---|
| opendevreview | Abhishek Kekane proposed openstack/glance master: Cache API endpoints https://review.opendev.org/c/openstack/glance/+/792022 | 06:18 |
| opendevreview | Abhishek Kekane proposed openstack/python-glanceclient master: Add support for Cache API https://review.opendev.org/c/openstack/python-glanceclient/+/800172 | 09:59 |
| abhishekk | dansmith, please have look when you have free time, https://review.opendev.org/c/openstack/glance/+/803937 (important change) | 10:05 |
| opendevreview | Rajat Dhasmana proposed openstack/glance_store master: Add volume multiattach handling https://review.opendev.org/c/openstack/glance_store/+/786410 | 11:10 |
| *** lbragstad__ is now known as lbragstad | 13:35 | |
| opendevreview | Rajat Dhasmana proposed openstack/glance master: Fix: glance cinder functional test https://review.opendev.org/c/openstack/glance/+/804090 | 13:55 |
| whoami-rajat__ | abhishekk, dansmith hey, can you take a look at the change above ^, it is a one liner and breaking gate on my multiattach feature work | 14:05 |
| abhishekk | whoami-rajat__, ack, will look shortly | 14:06 |
| whoami-rajat__ | thanks abhishekk | 14:06 |
| *** whoami-rajat__ is now known as whoami-rajat | 14:06 | |
| dansmith | whoami-rajat: done | 14:09 |
| whoami-rajat | thanks dansmith | 14:10 |
| opendevreview | Dan Smith proposed openstack/glance master: Add check_is_image_mutable() legacy helper https://review.opendev.org/c/openstack/glance/+/803776 | 14:50 |
| opendevreview | Dan Smith proposed openstack/glance master: Check delete_image policy in the API https://review.opendev.org/c/openstack/glance/+/798073 | 14:50 |
| opendevreview | Dan Smith proposed openstack/glance master: POC: Check deactivate, reactivate policy in the API https://review.opendev.org/c/openstack/glance/+/798266 | 14:50 |
| opendevreview | Dan Smith proposed openstack/glance master: Check delete_image policy in the API https://review.opendev.org/c/openstack/glance/+/798073 | 15:11 |
| opendevreview | Dan Smith proposed openstack/glance master: POC: Check deactivate, reactivate policy in the API https://review.opendev.org/c/openstack/glance/+/798266 | 15:11 |
| dansmith | abhishekk: croelandt sorry I hadn't seen the comments on the delete patch when I pushed earlier | 15:11 |
| abhishekk | dansmith, no problem | 15:11 |
| abhishekk | also, if you could please have a look at some of my patches | 15:12 |
| dansmith | abhishekk: yep, I have them up | 15:12 |
| abhishekk | cool, thank you | 15:12 |
| dansmith | just trying to process my review email backlog in order | 15:13 |
| abhishekk | no worries, take your time | 15:14 |
| opendevreview | Merged openstack/glance master: Fix: glance cinder functional test https://review.opendev.org/c/openstack/glance/+/804090 | 15:31 |
| croelandt | https://review.opendev.org/q/I9356bceb914327f526da7b727fa58522ae18856e <- I don't have +2 rights on stable/*, but this looks like a nice & easy series of backports by jokke_ | 15:55 |
| abhishekk | dansmith, going to make changes in admin APIs as well | 17:28 |
| dansmith | abhishekk: I'm really not trying to be difficult | 17:28 |
| abhishekk | no it makes sense, I was going to do it as follow up later, but its better to do it now | 17:29 |
| dansmith | oh you were? I thought you just meant that we never need to check this because they're admin-only | 17:29 |
| abhishekk | yeah, I forgot to add comment there and under impression that I have a comment there | 17:30 |
| dansmith | hmm, okay | 17:31 |
| abhishekk | hopefully will be ready by tomorrow this time | 17:32 |
| dansmith | well, it seems really confusing to admins to me to say "this is admin by default, and there are other knobs to be fine-grained about objects, tags, etc, but the API won't behave as you expect if you let people do the top-level thing but not the low-level thing" | 17:32 |
| dansmith | if we have private namespaces, even if admin-only created, | 17:32 |
| dansmith | but a user can still call the modify or delete and probe for 404/403 then we're exposing stuff | 17:33 |
| abhishekk | +1 | 17:33 |
| dansmith | maybe I didn't make that clear in my comment, but on your L404, for example, even if I can never run update() on a namespace, I can *try* and if I get 404, it really doesn't exist, but if I get 403, I know it exists, yeah? | 17:34 |
| abhishekk | yes | 17:35 |
| opendevreview | Merged openstack/glance master: Refactor gateway auth layer for metadef APIs https://review.opendev.org/c/openstack/glance/+/799632 | 17:48 |
| abhishekk | \o/ first of my patch in refactoring | 17:49 |
| opendevreview | Ade Lee proposed openstack/glance master: DNM: Add fips check jobs https://review.opendev.org/c/openstack/glance/+/790536 | 18:15 |
| ade_lee | rosmaita, ping | 18:22 |
| rosmaita | ade_lee: hello | 18:23 |
| ade_lee | rosmaita, actually - I was just thinking that what I was going to ask about is a slightly longer conversation than we could have on irc. | 18:24 |
| ade_lee | I might just post this to you guys mailing list instead | 18:24 |
| rosmaita | ok, sounds good | 18:24 |
| ade_lee | whats the mailing list for glance? | 18:25 |
| opendevreview | Merged openstack/glance master: Add check_is_image_mutable() legacy helper https://review.opendev.org/c/openstack/glance/+/803776 | 18:25 |
| rosmaita | openstack-discuss, but with [glance] in subject line | 18:25 |
| ade_lee | yeah ok -- I'll post something up and then we can discuss further. | 18:25 |
| rosmaita | thanks! | 18:26 |
| opendevreview | Merged openstack/glance master: Check delete_image policy in the API https://review.opendev.org/c/openstack/glance/+/798073 | 18:46 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef namespace policy checks in the API https://review.opendev.org/c/openstack/glance/+/799633 | 18:51 |
| abhishekk | dansmith, ^^, working on others | 18:52 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef object policy checks in the API https://review.opendev.org/c/openstack/glance/+/799634 | 19:18 |
| dansmith | abhishekk: you're going to hate me | 19:35 |
| abhishekk | no | 19:35 |
| abhishekk | but you might, I think I was right to show forbidden in delete and update call | 19:36 |
| abhishekk | reason is; | 19:36 |
| abhishekk | get call is open to all | 19:36 |
| abhishekk | which means user can see public namespaces in the list or show call | 19:37 |
| abhishekk | and now when he tries to update or delete anything from that list, he will get not found | 19:37 |
| dansmith | right, you need to do what I did, which is check to see if the user can see the namespace to decide 403/404 right? | 19:37 |
| abhishekk | :D | 19:38 |
| dansmith | left comments, but it seems like the first list_metadef check is inconsistent with the rest | 19:39 |
| dansmith | the others look good if you're going to keep the "inner" checks I think, like doing the checks upfront in create, that's ++ | 19:39 |
| dansmith | but you are doing the list_ check a bunch of times, always the same result, and throwing away the result with just a debug log, and a debug log that we normally do before we return a 403 | 19:40 |
| dansmith | at first, I thought you meant to do that check and do something, but then I saw your comment about ignoring the result | 19:40 |
| dansmith | it's late there, so if you want to pick this up tomorrow morning, we can jump on a voice chat or something too | 19:41 |
| abhishekk | no worries, I think i will stick with the old behavior and then we can enhance it later | 19:43 |
| dansmith | I think I have lost track of what the old behavior is.. keep the list_metadef_resource_types check and use that to return an empty list? | 19:44 |
| opendevreview | Merged openstack/glance stable/wallaby: Revert "Remove all usage of keystoneclient" https://review.opendev.org/c/openstack/glance/+/800101 | 19:44 |
| *** lbragstad_ is now known as lbragstad | 19:45 | |
| abhishekk | dansmith, I think I need to check as well, but I guess you are stating it correct | 19:46 |
| dansmith | okay I think that if you make that one change, then this patch is otherwise fine (aside from the 403/404 thing) because you're still checking and enforcing all the knobs in the other calls, AFAICT | 19:47 |
| abhishekk | ack | 19:49 |
| abhishekk | will confirm the old behavior and the sign out for the day | 19:49 |
| dansmith | okay, sorry again | 19:49 |
| abhishekk | no worries | 19:50 |
| dansmith | tomorrow as soon as I'm on we should sync up, validate this and get it merged to stop the bleeding | 19:50 |
| abhishekk | :D | 19:50 |
| abhishekk | I guess members patch also I need 403/404 thing | 19:51 |
| dansmith | probably | 19:55 |
| abhishekk | old behavior: if I disable list_resource_types then it is giving me 403 response | 19:55 |
| abhishekk | HTTP 403 Forbidden: You are not authorized to complete list_metadef_resource_types action. | 19:55 |
| dansmith | yeah, so, per my most recent comment, | 19:57 |
| dansmith | I was thinking this was per-namespace, but it's not, | 19:57 |
| dansmith | so you just need to check this once before the for loop right? | 19:57 |
| abhishekk | yes | 19:57 |
| abhishekk | and I think will persist with old behavior | 19:58 |
| dansmith | and I guess if the old behavior is to just 403, then keeping that is fine, although it was probably not intentional and just a result of the proxy layer stuff | 19:58 |
| abhishekk | ++ that's why I opted to make it correct | 19:59 |
| dansmith | yeah | 19:59 |
| dansmith | well, I'm fine with making it correct too, it just seems like we only need to check it once :) | 19:59 |
| abhishekk | but we probably need a reno to mention old vs new behavior, so better to do it later | 19:59 |
| dansmith | yeah | 19:59 |
| dansmith | sounds good | 19:59 |
| abhishekk | So if time permits I will do it later or next cycle | 20:00 |
| * dansmith nods | 20:00 | |
| abhishekk | me signing out for the day | 21:01 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: Move metadef namespace policy checks in the API https://review.opendev.org/c/openstack/glance/+/799633 | 21:58 |
| abhishekk | dansmith, ^^^ when you have time, will work on others tomorrow | 21:59 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!