| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef namespaces https://review.opendev.org/c/openstack/glance/+/798700 | 08:20 |
|---|---|---|
| opendevreview | Abhishek Kekane proposed openstack/glance master: Refactor gateway auth layer for member APIs https://review.opendev.org/c/openstack/glance/+/802525 | 08:41 |
| opendevreview | Abhishek Kekane proposed openstack/glance master: PoC Move member policy checks to API layer https://review.opendev.org/c/openstack/glance/+/802996 | 08:41 |
| opendevreview | Abhishek Kekane proposed openstack/glance stable/ussuri: DNM - Add housekeeping module and staging cleaner https://review.opendev.org/c/openstack/glance/+/785552 | 08:52 |
| opendevreview | Abhishek Kekane proposed openstack/glance stable/ussuri: DNM - Add housekeeping module and staging cleaner https://review.opendev.org/c/openstack/glance/+/785552 | 09:00 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef namespaces https://review.opendev.org/c/openstack/glance/+/798700 | 09:02 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef resource-types https://review.opendev.org/c/openstack/glance/+/799671 | 09:02 |
| opendevreview | Abhishek Kekane proposed openstack/glance stable/ussuri: DNM - Add housekeeping module and staging cleaner https://review.opendev.org/c/openstack/glance/+/785552 | 09:06 |
| opendevreview | Abhishek Kekane proposed openstack/glance stable/ussuri: DNM - Add housekeeping module and staging cleaner https://review.opendev.org/c/openstack/glance/+/785552 | 09:07 |
| opendevreview | Abhishek Kekane proposed openstack/glance stable/ussuri: Add housekeeping module and staging cleaner https://review.opendev.org/c/openstack/glance/+/785552 | 09:15 |
| akekane_ | croelandt, found out the reason of failure on stable/ussuri patch | 09:22 |
| akekane_ | commented on the patch | 09:22 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef objects https://review.opendev.org/c/openstack/glance/+/802054 | 11:27 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef properties https://review.opendev.org/c/openstack/glance/+/802055 | 11:27 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef tags https://review.opendev.org/c/openstack/glance/+/802056 | 11:27 |
| croelandt | abhishekk: I think it was fixed in b220030070adbea0bd253dfa390218d30b047154 | 14:02 |
| abhishekk | croelandt, lookung | 14:02 |
| croelandt | I'll backport it to ussuri | 14:03 |
| abhishekk | coool | 14:04 |
| abhishekk | kindly comment on the bug as well, https://bugs.launchpad.net/glance/+bug/1885825 | 14:05 |
| croelandt | yeah I'll mark it as fix released there | 14:05 |
| abhishekk | great, thank you | 14:07 |
| opendevreview | Cyril Roelandt proposed openstack/glance stable/ussuri: Make test-setup.sh compatible with mysql8 https://review.opendev.org/c/openstack/glance/+/803040 | 14:08 |
| croelandt | abhishekk: thanks for finding the issue :) | 14:10 |
| abhishekk | :D | 14:11 |
| -opendevstatus- NOTICE: There will be a brief outage of the Gerrit service on review.opendev.org starting at 15:00 UTC today as part of a routine project rename maintenance: http://lists.opendev.org/pipermail/service-announce/2021-July/000023.html | 14:11 | |
| abhishekk | dansmith, if you have some time (now or later) could you please verify the new RBAC defaults for members in our spreadsheet (I have tried to go as much as close as possible) | 14:32 |
| dansmith | abhishekk: second tab? | 14:33 |
| abhishekk | yes | 14:33 |
| dansmith | okay | 14:34 |
| abhishekk | thank you | 14:34 |
| dansmith | abhishekk: I'm a bit confused... based on the code, it looks like add_member should be ADMIN_OR_PROJECT_MEMBER, SHARED is checked outside the policy layer and probably should be | 14:44 |
| abhishekk | Yeah, so that check is while creating AUTH repo which we are going to remove, right ? | 14:44 |
| dansmith | shared is a glance architectural requirement for an image having members, so it _should_ be enforced in python | 14:45 |
| dansmith | yes, but we'll need to move it somewhere | 14:45 |
| abhishekk | SO are you suggesting to add this check additional API layer | 14:45 |
| dansmith | if it's in policy, an admin could remove the shared check from the rule and that would break things | 14:45 |
| dansmith | just check it in the API itself, yeah | 14:45 |
| abhishekk | Ok | 14:46 |
| opendevreview | Merged openstack/glance stable/ussuri: Make test-setup.sh compatible with mysql8 https://review.opendev.org/c/openstack/glance/+/803040 | 14:46 |
| dansmith | same goes for the rest of the delete and modify | 14:46 |
| abhishekk | So I guess, shared visibility will come out of every rule | 14:47 |
| dansmith | not sure if yeah | 14:47 |
| dansmith | s/not sure if// :) | 14:47 |
| abhishekk | :D | 14:47 |
| abhishekk | that is good for add_member and get_members I guess, what about others? | 14:49 |
| dansmith | yep, still looking at those | 14:50 |
| dansmith | abhishekk: currently delete is only admin or owner, *not* project member right? | 14:52 |
| abhishekk | Looking again | 14:52 |
| dansmith | I think if an admin wants to add "shared:visibility and project_id:member" to delete, they should be able to, but not by default based on the current behavior I think | 14:53 |
| abhishekk | its admin or project member | 14:53 |
| dansmith | if (self.image.owner == self.context.owner or self.context.is_admin) | 14:54 |
| dansmith | this is just owner or admin, no? | 14:54 |
| abhishekk | aah, I thought you are asking current default RBAC | 14:54 |
| abhishekk | Yes, I will remove shared visibility check from all rules and make it separate validation check at API layer | 14:55 |
| dansmith | no, I'm talking about the current regular behavior | 14:55 |
| dansmith | but the rbac is the same | 14:56 |
| abhishekk | yes, this condition you are pointing is stating this | 14:56 |
| abhishekk | but in API in delete call when you call get member it calls additional nested check | 14:57 |
| abhishekk | which I marked in red | 14:57 |
| dansmith | that's not used for remove, AFAICT | 14:58 |
| dansmith | it is for get and list (which makes no sense?) | 14:58 |
| abhishekk | :D | 14:58 |
| dansmith | right? | 14:58 |
| abhishekk | https://github.com/openstack/glance/blob/master/glance/api/v2/image_members.py#L79 | 14:58 |
| abhishekk | This _lookup_member is called from each API except add and get | 14:59 |
| dansmith | but then the actual remove won't let that happen will it? | 14:59 |
| abhishekk | So Based on the mutability check it adds different object to onion layer | 14:59 |
| dansmith | you won't get past here when the repo.remove is called: https://github.com/openstack/glance/blob/master/glance/api/authorization.py#L162 | 14:59 |
| dansmith | WAT | 15:00 |
| -opendevstatus- NOTICE: There will be a brief outage of the Gerrit service on review.opendev.org in the next few minutes as part of a routine project rename maintenance: http://lists.opendev.org/pipermail/service-announce/2021-July/000023.html | 15:00 | |
| abhishekk | This object, ImmutableMemberProxy | 15:01 |
| dansmith | right, so if we're a member, we won't get immutablememberproxy, right? and then we'll go on to call remove on ImageMemberProxy, and then fail the check I linked above right? | 15:01 |
| abhishekk | yes | 15:03 |
| abhishekk | You can add comments there | 15:05 |
| abhishekk | So image owner means any user from my project can initiate delete call, right? | 15:06 |
| dansmith | I guess, because owner is project | 15:07 |
| dansmith | but not any *member* as defined by the shared member list | 15:07 |
| abhishekk | right | 15:09 |
| dansmith | hmm, can I not "suggest" edits on a sheet? | 15:11 |
| abhishekk | I think you can | 15:13 |
| dansmith | I don't see how | 15:13 |
| dansmith | so I think the current rbac rules for add_member and delete_member are correct | 15:14 |
| dansmith | get_member and get_members should probably be ADMIN_OR_PROJECT_MEMBER_OR_SHARED_MEMBER | 15:15 |
| dansmith | where SHARED_MEMBER is %(project_id)s:member | 15:15 |
| abhishekk | ack | 15:16 |
| dansmith | actually reader I guess, so: | 15:16 |
| dansmith | "role:admin or (role:reader and project_id:%(project_id)s) or member:%(project_id)s" | 15:17 |
| dansmith | if you're a reader on the project that owns the image, then cool | 15:17 |
| dansmith | if admin, cool | 15:17 |
| dansmith | if shared with you as a member, then cool | 15:17 |
| dansmith | else, not cool | 15:17 |
| abhishekk | cool :D | 15:17 |
| dansmith | :P | 15:17 |
| dansmith | so if I share an image with you, you can modify the member list? | 15:19 |
| dansmith | I'm not sure I understand this: | 15:20 |
| dansmith | self.context.owner == image_member.member_id | 15:20 |
| dansmith | maybe members can modify their own memberness? not sure what they can change, but maybe that makes sense | 15:21 |
| dansmith | they can just change the status of the share I guess? | 15:22 |
| abhishekk | yes | 15:22 |
| abhishekk | accept or reject | 15:22 |
| dansmith | okay | 15:22 |
| abhishekk | I think I will be ready with patch by Monday eod for review | 15:23 |
| dansmith | okay, but it would be good if we could merge some too :) | 15:23 |
| dansmith | maybe croelandt is too caught up in celebrating france beating us in basketball... | 15:24 |
| abhishekk | he has to look at some down stream work as well | 15:24 |
| dansmith | *gasp* | 15:25 |
| abhishekk | Once lance is back things will start picking up speed | 15:25 |
| abhishekk | I think we need to follow distributed policy | 15:25 |
| abhishekk | I need to take close look at yours and you should be at mine, then those will be ready for others to have a look | 15:26 |
| dansmith | I think I'm +2 on the bottom few of your set | 15:27 |
| abhishekk | dansmith, on delete policy I am doubtful, I think I saw some functional test scenario where image member (with whom ) image is shared is deleting the image | 15:29 |
| dansmith | deleting the image itself? | 15:29 |
| abhishekk | no member ship | 15:29 |
| abhishekk | delete-member | 15:29 |
| dansmith | okay I don't see how that could work based on the code | 15:30 |
| abhishekk | Nah, # Image Member Cannot delete Image membership (this is the scenario, which means he is not) | 15:32 |
| dansmith | meaning the test you were thinking about is asserting that the member can *not* delete their own membership? | 15:32 |
| abhishekk | yeah | 15:32 |
| dansmith | cool | 15:32 |
| abhishekk | test_image_member_lifecycle | 15:33 |
| abhishekk | it has around 25+ scenarios covered :D | 15:34 |
| abhishekk | So just couple of minutes more | 15:34 |
| abhishekk | I think on testing part | 15:34 |
| abhishekk | we have job to check existing is working | 15:35 |
| abhishekk | We added job to cover it is working with RBAC enabled as well | 15:35 |
| abhishekk | AND we are adding new tests to cover policy enforcement at API layer | 15:36 |
| abhishekk | I think it is enough to ensure that we are going in right direction | 15:36 |
| dansmith | cool | 15:37 |
| abhishekk | great | 15:40 |
| abhishekk | modify_member new rule is ok ? | 15:40 |
| dansmith | what does "not project member" mean? | 15:42 |
| dansmith | presumably project member (i.e. owner) can modify membership right? | 15:42 |
| dansmith | or .. maybe they use add/delete member and modify is just for the share-ee | 15:42 |
| dansmith | ? | 15:42 |
| abhishekk | means the project member should not accept the member ship | 15:42 |
| abhishekk | s/member ship/membership | 15:42 |
| abhishekk | dansmith, right | 15:43 |
| dansmith | yeah, okay, makes sense | 15:44 |
| abhishekk | something I got right :D | 15:44 |
| dansmith | can't you just do ADMIN_OR_SHARED_MEMBER ? | 15:44 |
| dansmith | I mean, if they aren't the member, then image.member==None | 15:45 |
| dansmith | so I think "role:admin or member:%(project_id)s" will do everything you need | 15:45 |
| abhishekk | Yeah, I think so as well | 15:45 |
| abhishekk | will test accordingly | 15:46 |
| dansmith | cool | 15:46 |
| abhishekk | thank you, it took a lot time | 15:47 |
| dansmith | sorry :/ | 15:47 |
| abhishekk | no, I was saying thank you for putting your time :D | 15:48 |
| abhishekk | I know you have other priorities as well, so really appreciating your time/efforts here | 15:49 |
| * abhishekk going for dinner break | 15:49 | |
| dansmith | np, enjoy :) | 15:50 |
| abhishekk | :) | 15:50 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef namespaces https://review.opendev.org/c/openstack/glance/+/798700 | 16:59 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef resource-types https://review.opendev.org/c/openstack/glance/+/799671 | 16:59 |
| *** melwitt is now known as jgwentworth | 17:55 | |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef namespaces https://review.opendev.org/c/openstack/glance/+/798700 | 18:22 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef resource-types https://review.opendev.org/c/openstack/glance/+/799671 | 18:22 |
| * abhishekk signing out for the day | 18:45 | |
| * abhishekk have a great weekend ahead | 18:46 | |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef namespaces https://review.opendev.org/c/openstack/glance/+/798700 | 18:53 |
| opendevreview | Pranali Deore proposed openstack/glance master: Implement project personas for metadef resource-types https://review.opendev.org/c/openstack/glance/+/799671 | 19:10 |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!