From: Dan Wendlandt Date: Thu, 6 Sep 2012 05:43:22 +0000 (-0700) Subject: Make sure floating IPs + gateways must be on external nets X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=75e2dfffaf02dd8f1b85b510e58d2f3719d97f19;p=openstack-build%2Fneutron-build.git 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 --- diff --git a/etc/policy.json b/etc/policy.json index 52feddf92..df7c8e352 100644 --- a/etc/policy.json +++ b/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"]], diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py index 89a2974e8..e134439cb 100644 --- a/quantum/db/l3_db.py +++ b/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] diff --git a/quantum/extensions/extensions.py b/quantum/extensions/extensions.py index f6a74654a..26ff3222b 100644 --- a/quantum/extensions/extensions.py +++ b/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.""" diff --git a/quantum/extensions/l3.py b/quantum/extensions/l3.py index 47c4866e3..52245ea2c 100644 --- a/quantum/extensions/l3.py +++ b/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): diff --git a/quantum/plugins/linuxbridge/lb_quantum_plugin.py b/quantum/plugins/linuxbridge/lb_quantum_plugin.py index bc34e96b7..da420e59e 100644 --- a/quantum/plugins/linuxbridge/lb_quantum_plugin.py +++ b/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): diff --git a/quantum/plugins/openvswitch/ovs_quantum_plugin.py b/quantum/plugins/openvswitch/ovs_quantum_plugin.py index 41d347e7a..a0c6568db 100644 --- a/quantum/plugins/openvswitch/ovs_quantum_plugin.py +++ b/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): diff --git a/quantum/tests/unit/test_db_plugin.py b/quantum/tests/unit/test_db_plugin.py index 2fbb3bddf..0fcfcf732 100644 --- a/quantum/tests/unit/test_db_plugin.py +++ b/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) diff --git a/quantum/tests/unit/test_iptables_manager.py b/quantum/tests/unit/test_iptables_manager.py old mode 100755 new mode 100644 diff --git a/quantum/tests/unit/test_l3_plugin.py b/quantum/tests/unit/test_l3_plugin.py index a60864900..3ffeb5a91 100644 --- a/quantum/tests/unit/test_l3_plugin.py +++ b/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)