)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da983c96f8d8ce7deb028d011583b8b4b8fef2f9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"aab30485_84da819b","updated":"2023-07-11 17:03:43.000000000","message":"+1 over all ill loop back to this after you have time to respond to the comment but i don\u0027t really see anything directional wrong with this.\n\nyou have called out the ceph limiations and the workflow around snapshots so that was the main part i was concerned about.\n\nthere are some details of rescue that are worth considering but nothing major","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47659226e75dee1d3c945c3cf17adb5bcadb55ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bb9860d2_4d4d0fad","updated":"2023-07-11 19:22:38.000000000","message":"Thanks for reviewing and thanks for suggesting the spec amendment. I think including these details in the spec is helpful for everyone, including me.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"471b48dd0fc0376e90bca2d702bf534b9ac9415f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"090c8296_aa67fffe","updated":"2023-08-01 10:29:01.000000000","message":"there is still one nit but this is generally ok as is","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"}],"specs/2023.2/approved/ephemeral-storage-encryption.rst":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da983c96f8d8ce7deb028d011583b8b4b8fef2f9","unresolved":true,"context_lines":[{"line_number":120,"context_line":"* ``luks``  for the LUKSv1 format"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"To enable snapshot and shelve of instances using ephemeral encryption, the UUID"},{"line_number":123,"context_line":"of the encryption secret is stored in the key manager for the resultant image"},{"line_number":124,"context_line":"will be kept with the image as an image property:"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"* ``hw_ephemeral_encryption_secret_uuid``"}],"source_content_type":"text/x-rst","patch_set":2,"id":"2d865277_da7596d1","line":123,"range":{"start_line":123,"start_character":25,"end_line":123,"end_character":27},"updated":"2023-07-11 17:03:43.000000000","message":"nit: actully i was wrong is was nto requried just secret","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47659226e75dee1d3c945c3cf17adb5bcadb55ee","unresolved":true,"context_lines":[{"line_number":120,"context_line":"* ``luks``  for the LUKSv1 format"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"To enable snapshot and shelve of instances using ephemeral encryption, the UUID"},{"line_number":123,"context_line":"of the encryption secret is stored in the key manager for the resultant image"},{"line_number":124,"context_line":"will be kept with the image as an image property:"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"* ``hw_ephemeral_encryption_secret_uuid``"}],"source_content_type":"text/x-rst","patch_set":2,"id":"8fd2e32a_303adcea","line":123,"range":{"start_line":123,"start_character":25,"end_line":123,"end_character":27},"in_reply_to":"2d865277_da7596d1","updated":"2023-07-11 19:22:38.000000000","message":"Oops.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da983c96f8d8ce7deb028d011583b8b4b8fef2f9","unresolved":true,"context_lines":[{"line_number":180,"context_line":"   |                    |             |              | we could create a new secret using the instance\u0027s    |"},{"line_number":181,"context_line":"   |                    |             |              | user/project rather than the shelver\u0027s user/project  |"},{"line_number":182,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":183,"context_line":"   | Rescue disk        | disk (root) | Secret 11    | Secret 11 is stashed in the instance\u0027s system        |"},{"line_number":184,"context_line":"   | created by rescue  |             | (new secret  | metadata with key                                    |"},{"line_number":185,"context_line":"   | of Instance A      |             |  is created) | ``rescue_disk_ephemeral_encryption_secret_uuid``.    |"},{"line_number":186,"context_line":"   |                    |             |              | This is done because a BDM record for the rescue     |"},{"line_number":187,"context_line":"   |                    |             |              | disk is not going to be persisted to the database.   |"},{"line_number":188,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Snapshots of instances with ephemeral encryption"}],"source_content_type":"text/x-rst","patch_set":2,"id":"dec05d57_aae5f38c","line":187,"range":{"start_line":183,"start_character":5,"end_line":187,"end_character":106},"updated":"2023-07-11 17:03:43.000000000","message":"rescue is interesting as it may or may not use the same image and it needs to be able to decrpt the orgianl root disk. rescue has other uses but it is most often used to fix a broken bootloader or filesystem currption.\n\nso for rescue to be used in a encpyted sotrage context encyption fo the rescue disk is only need if the rescue image request it or the flavor does.\n\nrescue will also only work for the use that orgianl launched the vm as only they will ahve access to teh barbican secret to decypt the orgianl vms disks.\n\n\n\ni think all of this is fine but its imporant to note that unless an admin has intentionally weakened the barbarian config to allow admins to retirve end users secrets, admin will not be able to resuce end users disks.\n\nallowing that by default woudl allow admins to steal end users data and defeat the reason for intoducing this feature.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0e1b4f29d6a5124d02a74d4055968db672fd536c","unresolved":true,"context_lines":[{"line_number":180,"context_line":"   |                    |             |              | we could create a new secret using the instance\u0027s    |"},{"line_number":181,"context_line":"   |                    |             |              | user/project rather than the shelver\u0027s user/project  |"},{"line_number":182,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":183,"context_line":"   | Rescue disk        | disk (root) | Secret 11    | Secret 11 is stashed in the instance\u0027s system        |"},{"line_number":184,"context_line":"   | created by rescue  |             | (new secret  | metadata with key                                    |"},{"line_number":185,"context_line":"   | of Instance A      |             |  is created) | ``rescue_disk_ephemeral_encryption_secret_uuid``.    |"},{"line_number":186,"context_line":"   |                    |             |              | This is done because a BDM record for the rescue     |"},{"line_number":187,"context_line":"   |                    |             |              | disk is not going to be persisted to the database.   |"},{"line_number":188,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Snapshots of instances with ephemeral encryption"}],"source_content_type":"text/x-rst","patch_set":2,"id":"89df6a2b_e7ef7642","line":187,"range":{"start_line":183,"start_character":5,"end_line":187,"end_character":106},"in_reply_to":"53d9b2e5_a8f33c65","updated":"2023-07-11 20:51:52.000000000","message":"ya i think skiping is best either via regex or ideally by adding a compute feature flag for ephmeral encyption.\n\nin either case i think skiping it in the job is reasonable but im not sure how.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"471b48dd0fc0376e90bca2d702bf534b9ac9415f","unresolved":false,"context_lines":[{"line_number":180,"context_line":"   |                    |             |              | we could create a new secret using the instance\u0027s    |"},{"line_number":181,"context_line":"   |                    |             |              | user/project rather than the shelver\u0027s user/project  |"},{"line_number":182,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":183,"context_line":"   | Rescue disk        | disk (root) | Secret 11    | Secret 11 is stashed in the instance\u0027s system        |"},{"line_number":184,"context_line":"   | created by rescue  |             | (new secret  | metadata with key                                    |"},{"line_number":185,"context_line":"   | of Instance A      |             |  is created) | ``rescue_disk_ephemeral_encryption_secret_uuid``.    |"},{"line_number":186,"context_line":"   |                    |             |              | This is done because a BDM record for the rescue     |"},{"line_number":187,"context_line":"   |                    |             |              | disk is not going to be persisted to the database.   |"},{"line_number":188,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Snapshots of instances with ephemeral encryption"}],"source_content_type":"text/x-rst","patch_set":2,"id":"4ae5d90c_b82179bd","line":187,"range":{"start_line":183,"start_character":5,"end_line":187,"end_character":106},"in_reply_to":"5c4a724b_bf3aa409","updated":"2023-08-01 10:29:01.000000000","message":"Ack","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23ac8c8f3b3d927b5e1d744c61cb41cbc9f502bd","unresolved":true,"context_lines":[{"line_number":180,"context_line":"   |                    |             |              | we could create a new secret using the instance\u0027s    |"},{"line_number":181,"context_line":"   |                    |             |              | user/project rather than the shelver\u0027s user/project  |"},{"line_number":182,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":183,"context_line":"   | Rescue disk        | disk (root) | Secret 11    | Secret 11 is stashed in the instance\u0027s system        |"},{"line_number":184,"context_line":"   | created by rescue  |             | (new secret  | metadata with key                                    |"},{"line_number":185,"context_line":"   | of Instance A      |             |  is created) | ``rescue_disk_ephemeral_encryption_secret_uuid``.    |"},{"line_number":186,"context_line":"   |                    |             |              | This is done because a BDM record for the rescue     |"},{"line_number":187,"context_line":"   |                    |             |              | disk is not going to be persisted to the database.   |"},{"line_number":188,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Snapshots of instances with ephemeral encryption"}],"source_content_type":"text/x-rst","patch_set":2,"id":"5c4a724b_bf3aa409","line":187,"range":{"start_line":183,"start_character":5,"end_line":187,"end_character":106},"in_reply_to":"89df6a2b_e7ef7642","updated":"2023-07-18 03:00:51.000000000","message":"I think tempest compute feature flag will probably be the way to go, that\u0027s how most other test skips are done.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47659226e75dee1d3c945c3cf17adb5bcadb55ee","unresolved":true,"context_lines":[{"line_number":180,"context_line":"   |                    |             |              | we could create a new secret using the instance\u0027s    |"},{"line_number":181,"context_line":"   |                    |             |              | user/project rather than the shelver\u0027s user/project  |"},{"line_number":182,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":183,"context_line":"   | Rescue disk        | disk (root) | Secret 11    | Secret 11 is stashed in the instance\u0027s system        |"},{"line_number":184,"context_line":"   | created by rescue  |             | (new secret  | metadata with key                                    |"},{"line_number":185,"context_line":"   | of Instance A      |             |  is created) | ``rescue_disk_ephemeral_encryption_secret_uuid``.    |"},{"line_number":186,"context_line":"   |                    |             |              | This is done because a BDM record for the rescue     |"},{"line_number":187,"context_line":"   |                    |             |              | disk is not going to be persisted to the database.   |"},{"line_number":188,"context_line":"   +--------------------+-------------+--------------+------------------------------------------------------+"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Snapshots of instances with ephemeral encryption"}],"source_content_type":"text/x-rst","patch_set":2,"id":"53d9b2e5_a8f33c65","line":187,"range":{"start_line":183,"start_character":5,"end_line":187,"end_character":106},"in_reply_to":"dec05d57_aae5f38c","updated":"2023-07-11 19:22:38.000000000","message":"\u003e i think all of this is fine but its imporant to note that unless an admin has intentionally weakened the barbarian config to allow admins to retirve end users secrets, admin will not be able to resuce end users disks.\n\u003e \n\u003e allowing that by default woudl allow admins to steal end users data and defeat the reason for intoducing this feature.\n\nYes, that is true ... For testing purposes in my DNM nova CI jobs patch, I enabled admin to retrieve end user secrets in the barbican policy config because there is one tempest test that tests whether an admin can shelve/unshelve an end user\u0027s instance. The test obviously fails if admin is not enabled to retrieve end user secrets.\n\nSo maybe what we should actually do is not configure the policy in CI to allow that and instead skip that test or do an expected fail on it if ephemeral encryption is enabled on the instance.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da983c96f8d8ce7deb028d011583b8b4b8fef2f9","unresolved":true,"context_lines":[{"line_number":216,"context_line":"This behavior could be avoided however if there is some way we could create a"},{"line_number":217,"context_line":"new encryption secret using the instance\u0027s user and project rather than the"},{"line_number":218,"context_line":"shelver\u0027s user and project. If that is possible, we would not need to reuse the"},{"line_number":219,"context_line":"encryption secret."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Rescue disk images created by rescuing instances with ephemeral encryption"},{"line_number":222,"context_line":"``````````````````````````````````````````````````````````````````````````"}],"source_content_type":"text/x-rst","patch_set":2,"id":"450be517_38e450ec","line":219,"updated":"2023-07-11 17:03:43.000000000","message":"that woudl require either having a vlaid token for the user or allowing request with teh admin or service role to do that in barbican.\n\ni could see the service role ebing allowed to do this i dont hink admin should by default.\nwith that said it would requrie a change to barbican to enabel this and i think its fiar to declare that out of scope of this spec.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47659226e75dee1d3c945c3cf17adb5bcadb55ee","unresolved":true,"context_lines":[{"line_number":216,"context_line":"This behavior could be avoided however if there is some way we could create a"},{"line_number":217,"context_line":"new encryption secret using the instance\u0027s user and project rather than the"},{"line_number":218,"context_line":"shelver\u0027s user and project. If that is possible, we would not need to reuse the"},{"line_number":219,"context_line":"encryption secret."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Rescue disk images created by rescuing instances with ephemeral encryption"},{"line_number":222,"context_line":"``````````````````````````````````````````````````````````````````````````"}],"source_content_type":"text/x-rst","patch_set":2,"id":"f5c0ecbb_291bec10","line":219,"in_reply_to":"450be517_38e450ec","updated":"2023-07-11 19:22:38.000000000","message":"OK, that\u0027s fair.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"471b48dd0fc0376e90bca2d702bf534b9ac9415f","unresolved":false,"context_lines":[{"line_number":216,"context_line":"This behavior could be avoided however if there is some way we could create a"},{"line_number":217,"context_line":"new encryption secret using the instance\u0027s user and project rather than the"},{"line_number":218,"context_line":"shelver\u0027s user and project. If that is possible, we would not need to reuse the"},{"line_number":219,"context_line":"encryption secret."},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"Rescue disk images created by rescuing instances with ephemeral encryption"},{"line_number":222,"context_line":"``````````````````````````````````````````````````````````````````````````"}],"source_content_type":"text/x-rst","patch_set":2,"id":"20c254c8_62c8bc5b","line":219,"in_reply_to":"f5c0ecbb_291bec10","updated":"2023-08-01 10:29:01.000000000","message":"Ack","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da983c96f8d8ce7deb028d011583b8b4b8fef2f9","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"a3f82ecf_a6b5b3ea","line":245,"updated":"2023-07-11 17:03:43.000000000","message":"note as i said above this is only required if the rescue iamge request encyption or its enabeld in the falvor.\n\nif the recue image is unencypted and its not enabeld in teh falvor there is no need to encypt it just because the vm orgianly was encypted.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a3dcf84d8d8ee5981ab7f12fe72a2e0eb0a8da57","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"84fdf6fb_ad12d28a","line":245,"in_reply_to":"0952576f_ae100a4a","updated":"2023-07-20 11:02:23.000000000","message":"ya im not sure eitehr\n\nfor consitency it might be perferable to pull it.\n\nwithout openeing a can of worms here rescule is kind of odd\n\nin general i think we should be ignoring the rescue image metadata and always using the metadta for the root disk because you can do things like change the numa toplogy or enable cpu pinning in the rescule image metadata. you could also resuqet traits. if we allow using the rescue image metadata in general then we should be checking the rescue image for compatiablity with the host via the scheduelr like we do for rebuild.\n\nthe counter argument is that you can configure the disk/net/graphics model and you might need to use a diffent bus in the rescue image because it has different drivers aviabele so you would want to use the metadta in that case.\nalso with the stabel image rescue feature you might want to us ethe rescue buse options to provide stabel device ordering but only want to set that on the rescue image instead of all of the images.\n\n\nso i feel like the more complex approch of merging both would actully be the right solution.\n\ni.e. load the root disk metadata but then selectivly merge in a limite set of filed form the rescue iamge be that the user speicifed one or the config selected one.\n\nim kind of ok saying we dont need to decide that here and we should leave that to the code review to decided exactly which metadata we use for the config specifid imaage","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"23ac8c8f3b3d927b5e1d744c61cb41cbc9f502bd","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"d22aa21f_5f42ccff","line":245,"in_reply_to":"2c8ef93b_8fd36320","updated":"2023-07-18 03:00:51.000000000","message":"I agree with all that. Thanks for pointing out the [libvirt]rescue_image_id -- I did see it in the code but I definitely need to go through it specifically to make sure it\u0027s handled correctly.\n\nAnd I agree having the image in rescue_image_id being encrypted would be a bad UX. I\u0027ll make a note to add a warning to the config option help and docs.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3c5b84ebc67586da951f6e4cb7a9ae1242d082cd","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"2c8ef93b_8fd36320","line":245,"in_reply_to":"75f31930_d4f42062","updated":"2023-07-11 20:58:35.000000000","message":"i posted with out finsihng\n\nso the order is api request then fallback to config option then fall back to the instance orgianl iamge.\n\n\nin any case assuming the flavor does not enable it your logic really just need to check fi the image actully used for rescue has requested it regardless of the souce.\n\ni would hope the specific code that need to care about this does not need to care although since the rebuild image is a libvirt specfic config option on the comptue node it might need speical handeling.\n\nit actully is kind of bad ux to ever have the config image be encypted as that might break endusers ablity to rebuild if the specified image refernce a secert\ni.e. if the image as actully a snapshot image for exmaple that hte cloud admin created\n\nwe shoudl proably add a waring in the docs to not do that.\nor update the config option doc text to call that out","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0e1b4f29d6a5124d02a74d4055968db672fd536c","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"75f31930_d4f42062","line":245,"in_reply_to":"7a143b0f_19f4cedb","updated":"2023-07-11 20:51:52.000000000","message":"yep that is what i was thinking too.\n\nthere is one other case to consider.\nthe rescue_disk can also be set in the nova.conf\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#libvirt.rescue_image_id\n\nthis is not set by default and it is only used if the api request does nto provide an image.\n\nthe order of precidence is \napi request then fallback to config option then fall back to","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"76f5079cdbb943515af5ead9662090ede19b2386","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"d0784976_744cab23","line":245,"in_reply_to":"84fdf6fb_ad12d28a","updated":"2023-07-20 16:37:47.000000000","message":"I\u0027m good to go over it during code review as well. For the dead simple approach as a starting point, I have just added pull of image meta from the CONF.libvirt.rescue_image_id if it has been selected as the rescue image. That is at least consistent with what the other rescue images handling is doing.\n\nWe can change that if we want during code review.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47659226e75dee1d3c945c3cf17adb5bcadb55ee","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"7a143b0f_19f4cedb","line":245,"in_reply_to":"a3f82ecf_a6b5b3ea","updated":"2023-07-11 19:22:38.000000000","message":"That makes sense and I think that\u0027s what I\u0027ve currently got ... The rescue disk will be encrypted in the following cases:\n\n * the rescue image has image property hw_ephemeral_encryption\u003dtrue, or\n * the instance flavor has hw:ephemeral_encryption\u003dtrue, or\n * no rescue image was specified and the instance\u0027s original image has image property hw_ephemeral_encryption\u003dtrue\n \nSo in the specific case of unencrypted rescue image was specified and no hw:ephemeral_encryption in the flavor, the rescue disk will be unencrypted.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"471b48dd0fc0376e90bca2d702bf534b9ac9415f","unresolved":false,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"4413d7fd_ba5dd7d5","line":245,"in_reply_to":"d0784976_744cab23","updated":"2023-08-01 10:29:01.000000000","message":"Ack","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4f6b37daa1939fea3ee3e9c68391f718be1d796d","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"The new encryption secret for the rescue disk is deleted from the key manager"},{"line_number":244,"context_line":"and the virt driver secret is also deleted when the instance is unrescued."},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"Cleanup of ephemeral encryption secrets"},{"line_number":247,"context_line":"```````````````````````````````````````"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"0952576f_ae100a4a","line":245,"in_reply_to":"d22aa21f_5f42ccff","updated":"2023-07-19 23:31:06.000000000","message":"OK, so I just went through the code around CONF.libvirt.rescue_image_id and currently, its image metadata is not considered at all as it\u0027s chosen at the compute/manager layer. The image meta used is selected from other images with the following precedence: rescue image if one was specified, base image for instance\u0027s current image, instance\u0027s current image:\n\nhttps://github.com/openstack/nova/blob/b6f4c57/compute/manager.py#L4563\n\nNot sure if that was intentional or an oversight. I could add code to be able to pull image metadata from the CONF.libvirt.rescue_image_id image if it was an oversight.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da983c96f8d8ce7deb028d011583b8b4b8fef2f9","unresolved":true,"context_lines":[{"line_number":254,"context_line":"Encryption secrets that are created when a snapshot is created are *never*"},{"line_number":255,"context_line":"deleted by Nova. It would only be acceptable to delete the secret if and when"},{"line_number":256,"context_line":"the snapshot image is deleted. Cleanup of secrets whose images have been"},{"line_number":257,"context_line":"deleted from Glance must be deleted manually by the user or an admin."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":".. note::"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"020a074b_d3ad89f7","line":257,"updated":"2023-07-11 17:03:43.000000000","message":"given we have broken the depndency on the snapshot secrete it would be safe for galce to delete teh secrete if the snapshot is deleted.\n\nso that could be a follow up enhancement to glance.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47659226e75dee1d3c945c3cf17adb5bcadb55ee","unresolved":true,"context_lines":[{"line_number":254,"context_line":"Encryption secrets that are created when a snapshot is created are *never*"},{"line_number":255,"context_line":"deleted by Nova. It would only be acceptable to delete the secret if and when"},{"line_number":256,"context_line":"the snapshot image is deleted. Cleanup of secrets whose images have been"},{"line_number":257,"context_line":"deleted from Glance must be deleted manually by the user or an admin."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":".. note::"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"fed27dc8_71b16e59","line":257,"in_reply_to":"020a074b_d3ad89f7","updated":"2023-07-11 19:22:38.000000000","message":"Yes, exactly.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"471b48dd0fc0376e90bca2d702bf534b9ac9415f","unresolved":false,"context_lines":[{"line_number":254,"context_line":"Encryption secrets that are created when a snapshot is created are *never*"},{"line_number":255,"context_line":"deleted by Nova. It would only be acceptable to delete the secret if and when"},{"line_number":256,"context_line":"the snapshot image is deleted. Cleanup of secrets whose images have been"},{"line_number":257,"context_line":"deleted from Glance must be deleted manually by the user or an admin."},{"line_number":258,"context_line":""},{"line_number":259,"context_line":".. note::"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"34af3405_9e087151","line":257,"in_reply_to":"fed27dc8_71b16e59","updated":"2023-08-01 10:29:01.000000000","message":"Ack","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"da983c96f8d8ce7deb028d011583b8b4b8fef2f9","unresolved":true,"context_lines":[{"line_number":267,"context_line":"    its parent should be supported in the next release of Ceph."},{"line_number":268,"context_line":"    When we are able to require a Ceph version \u003e\u003d v18, copy-on-write cloning"},{"line_number":269,"context_line":"    with ephemeral encryption can be enabled."},{"line_number":270,"context_line":"    See https://github.com/ceph/ceph/commit/1d3de19 for reference."},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"BlockDeviceMapping changes"},{"line_number":273,"context_line":"--------------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"402935a1_cddf2f73","line":270,"updated":"2023-07-11 17:03:43.000000000","message":"i think this is also fair to declare out of the current scope to contol scope.\nbut it could be added as a follow enhancemnet ideally by checkign the ceph version or as a fallback by a config option.\n\nto my knowadge we do not have a mine ceph version for nova so we cant actully jsut enable it unconstionally withtou the version check unless we decied to intoduce one liek we have for libvirt and qemu in the future.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"47659226e75dee1d3c945c3cf17adb5bcadb55ee","unresolved":true,"context_lines":[{"line_number":267,"context_line":"    its parent should be supported in the next release of Ceph."},{"line_number":268,"context_line":"    When we are able to require a Ceph version \u003e\u003d v18, copy-on-write cloning"},{"line_number":269,"context_line":"    with ephemeral encryption can be enabled."},{"line_number":270,"context_line":"    See https://github.com/ceph/ceph/commit/1d3de19 for reference."},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"BlockDeviceMapping changes"},{"line_number":273,"context_line":"--------------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"78acaf66_61878d1a","line":270,"in_reply_to":"402935a1_cddf2f73","updated":"2023-07-11 19:22:38.000000000","message":"Ack, makes sense.","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"471b48dd0fc0376e90bca2d702bf534b9ac9415f","unresolved":false,"context_lines":[{"line_number":267,"context_line":"    its parent should be supported in the next release of Ceph."},{"line_number":268,"context_line":"    When we are able to require a Ceph version \u003e\u003d v18, copy-on-write cloning"},{"line_number":269,"context_line":"    with ephemeral encryption can be enabled."},{"line_number":270,"context_line":"    See https://github.com/ceph/ceph/commit/1d3de19 for reference."},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"BlockDeviceMapping changes"},{"line_number":273,"context_line":"--------------------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"6a5ec99b_cdd8d705","line":270,"in_reply_to":"78acaf66_61878d1a","updated":"2023-08-01 10:29:01.000000000","message":"Ack","commit_id":"5acad23725189bd7cc4ecd78db76b784b2564612"}]}