Merge "nova-net: Remove unused parameters"

nova/api/openstack/compute/security_groups.py

@@ -159,8 +159,7 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller):
 
         try:
             id = security_group_api.validate_id(id)
-            security_group = security_group_api.get(
+            security_group = security_group_api.get(context, id)
-                context, None, id, map_exception=True)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.Invalid as exp:
@@ -178,8 +177,7 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller):
 
         try:
             id = security_group_api.validate_id(id)
-            security_group = security_group_api.get(
+            security_group = security_group_api.get(context, id)
-                context, None, id, map_exception=True)
             security_group_api.destroy(context, security_group)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
@@ -222,7 +220,7 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller):
         try:
             security_group_api.validate_property(group_name, 'name', None)
             security_group_api.validate_property(group_description,
-                                                      'description', None)
+                                                 'description', None)
             group_ref = security_group_api.create_security_group(
                 context, group_name, group_description)
         except exception.Invalid as exp:
@@ -241,8 +239,7 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller):
 
         try:
             id = security_group_api.validate_id(id)
-            security_group = security_group_api.get(
+            security_group = security_group_api.get(context, id)
-                context, None, id, map_exception=True)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.Invalid as exp:
@@ -283,7 +280,7 @@ class SecurityGroupRulesController(SecurityGroupControllerBase,
             parent_group_id = security_group_api.validate_id(
                 sg_rule.get('parent_group_id'))
             security_group = security_group_api.get(
-                context, None, parent_group_id, map_exception=True)
+                context, parent_group_id)
             if group_id is not None:
                 group_id = security_group_api.validate_id(group_id)
 
