| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-08-27 | |||
| 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. | |
| 18:23:35 | gmaan | sean-k-mooney: you want me to do it in your glance revert or separate one as this is more than revert and add admin right also with serviec role? | |
| 18:24:20 | sean-k-mooney | gmaan: that valid. i have alreayd modifed it once. its up to you if you want to review the review/changeid go for ti | |
| 18:24:21 | whoami-rajat_ | modifications in the revert might not be clean and confuse people doing blame in the future, can we go for a new patch? | |
| 18:24:30 | sean-k-mooney | otherwise i can abandon it in favor of yours | |
| 18:25:08 | whoami-rajat_ | gmaan, just to be sure, you will be working on the cinder patch as well right? to use [glance] config section | |
| 18:25:18 | gmaan | ok let's do with new patch | |
| 18:25:43 | gmaan | whoami-rajat_: yes, whole series + devstack one and testing it together like I did for nova | |
| 18:25:51 | whoami-rajat_ | okay, thanks! | |
| 18:26:58 | whoami-rajat_ | thanks again gmaan and sean-k-mooney for the details, I will go now, let me know when the glance and cinder patches are ready and i can help in merging them | |
| 18:27:14 | gmaan | thanks, might be late for you | |
| 18:28:42 | whoami-rajat_ | generally I've stopped working late now but this was important so :D | |
| 18:28:45 | whoami-rajat_ | thanks again! | |
| 18:29:15 | gmaan | good for you :) | |
| 19:10:44 | sean-k-mooney | dansmith: so im more or less +w but before i hit it do you want to make this chagne https://review.opendev.org/c/openstack/placement/+/945465/comment/b1e034dd_f7b0b442/ or will i just approve as is | |
| 19:12:16 | sean-k-mooney | dansmith: the wrodign change is defintly minor enouch that i dont think it needs a respon and gibi resolved there nit when they +2'd | |
| 19:13:04 | sean-k-mooney | so in the absence of a prefence one way or another ill add +w before i finish for today | |
| 19:13:10 | sean-k-mooney | but jsut said i woudl ask | |
| 19:13:12 | dansmith | yeah, I dunno, I remember trying to figure out what to say there.. "does not increase existing overage" sounds a bit weird to me, even though I know what you mean | |
| 19:13:36 | dansmith | my preference is to +W so the code is right, and we can FUP the log text if we want | |
| 19:13:37 | sean-k-mooney | ya same that waws the second time i wrote that | |
| 19:13:50 | sean-k-mooney | ack ill do that so | |
| 19:15:04 | sean-k-mooney | done ok im going to try and finsin in the next 15 mins or so. i assume your going to backport those at some point | |
| 19:16:09 | sean-k-mooney | it could be nice to include that in 2024.1 before it goes unmaintained but that wont happen until 2025.2 is released so there si a little time still | |
| 19:25:48 | dansmith | yeah, should be backported | |
| 19:26:08 | dansmith | I guess I dunno how far it really needs, I was thinking just to 2025.1 | |
| 19:27:14 | opendevreview | Ghanshyam proposed openstack/nova master: Add service role in Nova policy https://review.opendev.org/c/openstack/nova/+/957578 | |
| 19:27:43 | sean-k-mooney | i woudl say what is ever open when you get aroudn to it but 2025.1 is a good baseline | |