)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"77a54e6d57c2bb7e81f094ba34491e4901ad6867","unresolved":false,"context_lines":[{"line_number":21,"context_line":"the instance owner project_id is granted sharing access to the image."},{"line_number":22,"context_line":"By default, this means the instance owner (tenant user) can get the"},{"line_number":23,"context_line":"image directly via the image ID if they know it, but otherwise the image"},{"line_number":24,"context_line":"is not listed for the user to avoid spamming their image listing. In the"},{"line_number":25,"context_line":"case of unshelve, the end user does not need to know the image ID since"},{"line_number":26,"context_line":"it is stored in the instance system_metadata. Regardless, the user could"},{"line_number":27,"context_line":"accept the pending image membership if they want to see the snapshot"},{"line_number":28,"context_line":"show up when listing available images."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Due to some very old legacy virt driver code which started in the"},{"line_number":31,"context_line":"libvirt driver and was copied to several other drivers, several virt"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fdfeff1_87c71e13","line":28,"range":{"start_line":24,"start_character":66,"end_line":28,"end_character":38},"updated":"2019-01-23 07:45:20.000000000","message":"I also found the instance user still can\u0027t remove the shelved image, since he is not own the image. I didn\u0027t see you talk about this, so it still sounds like a flaw for the shelve case?","commit_id":"1677b1c16de51c219b72a4553777cacfbb3974e1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5c3664b6fcf52974b128f066f48cd13b2c63e4a4","unresolved":false,"context_lines":[{"line_number":21,"context_line":"the instance owner project_id is granted sharing access to the image."},{"line_number":22,"context_line":"By default, this means the instance owner (tenant user) can get the"},{"line_number":23,"context_line":"image directly via the image ID if they know it, but otherwise the image"},{"line_number":24,"context_line":"is not listed for the user to avoid spamming their image listing. In the"},{"line_number":25,"context_line":"case of unshelve, the end user does not need to know the image ID since"},{"line_number":26,"context_line":"it is stored in the instance system_metadata. Regardless, the user could"},{"line_number":27,"context_line":"accept the pending image membership if they want to see the snapshot"},{"line_number":28,"context_line":"show up when listing available images."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Due to some very old legacy virt driver code which started in the"},{"line_number":31,"context_line":"libvirt driver and was copied to several other drivers, several virt"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fdfeff1_8c987ac0","line":28,"range":{"start_line":24,"start_character":66,"end_line":28,"end_character":38},"in_reply_to":"9fdfeff1_5f2b29ea","updated":"2019-01-24 23:24:21.000000000","message":"\u003e but with shared image, the image won\u0027t be deleted, since the user isn\u0027t own the image.\n\nI don\u0027t think that\u0027s true for unshelve. I don\u0027t think we use the request context (non-admin sharing user) to delete the image, we use the nova service user / admin credentials to delete the image:\n\nhttps://github.com/openstack/nova/blob/9419c3e05499e55beda93d664197a7b0f0011ff7/nova/compute/manager.py#L2757\n\nBut I\u0027ll need to test that out again in devstack since I don\u0027t have that recreate environment anymore.\n\nIt could be that I only tested that unshelve as the tenant user from the admin-owned snapshot image works but didn\u0027t notice that the image isn\u0027t deleted after unshelve because we don\u0027t fail unshelve in that case, we just log it.","commit_id":"1677b1c16de51c219b72a4553777cacfbb3974e1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c612d0f2fd8442f68d5c36fa9a49710848aff394","unresolved":false,"context_lines":[{"line_number":21,"context_line":"the instance owner project_id is granted sharing access to the image."},{"line_number":22,"context_line":"By default, this means the instance owner (tenant user) can get the"},{"line_number":23,"context_line":"image directly via the image ID if they know it, but otherwise the image"},{"line_number":24,"context_line":"is not listed for the user to avoid spamming their image listing. In the"},{"line_number":25,"context_line":"case of unshelve, the end user does not need to know the image ID since"},{"line_number":26,"context_line":"it is stored in the instance system_metadata. Regardless, the user could"},{"line_number":27,"context_line":"accept the pending image membership if they want to see the snapshot"},{"line_number":28,"context_line":"show up when listing available images."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Due to some very old legacy virt driver code which started in the"},{"line_number":31,"context_line":"libvirt driver and was copied to several other drivers, several virt"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fdfeff1_9c3769b1","line":28,"range":{"start_line":24,"start_character":66,"end_line":28,"end_character":38},"in_reply_to":"9fdfeff1_87c71e13","updated":"2019-01-23 14:10:13.000000000","message":"When the user unshelves the image is deleted. Otherwise why does the user need to care about the image really? If they want it gone, they could delete their server which will also delete the snapshot image.","commit_id":"1677b1c16de51c219b72a4553777cacfbb3974e1"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"308104e105ee3a1b7e56c2ee869c87e60a7c1e4d","unresolved":false,"context_lines":[{"line_number":21,"context_line":"the instance owner project_id is granted sharing access to the image."},{"line_number":22,"context_line":"By default, this means the instance owner (tenant user) can get the"},{"line_number":23,"context_line":"image directly via the image ID if they know it, but otherwise the image"},{"line_number":24,"context_line":"is not listed for the user to avoid spamming their image listing. In the"},{"line_number":25,"context_line":"case of unshelve, the end user does not need to know the image ID since"},{"line_number":26,"context_line":"it is stored in the instance system_metadata. Regardless, the user could"},{"line_number":27,"context_line":"accept the pending image membership if they want to see the snapshot"},{"line_number":28,"context_line":"show up when listing available images."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Due to some very old legacy virt driver code which started in the"},{"line_number":31,"context_line":"libvirt driver and was copied to several other drivers, several virt"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"9fdfeff1_5f2b29ea","line":28,"range":{"start_line":24,"start_character":66,"end_line":28,"end_character":38},"in_reply_to":"9fdfeff1_9c3769b1","updated":"2019-01-23 23:59:10.000000000","message":"Emm...maybe I\u0027m not clear my words. I mean the current behavior is \u0027when the user unshelves the image is deleted\u0027, but with shared image, the image won\u0027t be deleted, since the user isn\u0027t own the image. Even the server is deleted, I still don\u0027t think the user has the permission delete the image. Only the Admin have permission delete that image.","commit_id":"1677b1c16de51c219b72a4553777cacfbb3974e1"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0f8b79c7fb18da66ff48d5e1f76017d8612523f5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"9fdfeff1_aa01f06c","updated":"2019-03-04 11:46:30.000000000","message":"always impressed by your commit messages","commit_id":"35cc0f5e943642dd8d9dacbf0dac6e260f708d7d"}],"nova/image/glance.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c464b5792168af77ea9afe5474d8ac3d6fd360dc","unresolved":false,"context_lines":[{"line_number":472,"context_line":"        # not the owner of an instance, is creating the image, e.g. admin"},{"line_number":473,"context_line":"        # snapshots or shelves another user\u0027s instance. This is used to add"},{"line_number":474,"context_line":"        # member access to the image for the instance owner."},{"line_number":475,"context_line":"        sharing_member_id \u003d image_meta.pop(\u0027instance_owner\u0027, None)"},{"line_number":476,"context_line":"        sent_service_image_meta \u003d _translate_to_glance(image_meta)"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"bfdaf3ff_aa69ca25","line":475,"range":{"start_line":475,"start_character":28,"end_line":475,"end_character":66},"updated":"2019-01-14 21:20:26.000000000","message":"This should be looking in image_meta[\u0027properties\u0027].","commit_id":"2e9cef3c30adbf5c98994af750236cd98a62147e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f5c0da24b8353609f2daea45c9bd05350c34fdbb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bfdaf3ff_bf0f95df","updated":"2019-01-16 22:47:02.000000000","message":"Where is the test coverage for this delta? (See comments in the func test.)","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f5c0da24b8353609f2daea45c9bd05350c34fdbb","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        # empty data."},{"line_number":469,"context_line":"        force_activate \u003d data is None and image_meta.get(\u0027size\u0027) \u003d\u003d 0"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        # The \"instance_owner\" property is set in the API if a user, who is"},{"line_number":472,"context_line":"        # not the owner of an instance, is creating the image, e.g. admin"},{"line_number":473,"context_line":"        # snapshots or shelves another user\u0027s instance. This is used to add"},{"line_number":474,"context_line":"        # member access to the image for the instance owner."}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_9f3731e4","line":471,"range":{"start_line":471,"start_character":47,"end_line":471,"end_character":57},"updated":"2019-01-16 22:47:02.000000000","message":"in the metadata?","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7ab44a00a3aba8d79214ff800a3cf7085c749847","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        # empty data."},{"line_number":469,"context_line":"        force_activate \u003d data is None and image_meta.get(\u0027size\u0027) \u003d\u003d 0"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        # The \"instance_owner\" property is set in the API if a user, who is"},{"line_number":472,"context_line":"        # not the owner of an instance, is creating the image, e.g. admin"},{"line_number":473,"context_line":"        # snapshots or shelves another user\u0027s instance. This is used to add"},{"line_number":474,"context_line":"        # member access to the image for the instance owner."}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_b67c851f","line":471,"range":{"start_line":471,"start_character":47,"end_line":471,"end_character":57},"in_reply_to":"bfdaf3ff_9f3731e4","updated":"2019-01-17 23:16:20.000000000","message":"Well, set in image_meta by the API code...","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f5c0da24b8353609f2daea45c9bd05350c34fdbb","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        :param image_id: ID of the image on which to grant access"},{"line_number":502,"context_line":"        :param member_id: ID of the member project to grant access to the"},{"line_number":503,"context_line":"            image; this should not be the owner of the image"},{"line_number":504,"context_line":"        :returns: A Member schema object of the created image member"},{"line_number":505,"context_line":"        \"\"\""},{"line_number":506,"context_line":"        try:"},{"line_number":507,"context_line":"            return self._client.call("}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_3f4ba559","line":504,"range":{"start_line":504,"start_character":8,"end_line":504,"end_character":68},"updated":"2019-01-16 22:47:02.000000000","message":"...which isn\u0027t currently used anywhere, right?","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7ab44a00a3aba8d79214ff800a3cf7085c749847","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        :param image_id: ID of the image on which to grant access"},{"line_number":502,"context_line":"        :param member_id: ID of the member project to grant access to the"},{"line_number":503,"context_line":"            image; this should not be the owner of the image"},{"line_number":504,"context_line":"        :returns: A Member schema object of the created image member"},{"line_number":505,"context_line":"        \"\"\""},{"line_number":506,"context_line":"        try:"},{"line_number":507,"context_line":"            return self._client.call("}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_76729d4b","line":504,"range":{"start_line":504,"start_character":8,"end_line":504,"end_character":68},"in_reply_to":"bfdaf3ff_3f4ba559","updated":"2019-01-17 23:16:20.000000000","message":"Correct, I was following the _add_location method. I can nix the return if we don\u0027t care about it.","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"}],"nova/tests/functional/test_images.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f5c0da24b8353609f2daea45c9bd05350c34fdbb","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        self.assertIn(\u0027owner\u0027, metadata)"},{"line_number":99,"context_line":"        self.assertEqual(uuids.admin_project, metadata[\u0027owner\u0027])"},{"line_number":100,"context_line":"        # Assert the non-admin tenant user project is a member."},{"line_number":101,"context_line":"        self.assertIn(\u0027instance_owner\u0027, metadata)"},{"line_number":102,"context_line":"        self.assertEqual("},{"line_number":103,"context_line":"            self.api_fixture.project_id, metadata[\u0027instance_owner\u0027])"},{"line_number":104,"context_line":"        # Be sure we did not get a false positive by making sure the admin and"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_9f6f3191","line":101,"range":{"start_line":101,"start_character":0,"end_line":101,"end_character":49},"updated":"2019-01-16 22:47:02.000000000","message":"Hm.\n\nIRL what we send across the wire to glance doesn\u0027t contain \u0027instance_owner\u0027. We\u0027re hacking that into the metadata just long enough for it to travel into GlanceClientWrapper.create(), where we parlay it into a POST to the glance /v2/images/{image_id}/members endpoint. So IRL by the time we get here, we should *not* see \u0027instance_owner\u0027 in the metadata; but we *should* be able to do a GET /v2/images/{image_id}/members and see self.api_fixture.project_id in there.\n\nI haven\u0027t fully grokked at what point the glance fixture intercepts things. We may not be able to do a true simulation of that part of the flow here. But one way or another, we need test coverage for the delta in nova/image/glance.py, and I don\u0027t think we have it.","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"56aedf7f9477ca2d7763bc549f751f3ae15eccd7","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        self.assertIn(\u0027owner\u0027, metadata)"},{"line_number":99,"context_line":"        self.assertEqual(uuids.admin_project, metadata[\u0027owner\u0027])"},{"line_number":100,"context_line":"        # Assert the non-admin tenant user project is a member."},{"line_number":101,"context_line":"        self.assertIn(\u0027instance_owner\u0027, metadata)"},{"line_number":102,"context_line":"        self.assertEqual("},{"line_number":103,"context_line":"            self.api_fixture.project_id, metadata[\u0027instance_owner\u0027])"},{"line_number":104,"context_line":"        # Be sure we did not get a false positive by making sure the admin and"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_99a96a25","line":101,"range":{"start_line":101,"start_character":0,"end_line":101,"end_character":49},"in_reply_to":"bfdaf3ff_76a7fda8","updated":"2019-01-18 14:53:46.000000000","message":"Cool, so I\u0027m not asking for the fixture to be built up if that\u0027s too much work, but we should have tests (unit would be fine) that exercise the code that a) pops instance_owner off the image_meta and b) invokes that POST.","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b8cb7d0b66c2b17f772d3f1dba851fc61fe7c8bd","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        self.assertIn(\u0027owner\u0027, metadata)"},{"line_number":99,"context_line":"        self.assertEqual(uuids.admin_project, metadata[\u0027owner\u0027])"},{"line_number":100,"context_line":"        # Assert the non-admin tenant user project is a member."},{"line_number":101,"context_line":"        self.assertIn(\u0027instance_owner\u0027, metadata)"},{"line_number":102,"context_line":"        self.assertEqual("},{"line_number":103,"context_line":"            self.api_fixture.project_id, metadata[\u0027instance_owner\u0027])"},{"line_number":104,"context_line":"        # Be sure we did not get a false positive by making sure the admin and"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_3ecf7e80","line":101,"range":{"start_line":101,"start_character":0,"end_line":101,"end_character":49},"in_reply_to":"bfdaf3ff_99a96a25","updated":"2019-01-21 16:55:45.000000000","message":"\u003e Cool, so I\u0027m not asking for the fixture to be built up if that\u0027s\n \u003e too much work, but we should have tests (unit would be fine) that\n \u003e exercise the code that a) pops instance_owner off the image_meta\n \u003e and b) invokes that POST.\n\nDone.","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7ab44a00a3aba8d79214ff800a3cf7085c749847","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        self.assertIn(\u0027owner\u0027, metadata)"},{"line_number":99,"context_line":"        self.assertEqual(uuids.admin_project, metadata[\u0027owner\u0027])"},{"line_number":100,"context_line":"        # Assert the non-admin tenant user project is a member."},{"line_number":101,"context_line":"        self.assertIn(\u0027instance_owner\u0027, metadata)"},{"line_number":102,"context_line":"        self.assertEqual("},{"line_number":103,"context_line":"            self.api_fixture.project_id, metadata[\u0027instance_owner\u0027])"},{"line_number":104,"context_line":"        # Be sure we did not get a false positive by making sure the admin and"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfdaf3ff_76a7fda8","line":101,"range":{"start_line":101,"start_character":0,"end_line":101,"end_character":49},"in_reply_to":"bfdaf3ff_9f6f3191","updated":"2019-01-17 23:16:20.000000000","message":"Right we don\u0027t send instance_owner to \"real\" glance because we pop it out, I confirmed this when recreating and testing the fix in devstack.\n\nBut with the fake image service fixture, we don\u0027t do that pop:\n\nhttps://review.openstack.org/#/c/630769/3/nova/tests/unit/image/fake.py@251\n\nAnd it\u0027s also why we have to stash \u0027owner\u0027 in \u0027properties\u0027.\n\n\u003e but we *should* be able to do a GET /v2/images/{image_id}/members and see self.api_fixture.project_id in there.\n\nCorrect, but we don\u0027t have a fixture for that API, so instead of adding a bunch of new code to fixture-ize that API and track members internally within the fake image service fixture, I just left the instance_owner property in the metadata so I can assert it\u0027s passed down from the API.","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"8260bbb5af1aba9d2346ba9d581f60bc7becb362","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            self.api_fixture.project_id, metadata[\u0027instance_owner\u0027])"},{"line_number":104,"context_line":"        # Be sure we did not get a false positive by making sure the admin and"},{"line_number":105,"context_line":"        # tenant user API fixtures are not using the same project_id."},{"line_number":106,"context_line":"        self.assertNotEqual(uuids.admin_project, self.api_fixture.project_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5fc1f717_92963a93","line":106,"updated":"2019-03-12 13:58:36.000000000","message":"It will be great check the image with tenant user context in the end, just ensure the tenant user can access the image.","commit_id":"35cc0f5e943642dd8d9dacbf0dac6e260f708d7d"}],"releasenotes/notes/bug-1675791-snapshot-member-access-c40bba36606618f7.yaml":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"819e65464c67d848645b3b069338829c2b455a2e","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug 1675791`_ has been fixed by granting image membership access to"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bfdaf3ff_0bc6d99b","line":1,"updated":"2019-01-14 23:02:53.000000000","message":"I\u0027d also put something in the docs but we don\u0027t have any obvious user docs for snapshot that I can find. We have:\n\nhttps://docs.openstack.org/nova/latest/admin/migrate-instance-with-snapshot.html\n\nbut that\u0027s really an admin guide for using snapshot to migrate instances across openstack deployments (regions), so it\u0027s not really appropriate.","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aaf0c1f9bcff5a40f9af87c3435ef0f2cebf43a0","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    `Bug 1675791`_ has been fixed by granting image membership access to"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bfdaf3ff_ab3a2d95","line":1,"in_reply_to":"bfdaf3ff_0bc6d99b","updated":"2019-01-14 23:04:08.000000000","message":"Alternatively we could put something in the createImage/createBackup/shelve API reference docs about post-conditions where the owner of the server will have membership access to the snapshot image if the user that performed the action is not the owner of the server. That seems like a lot of low-level detail for the API reference though for an uncommon scenario.","commit_id":"fb4dcaccb041a6ae38a4e46c7ed83fc855c707db"}]}