@@ -354,8 +351,7 @@ class SecurityGroupRulesController(SecurityGroupControllerBase,
             id = security_group_api.validate_id(id)
             rule = security_group_api.get_rule(context, id)
             group_id = rule['parent_group_id']
-            security_group = security_group_api.get(
+            security_group = security_group_api.get(context, group_id)
-                context, None, group_id, map_exception=True)
             security_group_api.remove_rules(
                 context, security_group, [rule['id']])
         except exception.SecurityGroupNotFound as exp:

nova/compute/api.py

@@ -385,13 +385,10 @@ class API(base.Base):
 
         :param context: The nova request context.
         :type context: nova.context.RequestContext
-        :param secgroups: list of requested security group names, or uuids in
+        :param secgroups: list of requested security group names
-            the case of Neutron.
         :type secgroups: list
-        :returns: list of requested security group names unmodified if using
+        :returns: list of requested security group UUIDs; note that 'default'
-            nova-network. If using Neutron, the list returned is all uuids.
+            is a special case and will be unmodified if it's requested.
-            Note that 'default' is a special case and will be unmodified if
-            it's requested.
         """
         security_groups = []
         for secgroup in secgroups:
@@ -399,12 +396,8 @@ class API(base.Base):
             if secgroup == "default":
                 security_groups.append(secgroup)
                 continue
-            secgroup_dict = security_group_api.get(context, secgroup)
+            secgroup_uuid = security_group_api.validate_name(context, secgroup)
-            if not secgroup_dict:
+            security_groups.append(secgroup_uuid)
-                raise exception.SecurityGroupNotFoundForProject(
-                    project_id=context.project_id, security_group_id=secgroup)
-
-            security_groups.append(secgroup_dict['id'])
 
         return security_groups
 
@@ -922,17 +915,17 @@ class API(base.Base):
 
         # When using Neutron, _check_requested_secgroups will translate and
         # return any requested security group names to uuids.
-        security_groups = (
+        security_groups = self._check_requested_secgroups(
-            self._check_requested_secgroups(context, security_groups))
+            context, security_groups)
 
         # Note:  max_count is the number of instances requested by the user,
         # max_network_count is the maximum number of instances taking into
         # account any network quotas
-        max_network_count = self._check_requested_networks(context,
+        max_network_count = self._check_requested_networks(
-                                     requested_networks, max_count)
+            context, requested_networks, max_count)
 
         kernel_id, ramdisk_id = self._handle_kernel_and_ramdisk(
-                context, kernel_id, ramdisk_id, boot_meta)
+            context, kernel_id, ramdisk_id, boot_meta)
 
         config_drive = self._check_config_drive(config_drive)
 

nova/network/security_group_api.py

@@ -31,6 +31,7 @@ import six
 from six.moves import urllib
 from webob import exc
 
+from nova import context as nova_context
 from nova import exception
 from nova.i18n import _
 from nova.network import neutron as neutronapi
@@ -53,6 +54,33 @@ def validate_id(id):
     return id
 
 
+def validate_name(
+        context: nova_context.RequestContext,
+        name: str):
+    """Validate a security group name and return the corresponding UUID.
+
+    :param context: The nova request context.
+    :param name: The name of the security group.
+    :raises NoUniqueMatch: If there is no unique match for the provided name.
+    :raises SecurityGroupNotFound: If there's no match for the provided name.
+    :raises NeutronClientException: For all other exceptions.
+    """
+    neutron = neutronapi.get_client(context)
+    try:
+        return neutronv20.find_resourceid_by_name_or_id(
+            neutron, 'security_group', name, context.project_id)
+    except n_exc.NeutronClientNoUniqueMatch as e:
+        raise exception.NoUniqueMatch(six.text_type(e))
+    except n_exc.NeutronClientException as e:
+        exc_info = sys.exc_info()
+        if e.status_code == 404:
+            LOG.debug('Neutron security group %s not found', name)
+            raise exception.SecurityGroupNotFound(six.text_type(e))
+        else:
+            LOG.error('Neutron Error: %s', e)
+            six.reraise(*exc_info)
+
+
 def parse_cidr(cidr):
     if not cidr:
         return '0.0.0.0/0'
@@ -280,44 +308,26 @@ def _convert_to_nova_security_group_rule_format(rule):
     return nova_rule
 
 
-def get(context, name=None, id=None, map_exception=False):
+def get(context, id):
     neutron = neutronapi.get_client(context)
     try:
-        if not id and name:
-            # NOTE(flwang): The project id should be honoured so as to get
-            # the correct security group id when user(with admin role but
-            # non-admin project) try to query by name, so as to avoid
-            # getting more than duplicated records with the same name.
-            id = neutronv20.find_resourceid_by_name_or_id(
-                neutron, 'security_group', name, context.project_id)
         group = neutron.show_security_group(id).get('security_group')
         return _convert_to_nova_security_group_format(group)
-    except n_exc.NeutronClientNoUniqueMatch as e:
-        raise exception.NoUniqueMatch(six.text_type(e))
     except n_exc.NeutronClientException as e:
         exc_info = sys.exc_info()
         if e.status_code == 404:
-            LOG.debug("Neutron security group %s not found", name)
+            LOG.debug('Neutron security group %s not found', id)
             raise exception.SecurityGroupNotFound(six.text_type(e))
         else:
             LOG.error("Neutron Error: %s", e)
             six.reraise(*exc_info)
-    except TypeError as e:
-        LOG.error("Neutron Error: %s", e)
-        msg = _("Invalid security group name: %(name)s.") % {"name": name}
-        raise exception.SecurityGroupNotFound(six.text_type(msg))
 
 
-def list(context, names=None, ids=None, project=None,
+def list(context, project, search_opts=None):
-         search_opts=None):
     """Returns list of security group rules owned by tenant."""
     neutron = neutronapi.get_client(context)
     params = {}
     search_opts = search_opts if search_opts else {}
-    if names:
-        params['name'] = names
-    if ids:
-        params['id'] = ids
 
     # NOTE(jeffrey4l): list all the security groups when following
     # conditions are met
@@ -325,23 +335,25 @@ def list(context, names=None, ids=None, project=None,
     #   * it is admin context and all_tenants exist in search_opts.
     #   * project is not specified.
     list_all_tenants = (context.is_admin and
-                        'all_tenants' in search_opts and
+                        'all_tenants' in search_opts)
-                        not any([names, ids]))
+    # NOTE(jeffrey4l): neutron doesn't have `all-tenants` concept.
-    # NOTE(jeffrey4l): The neutron doesn't have `all-tenants` concept.
     # All the security group will be returned if the project/tenant
     # id is not passed.
-    if project and not list_all_tenants:
+    if not list_all_tenants:
         params['tenant_id'] = project
+
     try:
         security_groups = neutron.list_security_groups(**params).get(
             'security_groups')
     except n_exc.NeutronClientException:
         with excutils.save_and_reraise_exception():
             LOG.exception("Neutron Error getting security groups")
+
     converted_rules = []
     for security_group in security_groups:
         converted_rules.append(
             _convert_to_nova_security_group_format(security_group))
+
     return converted_rules
 
 

nova/tests/unit/compute/test_compute.py

@@ -8704,12 +8704,11 @@ class ComputeAPITestCase(BaseTestCase):
 
     def test_create_instance_associates_security_groups(self):
         # Make sure create associates security groups.
-        group = {'id': uuids.secgroup_id, 'name': 'testgroup'}
         with test.nested(
                 mock.patch.object(self.compute_api.compute_task_api,
                                   'schedule_and_build_instances'),
-                mock.patch('nova.network.security_group_api.get',
+                mock.patch('nova.network.security_group_api.validate_name',
-                          return_value=group),
+                           return_value=uuids.secgroup_id),
         ) as (mock_sbi, mock_secgroups):
             self.compute_api.create(
                 self.context,
@@ -8721,14 +8720,16 @@ class ComputeAPITestCase(BaseTestCase):
             reqspec = build_call[1]['request_spec'][0]
 
         self.assertEqual(1, len(reqspec.security_groups))
-        self.assertEqual(group['id'], reqspec.security_groups[0].uuid)
+        self.assertEqual(uuids.secgroup_id, reqspec.security_groups[0].uuid)
         mock_secgroups.assert_called_once_with(mock.ANY, 'testgroup')
 
     def test_create_instance_with_invalid_security_group_raises(self):
         pre_build_len = len(db.instance_get_all(self.context))
-        with mock.patch('nova.network.security_group_api.get',
+        with mock.patch(
-                        return_value=None) as mock_secgroups:
+            'nova.network.security_group_api.validate_name',
-            self.assertRaises(exception.SecurityGroupNotFoundForProject,
+            side_effect=exception.SecurityGroupNotFound('foo'),
+        ) as mock_secgroups:
+            self.assertRaises(exception.SecurityGroupNotFound,
                               self.compute_api.create,
                               self.context,
                               instance_type=self.default_flavor,

nova/tests/unit/compute/test_compute_api.py

@@ -6708,7 +6708,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
             access_ip_v4 = access_ip_v6 = config_drive = \
                 auto_disk_config = reservation_id = None
         # This tests that 'default' is unchanged, but 'fake-security-group'
-        # will be translated to a uuid for Neutron.
+        # will be translated to a UUID for Neutron.
         requested_secgroups = ['default', 'fake-security-group']
         # This will short-circuit _check_requested_networks
         requested_networks = objects.NetworkRequestList(objects=[
@@ -6716,8 +6716,8 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
         max_count = 1
         supports_port_resource_request = False
         with mock.patch(
-                'nova.network.security_group_api.get',
+                'nova.network.security_group_api.validate_name',
-                return_value={'id': uuids.secgroup_uuid}) as scget:
+                return_value=uuids.secgroup_uuid) as scget:
             base_options, max_network_count, key_pair, security_groups, \
                     network_metadata = (
                 self.compute_api._validate_and_build_base_options(

nova/tests/unit/network/test_security_group.py

@@ -78,47 +78,6 @@ class TestNeutronDriver(test.NoDBTestCase):
 
             mock_list_secgroup.assert_called_once_with(tenant_id=project_id)
 
-    def test_list_with_all_tenants_sec_name_and_admin_context(self):
-        project_id = '0af70a4d22cf4652824ddc1f2435dd85'
-        search_opts = {'all_tenants': 1}
-        security_group_names = ['secgroup_ssh']
-        security_groups_list = {'security_groups': []}
-        admin_context = context.RequestContext('user1', project_id, True)
-
-        with mock.patch.object(
-                self.mocked_client,
-                'list_security_groups',
-                return_value=security_groups_list) as mock_list_secgroup:
-            sg_api.list(admin_context, project=project_id,
-                        names=security_group_names,
-                        search_opts=search_opts)
-
-            mock_list_secgroup.assert_called_once_with(
-                name=security_group_names,
-                tenant_id=project_id)
-
-    def test_list_with_all_tenants_sec_name_ids_and_admin_context(self):
-        project_id = '0af70a4d22cf4652824ddc1f2435dd85'
-        search_opts = {'all_tenants': 1}
-        security_group_names = ['secgroup_ssh']
-        security_group_ids = ['id1']
-        security_groups_list = {'security_groups': []}
-        admin_context = context.RequestContext('user1', project_id, True)
-
-        with mock.patch.object(
-                self.mocked_client,
-                'list_security_groups',
-                return_value=security_groups_list) as mock_list_secgroup:
-            sg_api.list(admin_context, project=project_id,
-                        names=security_group_names,
-                        ids=security_group_ids,
-                        search_opts=search_opts)
-
-            mock_list_secgroup.assert_called_once_with(
-                name=security_group_names,
-                id=security_group_ids,
-                tenant_id=project_id)
-
     def test_list_with_all_tenants_not_admin(self):
         search_opts = {'all_tenants': 1}
         security_groups_list = {'security_groups': []}
@@ -133,36 +92,6 @@ class TestNeutronDriver(test.NoDBTestCase):
             mock_list_secgroup.assert_called_once_with(
                 tenant_id=self.context.project_id)
 
-    def test_get_with_name_duplicated(self):
-        sg_name = 'web_server'
-        expected_sg_id = '85cc3048-abc3-43cc-89b3-377341426ac5'
-        expected_sg = {'security_group': {'name': sg_name,
-                                 'id': expected_sg_id,
-                                 'tenant_id': self.context.project_id,
-                                 'description': 'server', 'rules': []}}
-        self.mocked_client.show_security_group.return_value = expected_sg
-
-        with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id',
-                               return_value=expected_sg_id):
-            observed_sg = sg_api.get(self.context, name=sg_name)
-        expected_sg['security_group']['project_id'] = self.context.project_id
-        del expected_sg['security_group']['tenant_id']
-        self.assertEqual(expected_sg['security_group'], observed_sg)
-        self.mocked_client.show_security_group.assert_called_once_with(
-            expected_sg_id)
-
-    def test_get_with_invalid_name(self):
-        sg_name = 'invalid_name'
-        expected_sg_id = '85cc3048-abc3-43cc-89b3-377341426ac5'
-        self.mocked_client.show_security_group.side_effect = TypeError
-
-        with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id',
-                               return_value=expected_sg_id):
-            self.assertRaises(exception.SecurityGroupNotFound,
-                              sg_api.get, self.context, name=sg_name)
-        self.mocked_client.show_security_group.assert_called_once_with(
-            expected_sg_id)
-
     def test_create_security_group_with_bad_request(self):
         name = 'test-security-group'
         description = None
@@ -228,6 +157,7 @@ class TestNeutronDriver(test.NoDBTestCase):
             body)
 
     def test_list_security_group_with_no_port_range_and_not_tcp_udp_icmp(self):
+        project_id = '0af70a4d22cf4652824ddc1f2435dd85'
         sg1 = {'description': 'default',
                'id': '07f1362f-34f6-4136-819a-2dcde112269e',
                'name': 'default',
@@ -247,7 +177,7 @@ class TestNeutronDriver(test.NoDBTestCase):
 
         self.mocked_client.list_security_groups.return_value = (
             {'security_groups': [sg1]})
-        result = sg_api.list(self.context)
+        result = sg_api.list(self.context, project=project_id)
         expected = [{'rules':
             [{'from_port': -1, 'protocol': '51', 'to_port': -1,
               'parent_group_id': '07f1362f-34f6-4136-819a-2dcde112269e',
@@ -257,7 +187,8 @@ class TestNeutronDriver(test.NoDBTestCase):
             'id': '07f1362f-34f6-4136-819a-2dcde112269e',
             'name': 'default', 'description': 'default'}]
         self.assertEqual(expected, result)
-        self.mocked_client.list_security_groups.assert_called_once_with()
+        self.mocked_client.list_security_groups.assert_called_once_with(
+            tenant_id=project_id)
 
     def test_instances_security_group_bindings(self, detailed=False):
         server_id = 'c5a20e8d-c4b0-47cf-9dca-ebe4f758acb1'