diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 7880ddb1754..f15a7e71ce8 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1439,6 +1439,8 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, # conflict, bubble up a retry instead that should bring things # back to sanity. raise os_db_exc.RetryRequest(e) + except ipam_exc.IPAddressChangeNotAllowed as e: + raise exc.BadRequest(resource='ports', msg=e) return self._make_port_dict(db_port) @db_api.retry_if_session_inactive() diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index 68b88cbcfa1..8bbd1955132 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -20,6 +20,9 @@ from neutron_lib.api.definitions import portbindings from neutron_lib import constants from neutron_lib.db import api as db_api from neutron_lib import exceptions as n_exc +from neutron_lib.plugins import constants as plugin_consts +from neutron_lib.plugins import directory + from oslo_db import exception as db_exc from oslo_log import log as logging from oslo_utils import excutils @@ -37,6 +40,20 @@ from neutron.objects import subnet as obj_subnet LOG = logging.getLogger(__name__) +def get_ip_update_not_allowed_device_owner_list(): + l3plugin = directory.get_plugin(plugin_consts.L3) + # The following list is for IPAM to prevent direct update of port + # IP address. Currently it only has some L3 related types. + # L2 plugin can add the same list here, but for now it is not required. + return getattr(l3plugin, 'IP_UPDATE_NOT_ALLOWED_LIST', []) + + +def is_neutron_built_in_router(context, router_id): + l3plugin = directory.get_plugin(plugin_consts.L3) + return bool(l3plugin and + l3plugin.router_supports_scheduling(context, router_id)) + + class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): def _get_failed_ips(self, all_ips, success_ips): @@ -287,6 +304,20 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): return fixed_ip_list + def _check_ip_changed_by_version(self, context, ip_list, version): + for ip in ip_list: + ip_address = ip.get('ip_address') + subnet_id = ip.get('subnet_id') + if ip_address: + ip_addr = netaddr.IPAddress(ip_address) + if ip_addr.version == version: + return True + elif subnet_id: + subnet = obj_subnet.Subnet.get_object(context, id=subnet_id) + if subnet and subnet.ip_version == version: + return True + return False + def _update_ips_for_port(self, context, port, host, original_ips, new_ips, mac): """Add or remove IPs from the port. IPAM version""" @@ -294,6 +325,16 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): removed = [] changes = self._get_changed_ips_for_port( context, original_ips, new_ips, port['device_owner']) + + not_allowed_list = get_ip_update_not_allowed_device_owner_list() + if (port['device_owner'] in not_allowed_list and + is_neutron_built_in_router(context, port['device_id'])): + ip_v4_changed = self._check_ip_changed_by_version( + context, changes.remove + changes.add, + constants.IP_VERSION_4) + if ip_v4_changed: + raise ipam_exc.IPAddressChangeNotAllowed(port_id=port['id']) + try: subnets = self._ipam_get_subnets( context, network_id=port['network_id'], host=host, diff --git a/neutron/extensions/l3_port_ip_change_not_allowed.py b/neutron/extensions/l3_port_ip_change_not_allowed.py new file mode 100644 index 00000000000..f0380face50 --- /dev/null +++ b/neutron/extensions/l3_port_ip_change_not_allowed.py @@ -0,0 +1,18 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron_lib.api.definitions import l3_port_ip_change_not_allowed as apidef +from neutron_lib.api import extensions + + +class L3_port_ip_change_not_allowed(extensions.APIExtensionDescriptor): + api_definition = apidef diff --git a/neutron/ipam/exceptions.py b/neutron/ipam/exceptions.py index 9df33add318..badeef82b4e 100644 --- a/neutron/ipam/exceptions.py +++ b/neutron/ipam/exceptions.py @@ -54,6 +54,10 @@ class InvalidSubnetRequest(exceptions.BadRequest): "%(reason)s") +class IPAddressChangeNotAllowed(exceptions.BadRequest): + message = _("IP updates for port %(port_id)s are not allowed") + + class AllocationOnAutoAddressSubnet(exceptions.InvalidInput): message = _("IPv6 address %(ip)s cannot be directly " "assigned to a port on subnet %(subnet_id)s as the " diff --git a/neutron/services/l3_router/l3_router_plugin.py b/neutron/services/l3_router/l3_router_plugin.py index 2fa77be73f0..be15b4cc340 100644 --- a/neutron/services/l3_router/l3_router_plugin.py +++ b/neutron/services/l3_router/l3_router_plugin.py @@ -87,12 +87,20 @@ class L3RouterPlugin(service_base.ServicePluginBase, "l3-ha", "router_availability_zone", "l3-flavors", "qos-fip", "fip-port-details", "floatingip-pools", - "qos-gateway-ip"] + "qos-gateway-ip", + "l3-port-ip-change-not-allowed"] __native_pagination_support = True __native_sorting_support = True __filter_validation_support = True + IP_UPDATE_NOT_ALLOWED_LIST = [ + n_const.DEVICE_OWNER_ROUTER_INTF, + n_const.DEVICE_OWNER_ROUTER_HA_INTF, + n_const.DEVICE_OWNER_HA_REPLICATED_INT, + n_const.DEVICE_OWNER_ROUTER_SNAT, + n_const.DEVICE_OWNER_DVR_INTERFACE] + @resource_registry.tracked_resources(router=l3_models.Router, floatingip=l3_models.FloatingIP) def __init__(self): diff --git a/neutron/tests/contrib/hooks/api_all_extensions b/neutron/tests/contrib/hooks/api_all_extensions index 990a22b02ed..7294208775f 100644 --- a/neutron/tests/contrib/hooks/api_all_extensions +++ b/neutron/tests/contrib/hooks/api_all_extensions @@ -25,6 +25,7 @@ NETWORK_API_EXTENSIONS+=",ip-substring-filtering" NETWORK_API_EXTENSIONS+=",l3-flavors" NETWORK_API_EXTENSIONS+=",l3-ha" NETWORK_API_EXTENSIONS+=",l3_agent_scheduler" +NETWORK_API_EXTENSIONS+=",l3-port-ip-change-not-allowed" NETWORK_API_EXTENSIONS+=",logging" NETWORK_API_EXTENSIONS+=",metering" NETWORK_API_EXTENSIONS+=",multi-provider" diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index 8547f66e417..58181d41752 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -36,11 +36,13 @@ from neutron.objects import agent as agent_obj from neutron.objects import l3agent as rb_obj from neutron.objects import router as router_obj from neutron.tests.unit.db import test_db_base_plugin_v2 +from neutron.tests.unit.extensions import test_l3 _uuid = uuidutils.generate_uuid -class FakeL3Plugin(common_db_mixin.CommonDbMixin, +class FakeL3Plugin(test_l3.TestL3PluginBaseAttributes, + common_db_mixin.CommonDbMixin, l3_dvr_db.L3_NAT_with_dvr_db_mixin, l3_dvrscheduler_db.L3_DVRsch_db_mixin, agents_db.AgentDbMixin): @@ -663,6 +665,28 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): self.assertEqual(1, len(csnat_ports)) self.assertEqual(2, len(csnat_ports[0]['fixed_ips'])) + def _test_update_router_interface_port_ip_not_allowed(self, device_owner): + router, subnet_v4, subnet_v6 = self._setup_router_with_v4_and_v6() + device_filter = {'device_owner': [device_owner]} + ports = self.core_plugin.get_ports(self.ctx, filters=device_filter) + self.assertRaises( + exceptions.BadRequest, + self.core_plugin.update_port, + self.ctx, ports[0]['id'], + {'port': {'fixed_ips': [ + {'ip_address': "20.0.0.100", + 'subnet_id': subnet_v4['subnet']['id']}, + {'ip_address': "20.0.0.101", + 'subnet_id': subnet_v4['subnet']['id']}]}}) + + def test_update_router_centralized_snat_port_ip_not_allowed(self): + self._test_update_router_interface_port_ip_not_allowed( + const.DEVICE_OWNER_ROUTER_SNAT) + + def test_update_router_interface_distributed_port_ip_not_allowed(self): + self._test_update_router_interface_port_ip_not_allowed( + const.DEVICE_OWNER_DVR_INTERFACE) + def test_remove_router_interface_csnat_ports_removal(self): router_dict = {'name': 'test_router', 'admin_state_up': True, 'distributed': True} @@ -797,24 +821,6 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): self.ctx, filters=dvr_filters) self.assertEqual(1, len(dvr_ports)) - def test_remove_router_interface_csnat_port_missing_ip(self): - # NOTE(kevinbenton): this is a contrived scenario to reproduce - # a condition observed in bug/1609540. Once we figure out why - # these ports lose their IP we can remove this test. - router, subnet_v4, subnet_v6 = self._setup_router_with_v4_and_v6() - self.mixin.remove_router_interface( - self.ctx, router['id'], - {'subnet_id': subnet_v4['subnet']['id']}) - csnat_filters = {'device_owner': - [const.DEVICE_OWNER_ROUTER_SNAT]} - csnat_ports = self.core_plugin.get_ports( - self.ctx, filters=csnat_filters) - self.core_plugin.update_port(self.ctx, csnat_ports[0]['id'], - {'port': {'fixed_ips': []}}) - self.mixin.remove_router_interface( - self.ctx, router['id'], - {'subnet_id': subnet_v6['subnet']['id']}) - def test__validate_router_migration_notify_advanced_services(self): router = {'name': 'foo_router', 'admin_state_up': False} router_db = self._create_router(router) diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index db93e6862be..7661a9fbf49 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -48,12 +48,14 @@ from neutron.objects import l3_hamode from neutron.scheduler import l3_agent_scheduler from neutron.services.revisions import revision_plugin from neutron.tests.common import helpers +from neutron.tests.unit.extensions import test_l3 from neutron.tests.unit import testlib_api _uuid = uuidutils.generate_uuid -class FakeL3PluginWithAgents(common_db_mixin.CommonDbMixin, +class FakeL3PluginWithAgents(test_l3.TestL3PluginBaseAttributes, + common_db_mixin.CommonDbMixin, l3_hamode_db.L3_HA_NAT_db_mixin, l3_agentschedulers_db.L3AgentSchedulerDbMixin, agents_db.AgentDbMixin): @@ -637,6 +639,28 @@ class L3HATestCase(L3HATestFramework): networks_after = self.core_plugin.get_networks(self.admin_ctx) self.assertEqual(networks_before, networks_after) + def test_update_router_ha_interface_port_ip_not_allow(self): + router = self._create_router() + network = self.plugin.get_ha_network(self.admin_ctx, + router['tenant_id']) + self.plugin.add_ha_port( + self.admin_ctx, router['id'], network.network_id, + router['tenant_id']) + + device_filter = { + 'device_id': [router['id']], + 'device_owner': [constants.DEVICE_OWNER_ROUTER_HA_INTF]} + ports = self.core_plugin.get_ports( + self.admin_ctx, filters=device_filter) + + port = {"port": {"fixed_ips": [ + {"ip_address": "169.254.192.100"}, + {"ip_address": "169.254.192.200"}]}} + self.assertRaises(n_exc.BadRequest, + self.core_plugin.update_port, + self.admin_ctx, ports[0]['id'], + port) + def test_ensure_vr_id_and_network_net_exists(self): router = self._create_router() router_db = self.plugin._get_router(self.admin_ctx, router['id']) @@ -1086,6 +1110,26 @@ class L3HAModeDbTestCase(L3HATestFramework): self.admin_ctx, [router['id']]) self.assertEqual(2, len(bindings)) + def test_update_ha_router_replicated_interface_port_ip_not_allowed(self): + router = self._create_router() + network_id = self._create_network(self.core_plugin, self.admin_ctx) + subnet = self._create_subnet(self.core_plugin, self.admin_ctx, + network_id) + interface_info = {'subnet_id': subnet['id']} + self.plugin.add_router_interface(self.admin_ctx, + router['id'], + interface_info) + filters = {'device_id': [router['id']], + 'device_owner': [constants.DEVICE_OWNER_HA_REPLICATED_INT]} + ports = self.core_plugin.get_ports(self.admin_ctx, filters=filters) + port = {'port': {'fixed_ips': [ + {'ip_address': '10.0.0.100'}, + {'ip_address': '10.0.0.101'}]}} + self.assertRaises(n_exc.BadRequest, + self.core_plugin.update_port, + self.admin_ctx, ports[0]['id'], + port) + def test_update_router_port_bindings_no_ports(self): self.plugin._update_router_port_bindings( self.admin_ctx, {}, self.agent1['host']) diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 31c5e8a19b1..738a0cd41d0 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -237,8 +237,22 @@ class L3NatExtensionTestCase(test_extensions_base.ExtensionTestCase): instance.add_router_interface.assert_called_with(mock.ANY, router_id) +class TestL3PluginBaseAttributes(object): + + IP_UPDATE_NOT_ALLOWED_LIST = [ + lib_constants.DEVICE_OWNER_ROUTER_INTF, + lib_constants.DEVICE_OWNER_ROUTER_HA_INTF, + lib_constants.DEVICE_OWNER_HA_REPLICATED_INT, + lib_constants.DEVICE_OWNER_ROUTER_SNAT, + lib_constants.DEVICE_OWNER_DVR_INTERFACE] + + def router_supports_scheduling(self, context, router_id): + return True + + # This base plugin class is for tests. -class TestL3NatBasePlugin(db_base_plugin_v2.NeutronDbPluginV2, +class TestL3NatBasePlugin(TestL3PluginBaseAttributes, + db_base_plugin_v2.NeutronDbPluginV2, external_net_db.External_net_db_mixin): __native_pagination_support = True @@ -319,7 +333,8 @@ class TestNoL3NatPlugin(TestL3NatBasePlugin): # A L3 routing service plugin class for tests with plugins that # delegate away L3 routing functionality -class TestL3NatServicePlugin(common_db_mixin.CommonDbMixin, +class TestL3NatServicePlugin(TestL3PluginBaseAttributes, + common_db_mixin.CommonDbMixin, l3_dvr_db.L3_NAT_with_dvr_db_mixin, l3_db.L3_NAT_db_mixin, dns_db.DNSDbMixin): @@ -1402,6 +1417,30 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): None, p['port']['id']) + def test_update_router_interface_port_ip_not_allowed(self): + with self.router() as r, ( + self.port()) as p: + body = self._router_interface_action('add', + r['router']['id'], + None, + p['port']['id']) + self.assertIn('port_id', body) + self.assertEqual(p['port']['id'], body['port_id']) + body = self._show('ports', p['port']['id']) + self.assertEqual(r['router']['id'], body['port']['device_id']) + + data = {'port': {'fixed_ips': [ + {'ip_address': '1.1.1.1'}, + {'ip_address': '2.2.2.2'}]}} + self._update('ports', p['port']['id'], data, + neutron_context=context.get_admin_context(), + expected_code=exc.HTTPBadRequest.code) + + self._router_interface_action('remove', + r['router']['id'], + None, + p['port']['id']) + def test_router_add_interface_delete_port_after_failure(self): with self.router() as r, self.subnet(enable_dhcp=False) as s: plugin = directory.get_plugin() @@ -3739,26 +3778,44 @@ class L3AgentDbTestCaseBase(L3NatTestCaseMixin): self.assertEqual(0, len(ifaces)) def test_l3_agent_routers_query_ignore_interfaces_with_moreThanOneIp(self): - with self.router() as r: - with self.subnet(cidr='9.0.1.0/24') as subnet: - with self.port(subnet=subnet, - fixed_ips=[{'ip_address': '9.0.1.3'}]) as p: - self._router_interface_action('add', - r['router']['id'], - None, - p['port']['id']) - port = {'port': {'fixed_ips': - [{'ip_address': '9.0.1.4', - 'subnet_id': subnet['subnet']['id']}, - {'ip_address': '9.0.1.5', - 'subnet_id': subnet['subnet']['id']}]}} - ctx = context.get_admin_context() - self.core_plugin.update_port(ctx, p['port']['id'], port) - routers = self.plugin.get_sync_data(ctx, None) - self.assertEqual(1, len(routers)) - interfaces = routers[0].get(lib_constants.INTERFACE_KEY, - []) - self.assertEqual(1, len(interfaces)) + with self.router() as r, self.subnet( + cidr='9.0.1.0/24') as subnet, self.port( + subnet=subnet, + fixed_ips=[{'ip_address': '9.0.1.3'}]) as p1, self.port( + subnet=subnet, + fixed_ips=[{'ip_address': '9.0.1.100'}, + {'ip_address': '9.0.1.101'}]) as p2: + # Cannot have multiple IPv4 subnets on router port, + # see neutron.db.l3_db line L752-L754. + self._router_interface_action( + 'add', r['router']['id'], + None, p2['port']['id'], + expected_code=exc.HTTPBadRequest.code) + + self._router_interface_action('add', + r['router']['id'], + None, + p1['port']['id']) + port = {'port': {'fixed_ips': + [{'ip_address': '9.0.1.4', + 'subnet_id': subnet['subnet']['id']}, + {'ip_address': '9.0.1.5', + 'subnet_id': subnet['subnet']['id']}]}} + ctx = context.get_admin_context() + self.assertRaises( + n_exc.BadRequest, + self.core_plugin.update_port, + ctx, p1['port']['id'], port) + + routers = self.plugin.get_sync_data(ctx, None) + self.assertEqual(1, len(routers)) + interfaces = routers[0].get(lib_constants.INTERFACE_KEY, + []) + self.assertEqual(1, len(interfaces)) + self._router_interface_action('remove', + r['router']['id'], + None, + p1['port']['id']) def test_l3_agent_routers_query_gateway(self): with self.router() as r: