Make sure floating IPs + gateways must be on external nets

bug #1042030

- adds admin-writable, world-readable router:external attribute to
the network object if L3 extension is loaded.
- prevents floating ips from being created unless network is external
- shortens L3 extensions alias from 'os-quantum-router' to 'router' to
make attribute extensions more readable.

WIP:
- Need to add policy logic so non-admin users can always see external
networks without requiring that these networks are shared (since VMs can
always create ports on shared networks, but provider may want to have
externals networks that VMs cannot directly plug into.

Random clean-up:
- prevent delete_network in plugins from implying it returns something
- modify extensions.py so that exceptions during calls to
get_extended_resources() will actually be logged if unexpected.
- unset executable bit on test_iptables_manager.py to make sure tox
actually runs it.

Change-Id: I5bbf063927b93458da7cb467d9ad5c92ebabbbf7

etc/policy.json

@@ -8,6 +8,9 @@
     "extension:provider_network:view": [["rule:admin_only"]],
     "extension:provider_network:set": [["rule:admin_only"]],
 
+    "extension:router:view": [["rule:regular_user"]],
+    "extension:router:set": [["rule:admin_only"]],
+
     "networks:private:read": [["rule:admin_or_owner"]],
     "networks:private:write": [["rule:admin_or_owner"]],
     "networks:shared:read": [["rule:regular_user"]],

quantum/db/l3_db.py

@@ -33,6 +33,7 @@ from quantum.db import model_base
 from quantum.db import models_v2
 from quantum.extensions import l3
 from quantum.openstack.common import cfg
+from quantum import policy
 
 
 LOG = logging.getLogger(__name__)
@@ -60,6 +61,12 @@ class Router(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant):
     gw_port = orm.relationship(models_v2.Port)
 
 
+class ExternalNetwork(model_base.BASEV2):
+    network_id = sa.Column(sa.String(36),
+                           sa.ForeignKey('networks.id', ondelete="CASCADE"),
+                           primary_key=True)
+
+
 class FloatingIP(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant):
     """Represents a floating IP, which may or many not be
        allocated to a tenant, and may or may not be associated with
@@ -145,8 +152,10 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
 
         network_id = info.get('network_id', None) if info else None
         if network_id:
-            #FIXME(danwent): confirm net-id is valid external network
             self._get_network(context, network_id)
+            if not self._network_is_external(context, network_id):
+                msg = "Network %s is not a valid external network" % network_id
+                raise q_exc.BadRequest(resource='router', msg=msg)
 
         # figure out if we need to delete existing port
         if gw_port and gw_port['network_id'] != network_id:
@@ -450,14 +459,17 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
         tenant_id = self._get_tenant_id_for_create(context, fip)
         fip_id = utils.str_uuid()
 
-        #TODO(danwent): validate that network_id is valid floatingip-network
+        f_net_id = fip['floating_network_id']
+        if not self._network_is_external(context, f_net_id):
+            msg = "Network %s is not a valid external network" % f_net_id
+            raise q_exc.BadRequest(resource='floatingip', msg=msg)
 
         # This external port is never exposed to the tenant.
         # it is used purely for internal system and admin use when
         # managing floating IPs.
         external_port = self.create_port(context, {
             'port':
-            {'network_id': fip['floating_network_id'],
+            {'network_id': f_net_id,
              'mac_address': attributes.ATTR_NOT_SPECIFIED,
              'fixed_ips': attributes.ATTR_NOT_SPECIFIED,
              'admin_state_up': True,
@@ -552,3 +564,80 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                 # should never happen
                 raise Exception('Multiple floating IPs found for port %s'
                                 % port_id)
+
+    def _check_l3_view_auth(self, context, network):
+        return policy.check(context,
+                            "extension:router:view",
+                            network)
+
+    def _enforce_l3_set_auth(self, context, network):
+        return policy.enforce(context,
+                              "extension:router:set",
+                              network)
+
+    def _network_is_external(self, context, net_id):
+        try:
+            context.session.query(ExternalNetwork).filter_by(
+                network_id=net_id).one()
+            return True
+        except exc.NoResultFound:
+            return False
+
+    def _extend_network_dict_l3(self, context, network):
+        if self._check_l3_view_auth(context, network):
+            network['router:external'] = self._network_is_external(
+                context, network['id'])
+
+    def _process_l3_create(self, context, net_data, net_id):
+        external = net_data.get('router:external')
+        external_set = attributes.is_attr_set(external)
+
+        if not external_set:
+            return
+
+        self._enforce_l3_set_auth(context, net_data)
+
+        if external:
+            # expects to be called within a plugin's session
+            context.session.add(ExternalNetwork(network_id=net_id))
+
+    def _process_l3_update(self, context, net_data, net_id):
+
+        new_value = net_data.get('router:external')
+        if not attributes.is_attr_set(new_value):
+            return
+
+        self._enforce_l3_set_auth(context, net_data)
+        existing_value = self._network_is_external(context, net_id)
+
+        if existing_value == new_value:
+            return
+
+        if new_value:
+            context.session.add(ExternalNetwork(network_id=net_id))
+        else:
+            # must make sure we do not have any external gateway ports
+            # (and thus, possible floating IPs) on this network before
+            # allow it to be update to external=False
+            try:
+                context.session.query(models_v2.Port).filter_by(
+                    device_owner=DEVICE_OWNER_ROUTER_GW,
+                    network_id=net_id).first()
+                raise ExternalNetworkInUse(net_id=net_id)
+            except exc.NoResultFound:
+                pass  # expected
+
+            context.session.query(ExternalNetwork).filter_by(
+                network_id=net_id).delete()
+
+    def _filter_nets_l3(self, context, nets, filters):
+        vals = filters.get('router:external', [])
+        if not vals:
+            return nets
+
+        ext_nets = set([en['network_id'] for en in
+                        context.session.query(ExternalNetwork).all()])
+        if vals[0]:
+            return [n for n in nets if n['id'] in ext_nets]
+        else:
+            return [n for n in nets if n['id'] not in ext_nets]

quantum/extensions/extensions.py

@@ -439,6 +439,8 @@ class ExtensionManager(object):
         wants to extend this map.
         """
         for ext in self.extensions.itervalues():
+            if not hasattr(ext, 'get_extended_resources'):
+                continue
             try:
                 extended_attrs = ext.get_extended_resources(version)
                 for resource, resource_attrs in extended_attrs.iteritems():
@@ -447,9 +449,8 @@ class ExtensionManager(object):
                     else:
                         attr_map[resource] = resource_attrs
             except AttributeError:
-                # Extensions aren't required to have extended
-                # attributes
-                pass
+                LOG.exception("Error fetching extended attributes for "
+                              "extension '%s'" % ext.get_name())
 
     def _check_extension(self, extension):
         """Checks for required methods in extension objects."""

quantum/extensions/l3.py

@@ -59,6 +59,11 @@ class L3PortInUse(qexception.InUse):
                 " cannot be deleted directly via the port API.")
 
 
