On 14/08/2025 09:25, Josephine Seifert wrote:
Hi nova folks,
We worked on the LUKS-based image encryption and the patches Cinder and Glance are in a good state now + we also have tempest tests. Here you can see the overall state:
https://review.opendev.org/q/topic:%22LUKS-image-encryption%22
There is one patch in nova, which adresses an issue with keys for the encrypted image/volume being either used directly or being hexlified beforehand. We would be really happy if you could review this nova patch: https://review.opendev.org/c/openstack/nova/+/926326
So we can hopefully finish our work on the Image encryption. Procedurally, the problem is you do not have an approved spec or blueprint for this feature in Nova, and it has been several releases since we last discussed this feature. So technically, this is not eligible to merge this cycle. One of the things that is not supported in your series is direct booting of an encrypted image.
I'm not sure if we are okay with that limitation. In the long term, we are not however it was already there when using the now deprecated options so its not making it any worse then the current state. We had already started implementing support form booting form encrypted media in Nova and had to pause it when we found the CVEs related to VMDK, etc., two years ago. Since you are using some of the same image properties that were created for local storage encryption, we need to ensure that they do not conflict with the usage described in https://specs.openstack.org/openstack/nova-specs/specs/2024.2/approved/ephem... In any case, the patch is incorrect as it is using new image properties without extending https://github.com/openstack/nova/blob/master/nova/objects/image_meta.py#L15... Nova does not support arbitrary image properties. We have a defined set of standard image properties in code, and any other image property is considered invalid, so you should extend that with the new os_* ones. I'm not seeing any consideration for scheduling based on this capability (there is no compute service bump or traits), so we can't select a host that supports this when creating a VM or moving it. This would also need to work for all relevant lifecycle operations. Looking at the patch again, you mainly seem to be replacing existing functionality with delegation to os-brick https://review.opendev.org/c/openstack/nova/+/926326/3/nova/virt/libvirt/dri... However, you have not raised the min version of os-brick, so that needs to be addressed. Obviously, os-brick needs to be released with the new version, and that needs to be updated in upper-constraints before this can merge on the Nova side. Have you considered how this will work in a partly upgraded cloud where some nodes use the new approach and others use the old approach? 2025.2 is not a slurp, but we still need to support 2025.1 compute nodes, so it should not be possible to snapshot a VM on a 2025.2 host, then move it to a 2025.1 host and fail to restore from the snapshot via a rebuild. Based on https://review.opendev.org/c/openstack/nova/+/926326/3/nova/tests/unit/compu... I think you are doing that properly, but these are all the concerns that should have been addressed by a Nova spec. By the way, in addition to the work Melanie started for local image encryption, there was also https://review.opendev.org/c/openstack/nova-specs/+/883516/3/specs/2023.2/ap... to support NFS volumes with LUKS encryption. All three of these efforts need to work together in the long term although the other two are currently dormant but intended to be resumed in a future release. Even though this is a small patch it is quite a lot to ask for Nova to review, especially when it was not discussed at the PTG with the Nova or Cinder teams. https://etherpad.opendev.org/p/r.bf5f1185e201e31ed8c3adeb45e3cf6d https://etherpad.opendev.org/p/r.c8d78881250998a7afc9b0929c99a8bf There was a meeting slot for this in the Glance room https://etherpad.opendev.org/p/r.cd74ef1b45e2f338a145173a65b98315#L96 but there are no comments or minutes related to that discussion if it happened. There is a lot of context not captured in the patch that would be needed to properly understand this change. So, procedurally, I'm tempted to say -2 on the Nova change because you have not followed the feature proposal process and got aproval do make this chagne for this cycle. This is a non-trivial thing to show up with three weeks before feature freeze, when we are ~6 weeks past the spec/blueprint approval deadline for this cycle(m2). The only reason I have not done that is this is a relatively small patch, and if all the concerns I raised have been addressed, it might be okay to proceed with. I am not going to have time to dig into this enough to prove that to myself, so really this is up to the other cores if they have time. Part of the reason we have the blueprint and spec freeze at M2 is to prevent late requests like this so we can complete the work that was planned for the release. We have quite a few planned enhancements still pending, so I want to spend most of my energy trying to land those.
greetings Josephine (Luzi)