Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-02-23
15:32:30 sean-k-mooney mfo: there is a singel non secureboot capable image at /usr/share/edk2/ovmf/OVMF_CODE.cc.fd
15:32:35 sean-k-mooney but that will not be checked
15:33:12 mfo sean-k-mooney, ok. so, if for some reason that patch is still required, we're better checking if there's another option available before ignoring secboot.fd.
15:33:27 sean-k-mooney yes
15:33:44 sean-k-mooney so redhat will be releaseign osp 17 based on stable wallaby on rhel 9 later this year
15:34:05 sean-k-mooney and the current patch would break uefi i belive in that case
15:34:32 sean-k-mooney however i dont know if we have the same issue where we woudl need to use the '/usr/share/OVMF/OVMF_CODE.fd' image if secure boot is not requested
15:34:52 bauzas gmann: around ?
15:34:56 bauzas or maybe dansmith ?
15:34:59 bauzas context is https://review.opendev.org/c/openstack/nova/+/828670/4/nova/api/openstack/compute/server_groups.py
15:35:08 gmann bauzas: hi
15:35:11 bauzas we'll change the API behaviour
15:35:27 bauzas as you need to be in the right project to getting the server groups
15:35:32 bauzas it's OK for me
15:35:32 mfo hmm, but this isnt an issue in wallaby, right? as it uses a diff method to pick the ovmf files, based on descriptor files from qemu (eg, /usr/share/qemu/firmware/)
15:35:33 bauzas but,
15:35:41 mfo sean-k-mooney, ^
15:35:50 dansmith bauzas: that's *your* project id
15:36:02 bauzas given it will change the behaviour, do we all agree we don't need a microversion ?
15:36:06 dansmith bauzas: so it should be no different than today
15:36:20 bauzas dansmith: well,
15:36:27 sean-k-mooney mfo: ack ya it might not be an issue there
15:36:30 bauzas previously you were giving none as a value
15:36:33 mfo sean-k-mooney, ok cool.
15:36:36 sean-k-mooney as you said its using a differnt method
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

Earlier   Later