+class ExternalNetworkInUse(qexception.InUse):
+    message = _("External network %(net_id)s cannot be updated to be made "
+                "non-external, since it has existing gateway ports")
+
+
 def _validate_uuid_or_none(data, valid_values=None):
     if data is None:
         return None
@@ -109,6 +114,14 @@ RESOURCE_ATTRIBUTE_MAP = {
     },
 }
 
+EXTENDED_ATTRIBUTES_2_0 = {
+    'networks': {'router:external': {'allow_post': True,
+                                     'allow_put': True,
+                                     'default': attr.ATTR_NOT_SPECIFIED,
+                                     'is_visible': True,
+                                     'convert_to': attr.convert_to_boolean,
+                                     'validate': {'type:boolean': None}}}}
+
 l3_quota_opts = [
     cfg.IntOpt('quota_router',
                default=10,
@@ -125,11 +138,11 @@ class L3(object):
 
     @classmethod
     def get_name(cls):
-        return "Quantum Router"
+        return "Quantum L3 Router"
 
     @classmethod
     def get_alias(cls):
-        return "os-quantum-router"
+        return "router"
 
     @classmethod
     def get_description(cls):
@@ -139,7 +152,7 @@ class L3(object):
 
     @classmethod
     def get_namespace(cls):
-        return "http://docs.openstack.org/ext/os-quantum-router/api/v1.0"
+        return "http://docs.openstack.org/ext/quantum/router/api/v1.0"
 
     @classmethod
     def get_updated(cls):
@@ -173,6 +186,12 @@ class L3(object):
 
         return exts
 
+    def get_extended_resources(self, version):
+        if version == "2.0":
+            return EXTENDED_ATTRIBUTES_2_0
+        else:
+            return {}
+
 
 class RouterPluginBase(object):
 

quantum/plugins/linuxbridge/lb_quantum_plugin.py

@@ -153,7 +153,7 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2,
     # is qualified by class
     __native_bulk_support = True
 
-    supported_extension_aliases = ["provider", "os-quantum-router"]
+    supported_extension_aliases = ["provider", "router"]
 
     def __init__(self):
         db.initialize()
@@ -215,7 +215,7 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2,
                               "extension:provider_network:set",
                               network)
 
-    def _extend_network_dict(self, context, network):
+    def _extend_network_dict_provider(self, context, network):
         if self._check_provider_view_auth(context, network):
             binding = db.get_network_binding(context.session, network['id'])
             network[provider.PHYSICAL_NETWORK] = binding.physical_network
@@ -310,7 +310,9 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2,
                                                                   network)
             db.add_network_binding(session, net['id'],
                                    physical_network, vlan_id)
