)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"634b280d0e4a1e21c4f022f01b192b727a5cb9c6","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Abhishek Kekane \u003cakekane@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-08-15 12:27:05 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Check policies for delete image fro store in API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This has a change in default RBAC policy for delete_image_location from"},{"line_number":10,"context_line":"\u0027admin only\u0027 to \u0027admin or project member\u0027. This change enforces policy"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7cda1d18_f0ffe191","line":7,"range":{"start_line":7,"start_character":32,"end_line":7,"end_character":35},"updated":"2021-08-16 18:24:17.000000000","message":"from","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f480218005347a526d149fb0af435414a23da0d9","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Abhishek Kekane \u003cakekane@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-08-15 12:27:05 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Check policies for delete image fro store in API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This has a change in default RBAC policy for delete_image_location from"},{"line_number":10,"context_line":"\u0027admin only\u0027 to \u0027admin or project member\u0027. This change enforces policy"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a500deb_1dee45ca","line":7,"range":{"start_line":7,"start_character":32,"end_line":7,"end_character":35},"in_reply_to":"7cda1d18_f0ffe191","updated":"2021-08-18 11:21:12.000000000","message":"Ack","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"634b280d0e4a1e21c4f022f01b192b727a5cb9c6","unresolved":true,"context_lines":[{"line_number":10,"context_line":"\u0027admin only\u0027 to \u0027admin or project member\u0027. This change enforces policy"},{"line_number":11,"context_line":"checks required for deleting image from specific store in API layer. Image"},{"line_number":12,"context_line":"mutability check is maintained if secure RBAC is not enabled in deployments."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Partially-Implements: blueprint policy-refactor"},{"line_number":15,"context_line":"Closes-Bug: #1939977"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7abe4ab4_c22e94dd","line":13,"updated":"2021-08-16 18:24:17.000000000","message":"Fix wrapping?","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"f480218005347a526d149fb0af435414a23da0d9","unresolved":false,"context_lines":[{"line_number":10,"context_line":"\u0027admin only\u0027 to \u0027admin or project member\u0027. This change enforces policy"},{"line_number":11,"context_line":"checks required for deleting image from specific store in API layer. Image"},{"line_number":12,"context_line":"mutability check is maintained if secure RBAC is not enabled in deployments."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Partially-Implements: blueprint policy-refactor"},{"line_number":15,"context_line":"Closes-Bug: #1939977"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"654b1fab_cb5c09d3","line":13,"in_reply_to":"7abe4ab4_c22e94dd","updated":"2021-08-18 11:21:12.000000000","message":"Ack","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"}],"glance/api/v2/images.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"898c3efcbbb58f775f3ba43d9f901577f6dabb78","unresolved":true,"context_lines":[{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"},{"line_number":735,"context_line":"        try:"},{"line_number":736,"context_line":"            api_pol.delete_locations()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3daf379a_2d00ff07","line":733,"updated":"2021-08-16 21:33:15.000000000","message":"Will this policy check eventually evolve to something like?\n\n  enforcer.enforce(\u0027get_image_location\u0027, image, context)","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3d73d7866fb311994965ffdb84703596898a5a69","unresolved":true,"context_lines":[{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"},{"line_number":735,"context_line":"        try:"},{"line_number":736,"context_line":"            api_pol.delete_locations()"}],"source_content_type":"text/x-python","patch_set":1,"id":"97352459_df917d01","line":733,"in_reply_to":"3daf379a_2d00ff07","updated":"2021-08-16 22:24:00.000000000","message":"internally it will be converted to dict using (policy.ImageTarget) but yes it will be like;\n\nenforcer.enforce(context, \u0027get_image_location\u0027, target)","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"70bd29eaa40432f1b9e1a58dc4c8781443aeed45","unresolved":false,"context_lines":[{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"},{"line_number":735,"context_line":"        try:"},{"line_number":736,"context_line":"            api_pol.delete_locations()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ce53a4a_e2586ca1","line":733,"in_reply_to":"97352459_df917d01","updated":"2021-08-17 22:01:47.000000000","message":"Ack","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"a53be40dd096874ca465156fb5ef09997238c65a","unresolved":true,"context_lines":[{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"},{"line_number":735,"context_line":"        try:"},{"line_number":736,"context_line":"            api_pol.delete_locations()"}],"source_content_type":"text/x-python","patch_set":4,"id":"a5272361_ff338235","line":733,"range":{"start_line":733,"start_character":0,"end_line":733,"end_character":36},"updated":"2021-08-19 16:16:15.000000000","message":"Don\u0027t we want to catch exception.Forbidden for this check?","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"f9bba38c6f50b406fb35e81a89cfaca1d792a3d4","unresolved":true,"context_lines":[{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"},{"line_number":735,"context_line":"        try:"},{"line_number":736,"context_line":"            api_pol.delete_locations()"}],"source_content_type":"text/x-python","patch_set":4,"id":"ed09bb70_70ac6718","line":733,"range":{"start_line":733,"start_character":0,"end_line":733,"end_character":36},"in_reply_to":"a5272361_ff338235","updated":"2021-08-19 16:23:17.000000000","message":"I think that\u0027s happening here https://review.opendev.org/c/openstack/glance/+/804585/4/glance/api/v2/policy.py#96","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"be47f1e5c9200b18eeac1b1e96586fee64b8e0e0","unresolved":true,"context_lines":[{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"},{"line_number":735,"context_line":"        try:"},{"line_number":736,"context_line":"            api_pol.delete_locations()"}],"source_content_type":"text/x-python","patch_set":4,"id":"d5b97548_2ef284c9","line":733,"range":{"start_line":733,"start_character":0,"end_line":733,"end_character":36},"in_reply_to":"a5272361_ff338235","updated":"2021-08-19 16:23:51.000000000","message":"No, this is a get call which is already raising webob.exc.HTTPForbidden, but in below case after actual policy enforcement we are checking legacy behavior which will raise exception.Forbidden and need to be caught.","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"aa2ad8c8c1f5a0a0712032dd54c3c357c7b5bcf6","unresolved":false,"context_lines":[{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"},{"line_number":735,"context_line":"        try:"},{"line_number":736,"context_line":"            api_pol.delete_locations()"}],"source_content_type":"text/x-python","patch_set":4,"id":"844136f6_d751d968","line":733,"range":{"start_line":733,"start_character":0,"end_line":733,"end_character":36},"in_reply_to":"d5b97548_2ef284c9","updated":"2021-08-19 19:43:02.000000000","message":"Ack","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7a923d453b9ce7469738a8c032e0a0ee19a86a1d","unresolved":true,"context_lines":[{"line_number":720,"context_line":"        try:"},{"line_number":721,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":722,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":723,"context_line":"            raise webob.exc.HTTPUnauthorized(explanation\u003de.msg)"},{"line_number":724,"context_line":"        except exception.NotFound:"},{"line_number":725,"context_line":"            msg \u003d (_(\"Failed to find image %(image_id)s\") %"},{"line_number":726,"context_line":"                   {\u0027image_id\u0027: image_id})"}],"source_content_type":"text/x-python","patch_set":7,"id":"05c094c4_41abce83","line":723,"updated":"2021-08-23 14:06:21.000000000","message":"Hmm, presumably this is something related to the glance store backend raising an auth problem? I\u0027m guessing that would only happen if the image exists, right? It\u0027s a slim window, but could expose the existence of images while some backend is down or something. Probably not for this patch, but maybe something to remember.","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"23859969c149689295cee4e3c728bfc6176be5b0","unresolved":false,"context_lines":[{"line_number":720,"context_line":"        try:"},{"line_number":721,"context_line":"            image \u003d image_repo.get(image_id)"},{"line_number":722,"context_line":"        except exception.NotAuthenticated as e:"},{"line_number":723,"context_line":"            raise webob.exc.HTTPUnauthorized(explanation\u003de.msg)"},{"line_number":724,"context_line":"        except exception.NotFound:"},{"line_number":725,"context_line":"            msg \u003d (_(\"Failed to find image %(image_id)s\") %"},{"line_number":726,"context_line":"                   {\u0027image_id\u0027: image_id})"}],"source_content_type":"text/x-python","patch_set":7,"id":"3d4bcc41_7cbce4af","line":723,"in_reply_to":"05c094c4_41abce83","updated":"2021-08-23 14:30:53.000000000","message":"Ack","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7a923d453b9ce7469738a8c032e0a0ee19a86a1d","unresolved":true,"context_lines":[{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # NOTE(abhishekk): Delete from store internally checks for"},{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"}],"source_content_type":"text/x-python","patch_set":7,"id":"8ac8c5b0_65fd27b7","line":731,"updated":"2021-08-23 14:06:21.000000000","message":"Internally where? We\u0027ve been not requiring get_image to do things like delete the image, so I\u0027m not sure why this needs to be any different. Internally we need to get and iterate that list, but we delete by id, so they have to (a) have delete permission and (b) have gotten the store_id from something.\n\nAnyway, this is a pretty deep operation, so if there\u0027s something important about it being different, that\u0027s cool, but I\u0027m just curious why.","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"121f3ba04ee3dca1c4e78b09f74227b748b875fa","unresolved":true,"context_lines":[{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # NOTE(abhishekk): Delete from store internally checks for"},{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"}],"source_content_type":"text/x-python","patch_set":7,"id":"cee93faf_33be8702","line":731,"in_reply_to":"6a72047e_5b4edfea","updated":"2021-08-23 14:45:48.000000000","message":"Ugh, okay, makes sense then. Might be good to update the comment above with just a little of that. Maybe just mention ImageLocationsProxy at least.","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"23859969c149689295cee4e3c728bfc6176be5b0","unresolved":true,"context_lines":[{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # NOTE(abhishekk): Delete from store internally checks for"},{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"}],"source_content_type":"text/x-python","patch_set":7,"id":"6a72047e_5b4edfea","line":731,"in_reply_to":"8ac8c5b0_65fd27b7","updated":"2021-08-23 14:30:53.000000000","message":"at line #759 we are calling image.locations.pop,\nimage.locations will return [1]  ImageLocationsProxy object which will call delete_image_location on pop and remove call [2] and get_image_location is called while retrieving particular item form location list. \n\n[1] https://github.com/openstack/glance/blob/master/glance/api/policy.py#L238\n[2] https://github.com/openstack/glance/blob/master/glance/api/policy.py#L420","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"571de9e2d119430e7b1463fea01c7d4fa6f1af53","unresolved":false,"context_lines":[{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # NOTE(abhishekk): Delete from store internally checks for"},{"line_number":730,"context_line":"        # get_image_location and delete_image_location policies, so this is"},{"line_number":731,"context_line":"        # the right place to check those policies"},{"line_number":732,"context_line":"        api_pol \u003d api_policy.ImageAPIPolicy(req.context, image, self.policy)"},{"line_number":733,"context_line":"        api_pol.get_image_location()"},{"line_number":734,"context_line":"        # This policy will check for legacy image ownership as well"}],"source_content_type":"text/x-python","patch_set":7,"id":"95b5dd78_f3046c50","line":731,"in_reply_to":"cee93faf_33be8702","updated":"2021-08-23 15:10:11.000000000","message":"Done","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"}],"glance/policies/image.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"898c3efcbbb58f775f3ba43d9f901577f6dabb78","unresolved":true,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003dbase.ADMIN_OR_PROJECT_MEMBER,"},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"01abe834_dea7dccc","line":150,"range":{"start_line":150,"start_character":32,"end_line":150,"end_character":46},"updated":"2021-08-16 21:33:15.000000000","message":"Ok - so this is going to open the ability to delete image location attributes up to regular end users, right?\n\nIs that kind of information sensitive at all? Or is it not really something we care to much about if the project owns the image anyway?","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3d73d7866fb311994965ffdb84703596898a5a69","unresolved":true,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003dbase.ADMIN_OR_PROJECT_MEMBER,"},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"224e9224_bf65126c","line":150,"range":{"start_line":150,"start_character":32,"end_line":150,"end_character":46},"in_reply_to":"01abe834_dea7dccc","updated":"2021-08-16 22:24:00.000000000","message":"hmm, I think the later part has considered when we decided to use this way to delete image from particular store.","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"70bd29eaa40432f1b9e1a58dc4c8781443aeed45","unresolved":true,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003dbase.ADMIN_OR_PROJECT_MEMBER,"},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"a2e482ed_73af8738","line":150,"range":{"start_line":150,"start_character":32,"end_line":150,"end_character":46},"in_reply_to":"224e9224_bf65126c","updated":"2021-08-17 22:01:47.000000000","message":"Ok - so we\u0027re fine with opening this up?","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"c65725138e801e2ab7cf058553578bf635248bd0","unresolved":true,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003dbase.ADMIN_OR_PROJECT_MEMBER,"},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":1,"id":"61dd90f6_c7a84966","line":150,"range":{"start_line":150,"start_character":32,"end_line":150,"end_character":46},"in_reply_to":"a2e482ed_73af8738","updated":"2021-08-17 22:10:18.000000000","message":"I think I will revert it to admin only, just wondering it was open before RBAC and we didn\u0027t had any check to restrict it for admin only in auth layer or other layers.","commit_id":"f9ce1ac0dc753a0ffcbfdd66de1ada1312a3ac4e"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"a53be40dd096874ca465156fb5ef09997238c65a","unresolved":true,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003d\"role:admin\","},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":4,"id":"7de548bd_70ed8d0f","line":150,"range":{"start_line":150,"start_character":0,"end_line":150,"end_character":31},"updated":"2021-08-19 16:16:15.000000000","message":"This seems unrelated to this patch.","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"be47f1e5c9200b18eeac1b1e96586fee64b8e0e0","unresolved":false,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003d\"role:admin\","},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":4,"id":"8710edec_ee2d8e8f","line":150,"range":{"start_line":150,"start_character":0,"end_line":150,"end_character":31},"in_reply_to":"7de548bd_70ed8d0f","updated":"2021-08-19 16:23:51.000000000","message":"Ack","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"f9bba38c6f50b406fb35e81a89cfaca1d792a3d4","unresolved":true,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003d\"role:admin\","},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":4,"id":"53f62dfc_da87e08e","line":150,"range":{"start_line":150,"start_character":0,"end_line":150,"end_character":31},"in_reply_to":"7de548bd_70ed8d0f","updated":"2021-08-19 16:23:17.000000000","message":"Yeah - I thought so, too. But I noticed that it does make the check_str definitions consistently double-quoted in this file.","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"6157f319b930e5131f3335cc640c94adab2702d0","unresolved":false,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":149,"context_line":"        name\u003d\"delete_image_location\","},{"line_number":150,"context_line":"        check_str\u003d\"role:admin\","},{"line_number":151,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027],"},{"line_number":152,"context_line":"        description\u003d\u0027Deletes the location of given image\u0027,"},{"line_number":153,"context_line":"        operations\u003d["}],"source_content_type":"text/x-python","patch_set":4,"id":"c7f70458_2917f7e8","line":150,"range":{"start_line":150,"start_character":0,"end_line":150,"end_character":31},"in_reply_to":"8710edec_ee2d8e8f","updated":"2021-08-19 17:19:10.000000000","message":"https://review.opendev.org/c/openstack/glance/+/805256","commit_id":"9fc8b4491fd485fb5e06c401e0f9b6f3ecee0984"}],"glance/tests/functional/v2/test_images_api_policy.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"846ba9e731ce4a3a81321fdaff18061cbdc90a06","unresolved":true,"context_lines":[{"line_number":294,"context_line":"        response \u003d self.api_delete(path)"},{"line_number":295,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        # Disabling get_image only, you will get 403"},{"line_number":298,"context_line":"        self.set_policy_rules({"},{"line_number":299,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":300,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"16e70812_62be2edf","line":297,"updated":"2021-08-18 16:26:55.000000000","message":"nit: Because the internals for delete_image_location relies on fetching the image, right?","commit_id":"5c409576c45d22f2eee4970f8ff93620164cd416"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1546f1e1c92677e0d335b45c4e2270b59b05d306","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        response \u003d self.api_delete(path)"},{"line_number":295,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        # Disabling get_image only, you will get 403"},{"line_number":298,"context_line":"        self.set_policy_rules({"},{"line_number":299,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":300,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"175cdca9_dfbc814e","line":297,"in_reply_to":"0044e750_b4c3aa22","updated":"2021-08-18 19:09:09.000000000","message":"Ack - cool thanks. That might be useful to include into the comment since it\u0027s not apparent from the assertions (it explains the 403). But, I\u0027m being nit picky and that\u0027s something we can do in future patches.","commit_id":"5c409576c45d22f2eee4970f8ff93620164cd416"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"29c4cfbd307f248ac9b5b97f8cc8596a67efbdfc","unresolved":true,"context_lines":[{"line_number":294,"context_line":"        response \u003d self.api_delete(path)"},{"line_number":295,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        # Disabling get_image only, you will get 403"},{"line_number":298,"context_line":"        self.set_policy_rules({"},{"line_number":299,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":300,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"0044e750_b4c3aa22","line":297,"in_reply_to":"16e70812_62be2edf","updated":"2021-08-18 16:51:15.000000000","message":"Right!","commit_id":"5c409576c45d22f2eee4970f8ff93620164cd416"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"bb4208ca455e5642f81bf66f105773c39254b32e","unresolved":false,"context_lines":[{"line_number":294,"context_line":"        response \u003d self.api_delete(path)"},{"line_number":295,"context_line":"        self.assertEqual(404, response.status_code)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        # Disabling get_image only, you will get 403"},{"line_number":298,"context_line":"        self.set_policy_rules({"},{"line_number":299,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":300,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"84d621be_eecf87f9","line":297,"in_reply_to":"175cdca9_dfbc814e","updated":"2021-08-19 16:05:08.000000000","message":"Done","commit_id":"5c409576c45d22f2eee4970f8ff93620164cd416"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7a923d453b9ce7469738a8c032e0a0ee19a86a1d","unresolved":true,"context_lines":[{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        # Disabling get_image only, you will get 403 because"},{"line_number":670,"context_line":"        # for deleting store location you need to get image"},{"line_number":671,"context_line":"        # details."},{"line_number":672,"context_line":"        self.set_policy_rules({"},{"line_number":673,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":674,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"e1c044ce_b068d449","line":671,"updated":"2021-08-23 14:06:21.000000000","message":"If you don\u0027t have get_image you should only notice a difference in how a 403 or 404 is returned to you, right? Why is get_image affecting us if we are allowed to do the delete-from-store? Re: L246 in this file.","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"571de9e2d119430e7b1463fea01c7d4fa6f1af53","unresolved":true,"context_lines":[{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        # Disabling get_image only, you will get 403 because"},{"line_number":670,"context_line":"        # for deleting store location you need to get image"},{"line_number":671,"context_line":"        # details."},{"line_number":672,"context_line":"        self.set_policy_rules({"},{"line_number":673,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":674,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"d391b604_435b0a14","line":671,"in_reply_to":"3e14cd6a_10815641","updated":"2021-08-23 15:10:11.000000000","message":"Nice catch, here I am getting 403 not because get image is restricted but because I am trying to delete image from only location it is available.","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37b69dc58b2ff341769a4720280cd89862d8bf4d","unresolved":true,"context_lines":[{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        # Disabling get_image only, you will get 403 because"},{"line_number":670,"context_line":"        # for deleting store location you need to get image"},{"line_number":671,"context_line":"        # details."},{"line_number":672,"context_line":"        self.set_policy_rules({"},{"line_number":673,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":674,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dc9ddee4_cd7bf02e","line":671,"in_reply_to":"d391b604_435b0a14","updated":"2021-08-23 15:40:48.000000000","message":"Aha, nice :)","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"23859969c149689295cee4e3c728bfc6176be5b0","unresolved":true,"context_lines":[{"line_number":668,"context_line":""},{"line_number":669,"context_line":"        # Disabling get_image only, you will get 403 because"},{"line_number":670,"context_line":"        # for deleting store location you need to get image"},{"line_number":671,"context_line":"        # details."},{"line_number":672,"context_line":"        self.set_policy_rules({"},{"line_number":673,"context_line":"            \u0027get_image\u0027: \u0027!\u0027,"},{"line_number":674,"context_line":"            \u0027delete_image_location\u0027: \u0027\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"3e14cd6a_10815641","line":671,"in_reply_to":"e1c044ce_b068d449","updated":"2021-08-23 14:30:53.000000000","message":"Ack, will check it manually again.","commit_id":"364871333cd8b37eca42cf96041ab6cac1886ef5"}]}