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

diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py
index ae58e71dfc4790b630c6ddcf6092d3d1f24392db..5c9f8dd755aa9bd03dbb95c636752e86e2bf1c7c 100644 (file)
--- 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 1db6464de9c4ace80955fdb44c7f8277dc6dbec7..24b4a5e38fdc970d804cebd6677fedc0dfa42bca 100644 (file)
--- 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 ff7c1fb1da3064c788671baceb9f857905acb865..e628192f45e7e66856fd9efb3bbdf14f17c817ce 100644 (file)
--- 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')))