)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"8ce3e87ed3008ce974244e8d315739f19fbf3633","unresolved":true,"context_lines":[{"line_number":17,"context_line":"Registers and unregisters encrypted images as secret consumers in the"},{"line_number":18,"context_line":"Key Manager upon image upload and image deletion accordingly."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Implements https://specs.openstack.org/openstack/glance-specs/specs/2024.2/approved/glance/standardized_image_encryption.html"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Co-Authored-By: Josephine Seifert \u003cjosephine.seifert@cloudandheat.com\u003e"},{"line_number":23,"context_line":"Change-Id: I3a030b1e044653a6470377f58eeed5baf779b7c3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"e4126fb0_f0db728c","line":20,"range":{"start_line":20,"start_character":11,"end_line":20,"end_character":124},"updated":"2026-05-18 09:29:10.000000000","message":"Let\u0027s link the recent approved spec: https://specs.openstack.org/openstack/glance-specs/specs/2026.2/approved/glance/standardized_image_encryption.html","commit_id":"f460329a350dddb16a5006a5c2a2618c6098d99d"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a10d4c3f32820542d1b73fb03c271b89c286f692","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"06641dd2_f5ee0045","updated":"2024-08-29 14:23:30.000000000","message":"Adding my -1 for visibility even though it\u0027s not specifically aimed at anything technical in this patch.","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"6d987a407000daee2c3de5f901bb2d172b19047e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5c5e1ed5_d3e6a9ee","updated":"2024-08-29 08:10:10.000000000","message":"I think we also need a detailed documentation for this.","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"dfeca7325e67962fdb5c9bf993557d716949b479","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"36d128ff_8788b29a","updated":"2024-08-29 07:07:54.000000000","message":"Please add the releasenote as well.","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ddfe8a739659d47e5b041808c8ccb084af0356b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b2009ec8_93fef8e2","updated":"2024-08-29 11:26:06.000000000","message":"Thank you for the review Pranali. I will look into the things you mentioned. Can you elaborate on the documentation request in the other comment?","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"943a92f287b64e88fb79d2f056c9bfe7dbeb95f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f9d9f80c_94467e60","in_reply_to":"36d128ff_8788b29a","updated":"2024-11-11 14:54:33.000000000","message":"Added.","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ddfe8a739659d47e5b041808c8ccb084af0356b6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"08c2074e_933855fe","in_reply_to":"5c5e1ed5_d3e6a9ee","updated":"2024-08-29 11:26:06.000000000","message":"Which kind of documentation do you have in mind? More architecture documentation (beyond the property explanations currently in the patchset) or guides on how to use this feature as an end user or consuming service?","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ff5b088589e4e36530d133a28034d1868a2e3f8c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d3f9ec2b_2dd9aa4c","updated":"2024-11-11 14:53:39.000000000","message":"I added unit test coverage for all exception branches of `ImagesController._register_as_secret_consumer`.","commit_id":"269a79ab5498c54f22bd0ce1b1a37d6afb27202c"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"155d19a3f61ed534acf19ec7c737facb210d416d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"f3888964_6297f30f","updated":"2024-12-16 19:46:55.000000000","message":"A few questions inline, I also agree with Abhishek\u0027s comments.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"6140f6a24fcc4d15541da9498f8a754b59d6d035","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"3d86ad02_8ec504ab","updated":"2024-11-22 09:19:31.000000000","message":"I think you can separate out doc changes in different patch","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"3a27aa86c0732a3ec0c8606218bb848f1b3e4d19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"94bc74bb_2ed8cff1","updated":"2025-02-28 21:45:06.000000000","message":"The test_default_disk_formats test needs to be updated to expect luks now.","commit_id":"e9b24e8d4c898e6b4bce5b9701a851a343773023"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"b99ff6cefc4a8da4f8703f054540466f8ef5e095","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"a6750b09_c9b5e441","updated":"2025-03-03 14:24:15.000000000","message":"Unfortunately we are at the end of the cycle, so we need to wait until next cycle starts to get this merged.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"c97955f07d41b19ddce263602ef9e12175e5ddf1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"d01d0d58_ca2b8132","updated":"2025-04-09 09:36:10.000000000","message":"I\u0027ll need to look into the image import methods to come up with a proper implementation if we want to add image encryption support for glance-direct and/or copy-image in this patchset as well.\n\nSo far, we simply excluded encrypted images from any of the import methods in this patchset to keep things simple, only allowing direct image upload for encrypted images.\nBut as Abishek rightfully commented, support for at least copy-image and/or glance-direct might be desired.","commit_id":"224527ac7f9906af443dd130047efa598e660e9f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"94cd66e0_ce3aa9c9","updated":"2025-04-09 09:06:06.000000000","message":"Thank you Rajat and Abishek for your reviews! I addressed a lot of your points in the latest change.","commit_id":"224527ac7f9906af443dd130047efa598e660e9f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"79c07f17c8b74e93a2a2a6a27ada8d0225a99146","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"49dd12f8_8d542fe7","updated":"2025-05-23 15:36:09.000000000","message":"I added support for image import using copy-image, glance-direct and web-download methods for encrypted images.","commit_id":"a785c567b8019c0ff3e45dd72919e02321c69729"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7f068464944b1724807ee6b33d28cb928c6c0df6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"41b072fe_c2a9fa24","updated":"2025-06-25 13:50:25.000000000","message":"Core functionality looks good to me, Need to check unit tests in detail","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f30834aff89a5ffc3e6e0a48255f660b7598c85d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"1a7d4247_7127de62","updated":"2025-08-28 15:46:16.000000000","message":"I\u0027m still a bit concerned about the ambiguity of knowing what is inside the encrypted container. Battle scars from the mega-CVE around disk formats...","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"63105792bf2e4401d3850e676f9294d058eade60","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"d04a3e6b_0e53f50a","updated":"2025-07-08 16:26:37.000000000","message":"This looks pretty good.\n\n1) I would like to wait for Dan to be back so he can continue the discussion about the format\n\n2) There are still lots of references to cinder_encryption_* in the tests. Is that on purpose?","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"95c6111113908a013140b5ec8a26748e29f40ac3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"2505a494_635e33ab","updated":"2026-05-13 14:37:09.000000000","message":"recheck to get fresh logs","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"0a6e881c9084f3293b084ec0cd0c54683aa2a431","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":14,"id":"54c50dc5_4b5d4ac3","in_reply_to":"d04a3e6b_0e53f50a","updated":"2025-07-10 14:00:21.000000000","message":"Regarding point 2)\nWith the introduction of the os_encrypt_\\* attributes we are entering a deprecation period for the cinder_encryption_\\* attributes.\n\nSo far, Cinder was the only producer of encrypted images (by snapshotting LUKS-encrypted volumes). In existing environments, there still might be older images originally created by Cinder with these old encryption attributes.\n\nWe do have a secondary patchset adding database migrations [1] but I can think of at least two edge cases here: a) Glance and Cinder not being upgraded at the same time (version mismatch for a period of time), b) images getting restored from an arbitrary backup solution including old metadata.\n\nFor the time being we need to make sure that the old attributes are still properly supported during a deprecation timeframe. That\u0027s why I think we should keep the related tests for now.\n\n[1] https://review.opendev.org/c/openstack/glance/+/926905","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"00357639d573edcc6147d87a955c38fa3f3b0ccf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"6242adf0_ff885754","updated":"2025-09-04 13:35:15.000000000","message":"Add","commit_id":"759bdc64390e22e2376832123791418ad9feae63"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"5249d137733ad73e22e1fbc5dad68cdbe3bdfd9b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"bb100378_231f00b2","updated":"2025-11-18 16:10:14.000000000","message":"This and the Cinder patchset have been updated to reflect the spec changes discussed during the PTG on 2025-10-30.\n\nIn short:\n\n* `luks` removed from `disk_format` enum, instead added to `container_format` enum\n* non-QCOW2 LUKS-encrypted images are now represented by `container_format\u003dluks` instead of `disk_format\u003dluks`, freeing up `disk_format` to further specify the inner payload format\n* `os_encrypt_disk_format` property removed as it is now represented by `disk_format`","commit_id":"6539876c8e94242af8bb4033fde39cf284c5bfc2"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"7e362741f4cf4c16d05ec7dd44d67199d9f1347f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"e4062020_59a48786","updated":"2025-12-09 15:53:09.000000000","message":"I added user documentation in `doc/source/user/image-encryption.rst`.","commit_id":"9f6d081765d934fe5a23492b00e69d299c01c031"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"633a824a_8da8a228","updated":"2025-12-12 18:22:51.000000000","message":"I haven\u0027t gone through the tests yet, but wanted to get an initial pass of the actual implementation before I disappear for the end of the year.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3840e7fa44cf058f0c110eae450c76d49274f42a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"29253c6f_4b1d0e31","updated":"2025-12-11 13:03:09.000000000","message":"recheck","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"8ce3e87ed3008ce974244e8d315739f19fbf3633","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"4637b8e9_c5887010","updated":"2026-05-18 09:29:10.000000000","message":"Need to fix the assertion failures as well in the unit tests.","commit_id":"f460329a350dddb16a5006a5c2a2618c6098d99d"}],"doc/source/user/common-image-properties.rst":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1ddead510e9df0443a80a7c0c3d5518dc2e2d3f3","unresolved":true,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"os_decrypt_size"},{"line_number":64,"context_line":"  Specifies the resulting image size after decryption. This property can be"},{"line_number":65,"context_line":"  used by consuming services to ensure enough space is aviailable and allocated"},{"line_number":66,"context_line":"  before attempting to decrrypt the image."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"os_encrypt_key_deletion_policy"}],"source_content_type":"text/x-rst","patch_set":11,"id":"6d048f73_c368015c","line":65,"range":{"start_line":65,"start_character":55,"end_line":65,"end_character":65},"updated":"2025-03-11 01:49:20.000000000","message":"nit: available","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"os_decrypt_size"},{"line_number":64,"context_line":"  Specifies the resulting image size after decryption. This property can be"},{"line_number":65,"context_line":"  used by consuming services to ensure enough space is aviailable and allocated"},{"line_number":66,"context_line":"  before attempting to decrrypt the image."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"os_encrypt_key_deletion_policy"}],"source_content_type":"text/x-rst","patch_set":11,"id":"9dcaebe1_05d91749","line":65,"range":{"start_line":65,"start_character":55,"end_line":65,"end_character":65},"in_reply_to":"6d048f73_c368015c","updated":"2025-04-09 09:06:06.000000000","message":"Done","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"1ddead510e9df0443a80a7c0c3d5518dc2e2d3f3","unresolved":true,"context_lines":[{"line_number":63,"context_line":"os_decrypt_size"},{"line_number":64,"context_line":"  Specifies the resulting image size after decryption. This property can be"},{"line_number":65,"context_line":"  used by consuming services to ensure enough space is aviailable and allocated"},{"line_number":66,"context_line":"  before attempting to decrrypt the image."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"os_encrypt_key_deletion_policy"},{"line_number":69,"context_line":"  States the condition under which the Image Service will delete the object"}],"source_content_type":"text/x-rst","patch_set":11,"id":"97f11f71_cd9ab7a1","line":66,"range":{"start_line":66,"start_character":23,"end_line":66,"end_character":31},"updated":"2025-03-11 01:49:20.000000000","message":"nit: decrypt","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[{"line_number":63,"context_line":"os_decrypt_size"},{"line_number":64,"context_line":"  Specifies the resulting image size after decryption. This property can be"},{"line_number":65,"context_line":"  used by consuming services to ensure enough space is aviailable and allocated"},{"line_number":66,"context_line":"  before attempting to decrrypt the image."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"os_encrypt_key_deletion_policy"},{"line_number":69,"context_line":"  States the condition under which the Image Service will delete the object"}],"source_content_type":"text/x-rst","patch_set":11,"id":"6bee21f4_8248f371","line":66,"range":{"start_line":66,"start_character":23,"end_line":66,"end_character":31},"in_reply_to":"97f11f71_cd9ab7a1","updated":"2025-04-09 09:06:06.000000000","message":"Done","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"}],"doc/source/user/formats.rst":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f30834aff89a5ffc3e6e0a48255f660b7598c85d","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"e5033260_5dac08d4","line":86,"updated":"2025-08-28 15:46:16.000000000","message":"Can we say something stronger about what the encapsulated format is? It\u0027d be great if we can say that \"this is a luks-encrypted GPT disk image\". Because \"raw\" is, as we know, a loaded term and can mean anything in its current form from a whole disk, to a partition image (i.e. just a filesystem) to a tarball.\n\nI\u0027m not sure that LUKS itself cares about the blob that it wraps, but for the purposes of glance, specificity would be helpful. So, if nova (or any other consumer) \"mounts\" or unwraps the LUKS and finds anything other than a GPT disk, we consider it invalid and bail. Is that reasonable?","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"4d3d4d2cc3a0875e59e7f6a283ccf4d893f69418","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"c8f6f55d_84b93a5d","line":86,"in_reply_to":"168cd4cc_247d2550","updated":"2025-09-04 19:52:06.000000000","message":"Note that while qemu-img calls in glance were the front-line concern for CVE-2024-32498, the bigger issue is that passing any unvetted image type to any of the qemu tools runs the risk that its type autodetection heuristics feed the image through an unsafe qemu driver with the ability to execute arbitrary code with the privileges of whatever user/process is running those tools... up to and including attempts to boot a VM from the image.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"00357639d573edcc6147d87a955c38fa3f3b0ccf","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"168cd4cc_247d2550","line":86,"in_reply_to":"61bb3e37_9d7da033","updated":"2025-09-04 13:35:15.000000000","message":"We\u0027re currently waiting on the Cinder team to catch up here and provide feedback about some of the cinder-\u003eglance specific issues. So yes, I\u0027m keenly aware of the challenges there :)","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"5249d137733ad73e22e1fbc5dad68cdbe3bdfd9b","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"8d8f5c1c_33ed008d","line":86,"in_reply_to":"6c5f8ad2_239feae2","updated":"2025-11-18 16:10:14.000000000","message":"As per PTG discussion, this is now changed to switch to `container_format` for specifying `luks`. Now `disk_format` can be used to specify `raw` or `gpt`, for instance.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3aa861c5011ee87de7aaf2bb76482bbcbc913213","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"6c5f8ad2_239feae2","line":86,"in_reply_to":"b4ae9b37_c9df4b9f","updated":"2025-09-11 14:34:14.000000000","message":"In summary, I see multiple challenges with the restrictions you are suggesting:\n\n1. converting volumes to images in Cinder: if Glance were to reject images with \"raw\" os_decrypt_disk_format (i.e. arbitrary encryption payload block data), some volume contents may be rejected without proper user feedback (images simply disappear / never appear) due to current workflow/UX issues\n2. is it technically possible and feasible to inspect *encrypted* contents of images to check their inner payload format (e.g. to check for gpt header), including huge TB-sized images, in the format inspector?\n3. currently Glance still accepts raw for regular images and there is no hard restriction limiting it to just gpt *yet* either as far as I am a aware\n\nI do understand your concerns but I worry that other parts are not ready yet, especially the Cinder \u003c-\u003e Glance interaction, to properly enforce a gpt-only format restriction on encrypted image payloads yet.\n\nWould it be viable alternative to make `os_decrypt_disk_format` mandatory but still allowing it to be `raw` for the time being?\nThis way, once the other necessary changes and improvements land, the attribute will already be there and just needs to be further restricted to add the additional hardening.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"293a484e19476e0632bf9bb980f18cad74016bf6","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"b4ae9b37_c9df4b9f","line":86,"in_reply_to":"c8f6f55d_84b93a5d","updated":"2025-09-04 20:01:52.000000000","message":"Yup. And we tend to focus on the case where we don\u0027t explicitly tell qemu-img (or qemu itself) what the image format is, which is a whole class of problems. However, we still have to validate the safety of images we _do_ know the format of before we hand them to either qemu or qemu-img.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"10dca2b03c23b871f382111c90e6abcd6faf0cd8","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"f71765d8_a68ee47d","line":86,"in_reply_to":"e5033260_5dac08d4","updated":"2025-09-02 19:25:57.000000000","message":"I just tried the following using an unencrypted volume type:\n\n1. as a regular tenant, create an empty volume in Cinder, an instance in Nova and attach the volume to the instance via `openstack server add volume ...`\n2. from within the instance, write arbitrary data to the block device directly (`echo \"HELLO\" \u003e /dev/vdb`)\n3. use `openstack image create --volume ...` to dump the volume to an image\n4. download the image via `openstack image save --file dump.raw ...` and inspect it using `head -c5 dump.raw`\n\n(I did this with unencrypted volume type to speed things up and avoiding the hassle of having to do all the decrypting after dumping the image locally)\n\nResult: it prints \"HELLO\" as the very first bytes of the image, no partition header or anything.\n\nWe have no control over what a tenant will do to their block devices from within a VM. Some weird software might do raw block writing or some esoteric formatting.\nAs a result, I think we cannot expect or enforce any format: when Cinder uploads a volume to an encrypted image it may produce image payload that has no recognizable data encapsulated in the encryption at any time.\n\nWith \"recognizable data\" I mean data that we can actually verify/interpret on Glance/Cinder side. Maybe the tenant is using OpenStack-unrelated application-layer encryption on the raw block device from within the VM. Then it would like garbage data to us.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ef7b8fa4f9c170a2c0874a4ab9bfe067ed62abf9","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"61bb3e37_9d7da033","line":86,"in_reply_to":"f033228a_b2f47f42","updated":"2025-09-04 08:37:06.000000000","message":"I understand your concerns given the past CVE. I think there are a few aspects to this that are important to consider.\n\n1)\nRegarding CVE-2024-32498: it is my current understanding (please correct me if I\u0027m wrong) that the vulnerability was in qemu-img calls outside of the hypervisor when converting/checking the image, not the converted image\u0027s contents doing things from within the guest in the hypervisor.\nWith an encrypted image, the format presented to the qemu-img handling workflow is the encapsulating encryption, currently either raw LUKS or qcow2+LUKS. The inner payload (encrypted data) is only present and used within the guest in the hypervisor, unless we are calling qemu-img conversion repeatedly/recursively which I\u0027m not aware of.\n\n2)\nLet\u0027s assume we have a qcow2+LUKS encrypted image. This has `container_format\u003dbare` and `disk_format\u003dqcow2` with `os_encrypt_*` parameters attached. If I understand your suggestion correctly, you now want it to additionally have a (mandatory) `os_encrypt_disk_format\u003dgpt` if it contains a GPT formatted partition within the encryption and want Glance to inspect the file and verify that it actually contains the GPT partition.\nIn contrast to unencrypted images, you won\u0027t be able to just look at the first few bytes of the file to verify this. You\u0027d have to decrypt it first.\nI\u0027m worried that we don\u0027t have the means to do a partial/streamed decryption of the LUKS (especially qcow2+LUKS) formats without needing to decrypt the full image, just to look at the first few bytes of it.\nI fear that with images potentially multiple TB in size, this could prove very costly and may even exceed resources available to the control plane nodes Glance is running on.\n\n3)\nRegarding the spec you linked and the statement about stop accepting raw data on image upload, please also consider the interaction between Cinder and Glance here.\nWhen `openstack image create --volume ...` (`os-volume_upload_image` action in Cinder API) is used, the image will start in `uploading` state. If anything goes wrong during image creation or upload, the image will vanish without a trace. There is no error state the image can enter and no exception message for the user to inspect (e.g. with `openstack image show` since the image is auto-deleted immediately).\nIf Glance rejects the image upload of an existing volume because the user did something to the volume which Glance doesn\u0027t like, the user will be left without a clue as to why images from some of their volumes can be created but not for others. In my humble opinion, that is the worst UX I can imagine from a user\u0027s point of view.\nTo clarify, I\u0027m not saying we should give up and simply always accept arbitrary data in Glance but the current interaction between Cinder and Glance is not able to handle this gracefully in a way that the user is properly informed that they knowingly or even unknowingly did something to their volume from within the VM that makes it now unacceptable to be exported as an image.\n\nPoint 3 is not directly related to this patchset and a bit out of scope but I wanted to bring it up for you to be aware of it.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"de59a2479ef58216f97a7c54466dc6d697cf9f64","unresolved":true,"context_lines":[{"line_number":83,"context_line":"luks"},{"line_number":84,"context_line":"  The `LUKS \u003chttps://en.wikipedia.org/wiki/Linux_Unified_Key_Setup\u003e`_ format"},{"line_number":85,"context_line":"  is an encrypted disk format. In contrast to the ``qcow2`` format with LUKS"},{"line_number":86,"context_line":"  encryption extension, this format is a raw LUKS block data image."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"The `AKI/AMI/ARI"},{"line_number":89,"context_line":"\u003chttp://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AMIs.html\u003e`_"}],"source_content_type":"text/x-rst","patch_set":14,"id":"f033228a_b2f47f42","line":86,"in_reply_to":"f71765d8_a68ee47d","updated":"2025-09-03 14:10:19.000000000","message":"You might want to read this if not familiar:\n\nhttps://specs.openstack.org/openstack/glance-specs/specs/2025.1/approved/glance/glance-as-defender.html\n\nWe\u0027ve been \"encouraged\" by qemu devs to stop using \"raw\" as a format and always be very direct when interacting with an image in terms of knowing what format it is ahead of time. Glance now rejects images that claim to be one thing but are another, and even has the ability to detect and enforce than an image is in gpt disk format with pending patches to add a new such format. My long-term plan is to get nova to stop agreeing to boot anything claiming to be \"raw\" and treat that as \"I don\u0027t know what this is, but I\u0027m trying to boot an instance, which requires a disk image not a random binary blob\".\n\nSo yes, I understand that today you can do anything you want in a raw, but I think we need to _stop_ accepting that behavior. I have the scars from CVE-2024-32498 to remind me that it\u0027s important.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"}],"doc/source/user/image-encryption.rst":[{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"8ce3e87ed3008ce974244e8d315739f19fbf3633","unresolved":true,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":"``os_encrypt_key_deletion_policy``"},{"line_number":107,"context_line":"   If this property is set to ``on_image_deletion``, the secret referenced by"},{"line_number":108,"context_line":"   ``os_encrypted_key_id`` will be deleted when the image is deleted."},{"line_number":109,"context_line":"   Otherwise, the image will only be deregistered as a secret consumer from"},{"line_number":110,"context_line":"   the secret upon image deletion."},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":29,"id":"85cd91e5_4a8563cd","line":108,"range":{"start_line":108,"start_character":5,"end_line":108,"end_character":24},"updated":"2026-05-18 09:29:10.000000000","message":"os_encrypt_key_id ?","commit_id":"f460329a350dddb16a5006a5c2a2618c6098d99d"}],"etc/metadefs/glance-common-image-props.json":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f30834aff89a5ffc3e6e0a48255f660b7598c85d","unresolved":true,"context_lines":[{"line_number":69,"context_line":"        },"},{"line_number":70,"context_line":"        \"os_encrypt_cipher\": {"},{"line_number":71,"context_line":"            \"title\": \"Image Encryption Cipher\","},{"line_number":72,"context_line":"            \"description\": \"Identifies the cipher suite used to encrypt the image. In conjunction with \u0027os_encrypt_format\u0027, this property determines which mechanism for decryption is to be used by consuming services.\","},{"line_number":73,"context_line":"            \"type\": \"string\""},{"line_number":74,"context_line":"        },"},{"line_number":75,"context_line":"        \"os_decrypt_container_format\": {"}],"source_content_type":"application/json","patch_set":14,"id":"c510478d_e1413836","line":72,"updated":"2025-08-28 15:46:16.000000000","message":"Can we be more specific about this? Are you thinking this is just something like \"aes256\" or one of the other more complicated specifiers like \"aes256+cbc+dh\" or whatever? I guess I\u0027m afraid of this being too unconstrained.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"f332d9c7f860d3386a9e1d221f00199ea96657c1","unresolved":true,"context_lines":[{"line_number":69,"context_line":"        },"},{"line_number":70,"context_line":"        \"os_encrypt_cipher\": {"},{"line_number":71,"context_line":"            \"title\": \"Image Encryption Cipher\","},{"line_number":72,"context_line":"            \"description\": \"Identifies the cipher suite used to encrypt the image. In conjunction with \u0027os_encrypt_format\u0027, this property determines which mechanism for decryption is to be used by consuming services.\","},{"line_number":73,"context_line":"            \"type\": \"string\""},{"line_number":74,"context_line":"        },"},{"line_number":75,"context_line":"        \"os_decrypt_container_format\": {"}],"source_content_type":"application/json","patch_set":14,"id":"69a92a6b_065f95ed","line":72,"in_reply_to":"a1246253_c81d75ba","updated":"2025-09-03 10:00:18.000000000","message":"For now, I\u0027ve adjusted the property description to better reflect its intention.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"6e769f3149758e0873e7c7e72bb3ab6b0e08ea61","unresolved":true,"context_lines":[{"line_number":69,"context_line":"        },"},{"line_number":70,"context_line":"        \"os_encrypt_cipher\": {"},{"line_number":71,"context_line":"            \"title\": \"Image Encryption Cipher\","},{"line_number":72,"context_line":"            \"description\": \"Identifies the cipher suite used to encrypt the image. In conjunction with \u0027os_encrypt_format\u0027, this property determines which mechanism for decryption is to be used by consuming services.\","},{"line_number":73,"context_line":"            \"type\": \"string\""},{"line_number":74,"context_line":"        },"},{"line_number":75,"context_line":"        \"os_decrypt_container_format\": {"}],"source_content_type":"application/json","patch_set":14,"id":"a1246253_c81d75ba","line":72,"in_reply_to":"c510478d_e1413836","updated":"2025-09-02 15:11:48.000000000","message":"This was originally intended to design the standard around the possibility for future extensions of encryption format support. Back when the standard was designed around GPG still, we actually needed this, but that isn\u0027t the case anymore:\n\nAs it stands right now with both the raw LUKS and qcow2+LUKS formats, this field is not set by Cinder (e.g., when uploading LUKS volume to image) and not interpreted by the consumin implementations. The reason is that everything is encoded in the qcow2 and LUKS headers.\nFor the currently supported formats, this ends up as optional and purely informational metadata but is not actually checked/used by OpenStack components,  like algorithm and key length for secrets in Barbican.\n\nIf we were to add a format that isn\u0027t LUKS and doesn\u0027t encode this information as part of the header/payload in the future, we might need this additional information to be able to properly decrypt the data.\n\nI see two feasible ways to go from there:\n\n1) remove \"os_encrypt_cipher\" from the current patchset entirely and leave any future format support for dedicated (future) patchsets to address\n\n2) keep \"os_encrypt_cipher\" as an optional attribute to be future-proof + adjust its description to better reflect the optional nature of it\n\nA third option could be to enforce it also for the LUKS-based formats but I would not be a fan of forcibly setting it to a value in the current implementation when it isn\u0027t actually considered by the involved implementations at all.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f30834aff89a5ffc3e6e0a48255f660b7598c85d","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        \"os_decrypt_container_format\": {"},{"line_number":76,"context_line":"            \"title\": \"Image Container Format after Decryption\","},{"line_number":77,"context_line":"            \"description\": \"Specifies the resulting Image Container Format after the image is decrypted.\","},{"line_number":78,"context_line":"            \"type\": \"string\""},{"line_number":79,"context_line":"        },"},{"line_number":80,"context_line":"        \"os_decrypt_size\": {"},{"line_number":81,"context_line":"            \"title\": \"Image Container Format after Decryption\","}],"source_content_type":"application/json","patch_set":14,"id":"2b920d1c_873a568a","line":78,"updated":"2025-08-28 15:46:16.000000000","message":"Hmm, in reference to my comment on the previous file, perhaps this is what you\u0027re expecting to use to indicate what we should find inside the LUKS (in this example)? Since this is `container_format` I\u0027m thinking maybe this is something different. If it\u0027s not, we probably want to make this `disk_format`.\n\nI guess I sort of want to make this super mandatory, and also make it clear that this needs to be one of our existing format (container or disk, based on the above) keys and not just any random string.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"f332d9c7f860d3386a9e1d221f00199ea96657c1","unresolved":true,"context_lines":[{"line_number":75,"context_line":"        \"os_decrypt_container_format\": {"},{"line_number":76,"context_line":"            \"title\": \"Image Container Format after Decryption\","},{"line_number":77,"context_line":"            \"description\": \"Specifies the resulting Image Container Format after the image is decrypted.\","},{"line_number":78,"context_line":"            \"type\": \"string\""},{"line_number":79,"context_line":"        },"},{"line_number":80,"context_line":"        \"os_decrypt_size\": {"},{"line_number":81,"context_line":"            \"title\": \"Image Container Format after Decryption\","}],"source_content_type":"application/json","patch_set":14,"id":"b24cc23f_eb9969af","line":78,"in_reply_to":"2b920d1c_873a568a","updated":"2025-09-03 10:00:18.000000000","message":"I agree with the naming. I\u0027ve renamed it to `os_decrypt_disk_format`. This is more in line with the existing terminology.\n\nRegarding making it mandatory or restricting the values, see my other comment.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"5249d137733ad73e22e1fbc5dad68cdbe3bdfd9b","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        \"os_decrypt_container_format\": {"},{"line_number":76,"context_line":"            \"title\": \"Image Container Format after Decryption\","},{"line_number":77,"context_line":"            \"description\": \"Specifies the resulting Image Container Format after the image is decrypted.\","},{"line_number":78,"context_line":"            \"type\": \"string\""},{"line_number":79,"context_line":"        },"},{"line_number":80,"context_line":"        \"os_decrypt_size\": {"},{"line_number":81,"context_line":"            \"title\": \"Image Container Format after Decryption\","}],"source_content_type":"application/json","patch_set":14,"id":"233702b6_4f2cee50","line":78,"in_reply_to":"b24cc23f_eb9969af","updated":"2025-11-18 16:10:14.000000000","message":"Since we moved to `container_format` for specifying LUKS now, this has moved to the regular `disk_format` property and `os_decrypt_disk_format` has been removed entirely.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":69,"context_line":"        },"},{"line_number":70,"context_line":"        \"os_encrypt_cipher\": {"},{"line_number":71,"context_line":"            \"title\": \"Image Encryption Cipher\","},{"line_number":72,"context_line":"            \"description\": \"Specifies cipher suite details used to encrypt the image. If present in addition to \u0027os_encrypt_format\u0027, this property helps selecting the correct mechanism parameters for decryption to be used by consuming services.\","},{"line_number":73,"context_line":"            \"type\": \"string\""},{"line_number":74,"context_line":"        },"},{"line_number":75,"context_line":"        \"os_decrypt_size\": {"}],"source_content_type":"application/json","patch_set":23,"id":"76dd94f7_843fe5cc","line":72,"updated":"2025-12-12 18:22:51.000000000","message":"I guess I\u0027m still a bit confused here. If this is specified and doesn\u0027t match what we pull out of the LUKS header, then we reject? And... under what cases will this actually be used/needed?\n\nIt sounds to me based on your replies to the other comments that this is not needed or used in the current form, but is there to \"future proof\" ... something ... but I\u0027m not sure what case that would be. I\u0027d prefer to not define something we don\u0027t really know about until we get to the point of needing it.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        },"},{"line_number":70,"context_line":"        \"os_encrypt_cipher\": {"},{"line_number":71,"context_line":"            \"title\": \"Image Encryption Cipher\","},{"line_number":72,"context_line":"            \"description\": \"Specifies cipher suite details used to encrypt the image. If present in addition to \u0027os_encrypt_format\u0027, this property helps selecting the correct mechanism parameters for decryption to be used by consuming services.\","},{"line_number":73,"context_line":"            \"type\": \"string\""},{"line_number":74,"context_line":"        },"},{"line_number":75,"context_line":"        \"os_decrypt_size\": {"}],"source_content_type":"application/json","patch_set":23,"id":"a8ad42dd_95cee6d9","line":72,"in_reply_to":"76dd94f7_843fe5cc","updated":"2025-12-16 10:30:37.000000000","message":"Last time this came up I suggested either keeping it for future extension or removing it altogether. Since we never mention this in the spec or anywhere else, I agree we should prefer not exposing it here yet.\n\nRemoved.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"}],"glance/api/v2/images.py":[{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"dfeca7325e67962fdb5c9bf993557d716949b479","unresolved":true,"context_lines":[{"line_number":117,"context_line":"            LOG.debug(msg)"},{"line_number":118,"context_line":"            self._key_manager.add_consumer(context, encryption_key_id,"},{"line_number":119,"context_line":"                                           consumer_data)"},{"line_number":120,"context_line":"        except castellan_exception.Forbidden as e:"},{"line_number":121,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":122,"context_line":"                   e.message)"},{"line_number":123,"context_line":"            raise webob.exc.HTTPForbidden(explanation\u003dmsg)"},{"line_number":124,"context_line":"        except (castellan_exception.ManagedObjectNotFoundError) as e:"},{"line_number":125,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":126,"context_line":"                   e.message)"}],"source_content_type":"text/x-python","patch_set":3,"id":"340c9ff0_298c020e","line":123,"range":{"start_line":120,"start_character":8,"end_line":123,"end_character":58},"updated":"2024-08-29 07:07:54.000000000","message":"Does this have unit test coverage?","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ff5b088589e4e36530d133a28034d1868a2e3f8c","unresolved":false,"context_lines":[{"line_number":117,"context_line":"            LOG.debug(msg)"},{"line_number":118,"context_line":"            self._key_manager.add_consumer(context, encryption_key_id,"},{"line_number":119,"context_line":"                                           consumer_data)"},{"line_number":120,"context_line":"        except castellan_exception.Forbidden as e:"},{"line_number":121,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":122,"context_line":"                   e.message)"},{"line_number":123,"context_line":"            raise webob.exc.HTTPForbidden(explanation\u003dmsg)"},{"line_number":124,"context_line":"        except (castellan_exception.ManagedObjectNotFoundError) as e:"},{"line_number":125,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":126,"context_line":"                   e.message)"}],"source_content_type":"text/x-python","patch_set":3,"id":"180c1443_06cf5a2d","line":123,"range":{"start_line":120,"start_character":8,"end_line":123,"end_character":58},"in_reply_to":"340c9ff0_298c020e","updated":"2024-11-11 14:53:39.000000000","message":"Done","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"dfeca7325e67962fdb5c9bf993557d716949b479","unresolved":true,"context_lines":[{"line_number":125,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":126,"context_line":"                   e.message)"},{"line_number":127,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":128,"context_line":"        # ValueError is thrown by barbicanclient if secret UUID is malformed"},{"line_number":129,"context_line":"        except ValueError as e:"},{"line_number":130,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":131,"context_line":"                   str(e))"},{"line_number":132,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":133,"context_line":"        # consider unreachable/broken Key Manager fatal and abort"},{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"}],"source_content_type":"text/x-python","patch_set":3,"id":"5be287af_5cef6fd6","line":132,"range":{"start_line":128,"start_character":8,"end_line":132,"end_character":59},"updated":"2024-08-29 07:07:54.000000000","message":"ditto","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ff5b088589e4e36530d133a28034d1868a2e3f8c","unresolved":false,"context_lines":[{"line_number":125,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":126,"context_line":"                   e.message)"},{"line_number":127,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":128,"context_line":"        # ValueError is thrown by barbicanclient if secret UUID is malformed"},{"line_number":129,"context_line":"        except ValueError as e:"},{"line_number":130,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":131,"context_line":"                   str(e))"},{"line_number":132,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":133,"context_line":"        # consider unreachable/broken Key Manager fatal and abort"},{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"}],"source_content_type":"text/x-python","patch_set":3,"id":"aa7a4298_8991ab80","line":132,"range":{"start_line":128,"start_character":8,"end_line":132,"end_character":59},"in_reply_to":"5be287af_5cef6fd6","updated":"2024-11-11 14:53:39.000000000","message":"Done","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a10d4c3f32820542d1b73fb03c271b89c286f692","unresolved":true,"context_lines":[{"line_number":171,"context_line":"        image_factory \u003d self.gateway.get_image_factory(req.context)"},{"line_number":172,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":173,"context_line":"        try:"},{"line_number":174,"context_line":"            self._validate_encryption_parameters(extra_properties)"},{"line_number":175,"context_line":"            is_encrypted \u003d extra_properties.get(\u0027os_encrypt_key_id\u0027, False)"},{"line_number":176,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":177,"context_line":"            # visibility to private because secrets in the Key Manager are"}],"source_content_type":"text/x-python","patch_set":3,"id":"50dfa714_ee004341","line":174,"updated":"2024-08-29 14:23:30.000000000","message":"This is sort of an awkward place to raise this, but:\n\nI think something we learned from the recent mega-CVE is that I think we need a new `disk_format` for luks-encrypted images. So much of the complexity of handling that CVE came from all the side vectors by which you can fool nova, glance, and cinder into doing something bad by saying an image is in one format, but actually sending another. Specifically, I think we have got to stop allowing `raw` to be both a catch-all format, as well as the thing we use when we really mean \"an image of a PC-like disk\".\n\nHaving to probe `raw` images to see if they smell like a LUKS disk, and if not, assume it\u0027s a regular raw is inviting more possibility for issue here I think.\n\nI\u0027ve got a spec proposed to make glance inspect and reject uploads that do not conform to the stated `disk_format` (i.e. you said it was `raw`, but uploaded a `vmdk` -\u003e fail). It\u0027s here:\n\nhttps://review.opendev.org/c/openstack/glance-specs/+/925111\n\nand I have a LUKS inspector proposed against format_inspector:\n\nhttps://review.opendev.org/c/openstack/oslo.utils/+/926809\n\nthose two things together will basically require that you declare a thing as `luks` before uploading it. One could also make the argument that we should use `container_format\u003dluks` and `disk_format\u003dgpt` for the typical arrangement, but I think that\u0027s more complicated. Either way, some discussion is required, IMHO.","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"38ecc283aeee39d61262e40139000ec5ead5a20e","unresolved":true,"context_lines":[{"line_number":171,"context_line":"        image_factory \u003d self.gateway.get_image_factory(req.context)"},{"line_number":172,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":173,"context_line":"        try:"},{"line_number":174,"context_line":"            self._validate_encryption_parameters(extra_properties)"},{"line_number":175,"context_line":"            is_encrypted \u003d extra_properties.get(\u0027os_encrypt_key_id\u0027, False)"},{"line_number":176,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":177,"context_line":"            # visibility to private because secrets in the Key Manager are"}],"source_content_type":"text/x-python","patch_set":3,"id":"88bd2c2f_f13d3649","line":174,"in_reply_to":"50dfa714_ee004341","updated":"2024-11-11 12:50:54.000000000","message":"As discussed during the vPTG on 2024-10-24, we will now introduce `disk_format\u003dluks` for raw LUKS images. Furthermore, we\u0027ll extend `disk_format\u003dqcow2` with encryption support by introducing the `os_encrypt_*` properties to it, including `os_encrypt_format\u003dLUKSv1`.\n\nAs a result, we end up with the following 2 possible encrypted image formats:\n\n- `disk_format\u003dluks` + `os_encrypt_format\u003dLUKSv1` \u003d LUKS v1 raw image\n- `disk_format\u003dqcow2` + `os_encrypt_format\u003dLUKSv1` \u003d QCOW2 image containing LUKS v1 encrypted data\n\nBoth can be identified by looking at the image metadata and can also be validated by looking at the first chunks (QCOW2 or LUKS headers).","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"5249d137733ad73e22e1fbc5dad68cdbe3bdfd9b","unresolved":true,"context_lines":[{"line_number":171,"context_line":"        image_factory \u003d self.gateway.get_image_factory(req.context)"},{"line_number":172,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":173,"context_line":"        try:"},{"line_number":174,"context_line":"            self._validate_encryption_parameters(extra_properties)"},{"line_number":175,"context_line":"            is_encrypted \u003d extra_properties.get(\u0027os_encrypt_key_id\u0027, False)"},{"line_number":176,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":177,"context_line":"            # visibility to private because secrets in the Key Manager are"}],"source_content_type":"text/x-python","patch_set":3,"id":"4936f16f_e534369f","line":174,"in_reply_to":"88bd2c2f_f13d3649","updated":"2025-11-18 16:10:14.000000000","message":"As a result of the PTG discussion on 2025-10-30 this is now slightly changed to:\n\n* `container_format\u003dluks disk_format\u003draw` + `os_encrypt_format\u003dLUKSv1` \u003d LUKS v1 raw image\n* `container_format\u003dluks disk_format\u003dgpt` + `os_encrypt_format\u003dLUKSv1` \u003d LUKS v1 image with GPT partition inside\n* `disk_format\u003dqcow2` + `os_encrypt_format\u003dLUKSv1` \u003d QCOW2 image containing LUKS v1 encrypted data","commit_id":"7e1761e36a8fb0ecddb8709aefae3ebe321a0b48"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"155d19a3f61ed534acf19ec7c737facb210d416d","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        props \u003d image.extra_properties"},{"line_number":100,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":101,"context_line":"        # deprecation period is over."},{"line_number":102,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":103,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":104,"context_line":"        if encryption_key_id is None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"25169fdf_6425e131","line":101,"range":{"start_line":101,"start_character":32,"end_line":101,"end_character":36},"updated":"2024-12-16 19:46:55.000000000","message":"Are we planning on making it easy for our users to move away from using cinder_encryption_key_id?","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3bce4e940a40af422328e0aad594987f068f0c46","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        props \u003d image.extra_properties"},{"line_number":100,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":101,"context_line":"        # deprecation period is over."},{"line_number":102,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":103,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":104,"context_line":"        if encryption_key_id is None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"33add8a8_232357ff","line":101,"range":{"start_line":101,"start_character":32,"end_line":101,"end_character":36},"in_reply_to":"25169fdf_6425e131","updated":"2025-02-26 15:07:01.000000000","message":"You mean for existing images that have been created by Cinder in the past? Like auto-updating the metadata key name in Glance\u0027s database?","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"b592f2e4102bc338fec4c45df675ce4a25ccdd83","unresolved":true,"context_lines":[{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        props \u003d image.extra_properties"},{"line_number":100,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":101,"context_line":"        # deprecation period is over."},{"line_number":102,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":103,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":104,"context_line":"        if encryption_key_id is None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"8b0b84d7_0a4b1ff6","line":101,"range":{"start_line":101,"start_character":32,"end_line":101,"end_character":36},"in_reply_to":"33add8a8_232357ff","updated":"2025-03-03 13:25:12.000000000","message":"fwiw, there is a dedicated patchset for migrating existing database entries to the new naming: https://review.opendev.org/c/openstack/glance/+/926905\n\nDoes this address your concern?","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"63105792bf2e4401d3850e676f9294d058eade60","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        \"\"\""},{"line_number":99,"context_line":"        props \u003d image.extra_properties"},{"line_number":100,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":101,"context_line":"        # deprecation period is over."},{"line_number":102,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":103,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":104,"context_line":"        if encryption_key_id is None:"}],"source_content_type":"text/x-python","patch_set":9,"id":"a103f3ff_3c465b95","line":101,"range":{"start_line":101,"start_character":32,"end_line":101,"end_character":36},"in_reply_to":"8b0b84d7_0a4b1ff6","updated":"2025-07-08 16:26:37.000000000","message":"Acknowledged","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2450a18a0dd1e2eccd45fe5735e55c9f058695e1","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        consumer_data \u003d {"},{"line_number":109,"context_line":"            \u0027service\u0027: \u0027glance\u0027,"},{"line_number":110,"context_line":"            \u0027resource_type\u0027: \u0027image\u0027,"},{"line_number":111,"context_line":"            \u0027resource_id\u0027: image.image_id"},{"line_number":112,"context_line":"        }"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"ab65a0b0_ab98aa7a","line":111,"range":{"start_line":111,"start_character":27,"end_line":111,"end_character":41},"updated":"2024-11-22 09:40:59.000000000","message":"I think you can directly pass image_id to this method","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"1f902e0fa2862ee81265ba596d6b8d89e708417d","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        consumer_data \u003d {"},{"line_number":109,"context_line":"            \u0027service\u0027: \u0027glance\u0027,"},{"line_number":110,"context_line":"            \u0027resource_type\u0027: \u0027image\u0027,"},{"line_number":111,"context_line":"            \u0027resource_id\u0027: image.image_id"},{"line_number":112,"context_line":"        }"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"e1641fb1_1a534312","line":111,"range":{"start_line":111,"start_character":27,"end_line":111,"end_character":41},"in_reply_to":"1f629cfb_06dcee02","updated":"2024-11-22 13:34:34.000000000","message":"Ack, I thought extra properties are passed as separate argument","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":28271,"name":"Josephine Seifert","email":"josephine.seifert@cloudandheat.com","username":"josei"},"change_message_id":"231cef6fd47b386e8af1ccc60f22128022776bf7","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        consumer_data \u003d {"},{"line_number":109,"context_line":"            \u0027service\u0027: \u0027glance\u0027,"},{"line_number":110,"context_line":"            \u0027resource_type\u0027: \u0027image\u0027,"},{"line_number":111,"context_line":"            \u0027resource_id\u0027: image.image_id"},{"line_number":112,"context_line":"        }"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f629cfb_06dcee02","line":111,"range":{"start_line":111,"start_character":27,"end_line":111,"end_character":41},"in_reply_to":"ab65a0b0_ab98aa7a","updated":"2024-11-22 12:55:51.000000000","message":"We need to also inspect the image properties, because registering is only necessary, when there is an encryption_key in the image properties.\n\nThe encryption key is present in either the \u0027os_encrypt_key_id\u0027 or \u0027cinder_encryption_key_id\u0027.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2450a18a0dd1e2eccd45fe5735e55c9f058695e1","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1496fa15_ea1e261f","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"updated":"2024-11-22 09:40:59.000000000","message":"This will return 500 error, so is it expected?","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"155d19a3f61ed534acf19ec7c737facb210d416d","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"2cc624dd_2e873b48","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"in_reply_to":"1496fa15_ea1e261f","updated":"2024-12-16 19:46:55.000000000","message":"Indeed, in what cases would KeyManagerError be thrown?","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"63105792bf2e4401d3850e676f9294d058eade60","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"f28feb81_5e67880c","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"in_reply_to":"2732ef27_0330a2be","updated":"2025-07-08 16:26:37.000000000","message":"@Abhishek so are we fine with throwing 500 or should we change something?","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3bce4e940a40af422328e0aad594987f068f0c46","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"333cb7c2_50d50844","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"in_reply_to":"2cc624dd_2e873b48","updated":"2025-02-26 15:07:01.000000000","message":"Looking at Castellan\u0027s current implementation [^1], any authentication error, HTTP client or server error that is not about the referenced secret not existing will raise an HTTPServerError in Castellan.\n\nIf the request gets this far, I think we can assume that the user is properly authenticated and authorized, given the preceeding process steps.\nLooking at all the other `except` clauses above this block, I think we can be pretty sure about an internal error (500) if we reach this statement as we caught everything else already above (bad secret UUID, secret not found, user not permitted to access that specific secret).\n\nDo you have any specific case in mind where reaching this spot might still be a user/client error (non-500) instead?\n\n[^1]: https://github.com/openstack/castellan/blob/660658fe1539ddbb959f0e73d0b0df35fd2e7571/castellan/key_manager/barbican_key_manager.py#L667-L675","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d442317bc1d94942abf25ab42427fd02cb0b1a4b","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"cb162a30_bc8909a7","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"in_reply_to":"333cb7c2_50d50844","updated":"2025-06-19 16:40:01.000000000","message":"No such scenario, I was just flagging that we were raising 500 error which we generally don\u0027t","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"501df8b1dcf0ca66696e17549e75f1e26a3f6587","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"487d0c2b_966f91b1","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"in_reply_to":"81a7af80_9ef33d22","updated":"2025-08-28 13:39:28.000000000","message":"Acknowledged","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"beee4ffd00e0c0933c425f4b0c5884b740664c53","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"2732ef27_0330a2be","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"in_reply_to":"cb162a30_bc8909a7","updated":"2025-07-08 13:33:39.000000000","message":"What would Glance generally do in such case? We can change the exception type if necessary. I wouldn\u0027t mind.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9040542b4f323621b43f96206dc5bb9b88eac908","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":135,"context_line":"            msg \u003d (\"Unable to register image as secret consumer: %s\" %"},{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"81a7af80_9ef33d22","line":137,"range":{"start_line":137,"start_character":28,"end_line":137,"end_character":43},"in_reply_to":"f28feb81_5e67880c","updated":"2025-07-10 13:59:12.000000000","message":"Looks good as is","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2450a18a0dd1e2eccd45fe5735e55c9f058695e1","unresolved":true,"context_lines":[{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def _validate_encryption_parameters(self, extra_properties):"},{"line_number":142,"context_line":"        all_keys \u003d {\u0027os_encrypt_format\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"9ca927db_6733bf77","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":14},"updated":"2024-11-22 09:40:59.000000000","message":"No need of this statement","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3bce4e940a40af422328e0aad594987f068f0c46","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                   e.message)"},{"line_number":137,"context_line":"            raise webob.exc.HTTPServerError(explanation\u003dmsg)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"        return"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def _validate_encryption_parameters(self, extra_properties):"},{"line_number":142,"context_line":"        all_keys \u003d {\u0027os_encrypt_format\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"6e7e0868_a4777ea1","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":14},"in_reply_to":"9ca927db_6733bf77","updated":"2025-02-26 15:07:01.000000000","message":"Removed.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"155d19a3f61ed534acf19ec7c737facb210d416d","unresolved":true,"context_lines":[{"line_number":141,"context_line":"    def _validate_encryption_parameters(self, extra_properties):"},{"line_number":142,"context_line":"        all_keys \u003d {\u0027os_encrypt_format\u0027,"},{"line_number":143,"context_line":"                    \u0027os_encrypt_key_deletion_policy\u0027,"},{"line_number":144,"context_line":"                    \u0027os_encrypt_key_id\u0027, \u0027os_decrypt_size\u0027,"},{"line_number":145,"context_line":"                    \u0027os_decrypt_container_format\u0027}"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        # if none of the encryption parameters were specified, this image does"}],"source_content_type":"text/x-python","patch_set":9,"id":"0bf307c9_1da15096","line":144,"range":{"start_line":144,"start_character":42,"end_line":144,"end_character":57},"updated":"2024-12-16 19:46:55.000000000","message":"It\u0027s easy to miss os_decrypt_size here, please put it on its own line.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3bce4e940a40af422328e0aad594987f068f0c46","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    def _validate_encryption_parameters(self, extra_properties):"},{"line_number":142,"context_line":"        all_keys \u003d {\u0027os_encrypt_format\u0027,"},{"line_number":143,"context_line":"                    \u0027os_encrypt_key_deletion_policy\u0027,"},{"line_number":144,"context_line":"                    \u0027os_encrypt_key_id\u0027, \u0027os_decrypt_size\u0027,"},{"line_number":145,"context_line":"                    \u0027os_decrypt_container_format\u0027}"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        # if none of the encryption parameters were specified, this image does"}],"source_content_type":"text/x-python","patch_set":9,"id":"3dee823b_083977ba","line":144,"range":{"start_line":144,"start_character":42,"end_line":144,"end_character":57},"in_reply_to":"0bf307c9_1da15096","updated":"2025-02-26 15:07:01.000000000","message":"Adjusted.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2450a18a0dd1e2eccd45fe5735e55c9f058695e1","unresolved":true,"context_lines":[{"line_number":421,"context_line":"                    \"os_encrypt_key_deletion_policy\""},{"line_number":422,"context_line":"                ]"},{"line_number":423,"context_line":"            ):"},{"line_number":424,"context_line":"                msg \u003d _(\"Image with encryption properties cannot be target \""},{"line_number":425,"context_line":"                        \"for import\")"},{"line_number":426,"context_line":"                raise exception.Conflict(msg)"},{"line_number":427,"context_line":"            if image.status \u003d\u003d \u0027active\u0027 and import_method !\u003d \"copy-image\":"},{"line_number":428,"context_line":"                msg \u003d _(\"Image with status active cannot be target for import\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"f7227bc4_cf8a0e83","line":425,"range":{"start_line":424,"start_character":25,"end_line":425,"end_character":36},"updated":"2024-11-22 09:40:59.000000000","message":"Image with encryption properties is not allowed for import.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"3bce4e940a40af422328e0aad594987f068f0c46","unresolved":false,"context_lines":[{"line_number":421,"context_line":"                    \"os_encrypt_key_deletion_policy\""},{"line_number":422,"context_line":"                ]"},{"line_number":423,"context_line":"            ):"},{"line_number":424,"context_line":"                msg \u003d _(\"Image with encryption properties cannot be target \""},{"line_number":425,"context_line":"                        \"for import\")"},{"line_number":426,"context_line":"                raise exception.Conflict(msg)"},{"line_number":427,"context_line":"            if image.status \u003d\u003d \u0027active\u0027 and import_method !\u003d \"copy-image\":"},{"line_number":428,"context_line":"                msg \u003d _(\"Image with status active cannot be target for import\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"a1dc301d_8467123f","line":425,"range":{"start_line":424,"start_character":25,"end_line":425,"end_character":36},"in_reply_to":"f7227bc4_cf8a0e83","updated":"2025-02-26 15:07:01.000000000","message":"Rephrased.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2450a18a0dd1e2eccd45fe5735e55c9f058695e1","unresolved":true,"context_lines":[{"line_number":815,"context_line":"                msg \u003d _(\"Property %s does not exist.\")"},{"line_number":816,"context_line":"                raise webob.exc.HTTPConflict(msg % path_root)"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"    def _unregister_as_secret_consumer(self, context, image):"},{"line_number":819,"context_line":"        \"\"\"Unregister image as secret consumer."},{"line_number":820,"context_line":""},{"line_number":821,"context_line":"        Removes the image from the secret consumers of the secret referenced"}],"source_content_type":"text/x-python","patch_set":9,"id":"f9354cbf_b61ba75e","line":818,"range":{"start_line":818,"start_character":4,"end_line":818,"end_character":61},"updated":"2024-11-22 09:40:59.000000000","message":"I think you can also separate out this work in different patch, this will help to breakdown tests in different patches as well.\n\nPatch 1, Consumer registration and related tests\nPatch 2, Consumer Un-registration and related tests\nPatch 3, doc changes","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"1f902e0fa2862ee81265ba596d6b8d89e708417d","unresolved":false,"context_lines":[{"line_number":815,"context_line":"                msg \u003d _(\"Property %s does not exist.\")"},{"line_number":816,"context_line":"                raise webob.exc.HTTPConflict(msg % path_root)"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"    def _unregister_as_secret_consumer(self, context, image):"},{"line_number":819,"context_line":"        \"\"\"Unregister image as secret consumer."},{"line_number":820,"context_line":""},{"line_number":821,"context_line":"        Removes the image from the secret consumers of the secret referenced"}],"source_content_type":"text/x-python","patch_set":9,"id":"617ab666_13a675ec","line":818,"range":{"start_line":818,"start_character":4,"end_line":818,"end_character":61},"in_reply_to":"5ade09c9_7afc21ef","updated":"2024-11-22 13:34:34.000000000","message":"Acknowledged","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":28271,"name":"Josephine Seifert","email":"josephine.seifert@cloudandheat.com","username":"josei"},"change_message_id":"231cef6fd47b386e8af1ccc60f22128022776bf7","unresolved":true,"context_lines":[{"line_number":815,"context_line":"                msg \u003d _(\"Property %s does not exist.\")"},{"line_number":816,"context_line":"                raise webob.exc.HTTPConflict(msg % path_root)"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"    def _unregister_as_secret_consumer(self, context, image):"},{"line_number":819,"context_line":"        \"\"\"Unregister image as secret consumer."},{"line_number":820,"context_line":""},{"line_number":821,"context_line":"        Removes the image from the secret consumers of the secret referenced"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ade09c9_7afc21ef","line":818,"range":{"start_line":818,"start_character":4,"end_line":818,"end_character":61},"in_reply_to":"f9354cbf_b61ba75e","updated":"2024-11-22 12:55:51.000000000","message":"I would like to have the registering and unregistering in the same patch.\nBut I can understand the separation of the doc changes.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2450a18a0dd1e2eccd45fe5735e55c9f058695e1","unresolved":true,"context_lines":[{"line_number":836,"context_line":"        consumer_data \u003d {"},{"line_number":837,"context_line":"            \u0027service\u0027: \u0027glance\u0027,"},{"line_number":838,"context_line":"            \u0027resource_type\u0027: \u0027image\u0027,"},{"line_number":839,"context_line":"            \u0027resource_id\u0027: image.image_id"},{"line_number":840,"context_line":"        }"},{"line_number":841,"context_line":""},{"line_number":842,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9740a0d9_1febce67","line":839,"range":{"start_line":839,"start_character":27,"end_line":839,"end_character":41},"updated":"2024-11-22 09:40:59.000000000","message":"ditto, pass image id instead of entire image object.","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"1f902e0fa2862ee81265ba596d6b8d89e708417d","unresolved":false,"context_lines":[{"line_number":836,"context_line":"        consumer_data \u003d {"},{"line_number":837,"context_line":"            \u0027service\u0027: \u0027glance\u0027,"},{"line_number":838,"context_line":"            \u0027resource_type\u0027: \u0027image\u0027,"},{"line_number":839,"context_line":"            \u0027resource_id\u0027: image.image_id"},{"line_number":840,"context_line":"        }"},{"line_number":841,"context_line":""},{"line_number":842,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"c437fab5_397d1cc5","line":839,"range":{"start_line":839,"start_character":27,"end_line":839,"end_character":41},"in_reply_to":"9740a0d9_1febce67","updated":"2024-11-22 13:34:34.000000000","message":"Done","commit_id":"c0cbb87f742ee70db1d157b284c5d85bbf6b1f3e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"af782785af1751ad3d77dbf7c3806e2bdf86f21e","unresolved":true,"context_lines":[{"line_number":99,"context_line":"        props \u003d image.extra_properties"},{"line_number":100,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":101,"context_line":"        # deprecation period is over."},{"line_number":102,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":103,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":104,"context_line":"        if encryption_key_id is None:"},{"line_number":105,"context_line":"            return"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        # kwargs as required by barbicanclient.secrets.add_consumer()"},{"line_number":108,"context_line":"        consumer_data \u003d {"},{"line_number":109,"context_line":"            \u0027service\u0027: \u0027glance\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"a313a9bc_f310f13e","line":106,"range":{"start_line":102,"start_character":8,"end_line":106,"end_character":1},"updated":"2025-03-03 14:47:49.000000000","message":"Instead of doing this inside registration function call I think this should be done before line 192 in if condition like if encryption_key_id is present then only call this register function.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        props \u003d image.extra_properties"},{"line_number":100,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":101,"context_line":"        # deprecation period is over."},{"line_number":102,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":103,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":104,"context_line":"        if encryption_key_id is None:"},{"line_number":105,"context_line":"            return"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        # kwargs as required by barbicanclient.secrets.add_consumer()"},{"line_number":108,"context_line":"        consumer_data \u003d {"},{"line_number":109,"context_line":"            \u0027service\u0027: \u0027glance\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"1b6c2724_bf0a330e","line":106,"range":{"start_line":102,"start_character":8,"end_line":106,"end_character":1},"in_reply_to":"a313a9bc_f310f13e","updated":"2025-04-09 09:06:06.000000000","message":"Moved to before the call.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"2ba0ca5edeecc3c24e9b1be728a5aaf972cd5b5a","unresolved":true,"context_lines":[{"line_number":154,"context_line":"        # On the other hand, the os_encrypt_* parameters describe the image"},{"line_number":155,"context_line":"        # encryption intent and need to be initialized from the start."},{"line_number":156,"context_line":"        mandatory_keys \u003d {"},{"line_number":157,"context_line":"            k for k in all_keys if not k.startswith(\u0027os_decrypt\u0027)"},{"line_number":158,"context_line":"        }"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        # all mandatory keys must be present"}],"source_content_type":"text/x-python","patch_set":11,"id":"b6fb11dc_b49d1ddb","line":157,"range":{"start_line":157,"start_character":32,"end_line":157,"end_character":65},"updated":"2025-03-11 01:45:14.000000000","message":"Personally I avoid negative conditionals as they cause confusion, given the keys we have, this could have been simply written as,\n\n    if k.startswith(\u0027os_encrypt\u0027)","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[{"line_number":154,"context_line":"        # On the other hand, the os_encrypt_* parameters describe the image"},{"line_number":155,"context_line":"        # encryption intent and need to be initialized from the start."},{"line_number":156,"context_line":"        mandatory_keys \u003d {"},{"line_number":157,"context_line":"            k for k in all_keys if not k.startswith(\u0027os_decrypt\u0027)"},{"line_number":158,"context_line":"        }"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        # all mandatory keys must be present"}],"source_content_type":"text/x-python","patch_set":11,"id":"e5a76253_591c56f9","line":157,"range":{"start_line":157,"start_character":32,"end_line":157,"end_character":65},"in_reply_to":"b6fb11dc_b49d1ddb","updated":"2025-04-09 09:06:06.000000000","message":"Changed.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"af782785af1751ad3d77dbf7c3806e2bdf86f21e","unresolved":true,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        try:"},{"line_number":415,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":416,"context_line":"            if any("},{"line_number":417,"context_line":"                encrypt_param in image.extra_properties"},{"line_number":418,"context_line":"                for encrypt_param in ["},{"line_number":419,"context_line":"                    \"os_encrypt_key_id\", \"os_encrypt_format\","},{"line_number":420,"context_line":"                    \"os_encrypt_key_deletion_policy\""},{"line_number":421,"context_line":"                ]"},{"line_number":422,"context_line":"            ):"},{"line_number":423,"context_line":"                msg \u003d _(\"Image with encryption properties is not allowed for \""},{"line_number":424,"context_line":"                        \"import\")"},{"line_number":425,"context_line":"                raise exception.Conflict(msg)"},{"line_number":426,"context_line":"            if image.status \u003d\u003d \u0027active\u0027 and import_method !\u003d \"copy-image\":"},{"line_number":427,"context_line":"                msg \u003d _(\"Image with status active cannot be target for import\")"},{"line_number":428,"context_line":"                raise exception.Conflict(msg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1360017c_30a6bd06","line":425,"range":{"start_line":416,"start_character":12,"end_line":425,"end_character":45},"updated":"2025-03-03 14:47:49.000000000","message":"I think here we should check if import_method !\u003d copy-image (or glance-direct?) otherwise it will not allow existing image with above properties to copy in another backend. Also if import method is not copy-image (or glance-direct) then in this case we should revert back the image status to queued or else image will remain in uploading state and then there is no use of that image and need to be deleted explicitly.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d442317bc1d94942abf25ab42427fd02cb0b1a4b","unresolved":false,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        try:"},{"line_number":415,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":416,"context_line":"            if any("},{"line_number":417,"context_line":"                encrypt_param in image.extra_properties"},{"line_number":418,"context_line":"                for encrypt_param in ["},{"line_number":419,"context_line":"                    \"os_encrypt_key_id\", \"os_encrypt_format\","},{"line_number":420,"context_line":"                    \"os_encrypt_key_deletion_policy\""},{"line_number":421,"context_line":"                ]"},{"line_number":422,"context_line":"            ):"},{"line_number":423,"context_line":"                msg \u003d _(\"Image with encryption properties is not allowed for \""},{"line_number":424,"context_line":"                        \"import\")"},{"line_number":425,"context_line":"                raise exception.Conflict(msg)"},{"line_number":426,"context_line":"            if image.status \u003d\u003d \u0027active\u0027 and import_method !\u003d \"copy-image\":"},{"line_number":427,"context_line":"                msg \u003d _(\"Image with status active cannot be target for import\")"},{"line_number":428,"context_line":"                raise exception.Conflict(msg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"87bbb70f_0a20ee2f","line":425,"range":{"start_line":416,"start_character":12,"end_line":425,"end_character":45},"in_reply_to":"0395a871_3d852dca","updated":"2025-06-19 16:40:01.000000000","message":"Acknowledged","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"c97955f07d41b19ddce263602ef9e12175e5ddf1","unresolved":true,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        try:"},{"line_number":415,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":416,"context_line":"            if any("},{"line_number":417,"context_line":"                encrypt_param in image.extra_properties"},{"line_number":418,"context_line":"                for encrypt_param in ["},{"line_number":419,"context_line":"                    \"os_encrypt_key_id\", \"os_encrypt_format\","},{"line_number":420,"context_line":"                    \"os_encrypt_key_deletion_policy\""},{"line_number":421,"context_line":"                ]"},{"line_number":422,"context_line":"            ):"},{"line_number":423,"context_line":"                msg \u003d _(\"Image with encryption properties is not allowed for \""},{"line_number":424,"context_line":"                        \"import\")"},{"line_number":425,"context_line":"                raise exception.Conflict(msg)"},{"line_number":426,"context_line":"            if image.status \u003d\u003d \u0027active\u0027 and import_method !\u003d \"copy-image\":"},{"line_number":427,"context_line":"                msg \u003d _(\"Image with status active cannot be target for import\")"},{"line_number":428,"context_line":"                raise exception.Conflict(msg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"ede0ff6d_215893a3","line":425,"range":{"start_line":416,"start_character":12,"end_line":425,"end_character":45},"in_reply_to":"1360017c_30a6bd06","updated":"2025-04-09 09:36:10.000000000","message":"I think I\u0027d need to put more research into that as I am unfortunately not that familiar with the interoperable image import stuff.\n\nIn case of copy-image I think we need a clear decision on how to handle the encryption key. Do we clone the key via key manager API and use the resulting new key_id or do we keep the original image\u0027s key_id and register the cloned image as another secret consumer to the same secret? Should any of that happen here in this function or in the import task implementation?\n\nIn case of glance-direct I think I need to do some hands-on testing on how it relates to the encryption metadata first.\n\n\u003e Also if import method is not copy-image (or glance-direct) then in this case we should revert back the image status to queued [...]\n\nIn this function directly?","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"79c07f17c8b74e93a2a2a6a27ada8d0225a99146","unresolved":true,"context_lines":[{"line_number":413,"context_line":""},{"line_number":414,"context_line":"        try:"},{"line_number":415,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":416,"context_line":"            if any("},{"line_number":417,"context_line":"                encrypt_param in image.extra_properties"},{"line_number":418,"context_line":"                for encrypt_param in ["},{"line_number":419,"context_line":"                    \"os_encrypt_key_id\", \"os_encrypt_format\","},{"line_number":420,"context_line":"                    \"os_encrypt_key_deletion_policy\""},{"line_number":421,"context_line":"                ]"},{"line_number":422,"context_line":"            ):"},{"line_number":423,"context_line":"                msg \u003d _(\"Image with encryption properties is not allowed for \""},{"line_number":424,"context_line":"                        \"import\")"},{"line_number":425,"context_line":"                raise exception.Conflict(msg)"},{"line_number":426,"context_line":"            if image.status \u003d\u003d \u0027active\u0027 and import_method !\u003d \"copy-image\":"},{"line_number":427,"context_line":"                msg \u003d _(\"Image with status active cannot be target for import\")"},{"line_number":428,"context_line":"                raise exception.Conflict(msg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"0395a871_3d852dca","line":425,"range":{"start_line":416,"start_character":12,"end_line":425,"end_character":45},"in_reply_to":"ede0ff6d_215893a3","updated":"2025-05-23 15:36:09.000000000","message":"It seems I misunderstood copy-image a bit. There is no need to clone the encryption key it seems as it is only duplicated into further backend stores but keeps the same image identity, thus representing the same secret consumer.\n\nI tested and added support for copy-image, glance-direct and web-download in the latest patchset update.\nI documented my devstack testing for all three methods at the end of https://gist.github.com/markus-hentsch/051e34252f4f9d0a4cbb7472665d872b","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"af782785af1751ad3d77dbf7c3806e2bdf86f21e","unresolved":true,"context_lines":[{"line_number":712,"context_line":"            api_pol \u003d api_policy.ImageAPIPolicy(req.context, image,"},{"line_number":713,"context_line":"                                                self.policy)"},{"line_number":714,"context_line":""},{"line_number":715,"context_line":"            # check for changes to os_encrypt_* image encryption properties"},{"line_number":716,"context_line":"            # beforehand and reject them; do not apply any partial updates"},{"line_number":717,"context_line":"            for change in changes:"},{"line_number":718,"context_line":"                path_root \u003d change[\u0027path\u0027][0]"},{"line_number":719,"context_line":"                if path_root.startswith(\"os_encrypt_\"):"},{"line_number":720,"context_line":"                    LOG.debug(\"Rejecting change to %s\", path_root)"},{"line_number":721,"context_line":"                    raise webob.exc.HTTPBadRequest("},{"line_number":722,"context_line":"                        \"Changes to os_encrypt properties are \""},{"line_number":723,"context_line":"                        \"not permitted after image creation\")"},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"            for change in changes:"},{"line_number":726,"context_line":"                change_method_name \u003d \u0027_do_%s\u0027 % change[\u0027op\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"84f41384_a1275e89","line":723,"range":{"start_line":715,"start_character":12,"end_line":723,"end_character":61},"updated":"2025-03-03 14:47:49.000000000","message":"better to this before policy check i.e. before line 709 0r 711","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[{"line_number":712,"context_line":"            api_pol \u003d api_policy.ImageAPIPolicy(req.context, image,"},{"line_number":713,"context_line":"                                                self.policy)"},{"line_number":714,"context_line":""},{"line_number":715,"context_line":"            # check for changes to os_encrypt_* image encryption properties"},{"line_number":716,"context_line":"            # beforehand and reject them; do not apply any partial updates"},{"line_number":717,"context_line":"            for change in changes:"},{"line_number":718,"context_line":"                path_root \u003d change[\u0027path\u0027][0]"},{"line_number":719,"context_line":"                if path_root.startswith(\"os_encrypt_\"):"},{"line_number":720,"context_line":"                    LOG.debug(\"Rejecting change to %s\", path_root)"},{"line_number":721,"context_line":"                    raise webob.exc.HTTPBadRequest("},{"line_number":722,"context_line":"                        \"Changes to os_encrypt properties are \""},{"line_number":723,"context_line":"                        \"not permitted after image creation\")"},{"line_number":724,"context_line":""},{"line_number":725,"context_line":"            for change in changes:"},{"line_number":726,"context_line":"                change_method_name \u003d \u0027_do_%s\u0027 % change[\u0027op\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"98810ac3_4c8054dc","line":723,"range":{"start_line":715,"start_character":12,"end_line":723,"end_character":61},"in_reply_to":"84f41384_a1275e89","updated":"2025-04-09 09:06:06.000000000","message":"Moved to the start of the function.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"af782785af1751ad3d77dbf7c3806e2bdf86f21e","unresolved":true,"context_lines":[{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":828,"context_line":"        # deprecation period is over."},{"line_number":829,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":830,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":831,"context_line":"        if encryption_key_id is None:"},{"line_number":832,"context_line":"            return"},{"line_number":833,"context_line":""},{"line_number":834,"context_line":"        # kwargs as required by barbicanclient.secrets.remove_consumer()"},{"line_number":835,"context_line":"        consumer_data \u003d {"}],"source_content_type":"text/x-python","patch_set":11,"id":"cbea6257_864c85d8","line":832,"range":{"start_line":829,"start_character":8,"end_line":832,"end_character":18},"updated":"2025-03-03 14:47:49.000000000","message":"ditto, I think this should be called before line 1067 similar to register the secret.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[{"line_number":826,"context_line":""},{"line_number":827,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_id once"},{"line_number":828,"context_line":"        # deprecation period is over."},{"line_number":829,"context_line":"        encryption_key_id \u003d props.get(\u0027os_encrypt_key_id\u0027) or props.get("},{"line_number":830,"context_line":"            \u0027cinder_encryption_key_id\u0027)"},{"line_number":831,"context_line":"        if encryption_key_id is None:"},{"line_number":832,"context_line":"            return"},{"line_number":833,"context_line":""},{"line_number":834,"context_line":"        # kwargs as required by barbicanclient.secrets.remove_consumer()"},{"line_number":835,"context_line":"        consumer_data \u003d {"}],"source_content_type":"text/x-python","patch_set":11,"id":"f29ff32e_1733f4fb","line":832,"range":{"start_line":829,"start_character":8,"end_line":832,"end_character":18},"in_reply_to":"cbea6257_864c85d8","updated":"2025-04-09 09:06:06.000000000","message":"Moved to before the call.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"2ba0ca5edeecc3c24e9b1be728a5aaf972cd5b5a","unresolved":true,"context_lines":[{"line_number":873,"context_line":""},{"line_number":874,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_deletion_policy"},{"line_number":875,"context_line":"        # once deprecation period is over."},{"line_number":876,"context_line":"        del_policy \u003d props.get(\u0027os_encrypt_key_deletion_policy\u0027) or props.get("},{"line_number":877,"context_line":"            \u0027cinder_encryption_key_deletion_policy\u0027)"},{"line_number":878,"context_line":"        if del_policy !\u003d \u0027on_image_deletion\u0027:"},{"line_number":879,"context_line":"            return"},{"line_number":880,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"4ac63783_b14834a8","line":877,"range":{"start_line":876,"start_character":21,"end_line":877,"end_character":52},"updated":"2025-03-11 01:45:14.000000000","message":"I feel repeating the same keys for checks, all across the code base, is vulnerable to errors, It would be best if we create a method that returns the right property based on a substring of the key specified,\n\n    def encryption_parameter(sub_key):\n        if \u0027key_id\u0027 in sub_key:\n            return props.get(\u0027os_encrypt_key_id\u0027) or props.get(\n            \u0027cinder_encryption_key_id\u0027)\n        elif \u0027key_deletion_policy\u0027 in sub_key:\n            return props.get(\u0027os_encrypt_key_deletion_policy\u0027) or props.get(\n            \u0027cinder_encryption_key_deletion_policy\u0027)\n        else:\n            # Some exception","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"020b38e012766efbb20a9a326c1d91b3034ce3bc","unresolved":false,"context_lines":[{"line_number":873,"context_line":""},{"line_number":874,"context_line":"        # NOTE(mhen): Remove support for cinder_encryption_key_deletion_policy"},{"line_number":875,"context_line":"        # once deprecation period is over."},{"line_number":876,"context_line":"        del_policy \u003d props.get(\u0027os_encrypt_key_deletion_policy\u0027) or props.get("},{"line_number":877,"context_line":"            \u0027cinder_encryption_key_deletion_policy\u0027)"},{"line_number":878,"context_line":"        if del_policy !\u003d \u0027on_image_deletion\u0027:"},{"line_number":879,"context_line":"            return"},{"line_number":880,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"2ed105ae_8de5dec6","line":877,"range":{"start_line":876,"start_character":21,"end_line":877,"end_character":52},"in_reply_to":"4ac63783_b14834a8","updated":"2025-04-09 09:06:06.000000000","message":"Thanks for the suggestion. I have outsourced this behavior to a dedicated function accordingly and added unit tests.","commit_id":"9e42aa0acf9cafb480fe2690f2a31b1561d0a907"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f30834aff89a5ffc3e6e0a48255f660b7598c85d","unresolved":true,"context_lines":[{"line_number":89,"context_line":"        self._key_manager \u003d key_manager.API(CONF)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @staticmethod"},{"line_number":92,"context_line":"    def _get_image_encryption_parameter(extra_properties, sub_key):"},{"line_number":93,"context_line":"        \"\"\"Retrieve an image encryption parameter from image properties."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        Looks up specific image encryption metadata properties that can appear"}],"source_content_type":"text/x-python","patch_set":14,"id":"6cb71752_f68e3315","line":92,"updated":"2025-08-28 15:46:16.000000000","message":"It took me a while to grok what was going on here. I think maybe a better name here would help. Something like `get_parameter_with_compat()` or something would imply that this is to retrieve the value of the new key, and if not, fall back to the old one.\n\nHowever, could we just have this be `get_key_details()` that decides new-or-old and returns a tuple of the two things instead of taking this \"sub_key\"?\n\nIt just seems a little overly complicated to me :)","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"7526bad04b41cbecfe4972c5a47a6c565feb4103","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self._key_manager \u003d key_manager.API(CONF)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @staticmethod"},{"line_number":92,"context_line":"    def _get_image_encryption_parameter(extra_properties, sub_key):"},{"line_number":93,"context_line":"        \"\"\"Retrieve an image encryption parameter from image properties."},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        Looks up specific image encryption metadata properties that can appear"}],"source_content_type":"text/x-python","patch_set":14,"id":"c18034d0_9500e2f8","line":92,"in_reply_to":"6cb71752_f68e3315","updated":"2025-09-08 15:54:25.000000000","message":"I have split this into two methods now and removed the quirkiness. Having this as a combined method turned out to be more hassle than it was worth and the key_deletion_policy one is used only once anyway.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f30834aff89a5ffc3e6e0a48255f660b7598c85d","unresolved":true,"context_lines":[{"line_number":180,"context_line":"        # encryption intent and need to be initialized from the start."},{"line_number":181,"context_line":"        mandatory_keys \u003d {"},{"line_number":182,"context_line":"            k for k in all_keys if k.startswith(\u0027os_encrypt\u0027)"},{"line_number":183,"context_line":"        }"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # all mandatory keys must be present"},{"line_number":186,"context_line":"        for k in mandatory_keys:"}],"source_content_type":"text/x-python","patch_set":14,"id":"1aa95abe_b100cf5b","line":183,"updated":"2025-08-28 15:46:16.000000000","message":"So the `os_decrypt_container_format` is not mandatory? Given the recent mega-CVE I\u0027d definitely prefer to have it be required.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":false,"context_lines":[{"line_number":180,"context_line":"        # encryption intent and need to be initialized from the start."},{"line_number":181,"context_line":"        mandatory_keys \u003d {"},{"line_number":182,"context_line":"            k for k in all_keys if k.startswith(\u0027os_encrypt\u0027)"},{"line_number":183,"context_line":"        }"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # all mandatory keys must be present"},{"line_number":186,"context_line":"        for k in mandatory_keys:"}],"source_content_type":"text/x-python","patch_set":14,"id":"8d514b46_f983c932","line":183,"in_reply_to":"1aa95abe_b100cf5b","updated":"2025-12-12 18:22:51.000000000","message":"Done","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d442317bc1d94942abf25ab42427fd02cb0b1a4b","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":202,"context_line":"            # visibility to private because secrets in the Key Manager are"},{"line_number":203,"context_line":"            # project-bound and cannot be shared across different projects."},{"line_number":204,"context_line":"            if \u0027visibility\u0027 not in image and is_encrypted:"},{"line_number":205,"context_line":"                image[\u0027visibility\u0027] \u003d \u0027private\u0027"},{"line_number":206,"context_line":"            if \u0027owner\u0027 not in image:"},{"line_number":207,"context_line":"                image[\u0027owner\u0027] \u003d req.context.project_id"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3a44b987_2b4448f8","line":205,"range":{"start_line":204,"start_character":0,"end_line":205,"end_character":47},"updated":"2025-06-19 16:40:01.000000000","message":"I am not sure whether we will reach here or not because if you don\u0027t explicitly pass visibility while creating image then by default it will be shared visibility.\n\nhttps://github.com/openstack/glance/blob/master/glance/domain/__init__.py#L71","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":28271,"name":"Josephine Seifert","email":"josephine.seifert@cloudandheat.com","username":"josei"},"change_message_id":"8ec7fe7cc3bed41923fc27965f6df587a54938f4","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":202,"context_line":"            # visibility to private because secrets in the Key Manager are"},{"line_number":203,"context_line":"            # project-bound and cannot be shared across different projects."},{"line_number":204,"context_line":"            if \u0027visibility\u0027 not in image and is_encrypted:"},{"line_number":205,"context_line":"                image[\u0027visibility\u0027] \u003d \u0027private\u0027"},{"line_number":206,"context_line":"            if \u0027owner\u0027 not in image:"},{"line_number":207,"context_line":"                image[\u0027owner\u0027] \u003d req.context.project_id"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"741c732f_d8ee9f02","line":205,"range":{"start_line":204,"start_character":0,"end_line":205,"end_character":47},"in_reply_to":"3a44b987_2b4448f8","updated":"2025-06-25 13:04:55.000000000","message":"The method you linked is called a few lines below here (line 213).\nSo this should not be a problem. But maybe we should add a comment, that the setting of the visibility has to happen before the call of `image_factory.new_image()`.","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7f068464944b1724807ee6b33d28cb928c6c0df6","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":202,"context_line":"            # visibility to private because secrets in the Key Manager are"},{"line_number":203,"context_line":"            # project-bound and cannot be shared across different projects."},{"line_number":204,"context_line":"            if \u0027visibility\u0027 not in image and is_encrypted:"},{"line_number":205,"context_line":"                image[\u0027visibility\u0027] \u003d \u0027private\u0027"},{"line_number":206,"context_line":"            if \u0027owner\u0027 not in image:"},{"line_number":207,"context_line":"                image[\u0027owner\u0027] \u003d req.context.project_id"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"b78703fb_3026b512","line":205,"range":{"start_line":204,"start_character":0,"end_line":205,"end_character":47},"in_reply_to":"741c732f_d8ee9f02","updated":"2025-06-25 13:50:25.000000000","message":"Acknowledged","commit_id":"509199ee53ba9896676316be0b32662df5e07699"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":210,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":211,"context_line":"            # visibility to private because secrets in the Key Manager are"},{"line_number":212,"context_line":"            # project-bound and cannot be shared across different projects."},{"line_number":213,"context_line":"            if \u0027visibility\u0027 not in image and is_encrypted:"},{"line_number":214,"context_line":"                image[\u0027visibility\u0027] \u003d \u0027private\u0027"},{"line_number":215,"context_line":"            if \u0027owner\u0027 not in image:"},{"line_number":216,"context_line":"                image[\u0027owner\u0027] \u003d req.context.project_id"}],"source_content_type":"text/x-python","patch_set":23,"id":"188c335b_e582f3e6","line":213,"updated":"2025-12-12 18:22:51.000000000","message":"I wonder if it would be better to just require visibility\u003dprivate when creating one of these things? The docs currently say that the default is \"shared\" but this changes that in cases where it\u0027s encrypted. I think having a complex default behavior is less desirable than just requiring the user to use `--private` for these images for consistency.\n\nAlso, AFAICT, if I provide a `visibility\u003d(public|shared|community)` you\u0027ll still create it despite (according to the comment) there being no way anyone other than the owner to read it. That seems non-ideal, but.. is it intentional?","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            # If the image has an associated encryption key, default the"},{"line_number":211,"context_line":"            # visibility to private because secrets in the Key Manager are"},{"line_number":212,"context_line":"            # project-bound and cannot be shared across different projects."},{"line_number":213,"context_line":"            if \u0027visibility\u0027 not in image and is_encrypted:"},{"line_number":214,"context_line":"                image[\u0027visibility\u0027] \u003d \u0027private\u0027"},{"line_number":215,"context_line":"            if \u0027owner\u0027 not in image:"},{"line_number":216,"context_line":"                image[\u0027owner\u0027] \u003d req.context.project_id"}],"source_content_type":"text/x-python","patch_set":23,"id":"5106aba8_2b159ac2","line":213,"in_reply_to":"188c335b_e582f3e6","updated":"2025-12-16 10:30:37.000000000","message":"Ok. I have changed it to make visibility\u003dprivate a hard requirement for encrypted images.\n\nThis will change the Cinder behavior a bit: it will now create its own images (via `os-volume_upload_image`) created from encrypted volumes with visibility set to private. Hopefully, Cinder folks will be okay with that.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":461,"context_line":"                ]"},{"line_number":462,"context_line":"            ) and import_method not in ["},{"line_number":463,"context_line":"                \"copy-image\", \"glance-direct\", \"web-download\""},{"line_number":464,"context_line":"            ]:"},{"line_number":465,"context_line":"                msg \u003d _(\"Image with encryption properties is not allowed \""},{"line_number":466,"context_line":"                        \"for import with method \u0027%s\u0027\") % import_method"},{"line_number":467,"context_line":"                raise exception.Conflict(msg)"}],"source_content_type":"text/x-python","patch_set":23,"id":"3a875f78_bcec8669","line":464,"updated":"2025-12-12 18:22:51.000000000","message":"I really hate this sort of very spread-out arrangement, especially embedded in the condition of an `if` statement as I think it\u0027s very hard to read, eye-parse, and validate.\n\nFirst, I think the list of params could/should be a shared constant. The `os_decrypt_size` is missing from here, but I assume it should be included? If it was a shared constant then it wouldn\u0027t be missed.\n\nSecond I\u0027d just prefer something like this for readability:\n```\nvalid_methods \u003d [\u0027copy-image\u0027, ...]\nhas_enc_params \u003d any(p in image.extra_properties for p in OS_ENCRYPT_PARAMS)\nif has_enc_params and import method not in valid_methods:\n    # fail\n```","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                ]"},{"line_number":462,"context_line":"            ) and import_method not in ["},{"line_number":463,"context_line":"                \"copy-image\", \"glance-direct\", \"web-download\""},{"line_number":464,"context_line":"            ]:"},{"line_number":465,"context_line":"                msg \u003d _(\"Image with encryption properties is not allowed \""},{"line_number":466,"context_line":"                        \"for import with method \u0027%s\u0027\") % import_method"},{"line_number":467,"context_line":"                raise exception.Conflict(msg)"}],"source_content_type":"text/x-python","patch_set":23,"id":"ff1da3b9_9ccf18e1","line":464,"in_reply_to":"3a875f78_bcec8669","updated":"2025-12-16 10:30:37.000000000","message":"Thank you for posting the example. I have adapted it accordingly.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":750,"context_line":"    def update(self, req, image_id, changes):"},{"line_number":751,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":752,"context_line":"        try:"},{"line_number":753,"context_line":"            for change in changes:"},{"line_number":754,"context_line":"                path_root \u003d change[\u0027path\u0027][0]"},{"line_number":755,"context_line":"                if path_root.startswith(\"os_encrypt_\"):"},{"line_number":756,"context_line":"                    LOG.debug(\"Rejecting change to %s\", path_root)"}],"source_content_type":"text/x-python","patch_set":23,"id":"faa9742b_9719186e","line":753,"updated":"2025-12-12 18:22:51.000000000","message":"This seems weird to iterate these a second time, and before we\u0027ve done the does-this-exist and policy check. That means the following precedence order affects the return code you might see:\n\n1. If encrypt-related, reject 400\n2. If no such image, reject 404\n3. If no permissions, 403\n4. If something checked in the DB layer fails, 400\n\nThat said, we enforce things like immutable properties in `RequestDeserializer` below. So I think that these \"write once\" properties should probably be checked and rejected there for consistency with other such rules.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":false,"context_lines":[{"line_number":750,"context_line":"    def update(self, req, image_id, changes):"},{"line_number":751,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":752,"context_line":"        try:"},{"line_number":753,"context_line":"            for change in changes:"},{"line_number":754,"context_line":"                path_root \u003d change[\u0027path\u0027][0]"},{"line_number":755,"context_line":"                if path_root.startswith(\"os_encrypt_\"):"},{"line_number":756,"context_line":"                    LOG.debug(\"Rejecting change to %s\", path_root)"}],"source_content_type":"text/x-python","patch_set":23,"id":"68c107c8_864c1c46","line":753,"in_reply_to":"faa9742b_9719186e","updated":"2025-12-16 10:30:37.000000000","message":"I think the original intention was to have a separate loop so that partial updates are not applied in the actual loop. But looking at it again it seems only the `save()` call at the end will make changes, so my initial assumption was wrong.\n\nAnyhow, I removed this and added a more simplified version of this to the `RequestDeserializer` as suggested. Thanks for the hint!","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":892,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":893,"context_line":"            msg \u003d (\u0027Failed to unregister as consumer for encryption key %s \u0027"},{"line_number":894,"context_line":"                   \u0027(\"%s\")\u0027 % (encryption_key_id, str(e)))"},{"line_number":895,"context_line":"            LOG.warning(msg)"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"    def _delete_encryption_key(self, context, image):"},{"line_number":898,"context_line":"        key_id \u003d self._get_encryption_key_id(image.extra_properties)"}],"source_content_type":"text/x-python","patch_set":23,"id":"a77c4526_e1782a21","line":895,"updated":"2025-12-12 18:22:51.000000000","message":"Even though this is subtly different, I feel like there should be some amount of commonality such that we can re-use some things between this and the `_register_*` method.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":true,"context_lines":[{"line_number":892,"context_line":"        except castellan_exception.KeyManagerError as e:"},{"line_number":893,"context_line":"            msg \u003d (\u0027Failed to unregister as consumer for encryption key %s \u0027"},{"line_number":894,"context_line":"                   \u0027(\"%s\")\u0027 % (encryption_key_id, str(e)))"},{"line_number":895,"context_line":"            LOG.warning(msg)"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"    def _delete_encryption_key(self, context, image):"},{"line_number":898,"context_line":"        key_id \u003d self._get_encryption_key_id(image.extra_properties)"}],"source_content_type":"text/x-python","patch_set":23,"id":"74019f8c_cbc9557d","line":895,"in_reply_to":"a77c4526_e1782a21","updated":"2025-12-16 10:30:37.000000000","message":"Can you elaborate on what you have in mind here? I can\u0027t really follow.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":1099,"context_line":"            image.delete()"},{"line_number":1100,"context_line":"            if self._get_encryption_key_id(image.extra_properties) is not None:"},{"line_number":1101,"context_line":"                self._unregister_as_secret_consumer(req.context, image)"},{"line_number":1102,"context_line":"                self._delete_encryption_key(req.context, image)"},{"line_number":1103,"context_line":"            image_repo.remove(image)"},{"line_number":1104,"context_line":"        except (glance_store.Forbidden, exception.Forbidden) as e:"},{"line_number":1105,"context_line":"            LOG.debug(\"User not permitted to delete image \u0027%s\u0027\", image_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"5b13307c_8ac0d1ba","line":1102,"updated":"2025-12-12 18:22:51.000000000","message":"I\u0027m not really sure what the best thing is  here, but if the remove fails, we will have unregistered and potentially deleted the secret on an image that we didn\u0027t delete. If that happens, the image remains active and with references to keys that no longer exist. If the user just tries to re-delete, then fine, but what happens  if they come back in an hour and think \"oh sh*t I\u0027m glad I couldn\u0027t delete that image\" and then .. can\u0027t use it?\n\nPoint being, I guess it feels better to me to delete the keys after we delete the image, even if it means we could leak (in the memory leak sense) key resources. Thoughts?","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"376cbaffad803e7e6803c8023ece7ac92c8368d0","unresolved":true,"context_lines":[{"line_number":1099,"context_line":"            image.delete()"},{"line_number":1100,"context_line":"            if self._get_encryption_key_id(image.extra_properties) is not None:"},{"line_number":1101,"context_line":"                self._unregister_as_secret_consumer(req.context, image)"},{"line_number":1102,"context_line":"                self._delete_encryption_key(req.context, image)"},{"line_number":1103,"context_line":"            image_repo.remove(image)"},{"line_number":1104,"context_line":"        except (glance_store.Forbidden, exception.Forbidden) as e:"},{"line_number":1105,"context_line":"            LOG.debug(\"User not permitted to delete image \u0027%s\u0027\", image_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"e635e4be_37d323d1","line":1102,"in_reply_to":"5b13307c_8ac0d1ba","updated":"2025-12-12 21:35:44.000000000","message":"Yeah I would recommend deleting the keys last for the reasons you described.\n\nI was careful about doing that while I was working on future ephemeral encryption in Nova -- always delete the disk first, then the keys. Yes, the user should just re-delete but they could change their mind given the opportunity and lament that they could otherwise still use the image if the keys were not gone. Since it\u0027s easy to prevent that situation, I say why not prevent it.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":false,"context_lines":[{"line_number":1099,"context_line":"            image.delete()"},{"line_number":1100,"context_line":"            if self._get_encryption_key_id(image.extra_properties) is not None:"},{"line_number":1101,"context_line":"                self._unregister_as_secret_consumer(req.context, image)"},{"line_number":1102,"context_line":"                self._delete_encryption_key(req.context, image)"},{"line_number":1103,"context_line":"            image_repo.remove(image)"},{"line_number":1104,"context_line":"        except (glance_store.Forbidden, exception.Forbidden) as e:"},{"line_number":1105,"context_line":"            LOG.debug(\"User not permitted to delete image \u0027%s\u0027\", image_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"8ba268b6_32f6ac25","line":1102,"in_reply_to":"e635e4be_37d323d1","updated":"2025-12-16 10:30:37.000000000","message":"Good catch, thanks for bringing this up! I shuffled things around as suggested and added a new unit test ensuring that a secret is not deleted/unregistered as a consumer when the image removal fails.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"}],"glance/async_/flows/api_image_import.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @property"},{"line_number":178,"context_line":"    def image_repo(self):"},{"line_number":179,"context_line":"        return self._image_repo"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def drop_lock_for_task(self):"},{"line_number":182,"context_line":"        \"\"\"Delete the import lock for our task."}],"source_content_type":"text/x-python","patch_set":23,"id":"c1e6d1cf_2b1fed89","line":179,"updated":"2025-12-12 18:22:51.000000000","message":"I\u0027m not sure why you\u0027re adding this, but I\u0027m worried it might be to circumvent the ImportActionWrapper interface which tries to provide atomicity during import.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @property"},{"line_number":178,"context_line":"    def image_repo(self):"},{"line_number":179,"context_line":"        return self._image_repo"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def drop_lock_for_task(self):"},{"line_number":182,"context_line":"        \"\"\"Delete the import lock for our task."}],"source_content_type":"text/x-python","patch_set":23,"id":"eef4b338_d4fa4b4a","line":179,"in_reply_to":"c1e6d1cf_2b1fed89","updated":"2025-12-16 10:30:37.000000000","message":"This was because I did not know how the wrapper worked. Removed.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"}],"glance/async_/flows/plugins/image_conversion.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":88,"context_line":"        and raising/logging if anything goes wrong. If nothing fails, return"},{"line_number":89,"context_line":"        the FileInspector for the file."},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        image \u003d self.image_repo.get(self.image_id)"},{"line_number":92,"context_line":"        if image.container_format \u003d\u003d \u0027luks\u0027:"},{"line_number":93,"context_line":"            LOG.error(\u0027Image with LUKS encryption container format cannot \u0027"},{"line_number":94,"context_line":"                      \u0027be converted: %s\u0027, self.image_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"0b742885_0202d891","line":91,"updated":"2025-12-12 18:22:51.000000000","message":"Yeah, you need to use `action` to get properties of the image (should be able to be passed in here from the caller which has it) and not fetch the image from the DB yourself. In this case, `action.image_container_format`.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        and raising/logging if anything goes wrong. If nothing fails, return"},{"line_number":89,"context_line":"        the FileInspector for the file."},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        image \u003d self.image_repo.get(self.image_id)"},{"line_number":92,"context_line":"        if image.container_format \u003d\u003d \u0027luks\u0027:"},{"line_number":93,"context_line":"            LOG.error(\u0027Image with LUKS encryption container format cannot \u0027"},{"line_number":94,"context_line":"                      \u0027be converted: %s\u0027, self.image_id)"}],"source_content_type":"text/x-python","patch_set":23,"id":"bf7dd09f_24d460a4","line":91,"in_reply_to":"0b742885_0202d891","updated":"2025-12-16 10:30:37.000000000","message":"Thank you very much for the guidance. I did not know how the wrapping worked so I missed that. I have changed this to access the action wrapper\u0027s properties instead now.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a853242af5cff5cbc8879f02c44b8066adba3835","unresolved":true,"context_lines":[{"line_number":96,"context_line":"        if \u0027os_encrypt_key_id\u0027 in image.extra_properties:"},{"line_number":97,"context_line":"            LOG.error(\u0027Image with encryption cannot be converted: %s\u0027,"},{"line_number":98,"context_line":"                      self.image_id)"},{"line_number":99,"context_line":"            raise RuntimeError(\u0027Encrypted image cannot be converted\u0027)"},{"line_number":100,"context_line":"        # Use our own cautious inspector module (if we have one for this"},{"line_number":101,"context_line":"        # format) to make sure a file is the format the submitter claimed"},{"line_number":102,"context_line":"        # it is and that it passes some basic safety checks _before_ we run"}],"source_content_type":"text/x-python","patch_set":23,"id":"41e60bfe_f69dd274","line":99,"updated":"2025-12-12 18:22:51.000000000","message":"This \"inspect\" method is specifically for the deep inspection we do on the image file itself (see the docstring) and not to enforce basically API policy, which is what you\u0027re doing here. To avoid confusing those two operations I\u0027d ask that you please move this check out of here and into something else.\n\nAs noted on the spec, I think this can be be done in `get_flow()` below, so that it\u0027s checked and rejected synchronously with the API call.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"8e67f9c21ed4f53a7cfbec870f196cd0cde704a6","unresolved":true,"context_lines":[{"line_number":96,"context_line":"        if \u0027os_encrypt_key_id\u0027 in image.extra_properties:"},{"line_number":97,"context_line":"            LOG.error(\u0027Image with encryption cannot be converted: %s\u0027,"},{"line_number":98,"context_line":"                      self.image_id)"},{"line_number":99,"context_line":"            raise RuntimeError(\u0027Encrypted image cannot be converted\u0027)"},{"line_number":100,"context_line":"        # Use our own cautious inspector module (if we have one for this"},{"line_number":101,"context_line":"        # format) to make sure a file is the format the submitter claimed"},{"line_number":102,"context_line":"        # it is and that it passes some basic safety checks _before_ we run"}],"source_content_type":"text/x-python","patch_set":23,"id":"75e00748_d2f23f43","line":99,"in_reply_to":"41e60bfe_f69dd274","updated":"2025-12-16 10:30:37.000000000","message":"I have tried this and it results in even worse behavior: if any exception is thrown in `get_flow()`, Glance interprets the import plugin as failing to load and will *skip* it. There will be a \"Could not load image_conversion\" log message in Glance API but the image will then simply *be imported in its original format* and will reach \u0027active\u0027 state. As a result, the configured image conversion is bypassed. I do not think this is desirable.\n\nI have not found a way to make this a synchronous fail early with the API call because the import plugins seem to be loaded in a lazy fashion when the import starts asynchronously in the background later on.\n\nHowever, I have now moved the checks from `_inspect_path()` to `_execute()` before the inspector call.","commit_id":"1fc81b8118a7431e5827587cf10dc8614ea499eb"}],"glance/tests/unit/async_/flows/plugins/test_image_conversion.py":[{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"8ce3e87ed3008ce974244e8d315739f19fbf3633","unresolved":true,"context_lines":[{"line_number":26,"context_line":"import glance.async_.flows.api_image_import as import_flow"},{"line_number":27,"context_line":"import glance.async_.flows.plugins.image_conversion as image_conversion"},{"line_number":28,"context_line":"from glance.async_ import utils as async_utils"},{"line_number":29,"context_line":"from glance.common.exception import Invalid"},{"line_number":30,"context_line":"from glance.common import utils"},{"line_number":31,"context_line":"from glance import domain"},{"line_number":32,"context_line":"from glance import gateway"}],"source_content_type":"text/x-python","patch_set":29,"id":"93a76bd1_a879aa9f","line":29,"in_reply_to":"b1740431_cecd280f","updated":"2026-05-18 09:29:10.000000000","message":"\u003e pep8: F401 \u0027glance.common.exception.Invalid\u0027 imported but unused\n\nPlease fix.","commit_id":"f460329a350dddb16a5006a5c2a2618c6098d99d"}],"glance/tests/unit/v2/test_images_resource.py":[{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"8ce3e87ed3008ce974244e8d315739f19fbf3633","unresolved":true,"context_lines":[{"line_number":3967,"context_line":"        fake_encryption_key \u003d self.controller._key_manager.store("},{"line_number":3968,"context_line":"            request.context, mock.Mock())"},{"line_number":3969,"context_line":"        props \u003d {"},{"line_number":3970,"context_line":"            \u0027cinder_encryptioin_key_id\u0027: fake_encryption_key,"},{"line_number":3971,"context_line":"        }"},{"line_number":3972,"context_line":"        image \u003d _domain_fixture("},{"line_number":3973,"context_line":"            UUID2, name\u003d\u0027image-2\u0027, owner\u003dTENANT2,"}],"source_content_type":"text/x-python","patch_set":29,"id":"b0db8bb5_eb8ae8d6","line":3970,"range":{"start_line":3970,"start_character":13,"end_line":3970,"end_character":38},"updated":"2026-05-18 09:29:10.000000000","message":"cinder_encryptioin_key_id -\u003e cinder_encryption_key_id ?","commit_id":"f460329a350dddb16a5006a5c2a2618c6098d99d"}],"releasenotes/notes/add-support-for-encrypted-image-formats-dd88088428f5ee17.yaml":[{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"8ce3e87ed3008ce974244e8d315739f19fbf3633","unresolved":true,"context_lines":[{"line_number":9,"context_line":"    new formats and metadata properties along with the corresponding handling."},{"line_number":10,"context_line":"upgrade:"},{"line_number":11,"context_line":"  - |"},{"line_number":12,"context_line":"    Introduces the ``luks`` disk_format for raw LUKS-encrypted images."},{"line_number":13,"context_line":"    Extends the existing ``qcow2`` disk_format by support for LUKS encryption."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"    Encrypted images referencing an encryption key in the key manager will now"}],"source_content_type":"text/x-yaml","patch_set":29,"id":"9409d7f3_e78b5d56","line":12,"range":{"start_line":12,"start_character":4,"end_line":12,"end_character":70},"updated":"2026-05-18 09:29:10.000000000","message":"luks is a container_format, right ? so shouldn\u0027t this be \"Introduces the ``luks`` container_format for LUKS-encrypted images with ``disk_format`` ``raw`` or ``gpt`\" ?","commit_id":"f460329a350dddb16a5006a5c2a2618c6098d99d"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"8ce3e87ed3008ce974244e8d315739f19fbf3633","unresolved":true,"context_lines":[{"line_number":22,"context_line":"    * ``os_encrypt_key_id`` (replacing ``cinder_encryption_key_id``)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    * ``os_encrypt_key_deletion_policy``"},{"line_number":25,"context_line":"      (replacing ``cinder_key_deletion_policy``)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    * ``os_encrypt_format``"},{"line_number":28,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":29,"id":"a3f29f7f_b5ab9348","line":25,"range":{"start_line":25,"start_character":19,"end_line":25,"end_character":45},"updated":"2026-05-18 09:29:10.000000000","message":"Nit: cinder_key_deletion_policy-\u003e cinder_encryption_key_deletion_policy ?","commit_id":"f460329a350dddb16a5006a5c2a2618c6098d99d"}]}