Merge "Support creating servers with RBAC SGs"

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
 

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:
+            try:
-                name_match = None
+                security_group_ids = self._get_security_group_ids(
-                uuid_match = None
+                    security_groups, user_security_groups)
-                for user_security_group in user_security_groups:
+            except exception.SecurityGroupNotFound:
-                    if user_security_group['name'] == security_group:
+                # Trigger a raise if the shared filter extension is not loaded,
-                        # If there was a name match in a previous iteration
+                # else we will trigger on the second call below when we pass
-                        # of the loop, we have a conflict.
+                # any shared security groups.
-                        if name_match:
+                if not sg_filter_ext:
-                            raise exception.NoUniqueMatch(
+                    raise
-                                _("Multiple security groups found matching"
-                                  " '%s'. Use an ID to be more specific.") %
-                                   security_group)
 
-                        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:
+                security_group_ids = self._get_security_group_ids(
-                        uuid_match = user_security_group['id']
+                    security_groups, user_security_groups)
-
-                # 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)
 
         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

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 = {
+        mock_neutron.list_security_groups.side_effect = [
-            'security_groups': [
+            {
-                {
+                'security_groups': [
-                    'id': uuids.sg1,
+                    {
-                    'name': 'sg1',
+                        'id': uuids.sg1,
-                },
+                        'name': 'sg1',
-                {
+                    }
-                    'id': uuids.sg3,
+                ]
-                    'name': 'sg3',
+            },
-                }
+            {
-            ]
+                '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(
+        mock_neutron.list_security_groups.assert_has_calls(
-            fields=['id', 'name'], tenant_id=uuids.project_id)
+            [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 = {
+        mock_neutron.list_security_groups.side_effect = [
-            'security_groups': [
+            {
-                {
+                'security_groups': [
-                    'id': uuids.sg1,
+                    {
-                    'name': 'nonunique-name',
+                        'id': uuids.sg1,
-                },
+                        'name': 'nonunique-name',
-                {
+                    }
-                    'id': uuids.sg2,
+                ]
-                    '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(
+        mock_neutron.list_security_groups.assert_has_calls(
-            fields=['id', 'name'], tenant_id=uuids.project_id)
+            [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')

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