-            self._extend_network_dict(context, net)
+            self._process_l3_create(context, network['network'], net['id'])
+            self._extend_network_dict_provider(context, net)
+            self._extend_network_dict_l3(context, net)
             # note - exception will rollback entire transaction
         return net
 
@@ -321,7 +323,9 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2,
         with session.begin(subtransactions=True):
             net = super(LinuxBridgePluginV2, self).update_network(context, id,
                                                                   network)
-            self._extend_network_dict(context, net)
+            self._process_l3_update(context, network['network'], id)
+            self._extend_network_dict_provider(context, net)
+            self._extend_network_dict_l3(context, net)
         return net
 
     def delete_network(self, context, id):
@@ -339,15 +343,20 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2,
 
     def get_network(self, context, id, fields=None):
         net = super(LinuxBridgePluginV2, self).get_network(context, id, None)
-        self._extend_network_dict(context, net)
+        self._extend_network_dict_provider(context, net)
+        self._extend_network_dict_l3(context, net)
         return self._fields(net, fields)
 
     def get_networks(self, context, filters=None, fields=None):
         nets = super(LinuxBridgePluginV2, self).get_networks(context, filters,
                                                              None)
         for net in nets:
-            self._extend_network_dict(context, net)
-        # TODO(rkukura): Filter on extended attributes.
+            self._extend_network_dict_provider(context, net)
+            self._extend_network_dict_l3(context, net)
+
+        # TODO(rkukura): Filter on extended provider attributes.
+        nets = self._filter_nets_l3(context, nets, filters)
+
         return [self._fields(net, fields) for net in nets]
 
     def update_port(self, context, id, port):

quantum/plugins/openvswitch/ovs_quantum_plugin.py

@@ -189,7 +189,7 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
     # bulk operations. Name mangling is used in order to ensure it
     # is qualified by class
     __native_bulk_support = True
-    supported_extension_aliases = ["provider", "os-quantum-router"]
+    supported_extension_aliases = ["provider", "router"]
 
     def __init__(self, configfile=None):
         ovs_db_v2.initialize()
@@ -266,7 +266,7 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
                               "extension:provider_network:set",
                               network)
 
-    def _extend_network_dict(self, context, network):
+    def _extend_network_dict_provider(self, context, network):
         if self._check_provider_view_auth(context, network):
             binding = ovs_db_v2.get_network_binding(context.session,
                                                     network['id'])
@@ -372,7 +372,10 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
                                                                  network)
             ovs_db_v2.add_network_binding(session, net['id'], network_type,
                                           physical_network, segmentation_id)
-            self._extend_network_dict(context, net)
+
+            self._process_l3_create(context, network['network'], net['id'])
+            self._extend_network_dict_provider(context, net)
+            self._extend_network_dict_l3(context, net)
             # note - exception will rollback entire transaction
         LOG.debug("Created network: %s" % net['id'])
         return net
@@ -384,15 +387,16 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
         with session.begin(subtransactions=True):
             net = super(OVSQuantumPluginV2, self).update_network(context, id,
                                                                  network)
-            self._extend_network_dict(context, net)
+            self._process_l3_update(context, network['network'], id)
+            self._extend_network_dict_provider(context, net)
+            self._extend_network_dict_l3(context, net)
         return net
 
     def delete_network(self, context, id):
         session = context.session
         with session.begin(subtransactions=True):
             binding = ovs_db_v2.get_network_binding(session, id)
