)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"87835c51175a062e661d07a1a0e60bc83a32eb2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9404662f_2c609a0a","updated":"2022-02-16 11:43:05.000000000","message":"I have questions inline","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c9278093e65711fbd282a15c56900aa8d4bc8da4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3a24bf10_e27e550e","updated":"2022-02-10 20:59:35.000000000","message":"recheck","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8333771c93c5213a7a8b93cddbaf8c048584599e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ebeac600_e91a06e3","updated":"2022-02-10 23:05:57.000000000","message":"recheck","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"46831e80_165c902b","updated":"2022-02-23 16:03:01.000000000","message":"Eventually turning my vote to +2 as I better understand the reasoning behind this large patch.\nThanks gmann for the hard work.","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f7036593deaf8d2f4f3d227a1fa685a316c3f44c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b23bfc2c_800c511c","updated":"2022-02-23 15:40:36.000000000","message":"question out loud","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"35dffd693694b0219ec2c5f8878531ef81c48404","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"52c509f3_e39ee3ad","updated":"2022-02-24 20:53:27.000000000","message":"recheck","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"02899a3cff87225470812eafbe2d8722ea4608c4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e9e483cd_e6009400","updated":"2022-02-20 04:30:14.000000000","message":"recheck","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/api/openstack/compute/server_groups.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"87835c51175a062e661d07a1a0e60bc83a32eb2f","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            # new defaults completly then we can remove the above check."},{"line_number":168,"context_line":"            # Until then, let\u0027s keep the old behaviour."},{"line_number":169,"context_line":"            context.can(sg_policies.POLICY_ROOT % \u0027index:all_projects\u0027,"},{"line_number":170,"context_line":"                        target\u003d{\u0027project_id\u0027: project_id})"},{"line_number":171,"context_line":"            sgs \u003d objects.InstanceGroupList.get_all(context)"},{"line_number":172,"context_line":"        else:"},{"line_number":173,"context_line":"            sgs \u003d objects.InstanceGroupList.get_by_project_id("}],"source_content_type":"text/x-python","patch_set":2,"id":"ab73b1f9_f9bfc0cf","line":170,"updated":"2022-02-16 11:43:05.000000000","message":"So this basically means that only project admins can list groups from all projects","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"40846d5f0f5c9b35b525ad892897cf4eee8434c8","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            # new defaults completly then we can remove the above check."},{"line_number":168,"context_line":"            # Until then, let\u0027s keep the old behaviour."},{"line_number":169,"context_line":"            context.can(sg_policies.POLICY_ROOT % \u0027index:all_projects\u0027,"},{"line_number":170,"context_line":"                        target\u003d{\u0027project_id\u0027: project_id})"},{"line_number":171,"context_line":"            sgs \u003d objects.InstanceGroupList.get_all(context)"},{"line_number":172,"context_line":"        else:"},{"line_number":173,"context_line":"            sgs \u003d objects.InstanceGroupList.get_by_project_id("}],"source_content_type":"text/x-python","patch_set":2,"id":"f46568cd_ec1995df","line":170,"in_reply_to":"50f81989_5253d6ca","updated":"2022-02-17 12:19:29.000000000","message":"Ack","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"29366af36330584659e0142181803fc6d4375407","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            # new defaults completly then we can remove the above check."},{"line_number":168,"context_line":"            # Until then, let\u0027s keep the old behaviour."},{"line_number":169,"context_line":"            context.can(sg_policies.POLICY_ROOT % \u0027index:all_projects\u0027,"},{"line_number":170,"context_line":"                        target\u003d{\u0027project_id\u0027: project_id})"},{"line_number":171,"context_line":"            sgs \u003d objects.InstanceGroupList.get_all(context)"},{"line_number":172,"context_line":"        else:"},{"line_number":173,"context_line":"            sgs \u003d objects.InstanceGroupList.get_by_project_id("}],"source_content_type":"text/x-python","patch_set":2,"id":"50f81989_5253d6ca","line":170,"in_reply_to":"ab73b1f9_f9bfc0cf","updated":"2022-02-17 04:40:54.000000000","message":"yes, this means system admin will not allowed. by passing the context\u0027s project_id we make sure that admin in project scoped only pass this policy.","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f7036593deaf8d2f4f3d227a1fa685a316c3f44c","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            # new defaults completly then we can remove the above check."},{"line_number":168,"context_line":"            # Until then, let\u0027s keep the old behaviour."},{"line_number":169,"context_line":"            context.can(sg_policies.POLICY_ROOT % \u0027index:all_projects\u0027,"},{"line_number":170,"context_line":"                        target\u003d{\u0027project_id\u0027: project_id})"},{"line_number":171,"context_line":"            sgs \u003d objects.InstanceGroupList.get_all(context)"},{"line_number":172,"context_line":"        else:"},{"line_number":173,"context_line":"            sgs \u003d objects.InstanceGroupList.get_by_project_id("}],"source_content_type":"text/x-python","patch_set":4,"id":"eebe0d13_cd7acd11","line":170,"updated":"2022-02-23 15:40:36.000000000","message":"not sure why we should limit to the specific project_id while we accept to get all of them as the argument is all_projects","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            # new defaults completly then we can remove the above check."},{"line_number":168,"context_line":"            # Until then, let\u0027s keep the old behaviour."},{"line_number":169,"context_line":"            context.can(sg_policies.POLICY_ROOT % \u0027index:all_projects\u0027,"},{"line_number":170,"context_line":"                        target\u003d{\u0027project_id\u0027: project_id})"},{"line_number":171,"context_line":"            sgs \u003d objects.InstanceGroupList.get_all(context)"},{"line_number":172,"context_line":"        else:"},{"line_number":173,"context_line":"            sgs \u003d objects.InstanceGroupList.get_by_project_id("}],"source_content_type":"text/x-python","patch_set":4,"id":"57851c03_6c429153","line":170,"in_reply_to":"eebe0d13_cd7acd11","updated":"2022-02-23 16:03:01.000000000","message":"we discussed this on IRC and dansmith explained us we need it either way. I trust him.","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/api/openstack/compute/server_topology.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        host_policy \u003d (st_policies.BASE_POLICY_NAME % \u0027host:index\u0027)"},{"line_number":38,"context_line":"        show_host_info \u003d context.can(host_policy,"},{"line_number":39,"context_line":"            target\u003d{\u0027project_id\u0027: instance.project_id}, fatal\u003dFalse)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"        return self._get_numa_topology(context, instance, show_host_info)"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"82060473_edb80a8f","line":39,"updated":"2022-02-23 16:03:01.000000000","message":"makes sense to restrict","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/api/openstack/compute/volumes.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"87835c51175a062e661d07a1a0e60bc83a32eb2f","unresolved":true,"context_lines":[{"line_number":507,"context_line":"        # the microversion, we should check the swap volume policy."},{"line_number":508,"context_line":"        # otherwise, check the volume update policy."},{"line_number":509,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":510,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027,"},{"line_number":511,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":512,"context_line":"        else:"},{"line_number":513,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"cb7662b3_894ec58c","line":510,"updated":"2022-02-16 11:43:05.000000000","message":"Is this API called by cinder? If so will this work by default after we scoped it to project? I mean will the cinder call have the proper project_id?","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85a998a2d3b243301e138f3000c040116208c51e","unresolved":false,"context_lines":[{"line_number":507,"context_line":"        # the microversion, we should check the swap volume policy."},{"line_number":508,"context_line":"        # otherwise, check the volume update policy."},{"line_number":509,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":510,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027,"},{"line_number":511,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":512,"context_line":"        else:"},{"line_number":513,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ad9b5f81_5b403d15","line":510,"in_reply_to":"4bdbf6c5_2139e7e0","updated":"2022-02-21 10:46:17.000000000","message":"Thanks. The plans with the service role sounds good as a long term solution","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f43eb6f43c7fd3542fc1601e756138386cadb4fa","unresolved":true,"context_lines":[{"line_number":507,"context_line":"        # the microversion, we should check the swap volume policy."},{"line_number":508,"context_line":"        # otherwise, check the volume update policy."},{"line_number":509,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":510,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027,"},{"line_number":511,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":512,"context_line":"        else:"},{"line_number":513,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"4bdbf6c5_2139e7e0","line":510,"in_reply_to":"6833a140_314e4ddd","updated":"2022-02-19 22:39:36.000000000","message":"You are right on cinder\u0027s project_id. I re-checked the way cinder call the swap volume while migrating volume to a specific host (even neutron case for server external event). And yes cinder will not have the right project_id and we cannot check that project_id in nova policy enforcement against instance.project_id. \n\nCinder uses the configured nova user for nova API call and project there can be different from server\u0027s project for which volume swap is called. In gate, cinder use the devstack created \u0027nova\u0027 user which has \u0027service\u0027 and \u0027admin\u0027 roles assigned in the project called \u0027service\u0027. This is a good example of cinder using different projects used to interact with nova than where the server belongs to.\n\nAt the end for service-to-service interaction (discussing in service role spec[1]), below can be the workflow (but will be finalized once keystone spec is merged and we start discussion in phase-2):\n\n- policy default is updated including the service role. Example:        \n    \"os_compute_api:os-volumes-attachments:swap\u003d\u0027role:admin or role:service\u0027\"\n    scope_type\u003d [\u0027project\u0027] \n- operator cerate a project scoped user in keystone with role as \u0027service\u0027 in \u0027service\u0027 project.\n- operator configure that service user to cinder (or neutron) in nova auth section in conder.conf.\n- Cinder will use that service user to call nova\u0027s volume swap API and policy will pass as user is \u0027service\u0027 role and project scoped. \n\nBut we need to wait for phase-2 to implement those in projects. Meanwhile, to keep the current behaviour, I will modify this policy default and remove the instance.project_id from policy target. \n\nThanks a lot for catching this.\n\n[1] https://review.opendev.org/c/openstack/keystone-specs/+/818616/1/specs/keystone/yoga/default-service-role.rst#72","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"29366af36330584659e0142181803fc6d4375407","unresolved":true,"context_lines":[{"line_number":507,"context_line":"        # the microversion, we should check the swap volume policy."},{"line_number":508,"context_line":"        # otherwise, check the volume update policy."},{"line_number":509,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":510,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027,"},{"line_number":511,"context_line":"                        target\u003d{\u0027project_id\u0027: instance.project_id})"},{"line_number":512,"context_line":"        else:"},{"line_number":513,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027update\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"6833a140_314e4ddd","line":510,"in_reply_to":"cb7662b3_894ec58c","updated":"2022-02-17 04:40:54.000000000","message":"humm, good question. As this APIs is mainly for cinder only I think we can open it for SYSTEM_ADMIN also so that cinder can request it when they do not know about project_id of server where volume is attached?\n\nThis is same question we are discussing for service-to-service role (which is for next phase of this goal). Mark is making the same point there https://review.opendev.org/c/openstack/keystone-specs/+/818616/1/specs/keystone/yoga/default-service-role.rst#72\n\n@Dan what you think on this? should we make all service-to-service role as ADMIN and [\u0027system\u0027, \u0027project\u0027] scoped at least until we decide the design in next phases? \n\nNOTE: existing context used by service to call service APIs will keep working due to legacy rules but if we remove the legacy rule then it may break.","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":510,"context_line":"        # is called by cinder which does not have correct project_id where"},{"line_number":511,"context_line":"        # server belongs to. By passing the empty target, we make sure that"},{"line_number":512,"context_line":"        # we do not check the requester project_id and allow users with"},{"line_number":513,"context_line":"        # allowed role to perform the swap volume."},{"line_number":514,"context_line":"        if only_swap or id !\u003d volume_id:"},{"line_number":515,"context_line":"            context.can(va_policies.POLICY_ROOT % \u0027swap\u0027, target\u003d{})"},{"line_number":516,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"d3da7a77_07855f17","line":513,"updated":"2022-02-23 16:03:01.000000000","message":"OKOK, thanks for explaining why we don\u0027t need it this time","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/policies/attach_interfaces.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":27,"context_line":"in nova 23.0.0 release."},{"line_number":28,"context_line":"\"\"\""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"DEPRECATED_INTERFACES_POLICY \u003d policy.DeprecatedRule("},{"line_number":31,"context_line":"    BASE_POLICY_NAME,"},{"line_number":32,"context_line":"    base.RULE_ADMIN_OR_OWNER,"},{"line_number":33,"context_line":"    deprecated_reason\u003dDEPRECATED_REASON,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa2b1195_1b004d27","line":30,"updated":"2022-02-23 16:03:01.000000000","message":"note for reviewers: this legacy policy continues to be default for Yoga, so no breaking change is expected for operators unless they turn on the new policy rules.","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":37,"context_line":"attach_interfaces_policies \u003d ["},{"line_number":38,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":39,"context_line":"        name\u003dPOLICY_ROOT % \u0027list\u0027,"},{"line_number":40,"context_line":"        check_str\u003dbase.PROJECT_READER,"},{"line_number":41,"context_line":"        description\u003d\"List port interfaces attached to a server\","},{"line_number":42,"context_line":"        operations\u003d["},{"line_number":43,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":4,"id":"70251aac_2f27c03e","line":40,"updated":"2022-02-23 16:03:01.000000000","message":"note for reviewers: this is a behavioural change but this needed by nature of the new policy rules as project admins should now be restricted to their only project and not span across all of the projects.","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/policies/instance_actions.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":38,"context_line":"instance_actions_policies \u003d ["},{"line_number":39,"context_line":"    policy.DocumentedRuleDefault("},{"line_number":40,"context_line":"        name\u003dBASE_POLICY_NAME % \u0027events:details\u0027,"},{"line_number":41,"context_line":"        check_str\u003dbase.PROJECT_ADMIN,"},{"line_number":42,"context_line":"        description\u003d\"\"\"Add \"details\" key in action events for a server."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"This check is performed only after the check"}],"source_content_type":"text/x-python","patch_set":4,"id":"00dc7153_3fc112fd","line":41,"updated":"2022-02-23 16:03:01.000000000","message":"ditto here, change of scope but we agreed on that direction before.","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/policies/quota_sets.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":52,"context_line":"        # project_id passed in request url is used as policy targets which"},{"line_number":53,"context_line":"        # would not match with context\u0027s project_id fetched for rule"},{"line_number":54,"context_line":"        # PROJECT_ADMIN check."},{"line_number":55,"context_line":"        check_str\u003d\u0027(\u0027 + base.PROJECT_READER + \u0027) or role:admin\u0027,"},{"line_number":56,"context_line":"        description\u003d\"Show a quota\","},{"line_number":57,"context_line":"        operations\u003d["},{"line_number":58,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":4,"id":"dde5c7e8_29bd5761","line":55,"updated":"2022-02-23 16:03:01.000000000","message":"oh gosh, this is ugly but I understand the reasoning.","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":77,"context_line":"        # TODO(gmann): Until we have domain admin or so to get other project\u0027s"},{"line_number":78,"context_line":"        # data, allow admin role(with scope check it will be project admin) to"},{"line_number":79,"context_line":"        # get other project quota."},{"line_number":80,"context_line":"        check_str\u003d\u0027(\u0027 + base.PROJECT_READER + \u0027) or role:admin\u0027,"},{"line_number":81,"context_line":"        description\u003d\"Show the detail of quota\","},{"line_number":82,"context_line":"        operations\u003d["},{"line_number":83,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":4,"id":"32897a1a_00b19db0","line":80,"updated":"2022-02-23 16:03:01.000000000","message":"ditto here","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/policies/volumes_attachments.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"87835c51175a062e661d07a1a0e60bc83a32eb2f","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff3ac173_3042b055","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"updated":"2022-02-16 11:43:05.000000000","message":"Either I misunderstood this or something is broken. In my understanding people having PROJECT_ADMIN creds are not supersets of people having PROJECT_MEMBER creds.","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f65d5845c0fea90a0387056b0d582a68dceb0d01","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":2,"id":"04535d32_9d2c6ad5","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"in_reply_to":"04c9e54e_7f4cc5d6","updated":"2022-02-22 09:31:54.000000000","message":"Ack","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"40846d5f0f5c9b35b525ad892897cf4eee8434c8","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":2,"id":"c463af9f_9ee25bd0","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"in_reply_to":"98b94d7b_f7a332d1","updated":"2022-02-17 12:19:29.000000000","message":"How I understood the above sentence:\nWe expect that a token that is accepted for \"update\" is also accepted for \"swap\". \n\nBut a project member token that is accepted for \"update\" will not be accepted for \"swap\" as that need at least an project admin token","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"386887223a21a3527eec234ed9a6edc06069a4e8","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":2,"id":"04c9e54e_7f4cc5d6","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"in_reply_to":"a8d6065b_670ca534","updated":"2022-02-21 16:49:34.000000000","message":"sure, I will update it along with adding context of it as note in followup patch.","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c34b173e00d961e703e42eca506ec7fdc83b5573","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":2,"id":"fef41aa6_54a0c073","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"in_reply_to":"c463af9f_9ee25bd0","updated":"2022-02-19 23:01:02.000000000","message":"It is a little tricky. In 2.86, this API is extended to update the volume also (previously it was only swap volume) and for that, we made the two separate policies for swap and update. And swap + update can be called together in \u003e2.85. so we have the below cases: \n\n1. only update: update policy is checked. All good.\n2. only swap: swap policy is checked. All good.\n3. swap + update (\u003e2.85): only swap policy is checked. This is the tricky part. \n\n   Case1:\n    - update policy is member role\n    - swap policy is admin role (this is a superset of update policy)\n      - this means any user having a member role can do the update. If as an admin user I call swap+update then it satisfies the swap policy and so does allow me to do the update. Which does not violate the update policy permission(admin is allowed for member-allowed policy).\n\n   Case2:\n    - update policy is member role\n    - swap policy is overridden to reader role by operator (this is not superset of update policy)\n      - As a reader user I will call swap+update and as we check only swap policy it passes and allow me to update the attachment. Here the reader role, I am able to update the volume attachment which violates the update policy permission (which is member role). And to avoid this case,  we mentioned keep swap policy as a superset of the update policy always.\n\nHope I explained it to understand it than making more confusing :)","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85a998a2d3b243301e138f3000c040116208c51e","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":2,"id":"a8d6065b_670ca534","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"in_reply_to":"fef41aa6_54a0c073","updated":"2022-02-21 10:46:17.000000000","message":"I think we should stop using superset terminology here. It is misleading.\n\nWhen you say that the admin role is superset of the member role then it can be understood two ways:\n\na) the people having the admin role can do a set of actions X while people having the member role can do a set of actions Y. And X is a superset of Y.\n\nb) the set of people having the admin role is the superset of the people having the member role. \n\nBased on your explanation the meaning a) is the one that is used in the text. But I was looked at this from the perspective of b) when I read this.\n\nI suggest to change the text to:\nWe expect \u003cswap policy\u003e to be always at least as restrictive as the \u003cupdate policy\u003e","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"29366af36330584659e0142181803fc6d4375407","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        check_str\u003dbase.PROJECT_MEMBER,"},{"line_number":61,"context_line":"        description\u003d\"\"\"Update a volume attachment."},{"line_number":62,"context_line":"New \u0027update\u0027 policy about \u0027swap + update\u0027 request (which is possible"},{"line_number":63,"context_line":"only \u003e2.85) only \u003cswap policy\u003e is checked. We expect \u003cswap policy\u003e to be"},{"line_number":64,"context_line":"always superset of this policy permission."},{"line_number":65,"context_line":"\"\"\","},{"line_number":66,"context_line":"        operations\u003d["},{"line_number":67,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":2,"id":"98b94d7b_f7a332d1","line":64,"range":{"start_line":63,"start_character":43,"end_line":64,"end_character":42},"in_reply_to":"ff3ac173_3042b055","updated":"2022-02-17 04:40:54.000000000","message":"PROJECT_ADMIN is always superset of PROJECT_MEMBER. As per keystone implied roles, admin has role of [\u0027admin\u0027, \u0027member\u0027, \u0027reader\u0027] and member role as [\u0027member\u0027, \u0027reader\u0027].\n\nHope I understood your question correctly?","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":80,"context_line":"        # we keep it open for all admin in any project. We cannot default it to"},{"line_number":81,"context_line":"        # PROJECT_ADMIN which has the project_id in check_str and will fail"},{"line_number":82,"context_line":"        # if cinder call it with other project_id."},{"line_number":83,"context_line":"        check_str\u003dbase.ADMIN,"},{"line_number":84,"context_line":"        description\u003d\"Update a volume attachment with a different volumeId\","},{"line_number":85,"context_line":"        operations\u003d["},{"line_number":86,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":4,"id":"05db1577_4536eddc","line":83,"updated":"2022-02-23 16:03:01.000000000","message":"OK, I understand why you use this ADMIN role and not like https://review.opendev.org/c/openstack/nova/+/828670/4/nova/policies/quota_sets.py#55","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/tests/unit/policies/test_attach_interfaces.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":50,"context_line":"        # With legacy rule and no scope checks, all admin, project members"},{"line_number":51,"context_line":"        # project reader or other project role(because legacy rule allow server"},{"line_number":52,"context_line":"        # owner- having same project id and no role check) is able to attach,"},{"line_number":53,"context_line":"        # detach an interface from a server."},{"line_number":54,"context_line":"        self.project_member_authorized_contexts \u003d ["},{"line_number":55,"context_line":"            self.legacy_admin_context, self.system_admin_context,"},{"line_number":56,"context_line":"            self.project_admin_context, self.project_member_context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ab878f03_e55adf88","line":53,"updated":"2022-02-23 16:03:01.000000000","message":"yeah, that\u0027s what I understood.","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":107,"context_line":"                                self.req, uuids.fake_id, uuids.fake_id)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"class AttachInterfacesNoLegacyNoScopePolicyTest(AttachInterfacesPolicyTest):"},{"line_number":111,"context_line":"    \"\"\"Test Attach Interfaces APIs policies with no legacy deprecated rules"},{"line_number":112,"context_line":"    and no scope checks."},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bf376ea1_0b9626f0","line":110,"updated":"2022-02-23 16:03:01.000000000","message":"++","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"12500c21275a7db390d533b18f81f2033f2d41fb","unresolved":true,"context_lines":[{"line_number":227,"context_line":"    def setUp(self):"},{"line_number":228,"context_line":"        super(AttachInterfacesScopeTypeNoLegacyPolicyTest, self).setUp()"},{"line_number":229,"context_line":"        # With no legacy and scope enable, only project admin, member,"},{"line_number":230,"context_line":"        # and reader will be able to allowed operation on server interface."},{"line_number":231,"context_line":"        self.project_member_authorized_contexts \u003d ["},{"line_number":232,"context_line":"            self.project_admin_context, self.project_member_context]"},{"line_number":233,"context_line":"        self.project_reader_authorized_contexts \u003d ["}],"source_content_type":"text/x-python","patch_set":4,"id":"42338bd9_e0cf4ac7","line":230,"updated":"2022-02-23 16:03:01.000000000","message":"++","commit_id":"d7be635fb4a2f51f39cb62b08cc6b7569f5f60f8"}],"nova/tests/unit/policies/test_server_groups.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"87835c51175a062e661d07a1a0e60bc83a32eb2f","unresolved":true,"context_lines":[{"line_number":63,"context_line":"            self.project_admin_context, self.project_member_context,"},{"line_number":64,"context_line":"            self.project_reader_context, self.project_foo_context,"},{"line_number":65,"context_line":"        ]"},{"line_number":66,"context_line":"        # By default, legacy rule are enable and scope check is disabled."},{"line_number":67,"context_line":"        # system admin, legacy admin, and project admin is able to get"},{"line_number":68,"context_line":"        # all projects SG."},{"line_number":69,"context_line":"        self.project_admin_authorized_contexts \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"53211746_77488897","line":66,"range":{"start_line":66,"start_character":38,"end_line":66,"end_character":44},"updated":"2022-02-16 11:43:05.000000000","message":"nit: enabled","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f43eb6f43c7fd3542fc1601e756138386cadb4fa","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            self.project_admin_context, self.project_member_context,"},{"line_number":64,"context_line":"            self.project_reader_context, self.project_foo_context,"},{"line_number":65,"context_line":"        ]"},{"line_number":66,"context_line":"        # By default, legacy rule are enable and scope check is disabled."},{"line_number":67,"context_line":"        # system admin, legacy admin, and project admin is able to get"},{"line_number":68,"context_line":"        # all projects SG."},{"line_number":69,"context_line":"        self.project_admin_authorized_contexts \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"d96df433_affc892f","line":66,"range":{"start_line":66,"start_character":38,"end_line":66,"end_character":44},"in_reply_to":"53211746_77488897","updated":"2022-02-19 22:39:36.000000000","message":"Done","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"87835c51175a062e661d07a1a0e60bc83a32eb2f","unresolved":true,"context_lines":[{"line_number":169,"context_line":"            self.project_admin_context, self.project_member_context,"},{"line_number":170,"context_line":"            self.project_reader_context]"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # Even eith no legacy rule, legacy admin is allowed as create SG"},{"line_number":173,"context_line":"        # use requesting context\u0027s project_id. Same for list SG."},{"line_number":174,"context_line":"        self.project_create_authorized_contexts \u003d ["},{"line_number":175,"context_line":"            self.legacy_admin_context, self.system_admin_context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"86a84645_0ac51715","line":172,"range":{"start_line":172,"start_character":60,"end_line":172,"end_character":62},"updated":"2022-02-16 11:43:05.000000000","message":"to","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"87835c51175a062e661d07a1a0e60bc83a32eb2f","unresolved":true,"context_lines":[{"line_number":169,"context_line":"            self.project_admin_context, self.project_member_context,"},{"line_number":170,"context_line":"            self.project_reader_context]"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # Even eith no legacy rule, legacy admin is allowed as create SG"},{"line_number":173,"context_line":"        # use requesting context\u0027s project_id. Same for list SG."},{"line_number":174,"context_line":"        self.project_create_authorized_contexts \u003d ["},{"line_number":175,"context_line":"            self.legacy_admin_context, self.system_admin_context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"a38b5efb_06fa27cc","line":172,"range":{"start_line":172,"start_character":15,"end_line":172,"end_character":19},"updated":"2022-02-16 11:43:05.000000000","message":"with","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f43eb6f43c7fd3542fc1601e756138386cadb4fa","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            self.project_admin_context, self.project_member_context,"},{"line_number":170,"context_line":"            self.project_reader_context]"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # Even eith no legacy rule, legacy admin is allowed as create SG"},{"line_number":173,"context_line":"        # use requesting context\u0027s project_id. Same for list SG."},{"line_number":174,"context_line":"        self.project_create_authorized_contexts \u003d ["},{"line_number":175,"context_line":"            self.legacy_admin_context, self.system_admin_context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"d53c4c57_f0ded269","line":172,"range":{"start_line":172,"start_character":60,"end_line":172,"end_character":62},"in_reply_to":"86a84645_0ac51715","updated":"2022-02-19 22:39:36.000000000","message":"Done","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f43eb6f43c7fd3542fc1601e756138386cadb4fa","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            self.project_admin_context, self.project_member_context,"},{"line_number":170,"context_line":"            self.project_reader_context]"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        # Even eith no legacy rule, legacy admin is allowed as create SG"},{"line_number":173,"context_line":"        # use requesting context\u0027s project_id. Same for list SG."},{"line_number":174,"context_line":"        self.project_create_authorized_contexts \u003d ["},{"line_number":175,"context_line":"            self.legacy_admin_context, self.system_admin_context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"b77ce901_4ece1795","line":172,"range":{"start_line":172,"start_character":15,"end_line":172,"end_character":19},"in_reply_to":"a38b5efb_06fa27cc","updated":"2022-02-19 22:39:36.000000000","message":"Done","commit_id":"7697d7fd2c0f296c16fdbdf123d928ad68bf8c58"}]}