)]}'
{"nova/tests/unit/policies/base.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"80f452f9e80e9d906a43e98eda41fcd83970e91a","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def common_policy_check(self, expected_success_contexts,"},{"line_number":46,"context_line":"                            expected_fail_contexts, rule_name,"},{"line_number":47,"context_line":"                            func, req, *arg, **kwarg):"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        def ensure_raises(req):"},{"line_number":50,"context_line":"            exc \u003d self.assertRaises("},{"line_number":51,"context_line":"                exception.PolicyNotAuthorized, func, req, *arg, **kwarg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_5be18ece","line":48,"updated":"2019-11-21 00:23:43.000000000","message":"I think we should make sure that expected_success_contexts + expected_fail_contexts \u003d full list of known contexts we have above.\n\nThis patch only has the three contexts here, but I think really we should add all the different scopes and roles in this patch, so its very clear what changes in the patches that are comming up.","commit_id":"cd11df037895833b44d8858727d0d64ef0ccc909"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7a5983c2a03f15a2f73e065ed8505895589dfc2f","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def common_policy_check(self, expected_success_contexts,"},{"line_number":46,"context_line":"                            expected_fail_contexts, rule_name,"},{"line_number":47,"context_line":"                            func, req, *arg, **kwarg):"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        def ensure_raises(req):"},{"line_number":50,"context_line":"            exc \u003d self.assertRaises("},{"line_number":51,"context_line":"                exception.PolicyNotAuthorized, func, req, *arg, **kwarg)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_2641f636","line":48,"in_reply_to":"3fa7e38b_5be18ece","updated":"2019-11-22 18:22:00.000000000","message":"done.","commit_id":"cd11df037895833b44d8858727d0d64ef0ccc909"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"41865f51bf53db25b816a2e6a5900949e4fc3b04","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                user_id\u003d\"reader\", roles\u003d[\u0027reader\u0027], system_scope\u003d\u0027all\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self.system_foo_context \u003d nova_context.RequestContext("},{"line_number":53,"context_line":"                user_id\u003d\"foo\", roles\u003d[\u0027foo\u0027], system_scope\u003d\u0027all\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # project scoped users"},{"line_number":56,"context_line":"        self.project_admin_context \u003d nova_context.RequestContext("}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_875f72e3","line":53,"updated":"2019-11-25 17:48:09.000000000","message":"It would be good to get a Keystone core to check this seems valid.","commit_id":"68f195928ee9b43d8e42d28c7309e17af48eb91c"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"37666531bb6c80c9b98b375e5fc1dfbfdcfd0443","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                user_id\u003d\"reader\", roles\u003d[\u0027reader\u0027], system_scope\u003d\u0027all\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self.system_foo_context \u003d nova_context.RequestContext("},{"line_number":53,"context_line":"                user_id\u003d\"foo\", roles\u003d[\u0027foo\u0027], system_scope\u003d\u0027all\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # project scoped users"},{"line_number":56,"context_line":"        self.project_admin_context \u003d nova_context.RequestContext("}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_e717469c","line":53,"in_reply_to":"3fa7e38b_875f72e3","updated":"2019-11-25 18:13:44.000000000","message":"++ this looks correct.\n\nI\u0027ve been thinking about this approach recently. The downside is that it generates *so* many tests (1,000+ for keystone alone).\n\nI\u0027m an advocate for robust testing, but my concern is that it will be a high bar for other projects and ultimately prevent them from developing their policies.\n\nNext week we\u0027re going to discuss a few different testing options during keystone\u0027s office hours (December 3rd).","commit_id":"68f195928ee9b43d8e42d28c7309e17af48eb91c"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"b4f6f646e46018bf6b0003bbc39d7d07db93e9f9","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                user_id\u003d\"reader\", roles\u003d[\u0027reader\u0027], system_scope\u003d\u0027all\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        self.system_foo_context \u003d nova_context.RequestContext("},{"line_number":53,"context_line":"                user_id\u003d\"foo\", roles\u003d[\u0027foo\u0027], system_scope\u003d\u0027all\u0027)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        # project scoped users"},{"line_number":56,"context_line":"        self.project_admin_context \u003d nova_context.RequestContext("}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_19d8b750","line":53,"in_reply_to":"3fa7e38b_e717469c","updated":"2019-11-26 15:51:53.000000000","message":"It looks like with the way the tests use mock to patch out the function call, they won\u0027t be as expensive to run as the keystone tests are.\n\nKeystone also has individual tests for each case (eg. test_system_admin_can_list_users, test_domain_reader_cannot_list_users, test_project_member_cannot_get_user_not_found etc) where these tests put all that functionality under one test, so it seems like the number of tests won\u0027t blow up that much.","commit_id":"68f195928ee9b43d8e42d28c7309e17af48eb91c"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"b4f6f646e46018bf6b0003bbc39d7d07db93e9f9","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        for context in authorized_contexts:"},{"line_number":105,"context_line":"            LOG.info(\"Testing authorized context: %s\", context)"},{"line_number":106,"context_line":"            req.environ[\u0027nova.context\u0027] \u003d context"},{"line_number":107,"context_line":"            func(req, *arg, **kwarg)"},{"line_number":108,"context_line":"        # Verify all the context not having allowed scope or roles fail"},{"line_number":109,"context_line":"        # the policy check."},{"line_number":110,"context_line":"        for context in unauthorized_contexts:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_59b70ffa","line":107,"range":{"start_line":107,"start_character":12,"end_line":107,"end_character":17},"updated":"2019-11-26 15:51:53.000000000","message":"This is a very black-and-white approach, there might be some times when you want to test not just whether it errors or not but the actual result. Maybe those cases can be special cased and handled separately.","commit_id":"68f195928ee9b43d8e42d28c7309e17af48eb91c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8d625b1401d9ff2715cce4232c6fd9c6f90ee691","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        for context in authorized_contexts:"},{"line_number":105,"context_line":"            LOG.info(\"Testing authorized context: %s\", context)"},{"line_number":106,"context_line":"            req.environ[\u0027nova.context\u0027] \u003d context"},{"line_number":107,"context_line":"            func(req, *arg, **kwarg)"},{"line_number":108,"context_line":"        # Verify all the context not having allowed scope or roles fail"},{"line_number":109,"context_line":"        # the policy check."},{"line_number":110,"context_line":"        for context in unauthorized_contexts:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_e1e161ff","line":107,"range":{"start_line":107,"start_character":12,"end_line":107,"end_character":17},"in_reply_to":"3fa7e38b_59b70ffa","updated":"2019-11-26 18:17:19.000000000","message":"+1, yeah we need to write those tests in a different manner. For exmaple when doing the get-all-tenant servers policy change we need to take care of the response based verification instead of just replying on API operation pass.","commit_id":"68f195928ee9b43d8e42d28c7309e17af48eb91c"}],"nova/tests/unit/policies/test_services.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"80f452f9e80e9d906a43e98eda41fcd83970e91a","unresolved":false,"context_lines":[{"line_number":25,"context_line":"        self.controller \u003d services_v21.ServiceController()"},{"line_number":26,"context_line":"        self.req \u003d fakes.HTTPRequest.blank(\u0027/services\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def test_delete_service_policy(self):"},{"line_number":29,"context_line":"        rule_name \u003d \"os_compute_api:os-services\""},{"line_number":30,"context_line":"        # Check that admin is able to delete the service"},{"line_number":31,"context_line":"        success_contexts \u003d [self.legacy_admin_context]"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_3bc7f247","line":28,"updated":"2019-11-21 00:23:43.000000000","message":"Reading ahead, I think we should actually cover all the different types of users you have in this patch (including the extra types of users I request that you add, i.e. \"foo\" role users):\nhttps://review.opendev.org/#/c/648480/18/nova/tests/unit/policies/test_services.py","commit_id":"cd11df037895833b44d8858727d0d64ef0ccc909"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"7a5983c2a03f15a2f73e065ed8505895589dfc2f","unresolved":false,"context_lines":[{"line_number":25,"context_line":"        self.controller \u003d services_v21.ServiceController()"},{"line_number":26,"context_line":"        self.req \u003d fakes.HTTPRequest.blank(\u0027/services\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def test_delete_service_policy(self):"},{"line_number":29,"context_line":"        rule_name \u003d \"os_compute_api:os-services\""},{"line_number":30,"context_line":"        # Check that admin is able to delete the service"},{"line_number":31,"context_line":"        success_contexts \u003d [self.legacy_admin_context]"}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_e678be8f","line":28,"in_reply_to":"3fa7e38b_3bc7f247","updated":"2019-11-22 18:22:00.000000000","message":"done","commit_id":"cd11df037895833b44d8858727d0d64ef0ccc909"},{"author":{"_account_id":8482,"name":"Colleen Murphy","email":"colleen@gazlene.net","username":"krinkle"},"change_message_id":"b4f6f646e46018bf6b0003bbc39d7d07db93e9f9","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        # Check that admin is able to change the service"},{"line_number":35,"context_line":"        self.admin_authorized_contexts \u003d ["},{"line_number":36,"context_line":"            self.legacy_admin_context, self.system_admin_context,"},{"line_number":37,"context_line":"            self.project_admin_context]"},{"line_number":38,"context_line":"        # Check that non-admin is not able to change the service"},{"line_number":39,"context_line":"        self.admin_unauthorized_contexts \u003d ["},{"line_number":40,"context_line":"            self.system_member_context, self.system_reader_context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_b9fac3b7","line":37,"range":{"start_line":37,"start_character":12,"end_line":37,"end_character":38},"updated":"2019-11-26 15:51:53.000000000","message":"When system scope is introduced, this context should no longer be allowed, I guess you\u0027re planning to remove it later?","commit_id":"68f195928ee9b43d8e42d28c7309e17af48eb91c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8d625b1401d9ff2715cce4232c6fd9c6f90ee691","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        # Check that admin is able to change the service"},{"line_number":35,"context_line":"        self.admin_authorized_contexts \u003d ["},{"line_number":36,"context_line":"            self.legacy_admin_context, self.system_admin_context,"},{"line_number":37,"context_line":"            self.project_admin_context]"},{"line_number":38,"context_line":"        # Check that non-admin is not able to change the service"},{"line_number":39,"context_line":"        self.admin_unauthorized_contexts \u003d ["},{"line_number":40,"context_line":"            self.system_member_context, self.system_reader_context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_41b635fc","line":37,"range":{"start_line":37,"start_character":12,"end_line":37,"end_character":38},"in_reply_to":"3fa7e38b_b9fac3b7","updated":"2019-11-26 18:17:19.000000000","message":"yeah, with scope patch we moved these context in the unauthorized list with separate class enabling the oslo scope checks. -https://review.opendev.org/#/c/645427/16/nova/tests/unit/policies/test_services.py","commit_id":"68f195928ee9b43d8e42d28c7309e17af48eb91c"}]}