Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-11-26
15:38:24 artom mriedem, it's been a while since we've had someone entirely new to openstack join RH, but we did try to help francoisp get up to speed when he came in
15:38:26 sean-k-mooney ya it looks liek they start going up last week
15:38:31 sean-k-mooney mriedem: summit videos
15:38:32 artom That was really the only example I can think of
15:38:40 artom Everyone else we hired from upstream, so to speak
15:38:47 mriedem sean-k-mooney: the vidoes i saw posted were sparse - mostly keynotes and some vendor things
15:39:43 mriedem i'm pretty sure there is also a welcome wagon sig or something in openstack
15:39:49 mriedem so there are resources, but people need to do some work to find them
15:43:38 artom stephenfin, hey, so https://review.opendev.org/#/c/672595/ is still around. Can I help in any way to get another round of review from you on that?
15:43:49 artom Split it? Take over something of yours?
15:44:37 stephenfin nah, I just need to do it, but I have my head stuck into the removal of the nova-network security group driver. Could I look at it tomorrow?
15:44:54 artom stephenfin, sure - no huge rush, at this point.
15:45:09 artom I wanted to backport it to Train, but I probably missed the train (zing!) on that
15:45:19 artom I'll try anyways when it merges, see how it goes
15:58:47 openstackgerrit John Garbutt proposed openstack/nova master: WIP: Enforce unified limits using oslo.limit https://review.opendev.org/615180
16:20:24 gmann tssurya: but we do not know whether it is wrong or right. if keystone permissions to GET project is more strict than nova user, then project might exist but not shown to nova
16:22:58 tssurya gmann: sure but the problem with the current implementation is that it treats everything as "right". Even if the project is invalid, we end up giving the user the right to feed anything into the flavor_projects table
16:28:14 gmann tssurya: but it is 403, in case of invalid it will hit 404 condition. not 404 means project is there
16:34:49 openstackgerrit Merged openstack/nova master: api-guide: remove empty sections about inter-service interactions https://review.opendev.org/695774
16:35:19 openstackgerrit Eric Fried proposed openstack/nova master: DNM: Test openstacksdk weakrefs https://review.opendev.org/695934
16:35:32 tssurya gmann: not really there can be a 403 on a case where the project doesn't exist right ?
16:35:52 tssurya because tenant doesn't have permission to verify if the project exists ?
16:39:19 gmann tssurya: not sure keystone does that and return 404 instead of 403. I know neutron return otherway- any PUT/DELETE request first call the GET and return 404 even resource exist and no permissions to GET(they do not return 403 because that is somehow telling resource exit).
16:40:23 tssurya gmann: yea I guess keystone is returning 403 which nova is silently ignoring
16:40:28 gmann i think keystone return 403 for permissions denied on GET project not 404. lbragstad ?
16:40:44 gmann lbragstad can confirm this for us.
16:40:47 lbragstad gmann it depends
16:41:09 lbragstad if you're technically authorized to get a project (e.g., you're a system administrator) you'll get a 404
16:41:30 lbragstad if you don't have the right authorization - then you get a 403
16:41:58 gmann yeah that is what nova assume- https://github.com/openstack/nova/blob/fd67f69cfdaf04620f2e8a5f1fbf5737096965d8/nova/api/openstack/identity.py#L26
16:42:22 lbragstad gmann ok - yeah, that looks right
16:42:24 gmann 403 means project is valid and exist and its only permission issue
16:42:35 gmann lbragstad: thanks for quick confirmation.
16:42:47 lbragstad well - the project may exist, but your first problem is that you're hitting a permission issue
16:43:00 tssurya gmann: I don't think 403 ensures project exists
16:43:19 lbragstad if you hit a 403, you should fix the permission issue and eventually retry
16:43:37 tssurya because we have a couple of non-existing entries in our flavor_projects
16:43:55 tssurya non-existing project entries*
16:44:14 lbragstad a 403 isn't conclusive that the project exists
16:44:20 gmann tssurya: but in those cases, we will get 404 right
16:44:26 tssurya lbragstad: yea that makes sense
16:44:31 tssurya gmann: nope we don't get the 404
16:45:01 tssurya I had a user inject "ABCD" project into the database :D
16:45:09 gmann tssurya: yeah it is 'may' exist
16:45:11 tssurya which doesn't exist
16:45:14 tssurya hehe
16:46:08 tssurya I've opened a bug here: https://bugs.launchpad.net/nova/+bug/1854053
16:46:08 openstack Launchpad bug 1854053 in OpenStack Compute (nova) "_add_tenant_access silently ignores 403" [Undecided,New]
16:46:42 tssurya probably nova should raise for a 403 or prompt user to retry or something
16:49:29 gmann tssurya: with default policy which is admin only should work but if overridden then yes it can make non-exist project entry in flavor access
16:50:11 tssurya gmann: yea that's true, in this case it was done from a service project by a user having admin role
16:50:51 mriedem tssurya: without reading all of the scrollback, "sure but the problem with the current implementation is that it treats everything as "right". Even if the project is invalid, we end up giving the user the right to feed anything into the flavor_projects table" is exactly how that api worked before that change to validate the project id provided
16:50:56 tssurya nevertheless allowing "ok" and then feeding whatever value is coming from the user is not ok
16:51:12 mriedem the project id validation is best effort
16:51:47 tssurya mriedem: aha, I didn't know the history
16:52:17 gmann tssurya: mriedem i think we should only proceed the request for 2xx case and rest all return 400 with error message of what we get from keystone
16:52:24 mriedem so in this case we're using the token from the request context or the configurable keystone auth in nova?
16:52:44 mriedem gmann: which is an api change
16:53:33 mriedem by configurable keystone auth in nova i mean this https://docs.openstack.org/nova/latest/configuration/config.html#keystone
16:53:37 gmann yeah
16:53:39 mriedem service user configured in nova to talk to keystone
16:53:44 mriedem like we do for neutron and cinder
16:54:21 mriedem so doesn't that just mean the service user configured in nova to talk to keystone doesn't have enough permissions to know if the given project_id exists?
16:54:40 tssurya mriedem: right
16:54:45 tssurya exactly
16:55:07 tssurya https://github.com/openstack/nova/blob/fd67f69cfdaf04620f2e8a5f1fbf5737096965d8/nova/api/openstack/identity.py#L62
16:55:39 mriedem yeah, so my point is, it sounds like this is a config issue in your deployment
16:55:54 mriedem i.e. why can't you grant access to the service user that is doing this check on behalf of nova?
16:56:51 tssurya the service user already has an admin role, I think this happens because the project is not admin project ?
16:57:19 mriedem idk
16:57:31 mriedem isn't that also configurable in keystone?
16:57:59 mriedem i.e. the policy to GET a project by id - it's not hard-coded to admin project only is it?
16:57:59 gmann mriedem: but point is users do not know it was permission issue for them or they are adding wrong project
16:58:20 tssurya probably, I'll see if it can be tweaked on the config side for our particular scenario,
16:58:49 tssurya but in general its strange nova allows entries from users that don't have permission
16:58:53 mriedem gmann: if the service user nova is configured with has permissions to ask the question "does this project exist" that's all this utility code is used for
16:59:07 gmann policy can be changed to make it work properly and with default policy it should work at long as keystone system scope is enabled
16:59:36 mriedem and again, this was added as best effort to close a leak where the flavor access and quotas apis would just blindly take whatever project id and assume it existed and shove it into the nova db
16:59:50 gmann mriedem: from API usage they do not know nova does this check internally ?
17:00:41 tssurya mriedem: yea if its best effort, I'll probably just tweak it downstream then and write it in our documentation
17:00:56 mriedem i don't understand the question. if configured properly and nova determines the project doesn't exist (404 from keystone) the compute API users will also get a 404
17:00:58 gmann mriedem: yeah agree on best effort but if API return the permissions issue to user and then they ask provider to modify the permission or ask other right user to add the tenant in flavor access
17:01:38 mriedem i see 3 options:
17:02:02 mriedem 1. close the bug as invalid - tweak your config (we could add docs, idk if that would be found or help) to do what you need to avoid the 403 from keystone
17:02:13 gmann mriedem: no. if permissions issue then keystone return 403 (which means project may exist or not) and nova proceed the request with 200. so user does not know if that is correct or not.
17:02:26 mriedem 2. change the 403 case as an error and raise it back to the compute api caller - maybe enough time has passed to not worry about backward compat with the old non-validating behavior
17:03:11 mriedem 3. add a config option to toggle what the behavior of the api should be in the 403 case, default to backward compat, but allow making it raise for those that want that, but that means you need to know to twiddle that config which if you know that, you likely know you can just fix the permissions for your service user
17:03:17 tssurya I vote for 2
17:03:29 mriedem of course you do :)
17:03:36 tssurya :D
17:05:22 mriedem one thing to point out for #2 is that the quota set and flavor access add/remove style write APIs are admin-only by default, so if we do change the behavior of that 403 it would only be on admin apis, not end-user apis
17:06:03 gmann 3rd is not good option. config driven API not good.
17:06:08 tssurya ack
17:06:12 gmann even admin only API it still API change in option 2
17:06:46 mriedem option 1 is the only non-api change
17:06:54 mriedem we're not going to microversion this imo
17:07:00 mriedem if that's what you're thinking
17:07:15 gmann with option 1 yes no microversion but option 2 need microverion
17:07:19 openstackgerrit Dan Smith proposed openstack/nova master: Add a way to exit early from a wait_for_instance_event() https://review.opendev.org/695985
17:07:25 mriedem not really

Earlier   Later