)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ecb4e8e28c1958b005e2e7d650cab36de83ebb22","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Abhishek Kekane \u003cakekane@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-08-02 17:12:44 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move metadef namepsace policy checks in the API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change also includes change in index method for \u0027GET\u0027 all"},{"line_number":10,"context_line":"namespaces to not include properties, objects, tags, associations"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"375262cc_93d09fa2","line":7,"range":{"start_line":7,"start_character":13,"end_line":7,"end_character":22},"updated":"2021-08-02 18:51:47.000000000","message":"Whoops, forgot to exercise my obvious -1 on this :)\n\nWorth fixing I think, especially since it\u0027s in the title.","commit_id":"b8b8cdbbc3ba2bb66c1f2472723ef46a627c1c4e"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"da375e778957b1f04146308b134630e2cd9579a1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Abhishek Kekane \u003cakekane@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2021-08-02 17:12:44 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Move metadef namepsace policy checks in the API"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change also includes change in index method for \u0027GET\u0027 all"},{"line_number":10,"context_line":"namespaces to not include properties, objects, tags, associations"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"49e3cc11_65852970","line":7,"range":{"start_line":7,"start_character":13,"end_line":7,"end_character":22},"in_reply_to":"375262cc_93d09fa2","updated":"2021-08-02 19:04:12.000000000","message":"Ack","commit_id":"b8b8cdbbc3ba2bb66c1f2472723ef46a627c1c4e"}],"glance/api/v2/metadef_namespaces.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            namespaces \u003d Namespaces()"},{"line_number":104,"context_line":"            namespaces.namespaces \u003d namespace_list"},{"line_number":105,"context_line":"            if len(namespace_list) !\u003d 0 and db_ns_count \u003d\u003d limit:"},{"line_number":106,"context_line":"                namespaces.next \u003d namespace_list[-1].namespace"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"        except exception.Forbidden as e:"},{"line_number":109,"context_line":"            LOG.debug(\"User not permitted to retrieve metadata namespaces \""}],"source_content_type":"text/x-python","patch_set":4,"id":"5f77c754_c394134e","line":106,"range":{"start_line":106,"start_character":34,"end_line":106,"end_character":62},"updated":"2021-07-16 14:16:21.000000000","message":"I think this paging logic could be a problem if the last item is the beginning of a long stretch of things I don\u0027t have access to see. For example, if I\u0027m doing a list with a page size of 10, and then there\u0027s a stretch of 15 items I don\u0027t have access to via policy. When I list the second page, all of them will be filtered out of ns_list by policy, which means there will be nothing returned *and* nothing to provide a link to the third page to continue with items I *can* see.\n\nIt\u0027s hard to care too much about this because we know metadef is broken for multi-tenant anyway, but, might be worth a comment or something.","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"cefb8f4b117bd4d6467150a85c90732cfb688774","unresolved":true,"context_lines":[{"line_number":103,"context_line":"            namespaces \u003d Namespaces()"},{"line_number":104,"context_line":"            namespaces.namespaces \u003d namespace_list"},{"line_number":105,"context_line":"            if len(namespace_list) !\u003d 0 and db_ns_count \u003d\u003d limit:"},{"line_number":106,"context_line":"                namespaces.next \u003d namespace_list[-1].namespace"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"        except exception.Forbidden as e:"},{"line_number":109,"context_line":"            LOG.debug(\"User not permitted to retrieve metadata namespaces \""}],"source_content_type":"text/x-python","patch_set":4,"id":"4f161d07_ccb3961a","line":106,"range":{"start_line":106,"start_character":34,"end_line":106,"end_character":62},"in_reply_to":"5f77c754_c394134e","updated":"2021-07-16 14:40:34.000000000","message":"AFAIK, this query will return only those records which are accessible to the current user.\nhttps://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/metadef_api/namespace.py#L55\n\nSo the chances of failure here are very less (unless I am wrong)","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":240,"context_line":"                req.context, authorization_layer\u003dFalse)"},{"line_number":241,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":242,"context_line":"                req.context,"},{"line_number":243,"context_line":"                enforcer\u003dself.policy).get_metadef_namespace()"},{"line_number":244,"context_line":"            namespace_obj \u003d ns_repo.get(namespace)"},{"line_number":245,"context_line":"            namespace_detail \u003d Namespace.to_wsme_model("},{"line_number":246,"context_line":"                namespace_obj,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fbd078b9_ac603228","line":243,"updated":"2021-07-16 14:16:21.000000000","message":"This checks if we can see a namepsace, shouldn\u0027t we be passing namespace_obj below?","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":253,"context_line":"            object_repo \u003d self.gateway.get_metadef_object_repo("},{"line_number":254,"context_line":"                req.context, authorization_layer\u003dFalse)"},{"line_number":255,"context_line":"            db_metaobject_list \u003d object_repo.list(filters\u003dns_filters)"},{"line_number":256,"context_line":"            object_list \u003d [MetadefObject.to_wsme_model("},{"line_number":257,"context_line":"                db_metaobject,"},{"line_number":258,"context_line":"                get_object_href(namespace, db_metaobject),"},{"line_number":259,"context_line":"                self.obj_schema_link) for db_metaobject in db_metaobject_list]"}],"source_content_type":"text/x-python","patch_set":4,"id":"edc5e3a7_225ba817","line":256,"range":{"start_line":256,"start_character":12,"end_line":256,"end_character":23},"updated":"2021-07-16 14:16:21.000000000","message":"Do we need to check these? Or is it based on being able to see the namespace?","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"cefb8f4b117bd4d6467150a85c90732cfb688774","unresolved":true,"context_lines":[{"line_number":253,"context_line":"            object_repo \u003d self.gateway.get_metadef_object_repo("},{"line_number":254,"context_line":"                req.context, authorization_layer\u003dFalse)"},{"line_number":255,"context_line":"            db_metaobject_list \u003d object_repo.list(filters\u003dns_filters)"},{"line_number":256,"context_line":"            object_list \u003d [MetadefObject.to_wsme_model("},{"line_number":257,"context_line":"                db_metaobject,"},{"line_number":258,"context_line":"                get_object_href(namespace, db_metaobject),"},{"line_number":259,"context_line":"                self.obj_schema_link) for db_metaobject in db_metaobject_list]"}],"source_content_type":"text/x-python","patch_set":4,"id":"2892c6a8_6d23cbe8","line":256,"range":{"start_line":256,"start_character":12,"end_line":256,"end_character":23},"in_reply_to":"edc5e3a7_225ba817","updated":"2021-07-16 14:40:34.000000000","message":"It is based on being able to see the namespace,\nIn Metadef, objects, properties, tags belongs to metadef namespaces only.","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":308,"context_line":"        try:"},{"line_number":309,"context_line":"            ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":310,"context_line":"            # NOTE(abhishekk): This will check whether user is authorized to"},{"line_number":311,"context_line":"            # update existing namespace or not."},{"line_number":312,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":313,"context_line":"                req.context,"},{"line_number":314,"context_line":"                md_resource\u003dns_obj,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f2beb71_e6f41f34","line":311,"updated":"2021-07-16 14:16:21.000000000","message":"Should we be checking against ns_obj? Otherwise this is \"can a user modify *any* namespace\" instead of \"can a user modify *this* namespace\"","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"cefb8f4b117bd4d6467150a85c90732cfb688774","unresolved":true,"context_lines":[{"line_number":308,"context_line":"        try:"},{"line_number":309,"context_line":"            ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":310,"context_line":"            # NOTE(abhishekk): This will check whether user is authorized to"},{"line_number":311,"context_line":"            # update existing namespace or not."},{"line_number":312,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":313,"context_line":"                req.context,"},{"line_number":314,"context_line":"                md_resource\u003dns_obj,"}],"source_content_type":"text/x-python","patch_set":4,"id":"06f0f25e_cd67dc0d","line":311,"in_reply_to":"7f2beb71_e6f41f34","updated":"2021-07-16 14:40:34.000000000","message":"Ack, I am passing the ns_obj to MetadefAPIPolicy, but forget to build target using it.","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":358,"context_line":"                req.context,"},{"line_number":359,"context_line":"                md_resource\u003dnamespace_obj,"},{"line_number":360,"context_line":"                enforcer\u003dself.policy).delete_metadef_namespace()"},{"line_number":361,"context_line":"            namespace_obj.delete()"},{"line_number":362,"context_line":"            namespace_repo.remove(namespace_obj)"},{"line_number":363,"context_line":"        except exception.Forbidden as e:"}],"source_content_type":"text/x-python","patch_set":4,"id":"03896de1_b285d84b","line":360,"updated":"2021-07-16 14:16:21.000000000","message":"same","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"cefb8f4b117bd4d6467150a85c90732cfb688774","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":358,"context_line":"                req.context,"},{"line_number":359,"context_line":"                md_resource\u003dnamespace_obj,"},{"line_number":360,"context_line":"                enforcer\u003dself.policy).delete_metadef_namespace()"},{"line_number":361,"context_line":"            namespace_obj.delete()"},{"line_number":362,"context_line":"            namespace_repo.remove(namespace_obj)"},{"line_number":363,"context_line":"        except exception.Forbidden as e:"}],"source_content_type":"text/x-python","patch_set":4,"id":"2e84737d_a62e84d7","line":360,"in_reply_to":"03896de1_b285d84b","updated":"2021-07-16 14:40:34.000000000","message":"Noted","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":379,"context_line":"            # So should we enforce it here???"},{"line_number":380,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":381,"context_line":"                req.context,"},{"line_number":382,"context_line":"                enforcer\u003dself.policy).delete_metadef_object()"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"            namespace_obj \u003d ns_repo.get(namespace)"},{"line_number":385,"context_line":"            namespace_obj.delete()"}],"source_content_type":"text/x-python","patch_set":4,"id":"0a0914c0_d9667e8d","line":382,"updated":"2021-07-16 14:16:21.000000000","message":"also this","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            # FIXME(abhishekk): remove_tags does not enforce any policy"},{"line_number":402,"context_line":"            # checks, but it does visibility check at db layer."},{"line_number":403,"context_line":"            # So should we enforce it here???"},{"line_number":404,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":405,"context_line":"                req.context,"},{"line_number":406,"context_line":"                enforcer\u003dself.policy).delete_metadef_tags()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a0469c4_a60bf76d","line":403,"updated":"2021-07-16 14:16:21.000000000","message":"Maybe check modify_namespace for namespace_obj before, and if it passes, then we let them remove tags?","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"cefb8f4b117bd4d6467150a85c90732cfb688774","unresolved":true,"context_lines":[{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            # FIXME(abhishekk): remove_tags does not enforce any policy"},{"line_number":402,"context_line":"            # checks, but it does visibility check at db layer."},{"line_number":403,"context_line":"            # So should we enforce it here???"},{"line_number":404,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":405,"context_line":"                req.context,"},{"line_number":406,"context_line":"                enforcer\u003dself.policy).delete_metadef_tags()"}],"source_content_type":"text/x-python","patch_set":4,"id":"99b10871_45603d2e","line":403,"in_reply_to":"5a0469c4_a60bf76d","updated":"2021-07-16 14:40:34.000000000","message":"Good idea, anyways it does visibility check at db layer so we can enforce the modify_namespace check here.","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43c26d357f61c948c0afbacf101e4cd4f8fbaedf","unresolved":true,"context_lines":[{"line_number":423,"context_line":"        try:"},{"line_number":424,"context_line":"            # FIXME(abhishekk): remove_properties does not enforce any policy"},{"line_number":425,"context_line":"            # checks, but it does visibility check at db layer."},{"line_number":426,"context_line":"            # So should we enforce it here???"},{"line_number":427,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":428,"context_line":"                req.context,"},{"line_number":429,"context_line":"                enforcer\u003dself.policy).remove_metadef_property()"}],"source_content_type":"text/x-python","patch_set":4,"id":"4eb24fca_10c1f562","line":426,"updated":"2021-07-16 14:16:21.000000000","message":"Same here?","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"cefb8f4b117bd4d6467150a85c90732cfb688774","unresolved":true,"context_lines":[{"line_number":423,"context_line":"        try:"},{"line_number":424,"context_line":"            # FIXME(abhishekk): remove_properties does not enforce any policy"},{"line_number":425,"context_line":"            # checks, but it does visibility check at db layer."},{"line_number":426,"context_line":"            # So should we enforce it here???"},{"line_number":427,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":428,"context_line":"                req.context,"},{"line_number":429,"context_line":"                enforcer\u003dself.policy).remove_metadef_property()"}],"source_content_type":"text/x-python","patch_set":4,"id":"65e0d5ae_65066d36","line":426,"in_reply_to":"4eb24fca_10c1f562","updated":"2021-07-16 14:40:34.000000000","message":"Noted.","commit_id":"095c05b3741a57b688be34e8c475b274e63ad917"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"359c333f4b051d21e2d586651fdccbd6603db4bf","unresolved":true,"context_lines":[{"line_number":89,"context_line":"                filters \u003d dict()"},{"line_number":90,"context_line":"                filters[\u0027namespace\u0027] \u003d db_namespace.namespace"},{"line_number":91,"context_line":"                rs_repo \u003d ("},{"line_number":92,"context_line":"                    self.gateway.get_metadef_resource_type_repo("},{"line_number":93,"context_line":"                        req.context, authorization_layer\u003dFalse))"},{"line_number":94,"context_line":"                repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":95,"context_line":"                resource_type_list \u003d [ResourceTypeAssociation.to_wsme_model("}],"source_content_type":"text/x-python","patch_set":8,"id":"14bcd2ad_f01998ff","line":92,"updated":"2021-07-21 18:50:49.000000000","message":"Not your problem, but we probably do not need to re-create this repo object each time through the loop, right?","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"830fd193262103b54fd720b4cedda215e53e6566","unresolved":true,"context_lines":[{"line_number":89,"context_line":"                filters \u003d dict()"},{"line_number":90,"context_line":"                filters[\u0027namespace\u0027] \u003d db_namespace.namespace"},{"line_number":91,"context_line":"                rs_repo \u003d ("},{"line_number":92,"context_line":"                    self.gateway.get_metadef_resource_type_repo("},{"line_number":93,"context_line":"                        req.context, authorization_layer\u003dFalse))"},{"line_number":94,"context_line":"                repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":95,"context_line":"                resource_type_list \u003d [ResourceTypeAssociation.to_wsme_model("}],"source_content_type":"text/x-python","patch_set":8,"id":"80fe10d2_f5a9224a","line":92,"in_reply_to":"14bcd2ad_f01998ff","updated":"2021-07-21 19:15:13.000000000","message":"Correct!","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9008cc240798ed29a9959ea3ef3125fef991b1ee","unresolved":false,"context_lines":[{"line_number":89,"context_line":"                filters \u003d dict()"},{"line_number":90,"context_line":"                filters[\u0027namespace\u0027] \u003d db_namespace.namespace"},{"line_number":91,"context_line":"                rs_repo \u003d ("},{"line_number":92,"context_line":"                    self.gateway.get_metadef_resource_type_repo("},{"line_number":93,"context_line":"                        req.context, authorization_layer\u003dFalse))"},{"line_number":94,"context_line":"                repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":95,"context_line":"                resource_type_list \u003d [ResourceTypeAssociation.to_wsme_model("}],"source_content_type":"text/x-python","patch_set":8,"id":"c93c765e_98ff213c","line":92,"in_reply_to":"80fe10d2_f5a9224a","updated":"2021-07-22 05:46:47.000000000","message":"Done","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"b04099f44678171b1c65e282bc2f2e3f3776ee67","unresolved":true,"context_lines":[{"line_number":275,"context_line":"            # behaviour by changing default policy then here I have made"},{"line_number":276,"context_line":"            # provision to simplify the enforcement by passing the resource"},{"line_number":277,"context_line":"            # to enforce all."},{"line_number":278,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":279,"context_line":"                req.context,"},{"line_number":280,"context_line":"                md_resource\u003dnamespace_obj,"},{"line_number":281,"context_line":"                enforcer\u003dself.policy).get_metadef_namespace()"},{"line_number":282,"context_line":"            namespace_detail \u003d Namespace.to_wsme_model("},{"line_number":283,"context_line":"                namespace_obj,"},{"line_number":284,"context_line":"                get_namespace_href(namespace_obj),"}],"source_content_type":"text/x-python","patch_set":12,"id":"9d9b3ca3_100841d5","line":281,"range":{"start_line":278,"start_character":12,"end_line":281,"end_character":61},"updated":"2021-07-30 17:19:36.000000000","message":"ToDo:\nI think here I need to catch forbidden and raise not found.","commit_id":"05d3b6c4bf75515b05afe795e8edf5acd4bc5964"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"840471edd01f852158e6e135da6933a0fb088986","unresolved":true,"context_lines":[{"line_number":100,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.msg)"},{"line_number":101,"context_line":"        except Exception as e:"},{"line_number":102,"context_line":"            LOG.error(encodeutils.exception_to_unicode(e))"},{"line_number":103,"context_line":"            raise webob.exc.HTTPInternalServerError()"},{"line_number":104,"context_line":"        return namespaces"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    @utils.mutating"}],"source_content_type":"text/x-python","patch_set":16,"id":"4903acbc_1ce5057e","side":"PARENT","line":103,"updated":"2021-08-04 19:12:43.000000000","message":"Was this here because of something in the authorization layer that we\u0027re not longer susceptible too anymore now that we\u0027re passing in authorization_layer\u003dFalse?","commit_id":"0cc8190d0edef73ffa7ee64c527e2d083cbb8bd1"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"423b8cae0c5ad81816579213b15c15c8377d6479","unresolved":true,"context_lines":[{"line_number":100,"context_line":"            raise webob.exc.HTTPNotFound(explanation\u003de.msg)"},{"line_number":101,"context_line":"        except Exception as e:"},{"line_number":102,"context_line":"            LOG.error(encodeutils.exception_to_unicode(e))"},{"line_number":103,"context_line":"            raise webob.exc.HTTPInternalServerError()"},{"line_number":104,"context_line":"        return namespaces"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    @utils.mutating"}],"source_content_type":"text/x-python","patch_set":16,"id":"faf74708_58df3406","side":"PARENT","line":103,"in_reply_to":"4903acbc_1ce5057e","updated":"2021-08-04 19:29:41.000000000","message":"Not sure why this was here.","commit_id":"0cc8190d0edef73ffa7ee64c527e2d083cbb8bd1"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"00c84a6e67d12d861b2cb3e1744eb7119ef6068a","unresolved":true,"context_lines":[{"line_number":91,"context_line":"                # Get resource type associations"},{"line_number":92,"context_line":"                filters \u003d dict()"},{"line_number":93,"context_line":"                filters[\u0027namespace\u0027] \u003d db_namespace.namespace"},{"line_number":94,"context_line":"                repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":95,"context_line":"                resource_type_list \u003d [ResourceTypeAssociation.to_wsme_model("},{"line_number":96,"context_line":"                    resource_type) for resource_type in repo_rs_type_list]"},{"line_number":97,"context_line":"                if resource_type_list:"}],"source_content_type":"text/x-python","patch_set":16,"id":"9e0cf39f_e947ff06","line":94,"range":{"start_line":94,"start_character":16,"end_line":94,"end_character":65},"updated":"2021-08-04 19:22:00.000000000","message":"May be I need to enforce \u0027list_metadef_resource_types\u0027 check here as well.\nAt the moment (master behavior) it is returning 403 error if policy is disabled or policy check fails, but looking at if condition at line #97 actual behavior should be ignoring the error with log and skip attaching resource_type for that namespace ?","commit_id":"9857fb65e944ab87d718f2948583574baade320d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4cabca439c3f59f3e0059ae09ddf7d19bb170fec","unresolved":true,"context_lines":[{"line_number":91,"context_line":"                # Get resource type associations"},{"line_number":92,"context_line":"                filters \u003d dict()"},{"line_number":93,"context_line":"                filters[\u0027namespace\u0027] \u003d db_namespace.namespace"},{"line_number":94,"context_line":"                repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":95,"context_line":"                resource_type_list \u003d [ResourceTypeAssociation.to_wsme_model("},{"line_number":96,"context_line":"                    resource_type) for resource_type in repo_rs_type_list]"},{"line_number":97,"context_line":"                if resource_type_list:"}],"source_content_type":"text/x-python","patch_set":16,"id":"0092798c_e7366d64","line":94,"range":{"start_line":94,"start_character":16,"end_line":94,"end_character":65},"in_reply_to":"9e0cf39f_e947ff06","updated":"2021-08-04 20:20:03.000000000","message":"I guess, but I\u0027m not sure I understand the point. We previously identified that the namespace is the thing with permissions on it, so I have a hard time understanding why we need to separately control the visibility of the things inside it.\n\nI guess your point is just that we have those policies right now, so we should make this equivalent before we deprecate and remove the superfluous ones? If so, then I guess that\u0027s fine, it\u0027s just annoying to have to do the dance :)","commit_id":"9857fb65e944ab87d718f2948583574baade320d"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"840471edd01f852158e6e135da6933a0fb088986","unresolved":true,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"            # NOTE(abhishekk): This will check whether user is authorized to"},{"line_number":129,"context_line":"            # create new namespace or not."},{"line_number":130,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":131,"context_line":"                req.context,"},{"line_number":132,"context_line":"                enforcer\u003dself.policy).add_metadef_namespace()"},{"line_number":133,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"50286e99_86284439","line":130,"range":{"start_line":130,"start_character":23,"end_line":130,"end_character":39},"updated":"2021-08-04 19:12:43.000000000","message":"I may have answered my own question here after looking at the API reference [0]. Documenting it here in case it\u0027s useful for anyone else.\n\nHere is an example where the target built in the MetadefAPIPolicy constructor won\u0027t have a target that corresponds to the metadef namespace being created, but instead it will just have a project_id attribute that matches the project_id of the context, nooping the policy check.\n\nI think that might only be a problem when the API accepts multiple ways of setting the project_id for a resource. For example, if you\u0027re allowed to set the project_id for a namespace in the body, and if that\u0027s not present then default to using the project_id from the user\u0027s context object (which comes from the token).\n\nI just wanted to make sure we weren\u0027t allowing for someone to bypass the authorization check by doing something like:\n\n  POST /v2/metadefs/namespaces\n  {\n    \"namespace\": \"foo:bar\",\n    \"display_name\": \"example namespace\",\n    \"description\": \"A metadata definitions namespace for example use.\",\n    \"visibility\": \"public\",\n    \"protected\": true,\n    \"project_id\": \"5deb011aa9ba46aeac186a6fddb56f99\"\n  }\n\nWhere I don\u0027t have any authorization on 5deb011aa9ba46aeac186a6fddb56f99.\n\n[0] https://docs.openstack.org/api-ref/image/v2/metadefs-index.html?expanded\u003dcreate-namespace-detail#metadata-definition-namespaces","commit_id":"9857fb65e944ab87d718f2948583574baade320d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d85d95b0c49d49c8780adf4b4d9acbd7320604e1","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                req.context,"},{"line_number":147,"context_line":"                enforcer\u003dself.policy).add_metadef_namespace()"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"            new_namespace \u003d ns_factory.new_namespace(**namespace.to_dict())"},{"line_number":150,"context_line":"            ns_repo.add(new_namespace)"},{"line_number":151,"context_line":"            namespace_created \u003d True"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"a9ab0178_956e8538","line":149,"range":{"start_line":149,"start_character":28,"end_line":149,"end_character":52},"updated":"2021-08-05 16:01:10.000000000","message":"I think similar to gateway.get_*_repo we need to refactor gatewat.get_*_factory method as well as while adding new resource the policy is injected using factory object (see line no 149).","commit_id":"ba608deb7daa8d309a02e85cd50da8f8ea6dce05"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a8d8e86f782f8de9f6ada9daf9a6011cae22aa0","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                req.context,"},{"line_number":147,"context_line":"                enforcer\u003dself.policy).add_metadef_namespace()"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"            new_namespace \u003d ns_factory.new_namespace(**namespace.to_dict())"},{"line_number":150,"context_line":"            ns_repo.add(new_namespace)"},{"line_number":151,"context_line":"            namespace_created \u003d True"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"799bbc0e_a04475ba","line":149,"range":{"start_line":149,"start_character":28,"end_line":149,"end_character":52},"in_reply_to":"9bdeee2a_a98aa2b1","updated":"2021-08-10 16:06:16.000000000","message":"Yeah, confirmed. Also, what a friggen mess of layer upon factory upon layer :/","commit_id":"ba608deb7daa8d309a02e85cd50da8f8ea6dce05"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d3ea696179598a87b9eaa0976aedbffdf457ca4f","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                req.context,"},{"line_number":147,"context_line":"                enforcer\u003dself.policy).add_metadef_namespace()"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"            new_namespace \u003d ns_factory.new_namespace(**namespace.to_dict())"},{"line_number":150,"context_line":"            ns_repo.add(new_namespace)"},{"line_number":151,"context_line":"            namespace_created \u003d True"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9bdeee2a_a98aa2b1","line":149,"range":{"start_line":149,"start_character":28,"end_line":149,"end_character":52},"in_reply_to":"a9ab0178_956e8538","updated":"2021-08-05 17:12:16.000000000","message":"No, this is not the case. Policies will be injected/enforced at line #150 only, but we do need to make this change to get rid of readonly checks at auth layer.","commit_id":"ba608deb7daa8d309a02e85cd50da8f8ea6dce05"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a8d8e86f782f8de9f6ada9daf9a6011cae22aa0","unresolved":true,"context_lines":[{"line_number":68,"context_line":"            # list namespace\" check. Each namespace is checked against"},{"line_number":69,"context_line":"            # get_metadef_namespace below."},{"line_number":70,"context_line":"            target \u003d {\u0027project_id\u0027: req.context.project_id}"},{"line_number":71,"context_line":"            self.policy.enforce(req.context, \u0027get_metadef_namespaces\u0027, target)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"            # Get namespace id"},{"line_number":74,"context_line":"            if marker:"}],"source_content_type":"text/x-python","patch_set":18,"id":"4e7863e9_acb4b834","line":71,"updated":"2021-08-10 16:06:16.000000000","message":"You added get_metadef_namespaces() to the policy module - don\u0027t you want to call that here?","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0a2800a611e18e0923bb509c7fc25515aab8c213","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            # list namespace\" check. Each namespace is checked against"},{"line_number":69,"context_line":"            # get_metadef_namespace below."},{"line_number":70,"context_line":"            target \u003d {\u0027project_id\u0027: req.context.project_id}"},{"line_number":71,"context_line":"            self.policy.enforce(req.context, \u0027get_metadef_namespaces\u0027, target)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"            # Get namespace id"},{"line_number":74,"context_line":"            if marker:"}],"source_content_type":"text/x-python","patch_set":18,"id":"caddb1a0_6646dc39","line":71,"in_reply_to":"4e7863e9_acb4b834","updated":"2021-08-10 16:46:44.000000000","message":"Ack","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a8d8e86f782f8de9f6ada9daf9a6011cae22aa0","unresolved":true,"context_lines":[{"line_number":99,"context_line":"                    api_policy.MetadefAPIPolicy("},{"line_number":100,"context_line":"                        req.context,"},{"line_number":101,"context_line":"                        enforcer\u003dself.policy).list_metadef_resource_types()"},{"line_number":102,"context_line":"                    repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":103,"context_line":"                    resource_type_list \u003d ["},{"line_number":104,"context_line":"                        ResourceTypeAssociation.to_wsme_model("},{"line_number":105,"context_line":"                            resource_type"},{"line_number":106,"context_line":"                        ) for resource_type in repo_rs_type_list]"},{"line_number":107,"context_line":"                except webob.exc.HTTPForbidden:"},{"line_number":108,"context_line":"                    LOG.debug(\"User not permitted to view metadata resource \""},{"line_number":109,"context_line":"                              \"types\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"e320251e_b291c230","line":106,"range":{"start_line":102,"start_character":0,"end_line":106,"end_character":65},"updated":"2021-08-10 16:06:16.000000000","message":"AFAIK, this stuff can\u0027t raise HTTPForbidden, so should probably be outside the try...except below, right?","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0a2800a611e18e0923bb509c7fc25515aab8c213","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                    api_policy.MetadefAPIPolicy("},{"line_number":100,"context_line":"                        req.context,"},{"line_number":101,"context_line":"                        enforcer\u003dself.policy).list_metadef_resource_types()"},{"line_number":102,"context_line":"                    repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":103,"context_line":"                    resource_type_list \u003d ["},{"line_number":104,"context_line":"                        ResourceTypeAssociation.to_wsme_model("},{"line_number":105,"context_line":"                            resource_type"},{"line_number":106,"context_line":"                        ) for resource_type in repo_rs_type_list]"},{"line_number":107,"context_line":"                except webob.exc.HTTPForbidden:"},{"line_number":108,"context_line":"                    LOG.debug(\"User not permitted to view metadata resource \""},{"line_number":109,"context_line":"                              \"types\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"c07e0bae_c72f7984","line":106,"range":{"start_line":102,"start_character":0,"end_line":106,"end_character":65},"in_reply_to":"e320251e_b291c230","updated":"2021-08-10 16:46:44.000000000","message":"Ack","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a8d8e86f782f8de9f6ada9daf9a6011cae22aa0","unresolved":true,"context_lines":[{"line_number":107,"context_line":"                except webob.exc.HTTPForbidden:"},{"line_number":108,"context_line":"                    LOG.debug(\"User not permitted to view metadata resource \""},{"line_number":109,"context_line":"                              \"types\")"},{"line_number":110,"context_line":"                    resource_type_list \u003d []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"                if resource_type_list:"},{"line_number":113,"context_line":"                    db_namespace.resource_type_associations \u003d ("}],"source_content_type":"text/x-python","patch_set":18,"id":"046a336b_d635d75c","line":110,"updated":"2021-08-10 16:06:16.000000000","message":"If we hit this forbidden, it means we don\u0027t need to proceed to the next namespace in ns_list, right? If so, we should break here. Not just to save the cycles, but also to avoid your LOG.debug multiple times, right?","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0a2800a611e18e0923bb509c7fc25515aab8c213","unresolved":true,"context_lines":[{"line_number":107,"context_line":"                except webob.exc.HTTPForbidden:"},{"line_number":108,"context_line":"                    LOG.debug(\"User not permitted to view metadata resource \""},{"line_number":109,"context_line":"                              \"types\")"},{"line_number":110,"context_line":"                    resource_type_list \u003d []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"                if resource_type_list:"},{"line_number":113,"context_line":"                    db_namespace.resource_type_associations \u003d ("}],"source_content_type":"text/x-python","patch_set":18,"id":"316f9e43_538329c7","line":110,"in_reply_to":"046a336b_d635d75c","updated":"2021-08-10 16:46:44.000000000","message":"I explained this in a comment at line no 94. I think I am adding namespace to the output without attaching resource types to it, but if you think we should persist to old behavior then I will break the loop here and return appropriate response to the user.","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"70435b93bf59b717168de4bacf4d0deb03ad99ee","unresolved":true,"context_lines":[{"line_number":107,"context_line":"                except webob.exc.HTTPForbidden:"},{"line_number":108,"context_line":"                    LOG.debug(\"User not permitted to view metadata resource \""},{"line_number":109,"context_line":"                              \"types\")"},{"line_number":110,"context_line":"                    resource_type_list \u003d []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"                if resource_type_list:"},{"line_number":113,"context_line":"                    db_namespace.resource_type_associations \u003d ("}],"source_content_type":"text/x-python","patch_set":18,"id":"f969c359_43e1bc42","line":110,"in_reply_to":"0958420b_8cfb57b8","updated":"2021-08-10 19:19:29.000000000","message":"Done,\nMakes sense,\nI should have stayed away for a day :D","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"200a573b3b8e7813914a708274c7643a5ee6444c","unresolved":true,"context_lines":[{"line_number":107,"context_line":"                except webob.exc.HTTPForbidden:"},{"line_number":108,"context_line":"                    LOG.debug(\"User not permitted to view metadata resource \""},{"line_number":109,"context_line":"                              \"types\")"},{"line_number":110,"context_line":"                    resource_type_list \u003d []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"                if resource_type_list:"},{"line_number":113,"context_line":"                    db_namespace.resource_type_associations \u003d ("}],"source_content_type":"text/x-python","patch_set":18,"id":"0958420b_8cfb57b8","line":110,"in_reply_to":"316f9e43_538329c7","updated":"2021-08-10 17:02:02.000000000","message":"No, not the old behavior. I\u0027m saying there\u0027s no reason to run L99 more than once. If it fails on iteration 0 it will fail on iteration 1. I\u0027m saying here, just set the empty list and break, but don\u0027t return or raise to the user.","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a8d8e86f782f8de9f6ada9daf9a6011cae22aa0","unresolved":true,"context_lines":[{"line_number":168,"context_line":"                api_policy.MetadefAPIPolicy("},{"line_number":169,"context_line":"                    req.context,"},{"line_number":170,"context_line":"                    enforcer\u003dself.policy"},{"line_number":171,"context_line":"                ).add_metadef_resource_type_association()"},{"line_number":172,"context_line":"                for resource_type in namespace.resource_type_associations:"},{"line_number":173,"context_line":"                    new_resource \u003d rs_factory.new_resource_type("},{"line_number":174,"context_line":"                        namespace\u003dnamespace.namespace,"}],"source_content_type":"text/x-python","patch_set":18,"id":"8b2e5c5c_e995c8dd","line":171,"updated":"2021-08-10 16:06:16.000000000","message":"If we fail this check we will have already created a namespace in the DB, but failed the add_metadef_resource_type_association check, we will not clean that up and also return 403, right? Seems like maybe the best plan is to check all the policies up front before we start creating anything.\n\nI think this bug was already present in the previous version, but the old model made it much harder to do this right. With the new model, we can do the right thing, I think.","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"70435b93bf59b717168de4bacf4d0deb03ad99ee","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                api_policy.MetadefAPIPolicy("},{"line_number":169,"context_line":"                    req.context,"},{"line_number":170,"context_line":"                    enforcer\u003dself.policy"},{"line_number":171,"context_line":"                ).add_metadef_resource_type_association()"},{"line_number":172,"context_line":"                for resource_type in namespace.resource_type_associations:"},{"line_number":173,"context_line":"                    new_resource \u003d rs_factory.new_resource_type("},{"line_number":174,"context_line":"                        namespace\u003dnamespace.namespace,"}],"source_content_type":"text/x-python","patch_set":18,"id":"8f87e1f2_7f88e54f","line":171,"in_reply_to":"8b2e5c5c_e995c8dd","updated":"2021-08-10 19:19:29.000000000","message":"Done","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a8d8e86f782f8de9f6ada9daf9a6011cae22aa0","unresolved":true,"context_lines":[{"line_number":404,"context_line":"            ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":405,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":406,"context_line":"                req.context,"},{"line_number":407,"context_line":"                enforcer\u003dself.policy).modify_metadef_namespace()"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"            ns_obj._old_namespace \u003d ns_obj.namespace"},{"line_number":410,"context_line":"            ns_obj.namespace \u003d wsme_utils._get_value(user_ns.namespace)"}],"source_content_type":"text/x-python","patch_set":18,"id":"f21ade68_4809df12","line":407,"updated":"2021-08-10 16:06:16.000000000","message":"Do we need to translate this to 404 if they can\u0027t see the namespace? Otherwise I could probe for existence by trying to update namespaces that might exist...","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"70435b93bf59b717168de4bacf4d0deb03ad99ee","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":405,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":406,"context_line":"                req.context,"},{"line_number":407,"context_line":"                enforcer\u003dself.policy).modify_metadef_namespace()"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"            ns_obj._old_namespace \u003d ns_obj.namespace"},{"line_number":410,"context_line":"            ns_obj.namespace \u003d wsme_utils._get_value(user_ns.namespace)"}],"source_content_type":"text/x-python","patch_set":18,"id":"41db7b8b_763ce39a","line":407,"in_reply_to":"4113b1cb_dcaa84f4","updated":"2021-08-10 19:19:29.000000000","message":"Done","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"200a573b3b8e7813914a708274c7643a5ee6444c","unresolved":true,"context_lines":[{"line_number":404,"context_line":"            ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":405,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":406,"context_line":"                req.context,"},{"line_number":407,"context_line":"                enforcer\u003dself.policy).modify_metadef_namespace()"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"            ns_obj._old_namespace \u003d ns_obj.namespace"},{"line_number":410,"context_line":"            ns_obj.namespace \u003d wsme_utils._get_value(user_ns.namespace)"}],"source_content_type":"text/x-python","patch_set":18,"id":"4113b1cb_dcaa84f4","line":407,"in_reply_to":"5bb6e65b_4c09a009","updated":"2021-08-10 17:02:02.000000000","message":"Admin-only by default, but this can be overridden right? I guess I keep leaning back to the question of how much of this we need to fix. If we\u0027re going to let them set it to non-admin *and* provide all the finer-grained policy knobs around objects, tags, etc, it seems like it should behave properly.","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0a2800a611e18e0923bb509c7fc25515aab8c213","unresolved":true,"context_lines":[{"line_number":404,"context_line":"            ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":405,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":406,"context_line":"                req.context,"},{"line_number":407,"context_line":"                enforcer\u003dself.policy).modify_metadef_namespace()"},{"line_number":408,"context_line":""},{"line_number":409,"context_line":"            ns_obj._old_namespace \u003d ns_obj.namespace"},{"line_number":410,"context_line":"            ns_obj.namespace \u003d wsme_utils._get_value(user_ns.namespace)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5bb6e65b_4c09a009","line":407,"in_reply_to":"f21ade68_4809df12","updated":"2021-08-10 16:46:44.000000000","message":"This is Admin only API so i think admin will be able to see all the namespaces (public/private both).","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a8d8e86f782f8de9f6ada9daf9a6011cae22aa0","unresolved":true,"context_lines":[{"line_number":443,"context_line":"            namespace_obj \u003d namespace_repo.get(namespace)"},{"line_number":444,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":445,"context_line":"                req.context,"},{"line_number":446,"context_line":"                enforcer\u003dself.policy).delete_metadef_namespace()"},{"line_number":447,"context_line":"            namespace_obj.delete()"},{"line_number":448,"context_line":"            namespace_repo.remove(namespace_obj)"},{"line_number":449,"context_line":"        except exception.Forbidden as e:"}],"source_content_type":"text/x-python","patch_set":18,"id":"91a12f0d_727b62bd","line":446,"updated":"2021-08-10 16:06:16.000000000","message":"Same here and below?","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0a2800a611e18e0923bb509c7fc25515aab8c213","unresolved":true,"context_lines":[{"line_number":443,"context_line":"            namespace_obj \u003d namespace_repo.get(namespace)"},{"line_number":444,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":445,"context_line":"                req.context,"},{"line_number":446,"context_line":"                enforcer\u003dself.policy).delete_metadef_namespace()"},{"line_number":447,"context_line":"            namespace_obj.delete()"},{"line_number":448,"context_line":"            namespace_repo.remove(namespace_obj)"},{"line_number":449,"context_line":"        except exception.Forbidden as e:"}],"source_content_type":"text/x-python","patch_set":18,"id":"f11326f0_a29db2b1","line":446,"in_reply_to":"91a12f0d_727b62bd","updated":"2021-08-10 16:46:44.000000000","message":"ditto, admin only API","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"70435b93bf59b717168de4bacf4d0deb03ad99ee","unresolved":false,"context_lines":[{"line_number":443,"context_line":"            namespace_obj \u003d namespace_repo.get(namespace)"},{"line_number":444,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":445,"context_line":"                req.context,"},{"line_number":446,"context_line":"                enforcer\u003dself.policy).delete_metadef_namespace()"},{"line_number":447,"context_line":"            namespace_obj.delete()"},{"line_number":448,"context_line":"            namespace_repo.remove(namespace_obj)"},{"line_number":449,"context_line":"        except exception.Forbidden as e:"}],"source_content_type":"text/x-python","patch_set":18,"id":"8c0f4f12_626df984","line":446,"in_reply_to":"f11326f0_a29db2b1","updated":"2021-08-10 19:19:29.000000000","message":"Done","commit_id":"1e4c9a890297a5da794d960f3e73b05daaa2a23a"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"04924797b7badbb717d3c6afae9b51be3e204a4f","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            # NOTE(abhishekk): This is just a \"do you have permission to"},{"line_number":68,"context_line":"            # list namespace\" check. Each namespace is checked against"},{"line_number":69,"context_line":"            # get_metadef_namespace below."},{"line_number":70,"context_line":"            target \u003d {\u0027project_id\u0027: req.context.project_id}"},{"line_number":71,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":72,"context_line":"                req.context,"},{"line_number":73,"context_line":"                target\u003dtarget,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5236d358_aee95c80","line":70,"updated":"2021-08-10 21:27:23.000000000","message":"This is a noop because somewhere in the lower layer we\u0027re filtering based on the context.owner/context.project_id?","commit_id":"caeb34a7804e1cd5f36fbd30d1e0bb6f482db792"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3c787cd6957a3bd2e5dc3f537aa099d2d6d6550d","unresolved":true,"context_lines":[{"line_number":67,"context_line":"            # NOTE(abhishekk): This is just a \"do you have permission to"},{"line_number":68,"context_line":"            # list namespace\" check. Each namespace is checked against"},{"line_number":69,"context_line":"            # get_metadef_namespace below."},{"line_number":70,"context_line":"            target \u003d {\u0027project_id\u0027: req.context.project_id}"},{"line_number":71,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":72,"context_line":"                req.context,"},{"line_number":73,"context_line":"                target\u003dtarget,"}],"source_content_type":"text/x-python","patch_set":19,"id":"d990f315_723d68a3","line":70,"in_reply_to":"5236d358_aee95c80","updated":"2021-08-10 21:47:42.000000000","message":"Correct!","commit_id":"caeb34a7804e1cd5f36fbd30d1e0bb6f482db792"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"66ac003144b829a2e8fb9147ef3ca135b5e32014","unresolved":true,"context_lines":[{"line_number":97,"context_line":"                # NOTE(abhishekk): Resource types are related to namespace, but"},{"line_number":98,"context_line":"                # as we have separate policy for the same, enforcing it here"},{"line_number":99,"context_line":"                # and ignoring the failure as this is call related to namespace"},{"line_number":100,"context_line":"                # and this should be actual behavior."},{"line_number":101,"context_line":"                try:"},{"line_number":102,"context_line":"                    api_policy.MetadefAPIPolicy("},{"line_number":103,"context_line":"                        req.context,"}],"source_content_type":"text/x-python","patch_set":19,"id":"954742f5_19cabf85","line":100,"updated":"2021-08-10 19:39:14.000000000","message":"Okay, so this is actually moving forward with ignoring this check right? Why even do it and ignore? And, we\u0027re still logging \"user not permitted\" like elsewhere, but we\u0027re not returning 403 like we do in those other places.\n\nIf you want to leave the check, should this be a warning? Like:\n\n LOG.warning(\u0027User not permitted to view metadef resource types according to deprecated list_metadef_resource_types policy; allowing anyway\u0027)\n\n?\n\nIf we are going to ignore the result (or not do the check) we need a reno for this right?","commit_id":"caeb34a7804e1cd5f36fbd30d1e0bb6f482db792"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"66ac003144b829a2e8fb9147ef3ca135b5e32014","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                except webob.exc.HTTPForbidden:"},{"line_number":106,"context_line":"                    LOG.debug(\"User not permitted to view metadata resource \""},{"line_number":107,"context_line":"                              \"types\")"},{"line_number":108,"context_line":"                    break"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"                repo_rs_type_list \u003d rs_repo.list(filters\u003dfilters)"},{"line_number":111,"context_line":"                resource_type_list \u003d ["}],"source_content_type":"text/x-python","patch_set":19,"id":"d1c3de70_76281d4c","line":108,"updated":"2021-08-10 19:39:14.000000000","message":"I didn\u0027t mention this before, because I got caught up in the failure path, but you\u0027re running this policy check N times for N namespaces, but the result will always be the same right? Can we just run it once before, especially as we\u0027re just ignoring the result?","commit_id":"caeb34a7804e1cd5f36fbd30d1e0bb6f482db792"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"66ac003144b829a2e8fb9147ef3ca135b5e32014","unresolved":true,"context_lines":[{"line_number":166,"context_line":"            if namespace.tags:"},{"line_number":167,"context_line":"                api_policy.MetadefAPIPolicy("},{"line_number":168,"context_line":"                    req.context,"},{"line_number":169,"context_line":"                    enforcer\u003dself.policy).add_metadef_tag()"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"            # NOTE(abhishekk): As we are getting rid of auth layer, this"},{"line_number":172,"context_line":"            # is the place where we should add owner if it is not specified"}],"source_content_type":"text/x-python","patch_set":19,"id":"cec6265b_d4ac99b9","line":169,"updated":"2021-08-10 19:39:14.000000000","message":"If we are keeping these, then doing them all up front like I mentioned is good.\n\nHowever, it seems inconsistent with your above change to check but ignore the finer-grained one. Are we deprecating and ignoring everything other than the namespace* policy checks (as in list), or are we honoring them all, like this seems to do?","commit_id":"caeb34a7804e1cd5f36fbd30d1e0bb6f482db792"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"3c787cd6957a3bd2e5dc3f537aa099d2d6d6550d","unresolved":true,"context_lines":[{"line_number":166,"context_line":"            if namespace.tags:"},{"line_number":167,"context_line":"                api_policy.MetadefAPIPolicy("},{"line_number":168,"context_line":"                    req.context,"},{"line_number":169,"context_line":"                    enforcer\u003dself.policy).add_metadef_tag()"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"            # NOTE(abhishekk): As we are getting rid of auth layer, this"},{"line_number":172,"context_line":"            # is the place where we should add owner if it is not specified"}],"source_content_type":"text/x-python","patch_set":19,"id":"b08ab1c5_76a01814","line":169,"in_reply_to":"cec6265b_d4ac99b9","updated":"2021-08-10 21:47:42.000000000","message":"I think I will persist with the old behavior here.","commit_id":"caeb34a7804e1cd5f36fbd30d1e0bb6f482db792"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"badb4f811c8331bcc40a3408fac33eb852997a12","unresolved":true,"context_lines":[{"line_number":158,"context_line":"            if namespace.tags:"},{"line_number":159,"context_line":"                api_policy.MetadefAPIPolicy("},{"line_number":160,"context_line":"                    req.context,"},{"line_number":161,"context_line":"                    enforcer\u003dself.policy).add_metadef_tag()"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"            # NOTE(abhishekk): As we are getting rid of auth layer, this"},{"line_number":164,"context_line":"            # is the place where we should add owner if it is not specified"}],"source_content_type":"text/x-python","patch_set":21,"id":"1be25390_a5ccb4e0","line":161,"range":{"start_line":161,"start_character":0,"end_line":161,"end_character":20},"updated":"2021-08-11 18:20:31.000000000","message":"Minor performance enhancement idea: we could create a single MetadefAPIPolicy object here and use it multiple times.","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b5ac9e4abd54ca971952b35c0b569acaee3651d","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            if namespace.tags:"},{"line_number":159,"context_line":"                api_policy.MetadefAPIPolicy("},{"line_number":160,"context_line":"                    req.context,"},{"line_number":161,"context_line":"                    enforcer\u003dself.policy).add_metadef_tag()"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"            # NOTE(abhishekk): As we are getting rid of auth layer, this"},{"line_number":164,"context_line":"            # is the place where we should add owner if it is not specified"}],"source_content_type":"text/x-python","patch_set":21,"id":"4cfb3997_13266f5f","line":161,"range":{"start_line":161,"start_character":0,"end_line":161,"end_character":20},"in_reply_to":"1be25390_a5ccb4e0","updated":"2021-08-11 19:06:31.000000000","message":"Done","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"badb4f811c8331bcc40a3408fac33eb852997a12","unresolved":true,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            # NOTE(abhishekk): We also need to fetch resource_types, objects,"},{"line_number":289,"context_line":"            # properties, tags associated with namespace, so better to check"},{"line_number":290,"context_line":"            # whether user has permissions for the same."},{"line_number":291,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":292,"context_line":"                req.context,"},{"line_number":293,"context_line":"                md_resource\u003dnamespace_obj,"}],"source_content_type":"text/x-python","patch_set":21,"id":"3a9c3a64_b8d4ace6","line":290,"range":{"start_line":290,"start_character":14,"end_line":290,"end_character":21},"updated":"2021-08-11 18:20:31.000000000","message":"Ditto","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"7b5ac9e4abd54ca971952b35c0b569acaee3651d","unresolved":false,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            # NOTE(abhishekk): We also need to fetch resource_types, objects,"},{"line_number":289,"context_line":"            # properties, tags associated with namespace, so better to check"},{"line_number":290,"context_line":"            # whether user has permissions for the same."},{"line_number":291,"context_line":"            api_policy.MetadefAPIPolicy("},{"line_number":292,"context_line":"                req.context,"},{"line_number":293,"context_line":"                md_resource\u003dnamespace_obj,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5448e2e1_3675c889","line":290,"range":{"start_line":290,"start_character":14,"end_line":290,"end_character":21},"in_reply_to":"3a9c3a64_b8d4ace6","updated":"2021-08-11 19:06:31.000000000","message":"Done","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"053bad074a9bfa35175f42c18e23eaa2dd5de98f","unresolved":true,"context_lines":[{"line_number":369,"context_line":"        namespace_repo \u003d self.gateway.get_metadef_namespace_repo("},{"line_number":370,"context_line":"            req.context, authorization_layer\u003dFalse)"},{"line_number":371,"context_line":"        try:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":374,"context_line":"            except exception.Forbidden:"},{"line_number":375,"context_line":"                # NOTE (abhishekk): Returning 404 Not Found as the"}],"source_content_type":"text/x-python","patch_set":21,"id":"690c9022_ab0eab8a","line":372,"updated":"2021-08-11 16:15:57.000000000","message":"I really don\u0027t like this nested try..except bit here. Not sure why you changed it from the previous PS, which just did this early and raised the HTTP version. What you have here is try..try..except raise1...except 1: raise2, which is confusing and not necessary, IMHO.","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6401de0713afd70d56414e42ca027a59d2a332f1","unresolved":true,"context_lines":[{"line_number":369,"context_line":"        namespace_repo \u003d self.gateway.get_metadef_namespace_repo("},{"line_number":370,"context_line":"            req.context, authorization_layer\u003dFalse)"},{"line_number":371,"context_line":"        try:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":374,"context_line":"            except exception.Forbidden:"},{"line_number":375,"context_line":"                # NOTE (abhishekk): Returning 404 Not Found as the"}],"source_content_type":"text/x-python","patch_set":21,"id":"9bfd2dc6_a4e18ab8","line":372,"in_reply_to":"690c9022_ab0eab8a","updated":"2021-08-11 16:36:05.000000000","message":"Ah, I bet this is because it can raise exception.NotFound as well and you need it translated. I guess me asking for the test case yielded that fix yeah? :)","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a925dff2bd92b5bd7d618e1a441096286b9f7f6","unresolved":true,"context_lines":[{"line_number":369,"context_line":"        namespace_repo \u003d self.gateway.get_metadef_namespace_repo("},{"line_number":370,"context_line":"            req.context, authorization_layer\u003dFalse)"},{"line_number":371,"context_line":"        try:"},{"line_number":372,"context_line":"            try:"},{"line_number":373,"context_line":"                ns_obj \u003d namespace_repo.get(namespace)"},{"line_number":374,"context_line":"            except exception.Forbidden:"},{"line_number":375,"context_line":"                # NOTE (abhishekk): Returning 404 Not Found as the"}],"source_content_type":"text/x-python","patch_set":21,"id":"088de463_54531183","line":372,"in_reply_to":"9bfd2dc6_a4e18ab8","updated":"2021-08-11 16:45:04.000000000","message":"Okay, I see you were catching both before in the same statement, so I guess not a bug.","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"}],"glance/api/v2/policy.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"840471edd01f852158e6e135da6933a0fb088986","unresolved":true,"context_lines":[{"line_number":64,"context_line":"                                               enforcer\u003dself.enforcer)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def _build_target(self):"},{"line_number":67,"context_line":"        target \u003d {"},{"line_number":68,"context_line":"            \"project_id\": self._context.project_id"},{"line_number":69,"context_line":"        }"},{"line_number":70,"context_line":"        if self._md_resource:"},{"line_number":71,"context_line":"            target[\u0027project_id\u0027] \u003d self._md_resource.owner"},{"line_number":72,"context_line":"            target[\u0027visibility\u0027] \u003d self._md_resource.visibility"}],"source_content_type":"text/x-python","patch_set":16,"id":"d281c1ab_4d69c4b7","line":69,"range":{"start_line":67,"start_character":8,"end_line":69,"end_character":9},"updated":"2021-08-04 19:12:43.000000000","message":"Is this always going to be overwritten at line 71?\n\nI guess my concern here is that the default behavior of the kwarg on line 55 potentially bypasses the policy check by making the target project_id explicitly match the context.project_id:\n\n  role:member and project_id:%(project_id)s\n  # with substitutions using line 67\n  role:member and 5deb011:5deb011\n\nCan we make it so the constructor always accepts an md_resource so that the target is built accurately?","commit_id":"9857fb65e944ab87d718f2948583574baade320d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"423b8cae0c5ad81816579213b15c15c8377d6479","unresolved":true,"context_lines":[{"line_number":64,"context_line":"                                               enforcer\u003dself.enforcer)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def _build_target(self):"},{"line_number":67,"context_line":"        target \u003d {"},{"line_number":68,"context_line":"            \"project_id\": self._context.project_id"},{"line_number":69,"context_line":"        }"},{"line_number":70,"context_line":"        if self._md_resource:"},{"line_number":71,"context_line":"            target[\u0027project_id\u0027] \u003d self._md_resource.owner"},{"line_number":72,"context_line":"            target[\u0027visibility\u0027] \u003d self._md_resource.visibility"}],"source_content_type":"text/x-python","patch_set":16,"id":"e245ae50_f4a8a4b8","line":69,"range":{"start_line":67,"start_character":8,"end_line":69,"end_character":9},"in_reply_to":"d281c1ab_4d69c4b7","updated":"2021-08-04 19:29:41.000000000","message":"Not clearly understood what you are saying :D","commit_id":"9857fb65e944ab87d718f2948583574baade320d"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"7e9e7576c2766e8bc24d4587c465f796966d6e92","unresolved":false,"context_lines":[{"line_number":64,"context_line":"                                               enforcer\u003dself.enforcer)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def _build_target(self):"},{"line_number":67,"context_line":"        target \u003d {"},{"line_number":68,"context_line":"            \"project_id\": self._context.project_id"},{"line_number":69,"context_line":"        }"},{"line_number":70,"context_line":"        if self._md_resource:"},{"line_number":71,"context_line":"            target[\u0027project_id\u0027] \u003d self._md_resource.owner"},{"line_number":72,"context_line":"            target[\u0027visibility\u0027] \u003d self._md_resource.visibility"}],"source_content_type":"text/x-python","patch_set":16,"id":"729e2820_a28818c6","line":69,"range":{"start_line":67,"start_character":8,"end_line":69,"end_character":9},"in_reply_to":"e245ae50_f4a8a4b8","updated":"2021-08-04 20:07:56.000000000","message":"I think I answered my question here in the previous file with my comment on line 130.","commit_id":"9857fb65e944ab87d718f2948583574baade320d"}],"glance/tests/functional/v2/test_metadef_namespace_api_policy.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"359c333f4b051d21e2d586651fdccbd6603db4bf","unresolved":true,"context_lines":[{"line_number":68,"context_line":"    def start_server(self):"},{"line_number":69,"context_line":"        with mock.patch.object(policy, \u0027Enforcer\u0027) as mock_enf:"},{"line_number":70,"context_line":"            mock_enf.return_value \u003d self.policy"},{"line_number":71,"context_line":"            super(TestMetadefNamespacesPolicy, self).start_server()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def test_namespace_list_basic(self):"},{"line_number":74,"context_line":"        self.start_server()"}],"source_content_type":"text/x-python","patch_set":8,"id":"5fdd3c6e_f1dd6f7f","line":71,"updated":"2021-07-21 18:50:49.000000000","message":"I was actually thinking we\u0027d put these three in a base test class to inherit from, but we can clean that up later.","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"830fd193262103b54fd720b4cedda215e53e6566","unresolved":true,"context_lines":[{"line_number":68,"context_line":"    def start_server(self):"},{"line_number":69,"context_line":"        with mock.patch.object(policy, \u0027Enforcer\u0027) as mock_enf:"},{"line_number":70,"context_line":"            mock_enf.return_value \u003d self.policy"},{"line_number":71,"context_line":"            super(TestMetadefNamespacesPolicy, self).start_server()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def test_namespace_list_basic(self):"},{"line_number":74,"context_line":"        self.start_server()"}],"source_content_type":"text/x-python","patch_set":8,"id":"bf272517_e2b1ec7e","line":71,"in_reply_to":"5fdd3c6e_f1dd6f7f","updated":"2021-07-21 19:15:13.000000000","message":"May be in last patch to avoid conflicts at the moment","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"359c333f4b051d21e2d586651fdccbd6603db4bf","unresolved":true,"context_lines":[{"line_number":120,"context_line":"        # Now disable add_metadef_namespace permissions and make sure any other"},{"line_number":121,"context_line":"        # attempts fail"},{"line_number":122,"context_line":"        self.set_policy_rules({\u0027add_metadef_namespace\u0027: \u0027!\u0027})"},{"line_number":123,"context_line":"        resp \u003d self.api_post(path, json\u003ddata)"},{"line_number":124,"context_line":"        self.assertEqual(403, resp.status_code)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def test_namespace_create_with_resource_type_associations(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"85091421_c0e8fa6d","line":123,"range":{"start_line":123,"start_character":40,"end_line":123,"end_character":44},"updated":"2021-07-21 18:50:49.000000000","message":"Might be good to make this a second namespace, to make sure we\u0027re not getting 403 because \"You can\u0027t overwrite a namespace\" or something like that.","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9008cc240798ed29a9959ea3ef3125fef991b1ee","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        # Now disable add_metadef_namespace permissions and make sure any other"},{"line_number":121,"context_line":"        # attempts fail"},{"line_number":122,"context_line":"        self.set_policy_rules({\u0027add_metadef_namespace\u0027: \u0027!\u0027})"},{"line_number":123,"context_line":"        resp \u003d self.api_post(path, json\u003ddata)"},{"line_number":124,"context_line":"        self.assertEqual(403, resp.status_code)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def test_namespace_create_with_resource_type_associations(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"d760bca0_58913c72","line":123,"range":{"start_line":123,"start_character":40,"end_line":123,"end_character":44},"in_reply_to":"0be0bcc9_549d7e70","updated":"2021-07-22 05:46:47.000000000","message":"Done","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"830fd193262103b54fd720b4cedda215e53e6566","unresolved":true,"context_lines":[{"line_number":120,"context_line":"        # Now disable add_metadef_namespace permissions and make sure any other"},{"line_number":121,"context_line":"        # attempts fail"},{"line_number":122,"context_line":"        self.set_policy_rules({\u0027add_metadef_namespace\u0027: \u0027!\u0027})"},{"line_number":123,"context_line":"        resp \u003d self.api_post(path, json\u003ddata)"},{"line_number":124,"context_line":"        self.assertEqual(403, resp.status_code)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def test_namespace_create_with_resource_type_associations(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"0be0bcc9_549d7e70","line":123,"range":{"start_line":123,"start_character":40,"end_line":123,"end_character":44},"in_reply_to":"85091421_c0e8fa6d","updated":"2021-07-21 19:15:13.000000000","message":"Sounds good, will fix it tomorrow.","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"359c333f4b051d21e2d586651fdccbd6603db4bf","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        self.assertIn(\u0027properties\u0027, md_resource)"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # Now disable get_metadef_properties policy to ensure that you are"},{"line_number":307,"context_line":"        # get namespace and empty list of objects"},{"line_number":308,"context_line":"        self.set_policy_rules({"},{"line_number":309,"context_line":"            \u0027get_metadef_objects\u0027: \u0027@\u0027,"},{"line_number":310,"context_line":"            \u0027get_metadef_namespace\u0027: \u0027@\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"56adbb77_7b5ec0ef","line":307,"range":{"start_line":307,"start_character":10,"end_line":307,"end_character":13},"updated":"2021-07-21 18:50:49.000000000","message":"\"able to get\", here and elsewhere.","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9008cc240798ed29a9959ea3ef3125fef991b1ee","unresolved":false,"context_lines":[{"line_number":304,"context_line":"        self.assertIn(\u0027properties\u0027, md_resource)"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # Now disable get_metadef_properties policy to ensure that you are"},{"line_number":307,"context_line":"        # get namespace and empty list of objects"},{"line_number":308,"context_line":"        self.set_policy_rules({"},{"line_number":309,"context_line":"            \u0027get_metadef_objects\u0027: \u0027@\u0027,"},{"line_number":310,"context_line":"            \u0027get_metadef_namespace\u0027: \u0027@\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3b4f9fba_5386d73d","line":307,"range":{"start_line":307,"start_character":10,"end_line":307,"end_character":13},"in_reply_to":"1b72c2a7_fce1a8ae","updated":"2021-07-22 05:46:47.000000000","message":"Done","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"830fd193262103b54fd720b4cedda215e53e6566","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        self.assertIn(\u0027properties\u0027, md_resource)"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        # Now disable get_metadef_properties policy to ensure that you are"},{"line_number":307,"context_line":"        # get namespace and empty list of objects"},{"line_number":308,"context_line":"        self.set_policy_rules({"},{"line_number":309,"context_line":"            \u0027get_metadef_objects\u0027: \u0027@\u0027,"},{"line_number":310,"context_line":"            \u0027get_metadef_namespace\u0027: \u0027@\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1b72c2a7_fce1a8ae","line":307,"range":{"start_line":307,"start_character":10,"end_line":307,"end_character":13},"in_reply_to":"56adbb77_7b5ec0ef","updated":"2021-07-21 19:15:13.000000000","message":"ack","commit_id":"d3a32eae038d11894c3edd729dce22419d0c3e9c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b20c66a6eb0b92519a2cbc1bbfc1765078ef3072","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                \"properties_target\": \"temp\""},{"line_number":136,"context_line":"            }],"},{"line_number":137,"context_line":"        }"},{"line_number":138,"context_line":"        NAME_SPACE1.update(data)"},{"line_number":139,"context_line":"        md_resource \u003d self._create_metadef_resource(path\u003dpath,"},{"line_number":140,"context_line":"                                                    data\u003dNAME_SPACE1)"},{"line_number":141,"context_line":"        self.assertEqual(\u0027MyNamespace\u0027, md_resource[\u0027namespace\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"df64a537_0edd964d","line":138,"range":{"start_line":138,"start_character":8,"end_line":138,"end_character":32},"updated":"2021-07-22 14:43:34.000000000","message":"This will change what other tests do depending on the ordering, and is a good way to get spurious failures. Can you make this:\n\n data.update(NAME_SPACE1)\n\nto avoid modifying the global?","commit_id":"0963a78749f3e4f93f5dad502c876d5d2af1e565"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"8ba7b5efd79d32551136234dfda5a646b9390141","unresolved":true,"context_lines":[{"line_number":135,"context_line":"                \"properties_target\": \"temp\""},{"line_number":136,"context_line":"            }],"},{"line_number":137,"context_line":"        }"},{"line_number":138,"context_line":"        NAME_SPACE1.update(data)"},{"line_number":139,"context_line":"        md_resource \u003d self._create_metadef_resource(path\u003dpath,"},{"line_number":140,"context_line":"                                                    data\u003dNAME_SPACE1)"},{"line_number":141,"context_line":"        self.assertEqual(\u0027MyNamespace\u0027, md_resource[\u0027namespace\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"e7e85971_5ec8046c","line":138,"range":{"start_line":138,"start_character":8,"end_line":138,"end_character":32},"in_reply_to":"df64a537_0edd964d","updated":"2021-07-22 14:45:48.000000000","message":"Sounds good, will update it in next revision!","commit_id":"0963a78749f3e4f93f5dad502c876d5d2af1e565"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"0fae1239806587944f1b7fd5375c4eb2deaff921","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                \"properties_target\": \"temp\""},{"line_number":136,"context_line":"            }],"},{"line_number":137,"context_line":"        }"},{"line_number":138,"context_line":"        NAME_SPACE1.update(data)"},{"line_number":139,"context_line":"        md_resource \u003d self._create_metadef_resource(path\u003dpath,"},{"line_number":140,"context_line":"                                                    data\u003dNAME_SPACE1)"},{"line_number":141,"context_line":"        self.assertEqual(\u0027MyNamespace\u0027, md_resource[\u0027namespace\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"23c7d031_69eb2fce","line":138,"range":{"start_line":138,"start_character":8,"end_line":138,"end_character":32},"in_reply_to":"e7e85971_5ec8046c","updated":"2021-07-22 18:14:49.000000000","message":"Done","commit_id":"0963a78749f3e4f93f5dad502c876d5d2af1e565"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cfda1d73fc425470830532298c328f0def38c53b","unresolved":true,"context_lines":[{"line_number":100,"context_line":"        # Now make sure \u0027get_metadef_namespaces\u0027 allows user to get all the"},{"line_number":101,"context_line":"        # namespaces"},{"line_number":102,"context_line":"        resp \u003d self.api_get(path)"},{"line_number":103,"context_line":"        md_resource \u003d jsonutils.loads(resp.text)"},{"line_number":104,"context_line":"        self.assertEqual(2, len(md_resource[\u0027namespaces\u0027]))"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        # Now disable get_metadef_namespaces permissions and make sure any"}],"source_content_type":"text/x-python","patch_set":12,"id":"e5db914f_4d9c6a90","line":103,"range":{"start_line":103,"start_character":22,"end_line":103,"end_character":48},"updated":"2021-08-02 14:51:55.000000000","message":"Ah, I missed these in the previous rev :)","commit_id":"05d3b6c4bf75515b05afe795e8edf5acd4bc5964"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"840471edd01f852158e6e135da6933a0fb088986","unresolved":true,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        # Now disable get_metadef_namespaces permissions and make sure any"},{"line_number":106,"context_line":"        # other attempts fail"},{"line_number":107,"context_line":"        self.set_policy_rules({\u0027get_metadef_namespaces\u0027: \u0027!\u0027})"},{"line_number":108,"context_line":"        resp \u003d self.api_get(path)"},{"line_number":109,"context_line":"        self.assertEqual(403, resp.status_code)"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"27047d92_2cf84356","line":107,"updated":"2021-08-04 19:12:43.000000000","message":"Ok, just so I understand, these tests aren\u0027t testing the secure RBAC personas, but really just testing that the policies are being called using a functional test and an API server with fake authorization middleware to remove the dependency on keystone?","commit_id":"9857fb65e944ab87d718f2948583574baade320d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"423b8cae0c5ad81816579213b15c15c8377d6479","unresolved":true,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        # Now disable get_metadef_namespaces permissions and make sure any"},{"line_number":106,"context_line":"        # other attempts fail"},{"line_number":107,"context_line":"        self.set_policy_rules({\u0027get_metadef_namespaces\u0027: \u0027!\u0027})"},{"line_number":108,"context_line":"        resp \u003d self.api_get(path)"},{"line_number":109,"context_line":"        self.assertEqual(403, resp.status_code)"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"22f54441_e566ac4a","line":107,"in_reply_to":"27047d92_2cf84356","updated":"2021-08-04 19:29:41.000000000","message":"This is because we haven\u0027t implemented RBAC for metadefs yet.","commit_id":"9857fb65e944ab87d718f2948583574baade320d"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"634517e52135843ce1c9c80daa888a0f653a2705","unresolved":true,"context_lines":[{"line_number":452,"context_line":"        self.assertEqual(\u0027MyNamespace\u0027, md_resource[\u0027namespace\u0027])"},{"line_number":453,"context_line":"        self.assertEqual(\u0027private\u0027, md_resource[\u0027visibility\u0027])"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"        # Now try to update the same namespace by different user"},{"line_number":456,"context_line":"        headers \u003d self._headers({"},{"line_number":457,"context_line":"            \u0027X-Tenant-Id\u0027: \u0027fake-tenant-id\u0027,"},{"line_number":458,"context_line":"            \u0027X-Roles\u0027: \u0027member\u0027,"},{"line_number":459,"context_line":"        })"},{"line_number":460,"context_line":"        path \u003d \u0027/v2/metadefs/namespaces/%s\u0027 % md_resource[\u0027namespace\u0027]"},{"line_number":461,"context_line":"        resp \u003d self.api_put(path, headers\u003dheaders, json\u003ddata)"},{"line_number":462,"context_line":"        self.assertEqual(404, resp.status_code)"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def test_namespace_delete_basic(self):"},{"line_number":465,"context_line":"        def _create_private_namespace(fn_call, data):"}],"source_content_type":"text/x-python","patch_set":21,"id":"1f05b2f0_2d719106","line":462,"range":{"start_line":455,"start_character":8,"end_line":462,"end_character":47},"updated":"2021-08-11 15:49:43.000000000","message":"I will push a follow-up patch to refactor this as it is used at various places.","commit_id":"7e919e8f9cd8cada42364207ea0aebd3a671a289"}]}