]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
l3_db: not use L2 plugin _get_port unnecessarily
authorIsaku Yamahata <isaku.yamahata@intel.com>
Wed, 15 Jul 2015 19:34:12 +0000 (12:34 -0700)
committerIsaku Yamahata <isaku.yamahata@intel.com>
Fri, 14 Aug 2015 01:37:49 +0000 (18:37 -0700)
This patch is clean up to prevent future breakage by eliminating
potentially dangerous code.

l3_db uses L2 plugin _get_port method unnecessarily instead of get_port.
It's dangerous because _get_port returns ORM db object which allows
the caller to update db rows directly. So the caller of _get_port may
update port db without notifying L2 plugin unintentionally.
In that case, L2 plugin or ML2 mechanism driver will be confused.
This patch replace _get_port with get_port method where possible.

Change-Id: I5a23f6cac5ea359645e6947fd69978f060c4ba97
Related-Bug: #1475093

neutron/db/l3_db.py
neutron/tests/unit/db/test_l3_dvr_db.py

index c09c5273d33a0d9a8217a6e26e4693747269fb93..1eb26a10fa4bed6652b3fde3d700957332682c39 100644 (file)
@@ -818,7 +818,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         Retrieve information concerning the internal port where
         the floating IP should be associated to.
         """
-        internal_port = self._core_plugin._get_port(context, fip['port_id'])
+        internal_port = self._core_plugin.get_port(context, fip['port_id'])
         if not internal_port['tenant_id'] == fip['tenant_id']:
             port_id = fip['port_id']
             if 'id' in fip:
@@ -1077,23 +1077,23 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         deletion checks.
         """
         try:
-            port_db = self._core_plugin._get_port(context, port_id)
+            port = self._core_plugin.get_port(context, port_id)
         except n_exc.PortNotFound:
             # non-existent ports don't need to be protected from deletion
             return
-        if port_db['device_owner'] in self.router_device_owners:
+        if port['device_owner'] in self.router_device_owners:
             # Raise port in use only if the port has IP addresses
             # Otherwise it's a stale port that can be removed
-            fixed_ips = port_db['fixed_ips']
+            fixed_ips = port['fixed_ips']
             if fixed_ips:
-                reason = _('has device owner %s') % port_db['device_owner']
-                raise n_exc.ServicePortInUse(port_id=port_db['id'],
+                reason = _('has device owner %s') % port['device_owner']
+                raise n_exc.ServicePortInUse(port_id=port['id'],
                                              reason=reason)
             else:
                 LOG.debug("Port %(port_id)s has owner %(port_owner)s, but "
                           "no IP address, so it can be deleted",
-                          {'port_id': port_db['id'],
-                           'port_owner': port_db['device_owner']})
+                          {'port_id': port['id'],
+                           'port_owner': port['device_owner']})
 
     def disassociate_floatingips(self, context, port_id):
         """Disassociate all floating IPs linked to specific port.
index 419e168fb7a29d8d361146d021c3b1d5e20affe7..fcb4fa1250fa92bf3e1346d6a772959837fb259d 100644 (file)
@@ -173,7 +173,7 @@ class L3DvrTestCase(testlib_api.SqlTestCase):
         with mock.patch.object(manager.NeutronManager, 'get_plugin') as gp:
             plugin = mock.Mock()
             gp.return_value = plugin
-            plugin._get_port.return_value = port
+            plugin.get_port.return_value = port
             self.assertRaises(exceptions.ServicePortInUse,
                               self.mixin.prevent_l3_port_deletion,
                               self.ctx,