Merge "nova-net: Remove unused parameters"

nova/api/openstack/compute/security_groups.py

@@ -159,8 +159,7 @@ def show(self, req, id):
 
         try:
             id = security_group_api.validate_id(id)
-            security_group = security_group_api.get(
-                context, None, id, map_exception=True)
+            security_group = security_group_api.get(context, id)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.Invalid as exp:
@@ -178,8 +177,7 @@ def delete(self, req, id):
 
         try:
             id = security_group_api.validate_id(id)
-            security_group = security_group_api.get(
-                context, None, id, map_exception=True)
+            security_group = security_group_api.get(context, id)
             security_group_api.destroy(context, security_group)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
@@ -222,7 +220,7 @@ def create(self, req, body):
         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 @@ def update(self, req, id, body):
 
         try:
             id = security_group_api.validate_id(id)
-            security_group = security_group_api.get(
-                context, None, id, map_exception=True)
+            security_group = security_group_api.get(context, id)
         except exception.SecurityGroupNotFound as exp:
             raise exc.HTTPNotFound(explanation=exp.format_message())
         except exception.Invalid as exp:
@@ -283,7 +280,7 @@ def create(self, req, body):
             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 @@ def delete(self, req, id):
             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(
-                context, None, group_id, map_exception=True)
+            security_group = security_group_api.get(context, group_id)
             security_group_api.remove_rules(
                 context, security_group, [rule['id']])
         except exception.SecurityGroupNotFound as exp:

nova/compute/api.py

@@ -385,26 +385,19 @@ def _check_requested_secgroups(self, context, secgroups):
 
         :param context: The nova request context.
         :type context: nova.context.RequestContext
-        :param secgroups: list of requested security group names, or uuids in
-            the case of Neutron.
+        :param secgroups: list of requested security group names
         :type secgroups: list
-        :returns: list of requested security group names unmodified if using
-            nova-network. If using Neutron, the list returned is all uuids.
-            Note that 'default' is a special case and will be unmodified if
-            it's requested.
+        :returns: list of requested security group UUIDs; note that 'default'
+            is a special case and will be unmodified if it's requested.
         """
         security_groups = []
         for secgroup in secgroups:
             # NOTE(sdague): default is handled special
             if secgroup == "default":
                 security_groups.append(secgroup)
                 continue
-            secgroup_dict = security_group_api.get(context, secgroup)
-            if not secgroup_dict:
-                raise exception.SecurityGroupNotFoundForProject(
-                    project_id=context.project_id, security_group_id=secgroup)
-
-            security_groups.append(secgroup_dict['id'])
+            secgroup_uuid = security_group_api.validate_name(context, secgroup)
+            security_groups.append(secgroup_uuid)
 
         return security_groups
 
@@ -922,17 +915,17 @@ def _validate_and_build_base_options(self, context, instance_type,
 
         # When using Neutron, _check_requested_secgroups will translate and
         # return any requested security group names to uuids.
-        security_groups = (
-            self._check_requested_secgroups(context, security_groups))
+        security_groups = self._check_requested_secgroups(
+            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,
-                                     requested_networks, max_count)
+        max_network_count = self._check_requested_networks(
+            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 @@
 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,68 +308,52 @@ 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,
-         search_opts=None):
+def list(context, project, 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
     #   * names and ids don't exist.
     #   * 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
-                        not any([names, ids]))
-    # NOTE(jeffrey4l): The neutron doesn't have `all-tenants` concept.
+                        'all_tenants' in search_opts)
+    # NOTE(jeffrey4l): 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 @@ def test_create_saves_flavor(self):
 
     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',
-                          return_value=group),
+                mock.patch('nova.network.security_group_api.validate_name',
+                           return_value=uuids.secgroup_id),
         ) as (mock_sbi, mock_secgroups):
             self.compute_api.create(
                 self.context,
@@ -8721,14 +8720,16 @@ def test_create_instance_associates_security_groups(self):
             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',
-                        return_value=None) as mock_secgroups:
-            self.assertRaises(exception.SecurityGroupNotFoundForProject,
+        with mock.patch(
+            'nova.network.security_group_api.validate_name',
+            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,16 +6708,16 @@ def test_validate_and_build_base_options_translate_neutron_secgroup(self):
             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=[
             objects.NetworkRequest(network_id='none')])
         max_count = 1
         supports_port_resource_request = False
         with mock.patch(
-                'nova.network.security_group_api.get',
-                return_value={'id': uuids.secgroup_uuid}) as scget:
+                'nova.network.security_group_api.validate_name',
+                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(