]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
l3: not use L2 plugin _get_subnet unnecessarily
authorIsaku Yamahata <isaku.yamahata@intel.com>
Thu, 16 Jul 2015 02:27:16 +0000 (19:27 -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 and related code use L2 plugin _get_subnet and related method
unnecessarily instead of get_subnet.
It's dangerous because _get_subnet returns ORM db object which allows
the caller to update db rows directly. So the caller of _get_subnet
may update subnet db without notifying L2 plugin unintentionally.
In that case, L2 plugin or ML2 mechanism driver will be confused.
This patch replaces _get_subnet and _get_subnets_by_network with
get_subnet, get_subnets_by_network where possible.

Change-Id: I85769e639a408a292b5bd70a9d9a1ac292e2b51c
Related-Bug: #1475093

neutron/db/db_base_plugin_v2.py
neutron/db/extraroute_db.py
neutron/db/l3_db.py

index 01fb2fc2d2958384dc1815a808f35f2db88f7ae8..87dffa6652cfc42ad459c1c269a035101e869ff2 100644 (file)
@@ -684,6 +684,10 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
         return self._get_collection_count(context, models_v2.Subnet,
                                           filters=filters)
 
+    def get_subnets_by_network(self, context, network_id):
+        return [self._make_subnet_dict(subnet_db) for subnet_db in
+                self._get_subnets_by_network(context, network_id)]
+
     def _create_subnetpool_prefix(self, context, cidr, subnetpool_id):
         prefix_args = {'cidr': cidr, 'subnetpool_id': subnetpool_id}
         subnetpool_prefix = models_v2.SubnetPoolPrefix(**prefix_args)
index 264bff163179e34e8399b9e46c02c093ea5bd79b..0bf0ae228a06da08ffbfef05c50db664446b5f21 100644 (file)
@@ -108,7 +108,7 @@ class ExtraRoute_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin):
         ips = []
         for port in ports:
             for ip in port['fixed_ips']:
-                cidrs.append(self._core_plugin._get_subnet(
+                cidrs.append(self._core_plugin.get_subnet(
                     context, ip['subnet_id'])['cidr'])
                 ips.append(ip['ip_address'])
         for route in routes:
@@ -162,8 +162,8 @@ class ExtraRoute_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin):
         super(ExtraRoute_dbonly_mixin,
             self)._confirm_router_interface_not_in_use(
             context, router_id, subnet_id)
-        subnet_db = self._core_plugin._get_subnet(context, subnet_id)
-        subnet_cidr = netaddr.IPNetwork(subnet_db['cidr'])
+        subnet = self._core_plugin.get_subnet(context, subnet_id)
+        subnet_cidr = netaddr.IPNetwork(subnet['cidr'])
         extra_routes = self._get_extra_routes_by_router_id(context, router_id)
         for route in extra_routes:
             if netaddr.all_matching_cidrs(route['nexthop'], [subnet_cidr]):
index 1eb26a10fa4bed6652b3fde3d700957332682c39..71b9ac1cc8c94d48e5749f7a1094380bec7bcf43 100644 (file)
@@ -310,8 +310,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                 msg = _("Network %s is not an external network") % network_id
                 raise n_exc.BadRequest(resource='router', msg=msg)
             if ext_ips:
-                subnets = self._core_plugin._get_subnets_by_network(context,
-                                                                    network_id)
+                subnets = self._core_plugin.get_subnets_by_network(context,
+                                                                   network_id)
                 for s in subnets:
                     if not s['gateway_ip']:
                         continue
@@ -361,8 +361,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
             new_network and (not router.gw_port or
                              router.gw_port['network_id'] != new_network))
         if new_valid_gw_port_attachment:
-            subnets = self._core_plugin._get_subnets_by_network(context,
-                                                                new_network)
+            subnets = self._core_plugin.get_subnets_by_network(context,
+                                                               new_network)
             for subnet in subnets:
                 self._check_for_dup_router_subnet(context, router,
                                                   new_network, subnet['id'],
@@ -471,8 +471,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                                % subnet_id)
                         raise n_exc.BadRequest(resource='router', msg=msg)
                     sub_id = ip['subnet_id']
-                    cidr = self._core_plugin._get_subnet(context.elevated(),
-                                                         sub_id)['cidr']
+                    cidr = self._core_plugin.get_subnet(context.elevated(),
+                                                        sub_id)['cidr']
                     ipnet = netaddr.IPNetwork(cidr)
                     match1 = netaddr.all_matching_cidrs(new_ipnet, [cidr])
                     match2 = netaddr.all_matching_cidrs(ipnet, [subnet_cidr])
@@ -533,8 +533,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
             fixed_ips = [ip for ip in port['fixed_ips']]
             subnets = []
             for fixed_ip in fixed_ips:
-                subnet = self._core_plugin._get_subnet(context,
-                                                       fixed_ip['subnet_id'])
+                subnet = self._core_plugin.get_subnet(context,
+                                                      fixed_ip['subnet_id'])
                 subnets.append(subnet)
                 self._check_for_dup_router_subnet(context, router,
                                                   port['network_id'],
@@ -562,7 +562,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                 return port
 
     def _add_interface_by_subnet(self, context, router, subnet_id, owner):
-        subnet = self._core_plugin._get_subnet(context, subnet_id)
+        subnet = self._core_plugin.get_subnet(context, subnet_id)
         if not subnet['gateway_ip']:
             msg = _('Subnet for router interface must have a gateway IP')
             raise n_exc.BadRequest(resource='router', msg=msg)
@@ -645,8 +645,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
 
     def _confirm_router_interface_not_in_use(self, context, router_id,
                                              subnet_id):
-        subnet_db = self._core_plugin._get_subnet(context, subnet_id)
-        subnet_cidr = netaddr.IPNetwork(subnet_db['cidr'])
+        subnet = self._core_plugin.get_subnet(context, subnet_id)
+        subnet_cidr = netaddr.IPNetwork(subnet['cidr'])
         fip_qry = context.session.query(FloatingIP)
         try:
             kwargs = {'context': context, 'subnet_id': subnet_id}
@@ -682,7 +682,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         if subnet_id and subnet_id not in port_subnet_ids:
             raise n_exc.SubnetMismatchForPort(
                 port_id=port_id, subnet_id=subnet_id)
-        subnets = [self._core_plugin._get_subnet(context, port_subnet_id)
+        subnets = [self._core_plugin.get_subnet(context, port_subnet_id)
                    for port_subnet_id in port_subnet_ids]
         for port_subnet_id in port_subnet_ids:
             self._confirm_router_interface_not_in_use(
@@ -695,7 +695,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                                     router_id, subnet_id, owner):
         self._confirm_router_interface_not_in_use(
             context, router_id, subnet_id)
-        subnet = self._core_plugin._get_subnet(context, subnet_id)
+        subnet = self._core_plugin.get_subnet(context, subnet_id)
 
         try:
             rport_qry = context.session.query(models_v2.Port).join(RouterPort)
@@ -777,9 +777,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
     def _get_router_for_floatingip(self, context, internal_port,
                                    internal_subnet_id,
                                    external_network_id):
-        subnet_db = self._core_plugin._get_subnet(context,
-                                                  internal_subnet_id)
-        if not subnet_db['gateway_ip']:
+        subnet = self._core_plugin.get_subnet(context, internal_subnet_id)
+        if not subnet['gateway_ip']:
             msg = (_('Cannot add floating IP to port on subnet %s '
                      'which has no gateway_ip') % internal_subnet_id)
             raise n_exc.BadRequest(resource='floatingip', msg=msg)