| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-05-20 | |||
| 19:14:17 | sean-k-mooney | but not lvm | |
| 19:14:26 | melwitt | ok. so it seems then that we would hold off on introducing hw_ephemeral_encryption_format until we work on the lvm support | |
| 19:14:31 | melwitt | right | |
| 19:14:37 | sean-k-mooney | and the exisitng lvm fature will be deprecate but not removed until we can adopt them into the new framework | |
| 19:15:23 | sean-k-mooney | yes we would only need hw_ephemeral_encryption_format if we supprot somethign other hten luksv1 | |
| 19:16:11 | sean-k-mooney | which also means if os_glance_encrypt_format is set to anything other then luks we shoudl reject that too right | |
| 19:16:31 | melwitt | ok, I think we all agree about that. so I'll update the spec to explain this in a lot of detail 😂 and that hw_ephemeral_encryption_format will not be part of this first chunk and that it would come into play only when we work on adding support for lvm | |
| 19:17:02 | sean-k-mooney | and do we agree the same is true for the config option | |
| 19:17:22 | melwitt | I think it would be reasonable to reject that yes, if os_glance_encrypt_format is not luks. we won't be able to honor it | |
| 19:17:40 | sean-k-mooney | ok | |
| 19:18:15 | melwitt | I think I made the config option a choices= where the only choice is luks | |
| 19:18:17 | melwitt | but I'll double check and make it that if it isn't already | |
| 19:18:30 | sean-k-mooney | ok | |
| 19:18:40 | sean-k-mooney | if its only luks then i dont think we need it personally | |
| 19:20:13 | melwitt | that's a fair point. I suppose all it could potentially be helping with transparency but I could see the argument not to have it | |
| 19:20:23 | opendevreview | Merged openstack/nova master: Enable OCaaS for several nova jobs https://review.opendev.org/c/openstack/nova/+/919738 | |
| 19:20:26 | sean-k-mooney | i think what we are setteling on is we only need 1 extra spec/image property the boolean hw:ephemeral_encryption/hw_ephemeral_encryption | |
| 19:20:40 | melwitt | that is also my understanding | |
| 19:20:44 | sean-k-mooney | and if that is unset and the image in encyped then the vm would be encypted too | |
| 19:20:53 | sean-k-mooney | meaning you dont have to opt in for encypted images | |
| 19:21:04 | melwitt | yes, that's what I understand as well | |
| 19:21:20 | sean-k-mooney | so there is only one last bit i want to check | |
| 19:22:00 | sean-k-mooney | will we allow an opt out via the flavor i.e. hw:ephemeral_encryption=false and os_glance_encrypt_format=luks | |
| 19:22:06 | sean-k-mooney | should that be a 409 conflict | |
| 19:22:09 | sean-k-mooney | or decypt the image | |
| 19:22:15 | melwitt | the main thing is I think we want to avoid an unpleasant surprise on that. similar to the encrypted rescue image, not to just decrypt it | |
| 19:22:47 | melwitt | I think the desire based on current code review comments is that it would decrypt in that case | |
| 19:22:51 | sean-k-mooney | personally im feeling like hw:ephemeral_encryption=false and os_glance_encrypt_format=luks woudl be a 409 conflict at least for now | |
| 19:23:09 | melwitt | yeah, I think it would be fair to start with that | |
| 19:23:50 | sean-k-mooney | the reason i hesitate there is it would allow anyoen in the same project (realy that has access to the image secret) to trivially/acidentaly decypt the image | |
| 19:24:14 | melwitt | and tack on a separate patch that enables the ability to request decryption | |
| 19:24:18 | melwitt | because it's like a user experience thing. not really "required" but nice to have to give users that ability | |
| 19:24:23 | sean-k-mooney | its not uncommon for peopel to query flavors by size and select the first one that they find | |
| 19:24:30 | melwitt | yeah. that would be my only concern with it as well | |
| 19:25:03 | sean-k-mooney | to be fair if thte image had os_glance_encrypt_format=luks hw_ephemeral_encryption=ture | |
| 19:25:27 | sean-k-mooney | i think we all agree it should be a 409 conflict if the flavor has hw:ephemeral_encryption=false | |
| 19:26:00 | sean-k-mooney | but since the arguement is being made that os_glance_encrypt_format=luks effectivly implies hw_ephemeral_encryption=ture | |
| 19:26:33 | sean-k-mooney | im wonderin if we shoudl also imply it when comparing ot a flavor with hw:ephemeral_encryption=false | |
| 19:26:35 | melwitt | I expect we all agree that's an ok first step. we've been trying to keep patches smaller in this thing as well so I have been splitting smaller things like that into their own patch | |
| 19:26:49 | sean-k-mooney | ok | |
| 19:27:28 | sean-k-mooney | so before this converstaion i was tending to say "ok ill approve the spec and we can take it to the implemeation" no i feel like we were not on the same page but have got much closer | |
| 19:27:45 | sean-k-mooney | so my stomach say i shoudl have breakfast/dinner | |
| 19:27:55 | sean-k-mooney | but i also dont really want to leave this open | |
| 19:28:04 | sean-k-mooney | so melwitt how do you want to proceed | |
| 19:28:18 | melwitt | yeah so I was thinking if os_glance_encrypt_format=luks (and really mainly just the presence of os_glance_encrypt_key_id) then if hw:ephemeral_encryption=false is in direct conflict and would act the same as the image conflict. until we offer decryption later if we do that | |
| 19:29:29 | sean-k-mooney | ack i think that is logicaly consitent | |
| 19:30:03 | sean-k-mooney | i feel like os_glance_encrypt_key_id is only valid if all the other os_glance_encrypt_* are dfeind | |
| 19:30:15 | melwitt | sean-k-mooney: I think it would probably be best to update the spec now. either way I will update it -- if it gets approved as-is I will still have an amendment proposed shortly. to not forget all this stuff | |
| 19:30:17 | melwitt | so either way is fine with me | |
| 19:30:26 | melwitt | I agree. I think that is the intent in the standardized properties anyway | |
| 19:30:29 | sean-k-mooney | so its not clear to me if we shoudl be checking any spefici one of those keys or if any of them exist | |
| 19:32:16 | melwitt | yeah.. I'll probably add validation for that just to make a better experience. I'm not sure if they are all required to be set from a glance perspective. like, will glance reject upload of an image that only sets one or a subset of them? or maybe if it does allow it, it will fill it defaults itself | |
| 19:32:29 | melwitt | so either way on the nova side we would expect/require them all to be set if one is set | |
| 19:32:38 | sean-k-mooney | so the glance spec used ot have a seperate nova spec https://review.opendev.org/c/openstack/nova-specs/+/608696/11/specs/train/approved/image-encryption.rst | |
| 19:32:43 | sean-k-mooney | for how we would use those | |
| 19:33:27 | melwitt | from my perspective at a minimum if os_glance_encrypt_key_id is set, then os_glance_encrypt_format is required | |
| 19:33:31 | sean-k-mooney | in that they explictly talk about adding supprot for decypting the encyped image when booting the instnace | |
| 19:33:59 | sean-k-mooney | melwitt: ya i think those two are requried and likely the type as well i.e. symetic | |
| 19:34:15 | melwitt | ah ok | |
| 19:34:17 | sean-k-mooney | having said that i think we are only going to support symmetric encption | |
| 19:34:26 | melwitt | ah right | |
| 19:34:30 | sean-k-mooney | so i guess if its single valued | |
| 19:34:34 | sean-k-mooney | it does not matter for now | |
| 19:36:01 | sean-k-mooney | melwitt: you know what would help | |
| 19:36:14 | melwitt | yeah, we can adapt that as we go along. if we think more than just those two should be required to be present to be valid | |
| 19:36:18 | melwitt | because the other use case is an individual user uploading an encrypted image | |
| 19:36:26 | sean-k-mooney | in your spec can you define the only values of the os_glance_encrypt* properties that will actully work for now | |
| 19:36:28 | melwitt | so not sure if they will be required to provide all image properties | |
| 19:37:53 | sean-k-mooney | like will you be using os_glance_decrypt_size? | |
| 19:38:11 | sean-k-mooney | i dont think you are your using virt_size + 1G in the check right | |
| 19:38:15 | melwitt | sure I can add specifics about which ones we are requiring | |
| 19:38:35 | melwitt | there is a check for that somewhere actually | |
| 19:38:39 | sean-k-mooney | well it might hjsut be good to say hay nova is not using os_glance_decrypt_size | |
| 19:39:12 | melwitt | today we validate image size in some way and I had to add the "plus overhead" to the calc if encrypted for it to pass | |
| 19:39:19 | sean-k-mooney | right | |
| 19:39:41 | sean-k-mooney | but if i rememebr reviewing that code we do not use os_glance_decrypt_size | |
| 19:40:22 | melwitt | yeah I guess really i could use os_glance_decrypt_size instead for the validation if the image is encrypted. since that's what we actually care about, is if the decrypted size will fit in the chosen flavor | |
| 19:40:46 | melwitt | no we didn't have os_glance* properties proposed at the time | |
| 19:40:49 | sean-k-mooney | os_glance_decrypt_size and os_glance_decrypt_container_format i think are for the usecase of booting form an encypted iamge and decypting it on the host | |
| 19:40:56 | melwitt | well, sorry | |
| 19:41:03 | sean-k-mooney | which would imply they expect the virtual_size to include the overhead | |
| 19:41:06 | melwitt | they were proposed already but we were not planning to use them initially | |
| 19:41:09 | sean-k-mooney | when its encypted | |
| 19:41:24 | melwitt | but at the ptg we agreed to use them so I will be changing where needed to use those instead of what I had put | |
| 19:41:52 | sean-k-mooney | sure but os_glance_decrypt_size is defied as "size after payload decryption" | |
| 19:42:07 | sean-k-mooney | which would be the size of the data insid ethe luks encyption | |
| 19:42:15 | sean-k-mooney | not the size on disk fo the encypted images | |
| 19:42:45 | sean-k-mooney | so i dont think it actully helps us | |
| 19:42:52 | melwitt | right.. maybe I'm getting confused | |
| 19:43:11 | sean-k-mooney | i think this is explcitly for the usecase of booting non encypted instance form an encypted image | |
| 19:43:26 | melwitt | the point I was meaning really is I'll use them when applicable | |
| 19:43:26 | sean-k-mooney | so that we woudl know how big it is after we strip the encyption from it | |
| 19:43:34 | sean-k-mooney | ack | |
| 19:44:04 | sean-k-mooney | ok ill chat ot you tomorrow o/ | |
| 19:44:14 | melwitt | ok I see what you're saying | |
| 19:44:18 | melwitt | ok, seeya tomorrow o/ | |
| 21:41:25 | opendevreview | Merged openstack/nova master: Enable virtio-scsi in nova-next https://review.opendev.org/c/openstack/nova/+/918458 | |
| #openstack-nova - 2024-05-21 | |||
| 04:36:36 | opendevreview | melanie witt proposed openstack/nova-specs master: Re-propose specs for ephemeral encryption https://review.opendev.org/c/openstack/nova-specs/+/907654 | |
| 06:05:22 | opendevreview | xielijie proposed openstack/nova master: Fix Timestamp Inconsistency During VM Creation https://review.opendev.org/c/openstack/nova/+/920074 | |
| 07:50:16 | opendevreview | Andrew Bonney proposed openstack/nova master: Set port dns_name when unshelving instances https://review.opendev.org/c/openstack/nova/+/919961 | |