-            result = super(OVSQuantumPluginV2, self).delete_network(context,
-                                                                    id)
+            super(OVSQuantumPluginV2, self).delete_network(context, id)
             if binding.network_type == constants.TYPE_GRE:
                 ovs_db_v2.release_tunnel(session, binding.segmentation_id,
                                          self.tunnel_id_ranges)
@@ -404,19 +408,23 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
             # the network record, so explicit removal is not necessary
         if self.agent_rpc:
             self.notifier.network_delete(self.rpc_context, id)
-        return result
 
     def get_network(self, context, id, fields=None):
         net = super(OVSQuantumPluginV2, self).get_network(context, id, None)
-        self._extend_network_dict(context, net)
+        self._extend_network_dict_provider(context, net)
+        self._extend_network_dict_l3(context, net)
         return self._fields(net, fields)
 
     def get_networks(self, context, filters=None, fields=None):
         nets = super(OVSQuantumPluginV2, self).get_networks(context, filters,
                                                             None)
         for net in nets:
-            self._extend_network_dict(context, net)
-        # TODO(rkukura): Filter on extended attributes.
+            self._extend_network_dict_provider(context, net)
+            self._extend_network_dict_l3(context, net)
+
+        # TODO(rkukura): Filter on extended provider attributes.
+        nets = self._filter_nets_l3(context, nets, filters)
+
         return [self._fields(net, fields) for net in nets]
 
     def update_port(self, context, id, port):

quantum/tests/unit/test_db_plugin.py

@@ -344,8 +344,8 @@ class QuantumDbPluginV2TestCase(unittest2.TestCase):
         self.assertEqual(res.status_int, expected_code)
         return self.deserialize('json', res)
 
-    def _list(self, resource):
-        req = self.new_list_request(resource)
+    def _list(self, resource, fmt='json', query_params=None):
+        req = self.new_list_request(resource, fmt, query_params)
         res = req.get_response(self._api_for_resource(resource))
         self.assertEqual(res.status_int, webob.exc.HTTPOk.code)
         return self.deserialize('json', res)

quantum/tests/unit/test_l3_plugin.py

@@ -87,9 +87,9 @@ class L3NatExtensionTestCase(unittest.TestCase):
         self._plugin_patcher = mock.patch(plugin, autospec=True)
         self.plugin = self._plugin_patcher.start()
 
-        # Instantiate mock plugin and enable the os-quantum-router  extension
+        # Instantiate mock plugin and enable the 'router' extension
         manager.QuantumManager.get_plugin().supported_extension_aliases = (
-            ["os-quantum-router"])
+            ["router"])
 
         ext_mgr = L3TestExtensionManager()
         self.ext_mdw = test_extensions.setup_extensions_middleware(ext_mgr)
@@ -218,7 +218,45 @@ class L3NatExtensionTestCase(unittest.TestCase):
 # This plugin class is just for testing
 class TestL3NatPlugin(db_base_plugin_v2.QuantumDbPluginV2,
                       l3_db.L3_NAT_db_mixin):
-    supported_extension_aliases = ["os-quantum-router"]
+    supported_extension_aliases = ["router"]
+
+    def create_network(self, context, network):
+        session = context.session
+        with session.begin(subtransactions=True):
+            net = super(TestL3NatPlugin, self).create_network(context,
+                                                              network)
+            self._process_l3_create(context, network['network'], net['id'])
+            self._extend_network_dict_l3(context, net)
+        return net
+
+    def update_network(self, context, id, network):
+
+        session = context.session
+        with session.begin(subtransactions=True):
+            net = super(TestL3NatPlugin, self).update_network(context, id,
+                                                              network)
+            self._process_l3_update(context, network['network'], id)
+            self._extend_network_dict_l3(context, net)
+        return net
+
+    def delete_network(self, context, id):
+        session = context.session
+        with session.begin(subtransactions=True):
+            net = super(TestL3NatPlugin, self).delete_network(context, id)
+
+    def get_network(self, context, id, fields=None):
+        net = super(TestL3NatPlugin, self).get_network(context, id, None)
+        self._extend_network_dict_l3(context, net)
+        return self._fields(net, fields)
+
+    def get_networks(self, context, filters=None, fields=None):
+        nets = super(TestL3NatPlugin, self).get_networks(context, filters,
+                                                         None)
+        for net in nets:
+            self._extend_network_dict_l3(context, net)
+        nets = self._filter_nets_l3(context, nets, filters)
+
+        return [self._fields(net, fields) for net in nets]
 
     def delete_port(self, context, id, l3_port_check=True):
         if l3_port_check:
