openstack/neutron
Code Issues Proposed changes

Avoid duplicating tenant check when creating resources

Browse Source 
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
This commit is contained in:
Mathieu Rohon 2015-11-04 17:49:40 +00:00
parent 0c07378509
commit 5d53dfb8d6
19 changed files with 71 additions and 44 deletions

3
neutron/db/address_scope_db.py

@ -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'],

13
neutron/db/common_db_mixin.py

@ -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()

8
neutron/db/db_base_plugin_v2.py

@ -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)

7
neutron/db/l3_db.py

@ -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,

3
neutron/db/metering/metering_db.py

@ -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)

3
neutron/db/rbac_db_mixin.py

@ -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)

9
neutron/db/securitygroups_db.py

@ -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]

2
neutron/plugins/ml2/plugin.py

@ -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)

6
neutron/tests/functional/scheduler/test_l3_agent_scheduler.py

@ -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(

1
neutron/tests/retargetable/client_fixtures.py

@ -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)

2
neutron/tests/unit/api/rpc/handlers/test_l3_rpc.py

@ -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',

33
neutron/tests/unit/api/v2/test_attributes.py

@ -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)

5
neutron/tests/unit/db/test_agentschedulers_db.py

@ -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,

4
neutron/tests/unit/db/test_l3_hamode_db.py

@ -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:

2
neutron/tests/unit/extensions/test_l3.py

@ -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)

2
neutron/tests/unit/extensions/test_portsecurity.py

@ -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(

6
neutron/tests/unit/extensions/test_securitygroup.py

@ -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)

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

@ -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']})

3
neutron/tests/unit/scheduler/test_l3_agent_scheduler.py

@ -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: