]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Avoid duplicating tenant check when creating resources
authorMathieu Rohon <mathieu.rohon@gmail.com>
Wed, 4 Nov 2015 17:49:40 +0000 (17:49 +0000)
committerMathieu Rohon <mathieu.rohon@gmail.com>
Tue, 5 Jan 2016 14:18:41 +0000 (14:18 +0000)
The check of the tenant done in the method _get_tenant_id_for_create()
is already did by the Neutron Controller in prepare_request_body(),
with a call to attributes.populate_tenant_id().
Moreover, when the Controller processes a "create" requests, it
will add the 'tenant_id' to the resource dict.
Thus, _get_tenant_id_for_create() can be deleted.
Calls to this method are replaced by the res['tenant_id'].

Changes have to be done in UT to explicitly add the tenant_id while
creating resources, since the UT framework is bypassing the controller code
that automatically adds the tenant_id to the resource.

Co-Authored-By: Hong Hui Xiao <xiaohhui@cn.ibm.com>
Closes-Bug: #1513825
Change-Id: Icea06dc81344e1120bdf986a97a6b1094bbb765e
Depends-On: I31022e9230fc5404c6a94edabbb08d2b079c3a09
Depends-On: Iea3f014ef17a1e1b755cd2efe99afd1a36ebbc6a
Depends-On: I604602d023e0cbf7f6591149f914d73217d7a574

19 files changed:
neutron/db/address_scope_db.py
neutron/db/common_db_mixin.py
neutron/db/db_base_plugin_v2.py
neutron/db/l3_db.py
neutron/db/metering/metering_db.py
neutron/db/rbac_db_mixin.py
neutron/db/securitygroups_db.py
neutron/plugins/ml2/plugin.py
neutron/tests/functional/scheduler/test_l3_agent_scheduler.py
neutron/tests/retargetable/client_fixtures.py
neutron/tests/unit/api/rpc/handlers/test_l3_rpc.py
neutron/tests/unit/api/v2/test_attributes.py
neutron/tests/unit/db/test_agentschedulers_db.py
neutron/tests/unit/db/test_l3_hamode_db.py
neutron/tests/unit/extensions/test_l3.py
neutron/tests/unit/extensions/test_portsecurity.py
neutron/tests/unit/extensions/test_securitygroup.py
neutron/tests/unit/plugins/ml2/test_plugin.py
neutron/tests/unit/scheduler/test_l3_agent_scheduler.py

index 20f1c963625d88ab6de1873dc9a3d1a2b7f0b241..d849dcbda8b0eebca2d1596ada198b8a5ad03b50 100644 (file)
@@ -74,10 +74,9 @@ class AddressScopeDbMixin(ext_address_scope.AddressScopePluginBase):
     def create_address_scope(self, context, address_scope):
         """Create an address scope."""
         a_s = address_scope['address_scope']
-        tenant_id = self._get_tenant_id_for_create(context, a_s)
         address_scope_id = a_s.get('id') or uuidutils.generate_uuid()
         with context.session.begin(subtransactions=True):
-            pool_args = {'tenant_id': tenant_id,
+            pool_args = {'tenant_id': a_s['tenant_id'],
                          'id': address_scope_id,
                          'name': a_s['name'],
                          'shared': a_s['shared'],
index 301e0ba5906253331d636780ab24d007ea9b040c..ba913bfbab3d618e37a0ba7d1f19dd2e4662de3b 100644 (file)
@@ -20,8 +20,6 @@ from sqlalchemy import and_
 from sqlalchemy import or_
 from sqlalchemy import sql
 
-from neutron._i18n import _
-from neutron.common import exceptions as n_exc
 from neutron.db import sqlalchemyutils
 
 
@@ -169,17 +167,6 @@ class CommonDbMixin(object):
                          if key in fields))
         return resource
 
-    def _get_tenant_id_for_create(self, context, resource):
-        if context.is_admin and 'tenant_id' in resource:
-            tenant_id = resource['tenant_id']
-        elif ('tenant_id' in resource and
-              resource['tenant_id'] != context.tenant_id):
-            reason = _('Cannot create resource for another tenant')
-            raise n_exc.AdminRequired(reason=reason)
-        else:
-            tenant_id = context.tenant_id
-        return tenant_id
-
     def _get_by_id(self, context, model, id):
         query = self._model_query(context, model)
         return query.filter(model.id == id).one()
