diff --git a/nova/network/constants.py b/nova/network/constants.py index d98e217f95d1..2f1c0a99c559 100644 --- a/nova/network/constants.py +++ b/nova/network/constants.py @@ -36,6 +36,7 @@ PORT_BINDING_EXTENDED = 'binding-extended' SUBSTR_PORT_FILTERING = 'ip-substring-filtering' SEGMENT = 'segment' RESOURCE_REQUEST_GROUPS = 'port-resource-request-groups' +SG_SHARED_FILTER = "security-groups-shared-filtering" # Third-party extensions diff --git a/nova/network/neutron.py b/nova/network/neutron.py index 9ce3ab6191f2..f24177de15d4 100644 --- a/nova/network/neutron.py +++ b/nova/network/neutron.py @@ -817,6 +817,63 @@ class API: security_groups = [] return security_groups + def _get_security_group_ids(self, security_groups, user_security_groups): + """Processes requested security groups based on existing user groups + + :param security_groups: list of security group names or IDs + :param user_security_groups: list of Neutron security groups found + :return: list of security group IDs + :raises nova.exception.NoUniqueMatch: If multiple security groups + are requested with the same name. + :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 = {} + # 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 + # prediction for the second for loop less reliable. + id_to_id = {} + + # Populate the dictionaries with 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 + # Map the ID to itself for easy lookup + id_to_id[sg_id] = sg_id + + # Initialize an empty list to store the resulting security group IDs + security_group_ids = [] + + # 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: + security_group_ids.append(id_to_id[security_group]) + # Raise an exception if the security group is not found in + # either dictionary + else: + raise exception.SecurityGroupNotFound( + security_group_id=security_group) + + # Return the list of security group IDs + return security_group_ids + def _process_security_groups(self, instance, neutron, security_groups): """Processes and validates requested security groups for allocation. @@ -848,36 +905,28 @@ class API: # got many security groups sg_fields = ['id', 'name'] search_opts = {'tenant_id': instance.project_id} + sg_filter_ext = self.has_sg_shared_filter_extension(client=neutron) user_security_groups = neutron.list_security_groups( fields=sg_fields, **search_opts).get('security_groups') - for security_group in security_groups: - name_match = None - uuid_match = None - for user_security_group in user_security_groups: - if user_security_group['name'] == security_group: - # If there was a name match in a previous iteration - # of the loop, we have a conflict. - if name_match: - raise exception.NoUniqueMatch( - _("Multiple security groups found matching" - " '%s'. Use an ID to be more specific.") % - security_group) + try: + security_group_ids = self._get_security_group_ids( + security_groups, user_security_groups) + except exception.SecurityGroupNotFound: + # Trigger a raise if the shared filter extension is not loaded, + # else we will trigger on the second call below when we pass + # any shared security groups. + if not sg_filter_ext: + raise - name_match = user_security_group['id'] + # NOTE(hangyang): Make another request to get the RBAC shared + # SGs accessible to the tenant + search_opts = {'shared': True} + user_security_groups += neutron.list_security_groups( + fields=sg_fields, **search_opts).get('security_groups') - if user_security_group['id'] == security_group: - uuid_match = user_security_group['id'] - - # If a user names the security group the same as - # another's security groups uuid, the name takes priority. - if name_match: - security_group_ids.append(name_match) - elif uuid_match: - security_group_ids.append(uuid_match) - else: - raise exception.SecurityGroupNotFound( - security_group_id=security_group) + security_group_ids = self._get_security_group_ids( + security_groups, user_security_groups) return security_group_ids @@ -1413,6 +1462,14 @@ class API: """ return self._has_extension(constants.DNS_INTEGRATION, context, client) + def has_sg_shared_filter_extension(self, context=None, client=None): + """Check if the 'security-groups-shared-filtering' extension is + enabled. + + This extension adds a 'shared' filter to security group APIs. + """ + return self._has_extension(constants.SG_SHARED_FILTER, context, client) + # TODO(gibi): Remove all branches where this is False after Neutron made # the this extension mandatory. In Xena this extension will be optional to # support the scenario where Neutron upgraded first. So Neutron can mark diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py index 767f83363999..fa794cb012bc 100644 --- a/nova/tests/unit/network/test_neutron.py +++ b/nova/tests/unit/network/test_neutron.py @@ -9272,7 +9272,7 @@ class TestNeutronClientForAdminScenarios(test.NoDBTestCase): class TestNeutronPortSecurity(test.NoDBTestCase): - def test__process_security_groups(self): + def test__process_security_groups_without_shared(self): instance = objects.Instance(project_id=uuids.project_id) mock_neutron = mock.Mock(spec=client.Client) mock_neutron.list_security_groups.return_value = { @@ -9291,6 +9291,8 @@ class TestNeutronPortSecurity(test.NoDBTestCase): } ] } + mock_neutron.list_extensions.return_value = { + 'extensions': []} api = neutronapi.API() api._process_security_groups( instance, mock_neutron, ["sg1", uuids.sg2]) @@ -9298,21 +9300,63 @@ class TestNeutronPortSecurity(test.NoDBTestCase): mock_neutron.list_security_groups.assert_called_once_with( fields=['id', 'name'], tenant_id=uuids.project_id) + def test__process_security_groups(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': 'sg1', + }, + { + 'id': uuids.sg2, + 'name': 'sg2', + } + ] + }, + { + 'security_groups': [ + { + 'id': uuids.sg3, + 'name': 'sg3', + }, + ] + } + ] + mock_neutron.list_extensions.return_value = { + 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} + api = neutronapi.API() + api._process_security_groups( + instance, mock_neutron, ["sg1", uuids.sg2]) + + mock_neutron.list_security_groups.assert_has_calls( + [mock.call(fields=['id', 'name'], tenant_id=uuids.project_id)]) + def test__process_security_groups_not_found(self): 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': 'sg1', - }, - { - 'id': uuids.sg3, - 'name': 'sg3', - } - ] - } + mock_neutron.list_security_groups.side_effect = [ + { + 'security_groups': [ + { + 'id': uuids.sg1, + 'name': 'sg1', + } + ] + }, + { + 'security_groups': [ + { + 'id': uuids.sg3, + 'name': 'sg3', + } + ] + } + ] + mock_neutron.list_extensions.return_value = { + 'extensions': [{'alias': constants.SG_SHARED_FILTER}]} api = neutronapi.API() ex = self.assertRaises( @@ -9320,24 +9364,33 @@ class TestNeutronPortSecurity(test.NoDBTestCase): instance, mock_neutron, ["sg1", uuids.sg2]) self.assertIn(uuids.sg2, str(ex)) - mock_neutron.list_security_groups.assert_called_once_with( - fields=['id', 'name'], tenant_id=uuids.project_id) + 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(self): 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_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() ex = self.assertRaises( @@ -9345,8 +9398,9 @@ class TestNeutronPortSecurity(test.NoDBTestCase): instance, mock_neutron, ["nonunique-name", uuids.sg2]) self.assertIn("nonunique-name", str(ex)) - mock_neutron.list_security_groups.assert_called_once_with( - fields=['id', 'name'], tenant_id=uuids.project_id) + 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)]) @mock.patch.object(neutronapi.API, 'get_instance_nw_info') @mock.patch.object(neutronapi.API, '_update_port_dns_name') diff --git a/releasenotes/notes/support-shared-security-groups-3651e1e1f56cfb7b.yaml b/releasenotes/notes/support-shared-security-groups-3651e1e1f56cfb7b.yaml new file mode 100644 index 000000000000..babb9bd56ab2 --- /dev/null +++ b/releasenotes/notes/support-shared-security-groups-3651e1e1f56cfb7b.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Support creating servers with RBAC shared security groups by using the new + ``shared`` filter for security groups. See `blueprint + shared-security-groups`_ for more details. + + .. _blueprint shared-security-groups: + https://blueprints.launchpad.net/nova/+spec/shared-security-groups