| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-22 | |||
| 17:53:22 | sean-k-mooney | that would be the logical thing to do yes | |
| 17:53:29 | sean-k-mooney | but manybet its not | |
| 17:53:58 | sean-k-mooney | https://github.com/openstack/glance/commit/1754c9e2b085ba0fc37a4369488c92a40268997a add the copy image support so im just skiming it quickly to see what it does | |
| 18:01:23 | sean-k-mooney | home ok i dont see how the propery gets updated in that but i also dont know how glance works internally so its not suprising. | |
| 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 | |