From f3b7d64eab3eb2c4b29ff38afad0f8b2acc92313 Mon Sep 17 00:00:00 2001 From: kyu0 Date: Thu, 14 May 2026 14:30:03 +0900 Subject: [PATCH 01/13] Fix VLAN network detection to use VlanTypeDriver ranges The _is_vlan_project_network method in DNSExtensionDriverML2 was only checking the static network_vlan_ranges configuration from ml2_conf.ini, ignoring the dynamic ranges provided by the network-segment-range service plugin. This caused a KeyError when creating subnets or VMs on VLAN networks with segment ranges managed via the API. This fix updates the method to use VlanTypeDriver.get_network_segment_ranges(), which properly handles both static configuration and DB-based dynamic ranges when the network-segment-range plugin is enabled. Closes-Bug: #2140291 Signed-off-by: Kyuyeong Lee Change-Id: I8f3a2b1c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a (cherry picked from commit cd8f88e3ec1001958c18147ed312bc61021414ac) --- .../plugins/ml2/extensions/dns_integration.py | 27 +++-- .../ml2/extensions/test_dns_integration.py | 105 ++++++++++++++++++ 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/neutron/plugins/ml2/extensions/dns_integration.py b/neutron/plugins/ml2/extensions/dns_integration.py index fab682a0a6f..aded3936ff7 100644 --- a/neutron/plugins/ml2/extensions/dns_integration.py +++ b/neutron/plugins/ml2/extensions/dns_integration.py @@ -23,7 +23,6 @@ from neutron_lib.exceptions import dns as dns_exc from neutron_lib.plugins import directory from neutron_lib.plugins.ml2 import api -from neutron_lib.plugins import utils as plugin_utils from oslo_config import cfg from oslo_log import log as logging @@ -349,9 +348,23 @@ def _get_details(self, context, network_id): class DNSExtensionDriverML2(DNSExtensionDriver): + def __init__(self): + super().__init__() + self._vlan_driver = None + self._plugin = None + def initialize(self): LOG.info("DNSExtensionDriverML2 initialization complete") + @property + def vlan_driver(self): + if not self._vlan_driver: + if not self._plugin: + self._plugin = directory.get_plugin() + self._vlan_driver = self._plugin.type_manager.drivers.get( + lib_const.TYPE_VLAN) + return self._vlan_driver + def _is_tunnel_project_network(self, provider_net): if provider_net['network_type'] == lib_const.TYPE_GENEVE: tunnel_ranges = cfg.CONF.ml2_type_geneve.vni_ranges @@ -369,15 +382,15 @@ def _is_tunnel_project_network(self, provider_net): return int(tun_min) <= segmentation_id <= int(tun_max) def _is_vlan_project_network(self, provider_net): - network_vlan_ranges = plugin_utils.parse_network_vlan_ranges( - cfg.CONF.ml2_type_vlan.network_vlan_ranges) - vlan_ranges = network_vlan_ranges[provider_net['physical_network']] + if not self.vlan_driver: + return False + network_vlan_ranges = self.vlan_driver.obj.get_network_segment_ranges() + vlan_ranges = network_vlan_ranges.get(provider_net['physical_network']) if not vlan_ranges: return False segmentation_id = int(provider_net['segmentation_id']) - for vlan_range in vlan_ranges: - if vlan_range[0] <= segmentation_id <= vlan_range[1]: - return True + return any(vlan_range[0] <= segmentation_id <= vlan_range[1] + for vlan_range in vlan_ranges) def external_dns_not_needed(self, context, network, subnets): dns_driver = _get_dns_driver() diff --git a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py index c7c349fab56..777e3af7801 100644 --- a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py +++ b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py @@ -760,6 +760,111 @@ def test_update_fixed_ips_no_effect_after_clearing_dns_domain(self, dns_data_db, dns_data_db_1, dns_data_db_2) +class DNSExtensionDriverML2TestCase(testtools.TestCase): + + def setUp(self): + super().setUp() + self.driver = dns_integration.DNSExtensionDriverML2() + self.mock_vlan_driver = mock.Mock() + self.mock_vlan_driver_property = mock.patch.object( + type(self.driver), 'vlan_driver', + new_callable=mock.PropertyMock, + return_value=self.mock_vlan_driver).start() + + def test__is_vlan_project_network_with_multiple_ranges(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = { + 'physnet1': [(100, 200), (300, 400)], + 'physnet2': [(500, 600)] + } + + provider_net_1 = { + 'physical_network': 'physnet1', + 'segmentation_id': 150 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_1)) + + provider_net_2 = { + 'physical_network': 'physnet1', + 'segmentation_id': 250 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_2)) + + provider_net_3 = { + 'physical_network': 'physnet1', + 'segmentation_id': 350 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_3)) + + provider_net_4 = { + 'physical_network': 'physnet2', + 'segmentation_id': 550 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_4)) + + provider_net_5 = { + 'physical_network': 'physnet2', + 'segmentation_id': 650 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_5)) + + def test__is_vlan_project_network_boundary_values(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = { + 'physnet1': [(100, 200)] + } + + provider_net_1 = { + 'physical_network': 'physnet1', + 'segmentation_id': 100 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_1)) + + provider_net_2 = { + 'physical_network': 'physnet1', + 'segmentation_id': 200 + } + self.assertTrue(self.driver._is_vlan_project_network(provider_net_2)) + + provider_net_3 = { + 'physical_network': 'physnet1', + 'segmentation_id': 99 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_3)) + + provider_net_4 = { + 'physical_network': 'physnet1', + 'segmentation_id': 201 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net_4)) + + def test__is_vlan_project_network_physnet_not_found(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = { + 'physnet1': [(100, 200)] + } + + provider_net = { + 'physical_network': 'physnet2', + 'segmentation_id': 150 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net)) + + def test__is_vlan_project_network_empty_ranges(self): + self.mock_vlan_driver.obj.get_network_segment_ranges.return_value = {} + + provider_net = { + 'physical_network': 'physnet1', + 'segmentation_id': 100 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net)) + + def test__is_vlan_project_network_no_vlan_driver(self): + self.mock_vlan_driver_property.return_value = None + provider_net = { + 'physical_network': 'physnet1', + 'segmentation_id': 100 + } + self.assertFalse(self.driver._is_vlan_project_network(provider_net)) + + class TestDesignateClientKeystoneV3(testtools.TestCase): """Test case for designate clients """ From d540e9df0b3d26880bd6c5c9e2f55d8bc8b1cbe8 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Thu, 21 May 2026 17:50:47 -0400 Subject: [PATCH 02/13] Fix PF GET/PUT parent floating IP validation Verify the port-forwarding floating IP id matches the child floating IP id so we satisfy the policy requirements for GET and PUT operations. Closes-bug: #2150121 Change-Id: Ie68e57da6222965e79d015a5568190d8bc29b9e8 Signed-off-by: Brian Haley Assisted-by: Claude Sonnet 4.6 --- neutron/services/portforwarding/pf_plugin.py | 4 +-- .../services/portforwarding/test_pf_plugin.py | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/neutron/services/portforwarding/pf_plugin.py b/neutron/services/portforwarding/pf_plugin.py index 16506b43ab5..8519801b0cb 100644 --- a/neutron/services/portforwarding/pf_plugin.py +++ b/neutron/services/portforwarding/pf_plugin.py @@ -416,7 +416,7 @@ def update_floatingip_port_forwarding(self, context, id, floatingip_id, with db_api.CONTEXT_WRITER.using(context): fip_obj = self._get_fip_obj(context, floatingip_id) pf_obj = pf.PortForwarding.get_object(context, id=id) - if not pf_obj: + if not pf_obj or pf_obj.floatingip_id != floatingip_id: raise pf_exc.PortForwardingNotFound(id=id) original_pf_obj = copy.deepcopy(pf_obj) ori_internal_port_id = pf_obj.internal_port_id @@ -670,7 +670,7 @@ def get_floatingip_port_forwarding(self, context, id, floatingip_id, fields=None): self._get_fip_obj(context, floatingip_id) obj = pf.PortForwarding.get_object(context, id=id) - if not obj: + if not obj or obj.floatingip_id != floatingip_id: raise pf_exc.PortForwardingNotFound(id=id) return obj diff --git a/neutron/tests/unit/services/portforwarding/test_pf_plugin.py b/neutron/tests/unit/services/portforwarding/test_pf_plugin.py index 1d73650da09..298e134e122 100644 --- a/neutron/tests/unit/services/portforwarding/test_pf_plugin.py +++ b/neutron/tests/unit/services/portforwarding/test_pf_plugin.py @@ -90,10 +90,20 @@ def setUp(self): @mock.patch.object(port_forwarding.PortForwarding, 'get_object') def test_get_floatingip_port_forwarding(self, get_object_mock): + get_object_mock.return_value = mock.Mock(floatingip_id='test-fip-id') self.pf_plugin.get_floatingip_port_forwarding( self.ctxt, 'pf_id', 'test-fip-id', fields=None) get_object_mock.assert_called_once_with(self.ctxt, id='pf_id') + @mock.patch.object(port_forwarding.PortForwarding, 'get_object') + def test_negative_get_floatingip_port_forwarding_wrong_parent( + self, get_object_mock): + get_object_mock.return_value = mock.Mock(floatingip_id='other-fip-id') + self.assertRaises( + pf_exc.PortForwardingNotFound, + self.pf_plugin.get_floatingip_port_forwarding, + self.ctxt, 'pf_id', 'test-fip-id', fields=None) + @mock.patch.object(port_forwarding.PortForwarding, 'get_object', return_value=None) def test_negative_get_floatingip_port_forwarding(self, get_object_mock): @@ -186,6 +196,7 @@ def test_update_floatingip_port_forwarding( pf_obj.internal_ip_address = "10.0.0.1" pf_obj.internal_port = 22 pf_obj.external_port = 222 + pf_obj.floatingip_id = 'fip_id' mock_pf_get_object.return_value = pf_obj port_dict = {'id': 'ID', 'fixed_ips': [{"subnet_id": "test-subnet-id", "ip_address": "10.0.0.1"}]} @@ -293,6 +304,22 @@ def test_negative_update_floatingip_port_forwarding( self.pf_plugin.update_floatingip_port_forwarding, self.ctxt, 'pf_id', **pf_input) + @mock.patch.object(port_forwarding.PortForwarding, 'get_object') + def test_negative_update_floatingip_port_forwarding_wrong_parent( + self, mock_pf_get_object): + pf_input = { + 'port_forwarding': + {'port_forwarding': { + 'internal_ip_address': '1.1.1.1', + 'floatingip_id': 'fip_id'}}, + 'floatingip_id': 'fip_id'} + mock_pf_get_object.return_value = mock.Mock( + floatingip_id='other-fip-id') + self.assertRaises( + pf_exc.PortForwardingNotFound, + self.pf_plugin.update_floatingip_port_forwarding, + self.ctxt, 'pf_id', **pf_input) + @mock.patch.object(pf_plugin.PortForwardingPlugin, '_check_port_has_binding_floating_ip') @mock.patch.object(obj_base.NeutronDbObject, 'update_objects') From 8ab37df7041ab35ddf0d25d4f441b512e7770a87 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 27 May 2026 12:33:51 +0200 Subject: [PATCH 03/13] Fix port RBAC policies to require network ownership Several default port policies that require network ownership incorrectly included PROJECT_MANAGER. That rule checks the port project_id, not network ownership, so any project manager could perform those actions on shared/RBAC networks where they do not own the network. Remove PROJECT_MANAGER from the affected create/update port policies and rely on NET_OWNER_MEMBER or ADMIN_OR_NET_OWNER_MEMBER instead. Project managers who own the network remain authorized through the default Keystone role implication chain (manager implies member). Closes-Bug: #2152115 Assisted-By: Claude Composer 2.5 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I4e258d28cdf72adcc13fc9d03749256c65881c45 (cherry picked from commit d0f61db2e500ce01141dd8e796d2c1eb5da31876) --- neutron/conf/policies/base.py | 1 + neutron/conf/policies/port.py | 19 +- neutron/tests/unit/conf/policies/test_port.py | 569 ++++-------------- ...etwork-owner-manager-a4b8c2d1e3f56789.yaml | 31 + 4 files changed, 137 insertions(+), 483 deletions(-) create mode 100644 releasenotes/notes/fix-port-device-owner-rbac-network-owner-manager-a4b8c2d1e3f56789.yaml diff --git a/neutron/conf/policies/base.py b/neutron/conf/policies/base.py index 9573eb9bfb2..63765c80154 100644 --- a/neutron/conf/policies/base.py +++ b/neutron/conf/policies/base.py @@ -76,6 +76,7 @@ # related to the "network owner" and network isn't really parent of the subnet # or port. Because of that, using parent owner in those cases may be # missleading for users so it's better to keep also "network owner" rules. +NET_OWNER_MANAGER = 'role:manager and ' + RULE_NET_OWNER NET_OWNER_MEMBER = 'role:member and ' + RULE_NET_OWNER NET_OWNER_READER = 'role:reader and ' + RULE_NET_OWNER ADMIN_OR_NET_OWNER_MEMBER = ( diff --git a/neutron/conf/policies/port.py b/neutron/conf/policies/port.py index 2c2e7cb181a..daeda35341c 100644 --- a/neutron/conf/policies/port.py +++ b/neutron/conf/policies/port.py @@ -100,7 +100,6 @@ check_str=neutron_policy.policy_or( 'not rule:network_device', base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -119,7 +118,6 @@ name='create_port:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER), scope_types=['project'], description='Specify ``mac_address`` attribute when creating a port', @@ -136,7 +134,6 @@ name='create_port:fixed_ips', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, 'rule:shared'), scope_types=['project'], @@ -155,7 +152,6 @@ name='create_port:fixed_ips:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER), scope_types=['project'], description='Specify IP address in ``fixed_ips`` when creating a port', @@ -172,7 +168,6 @@ name='create_port:fixed_ips:subnet_id', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, 'rule:shared'), scope_types=['project'], @@ -191,7 +186,6 @@ name='create_port:port_security_enabled', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER), scope_types=['project'], description=( @@ -258,7 +252,6 @@ name='create_port:allowed_address_pairs', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -276,7 +269,6 @@ name='create_port:allowed_address_pairs:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -294,7 +286,6 @@ name='create_port:allowed_address_pairs:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -496,7 +487,6 @@ check_str=neutron_policy.policy_or( 'not rule:network_device', base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, ), scope_types=['project'], @@ -515,7 +505,7 @@ name='update_port:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER + base.NET_OWNER_MANAGER, ), scope_types=['project'], description='Update ``mac_address`` attribute of a port', @@ -532,7 +522,6 @@ name='update_port:fixed_ips', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -550,7 +539,6 @@ name='update_port:fixed_ips:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -571,7 +559,6 @@ name='update_port:fixed_ips:subnet_id', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER, 'rule:shared' ), @@ -594,7 +581,6 @@ name='update_port:port_security_enabled', check_str=neutron_policy.policy_or( base.ADMIN_OR_SERVICE, - base.PROJECT_MANAGER, base.NET_OWNER_MEMBER ), scope_types=['project'], @@ -653,7 +639,6 @@ name='update_port:allowed_address_pairs', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description='Update ``allowed_address_pairs`` attribute of a port', @@ -668,7 +653,6 @@ name='update_port:allowed_address_pairs:mac_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( @@ -686,7 +670,6 @@ name='update_port:allowed_address_pairs:ip_address', check_str=neutron_policy.policy_or( base.ADMIN_OR_NET_OWNER_MEMBER, - base.PROJECT_MANAGER, base.SERVICE), scope_types=['project'], description=( diff --git a/neutron/tests/unit/conf/policies/test_port.py b/neutron/tests/unit/conf/policies/test_port.py index f078c405da2..9c27736f9d9 100644 --- a/neutron/tests/unit/conf/policies/test_port.py +++ b/neutron/tests/unit/conf/policies/test_port.py @@ -41,6 +41,12 @@ def setUp(self): 'project_id': self.alt_project_id, 'network_id': self.alt_network['id'], 'ext_parent_network_id': self.alt_network['id']} + # This port belongs to "project_id", but the network belongs to + # "alt_project_id". + self.target_net_alt_target = { + 'project_id': self.project_id, + 'network_id': self.alt_network['id'], + 'ext_parent_network_id': self.alt_network['id']} networks = { self.network['id']: self.network, @@ -56,6 +62,33 @@ def get_network(context, id, fields=None): 'neutron_lib.plugins.directory.get_plugin', return_value=self.plugin_mock).start() + def _assert_network_owner_policy(self, action, target=None, + alt_target=None, + target_net_alt_target=None): + target = target if target is not None else self.target + alt_target = alt_target if alt_target is not None else self.alt_target + if target_net_alt_target is None: + target_net_alt_target = self.target_net_alt_target + self.assertTrue(policy.enforce(self.context, action, target)) + self.assertRaises( + base_policy.PolicyNotAuthorized, + policy.enforce, self.context, action, alt_target) + self.assertRaises( + base_policy.PolicyNotAuthorized, + policy.enforce, self.context, action, target_net_alt_target) + + def _assert_denied_network_owner_policy(self, action, target=None, + alt_target=None, + target_net_alt_target=None): + target = target if target is not None else self.target + alt_target = alt_target if alt_target is not None else self.alt_target + if target_net_alt_target is None: + target_net_alt_target = self.target_net_alt_target + for test_target in (target, alt_target, target_net_alt_target): + self.assertRaises( + base_policy.PolicyNotAuthorized, + policy.enforce, self.context, action, test_target) + class SystemAdminTests(PortAPITestCase): @@ -868,57 +901,26 @@ def test_create_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'create_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_network_owner_policy( + 'create_port:device_owner', target, alt_target, + target_net_alt_target) def test_create_port_with_mac_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:mac_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.alt_target) + self._assert_network_owner_policy('create_port:mac_address') def test_create_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.alt_target) + self._assert_network_owner_policy('create_port:fixed_ips') def test_create_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:fixed_ips:ip_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.alt_target) + self._assert_network_owner_policy('create_port:fixed_ips:ip_address') def test_create_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:fixed_ips:subnet_id', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_network_owner_policy('create_port:fixed_ips:subnet_id') def test_create_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:port_security_enabled', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.alt_target) + self._assert_network_owner_policy('create_port:port_security_enabled') def test_create_port_with_binding_host_id(self): self.assertRaises( @@ -950,36 +952,15 @@ def test_create_port_with_binding_vnic_type(self): self.alt_target) def test_create_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:allowed_address_pairs', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.alt_target) + self._assert_network_owner_policy('create_port:allowed_address_pairs') def test_create_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_network_owner_policy( + 'create_port:allowed_address_pairs:mac_address') def test_create_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'create_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_network_owner_policy( + 'create_port:allowed_address_pairs:ip_address') def test_create_port_with_hints(self): self.assertRaises( @@ -1116,57 +1097,26 @@ def test_update_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'update_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_network_owner_policy( + 'update_port:device_owner', target, alt_target, + target_net_alt_target) def test_update_port_with_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:mac_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:mac_address', - self.alt_target) + self._assert_network_owner_policy('update_port:mac_address') def test_update_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips', - self.alt_target) + self._assert_network_owner_policy('update_port:fixed_ips') def test_update_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:fixed_ips:ip_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:ip_address', - self.alt_target) + self._assert_network_owner_policy('update_port:fixed_ips:ip_address') def test_update_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:fixed_ips:subnet_id', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_network_owner_policy('update_port:fixed_ips:subnet_id') def test_update_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:port_security_enabled', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.alt_target) + self._assert_network_owner_policy('update_port:port_security_enabled') def test_update_port_with_binding_host_id(self): self.assertRaises( @@ -1198,36 +1148,15 @@ def test_update_port_with_binding_vnic_type(self): self.alt_target) def test_update_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:allowed_address_pairs', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.alt_target) + self._assert_network_owner_policy('update_port:allowed_address_pairs') def test_update_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_network_owner_policy( + 'update_port:allowed_address_pairs:mac_address') def test_update_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, - 'update_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_network_owner_policy( + 'update_port:allowed_address_pairs:ip_address') def test_update_port_data_plane_status(self): self.assertRaises( @@ -1280,184 +1209,8 @@ def setUp(self): super().setUp() self.context = self.project_member_ctx - def test_create_port_with_device_owner(self): - target = self.target.copy() - target['device_owner'] = 'network:test' - alt_target = self.alt_target.copy() - alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'create_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - alt_target) - - def test_create_port_with_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:mac_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.alt_target) - - def test_create_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, 'create_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.alt_target) - - def test_create_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:fixed_ips:ip_address', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.alt_target) - - def test_create_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:fixed_ips:subnet_id', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.alt_target) - - def test_create_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:port_security_enabled', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.alt_target) - - def test_create_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:allowed_address_pairs', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.alt_target) - - def test_create_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.alt_target) - - def test_create_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce( - self.context, 'create_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.alt_target) - - def test_update_port_with_device_owner(self): - target = self.target.copy() - target['device_owner'] = 'network:test' - alt_target = self.alt_target.copy() - alt_target['device_owner'] = 'network:test' - self.assertTrue( - policy.enforce(self.context, 'update_port:device_owner', target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - alt_target) - def test_update_port_with_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:mac_address', - self.alt_target) - - def test_update_port_with_fixed_ips(self): - self.assertTrue( - policy.enforce(self.context, 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips', - self.alt_target) - - def test_update_port_with_fixed_ips_and_ip_address(self): - self.assertTrue( - policy.enforce(self.context, 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:ip_address', - self.alt_target) - - def test_update_port_with_fixed_ips_and_subnet_id(self): - self.assertTrue( - policy.enforce(self.context, 'update_port:fixed_ips', self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:fixed_ips:subnet_id', - self.alt_target) - - def test_update_port_with_port_security_enabled(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:port_security_enabled', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.alt_target) - - def test_update_port_with_allowed_address_pairs(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:allowed_address_pairs', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.alt_target) - - def test_update_port_with_allowed_address_pairs_and_mac_address(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:allowed_address_pairs:mac_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.alt_target) - - def test_update_port_with_allowed_address_pairs_and_ip_address(self): - self.assertTrue( - policy.enforce( - self.context, 'update_port:allowed_address_pairs:ip_address', - self.target)) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy('update_port:mac_address') class ProjectReaderTests(ProjectMemberTests): @@ -1479,24 +1232,14 @@ def test_create_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_denied_network_owner_policy( + 'create_port:device_owner', target, alt_target, + target_net_alt_target) def test_create_port_with_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:mac_address', - self.alt_target) + self._assert_denied_network_owner_policy('create_port:mac_address') def test_create_port_with_device_id(self): self.assertRaises( @@ -1509,80 +1252,31 @@ def test_create_port_with_device_id(self): self.alt_target) def test_create_port_with_fixed_ips(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips', - self.alt_target) + self._assert_denied_network_owner_policy('create_port:fixed_ips') def test_create_port_with_fixed_ips_and_subnet_id(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:fixed_ips:subnet_id') def test_create_port_with_port_security_enabled(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:port_security_enabled', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:port_security_enabled') def test_create_port_with_allowed_address_pairs(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:allowed_address_pairs') def test_create_port_with_allowed_address_pairs_and_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:allowed_address_pairs:mac_address') def test_create_port_with_allowed_address_pairs_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'create_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:allowed_address_pairs:ip_address') def test_create_port_with_fixed_ips_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'create_port:fixed_ips:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'create_port:fixed_ips:ip_address') def test_create_port_with_binding_vnic_type(self): self.assertRaises( @@ -1615,14 +1309,14 @@ def test_update_port_with_device_owner(self): target['device_owner'] = 'network:test' alt_target = self.alt_target.copy() alt_target['device_owner'] = 'network:test' - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:device_owner', - alt_target) + target_net_alt_target = self.target_net_alt_target.copy() + target_net_alt_target['device_owner'] = 'network:test' + self._assert_denied_network_owner_policy( + 'update_port:device_owner', target, alt_target, + target_net_alt_target) + + def test_update_port_with_mac_address(self): + self._assert_denied_network_owner_policy('update_port:mac_address') def test_update_port_with_device_id(self): self.assertRaises( @@ -1635,86 +1329,31 @@ def test_update_port_with_device_id(self): self.alt_target) def test_update_port_with_port_security_enabled(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, self.context, 'update_port:port_security_enabled', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:port_security_enabled') def test_update_port_with_allowed_address_pairs(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:allowed_address_pairs') def test_update_port_with_allowed_address_pairs_and_mac_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:mac_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:allowed_address_pairs:mac_address') def test_update_port_with_allowed_address_pairs_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:allowed_address_pairs:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:allowed_address_pairs:ip_address') def test_update_port_with_fixed_ips(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips', - self.alt_target) + self._assert_denied_network_owner_policy('update_port:fixed_ips') def test_update_port_with_fixed_ips_and_ip_address(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:ip_address', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:ip_address', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:fixed_ips:ip_address') def test_update_port_with_fixed_ips_and_subnet_id(self): - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:subnet_id', - self.target) - self.assertRaises( - base_policy.PolicyNotAuthorized, - policy.enforce, - self.context, 'update_port:fixed_ips:subnet_id', - self.alt_target) + self._assert_denied_network_owner_policy( + 'update_port:fixed_ips:subnet_id') def test_update_port_with_binding_vnic_type(self): self.assertRaises( diff --git a/releasenotes/notes/fix-port-device-owner-rbac-network-owner-manager-a4b8c2d1e3f56789.yaml b/releasenotes/notes/fix-port-device-owner-rbac-network-owner-manager-a4b8c2d1e3f56789.yaml new file mode 100644 index 00000000000..356a4f8a92b --- /dev/null +++ b/releasenotes/notes/fix-port-device-owner-rbac-network-owner-manager-a4b8c2d1e3f56789.yaml @@ -0,0 +1,31 @@ +--- +security: + - | + Fixed overly permissive default RBAC policies for several port actions + that require network ownership. Those policies previously included + ``PROJECT_MANAGER``, which allowed any project manager to perform the + action even when not owning the network (for example, on shared/RBAC + networks). They now rely on ``NET_OWNER_MEMBER`` or + ``ADMIN_OR_NET_OWNER_MEMBER`` only. Project managers who own the + network remain authorized through the default Keystone role implication + chain (``manager`` implies ``member``). + + Affected actions: ``create_port:device_owner``, + ``update_port:device_owner``, ``create_port:mac_address``, + ``update_port:mac_address``, ``create_port:fixed_ips``, + ``create_port:fixed_ips:ip_address``, ``create_port:fixed_ips:subnet_id``, + ``update_port:fixed_ips``, ``update_port:fixed_ips:ip_address``, + ``update_port:fixed_ips:subnet_id``, ``create_port:port_security_enabled``, + ``update_port:port_security_enabled``, + ``create_port:allowed_address_pairs``, + ``create_port:allowed_address_pairs:mac_address``, + ``create_port:allowed_address_pairs:ip_address``, + ``update_port:allowed_address_pairs``, + ``update_port:allowed_address_pairs:mac_address``, and + ``update_port:allowed_address_pairs:ip_address``. +upgrade: + - | + Default RBAC policies for the port actions listed above no longer + include ``PROJECT_MANAGER``. Deployments with customized policy files + should review and update those rules if they rely on the previous + behaviour. From 1c9bad3b6d55dc33f5656f858a53ce831ae3f55e Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 26 May 2026 15:07:41 +0200 Subject: [PATCH 04/13] Skip OVNL3 post-fork init in non-API workers The ``OVNL3RouterPlugin`` registers OVN OVSDB monitor events during ``_post_fork_initialize``. That callback is also invoked from neutron-periodic-workers and other non-API processes, where the OVN IDL connections are not initialized. Checking OVN readiness before filtering by worker type raised ``MechanismDriverOVNNotReady`` and blocked worker startup. Return early when the trigger is not a Neutron API ``WorkerService``, and only then validate OVN readiness and register monitor events. Conflicts: neutron/services/ovn_l3/plugin.py Related-Bug: #2154192 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I9e43fdb06ce7868e5fa699657d8da9a4aa4d02fb (cherry picked from commit b6d260e10c88f524523e3663e367f6564d8f6911) --- neutron/services/ovn_l3/plugin.py | 18 ++++++++++-------- ...-skip-non-api-workers-b2c3d4e5f6a78901.yaml | 8 ++++++++ 2 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/ovnl3-post-fork-init-skip-non-api-workers-b2c3d4e5f6a78901.yaml diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 74ac89b1f7c..5242489e0bc 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -183,18 +183,20 @@ def subscribe(self): cancellable=True) def _post_fork_initialize(self, resource, event, trigger, payload=None): + # TODO(ralonsoh): once [1] is released and required in Neutron, it + # won't be needed the ``get_method_class`` method. + # [1] https://review.opendev.org/c/openstack/neutron-lib/+/988563 + if utils.get_method_class(trigger) != wsgi.WorkerService: + return + if not self._nb_ovn or not self._sb_ovn: raise ovn_l3_exc.MechanismDriverOVNNotReady() # Register needed events, only for the Neutron API workers. - # TODO(ralonsoh): once [1] is released and required in Neutron, it - # won't be needed the ``get_method_class`` method. - # [1] https://review.opendev.org/c/openstack/neutron-lib/+/988563 - if utils.get_method_class(trigger) == wsgi.WorkerService: - self._nb_ovn.idl.notify_handler.watch_events([ - ovsdb_monitor.LogicalRouterPortEvent(self), - ovsdb_monitor.LogicalRouterPortGatewayChassisEvent(self), - ]) + self._nb_ovn.idl.notify_handler.watch_events([ + ovsdb_monitor.LogicalRouterPortEvent(self), + ovsdb_monitor.LogicalRouterPortGatewayChassisEvent(self), + ]) def _add_neutron_router_interface(self, context, router_id, interface_info): diff --git a/releasenotes/notes/ovnl3-post-fork-init-skip-non-api-workers-b2c3d4e5f6a78901.yaml b/releasenotes/notes/ovnl3-post-fork-init-skip-non-api-workers-b2c3d4e5f6a78901.yaml new file mode 100644 index 00000000000..3b0f3dadcf1 --- /dev/null +++ b/releasenotes/notes/ovnl3-post-fork-init-skip-non-api-workers-b2c3d4e5f6a78901.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + The ``OVNL3RouterPlugin._post_fork_initialize`` method now returns early + when the worker is not a Neutron API ``WorkerService`` instance. This + prevents ``MechanismDriverOVNNotReady`` errors in the + ``neutron-periodic-workers`` and other non-API processes, where the OVN + IDL connections are not initialized. From 1941988ae24fd36f69311d8de0fba1413b9e17c8 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 28 May 2026 13:39:37 +0200 Subject: [PATCH 05/13] Fix tunnel network detection to use tunnel type driver ranges The ``_is_tunnel_project_network`` method in ``DNSExtensionDriverML2`` was only checking the static tunnel ranges from ml2_conf.ini, ignoring the dynamic ranges provided by the ``network-segment-range service`` plugin. When segment ranges are managed via the API, external DNS eligibility for tenant tunnel networks could be wrong. Update the method to use the ML2 tunnel type driver's ``get_network_segment_ranges()``, which handles both static configuration and DB-based ranges when the network-segment-range plugin is enabled. Closes-Bug: #2154266 Related-Bug: #2140291 Assisted-By: Cursor Composer 2.5 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I5d96efa626d18b7589cf2563fdac3a8d399a69a8 (cherry picked from commit 6913303509726a61fdff5246df44aa8f47a39813) --- .../plugins/ml2/extensions/dns_integration.py | 39 +++++---- .../ml2/extensions/test_dns_integration.py | 80 +++++++++++++++++++ 2 files changed, 103 insertions(+), 16 deletions(-) diff --git a/neutron/plugins/ml2/extensions/dns_integration.py b/neutron/plugins/ml2/extensions/dns_integration.py index aded3936ff7..ba2e28ca67d 100644 --- a/neutron/plugins/ml2/extensions/dns_integration.py +++ b/neutron/plugins/ml2/extensions/dns_integration.py @@ -351,35 +351,42 @@ class DNSExtensionDriverML2(DNSExtensionDriver): def __init__(self): super().__init__() self._vlan_driver = None + self._tunnel_drivers = {} self._plugin = None def initialize(self): LOG.info("DNSExtensionDriverML2 initialization complete") + @property + def plugin(self): + if not self._plugin: + self._plugin = directory.get_plugin() + return self._plugin + @property def vlan_driver(self): if not self._vlan_driver: - if not self._plugin: - self._plugin = directory.get_plugin() - self._vlan_driver = self._plugin.type_manager.drivers.get( + self._vlan_driver = self.plugin.type_manager.drivers.get( lib_const.TYPE_VLAN) return self._vlan_driver - def _is_tunnel_project_network(self, provider_net): - if provider_net['network_type'] == lib_const.TYPE_GENEVE: - tunnel_ranges = cfg.CONF.ml2_type_geneve.vni_ranges - elif provider_net['network_type'] == lib_const.TYPE_VXLAN: - tunnel_ranges = cfg.CONF.ml2_type_vxlan.vni_ranges - else: - tunnel_ranges = cfg.CONF.ml2_type_gre.tunnel_id_ranges + def get_tunnel_driver(self, network_type): + if network_type not in self._tunnel_drivers: + self._tunnel_drivers[network_type] = ( + self.plugin.type_manager.drivers.get(network_type)) + return self._tunnel_drivers[network_type] + def _is_tunnel_project_network(self, provider_net): + network_type = provider_net['network_type'] + tunnel_driver = self.get_tunnel_driver(network_type) + if not tunnel_driver: + return False + tunnel_ranges = tunnel_driver.obj.get_network_segment_ranges() + if not tunnel_ranges: + return False segmentation_id = int(provider_net['segmentation_id']) - for entry in tunnel_ranges: - entry = entry.strip() - tun_min, tun_max = entry.split(':') - tun_min = tun_min.strip() - tun_max = tun_max.strip() - return int(tun_min) <= segmentation_id <= int(tun_max) + return any(tun_min <= segmentation_id <= tun_max + for tun_min, tun_max in tunnel_ranges) def _is_vlan_project_network(self, provider_net): if not self.vlan_driver: diff --git a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py index 777e3af7801..d9c1a1ea594 100644 --- a/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py +++ b/neutron/tests/unit/plugins/ml2/extensions/test_dns_integration.py @@ -864,6 +864,86 @@ def test__is_vlan_project_network_no_vlan_driver(self): } self.assertFalse(self.driver._is_vlan_project_network(provider_net)) + def _mock_tunnel_driver(self, network_type, ranges): + mock_tunnel_driver = mock.Mock() + mock_tunnel_driver.obj.get_network_segment_ranges.return_value = ranges + return mock.patch.object( + self.driver, 'get_tunnel_driver', + return_value=mock_tunnel_driver) + + def test__is_tunnel_project_network_with_multiple_ranges(self): + with self._mock_tunnel_driver( + constants.TYPE_VXLAN, [(100, 200), (300, 400)]): + provider_net_in_range = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 150, + } + self.assertTrue( + self.driver._is_tunnel_project_network(provider_net_in_range)) + + provider_net_between_ranges = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 250, + } + self.assertFalse(self.driver._is_tunnel_project_network( + provider_net_between_ranges)) + + provider_net_second_range = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 350, + } + self.assertTrue(self.driver._is_tunnel_project_network( + provider_net_second_range)) + + def test__is_tunnel_project_network_boundary_values(self): + with self._mock_tunnel_driver(constants.TYPE_GENEVE, [(100, 200)]): + provider_net_min = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 100, + } + self.assertTrue( + self.driver._is_tunnel_project_network(provider_net_min)) + + provider_net_max = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 200, + } + self.assertTrue( + self.driver._is_tunnel_project_network(provider_net_max)) + + provider_net_below = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 99, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net_below)) + + provider_net_above = { + 'network_type': constants.TYPE_GENEVE, + 'segmentation_id': 201, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net_above)) + + def test__is_tunnel_project_network_empty_ranges(self): + with self._mock_tunnel_driver(constants.TYPE_GRE, []): + provider_net = { + 'network_type': constants.TYPE_GRE, + 'segmentation_id': 100, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net)) + + def test__is_tunnel_project_network_no_tunnel_driver(self): + with mock.patch.object( + self.driver, 'get_tunnel_driver', return_value=None): + provider_net = { + 'network_type': constants.TYPE_VXLAN, + 'segmentation_id': 100, + } + self.assertFalse( + self.driver._is_tunnel_project_network(provider_net)) + class TestDesignateClientKeystoneV3(testtools.TestCase): """Test case for designate clients """ From 895376b513d30f01505a531206061454d35b0db6 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Mon, 1 Jun 2026 12:02:58 +0200 Subject: [PATCH 06/13] [OVS FW] Clean invalid conntrack entries after OF rules are applied The openvswitch firewall driver has got OF rules to mark some connections as invalid using mark CT_MARK_INVALID in conntrack. Later packets which belongs to such invalid connection are dropped by different OF rules in br-int. The problem is that such conntrack entries were cleaned for each of the ports for which filters were updated, even with deferred apply on. In such case new OpenFlow rules are applied in deferred way after all ports are processed by the agent. That could lead to the situation where conntrack entries to mark some kind of connection as invalid was deleted by the fw driver, but new OF rules were not yet applied in the br-int. If during that time the same packets were still flowing to/from the port, new conntrack entry to mark such packets would be added back and that would not be cleaned after OF rules were actually applied on the `br-int` bridge. This patch fixes that issue be deferring cleanup of the conntrack entries for all ports to be done after OF rules are actually updated in `br-int`. Assisted-by: claude-opus-4.6 Closes-bug: #2154561 Change-Id: I8655da7f7a0cf033ce4a7145f5345195881238e5 Signed-off-by: Slawek Kaplonski (cherry picked from commit 8fde7d461a3ee44be266946dd74d3716df714818) --- .../linux/openvswitch_firewall/firewall.py | 26 +++++++++++++++---- .../openvswitch_firewall/test_firewall.py | 12 +++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 5c19281d364..8c637f5e13c 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -557,6 +557,7 @@ def __init__(self, integration_bridge): self._initialize_sg() self._update_cookie = None self._deferred = False + self._ports_pending_invalid_ct_cleanup = [] self.iptables_helper = iptables.Helper(self.int_br.br) self.iptables_helper.load_driver_if_needed() self.ipconntrack = ip_conntrack.OvsIpConntrackManager() @@ -702,11 +703,22 @@ def _get_port_physical_network(self, port_name): return get_physical_network_from_other_config( self.int_br.br, port_name) - def _delete_invalid_conntrack_entries_for_port(self, port, of_port): - port['of_port'] = of_port + def _delete_invalid_conntrack_entries_for_port(self, ports): for ethertype in [lib_const.IPv4, lib_const.IPv6]: self.ipconntrack.delete_conntrack_state_by_remote_ips( - [port], ethertype, set(), mark=ovsfw_consts.CT_MARK_INVALID) + ports, ethertype, set(), mark=ovsfw_consts.CT_MARK_INVALID) + + def _schedule_invalid_conntrack_entries_cleanup(self, port): + if self._deferred: + self._ports_pending_invalid_ct_cleanup.append(port) + else: + self._delete_invalid_conntrack_entries_for_port([port]) + + def _flush_pending_invalid_conntrack_cleanup(self): + self._delete_invalid_conntrack_entries_for_port( + self._ports_pending_invalid_ct_cleanup + ) + self._ports_pending_invalid_ct_cleanup = [] def get_ofport(self, port): port_id = port['device'] @@ -768,7 +780,8 @@ def prepare_port_filter(self, port): self._update_flows_for_port(of_port, old_of_port) else: self._set_port_filters(of_port) - self._delete_invalid_conntrack_entries_for_port(port, of_port) + port['of_port'] = of_port + self._schedule_invalid_conntrack_entries_cleanup(port) except exceptions.OVSFWPortNotFound as not_found_error: LOG.info("port %(port_id)s does not exist in ovsdb: %(err)s.", {'port_id': port['device'], @@ -808,7 +821,8 @@ def update_port_filter(self, port): else: self._set_port_filters(of_port) - self._delete_invalid_conntrack_entries_for_port(port, of_port) + port['of_port'] = of_port + self._schedule_invalid_conntrack_entries_cleanup(port) except exceptions.OVSFWPortNotFound as not_found_error: LOG.info("port %(port_id)s does not exist in ovsdb: %(err)s.", @@ -897,11 +911,13 @@ def remove_trusted_ports(self, port_ids): def filter_defer_apply_on(self): self._deferred = True + self._ports_pending_invalid_ct_cleanup = [] def filter_defer_apply_off(self): if self._deferred: self._cleanup_stale_sg() self.int_br.apply_flows() + self._flush_pending_invalid_conntrack_cleanup() self._deferred = False @property diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 91dc8e59406..27a1e3133d0 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -1032,15 +1032,23 @@ def test_update_port_filter_port_security_disabled(self): self.assertTrue(self.mock_bridge.br.delete_flows.called) self.delete_invalid_conntrack_entries_mock.assert_not_called() - def test_update_port_filter_applies_added_flows(self): - """Check flows are applied right after _set_flows is called.""" + def test_update_port_filter_applies_added_flows_and_clean_conntrack(self): + """Check flows are applied right after _set_flows is called. + + Additionally this test checks also if conntrack entries marked as + CT_MARK_INVALID are cleaned once after new flows are set. + """ + port_dict = {'device': 'port-id', 'security_groups': [1]} self._prepare_security_group() self.firewall.prepare_port_filter(port_dict) + self.delete_invalid_conntrack_entries_mock.reset_mock() with self.firewall.defer_apply(): self.firewall.update_port_filter(port_dict) + self.delete_invalid_conntrack_entries_mock.assert_not_called() self.mock_bridge.apply_flows.assert_called_once() + self._assert_invalid_conntrack_entries_deleted(port_dict) def test_update_port_filter_clean_when_port_not_found(self): """Check flows are cleaned if port is not found in the bridge.""" From 27e972e43932f526ce6fde737f9344e10bfcb4b9 Mon Sep 17 00:00:00 2001 From: Eduardo Olivares Date: Tue, 28 Apr 2026 15:03:27 +0200 Subject: [PATCH 07/13] Handle invalid MAC in metadata agent port provisioning Ports with mac=['unknown'] (e.g. network:dhcp ports without port security) cause the metadata agent to crash with a ValueError when iterating chassis ports during datapath provisioning. Add an explicit check for mac=['unknown'] alongside the existing empty-MAC guard to skip these ports gracefully. Closes-Bug: #2150434 Assisted-By: Claude Opus 4.6 Change-Id: I966c7a22eca589770793a0ef30c3bbc591c41e41 Signed-off-by: Eduardo Olivares (cherry picked from commit fecac2cc888496a2ef4d0ce627e72cff13174703) --- neutron/agent/ovn/metadata/agent.py | 5 +++ .../unit/agent/ovn/metadata/test_agent.py | 39 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 8619deeaf1b..fc0c9adcac4 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -629,6 +629,11 @@ def _get_port_ip4_ips_and_ip6_flag(self, port): LOG.warning("Port %s MAC column is empty, cannot retrieve IP " "addresses", port.uuid) return [], False + if port.mac == [ovn_const.UNKNOWN_ADDR]: + LOG.warning("Port %s MAC column value is %s, this port will " + "not have metadata service", port.uuid, port.mac) + return [], False + mac, ips = ovn_utils.get_mac_and_ips_from_port_binding(port) if not ips: LOG.debug("Port %s IP addresses were not retrieved from the " diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 839c3d7164f..d0613989108 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_agent.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_agent.py @@ -483,6 +483,36 @@ def test__get_provision_params_returns_provision_parameters_ipv6(self): self._test__get_provision_params_returns_provision_parameters( 'fe80::f816:3eff:feb6:c0c0') + def test__get_provision_params_skips_port_with_unknown_mac(self): + """Should skip ports with mac=['unknown'] and return valid ports.""" + network_id = '1' + datapath = DatapathInfo(uuid='test123', + external_ids={'name': f'neutron-{network_id}'}) + valid_port = makePort(datapath, + mac=['fa:16:3e:e7:ac:ab 1.2.3.4']) + unknown_mac_port = mock.Mock() + unknown_mac_port.type = '' + unknown_mac_port.mac = [ovn_const.UNKNOWN_ADDR] + unknown_mac_port.datapath = datapath + unknown_mac_port.uuid = 'fake-uuid' + metadadata_port = makePort( + datapath, + mac=['fa:16:3e:22:65:18 10.204.0.1'], + external_ids={'neutron:cidrs': '10.204.0.10/29'}, + logical_port='3b66c176-199b-48ec-8331-c1fd3f6e2b44') + + with mock.patch.object(self.agent.sb_idl, 'get_metadata_port', + return_value=metadadata_port),\ + mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', + return_value=[valid_port, unknown_mac_port]): + actual_params = self.agent._get_provision_params(datapath) + + net_name, datapath_port_ips, any_ip6, metadata_port_info = ( + actual_params) + self.assertEqual(network_id, net_name) + self.assertListEqual(['1.2.3.4'], datapath_port_ips) + self.assertFalse(any_ip6) + def test__get_port_ip4_ips_and_ip6_flag_empty_mac(self): """Should return empty IPs for ports with empty MAC column.""" port = mock.Mock() @@ -492,6 +522,15 @@ def test__get_port_ip4_ips_and_ip6_flag_empty_mac(self): self.assertEqual([], ip4_ips) self.assertFalse(any_ip6) + def test__get_port_ip4_ips_and_ip6_flag_unknown_mac(self): + """Should return empty IPs for ports with mac=['unknown'].""" + port = mock.Mock() + port.mac = [ovn_const.UNKNOWN_ADDR] + port.uuid = 'fake-uuid' + ip4_ips, any_ip6 = self.agent._get_port_ip4_ips_and_ip6_flag(port) + self.assertEqual([], ip4_ips) + self.assertFalse(any_ip6) + def _test_provision_datapath(self, ipv6_enabled): """Test datapath provisioning. From 57f12d99d17582d6b9933691042822d0d47ae9ac Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 4 Jun 2026 13:36:05 +0200 Subject: [PATCH 08/13] Fix cross-project access to router conntrack helpers Singleton conntrack helper API operations (GET/PUT/DELETE on /routers/{router_id}/conntrack_helpers/{id}) authorized the request against the URL ``router_id`` but loaded the helper by child ID only. A project member with access to one router could therefore operate on another tenant's helper by reusing its UUID in the path. Validate that the loaded helper's ``router_id`` matches the URL ``router_id`` before returning, updating, or deleting it, following the same pattern used for floating IP port forwarding sub-resources. Closes-Bug: #2152109 Assisted-By: Claude Opus 4.6 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: Icb4a7d2e8f1c35609d8a4b7e2f6c9d0a1b3e5f78 (cherry picked from commit bc8a55497fdf224e4f9672088a196a9b8cd0ed12) --- neutron/services/conntrack_helper/plugin.py | 10 ++-- .../conntrack_helper/test_conntrack_helper.py | 27 ++++++++++ .../services/conntrack_helper/test_plugin.py | 49 ++++++++++++++++--- ...cross-project-access-ba1425c06abc45d5.yaml | 8 +++ 4 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/fix-router-conntrack-helper-cross-project-access-ba1425c06abc45d5.yaml diff --git a/neutron/services/conntrack_helper/plugin.py b/neutron/services/conntrack_helper/plugin.py index d05d1b873df..88b71217096 100644 --- a/neutron/services/conntrack_helper/plugin.py +++ b/neutron/services/conntrack_helper/plugin.py @@ -108,9 +108,9 @@ def _find_existing_conntrack_helper(self, context, router_id, if objs: return (objs[0], param) - def _get_conntrack_helper(self, context, id): + def _get_conntrack_helper(self, context, id, router_id): cth_obj = cth.ConntrackHelper.get_object(context, id=id) - if not cth_obj: + if not cth_obj or cth_obj.router_id != router_id: raise cth_exc.ConntrackHelperNotFound(id=id) return cth_obj @@ -153,7 +153,7 @@ def update_router_conntrack_helper(self, context, id, router_id, conntrack_helper = conntrack_helper.get(apidef.RESOURCE_NAME) try: with db_api.CONTEXT_WRITER.using(context): - cth_obj = self._get_conntrack_helper(context, id) + cth_obj = self._get_conntrack_helper(context, id, router_id) cth_obj.update_fields(conntrack_helper, reset_changes=True) self._check_conntrack_helper_constraints(cth_obj) cth_obj.update() @@ -171,7 +171,7 @@ def update_router_conntrack_helper(self, context, id, router_id, @db_base_plugin_common.make_result_with_fields @db_base_plugin_common.convert_result_to_dict def get_router_conntrack_helper(self, context, id, router_id, fields=None): - return self._get_conntrack_helper(context, id) + return self._get_conntrack_helper(context, id, router_id) @db_base_plugin_common.make_result_with_fields @db_base_plugin_common.convert_result_to_dict @@ -185,7 +185,7 @@ def get_router_conntrack_helpers(self, context, router_id=None, router_id=router_id, **filters) def delete_router_conntrack_helper(self, context, id, router_id): - cth_obj = self._get_conntrack_helper(context, id) + cth_obj = self._get_conntrack_helper(context, id, router_id) with db_api.CONTEXT_WRITER.using(context): cth_obj.delete() self.push_api.push(context, [cth_obj], rpc_events.DELETED) diff --git a/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py b/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py index 8e5345e919e..799e58bd5d5 100644 --- a/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py +++ b/neutron/tests/functional/services/conntrack_helper/test_conntrack_helper.py @@ -127,3 +127,30 @@ def test_negative_delete_conntrack_helper(self): self.assertRaises(cth_exc.ConntrackHelperNotFound, self.cth_plugin.delete_router_conntrack_helper, self.context, INVALID_ID, self.router['id']) + + def test_negative_singleton_operations_wrong_router(self): + res = self.cth_plugin.create_router_conntrack_helper( + self.context, self.router['id'], self.conntack_helper) + other_router = self._create_router(distributed=True) + new_conntack_helper = { + apidef.RESOURCE_NAME: + {apidef.PROTOCOL: 'udp', + apidef.PORT: 6969, + apidef.HELPER: 'tftp'} + } + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.get_router_conntrack_helper, + self.context, res['id'], other_router['id']) + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.update_router_conntrack_helper, + self.context, res['id'], other_router['id'], + new_conntack_helper) + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.delete_router_conntrack_helper, + self.context, res['id'], other_router['id']) + helper = self.cth_plugin.get_router_conntrack_helper( + self.context, res['id'], self.router['id']) + self.assertEqual(res['id'], helper['id']) diff --git a/neutron/tests/unit/services/conntrack_helper/test_plugin.py b/neutron/tests/unit/services/conntrack_helper/test_plugin.py index 93b258ee25f..e3c53662361 100644 --- a/neutron/tests/unit/services/conntrack_helper/test_plugin.py +++ b/neutron/tests/unit/services/conntrack_helper/test_plugin.py @@ -193,9 +193,10 @@ def test_update_conntrack_helper(self, mock_cth_get_object, mock_rpc_push): cth_obj = mock.Mock() cth_obj.helper = 'tftp' cth_obj.protocol = 'udp' + cth_obj.router_id = 'fake-router' mock_cth_get_object.return_value = cth_obj self.cth_plugin.update_router_conntrack_helper( - self.ctxt, 'cth_id', mock.ANY, **cth_input) + self.ctxt, 'cth_id', 'fake-router', **cth_input) mock_cth_get_object.assert_called_once_with(self.ctxt, id='cth_id') self.assertTrue(cth_obj.update_fields) self.assertTrue(cth_obj.update) @@ -216,12 +217,31 @@ def test_negative_update_conntrack_helper(self, mock_cth_get_object): self.assertRaises( cth_exc.ConntrackHelperNotFound, self.cth_plugin.update_router_conntrack_helper, - self.ctxt, 'cth_id', mock.ANY, **cth_input) + self.ctxt, 'cth_id', 'fake-router', **cth_input) + + @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') + def test_negative_update_conntrack_helper_wrong_router( + self, mock_cth_get_object): + cth_input = { + 'conntrack_helper': { + 'conntrack_helper': { + 'protocol': 'udp', + 'port': 69, + 'helper': 'tftp'} + } + } + mock_cth_get_object.return_value = mock.Mock( + router_id='correct-router') + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.update_router_conntrack_helper, + self.ctxt, 'cth_id', 'other-router', **cth_input) @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') def test_get_conntrack_helper(self, get_object_mock): + get_object_mock.return_value = mock.Mock(router_id='fake-router') self.cth_plugin.get_router_conntrack_helper( - self.ctxt, 'cth_id', mock.ANY, fields=None) + self.ctxt, 'cth_id', 'fake-router', fields=None) get_object_mock.assert_called_once_with(self.ctxt, id='cth_id') @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') @@ -230,7 +250,16 @@ def test_negative_get_conntrack_helper(self, get_object_mock): self.assertRaises( cth_exc.ConntrackHelperNotFound, self.cth_plugin.get_router_conntrack_helper, - self.ctxt, 'cth_id', mock.ANY, fields=None) + self.ctxt, 'cth_id', 'fake-router', fields=None) + + @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') + def test_negative_get_conntrack_helper_wrong_router(self, + get_object_mock): + get_object_mock.return_value = mock.Mock(router_id='victim-router') + self.assertRaises( + cth_exc.ConntrackHelperNotFound, + self.cth_plugin.get_router_conntrack_helper, + self.ctxt, 'cth_id', 'other-router', fields=None) @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_objects') def test_get_conntrack_helpers(self, get_objects_mock): @@ -248,7 +277,7 @@ def test_delete_conntrack_helper(self, get_object_mock, mock_rpc_push): helper='tftp') get_object_mock.return_value = cth_obj self.cth_plugin.delete_router_conntrack_helper(self.ctxt, 'cth_id', - mock.ANY) + 'fake-router') cth_obj.delete.assert_called() mock_rpc_push.assert_called_once_with( self.ctxt, mock.ANY, rpc_events.DELETED) @@ -258,4 +287,12 @@ def test_negative_delete_conntrack_helper(self, get_object_mock): get_object_mock.return_value = None self.assertRaises(cth_exc.ConntrackHelperNotFound, self.cth_plugin.delete_router_conntrack_helper, - self.ctxt, 'cth_id', mock.ANY) + self.ctxt, 'cth_id', 'fake-router') + + @mock.patch.object(conntrack_helper.ConntrackHelper, 'get_object') + def test_negative_delete_conntrack_helper_wrong_router( + self, get_object_mock): + get_object_mock.return_value = mock.Mock(router_id='correct-router') + self.assertRaises(cth_exc.ConntrackHelperNotFound, + self.cth_plugin.delete_router_conntrack_helper, + self.ctxt, 'cth_id', 'other-router') diff --git a/releasenotes/notes/fix-router-conntrack-helper-cross-project-access-ba1425c06abc45d5.yaml b/releasenotes/notes/fix-router-conntrack-helper-cross-project-access-ba1425c06abc45d5.yaml new file mode 100644 index 00000000000..df142ec3cff --- /dev/null +++ b/releasenotes/notes/fix-router-conntrack-helper-cross-project-access-ba1425c06abc45d5.yaml @@ -0,0 +1,8 @@ +--- +security: + - | + Fixed an issue in the router conntrack helper API where a project + member authorized on one router could read, update, or delete another + project's conntrack helper by supplying an incorrect helper ID under their + own router ID in the request URL. Singleton operations now verify that + the conntrack helper belongs to the router specified in the URL. From 88d6c7979d29676bb7a4e5a7b174bb78d60b1011 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 9 Jun 2026 14:07:27 +0200 Subject: [PATCH 09/13] Fix ``update_router:enable_default_route_*`` policies The operation for these actions is PUT, not POST. Closes-Bug: #2156054 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: Iec4b8ddf3717ddc781acfb46ada81839f853bdea (cherry picked from commit 8ccf3f9e7990ea417adfb6776e99809437757e07) --- neutron/conf/policies/router.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/neutron/conf/policies/router.py b/neutron/conf/policies/router.py index 52cdd8d13ea..419d7e6f8a9 100644 --- a/neutron/conf/policies/router.py +++ b/neutron/conf/policies/router.py @@ -315,7 +315,7 @@ scope_types=['project'], description=('Specify ``enable_default_route_bfd`` attribute when ' 'updating a router'), - operations=ACTION_POST, + operations=ACTION_PUT, ), policy.DocumentedRuleDefault( name='update_router:enable_default_route_ecmp', @@ -323,7 +323,7 @@ scope_types=['project'], description=('Specify ``enable_default_route_ecmp`` attribute when ' 'updating a router'), - operations=ACTION_POST, + operations=ACTION_PUT, ), policy.DocumentedRuleDefault( name='update_router:tags', From 50aa8f73602c03fe6bee92b02c162afd5ba498eb Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 9 Jun 2026 12:28:37 +0200 Subject: [PATCH 10/13] ovs: skip OF operations for ports with invalid ofport When a VIF port is picked up by the OVS agent ``rpc_loop`` before OVS has assigned it a valid ofport (the underlying TAP device may not yet exist), ``port_alive()`` and ``port_dead()`` pass the invalid value (``[]`` or ``-1``) through to ``uninstall_flows()`` / ``drop_port()``. The resulting OpenFlow FlowMod with ``in_port=None`` causes os-ken's ``send_msg`` to hang until the ``of_request_timeout`` (300 s) fires, blocking the ``rpc_loop`` for the entire duration and preventing all subsequent port processing. Guard ``port_alive()``, ``port_dead()`` and ``treat_vif_port()`` so they return early when the ofport is unassigned or invalid. ``treat_vif_port()`` returns False so the port is not marked as bound; the OVSDB monitor will detect the ofport change on a later iteration and re-trigger processing. Closes-Bug: #2155883 Assisted-By: Claude Opus 4.6 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: Ic137f8a2862794c3c7ac670e643ca532873e474b (cherry picked from commit 8a4a4b13f55f0729f7fd0e7fc1ec19639d5bf97a) --- .../openvswitch/agent/ovs_neutron_agent.py | 31 ++++++----- .../agent/test_ovs_neutron_agent.py | 55 ++++++++++++++++++- ...f-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml | 13 +++++ 3 files changed, 85 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/ovs-agent-skip-of-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 53f73f080b3..ceaadbb2e3a 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1489,7 +1489,15 @@ def port_unbound(self, vif_id, net_uuid=None): if not lvm.vif_ports: self.reclaim_local_vlan(net_uuid, lvm.segmentation_id) + @staticmethod + def _has_valid_ofport(port): + return port.ofport and port.ofport != ovs_lib.INVALID_OFPORT + def port_alive(self, port, log_errors=True): + if not self._has_valid_ofport(port): + LOG.warning("port_alive skipped for port %s: invalid ofport %s", + port.port_name, port.ofport) + return cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag", log_errors=log_errors) # Port normal vlan tag is set correctly, remove the drop flows @@ -1505,6 +1513,10 @@ def port_dead(self, port, log_errors=True): :param port: an ovs_lib.VifPort object. ''' + if not self._has_valid_ofport(port): + LOG.warning("port_dead skipped for port %s: invalid ofport %s", + port.port_name, port.ofport) + return # Don't kill a port if it's already dead cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag", log_errors=log_errors) @@ -1964,19 +1976,12 @@ def treat_vif_port(self, vif_port, port_id, network_id, network_type, physical_network, segmentation_id, admin_state_up, fixed_ips, device_owner, provisioning_needed): port_needs_binding = True - if not vif_port.ofport: - # Log an error if the VIF port has no ofport, which indicates - # that the port might not be able to transmit traffic. - LOG.error("VIF port: %s has no ofport and might not " - "be able to transmit.", vif_port.vif_id) - elif vif_port.ofport == ovs_lib.INVALID_OFPORT: - # When the ofport is set to INVALID_OFPORT, it indicates that - # the port is in a transitional state and has not yet been fully - # configured. - LOG.info("VIF port: %s is in a transitional state and has not " - "yet been assigned a valid ofport. This is expected " - "during port initialization. (ofport=%s)", - vif_port.vif_id, vif_port.ofport) + if not self._has_valid_ofport(vif_port): + LOG.warning("VIF port: %s has no valid ofport (ofport=%s), " + "skipping OF operations; the port will be " + "retried on the next iteration.", + vif_port.vif_id, vif_port.ofport) + return False if admin_state_up: port_needs_binding = self.port_bound( vif_port, network_id, network_type, diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 3427631c3f6..161e9956c6f 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -411,6 +411,59 @@ def test_port_dead_with_port_already_dead(self): def test_port_dead_with_valid_tag(self): self._test_port_dead(cur_tag=1) + def test_port_dead_invalid_ofport_unassigned(self): + port = mock.Mock() + port.ofport = ovs_lib.UNASSIGNED_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_dead(port) + int_br.set_db_attribute.assert_not_called() + int_br.drop_port.assert_not_called() + + def test_port_dead_invalid_ofport_negative(self): + port = mock.Mock() + port.ofport = ovs_lib.INVALID_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_dead(port) + int_br.set_db_attribute.assert_not_called() + int_br.drop_port.assert_not_called() + + def test_port_alive_invalid_ofport_unassigned(self): + port = mock.Mock() + port.ofport = ovs_lib.UNASSIGNED_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_alive(port) + int_br.db_get_val.assert_not_called() + int_br.uninstall_flows.assert_not_called() + + def test_port_alive_invalid_ofport_negative(self): + port = mock.Mock() + port.ofport = ovs_lib.INVALID_OFPORT + port.port_name = 'tap1234' + with mock.patch.object(self.agent, 'int_br') as int_br: + self.agent.port_alive(port) + int_br.db_get_val.assert_not_called() + int_br.uninstall_flows.assert_not_called() + + def test_treat_vif_port_invalid_ofport_returns_false(self): + for ofport in (ovs_lib.UNASSIGNED_OFPORT, ovs_lib.INVALID_OFPORT, 0): + vif_port = mock.Mock() + vif_port.ofport = ofport + vif_port.vif_id = 'test-port-id' + with mock.patch.object( + self.agent, 'port_bound' + ) as port_bound, mock.patch.object( + self.agent, 'port_alive' + ) as port_alive: + result = self.agent.treat_vif_port( + vif_port, 'port-id', 'net-id', 'vxlan', + None, 100, True, [], 'compute:nova', False) + self.assertFalse(result) + port_bound.assert_not_called() + port_alive.assert_not_called() + def mock_scan_ports(self, vif_port_set=None, registered_ports=None, updated_ports=None, port_tags_dict=None, sync=False): if port_tags_dict is None: # Because empty dicts evaluate as False. @@ -1152,7 +1205,7 @@ def test_treat_vif_port_shut_down_port(self): "iface-id": "407a79e0-e0be-4b7d-92a6-513b2161011b", "vif_mac": "fa:16:3e:68:46:7b", "port_name": "qr-407a79e0-e0", - "ofport": -1, + "ofport": 10, "bridge_name": "br-int"}) with mock.patch.object( self.agent.plugin_rpc, 'update_device_down' diff --git a/releasenotes/notes/ovs-agent-skip-of-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml b/releasenotes/notes/ovs-agent-skip-of-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml new file mode 100644 index 00000000000..49ea45a9ce7 --- /dev/null +++ b/releasenotes/notes/ovs-agent-skip-of-ops-invalid-ofport-a1b2c3d4e5f6a7b8.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Fixed a race condition in the OVS agent where ``port_alive`` and + ``port_dead`` could issue OpenFlow operations with an invalid + ``in_port`` value when a VIF port had an unassigned or error ofport + (e.g. ``[]`` or ``-1``). The malformed OpenFlow message caused the + os-ken ``send_msg`` call to hang until the 300-second + ``of_request_timeout`` expired, which blocked the agent's + ``rpc_loop`` and prevented all subsequent port processing for the + remainder of the timeout period. The functions now validate the + ofport before performing any OpenFlow operations and log a warning + so the port is retried on the next iteration. From e97c10f238db683b88bb2dbbfd8de01420dc5efe Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 10 Jun 2026 11:03:20 +0200 Subject: [PATCH 11/13] ovs: defer ports with invalid ofport in ``_process_port`` When a port is inserted into OVS, the kernel may transiently assign ``ofport=-1`` (``INVALID_OFPORT``) before settling on a valid value. Previously, ``_process_port`` only deferred ports with ``UNASSIGNED_OFPORT`` (``[]``), so a port arriving with ``ofport=-1`` was immediately added to the processing pipeline. Patch [1] then correctly skipped OF operations in ``treat_vif_port`` for such ports but never re-queued them, which meant the ``network-vif-plugged`` event was never sent to Nova, causing a 300 s VIF-plug timeout. Extend the ``_process_port`` check to also defer ``INVALID_OFPORT`` ports via the existing ``ports_not_ready_yet`` mechanism. On the next ``rpc_loop`` iteration the port attributes are re-read from OVS; if the ofport is now valid the port is processed normally, otherwise it is deferred again. [1]https://review.opendev.org/c/openstack/neutron/+/992423 Closes-Bug: #2155883 Assisted-By: Claude Opus 4.6 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I9fbc8f75f8084461901a004dfcb07e42f76db62b (cherry picked from commit 7ebcce0b8dcd4cd482da92672f5ae7c51530a429) --- .../ml2/drivers/openvswitch/agent/ovs_neutron_agent.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index ceaadbb2e3a..700eb469499 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1868,9 +1868,11 @@ def _process_port(port, ports, ancillary_ports): iface_id = self.int_br.portid_from_external_ids( port['external_ids']) if iface_id: - if port['ofport'] == ovs_lib.UNASSIGNED_OFPORT: - LOG.debug("Port %s not ready yet on the bridge", - iface_id) + if port['ofport'] in (ovs_lib.UNASSIGNED_OFPORT, + ovs_lib.INVALID_OFPORT): + LOG.debug("Port %s not ready yet on the bridge " + "(ofport=%s)", + iface_id, port['ofport']) ports_not_ready_yet.add(port['name']) return # check if port belongs to ancillary bridge From a9d80557f3ac3b82891448be619a2f54ba205e5b Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 8 Jun 2026 12:43:43 +0200 Subject: [PATCH 12/13] quota: Fix quota details API error with unloaded service plugins When a service plugin package (e.g. neutron-fwaas, neutron-vpnaas, networking-sfc) is installed but its service plugin is not configured in ``service_plugins``, the quota details API endpoint (GET /v2.0/quotas/{project_id}/details) returns a 500 Server Error. The installed package registers quota resources (e.g. firewall_group, firewall_policy, firewall_rule) at import time via ``resource_helper.build_resource_info(register_quota=True)``. When the quota details endpoint iterates over all registered resources to count usage, it calls ``_count_resource()`` which looks for a plugin that provides ``get__count`` or ``get_``. Since the service plugin is not loaded, no plugin supports counting those resources, and a ``NotImplementedError`` is raised. Catch the ``NotImplementedError`` in ``DbQuotaDriver.get_detailed_project_quotas()`` and skip the resource instead of letting the exception propagate as a 500 error. Also guard the project-specific limit update loop against skipped resources. Closes-Bug: #2155846 Assisted-By: Claude Opus 4.6 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I923e90279edf3de3fa85c83fd46e1b5dec0468de --- neutron/db/quota/driver.py | 10 +++++++-- neutron/tests/unit/db/quota/test_driver.py | 22 +++++++++++++++++++ ...oaded-service-plugin-f4c2a0766eb045b0.yaml | 10 +++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/fix-quota-details-unloaded-service-plugin-f4c2a0766eb045b0.yaml diff --git a/neutron/db/quota/driver.py b/neutron/db/quota/driver.py index 694a40735fa..fc6dffc52c4 100644 --- a/neutron/db/quota/driver.py +++ b/neutron/db/quota/driver.py @@ -97,7 +97,12 @@ def get_detailed_project_quotas(self, context, resources, project_id): # pass it regardless to keep the quota driver API intact plugins = directory.get_plugins() plugin = plugins.get(key, plugins[constants.CORE]) - used = resource.count(context, plugin, project_id) + try: + used = resource.count(context, plugin, project_id) + except NotImplementedError: + LOG.info('Skipping quota resource %s: no plugin loaded ' + 'that supports counting it.', key) + continue project_quota_ext[key] = { 'limit': resource.default, @@ -108,7 +113,8 @@ def get_detailed_project_quotas(self, context, resources, project_id): quota_objs = quota_obj.Quota.get_objects(context, project_id=project_id) for item in quota_objs: - project_quota_ext[item['resource']]['limit'] = item['limit'] + if item['resource'] in project_quota_ext: + project_quota_ext[item['resource']]['limit'] = item['limit'] return project_quota_ext @staticmethod diff --git a/neutron/tests/unit/db/quota/test_driver.py b/neutron/tests/unit/db/quota/test_driver.py index d3f0f7c3cc7..362279efb23 100644 --- a/neutron/tests/unit/db/quota/test_driver.py +++ b/neutron/tests/unit/db/quota/test_driver.py @@ -308,6 +308,28 @@ def test_get_detailed_project_quotas_multiple_resource(self): self.assertEqual(7, detailed_quota[self.resource_2]['reserved']) self.assertEqual(3, detailed_quota[self.resource_2]['used']) + def test_get_detailed_project_quotas_skips_uncount_resource(self): + def _raise_not_implemented(context, resource, project_id): + raise NotImplementedError( + 'No plugins that support counting %s found.' % resource) + + resources = { + self.resource_1: + TestTrackedResource(self.resource_1, test_quota.MehModel), + self.resource_2: + TestCountableResource(self.resource_2, + _raise_not_implemented)} + self.plugin.update_quota_limit(self.context, self.project_1, + self.resource_1, 6) + self.plugin.update_quota_limit(self.context, self.project_1, + self.resource_2, 9) + detailed_quota = self.plugin.get_detailed_project_quotas( + self.context, resources, self.project_1) + + self.assertIn(self.resource_1, detailed_quota) + self.assertNotIn(self.resource_2, detailed_quota) + self.assertEqual(6, detailed_quota[self.resource_1]['limit']) + def test_quota_limit_check(self): resources = self._create_resources() self.plugin.update_quota_limit(self.context, self.project_1, diff --git a/releasenotes/notes/fix-quota-details-unloaded-service-plugin-f4c2a0766eb045b0.yaml b/releasenotes/notes/fix-quota-details-unloaded-service-plugin-f4c2a0766eb045b0.yaml new file mode 100644 index 00000000000..636cd66722d --- /dev/null +++ b/releasenotes/notes/fix-quota-details-unloaded-service-plugin-f4c2a0766eb045b0.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixed a 500 Internal Server Error when querying the quota details API + (``GET /v2.0/quotas/{project_id}/details``) while a service plugin + package (e.g. ``neutron-fwaas``, ``neutron-vpnaas``, + ``networking-sfc``) is installed but its service plugin is not + configured in ``service_plugins``. The quota engine now gracefully + skips resources whose service plugin is not loaded instead of raising + a ``NotImplementedError``. From 0b06dc038b685f3340c8a7cf7b07128ed5c39b27 Mon Sep 17 00:00:00 2001 From: Seyeong Kim Date: Fri, 12 Jun 2026 07:06:19 +0000 Subject: [PATCH 13/13] Guard tun_br in _restore_local_vlan_map when no br-tun With tunneling disabled (enable_tunneling=False), there is no br-tun and self.tun_br is None. _restore_local_vlan_map(), called from __init__(), calls self.tun_br.get_flood_to_tun_ofports() unconditionally for every integration-bridge port that carries a net_uuid, so the agent crashes: AttributeError: 'NoneType' object has no attribute 'get_flood_to_tun_ofports' The branch is only reached once the agent has populated net_uuid on the ports, so the crash hits on agent restart and recurs on every restart, preventing the agent from restoring dataplane flows until manual recovery. Skip the flood-port restore when tunneling is disabled, leaving tun_ofports as an empty set as for ports with no tunnel flood entries. Closes-Bug: #2156566 Change-Id: I4741480f8a7fbce51f521579e78d5687773c29b8 Signed-off-by: Seyeong Kim (cherry picked from commit 92181b39e793e917483b9a9536bc6925961060cb) --- .../openvswitch/agent/ovs_neutron_agent.py | 6 ++-- .../agent/test_ovs_neutron_agent.py | 36 ++++++++++++++----- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 700eb469499..9199a11352d 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -517,8 +517,10 @@ def _restore_local_vlan_map(self): self.available_local_vlans.remove(local_vlan) # Restore the br-tun flood output ports # See LP #1978088 - tun_ofports = self.tun_br.get_flood_to_tun_ofports( - local_vlan) + tun_ofports = set() + if self.enable_tunneling: + tun_ofports = self.tun_br.get_flood_to_tun_ofports( + local_vlan) self._local_vlan_hints[key] = { 'vlan': local_vlan, 'tun_ofports': tun_ofports} diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 161e9956c6f..070a42452d5 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -286,7 +286,14 @@ def test_agent_available_local_vlans(self): self.assertNotIn(tag, available_vlan) def _test_restore_local_vlan_maps(self, tag, segmentation_id='1', - tun_ofports=None): + tun_ofports=None, no_tun_br=False): + if no_tun_br: + self.agent.enable_tunneling = False + self.agent.tun_br = None + else: + # _make_agent() leaves enable_tunneling False (tunnel_types=[]); + # enable it so the gated get_flood_to_tun_ofports() runs. + self.agent.enable_tunneling = True tun_ofports = tun_ofports or set() port = mock.Mock() port.port_name = 'fake_port' @@ -318,19 +325,25 @@ def _test_restore_local_vlan_maps(self, tag, segmentation_id='1', 'other_config': local_vlan_map, 'tag': tag}] - with mock.patch.object(self.agent.int_br, - 'get_ports_attributes', - side_effect=[get_interfaces, - get_ports]) as gpa,\ - mock.patch.object(self.agent.tun_br, - 'get_flood_to_tun_ofports') as gftto: - gftto.return_value = tun_ofports + if no_tun_br: + # enable_tunneling=False: no br-tun to mock. + tun_ctx = contextlib.nullcontext() + expected_tun_ofports = set() + else: + tun_ctx = mock.patch.object(self.agent.tun_br, + 'get_flood_to_tun_ofports', + return_value=tun_ofports) + expected_tun_ofports = tun_ofports + with tun_ctx, mock.patch.object(self.agent.int_br, + 'get_ports_attributes', + side_effect=[get_interfaces, + get_ports]) as gpa: self.agent._restore_local_vlan_map() expected_hints = {} if tag: key = f"{net_uuid}/{segmentation_id}" expected_hints[key] = {'vlan': tag, - 'tun_ofports': tun_ofports} + 'tun_ofports': expected_tun_ofports} self.assertEqual(expected_hints, self.agent._local_vlan_hints) # make sure invalid and unassigned ports were skipped gpa.assert_has_calls([ @@ -353,6 +366,11 @@ def test_restore_local_vlan_map_segmentation_id_compat(self): def test_restore_local_vlan_map_tun_ofports(self): self._test_restore_local_vlan_maps(2, tun_ofports={2, 3}) + def test_restore_local_vlan_map_no_tun_br(self): + # Non-tunneling deployment (enable_tunneling=False): tun_br is None, + # so the flood-port restore must be skipped instead of crashing. + self._test_restore_local_vlan_maps(2, no_tun_br=True) + def test_check_agent_configurations_for_dvr_raises(self): self.agent.enable_distributed_routing = True self.agent.enable_tunneling = True