)]}'
{"nova/api/openstack/compute/plugins/v3/security_groups.py":[{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"dc0cd787f1eea8249d0e3a19e3c7f3d920408c1e","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        if not len(servers):"},{"line_number":436,"context_line":"            return"},{"line_number":437,"context_line":"        key \u003d \"security_groups\""},{"line_number":438,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":439,"context_line":"        if not openstack_driver.is_neutron_security_groups():"},{"line_number":440,"context_line":"            for server in servers:"},{"line_number":441,"context_line":"                instance \u003d req.get_db_instance(server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_a5536015","line":438,"updated":"2015-02-26 08:36:00.000000000","message":"I see _extend_servers is only called by _show and detail, they all do softauth before call  _extend_servers, how about move the softauth inside _extend_servers to reduce the code ?","commit_id":"0e64d4a2e23a75a6e5f95f39df904777472cd5df"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"fa347dabe072e660fedb60d129ef797bca7eb854","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        if not len(servers):"},{"line_number":436,"context_line":"            return"},{"line_number":437,"context_line":"        key \u003d \"security_groups\""},{"line_number":438,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"},{"line_number":439,"context_line":"        if not openstack_driver.is_neutron_security_groups():"},{"line_number":440,"context_line":"            for server in servers:"},{"line_number":441,"context_line":"                instance \u003d req.get_db_instance(server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba7be1f8_1e2519ab","line":438,"in_reply_to":"ba7be1f8_a5536015","updated":"2015-02-27 13:14:53.000000000","message":"yep good proposal","commit_id":"0e64d4a2e23a75a6e5f95f39df904777472cd5df"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"bd75ec68cd4b1ee5d77972ea68b4f2c3e0085c0c","unresolved":false,"context_lines":[{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def _show(self, req, resp_obj):"},{"line_number":472,"context_line":"        if not softauth(req.environ[\u0027nova.context\u0027]):"},{"line_number":473,"context_line":"            return"},{"line_number":474,"context_line":"        if \u0027server\u0027 in resp_obj.obj:"},{"line_number":475,"context_line":"            self._extend_servers(req, [resp_obj.obj[\u0027server\u0027]])"},{"line_number":476,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baa041b7_ba458118","side":"PARENT","line":473,"updated":"2015-04-01 15:40:48.000000000","message":"here?","commit_id":"4103a86f30bf7da3e4616d9d769213b128f0946d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"ca7bff3cc9695dd5cf8a11e5a2b081af054d8313","unresolved":false,"context_lines":[{"line_number":481,"context_line":""},{"line_number":482,"context_line":"    @wsgi.extends"},{"line_number":483,"context_line":"    def create(self, req, resp_obj, body):"},{"line_number":484,"context_line":"        return self._show(req, resp_obj)"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    @wsgi.extends"},{"line_number":487,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_22e1d5fe","line":484,"updated":"2015-04-01 08:01:54.000000000","message":"emm...This fixed another: action create missing soft auth before, just use hard auth, this is wrong. This patch fixed this problem at sametime.","commit_id":"1deff4206445c62c6af9105fdb3dcf4cc493d8f0"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"864685bd41f26bf1e3bc3375fa056cb117def8dc","unresolved":false,"context_lines":[{"line_number":481,"context_line":""},{"line_number":482,"context_line":"    @wsgi.extends"},{"line_number":483,"context_line":"    def create(self, req, resp_obj, body):"},{"line_number":484,"context_line":"        return self._show(req, resp_obj)"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    @wsgi.extends"},{"line_number":487,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa041b7_8f738c4d","line":484,"in_reply_to":"baa041b7_ba5c61ee","updated":"2015-04-02 02:26:00.000000000","message":"oops, you are right, so this patch just something cleanup. so pretty sure it isn\u0027t critical for now.","commit_id":"1deff4206445c62c6af9105fdb3dcf4cc493d8f0"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"bd75ec68cd4b1ee5d77972ea68b4f2c3e0085c0c","unresolved":false,"context_lines":[{"line_number":481,"context_line":""},{"line_number":482,"context_line":"    @wsgi.extends"},{"line_number":483,"context_line":"    def create(self, req, resp_obj, body):"},{"line_number":484,"context_line":"        return self._show(req, resp_obj)"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    @wsgi.extends"},{"line_number":487,"context_line":"    def detail(self, req, resp_obj):"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa041b7_ba5c61ee","line":484,"in_reply_to":"da9b358b_22e1d5fe","updated":"2015-04-01 15:40:48.000000000","message":"no, see left .. seems still have softauth","commit_id":"1deff4206445c62c6af9105fdb3dcf4cc493d8f0"}],"nova/tests/unit/api/openstack/compute/contrib/test_security_groups.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"e9f94d1528f474c63ba422b7c67458f878a78f51","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"    def test_show_policy_failed(self):"},{"line_number":1476,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1477,"context_line":""},{"line_number":1478,"context_line":"    def test_create_policy_failed(self):"},{"line_number":1479,"context_line":"        self.controller.create(self.req, self.fake_res, {})"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aa745d1_fbf2cb99","line":1476,"updated":"2015-04-16 15:33:08.000000000","message":"This test didn\u0027t ensure the policy failed.","commit_id":"f090c2ee4b410515bfca93d649d512bd47e50dee"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"4b332b21f92c560f2f709b4358ca0dc9682eed3e","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"    def test_show_policy_failed(self):"},{"line_number":1476,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1477,"context_line":""},{"line_number":1478,"context_line":"    def test_create_policy_failed(self):"},{"line_number":1479,"context_line":"        self.controller.create(self.req, self.fake_res, {})"}],"source_content_type":"text/x-python","patch_set":4,"id":"dac4157f_b302586e","line":1476,"in_reply_to":"1ab14d0d_3b76173f","updated":"2015-04-20 03:11:02.000000000","message":"If you  think the former test case is not  strict enough, I can improve it as follow, \n\n --- a/nova/tests/unit/api/openstack/compute/contrib/test_security_groups.py\n +++ b/nova/tests/unit/api/openstack/compute/contrib/test_security_groups.py\n @@ -1468,13 +1468,32 @@ class SecurityGroupsOutputPolicyEnforcementV21(test.NoDBTestCase):\n          self.rule_name \u003d \"os_compute_api:os-security-groups\"\n          self.rule \u003d {self.rule_name: \"project:non_fake\"}\n          self.policy.set_rules(self.rule)\n +        self.softauth \u003d secgroups_v21.softauth\n          self.fake_res \u003d wsgi.ResponseObject({\n              \u0027server\u0027: {\u0027id\u0027: \u00270\u0027},\n              \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})\n\n -    def test_show_policy_failed(self):\n +    def wrap_softauth(self, softauth_return):\n +        if softauth_return:\n +            raise Exception(\"softauth is successful\")\n +        raise Exception(\"softauth is failed\")\n +\n +    # make sure following routine of softauth is never executed.\n +    @mock.patch.object(nova.network.security_group.openstack_driver,\n +        \"is_neutron_security_groups\",\n +        side_effect\u003dException(\u0027This should need be called!\u0027))\n +    def test_show_policy_failed(self, _is_neutron_security_groups):\n +        # make sure the failed routine can be executed. I can also assert the return is None. \n          self.controller.show(self.req, self.fake_res, FAKE_UUID1)\n\n +        # make sure the softauth is be called.\n +        self.stubs.Set(secgroups_v21, \"softauth\",\n +                       lambda *arg, **kwarg:\n +                       self.wrap_softauth(self.softauth(*arg, **kwarg)))\n +        exc \u003d self.assertRaises(Exception, self.controller.show, self.req,\n +                                self.fake_res, FAKE_UUID1)\n +        self.assertEqual(\"softauth is failed\", exc.message)\n +\n      def test_create_policy_failed(self):\n         self.controller.create(self.req, self.fake_res, {})","commit_id":"f090c2ee4b410515bfca93d649d512bd47e50dee"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"c75e2da521679caab650224220e47b75b5b401c5","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"    def test_show_policy_failed(self):"},{"line_number":1476,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1477,"context_line":""},{"line_number":1478,"context_line":"    def test_create_policy_failed(self):"},{"line_number":1479,"context_line":"        self.controller.create(self.req, self.fake_res, {})"}],"source_content_type":"text/x-python","patch_set":4,"id":"dac4157f_5b81ce8e","line":1476,"in_reply_to":"1ab14d0d_3b76173f","updated":"2015-04-20 03:20:23.000000000","message":"The test case enhancement as I mentioned is necessary? \nAs the commit says this patch is removing the redundant policy check. The policy check run twice is not necessary, \n I do not change the any logic of this test case, just pass an valid parameter for show/create/detail. \n Actually the first version patch dose not need this change the testcase. https://review.openstack.org/#/c/159369/1/nova/api/openstack/compute/plugins/v3/security_groups.py","commit_id":"f090c2ee4b410515bfca93d649d512bd47e50dee"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"2aef7ff66e37f6cdc72482635deffc9b4ad50356","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"    def test_show_policy_failed(self):"},{"line_number":1476,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1477,"context_line":""},{"line_number":1478,"context_line":"    def test_create_policy_failed(self):"},{"line_number":1479,"context_line":"        self.controller.create(self.req, self.fake_res, {})"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ab14d0d_3b76173f","line":1476,"in_reply_to":"3ab451fb_20c5a68d","updated":"2015-04-17 23:15:22.000000000","message":"This is the scope of this patch. Before this patch, if the policy enforcement failed, the test will failed also. We can assert the return value to make sure the policy enforcement failed.","commit_id":"f090c2ee4b410515bfca93d649d512bd47e50dee"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"827c31fb4d613c2a55c12c9796a29bed9f93db44","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"    def test_show_policy_failed(self):"},{"line_number":1476,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1477,"context_line":""},{"line_number":1478,"context_line":"    def test_create_policy_failed(self):"},{"line_number":1479,"context_line":"        self.controller.create(self.req, self.fake_res, {})"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ab451fb_20c5a68d","line":1476,"in_reply_to":"3ab451fb_faa3ec4d","updated":"2015-04-17 14:31:22.000000000","message":"But ensure the policy failed is not the patch scope, \nI can summit another patch to fix it.","commit_id":"f090c2ee4b410515bfca93d649d512bd47e50dee"},{"author":{"_account_id":14131,"name":"shaohef","email":"shaohe.feng@intel.com","username":"shaohefeng"},"change_message_id":"fa39fb1e956c0ac8722536d608c0883f1ab3d65e","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"    def test_show_policy_failed(self):"},{"line_number":1476,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1477,"context_line":""},{"line_number":1478,"context_line":"    def test_create_policy_failed(self):"},{"line_number":1479,"context_line":"        self.controller.create(self.req, self.fake_res, {})"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ab451fb_faa3ec4d","line":1476,"in_reply_to":"5aa745d1_fbf2cb99","updated":"2015-04-17 14:28:11.000000000","message":"yes. obvious it can not.  Also we discussed this before and agree to keep this way.\n\n So this patch, I do not make any change for it.\n\n But I can do a hack to ensure it if is necessary.  It is easy for me.\n\n two ways: \n 1. to wrap the softauth to raise a Exception. \n 2. to mock some function followed by softauth to raise a Exception.","commit_id":"f090c2ee4b410515bfca93d649d512bd47e50dee"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"0330e1f1a06971f49be2e814b4e4fb90cd409196","unresolved":false,"context_lines":[{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":"    def softauth_side_effect(self, *argv, **wargk):"},{"line_number":1477,"context_line":"        if not self.softauth(*argv, **wargk):"},{"line_number":1478,"context_line":"            raise exception.PolicyNotAuthorized(\"policy check failed\")"},{"line_number":1479,"context_line":""},{"line_number":1480,"context_line":"    # make sure the following routine of softauth is never executed."},{"line_number":1481,"context_line":"    @mock.patch.object(nova.network.security_group.openstack_driver,"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_77bb6b0f","line":1478,"updated":"2015-07-11 21:48:05.000000000","message":"This isn\u0027t the real behavior in the code. This is softauth, it won\u0027t raise exception.","commit_id":"091a1518ee0e70b847154ece302cffdd5e4d7c0d"}],"nova/tests/unit/api/openstack/compute/test_security_groups.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2d8593fcca010e84a47c88f8ba4ec3c071c656c8","unresolved":false,"context_lines":[{"line_number":1411,"context_line":"        self.rule_name \u003d \"os_compute_api:os-security-groups\""},{"line_number":1412,"context_line":"        self.rule \u003d {self.rule_name: \"project:non_fake\"}"},{"line_number":1413,"context_line":"        self.policy.set_rules(self.rule)"},{"line_number":1414,"context_line":"        self.softauth \u003d secgroups_v21.softauth"},{"line_number":1415,"context_line":"        self.fake_res \u003d wsgi.ResponseObject({"},{"line_number":1416,"context_line":"            \u0027server\u0027: {\u0027id\u0027: \u00270\u0027},"},{"line_number":1417,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"}],"source_content_type":"text/x-python","patch_set":6,"id":"da20952f_82f6578d","line":1414,"updated":"2015-09-04 17:14:08.000000000","message":"what does this do?","commit_id":"27ba82ceeadb5c97da78441cfea93d01ada7b365"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"fd92bd4b3d27d565c993dc9dbb11591bd65130d8","unresolved":false,"context_lines":[{"line_number":1411,"context_line":"        self.rule_name \u003d \"os_compute_api:os-security-groups\""},{"line_number":1412,"context_line":"        self.rule \u003d {self.rule_name: \"project:non_fake\"}"},{"line_number":1413,"context_line":"        self.policy.set_rules(self.rule)"},{"line_number":1414,"context_line":"        self.softauth \u003d secgroups_v21.softauth"},{"line_number":1415,"context_line":"        self.fake_res \u003d wsgi.ResponseObject({"},{"line_number":1416,"context_line":"            \u0027server\u0027: {\u0027id\u0027: \u00270\u0027},"},{"line_number":1417,"context_line":"            \u0027servers\u0027: [{\u0027id\u0027: \u00270\u0027}, {\u0027id\u0027: \u00272\u0027}]})"}],"source_content_type":"text/x-python","patch_set":6,"id":"da20952f_6bcf4529","line":1414,"in_reply_to":"da20952f_82f6578d","updated":"2015-09-06 02:21:18.000000000","message":"seems we can remove this one.","commit_id":"27ba82ceeadb5c97da78441cfea93d01ada7b365"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2d8593fcca010e84a47c88f8ba4ec3c071c656c8","unresolved":false,"context_lines":[{"line_number":1420,"context_line":"        \"is_neutron_security_groups\")"},{"line_number":1421,"context_line":"    @mock.patch.object(secgroups_v21, \"softauth\")"},{"line_number":1422,"context_line":"    def test_show_policy_failed(self,"},{"line_number":1423,"context_line":"                                _softauth, is_neutron_security_groups):"},{"line_number":1424,"context_line":"        _softauth.return_value \u003d False"},{"line_number":1425,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1426,"context_line":"        self.assertFalse(is_neutron_security_groups.called)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da20952f_82cf3755","line":1423,"updated":"2015-09-04 17:14:08.000000000","message":"Nit: why _softauth and not just softauth or mock_softauth?","commit_id":"27ba82ceeadb5c97da78441cfea93d01ada7b365"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"fd92bd4b3d27d565c993dc9dbb11591bd65130d8","unresolved":false,"context_lines":[{"line_number":1420,"context_line":"        \"is_neutron_security_groups\")"},{"line_number":1421,"context_line":"    @mock.patch.object(secgroups_v21, \"softauth\")"},{"line_number":1422,"context_line":"    def test_show_policy_failed(self,"},{"line_number":1423,"context_line":"                                _softauth, is_neutron_security_groups):"},{"line_number":1424,"context_line":"        _softauth.return_value \u003d False"},{"line_number":1425,"context_line":"        self.controller.show(self.req, self.fake_res, FAKE_UUID1)"},{"line_number":1426,"context_line":"        self.assertFalse(is_neutron_security_groups.called)"}],"source_content_type":"text/x-python","patch_set":6,"id":"da20952f_4bd921f2","line":1423,"in_reply_to":"da20952f_82cf3755","updated":"2015-09-06 02:21:18.000000000","message":"a mock_ prefix will be better to understand in the test case.","commit_id":"27ba82ceeadb5c97da78441cfea93d01ada7b365"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2d8593fcca010e84a47c88f8ba4ec3c071c656c8","unresolved":false,"context_lines":[{"line_number":1439,"context_line":"    @mock.patch.object(secgroups_v21, \"softauth\")"},{"line_number":1440,"context_line":"    def test_detail_policy_failed(self, _softauth,"},{"line_number":1441,"context_line":"                                  is_neutron_security_groups):"},{"line_number":1442,"context_line":"        _softauth.return_value \u003d False"},{"line_number":1443,"context_line":"        self.controller.detail(self.req, self.fake_res)"},{"line_number":1444,"context_line":"        self.assertFalse(is_neutron_security_groups.called)"},{"line_number":1445,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"da20952f_0231e759","line":1442,"updated":"2015-09-04 17:14:08.000000000","message":"what about checking the positive case?","commit_id":"27ba82ceeadb5c97da78441cfea93d01ada7b365"}]}