Merge "policy: cache extracted parent fields for OwnerCheck"

neutron/common/cache_utils.py

@@ -51,6 +51,19 @@ def _get_cache_region(conf):
     return region
 
 
+def _get_memory_cache_region(expiration_time=5):
+    conf = cfg.ConfigOpts()
+    register_oslo_configs(conf)
+    cache_conf_dict = {
+        'enabled': True,
+        'backend': 'oslo_cache.dict',
+        'expiration_time': expiration_time,
+    }
+    for k, v in cache_conf_dict.items():
+        conf.set_override(k, v, group='cache')
+    return _get_cache_region(conf)
+
+
 def _get_cache_region_for_legacy(url):
     parsed = parse.urlparse(url)
     backend = parsed.scheme
@@ -65,17 +78,8 @@ def _get_cache_region_for_legacy(url):
         if not query and '?' in parsed.path:
             query = parsed.path.split('?', 1)[-1]
         parameters = parse.parse_qs(query)
-
-        conf = cfg.ConfigOpts()
-        register_oslo_configs(conf)
-        cache_conf_dict = {
-            'enabled': True,
-            'backend': 'oslo_cache.dict',
-            'expiration_time': int(parameters.get('default_ttl', [0])[0]),
-        }
-        for k, v in cache_conf_dict.items():
-            conf.set_override(k, v, group='cache')
-        return _get_cache_region(conf)
+        return _get_memory_cache_region(
+            expiration_time=int(parameters.get('default_ttl', [0])[0]))
     else:
         raise RuntimeError(_('Old style configuration can use only memory '
                              '(dict) backend'))

neutron/policy.py

@@ -29,6 +29,7 @@ import six
 
 from neutron._i18n import _, _LE, _LW
 from neutron.api.v2 import attributes
+from neutron.common import cache_utils as cache
 from neutron.common import constants as const
 
 
@@ -209,8 +210,35 @@ class OwnerCheck(policy.Check):
             raise exceptions.PolicyInitError(
                 policy="%s:%s" % (kind, match),
                 reason=err_reason)
+        self._cache = cache._get_memory_cache_region(expiration_time=5)
         super(OwnerCheck, self).__init__(kind, match)
 
+    @cache.cache_method_results
+    def _extract(self, resource_type, resource_id, field):
+        # NOTE(salv-orlando): This check currently assumes the parent
+        # resource is handled by the core plugin. It might be worth
+        # having a way to map resources to plugins so to make this
+        # check more general
+        f = getattr(directory.get_plugin(), 'get_%s' % resource_type)
+        # f *must* exist, if not found it is better to let neutron
+        # explode. Check will be performed with admin context
+        context = importutils.import_module('neutron.context')
+        try:
+            data = f(context.get_admin_context(),
+                     resource_id,
+                     fields=[field])
+        except exceptions.NotFound as e:
+            # NOTE(kevinbenton): a NotFound exception can occur if a
+            # list operation is happening at the same time as one of
+            # the parents and its children being deleted. So we issue
+            # a RetryRequest so the API will redo the lookup and the
+            # problem items will be gone.
+            raise db_exc.RetryRequest(e)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                LOG.exception(_LE('Policy check error while calling %s!'), f)
+        return data[field]
+
     def __call__(self, target, creds, enforcer):
         if self.target_field not in target:
             # policy needs a plugin check
@@ -248,30 +276,10 @@ class OwnerCheck(policy.Check):
                 raise exceptions.PolicyCheckError(
                     policy="%s:%s" % (self.kind, self.match),
                     reason=err_reason)
-            # NOTE(salv-orlando): This check currently assumes the parent
-            # resource is handled by the core plugin. It might be worth
-            # having a way to map resources to plugins so to make this
-            # check more general
-            f = getattr(directory.get_plugin(), 'get_%s' % parent_res)
-            # f *must* exist, if not found it is better to let neutron
-            # explode. Check will be performed with admin context
-            context = importutils.import_module('neutron.context')
-            try:
-                data = f(context.get_admin_context(),
-                         target[parent_foreign_key],
-                         fields=[parent_field])
-                target[self.target_field] = data[parent_field]
-            except exceptions.NotFound as e:
-                # NOTE(kevinbenton): a NotFound exception can occur if a
-                # list operation is happening at the same time as one of
-                # the parents and its children being deleted. So we issue
-                # a RetryRequest so the API will redo the lookup and the
-                # problem items will be gone.
-                raise db_exc.RetryRequest(e)
-            except Exception:
-                with excutils.save_and_reraise_exception():
-                    LOG.exception(_LE('Policy check error while calling %s!'),
-                                  f)
+
+            target[self.target_field] = self._extract(
+                parent_res, target[parent_foreign_key], parent_field)
+
         match = self.match % target
         if self.kind in creds:
             return match == six.text_type(creds[self.kind])

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -685,6 +685,28 @@ class TestMl2DbOperationBoundsTenant(TestMl2DbOperationBounds):
     admin = False
 
 
+class TestMl2DbOperationBoundsTenantRbac(TestMl2DbOperationBoundsTenant):
+
+    def make_port_in_shared_network(self):
+        context_ = self._get_context()
+        # create shared network owned by the tenant; we use direct driver call
+        # because default policy does not allow users to create shared networks
+        net = self.driver.create_network(
+            context.get_admin_context(),
+            {'network': {'name': 'net1',
+                         'tenant_id': context_.tenant,
+                         'admin_state_up': True,
+                         'shared': True}})
+        # create port that belongs to another tenant
+        return self._make_port(
+            self.fmt, net['id'],
+            set_context=True, tenant_id='fake_tenant')
+
+    def test_port_list_in_shared_network_queries_constant(self):
+        self._assert_object_list_queries_constant(
+            self.make_port_in_shared_network, 'ports')
+
+
 class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
 
     def test__port_provisioned_with_blocks(self):

neutron/tests/unit/test_policy.py

@@ -570,6 +570,17 @@ class NeutronPolicyTestCase(base.BaseTestCase):
             oslo_policy.Rules.from_dict,
             {'test_policy': 'tenant_id:(wrong_stuff)'})
 
+    def test_tenant_id_check_caches_extracted_fields(self):
+
+        plugin = directory.get_plugin()
+        with mock.patch.object(plugin, 'get_network',
+                               return_value={'tenant_id': 'fake'}) as getter:
+            action = "create_port:mac"
+            for i in range(2):
+                target = {'network_id': 'whatever'}
+                policy.enforce(self.context, action, target)
+        self.assertEqual(1, getter.call_count)
+
     def _test_enforce_tenant_id_raises(self, bad_rule):
         self._set_rules(admin_or_owner=bad_rule)
         # Trigger a policy with rule admin_or_owner