)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"717d76ed2f659bad8f4fb5e543c5ae8189a0eb52","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Related bp: improve-encrypted-volume"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I1f4ea35cd05f4c43a5ae07d8a541ff6495d5f8e9"},{"line_number":20,"context_line":"(cherry picked from commit 14e3470fdcfff00164dd814e74325b5bba10554b)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"bff0334d_b2e4450e","line":20,"range":{"start_line":20,"start_character":0,"end_line":20,"end_character":68},"updated":"2017-04-11 16:47:25.000000000","message":"Don\u0027t think you meant to have this in here.","commit_id":"32dd1309de39aac59b5cf9886f9ca73df45927e0"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"7678ab5d584e73e3eb6346d5a2011fe6e6d69e06","unresolved":false,"context_lines":[{"line_number":12,"context_line":"is copied directly.  If the keys don\u0027t match, the creation"},{"line_number":13,"context_line":"fails.   (Before this fix, you end up with a useless"},{"line_number":14,"context_line":"doubly-encrypted volume.)"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Related-Bug: #1485449"},{"line_number":17,"context_line":"Related bp: improve-encrypted-volume"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"bff0334d_363c82df","line":15,"updated":"2017-04-12 14:48:46.000000000","message":"Should note that this currently only solves the issue when using ConfKeyManager and not barbican.\n\nMaking this work w/ barbican requires a bit more code.","commit_id":"4d017c0bcaeda017df00d5e1e92115aa02b8ece6"}],"cinder/tests/unit/volume/flows/test_create_volume_flow.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0f4768c11fb2d4609f26cbc7a3db87b0fca86211","unresolved":false,"context_lines":[{"line_number":836,"context_line":"        image_meta[\u0027id\u0027] \u003d image_id"},{"line_number":837,"context_line":"        image_meta[\u0027status\u0027] \u003d \u0027active\u0027"},{"line_number":838,"context_line":"        image_meta[\u0027size\u0027] \u003d 1"},{"line_number":839,"context_line":"        image_meta[\u0027cinder_metadata\u0027] \u003d \\"},{"line_number":840,"context_line":"            \u0027{\"encryption_key_id\": \"00000000-0000-0000-0000-000000000000\"}\u0027"},{"line_number":841,"context_line":"        image_location \u003d \u0027abc\u0027"},{"line_number":842,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9ff52f3f_6566c254","line":839,"range":{"start_line":839,"start_character":8,"end_line":839,"end_character":41},"updated":"2017-04-12 23:27:02.000000000","message":"Needs update","commit_id":"bb338bf67db373c6f76f6300090242e6f69aa677"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"4839c4f716d43ad0abbd29d2e51b4197488436c9","unresolved":false,"context_lines":[{"line_number":840,"context_line":"            \u0027{\"encryption_key_id\": \"00000000-0000-0000-0000-000000000000\"}\u0027"},{"line_number":841,"context_line":"        image_location \u003d \u0027abc\u0027"},{"line_number":842,"context_line":""},{"line_number":843,"context_line":"        # This should fail since the keys are different..."},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        fake_db.volume_update.return_value \u003d volume"},{"line_number":846,"context_line":"        fake_manager._create_from_image(self.ctxt, volume,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9ff52f3f_6567e227","line":843,"updated":"2017-04-12 23:34:23.000000000","message":"Does this whole test need rethinking given the way the volume is now created with the image\u0027s key? Not sure of the full scope of your \"needs update\" comment.","commit_id":"bb338bf67db373c6f76f6300090242e6f69aa677"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"110d14a83b12b48100868614c8b0d5b112337edd","unresolved":false,"context_lines":[{"line_number":840,"context_line":"            \u0027{\"encryption_key_id\": \"00000000-0000-0000-0000-000000000000\"}\u0027"},{"line_number":841,"context_line":"        image_location \u003d \u0027abc\u0027"},{"line_number":842,"context_line":""},{"line_number":843,"context_line":"        # This should fail since the keys are different..."},{"line_number":844,"context_line":""},{"line_number":845,"context_line":"        fake_db.volume_update.return_value \u003d volume"},{"line_number":846,"context_line":"        fake_manager._create_from_image(self.ctxt, volume,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9ff52f3f_08126b59","line":843,"in_reply_to":"9ff52f3f_6567e227","updated":"2017-04-12 23:41:36.000000000","message":"The test is doing the right thing, the comment is just wrong here.","commit_id":"bb338bf67db373c6f76f6300090242e6f69aa677"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0f4768c11fb2d4609f26cbc7a3db87b0fca86211","unresolved":false,"context_lines":[{"line_number":928,"context_line":"                      \u0027size\u0027: 1024,"},{"line_number":929,"context_line":"                      \u0027owner\u0027: owner or self.ctxt.project_id,"},{"line_number":930,"context_line":"                      \u0027virtual_size\u0027: None,"},{"line_number":931,"context_line":"                      \u0027cinder_metadata\u0027: None}"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"        fake_driver.clone_image.return_value \u003d (None, False)"},{"line_number":934,"context_line":"        fake_db.volume_get_all_by_host.return_value \u003d [image_volume]"}],"source_content_type":"text/x-python","patch_set":11,"id":"9ff52f3f_0555b650","line":931,"updated":"2017-04-12 23:27:02.000000000","message":"Needs update","commit_id":"bb338bf67db373c6f76f6300090242e6f69aa677"}],"cinder/tests/unit/volume/test_image.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"717d76ed2f659bad8f4fb5e543c5ae8189a0eb52","unresolved":false,"context_lines":[{"line_number":496,"context_line":"        volume \u003d objects.Volume(context\u003dself.context, **kwargs)"},{"line_number":497,"context_line":"        volume.create()"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        NON_EXISTENT_IMAGE_ID \u003d \u0027003f540f-ec6b-4293-a3f9-7c68646b0f5c\u0027"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        self.assertRaises(exception.ImageNotFound,"},{"line_number":502,"context_line":"                          self.volume.create_volume,"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_7298fd73","line":499,"range":{"start_line":499,"start_character":8,"end_line":499,"end_character":29},"updated":"2017-04-11 16:47:25.000000000","message":"Why are we doing this in two different places?  Can\u0027t it just be made a global?","commit_id":"32dd1309de39aac59b5cf9886f9ca73df45927e0"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"8a2e7ffd0ee59ff24d6139ed32528c2991e1edb7","unresolved":false,"context_lines":[{"line_number":496,"context_line":"        volume \u003d objects.Volume(context\u003dself.context, **kwargs)"},{"line_number":497,"context_line":"        volume.create()"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        NON_EXISTENT_IMAGE_ID \u003d \u0027003f540f-ec6b-4293-a3f9-7c68646b0f5c\u0027"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        self.assertRaises(exception.ImageNotFound,"},{"line_number":502,"context_line":"                          self.volume.create_volume,"}],"source_content_type":"text/x-python","patch_set":6,"id":"bff0334d_884eb4be","line":499,"range":{"start_line":499,"start_character":8,"end_line":499,"end_character":29},"in_reply_to":"bff0334d_7298fd73","updated":"2017-04-11 17:50:52.000000000","message":"I guess we could define it somewhere, but this just means that people have to hunt through more code if a unit test fails -- not sure what the advantage is.","commit_id":"32dd1309de39aac59b5cf9886f9ca73df45927e0"}],"cinder/volume/flows/api/create_volume.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"c327cfde5914f3473ab98260cdeb0a8147efe55e","unresolved":false,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        if image_id is not None and \u0027encryption_key_id\u0027 in image_meta:"},{"line_number":451,"context_line":"            # encryption key id is specified by Glance metadata"},{"line_number":452,"context_line":"            encryption_key_id \u003d image_meta[\u0027encryption_key_id\u0027]"},{"line_number":453,"context_line":"        else:"},{"line_number":454,"context_line":"            # look up encryption key via volume type"},{"line_number":455,"context_line":"            encryption_key_id \u003d self._get_encryption_key_id(key_manager,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_07ca186c","line":452,"range":{"start_line":452,"start_character":32,"end_line":452,"end_character":63},"updated":"2017-04-06 20:36:08.000000000","message":"This is wrong, needs to look at image_meta[\u0027properties\u0027][\u0027cinder_metadata\u0027] for encryption_key_id.","commit_id":"cf8f8a734814dd5de6213e993618839401dfa794"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"610b5313b6e0190b84373732c97fb9107f8f03ab","unresolved":false,"context_lines":[{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        image_meta \u003d {}"},{"line_number":425,"context_line":"        if image_id is not None:"},{"line_number":426,"context_line":"            image_meta \u003d self.image_service.show(context, image_id)"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"        # TODO(joel-coffman): This special handling of snapshots to ensure that"},{"line_number":429,"context_line":"        # their volume type matches the source volume is too convoluted. We"}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_d1cf27ae","line":426,"updated":"2017-04-07 15:18:34.000000000","message":"??: I\u0027m failing to see why we are making these changes, since we don\u0027t seem to need the image metadata in this method and it\u0027s only needed by _get_image_volume_type....","commit_id":"14e3470fdcfff00164dd814e74325b5bba10554b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"084f4d386949ffc2814e2ecc8cc478ed1b272e65","unresolved":false,"context_lines":[{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        image_meta \u003d {}"},{"line_number":425,"context_line":"        if image_id is not None:"},{"line_number":426,"context_line":"            image_meta \u003d self.image_service.show(context, image_id)"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"        # TODO(joel-coffman): This special handling of snapshots to ensure that"},{"line_number":429,"context_line":"        # their volume type matches the source volume is too convoluted. We"}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_499a4b6e","line":426,"in_reply_to":"bff0334d_a090cb4f","updated":"2017-04-07 20:05:47.000000000","message":"Done","commit_id":"14e3470fdcfff00164dd814e74325b5bba10554b"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"ae67701da4ab2d97bb49e89904fb5163936b84fa","unresolved":false,"context_lines":[{"line_number":423,"context_line":""},{"line_number":424,"context_line":"        image_meta \u003d {}"},{"line_number":425,"context_line":"        if image_id is not None:"},{"line_number":426,"context_line":"            image_meta \u003d self.image_service.show(context, image_id)"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"        # TODO(joel-coffman): This special handling of snapshots to ensure that"},{"line_number":429,"context_line":"        # their volume type matches the source volume is too convoluted. We"}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_a090cb4f","line":426,"in_reply_to":"bff0334d_d1cf27ae","updated":"2017-04-07 18:03:07.000000000","message":"I agree the original abstraction seems cleaner.","commit_id":"14e3470fdcfff00164dd814e74325b5bba10554b"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"ae67701da4ab2d97bb49e89904fb5163936b84fa","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        # This strategy avoids any dependency upon the encrypted volume type."},{"line_number":433,"context_line":"        def_vol_type \u003d volume_types.get_default_volume_type()"},{"line_number":434,"context_line":"        if not volume_type and not source_volume and not snapshot:"},{"line_number":435,"context_line":"            image_volume_type \u003d None"},{"line_number":436,"context_line":"            if image_id is not None:"},{"line_number":437,"context_line":"                image_volume_type \u003d self._get_image_volume_type(context,"},{"line_number":438,"context_line":"                                                                image_meta)"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"            volume_type \u003d (image_volume_type if image_volume_type else"},{"line_number":441,"context_line":"                           def_vol_type)"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        # When creating a clone of a replica (replication test), we can\u0027t"},{"line_number":444,"context_line":"        # use the volume type of the replica, therefore, we use the default."}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_20c27bea","line":441,"range":{"start_line":435,"start_character":1,"end_line":441,"end_character":40},"updated":"2017-04-07 18:03:07.000000000","message":"If you keep the original _get_image_volume_type() API then this is pretty clean:\n\nif image_id:\n    volume_type \u003d self._get_image_volume_type(context, image_id)\nelse:\n    volume_type \u003d def_vol_type","commit_id":"14e3470fdcfff00164dd814e74325b5bba10554b"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3f36ae0af04be76e8d0eb73db78487e35385cba7","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    def _check_image_metadata(self, context, image_id, size):"},{"line_number":202,"context_line":"        \"\"\"Checks image existence and validates that the image metadata."},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        Returns: encryption key from image metadata or None"},{"line_number":205,"context_line":"        \"\"\""},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"        # Check image existence"}],"source_content_type":"text/x-python","patch_set":10,"id":"bff0334d_0ca60610","line":204,"updated":"2017-04-12 21:11:05.000000000","message":"This feels pretty arbitrary given the function\u0027s original purpose and its name. I think it would be cleaner if it were repurposed to something like _get_image_metadata(). It would still implement the checks as it currently does.\n\nSee my comments later in this file for how the whole thing would work.","commit_id":"55a76c7db46c289fdac59b2cd763ae3c489cd590"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3f36ae0af04be76e8d0eb73db78487e35385cba7","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            msg \u003d msg % {\u0027volume_size\u0027: size, \u0027min_disk\u0027: min_disk}"},{"line_number":236,"context_line":"            raise exception.InvalidInput(reason\u003dmsg)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        image_encryption_key_id \u003d image_meta.get(\u0027cinder_encryption_key_id\u0027)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"        return image_encryption_key_id"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def _get_image_volume_type(self, context, image_id):"},{"line_number":243,"context_line":"        \"\"\"Get cinder_img_volume_type property from the image metadata.\"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"bff0334d_6c86b268","line":240,"range":{"start_line":238,"start_character":0,"end_line":240,"end_character":38},"updated":"2017-04-12 21:11:05.000000000","message":"Forget about the key for now, just return image_meta.","commit_id":"55a76c7db46c289fdac59b2cd763ae3c489cd590"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3f36ae0af04be76e8d0eb73db78487e35385cba7","unresolved":false,"context_lines":[{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    def _get_encryption_key_id(self, key_manager, context, volume_type_id,"},{"line_number":366,"context_line":"                               snapshot, source_volume,"},{"line_number":367,"context_line":"                               image_encryption_key_id):"},{"line_number":368,"context_line":"        encryption_key_id \u003d None"},{"line_number":369,"context_line":"        if volume_types.is_encrypted(context, volume_type_id):"},{"line_number":370,"context_line":"            if snapshot is not None:  # creating from snapshot"}],"source_content_type":"text/x-python","patch_set":10,"id":"bff0334d_4c89ae72","line":367,"updated":"2017-04-12 21:11:05.000000000","message":"Change this to supply the image_meta and not the key","commit_id":"55a76c7db46c289fdac59b2cd763ae3c489cd590"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3f36ae0af04be76e8d0eb73db78487e35385cba7","unresolved":false,"context_lines":[{"line_number":371,"context_line":"                encryption_key_id \u003d snapshot[\u0027encryption_key_id\u0027]"},{"line_number":372,"context_line":"            elif source_volume is not None:  # cloning volume"},{"line_number":373,"context_line":"                encryption_key_id \u003d source_volume[\u0027encryption_key_id\u0027]"},{"line_number":374,"context_line":"            elif image_encryption_key_id is not None:  # creating from image"},{"line_number":375,"context_line":"                encryption_key_id \u003d image_encryption_key_id"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"            # NOTE(joel-coffman): References to the encryption key should *not*"}],"source_content_type":"text/x-python","patch_set":10,"id":"bff0334d_4cee4e22","line":374,"updated":"2017-04-12 21:11:05.000000000","message":"elif image_meta is not None:\n  encryption_key_id \u003d image_meta.get[\u0027cinder_encryption_key_id\u0027]","commit_id":"55a76c7db46c289fdac59b2cd763ae3c489cd590"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3f36ae0af04be76e8d0eb73db78487e35385cba7","unresolved":false,"context_lines":[{"line_number":430,"context_line":"        cgsnapshot_id \u003d self._extract_cgsnapshot(cgsnapshot)"},{"line_number":431,"context_line":"        group_id \u003d self._extract_group(group)"},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"        image_encryption_key_id \u003d self._check_image_metadata(context,"},{"line_number":434,"context_line":"                                                             image_id,"},{"line_number":435,"context_line":"                                                             size)"},{"line_number":436,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"bff0334d_0cf4c6ef","line":433,"updated":"2017-04-12 21:11:05.000000000","message":"This would become:\n\nimage_meta \u003d self._get_image_metadata(...), with the side effect that it also checks the metadata","commit_id":"55a76c7db46c289fdac59b2cd763ae3c489cd590"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3f36ae0af04be76e8d0eb73db78487e35385cba7","unresolved":false,"context_lines":[{"line_number":465,"context_line":"            volume_type_id,"},{"line_number":466,"context_line":"            snapshot,"},{"line_number":467,"context_line":"            source_volume,"},{"line_number":468,"context_line":"            image_encryption_key_id)"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"        specs \u003d {}"},{"line_number":471,"context_line":"        if volume_type_id:"}],"source_content_type":"text/x-python","patch_set":10,"id":"bff0334d_4cd76e4c","line":468,"updated":"2017-04-12 21:11:05.000000000","message":"Pass in image_meta instead of image_encryption_key_id","commit_id":"55a76c7db46c289fdac59b2cd763ae3c489cd590"}],"cinder/volume/flows/manager/create_volume.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"0efbf11fb788dc5e5c9f7f94401f688e3232cc2b","unresolved":false,"context_lines":[{"line_number":532,"context_line":"            image_info \u003d image_service.show(context, image_id)"},{"line_number":533,"context_line":"            image_cinder_metadata \u003d image_info[\u0027properties\u0027].get("},{"line_number":534,"context_line":"                \u0027cinder_metadata\u0027,"},{"line_number":535,"context_line":"                {})"},{"line_number":536,"context_line":"            image_cinder_metadata \u003d jsonutils.loads(image_cinder_metadata)"},{"line_number":537,"context_line":"            image_encryption_key \u003d image_cinder_metadata.get("},{"line_number":538,"context_line":"                \u0027encryption_key_id\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bff0334d_520db23c","line":535,"updated":"2017-04-05 18:39:52.000000000","message":"Should be \u0027{}\u0027 (json string) instead of empty dict.","commit_id":"80c32f98e4d7ade7f469b49ea4664b44f9366e61"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"8ad514334e287743e23392b2ac60fc3791e566fd","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            already_encrypted \u003d image_encryption_key is not None"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"            if volume.encryption_key_id and not already_encrypted:"},{"line_number":542,"context_line":"                if image_encryption_key is not None:"},{"line_number":543,"context_line":"                    # Only works if keys match."},{"line_number":544,"context_line":"                    if volume.encryption_key_id !\u003d image_encryption_key:"},{"line_number":545,"context_line":"                        raise exception.ImageCopyFailure("}],"source_content_type":"text/x-python","patch_set":2,"id":"bff0334d_5b94ad44","line":542,"updated":"2017-04-05 19:55:55.000000000","message":"This logic looks wrong for this case, the \"image_encryption_key is not None\" case needs to be under \"already_encrypted\" (else) instead of here.","commit_id":"cf8f8a734814dd5de6213e993618839401dfa794"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"23122b0c8b99ae907967a7469712573829e2dab3","unresolved":false,"context_lines":[{"line_number":529,"context_line":"                  {\u0027image_id\u0027: image_id, \u0027volume_id\u0027: volume.id,"},{"line_number":530,"context_line":"                   \u0027image_location\u0027: image_location})"},{"line_number":531,"context_line":"        try:"},{"line_number":532,"context_line":"            image_info \u003d image_service.show(context, image_id)"},{"line_number":533,"context_line":"            image_properties \u003d image_info.get(\u0027properties\u0027, {})"},{"line_number":534,"context_line":"            image_cinder_metadata \u003d image_properties.get("},{"line_number":535,"context_line":"                \u0027cinder_metadata\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_feb2fae5","line":532,"updated":"2017-04-07 15:04:27.000000000","message":"-1: We have already retrieved this information when preparing the specs (L318) no need to query Glance again, just pass it from L776.","commit_id":"14e3470fdcfff00164dd814e74325b5bba10554b"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"084f4d386949ffc2814e2ecc8cc478ed1b272e65","unresolved":false,"context_lines":[{"line_number":529,"context_line":"                  {\u0027image_id\u0027: image_id, \u0027volume_id\u0027: volume.id,"},{"line_number":530,"context_line":"                   \u0027image_location\u0027: image_location})"},{"line_number":531,"context_line":"        try:"},{"line_number":532,"context_line":"            image_info \u003d image_service.show(context, image_id)"},{"line_number":533,"context_line":"            image_properties \u003d image_info.get(\u0027properties\u0027, {})"},{"line_number":534,"context_line":"            image_cinder_metadata \u003d image_properties.get("},{"line_number":535,"context_line":"                \u0027cinder_metadata\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_9af9afc9","line":532,"in_reply_to":"bff0334d_feb2fae5","updated":"2017-04-07 20:05:47.000000000","message":"Done","commit_id":"14e3470fdcfff00164dd814e74325b5bba10554b"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"aeadf831023908429c4c0b6aaac990f633dee516","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            image_encryption_key \u003d image_cinder_metadata.get("},{"line_number":540,"context_line":"                \u0027encryption_key_id\u0027)"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"            if volume.encryption_key_id and image_encryption_key:"},{"line_number":543,"context_line":"                # Only works if keys match."},{"line_number":544,"context_line":"                if volume.encryption_key_id !\u003d image_encryption_key:"},{"line_number":545,"context_line":"                    raise exception.ImageCopyFailure("}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_e106b91c","line":542,"updated":"2017-04-10 08:38:54.000000000","message":"-1\n\nI am wondering under what scenario encryption id of a new volume is same as image\u0027s. Please don\u0027t use fixed key to do test. \n\nAs Cinder create new encryption id when creating volume:  \nhttps://github.com/openstack/cinder/blob/master/cinder/volu\nme/flows/api/create_volume.py#L375-L386\n\nShould we consider similar way like snapshot or source volume when creating encryption key?\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/flows/api/create_volume.py#L362-L365","commit_id":"460bdd4548e96591aa859e22b2907d3347236452"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"77067c0a8e647a5d0745b761e0e4fa19d10219be","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            image_encryption_key \u003d image_cinder_metadata.get("},{"line_number":540,"context_line":"                \u0027encryption_key_id\u0027)"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"            if volume.encryption_key_id and image_encryption_key:"},{"line_number":543,"context_line":"                # Only works if keys match."},{"line_number":544,"context_line":"                if volume.encryption_key_id !\u003d image_encryption_key:"},{"line_number":545,"context_line":"                    raise exception.ImageCopyFailure("}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_4f79d691","line":542,"in_reply_to":"bff0334d_9d15f0e7","updated":"2017-04-11 16:07:53.000000000","message":"I think I better understand the concern here now -- I was assuming we could reuse keys between different volumes even when using Barbican.  This doesn\u0027t really work because Cinder deletes the key when the volume is deleted.\n\nThis is being resolved in the patch prior to this one by not reusing volume keys.\n\nThis code should handle both the fixed key manager and barbican keys in a useful way.","commit_id":"460bdd4548e96591aa859e22b2907d3347236452"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"fcf792f72d982640ab03c819dfd06cfacd1a1e1f","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            image_encryption_key \u003d image_cinder_metadata.get("},{"line_number":540,"context_line":"                \u0027encryption_key_id\u0027)"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"            if volume.encryption_key_id and image_encryption_key:"},{"line_number":543,"context_line":"                # Only works if keys match."},{"line_number":544,"context_line":"                if volume.encryption_key_id !\u003d image_encryption_key:"},{"line_number":545,"context_line":"                    raise exception.ImageCopyFailure("}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_9d15f0e7","line":542,"in_reply_to":"bff0334d_cc264216","updated":"2017-04-11 00:52:37.000000000","message":"In fixed key case, it always uses same key: \n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/flows/api/create_volume.py#L385\n\nhttps://github.com/openstack/cinder/blob/master/cinder/keymgr/conf_key_mgr.py#L100\n\nYou\u0027d better to use Barbican to test.","commit_id":"460bdd4548e96591aa859e22b2907d3347236452"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"635186bca981eccccd231502405bf9282744e907","unresolved":false,"context_lines":[{"line_number":539,"context_line":"            image_encryption_key \u003d image_cinder_metadata.get("},{"line_number":540,"context_line":"                \u0027encryption_key_id\u0027)"},{"line_number":541,"context_line":""},{"line_number":542,"context_line":"            if volume.encryption_key_id and image_encryption_key:"},{"line_number":543,"context_line":"                # Only works if keys match."},{"line_number":544,"context_line":"                if volume.encryption_key_id !\u003d image_encryption_key:"},{"line_number":545,"context_line":"                    raise exception.ImageCopyFailure("}],"source_content_type":"text/x-python","patch_set":5,"id":"bff0334d_cc264216","line":542,"in_reply_to":"bff0334d_e106b91c","updated":"2017-04-10 13:37:59.000000000","message":"IMO fixed keys are a valid case here -- for better or worse, some deployments use them, and handling them correctly would be better than the current behavior.\n\nWould it be better to test for key ids equal to \n\"00000000-0000-0000-0000-000000000000\" instead?","commit_id":"460bdd4548e96591aa859e22b2907d3347236452"}]}