)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2031,"name":"Nachi Ueno","email":"nati.ueno@gmail.com","username":"nati-ueno"},"unresolved":false,"context_lines":[{"line_number":11,"context_line":"This patch implements part #3 of this blueprint, according to its"},{"line_number":12,"context_line":"specification."},{"line_number":13,"context_line":"It does so by allowing the view generator in the API layer to strip"},{"line_number":14,"context_line":"off fields which do not satify authorization policies."},{"line_number":15,"context_line":"Also, some checks in unit tests for plugins relied on the"},{"line_number":16,"context_line":"capability of the plugin to invoke directly the policy engine."},{"line_number":17,"context_line":"This checks have been removed and replaced by equivalent unit tests."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAAOn%2F%2FZ78%3D","line":14,"updated":"2013-05-10 17:21:41.000000000","message":"nit: satisfy?","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":11,"context_line":"This patch implements part #3 of this blueprint, according to its"},{"line_number":12,"context_line":"specification."},{"line_number":13,"context_line":"It does so by allowing the view generator in the API layer to strip"},{"line_number":14,"context_line":"off fields which do not satify authorization policies."},{"line_number":15,"context_line":"Also, some checks in unit tests for plugins relied on the"},{"line_number":16,"context_line":"capability of the plugin to invoke directly the policy engine."},{"line_number":17,"context_line":"This checks have been removed and replaced by equivalent unit tests."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAAOn%2F%2FY4Y%3D","line":14,"in_reply_to":"AAAAOn%2F%2FZ78%3D","updated":"2013-05-11 16:44:25.000000000","message":"thanks","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"}],"etc/policy.json":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"30224d8da7b47260868ad5c159583a49cad8decb","unresolved":false,"context_lines":[{"line_number":14,"context_line":"    \"extension:router:view\": \"rule:regular_user\","},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    \"extension:port_binding:view\": \"rule:admin_only\","},{"line_number":17,"context_line":"    \"extension:port_binding:set\": \"rule:admin_only\","},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    \"subnets:private:read\": \"rule:admin_or_owner\","},{"line_number":20,"context_line":"    \"subnets:private:write\": \"rule:admin_or_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2FfRs%3D","side":"PARENT","line":17,"updated":"2013-05-08 03:23:42.000000000","message":"Just a question:  Should we keep bw compatibilty at here?","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":14,"context_line":"    \"extension:router:view\": \"rule:regular_user\","},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    \"extension:port_binding:view\": \"rule:admin_only\","},{"line_number":17,"context_line":"    \"extension:port_binding:set\": \"rule:admin_only\","},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    \"subnets:private:read\": \"rule:admin_or_owner\","},{"line_number":20,"context_line":"    \"subnets:private:write\": \"rule:admin_or_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2FY4g%3D","side":"PARENT","line":17,"in_reply_to":"AAAAOn%2F%2Fa9U%3D","updated":"2013-05-11 16:44:25.000000000","message":"I would love following Bob\u0027s advice, as it would be simple for me.\nHowever, we should follow a practice of deprecating a configuration item before removing it.\nI am not aware of any openstack projects that does this for policy, but most do this for conf files. (and we\u0027re the only one with rule which did not follow the policy language).\n\nI will do this in a separate patch for keeping reviewers\u0027 job simple.","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"30224d8da7b47260868ad5c159583a49cad8decb","unresolved":false,"context_lines":[{"line_number":14,"context_line":"    \"extension:router:view\": \"rule:regular_user\","},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    \"extension:port_binding:view\": \"rule:admin_only\","},{"line_number":17,"context_line":"    \"extension:port_binding:set\": \"rule:admin_only\","},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    \"subnets:private:read\": \"rule:admin_or_owner\","},{"line_number":20,"context_line":"    \"subnets:private:write\": \"rule:admin_or_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2Fa9U%3D","side":"PARENT","line":17,"in_reply_to":"AAAAOn%2F%2FcYw%3D","updated":"2013-05-10 02:53:57.000000000","message":"+1, good for any configuration file changing","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"unresolved":false,"context_lines":[{"line_number":14,"context_line":"    \"extension:router:view\": \"rule:regular_user\","},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    \"extension:port_binding:view\": \"rule:admin_only\","},{"line_number":17,"context_line":"    \"extension:port_binding:set\": \"rule:admin_only\","},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    \"subnets:private:read\": \"rule:admin_or_owner\","},{"line_number":20,"context_line":"    \"subnets:private:write\": \"rule:admin_or_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2FcYw%3D","side":"PARENT","line":17,"in_reply_to":"AAAAOn%2F%2FeCA%3D","updated":"2013-05-09 14:39:14.000000000","message":"My recommendation is to not go to extremes to keep the old mechanism working along with the new, since the overall goal here is to simplify the code. If this were being backported to a stable branch, that would be needed. Here, maybe just logging a warning (or logging an error and exiting) if the old policies are still present would be sufficient.","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":14,"context_line":"    \"extension:router:view\": \"rule:regular_user\","},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"    \"extension:port_binding:view\": \"rule:admin_only\","},{"line_number":17,"context_line":"    \"extension:port_binding:set\": \"rule:admin_only\","},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    \"subnets:private:read\": \"rule:admin_or_owner\","},{"line_number":20,"context_line":"    \"subnets:private:write\": \"rule:admin_or_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2FeCA%3D","side":"PARENT","line":17,"in_reply_to":"AAAAOn%2F%2FfRs%3D","updated":"2013-05-08 18:39:51.000000000","message":"This is an interesting point.\nThese checks were pretty much hardcoded so far.\nHowever, users might have customized them.\n\nAchieving backward compatibility won\u0027t be easy, but I\u0027ll have a crack at it.","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"cb094095b26fc1b38df5696199492bc02d1cba64","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    \"get_port\": \"rule:admin_or_owner\","},{"line_number":44,"context_line":"    \"get_port:queue_id\": \"rule:admin_only\","},{"line_number":45,"context_line":"    \"get_port:binding:vif_type\": \"rule:admin_only\","},{"line_number":46,"context_line":"    \"get_port:binding:capabilities\": \"rule:admin_only\","},{"line_number":47,"context_line":"    \"update_port\": \"rule:admin_or_owner\","},{"line_number":48,"context_line":"    \"update_port:fixed_ips\": \"rule:admin_or_network_owner\","},{"line_number":49,"context_line":"    \"update_port:port_security_enabled\": \"rule:admin_or_network_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2FfZs%3D","line":46,"updated":"2013-05-08 01:20:53.000000000","message":"There should also be policies for get_port:binding:host_id and get_port:binding:profile similar to the others since these attributes are defined in portbindings.py. The existing plugins don\u0027t implement them (yet), but ml2 will at least need host_id.\n\nAlso, I think there should be create_port and update_port policies for binding:host_id and binding:profile since these both have allow_post and allow_put set to True.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":43,"context_line":"    \"get_port\": \"rule:admin_or_owner\","},{"line_number":44,"context_line":"    \"get_port:queue_id\": \"rule:admin_only\","},{"line_number":45,"context_line":"    \"get_port:binding:vif_type\": \"rule:admin_only\","},{"line_number":46,"context_line":"    \"get_port:binding:capabilities\": \"rule:admin_only\","},{"line_number":47,"context_line":"    \"update_port\": \"rule:admin_or_owner\","},{"line_number":48,"context_line":"    \"update_port:fixed_ips\": \"rule:admin_or_network_owner\","},{"line_number":49,"context_line":"    \"update_port:port_security_enabled\": \"rule:admin_or_network_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2FY4c%3D","line":46,"in_reply_to":"AAAAOn%2F%2Fcao%3D","updated":"2013-05-11 16:44:25.000000000","message":"Yes, adding those line here won\u0027t make any harm.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"unresolved":false,"context_lines":[{"line_number":43,"context_line":"    \"get_port\": \"rule:admin_or_owner\","},{"line_number":44,"context_line":"    \"get_port:queue_id\": \"rule:admin_only\","},{"line_number":45,"context_line":"    \"get_port:binding:vif_type\": \"rule:admin_only\","},{"line_number":46,"context_line":"    \"get_port:binding:capabilities\": \"rule:admin_only\","},{"line_number":47,"context_line":"    \"update_port\": \"rule:admin_or_owner\","},{"line_number":48,"context_line":"    \"update_port:fixed_ips\": \"rule:admin_or_network_owner\","},{"line_number":49,"context_line":"    \"update_port:port_security_enabled\": \"rule:admin_or_network_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2Fcao%3D","line":46,"in_reply_to":"AAAAOn%2F%2FeB4%3D","updated":"2013-05-09 14:39:14.000000000","message":"Looks like the portbinding attributes are being addressed in https://review.openstack.org/#/c/24969/8/etc/policy.json, so coordination of merges might be all that\u0027s needed.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":43,"context_line":"    \"get_port\": \"rule:admin_or_owner\","},{"line_number":44,"context_line":"    \"get_port:queue_id\": \"rule:admin_only\","},{"line_number":45,"context_line":"    \"get_port:binding:vif_type\": \"rule:admin_only\","},{"line_number":46,"context_line":"    \"get_port:binding:capabilities\": \"rule:admin_only\","},{"line_number":47,"context_line":"    \"update_port\": \"rule:admin_or_owner\","},{"line_number":48,"context_line":"    \"update_port:fixed_ips\": \"rule:admin_or_network_owner\","},{"line_number":49,"context_line":"    \"update_port:port_security_enabled\": \"rule:admin_or_network_owner\","}],"source_content_type":"application/json","patch_set":4,"id":"AAAAOn%2F%2FeB4%3D","line":46,"in_reply_to":"AAAAOn%2F%2FfZs%3D","updated":"2013-05-08 18:39:51.000000000","message":"Thanks Bob. I\u0027ve been ignoring attributes not yet used by plugin, but it makes sense adding them.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"}],"quantum/api/v2/base.py":[{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"cb094095b26fc1b38df5696199492bc02d1cba64","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        return getattr(self._plugin, native_sorting_attr_name, False)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _is_visible(self, context, attr, data):"},{"line_number":120,"context_line":"        action \u003d \"%s:%s\" % (self._plugin_handlers[self.SHOW], attr)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            authz_check \u003d policy.check_if_exists("},{"line_number":123,"context_line":"                context, action, data)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FfaM%3D","line":120,"updated":"2013-05-08 01:20:53.000000000","message":"Could this logic be made conditional on enforce_policy being True for the attribute?","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":117,"context_line":"        return getattr(self._plugin, native_sorting_attr_name, False)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _is_visible(self, context, attr, data):"},{"line_number":120,"context_line":"        action \u003d \"%s:%s\" % (self._plugin_handlers[self.SHOW], attr)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            authz_check \u003d policy.check_if_exists("},{"line_number":123,"context_line":"                context, action, data)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FY38%3D","line":120,"in_reply_to":"AAAAOn%2F%2FcaM%3D","updated":"2013-05-11 16:44:25.000000000","message":"I know that the policy engine so far has a HUGE performance impact, because we have rules which are hitting the plugin, and we shouldn\u0027t.\n\nThat would be part of a separate patch.\nThis would be consistent with enforce_policy employed on POST/PUT. However, on the other hand this will trigger more policy evaluations on POST/PUT requests.\n\nMeasurements will make sense once we get rid of the plugin access.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"unresolved":false,"context_lines":[{"line_number":117,"context_line":"        return getattr(self._plugin, native_sorting_attr_name, False)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _is_visible(self, context, attr, data):"},{"line_number":120,"context_line":"        action \u003d \"%s:%s\" % (self._plugin_handlers[self.SHOW], attr)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            authz_check \u003d policy.check_if_exists("},{"line_number":123,"context_line":"                context, action, data)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FcaM%3D","line":120,"in_reply_to":"AAAAOn%2F%2FeA0%3D","updated":"2013-05-09 14:39:14.000000000","message":"OK, I had not considered the use cases of hiding normal attributes. My concern was about performance impact, and I don\u0027t really know whether or not calling the policy code for each attribute is significant overhead. Maybe checking whether this patch effects the unit test times would be a quick \u0026 easy way to see if this should be a concern.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":117,"context_line":"        return getattr(self._plugin, native_sorting_attr_name, False)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _is_visible(self, context, attr, data):"},{"line_number":120,"context_line":"        action \u003d \"%s:%s\" % (self._plugin_handlers[self.SHOW], attr)"},{"line_number":121,"context_line":"        try:"},{"line_number":122,"context_line":"            authz_check \u003d policy.check_if_exists("},{"line_number":123,"context_line":"                context, action, data)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FeA0%3D","line":120,"in_reply_to":"AAAAOn%2F%2FfaM%3D","updated":"2013-05-08 18:39:51.000000000","message":"It could, but this would cause that if a user adds a rule for hiding an attribute than the policy itself won\u0027t be enforce because the field is not \"policy-able\".\n\nLet me think about it.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":123,"context_line":"        try:"},{"line_number":124,"context_line":"            attr \u003d (attributes.RESOURCE_ATTRIBUTE_MAP"},{"line_number":125,"context_line":"                    [self._collection].get(attr_name))"},{"line_number":126,"context_line":"            if attr and attr.get(\u0027enforce_policy\u0027):"},{"line_number":127,"context_line":"                authz_check \u003d policy.check_if_exists("},{"line_number":128,"context_line":"                    context, action, data)"},{"line_number":129,"context_line":"        except KeyError:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2FX9w%3D","line":126,"updated":"2013-05-13 07:55:17.000000000","message":"This behavior is good to me, though I don\u0027t have a strong opinion whether policy check should be done only when enforce_policy is defined. (I read the discussion in patch set 4.)\n\nIt is easy to understand policy for attribute is applied when enforce_policy is declared in the attribute map. I think hiding additonal fields is a special case and it would be better to add enforce_policy to attrmap if the demand is generic.","commit_id":"7f0b447bdd2e57974ffc7198894c5391c656ea6a"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":123,"context_line":"        try:"},{"line_number":124,"context_line":"            attr \u003d (attributes.RESOURCE_ATTRIBUTE_MAP"},{"line_number":125,"context_line":"                    [self._collection].get(attr_name))"},{"line_number":126,"context_line":"            if attr and attr.get(\u0027enforce_policy\u0027):"},{"line_number":127,"context_line":"                authz_check \u003d policy.check_if_exists("},{"line_number":128,"context_line":"                    context, action, data)"},{"line_number":129,"context_line":"        except KeyError:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2FX64%3D","line":126,"in_reply_to":"AAAAOn%2F%2FX9w%3D","updated":"2013-05-13 08:33:16.000000000","message":"Ok - I don\u0027t have a strong opinion either.\nIt was pointed out, and I noticed we do the same thing for PUT/POST requests.\nso I thought it was more consistent.","commit_id":"7f0b447bdd2e57974ffc7198894c5391c656ea6a"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":131,"context_line":"            # to the global resource attribute map. Policy check should"},{"line_number":132,"context_line":"            # not be performed"},{"line_number":133,"context_line":"            LOG.debug(_(\"The resource %(resource)s was not found in the \""},{"line_number":134,"context_line":"                      \"RESOURCE_ATTRIBUTE_MAP; unable to perform auhZ check\""},{"line_number":135,"context_line":"                      \"for attribute %(attr)s\","},{"line_number":136,"context_line":"                      {\u0027resource\u0027: self._collection,"},{"line_number":137,"context_line":"                       \u0027attr\u0027: attr}))"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2FYAE%3D","line":134,"updated":"2013-05-13 07:55:17.000000000","message":"auhZ -\u003e authZ\n\nNeeds a space at the end of line.","commit_id":"7f0b447bdd2e57974ffc7198894c5391c656ea6a"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":132,"context_line":"            # not be performed"},{"line_number":133,"context_line":"            LOG.debug(_(\"The resource %(resource)s was not found in the \""},{"line_number":134,"context_line":"                      \"RESOURCE_ATTRIBUTE_MAP; unable to perform auhZ check\""},{"line_number":135,"context_line":"                      \"for attribute %(attr)s\","},{"line_number":136,"context_line":"                      {\u0027resource\u0027: self._collection,"},{"line_number":137,"context_line":"                       \u0027attr\u0027: attr}))"},{"line_number":138,"context_line":"        except exceptions.PolicyRuleNotFound:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2FYLY%3D","line":135,"updated":"2013-05-13 07:55:17.000000000","message":"Need a closing parenthesis\n\nNeed to adjust the indentation level.","commit_id":"7f0b447bdd2e57974ffc7198894c5391c656ea6a"}],"quantum/plugins/linuxbridge/lb_quantum_plugin.py":[{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"cb094095b26fc1b38df5696199492bc02d1cba64","unresolved":false,"context_lines":[{"line_number":271,"context_line":"            self.network_vlan_ranges[physical_network] \u003d []"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    # REVISIT(rkukura) Use core mechanism for attribute authorization"},{"line_number":274,"context_line":"    # when available."},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def _extend_network_dict_provider(self, context, network):"},{"line_number":277,"context_line":"        binding \u003d db.get_network_binding(context.session, network[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2Ffa8%3D","line":274,"updated":"2013-05-08 01:20:53.000000000","message":"Please remove the above REVISIT comment, since I think this patch completes what it asks for.","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":271,"context_line":"            self.network_vlan_ranges[physical_network] \u003d []"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    # REVISIT(rkukura) Use core mechanism for attribute authorization"},{"line_number":274,"context_line":"    # when available."},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def _extend_network_dict_provider(self, context, network):"},{"line_number":277,"context_line":"        binding \u003d db.get_network_binding(context.session, network[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FeAw%3D","line":274,"in_reply_to":"AAAAOn%2F%2Ffa8%3D","updated":"2013-05-08 18:39:51.000000000","message":"Ah thanks!","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"}],"quantum/plugins/nicira/QuantumPlugin.py":[{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":2085,"context_line":"        nvplib.delete_lqueue(self.cluster, id)"},{"line_number":2086,"context_line":"        return super(NvpPluginV2, self).delete_qos_queue(context, id)"},{"line_number":2087,"context_line":""},{"line_number":2088,"context_line":"    def get_qos_queue(self, context, id, fields\u003dNone):"},{"line_number":2089,"context_line":"        return super(NvpPluginV2, self).get_qos_queue(context, id, fields)"},{"line_number":2090,"context_line":""},{"line_number":2091,"context_line":"    def get_qos_queues(self, context, filters\u003dNone, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXzM%3D","line":2088,"updated":"2013-05-13 10:38:08.000000000","message":"how about removing this function totally?","commit_id":"c4eaec86fb56400ec10a4de6e20e0421fd141be0"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":2085,"context_line":"        nvplib.delete_lqueue(self.cluster, id)"},{"line_number":2086,"context_line":"        return super(NvpPluginV2, self).delete_qos_queue(context, id)"},{"line_number":2087,"context_line":""},{"line_number":2088,"context_line":"    def get_qos_queue(self, context, id, fields\u003dNone):"},{"line_number":2089,"context_line":"        return super(NvpPluginV2, self).get_qos_queue(context, id, fields)"},{"line_number":2090,"context_line":""},{"line_number":2091,"context_line":"    def get_qos_queues(self, context, filters\u003dNone, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXwY%3D","line":2088,"in_reply_to":"AAAAOn%2F%2FXzM%3D","updated":"2013-05-13 11:12:19.000000000","message":"can do","commit_id":"c4eaec86fb56400ec10a4de6e20e0421fd141be0"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":2088,"context_line":"    def get_qos_queue(self, context, id, fields\u003dNone):"},{"line_number":2089,"context_line":"        return super(NvpPluginV2, self).get_qos_queue(context, id, fields)"},{"line_number":2090,"context_line":""},{"line_number":2091,"context_line":"    def get_qos_queues(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":2092,"context_line":"        return super(NvpPluginV2, self).get_qos_queues(context, filters,"},{"line_number":2093,"context_line":"                                                       fields)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXzI%3D","line":2091,"updated":"2013-05-13 10:38:08.000000000","message":"how about removing this function totally?","commit_id":"c4eaec86fb56400ec10a4de6e20e0421fd141be0"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":2088,"context_line":"    def get_qos_queue(self, context, id, fields\u003dNone):"},{"line_number":2089,"context_line":"        return super(NvpPluginV2, self).get_qos_queue(context, id, fields)"},{"line_number":2090,"context_line":""},{"line_number":2091,"context_line":"    def get_qos_queues(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":2092,"context_line":"        return super(NvpPluginV2, self).get_qos_queues(context, filters,"},{"line_number":2093,"context_line":"                                                       fields)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXwU%3D","line":2091,"in_reply_to":"AAAAOn%2F%2FXzI%3D","updated":"2013-05-13 11:12:19.000000000","message":"can do","commit_id":"c4eaec86fb56400ec10a4de6e20e0421fd141be0"}],"quantum/policy.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def check_if_exists(context, action, target):"},{"line_number":196,"context_line":"    \"\"\"Verify is action can be authorized, and raise if action is unknown."},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    Check whether the action can be performed on the target within this"},{"line_number":199,"context_line":"    context, and raise a PolicyRuleNotFound exception if the action is"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FcaQ%3D","line":196,"updated":"2013-05-09 14:31:05.000000000","message":"is -\u003e if","commit_id":"dea55b3017bb2e141d825cb1ee835a51ba460703"}],"quantum/tests/unit/_test_extension_portbindings.py":[{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"cb094095b26fc1b38df5696199492bc02d1cba64","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"},{"line_number":64,"context_line":"                                  read_deleted\u003d\"no\")"},{"line_number":65,"context_line":"            non_admin_port \u003d plugin.get_port(ctx, port_id)"},{"line_number":66,"context_line":"            self._check_response_no_portbindings(non_admin_port)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def test_ports_vif_details(self):"},{"line_number":69,"context_line":"        plugin \u003d QuantumManager.get_plugin()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FfZk%3D","side":"PARENT","line":66,"updated":"2013-05-08 01:20:53.000000000","message":"Is there some other test that verifies that a non-admin user does not get the portbinding attributes?","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"unresolved":false,"context_lines":[{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"},{"line_number":64,"context_line":"                                  read_deleted\u003d\"no\")"},{"line_number":65,"context_line":"            non_admin_port \u003d plugin.get_port(ctx, port_id)"},{"line_number":66,"context_line":"            self._check_response_no_portbindings(non_admin_port)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def test_ports_vif_details(self):"},{"line_number":69,"context_line":"        plugin \u003d QuantumManager.get_plugin()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FcZU%3D","side":"PARENT","line":66,"in_reply_to":"AAAAOn%2F%2FeAg%3D","updated":"2013-05-09 14:39:14.000000000","message":"OK, I see that now. I agree testing the mechanism once is preferable to testing every attribute to which the mechanism is applied.","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"},{"line_number":64,"context_line":"                                  read_deleted\u003d\"no\")"},{"line_number":65,"context_line":"            non_admin_port \u003d plugin.get_port(ctx, port_id)"},{"line_number":66,"context_line":"            self._check_response_no_portbindings(non_admin_port)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def test_ports_vif_details(self):"},{"line_number":69,"context_line":"        plugin \u003d QuantumManager.get_plugin()"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOn%2F%2FeAg%3D","side":"PARENT","line":66,"in_reply_to":"AAAAOn%2F%2FfZk%3D","updated":"2013-05-08 18:39:51.000000000","message":"Yes, I added those tests to test_api_v2","commit_id":"f94126739a48993efaf1d1439dcd3dadb0c69742"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":57,"context_line":"            ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"            port \u003d plugin.get_port(ctx, port_id)"},{"line_number":59,"context_line":"            self._check_response_portbindings(port)"},{"line_number":60,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":61,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":62,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXy8%3D","side":"PARENT","line":60,"updated":"2013-05-13 10:38:08.000000000","message":"Why just remove these lines? we actually need to do this kind of testing.\nif u would like, mark a \u0027todo(gongysh) in portbinding-ex-db bp\u0027","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":57,"context_line":"            ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"            port \u003d plugin.get_port(ctx, port_id)"},{"line_number":59,"context_line":"            self._check_response_portbindings(port)"},{"line_number":60,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":61,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":62,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FVqk%3D","side":"PARENT","line":60,"in_reply_to":"AAAAOn%2F%2FWVw%3D","updated":"2013-05-14 07:54:12.000000000","message":"I was doing the same thing; I\u0027ve seen you don\u0027t have a test for \u0027show\u0027 as nonadmin, which was present here.\n\nI don\u0027t think it matters a lot, anyway.","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":57,"context_line":"            ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"            port \u003d plugin.get_port(ctx, port_id)"},{"line_number":59,"context_line":"            self._check_response_portbindings(port)"},{"line_number":60,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":61,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":62,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FWVw%3D","side":"PARENT","line":60,"in_reply_to":"AAAAOn%2F%2FWYw%3D","updated":"2013-05-13 23:57:02.000000000","message":"I have done it here:\nhttps://review.openstack.org/#/c/24969/9/quantum/tests/unit/_test_extension_portbindings.py\n\nsince it was merged, we can discard the change of this file in this patch.","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":57,"context_line":"            ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"            port \u003d plugin.get_port(ctx, port_id)"},{"line_number":59,"context_line":"            self._check_response_portbindings(port)"},{"line_number":60,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":61,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":62,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FWYw%3D","side":"PARENT","line":60,"in_reply_to":"AAAAOn%2F%2FXuw%3D","updated":"2013-05-13 23:09:49.000000000","message":"I am ok with doing more punctual tests, although they would be merely verifying the policies in the builtin policy.json are correctly enforced.\n\nThose tests however cannot stay here; as the check is not  anymore performed in the plugin, the call at line 65 will never fail!\n\nI have thought about tests for extension port_binding, but there\u0027s no extension specific test module for them.\n\nThey\u0027re spred across all plugins.\n\nI will see what I can do.","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":57,"context_line":"            ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"            port \u003d plugin.get_port(ctx, port_id)"},{"line_number":59,"context_line":"            self._check_response_portbindings(port)"},{"line_number":60,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":61,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":62,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXuw%3D","side":"PARENT","line":60,"in_reply_to":"AAAAOn%2F%2FXy8%3D","updated":"2013-05-13 11:49:54.000000000","message":"The new test in test_api_v2 is a generic test to check admin_only attributes are only visible to admin.\n\nTests which verify policy rules for a specific attribute is properly defined also seems to be useful. From this point of view, it is better to keep these tests. It is not limited to port-binding attributes.\n\nWhat we need to consider is that these kind of tests should be run once per extension instead of per plugin. Though I have not had a concrete idea yet, a separate test class would be better. (I don\u0027t think it needs to be covered in this patch)","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":57,"context_line":"            ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"            port \u003d plugin.get_port(ctx, port_id)"},{"line_number":59,"context_line":"            self._check_response_portbindings(port)"},{"line_number":60,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":61,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":62,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":63,"context_line":"                                  is_admin\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXwE%3D","side":"PARENT","line":60,"in_reply_to":"AAAAOn%2F%2FXy8%3D","updated":"2013-05-13 11:12:19.000000000","message":"These checks have been replaced by equivalent tests in https://review.openstack.org/#/c/28249/9/quantum/tests/unit/test_api_v2.py\n\nSee discussion about this change in patch set 4.\n\nPlease not that the removed lines only verify the policy engine strips off \u0027admin_only\u0027 attributes.\nI believe that the tests in test_api_v2 provide the same level of coverage.\n\nHowever if you feel differently I will add the #TODO for you.\nI have removed those lines because they\u0027re relying on the fact that the authZ check if performed in the plugin.\nAs it\u0027s now moved outside of the plugin, these checks fail.","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self.assertEqual(len(ports), 2)"},{"line_number":75,"context_line":"            for port in ports:"},{"line_number":76,"context_line":"                self._check_response_portbindings(port)"},{"line_number":77,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":78,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":79,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":80,"context_line":"                                  is_admin\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXy4%3D","side":"PARENT","line":77,"updated":"2013-05-13 10:38:08.000000000","message":"ditto","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"}],"quantum/tests/unit/hyperv/test_hyperv_quantum_plugin.py":[{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":60,"context_line":"                                  read_deleted\u003d\"no\")"},{"line_number":61,"context_line":"            non_admin_port \u003d plugin.get_port(ctx, port_id)"},{"line_number":62,"context_line":"            self.assertTrue(\u0027status\u0027 in non_admin_port)"},{"line_number":63,"context_line":"            self.assertFalse(\u0027binding:vif_type\u0027 in non_admin_port)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def test_ports_vif_details(self):"},{"line_number":66,"context_line":"        cfg.CONF.set_default(\u0027allow_overlapping_ips\u0027, True)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXyk%3D","side":"PARENT","line":63,"updated":"2013-05-13 10:38:08.000000000","message":"ditto","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":60,"context_line":"                                  read_deleted\u003d\"no\")"},{"line_number":61,"context_line":"            non_admin_port \u003d plugin.get_port(ctx, port_id)"},{"line_number":62,"context_line":"            self.assertTrue(\u0027status\u0027 in non_admin_port)"},{"line_number":63,"context_line":"            self.assertFalse(\u0027binding:vif_type\u0027 in non_admin_port)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def test_ports_vif_details(self):"},{"line_number":66,"context_line":"        cfg.CONF.set_default(\u0027allow_overlapping_ips\u0027, True)"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXv4%3D","side":"PARENT","line":63,"in_reply_to":"AAAAOn%2F%2FXyk%3D","updated":"2013-05-13 11:12:19.000000000","message":"please see reply in other line","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":73,"context_line":"                self.assertEqual(port[\u0027binding:vif_type\u0027],"},{"line_number":74,"context_line":"                                 portbindings.VIF_TYPE_HYPERV)"},{"line_number":75,"context_line":"            # By default user is admin - now test non admin user"},{"line_number":76,"context_line":"            ctx \u003d context.Context(user_id\u003dNone,"},{"line_number":77,"context_line":"                                  tenant_id\u003dself._tenant_id,"},{"line_number":78,"context_line":"                                  is_admin\u003dFalse,"},{"line_number":79,"context_line":"                                  read_deleted\u003d\"no\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAOn%2F%2FXyg%3D","side":"PARENT","line":76,"updated":"2013-05-13 10:38:08.000000000","message":"ditto","commit_id":"84fc2cf4560f7a34da9d1e40ad865a81fde47d1c"}],"quantum/tests/unit/nicira/test_nicira_plugin.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":757,"context_line":"        quantum_context \u003d context.Context(\u0027\u0027, \u0027not_admin\u0027)"},{"line_number":758,"context_line":"        port \u003d self._update(\u0027ports\u0027, port[\u0027port\u0027][\u0027id\u0027], data,"},{"line_number":759,"context_line":"                            quantum_context\u003dquantum_context)"},{"line_number":760,"context_line":"        self.assertEqual(ext_qos.QUEUE in port[\u0027port\u0027], False)"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"    def test_rxtx_factor(self):"},{"line_number":763,"context_line":"        with self.qos_queue(max\u003d10) as q1:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2FYB0%3D","line":760,"updated":"2013-05-13 07:55:17.000000000","message":"assertFalse would be better.","commit_id":"7f0b447bdd2e57974ffc7198894c5391c656ea6a"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":757,"context_line":"        quantum_context \u003d context.Context(\u0027\u0027, \u0027not_admin\u0027)"},{"line_number":758,"context_line":"        port \u003d self._update(\u0027ports\u0027, port[\u0027port\u0027][\u0027id\u0027], data,"},{"line_number":759,"context_line":"                            quantum_context\u003dquantum_context)"},{"line_number":760,"context_line":"        self.assertEqual(ext_qos.QUEUE in port[\u0027port\u0027], False)"},{"line_number":761,"context_line":""},{"line_number":762,"context_line":"    def test_rxtx_factor(self):"},{"line_number":763,"context_line":"        with self.qos_queue(max\u003d10) as q1:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAOn%2F%2FX68%3D","line":760,"in_reply_to":"AAAAOn%2F%2FYB0%3D","updated":"2013-05-13 08:33:16.000000000","message":"Done","commit_id":"7f0b447bdd2e57974ffc7198894c5391c656ea6a"}]}