)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"c11e087a442d3b7d9a1cd3b1da0377a2d4200670","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add separate policy rules for each CRUD operation"},{"line_number":8,"context_line":"on server_groups."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"same policy rule (os_compute_api:os-server-groups) is being"},{"line_number":11,"context_line":"used for all actions (show, index, delete, create) for"},{"line_number":12,"context_line":"server_groups REST APIs. It is thus impossible to provide"},{"line_number":13,"context_line":"different RBAC for specific actions based on roles. To get"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba5da102_91a403a7","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":4},"updated":"2016-10-28 20:59:22.000000000","message":"The same","commit_id":"c465824e8b81ce5256598828bf9d56ac70d43b4a"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"c11e087a442d3b7d9a1cd3b1da0377a2d4200670","unresolved":false,"context_lines":[{"line_number":12,"context_line":"server_groups REST APIs. It is thus impossible to provide"},{"line_number":13,"context_line":"different RBAC for specific actions based on roles. To get"},{"line_number":14,"context_line":"rid of this behavior changes are made to have separate"},{"line_number":15,"context_line":"policy rules for each of the action on server_groups"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ica9e07f6e80257902b4a0cc44b65fd6bad008bba"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba5da102_d1775b38","line":15,"range":{"start_line":15,"start_character":29,"end_line":15,"end_character":35},"updated":"2016-10-28 20:59:22.000000000","message":"actions, and missing period at the end of this sentence.","commit_id":"c465824e8b81ce5256598828bf9d56ac70d43b4a"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"c11e087a442d3b7d9a1cd3b1da0377a2d4200670","unresolved":false,"context_lines":[{"line_number":14,"context_line":"rid of this behavior changes are made to have separate"},{"line_number":15,"context_line":"policy rules for each of the action on server_groups"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ica9e07f6e80257902b4a0cc44b65fd6bad008bba"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ba5da102_71868f14","line":17,"updated":"2016-10-28 20:59:22.000000000","message":"need to add \"Closes-Bug: #1636157\"","commit_id":"c465824e8b81ce5256598828bf9d56ac70d43b4a"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"063d01234460e1ccf7d3496e5c764a4b91099f6c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     prashkre \u003cprashkre@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-11-01 18:45:04 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add separate policy rules for each CRUD operation"},{"line_number":8,"context_line":"on server_groups."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"The same policy rule (os_compute_api:os-server-groups) is being"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba5da102_d44d423f","line":7,"updated":"2016-11-01 21:11:00.000000000","message":"When I read this the first time I missed that the commit title spanned 2 lines. You can\u0027t do that. It has to be short enough to fit on a single line. Try \"Separate CRUD policy for server_groups\"","commit_id":"7175753a2516b8a7b436903468e16b314e308bc1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"323b226db8244d164c305606e4be34bf8e40feb2","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     b7a8086e (Merge \"Updated from global requirements\")"},{"line_number":2,"context_line":"Author:     Prashanth kumar reddy \u003cprashkre@in.ibm.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2016-10-27 07:09:01 -0400"},{"line_number":4,"context_line":"Commit:     prashkre \u003cprashkre@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-11-08 20:54:19 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Separate CRUD policy for server_groups"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a629dbe_70abdfd6","line":4,"range":{"start_line":4,"start_character":12,"end_line":4,"end_character":20},"updated":"2016-11-08 21:04:22.000000000","message":"did you change this manually? Odd... I thought it was right before, and that it would match line 2 above.","commit_id":"f7dcbf321eafcbff2453635118b1141cd55e049f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":11,"context_line":"server_groups REST APIs. It is thus impossible to provide"},{"line_number":12,"context_line":"different RBAC for specific actions based on roles. To get"},{"line_number":13,"context_line":"rid of this behavior changes are made to have separate"},{"line_number":14,"context_line":"policy rules for each of the actions on server_groups"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ica9e07f6e80257902b4a0cc44b65fd6bad008bba"},{"line_number":17,"context_line":"Closes-Bug: #1636157"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9a629dbe_c9ba346e","line":14,"updated":"2016-11-09 09:59:29.000000000","message":"We should really make the case for why each rule is needed.\n\nI get why you would want someone to have read access, but not able to create or delete. I get why you might have only read and create access, but be unable to delete.\n\nBeing able to list but not show doesn\u0027t make sense to me, so I don\u0027t think we should have separate rules for those actions. But maybe I am just not seeing the use case?","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"c3187b64ce8661227fbe8b81c61022aea538061c","unresolved":false,"context_lines":[{"line_number":11,"context_line":"server_groups REST APIs. It is thus impossible to provide"},{"line_number":12,"context_line":"different RBAC for specific actions based on roles. To get"},{"line_number":13,"context_line":"rid of this behavior changes are made to have separate"},{"line_number":14,"context_line":"policy rules for each of the actions on server_groups"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ica9e07f6e80257902b4a0cc44b65fd6bad008bba"},{"line_number":17,"context_line":"Closes-Bug: #1636157"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"7a77a97e_aa19c924","line":14,"in_reply_to":"7a77a97e_8cac308e","updated":"2016-11-21 13:48:01.000000000","message":"\"I get folks need custom RBAC, but I strongly feel we should limit the ability to effectively change the semantics of the API\" \u003c-- I very much agree with this, actually. We\u0027ve made very bad implementation choices on both sides of the line between what we hardcode vs. what we make you do in policy. I just don\u0027t want to bite off that very large issue here. Just going for consistency with what is the current standard practice here, and we can dig into that as a larger, more general policy rewrite discussion separately.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2f9dc74b62efdceebbfba79ce1a341dba890dd81","unresolved":false,"context_lines":[{"line_number":11,"context_line":"server_groups REST APIs. It is thus impossible to provide"},{"line_number":12,"context_line":"different RBAC for specific actions based on roles. To get"},{"line_number":13,"context_line":"rid of this behavior changes are made to have separate"},{"line_number":14,"context_line":"policy rules for each of the actions on server_groups"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ica9e07f6e80257902b4a0cc44b65fd6bad008bba"},{"line_number":17,"context_line":"Closes-Bug: #1636157"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"7a77a97e_8cac308e","line":14,"in_reply_to":"9a629dbe_8356ef03","updated":"2016-11-21 12:15:28.000000000","message":"Those sounds like hierarchical tenancy that is being hacked into the policy.\n\nI get folks need custom RBAC, but I strongly feel we should limit the ability to effectively change the semantics of the API.\n\nI will not block the patch on this, as we clearly don\u0027t have consensus on the approach yet.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":11,"context_line":"server_groups REST APIs. It is thus impossible to provide"},{"line_number":12,"context_line":"different RBAC for specific actions based on roles. To get"},{"line_number":13,"context_line":"rid of this behavior changes are made to have separate"},{"line_number":14,"context_line":"policy rules for each of the actions on server_groups"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ica9e07f6e80257902b4a0cc44b65fd6bad008bba"},{"line_number":17,"context_line":"Closes-Bug: #1636157"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9a629dbe_fbffaca7","line":14,"in_reply_to":"9a629dbe_c9ba346e","updated":"2016-11-09 15:08:45.000000000","message":"Most of the time you\u0027re right, list and show would use the same rule, but not always. I have seen cases where someone is allowed to show some things but not others based on some criteria, and so they\u0027re not allowed to list.\n\nMost other places (in nova and beyond) have separate rules for index and show, so this is also a consistency issue. And consistency is important. Policy is complicated enough without having to deal with issues of inconsistency between different APIs.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":13637,"name":"Divya K Konoor","email":"dikonoor@in.ibm.com","username":"dikonoor"},"change_message_id":"dbc272e748930740c11e472db55f91cf7ce37022","unresolved":false,"context_lines":[{"line_number":11,"context_line":"server_groups REST APIs. It is thus impossible to provide"},{"line_number":12,"context_line":"different RBAC for specific actions based on roles. To get"},{"line_number":13,"context_line":"rid of this behavior changes are made to have separate"},{"line_number":14,"context_line":"policy rules for each of the actions on server_groups"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ica9e07f6e80257902b4a0cc44b65fd6bad008bba"},{"line_number":17,"context_line":"Closes-Bug: #1636157"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"9a629dbe_8356ef03","line":14,"in_reply_to":"9a629dbe_fbffaca7","updated":"2016-11-11 11:27:21.000000000","message":"Agree with Matthew. Providing a separate policy rule for each CRUD operation (including show and index) provides granular control and that\u0027s how we have it defined across projects for all rest apis. If we want the same set of roles to have permissions for index and show, with this implementation that can always be configured in the policy.json file.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"}],"nova/api/openstack/compute/server_groups.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        \"\"\"Return data about the given server group.\"\"\""},{"line_number":78,"context_line":"        context \u003d _authorize_context(req, \u0027show\u0027)"},{"line_number":79,"context_line":"        try:"},{"line_number":80,"context_line":"            sg \u003d objects.InstanceGroup.get_by_uuid(context, id)"},{"line_number":81,"context_line":"        except nova.exception.InstanceGroupNotFound as e:"},{"line_number":82,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":83,"context_line":"        return {\u0027server_group\u0027: self._format_server_group(context, sg, req)}"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_29f49084","line":80,"updated":"2016-11-09 09:59:29.000000000","message":"So this instance group belongs to a specific tenant. Right now we will 404 if the context doesn\u0027t match the requested instance group here.\n\nThe policy is actually enforced here:\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L6500\nhttps://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L313\n\nNow I am fine with that, because its consistent, but I just wanted to be clear that the policy here really can only restrict access. You can\u0027t have a non-admin token read another tenant\u0027s server group, no matter what is done here.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        \"\"\"Return data about the given server group.\"\"\""},{"line_number":78,"context_line":"        context \u003d _authorize_context(req, \u0027show\u0027)"},{"line_number":79,"context_line":"        try:"},{"line_number":80,"context_line":"            sg \u003d objects.InstanceGroup.get_by_uuid(context, id)"},{"line_number":81,"context_line":"        except nova.exception.InstanceGroupNotFound as e:"},{"line_number":82,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":83,"context_line":"        return {\u0027server_group\u0027: self._format_server_group(context, sg, req)}"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_a521974f","line":80,"in_reply_to":"9a629dbe_29f49084","updated":"2016-11-09 15:08:45.000000000","message":"I assume you\u0027re bringing up tenants because the default value for this rule is admin OR OWNER, and the OR OWNER part shouldn\u0027t be necessary because that scope check should be hardcoded regardless of what you set in policy. If so, I wholeheartedly agree. But I think that\u0027s cleanup for a different patch, and an issue that goes far beyond server groups. The reason this patch includes the OR OWNER is simply because it\u0027s pointing to the old rule that always did that, i.e., no change.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2f9dc74b62efdceebbfba79ce1a341dba890dd81","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        \"\"\"Return data about the given server group.\"\"\""},{"line_number":78,"context_line":"        context \u003d _authorize_context(req, \u0027show\u0027)"},{"line_number":79,"context_line":"        try:"},{"line_number":80,"context_line":"            sg \u003d objects.InstanceGroup.get_by_uuid(context, id)"},{"line_number":81,"context_line":"        except nova.exception.InstanceGroupNotFound as e:"},{"line_number":82,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.format_message())"},{"line_number":83,"context_line":"        return {\u0027server_group\u0027: self._format_server_group(context, sg, req)}"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a77a97e_2c5ebc76","line":80,"in_reply_to":"9a629dbe_a521974f","updated":"2016-11-21 12:15:28.000000000","message":"I am just raising that these new rules have limited use due to how the DB layer works. You can only really restrict things, you can\u0027t write a rule that allows a non-admin to show something owned by another tenant.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"}],"nova/policies/server_groups.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7375e9500e2729393795ef2cf421140597e2da78","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        check_str\u003dbase.RULE_ANY),"},{"line_number":28,"context_line":"    policy.RuleDefault("},{"line_number":29,"context_line":"        name\u003dPOLICY_ROOT % \u0027create\u0027,"},{"line_number":30,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_OWNER),"},{"line_number":31,"context_line":"    policy.RuleDefault("},{"line_number":32,"context_line":"        name\u003dPOLICY_ROOT % \u0027delete\u0027,"},{"line_number":33,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_OWNER),"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_853bee37","line":30,"updated":"2016-11-08 13:29:27.000000000","message":"Could we leave the \"os_compute_api:os-server-groups\" rule present.\n\nInstead add these as extra rules, that default to the existing rule, i.e. we make this: \"rule:os_compute_api:os-server-groups\"\n\nThat way across upgrade this should ensure if an existing admin has customised the rule, it keeps working, but folks that know about the new setting can over-ride the default rule.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"1d3f30856b64cc6d5e33a8e7045f0581b5148c44","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        check_str\u003dbase.RULE_ANY),"},{"line_number":28,"context_line":"    policy.RuleDefault("},{"line_number":29,"context_line":"        name\u003dPOLICY_ROOT % \u0027create\u0027,"},{"line_number":30,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_OWNER),"},{"line_number":31,"context_line":"    policy.RuleDefault("},{"line_number":32,"context_line":"        name\u003dPOLICY_ROOT % \u0027delete\u0027,"},{"line_number":33,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_OWNER),"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_9ec6d447","line":30,"in_reply_to":"9a629dbe_287d976e","updated":"2016-11-08 15:35:23.000000000","message":"I\u0027m ok with John\u0027s suggestion. How do we eventually, some number of releases down the road, get rid of the old rule? Is there a deprecation process for policy rules that we should be doing something to follow here?","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"52576b3aebd0bf378b952df8c1a698c02f829fa9","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        check_str\u003dbase.RULE_ANY),"},{"line_number":28,"context_line":"    policy.RuleDefault("},{"line_number":29,"context_line":"        name\u003dPOLICY_ROOT % \u0027create\u0027,"},{"line_number":30,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_OWNER),"},{"line_number":31,"context_line":"    policy.RuleDefault("},{"line_number":32,"context_line":"        name\u003dPOLICY_ROOT % \u0027delete\u0027,"},{"line_number":33,"context_line":"        check_str\u003dbase.RULE_ADMIN_OR_OWNER),"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_287d976e","line":30,"in_reply_to":"9a629dbe_853bee37","updated":"2016-11-08 13:34:56.000000000","message":"+1.\n\nWhen user already override the base rule, it works correctly.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"00791852433d0b5305abc3f3ad219f69fa7b4876","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"BASE_POLICY_NAME \u003d \u0027os_compute_api:os-server-groups\u0027"},{"line_number":22,"context_line":"POLICY_ROOT \u003d \u0027os_compute_api:os-server-groups:%s\u0027"},{"line_number":23,"context_line":"BASE_POLICY_RULE \u003d \u0027rule:os_compute_api:os-server-groups\u0027"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"server_groups_policies \u003d ["}],"source_content_type":"text/x-python","patch_set":7,"id":"9a629dbe_fd8b481c","line":23,"updated":"2016-11-08 20:54:43.000000000","message":"The following would be a little better IMHO:\n\n\u0027rule:%s\u0027 % BASE_POLICY_NAME","commit_id":"285ff33a28516d013f41b8fc88ed528b622f4b9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    policy.RuleDefault("},{"line_number":37,"context_line":"        name\u003dPOLICY_ROOT % \u0027delete\u0027,"},{"line_number":38,"context_line":"        check_str\u003dBASE_POLICY_RULE),"},{"line_number":39,"context_line":"    policy.RuleDefault("},{"line_number":40,"context_line":"        name\u003dPOLICY_ROOT % \u0027index\u0027,"},{"line_number":41,"context_line":"        check_str\u003dBASE_POLICY_RULE),"},{"line_number":42,"context_line":"    policy.RuleDefault("},{"line_number":43,"context_line":"        name\u003dPOLICY_ROOT % \u0027show\u0027,"},{"line_number":44,"context_line":"        check_str\u003dBASE_POLICY_RULE),"},{"line_number":45,"context_line":"]"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_8e1cc278","line":44,"range":{"start_line":39,"start_character":0,"end_line":44,"end_character":36},"updated":"2016-11-09 09:59:29.000000000","message":"Sorry I didn\u0027t spot this question before...\n\nDo we really need these as separate rules? I think the default rule defining read access seems good enough. It feels bad that you would have separate rules for index and show, from an interop point of view.\n\nI totally get why folks would want to separately restrict create and delete, that was an oversight when this was added.\n\nIt would be easier to have these conversations in a spec. This is really a feature not a bug fix. But we have got this far, so lets go with it.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    policy.RuleDefault("},{"line_number":37,"context_line":"        name\u003dPOLICY_ROOT % \u0027delete\u0027,"},{"line_number":38,"context_line":"        check_str\u003dBASE_POLICY_RULE),"},{"line_number":39,"context_line":"    policy.RuleDefault("},{"line_number":40,"context_line":"        name\u003dPOLICY_ROOT % \u0027index\u0027,"},{"line_number":41,"context_line":"        check_str\u003dBASE_POLICY_RULE),"},{"line_number":42,"context_line":"    policy.RuleDefault("},{"line_number":43,"context_line":"        name\u003dPOLICY_ROOT % \u0027show\u0027,"},{"line_number":44,"context_line":"        check_str\u003dBASE_POLICY_RULE),"},{"line_number":45,"context_line":"]"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_2513c7b9","line":44,"range":{"start_line":39,"start_character":0,"end_line":44,"end_character":36},"in_reply_to":"9a629dbe_8e1cc278","updated":"2016-11-09 15:08:45.000000000","message":"answered this in the similar comment against the commit message.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"797a3e7cebfc115716ccd21da9bfa2fb8834bec8","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"BASE_POLICY_NAME \u003d \u0027os_compute_api:os-server-groups\u0027"},{"line_number":22,"context_line":"POLICY_ROOT \u003d \u0027os_compute_api:os-server-groups:%s\u0027"},{"line_number":23,"context_line":"BASE_POLICY_RULE \u003d \u0027rule:%s\u0027 % BASE_POLICY_NAME"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"server_groups_policies \u003d ["}],"source_content_type":"text/x-python","patch_set":13,"id":"df3967d1_891b687f","line":23,"range":{"start_line":23,"start_character":0,"end_line":23,"end_character":47},"updated":"2017-08-03 18:10:23.000000000","message":"How does this make any sense? Shouldn\u0027t this be something like check_str\u003dbase.RULE_ADMIN_OR_OWNER that is using it in the granular rules?","commit_id":"4a09c2210b3c07343411a06c676c2d85aa0e214f"},{"author":{"_account_id":24005,"name":"prashkre","email":"prashkre@in.ibm.com","username":"prashkre"},"change_message_id":"09bbd230e3d8b13e6511df785fc531d9f35d31b1","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"BASE_POLICY_NAME \u003d \u0027os_compute_api:os-server-groups\u0027"},{"line_number":22,"context_line":"POLICY_ROOT \u003d \u0027os_compute_api:os-server-groups:%s\u0027"},{"line_number":23,"context_line":"BASE_POLICY_RULE \u003d \u0027rule:%s\u0027 % BASE_POLICY_NAME"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"server_groups_policies \u003d ["}],"source_content_type":"text/x-python","patch_set":13,"id":"df3967d1_df25a20a","line":23,"range":{"start_line":23,"start_character":0,"end_line":23,"end_character":47},"in_reply_to":"df3967d1_1c6fb8fc","updated":"2017-08-03 19:15:18.000000000","message":"Ignore my previous comment.\n\nIt was set as \"BASE_POLICY_RULE \u003d \u0027rule:%s\u0027 % BASE_POLICY_NAME\" because in order not to brake the environment of users who already started using the policy rule \"os_compute_api:os-server-groups\" in their config. \n\nSuppose if some one defined something like \"os_compute_api:os-server-groups\": \"rule:admin\",\n\nthen all below CRUD rules will follow above generic rule\n\"os_compute_api:os-server-groups:index\": \n\"os_compute_api:os-server-groups:show\": \n\"os_compute_api:os-server-groups:create\": \n\"os_compute_api:os-server-groups:delete\":","commit_id":"4a09c2210b3c07343411a06c676c2d85aa0e214f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"90d7d1138adde580e962d746eaa32febf7acae84","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"BASE_POLICY_NAME \u003d \u0027os_compute_api:os-server-groups\u0027"},{"line_number":22,"context_line":"POLICY_ROOT \u003d \u0027os_compute_api:os-server-groups:%s\u0027"},{"line_number":23,"context_line":"BASE_POLICY_RULE \u003d \u0027rule:%s\u0027 % BASE_POLICY_NAME"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"server_groups_policies \u003d ["}],"source_content_type":"text/x-python","patch_set":13,"id":"df3967d1_bcd78cee","line":23,"range":{"start_line":23,"start_character":0,"end_line":23,"end_character":47},"in_reply_to":"df3967d1_891b687f","updated":"2017-08-03 18:35:59.000000000","message":"Nevermind, I get it now.","commit_id":"4a09c2210b3c07343411a06c676c2d85aa0e214f"},{"author":{"_account_id":24005,"name":"prashkre","email":"prashkre@in.ibm.com","username":"prashkre"},"change_message_id":"4316075edf39cd29356e76c44bb5394e58e86d13","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"BASE_POLICY_NAME \u003d \u0027os_compute_api:os-server-groups\u0027"},{"line_number":22,"context_line":"POLICY_ROOT \u003d \u0027os_compute_api:os-server-groups:%s\u0027"},{"line_number":23,"context_line":"BASE_POLICY_RULE \u003d \u0027rule:%s\u0027 % BASE_POLICY_NAME"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"server_groups_policies \u003d ["}],"source_content_type":"text/x-python","patch_set":13,"id":"df3967d1_1c6fb8fc","line":23,"range":{"start_line":23,"start_character":0,"end_line":23,"end_character":47},"in_reply_to":"df3967d1_891b687f","updated":"2017-08-03 18:45:27.000000000","message":"Thanks Matt for pointing this out. I apologize for this copy paste error, this happened while refactoring the changes from patch 6 to patch7. \n\n\"BASE_POLICY_RULE \u003d \u0027rule:%s\u0027 % BASE_POLICY_NAME\" can be removed entirely and s/BASE_POLICY_RULE/base.RULE_ADMIN_OR_OWNER at 35, 38, 41, 44.","commit_id":"4a09c2210b3c07343411a06c676c2d85aa0e214f"}],"nova/tests/unit/api/openstack/compute/test_server_groups.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"e291b44c84756fd03020096d2dac3df0c42ad246","unresolved":false,"context_lines":[{"line_number":446,"context_line":"            exc.format_message())"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    def test_index_server_group_policy_fail(self):"},{"line_number":449,"context_line":"        rule_name \u003d \"os_compute_api:os-server-groups:index\""},{"line_number":450,"context_line":"        rules \u003d {rule_name: \"role:admin or project_id:non_fake\"}"},{"line_number":451,"context_line":"        self._common_policy_check(rules, rule_name,"},{"line_number":452,"context_line":"                                  self.controller.index, self.req)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_c55ec611","line":450,"range":{"start_line":449,"start_character":2,"end_line":450,"end_character":64},"updated":"2016-11-08 13:14:57.000000000","message":"John pointed out we should test on the default rule, not the override one. So I think we can begin to test the default rule for those new added test.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"875601ba9b5e1525c8c36b6e2593d5ee60f52faa","unresolved":false,"context_lines":[{"line_number":446,"context_line":"            exc.format_message())"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    def test_index_server_group_policy_fail(self):"},{"line_number":449,"context_line":"        rule_name \u003d \"os_compute_api:os-server-groups:index\""},{"line_number":450,"context_line":"        rules \u003d {rule_name: \"role:admin or project_id:non_fake\"}"},{"line_number":451,"context_line":"        self._common_policy_check(rules, rule_name,"},{"line_number":452,"context_line":"                                  self.controller.index, self.req)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_10360ce6","line":450,"range":{"start_line":449,"start_character":2,"end_line":450,"end_character":64},"in_reply_to":"9a629dbe_2d5f3b0f","updated":"2016-11-09 10:20:49.000000000","message":"I should be clearer, I am fine if we want to test override of the policy, but we need to check the default works first.\n\nI would prefer the override tests to check a pass (I guess and admin context, given the chosen rules) as well as a fail.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1d957969639706c97056bbaa398bd858b275e561","unresolved":false,"context_lines":[{"line_number":446,"context_line":"            exc.format_message())"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    def test_index_server_group_policy_fail(self):"},{"line_number":449,"context_line":"        rule_name \u003d \"os_compute_api:os-server-groups:index\""},{"line_number":450,"context_line":"        rules \u003d {rule_name: \"role:admin or project_id:non_fake\"}"},{"line_number":451,"context_line":"        self._common_policy_check(rules, rule_name,"},{"line_number":452,"context_line":"                                  self.controller.index, self.req)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_2d5f3b0f","line":450,"range":{"start_line":449,"start_character":2,"end_line":450,"end_character":64},"in_reply_to":"9a629dbe_5e80bc46","updated":"2016-11-09 10:06:28.000000000","message":"First we need to test you can fail the default policy. That is not being done anywhere, as I understand it. Since we add the rule, we should check its working correctly.\n\nWe could check an override, but thats a separate test in my mind, and because the policy is not given any target, it seems to have limited use.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"1d3f30856b64cc6d5e33a8e7045f0581b5148c44","unresolved":false,"context_lines":[{"line_number":446,"context_line":"            exc.format_message())"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    def test_index_server_group_policy_fail(self):"},{"line_number":449,"context_line":"        rule_name \u003d \"os_compute_api:os-server-groups:index\""},{"line_number":450,"context_line":"        rules \u003d {rule_name: \"role:admin or project_id:non_fake\"}"},{"line_number":451,"context_line":"        self._common_policy_check(rules, rule_name,"},{"line_number":452,"context_line":"                                  self.controller.index, self.req)"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_5e80bc46","line":450,"range":{"start_line":449,"start_character":2,"end_line":450,"end_character":64},"in_reply_to":"9a629dbe_c55ec611","updated":"2016-11-08 15:35:23.000000000","message":"I think the point here is to test that if someone overrides the new rule (which is why it\u0027s being created), that will work. I.e. the rule is being checked properly. I believe the defaults should be tested in test_policy.py.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        self.req \u003d fakes.HTTPRequest.blank(\u0027\u0027)"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def _common_policy_check(self, rules, rule_name, func, *arg, **kwarg):"},{"line_number":441,"context_line":"        self.policy.set_rules(rules)"},{"line_number":442,"context_line":"        exc \u003d self.assertRaises("},{"line_number":443,"context_line":"             exception.PolicyNotAuthorized, func, *arg, **kwarg)"},{"line_number":444,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_694e8870","line":441,"updated":"2016-11-09 09:59:29.000000000","message":"Why do we set the rules here, we should instead test the default rules, and send in bad context when needed.\n\nI get this follows the existing patterns, but they are wrong with the current version of the code.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f58fb16e757f2ca3abe112d2f194dceb6ad1b613","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        self.req \u003d fakes.HTTPRequest.blank(\u0027\u0027)"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def _common_policy_check(self, rules, rule_name, func, *arg, **kwarg):"},{"line_number":441,"context_line":"        self.policy.set_rules(rules)"},{"line_number":442,"context_line":"        exc \u003d self.assertRaises("},{"line_number":443,"context_line":"             exception.PolicyNotAuthorized, func, *arg, **kwarg)"},{"line_number":444,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_d0de546c","line":441,"in_reply_to":"9a629dbe_694e8870","updated":"2016-11-09 10:24:12.000000000","message":"Oops, wrong patchset.\n\nI am OK having a test for overriding the rules, but I we should have tests to prove the default rules pass/fail as expected at a minimum.\n\nI know this is not actually included currently for any other API, thats on our TODO list for tidying up after the policy refactoring work last cycle.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":438,"context_line":"        self.req \u003d fakes.HTTPRequest.blank(\u0027\u0027)"},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"    def _common_policy_check(self, rules, rule_name, func, *arg, **kwarg):"},{"line_number":441,"context_line":"        self.policy.set_rules(rules)"},{"line_number":442,"context_line":"        exc \u003d self.assertRaises("},{"line_number":443,"context_line":"             exception.PolicyNotAuthorized, func, *arg, **kwarg)"},{"line_number":444,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_5ba6d810","line":441,"in_reply_to":"9a629dbe_d0de546c","updated":"2016-11-09 15:08:45.000000000","message":"I think the expectation was that test_policy covered that, but I guess that just tests the policy check, it doesn\u0027t actually call the controller method. So I\u0027d be fine with testing both default and overrides here.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":454,"context_line":"    def test_index_server_group_policy_pass(self):"},{"line_number":455,"context_line":"        rule_name \u003d \"os_compute_api:os-server-groups:index\""},{"line_number":456,"context_line":"        rules \u003d {rule_name: \"role:admin or project_id:fake\"}"},{"line_number":457,"context_line":"        self.policy.set_rules(rules)"},{"line_number":458,"context_line":"        result \u003d self.controller.index(self.req)"},{"line_number":459,"context_line":"        self.assertEqual(result[\u0027server_groups\u0027], [])"},{"line_number":460,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_e969b8bf","line":457,"updated":"2016-11-09 09:59:29.000000000","message":"We shoudl totally pass with the default rule.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b39e7f4c85936201354796e81ea0787d6c828be8","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    def _setup_controller(self):"},{"line_number":88,"context_line":"        self.controller \u003d sg_v21.ServerGroupController()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _set_rbac_policy(self, rules\u003dNone):"},{"line_number":91,"context_line":"        rule_dict \u003d {}"},{"line_number":92,"context_line":"        # add defaults"},{"line_number":93,"context_line":"        for rule in base_policies.list_rules():"},{"line_number":94,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":95,"context_line":"        for rule in sg_policies.list_rules():"},{"line_number":96,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":97,"context_line":"        # add caller-specified rules (overriding defaults)"},{"line_number":98,"context_line":"        if rules is not None:"},{"line_number":99,"context_line":"            for key, value in rules.items():"},{"line_number":100,"context_line":"                rule_dict[key] \u003d value"},{"line_number":101,"context_line":"        self.policy.set_rules(rule_dict)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def test_create_server_group_with_no_policies(self):"},{"line_number":104,"context_line":"        sgroup \u003d server_group_template()"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_0fe8f90f","line":101,"range":{"start_line":90,"start_character":7,"end_line":101,"end_character":40},"updated":"2016-11-15 12:49:36.000000000","message":"We can reuse the RealPolicyFixture at here https://github.com/openstack/nova/blob/master/nova/tests/unit/policy_fixture.py#L31 it will load the default policy file, and that file is empty now. So it is equal to default rules.\n\nWe probably create new class at here. And we override the policy fixture which created in the base class https://github.com/openstack/nova/blob/master/nova/test.py#L252 with RealPolicyFixture.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"f8cc17d1887aaf8658f7c9da2d694b6dc93e799b","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    def _setup_controller(self):"},{"line_number":88,"context_line":"        self.controller \u003d sg_v21.ServerGroupController()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _set_rbac_policy(self, rules\u003dNone):"},{"line_number":91,"context_line":"        rule_dict \u003d {}"},{"line_number":92,"context_line":"        # add defaults"},{"line_number":93,"context_line":"        for rule in base_policies.list_rules():"},{"line_number":94,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":95,"context_line":"        for rule in sg_policies.list_rules():"},{"line_number":96,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":97,"context_line":"        # add caller-specified rules (overriding defaults)"},{"line_number":98,"context_line":"        if rules is not None:"},{"line_number":99,"context_line":"            for key, value in rules.items():"},{"line_number":100,"context_line":"                rule_dict[key] \u003d value"},{"line_number":101,"context_line":"        self.policy.set_rules(rule_dict)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def test_create_server_group_with_no_policies(self):"},{"line_number":104,"context_line":"        sgroup \u003d server_group_template()"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_fa5ffa0b","line":101,"range":{"start_line":90,"start_character":7,"end_line":101,"end_character":40},"in_reply_to":"7a77a97e_0fe8f90f","updated":"2016-11-15 15:20:05.000000000","message":"We might be able to use a combination of RealPolicyFixture and something allowing us to override specific rules, but the latter capability does not currently exist in policy_fixture.\n\nI\u0027m reluctant to use RoleBasedPolicyFixture because that seems like a bad idea in general. It is much better practice to only override the specific rules you want to test than to override everything. I did start down that path, and kept hitting one issue after another where things aren\u0027t written well enough to do this. First, RoleBasedPolicyFixture only currently works with NoDbTestCase (because of its unnecessary use of CONF). Second, fakes.HTTPRequest doesn\u0027t give us a way to fake a role (only isadmin:True). Who knows what I\u0027ll find next if I keep going down that rabbit hole.\n\nIt feels like we\u0027re getting into rewriting how we do policy-related UT in general. While useful, that is outside the scope of this particular fix. I think the current proposal is the best way to test this with the facilities that exist today.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"d21bd0dbff37455926e68a0416eb066cbe53ed46","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    def _setup_controller(self):"},{"line_number":88,"context_line":"        self.controller \u003d sg_v21.ServerGroupController()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _set_rbac_policy(self, rules\u003dNone):"},{"line_number":91,"context_line":"        rule_dict \u003d {}"},{"line_number":92,"context_line":"        # add defaults"},{"line_number":93,"context_line":"        for rule in base_policies.list_rules():"},{"line_number":94,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":95,"context_line":"        for rule in sg_policies.list_rules():"},{"line_number":96,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":97,"context_line":"        # add caller-specified rules (overriding defaults)"},{"line_number":98,"context_line":"        if rules is not None:"},{"line_number":99,"context_line":"            for key, value in rules.items():"},{"line_number":100,"context_line":"                rule_dict[key] \u003d value"},{"line_number":101,"context_line":"        self.policy.set_rules(rule_dict)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def test_create_server_group_with_no_policies(self):"},{"line_number":104,"context_line":"        sgroup \u003d server_group_template()"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_b354e421","line":101,"range":{"start_line":90,"start_character":7,"end_line":101,"end_character":40},"in_reply_to":"7a77a97e_71d7afbf","updated":"2016-11-21 14:54:03.000000000","message":"sure, I can do that.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"ad41b04b96102eb3caa3349cd499fac4529db901","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    def _setup_controller(self):"},{"line_number":88,"context_line":"        self.controller \u003d sg_v21.ServerGroupController()"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def _set_rbac_policy(self, rules\u003dNone):"},{"line_number":91,"context_line":"        rule_dict \u003d {}"},{"line_number":92,"context_line":"        # add defaults"},{"line_number":93,"context_line":"        for rule in base_policies.list_rules():"},{"line_number":94,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":95,"context_line":"        for rule in sg_policies.list_rules():"},{"line_number":96,"context_line":"            rule_dict[rule.name] \u003d rule.check_str"},{"line_number":97,"context_line":"        # add caller-specified rules (overriding defaults)"},{"line_number":98,"context_line":"        if rules is not None:"},{"line_number":99,"context_line":"            for key, value in rules.items():"},{"line_number":100,"context_line":"                rule_dict[key] \u003d value"},{"line_number":101,"context_line":"        self.policy.set_rules(rule_dict)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def test_create_server_group_with_no_policies(self):"},{"line_number":104,"context_line":"        sgroup \u003d server_group_template()"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_71d7afbf","line":101,"range":{"start_line":90,"start_character":7,"end_line":101,"end_character":40},"in_reply_to":"7a77a97e_fa5ffa0b","updated":"2016-11-21 11:55:35.000000000","message":"@Matthew, actually the goal of RealPolicyFixture is that loading the default policy rule by using the code of nova. That is what the line 93-96 trying to fake. We needn\u0027t fake that, we just load the real default rule in real way.\n\nThen you can just override the specific rule like line 101, then you needn\u0027t that extra help method \u0027_set_rbac_policy\u0027","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5d21529ab72e13807b1c839ec0b7bb99747dee7b","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.controller.create(self.admin_req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_8ce6f042","line":133,"updated":"2016-11-21 12:03:50.000000000","message":"We should really test another user from a different tenant fails using the default rules.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"81e912a23c39e4bb7d2d5f00404d9f57ea1a9261","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.controller.create(self.admin_req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_4e6cc208","line":133,"in_reply_to":"7a77a97e_5ea2b79b","updated":"2016-11-21 16:19:44.000000000","message":"ok, so you can\u0027t specify a project on the create request (it will just create it in the project of your context), so this isn\u0027t an issue for create. But we can check this on show.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"d21bd0dbff37455926e68a0416eb066cbe53ed46","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.controller.create(self.admin_req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_be302bea","line":133,"in_reply_to":"7a77a97e_8ce6f042","updated":"2016-11-21 14:54:03.000000000","message":"it won\u0027t, though, because server_groups don\u0027t have a tenant owner. They aren\u0027t tenant-specific. The default rule should really be to allow all, rather than admin or owner. I assumed that would be a different patch, but I could do it here if you want.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"bfe853d809edd38bcbc19ee856cd8fdf50ece070","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        self.controller.create(self.admin_req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_5ea2b79b","line":133,"in_reply_to":"7a77a97e_be302bea","updated":"2016-11-21 15:04:27.000000000","message":"I think I spoke too quickly here. I\u0027ll look into this.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"b39e7f4c85936201354796e81ea0787d6c828be8","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"},{"line_number":137,"context_line":"        rules \u003d {rule_name: \u0027is_admin:True\u0027}"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_cff9a1ca","line":134,"updated":"2016-11-15 12:49:36.000000000","message":"We probably can separate this into two testcases, one for testing the default rule. another one for testing the separate rule override the base one.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"d21bd0dbff37455926e68a0416eb066cbe53ed46","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"},{"line_number":137,"context_line":"        rules \u003d {rule_name: \u0027is_admin:True\u0027}"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_3e5b3b30","line":134,"in_reply_to":"7a77a97e_0c402007","updated":"2016-11-21 14:54:03.000000000","message":"ok, fine.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5d21529ab72e13807b1c839ec0b7bb99747dee7b","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"},{"line_number":137,"context_line":"        rules \u003d {rule_name: \u0027is_admin:True\u0027}"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_0c402007","line":134,"in_reply_to":"7a77a97e_91928bf5","updated":"2016-11-21 12:03:50.000000000","message":"+1 ideally keep the testing of the defaults and the overrides to two separate tests. This second one seems like overkill, but I am not against having it.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"f8cc17d1887aaf8658f7c9da2d694b6dc93e799b","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"},{"line_number":137,"context_line":"        rules \u003d {rule_name: \u0027is_admin:True\u0027}"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_e4c4bf41","line":134,"in_reply_to":"7a77a97e_cff9a1ca","updated":"2016-11-15 15:20:05.000000000","message":"the default rules are already tested in all the other tests that cover create, e.g. test_create_server_group. I just did it here again to be super-explicit and to ensure that it works for non-admin by default but not when overriden... i.e., the two need to go together because they\u0027re really part of the same test.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"ad41b04b96102eb3caa3349cd499fac4529db901","unresolved":false,"context_lines":[{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # test as non-admin"},{"line_number":133,"context_line":"        self.controller.create(self.req, body\u003d{\u0027server_group\u0027: sgroup})"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        # override policy to restrict to admin"},{"line_number":136,"context_line":"        rule_name \u003d sg_policies.POLICY_ROOT % \u0027create\u0027"},{"line_number":137,"context_line":"        rules \u003d {rule_name: \u0027is_admin:True\u0027}"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a77a97e_91928bf5","line":134,"in_reply_to":"7a77a97e_e4c4bf41","updated":"2016-11-21 11:55:35.000000000","message":"actually we separate the test into two, and have two different name, then when they failed, you can clear to know which case failed.","commit_id":"c1ce9ab49922886eef84f1b147a713559e9f2f88"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"ff5ce4183ce0c5cb8342a75fdbc670b4e04b9f74","unresolved":false,"context_lines":[{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        # test as non-admin, different project"},{"line_number":320,"context_line":"        self.assertRaises(webob.exc.HTTPNotFound,"},{"line_number":321,"context_line":"                          self.controller.show, self.foo_req, ig_uuid)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    def test_display_members_rbac_admin_only(self):"},{"line_number":324,"context_line":"        ctx \u003d context.RequestContext(\u0027fake_user\u0027, \u0027fake\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"1a6eadb0_7ed02f54","line":321,"updated":"2016-12-13 14:42:04.000000000","message":"Nit: So this isn\u0027t actually failing due to policy, this is testing the checks in the DB layer. Either way, its the correct return value.","commit_id":"4a09c2210b3c07343411a06c676c2d85aa0e214f"}],"nova/tests/unit/fake_policy.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"875601ba9b5e1525c8c36b6e2593d5ee60f52faa","unresolved":false,"context_lines":[{"line_number":101,"context_line":"    \"os_compute_api:os-server-tags:delete\": \"\","},{"line_number":102,"context_line":"    \"os_compute_api:os-server-tags:delete_all\": \"\","},{"line_number":103,"context_line":"    \"os_compute_api:os-server-usage\": \"\","},{"line_number":104,"context_line":"    \"os_compute_api:os-server-groups:show\": \"\","},{"line_number":105,"context_line":"    \"os_compute_api:os-server-groups:index\": \"\","},{"line_number":106,"context_line":"    \"os_compute_api:os-server-groups:create\": \"\","},{"line_number":107,"context_line":"    \"os_compute_api:os-server-groups:delete\": \"\","}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_504ec46f","line":104,"updated":"2016-11-09 10:20:49.000000000","message":"Do we need to add this any more?\n\nI would rather we take out the \"os_compute_api:os-server-usage\" rule.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f58fb16e757f2ca3abe112d2f194dceb6ad1b613","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    \"os_compute_api:os-server-tags:delete_all\": \"\","},{"line_number":103,"context_line":"    \"os_compute_api:os-server-usage\": \"\","},{"line_number":104,"context_line":"    \"os_compute_api:os-server-groups\": \"\","},{"line_number":105,"context_line":"    \"os_compute_api:os-server-groups:show\": \"\","},{"line_number":106,"context_line":"    \"os_compute_api:os-server-groups:index\": \"\","},{"line_number":107,"context_line":"    \"os_compute_api:os-server-groups:create\": \"\","},{"line_number":108,"context_line":"    \"os_compute_api:os-server-groups:delete\": \"\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_b0ce40b2","line":105,"updated":"2016-11-09 10:24:12.000000000","message":"Nit: I don\u0027t know why we need this here any more, I would rather we delete \"os_compute_api:os-server-groups\" rather than add the new ones.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":13637,"name":"Divya K Konoor","email":"dikonoor@in.ibm.com","username":"dikonoor"},"change_message_id":"6d5a1a00435d628660ece1a787610b4ea09872cd","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    \"os_compute_api:os-server-tags:delete_all\": \"\","},{"line_number":103,"context_line":"    \"os_compute_api:os-server-usage\": \"\","},{"line_number":104,"context_line":"    \"os_compute_api:os-server-groups\": \"\","},{"line_number":105,"context_line":"    \"os_compute_api:os-server-groups:show\": \"\","},{"line_number":106,"context_line":"    \"os_compute_api:os-server-groups:index\": \"\","},{"line_number":107,"context_line":"    \"os_compute_api:os-server-groups:create\": \"\","},{"line_number":108,"context_line":"    \"os_compute_api:os-server-groups:delete\": \"\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_fe2e5ea3","line":105,"in_reply_to":"9a629dbe_a3a77309","updated":"2016-11-11 12:32:47.000000000","message":"I think I missed the point that we are retaining this rule for backward compatibility .So, it\u0027s ok for it to be around till the support for the old rule is removed to ensure that the old rule works.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    \"os_compute_api:os-server-tags:delete_all\": \"\","},{"line_number":103,"context_line":"    \"os_compute_api:os-server-usage\": \"\","},{"line_number":104,"context_line":"    \"os_compute_api:os-server-groups\": \"\","},{"line_number":105,"context_line":"    \"os_compute_api:os-server-groups:show\": \"\","},{"line_number":106,"context_line":"    \"os_compute_api:os-server-groups:index\": \"\","},{"line_number":107,"context_line":"    \"os_compute_api:os-server-groups:create\": \"\","},{"line_number":108,"context_line":"    \"os_compute_api:os-server-groups:delete\": \"\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_c5084b30","line":105,"in_reply_to":"9a629dbe_b0ce40b2","updated":"2016-11-09 15:08:45.000000000","message":"without this test_all_rules_in_sample_file will break","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":13637,"name":"Divya K Konoor","email":"dikonoor@in.ibm.com","username":"dikonoor"},"change_message_id":"ccdfeb16ed74f0115e7a13e336ad75bcbfdf9a76","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    \"os_compute_api:os-server-tags:delete_all\": \"\","},{"line_number":103,"context_line":"    \"os_compute_api:os-server-usage\": \"\","},{"line_number":104,"context_line":"    \"os_compute_api:os-server-groups\": \"\","},{"line_number":105,"context_line":"    \"os_compute_api:os-server-groups:show\": \"\","},{"line_number":106,"context_line":"    \"os_compute_api:os-server-groups:index\": \"\","},{"line_number":107,"context_line":"    \"os_compute_api:os-server-groups:create\": \"\","},{"line_number":108,"context_line":"    \"os_compute_api:os-server-groups:delete\": \"\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_a3a77309","line":105,"in_reply_to":"9a629dbe_c5084b30","updated":"2016-11-11 11:29:53.000000000","message":"then shouldn\u0027t we go and fix that testcase ? If \"os_compute_api:os-server-groups\" is not a rule that is used by any REST flow, I don\u0027t think it should be listed here.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"}],"nova/tests/unit/test_policy.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"875601ba9b5e1525c8c36b6e2593d5ee60f52faa","unresolved":false,"context_lines":[{"line_number":401,"context_line":"\"os_compute_api:os-security-groups\","},{"line_number":402,"context_line":"\"os_compute_api:os-server-password\","},{"line_number":403,"context_line":"\"os_compute_api:os-server-usage\","},{"line_number":404,"context_line":"\"os_compute_api:os-server-groups:index\","},{"line_number":405,"context_line":"\"os_compute_api:os-server-groups:show\","},{"line_number":406,"context_line":"\"os_compute_api:os-server-groups:create\","},{"line_number":407,"context_line":"\"os_compute_api:os-server-groups:delete\","}],"source_content_type":"text/x-python","patch_set":6,"id":"9a629dbe_d042d43d","line":404,"updated":"2016-11-09 10:20:49.000000000","message":"Do we need to add this any more?\n\nI would rather we take out the \"os_compute_api:os-server-usage\" rule.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"323b226db8244d164c305606e4be34bf8e40feb2","unresolved":false,"context_lines":[{"line_number":16,"context_line":"\"\"\"Test of Policy Engine For Nova.\"\"\""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import os.path"},{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":"from oslo_policy import policy as oslo_policy"},{"line_number":21,"context_line":"from oslo_serialization import jsonutils"},{"line_number":22,"context_line":"import requests_mock"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a629dbe_906313dc","line":19,"updated":"2016-11-08 21:04:22.000000000","message":"The blank line that you\u0027ve removed here is actually desired. See http://docs.openstack.org/developer/hacking/#import-order-template","commit_id":"f7dcbf321eafcbff2453635118b1141cd55e049f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f58fb16e757f2ca3abe112d2f194dceb6ad1b613","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import os.path"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"import mock"},{"line_number":21,"context_line":"from oslo_policy import policy as oslo_policy"},{"line_number":22,"context_line":"from oslo_serialization import jsonutils"},{"line_number":23,"context_line":"import requests_mock"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_b0456018","line":20,"updated":"2016-11-09 10:24:12.000000000","message":"please don\u0027t make unrelated changes here.\n\nFWIW, mock is a standard library in python3, hence its included in the other group in some place. It will probably have to get moved there eventually anyways.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f58fb16e757f2ca3abe112d2f194dceb6ad1b613","unresolved":false,"context_lines":[{"line_number":402,"context_line":"\"os_compute_api:os-server-password\","},{"line_number":403,"context_line":"\"os_compute_api:os-server-usage\","},{"line_number":404,"context_line":"\"os_compute_api:os-server-groups\","},{"line_number":405,"context_line":"\"os_compute_api:os-server-groups:index\","},{"line_number":406,"context_line":"\"os_compute_api:os-server-groups:show\","},{"line_number":407,"context_line":"\"os_compute_api:os-server-groups:create\","},{"line_number":408,"context_line":"\"os_compute_api:os-server-groups:delete\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_10c0cc7a","line":405,"updated":"2016-11-09 10:24:12.000000000","message":"Nit: I don\u0027t know why we need this here any more, I would rather we delete \"os_compute_api:os-server-groups\" rather than add the new ones.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":13637,"name":"Divya K Konoor","email":"dikonoor@in.ibm.com","username":"dikonoor"},"change_message_id":"969e77e94e6521a757bbf90c569e10e8dbdf2e9f","unresolved":false,"context_lines":[{"line_number":402,"context_line":"\"os_compute_api:os-server-password\","},{"line_number":403,"context_line":"\"os_compute_api:os-server-usage\","},{"line_number":404,"context_line":"\"os_compute_api:os-server-groups\","},{"line_number":405,"context_line":"\"os_compute_api:os-server-groups:index\","},{"line_number":406,"context_line":"\"os_compute_api:os-server-groups:show\","},{"line_number":407,"context_line":"\"os_compute_api:os-server-groups:create\","},{"line_number":408,"context_line":"\"os_compute_api:os-server-groups:delete\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_c3238754","line":405,"in_reply_to":"9a629dbe_10c0cc7a","updated":"2016-11-11 11:39:46.000000000","message":"I tend to agree with John on removing an obsolete rule. I think we should fix the testcase instead of keeping a rule that is not applicable to any rest api call.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":402,"context_line":"\"os_compute_api:os-server-password\","},{"line_number":403,"context_line":"\"os_compute_api:os-server-usage\","},{"line_number":404,"context_line":"\"os_compute_api:os-server-groups\","},{"line_number":405,"context_line":"\"os_compute_api:os-server-groups:index\","},{"line_number":406,"context_line":"\"os_compute_api:os-server-groups:show\","},{"line_number":407,"context_line":"\"os_compute_api:os-server-groups:create\","},{"line_number":408,"context_line":"\"os_compute_api:os-server-groups:delete\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_05a7a34f","line":405,"in_reply_to":"9a629dbe_10c0cc7a","updated":"2016-11-09 15:08:45.000000000","message":"this has to be here or test_rule_missing will fail. It\u0027s also used in test_admin_or_owner_rules","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":13637,"name":"Divya K Konoor","email":"dikonoor@in.ibm.com","username":"dikonoor"},"change_message_id":"6d5a1a00435d628660ece1a787610b4ea09872cd","unresolved":false,"context_lines":[{"line_number":402,"context_line":"\"os_compute_api:os-server-password\","},{"line_number":403,"context_line":"\"os_compute_api:os-server-usage\","},{"line_number":404,"context_line":"\"os_compute_api:os-server-groups\","},{"line_number":405,"context_line":"\"os_compute_api:os-server-groups:index\","},{"line_number":406,"context_line":"\"os_compute_api:os-server-groups:show\","},{"line_number":407,"context_line":"\"os_compute_api:os-server-groups:create\","},{"line_number":408,"context_line":"\"os_compute_api:os-server-groups:delete\","}],"source_content_type":"text/x-python","patch_set":10,"id":"9a629dbe_7e116e5c","line":405,"in_reply_to":"9a629dbe_c3238754","updated":"2016-11-11 12:32:47.000000000","message":"I think I missed the point that we are retaining this rule for backward compatibility .So, it\u0027s ok for it to be around till the support for the old rule is removed to ensure that the old rule works.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"}],"releasenotes/notes/bug-1636157-2148ea3675969a5d.yaml":[{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"07da22e4eedc9dadb81be9ba29087ae0077ff39d","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     is being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It is thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To get rid "},{"line_number":8,"context_line":"     of this behavior changes are made to have separate policy rules "}],"source_content_type":"text/x-yaml","patch_set":5,"id":"9a629dbe_fc49c68f","line":5,"range":{"start_line":5,"start_character":5,"end_line":5,"end_character":7},"updated":"2016-11-07 19:13:03.000000000","message":"s/is/were/","commit_id":"a0851102e38492001872993724c94cdc09dd4dab"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"07da22e4eedc9dadb81be9ba29087ae0077ff39d","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     is being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It is thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To get rid "},{"line_number":8,"context_line":"     of this behavior changes are made to have separate policy rules "},{"line_number":9,"context_line":"     for each of the actions on server_groups."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"9a629dbe_1c4f5a93","line":6,"range":{"start_line":6,"start_character":37,"end_line":6,"end_character":39},"updated":"2016-11-07 19:13:03.000000000","message":"s/is/was/","commit_id":"a0851102e38492001872993724c94cdc09dd4dab"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"07da22e4eedc9dadb81be9ba29087ae0077ff39d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     is being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It is thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To get rid "},{"line_number":8,"context_line":"     of this behavior changes are made to have separate policy rules "},{"line_number":9,"context_line":"     for each of the actions on server_groups."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"9a629dbe_fcbb8673","line":7,"updated":"2016-11-07 19:13:03.000000000","message":"remove space from end of line","commit_id":"a0851102e38492001872993724c94cdc09dd4dab"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"07da22e4eedc9dadb81be9ba29087ae0077ff39d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     is being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It is thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To get rid "},{"line_number":8,"context_line":"     of this behavior changes are made to have separate policy rules "},{"line_number":9,"context_line":"     for each of the actions on server_groups."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"9a629dbe_7c5e9646","line":8,"range":{"start_line":7,"start_character":60,"end_line":8,"end_character":7},"updated":"2016-11-07 19:13:03.000000000","message":"correct","commit_id":"a0851102e38492001872993724c94cdc09dd4dab"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"07da22e4eedc9dadb81be9ba29087ae0077ff39d","unresolved":false,"context_lines":[{"line_number":5,"context_line":"     is being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It is thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To get rid "},{"line_number":8,"context_line":"     of this behavior changes are made to have separate policy rules "},{"line_number":9,"context_line":"     for each of the actions on server_groups."}],"source_content_type":"text/x-yaml","patch_set":5,"id":"9a629dbe_1cc11ae5","line":8,"updated":"2016-11-07 19:13:03.000000000","message":"remove space from end of line","commit_id":"a0851102e38492001872993724c94cdc09dd4dab"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"e291b44c84756fd03020096d2dac3df0c42ad246","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_e2bca06e","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":5},"updated":"2016-11-08 13:14:57.000000000","message":"This should be a upgrade note","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"1d3f30856b64cc6d5e33a8e7045f0581b5148c44","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_9e1c1415","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":5},"in_reply_to":"9a629dbe_05c0bedf","updated":"2016-11-08 15:35:23.000000000","message":"seems more like a fix than a feature to me. I think I agree about needing to call out that there\u0027s an upgrade impact, at least as currently coded. But if it\u0027s rewritten per John\u0027s suggestion to continue to use the old rule by default, then wouldn\u0027t that remove the upgrade impact?","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1d957969639706c97056bbaa398bd858b275e561","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_cdfa3fbd","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":5},"in_reply_to":"9a629dbe_9e1c1415","updated":"2016-11-09 10:06:28.000000000","message":"This is new functionality, you can customise create/delete separate to show/list using a new policy rule.\n\nI know missing functionality can look like a \"bug\", but there isn\u0027t technically anything broken with the existing RBAC. This just adds more granularity.\n\nI am not convinced (yet) that every API should have a separate independent policy rule. It leads to many non-sensical configurations. But I discuss that more on the follow up patch.\n\nUsually I would request this gets a blueprint or a spec to track it, but as we have release notes to track the new functionality, I think thats OK.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7375e9500e2729393795ef2cf421140597e2da78","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_05c0bedf","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":5},"in_reply_to":"9a629dbe_e2bca06e","updated":"2016-11-08 13:29:27.000000000","message":"Or feature I guess.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"e291b44c84756fd03020096d2dac3df0c42ad246","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It was thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To correct"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_a2fd483c","line":4,"range":{"start_line":4,"start_character":5,"end_line":4,"end_character":59},"updated":"2016-11-08 13:14:57.000000000","message":"There is check for raise error when any unregistered rule in the policy.json. I think we should tell the user explicitly that you need to remove the old rule \u0027os_compute_api:os-server-groups\u0027 from policy.json, otherwise the nova-api startup failed.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"1d3f30856b64cc6d5e33a8e7045f0581b5148c44","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It was thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To correct"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"9a629dbe_bef2b8d0","line":4,"range":{"start_line":4,"start_character":5,"end_line":4,"end_character":59},"in_reply_to":"9a629dbe_a2fd483c","updated":"2016-11-08 15:35:23.000000000","message":"Unless we rewrite it per John\u0027s suggestion to continue using the old rule by default, yes.","commit_id":"553081e9a8fd02e3be9239ada2f25ed255448296"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"00791852433d0b5305abc3f3ad219f69fa7b4876","unresolved":false,"context_lines":[{"line_number":6,"context_line":"     for server_groups REST APIs. It was thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To correct"},{"line_number":8,"context_line":"     this behavior changes are made to have separate policy rules"},{"line_number":9,"context_line":"     for each of the actions on server_groups."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"9a629dbe_7dac9835","line":9,"updated":"2016-11-08 20:54:43.000000000","message":"I would suggest adding something stating that the original rule is left for backward compatibility but will be removed in a future release.","commit_id":"285ff33a28516d013f41b8fc88ed528b622f4b9a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"}],"source_content_type":"text/x-yaml","patch_set":10,"id":"9a629dbe_a90d0092","line":2,"updated":"2016-11-09 09:59:29.000000000","message":"I think we said this is the wrong section.\n\nTo me this is a feature, or possibly an upgrade note.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"}],"source_content_type":"text/x-yaml","patch_set":10,"id":"9a629dbe_80480189","line":2,"in_reply_to":"9a629dbe_a90d0092","updated":"2016-11-09 15:08:45.000000000","message":"I see this as a defect (inconsistency with other APIs), but I can see the argument that it\u0027s a feature (missing capability). Depends on your viewpoint. I\u0027m fine either way.\n\nThere are no longer any immediate upgrade implications since the old policy rule is now referenced by the new rules in the later change sets. I could see having an upgrade note if we wanted to warn people to change to the new rules because the old rules will be going away (or we could have such an upgrade note when they actually do go away), but whether they will go away is up for debate (next comment).","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"     The same policy rule (os_compute_api:os-server-groups)"},{"line_number":5,"context_line":"     were being used for all actions (show, index, delete, create)"},{"line_number":6,"context_line":"     for server_groups REST APIs. It was thus impossible to provide"},{"line_number":7,"context_line":"     different RBAC for specific actions based on roles. To correct"},{"line_number":8,"context_line":"     this behavior changes are made to have separate policy rules"}],"source_content_type":"text/x-yaml","patch_set":10,"id":"9a629dbe_09234c06","line":5,"range":{"start_line":5,"start_character":5,"end_line":5,"end_character":9},"updated":"2016-11-09 09:59:29.000000000","message":"Nit: was","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70ffbdb241f0635f9f85e508a0e95cd9a12de2b6","unresolved":false,"context_lines":[{"line_number":8,"context_line":"     this behavior changes are made to have separate policy rules"},{"line_number":9,"context_line":"     for each of the actions on server_groups. The original rule"},{"line_number":10,"context_line":"     (os_compute_api:os-server-groups) is left unchanged for backward"},{"line_number":11,"context_line":"     compatibility but it will be removed in a future release."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"9a629dbe_8964bc49","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":62},"updated":"2016-11-09 09:59:29.000000000","message":"I don\u0027t think we should remove this in a future release.\n\nThe default for all the rules comes from os-server-groups, so if you want to limit the whole API, you can just tweak the one rule.\n\nIf you want only to change who can create or delete the group, you can override that independently.\n\nPlease note, the default policy file is now an empty file. This changes how I expect people to modify the policy file.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":10608,"name":"Matthew Edmonds","email":"edmondsw@us.ibm.com","username":"edmondsw"},"change_message_id":"3be5d0585d13a4acfa3e6d2fa1a34bafcbd7b2af","unresolved":false,"context_lines":[{"line_number":8,"context_line":"     this behavior changes are made to have separate policy rules"},{"line_number":9,"context_line":"     for each of the actions on server_groups. The original rule"},{"line_number":10,"context_line":"     (os_compute_api:os-server-groups) is left unchanged for backward"},{"line_number":11,"context_line":"     compatibility but it will be removed in a future release."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"9a629dbe_a05fc53f","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":62},"in_reply_to":"9a629dbe_8964bc49","updated":"2016-11-09 15:08:45.000000000","message":"I see what you\u0027re saying. Leaving this makes it inconsistent with most other APIs, though. Consistency is an important goal as we\u0027re cleaning up these kinds of issues. If we agree to leave this, then I think we need to go add something similar to all other APIs. Not under this changeset, of course. Agreed?","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"2f9dc74b62efdceebbfba79ce1a341dba890dd81","unresolved":false,"context_lines":[{"line_number":8,"context_line":"     this behavior changes are made to have separate policy rules"},{"line_number":9,"context_line":"     for each of the actions on server_groups. The original rule"},{"line_number":10,"context_line":"     (os_compute_api:os-server-groups) is left unchanged for backward"},{"line_number":11,"context_line":"     compatibility but it will be removed in a future release."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"7a77a97e_473911d8","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":62},"in_reply_to":"9a629dbe_a05fc53f","updated":"2016-11-21 12:15:28.000000000","message":"Yes, that rework of the policy rules is pencilled in for next cycle.\n\nMy problem is telling people this rule is going away, even though I would -2 such a patch. We need a proper deprecation system before we could remove the rule, and I rather think we should keep those rules alive.","commit_id":"c13b7f2a573eec6652d8ec3cb57275640b027fd1"}]}