| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-10-05 | |||
| 18:15:32 | dansmith | sean-k-mooney: this ^ will not work for instances you don't own if you are not admin | |
| 18:15:42 | gmann | either project scope or both or need service user specific role for such operation | |
| 18:15:52 | sean-k-mooney | do we have a db level check to enforce that | |
| 18:16:22 | dansmith | sean-k-mooney: that's how that interface works right? | |
| 18:16:35 | dansmith | I mean, it certainly was when the code was written :) | |
| 18:17:25 | sean-k-mooney | in the port bindign case the event are all submited as tyepically the nova user regradesss of who owns the port | |
| 18:17:33 | sean-k-mooney | dansmith: the user token is never used | |
| 18:17:57 | dansmith | ...right, as admin yes? | |
| 18:18:03 | sean-k-mooney | also even if it was the even it decupled form a user action | |
| 18:18:07 | sean-k-mooney | yes as admin | |
| 18:18:17 | dansmith | what are we arguing about again? | |
| 18:19:04 | sean-k-mooney | wether the resouce that is being evented on is own by the user/project assocated with teh keyton token used to send the event to nova i think | |
| 18:19:43 | sean-k-mooney | i kind of got confused by what you were saying would and would not work and im not sure you were following me either | |
| 18:20:18 | dansmith | https://github.com/openstack/nova/blob/7967ad78649a1f8d7ffc34ae28274ee89b0011cf/nova/db/main/api.py#L1413-L1416 | |
| 18:20:32 | dansmith | this ^ is where we enforce you can only see your own instances in the DB layer, if not admin | |
| 18:20:55 | dansmith | which makes that API not allow non-admins (if so granted) able to send events to instances they do not own | |
| 18:21:14 | sean-k-mooney | i see | |
| 18:21:19 | dansmith | if the nova user neutron uses has admin, that's why it can send events to any instance, as expected | |
| 18:21:43 | dansmith | I'm talking about if someone were to drop the admin requirement from the policy, or if we later add a human-initiated event, the API is not insecure | |
| 18:22:09 | sean-k-mooney | right i tought we were trying to remove those low level db enfrocements form the code | |
| 18:22:14 | gmann | dansmith: so your proposal is to allow only project scoped (owner or admin) not system right? | |
| 18:22:14 | dansmith | [11:09:33] <sean-k-mooney> i dont think it is either today | |
| 18:22:14 | dansmith | [11:09:14] <lbragstad> correct - but it's not enforcing tenancy in any way - from what i can tell | |
| 18:22:14 | dansmith | in response to this: | |
| 18:22:47 | dansmith | sean-k-mooney: not those, AFAIK.. tons of stuff would need to change at the top, and always provide a project-level filter down or we'd be querying tons of information all the time that we need to filter in python | |
| 18:22:48 | sean-k-mooney | dansmith: right so its the db laywer not the api that enforcing that | |
| 18:23:16 | dansmith | gmann: no I'm saying we allow either and enforce scope type based on the event_type in the event, which are probably all system right now (but need to look) | |
| 18:23:16 | gmann | sean-k-mooney: we have that as TODO to move admin-checks from DB to API layer side | |
| 18:24:03 | dansmith | gmann: you're not talking about removing the project filtering from the DB queries entirely right? | |
| 18:24:10 | gmann | yeah it is all system-admin for now | |
| 18:24:59 | dansmith | (not that it matters for this conversation, because this API behaves the same as all our other ones at the moment, which rely on db filtering) | |
| 18:25:16 | gmann | dansmith: not filtering but like if admin then we check admin policy and call separate DB interface itself at API layer and do not check is_admin in DB | |
| 18:25:27 | dansmith | gmann: right ++ | |
| 18:27:37 | gmann | on event type: keeping system scope is for human-initiated event ? | |
| 18:28:15 | dansmith | no | |
| 18:28:24 | gmann | making them system-only was not right thing at initial implementation | |
| 18:28:48 | dansmith | honestly, I don't even know what to say anymore: I think neutron declaring a project id when making the call is not bad, | |
| 18:28:50 | sean-k-mooney | i think we just converted admin_api to system_admin_api as an itital step | |
| 18:28:53 | dansmith | which makes it a project-scoped interface It hink | |
| 18:29:11 | gmann | yeah | |
| 18:29:12 | dansmith | I'm just saying I don't want this interface to just be forever tagged as system-only | |
| 18:29:32 | dansmith | if we want to make some events system-only, then let's do that per-event, | |
| 18:29:45 | dansmith | but even in the neutron case, I think scoping it by project is probably not a bad idea anyway | |
| 18:30:05 | sean-k-mooney | dansmith: so currently neutron get the credentails form the config so if we wanted to make it non admin they would need to have some other way to generate the token that is used | |
| 18:30:24 | dansmith | sean-k-mooney: not saying non-admin for network events | |
| 18:30:33 | dansmith | I'm saying probably not system | |
| 18:30:50 | dansmith | there's like a 3D chart here I think :) | |
| 18:30:56 | dansmith | or maybe 7D I dunno | |
| 18:31:05 | gmann | dansmith: agree, and that per event check we can do in code itself as you mentioned earlier. if x_event then context.scope == 'system' | |
| 18:31:14 | dansmith | gmann: exactly | |
| 18:31:36 | dansmith | btw, | |
| 18:32:04 | dansmith | I thought at one point we were supposed to get the ability for neutron to say "here's the user's token for context, and here's my token for auth" | |
| 18:32:18 | sean-k-mooney | right but the network-vif-pluged event for example is sent when but the dhcp server has configured the enttry for the ip and the l2 agent has installed flow rules ectra. so it has to create the token form a config file | |
| 18:32:23 | sean-k-mooney | it cannot use the user token | |
| 18:32:26 | dansmith | so we could scope to the user's project from their token, but elevate the ability to do systemy things with the service's token | |
| 18:32:49 | lbragstad | i think there is a policy check for that? | |
| 18:32:53 | dansmith | sean-k-mooney: it doesn't have to use the user's token | |
| 18:33:47 | dansmith | sean-k-mooney: are you saying it doesn't know anything about the project that owns the port? | |
| 18:33:53 | dansmith | because all it needs is the project_id | |
| 18:34:07 | sean-k-mooney | no the port has a project_id assocatied with it | |
| 18:34:30 | dansmith | right, so it can use an admin token generated from creds from the conf, scoped to that project yeah? | |
| 18:34:34 | sean-k-mooney | but the xproejct_override work that lbragstad has been workign on only works with system tokens | |
| 18:34:44 | dansmith | fine? | |
| 18:35:28 | sean-k-mooney | so its a system admin toke with the project_id also set which is what we started with | |
| 18:35:42 | sean-k-mooney | your just saying the project_id should come form the port | |
| 18:35:48 | sean-k-mooney | not be hardcoded in the config | |
| 18:35:49 | dansmith | what we started with was the assertion that it *has* to be that way | |
| 18:35:54 | dansmith | actuall,y | |
| 18:36:08 | dansmith | I think what we started with was "let's discuss this high-bandwidth at ptg" which is *really* what I'd like | |
| 18:38:44 | sean-k-mooney | ok sure. although this is the second deiscussion i have had on this topic as i started talking to lbragstad about it a few weeks ago | |
| 18:39:36 | dansmith | yeah, clearly he needs to be in said high-bandwidth discussion | |
| 18:39:45 | sean-k-mooney | so i was strating form an assumtion that the neutron config would have the creds for a system scoped token and possibly use the project override mechanium | |
| 18:40:24 | sean-k-mooney | im totally fine with and actully like the idea that neutron would overried the project_id with the ide of the port or network owner | |
| 18:40:48 | lbragstad | override == use the project id pass-through functinality? | |
| 18:41:01 | sean-k-mooney | but we woudl need code change to neutron to do that. today the best we can do via the config would be a system_admin token | |
| 18:41:07 | sean-k-mooney | lbragstad: yes | |
| 18:41:21 | sean-k-mooney | project_id passtough is what i mean by overried | |
| 18:41:55 | hemna | can you boot an instance on behalf of another tenant (as admin) ? | |
| 18:42:05 | sean-k-mooney | lbragstad: neutron currently uses the same nova client to send all external events to nova https://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L84-L91 | |
| 18:43:33 | sean-k-mooney | hemna: as in server create no | |
| 18:43:38 | lbragstad | yeah - i hit that issue when i configured nova to enforce scope and worked on the project-id passthrough functionality in ksa and ksm | |
| 18:43:41 | sean-k-mooney | hemna:you can start a server | |
| 18:43:51 | hemna | ok dang. thanks | |
| 18:44:32 | sean-k-mooney | lbragstad: neutron also batchs sending events today | |
| 18:44:50 | sean-k-mooney | so i think they reuse the client for multiple events for diffeenr resouces | |
| 18:45:03 | lbragstad | ok - so that's the PUT portion of that API, right? | |
| 18:45:43 | sean-k-mooney | its a post but yes https://docs.openstack.org/api-ref/compute/#create-external-events-os-server-external-events | |
| 18:46:29 | dansmith | sean-k-mooney: they never batched before, are you sure? | |
| 18:46:49 | lbragstad | ok - nevermind, i thought i saw that API expose a PUT method | |
| 18:46:50 | sean-k-mooney | https://github.com/openstack/neutron/blob/master/neutron/notifiers/nova.py#L146 | |
| 18:46:53 | dansmith | sean-k-mooney: or do you mean batches of ports for a single instance? | |
| 18:46:54 | sean-k-mooney | dansmith: ^ | |
| 18:47:36 | dansmith | okay I thought you said you didn't know the interface was intended for batches? :) | |
| 18:47:58 | dansmith | pretty sure that's new (since I last looked) because I was surprised how long after the interface was written that it was still doing singles | |
| 18:48:32 | dansmith | oh, this is network-changed I think | |
| 18:48:56 | sean-k-mooney | this was network changed yes | |
| 18:49:06 | lbragstad | ok - and the goal is to continue allowing this stuff work to work without having to refactor neutron to use system-scope? | |
| 18:49:12 | dansmith | ack, I would have been looking at vif-plugged | |
| 18:50:17 | dansmith | (which looks to be single-shot) | |
| 18:50:31 | dansmith | lbragstad: I don't think that's a goal | |