| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-10-31 | |||
| 16:42:54 | dansmith | yeah it kinda needs policy to check the service token, but the rest of cinder should use user token for scoping and audit logging | |
| 16:43:05 | gmann | maybe we need to pass both user and service token in oslo.policy | |
| 16:43:13 | gmann | dansmith: yeah | |
| 16:43:17 | sean-k-mooney | gmann: i think ^ is likely needed yes | |
| 16:43:29 | sean-k-mooney | both tokens and likely an extention to policy.json | |
| 16:43:37 | gmann | we need to make sure overriden policy keep working and it is configurable | |
| 16:43:37 | dansmith | gmann: can we have a way to say "apply this rule to service token"? then that can be the default and you could reset it to user if you prefer | |
| 16:43:41 | sean-k-mooney | so that we can express the requireemnt for for roles on each one | |
| 16:44:45 | sean-k-mooney | dansmith: ya i think wee need to be able to say somethign like "service_token_required_role=service or default_token_required_role=admin" | |
| 16:44:46 | gmann | dansmith: need to check if such fallback there in oslo.policy otherwise we can combine the both token in the targets to oslo.policy | |
| 16:46:29 | gmann | if we keep rule as 'role:service' only and passing both token (service and user) as targets to oslo.policy then it will work for our default (service only) as well as for any overridden rule too | |
| 16:47:04 | sean-k-mooney | gmann: that would allow a user that was given the service role to make a request with a single token | |
| 16:47:10 | sean-k-mooney | im not sure we want to allow that | |
| 16:47:12 | gmann | if we will use rule default as 'admin or service' then it will be comlicated to disallow only admin to pass the checks | |
| 16:47:40 | sean-k-mooney | i wanted to disallow the user_token to pass if it had the service role without also having a service token | |
| 16:47:47 | gmann | sean-k-mooney: but admin-or-service also allow such user having service role | |
| 16:47:50 | sean-k-mooney | perhaps that shoudl be allowed but im not convinece it shoudl | |
| 16:48:06 | sean-k-mooney | gmann: it woudl allow it because of the admin role | |
| 16:48:11 | sean-k-mooney | not because of the service role | |
| 16:48:13 | dansmith | well, I was thinking more like "this rule applies to the user token" or "this rule applies to the service token" and default to user, which is current behavior | |
| 16:49:04 | gmann | dansmith: ohk. and change default behavior to check rule for service token right? | |
| 16:49:11 | dansmith | so if you set the swap_volume rule to service token, then it will use the service token to decide if a request can or cannot run that operation instead of the user token (and require it be present of course) | |
| 16:49:23 | gmann | so service token check first if it pass then go for user token check otherwise fail early aonly | |
| 16:49:30 | dansmith | gmann: change default just for swap_volume you mean right? | |
| 16:49:52 | sean-k-mooney | swap_volume and i think external events are the two api this would apply too for now | |
| 16:49:59 | dansmith | and detach | |
| 16:50:03 | gmann | dansmith: I mean first we check service token rule and then user token rule | |
| 16:50:18 | dansmith | gmann: no, not what I was thinking | |
| 16:50:26 | gmann | humm | |
| 16:50:33 | sean-k-mooney | dansmith: well volume detach instnace action will not need this change | |
| 16:50:36 | dansmith | for swap_volume, we only care about the service_token for *authorization* so policy only needs to check that | |
| 16:51:03 | dansmith | cinder still uses the user_token for querying the DB and logging, but the ability to do the operation is authorized by service token | |
| 16:51:16 | dansmith | create_volume would still be authorized by user token only | |
| 16:51:23 | gmann | and default policy will be "role:service" right? | |
| 16:51:43 | sean-k-mooney | "roleLservice token:service" | |
| 16:51:54 | dansmith | sean-k-mooney: well, detach is currently checking the service token explicitly right? so if we extend the syntax to allow saying "this operation is authorized by the service token" we'd want that rule to work the same way Itink | |
| 16:52:12 | dansmith | okay I think we're getting confused here | |
| 16:52:17 | sean-k-mooney | where "role:service" woudl imply "role:service token:user" or current behavior | |
| 16:52:19 | dansmith | perhaps we should take this outside the nova meeting | |
| 16:52:27 | sean-k-mooney | dansmith: detach on the cinder api is | |
| 16:52:34 | bauzas | yeah, I think we need some gmeet | |
| 16:52:46 | gmann | yeah, maybe let's have a call | |
| 16:53:09 | bauzas | I can organize it, but I won't be able to attend it I guess | |
| 16:53:14 | sean-k-mooney | dansmith: ya so i think you and i are on the same page | |
| 16:53:46 | sean-k-mooney | we need a syntax in the role to say this is authorised by the service token | |
| 16:53:52 | dansmith | yeah | |
| 16:53:52 | gmann | let me scheudle one after TC meeting today (which is after 3 hr from now). sean-k-mooney is that late for you? | |
| 16:54:03 | dansmith | gmann: can we make it a different day// | |
| 16:54:09 | dansmith | today is already busy and full of meetings for me | |
| 16:54:09 | gmann | sure | |
| 16:54:12 | dansmith | I'm about meetinged out | |
| 16:54:34 | gmann | tomorrow same time as nova meeting today? | |
| 16:54:38 | bauzas | I'm off this wed and this friday, but please do this when I'm not here :) | |
| 16:54:43 | dansmith | gmann: +1h from this meeting I think | |
| 16:54:58 | dansmith | gmann: we have a downstream meeting this time tomorrow | |
| 16:55:00 | gmann | sean-k-mooney: yeah, i got point for service_token vs service role. but need to check syntext and how oslo.policy going to enforce it | |
| 16:55:01 | dansmith | or 1h before this one would work | |
| 16:55:40 | gmann | +1 hr is good for me. sean-k-mooney how about you ? tomorrow 17 UTC | |
| 16:55:45 | dansmith | bauzas: you mean you're delegating and we'll brief you when we get a solution :) | |
| 16:55:52 | bauzas | dansmith: exactly, sold | |
| 16:55:59 | sean-k-mooney | ill ateend whenever | |
| 16:56:11 | gmann | cool. i will send invite | |
| 16:56:29 | sean-k-mooney | i think dansmith more or less is on the same page as me so if there there its likely fine and ill attend if im around | |
| 16:56:40 | bauzas | I just pass my PTL service token to dansmith and sean-k-mooney and tell to use my auth token for signing-off | |
| 16:56:52 | sean-k-mooney | i shoudl be free at that time for what tis worth | |
| 16:56:58 | dansmith | bauzas: badum tish | |
| 16:58:33 | bauzas | love it when a plan comes together ! | |
| 16:58:50 | bauzas | and ends the meeting | |
| 16:59:07 | bauzas | thanks all | |
| 16:59:11 | bauzas | #endmeeting | |
| 16:59:12 | opendevmeet | Log: https://meetings.opendev.org/meetings/nova/2023/nova.2023-10-31-16.00.log.html | |
| 16:59:12 | opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/nova/2023/nova.2023-10-31-16.00.txt | |
| 16:59:12 | opendevmeet | Minutes: https://meetings.opendev.org/meetings/nova/2023/nova.2023-10-31-16.00.html | |
| 16:59:12 | opendevmeet | Meeting ended Tue Oct 31 16:59:11 2023 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | |
| 16:59:19 | elodilles | thanks o/ | |
| 17:00:58 | opendevreview | John Garbutt proposed openstack/nova-specs master: WIP: Add spec for PCI Groups https://review.opendev.org/c/openstack/nova-specs/+/899719 | |
| 17:02:15 | sean-k-mooney | :) i should add ^ to my reading list | |
| 17:03:47 | bauzas | sean-k-mooney: dansmith: when you're around, fwiw, I wrote a bugfix for the sriov gpu problem https://review.opendev.org/c/openstack/nova/+/899625 | |
| 17:04:00 | bauzas | we basically agreed on the solution but I needed to add another meat | |
| 17:04:42 | bauzas | since we create a new mediated device when spawning, there is some race condition until the periodic update_rp() runs | |
| 17:06:02 | bauzas | so I tried to find a solution, which is to call the update_rp() method after the contextmanager run | |
| 17:12:56 | dansmith | bauzas: okay I need to catch up on context | |
| 17:13:22 | bauzas | as a reminder, a GPU has multiple VFs | |
| 17:13:33 | bauzas | each of the VFs can have only one mdev | |
| 17:13:37 | bauzas | but, | |
| 17:13:46 | bauzas | this depends on the vgpu type | |
| 17:14:23 | bauzas | say a vgpu type only supports 2 vGPUs, then if a GPU has 16 VFs, only two of them can have a mdev | |
| 17:15:12 | bauzas | for the first mdev, this doesn't change the sysfs : we see that one VF has available_instances be 0 while the 15 else have available_instances = 1 | |
| 17:15:29 | dansmith | I think this was discussed in the nova room after I had to leave for tc stuff right? | |
| 17:15:45 | bauzas | but once we use another VF for creating a mdev, then all of the 14 else now have available_instances = 0 | |
| 17:16:10 | bauzas | dansmith: well, we discussed this before I think but I haven't verified whether you were around | |
| 17:16:40 | bauzas | https://etherpad.opendev.org/p/nova-caracal-ptg#L678 | |
| 17:17:05 | dansmith | yeah I wasn't, AFAIK | |
| 17:17:16 | bauzas | so, the problem was due to https://github.com/openstack/nova/blob/9c9cd3d9b6d1d1e6f62012cd8a86fd588fb74dc2/nova/virt/libvirt/driver.py#L9110-L9111 | |
| 17:17:18 | dansmith | I'm in another convo right now leading up to the TC meeting soon, but I'll circle back | |
| 17:18:09 | bauzas | basically, the libvirt driver correctly numbers the values and then it passes a total=0 | |
| 17:18:22 | bauzas | so we need now to delete the RP if so | |
| 17:18:43 | bauzas | dansmith: sure, no worries, I understand this needs a lot of context | |
| 17:19:11 | bauzas | and probably some hardware environment :) | |
| 21:25:38 | opendevreview | sean mooney proposed openstack/nova master: add pyproject.toml to support pip 23.1 https://review.opendev.org/c/openstack/nova/+/899753 | |