| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-23 | |||
| 15:36:49 | bauzas | dansmith: tomorrow, you'll get an exception, right? | |
| 15:36:49 | sean-k-mooney | but we need tobe careful not to break other distors is my point | |
| 15:37:04 | sean-k-mooney | mfo: so we might need to sort the options or soemthing instead | |
| 15:37:11 | dansmith | bauzas: this is already limited to admin yeah? | |
| 15:37:12 | mfo | sean-k-mooney, absolutely; thx for your insight on that. | |
| 15:37:38 | bauzas | dansmith: ah i see | |
| 15:38:08 | mfo | sean-k-mooney, i'll go check the secboot ovmf image boot issues, and abandon/update the patches as appropriate. | |
| 15:38:11 | bauzas | dansmith: you'll get all the servergroups for your project id | |
| 15:38:15 | dansmith | bauzas: this is just providing a basically no-op target object so that the check that includes admin and project_id works | |
| 15:38:18 | mfo | sean-k-mooney, thanks again for you help and advice on this. | |
| 15:38:23 | gmann | bauzas: dansmith humm this is "all_projects server group list" which we said to allow all admin from any proejct | |
| 15:38:40 | dansmith | bauzas: wait, are you talking about server groups or all_projects? | |
| 15:38:50 | gmann | bauzas: dansmith if we do not pass project_id then it will fail as PROJECT_ADMIN expect project id | |
| 15:38:50 | bauzas | dansmith: then my only concern is why it's for "all_projects" ? | |
| 15:39:03 | bauzas | dansmith: my question is for https://review.opendev.org/c/openstack/nova/+/828670/4/nova/api/openstack/compute/server_groups.py | |
| 15:39:05 | dansmith | oh, I was looking at the wrong window, not the one from your link sorry | |
| 15:39:21 | bauzas | dansmith: here, we go into all_projects but we restrict to the specific project_id | |
| 15:39:49 | bauzas | I don't see why we need it then | |
| 15:39:53 | gmann | bauzas: it is context_project id so any admin requesting can pass the policy | |
| 15:40:04 | dansmith | bauzas: it's just because the target needs to have the things we're checking | |
| 15:40:05 | gmann | we can see here, legacy admin is allowed https://review.opendev.org/c/openstack/nova/+/828670/4/nova/tests/unit/policies/test_server_groups.py#224 | |
| 15:40:07 | dansmith | it doesn't do any filtering | |
| 15:40:10 | gmann | yeah | |
| 15:40:29 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Add functional tests to reproduce bug #1960412 https://review.opendev.org/c/openstack/nova/+/830010 | |
| 15:40:30 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Run clean up calls when queued live migration is aborted https://review.opendev.org/c/openstack/nova/+/828570 | |
| 15:41:24 | bauzas | gmann: dansmith: this, I understand but why are we targeting the specific project_id ?. | |
| 15:41:37 | bauzas | in the context I mean | |
| 15:41:53 | dansmith | it's *your* project_id, not an instance project_id or a server group project_id | |
| 15:41:57 | dansmith | so it will always match | |
| 15:42:11 | dansmith | because it's in the rule | |
| 15:42:14 | bauzas | dansmith: it's noop, right? | |
| 15:42:18 | gmann | bauzas: that is our default checks as per PROJECT_ADMIN check_str which require project_id to be present | |
| 15:42:37 | dansmith | yes, but since we re-use that rule elsewhere, we need a project_id in there that will match, since we don't care about actual project_id checking for this resource | |
| 15:42:56 | dansmith | it's confusing for sure.. it's one of the reasons I *hate* our policy engine | |
| 15:42:59 | gmann | https://github.com/openstack/nova/blob/master/nova/policies/base.py#L118 | |
| 15:43:09 | dansmith | it's just impossible to look at and understand without study each time | |
| 15:43:10 | bauzas | dansmith: OK, I trust you then | |
| 15:43:23 | gmann | dansmith: at least now it will protect s to dis-allow system admin | |
| 15:43:23 | bauzas | will remove my -1 | |
| 15:43:28 | dansmith | which is also why I think we need to be working towards a future where operators aren't expected to be able to tweak it like they are today | |
| 15:44:44 | gmann | having project_id in check_str is good way to differentiate from system user. once we enable scope as hardcoded then we can remove all these noop checks with context.project_id | |
| 15:46:11 | bauzas | gmann: explain me why we need to touch https://review.opendev.org/c/openstack/nova/+/828670/4/nova/policies/attach_interfaces.py | |
| 15:46:27 | bauzas | and why the role no longer needs admins | |
| 15:46:53 | bauzas | oh, because we want to remove those wrong system-scoped roles | |
| 15:47:16 | gmann | bauzas: yeah, and GET project reader can do and create/delete interface project member | |
| 15:47:45 | bauzas | gmann: but then, if I'm admin of project 10, I can't longer attach interfaces for servers owned by project 2 ? | |
| 15:48:19 | bauzas | or even list them ? | |
| 15:48:45 | bauzas | sorry if those sound silly questions, but I better need to ramp up on the policies things in order to be able to merge this stuff before tomorrow | |
| 15:48:55 | gmann | bauzas: with new policy only yes, that is isolation we are doing. keep project admin restrictive to their project operatrions (except list all project resources if we have in that API) | |
| 15:49:42 | gmann | projectA admin will be restrictive to do projectA things not projectB things. (with all project resource list case) | |
| 15:49:51 | bauzas | gmann: is that a breaking change if we merge this change by now ? | |
| 15:50:08 | bauzas | or can admins use the legacy policies for a while ? | |
| 15:50:25 | gmann | bauzas: we do support legacy policy as deprecated so until we remove them it will keep woprking with old token | |
| 15:50:48 | gmann | bauzas: yes, by default new policy are disabled. scope check as well as new default | |
| 15:51:04 | bauzas | ok | |
| 15:51:11 | bauzas | I better understand :) | |
| 15:51:40 | bauzas | gmann: just tbc, until https://review.opendev.org/c/openstack/nova/+/828670/4/nova/policies/attach_interfaces.py#30 is removed or default changes to new policies, nothing changes ? | |
| 15:51:46 | gmann | bauzas: this is good way to know what all allowed as default with these policy change https://review.opendev.org/c/openstack/nova/+/828670/4/nova/tests/unit/policies/test_attach_interfaces.py#29 | |
| 15:52:07 | opendevreview | Jonathan Race proposed openstack/nova master: driver/secheduler/docs for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053 | |
| 15:52:08 | opendevreview | Jonathan Race proposed openstack/nova master: zuul-job for Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/828372 | |
| 15:52:08 | bauzas | gmann: heh, the patch is hairy, I'm not yet there | |
| 15:52:27 | bauzas | gosh, we're rushing out of time | |
| 15:52:29 | gmann | bauzas: true, or operator enable it with config option enforce_scope=True, enforce_new_defaults=True | |
| 15:52:51 | bauzas | gmann: ok, perfect and I guess this is correctly documented so I'm not afraid | |
| 15:53:03 | gmann | both config are false (disabled) by default in yoga | |
| 15:53:06 | bauzas | gmann: thanks gmann, continuing to review | |
| 15:53:25 | chateaulav | gibi: added final changes in this last patchset to the driver patch, and the zuul ci should be good this time around (fingers crossed) | |
| 15:53:57 | gibi | chateaulav: ack, I will check it before I leave today | |
| 15:55:00 | chateaulav | ccol | |
| 15:55:28 | bauzas | gibi: fwiw, I'm on the policies series that you +1d, I'm leaving a ton of comments to help you better understandingh | |
| 15:57:33 | gibi | bauzas: thanks that will help if you want to convince me to upgrade my vote | |
| 15:58:43 | gmann | bauzas: gibi thanks for reviews. I need to go away for 20 min (taking breakfast) and then will return if any query while review. | |
| 16:17:37 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Ignore LibvirtConfigObject kwargs https://review.opendev.org/c/openstack/nova/+/830644 | |
| 16:17:37 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Remove unnecessary TODO https://review.opendev.org/c/openstack/nova/+/830645 | |
| 16:17:38 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646 | |
| 16:17:38 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Rename some config objects https://review.opendev.org/c/openstack/nova/+/830647 | |
| 16:17:39 | opendevreview | Stephen Finucane proposed openstack/nova master: libvirt: Shuffle methods around https://review.opendev.org/c/openstack/nova/+/830648 | |
| 16:17:39 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Remove handling for older libvirt versions https://review.opendev.org/c/openstack/nova/+/830649 | |
| 16:17:40 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Return objects from Guest.get_interfaces https://review.opendev.org/c/openstack/nova/+/830650 | |
| 16:17:40 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Don't fetch guest architecture repeatedly https://review.opendev.org/c/openstack/nova/+/830651 | |
| 16:17:41 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Prepare for manual PCIe address management https://review.opendev.org/c/openstack/nova/+/830652 | |
| 16:17:42 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: libvirt: Start managing PCIe address allocation https://review.opendev.org/c/openstack/nova/+/830653 | |
| 16:18:01 | stephenfin | Ignore those. They're nowhere near done. Just pushing so I remember to finish them at some point | |
| 16:19:03 | sean-k-mooney | stephenfin: am... i tought we were not gong to tdo that | |
| 16:19:06 | sean-k-mooney | but ok | |
| 16:19:51 | sean-k-mooney | i mean its been in our downstream backlog for ever | |
| 16:21:57 | opendevreview | Pedro Monteiro Azevedo de Moura Almeida proposed openstack/nova master: Update live_migration_downtime definition https://review.opendev.org/c/openstack/nova/+/828387 | |
| 16:26:31 | opendevreview | Elod Illes proposed openstack/nova stable/wallaby: skip test_tagged_attachment in nova-next https://review.opendev.org/c/openstack/nova/+/830656 | |
| 16:27:37 | bauzas | gmann: once you're back, question in https://review.opendev.org/c/openstack/nova/+/829626/9/nova/api/openstack/compute/views/servers.py#443 | |
| 16:28:17 | gmann | checking | |
| 16:31:49 | bauzas | gibi: other cores : I'm +2 on the whole new-policy-rules series except one patch, reviews welcome | |
| 16:33:33 | bauzas | gmann: can you then please answer my question ? | |
| 16:33:57 | bauzas | I have a hardstop in 30 mins and I want to briefly look at the last unified-limits patch before I go | |
| 16:34:43 | gmann | ok | |
| 16:39:00 | gmann | bauzas: replied, by default if target are not passed in policy it take context'sproject_id itself | |
| 16:39:44 | gmann | and DB query are based on requester project_id so it gets only requesting project instances until all-tenant is requested | |
| 16:40:36 | bauzas | gmann: ok, then I'm confused, why did we need to add the target be explicitely project_id ? | |
| 16:41:55 | gmann | bauzas: same as discussed before. PROJECT_ADMIN|MEMBER|READER has project_id i check_str of rule so we need to pass project_id in target which can pass. it is done to separate out the system user if scope are disabled. otherwise system reader can also pass policy. | |
| 16:42:18 | stephenfin | sean-k-mooney: We're still not necessarily going to do it, but it's come up so often that it seems something we should _eventually_ do | |
| 16:42:20 | gmann | bauzas: once we remove the enforce_scope as configurable (enable it hardcoded) then we can remove those and cleanup | |