policy: cache extracted parent fields for OwnerCheck

Parent foreign key extraction requires another database fetch per object
validates, which does not scale well. To mitigate the issue, cache
extracted parent key values in a global policy cache dictionary.

Use oslo.cache to maintain cache. Hardcode expiration timeout for
policy lookups to 5 secs.

Change-Id: I6b3d7c96b7487c9bef6d39a28c76fea0721c3098
Related-Bug: #1513782

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

@@ -670,6 +670,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