Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-02-23
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 bauzas dansmith: then my only concern is why it's for "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: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 bauzas will remove my -1
15:43:23 gmann dansmith: at least now it will protect s to dis-allow system admin
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 bauzas gmann: heh, the patch is hairy, I'm not yet there
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: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: Remove unnecessary TODO https://review.opendev.org/c/openstack/nova/+/830645
16:17:37 opendevreview Stephen Finucane proposed openstack/nova master: libvirt: Ignore LibvirtConfigObject kwargs https://review.opendev.org/c/openstack/nova/+/830644
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:38 opendevreview Stephen Finucane proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646
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:39 opendevreview Stephen Finucane proposed openstack/nova master: libvirt: Shuffle methods around https://review.opendev.org/c/openstack/nova/+/830648
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: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: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
16:42:29 stephenfin Now just to find the time to actually do it, heh
16:42:55 gmann bauzas: passing requester project_id and checking that against requester itself is ally bogus thing we have to do for now.
16:43:20 bauzas ok, it's a bit of a spaghetti code to me but I trust you
16:43:40 bauzas either way, +2d the whole series

Earlier   Later