Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-08-27
17:14:47 sean-k-mooney gmaan: i didnt have a chance to revew the dnms but did you get clean runs on the service role the last time
17:15:47 gmaan sean-k-mooney: yes, all good. stable job on tempest change was failing in experimental pipeline so I fixed tests to continue using admin on stable and service on master
17:15:50 sean-k-mooney i was mostly happy with your nova change last time i looked so i think we can likely land that today or tomorrow
17:16:11 sean-k-mooney did you see my quetion on the tempest change
17:16:14 gmaan I fixed your doc/release notes comment too
17:16:21 gmaan oh, not yet, checking
17:16:28 sean-k-mooney baout wither the primary role could be service?
17:16:32 sean-k-mooney instead of admin
17:16:45 gmaan sean-k-mooney: oh, that one i replied yes
17:17:16 sean-k-mooney https://review.opendev.org/c/openstack/cinder/+/958582 now passes pep8
17:17:26 gmaan its not admin. actually 'credential' var in tests is little weird and unreadable
17:18:13 gmaan credentials = ['primary', 'admin', ['service_user', 'admin', 'service']] means: it create three users - 1. user with member role (primary), 2. user with admin (admin), 3. user with admin and saervice role (service_user)
17:18:24 gibi here is the futurist bug https://bugs.launchpad.net/futurist/+bug/2121545
17:18:36 sean-k-mooney this was a case of "i dont understand how to read that list" so i was just asking why primary was admin for what its worth
17:19:10 sean-k-mooney oh
17:19:18 sean-k-mooney thats how you read it
17:19:42 sean-k-mooney gmaan: ok that not what i was expectign at all ok
17:19:44 gmaan yeah, it take, normal predefined users (primary, admin, reader, alt_admin etc), users with coustom name and roles so it is not so easy to read.
17:20:20 gmaan and with historic alias in tempest, primary == member etc make it more unreadable
17:20:35 gmaan I think we need to get rid of primary and use member consistently
17:20:44 sean-k-mooney ya a dict of username to list of roels would be simpler also
17:21:14 sean-k-mooney ya the indirection though primary also was not somethign i was aware of
17:21:31 gmaan it keep becoming complex with more personas and we need to keep old predefined personas for compatibility
17:21:42 sean-k-mooney i assme that was to deall with "member" vs "_member_"
17:22:34 gmaan self.os is another name of promary/member but we do not use that in many tests :)
17:22:56 gmaan but I have that cleanup in my list
17:23:39 gmaan at least use latest personas name in tests and keep old one just for compatibility if tempest plugin are using
17:23:57 sean-k-mooney ya that woudl help
17:24:14 sean-k-mooney so i need to finsh the sev revew but yuou thin the issue is https://review.opendev.org/c/openstack/cinder/+/958582/2/cinder/image/glance.py#148
17:24:47 sean-k-mooney evne though i am creating the session form the config now its still not useing tha tfor the user toekn an pulling that form the context
17:27:00 sean-k-mooney nova does https://github.com/openstack/nova/blob/master/nova/service_auth.py#L33-L55 cidner does almost the same https://github.com/openstack/cinder/blob/master/cinder/service_auth.py#L80-L91
17:27:42 gmaan sean-k-mooney: you are creating the sessions from conf but passing the auth plugin to keystonauth https://review.opendev.org/c/openstack/cinder/+/958582/2/cinder/image/glance.py#145
17:28:10 sean-k-mooney if you have na idea of how to properly udpat that patch or can comment i can respien it later
17:28:18 sean-k-mooney but i see i can pass auth as a second parmater
17:28:31 sean-k-mooney so i just need to find an incanation to copy past for that
17:28:35 gmaan sean-k-mooney: that is all ok but in that method nova pass the loaded conf service user and cinder just context so user aith plugin loaded from user context
17:28:39 gmaan yes
17:29:37 gmaan for nova case this is here cinder load the internal service user from conf https://github.com/openstack/cinder/blob/master/cinder/compute/nova.py#L98-L100
17:29:57 gmaan and wrap that with service token in get_auth_plugin
17:30:43 gmaan and then keystonmiddlware gets the right user_token and service_token headres and populate the requext context which is passed to services
17:31:18 sean-k-mooney oh so i need basiclly all of this right https://github.com/openstack/cinder/blob/master/cinder/compute/nova.py#L98-L118
17:32:19 gmaan yes, you can do all. so like cinder call nova based on what they call they can with pass privileged_user to true or not
17:32:22 sean-k-mooney and ideally cinder would have a glance section like the nova one but i could steal the nova option or the keystone_auth ones just to who it works
17:33:14 sean-k-mooney what im trying to show is that with a change to how cidner calls glance the revert is valid to show that its a cidner bug now a glance one
17:33:22 gmaan sean-k-mooney: that can be wrong but that is why I am checking in cinder code if they register the keystoneauth options for glance section also like they do for nova
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

Earlier   Later