opendevreview | Ghanshyam proposed openstack/nova-specs master: Propose API policy manager role spec https://review.opendev.org/c/openstack/nova-specs/+/937650 | 00:15 |
---|---|---|
opendevreview | Ghanshyam proposed openstack/nova-specs master: Propose API policy service role spec https://review.opendev.org/c/openstack/nova-specs/+/951218 | 00:23 |
opendevreview | Ghanshyam proposed openstack/nova-specs master: Re-propose API policy service role spec https://review.opendev.org/c/openstack/nova-specs/+/951218 | 00:23 |
opendevreview | Ghanshyam proposed openstack/nova-specs master: Propose API policy manager role spec https://review.opendev.org/c/openstack/nova-specs/+/937650 | 00:27 |
gmaan | sean-k-mooney: ^^ proposed the manager role spec (fixing the review comment from code/last cycle review) and also re-proposing the service role spec, please review. proposed both spec separately | 00:34 |
gmaan | I added the new policies proposal also to filter out the host-thing from manager permission | 00:37 |
opendevreview | Callum Dickinson proposed openstack/nova master: Add image meta to libvirt XML metadata https://review.opendev.org/c/openstack/nova/+/942766 | 02:43 |
opendevreview | sean mooney proposed openstack/nova-specs master: Add cpu-teiring with cpu_shares spec https://review.opendev.org/c/openstack/nova-specs/+/951222 | 03:51 |
opendevreview | Takashi Kajinami proposed openstack/nova master: Replace license classifier https://review.opendev.org/c/openstack/nova/+/951226 | 05:05 |
opendevreview | Callum Dickinson proposed openstack/nova master: Add image meta to libvirt XML metadata https://review.opendev.org/c/openstack/nova/+/942766 | 05:20 |
opendevreview | Kamil Sambor proposed openstack/nova master: Replace eventlet.event.Event with threading.Event https://review.opendev.org/c/openstack/nova/+/949754 | 06:47 |
roger_wang | I wonder if anyone could review this patch of Improve Error Handling for Placement API Responses (https://review.opendev.org/c/openstack/nova/+/943530). much appreciated. | 09:09 |
opendevreview | Takashi Kajinami proposed openstack/nova master: Replace license classifier https://review.opendev.org/c/openstack/nova/+/951226 | 13:09 |
opendevreview | Merged openstack/nova-specs master: Re-propose vTPM live migration https://review.opendev.org/c/openstack/nova-specs/+/947542 | 16:11 |
gmaan | sean-k-mooney: thanks for reviews on RBAC specs, I would like to highlight this and if you feel strong to change it? https://review.opendev.org/c/openstack/nova-specs/+/951218/2/specs/2025.2/approved/policy-service-role-default.rst#49 | 16:42 |
sean-k-mooney | gmaan: so orgianlly i ithink i suggested starting with service or admin for upgrade reaons | 16:48 |
gmaan | yeah | 16:48 |
sean-k-mooney | if we think we can miniums the upgred impact such that we can directly go to service | 16:48 |
sean-k-mooney | then im ok to try that | 16:48 |
sean-k-mooney | the real concern is it required cinder, neutron and cyborg | 16:49 |
sean-k-mooney | to be deploywed with teh service role for there own user | 16:49 |
sean-k-mooney | which they should have but that is the main upgrade impact in my view | 16:49 |
gmaan | ok, having old defaults still supported and there is a way to keep admin perform these operation, my first preference is to go with 'service' only | 16:50 |
sean-k-mooney | i expect most deployment tools shoudl have made that chane already but i have not checked | 16:50 |
gmaan | yeah but honestly saying that is what we want to restrict, no user should use these APIs | 16:50 |
sean-k-mooney | right | 16:50 |
sean-k-mooney | so if other are also ok with this proposal then lets try going directly to the desired state | 16:51 |
gmaan | I am working on job to enable it 'service' only token from cinder, neutron so let's see how that testing goes | 16:51 |
sean-k-mooney | we jsut need to coridate the changes in devstack ectra to make this work | 16:51 |
gmaan | ack | 16:51 |
gmaan | yeah | 16:51 |
melwitt | sean-k-mooney: thanks from me also for the spec review | 16:52 |
sean-k-mooney | our new downstream installer deployws all service user with both admin an service so it is prepared for the change but not prepared to drop the admin role any tiem soon | 16:52 |
sean-k-mooney | melwitt: it was on my todo list all week but im glad i got to it before firday at like 6 pm :) | 16:53 |
melwitt | that is a win :) | 16:54 |
sean-k-mooney | my spec review brian is more or less spent for today which is why im makign the terible desion to update a jira thick isntead. | 16:55 |
sean-k-mooney | gmaan: stephenfin has two small spec related to the api that you may have interest in https://review.opendev.org/c/openstack/nova-specs/+/940440 and https://review.opendev.org/c/openstack/nova-specs/+/947210 | 16:58 |
sean-k-mooney | the latter came form a code review as part of the openapi schema work | 16:58 |
gmaan | sure, will check today | 17:00 |
gmaan | sean-k-mooney: replied for the evacuate policy change, that is something we should discuss more. especially if that can be used to know infra/service info. | 18:03 |
gmaan | gibi: when you have time, can you provide your opinion on evacuate policy default change (specially on Does it leak information about infra/services? part) https://review.opendev.org/c/openstack/nova-specs/+/937650/7/specs/2025.2/approved/policy-manager-role-default.rst#127 | 18:03 |
gmaan | we do return 400 if evacuate is tried and compute is up but host info are returned in error message | 18:04 |
sean-k-mooney | gmaan: i dont think we would return a 400 | 18:11 |
sean-k-mooney | for a normal user they will get a 403 or 401 but an admin i would expect to get a 409 | 18:12 |
gmaan | 409? | 18:12 |
sean-k-mooney | if the host was up ya | 18:12 |
sean-k-mooney | it might be a 400 | 18:12 |
gmaan | yeah, that is something I was thikning why we did not return 409 in this case | 18:12 |
sean-k-mooney | i.e. it may be one of the api we missed when we treid to use 409 better | 18:12 |
gmaan | yeah | 18:13 |
sean-k-mooney | gmaan: is the concern that tehy will knwo the mangment of the host is still up? | 18:14 |
sean-k-mooney | like if they try to evacuate and the nova-compute is up | 18:15 |
gmaan | sean-k-mooney: that one + current code return instance' host in the error message which we can change | 18:15 |
sean-k-mooney | then they could just cold/live migratiate instead right? | 18:15 |
sean-k-mooney | gmaan: oh right i didnt look at the respocne | 18:15 |
gmaan | they can migrate other way but it is that they will have a way to know compute service is up | 18:16 |
gmaan | which they know indirectly because instance is up :) that is why I am not 100% sure if this is infra info leak or not | 18:17 |
sean-k-mooney | well they can infer but doint instnace actions on the instnace | 18:17 |
sean-k-mooney | like server diagnostics | 18:17 |
gmaan | that is admin only | 18:17 |
sean-k-mooney | is it? | 18:17 |
sean-k-mooney | ok but its valid | 18:17 |
gmaan | I do not think manager will know host/info from any API | 18:17 |
sean-k-mooney | we need too decide how mcuh awareness of this info is ok for the manager role to have | 18:18 |
gmaan | yeah | 18:18 |
sean-k-mooney | right i dont think they should eitehr | 18:18 |
gmaan | that is evacuate is little tricky because it depends on service availability checks | 18:18 |
sean-k-mooney | yep | 18:19 |
gmaan | now, I do not have any strong opinion to open it for manager at least until someone have use case and ask us to do | 18:19 |
sean-k-mooney | by the way even if diagnostics was admin only console log show or hardreboot | 18:19 |
sean-k-mooney | can all indreictlly be used to prob if the instance is still managable | 18:20 |
gmaan | reboot is member-or-admin https://github.com/openstack/nova/blob/221a3e89e8988bc664298106ee691a4e41ca71f9/nova/policies/servers.py#L376 | 18:21 |
gmaan | console log too https://github.com/openstack/nova/blob/221a3e89e8988bc664298106ee691a4e41ca71f9/nova/policies/console_output.py#L27 | 18:21 |
sean-k-mooney | i expected https://docs.openstack.org/api-ref/compute/#show-console-output-os-getconsoleoutput-action to be reader | 18:22 |
sean-k-mooney | gmaan: im less concered with the service up vs leaking hostname/fqdn | 18:23 |
sean-k-mooney | we could defer the evacuate change for now or see what gibi and other thing before updating the spec | 18:23 |
gmaan | yeah, sounds good. | 18:23 |
sean-k-mooney | the calculus im doing is i dont know waht it gets you if you knwo nova-compute on a host is not up in general. the fqdn can tell you a lot about a host if your a bad actor | 18:24 |
sean-k-mooney | public cloud still may not want to expose that but since noone will have the manager role by default | 18:25 |
gmaan | yeah | 18:25 |
sean-k-mooney | its kind of a decison they can make | 18:25 |
opendevreview | Merged openstack/nova-specs master: Add remove-os-volumes_boot-api spec https://review.opendev.org/c/openstack/nova-specs/+/947210 | 20:51 |
opendevreview | Merged openstack/nova master: libvirt: Use common naming convention for ephemeral disk labels https://review.opendev.org/c/openstack/nova/+/947541 | 21:13 |
opendevreview | Merged openstack/nova master: api: Adjust validation helpers for a single-method future https://review.opendev.org/c/openstack/nova/+/936365 | 23:52 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!