)]}'
{"glance/api/middleware/cache.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"845ec73da70bd1cf0b61f071798e18085baf7946","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            # NOTE(abhishekk): This means image is present in cache and before"},{"line_number":171,"context_line":"            # request is coming to API we are enforcing this check in the"},{"line_number":172,"context_line":"            # middleware"},{"line_number":173,"context_line":"            self._enforce(request, \u0027download_image\u0027, target\u003dimage_metadata)"},{"line_number":174,"context_line":"        except exception.Forbidden:"},{"line_number":175,"context_line":"            return None"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"0f374115_b28c9f45","line":173,"updated":"2021-08-16 17:38:11.000000000","message":"I\u0027m concerned that we\u0027re doing the check with a different thing here, and that we could end up with different behavior whether the image is cached or not. Why not just use our api_pol pattern for this as well?","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"66d325557ffbb55bbbb4876e89fa5f4dda73f6f7","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            # NOTE(abhishekk): This means image is present in cache and before"},{"line_number":171,"context_line":"            # request is coming to API we are enforcing this check in the"},{"line_number":172,"context_line":"            # middleware"},{"line_number":173,"context_line":"            self._enforce(request, \u0027download_image\u0027, target\u003dimage_metadata)"},{"line_number":174,"context_line":"        except exception.Forbidden:"},{"line_number":175,"context_line":"            return None"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b8b70d99_9986ce3b","line":173,"in_reply_to":"0f374115_b28c9f45","updated":"2021-08-16 18:00:27.000000000","message":"this image_metadat is nothing but the instance of ImageTarget only see,\n\nhttps://review.opendev.org/c/openstack/glance/+/804547/1/glance/api/middleware/cache.py@118\n\nIf we need to use our api_pol pattern I need to make other changes as well, but if you insist then I could refactor this to use our api_pol.","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"64b2c6dceb37dfe182efcdecdfba8bd6a0560095","unresolved":true,"context_lines":[{"line_number":170,"context_line":"            # NOTE(abhishekk): This means image is present in cache and before"},{"line_number":171,"context_line":"            # request is coming to API we are enforcing this check in the"},{"line_number":172,"context_line":"            # middleware"},{"line_number":173,"context_line":"            self._enforce(request, \u0027download_image\u0027, target\u003dimage_metadata)"},{"line_number":174,"context_line":"        except exception.Forbidden:"},{"line_number":175,"context_line":"            return None"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"cbf723f0_b61d9ef6","line":173,"in_reply_to":"b8b70d99_9986ce3b","updated":"2021-08-16 18:07:22.000000000","message":"\u003e this image_metadat is nothing but the instance of ImageTarget only see,\n\u003e \n\u003e https://review.opendev.org/c/openstack/glance/+/804547/1/glance/api/middleware/cache.py@118\n\nYeah, I know it\u0027s just ImageTarget, but that\u0027s kinda the point. We\u0027re wrapping everything up in v2/policy, which means it *looks* like any changes we need to make to image visibility can be made there, except this is separate.\n\nMore specifically, what if we added more behavior around download_image (like the if not secure_rbac legacy stuff) in v2/policy and then didn\u0027t replicate it here.\n\n\u003e If we need to use our api_pol pattern I need to make other changes as well, but if you insist then I could refactor this to use our api_pol.\n\nWhat all is required? We have self.policy and image here, which is all we need to instantiate api_pol. What else do we need?","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"df93facdb0e46109e2f9831ddc71e97e3b7b5009","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            # NOTE(abhishekk): This means image is present in cache and before"},{"line_number":171,"context_line":"            # request is coming to API we are enforcing this check in the"},{"line_number":172,"context_line":"            # middleware"},{"line_number":173,"context_line":"            self._enforce(request, \u0027download_image\u0027, target\u003dimage_metadata)"},{"line_number":174,"context_line":"        except exception.Forbidden:"},{"line_number":175,"context_line":"            return None"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f46f5b47_d9a906c8","line":173,"in_reply_to":"cbf723f0_b61d9ef6","updated":"2021-08-16 19:29:19.000000000","message":"Ack, I have tried to keep other impact as minimal as possible.","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c42483ca834101bd57a46a5d4444d6bf23c2e574","unresolved":true,"context_lines":[{"line_number":156,"context_line":"        # NOTE(abhishekk): This means image is present in cache and before"},{"line_number":157,"context_line":"        # request is coming to API we are enforcing this check in the"},{"line_number":158,"context_line":"        # middleware"},{"line_number":159,"context_line":"        self._enforce(request, image)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        LOG.debug(\"Cache hit for image \u0027%s\u0027\", image_id)"},{"line_number":162,"context_line":"        image_iterator \u003d self.get_from_cache(image_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3905d021_67a3c253","line":159,"updated":"2021-08-16 21:33:46.000000000","message":"This could be a single line like we do it in other places, which would make this actually more DRY without self._enforce(). No need to change it, I\u0027m just saying.","commit_id":"004402ed675ec8003973522429b03a9d4e7da336"}],"glance/api/v2/policy.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"845ec73da70bd1cf0b61f071798e18085baf7946","unresolved":true,"context_lines":[{"line_number":157,"context_line":"            check_is_image_mutable(self._context, self._image)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def download_image(self):"},{"line_number":160,"context_line":"        self._enforce(\u0027download_image\u0027)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class MetadefAPIPolicy(APIPolicyBase):"}],"source_content_type":"text/x-python","patch_set":1,"id":"04d29973_de54a6ae","line":160,"updated":"2021-08-16 17:38:11.000000000","message":"Okay, we don\u0027t need the legacy check here because this isn\u0027t modifying the image.","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"66d325557ffbb55bbbb4876e89fa5f4dda73f6f7","unresolved":true,"context_lines":[{"line_number":157,"context_line":"            check_is_image_mutable(self._context, self._image)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def download_image(self):"},{"line_number":160,"context_line":"        self._enforce(\u0027download_image\u0027)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class MetadefAPIPolicy(APIPolicyBase):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5c97cc55_8762bebf","line":160,"in_reply_to":"04d29973_de54a6ae","updated":"2021-08-16 18:00:27.000000000","message":"right!","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"}],"glance/tests/functional/v2/test_images_api_policy.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"845ec73da70bd1cf0b61f071798e18085baf7946","unresolved":true,"context_lines":[{"line_number":328,"context_line":"        response \u003d self.api_get(path)"},{"line_number":329,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        # Now allow dowmload, but disallow get_image, just to prove that"},{"line_number":332,"context_line":"        # you do not need get_image in order to be granted download, and"},{"line_number":333,"context_line":"        # that we only use it for error code determination if"},{"line_number":334,"context_line":"        # permission is denied."}],"source_content_type":"text/x-python","patch_set":1,"id":"acab24f9_df6b143e","line":331,"range":{"start_line":331,"start_character":20,"end_line":331,"end_character":28},"updated":"2021-08-16 17:38:11.000000000","message":"download","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"66d325557ffbb55bbbb4876e89fa5f4dda73f6f7","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        response \u003d self.api_get(path)"},{"line_number":329,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        # Now allow dowmload, but disallow get_image, just to prove that"},{"line_number":332,"context_line":"        # you do not need get_image in order to be granted download, and"},{"line_number":333,"context_line":"        # that we only use it for error code determination if"},{"line_number":334,"context_line":"        # permission is denied."}],"source_content_type":"text/x-python","patch_set":1,"id":"75577bfa_7255dded","line":331,"range":{"start_line":331,"start_character":20,"end_line":331,"end_character":28},"in_reply_to":"acab24f9_df6b143e","updated":"2021-08-16 18:00:27.000000000","message":"Ack","commit_id":"35c60ba7a5d47692fa864090f0dde27b51af5e08"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"de007f38c731c71485308783ec84ac3f2966265d","unresolved":true,"context_lines":[{"line_number":340,"context_line":"        # see the image, we can download it"},{"line_number":341,"context_line":"        response \u003d self.api_get(path)"},{"line_number":342,"context_line":"        self.assertEqual(200, response.status_code)"},{"line_number":343,"context_line":"        self.assertEqual(\u0027IMAGEDATA\u0027, response.text)"}],"source_content_type":"text/x-python","patch_set":3,"id":"709a1157_f888de21","line":343,"updated":"2021-08-16 21:15:39.000000000","message":"Glad these are here so we can at least prove the decoupling happened.","commit_id":"004402ed675ec8003973522429b03a9d4e7da336"}],"glance/tests/unit/v2/test_image_data_resource.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1eedd8d337acfbe5b0be2e96b6eb691eeedeeb2f","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        self.policy \u003d policy"},{"line_number":109,"context_line":"        self.repo \u003d repo"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def get_repo(self, context, authorization_layer\u003dTrue):"},{"line_number":112,"context_line":"        return self.repo"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"767134f3_0ee2b745","line":111,"updated":"2021-08-18 16:20:09.000000000","message":"nit (something we can fix later, but I\u0027m curious): When I pulled this down locally and tested it, this is only for kwarg purposes.\n\nThe authorization_layer is going to be False when we remove the \"authorization bits\" out of the onion and keep it at the API layer, right?\n\nAny reason to not just have this default to False and future proof for that case? I ran it with authorization_layer\u003dFalse locally and things passed, so I imagine nothing in this test suite actually tests authorization logic anyway.","commit_id":"25333dc7dff2ba065a2114a6b42d2d51daeb5f34"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"b2004ebdfe9ce6073511340971ec712b3c71f22a","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        self.policy \u003d policy"},{"line_number":109,"context_line":"        self.repo \u003d repo"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def get_repo(self, context, authorization_layer\u003dTrue):"},{"line_number":112,"context_line":"        return self.repo"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"8bb3088b_2541558b","line":111,"in_reply_to":"618ed30a_6daf5781","updated":"2021-08-18 18:31:52.000000000","message":"Ack","commit_id":"25333dc7dff2ba065a2114a6b42d2d51daeb5f34"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f5ecc7db9e2c29f4178862e06dff2f325a082e0c","unresolved":true,"context_lines":[{"line_number":108,"context_line":"        self.policy \u003d policy"},{"line_number":109,"context_line":"        self.repo \u003d repo"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def get_repo(self, context, authorization_layer\u003dTrue):"},{"line_number":112,"context_line":"        return self.repo"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"618ed30a_6daf5781","line":111,"in_reply_to":"767134f3_0ee2b745","updated":"2021-08-18 16:52:53.000000000","message":"No reason, just kept it same as default to what we have in code.\nWill change it to False here if we need a respin","commit_id":"25333dc7dff2ba065a2114a6b42d2d51daeb5f34"}]}