)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"b36c3b6f5ab55f54c56b5077abbbf0e75d6a1c47","unresolved":false,"context_lines":[{"line_number":11,"context_line":"for them will be removed in the future. So this patch adds a warning"},{"line_number":12,"context_line":"message when people use a rule that is user-based."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Partially implements blueprint user_id_based_policy_enforcement"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Co-Authored-By: Ed Leafe \u003ced@leafe.com\u003e"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"fa7ab95a_46c1a253","line":14,"range":{"start_line":14,"start_character":31,"end_line":14,"end_character":63},"updated":"2016-08-29 11:27:02.000000000","message":"this should be \u0027user-id-based-policy-enforcement\u0027","commit_id":"c95413279e68e63afaec8645889efbac377b2f29"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"ec3909351aab3363df0f5d829c739d7ddef6e439","unresolved":false,"context_lines":[{"line_number":11,"context_line":"for them will be removed in the future. So this patch adds a warning"},{"line_number":12,"context_line":"message when people use a rule that is user-based."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Partially implements blueprint user_id_based_policy_enforcement"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Co-Authored-By: Ed Leafe \u003ced@leafe.com\u003e"},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"fa7ab95a_ac15ef40","line":14,"range":{"start_line":14,"start_character":31,"end_line":14,"end_character":63},"in_reply_to":"fa7ab95a_46c1a253","updated":"2016-08-29 11:46:02.000000000","message":"yea, thanks","commit_id":"c95413279e68e63afaec8645889efbac377b2f29"}],"nova/context.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"afc4e9917667b07af74f5c5a7ec9008672e230bf","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            If None, then this default target will be considered:"},{"line_number":246,"context_line":"            {\u0027project_id\u0027: self.project_id, \u0027user_id\u0027: self.user_id}"},{"line_number":247,"context_line":"        :param fatal: if False, will return False when an exception.Forbidden"},{"line_number":248,"context_line":"           occurs."},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        :raises nova.exception.Forbidden: if verification fails and fatal is"},{"line_number":251,"context_line":"            True."}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_ae592690","line":248,"updated":"2016-08-18 09:25:50.000000000","message":"Coud you keep the docstring in sync with the signature?","commit_id":"c74dca852613baa361b21814312527ed14c2c9e0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"afc4e9917667b07af74f5c5a7ec9008672e230bf","unresolved":false,"context_lines":[{"line_number":268,"context_line":"            if rule in rules:"},{"line_number":269,"context_line":"                keys \u003d re.findall(\u0027%\\((\\w+)\\)s\u0027, str(rules[rule]))"},{"line_number":270,"context_line":"                if set(keys) - set([\u0027project_id\u0027]) and not user_based:"},{"line_number":271,"context_line":"                    LOG.warning(_LW(\u0027Only the project_id attribute is \u0027"},{"line_number":272,"context_line":"                                    \u0027supported in target for this resource. \u0027"},{"line_number":273,"context_line":"                                    \u0027Other attributes will be deprecated\u0027))"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_09b4b432","line":271,"updated":"2016-08-18 09:25:50.000000000","message":"As far as I understand this will log the warning every time the affected API is used, e.g. every time a /os-keypairs is called. It will spam the log with repetitive messages.","commit_id":"c74dca852613baa361b21814312527ed14c2c9e0"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"1f61b131b06dcc31abcdc729a6747017819ca6f9","unresolved":false,"context_lines":[{"line_number":268,"context_line":"            if rule in rules:"},{"line_number":269,"context_line":"                keys \u003d re.findall(\u0027%\\((\\w+)\\)s\u0027, str(rules[rule]))"},{"line_number":270,"context_line":"                if set(keys) - set([\u0027project_id\u0027]) and not user_based:"},{"line_number":271,"context_line":"                    LOG.warning(_LW(\u0027Only the project_id attribute is \u0027"},{"line_number":272,"context_line":"                                    \u0027supported in target for this resource. \u0027"},{"line_number":273,"context_line":"                                    \u0027Other attributes will be deprecated\u0027))"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_5b52d967","line":271,"in_reply_to":"3ac371cc_09b4b432","updated":"2016-08-18 14:33:02.000000000","message":"Yeah, could we not check here:\nhttps://github.com/openstack/nova/blob/master/nova/policy.py#L58\n\nI think we can warn the user if *any* rules include user_id","commit_id":"c74dca852613baa361b21814312527ed14c2c9e0"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f9ca2aa29decc39a88881cc47980106d8b751ed6","unresolved":false,"context_lines":[{"line_number":268,"context_line":"            if rule in rules:"},{"line_number":269,"context_line":"                keys \u003d re.findall(\u0027%\\((\\w+)\\)s\u0027, str(rules[rule]))"},{"line_number":270,"context_line":"                if set(keys) - set([\u0027project_id\u0027]) and not user_based:"},{"line_number":271,"context_line":"                    LOG.warning(_LW(\u0027Only the project_id attribute is \u0027"},{"line_number":272,"context_line":"                                    \u0027supported in target for this resource. \u0027"},{"line_number":273,"context_line":"                                    \u0027Other attributes will be deprecated\u0027))"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3ac371cc_a1ac7e8c","line":271,"in_reply_to":"3ac371cc_5b52d967","updated":"2016-08-18 14:51:26.000000000","message":"Balazs, John, yes, that is good point. I will take a look at the way warn th e message when startup","commit_id":"c74dca852613baa361b21814312527ed14c2c9e0"}],"nova/policy.py":[{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"8accbd52cf45c22991f808d69f37e6f23aa22038","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    \"\"\""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    global _ENFORCER"},{"line_number":57,"context_line":"    global saved_rules"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    if not _ENFORCER:"},{"line_number":60,"context_line":"        _ENFORCER \u003d policy.Enforcer(CONF,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_ee1c77c3","line":57,"range":{"start_line":57,"start_character":4,"end_line":57,"end_character":22},"updated":"2016-08-19 07:50:33.000000000","message":"imho it\u0027s worth a comment about that saved_rules cache. Also could you please highlight use case for such caching?","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"137dc23535fc59400d07c5f5bb8597fcd62d5292","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    \"\"\""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    global _ENFORCER"},{"line_number":57,"context_line":"    global saved_rules"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    if not _ENFORCER:"},{"line_number":60,"context_line":"        _ENFORCER \u003d policy.Enforcer(CONF,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_0f510a0f","line":57,"range":{"start_line":57,"start_character":4,"end_line":57,"end_character":22},"in_reply_to":"3ac371cc_ee1c77c3","updated":"2016-08-19 09:22:17.000000000","message":"yea, will update","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"8accbd52cf45c22991f808d69f37e6f23aa22038","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"def _warning_for_deprecated_user_based_rules(rules):"},{"line_number":84,"context_line":"    user_based_resources \u003d [\u0027os-keypairs\u0027]"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    for rule in rules:"},{"line_number":87,"context_line":"        user_based_rule \u003d [resource for resource in user_based_resources"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_8ee6a3e6","line":84,"range":{"start_line":84,"start_character":4,"end_line":84,"end_character":42},"updated":"2016-08-19 07:50:33.000000000","message":"what about moving this to constant?","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"137dc23535fc59400d07c5f5bb8597fcd62d5292","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"def _warning_for_deprecated_user_based_rules(rules):"},{"line_number":84,"context_line":"    user_based_resources \u003d [\u0027os-keypairs\u0027]"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    for rule in rules:"},{"line_number":87,"context_line":"        user_based_rule \u003d [resource for resource in user_based_resources"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_6f56be07","line":84,"range":{"start_line":84,"start_character":4,"end_line":84,"end_character":42},"in_reply_to":"3ac371cc_8ee6a3e6","updated":"2016-08-19 09:22:17.000000000","message":"Emm...i\u0027m ok both","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"8accbd52cf45c22991f808d69f37e6f23aa22038","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    user_based_resources \u003d [\u0027os-keypairs\u0027]"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    for rule in rules:"},{"line_number":87,"context_line":"        user_based_rule \u003d [resource for resource in user_based_resources"},{"line_number":88,"context_line":"                           if resource in rule]"},{"line_number":89,"context_line":"        keys \u003d re.findall(\u0027%\\((\\w+)\\)s\u0027, str(rules[rule]))"},{"line_number":90,"context_line":"        if \u0027user_id\u0027 in keys and not user_based_rule:"},{"line_number":91,"context_line":"            LOG.warning(_LW(\"The user_id attribute isn\u0027t supported in the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_2ed7af24","line":88,"range":{"start_line":87,"start_character":8,"end_line":88,"end_character":47},"updated":"2016-08-19 07:50:33.000000000","message":"I\u0027d check that first, before the regexp, smth like:\n\nif user_base_rule:\n    continue","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"137dc23535fc59400d07c5f5bb8597fcd62d5292","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    user_based_resources \u003d [\u0027os-keypairs\u0027]"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    for rule in rules:"},{"line_number":87,"context_line":"        user_based_rule \u003d [resource for resource in user_based_resources"},{"line_number":88,"context_line":"                           if resource in rule]"},{"line_number":89,"context_line":"        keys \u003d re.findall(\u0027%\\((\\w+)\\)s\u0027, str(rules[rule]))"},{"line_number":90,"context_line":"        if \u0027user_id\u0027 in keys and not user_based_rule:"},{"line_number":91,"context_line":"            LOG.warning(_LW(\"The user_id attribute isn\u0027t supported in the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_8f3dfabd","line":88,"range":{"start_line":87,"start_character":8,"end_line":88,"end_character":47},"in_reply_to":"3ac371cc_2ed7af24","updated":"2016-08-19 09:22:17.000000000","message":"good point, will update.","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b152af6ae8f56e9b34addb1f4df16d7b56ce821a","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        register_rules(_ENFORCER)"},{"line_number":71,"context_line":"        _ENFORCER.load_rules()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    current_rules \u003d get_rules()"},{"line_number":74,"context_line":"    if _serialize_rules(saved_rules) !\u003d _serialize_rules(current_rules):"},{"line_number":75,"context_line":"        _warning_for_deprecated_user_based_rules(current_rules)"},{"line_number":76,"context_line":"        saved_rules \u003d copy.deepcopy(current_rules)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3ac371cc_45ebe1b0","line":73,"updated":"2016-08-19 09:58:06.000000000","message":"If feels like we should only do this check for the case where _ENFORCER is None?\n\nI guess we might end up logging this once for every api file that is loaded, if we don\u0027t do that?","commit_id":"bed9b86d901784c68d1e34215ff6fe5f7e83ca2e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a8d3b13308912ae2caebba0c213303ece8751c1f","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        _ENFORCER.load_rules()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    current_rules \u003d get_rules()"},{"line_number":74,"context_line":"    if _serialize_rules(saved_rules) !\u003d _serialize_rules(current_rules):"},{"line_number":75,"context_line":"        _warning_for_deprecated_user_based_rules(current_rules)"},{"line_number":76,"context_line":"        saved_rules \u003d copy.deepcopy(current_rules)"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1ac06dbe_432ca6ab","line":74,"updated":"2016-08-19 10:17:46.000000000","message":"ah, so I see the idea here is to detect the reload of the policy.\n\nThis seems really expensive for every API call, maybe we should just only check for this at start up?\n\nWe could wait till we have some oslo.polcy callback for when a reload occurs?","commit_id":"bed9b86d901784c68d1e34215ff6fe5f7e83ca2e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5a38831a10828296279bbdbd8d2a4cab89307d23","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    current_rules \u003d get_rules()"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    # Checks whether the rules are updated in the runtime"},{"line_number":78,"context_line":"    if _serialize_rules(saved_rules) !\u003d _serialize_rules(current_rules):"},{"line_number":79,"context_line":"        _warning_for_deprecated_user_based_rules(current_rules)"},{"line_number":80,"context_line":"        saved_rules \u003d copy.deepcopy(current_rules)"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_9278a291","line":78,"updated":"2016-08-19 11:28:27.000000000","message":"Not sure I like doing this on every API call.\n\nI would rather we only check the policy on reboot, even though the policy may have been reloaded.","commit_id":"b18b635995196a640dc73ed441e79df41f3611f8"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"a1c3fa042b9af46a744c92b7fc599ab348c1879d","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    current_rules \u003d get_rules()"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    # Checks whether the rules are updated in the runtime"},{"line_number":78,"context_line":"    if _serialize_rules(saved_rules) !\u003d _serialize_rules(current_rules):"},{"line_number":79,"context_line":"        _warning_for_deprecated_user_based_rules(current_rules)"},{"line_number":80,"context_line":"        saved_rules \u003d copy.deepcopy(current_rules)"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_784ea5c2","line":78,"in_reply_to":"1ac06dbe_9278a291","updated":"2016-08-19 12:08:10.000000000","message":"so, honestly, I think I\u0027m ok with this on every evaluation, because it will be clear that this API call isn\u0027t getting enforced the way that people think.","commit_id":"b18b635995196a640dc73ed441e79df41f3611f8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"7f565e3e4525d4f5888cb37db2188e255cc10d51","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        # policy enforcement."},{"line_number":102,"context_line":"        if [resource for resource in USER_BASED_RESOURCES if resource in rule]:"},{"line_number":103,"context_line":"            continue"},{"line_number":104,"context_line":"        keys \u003d re.findall(\u0027%\\((\\w+)\\)s\u0027, str(rules[rule]))"},{"line_number":105,"context_line":"        if \u0027user_id\u0027 in keys:"},{"line_number":106,"context_line":"            LOG.warning(_LW(\"The user_id attribute isn\u0027t supported in the \""},{"line_number":107,"context_line":"                            \"rule \u0027%s\u0027. All the user_id based policy \""}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_dd850b08","line":104,"updated":"2016-08-24 13:16:46.000000000","message":"So maybe we should compile this one, it would make me feel better about the whole thing.","commit_id":"b18b635995196a640dc73ed441e79df41f3611f8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5a38831a10828296279bbdbd8d2a4cab89307d23","unresolved":false,"context_lines":[{"line_number":150,"context_line":"           authorized, and the exact value False if not authorized and"},{"line_number":151,"context_line":"           do_raise is False."},{"line_number":152,"context_line":"    \"\"\""},{"line_number":153,"context_line":"    init()"},{"line_number":154,"context_line":"    credentials \u003d context.to_dict()"},{"line_number":155,"context_line":"    if not exc:"},{"line_number":156,"context_line":"        exc \u003d exception.PolicyNotAuthorized"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ac06dbe_123792e4","line":153,"updated":"2016-08-19 11:28:27.000000000","message":"turns out we call init quite a lot.","commit_id":"b18b635995196a640dc73ed441e79df41f3611f8"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"f4eb440077d07d28fdd9213d1e2bb63a249f1ec5","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    credentials \u003d context.to_dict()"},{"line_number":152,"context_line":"    if not exc:"},{"line_number":153,"context_line":"        exc \u003d exception.PolicyNotAuthorized"},{"line_number":154,"context_line":"    try:"},{"line_number":155,"context_line":"        result \u003d _ENFORCER.authorize(action, target, credentials,"},{"line_number":156,"context_line":"                                     do_raise\u003ddo_raise, exc\u003dexc, action\u003daction)"},{"line_number":157,"context_line":"    except policy.PolicyNotRegistered:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ac06dbe_97f7dd8b","line":154,"updated":"2016-08-24 18:25:09.000000000","message":"I would expect it to be cleaner to just log when enforcement happens here. And seeing the complexity of what\u0027s being done above I\u0027m now in favor of just logging on each violating enforcement.\n\nHowever, all of this parsing seems overly complex. Why not just log a message at startup that says that any rules which use user_id as a target, except keypairs, are deprecated? Don\u0027t even worry about parsing to see if one of these rules are in use. An operator should be able to \u0027grep user_id /etc/nova/policy.json\u0027 to see if they violate this.","commit_id":"0faa656d3ffd06998bda217bbf93af997f6dba68"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"76e618622d487448c824e40151edf1bdf7572777","unresolved":false,"context_lines":[{"line_number":151,"context_line":"    credentials \u003d context.to_dict()"},{"line_number":152,"context_line":"    if not exc:"},{"line_number":153,"context_line":"        exc \u003d exception.PolicyNotAuthorized"},{"line_number":154,"context_line":"    try:"},{"line_number":155,"context_line":"        result \u003d _ENFORCER.authorize(action, target, credentials,"},{"line_number":156,"context_line":"                                     do_raise\u003ddo_raise, exc\u003dexc, action\u003daction)"},{"line_number":157,"context_line":"    except policy.PolicyNotRegistered:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ac06dbe_75f77959","line":154,"in_reply_to":"1ac06dbe_97f7dd8b","updated":"2016-08-25 14:30:17.000000000","message":"yea, that sounds a easy solution.","commit_id":"0faa656d3ffd06998bda217bbf93af997f6dba68"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"848bacb27bcea0f78ba60ea64b5aaea74b5856d9","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    current_file_rules_set \u003d _get_rules_set(current_file_rules)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    # Checks whether the rules are updated in the runtime"},{"line_number":82,"context_line":"    if saved_file_rules_set !\u003d current_file_rules_set:"},{"line_number":83,"context_line":"        _warning_for_deprecated_user_based_rules(current_file_rules)"},{"line_number":84,"context_line":"        saved_file_rules_set \u003d current_file_rules_set"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa7ab95a_b9c84576","line":82,"range":{"start_line":82,"start_character":4,"end_line":82,"end_character":54},"updated":"2016-08-26 05:36:15.000000000","message":"Only the code before this line will be executed in each API call. \n\nI did some optimize for the code.\n* In line 78, only read the rules which read from file. The default rule won\u0027t be changed, the changed rules are only come from the policy.json. This reduce the number of rules we want to compare.\n* In line 79, just generate a set for rules instead of generate a string for each rule.\n\nI tested this code execute 1000 times\nWhen user only uses the default rule, it uses 0.009 seconds\nWhen user custom all the policy rules, it uses 0.37 seconds","commit_id":"825f891f10b64adfc5b3aad1ccaf833de52536bb"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0b6554374b1b469783b43e8f0e783110361d31de","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":"_ENFORCER \u003d None"},{"line_number":34,"context_line":"# This list is about the resources which support user based policy enforcement."},{"line_number":35,"context_line":"# Avoid to send deprecation warning for those resources."},{"line_number":36,"context_line":"USER_BASED_RESOURCES \u003d [\u0027os-keypairs\u0027]"},{"line_number":37,"context_line":"# Due to oslo_policy will read the policy configure file again when the file"},{"line_number":38,"context_line":"# is changed in runtime, the old policy rules will be saved to saved_rules and"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_52baa0e2","line":35,"updated":"2016-08-26 11:00:21.000000000","message":"nit: s/to send/sending/","commit_id":"c45e6597c23d61db3659a87f490be4c4ff95f969"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f4e875c6b07f3661b619346e0a38872bf609ef87","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":"_ENFORCER \u003d None"},{"line_number":34,"context_line":"# This list is about the resources which support user based policy enforcement."},{"line_number":35,"context_line":"# Avoid to send deprecation warning for those resources."},{"line_number":36,"context_line":"USER_BASED_RESOURCES \u003d [\u0027os-keypairs\u0027]"},{"line_number":37,"context_line":"# Due to oslo_policy will read the policy configure file again when the file"},{"line_number":38,"context_line":"# is changed in runtime, the old policy rules will be saved to saved_rules and"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_bedad276","line":35,"in_reply_to":"fa7ab95a_52baa0e2","updated":"2016-08-26 12:38:23.000000000","message":"Done","commit_id":"c45e6597c23d61db3659a87f490be4c4ff95f969"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0b6554374b1b469783b43e8f0e783110361d31de","unresolved":false,"context_lines":[{"line_number":37,"context_line":"# Due to oslo_policy will read the policy configure file again when the file"},{"line_number":38,"context_line":"# is changed in runtime, the old policy rules will be saved to saved_rules and"},{"line_number":39,"context_line":"# it is used to compare with new rules to ensure the rules whether updated in"},{"line_number":40,"context_line":"# runtime."},{"line_number":41,"context_line":"saved_file_rules_set \u003d set([])"},{"line_number":42,"context_line":"KEY_EXPR \u003d re.compile(r\u0027%\\((\\w+)\\)s\u0027)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_d2a3109d","line":40,"updated":"2016-08-26 11:00:21.000000000","message":"nit:\n\noslo_policy will read the policy configuration file again when the file is changed in runtime so the old policy rules will be saved to saved_rules and used to compare with the new rules to determine whether the rules were updated.","commit_id":"c45e6597c23d61db3659a87f490be4c4ff95f969"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f4e875c6b07f3661b619346e0a38872bf609ef87","unresolved":false,"context_lines":[{"line_number":37,"context_line":"# Due to oslo_policy will read the policy configure file again when the file"},{"line_number":38,"context_line":"# is changed in runtime, the old policy rules will be saved to saved_rules and"},{"line_number":39,"context_line":"# it is used to compare with new rules to ensure the rules whether updated in"},{"line_number":40,"context_line":"# runtime."},{"line_number":41,"context_line":"saved_file_rules_set \u003d set([])"},{"line_number":42,"context_line":"KEY_EXPR \u003d re.compile(r\u0027%\\((\\w+)\\)s\u0027)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_3e6322b3","line":40,"in_reply_to":"fa7ab95a_d2a3109d","updated":"2016-08-26 12:38:23.000000000","message":"thanks","commit_id":"c45e6597c23d61db3659a87f490be4c4ff95f969"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"0b6554374b1b469783b43e8f0e783110361d31de","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                            \"rule \u0027%s\u0027. All the user_id based policy \""},{"line_number":105,"context_line":"                            \"enforcement will be removed in the \""},{"line_number":106,"context_line":"                            \"future.\") % rule)"},{"line_number":107,"context_line":"            pass"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"def set_rules(rules, overwrite\u003dTrue, use_conf\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_b2112caf","line":107,"updated":"2016-08-26 11:00:21.000000000","message":"What does this do, if anything?","commit_id":"c45e6597c23d61db3659a87f490be4c4ff95f969"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f4e875c6b07f3661b619346e0a38872bf609ef87","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                            \"rule \u0027%s\u0027. All the user_id based policy \""},{"line_number":105,"context_line":"                            \"enforcement will be removed in the \""},{"line_number":106,"context_line":"                            \"future.\") % rule)"},{"line_number":107,"context_line":"            pass"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"def set_rules(rules, overwrite\u003dTrue, use_conf\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_7ef84ae6","line":107,"in_reply_to":"fa7ab95a_b2112caf","updated":"2016-08-26 12:38:23.000000000","message":"oops, this added when I debug the code, forget to remove it. thanks.","commit_id":"c45e6597c23d61db3659a87f490be4c4ff95f969"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"e506066b6736962c742edbd5f90f8910aebcb94d","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    \"\"\"Serialize all the Rule object as string and put into the set which is"},{"line_number":88,"context_line":"    used to compare the rules set."},{"line_number":89,"context_line":"    \"\"\""},{"line_number":90,"context_line":"    return set([str(rule) for rule in six.itervalues(rules)])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def _warning_for_deprecated_user_based_rules(rules):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_6416db56","line":90,"updated":"2016-08-26 20:33:03.000000000","message":"I think what was in patch #6 was more correct, whereby your result was based on the tuple of the rule name (which should be unique, e.g. \"os_compute_api:servers:detail:get_all_tenants\") and rule (which is not guaranteed to be unique, e.g. \"is_admin:True\"). If you are going for a simpler list to compare with, I think iterating the keys would be better than values as here.","commit_id":"9bc7822c31454963aa92b4f4458d68c338436c9b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"c3109b9820cfea5f2ebb3951683ac44f7f332117","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    \"\"\"Serialize all the Rule object as string and put into the set which is"},{"line_number":88,"context_line":"    used to compare the rules set."},{"line_number":89,"context_line":"    \"\"\""},{"line_number":90,"context_line":"    return set([str(rule) for rule in six.itervalues(rules)])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def _warning_for_deprecated_user_based_rules(rules):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa7ab95a_62322712","line":90,"in_reply_to":"fa7ab95a_6416db56","updated":"2016-08-29 02:57:32.000000000","message":"John, you are right, I missed that point, thanks. I need to detect the change for key and value, so I probably need to back to ps6","commit_id":"9bc7822c31454963aa92b4f4458d68c338436c9b"}],"nova/tests/unit/test_policy.py":[{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"8accbd52cf45c22991f808d69f37e6f23aa22038","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        policy.authorize(admin_context, lowercase_action, self.target)"},{"line_number":158,"context_line":"        policy.authorize(admin_context, uppercase_action, self.target)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    @mock.patch.object(policy.LOG, \u0027warning\u0027)"},{"line_number":161,"context_line":"    def test_warning_when_deprecated_user_based_rule_used(self, mock_warning):"},{"line_number":162,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":163,"context_line":"            oslo_policy.Rules.from_dict({"},{"line_number":164,"context_line":"                \"os_compute_api:servers:index\":"},{"line_number":165,"context_line":"                \"project_id:%(project_id)s or user_id:%(user_id)s\"}))"},{"line_number":166,"context_line":"        mock_warning.assert_called_once_with("},{"line_number":167,"context_line":"            u\"The user_id attribute isn\u0027t supported in the rule \""},{"line_number":168,"context_line":"             \"\u0027os_compute_api:servers:index\u0027. All the user_id based policy \""},{"line_number":169,"context_line":"             \"enforcement will be removed in the future.\")"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    @mock.patch.object(policy.LOG, \u0027warning\u0027)"},{"line_number":172,"context_line":"    def test_no_warning_for_user_based_resource(self, mock_warning):"},{"line_number":173,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":174,"context_line":"            oslo_policy.Rules.from_dict({"},{"line_number":175,"context_line":"                \"os_compute_api:os-keypairs:index\":"},{"line_number":176,"context_line":"                \"user_id:%(user_id)s\"}))"},{"line_number":177,"context_line":"        mock_warning.assert_not_called()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"class IsAdminCheckTestCase(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_6ed527b8","line":177,"range":{"start_line":160,"start_character":3,"end_line":177,"end_character":40},"updated":"2016-08-19 07:50:33.000000000","message":"what about an additional test, that will check that there are no warning fo no user-based rule?","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"137dc23535fc59400d07c5f5bb8597fcd62d5292","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        policy.authorize(admin_context, lowercase_action, self.target)"},{"line_number":158,"context_line":"        policy.authorize(admin_context, uppercase_action, self.target)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    @mock.patch.object(policy.LOG, \u0027warning\u0027)"},{"line_number":161,"context_line":"    def test_warning_when_deprecated_user_based_rule_used(self, mock_warning):"},{"line_number":162,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":163,"context_line":"            oslo_policy.Rules.from_dict({"},{"line_number":164,"context_line":"                \"os_compute_api:servers:index\":"},{"line_number":165,"context_line":"                \"project_id:%(project_id)s or user_id:%(user_id)s\"}))"},{"line_number":166,"context_line":"        mock_warning.assert_called_once_with("},{"line_number":167,"context_line":"            u\"The user_id attribute isn\u0027t supported in the rule \""},{"line_number":168,"context_line":"             \"\u0027os_compute_api:servers:index\u0027. All the user_id based policy \""},{"line_number":169,"context_line":"             \"enforcement will be removed in the future.\")"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    @mock.patch.object(policy.LOG, \u0027warning\u0027)"},{"line_number":172,"context_line":"    def test_no_warning_for_user_based_resource(self, mock_warning):"},{"line_number":173,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":174,"context_line":"            oslo_policy.Rules.from_dict({"},{"line_number":175,"context_line":"                \"os_compute_api:os-keypairs:index\":"},{"line_number":176,"context_line":"                \"user_id:%(user_id)s\"}))"},{"line_number":177,"context_line":"        mock_warning.assert_not_called()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"class IsAdminCheckTestCase(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_2fcfa6b4","line":177,"range":{"start_line":160,"start_character":3,"end_line":177,"end_character":40},"in_reply_to":"3ac371cc_6ed527b8","updated":"2016-08-19 09:22:17.000000000","message":"Actually test_warning_when_deprecated_user_based_rule_used can ensure there is only one warning. For no user-baed rule, there is no warning send out.","commit_id":"109e4f9293698e7958f47b4d54868b4b4d030613"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"b36c3b6f5ab55f54c56b5077abbbf0e75d6a1c47","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":173,"context_line":"            [(\"os_compute_api:os-keypairs:index\","},{"line_number":174,"context_line":"                \"user_id:%(user_id)s\")])"},{"line_number":175,"context_line":"        mock_warning.assert_not_called()"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch.object(policy.LOG, \u0027warning\u0027)"},{"line_number":178,"context_line":"    def test_no_warning_for_no_user_based_rule(self, mock_warning):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_4106ec1d","line":175,"range":{"start_line":175,"start_character":8,"end_line":175,"end_character":40},"updated":"2016-08-29 11:27:02.000000000","message":"it seems that I see somewhere that using below is better:\n\n    self.assertFalse(mock_warning.called)","commit_id":"c95413279e68e63afaec8645889efbac377b2f29"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"ec3909351aab3363df0f5d829c739d7ddef6e439","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":173,"context_line":"            [(\"os_compute_api:os-keypairs:index\","},{"line_number":174,"context_line":"                \"user_id:%(user_id)s\")])"},{"line_number":175,"context_line":"        mock_warning.assert_not_called()"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch.object(policy.LOG, \u0027warning\u0027)"},{"line_number":178,"context_line":"    def test_no_warning_for_no_user_based_rule(self, mock_warning):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_e1a8e022","line":175,"range":{"start_line":175,"start_character":8,"end_line":175,"end_character":40},"in_reply_to":"fa7ab95a_4106ec1d","updated":"2016-08-29 11:46:02.000000000","message":"is there any any reason why use self.assertFalse(mock_warning.called) is better?","commit_id":"c95413279e68e63afaec8645889efbac377b2f29"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"292d6f98217c49dbb07b3df809eb295372d747b1","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":173,"context_line":"            [(\"os_compute_api:os-keypairs:index\","},{"line_number":174,"context_line":"                \"user_id:%(user_id)s\")])"},{"line_number":175,"context_line":"        mock_warning.assert_not_called()"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @mock.patch.object(policy.LOG, \u0027warning\u0027)"},{"line_number":178,"context_line":"    def test_no_warning_for_no_user_based_rule(self, mock_warning):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_1947fc4b","line":175,"range":{"start_line":175,"start_character":8,"end_line":175,"end_character":40},"in_reply_to":"fa7ab95a_e1a8e022","updated":"2016-08-29 12:18:38.000000000","message":"No, this is fine. I think the advice was to not use mock.call_count \u003d\u003d 0 or something like that.","commit_id":"c95413279e68e63afaec8645889efbac377b2f29"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"b36c3b6f5ab55f54c56b5077abbbf0e75d6a1c47","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        policy._warning_for_deprecated_user_based_rules("},{"line_number":180,"context_line":"            [(\"os_compute_api:servers:index\","},{"line_number":181,"context_line":"                \"project_id:%(project_id)s\")])"},{"line_number":182,"context_line":"        mock_warning.assert_not_called()"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"class IsAdminCheckTestCase(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa7ab95a_c112fc56","line":182,"updated":"2016-08-29 11:27:02.000000000","message":"the same","commit_id":"c95413279e68e63afaec8645889efbac377b2f29"}]}