)]}'
{"glance/api/v2/image_data.py":[{"author":{"_account_id":9096,"name":"Cindy Pallares","email":"u1@cindy.sh","username":"cpallares"},"change_message_id":"dd00eeb283ffc25cda73fad76b9f67dd0d44391c","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if image.status \u003d\u003d \u0027deactivated\u0027 or image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":199,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":200,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":201,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_4ab408d0","line":198,"updated":"2015-09-28 19:31:42.000000000","message":"Should we perhaps check for \u0027saving\u0027 since attempting to download an image that\u0027s saving could cause a race condition?","commit_id":"9de35bf71383489305141e71267835ade3b7b230"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"e1e1081fb423536cb29446b044106cf247c991dd","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if image.status \u003d\u003d \u0027deactivated\u0027 or image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":199,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":200,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":201,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_b9c3adec","line":198,"in_reply_to":"9a1a9d01_0a7a8b77","updated":"2015-09-29 08:16:29.000000000","message":"About \u0027deactivated\u0027 status... Does an admin have the ability to download it in this case? I think he should be able... \n\nBut in common, I agree with Sabari, this check may be just \"if image.status !\u003d \u0027active\u0027\" or \"if image.status !\u003d \u0027active\u0027 or (image.status \u003d\u003d \u0027deactivated\u0027 and req.contex.is_admin\u0027\"","commit_id":"9de35bf71383489305141e71267835ade3b7b230"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"f407db22670c081765b9432fc046ed22c115a9b1","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if image.status \u003d\u003d \u0027deactivated\u0027 or image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":199,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":200,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":201,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_0a7a8b77","line":198,"in_reply_to":"9a1a9d01_27e0eaf9","updated":"2015-09-28 21:14:33.000000000","message":"During, pending_deletion state, the image could be scrubbed off and may result in interrupted download anyway :).\n\nI wonder if it\u0027s useful to allow download in such cases. But if this is the expectation from wide range of users, then we may break compatibility. Honestly, I will leave it for others to comment here. AFAIK, the deployments I work on, do not carry this expectation.","commit_id":"9de35bf71383489305141e71267835ade3b7b230"},{"author":{"_account_id":7575,"name":"Sabari","email":"smurugesan@vmware.com","username":"sabari"},"change_message_id":"7eef38506858c68d4e177170187b27ee65524f57","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if image.status \u003d\u003d \u0027deactivated\u0027 or image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":199,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":200,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":201,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_bbb5f431","line":198,"in_reply_to":"9a1a9d01_4ab408d0","updated":"2015-09-28 20:29:37.000000000","message":"IMO, any state other than \u0027active\u0027 shouldn\u0027t allow download.","commit_id":"9de35bf71383489305141e71267835ade3b7b230"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a85e9c7e8064d59233abd841f385364e16dc80cc","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if image.status \u003d\u003d \u0027deactivated\u0027 or image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":199,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":200,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":201,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_bfd70d4a","line":198,"in_reply_to":"9a1a9d01_9c76d7a2","updated":"2015-09-29 08:54:17.000000000","message":"++ agree with Mike and Kairat","commit_id":"9de35bf71383489305141e71267835ade3b7b230"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"2b468d998b27a84a65e28e433447069fe69eaa70","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if image.status \u003d\u003d \u0027deactivated\u0027 or image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":199,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":200,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":201,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_9c76d7a2","line":198,"in_reply_to":"9a1a9d01_b9c3adec","updated":"2015-09-29 08:23:09.000000000","message":"+1 for Mike. I have seen the bug recently where we allowed to download an image in de-activated state. It looks reasonable.","commit_id":"9de35bf71383489305141e71267835ade3b7b230"},{"author":{"_account_id":9096,"name":"Cindy Pallares","email":"u1@cindy.sh","username":"cpallares"},"change_message_id":"8b156543c207144620fdbccd55b4619b471e2664","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        image_repo \u003d self.gateway.get_repo(req.context)"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if image.status \u003d\u003d \u0027deactivated\u0027 or image.status \u003d\u003d \u0027queued\u0027:"},{"line_number":199,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":200,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":201,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_27e0eaf9","line":198,"in_reply_to":"9a1a9d01_bbb5f431","updated":"2015-09-28 20:56:55.000000000","message":"I was thinking that could be the way, but some people depend on being able to download during the pending_delete status, since there is no current way to cancel a pending deletion.","commit_id":"9de35bf71383489305141e71267835ade3b7b230"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6cba69d9b79b902f9c89fe556d50e37f6e298229","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if (image.status !\u003d \u0027active\u0027 or"},{"line_number":199,"context_line":"                    (image.status \u003d\u003d \u0027deactivated\u0027 and req.contex.is_admin)):"},{"line_number":200,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":201,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":202,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_663e602b","line":199,"updated":"2015-10-01 13:46:57.000000000","message":"I agree with the discussion on the previous patch.  The only legitimate download status for non-admin is \u0027active\u0027.  The pending_delete status should be regarded as \u0027deleted\u0027 for all the reasons Sabari mentions.  Mike is correct that an admin should be able to download a deactivated image (the use-case for deactivation is that an admin wants to temporarily \"suspend\" the image while it\u0027s under investigation, presumably part of the investigation would require access to the image to examine it).\n\nThat being said, I think the condition is incorrect here.  If the image.status is \u0027deactivated\u0027, the first disjunct will be true and the second part won\u0027t be checked, so this won\u0027t allow admin download.  Further, I think the second disjunct is incorrect anyway: if the image is deactivated and the context is admin, it will evaluate to true and we\u0027ll raise the exception, which is *not* what we want here.  Maybe something like:\n\nif ((image.status \u003d\u003d \u0027deactivated\u0027 and not req.context.is_admin)\n    or image.status !\u003d \u0027active\u0027):\n\nAlso, notice that you\u0027ve got req.contex in the code, not req.context.","commit_id":"78f797e015d6decabacd1bc91ba9933764c061c0"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"ffd407ad9b5662a8e56ddcf5dc147747200c7b44","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if (image.status !\u003d \u0027active\u0027 or"},{"line_number":199,"context_line":"                    (image.status \u003d\u003d \u0027deactivated\u0027 and req.contex.is_admin)):"},{"line_number":200,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":201,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":202,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_40c77f64","line":199,"updated":"2015-10-01 14:26:42.000000000","message":"and not req.contex.is_admin","commit_id":"78f797e015d6decabacd1bc91ba9933764c061c0"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6cba69d9b79b902f9c89fe556d50e37f6e298229","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if (image.status !\u003d \u0027active\u0027 or"},{"line_number":199,"context_line":"                    (image.status \u003d\u003d \u0027deactivated\u0027 and req.contex.is_admin)):"},{"line_number":200,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":201,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":202,"context_line":"                raise exception.Forbidden(message\u003dmsg)"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_6653c06d","line":200,"updated":"2015-10-01 13:46:57.000000000","message":"I think you need to change the first part of the message a bit now that statuses other than \u0027deactivated\u0027 will show up.  Maybe:\n\u0027The requested image is in status %s. \u0027","commit_id":"78f797e015d6decabacd1bc91ba9933764c061c0"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"1b5808975ccee2f5cff7ba9c36661387ea811d97","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":198,"context_line":"            if (image.status !\u003d \u0027active\u0027 or (image.status \u003d\u003d \u0027deactivated\u0027"},{"line_number":199,"context_line":"                                             and not req.contex.is_admin)):"},{"line_number":200,"context_line":"                msg \u003d _(\u0027The requested image has been %s. \u0027"},{"line_number":201,"context_line":"                        \u0027Image data download is forbidden.\u0027) % image.status"},{"line_number":202,"context_line":"                raise exception.Forbidden(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a1a9d01_0a6387dd","line":199,"updated":"2015-10-05 07:19:05.000000000","message":"req.contex \u003e\u003e\u003e req.context as pointed out by Brian in previous PS. see line #195","commit_id":"6ae49db7ebbb77ec3fd9b244368f892f7db38879"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"e777c1ec4c82af667df8fdcd7db640742c190d7a","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                    pass"},{"line_number":118,"context_line":"                raise webob.exc.HTTPUnauthorized(explanation\u003dmsg,"},{"line_number":119,"context_line":"                                                 request\u003dreq,"},{"line_number":120,"context_line":"                                                 content_type\u003d\u0027text/plain\u0027)"},{"line_number":121,"context_line":"        except ValueError as e:"},{"line_number":122,"context_line":"            LOG.debug(\"Cannot save data for image %(id)s: %(e)s\","},{"line_number":123,"context_line":"                      {\u0027id\u0027: image_id,"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a2fa921_0143bb6d","side":"PARENT","line":120,"updated":"2015-10-12 15:18:26.000000000","message":"Hey... It was my security fix. You shouldn\u0027t remove it :)","commit_id":"15900039043d49ce4299f13d0ba88623ba594991"}],"glance/tests/functional/test_cache_middleware.py":[{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"e777c1ec4c82af667df8fdcd7db640742c190d7a","unresolved":false,"context_lines":[{"line_number":386,"context_line":"        response, content \u003d http.request(path, \u0027GET\u0027)"},{"line_number":387,"context_line":"        self.assertEqual(403, response.status)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        # Download the image with v2."},{"line_number":390,"context_line":"        path \u003d \"http://%s:%d/v2/images/%s/file\" % (\"127.0.0.1\", self.api_port,"},{"line_number":391,"context_line":"                                                   image_id)"},{"line_number":392,"context_line":"        http \u003d httplib2.Http()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a2fa921_e1ac7f37","line":389,"updated":"2015-10-12 15:18:26.000000000","message":"It\u0027s really strange behavior - when you can download image with v2, but can\u0027t do it with v1. It seems you do something wrong here.","commit_id":"1716050867c509fde14664211be8a12852e60c05"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"ae98f920c2bca35c005f0a97559050e25048cfbb","unresolved":false,"context_lines":[{"line_number":386,"context_line":"        response, content \u003d http.request(path, \u0027GET\u0027)"},{"line_number":387,"context_line":"        self.assertEqual(403, response.status)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        # Download the image with v2."},{"line_number":390,"context_line":"        path \u003d \"http://%s:%d/v2/images/%s/file\" % (\"127.0.0.1\", self.api_port,"},{"line_number":391,"context_line":"                                                   image_id)"},{"line_number":392,"context_line":"        http \u003d httplib2.Http()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba8a016a_8f823fc7","line":389,"in_reply_to":"7a2fa921_245b6530","updated":"2015-11-23 14:41:05.000000000","message":"Actually not ... it was deactivated there before this change and the v1 was still forbidden. This change indeed caused inconsistency between v1 and v2.","commit_id":"1716050867c509fde14664211be8a12852e60c05"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"05cf92bd664a6fb681bf40b66f83c502cf28fbeb","unresolved":false,"context_lines":[{"line_number":386,"context_line":"        response, content \u003d http.request(path, \u0027GET\u0027)"},{"line_number":387,"context_line":"        self.assertEqual(403, response.status)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        # Download the image with v2."},{"line_number":390,"context_line":"        path \u003d \"http://%s:%d/v2/images/%s/file\" % (\"127.0.0.1\", self.api_port,"},{"line_number":391,"context_line":"                                                   image_id)"},{"line_number":392,"context_line":"        http \u003d httplib2.Http()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a2fa921_245b6530","line":389,"in_reply_to":"7a2fa921_e1ac7f37","updated":"2015-10-12 15:23:39.000000000","message":"Oops, I wasn\u0027t right here. I missed that the image was deactivated with v2 on line 375. So it\u0027s okay.","commit_id":"1716050867c509fde14664211be8a12852e60c05"}],"glance/tests/unit/v2/test_image_data_resource.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6cba69d9b79b902f9c89fe556d50e37f6e298229","unresolved":false,"context_lines":[{"line_number":120,"context_line":"                                      \u0027metadata\u0027: {}, \u0027status\u0027: \u0027active\u0027}])"},{"line_number":121,"context_line":"        self.image_repo.result \u003d image"},{"line_number":122,"context_line":"        self.assertRaises(webob.exc.HTTPForbidden, self.controller.download,"},{"line_number":123,"context_line":"                          request, str(uuid.uuid4()))"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def test_download_no_location(self):"},{"line_number":126,"context_line":"        request \u003d unit_test_utils.get_fake_request()"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a1a9d01_314ae056","line":123,"updated":"2015-10-01 13:46:57.000000000","message":"Should probably add a new  \u0027test_download_deactivated_as_admin\u0027 here.","commit_id":"78f797e015d6decabacd1bc91ba9933764c061c0"}]}