From 05a81e5f6748a2ceb230f98b06984502311d9410 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Thu, 10 Apr 2025 13:43:30 +0100 Subject: [PATCH 1/3] Add reproducer test for bug 2105896 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we added support for neutron security groups that were shared via rbac policy in epoxy the way we detected duplicate groups was implemented incorrectly. NoUniqueMatchova should reject requests by name if there are two security groups with the same name however we also reject requests when you use the uuid. That is a bug. Related-Bug: #2105896 Co-Authored-By: René Ribaud Change-Id: I0e1dda07110a99daee1137d7692689a6dd274af8 Signed-off-by: René Ribaud (cherry picked from commit 47ed32399aabc2f55082c78377a55cabfef337bd) (cherry picked from commit 909aa8c7faa99b00ad540fac5a60fe32d7fa13f7) (cherry picked from commit 5e0a9886367976af87d004235395641ed6b6d6ae) --- nova/tests/unit/network/test_neutron.py | 94 +++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 935cc6e71f3..cc1ccb9226b 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -9433,6 +9433,100 @@ def test__process_security_groups_non_unique_match(self): [mock.call(fields=['id', 'name'], tenant_id=uuids.project_id), mock.call(fields=['id', 'name'], shared=True)]) + def test__process_security_groups_unique_uuids(self): + instance = objects.Instance(project_id=uuids.project_id) + mock_neutron = mock.Mock(spec=client.Client) + mock_neutron.list_security_groups.side_effect = [ + { + 'security_groups': [ + { + 'id': uuids.sg1, + 'name': 'nonunique-name', + } + ] + }, + { + 'security_groups': [ + { + 'id': uuids.sg2, + 'name': 'nonunique-name', + } + ] + } + ] + mock_neutron.list_extensions.return_value = { + 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} + api = neutronapi.API() + + # FIXME(sean-k-mooney): this is bug 2105896 + # it is ok for security groups to have the same name if we + # request them by uuid. + ex = self.assertRaises( + exception.NoUniqueMatch, api._process_security_groups, + instance, mock_neutron, [uuids.sg1, uuids.sg2]) + + self.assertIn("nonunique-name", str(ex)) + mock_neutron.list_security_groups.assert_has_calls( + [mock.call(fields=['id', 'name'], tenant_id=uuids.project_id), + mock.call(fields=['id', 'name'], shared=True)]) + + def test__process_security_groups_non_unique_match_same_tenant(self): + """Test that duplicate names within the same tenant raise + NoUniqueMatch when requested by name. + """ + instance = objects.Instance(project_id=uuids.project_id) + mock_neutron = mock.Mock(spec=client.Client) + mock_neutron.list_security_groups.return_value = { + 'security_groups': [ + { + 'id': uuids.sg1, + 'name': 'nonunique-name', + }, + { + 'id': uuids.sg2, + 'name': 'nonunique-name', + } + ] + } + mock_neutron.list_extensions.return_value = { + 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} + api = neutronapi.API() + + ex = self.assertRaises( + exception.NoUniqueMatch, api._process_security_groups, + instance, mock_neutron, ["nonunique-name"]) + self.assertIn("nonunique-name", str(ex)) + + def test__process_security_groups_unique_uuids_same_tenant(self): + """Test that duplicate names within the same tenant are handled + when requested by UUID. + """ + instance = objects.Instance(project_id=uuids.project_id) + mock_neutron = mock.Mock(spec=client.Client) + mock_neutron.list_security_groups.return_value = { + 'security_groups': [ + { + 'id': uuids.sg1, + 'name': 'nonunique-name', + }, + { + 'id': uuids.sg2, + 'name': 'nonunique-name', + } + ] + } + mock_neutron.list_extensions.return_value = { + 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} + api = neutronapi.API() + + # FIXME(sean-k-mooney): this is bug 2105896 + # it is ok for security groups to have the same name if we + # request them by uuid. + ex = self.assertRaises( + exception.NoUniqueMatch, api._process_security_groups, + instance, mock_neutron, [uuids.sg1, uuids.sg2]) + self.assertIn("nonunique-name", str(ex)) + @mock.patch.object(neutronapi.API, 'get_instance_nw_info') @mock.patch.object(neutronapi.API, '_update_port_dns_name') @mock.patch.object(neutronapi.API, '_create_port_minimal') From dbe586bc2fc6af1ba7c3fdc237ee885aa7880e40 Mon Sep 17 00:00:00 2001 From: cw0306-lee Date: Wed, 10 Dec 2025 14:07:24 +0900 Subject: [PATCH 2/3] Fix error when multiple security group in a project with same name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server build fail when user give any security group and there are same name security exists in project. I fixed it occurs error when user give security group with duplicated name. NOTE(rribaud): A conflict occurred in nova/network/neutron.py because the import "from keystoneauth1 import loading as ks_loading" exists in this stable/2025.2 branch but not upstream. Closes-Bug: #2105896 Change-Id: Ie061550c6eecb51951cebd9c323f31b93b748ff5 Co-Authored-By: René Ribaud Signed-off-by: cw0306-lee (cherry picked from commit 96a0b17c7cdd9e64c558b6f005ff2b260020cbe6) (cherry picked from commit 556ba4c9ccef03216ae465d5c4eb85ce2255a4c9) (cherry picked from commit bf6e39e5b379845776956c0157d4802c4b5a1121) --- nova/network/neutron.py | 41 +++++++++---------- nova/tests/unit/network/test_neutron.py | 33 +++++++-------- .../notes/bug-2105896-2bebad3d9eacd346.yaml | 11 +++++ 3 files changed, 48 insertions(+), 37 deletions(-) create mode 100644 releasenotes/notes/bug-2105896-2bebad3d9eacd346.yaml diff --git a/nova/network/neutron.py b/nova/network/neutron.py index 33ad71306a5..93ce347cb55 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -24,6 +24,8 @@ import time import typing as ty +from collections import defaultdict + from keystoneauth1 import loading as ks_loading from neutronclient.common import exceptions as neutron_client_exc from neutronclient.v2_0 import client as clientv20 @@ -831,12 +833,12 @@ def _get_security_group_ids(self, security_groups, user_security_groups): :raises nova.exception.SecurityGroupNotFound: If a given security group is not found. """ - # Initialize two dictionaries to map security group names and IDs to - # their corresponding IDs - name_to_id = {} + # Map security group names to their IDs (a name can have + # multiple IDs since Neutron does not enforce uniqueness) + name_to_id = defaultdict(list) # NOTE(sean-k-mooney): using a dict here instead of a set is faster # probably due to l1 code cache misses due to the introduction - # of set lookup in addition to dict lookups making the branch + # of set lookups in addition to dict lookups making the branch # prediction for the second for loop less reliable. id_to_id = {} @@ -844,14 +846,8 @@ def _get_security_group_ids(self, security_groups, user_security_groups): for user_security_group in user_security_groups: name = user_security_group['name'] sg_id = user_security_group['id'] - - # Check for duplicate names and raise an exception if found - if name in name_to_id: - raise exception.NoUniqueMatch( - _("Multiple security groups found matching" - " '%s'. Use an ID to be more specific.") % name) - # Map the name to its corresponding ID - name_to_id[name] = sg_id + # Append the ID to the list for this name + name_to_id[name].append(sg_id) # Map the ID to itself for easy lookup id_to_id[sg_id] = sg_id @@ -860,16 +856,19 @@ def _get_security_group_ids(self, security_groups, user_security_groups): # Iterate over the requested security groups for security_group in security_groups: - # Check if the security group is in the name-to-ID dictionary - # as if a user names the security group the same as - # another's security groups uuid, the name takes priority. - if security_group in name_to_id: - security_group_ids.append(name_to_id[security_group]) - # Check if the security group is in the ID-to-ID dictionary - elif security_group in id_to_id: + # Check UUID first since it is always unique + if security_group in id_to_id: security_group_ids.append(id_to_id[security_group]) - # Raise an exception if the security group is not found in - # either dictionary + # Then check by name + elif security_group in name_to_id: + # If there are multiple IDs for this name, raise exception + if len(name_to_id[security_group]) > 1: + raise exception.NoUniqueMatch( + _("Multiple security groups found matching" + " '%s'. Use an ID to be more specific.") + % security_group) + security_group_ids.append(name_to_id[security_group][0]) + # Raise an exception if the security group is not found else: raise exception.SecurityGroupNotFound( security_group_id=security_group) diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index cc1ccb9226b..686156e4749 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -9458,21 +9458,22 @@ def test__process_security_groups_unique_uuids(self): 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} api = neutronapi.API() - # FIXME(sean-k-mooney): this is bug 2105896 - # it is ok for security groups to have the same name if we - # request them by uuid. - ex = self.assertRaises( - exception.NoUniqueMatch, api._process_security_groups, + # Bug 2105896: it is ok for security groups to have the same + # name if we request them by uuid. + result = api._process_security_groups( instance, mock_neutron, [uuids.sg1, uuids.sg2]) - self.assertIn("nonunique-name", str(ex)) + self.assertEqual([uuids.sg1, uuids.sg2], result) mock_neutron.list_security_groups.assert_has_calls( [mock.call(fields=['id', 'name'], tenant_id=uuids.project_id), mock.call(fields=['id', 'name'], shared=True)]) def test__process_security_groups_non_unique_match_same_tenant(self): - """Test that duplicate names within the same tenant raise - NoUniqueMatch when requested by name. + """Test that duplicate names within the same tenant are handled. + + When two SGs in the same tenant have the same name, requesting + by name should raise NoUniqueMatch, but requesting by UUID + should succeed. """ instance = objects.Instance(project_id=uuids.project_id) mock_neutron = mock.Mock(spec=client.Client) @@ -9492,14 +9493,15 @@ def test__process_security_groups_non_unique_match_same_tenant(self): 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} api = neutronapi.API() + # Requesting by name should raise NoUniqueMatch ex = self.assertRaises( exception.NoUniqueMatch, api._process_security_groups, instance, mock_neutron, ["nonunique-name"]) self.assertIn("nonunique-name", str(ex)) def test__process_security_groups_unique_uuids_same_tenant(self): - """Test that duplicate names within the same tenant are handled - when requested by UUID. + """Test that duplicate names within the same tenant are accepted + when requested by UUID (bug 2105896). """ instance = objects.Instance(project_id=uuids.project_id) mock_neutron = mock.Mock(spec=client.Client) @@ -9519,13 +9521,12 @@ def test__process_security_groups_unique_uuids_same_tenant(self): 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} api = neutronapi.API() - # FIXME(sean-k-mooney): this is bug 2105896 - # it is ok for security groups to have the same name if we - # request them by uuid. - ex = self.assertRaises( - exception.NoUniqueMatch, api._process_security_groups, + result = api._process_security_groups( instance, mock_neutron, [uuids.sg1, uuids.sg2]) - self.assertIn("nonunique-name", str(ex)) + self.assertEqual([uuids.sg1, uuids.sg2], result) + # Only one call since both SGs are in the tenant list + mock_neutron.list_security_groups.assert_called_once_with( + fields=['id', 'name'], tenant_id=uuids.project_id) @mock.patch.object(neutronapi.API, 'get_instance_nw_info') @mock.patch.object(neutronapi.API, '_update_port_dns_name') diff --git a/releasenotes/notes/bug-2105896-2bebad3d9eacd346.yaml b/releasenotes/notes/bug-2105896-2bebad3d9eacd346.yaml new file mode 100644 index 00000000000..8d74f6df1cc --- /dev/null +++ b/releasenotes/notes/bug-2105896-2bebad3d9eacd346.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + `Bug #2105896`_: Fix error when multiple security groups in a project + share the same name. The security group resolution now checks UUIDs + before names, ensuring that a request using a specific UUID is never + blocked by a naming collision. When a security group is requested by + name and multiple groups share that name, a ``NoUniqueMatch`` error is + raised prompting the user to use a UUID instead. + + .. _Bug #2105896: https://launchpad.net/bugs/2105896 From 355edcbbc66233328a9b29014cf69e081f6d663e Mon Sep 17 00:00:00 2001 From: Sylvain Bauza Date: Tue, 12 May 2026 14:31:14 +0200 Subject: [PATCH 3/3] Strip internal _nova-prefixed scheduler hints on create User-supplied scheduler hints can include internal keys like "_nova_check_type" which cause the scheduler to bypass Placement candidate selection, request pre-filters, and resource claims. This can lead to instances being created without proper resource accounting. Rather than rejecting the request, silently strip any _nova-prefixed hints before they reach the scheduler. This is consistent with the existing hints behavior of ignoring unknown ones and ensures the probe attempt still costs the attacker money. Assisted-By: Cursor Change-Id: Iac4fef93bef0bab3060d40a9ea3e0ebd69a38c37 Closes-Bug: #2151252 Signed-off-by: Sylvain Bauza (cherry picked from commit 9666894b46db4c2c66824f1b6e89584de3ab17b2) (cherry picked from commit 32bd8471f86fa33555dc9890b8730d83296da7b0) (cherry picked from commit 0e969c5246e3f99b48ed99b58438fc11891b1caa) --- nova/compute/api.py | 4 ++++ nova/tests/unit/compute/test_api.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/nova/compute/api.py b/nova/compute/api.py index 47c6b27d855..9515ef9d271 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2232,6 +2232,10 @@ def create( msg = _('The requested availability zone is not available') raise exception.InvalidRequest(msg) + if scheduler_hints: + scheduler_hints = {k: v for k, v in scheduler_hints.items() + if not k.startswith('_nova')} + filter_properties = scheduler_utils.build_filter_properties( scheduler_hints, forced_host, forced_node, flavor) diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py index 8cc4d4d26da..21c41f29f45 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -220,6 +220,24 @@ def _obj_to_list_obj(self, list_obj, obj): list_obj.obj_reset_changes() return list_obj + @mock.patch('nova.scheduler.utils.build_filter_properties') + def test_create_strips_internal_scheduler_hints(self, + mock_build_filter): + mock_build_filter.side_effect = ( + test.TestingException('stop early')) + flavor = self._create_flavor() + self.assertRaises( + test.TestingException, + self.compute_api.create, + self.context, flavor, 'image_id', + scheduler_hints={ + '_nova_check_type': 'rebuild', + '_nova_future': 'something', + 'group': 'valid-group-uuid', + }) + actual_hints = mock_build_filter.call_args[0][0] + self.assertEqual({'group': 'valid-group-uuid'}, actual_hints) + @mock.patch( 'nova.network.neutron.API.is_remote_managed_port', new=mock.Mock(return_value=False),