| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-08-27 | |||
| 17:33:26 | sean-k-mooney | if i can demonstrate tha then we coudl fix it properly | |
| 17:33:53 | sean-k-mooney | gmaan: no not that i can see form there config ref | |
| 17:36:38 | gmaan | sean-k-mooney: I checked what all method cinder should pass user token (like they do currently) and I think these are the two call cinder need to pass config service user to glance - https://github.com/openstack/cinder/blob/master/cinder/image/glance.py#L345-L370 | |
| 17:37:15 | gmaan | so they can pass privileged_user =True in common mehtod you mentioned to copy from nova file to glance | |
| 17:38:13 | gmaan | those are two glance call have service role for their policy chgeck https://github.com/search?q=repo%3Aopenstack%2Fglance+base.SERVICE&type=code | |
| 17:39:51 | gmaan | rest all other call can continue using the user token. for example, this is nova case where cinder get servers volume from nova and just call it with user token (not configured service user) + service token https://github.com/openstack/cinder/blob/master/cinder/compute/nova.py#L243-L246 | |
| 17:39:58 | sean-k-mooney | ok i was hopign thiw woudl take me a few second to poc | |
| 17:40:07 | sean-k-mooney | but it will take me more time then i have | |
| 17:40:24 | sean-k-mooney | gmaan: is this somethign you could try? | |
| 17:40:41 | sean-k-mooney | gmaan: my concern is releaseing with the glance policy change | |
| 17:40:55 | gmaan | sure, let me update it | |
| 17:41:05 | sean-k-mooney | thanks | |
| 17:41:19 | gmaan | is glance policy changed in this cycle or they are released last cycle? | |
| 17:41:50 | sean-k-mooney | the one in this cycle that was doen last week | |
| 17:42:01 | sean-k-mooney | the thing this reverts https://review.opendev.org/c/openstack/glance/+/958567 | |
| 17:42:13 | gmaan | because that can create issue (leak/security etc) where user with no privilege to get/add location can do it via cinder | |
| 17:42:20 | gmaan | ohk, cook | |
| 17:42:22 | gmaan | cool | |
| 17:42:39 | gmaan | let me update your cinder change | |
| 17:57:03 | gmaan | sean-k-mooney: it is more work then that, cinder does not have service user configuration for glance so we need to 1. add those new conf in cinder 2. set those in devstack () and document it for operator to do the same) 3. load and pass that configured user to glance serviec APIs call | |
| 17:57:27 | gmaan | I can push the changes but I am not sure all these can make it before FF | |
| 17:58:08 | gmaan | whoami-rajat_: ^^ just CC you as you are involved in the service role stuff | |
| 17:59:49 | gmaan | anyways we should do this discussion in glance or cinder channel and can decide what best can be done before FF | |
| 17:59:55 | whoami-rajat_ | i just want to mention that currently cinder passes service_roles:service and not role:service so please modify the policies accordingly to support both scenarios (role:service OR service_roles:service) until we reach a consensus on the right way to do it | |
| 18:00:41 | whoami-rajat_ | I'm trying to get in my patch to adopt GET location API which relies on the above ^ | |
| 18:01:09 | gmaan | sean-k-mooney: and that is released in 2025.1 https://github.com/openstack/glance/blob/stable/2025.1/glance/policies/base.py#L116 | |
| 18:02:43 | sean-k-mooney | whoami-rajat_: so service_roles: service is refering to roles on teh service token | |
| 18:03:01 | sean-k-mooney | which is not somehtign that glance or nova shoudl be checkign in this context | |
| 18:03:09 | gmaan | whoami-rajat_: there is one issue in that. any user who is not allowed to get/add image location and can call cinder API will be able to get/add image location because service_role: will always be successful rule in glance | |
| 18:03:45 | gmaan | whoami-rajat_: I try to add all use case here (5th reply from last ) https://review.opendev.org/c/openstack/nova/+/957578/comment/a097d558_fee01375/ | |
| 18:04:20 | whoami-rajat_ | it shouldn't be if we haven't configured the [service_user] send_service_user_token = True | |
| 18:04:45 | sean-k-mooney | that shoudl more or less always be true | |
| 18:04:55 | gmaan | sean-k-mooney: yes | |
| 18:05:18 | gmaan | whoami-rajat_: operator need to do that which is for other purpose than RBAC. user token expiry use case | |
| 18:05:31 | sean-k-mooney | ^ that | |
| 18:06:03 | gmaan | both are for service to service communication but for separate purpose. that is what I was also confused with and was doing it in nova until sean-k-mooney corrected me | |
| 18:06:40 | whoami-rajat_ | I understand, just my main focus is not to break existing functionality, glance and cinder (and even nova) has been interacting in this way so we should keep the behavior when implementing the new one | |
| 18:07:02 | gmaan | sean-k-mooney: so 2024.2 policy was correct, I am wondering how it was working in cinder and your revert which is logically same as this is failing? https://github.com/openstack/glance/blob/stable/2024.2/glance/policies/base.py#L114 | |
| 18:07:06 | whoami-rajat_ | and TBH, i really don't like the redundancy as highlighted here https://review.opendev.org/c/openstack/glance/+/958567/comments/37c4390c_90f23c9d | |
| 18:07:21 | sean-k-mooney | whoami-rajat_: the current glance bevharie as fo 7 days ago or wheneve ris portiotaly a security problem | |
| 18:07:44 | sean-k-mooney | its using the service token for something ti was not intended to be used for | |
| 18:07:58 | sean-k-mooney | we did a one off exccption for the cinder cve | |
| 18:08:14 | sean-k-mooney | btut that is not a policy check via the polciy framwork | |
| 18:08:25 | sean-k-mooney | that a special case that woudl eventually go away | |
| 18:08:42 | sean-k-mooney | by just checkign for the service role on the standard token | |
| 18:08:44 | gmaan | whoami-rajat_: just imagine one scenario. cinder calling to glance will ALWAYS pass the policy. so glance policy is basically noop | |
| 18:09:39 | gmaan | send_service_user_token has to be true always irrespective of RBAC, as you know - https://docs.openstack.org/cinder/latest/configuration/block-storage/service-token.html | |
| 18:10:40 | whoami-rajat_ | I understand and I'm not questioning that, but cinder doesn't support the new way to configure a glance section, read from it and pass role:[service] in request | |
| 18:10:52 | whoami-rajat_ | and i doubt it does for nova either, haven't tried that | |
| 18:11:07 | whoami-rajat_ | so it puts cinder in a condition where we can't really adopt the service role | |
| 18:11:12 | sean-k-mooney | there is code for nova | |
| 18:11:29 | gmaan | whoami-rajat_: oh, yeah that's true for glance. but It does not for nova - https://github.com/openstack/cinder/blob/master/cinder/compute/nova.py#L54-L59 | |
| 18:11:53 | sean-k-mooney | whoami-rajat_: you can use the service user section or keystone_auth section or nova section in the absence of a glance section | |
| 18:11:55 | gmaan | and this is where it is set in conder conf by devstack https://zuul.opendev.org/t/openstack/build/c3c5a139509b47748977f00985b18d2e/log/controller/logs/etc/cinder/cinder_conf.txt#74-85 | |
| 18:12:09 | whoami-rajat_ | ah yes, because we used to send the admin request to nova for operations, now that admin creds with be switched to service one | |
| 18:12:25 | sean-k-mooney | whoami-rajat_: exactly | |
| 18:12:49 | whoami-rajat_ | yeah there was no use case for glance so we never did it | |
| 18:13:03 | whoami-rajat_ | so what would be the next steps here? | |
| 18:13:14 | whoami-rajat_ | Cinder can't really adopt the new location APIs then, at least in 2025.2 | |
| 18:13:49 | sean-k-mooney | the best outcome woudl be to add a new galnce section adn fallback to the keystone_auth or service_user section when not defiend | |
| 18:13:50 | whoami-rajat_ | if we revert the glance changes ^ which i think now is right to do | |
| 18:13:55 | gmaan | whoami-rajat_: sean-k-mooney before going to next step, I would like to know how it was working in stable/2024.2 where glance policy is same as sean-k-mooney revert and cinder failing in revert - https://github.com/openstack/glance/blob/stable/2024.2/glance/policies/base.py#L114 | |
| 18:13:59 | sean-k-mooney | both of those will have the corect roels | |
| 18:14:25 | whoami-rajat_ | in 2024.2, we didn't adopt the location APIs | |
| 18:14:38 | gmaan | ohh | |
| 18:14:40 | whoami-rajat_ | so the feature in glance was merged in 2024.2 but we adopted it in 2025.2 (we=nova and cinder) | |
| 18:14:45 | gmaan | got it | |
| 18:15:12 | sean-k-mooney | the nova change mergd before the glance plicy change however right | |
| 18:15:18 | whoami-rajat_ | yes | |
| 18:15:26 | whoami-rajat_ | the policy change is recent, last week | |
| 18:15:27 | gmaan | best way to not break user and provide option to adopt new way also can be: | |
| 18:15:32 | sean-k-mooney | nova worked because we had an "admin" client | |
| 18:15:43 | sean-k-mooney | with the service role to use | |
| 18:16:03 | gmaan | 1. modify glance policy to service + admin right : this will allow cinder talk to glance with user token as it is currently | |
| 18:16:37 | gmaan | 2. add keystoneauth service user conf option in glance section in cinder.conf | |
| 18:16:51 | gmaan | 3. cinder pass that service user to glance which has service role | |
| 18:17:12 | gmaan | 4. ask operator to configure the same in their deployment via releasenotes or doc | |
| 18:17:32 | whoami-rajat_ | 3 and 4 could be the same cinder patch, which i can work on | |
| 18:17:44 | gmaan | 5, once we know operator are ok (or at least after one SLURP), make glance API to role:service only | |
| 18:18:08 | whoami-rajat_ | sounds good to me | |
| 18:18:43 | sean-k-mooney | that effectivly what we are doing in the nvoa changes for the external events api ectrs | |
| 18:19:01 | gmaan | yes, even we have the service user config option available for so ling | |
| 18:19:03 | sean-k-mooney | so yes i think thiat is the correc approch as well | |
| 18:19:19 | whoami-rajat_ | and if possible, can we document all the details we've discussed here in some RBAC doc, it's really confusing to know the "right" way to adopt a new thing like SRBAC which created the confusion for me at least | |
| 18:19:45 | gmaan | whoami-rajat_: I can propose the changes if you are ok and help in review? bit if you want to do that also fine | |
| 18:19:58 | sean-k-mooney | one thing to note | |
| 18:20:19 | sean-k-mooney | yuo can achive the same effect without have per service config seciton with auth info | |
| 18:20:34 | sean-k-mooney | nova's approch si way more flexible and i think cleaner | |
| 18:20:34 | whoami-rajat_ | gmaan, sure, that will be faster as i can be one of the cores approving things | |
| 18:20:37 | gmaan | whoami-rajat_: yeah, agree. that is my bad not to have proper doc for service role until I started working on those in nova, But I will do that as part of RBAC goal champion | |
| 18:20:42 | gmaan | whoami-rajat_: cool | |
| 18:21:03 | sean-k-mooney | but watcher ueses the keyston_authoken user/password for all service eclients | |
| 18:21:46 | sean-k-mooney | im not sayint that ig the correct thing to do but you already need a cinder user in that section and it should have the service role | |
| 18:21:57 | gmaan | sean-k-mooney: yeah, we can do that but we need to formalize that in most of the services. and I am not sure if there are use case of per service user permission in any demployment or they are ok with same user for all serviecsd | |
| 18:22:20 | sean-k-mooney | i think we shoudl not make that default | |
| 18:22:26 | gmaan | nova also has per service config users | |
| 18:22:33 | sean-k-mooney | having a per service config section i think is better | |
| 18:22:47 | gmaan | yeah, do not know what use we can break | |
| 18:22:47 | sean-k-mooney | but you could have one fallback to the other if not configured | |
| 18:23:06 | gmaan | ok, let me propose the changes today and will add you both as reviewer. | |