)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"c4d22768c2080e3d95be307916d4d6c1f3fa9e54","unresolved":false,"context_lines":[{"line_number":13,"context_line":"makes the volume update code check the appropriate policy based"},{"line_number":14,"context_line":"on what action is being performed."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"FIXME: Needs a reno"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Partial implement blueprint destroy-instance-with-datavolume"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"df33271e_9c57fdca","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":19},"updated":"2020-03-31 08:32:38.000000000","message":"addressed.","commit_id":"4203e13edf38bca3952eb091ebc8885fe5cac68b"}],"nova/api/openstack/compute/volumes.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"34b8f47d7c10d95395fbcb3dcf677e0604e037b2","unresolved":false,"context_lines":[{"line_number":442,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":443,"context_line":"            # This is a swap operation"},{"line_number":444,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027,"},{"line_number":445,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":446,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"        context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"df33271e_2cb351ec","line":445,"range":{"start_line":445,"start_character":24,"end_line":445,"end_character":66},"updated":"2020-03-27 17:37:16.000000000","message":"for default of this policy which is admin, we do not need to pass the project_id but if we want to support the override rule with the admin-or-owner then there is no harm of passing.","commit_id":"dec28a4aec1bc159c3940dfcd1cf5b43bbfbf5b7"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"2de14064d36b7d07630c8e035bec6a4db359c941","unresolved":false,"context_lines":[{"line_number":442,"context_line":"        if only_swap or id !\u003d body[\u0027volumeAttachment\u0027][\u0027volumeId\u0027]:"},{"line_number":443,"context_line":"            # This is a swap operation"},{"line_number":444,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027,"},{"line_number":445,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":446,"context_line":"            return self._update_volume_swap(req, server_id, id, body)"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"        context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"df33271e_1723faa9","line":445,"range":{"start_line":445,"start_character":24,"end_line":445,"end_character":66},"in_reply_to":"df33271e_2cb351ec","updated":"2020-03-27 17:57:55.000000000","message":"no, it would help override rule also as this polciy is scoped as \u0027system\u0027 only. we can remove this.","commit_id":"dec28a4aec1bc159c3940dfcd1cf5b43bbfbf5b7"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"966b5e37673e431d7af8f4f6a580199f65987c1d","unresolved":false,"context_lines":[{"line_number":477,"context_line":""},{"line_number":478,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":479,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.85\u0027)"},{"line_number":480,"context_line":""},{"line_number":481,"context_line":"        # NOTE(brinzhang): If the \u0027volumeId\u0027 requested by the user is"},{"line_number":482,"context_line":"        # different from the \u0027id\u0027 in the url path, we should first check"},{"line_number":483,"context_line":"        # the policy of the swap volume, and then according to the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_1a3e23b7","line":480,"range":{"start_line":480,"start_character":0,"end_line":480,"end_character":0},"updated":"2020-04-01 17:26:32.000000000","message":"I showed the policy enforcement in exmaple patch - https://review.opendev.org/#/c/716679/\n\nthat does:\n- enforce swap policy first for swap-only (with either microverison) or swap+update case\n- enforce update policy if it is actual update request. so that we do not fail on update policy for swap only operation.\n\nas gibi mentioned, we need to add tests for all those cases so that we do not miss any.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"1fd7061858d94835753342af924f833d3e4e289e","unresolved":false,"context_lines":[{"line_number":477,"context_line":""},{"line_number":478,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":479,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.85\u0027)"},{"line_number":480,"context_line":""},{"line_number":481,"context_line":"        # NOTE(brinzhang): If the \u0027volumeId\u0027 requested by the user is"},{"line_number":482,"context_line":"        # different from the \u0027id\u0027 in the url path, we should first check"},{"line_number":483,"context_line":"        # the policy of the swap volume, and then according to the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_f615534a","line":480,"range":{"start_line":480,"start_character":0,"end_line":480,"end_character":0},"in_reply_to":"df33271e_1a3e23b7","updated":"2020-04-02 03:47:33.000000000","message":"Thanks, I will merge that.\n- If the user want to swap, need to check the v2.85 and swap policy. \n- If the user want to update, they should check update policy for d-o-t, because of other parameters we doesnot allow to update.\n- \u0027swap\u0027 policy *contains* \u0027update\u0027 policy. admin role can do anything in this update api.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"2f35d21b099385d8d02c6d0896a0a32931349d2f","unresolved":false,"context_lines":[{"line_number":477,"context_line":""},{"line_number":478,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":479,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.85\u0027)"},{"line_number":480,"context_line":""},{"line_number":481,"context_line":"        # NOTE(brinzhang): If the \u0027volumeId\u0027 requested by the user is"},{"line_number":482,"context_line":"        # different from the \u0027id\u0027 in the url path, we should first check"},{"line_number":483,"context_line":"        # the policy of the swap volume, and then according to the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_c5ca0e9f","line":480,"range":{"start_line":480,"start_character":0,"end_line":480,"end_character":0},"in_reply_to":"df33271e_c5d4cf4a","updated":"2020-04-02 08:43:26.000000000","message":"Yes, I think it would be sufficient if we could override the default policy settings.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e156ead03ebd77f5ba96b2f2f50a593a3f812cba","unresolved":false,"context_lines":[{"line_number":477,"context_line":""},{"line_number":478,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":479,"context_line":"        only_swap \u003d not api_version_request.is_supported(req, \u00272.85\u0027)"},{"line_number":480,"context_line":""},{"line_number":481,"context_line":"        # NOTE(brinzhang): If the \u0027volumeId\u0027 requested by the user is"},{"line_number":482,"context_line":"        # different from the \u0027id\u0027 in the url path, we should first check"},{"line_number":483,"context_line":"        # the policy of the swap volume, and then according to the"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_c5d4cf4a","line":480,"range":{"start_line":480,"start_character":0,"end_line":480,"end_character":0},"in_reply_to":"df33271e_f615534a","updated":"2020-04-02 08:15:48.000000000","message":"\u003e - \u0027swap\u0027 policy *contains* \u0027update\u0027 policy. admin role can do\n \u003e anything in this update api.\n\nYes, if the policy is kept on default settings then swap policy contains update policy. BUT policy is configurable by the deployer so we cannot assume this containment holds for any deployment in general.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1f5be464ab47d8628c3f11ce1ed9c9515d438e81","unresolved":false,"context_lines":[{"line_number":485,"context_line":"        volume_id \u003d attachment[\u0027volumeId\u0027]"},{"line_number":486,"context_line":"        authorized_swap \u003d False"},{"line_number":487,"context_line":"        if id !\u003d volume_id:"},{"line_number":488,"context_line":"            authorized_swap \u003d context.can(va_policies.POLICY_ROOT % \u0027swap\u0027)"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        if only_swap and authorized_swap:"},{"line_number":491,"context_line":"            # NOTE(danms): Original behavior is always call swap on PUT"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_4f7bd926","line":488,"updated":"2020-04-01 09:04:38.000000000","message":"When fatal\u003dTrue (the default) then it does not make sense to use the return value of can() as this call raises Forbidden exception if the check fails","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1f5be464ab47d8628c3f11ce1ed9c9515d438e81","unresolved":false,"context_lines":[{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        if only_swap and authorized_swap:"},{"line_number":491,"context_line":"            # NOTE(danms): Original behavior is always call swap on PUT"},{"line_number":492,"context_line":"            self._update_volume_swap(req, server_id, id, body)"},{"line_number":493,"context_line":"        else:"},{"line_number":494,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":495,"context_line":"            # properties first, and then call swap if volumeId differs"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_6fa5fda1","line":492,"updated":"2020-04-01 09:04:38.000000000","message":"I suggest to simply add the \n\n  context.can(va_policies.POLICY_ROOT % \u0027swap\u0027)\n\ncheck here","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"dc8ea4bb8634599f2c3311c5c652f0fa502d9f00","unresolved":false,"context_lines":[{"line_number":490,"context_line":"        if only_swap and authorized_swap:"},{"line_number":491,"context_line":"            # NOTE(danms): Original behavior is always call swap on PUT"},{"line_number":492,"context_line":"            self._update_volume_swap(req, server_id, id, body)"},{"line_number":493,"context_line":"        else:"},{"line_number":494,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":495,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":496,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":497,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":498,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":499,"context_line":"            if authorized_swap:"},{"line_number":500,"context_line":"                self._update_volume_swap(req, server_id, id, body)"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_f781dac4","line":497,"range":{"start_line":493,"start_character":0,"end_line":497,"end_character":67},"updated":"2020-04-01 17:15:45.000000000","message":"this else block does not means request is for update. this can be swap-only so failing on update policy before we actually check that request is for update or not in _update_volume_regular is not good. \n\nScenario: I am PUTing request by replicating GET response with VolumeId changed (swap only but keeping all other field same as existing value.) in that case update policy can fail the swap operation.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1f5be464ab47d8628c3f11ce1ed9c9515d438e81","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        else:"},{"line_number":494,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":495,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":496,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":497,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":498,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":499,"context_line":"            if authorized_swap:"},{"line_number":500,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @wsgi.response(202)"},{"line_number":503,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_cf0b698b","line":500,"range":{"start_line":496,"start_character":0,"end_line":500,"end_character":66},"updated":"2020-04-01 09:04:38.000000000","message":"This might be a bit more complex. A PUT request that triggers just an update should be guarded with the \u0027update\u0027 policy but update + swap should be guarded with both the \u0027update\u0027 and the \u0027swap\u0027 policy.  Also as update + swap that fails the \u0027swap\u0027 policy should not do a partial update.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"1fd7061858d94835753342af924f833d3e4e289e","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        else:"},{"line_number":494,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":495,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":496,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":497,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":498,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":499,"context_line":"            if authorized_swap:"},{"line_number":500,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @wsgi.response(202)"},{"line_number":503,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_56623f0b","line":500,"range":{"start_line":496,"start_character":0,"end_line":500,"end_character":66},"in_reply_to":"df33271e_b7bab2bd","updated":"2020-04-02 03:47:33.000000000","message":"Ok, agree.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"dc8ea4bb8634599f2c3311c5c652f0fa502d9f00","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        else:"},{"line_number":494,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":495,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":496,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":497,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":498,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":499,"context_line":"            if authorized_swap:"},{"line_number":500,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @wsgi.response(202)"},{"line_number":503,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_b7bab2bd","line":500,"range":{"start_line":496,"start_character":0,"end_line":500,"end_character":66},"in_reply_to":"df33271e_cf0b698b","updated":"2020-04-01 17:15:45.000000000","message":"+1. I mentioned the same in yesterday\u0027s IRC discussions. we really need update+swap as \u0027complete success\u0027 or \u0027failure\u0027 even for policy case. Otherwise it end up with HTTP 207 multi status things which is bad.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"24d66269aa0d3f4264b49900df93df1520a4b4c2","unresolved":false,"context_lines":[{"line_number":493,"context_line":"        else:"},{"line_number":494,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":495,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":496,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":497,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":498,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":499,"context_line":"            if authorized_swap:"},{"line_number":500,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @wsgi.response(202)"},{"line_number":503,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_4faf5929","line":500,"range":{"start_line":496,"start_character":0,"end_line":500,"end_character":66},"in_reply_to":"df33271e_cf0b698b","updated":"2020-04-01 09:13:27.000000000","message":"Yes, I said in https://review.opendev.org/#/c/693828/24/nova/api/openstack/compute/volumes.py@496, I also think this is strage, hope @Dan have some good idea.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5da8dd4902cdd52234458363f413023db4e64251","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":462,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":463,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":464,"context_line":"                # NOTE(brinzhang): This is request for update because updatable"},{"line_number":465,"context_line":"                # value are changed that we should check \u0027update\u0027 policy. If we"},{"line_number":466,"context_line":"                # are not in this block there is possibility that request has"},{"line_number":467,"context_line":"                # these fields but no change in value so does no update request"},{"line_number":468,"context_line":"                context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":469,"context_line":"                            target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":470,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":471,"context_line":"            bdm.save()"},{"line_number":472,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_eed72fbe","line":469,"range":{"start_line":464,"start_character":0,"end_line":469,"end_character":71},"updated":"2020-04-02 11:10:09.000000000","message":"Do we actually need to do this? Even if they\u0027re not changing anything, the fact that they\u0027re even trying means we should reject the request, no?","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"cddb6f52cc7d3ee095c29336186bae884401f70f","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":462,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":463,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":464,"context_line":"                # NOTE(brinzhang): This is request for update because updatable"},{"line_number":465,"context_line":"                # value are changed that we should check \u0027update\u0027 policy. If we"},{"line_number":466,"context_line":"                # are not in this block there is possibility that request has"},{"line_number":467,"context_line":"                # these fields but no change in value so does no update request"},{"line_number":468,"context_line":"                context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":469,"context_line":"                            target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":470,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":471,"context_line":"            bdm.save()"},{"line_number":472,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_87263867","line":469,"range":{"start_line":464,"start_character":0,"end_line":469,"end_character":71},"in_reply_to":"df33271e_e269fb24","updated":"2020-04-02 23:29:39.000000000","message":"I am still not very clear on how we can enforce the both policy as per their request. \n\nI agree on that if the request is made then irrespective of whether value is changed or not we can consider the PUT as valid PUT request and enforce the policy at starting. \n\nHere things are little different because of two policies and two operation. For example. If I am doing only swap request (\u003e2.85) by\n- copy paste the GET response and _only_ change the volumeId\n\nThat should be consider as swap-only request right and in that case if we check PUT policy also then it is not valid. We can consider the \u0027swap\u0027 policy is superset of \u0027update\u0027 policy as per defaults but with override value we cannot consider that. Operator can override them in any way.\n\nBeing said that, let\u0027s rebase this on latest base patch and see the pic more clear.","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c8c12ddb756cf3015db5f64bb70eda20dd7e7ae1","unresolved":false,"context_lines":[{"line_number":461,"context_line":"                                         \u0027not mutable\u0027)"},{"line_number":462,"context_line":"            if attachment.get(\u0027delete_on_termination\u0027) not in ("},{"line_number":463,"context_line":"                    None, bdm.delete_on_termination):"},{"line_number":464,"context_line":"                # NOTE(brinzhang): This is request for update because updatable"},{"line_number":465,"context_line":"                # value are changed that we should check \u0027update\u0027 policy. If we"},{"line_number":466,"context_line":"                # are not in this block there is possibility that request has"},{"line_number":467,"context_line":"                # these fields but no change in value so does no update request"},{"line_number":468,"context_line":"                context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":469,"context_line":"                            target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":470,"context_line":"                bdm.delete_on_termination \u003d attachment[\u0027delete_on_termination\u0027]"},{"line_number":471,"context_line":"            bdm.save()"},{"line_number":472,"context_line":"        except exception.VolumeBDMNotFound as e:"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_e269fb24","line":469,"range":{"start_line":464,"start_character":0,"end_line":469,"end_character":71},"in_reply_to":"df33271e_eed72fbe","updated":"2020-04-02 16:23:48.000000000","message":"Yeah agree, doing this here makes no sense. We should authorize the PUT or swap early in the call regardless.","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5da8dd4902cdd52234458363f413023db4e64251","unresolved":false,"context_lines":[{"line_number":500,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":501,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":502,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":503,"context_line":"            if authorized_swap:"},{"line_number":504,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":505,"context_line":""},{"line_number":506,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_ae5d277a","line":503,"updated":"2020-04-02 11:10:09.000000000","message":"I\u0027m not a fan of this. The name suggests this is checking whether you\u0027re allowed to swap, but in reality if that check failed we\u0027d have raised an exception already, so this is simply checking id !\u003d volume_id.\n\nCould we keep this check as-is and simply add a \u0027context.can\u0027 call before the two calls to \u0027_update_volume_swap\u0027? Alternatively, do it inside that function, though I\u0027m assuming you don\u0027t want to do that because it makes mocking harder..","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"e251ea915cab1890457b530880158f27589ada9f","unresolved":false,"context_lines":[{"line_number":500,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":501,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":502,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":503,"context_line":"            if authorized_swap:"},{"line_number":504,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":505,"context_line":""},{"line_number":506,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_ed2e038a","line":503,"in_reply_to":"df33271e_4791d046","updated":"2020-04-03 01:17:46.000000000","message":"After checking your chat record [1], and that comments in this, I have not get more clear thing what I need to do in next patch, so I will wait @Dan update.\n\n\u003e the \u0027swap\u0027 policy is superset of \u0027update\u0027 policy as per defaults\n\nAgreed, we can check PUT policy when start request update request.\n\n\u003e but but with override value we cannot consider that.\n\nIMO, this is beyond our default settings and can be ignored. But once the user changes one of the PUT or swap policies, we must at least ensure that the other is available.\n\n\u003e or we are saying swap (with or without other value changed) is always PUT+swap so checking \u0027updte\u0027 policy always and then check swap policy if swap requested ?\n\nI\u0027m not sure about that either.This means that regardless of the form of the request, it has only two results, either failure or success.\n\n[1]http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2020-04-02.log.html#t2020-04-02T16:08:14","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"cddb6f52cc7d3ee095c29336186bae884401f70f","unresolved":false,"context_lines":[{"line_number":500,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":501,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":502,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":503,"context_line":"            if authorized_swap:"},{"line_number":504,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":505,"context_line":""},{"line_number":506,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_e7974444","line":503,"in_reply_to":"df33271e_82f2af6e","updated":"2020-04-02 23:29:39.000000000","message":"I might be overthinking on this but 1 question (i think \n mentioned in early comment also). Should not we consider the below request as swap only (not PUT with swap) request \u003e2.85\n- requesting with volumeId change but passing all other value unchanged or not passing them.","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c8c12ddb756cf3015db5f64bb70eda20dd7e7ae1","unresolved":false,"context_lines":[{"line_number":500,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":501,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":502,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":503,"context_line":"            if authorized_swap:"},{"line_number":504,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":505,"context_line":""},{"line_number":506,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_82f2af6e","line":503,"in_reply_to":"df33271e_ae5d277a","updated":"2020-04-02 16:23:48.000000000","message":"I also don\u0027t want to do that because it means we would have completed the regular PUT part of the call and then fail on the auth check for swap, having completed half of the update and not the other.\n\nPersonally, I think we should determine, early in the call, whether this is a regular PUT or a PUT with swap, and authorize based on the appropriate policy. PUT means you can change anything mutable other than volumeId (which is currently just d-o-t). PUT with swap means you have the aforementioned power, as well as power to change volumeId.","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"67bf9ad80b838ba6d4e5afafe9c7cc367400f90d","unresolved":false,"context_lines":[{"line_number":500,"context_line":"            # NOTE(danms): New behavior is update any supported attachment"},{"line_number":501,"context_line":"            # properties first, and then call swap if volumeId differs"},{"line_number":502,"context_line":"            self._update_volume_regular(req, server_id, id, body)"},{"line_number":503,"context_line":"            if authorized_swap:"},{"line_number":504,"context_line":"                self._update_volume_swap(req, server_id, id, body)"},{"line_number":505,"context_line":""},{"line_number":506,"context_line":"    @wsgi.response(202)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df33271e_4791d046","line":503,"in_reply_to":"df33271e_e7974444","updated":"2020-04-02 23:34:55.000000000","message":"or we are saying swap (with or without other value changed) is always PUT+swap so checking \u0027updte\u0027 policy always and then check swap policy if swap requested ?","commit_id":"c6c6ca436dadf74840875e5db93fa7da8afe427d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98ad58a4ba62375dbd993642954a2818f301d0c3","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        # Later, split off the swap_volume permission and check the correct"},{"line_number":470,"context_line":"        # policy based on what is being asked by the client."},{"line_number":471,"context_line":"        context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":472,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":475,"context_line":"        volume_id \u003d attachment[\u0027volumeId\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_0a2131fc","side":"PARENT","line":472,"range":{"start_line":472,"start_character":20,"end_line":472,"end_character":62},"updated":"2020-04-07 16:58:35.000000000","message":"I\u0027m confused. Should we have been passing this through previously since it was a SYSTEM_ADMIN policy?","commit_id":"cd16ae25c865f25dbb313976b3d8ef9372db80af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"41af64413839011ee1f19d77e66f127ab9d79a90","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        # Later, split off the swap_volume permission and check the correct"},{"line_number":470,"context_line":"        # policy based on what is being asked by the client."},{"line_number":471,"context_line":"        context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":472,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":475,"context_line":"        volume_id \u003d attachment[\u0027volumeId\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_ca02a93e","side":"PARENT","line":472,"range":{"start_line":472,"start_character":20,"end_line":472,"end_character":62},"in_reply_to":"df33271e_0a2131fc","updated":"2020-04-07 17:02:08.000000000","message":"Looks like a mistake in [1]\n\n[1] https://review.opendev.org/#/c/709955/3/nova/api/openstack/compute/volumes.py@401","commit_id":"cd16ae25c865f25dbb313976b3d8ef9372db80af"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"06e6c11194665c57f5220d4bcc6e8fa47538f73d","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        # Later, split off the swap_volume permission and check the correct"},{"line_number":470,"context_line":"        # policy based on what is being asked by the client."},{"line_number":471,"context_line":"        context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":472,"context_line":"                    target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"        attachment \u003d body[\u0027volumeAttachment\u0027]"},{"line_number":475,"context_line":"        volume_id \u003d attachment[\u0027volumeId\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_ed5d0716","side":"PARENT","line":472,"range":{"start_line":472,"start_character":20,"end_line":472,"end_character":62},"in_reply_to":"df33271e_ca02a93e","updated":"2020-04-07 17:36:58.000000000","message":"yeah with system scope only access, project id is no use.","commit_id":"cd16ae25c865f25dbb313976b3d8ef9372db80af"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"06e6c11194665c57f5220d4bcc6e8fa47538f73d","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        # otherwise, check the volume update policy."},{"line_number":476,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":477,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027, target\u003d{})"},{"line_number":478,"context_line":"        else:"},{"line_number":479,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":480,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":481,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_cd20034d","line":478,"range":{"start_line":478,"start_character":0,"end_line":478,"end_character":13},"updated":"2020-04-07 17:36:58.000000000","message":"this leave one case to solve which might be my overthinking and not something user do but I want to bring here.\n\nIf request if for swap + update then we only check swap policy. If with override rules, swap policy is not superset of update policy then we end up allowing update for all users have swap access. \n\nWith defaults, it is no issue as swap is a superset of update policy.\n\nI was thinking to make \u0027update\u0027 as default check here and swap as extra check:\n\n- enforce \u0027update\u0027 policy always (for \u003e2.85) as PUT request means you are updating something.\n- enforce swap if it is swap request. This is nothing but an extra requested operation for *update* request,","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0a64a8efcbfbd8a974ee870242df19dee9420e2e","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        # otherwise, check the volume update policy."},{"line_number":476,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":477,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027, target\u003d{})"},{"line_number":478,"context_line":"        else:"},{"line_number":479,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":480,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":481,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_906f34c4","line":478,"range":{"start_line":478,"start_character":0,"end_line":478,"end_character":13},"in_reply_to":"df33271e_cd20034d","updated":"2020-04-07 18:09:42.000000000","message":"we discussed it on IRC and we cannot handle the all possible cases here (existing deployement, override cases in all possible permission etc).\n\nDoing the current way is best as it does not impact the existing users and we can update the policy doc of new \u0027update\u0027 policy about \u0027for swap + update\u0027 request (which is possible only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be always superset of this policy permission.","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98ad58a4ba62375dbd993642954a2818f301d0c3","unresolved":false,"context_lines":[{"line_number":477,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027, target\u003d{})"},{"line_number":478,"context_line":"        else:"},{"line_number":479,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":480,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        if only_swap:"},{"line_number":483,"context_line":"            # NOTE(danms): Original behavior is always call swap on PUT"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_0a4811ca","line":480,"updated":"2020-04-07 16:58:35.000000000","message":"Okay, so in effect we\u0027re renaming update to swap and then adding update which only applies if using the newer microversion and are definitely not swapping.","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"8161d15d9894c44830cea676da331a7731ea2a75","unresolved":false,"context_lines":[{"line_number":477,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027, target\u003d{})"},{"line_number":478,"context_line":"        else:"},{"line_number":479,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":480,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"        if only_swap:"},{"line_number":483,"context_line":"            # NOTE(danms): Original behavior is always call swap on PUT"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_58f3050b","line":480,"in_reply_to":"df33271e_0a4811ca","updated":"2020-04-08 02:39:14.000000000","message":"Yes, the swap policy is the superset of update policy. So if the swap policy not allow, that will be not allow to swap volume.","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"337044b696be792b82b9aa3aff267c786a952fbb","unresolved":false,"context_lines":[{"line_number":474,"context_line":"        # the microversion, we should check the swap volume policy."},{"line_number":475,"context_line":"        # otherwise, check the volume update policy."},{"line_number":476,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":477,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027, target\u003d{})"},{"line_number":478,"context_line":"        else:"},{"line_number":479,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":480,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"}],"source_content_type":"text/x-python","patch_set":17,"id":"df33271e_695c397e","line":477,"updated":"2020-04-08 08:00:02.000000000","message":"OK, so changing the volume_id and d-o-t in the same request can be done with swap policy. In this case the d-o-t value is used for the new volume_id. I think in this case requiring swap and update policy would be a more correct behavior in theory. But I accept that in reality if I can swap volumes then I should be able to define the params of the swapped in volume.","commit_id":"983a57d4060be59cf883a74979020f608a437ea4"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"aa87776a62ad67362630ac72bdbb7fdc3d295f3e","unresolved":false,"context_lines":[{"line_number":474,"context_line":"        # the microversion, we should check the swap volume policy."},{"line_number":475,"context_line":"        # otherwise, check the volume update policy."},{"line_number":476,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":477,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027, target\u003d{})"},{"line_number":478,"context_line":"        else:"},{"line_number":479,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"},{"line_number":480,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"}],"source_content_type":"text/x-python","patch_set":17,"id":"df33271e_a4dca874","line":477,"in_reply_to":"df33271e_695c397e","updated":"2020-04-08 08:16:45.000000000","message":"+1","commit_id":"983a57d4060be59cf883a74979020f608a437ea4"}],"nova/policies/volumes_attachments.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"df5b103477d6f4371d24c4341a73776755b24dab","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            }"},{"line_number":80,"context_line":"        ],"},{"line_number":81,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027]),"},{"line_number":82,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":83,"context_line":"        name\u003dPOLICY_ROOT % \u0027update_volume\u0027,"},{"line_number":84,"context_line":"        check_str\u003dbase.PROJECT_MEMBER_OR_SYSTEM_ADMIN,"},{"line_number":85,"context_line":"        description\u003d\"Patch a volume attachment\","},{"line_number":86,"context_line":"        operations\u003d["},{"line_number":87,"context_line":"            {"},{"line_number":88,"context_line":"                \u0027method\u0027: \u0027PATCH\u0027,"},{"line_number":89,"context_line":"                \u0027path\u0027:"},{"line_number":90,"context_line":"                 \u0027/servers/{server_id}/os-volume_attachments/{volume_id}\u0027"},{"line_number":91,"context_line":"            }"},{"line_number":92,"context_line":"        ],"},{"line_number":93,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027]),"},{"line_number":94,"context_line":"]"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"df33271e_7f5fd471","line":93,"range":{"start_line":82,"start_character":0,"end_line":93,"end_character":43},"updated":"2020-03-26 17:15:51.000000000","message":"because we are adding this in PUT API, we discussed the polciy things also on IRC[1].\n\nLet\u0027s do:\n1. add this new policy with name\u003dPOLICY_ROOT % \u0027update\u0027, with default to same what you have already (PROJECT_MEMBER_OR_SYSTEM_ADMIN)\n\n2.  change the existing policy @l59 to name\u003dPOLICY_ROOT % \u0027swap\u0027 keeping everything else like default same there\n\n3. add releasenotes about this change. something like: existing policy \u0027os-volumes-attachments:update\u0027 is used for updating the resources with the change in its default value from SYSTEM_ADMIN to PROJECT_MEMBER_OR_SYSTEM_ADMIN. New policy \u0027os-volumes-attachments:swap\u0027 is introduced for swapping the attachment of servers with default to SYSTEM_ADMIN.\n\n[1] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2020-03-26.log.html#t2020-03-26T16:53:07","commit_id":"4f3ca70187901aba60dfeae01c8f4706205e0f62"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1f5be464ab47d8628c3f11ce1ed9c9515d438e81","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":59,"context_line":"        name\u003dPOLICY_ROOT % \u0027update\u0027,"},{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER_OR_SYSTEM_ADMIN,"},{"line_number":61,"context_line":"        description\u003d\"Update a volume attachment\","},{"line_number":62,"context_line":"        operations\u003d["},{"line_number":63,"context_line":"            {"},{"line_number":64,"context_line":"                \u0027method\u0027: \u0027PUT\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_6f417da9","line":61,"updated":"2020-04-01 09:04:38.000000000","message":"Update parameters of the current volume attachment, except changing the volumeId.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98ad58a4ba62375dbd993642954a2818f301d0c3","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027]),"},{"line_number":58,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":59,"context_line":"        name\u003dPOLICY_ROOT % \u0027update\u0027,"},{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER_OR_SYSTEM_ADMIN,"},{"line_number":61,"context_line":"        description\u003d\"Update a volume attachment\","},{"line_number":62,"context_line":"        operations\u003d["},{"line_number":63,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_2a08758f","line":60,"updated":"2020-04-07 16:58:35.000000000","message":"Naturally the main issue with changing this is that anyone using full policy.yaml file is not going to pick up this change. No easy way around that though.","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"06e6c11194665c57f5220d4bcc6e8fa47538f73d","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        scope_types\u003d[\u0027system\u0027, \u0027project\u0027]),"},{"line_number":58,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":59,"context_line":"        name\u003dPOLICY_ROOT % \u0027update\u0027,"},{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER_OR_SYSTEM_ADMIN,"},{"line_number":61,"context_line":"        description\u003d\"Update a volume attachment\","},{"line_number":62,"context_line":"        operations\u003d["},{"line_number":63,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_4d5fd3e4","line":60,"in_reply_to":"df33271e_2a08758f","updated":"2020-04-07 17:36:58.000000000","message":"true, we discussed it sometime on IRC and decided not to go with deprecation way. swap operation is really not many user use direcrtly and more of call from cinder side.","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"337044b696be792b82b9aa3aff267c786a952fbb","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER_OR_SYSTEM_ADMIN,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":17,"id":"df33271e_c9f8e54a","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"updated":"2020-04-08 08:00:02.000000000","message":"thanks for documenting the assumption.","commit_id":"983a57d4060be59cf883a74979020f608a437ea4"}],"nova/tests/unit/policies/test_volumes.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1f5be464ab47d8628c3f11ce1ed9c9515d438e81","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                 rule_name, self.controller.update,"},{"line_number":174,"context_line":"                                 req, FAKE_UUID,"},{"line_number":175,"context_line":"                                 FAKE_UUID_A, body\u003dbody)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch(\u0027nova.compute.api.API.detach_volume\u0027)"},{"line_number":178,"context_line":"    def test_delete_volume_attach_policy(self, mock_detach_volume):"},{"line_number":179,"context_line":"        rule_name \u003d self.policy_root % \"delete\""}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_2fd015c9","line":176,"updated":"2020-04-01 09:04:38.000000000","message":"You need test cases for \n* policy check fails for update\n* policy check fails for swap\n* policy check passes for swap + update\n* policy check fails for swap + update due to swap policy failure\n* policy check fails for swap + update due to update policy failure","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"1fd7061858d94835753342af924f833d3e4e289e","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                 rule_name, self.controller.update,"},{"line_number":174,"context_line":"                                 req, FAKE_UUID,"},{"line_number":175,"context_line":"                                 FAKE_UUID_A, body\u003dbody)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch(\u0027nova.compute.api.API.detach_volume\u0027)"},{"line_number":178,"context_line":"    def test_delete_volume_attach_policy(self, mock_detach_volume):"},{"line_number":179,"context_line":"        rule_name \u003d self.policy_root % \"delete\""}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_b9c3bcec","line":176,"in_reply_to":"df33271e_2fd015c9","updated":"2020-04-02 03:47:33.000000000","message":"These test scenarios all addressed.\n\n\u003e You need test cases for\n \u003e * policy check fails for update\n\ntest_update_volume_attach_policy_failed()\n\n \u003e * policy check fails for swap\n\ntest_swap_volume_attach_policy_failed()\n\n \u003e * policy check passes for swap + update\n\ntest_pass_swap_and_update_volume_attach_policy()\n\n \u003e * policy check fails for swap + update due to swap policy failure\n\ntest_check_swap_and_update_failed_due_swap_policy()\n\n \u003e * policy check fails for swap + update due to update policy failure\n\nI add test_check_swap_and_update_failed_due_update_policy(), but that cannot pass the swap policy check(cannot get something what I want to get), see http://paste.openstack.org/show/791501/.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e156ead03ebd77f5ba96b2f2f50a593a3f812cba","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                 rule_name, self.controller.update,"},{"line_number":174,"context_line":"                                 req, FAKE_UUID,"},{"line_number":175,"context_line":"                                 FAKE_UUID_A, body\u003dbody)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch(\u0027nova.compute.api.API.detach_volume\u0027)"},{"line_number":178,"context_line":"    def test_delete_volume_attach_policy(self, mock_detach_volume):"},{"line_number":179,"context_line":"        rule_name \u003d self.policy_root % \"delete\""}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_e538f2d3","line":176,"in_reply_to":"df33271e_b9c3bcec","updated":"2020-04-02 08:15:48.000000000","message":"For some reason the last test passes in VolumeAttachPolicyTest but fails in VolumeAttachNoLegacyPolicyTest and in VolumeAttachScopeTypePolicyTest . I\u0027m lost in the policy code so I hope gmann can look at it.","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"188a3380f67e81788a3ad98dcae0e8c2337fde62","unresolved":false,"context_lines":[{"line_number":173,"context_line":"                                 rule_name, self.controller.update,"},{"line_number":174,"context_line":"                                 req, FAKE_UUID,"},{"line_number":175,"context_line":"                                 FAKE_UUID_A, body\u003dbody)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch(\u0027nova.compute.api.API.detach_volume\u0027)"},{"line_number":178,"context_line":"    def test_delete_volume_attach_policy(self, mock_detach_volume):"},{"line_number":179,"context_line":"        rule_name \u003d self.policy_root % \"delete\""}],"source_content_type":"text/x-python","patch_set":11,"id":"df33271e_cb121d62","line":176,"in_reply_to":"df33271e_e538f2d3","updated":"2020-04-02 10:40:32.000000000","message":"Done","commit_id":"45d8b0f18880d48bd1d8b338dbdb5900560159f0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ea3500ea946ca170d598e314ab1fefe081803f6c","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                          mock_swap_volume,"},{"line_number":243,"context_line":"                                                          mock_bdm_save):"},{"line_number":244,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":245,"context_line":"        self.req.environ[\u0027nova.context\u0027].user_id \u003d \u0027other-user\u0027"},{"line_number":246,"context_line":"        self.policy.set_rules({rule_name: \"user_id:%(user_id)s\"})"},{"line_number":247,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":248,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"}],"source_content_type":"text/x-python","patch_set":12,"id":"df33271e_0b62450a","line":245,"updated":"2020-04-02 10:38:59.000000000","message":"You configure this...","commit_id":"efa3c33f575b7912b88525042da795ffcc94159b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"188a3380f67e81788a3ad98dcae0e8c2337fde62","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                          mock_swap_volume,"},{"line_number":243,"context_line":"                                                          mock_bdm_save):"},{"line_number":244,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":245,"context_line":"        self.req.environ[\u0027nova.context\u0027].user_id \u003d \u0027other-user\u0027"},{"line_number":246,"context_line":"        self.policy.set_rules({rule_name: \"user_id:%(user_id)s\"})"},{"line_number":247,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":248,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"}],"source_content_type":"text/x-python","patch_set":12,"id":"df33271e_ee38cfd3","line":245,"in_reply_to":"df33271e_0b62450a","updated":"2020-04-02 10:40:32.000000000","message":"yes, it is invalid in this case.","commit_id":"efa3c33f575b7912b88525042da795ffcc94159b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ea3500ea946ca170d598e314ab1fefe081803f6c","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":245,"context_line":"        self.req.environ[\u0027nova.context\u0027].user_id \u003d \u0027other-user\u0027"},{"line_number":246,"context_line":"        self.policy.set_rules({rule_name: \"user_id:%(user_id)s\"})"},{"line_number":247,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":248,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"},{"line_number":249,"context_line":"            \u0027delete_on_termination\u0027: True}}"},{"line_number":250,"context_line":"        exc \u003d self.assertRaises("}],"source_content_type":"text/x-python","patch_set":12,"id":"df33271e_ab3cf9e0","line":247,"updated":"2020-04-02 10:38:59.000000000","message":"...but you don\u0027t use it. This is using the default context for the fake HTTPRequest [1]. You need to either set \u0027version\u0027 on \u0027self.req\u0027 or configure \u0027environ\u0027 on \u0027req\u0027\n\n[1] https://github.com/openstack/nova/blob/241cffbebafde5ae30c0c330c3f01a327adcb656/nova/tests/unit/api/openstack/fakes.py#L264-L267","commit_id":"efa3c33f575b7912b88525042da795ffcc94159b"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"188a3380f67e81788a3ad98dcae0e8c2337fde62","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":245,"context_line":"        self.req.environ[\u0027nova.context\u0027].user_id \u003d \u0027other-user\u0027"},{"line_number":246,"context_line":"        self.policy.set_rules({rule_name: \"user_id:%(user_id)s\"})"},{"line_number":247,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":248,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"},{"line_number":249,"context_line":"            \u0027delete_on_termination\u0027: True}}"},{"line_number":250,"context_line":"        exc \u003d self.assertRaises("}],"source_content_type":"text/x-python","patch_set":12,"id":"df33271e_4e345bde","line":247,"in_reply_to":"df33271e_ab3cf9e0","updated":"2020-04-02 10:40:32.000000000","message":"Done","commit_id":"efa3c33f575b7912b88525042da795ffcc94159b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"06e6c11194665c57f5220d4bcc6e8fa47538f73d","unresolved":false,"context_lines":[{"line_number":174,"context_line":"                                 req, FAKE_UUID,"},{"line_number":175,"context_line":"                                 FAKE_UUID_A, body\u003dbody)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":178,"context_line":"    def test_update_volume_attach_policy_failed(self, mock_bdm_save):"},{"line_number":179,"context_line":"        rule_name \u003d self.policy_root % \"update\""},{"line_number":180,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":181,"context_line":"        req.environ[\u0027nova.context\u0027] \u003d self.system_foo_context"},{"line_number":182,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {"},{"line_number":183,"context_line":"            \u0027volumeId\u0027: FAKE_UUID_A,"},{"line_number":184,"context_line":"            \u0027delete_on_termination\u0027: True}}"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_ed718746","line":181,"range":{"start_line":177,"start_character":0,"end_line":181,"end_character":61},"updated":"2020-04-07 17:36:58.000000000","message":"this is not needed as it is covered in previous test case where self.system_foo_context is there in  self.admin_or_owner_unauthorized_contexts","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"2e9071608652531880ae4b1051615bc797b406f4","unresolved":false,"context_lines":[{"line_number":174,"context_line":"                                 req, FAKE_UUID,"},{"line_number":175,"context_line":"                                 FAKE_UUID_A, body\u003dbody)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":178,"context_line":"    def test_update_volume_attach_policy_failed(self, mock_bdm_save):"},{"line_number":179,"context_line":"        rule_name \u003d self.policy_root % \"update\""},{"line_number":180,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":181,"context_line":"        req.environ[\u0027nova.context\u0027] \u003d self.system_foo_context"},{"line_number":182,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {"},{"line_number":183,"context_line":"            \u0027volumeId\u0027: FAKE_UUID_A,"},{"line_number":184,"context_line":"            \u0027delete_on_termination\u0027: True}}"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_35128c0a","line":181,"range":{"start_line":177,"start_character":0,"end_line":181,"end_character":61},"in_reply_to":"df33271e_ed718746","updated":"2020-04-08 02:02:15.000000000","message":"Done","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"06e6c11194665c57f5220d4bcc6e8fa47538f73d","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            \"Policy doesn\u0027t allow %s to be performed.\" % rule_name,"},{"line_number":220,"context_line":"            exc.format_message())"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":223,"context_line":"    @mock.patch(\u0027nova.compute.api.API.swap_volume\u0027)"},{"line_number":224,"context_line":"    def test_pass_swap_and_update_volume_attach_policy(self,"},{"line_number":225,"context_line":"                                                       mock_swap_volume,"},{"line_number":226,"context_line":"                                                       mock_bdm_save):"},{"line_number":227,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":228,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":229,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"},{"line_number":230,"context_line":"            \u0027delete_on_termination\u0027: True}}"},{"line_number":231,"context_line":"        self.common_policy_check(self.admin_authorized_contexts,"},{"line_number":232,"context_line":"                                 self.admin_unauthorized_contexts,"},{"line_number":233,"context_line":"                                 rule_name, self.controller.update,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_adc5bf3f","line":230,"range":{"start_line":222,"start_character":0,"end_line":230,"end_character":43},"updated":"2020-04-07 17:36:58.000000000","message":"this scenaio i was talking in API file. this only enforce swap policy.","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"2e9071608652531880ae4b1051615bc797b406f4","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            \"Policy doesn\u0027t allow %s to be performed.\" % rule_name,"},{"line_number":220,"context_line":"            exc.format_message())"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    @mock.patch.object(block_device_obj.BlockDeviceMapping, \u0027save\u0027)"},{"line_number":223,"context_line":"    @mock.patch(\u0027nova.compute.api.API.swap_volume\u0027)"},{"line_number":224,"context_line":"    def test_pass_swap_and_update_volume_attach_policy(self,"},{"line_number":225,"context_line":"                                                       mock_swap_volume,"},{"line_number":226,"context_line":"                                                       mock_bdm_save):"},{"line_number":227,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":228,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":229,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"},{"line_number":230,"context_line":"            \u0027delete_on_termination\u0027: True}}"},{"line_number":231,"context_line":"        self.common_policy_check(self.admin_authorized_contexts,"},{"line_number":232,"context_line":"                                 self.admin_unauthorized_contexts,"},{"line_number":233,"context_line":"                                 rule_name, self.controller.update,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_152b08eb","line":230,"range":{"start_line":222,"start_character":0,"end_line":230,"end_character":43},"in_reply_to":"df33271e_adc5bf3f","updated":"2020-04-08 02:02:15.000000000","message":"Done","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"0a64a8efcbfbd8a974ee870242df19dee9420e2e","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                          mock_bdm_save):"},{"line_number":243,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":244,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":245,"context_line":"        req.environ[\u0027nova.context\u0027] \u003d self.system_foo_context"},{"line_number":246,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"},{"line_number":247,"context_line":"            \u0027delete_on_termination\u0027: True}}"},{"line_number":248,"context_line":"        exc \u003d self.assertRaises("}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_30862867","line":245,"range":{"start_line":245,"start_character":38,"end_line":245,"end_character":61},"updated":"2020-04-07 18:09:42.000000000","message":"this context is always fail in any case. so we cannot guarantee which rule is failing.\n\nwe can cover this scenario in L210 test cases - test_swap_volume_attach_policy_failed\n\nby updating the request body to update the delete_on_termination flag.","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"2e9071608652531880ae4b1051615bc797b406f4","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                          mock_bdm_save):"},{"line_number":243,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":244,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"},{"line_number":245,"context_line":"        req.environ[\u0027nova.context\u0027] \u003d self.system_foo_context"},{"line_number":246,"context_line":"        body \u003d {\u0027volumeAttachment\u0027: {\u0027volumeId\u0027: FAKE_UUID_B,"},{"line_number":247,"context_line":"            \u0027delete_on_termination\u0027: True}}"},{"line_number":248,"context_line":"        exc \u003d self.assertRaises("}],"source_content_type":"text/x-python","patch_set":14,"id":"df33271e_b5ee9cb1","line":245,"range":{"start_line":245,"start_character":38,"end_line":245,"end_character":61},"in_reply_to":"df33271e_30862867","updated":"2020-04-08 02:02:15.000000000","message":"Done","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"337044b696be792b82b9aa3aff267c786a952fbb","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def test_swap_volume_attach_policy_failed(self,"},{"line_number":197,"context_line":"                                              mock_swap_volume,"},{"line_number":198,"context_line":"                                              mock_bdm_save):"},{"line_number":199,"context_line":"        \"\"\"Policy check fails for swap + update due to update policy failture."},{"line_number":200,"context_line":"        \"\"\""},{"line_number":201,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":202,"context_line":"        self.req.environ[\u0027nova.context\u0027].user_id \u003d \u0027other-user\u0027"},{"line_number":203,"context_line":"        self.policy.set_rules({rule_name: \"user_id:%(user_id)s\"})"}],"source_content_type":"text/x-python","patch_set":17,"id":"df33271e_49bb3571","line":200,"range":{"start_line":199,"start_character":1,"end_line":200,"end_character":11},"updated":"2020-04-08 08:00:02.000000000","message":"Does this test really exercise swap + update? Below I see a request body that only change the volume_id, nothing else.\n\nalso with the current implementation there is no way that the swap+update fail due to update policy as in case of swap+update only swap policy is checked.","commit_id":"983a57d4060be59cf883a74979020f608a437ea4"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"aa87776a62ad67362630ac72bdbb7fdc3d295f3e","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def test_swap_volume_attach_policy_failed(self,"},{"line_number":197,"context_line":"                                              mock_swap_volume,"},{"line_number":198,"context_line":"                                              mock_bdm_save):"},{"line_number":199,"context_line":"        \"\"\"Policy check fails for swap + update due to update policy failture."},{"line_number":200,"context_line":"        \"\"\""},{"line_number":201,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":202,"context_line":"        self.req.environ[\u0027nova.context\u0027].user_id \u003d \u0027other-user\u0027"},{"line_number":203,"context_line":"        self.policy.set_rules({rule_name: \"user_id:%(user_id)s\"})"}],"source_content_type":"text/x-python","patch_set":17,"id":"df33271e_445afc29","line":200,"range":{"start_line":199,"start_character":1,"end_line":200,"end_character":11},"in_reply_to":"df33271e_49bb3571","updated":"2020-04-08 08:16:45.000000000","message":"Yes, that I missed the d-o-t and api_version in this case.","commit_id":"983a57d4060be59cf883a74979020f608a437ea4"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1b63fc4cc78765a9f306eed7ee0e56b28d38706c","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def test_swap_volume_attach_policy_failed(self,"},{"line_number":197,"context_line":"                                              mock_swap_volume,"},{"line_number":198,"context_line":"                                              mock_bdm_save):"},{"line_number":199,"context_line":"        \"\"\"Policy check fails for swap + update due to update policy failture."},{"line_number":200,"context_line":"        \"\"\""},{"line_number":201,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":202,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df33271e_846ec493","line":199,"range":{"start_line":199,"start_character":69,"end_line":199,"end_character":77},"updated":"2020-04-08 08:19:42.000000000","message":"failure","commit_id":"ed5dcb9d87045464a5e7d9be4a02a106026f72ff"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"360d3a18ea2d6e6b81bc904ef8fee8124c865955","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def test_swap_volume_attach_policy_failed(self,"},{"line_number":197,"context_line":"                                              mock_swap_volume,"},{"line_number":198,"context_line":"                                              mock_bdm_save):"},{"line_number":199,"context_line":"        \"\"\"Policy check fails for swap + update due to update policy failture."},{"line_number":200,"context_line":"        \"\"\""},{"line_number":201,"context_line":"        rule_name \u003d self.policy_root % \"swap\""},{"line_number":202,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027\u0027, version\u003d\u00272.85\u0027)"}],"source_content_type":"text/x-python","patch_set":18,"id":"df33271e_84452418","line":199,"range":{"start_line":199,"start_character":55,"end_line":199,"end_character":61},"updated":"2020-04-08 08:18:46.000000000","message":"swap","commit_id":"ed5dcb9d87045464a5e7d9be4a02a106026f72ff"}],"releasenotes/notes/separate-update-and-swap-volume-policy-for-attachment-e4c20d4907a52fa7.yaml":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98ad58a4ba62375dbd993642954a2818f301d0c3","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.85, existing policy \u0027os-volumes-attachments:update\u0027"},{"line_number":5,"context_line":"    is used for updating the resources with the change in its default value"},{"line_number":6,"context_line":"    from`` SYSTEM_ADMIN`` to ``PROJECT_MEMBER_OR_SYSTEM_ADMIN``."},{"line_number":7,"context_line":"    New policy \u0027os-volumes-attachments:swap\u0027 is introduced for swapping the"}],"source_content_type":"text/x-yaml","patch_set":14,"id":"df33271e_ca6bc91c","line":4,"range":{"start_line":4,"start_character":44,"end_line":4,"end_character":75},"updated":"2020-04-07 16:58:35.000000000","message":"``literal``","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"2e9071608652531880ae4b1051615bc797b406f4","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.85, existing policy \u0027os-volumes-attachments:update\u0027"},{"line_number":5,"context_line":"    is used for updating the resources with the change in its default value"},{"line_number":6,"context_line":"    from`` SYSTEM_ADMIN`` to ``PROJECT_MEMBER_OR_SYSTEM_ADMIN``."},{"line_number":7,"context_line":"    New policy \u0027os-volumes-attachments:swap\u0027 is introduced for swapping the"}],"source_content_type":"text/x-yaml","patch_set":14,"id":"df33271e_5583d0cc","line":4,"range":{"start_line":4,"start_character":44,"end_line":4,"end_character":75},"in_reply_to":"df33271e_ca6bc91c","updated":"2020-04-08 02:02:15.000000000","message":"Done","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"98ad58a4ba62375dbd993642954a2818f301d0c3","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.85, existing policy \u0027os-volumes-attachments:update\u0027"},{"line_number":5,"context_line":"    is used for updating the resources with the change in its default value"},{"line_number":6,"context_line":"    from`` SYSTEM_ADMIN`` to ``PROJECT_MEMBER_OR_SYSTEM_ADMIN``."},{"line_number":7,"context_line":"    New policy \u0027os-volumes-attachments:swap\u0027 is introduced for swapping the"},{"line_number":8,"context_line":"    attachment of servers with default to ``SYSTEM_ADMIN``."}],"source_content_type":"text/x-yaml","patch_set":14,"id":"df33271e_aa664554","line":6,"range":{"start_line":6,"start_character":8,"end_line":6,"end_character":11},"updated":"2020-04-07 16:58:35.000000000","message":"space before ``","commit_id":"bef88246180b44b20d130446729436444b1add3c"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"2e9071608652531880ae4b1051615bc797b406f4","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    With microversion 2.85, existing policy \u0027os-volumes-attachments:update\u0027"},{"line_number":5,"context_line":"    is used for updating the resources with the change in its default value"},{"line_number":6,"context_line":"    from`` SYSTEM_ADMIN`` to ``PROJECT_MEMBER_OR_SYSTEM_ADMIN``."},{"line_number":7,"context_line":"    New policy \u0027os-volumes-attachments:swap\u0027 is introduced for swapping the"},{"line_number":8,"context_line":"    attachment of servers with default to ``SYSTEM_ADMIN``."}],"source_content_type":"text/x-yaml","patch_set":14,"id":"df33271e_f57b04f1","line":6,"range":{"start_line":6,"start_character":8,"end_line":6,"end_character":11},"in_reply_to":"df33271e_aa664554","updated":"2020-04-08 02:02:15.000000000","message":"Done","commit_id":"bef88246180b44b20d130446729436444b1add3c"}]}