openstack/nova
Code Issues Proposed changes

nova-net: Remove unused parameters

Browse Source 
We only care about neutron security groups now, so a lot of nova-network
only cruft can be removed. Do just that.

Change-Id: I2a360e766261a186f9edf6ceb47a786aea2957eb
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
This commit is contained in:
Stephen Finucane 2020-01-23 10:38:39 +00:00
parent 998475f5bd
commit 7e0d2547c1
6 changed files with 68 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
nova/api/openstack/compute/security_groups.py
View File

@ -159,8 +159,7 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller):
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 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller):
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 @@ 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(
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 @@ 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(
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:

27
nova/compute/api.py
View File

@ -384,13 +384,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
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:
@ -398,12 +395,8 @@ class API(base.Base):
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
@ -895,17 +888,17 @@ class API(base.Base):
# 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)

62
nova/network/security_group_api.py
View File

@ -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,
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
@ -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
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

15
nova/tests/unit/compute/test_compute.py
View File

@ -8702,12 +8702,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',
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,
@ -8719,14 +8718,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',
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,

6
nova/tests/unit/compute/test_compute_api.py
View File

@ -6612,7 +6612,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=[
@ -6620,8 +6620,8 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
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(

77
nova/tests/unit/network/test_security_group.py
View File

@ -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'