)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28271,"name":"Josephine Seifert","email":"josephine.seifert@cloudandheat.com","username":"josei"},"change_message_id":"d83b57a933879144609fdb653613be6109912cc4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"15c005c3_d3a7e61e","updated":"2022-07-08 04:45:45.000000000","message":"I know, this is taking a lot of time, but we are waiting for the Secret Consumers in Barbican. ","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"b1c0e4d9028d02d9986a0c569f24cf1faba16757","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"084df48a_e125f706","updated":"2023-12-05 14:54:41.000000000","message":"This patchset is now superseded by https://review.opendev.org/c/openstack/glance/+/902648","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"ecf76ab56cade50ab5b5d26bfd07138918fa3d15","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b3aba597_1b409abd","in_reply_to":"15c005c3_d3a7e61e","updated":"2022-07-28 14:20:20.000000000","message":"Ack, let me know if anything else is needed from glance side.","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"}],"glance/api/v2/images.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"e4ad2482c138e04af539a5cd69e3468ab31078a4","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if encryption_key_id is None:"},{"line_number":74,"context_line":"            return"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        #TODO(Luzi): activate this, when registering and unregistering as"},{"line_number":77,"context_line":"        # consumer of a secret in Barbican and thus in Castellan is available"},{"line_number":78,"context_line":"        # try:"},{"line_number":79,"context_line":"        #     self._key_manager.register_consumer(context, encryption_key_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_4a672aea","line":76,"updated":"2020-05-20 10:12:28.000000000","message":"pep8: E265 block comment should start with \u0027# \u0027","commit_id":"19f54a62cb825a625d29862bf7a57236f80cc53a"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"e4ad2482c138e04af539a5cd69e3468ab31078a4","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        return"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    def _validate_encryption_parameters(self,**api_params):"},{"line_number":96,"context_line":"        enc_cipher \u003d \u0027os_encrypt_cipher\u0027"},{"line_number":97,"context_line":"        enc_provider \u003d \u0027os_encrypt_provider\u0027"},{"line_number":98,"context_line":"        enc_type \u003d \u0027os_encrypt_type\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_aa6246d6","line":95,"updated":"2020-05-20 10:12:28.000000000","message":"pep8: E231 missing whitespace after \u0027,\u0027","commit_id":"19f54a62cb825a625d29862bf7a57236f80cc53a"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"e4ad2482c138e04af539a5cd69e3468ab31078a4","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        cf \u003d extra_properties.get(\u0027container_format\u0027, None)"},{"line_number":138,"context_line":"        try:"},{"line_number":139,"context_line":"            # validate encryption parameters"},{"line_number":140,"context_line":"            self._validate_encryption_parameters(**dict(image, **extra_properties))"},{"line_number":141,"context_line":"            # In case the default visibility changes at some point in the"},{"line_number":142,"context_line":"            # future, we need to ensure that encrypted images are only visible"},{"line_number":143,"context_line":"            # for the project which owns the encryption key:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_8a5dc216","line":140,"updated":"2020-05-20 10:12:28.000000000","message":"pep8: E501 line too long (83 \u003e 79 characters)","commit_id":"19f54a62cb825a625d29862bf7a57236f80cc53a"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"e4ad2482c138e04af539a5cd69e3468ab31078a4","unresolved":false,"context_lines":[{"line_number":453,"context_line":"        if encryption_key_id is None:"},{"line_number":454,"context_line":"            return"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"        #TODO(Luzi): activate this, when registering and unregistering as"},{"line_number":457,"context_line":"        # consumer of a secret in Barbican and thus in Castellan is available"},{"line_number":458,"context_line":"        # try:"},{"line_number":459,"context_line":"        #     self._key_manager.unregister_consumer(context, encryption_key_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_ea9d5ebc","line":456,"updated":"2020-05-20 10:12:28.000000000","message":"pep8: E265 block comment should start with \u0027# \u0027","commit_id":"19f54a62cb825a625d29862bf7a57236f80cc53a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e34f1377824cd7ed918c6a11e256dfc3edceed7","unresolved":true,"context_lines":[{"line_number":91,"context_line":"        # except castellan_exception.KeyManagerError:"},{"line_number":92,"context_line":"        #     msg \u003d (\u0027Failed to register as consumer for encryption key %s\u0027 %"},{"line_number":93,"context_line":"        #            encryption_key_id)"},{"line_number":94,"context_line":"        #     LOG.warn(msg)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        return"},{"line_number":97,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7b146dd0_643c00f1","line":94,"updated":"2021-06-14 16:10:42.000000000","message":"I dunno about other reviewers, but I personally really don\u0027t like to leave commented-out code in the tree for more than just a very short time. It\u0027s confusing to read in a git-diff, among other things. Personally, I\u0027d rather this code just be put into a WIP patch on top of this, with a comment here about being a placeholder. Then when we\u0027re ready, we can clean up and merge the WIP patch.","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e34f1377824cd7ed918c6a11e256dfc3edceed7","unresolved":true,"context_lines":[{"line_number":115,"context_line":"            for param in all_keys:"},{"line_number":116,"context_line":"                if param in metadata:"},{"line_number":117,"context_line":"                    if not isinstance(metadata[param], str) or \\"},{"line_number":118,"context_line":"                            len(metadata[param]) \u003d\u003d 0:"},{"line_number":119,"context_line":"                        string_type \u003d \u0027non-empty string\u0027"},{"line_number":120,"context_line":"                        if param \u003d\u003d enc_key_id:"},{"line_number":121,"context_line":"                            string_type \u003d \u0027uuid\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3dc0d4ba_a6573839","line":118,"updated":"2021-06-14 16:10:42.000000000","message":"You\u0027re very deeply nested here for no real reason. Can you check the container_format and other base requirements at the beginning and just return if it\u0027s not one we care about or raise if things are missing? This would leave just the for loop to process the items without having the whole thing be inside multiple conditionals. In summary:\n\n if container_format !\u003d \u0027encrypted\u0027:\n     # Don\u0027t care\n     return\n\n if not metadata:\n     raise webob.exc.HTTPBadRequest(..)\n\n all_keys \u003d set([enc_cipher, ...])\n if not set(metadata.keys()).issuperset(all_keys):\n     raise webob.exc(HTTPBadRequest(...))\n\n for param in all_keys:\n     # All the keys are provided, so just check and process them here","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e34f1377824cd7ed918c6a11e256dfc3edceed7","unresolved":true,"context_lines":[{"line_number":118,"context_line":"                            len(metadata[param]) \u003d\u003d 0:"},{"line_number":119,"context_line":"                        string_type \u003d \u0027non-empty string\u0027"},{"line_number":120,"context_line":"                        if param \u003d\u003d enc_key_id:"},{"line_number":121,"context_line":"                            string_type \u003d \u0027uuid\u0027"},{"line_number":122,"context_line":"                        msg \u003d \"The API parameter \u0027%s\u0027 must be a %s.\" \\"},{"line_number":123,"context_line":"                              % (param, string_type)"},{"line_number":124,"context_line":"                        raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"44a47355_9225484c","line":121,"updated":"2021-06-14 16:10:42.000000000","message":"You\u0027re not really checking that this is a uuid, you\u0027re just giving a different reason if the uuid is empty or not a string. That might be confusing behavior.\n\nRegardless, can\u0027t we validate all of this with JSON schema to avoid the need to worry about all these formatting details?","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e34f1377824cd7ed918c6a11e256dfc3edceed7","unresolved":true,"context_lines":[{"line_number":145,"context_line":"            # In case the default visibility changes at some point in the"},{"line_number":146,"context_line":"            # future, we need to ensure that encrypted images are only visible"},{"line_number":147,"context_line":"            # for the project which owns the encryption key:"},{"line_number":148,"context_line":"            if \u0027visibility\u0027 not in image:"},{"line_number":149,"context_line":"                if cf \u003d\u003d \u0027encrypted\u0027:"},{"line_number":150,"context_line":"                    image[\u0027visibility\u0027] \u003d \u0027private\u0027"},{"line_number":151,"context_line":"            image \u003d image_factory.new_image(extra_properties\u003dextra_properties,"},{"line_number":152,"context_line":"                                            tags\u003dtags, **image)"}],"source_content_type":"text/x-python","patch_set":4,"id":"b1d0a8c8_4155eda5","line":149,"range":{"start_line":148,"start_character":0,"end_line":149,"end_character":37},"updated":"2021-06-14 16:10:42.000000000","message":"Can you make this a single if with \"$cond1 and $cond2\"?","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e34f1377824cd7ed918c6a11e256dfc3edceed7","unresolved":true,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        # if it is an encrypted image, register as secret consumer in Barbican"},{"line_number":176,"context_line":"        if cf \u003d\u003d \u0027encrypted\u0027:"},{"line_number":177,"context_line":"            self._register_as_secret_consumer(req.context, image)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"        return image"},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ec8bc13c_1baecbe7","line":177,"updated":"2021-06-14 16:10:42.000000000","message":"What happens if this fails? It looks to me like your proposed code will just silently ignore failures, which seems un-ideal, especially if the result is that the user could lose all their data by deleting a key that isn\u0027t refcounted like it is supposed to be.\n\nSeems like we should delete the image we just created if we were unable to register, no?","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3e34f1377824cd7ed918c6a11e256dfc3edceed7","unresolved":true,"context_lines":[{"line_number":763,"context_line":"                        \"image data has failed because \""},{"line_number":764,"context_line":"                        \"it cannot be found at %(fn)s\"), {\u0027fn\u0027: file_path})"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":"            self._unregister_as_secret_consumer(req.context, image)"},{"line_number":767,"context_line":"            image.delete()"},{"line_number":768,"context_line":"            self._delete_encryption_key(req.context, image)"},{"line_number":769,"context_line":"            image_repo.remove(image)"}],"source_content_type":"text/x-python","patch_set":4,"id":"4b11f765_578e86f2","line":766,"updated":"2021-06-14 16:10:42.000000000","message":"Presumably we could fail the delete below if we don\u0027t succeed in making a call to cinder or swift or something, but we\u0027ve already dropped our reference to the key. If the delete failed, but the user changes their mind, they won\u0027t realize that their key is now exposed and able to be deleted, right?","commit_id":"e409bf8ebf284a7dfdba7ffd73196e46116581d8"}]}