diff --git a/neutron/db/allowedaddresspairs_db.py b/neutron/db/allowedaddresspairs_db.py index a4fff46d3bd..0cdcf20fceb 100644 --- a/neutron/db/allowedaddresspairs_db.py +++ b/neutron/db/allowedaddresspairs_db.py @@ -17,6 +17,8 @@ import collections from neutron_lib.api.definitions import allowedaddresspairs as addr_apidef from neutron_lib.api.definitions import port as port_def from neutron_lib.api import validators +from neutron_lib.callbacks import events +from neutron_lib.callbacks import registry from neutron_lib.db import api as db_api from neutron_lib.db import resource_extend from neutron_lib.db import utils as db_utils @@ -36,6 +38,20 @@ class AllowedAddressPairsMixin: allowed_address_pairs): if not validators.is_attr_set(allowed_address_pairs): return [] + + desired_state = { + 'context': context, + 'network_id': port['network_id'], + 'allowed_address_pairs': allowed_address_pairs, + } + # TODO(slaweq): use constant from neutron_lib.callbacks.resources once + # it will be available and released + registry.publish( + 'allowed_address_pair', events.BEFORE_CREATE, self, + payload=events.DBEventPayload( + context, + resource_id=port['id'], + desired_state=desired_state)) try: with db_api.CONTEXT_WRITER.using(context): for address_pair in allowed_address_pairs: diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index b74d1d002fd..38fd65fc3d2 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -18,6 +18,7 @@ import copy import datetime import functools import multiprocessing +import netaddr import operator import threading import types @@ -304,6 +305,11 @@ class OVNMechanismDriver(api.MechanismDriver): registry.subscribe(self.delete_segment_provnet_port, resources.SEGMENT, events.AFTER_DELETE) + # TODO(slaweq): use constant from neutron_lib.callbacks.resources once + # it will be available and released + registry.subscribe(self._validate_allowed_address_pairs, + 'allowed_address_pair', + events.BEFORE_CREATE) # Handle security group/rule or address group notifications if self.sg_enabled: @@ -626,6 +632,49 @@ class OVNMechanismDriver(api.MechanismDriver): ) raise n_exc.InvalidInput(error_message=m) + def _validate_allowed_address_pairs(self, resource, event, trigger, + payload): + context = payload.desired_state['context'] + allowed_address_pairs = payload.desired_state['allowed_address_pairs'] + network_id = payload.desired_state['network_id'] + if not allowed_address_pairs: + return + + port_allowed_address_pairs_ip_addresses = [ + netaddr.IPNetwork(pair['ip_address']) + for pair in allowed_address_pairs] + + distributed_ports = self._plugin.get_ports( + context.elevated(), + filters={'device_owner': [const.DEVICE_OWNER_DISTRIBUTED], + 'network_id': [network_id]}) + if not distributed_ports: + return + + def _get_common_ips(ip_addresses, ip_networks): + common_ips = set() + for ip_address in ip_addresses: + if any(ip_address in ip_net for ip_net in ip_networks): + common_ips.add(str(ip_address)) + return common_ips + + for distributed_port in distributed_ports: + distributed_port_ip_addresses = [ + netaddr.IPAddress(fixed_ip['ip_address']) for fixed_ip in + distributed_port.get('fixed_ips', [])] + + common_ips = _get_common_ips( + distributed_port_ip_addresses, + port_allowed_address_pairs_ip_addresses) + + if common_ips: + err_msg = ( + _("IP addresses '%s' already used by the '%s' port(s) in " + "the same network" % (";".join(common_ips), + const.DEVICE_OWNER_DISTRIBUTED)) + ) + raise n_exc.InvalidInput(error_message=err_msg) + def create_segment_provnet_port(self, resource, event, trigger, payload=None): segment = payload.latest_state diff --git a/neutron/tests/common/test_db_base_plugin_v2.py b/neutron/tests/common/test_db_base_plugin_v2.py index 3db2c567c2b..49e2ea4dc95 100644 --- a/neutron/tests/common/test_db_base_plugin_v2.py +++ b/neutron/tests/common/test_db_base_plugin_v2.py @@ -660,7 +660,7 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): as_admin=False, **kwargs): res = self._create_port(fmt, net_id, expected_res_status, is_admin=as_admin, **kwargs) - self._check_http_response(res) + self._check_http_response(res, expected_res_status) return self.deserialize(fmt, res) def _make_security_group(self, fmt, name=None, expected_res_status=None, diff --git a/neutron/tests/unit/db/test_l3_db.py b/neutron/tests/unit/db/test_l3_db.py index f159512a4f4..de57922d6a3 100644 --- a/neutron/tests/unit/db/test_l3_db.py +++ b/neutron/tests/unit/db/test_l3_db.py @@ -1081,6 +1081,11 @@ class L3TestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): self.mixin, payload=mock.ANY), mock.call(resources.PORT, events.BEFORE_CREATE, mock.ANY, payload=mock.ANY), + # TODO(slaweq): use constant from + # neutron_lib.callbacks.resources once it will be available + # and released + mock.call('allowed_address_pair', events.BEFORE_CREATE, + mock.ANY, payload=mock.ANY), mock.call(resources.PORT, events.PRECOMMIT_CREATE, mock.ANY, payload=mock.ANY), mock.call(resources.PORT, events.AFTER_CREATE, diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index cf201f0fcec..4e1764874cb 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -3185,6 +3185,47 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): def test_node_uuid_no_worker_id(self, *args): self.assertEqual(123456789, self.mech_driver.node_uuid) + def test_create_port_with_allowed_address_pairs(self): + with self.network() as network: + with self.subnet(network, cidr='10.0.0.0/24'): + self._make_port( + self.fmt, network['network']['id'], + device_owner=const.DEVICE_OWNER_DISTRIBUTED, + fixed_ips=[{'ip_address': '10.0.0.2'}], + as_admin=True, + arg_list=('device_owner', 'fixed_ips')) + port1 = self._make_port( + self.fmt, network['network']['id'], + allowed_address_pairs=[{'ip_address': '10.0.0.3'}], + as_admin=True, + arg_list=('allowed_address_pairs',))['port'] + self.assertEqual( + [{'ip_address': '10.0.0.3', + 'mac_address': port1['mac_address']}], + port1['allowed_address_pairs']) + self._make_port( + self.fmt, network['network']['id'], + allowed_address_pairs=[{'ip_address': '10.0.0.2'}], + expected_res_status=exc.HTTPBadRequest.code, + arg_list=('allowed_address_pairs',)) + port2 = self._show('ports', port1['id'])['port'] + self.assertEqual( + [{'ip_address': '10.0.0.3', + 'mac_address': port2['mac_address']}], + port2['allowed_address_pairs']) + + # Now test the same but giving a subnet as allowed address pair + self._make_port( + self.fmt, network['network']['id'], + allowed_address_pairs=[{'ip_address': '10.0.0.2/26'}], + expected_res_status=exc.HTTPBadRequest.code, + arg_list=('allowed_address_pairs',)) + port3 = self._show('ports', port1['id'])['port'] + self.assertEqual( + [{'ip_address': '10.0.0.3', + 'mac_address': port3['mac_address']}], + port3['allowed_address_pairs']) + class OVNMechanismDriverTestCase(MechDriverSetupBase, test_plugin.Ml2PluginV2TestCase): diff --git a/releasenotes/notes/block-metadata-port-IP-address-to-be-used-as-virtual-ip-by-ovn-driver-0d46fed7652fea7a.yaml b/releasenotes/notes/block-metadata-port-IP-address-to-be-used-as-virtual-ip-by-ovn-driver-0d46fed7652fea7a.yaml new file mode 100644 index 00000000000..500714a6be1 --- /dev/null +++ b/releasenotes/notes/block-metadata-port-IP-address-to-be-used-as-virtual-ip-by-ovn-driver-0d46fed7652fea7a.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + When ML2/OVN backend is used, usage of the metadata port IP address as a + virtual IP address is blocked. That means that setting such IP address as + allowed_address_pair for other port is not allowed and API will return 400 + error in such case. For more information, see bug + `2116249 `_.