| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-07-25 | |||
| 13:01:57 | andreykurilin | sean-k-mooney: thanks for reply. “custom api policy” - yes and no. I can simplify the case - restricting to manipulate security groups for ‘less-privileged’ roles. It does not require a custom policy and reuses everything ‘builtin’. addSecurityGroup action reraises neutron’s policy error which gives end-user enough information what is going on, versus 500 by removeSecurityGroup | |
| 13:10:41 | andreykurilin | Neutron plugin I mentioned previously is entirely on neutron’s project side. It is not nova specific, i.e., it does not modify/extend nova-api. I just tried to give more context about what I’m trying to achieve. it looks like I gave more misleading info rather than useful one :) | |
| 13:12:03 | sean-k-mooney | andreykurilin: right btu nova does not supprot neutron srbac funcitonaltiy or any non default policy/roles | |
| 13:13:12 | sean-k-mooney | andreykurilin: so https://docs.openstack.org/neutron/latest/admin/config-rbac.html is not supproted by nova because the netruon folks never created a spec or worked with use to add support | |
| 13:13:35 | sean-k-mooney | we ocationaly get one of request ot enable subset of it | |
| 13:14:01 | sean-k-mooney | but there has never been an effort to actully supprot it or update it when neturon adds new fucntionaltiy | |
| 13:14:54 | sean-k-mooney | andreykurilin: if you want to use any fucntionaly restricte by that then you cannot use nova proxy apis anymore | |
| 13:15:17 | sean-k-mooney | you have to do the secrity group config via neutron api only | |
| 13:16:09 | andreykurilin | sean-k-mooney: yes, I understand that nova should not care about neutron rbac. The only problem for me here is that the end user who sees a 500 internal error (with fill out a bug report message) is less happy than someone who sees a propagated error from neutron. As I mentioned, one method already does this. | |
| 13:16:49 | sean-k-mooney | what error code does neutron return in this case | |
| 13:16:52 | andreykurilin | 400 | |
| 13:17:47 | sean-k-mooney | 400 bad request is technially not correct for a permission denial | |
| 13:17:56 | sean-k-mooney | do you have an example of the neutorn error respocne you can post | |
| 13:18:18 | andreykurilin | I could be incorrect, let me recheck quickly | |
| 13:19:09 | andreykurilin | ps: I was referencing https://github.com/openstack/nova/blob/master/nova/network/security_group_api.py#L635-L638 versus https://github.com/openstack/nova/blob/master/nova/network/security_group_api.py#L695-L699 | |
| 13:19:55 | sean-k-mooney | so its partly because removing a secrity group is never ment to be somethign that can fail | |
| 13:21:20 | sean-k-mooney | we may be able to handel the 400 case there safely | |
| 13:21:35 | sean-k-mooney | we woudl need to knwo if there is any other case that neutron returns a 400 | |
| 13:21:50 | sean-k-mooney | but we are only updatign the securit_group part of the port in that requst | |
| 13:21:54 | sean-k-mooney | so its proably ok | |
| 13:23:01 | sean-k-mooney | i dont like that we are returing a 400 form nova in this case by the way | |
| 13:24:03 | sean-k-mooney | it really should be a 403 | |
| 13:24:47 | andreykurilin | sean-k-mooney: rechecked, I was incorrect, sorry. Neutron actually raises 403. I thought that my simplified case for custom neutron logic is similar to policy errors | |
| 13:25:25 | sean-k-mooney | ok so a 403 is what i would expect becuase this is being deigned based on the permission of the roles the request was made with | |
| 13:26:51 | sean-k-mooney | so nova should be handleling that gracefuly and returning a 403 as well so i think we can catch that and provide a better error message | |
| 13:27:30 | andreykurilin | But what about 400 ? :) Are you still ok with handling it the same way as in addSecurityGroup ? | |
| 13:27:31 | sean-k-mooney | but i would also consider this to be skirting what is and is not in scope to fix in nova | |
| 13:28:10 | sean-k-mooney | andreykurilin: i would prefer that to also be a 403 but that woudl require a microverison update | |
| 13:28:16 | sean-k-mooney | and im not sure we want that | |
| 13:29:14 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/exception.py#L1054 | |
| 13:29:31 | sean-k-mooney | that unfortunetly inhrits form invlaide and default to a 400 as a result | |
| 13:30:04 | sean-k-mooney | that was a bug form when we first added the proxy api https://github.com/openstack/nova/commit/c3ed3dfcf99e9162616960bad5c7b7a36d3b2453 | |
| 13:30:28 | sean-k-mooney | we have got better at using sematicly correct http respoce codes since then | |
| 13:30:50 | andreykurilin | Yes, it is 400. It is set separately here https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/security_groups.py#L430 | |
| 13:32:07 | sean-k-mooney | so if we want to avoid a micor version bump we might be forced to use 400 https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/security_groups.py#L432 | |
| 13:32:26 | sean-k-mooney | although i belive we are alwasy allowed to return a 401 and 403 | |
| 13:32:34 | sean-k-mooney | btu we shoudl disucss that with gmann ^ | |
| 13:32:46 | sean-k-mooney | andreykurilin: if you want to create a patch for this do | |
| 13:33:11 | sean-k-mooney | we can discuss the exacat details of the respoce code and message content as part of the review | |
| 13:34:14 | andreykurilin | sean-k-mooney: great, thanks! I already have a patch for handling 400 as a part of removeSecurityGroup action https://review.opendev.org/c/openstack/nova/+/924399 . I’ll add 403 handling there as well | |
| 13:35:14 | sean-k-mooney | can you actully file a bug for this too | |
| 13:35:29 | sean-k-mooney | and add a release note please. | |
| 13:35:39 | andreykurilin | sure | |
| 13:35:39 | sean-k-mooney | we would ideally have both if we are going to backport this | |
| 14:09:36 | dansmith | JayF: TheJulia: Someone told me that ironic is using glance's ARI format in some way, presumably to store your ramdisk image, is that right? | |
| 14:11:24 | opendevreview | Merged openstack/nova stable/2023.2: Change force_format strategy to catch mismatches https://review.opendev.org/c/openstack/nova/+/924733 | |
| 14:15:56 | TheJulia | dansmith: yeah, aki and ari for the raw image artifacts set by an admin to facilitate ops, they never ever get handed off to a conversion operation | |
| 14:16:39 | dansmith | TheJulia: okay, I feel like that's the wrong image type to be using.. what gets stored in the aki? | |
| 14:17:02 | sean-k-mooney | those are not required as far as i am aware by the way | |
| 14:17:04 | dansmith | oh, the kernel for a pxeboot operation? | |
| 14:17:14 | TheJulia | dansmith: yes | |
| 14:17:27 | sean-k-mooney | what i ment by that is using ari and aki | |
| 14:17:38 | sean-k-mooney | to store the ramdisk and kernel images | |
| 14:17:42 | dansmith | the reason is that AKI, ARI are not just kernel and ramdisk, they are *amazon* kernel and ramdisk, which at least when it was defined meant "a paravirt kernel" and "a ramdisk with paravirt drivers in it" | |
| 14:17:58 | dansmith | that said, TheJulia, those things never flow through nova at all, is that right? | |
| 14:18:04 | sean-k-mooney | i dont think there is any ckecking that the ramdisk image and kernel iamge uuid are in those formats | |
| 14:18:12 | TheJulia | dansmith: well, it is possible to do, given how they can be constructed | |
| 14:18:15 | dansmith | asking because I want to (thermo) nuke AMI,ARI,AKI from nova | |
| 14:18:16 | TheJulia | dansmith: never flow through nova | |
| 14:18:28 | dansmith | okay | |
| 14:19:08 | frickler | dansmith: sean-k-mooney: I lost track a bit, are there fixes already that should make https://review.opendev.org/c/openstack/nova/+/924734 pass or does that still need further investigation? or just enough rechecks? ;) | |
| 14:19:33 | sean-k-mooney | dansmith: TheJulia nova support suing sepreate ram/kernel disk indepenent of the format of the root disk as far as im aware | |
| 14:19:44 | sean-k-mooney | so if you took an ami and uploaed all 3 parts as raw | |
| 14:19:50 | sean-k-mooney | it would work | |
| 14:20:10 | sean-k-mooney | as would uploading a qcow as qcow and refeince seperate kerenl/ram disk | |
| 14:20:10 | TheJulia | sean-k-mooney: yeah, it *should* and I would expect it to actually | |
| 14:20:18 | TheJulia | I think this goes back to age old convention | |
| 14:20:19 | dansmith | sean-k-mooney: yeah, I'm sure we're not doing any specific checking on the aki,ari bits | |
| 14:20:30 | TheJulia | set like 10 years ago because people didn't think of raw | |
| 14:20:39 | sean-k-mooney | to me the only purposue today is lableing them as not bootable | |
| 14:20:44 | TheJulia | perfect | |
| 14:20:45 | sean-k-mooney | for filtering the images | |
| 14:21:05 | dansmith | TheJulia: no, raw has been around forever, it's because the disk image of an ARI,AKI booted thing is not a disk | |
| 14:21:25 | TheJulia | dansmith: indeed and correct | |
| 14:21:47 | TheJulia | ari/aki has also been in the docs for ages for some human delination purpose I never understood | |
| 14:21:57 | sean-k-mooney | frickler: checkign | |
| 14:22:03 | TheJulia | other than it is an explicit kernel and explicit ramdisk | |
| 14:22:19 | dansmith | TheJulia: it's not for humans specifically, it's for amazon compatibility which has long since gone away in direct form | |
| 14:22:31 | sean-k-mooney | frickler: i think there is a requirement repo change to bump packaging to the 2023.2 version to fix the setuptools issues | |
| 14:22:49 | TheJulia | dansmith: wasn't explicitly aware of that, but likely humans just didn't know to update ironic's docs. | |
| 14:23:03 | dansmith | TheJulia: virtual machines booted this way don't use a bootloader on the disk and are sort of warm booted from a pre-crafted memory state | |
| 14:23:21 | frickler | sean-k-mooney: ah, that was merged two days ago, so I'll try to recheck then https://review.opendev.org/c/openstack/requirements/+/924764 | |
| 14:23:34 | sean-k-mooney | yep that the patch i was looking for | |
| 14:23:36 | dansmith | TheJulia: right, I think the human problem is that people saw "ramdisk" and "kernel" and thought "oh these are what I should use" without considering what the "amazon-" prefix meant in context | |
| 14:23:36 | sean-k-mooney | perfect | |
| 14:24:00 | dansmith | TheJulia: anyway, the thing I wanted to confirm is that if we were to drop all that special behavior from nova it's not going to impact ironic and it sounds like that's true | |
| 14:24:16 | TheJulia | dansmith: yeah, its just a doc issue really | |
| 14:24:27 | TheJulia | dansmith: correct | |
| 14:24:51 | dansmith | TheJulia: you mean a doc issue because you could easily store all those in 'raw' images and it would all work fine right? | |
| 14:26:13 | TheJulia | yeah | |
| 14:26:16 | dansmith | ack | |
| 14:26:56 | TheJulia | at least *should* afaik | |
| 14:27:30 | TheJulia | I would expect it to in other words | |
| 14:28:25 | sean-k-mooney | unless ye have also specical cased it for some archane reason im not sure why it woudl not | |
| 14:28:48 | sean-k-mooney | the format you mark somethign in in glance really does not change the content of the data | |
| 14:29:08 | dansmith | right, glance is going to start using inspector to enforce the content of the stream at some point, | |
| 14:29:29 | dansmith | and if we were to get particular about it, we'd need to decide what we're going to accept for those formats | |
| 14:32:41 | TheJulia | Can the format be changed post upload? | |
| 14:32:59 | dansmith | no | |
| 14:34:11 | TheJulia | oh joy | |