)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"b1d5a6659c14aac908e1abeba8dc46c884421fda","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add support for handling multiple error codes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Patrole currently cannot handle the scenario where two possible"},{"line_number":10,"context_line":"error codes can returned by Neutron policy enforcement for a"},{"line_number":11,"context_line":"failed policy check (403 Forbidden and 404 NotFound), depending"},{"line_number":12,"context_line":"on what role is being tested. Patrole framework can only handle"},{"line_number":13,"context_line":"one expected_exception."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_991a207a","line":10,"range":{"start_line":10,"start_character":12,"end_line":10,"end_character":15},"updated":"2018-06-19 18:53:19.000000000","message":"nit: can be","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"b1d5a6659c14aac908e1abeba8dc46c884421fda","unresolved":false,"context_lines":[{"line_number":10,"context_line":"error codes can returned by Neutron policy enforcement for a"},{"line_number":11,"context_line":"failed policy check (403 Forbidden and 404 NotFound), depending"},{"line_number":12,"context_line":"on what role is being tested. Patrole framework can only handle"},{"line_number":13,"context_line":"one expected_exception."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change builds upon the recent multi-policy support to allow"},{"line_number":16,"context_line":"the tester to specify multiple policy actions for one API test."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_811aa66c","line":13,"range":{"start_line":13,"start_character":4,"end_line":13,"end_character":23},"updated":"2018-06-19 18:53:19.000000000","message":"nit: expected exception or \u0027expected_exception\u0027","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"b1d5a6659c14aac908e1abeba8dc46c884421fda","unresolved":false,"context_lines":[{"line_number":12,"context_line":"on what role is being tested. Patrole framework can only handle"},{"line_number":13,"context_line":"one expected_exception."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change builds upon the recent multi-policy support to allow"},{"line_number":16,"context_line":"the tester to specify multiple policy actions for one API test."},{"line_number":17,"context_line":"For each policy action, the tester would need to specify an"},{"line_number":18,"context_line":"error code that is expected if the action should fail. If multiple"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_2158722f","line":15,"range":{"start_line":15,"start_character":55,"end_line":15,"end_character":56},"updated":"2018-06-19 18:53:19.000000000","message":"[0]","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"b1d5a6659c14aac908e1abeba8dc46c884421fda","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change builds upon the recent multi-policy support to allow"},{"line_number":16,"context_line":"the tester to specify multiple policy actions for one API test."},{"line_number":17,"context_line":"For each policy action, the tester would need to specify an"},{"line_number":18,"context_line":"error code that is expected if the action should fail. If multiple"},{"line_number":19,"context_line":"policy actions fail, the error code for the first policy action"},{"line_number":20,"context_line":"that fails will be expected to be returned from the service."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_c1673eea","line":17,"range":{"start_line":17,"start_character":28,"end_line":17,"end_character":56},"updated":"2018-06-19 18:53:19.000000000","message":"test specifies","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"b1d5a6659c14aac908e1abeba8dc46c884421fda","unresolved":false,"context_lines":[{"line_number":15,"context_line":"This change builds upon the recent multi-policy support to allow"},{"line_number":16,"context_line":"the tester to specify multiple policy actions for one API test."},{"line_number":17,"context_line":"For each policy action, the tester would need to specify an"},{"line_number":18,"context_line":"error code that is expected if the action should fail. If multiple"},{"line_number":19,"context_line":"policy actions fail, the error code for the first policy action"},{"line_number":20,"context_line":"that fails will be expected to be returned from the service."},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_4199eecd","line":18,"range":{"start_line":18,"start_character":42,"end_line":18,"end_character":53},"updated":"2018-06-19 18:53:19.000000000","message":"fails","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"b1d5a6659c14aac908e1abeba8dc46c884421fda","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This handles the cases in Neutron where Neutron may use a second"},{"line_number":23,"context_line":"policy rule to determine whether or not to return a 403 error"},{"line_number":24,"context_line":"code or a 404 error code. The tester is expected to list out"},{"line_number":25,"context_line":"which policy rules are being tested by the API endpoint test."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I5cd861e184da90bb27f8ba454c94fa4d4f99c269"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_81b6263e","line":24,"range":{"start_line":24,"start_character":26,"end_line":24,"end_character":56},"updated":"2018-06-19 18:53:19.000000000","message":"It is expected that the test lists","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":17896,"name":"Rick Bartra","email":"rickbartra@microsoft.com","username":"rb560u"},"change_message_id":"b1d5a6659c14aac908e1abeba8dc46c884421fda","unresolved":false,"context_lines":[{"line_number":23,"context_line":"policy rule to determine whether or not to return a 403 error"},{"line_number":24,"context_line":"code or a 404 error code. The tester is expected to list out"},{"line_number":25,"context_line":"which policy rules are being tested by the API endpoint test."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I5cd861e184da90bb27f8ba454c94fa4d4f99c269"},{"line_number":28,"context_line":"Closes-Bug: #1772710"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"5f7c97a3_e1391acf","line":26,"updated":"2018-06-19 18:53:19.000000000","message":"[0] https://review.openstack.org/#/c/554764/","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"}],"patrole_tempest_plugin/config.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"5c8e4073d3153adb5811b32629c7226f08d9e030","unresolved":false,"context_lines":[{"line_number":88,"context_line":"             - add_image"},{"line_number":89,"context_line":"allowed_role \u003d the Keystone role that is allowed to perform the API"},{"line_number":90,"context_line":"\"\"\"),"},{"line_number":91,"context_line":"    cfg.StrOpt(\u0027expected_result_overrides_file\u0027,"},{"line_number":92,"context_line":"               default\u003dNone,"},{"line_number":93,"context_line":"               help\u003d\"\"\""},{"line_number":94,"context_line":"File path of the json file that defines the policy actions that are"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_955f3240","line":91,"updated":"2018-05-23 23:26:43.000000000","message":"General commentary regarding the design: If we want a requirements-driven approach for this, then there\u0027s a better place to include this information. I\u0027m thinking here: https://github.com/openstack/patrole/blob/1f2ba24f4df09ba3711aa520bdf1bedb63e7797a/patrole_tempest_plugin/config.py#L71\n\nThis requirements file is already being used for our downstream use case. It can be modified to support something like:\n\n```\nneutron:\n  \u003capi_action\u003e:\n    - role: \u003callowed_role\u003e\n       expected_error_codes:\n         PUT: 403\n         GET: 404\n         ...\n    - role: \u003callowed_role_2\u003e\n       expected_error_codes:\n         PUT: 404\n         GET: 404\n         ...\n  \u003capi_action_2\u003e:\n    - \u003callowed_role\u003e\n    - \u003callowed_role\u003e\n\u003cservice_2\u003e\n  \u003capi_action\u003e:\n    - role: \u003callowed_role\u003e\n```\n\nThis would require refactoring the requirements file downstream though. (Personally, I think YAML anchors/pointers should be used to vastly decrease the number of lines of code in the requirements file, as an aside.)\n\nTo avoid refactoring, you could do something like this:\n\n```\n\u003cservice_1\u003e:\n  \u003capi_action\u003e:\n    - \u003callowed_role\u003e\n    - \u003callowed_role\u003e\n    - \u003callowed_role\u003e\n  \u003capi_action\u003e:\n    - \u003callowed_role\u003e\n    - \u003callowed_role\u003e\n\u003cservice_2\u003e:\n  \u003capi_action\u003e:\n    - \u003callowed_role\u003e\n---\nexpected_error_codes:\n   neutron:\n      \u003capi_action\u003e:\n         GET: 404\n         PUT: 403\n      \u003capi_action_2\u003e:\n         ...\n```\n\nThen when you read in the requirements file you use yaml.safe_load_all() and the second item is the expected error codes dictionary in memory.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"98a05c1c3ac64f3e56af8431b2c8b202dadfb036","unresolved":false,"context_lines":[{"line_number":88,"context_line":"             - add_image"},{"line_number":89,"context_line":"allowed_role \u003d the Keystone role that is allowed to perform the API"},{"line_number":90,"context_line":"\"\"\"),"},{"line_number":91,"context_line":"    cfg.StrOpt(\u0027expected_result_overrides_file\u0027,"},{"line_number":92,"context_line":"               default\u003dNone,"},{"line_number":93,"context_line":"               help\u003d\"\"\""},{"line_number":94,"context_line":"File path of the json file that defines the policy actions that are"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_2bb324eb","line":91,"in_reply_to":"5f7c97a3_955f3240","updated":"2018-05-29 14:53:46.000000000","message":"Thanks for the suggestions. I like the idea of using the custom requirements file for this purpose. Let me think about which of your suggestions is better to implement.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"5c8e4073d3153adb5811b32629c7226f08d9e030","unresolved":false,"context_lines":[{"line_number":108,"context_line":"```"},{"line_number":109,"context_line":"["},{"line_number":110,"context_line":"    {"},{"line_number":111,"context_line":"        \"service\": \"neutron\","},{"line_number":112,"context_line":"        \"expected_result_overrides\":"},{"line_number":113,"context_line":"            ["},{"line_number":114,"context_line":"                {\"role\": \"role1\","}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_d5e92a80","line":111,"updated":"2018-05-23 23:26:43.000000000","message":"This means that the default behavior won\u0027t be validated unless we add a file like this. Patrole needs to be able to validate the default behavior.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"491c613dd6057b0369f4576de9dc8a7c431452c4","unresolved":false,"context_lines":[{"line_number":108,"context_line":"```"},{"line_number":109,"context_line":"["},{"line_number":110,"context_line":"    {"},{"line_number":111,"context_line":"        \"service\": \"neutron\","},{"line_number":112,"context_line":"        \"expected_result_overrides\":"},{"line_number":113,"context_line":"            ["},{"line_number":114,"context_line":"                {\"role\": \"role1\","}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_f773ce4c","line":111,"in_reply_to":"5f7c97a3_6b303c57","updated":"2018-05-29 19:09:44.000000000","message":"I\u0027d have to validate that because Member/admin default behavior leaves testing gaps. This may be why the issue you\u0027re seeing downstream currently doesn\u0027t matter.\n\nNonetheless Patrole currently supports:\n\n* requirements-driven approach to testing\n* oslo.policy-driven approach to testing\n\nShould be able to implement the details for either one separately.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"98a05c1c3ac64f3e56af8431b2c8b202dadfb036","unresolved":false,"context_lines":[{"line_number":108,"context_line":"```"},{"line_number":109,"context_line":"["},{"line_number":110,"context_line":"    {"},{"line_number":111,"context_line":"        \"service\": \"neutron\","},{"line_number":112,"context_line":"        \"expected_result_overrides\":"},{"line_number":113,"context_line":"            ["},{"line_number":114,"context_line":"                {\"role\": \"role1\","}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_6b303c57","line":111,"in_reply_to":"5f7c97a3_d5e92a80","updated":"2018-05-29 14:53:46.000000000","message":"Actually, we don\u0027t need this file for validating default behavior.  Without this file, Patrole would run exactly as it does today. This file just allows the tester to customize/override the expected error codes for only the tests that need it.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"5c8e4073d3153adb5811b32629c7226f08d9e030","unresolved":false,"context_lines":[{"line_number":109,"context_line":"["},{"line_number":110,"context_line":"    {"},{"line_number":111,"context_line":"        \"service\": \"neutron\","},{"line_number":112,"context_line":"        \"expected_result_overrides\":"},{"line_number":113,"context_line":"            ["},{"line_number":114,"context_line":"                {\"role\": \"role1\","},{"line_number":115,"context_line":"                 \"action_list\":"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_1531e20b","line":112,"updated":"2018-05-23 23:26:43.000000000","message":"This renders the expected_error_code kwarg in rbac_rule_validation redundant as well.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"98a05c1c3ac64f3e56af8431b2c8b202dadfb036","unresolved":false,"context_lines":[{"line_number":109,"context_line":"["},{"line_number":110,"context_line":"    {"},{"line_number":111,"context_line":"        \"service\": \"neutron\","},{"line_number":112,"context_line":"        \"expected_result_overrides\":"},{"line_number":113,"context_line":"            ["},{"line_number":114,"context_line":"                {\"role\": \"role1\","},{"line_number":115,"context_line":"                 \"action_list\":"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_6b7efce4","line":112,"in_reply_to":"5f7c97a3_1531e20b","updated":"2018-05-29 14:53:46.000000000","message":"In this design, the expected_error_code would be overridden by the value in this file, but only if the action being validated is contained in the file.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"491c613dd6057b0369f4576de9dc8a7c431452c4","unresolved":false,"context_lines":[{"line_number":109,"context_line":"["},{"line_number":110,"context_line":"    {"},{"line_number":111,"context_line":"        \"service\": \"neutron\","},{"line_number":112,"context_line":"        \"expected_result_overrides\":"},{"line_number":113,"context_line":"            ["},{"line_number":114,"context_line":"                {\"role\": \"role1\","},{"line_number":115,"context_line":"                 \"action_list\":"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_979a9a5f","line":112,"in_reply_to":"5f7c97a3_6b7efce4","updated":"2018-05-29 19:09:44.000000000","message":"But I think both should use a similar interface for maintenance reasons. Because supporting expected_error_code and expected_result_overrides_file is not intuitive to someone not well familiarized with the code base.\n\nCould just rename both to expected_error_codes and support both behaviors to simplify interface.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"ddac91332628ee75f9fafeb60937eff9502e6b4f","unresolved":false,"context_lines":[{"line_number":88,"context_line":"  \u003cservice\u003e:"},{"line_number":89,"context_line":"    - role: \u003cdisallowed_role\u003e"},{"line_number":90,"context_line":"      action_list:"},{"line_number":91,"context_line":"        - action: \u003capi_action\u003e"},{"line_number":92,"context_line":"          error: \u003cerror_code_expected\u003e"},{"line_number":93,"context_line":"        - action: \u003capi_action\u003e"},{"line_number":94,"context_line":"          error: \u003cerror_code_expected\u003e"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_4fa68cc3","line":91,"range":{"start_line":91,"start_character":10,"end_line":91,"end_character":16},"updated":"2018-05-30 22:41:48.000000000","message":"1) What does \"api_action\" mean? Is that the name of a policy? Or is it an HTTP verb (i.e. GET, PUT, etc.)?\n\n2) (Assuming that \"api_action\" is just a policy name): I don\u0027t think this will always work because this assumes policy granularity. For example, take a policy action called \"foo\" which is used for GET, PUT, DELETE, etc. Now if role \"_member_\" cannot execute GET and it throws a 403 and if role \"_member_\" cannot execute PUT and it throws a 404 (and both GET and PUT enforce \"foo\") then this means that your entries here will look like:\n\n  - role: \"_member_\"\n    action_list:\n      - action: \"foo\"\n         error: 403\n      - action: \"foo\"\n         error: 404\n\nWhich means you have ambiguity and you don\u0027t know which one to pick.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"5e02a642aa403316926467ca98b12a8a40ca4729","unresolved":false,"context_lines":[{"line_number":88,"context_line":"  \u003cservice\u003e:"},{"line_number":89,"context_line":"    - role: \u003cdisallowed_role\u003e"},{"line_number":90,"context_line":"      action_list:"},{"line_number":91,"context_line":"        - action: \u003capi_action\u003e"},{"line_number":92,"context_line":"          error: \u003cerror_code_expected\u003e"},{"line_number":93,"context_line":"        - action: \u003capi_action\u003e"},{"line_number":94,"context_line":"          error: \u003cerror_code_expected\u003e"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_c4da7549","line":91,"range":{"start_line":91,"start_character":10,"end_line":91,"end_character":16},"in_reply_to":"5f7c97a3_4fa68cc3","updated":"2018-05-31 14:15:22.000000000","message":"1) api_action is described on line 105: it is the policy action being tested, for example in Neutron: \"create_network\".\n\n2) You are calling \"foo\" an action, but isn\u0027t it actually a resource in your example? And actions typically take the form of \"create_foo\", \"update_foo\", \"delete_foo\", or \"get_foo\", which removes the ambiguity that you\u0027re talking about.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"4a5e4d63dac6027df8ada24935160555e87024a0","unresolved":false,"context_lines":[{"line_number":63,"context_line":"\"\"\"),"},{"line_number":64,"context_line":"    cfg.StrOpt(\u0027custom_requirements_file\u0027,"},{"line_number":65,"context_line":"               help\u003d\"\"\""},{"line_number":66,"context_line":"\u003c\u003c\u003c\u003c\u003c\u003c\u003c c9bc9db4b45b8112cee9d8e06304e620d0a5c13c"},{"line_number":67,"context_line":"File path of the YAML file that defines your RBAC requirements. This"},{"line_number":68,"context_line":"file must be located on the same host that Patrole runs on. The file"},{"line_number":69,"context_line":"specifies the allowed roles for each of the API actions defined for"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_f1d516e7","line":66,"range":{"start_line":66,"start_character":0,"end_line":66,"end_character":48},"updated":"2018-06-12 03:00:03.000000000","message":"Rebase error here","commit_id":"21450cd69fc2e295ebe49a0cc904ac1c90d61256"}],"patrole_tempest_plugin/rbac_rule_validation.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"5c8e4073d3153adb5811b32629c7226f08d9e030","unresolved":false,"context_lines":[{"line_number":341,"context_line":"                            action_overrides \u003d role_overrides[\u0027action_list\u0027]"},{"line_number":342,"context_line":"                            for override in action_overrides:"},{"line_number":343,"context_line":"                                if override[\u0027action\u0027] \u003d\u003d action:"},{"line_number":344,"context_line":"                                    return override[\u0027exp_error\u0027]"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    return None"},{"line_number":347,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_35989e19","line":344,"range":{"start_line":344,"start_character":36,"end_line":344,"end_character":64},"updated":"2018-05-23 23:26:43.000000000","message":"This sort of thing is dangerous because we don\u0027t ever want to support anything that isn\u0027t in (403, 404) or else raise an exception.","commit_id":"d14d2cd6aff2aced8c37f38429f91775d8026330"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"2ca0f39f4160d0d8eeee07e2463baebe0b716b3d","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            A 404 should not be provided *unless* the endpoint masks a"},{"line_number":91,"context_line":"            ``Forbidden`` exception as a ``NotFound`` exception."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    :param list expected_error_code_all_fail: When the rules list parameter is"},{"line_number":94,"context_line":"        used, then this is the error code to expect only if all of the rules"},{"line_number":95,"context_line":"        do not allow the role being tested."},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_20147f4c","line":93,"range":{"start_line":93,"start_character":11,"end_line":93,"end_character":15},"updated":"2018-06-12 21:36:05.000000000","message":"int","commit_id":"7032592692f57e72d44c465b2307feb4d252cee2"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"2ca0f39f4160d0d8eeee07e2463baebe0b716b3d","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                    disallowed_rules.append(rule)"},{"line_number":146,"context_line":"                allowed \u003d allowed and _allowed"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if not allowed and len(disallowed_rules) \u003d\u003d len(rules):"},{"line_number":149,"context_line":"                exp_error_code \u003d expected_error_code_all_fail"},{"line_number":150,"context_line":"            else:"},{"line_number":151,"context_line":"                exp_error_code \u003d expected_error_code"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_c00eeb39","line":148,"updated":"2018-06-12 21:36:05.000000000","message":"Currently Proposed Idea\n--------------------\n\nSeems like you are accounting for a 403 if one of the rules fails but a 404 if both of the rules fail. However, I\u0027m not convinced this is correct behavior because what if ONLY \"get_router\" fails -- shouldn\u0027t this be a 404?\n\nAlternative Idea\n--------------------\n\nDetermine the expected error code based off whether the FIRST policy (\"rule\") that fails. This requires that the policies in \"rules\" be enumerated in precisely the order in which they\u0027re enforced server side.\n\nChange:\n\n    def action(service, rule\u003d\u0027\u0027, rules\u003dNone, expected_error_codes\u003d[403],\n               extra_target_data\u003dNone):\n\nWould it be possible to enhance expected_error_code to do this (assuming the \"get_router\" is enforced prior to \"add_router_interface\"):\n\n    expected_error_codes \u003d [404, 403]\n\nTo coincide precisely with the order of the rules:\n\n    rules \u003d [\"get_router\", \"add_router_interface\"]\n\nWhich implies this:\n\n    \"add_router_interface\" \u003d\u003e 403 upon failure\n    \"get_router\" \u003d\u003e 404 upon failure\n\nMaking the following possible (perhaps):\n\n    # Not 100% sure how to best handle this here...\n    if len(rules) !\u003dlen(expected_error_codes):\n        expected_error_codes \u003d [expected_error_codes[0] * len(rules)]\n\n    exp_error_code \u003d None\n    if disallowed_rules:\n        first_error_idx \u003d rules.index(disallowed_rules[0])\n        exp_error_code \u003d expected_error_code[first_error_idx]\n\n    # Need better logging here...\n    for idx, rule in enumerate(rules):\n        LOG.debug(\"%s: Expecting %d be raised for policy name: %s\", test_func.__name__, expected_error_codes[idx], rule)","commit_id":"7032592692f57e72d44c465b2307feb4d252cee2"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"211f9c7c78401fb6e6c3a81da8dc5ae024912ed8","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                    disallowed_rules.append(rule)"},{"line_number":146,"context_line":"                allowed \u003d allowed and _allowed"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if not allowed and len(disallowed_rules) \u003d\u003d len(rules):"},{"line_number":149,"context_line":"                exp_error_code \u003d expected_error_code_all_fail"},{"line_number":150,"context_line":"            else:"},{"line_number":151,"context_line":"                exp_error_code \u003d expected_error_code"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_1b0e968b","line":148,"in_reply_to":"5f7c97a3_bb3f6223","updated":"2018-06-12 21:42:19.000000000","message":"4) \"add_router_interface\" passes, \"get_router\" passes \u003d\u003e 2xx","commit_id":"7032592692f57e72d44c465b2307feb4d252cee2"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"e13e110176206b81d0c968c07f781b73368cfc72","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                    disallowed_rules.append(rule)"},{"line_number":146,"context_line":"                allowed \u003d allowed and _allowed"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            if not allowed and len(disallowed_rules) \u003d\u003d len(rules):"},{"line_number":149,"context_line":"                exp_error_code \u003d expected_error_code_all_fail"},{"line_number":150,"context_line":"            else:"},{"line_number":151,"context_line":"                exp_error_code \u003d expected_error_code"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_bb3f6223","line":148,"in_reply_to":"5f7c97a3_c00eeb39","updated":"2018-06-12 21:41:26.000000000","message":"Further elaboration (for pedantry):\n\nThere are 3 cases in the above example:\n1) \"add_router_interface\" passes, \"get_router\" fails \u003d\u003e 404\n2) \"add_router_interface\" fails, \"get_router\" passes \u003d\u003e 403\n3) \"add_router_interface\" fails, \"get_router\" fails \u003d\u003e 404 (I assume \"get_router\" is enforced first)","commit_id":"7032592692f57e72d44c465b2307feb4d252cee2"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a2db28e88579c89fb0675d863c94ab8c25393b34","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    :param list expected_error_codes: When the ``rules`` list parameter is"},{"line_number":96,"context_line":"        used, then this list indicates the expected error code to use if one"},{"line_number":97,"context_line":"        of the rules does not allow the role being tested. This list must"},{"line_number":98,"context_line":"        coincide and its elements remain in the same order as the rules in the"},{"line_number":99,"context_line":"        rules list. Example:"},{"line_number":100,"context_line":"            rules\u003d[\"api_action1\", \"api_action2\"]"},{"line_number":101,"context_line":"            expected_error_codes\u003d[404, 403]"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_31f6c592","line":98,"range":{"start_line":98,"start_character":16,"end_line":98,"end_character":61},"updated":"2018-06-15 22:21:19.000000000","message":"with, and its elements remain the same order as,","commit_id":"6d6ec5460b2270ac58c7d1a9828c30abd5a49362"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a2db28e88579c89fb0675d863c94ab8c25393b34","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        used, then this list indicates the expected error code to use if one"},{"line_number":97,"context_line":"        of the rules does not allow the role being tested. This list must"},{"line_number":98,"context_line":"        coincide and its elements remain in the same order as the rules in the"},{"line_number":99,"context_line":"        rules list. Example:"},{"line_number":100,"context_line":"            rules\u003d[\"api_action1\", \"api_action2\"]"},{"line_number":101,"context_line":"            expected_error_codes\u003d[404, 403]"},{"line_number":102,"context_line":"        a) If api_action1 fails and api_action2 passes, then the expected"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_51f341a0","line":99,"range":{"start_line":99,"start_character":27,"end_line":99,"end_character":28},"updated":"2018-06-15 22:21:19.000000000","message":"::","commit_id":"6d6ec5460b2270ac58c7d1a9828c30abd5a49362"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a2db28e88579c89fb0675d863c94ab8c25393b34","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        coincide and its elements remain in the same order as the rules in the"},{"line_number":99,"context_line":"        rules list. Example:"},{"line_number":100,"context_line":"            rules\u003d[\"api_action1\", \"api_action2\"]"},{"line_number":101,"context_line":"            expected_error_codes\u003d[404, 403]"},{"line_number":102,"context_line":"        a) If api_action1 fails and api_action2 passes, then the expected"},{"line_number":103,"context_line":"           error code is 404."},{"line_number":104,"context_line":"        b) if api_action2 fails and api_action1 passes, then the expected"}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_f1efedae","line":101,"updated":"2018-06-15 22:21:19.000000000","message":"Add new lines before and after the code example to render properly.","commit_id":"6d6ec5460b2270ac58c7d1a9828c30abd5a49362"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a2db28e88579c89fb0675d863c94ab8c25393b34","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        else:"},{"line_number":262,"context_line":"            exp_error_codes.append(exp_error_code)"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    # Make sure there\u0027s an error code for each rule. If not, then add the"},{"line_number":265,"context_line":"    # default error code for each rule that doesn\u0027t have one."},{"line_number":266,"context_line":"    num_err_codes \u003d len(exp_error_codes)"},{"line_number":267,"context_line":"    for i in range(len(rules)):"},{"line_number":268,"context_line":"        if num_err_codes \u003c\u003d i:"},{"line_number":269,"context_line":"            exp_error_codes.append(_DEFAULT_ERROR_CODE)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    return rules, exp_error_codes"},{"line_number":272,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"5f7c97a3_3f6a0b81","line":269,"range":{"start_line":264,"start_character":0,"end_line":269,"end_character":55},"updated":"2018-06-15 22:21:19.000000000","message":"I think there\u0027s 2 cases:\n\n1) exp_error_codes is empty \u003d\u003e default to 403 for backward compatibility (also so that we don\u0027t have to modify every single test currently in Patrole)\n2) if len(exp_error_codes) !\u003d len(rules) \u003d\u003e raise exception (exception could be RuntimeError or something else more nuanced). Adding 403 automatically to the exp_error_codes can make it harder to pin down bugs w.r.t improper usage of the decorator.","commit_id":"6d6ec5460b2270ac58c7d1a9828c30abd5a49362"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"0ff427b436272f845a6b6a821a85bc819663ea18","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"            Patrole currently only supports custom JSON policy files."},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    :param int expected_error_code: Overrides default value of 403 (Forbidden)"},{"line_number":86,"context_line":"        with endpoint-specific error code. Currently only supports 403 and 404."},{"line_number":87,"context_line":"        Support for 404 is needed because some services, like Neutron,"},{"line_number":88,"context_line":"        intentionally throw a 404 for security reasons."}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_d5563dcf","line":85,"range":{"start_line":85,"start_character":36,"end_line":85,"end_character":45},"updated":"2018-06-18 16:40:45.000000000","message":"nit: Can prefix here with (DEPRECATED). I.e.:\n\n    :param int expected_error_code: (DEPRECATED) Overrides ...","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"0ff427b436272f845a6b6a821a85bc819663ea18","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        coincide with and its elements remain in the same order as the rules"},{"line_number":99,"context_line":"        in the rules list."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        Example::"},{"line_number":102,"context_line":"            rules\u003d[\"api_action1\", \"api_action2\"]"},{"line_number":103,"context_line":"            expected_error_codes\u003d[404, 403]"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_b5bf01fb","line":101,"updated":"2018-06-18 16:40:45.000000000","message":"nit: Missing newline after Example:: needed to render code block correctly. You can see here by searching for it: http://logs.openstack.org/62/570262/29/check/build-openstack-sphinx-docs/d0633c7/html/framework/rbac_validation.html\n\nThis is a trivial issue and we can fix it later.","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"0ff427b436272f845a6b6a821a85bc819663ea18","unresolved":false,"context_lines":[{"line_number":279,"context_line":"    # where no exp_error_codes array is provided, so the error codes must be"},{"line_number":280,"context_line":"    # set to the default error code value and there must be the same number"},{"line_number":281,"context_line":"    # of error codes as rules."},{"line_number":282,"context_line":"    num_ecs \u003d len(exp_error_codes)"},{"line_number":283,"context_line":"    num_rules \u003d len(rules)"},{"line_number":284,"context_line":"    if (num_ecs \u003c num_rules):"},{"line_number":285,"context_line":"        for i in range(num_rules - num_ecs):"},{"line_number":286,"context_line":"            exp_error_codes.append(_DEFAULT_ERROR_CODE)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    return rules, exp_error_codes"},{"line_number":289,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"5f7c97a3_b5e061fa","line":286,"range":{"start_line":282,"start_character":0,"end_line":286,"end_character":55},"updated":"2018-06-18 16:40:45.000000000","message":"I think this is OK. I think that having to explicitly declare that the expected_error_codes for anything that does multi-policy validation in Nova as [403, 403, ..., 403] is overkill as everything except Neutron even uses 404.\n\nMy one concern was doing something like this:\n\n    @rbac_rule_validation.action(service\u003d\"neutron\",\n                                 rules\u003d[\"get_router\", \"add_router_interface\", \"another_random_policy\"],\n                                 expected_error_codes\u003d[404, 403])\n     # note that another_random_policy defaults to 403 when theoretically it could be 404\n\nBut I think we will simply have to rely on some degree of scenario testing in the future to properly test policy gaps w.r.t to multiple policies.","commit_id":"35a77113fccfd5659c123adb2cc142836cdd764d"}],"patrole_tempest_plugin/requirements_authority.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"ddac91332628ee75f9fafeb60937eff9502e6b4f","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    @staticmethod"},{"line_number":42,"context_line":"    def parse(component):"},{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            roles_dict \u003d None"},{"line_number":45,"context_line":"            ec_dict \u003d None"},{"line_number":46,"context_line":"            rbac_map \u003d RequirementsParser.Inner._rbac_map"},{"line_number":47,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_8feca4bb","line":44,"updated":"2018-05-30 22:41:48.000000000","message":"Could verify that api_action in rbac_map[1][\u0027expected_error_codes\u0027][component] is present in rbac_map[0][component].\n\nIdeally, this requirements file should be validated using a schema in the future.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"5e02a642aa403316926467ca98b12a8a40ca4729","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    @staticmethod"},{"line_number":42,"context_line":"    def parse(component):"},{"line_number":43,"context_line":"        try:"},{"line_number":44,"context_line":"            roles_dict \u003d None"},{"line_number":45,"context_line":"            ec_dict \u003d None"},{"line_number":46,"context_line":"            rbac_map \u003d RequirementsParser.Inner._rbac_map"},{"line_number":47,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_64704141","line":44,"in_reply_to":"5f7c97a3_8feca4bb","updated":"2018-05-31 14:15:22.000000000","message":"I guess you mean I should verify that there is at least one action listed for the component.  That\u0027s fair, I can add that check.\n\nAgreed, the file should be validated against a schema. I\u0027m not sure when or where that validation should take place though.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"ddac91332628ee75f9fafeb60937eff9502e6b4f","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def get_expected_error(self, role, rule):"},{"line_number":87,"context_line":"        \"\"\"Returns the error code expected for the given role being tested"},{"line_number":88,"context_line":"        against the given rule(action)."},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        :param role: the role being tested"},{"line_number":91,"context_line":"        :param rule: the rule (action) being performed by the test"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_0f365452","line":88,"range":{"start_line":88,"start_character":31,"end_line":88,"end_character":37},"updated":"2018-05-30 22:41:48.000000000","message":"Yeah this nomenclature is really old and we haven\u0027t updated yet because the rbac_rule_validation decorator itself is called action which results in some shadowing so we\u0027d have to rename things across the board carefully at some point.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"5e02a642aa403316926467ca98b12a8a40ca4729","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def get_expected_error(self, role, rule):"},{"line_number":87,"context_line":"        \"\"\"Returns the error code expected for the given role being tested"},{"line_number":88,"context_line":"        against the given rule(action)."},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        :param role: the role being tested"},{"line_number":91,"context_line":"        :param rule: the rule (action) being performed by the test"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_c496f53e","line":88,"range":{"start_line":88,"start_character":31,"end_line":88,"end_character":37},"in_reply_to":"5f7c97a3_0f365452","updated":"2018-05-31 14:15:22.000000000","message":"yes, it bugs me, too.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"}],"patrole_tempest_plugin/tests/api/network/test_routers_rbac.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"2ca0f39f4160d0d8eeee07e2463baebe0b716b3d","unresolved":false,"context_lines":[{"line_number":330,"context_line":"            self.routers_client.delete_router(router[\u0027id\u0027])"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":333,"context_line":"                                 rules\u003d[\"add_router_interface\","},{"line_number":334,"context_line":"                                        \"get_router\"],"},{"line_number":335,"context_line":"                                 expected_error_code_all_fail\u003d404)"},{"line_number":336,"context_line":"    @decorators.idempotent_id(\u0027a0627778-d68d-4913-881b-e345360cca19\u0027)"},{"line_number":337,"context_line":"    def test_add_router_interface(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_4015fb9a","line":334,"range":{"start_line":333,"start_character":40,"end_line":334,"end_character":52},"updated":"2018-06-12 21:36:05.000000000","message":"nit: One nice to have is to try to include the policies in the order in which they\u0027re invoked just to keep the workflow clearer in one\u0027s mind.","commit_id":"7032592692f57e72d44c465b2307feb4d252cee2"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"2ca0f39f4160d0d8eeee07e2463baebe0b716b3d","unresolved":false,"context_lines":[{"line_number":332,"context_line":"    @rbac_rule_validation.action(service\u003d\"neutron\","},{"line_number":333,"context_line":"                                 rules\u003d[\"add_router_interface\","},{"line_number":334,"context_line":"                                        \"get_router\"],"},{"line_number":335,"context_line":"                                 expected_error_code_all_fail\u003d404)"},{"line_number":336,"context_line":"    @decorators.idempotent_id(\u0027a0627778-d68d-4913-881b-e345360cca19\u0027)"},{"line_number":337,"context_line":"    def test_add_router_interface(self):"},{"line_number":338,"context_line":"        \"\"\"Add Router Interface"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f7c97a3_0011435d","line":335,"updated":"2018-06-12 21:36:05.000000000","message":"(Copied over from previous comment for better context)\n\nWould it be possible to enhance expected_error_code to do this:\n\n    expected_error_code \u003d [403, 404]\n\nTo coincide precisely with the order of the rules:\n\n    rules \u003d [\"add_router_interface\", \"get_router\"]\n\nWhich implies this:\n\n    \"add_router_interface\" \u003d\u003e 403 upon failure\n    \"get_router\" \u003d\u003e 404 upon failure","commit_id":"7032592692f57e72d44c465b2307feb4d252cee2"}],"patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"ddac91332628ee75f9fafeb60937eff9502e6b4f","unresolved":false,"context_lines":[{"line_number":435,"context_line":"            mock.sentinel.action,"},{"line_number":436,"context_line":"            \"Denied\","},{"line_number":437,"context_line":"            \"Denied\")"},{"line_number":438,"context_line":""},{"line_number":439,"context_line":"    @mock.patch.object(rbac_rv, \u0027RBACLOG\u0027, autospec\u003dTrue)"},{"line_number":440,"context_line":"    @mock.patch.object(rbac_rv, \u0027requirements_authority\u0027, autospec\u003dTrue)"},{"line_number":441,"context_line":"    def test_error_code_overridden_403_by_requirements_auth(self,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_2f647049","line":438,"updated":"2018-05-30 22:41:48.000000000","message":"Also think it\u0027s worth putting in a test that verifies that overriding to like a 400 or 405 or something results in an expected error.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"5e02a642aa403316926467ca98b12a8a40ca4729","unresolved":false,"context_lines":[{"line_number":435,"context_line":"            mock.sentinel.action,"},{"line_number":436,"context_line":"            \"Denied\","},{"line_number":437,"context_line":"            \"Denied\")"},{"line_number":438,"context_line":""},{"line_number":439,"context_line":"    @mock.patch.object(rbac_rv, \u0027RBACLOG\u0027, autospec\u003dTrue)"},{"line_number":440,"context_line":"    @mock.patch.object(rbac_rv, \u0027requirements_authority\u0027, autospec\u003dTrue)"},{"line_number":441,"context_line":"    def test_error_code_overridden_403_by_requirements_auth(self,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_44aae570","line":438,"in_reply_to":"5f7c97a3_2f647049","updated":"2018-05-31 14:15:22.000000000","message":"Yes, good suggestion, I will add it.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"ddac91332628ee75f9fafeb60937eff9502e6b4f","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        def test_policy(*args):"},{"line_number":458,"context_line":"            raise exceptions.Forbidden()"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        test_policy(self.mock_test_args)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        mock_rbaclog.info.assert_called_once_with("},{"line_number":463,"context_line":"            \"[Service]: %s, [Test]: %s, [Rule]: %s, \""}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_0f8414d2","line":460,"updated":"2018-05-30 22:41:48.000000000","message":"If RequirementsAuthority says that a 403 should be returned, then that 403 on L451 should replace the expected_error_code\u003d404 on L456. And actual result is 404 judging by L458.\n\nSo exected\u003d\u003d 403 !\u003d actual \u003d\u003d 404. So this should thrown an error.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"5e02a642aa403316926467ca98b12a8a40ca4729","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        def test_policy(*args):"},{"line_number":458,"context_line":"            raise exceptions.Forbidden()"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        test_policy(self.mock_test_args)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        mock_rbaclog.info.assert_called_once_with("},{"line_number":463,"context_line":"            \"[Service]: %s, [Test]: %s, [Rule]: %s, \""}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_a4c89954","line":460,"in_reply_to":"5f7c97a3_0f8414d2","updated":"2018-05-31 14:15:22.000000000","message":"Actually, Forbidden exception on L458 is a 403.","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"ddac91332628ee75f9fafeb60937eff9502e6b4f","unresolved":false,"context_lines":[{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        test_policy(self.mock_test_args)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        mock_rbaclog.info.assert_called_once_with("},{"line_number":463,"context_line":"            \"[Service]: %s, [Test]: %s, [Rule]: %s, \""},{"line_number":464,"context_line":"            \"[Expected]: %s, [Actual]: %s\","},{"line_number":465,"context_line":"            mock.sentinel.service, \u0027test_policy\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_6f74a8fe","line":462,"updated":"2018-05-30 22:41:48.000000000","message":"It\u0027d be more interesting to validate this behavior instead:\n\n        LOG.debug(\"[Action]: %s, [Role]: %s is NOT allowed! \"\n                  \"Error code expected: %s\",\n                  rule, role, expected_error_code)","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"},{"author":{"_account_id":22636,"name":"Cliff Parsons","email":"cliffhparsons@aol.com","username":"cliffparsons"},"change_message_id":"5e02a642aa403316926467ca98b12a8a40ca4729","unresolved":false,"context_lines":[{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        test_policy(self.mock_test_args)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        mock_rbaclog.info.assert_called_once_with("},{"line_number":463,"context_line":"            \"[Service]: %s, [Test]: %s, [Rule]: %s, \""},{"line_number":464,"context_line":"            \"[Expected]: %s, [Actual]: %s\","},{"line_number":465,"context_line":"            mock.sentinel.service, \u0027test_policy\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_44d8a51c","line":462,"in_reply_to":"5f7c97a3_6f74a8fe","updated":"2018-05-31 14:15:22.000000000","message":"Yes, I can do that as well. No harm in validating both!","commit_id":"4973852334a6e5deb40f132f9c46efe5387cb68e"}],"releasenotes/notes/multi-policy-support-4e5c8b4e9e25ad9d.yaml":[{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a2db28e88579c89fb0675d863c94ab8c25393b34","unresolved":false,"context_lines":[{"line_number":14,"context_line":"    ``expected_error_codes`` argument has been added to the"},{"line_number":15,"context_line":"    ``rbac_rule_validation.action`` decorator, which is a list of error codes"},{"line_number":16,"context_line":"    expected when the corresponding rule in the ``rules`` list is disallowed"},{"line_number":17,"context_line":"    to perform the API action. For this reason, the error codes in the "},{"line_number":18,"context_line":"    ``expected_error_codes`` list must be appear in the same order as their"},{"line_number":19,"context_line":"    corresponding rules in the ``rules`` list. For example:"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":16,"id":"5f7c97a3_d1de710f","line":17,"range":{"start_line":17,"start_character":70,"end_line":17,"end_character":71},"updated":"2018-06-15 22:21:19.000000000","message":"WS","commit_id":"6d6ec5460b2270ac58c7d1a9828c30abd5a49362"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a2db28e88579c89fb0675d863c94ab8c25393b34","unresolved":false,"context_lines":[{"line_number":15,"context_line":"    ``rbac_rule_validation.action`` decorator, which is a list of error codes"},{"line_number":16,"context_line":"    expected when the corresponding rule in the ``rules`` list is disallowed"},{"line_number":17,"context_line":"    to perform the API action. For this reason, the error codes in the "},{"line_number":18,"context_line":"    ``expected_error_codes`` list must be appear in the same order as their"},{"line_number":19,"context_line":"    corresponding rules in the ``rules`` list. For example:"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"        expected_error_codes[0] is the error code for the rules[0] rule."}],"source_content_type":"text/x-yaml","patch_set":16,"id":"5f7c97a3_d1075181","line":18,"range":{"start_line":18,"start_character":39,"end_line":18,"end_character":48},"updated":"2018-06-15 22:21:19.000000000","message":"Pick one of these verbs","commit_id":"6d6ec5460b2270ac58c7d1a9828c30abd5a49362"},{"author":{"_account_id":23186,"name":"Felipe Monteiro","email":"felipe.carneiro.monteiro@gmail.com","username":"felipe.monteiro"},"change_message_id":"a2db28e88579c89fb0675d863c94ab8c25393b34","unresolved":false,"context_lines":[{"line_number":26,"context_line":"  - |"},{"line_number":27,"context_line":"    The ``rule`` argument in the ``rbac_rule_validation.action`` decorator has"},{"line_number":28,"context_line":"    been deprecated in favor of ``rules``."},{"line_number":29,"context_line":"    "},{"line_number":30,"context_line":"    The ``expected_error_code`` argument in the ``rbac_rule_validation.action``"},{"line_number":31,"context_line":"    decorator has been deprecated in favor of ``expected_error_codes``."}],"source_content_type":"text/x-yaml","patch_set":16,"id":"5f7c97a3_71e3ddd9","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":4},"updated":"2018-06-15 22:21:19.000000000","message":"WS","commit_id":"6d6ec5460b2270ac58c7d1a9828c30abd5a49362"}]}