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
This commit is contained in:
Salvatore Orlando 2013-08-08 10:38:11 -07:00
parent a34f6f0e86
commit 274481bf30
3 changed files with 43 additions and 4 deletions

View File

@ -280,6 +280,11 @@ class GatewayConflictWithAllocationPools(InUse):
"allocation pool %(pool)s") "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): class NetworkVlanRangeError(NeutronException):
message = _("Invalid network VLAN range: '%(vlan_range)s' - '%(error)s'") message = _("Invalid network VLAN range: '%(vlan_range)s' - '%(error)s'")

View File

@ -1022,7 +1022,7 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
"the ip_version '%(ip_version)s'") % data "the ip_version '%(ip_version)s'") % data
raise q_exc.InvalidInput(error_message=msg) 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.""" """Validate a subnet spec."""
# This method will validate attributes which may change during # This method will validate attributes which may change during
@ -1044,6 +1044,20 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
s['gateway_ip'])): s['gateway_ip'])):
error_message = _("Gateway is not valid on subnet") error_message = _("Gateway is not valid on subnet")
raise q_exc.InvalidInput(error_message=error_message) 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 attributes.is_attr_set(s.get('dns_nameservers')):
if len(s['dns_nameservers']) > cfg.CONF.max_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'], self._validate_gw_out_of_pools(s['gateway_ip'],
s['allocation_pools']) s['allocation_pools'])
self._validate_subnet(s) self._validate_subnet(context, s)
tenant_id = self._get_tenant_id_for_create(context, s) tenant_id = self._get_tenant_id_for_create(context, s)
with context.session.begin(subtransactions=True): 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['ip_version'] = db_subnet.ip_version
s['cidr'] = db_subnet.cidr s['cidr'] = db_subnet.cidr
s['id'] = db_subnet.id 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: if 'gateway_ip' in s and s['gateway_ip'] is not None:
allocation_pools = [{'start': p['first_ip'], 'end': p['last_ip']} allocation_pools = [{'start': p['first_ip'], 'end': p['last_ip']}

View File

@ -2966,6 +2966,23 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
res = req.get_response(self.api) res = req.get_response(self.api)
self.assertEqual(res.status_int, 400) 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): def test_update_subnet_inconsistent_ipv4_gatewayv6(self):
with self.network() as network: with self.network() as network:
with self.subnet(network=network) as subnet: with self.subnet(network=network) as subnet:
@ -3446,7 +3463,10 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
'nexthop': '1.2.3.4'}]} 'nexthop': '1.2.3.4'}]}
plugin = NeutronManager.get_plugin() plugin = NeutronManager.get_plugin()
e = self.assertRaises(exception, e = self.assertRaises(exception,
plugin._validate_subnet, subnet) plugin._validate_subnet,
context.get_admin_context(
load_admin_roles=False),
subnet)
self.assertThat( self.assertThat(
str(e), str(e),
matchers.Not(matchers.Contains('built-in function id'))) matchers.Not(matchers.Contains('built-in function id')))