@@ -403,6 +441,7 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
     def test_router_add_gateway(self):
         with self.router() as r:
             with self.subnet() as s:
+                self._set_net_external(s['subnet']['network_id'])
                 self._add_external_gateway_to_router(
                     r['router']['id'],
                     s['subnet']['network_id'])
@@ -422,9 +461,19 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
                 r['router']['id'],
                 "foobar", expected_code=exc.HTTPNotFound.code)
 
+    def test_router_add_gateway_net_not_external(self):
+        with self.router() as r:
+            with self.subnet() as s:
+                # intentionally do not set net as external
+                self._add_external_gateway_to_router(
+                    r['router']['id'],
+                    s['subnet']['network_id'],
+                    expected_code=exc.HTTPBadRequest.code)
+
     def test_router_add_gateway_no_subnet(self):
         with self.router() as r:
             with self.network() as n:
+                self._set_net_external(n['network']['id'])
                 self._add_external_gateway_to_router(
                     r['router']['id'],
                     n['network']['id'], expected_code=exc.HTTPBadRequest.code)
@@ -488,6 +537,10 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
                     # remove extra port created
                     self._delete('ports', p2['port']['id'])
 
+    def _set_net_external(self, net_id):
+        self._update('networks', net_id,
+                     {'network': {'router:external': True}})
+
     def _create_floatingip(self, fmt, network_id, port_id=None,
                            fixed_ip=None):
         data = {'floatingip': {'floating_network_id': network_id,
@@ -512,6 +565,7 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
     @contextlib.contextmanager
     def floatingip_with_assoc(self, port_id=None, fmt='json'):
         with self.subnet() as public_sub:
+            self._set_net_external(public_sub['subnet']['network_id'])
             with self.port() as private_port:
                 with self.router() as r:
                     sid = private_port['port']['fixed_ips'][0]['subnet_id']
@@ -542,6 +596,7 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
     @contextlib.contextmanager
     def floatingip_no_assoc(self, private_sub, fmt='json'):
         with self.subnet() as public_sub:
+            self._set_net_external(public_sub['subnet']['network_id'])
             with self.router() as r:
                 self._add_external_gateway_to_router(
                     r['router']['id'],
@@ -643,6 +698,7 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
 
     def test_create_floatingip_no_ext_gateway_return_404(self):
         with self.subnet() as public_sub:
+            self._set_net_external(public_sub['subnet']['network_id'])
             with self.port() as private_port:
                 with self.router() as r:
                     res = self._create_floatingip(
@@ -652,6 +708,17 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
                     # this should be some kind of error
                     self.assertEqual(res.status_int, exc.HTTPNotFound.code)
 
+    def test_create_floating_non_ext_network_returns_400(self):
+        with self.subnet() as public_sub:
+            # normally we would set the network of public_sub to be
+            # external, but the point of this test is to handle when
+            # that is not the case
+            with self.router() as r:
+                res = self._create_floatingip(
+                    'json',
+                    public_sub['subnet']['network_id'])
+                self.assertEqual(res.status_int, exc.HTTPBadRequest.code)
+
     def test_create_floatingip_no_public_subnet_returns_400(self):
         with self.network() as public_network:
             with self.port() as private_port:
@@ -690,3 +757,18 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
         res = self._create_floatingip('json', utils.str_uuid(),
                                       utils.str_uuid(), 'iamnotnanip')
         self.assertEqual(res.status_int, 422)
+
+    def test_list_nets_external(self):
+        with self.network() as n1:
+            self._set_net_external(n1['network']['id'])
+            with self.network() as n2:
+                body = self._list('networks')
+                self.assertEquals(len(body['networks']), 2)
+
+                body = self._list('networks',
+                                  query_params="router:external=True")
+                self.assertEquals(len(body['networks']), 1)
+
+                body = self._list('networks',
+                                  query_params="router:external=False")
+                self.assertEquals(len(body['networks']), 1)