Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-06-22
18:01:32 sean-k-mooney oh time for a call...
18:19:53 dansmith melwitt: a while back I asked about getting admin credentials for glance and you pointed me to something I ignored because I decided I didn't need admin
18:20:03 dansmith melwitt: do you remember that and if so can you point me again?
18:26:35 melwitt heh, sec
18:29:19 melwitt dansmith: it might have been this commit https://github.com/openstack/nova/commit/aab4b7a0e2504c04e08389145bcb1414dea63631
18:29:58 melwitt just as an example of a place where we needed to use an admin cred to make a particular API call
18:34:08 dansmith melwitt: okay that's just a flag to the neutron client right?
18:34:47 dansmith I thought there was something more general
18:37:46 melwitt dansmith: yeah, I think when I linked you I was just saying, it is normal/expected for us to have to selectively use admin to call other APIs and that was a recent example of us doing it
18:38:33 dansmith oh, okay, that's common in a lot of places, yeah.. what I need is a way to get admin creds to talk to glance
18:38:54 dansmith I don't really know how we do that for neutron.. I think long ago we had credentials in our config, but that's gone now right?
18:41:28 melwitt I don't know off the top of my head. I thought we did have creds but I don't know about them being gone. I'm looking through the code now to see if it's obvious
18:42:34 dansmith I thought there was some service user thing we use now, but yeah I don't really know
18:53:03 melwitt based on this code block, there are supposed to be creds used from nova.conf https://github.com/openstack/nova/blob/f1ebc15dfc8ffb7f23b2cb9879f0ca9376931a90/nova/network/neutron.py#L191
19:01:42 melwitt and here's a config file from a nova-next run showing what look to be service user creds for neutron and placement https://zuul.opendev.org/t/openstack/build/785733b6379b40a5982f710a62302c21/log/controller/logs/etc/nova/nova_cell1_conf.txt#40
19:07:26 dansmith melwitt: sorry in three conversations here
19:07:38 dansmith melwitt: yeah, okay, I thought we had moved past that at some point, but it looks like not
19:07:40 melwitt np. I'm still gathering info
19:08:16 melwitt we implemented this https://specs.openstack.org/openstack/nova-specs/specs/ocata/implemented/use-service-tokens.html which says it should have docs for setting up the service user stuff in conf but I don't find any docs so far
19:08:23 dansmith glance is kinda half requiring admin to do the image copy-to-rbd thing.. if that's intentional, then we'll need admin creds for glance too, which really sucks
19:08:29 dansmith heh
19:08:56 dansmith maybe it's done and devstack is just still using the old method?
19:10:18 melwitt I'd think that unlikely...
19:10:31 melwitt this is what I find for a change that went into devstack to enable service tokens. https://review.opendev.org/#/c/409329/8/lib/nova
19:11:07 melwitt (I'm looking through https://review.opendev.org/#/q/topic:bp/use-service-tokens)
19:11:10 dansmith melwitt: come on, be optimistic with me! :P
19:11:16 melwitt lol :)
19:11:31 melwitt well, the service token stuff involves setting creds in conf
19:11:55 melwitt for the service user
19:11:58 dansmith hmm, okay maybe the service token still means we get creds, just not creds that are general purpose admins?
19:12:04 dansmith I thought it was better than that
19:12:08 melwitt how that's different I don't really know
19:13:14 sean-k-mooney dansmith: i dont think devstack conigure the service user stuff for us by default
19:13:33 sean-k-mooney i have not looked in a while but last time i did i did not see it
19:13:55 melwitt dansmith: so.... it "seems" like you would probably do a patch similar to this one https://review.opendev.org/410394
19:14:41 melwitt that ^ adds the nova-neutron interaction. and the groundwork was added in a prior patch for the nova-cinder interaction https://review.opendev.org/397399
19:15:09 dansmith ack yeah, okay
19:15:10 dansmith well, I guess I'll start by arguing that we shouldn't need admin to do this
19:15:49 dansmith melwitt: and just to stitch that stuff together, you think that when we do admin=True to the neutronclient, we're now getting the service user's auth token?
19:16:01 dansmith the logic on L135 there is a bit confusing
19:16:05 melwitt sean-k-mooney: we enable it in nova-next https://github.com/openstack/nova/blob/f1ebc15dfc8ffb7f23b2cb9879f0ca9376931a90/.zuul.yaml#L180
19:16:23 sean-k-mooney melwitt: ah ok but not in dansmith's job https://zuul.opendev.org/t/openstack/build/18e4701c1a374bf09269778479160f25/log/controller/logs/etc/nova/nova-cpu_conf.txt
19:16:54 sean-k-mooney there is no service_user group in the config
19:17:07 melwitt yeah you'd have to set the env var for the job
19:17:58 sean-k-mooney could you just add the glance credetials like we do for neutron https://zuul.opendev.org/t/openstack/build/18e4701c1a374bf09269778479160f25/log/controller/logs/etc/nova/nova-cpu_conf.txt#43-50
19:18:36 dansmith sean-k-mooney: that's what melwitt said above
19:18:40 sean-k-mooney service_user support was more fo the case where i am doing a long running request but my user token expired so we fallback to a admin service user instead
19:18:59 dansmith and I'm guessing the answer is yes, but I'd like to not have to do that (meaning not need to have admin for this)
19:19:57 sean-k-mooney ya given you dont own the image however im not surprised that glance is unhappy
19:20:14 sean-k-mooney you could argue that if its a public image then maybe this should be allowed
19:20:17 melwitt dansmith: yeah... agreed it looks confusing. but I think yeah, passing admin=True is having it load the auth plugin from conf, which presumably will pick up the [service_user] config section
19:20:34 dansmith sean-k-mooney: it's public and the API is letting me do the operation
19:20:42 sean-k-mooney melwitt: i dont think it will by default
19:21:08 sean-k-mooney dansmith: as in the api is accepting the import
19:21:14 dansmith right
19:21:22 sean-k-mooney ya so it feels like a glance but
19:21:25 sean-k-mooney *bug
19:21:40 sean-k-mooney but for now you might need to use admin to work around it
19:21:43 dansmith I hath already filed it thusly
19:22:16 sean-k-mooney :)
19:24:00 sean-k-mooney ok so ya it looks like we have the service auth support in the image module https://github.com/openstack/nova/blob/master/nova/image/glance.py#L68
19:25:54 sean-k-mooney without modifying noava code with admin=true on that call however its not going to elevate unless the token is expired so i guess you will have to do that too which kind of sucks
19:26:16 sean-k-mooney i mean i guess you can do that in the DNM patch
19:26:39 dansmith just setting admin=True on the nova context isn't going to do it
19:26:49 dansmith I'd have to actually get an admin-granted token from keystone
19:30:13 sean-k-mooney not on the context but when you create the client cant you just pass admin=true .e.g get_cline(ctx, admin=true)
19:30:48 sean-k-mooney like we do with neutron https://opendev.org/openstack/nova/src/commit/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/network/neutron.py#L397
19:32:31 sean-k-mooney although that is a custom get_client function https://opendev.org/openstack/nova/src/commit/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/network/neutron.py#L234-L257
19:34:44 sean-k-mooney i guess not the glance module does not ever use admin currently so it does not pass it to https://opendev.org/openstack/nova/src/commit/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/image/glance.py#L60
21:30:02 openstackgerrit Dan Smith proposed openstack/nova master: DNM: Try to make a glance multistore job https://review.opendev.org/734184
22:01:27 sean-k-mooney dansmith: oh you fixed it in glance by having it constuct the task factory with an admin context. that is much better then working around it in nova with an admin context
22:03:46 sean-k-mooney having the user download and reupload the image does seem very iniffiecnt.
22:04:34 sean-k-mooney a alternitive would be for the copy image api to create a new iamge which the current user now owns form the old image but that also does not fit with the current import api in my view
22:04:56 sean-k-mooney so ya i think your patch makes sense
22:09:08 dansmith sean-k-mooney: well, that's mostly just a minimal hammer approach to get past this block (I think).. as I hedged in the commit message, it may very well be that we should be only constructing the image pool with that admin context, or something more detailed
22:09:33 dansmith I haven't chased all the implications of doing this, I just put it up to try to move on and so someone can show me what the right way is, if indeed the user is supposed to be allowed to do this
22:09:41 dansmith totally possible that someone will say it should be admin-only
22:10:04 sean-k-mooney ya although the api in principal should have determined if yo are allowed do something before you get to that point
22:10:05 dansmith but you know, best way to get something done on the internet is to do it wrong so someone will fix it out of anger :)
22:10:09 dansmith agreed
22:10:17 dansmith if not this, then the api needs more checks
22:11:19 sean-k-mooney making image copy admin only would be strange as the other import methods are not and i think it would be the same policy endpoint?
22:12:17 dansmith well, it's a different case I think
22:12:32 dansmith the other import methods are for actually importing the image
22:12:41 dansmith this import being used for copy is a little bit weird,
22:13:08 dansmith because for the others, you wouldn't have one person create the image and another provide its data generally,
22:13:10 dansmith but that's a little bit of what this is,
22:13:19 dansmith but since it's controlled enough I would think that allowing one user that can use an image to copy it to another store in the system is fine,
22:13:30 dansmith just like I said.. I could download and re-upload it myself to get the same effect
22:13:40 sean-k-mooney i mean by extening the import workflow to me i think that implies that they are treating glance as just another data source like a url or file
22:13:44 dansmith and of course, if we want to limit some users from being able to do this, we need a more fine-grained policy knobv
22:13:56 dansmith correct,
22:14:25 dansmith but for the other import mechanisms, you wouldn't want a non-admin-or-owner to be able to do the import from-url or whatever
22:14:37 dansmith which maybe means we should only do this admin context thing if we're doing copy-to-store, I dunno
22:14:42 dansmith but I'll let them opine
22:14:55 sean-k-mooney dansmith: well not for an image they can see but not own
22:15:58 sean-k-mooney if i was to redsign this i would be tempted to invert the workflow and make the copy part of get
22:16:35 sean-k-mooney e.g. if i try to get an image form a store where it is not currently present have it be copied on the backend in paralle to streaming it to the user
22:17:01 dansmith personally I think that this should be a PUT /images/foo {'stores': ['new-store', 'existing-store']
22:17:25 dansmith I think they had a task-based approach to image importing so they put this in there since it was the minimal amount of work

Earlier   Later