diff --git a/neutron/db/allowedaddresspairs_db.py b/neutron/db/allowedaddresspairs_db.py index 34e17764ed3..fe6ced0dae9 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(object): 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 99c9ebf8d36..13663ca8644 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -23,6 +23,7 @@ import threading import types import uuid +import netaddr from neutron_lib.api.definitions import portbindings from neutron_lib.api.definitions import provider_net from neutron_lib.api.definitions import segment as segment_def @@ -257,6 +258,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: @@ -569,6 +575,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/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 3debde98d16..ad2d44db4d6 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -657,7 +657,10 @@ 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) + if not expected_res_status: + self._check_http_response(res) + else: + self.assertEqual(expected_res_status, res.status_int) 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 5a74f3b6a65..7d4f1624008 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 979e21d0c7b..d6d4054f791 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 @@ -3140,6 +3140,47 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): tag=new_vlan_tag, if_exists=True) self.nb_ovn.set_lswitch_port.assert_has_calls([expected_call]) + 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 `_.