index fdc92fa2955cc5a715310ade8db006d74d82667a..aa155048c9cd911de2ebb711acfbde205f49bf79 100644 (file)
@@ -315,7 +315,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
         n = network['network']
         # NOTE(jkoelker) Get the tenant_id outside of the session to avoid
         #                unneeded db action if the operation raises
-        tenant_id = self._get_tenant_id_for_create(context, n)
+        tenant_id = n['tenant_id']
         with context.session.begin(subtransactions=True):
             args = {'tenant_id': tenant_id,
                     'id': n.get('id') or uuidutils.generate_uuid(),
@@ -646,7 +646,6 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
             net = netaddr.IPNetwork(s['cidr'])
             subnet['subnet']['cidr'] = '%s/%s' % (net.network, net.prefixlen)
 
-        s['tenant_id'] = self._get_tenant_id_for_create(context, s)
         subnetpool_id = self._get_subnetpool_id(context, s)
         if subnetpool_id:
             self.ipam.validate_pools_with_subnetpool(s)
@@ -955,9 +954,8 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
         self._validate_address_scope_id(context, sp_reader.address_scope_id,
                                         id, sp_reader.prefixes,
                                         sp_reader.ip_version)
-        tenant_id = self._get_tenant_id_for_create(context, sp)
         with context.session.begin(subtransactions=True):
-            pool_args = {'tenant_id': tenant_id,
+            pool_args = {'tenant_id': sp['tenant_id'],
                          'id': sp_reader.id,
                          'name': sp_reader.name,
                          'ip_version': sp_reader.ip_version,
@@ -1165,7 +1163,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
         network_id = p['network_id']
         # NOTE(jkoelker) Get the tenant_id outside of the session to avoid
         #                unneeded db action if the operation raises
-        tenant_id = self._get_tenant_id_for_create(context, p)
+        tenant_id = p['tenant_id']
         if p.get('device_owner'):
             self._enforce_device_owner_not_router_intf_or_device_id(
                 context, p.get('device_owner'), p.get('device_id'), tenant_id)
index ee6ad5142cd87f5ace44880f376547ed0bd9e613..81af764dc539d3b1bef2436a6d1039ffe5bba3b8 100644 (file)
@@ -178,8 +178,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
     def create_router(self, context, router):
         r = router['router']
         gw_info = r.pop(EXTERNAL_GW_INFO, None)
-        tenant_id = self._get_tenant_id_for_create(context, r)
-        router_db = self._create_router_db(context, r, tenant_id)
+        router_db = self._create_router_db(context, r, r['tenant_id'])
         try:
             if gw_info:
                 self._update_router_gw_info(context, router_db['id'],
@@ -956,7 +955,6 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
     def _create_floatingip(self, context, floatingip,
             initial_status=l3_constants.FLOATINGIP_STATUS_ACTIVE):
         fip = floatingip['floatingip']
-        tenant_id = self._get_tenant_id_for_create(context, fip)
         fip_id = uuidutils.generate_uuid()
 
         f_net_id = fip['floating_network_id']
@@ -1003,12 +1001,11 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
             floating_ip_address = floating_fixed_ip['ip_address']
             floatingip_db = FloatingIP(
                 id=fip_id,
-                tenant_id=tenant_id,
+                tenant_id=fip['tenant_id'],
                 status=initial_status,
                 floating_network_id=fip['floating_network_id'],
                 floating_ip_address=floating_ip_address,
                 floating_port_id=external_port['id'])
-            fip['tenant_id'] = tenant_id
             # Update association with internal port
             # and define external IP address
             self._update_fip_assoc(context, fip,
index 51f0f57370a1659664af8dcc98539e56f7bd6e6f..a9af2e27fcd574c3ad35b5870b73229cba0b548a 100644 (file)
@@ -68,12 +68,11 @@ class MeteringDbMixin(metering.MeteringPluginBase,
 
     def create_metering_label(self, context, metering_label):
         m = metering_label['metering_label']
-        tenant_id = self._get_tenant_id_for_create(context, m)
 
         with context.session.begin(subtransactions=True):
             metering_db = MeteringLabel(id=uuidutils.generate_uuid(),
                                         description=m['description'],
-                                        tenant_id=tenant_id,
+                                        tenant_id=m['tenant_id'],
                                         name=m['name'],
                                         shared=m['shared'])
             context.session.add(metering_db)
index d5692f67c4514433f2377f6c4e93701225e6c563..f2efb6465edae57043a947cddcc6943dafc04070 100644 (file)
@@ -42,12 +42,11 @@ class RbacPluginMixin(common_db_mixin.CommonDbMixin):
         except c_exc.CallbackFailure as e:
             raise n_exc.InvalidInput(error_message=e)
         dbmodel = models.get_type_model_map()[e['object_type']]
-        tenant_id = self._get_tenant_id_for_create(context, e)
         with context.session.begin(subtransactions=True):
             db_entry = dbmodel(object_id=e['object_id'],
                                target_tenant=e['target_tenant'],
                                action=e['action'],
-                               tenant_id=tenant_id)
+                               tenant_id=e['tenant_id'])
             context.session.add(db_entry)
         return self._make_rbac_policy_dict(db_entry)
 
index 8cbcc22e6064cffd12bbe441c0974e8b6dd66b6b..a5a7bfea071b74ad6c7ca01d860755f1d39fcb97 100644 (file)
@@ -149,7 +149,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
         except exceptions.CallbackFailure as e:
             raise ext_sg.SecurityGroupConflict(reason=e)
 
-        tenant_id = self._get_tenant_id_for_create(context, s)
+        tenant_id = s['tenant_id']
 
         if not default_sg:
             self._ensure_default_security_group(context, tenant_id)
@@ -393,11 +393,10 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
         except exceptions.CallbackFailure as e:
             raise ext_sg.SecurityGroupConflict(reason=e)
 
-        tenant_id = self._get_tenant_id_for_create(context, rule_dict)
         with context.session.begin(subtransactions=True):
             db = SecurityGroupRule(
                 id=(rule_dict.get('id') or uuidutils.generate_uuid()),
-                tenant_id=tenant_id,
+                tenant_id=rule_dict['tenant_id'],
                 security_group_id=rule_dict['security_group_id'],
                 direction=rule_dict['direction'],
                 remote_group_id=rule_dict.get('remote_group_id'),
@@ -729,8 +728,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
         port = port['port']
         if port.get('device_owner') and utils.is_port_trusted(port):
             return
-        tenant_id = self._get_tenant_id_for_create(context, port)
-        default_sg = self._ensure_default_security_group(context, tenant_id)
+        default_sg = self._ensure_default_security_group(context,
+                                                         port['tenant_id'])
         if not attributes.is_attr_set(port.get(ext_sg.SECURITYGROUPS)):
             port[ext_sg.SECURITYGROUPS] = [default_sg]
 
index 3648deb8404100611ae20821374c69ebeee88093..24f653770a9452b9cac8788c11792ff7e563643a 100644 (file)
@@ -616,7 +616,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
 
     def _create_network_db(self, context, network):
         net_data = network[attributes.NETWORK]
-        tenant_id = self._get_tenant_id_for_create(context, net_data)
+        tenant_id = net_data['tenant_id']
         session = context.session
         with session.begin(subtransactions=True):
             self._ensure_default_security_group(context, tenant_id)
index 45d22b6d4466a55db90f74acae7525cd75da6d96..2f94b8d0dc3f2fbba921ed147de36eb735394441 100644 (file)
@@ -47,7 +47,8 @@ class L3SchedulerBaseTest(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
         return agent
 
     def _create_router(self, name):
-        router = {'name': name, 'admin_state_up': True}
+        router = {'name': name, 'admin_state_up': True,
+                  'tenant_id': self.adminContext.tenant_id}
         return self.l3_plugin.create_router(
             self.adminContext, {'router': router})
 
@@ -304,7 +305,8 @@ class L3AZSchedulerBaseTest(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
 
     def _create_router(self, az_hints, ha):
         router = {'name': 'router1', 'admin_state_up': True,
-                  'availability_zone_hints': az_hints}
+                  'availability_zone_hints': az_hints,
+                  'tenant_id': self._tenant_id}
         if ha:
             router['ha'] = True
         return self.l3_plugin.create_router(
index 6c4efb75072adcb4e45192c42b6e5dc623499c8d..4c4e9a2a795dfe48ac14c812144e7afd272ed0bd 100644 (file)
@@ -96,6 +96,7 @@ class PluginClientFixture(AbstractClientFixture):
         # framework
         kwargs.setdefault('admin_state_up', True)
         kwargs.setdefault('shared', False)
+        kwargs.setdefault('tenant_id', self.ctx.tenant_id)
         data = dict(network=kwargs)
         result = self.plugin.create_network(self.ctx, data)
         return base.AttributeDict(result)
index 341436af0459b390eae0646236ccb4cee13e481c..eac6d97371f64ff2b4ac8b5492fbc702573b3225 100644 (file)
@@ -37,11 +37,13 @@ class TestL3RpcCallback(testlib_api.SqlTestCase):
     def _prepare_network(self):
         network = {'network': {'name': 'abc',
                                'shared': False,
+                               'tenant_id': 'tenant_id',
                                'admin_state_up': True}}
         return self.plugin.create_network(self.ctx, network)
 
     def _prepare_ipv6_pd_subnet(self):
         subnet = {'subnet': {'network_id': self.network['id'],
+                             'tenant_id': 'tenant_id',
                              'cidr': None,
                              'ip_version': 6,
                              'name': 'ipv6_pd',
index e83fbf84c38dd39fa5a05df511b41a89a03cb316..c3324793c8009140370ee23464d82cb6d5cda0e1 100644 (file)
@@ -18,11 +18,15 @@ import testtools
 
 import mock
 import netaddr
+import webob.exc
+
+from oslo_utils import uuidutils
 
 from neutron._i18n import _
 from neutron.api.v2 import attributes
 from neutron.common import constants
 from neutron.common import exceptions as n_exc
+from neutron import context
 from neutron.tests import base
 from neutron.tests import tools
 
@@ -1009,3 +1013,32 @@ class TestResDict(base.BaseTestCase):
                           attr_info, {'key': 1}, {'key': 1})
         self.assertRaises(self._EXC_CLS, attributes.convert_value,
                           attr_info, {'key': 1}, self._EXC_CLS)
+
+    def test_populate_tenant_id(self):
+        tenant_id_1 = uuidutils.generate_uuid()
+        tenant_id_2 = uuidutils.generate_uuid()
+        # apart from the admin, nobody can create a res on behalf of another
+        # tenant
+        ctx = context.Context(user_id=None, tenant_id=tenant_id_1)
+        res_dict = {'tenant_id': tenant_id_2}
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          attributes.populate_tenant_id,
+                          ctx, res_dict, None, None)
+        ctx.is_admin = True
+        self.assertIsNone(attributes.populate_tenant_id(ctx, res_dict,
+                                                        None, None))
+
+        # for each create request, the tenant_id should be added to the
+        # req body
+        res_dict2 = {}
+        attributes.populate_tenant_id(ctx, res_dict2, None, True)
+        self.assertEqual({'tenant_id': ctx.tenant_id}, res_dict2)
+
+        # if the tenant_id is mandatory for the resource and not specified
+        # in the request nor in the context, an exception should be raised
+        res_dict3 = {}
+        attr_info = {'tenant_id': {'allow_post': True}, }
+        ctx.tenant_id = None
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          attributes.populate_tenant_id,
+                          ctx, res_dict3, attr_info, True)
index 582794f099b47734e1d0232e788d6df5b591732f..65d9ddb84e52136fd74e0ea42282304795e24bad 100644 (file)
@@ -783,6 +783,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
             self._set_net_external(net_id)
             router = {'name': 'router1',
                       'admin_state_up': True,
+                      'tenant_id': 'tenant_id',
                       'external_gateway_info': {'network_id': net_id},
                       'distributed': True}
             r = self.l3plugin.create_router(
@@ -1087,6 +1088,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
 
             router = {'name': 'router1',
                       'external_gateway_info': {'network_id': net_id},
+                      'tenant_id': 'tenant_id',
                       'admin_state_up': True,
                       'distributed': True}
             r = self.l3plugin.create_router(self.adminContext,
@@ -1119,6 +1121,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
             self._set_net_external(net_id)
 
             router = {'name': 'router1',
+                      'tenant_id': 'tenant_id',
                       'admin_state_up': True,
                       'distributed': True}
             r = self.l3plugin.create_router(self.adminContext,
@@ -1158,6 +1161,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
 
             router = {'name': 'router1',
                       'external_gateway_info': {'network_id': net_id},
+                      'tenant_id': 'tenant_id',
                       'admin_state_up': True,
                       'distributed': True}
             r = self.l3plugin.create_router(self.adminContext,
@@ -1186,6 +1190,7 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase):
 
             router = {'name': 'router1',
                       'external_gateway_info': {'network_id': net_id},
+                      'tenant_id': 'tenant_id',
                       'admin_state_up': True,
                       'distributed': True}
             r = self.l3plugin.create_router(self.adminContext,
index 8e22257eeb2269ab3301f9dcca57246f68be0543..c4a00094fa679358d9e2f4014db16830b0e60fc5 100644 (file)
@@ -68,7 +68,9 @@ class L3HATestFramework(testlib_api.SqlTestCase):
         if ctx is None:
             ctx = self.admin_ctx
         ctx.tenant_id = tenant_id
-        router = {'name': 'router1', 'admin_state_up': True}
+        router = {'name': 'router1',
+                  'admin_state_up': True,
+                  'tenant_id': tenant_id}
         if ha is not None:
             router['ha'] = ha
         if distributed is not None:
index 7d57c6b1214f585bc3d6aef1d6d485e3361256ac..b1b27326fe70dde5c5ee2558de3fb2462141eae7 100644 (file)
@@ -2471,6 +2471,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
         plugin = manager.NeutronManager.get_service_plugins()[
                     service_constants.L3_ROUTER_NAT]
         router_req = {'router': {'id': _uuid(), 'name': 'router',
+                                 'tenant_id': 'foo',
                                  'admin_state_up': True}}
         result = plugin.create_router(context.Context('', 'foo'), router_req)
         self.assertEqual(result['id'], router_req['router']['id'])
@@ -2976,6 +2977,7 @@ class L3NatDBTestCaseMixin(object):
         with self.network() as n:
             data = {'router': {
                 'name': 'router1', 'admin_state_up': True,
+                'tenant_id': ctx.tenant_id,
                 'external_gateway_info': {'network_id': n['network']['id']}}}
 
             self.assertRaises(MyException, plugin.create_router, ctx, data)
index 734ad8ca1b25d5d6c825318a3999ad73de754d42..0179ca5752163648411c18c2a6a59e2c637b9f17 100644 (file)
@@ -60,7 +60,7 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
     supported_extension_aliases = ["security-group", "port-security"]
 
     def create_network(self, context, network):
-        tenant_id = self._get_tenant_id_for_create(context, network['network'])
+        tenant_id = network['network'].get('tenant_id')
         self._ensure_default_security_group(context, tenant_id)
         with context.session.begin(subtransactions=True):
             neutron_db = super(PortSecurityTestPlugin, self).create_network(
index c7195803b9d214477c5bad874547f51db774432c..e8fb5573429ebbafb3db2ccc2e5f8f4a5a05a78e 100644 (file)
@@ -177,7 +177,7 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
     supported_extension_aliases = ["security-group"]
 
     def create_port(self, context, port):
-        tenant_id = self._get_tenant_id_for_create(context, port['port'])
+        tenant_id = port['port']['tenant_id']
         default_sg = self._ensure_default_security_group(context, tenant_id)
         if not attr.is_attr_set(port['port'].get(ext_sg.SECURITYGROUPS)):
             port['port'][ext_sg.SECURITYGROUPS] = [default_sg]
@@ -207,8 +207,8 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
         return port
 
     def create_network(self, context, network):
-        tenant_id = self._get_tenant_id_for_create(context, network['network'])
-        self._ensure_default_security_group(context, tenant_id)
+        self._ensure_default_security_group(context,
+                                            network['network']['tenant_id'])
         return super(SecurityGroupTestPlugin, self).create_network(context,
                                                                    network)
 
index 2d13438acd455586d31ad9c72b4af9bbb68c552d..1e3a22482c21b128bcceca943b16a31c9d342e2f 100644 (file)
@@ -867,7 +867,8 @@ class TestMl2DvrPortsV2(TestMl2PortsV2):
             p_const.L3_ROUTER_NAT]
         r = plugin.create_router(
             self.context,
-            {'router': {'name': 'router', 'admin_state_up': True}})
+            {'router': {'name': 'router', 'admin_state_up': True,
+             'tenant_id': self.context.tenant_id}})
         with self.subnet() as s:
             p = plugin.add_router_interface(self.context, r['id'],
                                             {'subnet_id': s['subnet']['id']})
index 243d6361e574128e668414e1d0562278d0e9bea2..b320d1a5fa9e1e663e370464c6f52f330020d3a3 100644 (file)
@@ -1797,7 +1797,8 @@ class L3HATestCaseMixin(testlib_api.SqlTestCase,
 
     def _create_ha_router(self, ha=True, tenant_id='tenant1', az_hints=None):
         self.adminContext.tenant_id = tenant_id
-        router = {'name': 'router1', 'admin_state_up': True}
+        router = {'name': 'router1', 'admin_state_up': True,
+                  'tenant_id': tenant_id}
         if ha is not None:
             router['ha'] = ha
         if az_hints is None: