From 274481bf30150f3900e0a860ca7321cdd682850e Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Thu, 8 Aug 2013 10:38:11 -0700 Subject: [PATCH] Forbid update of subnet gateway ip when in use by a port Bug 1186322 If a port is currently using the subnet's gateway IP, which usually happens for router interfaces, do not allow updates to the gateway IP. This patch adds an extra query on the IPAllocation model, which returns at most a single record, and is executed in _validate_subnet only when the subnet is updated. Change-Id: Ie29be1b83f9a639562bfc84f3f2c082833126d32 --- neutron/common/exceptions.py | 5 +++++ neutron/db/db_base_plugin_v2.py | 20 +++++++++++++++++--- neutron/tests/unit/test_db_plugin.py | 22 +++++++++++++++++++++- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index ae58e71df..5c9f8dd75 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -280,6 +280,11 @@ class GatewayConflictWithAllocationPools(InUse): "allocation pool %(pool)s") +class GatewayIpInUse(InUse): + message = _("Current gateway ip %(ip_address)s already in use " + "by port %(port_id)s. Unable to update.") + + class NetworkVlanRangeError(NeutronException): message = _("Invalid network VLAN range: '%(vlan_range)s' - '%(error)s'") diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 1db6464de..24b4a5e38 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1022,7 +1022,7 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, "the ip_version '%(ip_version)s'") % data raise q_exc.InvalidInput(error_message=msg) - def _validate_subnet(self, s): + def _validate_subnet(self, context, s, cur_subnet=None): """Validate a subnet spec.""" # This method will validate attributes which may change during @@ -1044,6 +1044,20 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, s['gateway_ip'])): error_message = _("Gateway is not valid on subnet") raise q_exc.InvalidInput(error_message=error_message) + # Ensure the gateway IP is not assigned to any port + # skip this check in case of create (s parameter won't have id) + # NOTE(salv-orlando): There is slight chance of a race, when + # a subnet-update and a router-interface-add operation are + # executed concurrently + if cur_subnet: + alloc_qry = context.session.query(models_v2.IPAllocation) + allocated = alloc_qry.filter_by( + ip_address=cur_subnet['gateway_ip'], + subnet_id=cur_subnet['id']).first() + if allocated and allocated['port_id']: + raise q_exc.GatewayIpInUse( + ip_address=cur_subnet['gateway_ip'], + port_id=allocated['port_id']) if attributes.is_attr_set(s.get('dns_nameservers')): if len(s['dns_nameservers']) > cfg.CONF.max_dns_nameservers: @@ -1094,7 +1108,7 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, self._validate_gw_out_of_pools(s['gateway_ip'], s['allocation_pools']) - self._validate_subnet(s) + self._validate_subnet(context, s) tenant_id = self._get_tenant_id_for_create(context, s) with context.session.begin(subtransactions=True): @@ -1157,7 +1171,7 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, s['ip_version'] = db_subnet.ip_version s['cidr'] = db_subnet.cidr s['id'] = db_subnet.id - self._validate_subnet(s) + self._validate_subnet(context, s, cur_subnet=db_subnet) if 'gateway_ip' in s and s['gateway_ip'] is not None: allocation_pools = [{'start': p['first_ip'], 'end': p['last_ip']} diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py index ff7c1fb1d..e628192f4 100644 --- a/neutron/tests/unit/test_db_plugin.py +++ b/neutron/tests/unit/test_db_plugin.py @@ -2966,6 +2966,23 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): res = req.get_response(self.api) self.assertEqual(res.status_int, 400) + def test_update_subnet_gw_ip_in_use_returns_409(self): + with self.network() as network: + with self.subnet( + network=network, + allocation_pools=[{'start': '10.0.0.100', + 'end': '10.0.0.253'}]) as subnet: + subnet_data = subnet['subnet'] + with self.port( + subnet=subnet, + fixed_ips=[{'subnet_id': subnet_data['id'], + 'ip_address': subnet_data['gateway_ip']}]): + data = {'subnet': {'gateway_ip': '10.0.0.99'}} + req = self.new_update_request('subnets', data, + subnet_data['id']) + res = req.get_response(self.api) + self.assertEqual(res.status_int, 409) + def test_update_subnet_inconsistent_ipv4_gatewayv6(self): with self.network() as network: with self.subnet(network=network) as subnet: @@ -3446,7 +3463,10 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): 'nexthop': '1.2.3.4'}]} plugin = NeutronManager.get_plugin() e = self.assertRaises(exception, - plugin._validate_subnet, subnet) + plugin._validate_subnet, + context.get_admin_context( + load_admin_roles=False), + subnet) self.assertThat( str(e), matchers.Not(matchers.Contains('built-in function id'))) -- 2.45.2