From 110a683486b750488ee147b16a17451750af4c5e Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 29 Nov 2019 11:17:06 +0000 Subject: [PATCH] nova-net: Make the security group API a module We're wrestling with multiple imports for this thing and have introduced a cache to avoid having to load the thing repeatedly. However, Python already has a way to ensure this doesn't happen: the use of a module. Given that we don't have any state, we can straight up drop the class and just call functions directly. Along the way, we drop the 'ensure_default' function, which is a no-op for neutron and switch all the mocks over, where necessary. Change-Id: Ia8dbe8ba61ec6d1b8498918a53a103a6eff4d488 Signed-off-by: Stephen Finucane --- nova/api/metadata/base.py | 5 +- nova/api/openstack/compute/security_groups.py | 99 ++- nova/api/openstack/compute/views/servers.py | 9 +- nova/compute/api.py | 11 +- nova/network/security_group/__init__.py | 0 nova/network/security_group/neutron_driver.py | 556 -------------- .../security_group/openstack_driver.py | 30 - .../security_group/security_group_base.py | 253 ------ nova/network/security_group_api.py | 717 ++++++++++++++++++ nova/test.py | 3 - .../api_sample_tests/test_security_groups.py | 21 +- .../openstack/compute/test_security_groups.py | 29 +- nova/tests/unit/api/openstack/fakes.py | 35 +- nova/tests/unit/compute/test_compute.py | 10 +- nova/tests/unit/compute/test_compute_api.py | 32 +- nova/tests/unit/network/test_config.py | 29 - ...utron_driver.py => test_security_group.py} | 25 +- 17 files changed, 827 insertions(+), 1037 deletions(-) delete mode 100644 nova/network/security_group/__init__.py delete mode 100644 nova/network/security_group/neutron_driver.py delete mode 100644 nova/network/security_group/openstack_driver.py delete mode 100644 nova/network/security_group/security_group_base.py create mode 100644 nova/network/security_group_api.py delete mode 100644 nova/tests/unit/network/test_config.py rename nova/tests/unit/network/{security_group/test_neutron_driver.py => test_security_group.py} (94%) diff --git a/nova/api/metadata/base.py b/nova/api/metadata/base.py index 311a78a8b914..ccc72da14415 100644 --- a/nova/api/metadata/base.py +++ b/nova/api/metadata/base.py @@ -33,7 +33,7 @@ import nova.conf from nova import context from nova import exception from nova.network import neutron -from nova.network.security_group import openstack_driver +from nova.network import security_group_api from nova import objects from nova.objects import virt_device_metadata as metadata_obj from nova import utils @@ -140,8 +140,7 @@ class InstanceMetadata(object): self.availability_zone = instance.get('availability_zone') - secgroup_api = openstack_driver.get_openstack_security_group_driver() - self.security_groups = secgroup_api.get_instance_security_groups( + self.security_groups = security_group_api.get_instance_security_groups( ctxt, instance) self.mappings = _format_instance_mapping(ctxt, instance) diff --git a/nova/api/openstack/compute/security_groups.py b/nova/api/openstack/compute/security_groups.py index 5ac636947a56..90942beea1c2 100644 --- a/nova/api/openstack/compute/security_groups.py +++ b/nova/api/openstack/compute/security_groups.py @@ -28,7 +28,7 @@ from nova.api import validation from nova.compute import api as compute from nova import exception from nova.i18n import _ -from nova.network.security_group import openstack_driver +from nova.network import security_group_api from nova.policies import security_groups as sg_policies from nova.virt import netutils @@ -48,10 +48,7 @@ class SecurityGroupControllerBase(object): def __init__(self): super(SecurityGroupControllerBase, self).__init__() - self.security_group_api = ( - openstack_driver.get_openstack_security_group_driver()) - self.compute_api = compute.API( - security_group_api=self.security_group_api) + self.compute_api = compute.API() def _format_security_group_rule(self, context, rule, group_rule_data=None): """Return a security group rule in desired API response format. @@ -71,7 +68,7 @@ class SecurityGroupControllerBase(object): sg_rule['group'] = group_rule_data elif rule['group_id']: try: - source_group = self.security_group_api.get( + source_group = security_group_api.get( context, id=rule['group_id']) except exception.SecurityGroupNotFound: # NOTE(arosen): There is a possible race condition that can @@ -127,7 +124,7 @@ class SecurityGroupControllerBase(object): if (rule_group_id and rule_group_id not in group_rule_data_by_rule_group_id): try: - source_group = self.security_group_api.get( + source_group = security_group_api.get( context, id=rule['group_id']) group_rule_data_by_rule_group_id[rule_group_id] = { 'name': source_group.get('name'), @@ -161,9 +158,9 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller): context = _authorize_context(req) try: - id = self.security_group_api.validate_id(id) - security_group = self.security_group_api.get(context, None, id, - map_exception=True) + id = security_group_api.validate_id(id) + security_group = security_group_api.get( + context, None, id, map_exception=True) except exception.SecurityGroupNotFound as exp: raise exc.HTTPNotFound(explanation=exp.format_message()) except exception.Invalid as exp: @@ -180,10 +177,10 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller): context = _authorize_context(req) try: - id = self.security_group_api.validate_id(id) - security_group = self.security_group_api.get(context, None, id, - map_exception=True) - self.security_group_api.destroy(context, security_group) + id = security_group_api.validate_id(id) + security_group = security_group_api.get( + 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()) except exception.Invalid as exp: @@ -200,9 +197,8 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller): search_opts.update(req.GET) project_id = context.project_id - raw_groups = self.security_group_api.list(context, - project=project_id, - search_opts=search_opts) + raw_groups = security_group_api.list( + context, project=project_id, search_opts=search_opts) limited_list = common.limited(raw_groups, req) result = [self._format_security_group(context, group) @@ -224,10 +220,10 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller): group_description = security_group.get('description', None) try: - self.security_group_api.validate_property(group_name, 'name', None) - self.security_group_api.validate_property(group_description, + security_group_api.validate_property(group_name, 'name', None) + security_group_api.validate_property(group_description, 'description', None) - group_ref = self.security_group_api.create_security_group( + group_ref = security_group_api.create_security_group( context, group_name, group_description) except exception.Invalid as exp: raise exc.HTTPBadRequest(explanation=exp.format_message()) @@ -244,9 +240,9 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller): context = _authorize_context(req) try: - id = self.security_group_api.validate_id(id) - security_group = self.security_group_api.get(context, None, id, - map_exception=True) + id = security_group_api.validate_id(id) + security_group = security_group_api.get( + context, None, id, map_exception=True) except exception.SecurityGroupNotFound as exp: raise exc.HTTPNotFound(explanation=exp.format_message()) except exception.Invalid as exp: @@ -257,10 +253,10 @@ class SecurityGroupController(SecurityGroupControllerBase, wsgi.Controller): group_description = security_group_data.get('description', None) try: - self.security_group_api.validate_property(group_name, 'name', None) - self.security_group_api.validate_property(group_description, - 'description', None) - group_ref = self.security_group_api.update_security_group( + security_group_api.validate_property(group_name, 'name', None) + security_group_api.validate_property( + group_description, 'description', None) + group_ref = security_group_api.update_security_group( context, security_group, group_name, group_description) except exception.SecurityGroupNotFound as exp: raise exc.HTTPNotFound(explanation=exp.format_message()) @@ -284,15 +280,14 @@ class SecurityGroupRulesController(SecurityGroupControllerBase, source_group = {} try: - parent_group_id = self.security_group_api.validate_id( + parent_group_id = security_group_api.validate_id( sg_rule.get('parent_group_id')) - security_group = self.security_group_api.get(context, None, - parent_group_id, - map_exception=True) + security_group = security_group_api.get( + context, None, parent_group_id, map_exception=True) if group_id is not None: - group_id = self.security_group_api.validate_id(group_id) + group_id = security_group_api.validate_id(group_id) - source_group = self.security_group_api.get( + source_group = security_group_api.get( context, id=group_id) new_rule = self._rule_args_to_dict(context, to_port=sg_rule.get('to_port'), @@ -324,7 +319,7 @@ class SecurityGroupRulesController(SecurityGroupControllerBase, 'tenant_id': source_group.get('project_id')} security_group_rule = ( - self.security_group_api.create_security_group_rule( + security_group_api.create_security_group_rule( context, security_group, new_rule)) except exception.Invalid as exp: raise exc.HTTPBadRequest(explanation=exp.format_message()) @@ -342,12 +337,12 @@ class SecurityGroupRulesController(SecurityGroupControllerBase, ip_protocol=None, cidr=None, group_id=None): if group_id is not None: - return self.security_group_api.new_group_ingress_rule( - group_id, ip_protocol, from_port, to_port) + return security_group_api.new_group_ingress_rule( + group_id, ip_protocol, from_port, to_port) else: - cidr = self.security_group_api.parse_cidr(cidr) - return self.security_group_api.new_cidr_ingress_rule( - cidr, ip_protocol, from_port, to_port) + cidr = security_group_api.parse_cidr(cidr) + return security_group_api.new_cidr_ingress_rule( + cidr, ip_protocol, from_port, to_port) @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION) @wsgi.expected_errors((400, 404, 409)) @@ -356,14 +351,13 @@ class SecurityGroupRulesController(SecurityGroupControllerBase, context = _authorize_context(req) try: - id = self.security_group_api.validate_id(id) - rule = self.security_group_api.get_rule(context, id) + id = security_group_api.validate_id(id) + rule = security_group_api.get_rule(context, id) group_id = rule['parent_group_id'] - security_group = self.security_group_api.get(context, None, - group_id, - map_exception=True) - self.security_group_api.remove_rules(context, security_group, - [rule['id']]) + security_group = security_group_api.get( + context, None, group_id, map_exception=True) + security_group_api.remove_rules( + context, security_group, [rule['id']]) except exception.SecurityGroupNotFound as exp: raise exc.HTTPNotFound(explanation=exp.format_message()) except exception.NoUniqueMatch as exp: @@ -379,11 +373,9 @@ class ServerSecurityGroupController(SecurityGroupControllerBase): """Returns a list of security groups for the given instance.""" context = _authorize_context(req) - self.security_group_api.ensure_default(context) - instance = common.get_instance(self.compute_api, context, server_id) try: - groups = self.security_group_api.get_instance_security_groups( + groups = security_group_api.get_instance_security_groups( context, instance, True) except (exception.SecurityGroupNotFound, exception.InstanceNotFound) as exp: @@ -408,10 +400,7 @@ class ServerSecurityGroupController(SecurityGroupControllerBase): class SecurityGroupActionController(wsgi.Controller): def __init__(self): super(SecurityGroupActionController, self).__init__() - self.security_group_api = ( - openstack_driver.get_openstack_security_group_driver()) - self.compute_api = compute.API( - security_group_api=self.security_group_api) + self.compute_api = compute.API() def _parse(self, body, action): try: @@ -443,7 +432,7 @@ class SecurityGroupActionController(wsgi.Controller): group_name = self._parse(body, 'addSecurityGroup') try: - return self._invoke(self.security_group_api.add_to_instance, + return self._invoke(security_group_api.add_to_instance, context, id, group_name) except (exception.SecurityGroupNotFound, exception.InstanceNotFound) as exp: @@ -464,7 +453,7 @@ class SecurityGroupActionController(wsgi.Controller): group_name = self._parse(body, 'removeSecurityGroup') try: - return self._invoke(self.security_group_api.remove_from_instance, + return self._invoke(security_group_api.remove_from_instance, context, id, group_name) except (exception.SecurityGroupNotFound, exception.InstanceNotFound) as exp: diff --git a/nova/api/openstack/compute/views/servers.py b/nova/api/openstack/compute/views/servers.py index 89f765eb02bf..344c68060524 100644 --- a/nova/api/openstack/compute/views/servers.py +++ b/nova/api/openstack/compute/views/servers.py @@ -27,7 +27,7 @@ from nova.compute import api as compute from nova.compute import vm_states from nova import context as nova_context from nova import exception -from nova.network.security_group import openstack_driver +from nova.network import security_group_api from nova import objects from nova.objects import fields from nova.objects import virtual_interface @@ -70,8 +70,6 @@ class ViewBuilder(common.ViewBuilder): self._image_builder = views_images.ViewBuilder() self._flavor_builder = views_flavors.ViewBuilder() self.compute_api = compute.API() - self.security_group_api = ( - openstack_driver.get_openstack_security_group_driver()) def create(self, request, instance): """View that should be returned when an instance is created.""" @@ -657,9 +655,8 @@ class ViewBuilder(common.ViewBuilder): if not create_request: context = req.environ['nova.context'] sg_instance_bindings = ( - self.security_group_api - .get_instances_security_groups_bindings(context, - servers)) + security_group_api.get_instances_security_groups_bindings( + context, servers)) for server in servers: groups = sg_instance_bindings.get(server['id']) if groups: diff --git a/nova/compute/api.py b/nova/compute/api.py index 439d20818745..d682933a9d93 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -62,7 +62,7 @@ from nova import image from nova.network import constants from nova.network import model as network_model from nova.network import neutron -from nova.network.security_group import openstack_driver +from nova.network import security_group_api from nova import objects from nova.objects import base as obj_base from nova.objects import block_device as block_device_obj @@ -287,13 +287,11 @@ class API(base.Base): """API for interacting with the compute manager.""" def __init__(self, image_api=None, network_api=None, volume_api=None, - security_group_api=None, **kwargs): + **kwargs): self.image_api = image_api or image.API() self.network_api = network_api or neutron.API() self.volume_api = volume_api or cinder.API() self._placementclient = None # Lazy-load on first access. - self.security_group_api = (security_group_api or - openstack_driver.get_openstack_security_group_driver()) self.compute_rpcapi = compute_rpcapi.ComputeAPI() self.compute_task_api = conductor.ComputeTaskAPI() self.servicegroup_api = servicegroup.API() @@ -399,7 +397,7 @@ class API(base.Base): if secgroup == "default": security_groups.append(secgroup) continue - secgroup_dict = self.security_group_api.get(context, secgroup) + secgroup_dict = security_group_api.get(context, secgroup) if not secgroup_dict: raise exception.SecurityGroupNotFoundForProject( project_id=context.project_id, security_group_id=secgroup) @@ -1167,9 +1165,8 @@ class API(base.Base): # Check quotas num_instances = compute_utils.check_num_instances_quota( context, instance_type, min_count, max_count) - security_groups = self.security_group_api.populate_security_groups( + security_groups = security_group_api.populate_security_groups( security_groups) - self.security_group_api.ensure_default(context) port_resource_requests = base_options.pop('port_resource_requests') instances_to_build = [] # We could be iterating over several instances with several BDMs per diff --git a/nova/network/security_group/__init__.py b/nova/network/security_group/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/nova/network/security_group/neutron_driver.py b/nova/network/security_group/neutron_driver.py deleted file mode 100644 index c6f8803dcf4f..000000000000 --- a/nova/network/security_group/neutron_driver.py +++ /dev/null @@ -1,556 +0,0 @@ -# Copyright 2013 Nicira, Inc. -# All Rights Reserved -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import sys - -import netaddr -from neutronclient.common import exceptions as n_exc -from neutronclient.neutron import v2_0 as neutronv20 -from oslo_log import log as logging -from oslo_utils import excutils -from oslo_utils import uuidutils -import six -from webob import exc - -from nova import exception -from nova.i18n import _ -from nova.network import neutron as neutronapi -from nova.network.security_group import security_group_base -from nova import utils - - -LOG = logging.getLogger(__name__) - -# NOTE: Neutron client has a max URL length of 8192, so we have -# to limit the number of IDs we include in any single search. Really -# doesn't seem to be any point in making this a config value. -MAX_SEARCH_IDS = 150 - - -class SecurityGroupAPI(security_group_base.SecurityGroupBase): - - id_is_uuid = True - - def create_security_group(self, context, name, description): - neutron = neutronapi.get_client(context) - body = self._make_neutron_security_group_dict(name, description) - try: - security_group = neutron.create_security_group( - body).get('security_group') - except n_exc.BadRequest as e: - raise exception.Invalid(six.text_type(e)) - except n_exc.NeutronClientException as e: - exc_info = sys.exc_info() - LOG.exception("Neutron Error creating security group %s", name) - if e.status_code == 401: - # TODO(arosen) Cannot raise generic response from neutron here - # as this error code could be related to bad input or over - # quota - raise exc.HTTPBadRequest() - elif e.status_code == 409: - self.raise_over_quota(six.text_type(e)) - six.reraise(*exc_info) - return self._convert_to_nova_security_group_format(security_group) - - def update_security_group(self, context, security_group, - name, description): - neutron = neutronapi.get_client(context) - body = self._make_neutron_security_group_dict(name, description) - try: - security_group = neutron.update_security_group( - security_group['id'], body).get('security_group') - except n_exc.NeutronClientException as e: - exc_info = sys.exc_info() - LOG.exception("Neutron Error updating security group %s", name) - if e.status_code == 401: - # TODO(arosen) Cannot raise generic response from neutron here - # as this error code could be related to bad input or over - # quota - raise exc.HTTPBadRequest() - six.reraise(*exc_info) - return self._convert_to_nova_security_group_format(security_group) - - def validate_property(self, value, property, allowed): - """Validate given security group property. - - :param value: the value to validate, as a string or unicode - :param property: the property, either 'name' or 'description' - :param allowed: the range of characters allowed, but not used because - Neutron is allowing any characters. - """ - utils.check_string_length(value, name=property, min_length=0, - max_length=255) - - def _convert_to_nova_security_group_format(self, security_group): - nova_group = {} - nova_group['id'] = security_group['id'] - nova_group['description'] = security_group['description'] - nova_group['name'] = security_group['name'] - nova_group['project_id'] = security_group['tenant_id'] - nova_group['rules'] = [] - for rule in security_group.get('security_group_rules', []): - if rule['direction'] == 'ingress': - nova_group['rules'].append( - self._convert_to_nova_security_group_rule_format(rule)) - - return nova_group - - def _convert_to_nova_security_group_rule_format(self, rule): - nova_rule = {} - nova_rule['id'] = rule['id'] - nova_rule['parent_group_id'] = rule['security_group_id'] - nova_rule['protocol'] = rule['protocol'] - if (nova_rule['protocol'] and rule.get('port_range_min') is None and - rule.get('port_range_max') is None): - if rule['protocol'].upper() in ['TCP', 'UDP']: - nova_rule['from_port'] = 1 - nova_rule['to_port'] = 65535 - else: - nova_rule['from_port'] = -1 - nova_rule['to_port'] = -1 - else: - nova_rule['from_port'] = rule.get('port_range_min') - nova_rule['to_port'] = rule.get('port_range_max') - nova_rule['group_id'] = rule['remote_group_id'] - nova_rule['cidr'] = self.parse_cidr(rule.get('remote_ip_prefix')) - return nova_rule - - def get(self, context, name=None, id=None, map_exception=False): - 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 self._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) - 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(self, context, names=None, ids=None, project=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 - # * 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 the security group will be returned if the project/tenant - # id is not passed. - if project and 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( - self._convert_to_nova_security_group_format(security_group)) - return converted_rules - - def validate_id(self, id): - if not uuidutils.is_uuid_like(id): - msg = _("Security group id should be uuid") - self.raise_invalid_property(msg) - return id - - def destroy(self, context, security_group): - """This function deletes a security group.""" - - neutron = neutronapi.get_client(context) - try: - neutron.delete_security_group(security_group['id']) - except n_exc.NeutronClientException as e: - exc_info = sys.exc_info() - if e.status_code == 404: - self.raise_not_found(six.text_type(e)) - elif e.status_code == 409: - self.raise_invalid_property(six.text_type(e)) - else: - LOG.error("Neutron Error: %s", e) - six.reraise(*exc_info) - - def add_rules(self, context, id, name, vals): - """Add security group rule(s) to security group. - - Note: the Nova security group API doesn't support adding multiple - security group rules at once but the EC2 one does. Therefore, - this function is written to support both. Multiple rules are - installed to a security group in neutron using bulk support. - """ - - neutron = neutronapi.get_client(context) - body = self._make_neutron_security_group_rules_list(vals) - try: - rules = neutron.create_security_group_rule( - body).get('security_group_rules') - except n_exc.NeutronClientException as e: - exc_info = sys.exc_info() - if e.status_code == 404: - LOG.exception("Neutron Error getting security group %s", name) - self.raise_not_found(six.text_type(e)) - elif e.status_code == 409: - LOG.exception("Neutron Error adding rules to security " - "group %s", name) - self.raise_over_quota(six.text_type(e)) - elif e.status_code == 400: - LOG.exception("Neutron Error: %s", e) - self.raise_invalid_property(six.text_type(e)) - else: - six.reraise(*exc_info) - converted_rules = [] - for rule in rules: - converted_rules.append( - self._convert_to_nova_security_group_rule_format(rule)) - return converted_rules - - def _make_neutron_security_group_dict(self, name, description): - return {'security_group': {'name': name, - 'description': description}} - - def _make_neutron_security_group_rules_list(self, rules): - new_rules = [] - for rule in rules: - new_rule = {} - # nova only supports ingress rules so all rules are ingress. - new_rule['direction'] = "ingress" - new_rule['protocol'] = rule.get('protocol') - - # FIXME(arosen) Nova does not expose ethertype on security group - # rules. Therefore, in the case of self referential rules we - # should probably assume they want to allow both IPv4 and IPv6. - # Unfortunately, this would require adding two rules in neutron. - # The reason we do not do this is because when the user using the - # nova api wants to remove the rule we'd have to have some way to - # know that we should delete both of these rules in neutron. - # For now, self referential rules only support IPv4. - if not rule.get('cidr'): - new_rule['ethertype'] = 'IPv4' - else: - version = netaddr.IPNetwork(rule.get('cidr')).version - new_rule['ethertype'] = 'IPv6' if version == 6 else 'IPv4' - new_rule['remote_ip_prefix'] = rule.get('cidr') - new_rule['security_group_id'] = rule.get('parent_group_id') - new_rule['remote_group_id'] = rule.get('group_id') - if 'from_port' in rule and rule['from_port'] != -1: - new_rule['port_range_min'] = rule['from_port'] - if 'to_port' in rule and rule['to_port'] != -1: - new_rule['port_range_max'] = rule['to_port'] - new_rules.append(new_rule) - return {'security_group_rules': new_rules} - - def remove_rules(self, context, security_group, rule_ids): - neutron = neutronapi.get_client(context) - rule_ids = set(rule_ids) - try: - # The ec2 api allows one to delete multiple security group rules - # at once. Since there is no bulk delete for neutron the best - # thing we can do is delete the rules one by one and hope this - # works.... :/ - for rule_id in range(0, len(rule_ids)): - neutron.delete_security_group_rule(rule_ids.pop()) - except n_exc.NeutronClientException: - with excutils.save_and_reraise_exception(): - LOG.exception("Neutron Error unable to delete %s", rule_ids) - - def get_rule(self, context, id): - neutron = neutronapi.get_client(context) - try: - rule = neutron.show_security_group_rule( - id).get('security_group_rule') - except n_exc.NeutronClientException as e: - exc_info = sys.exc_info() - if e.status_code == 404: - LOG.debug("Neutron security group rule %s not found", id) - self.raise_not_found(six.text_type(e)) - else: - LOG.error("Neutron Error: %s", e) - six.reraise(*exc_info) - return self._convert_to_nova_security_group_rule_format(rule) - - def _get_ports_from_server_list(self, servers, neutron): - """Returns a list of ports used by the servers.""" - - def _chunk_by_ids(servers, limit): - ids = [] - for server in servers: - ids.append(server['id']) - if len(ids) >= limit: - yield ids - ids = [] - if ids: - yield ids - - # Note: Have to split the query up as the search criteria - # form part of the URL, which has a fixed max size - ports = [] - for ids in _chunk_by_ids(servers, MAX_SEARCH_IDS): - search_opts = {'device_id': ids} - try: - ports.extend(neutron.list_ports(**search_opts).get('ports')) - except n_exc.PortNotFoundClient: - # There could be a race between deleting an instance and - # retrieving its port groups from Neutron. In this case - # PortNotFoundClient is raised and it can be safely ignored - LOG.debug("Port not found for device with id %s", ids) - - return ports - - def _get_secgroups_from_port_list(self, ports, neutron, fields=None): - """Returns a dict of security groups keyed by their ids.""" - - def _chunk_by_ids(sg_ids, limit): - sg_id_list = [] - for sg_id in sg_ids: - sg_id_list.append(sg_id) - if len(sg_id_list) >= limit: - yield sg_id_list - sg_id_list = [] - if sg_id_list: - yield sg_id_list - - # Find the set of unique SecGroup IDs to search for - sg_ids = set() - for port in ports: - sg_ids.update(port.get('security_groups', [])) - - # Note: Have to split the query up as the search criteria - # form part of the URL, which has a fixed max size - security_groups = {} - for sg_id_list in _chunk_by_ids(sg_ids, MAX_SEARCH_IDS): - sg_search_opts = {'id': sg_id_list} - if fields: - sg_search_opts['fields'] = fields - search_results = neutron.list_security_groups(**sg_search_opts) - for sg in search_results.get('security_groups'): - security_groups[sg['id']] = sg - - return security_groups - - def get_instances_security_groups_bindings(self, context, servers, - detailed=False): - """Returns a dict(instance_id, [security_groups]) to allow obtaining - all of the instances and their security groups in one shot. - If detailed is False only the security group name is returned. - """ - - neutron = neutronapi.get_client(context) - - ports = self._get_ports_from_server_list(servers, neutron) - - # If detailed is True, we want all fields from the security groups - # including the potentially slow-to-join security_group_rules field. - # But if detailed is False, only get the id and name fields since - # that's all we'll use below. - fields = None if detailed else ['id', 'name'] - security_groups = self._get_secgroups_from_port_list( - ports, neutron, fields=fields) - - instances_security_group_bindings = {} - for port in ports: - for port_sg_id in port.get('security_groups', []): - - # Note: have to check we found port_sg as its possible - # the port has an SG that this user doesn't have access to - port_sg = security_groups.get(port_sg_id) - if port_sg: - if detailed: - sg_entry = self._convert_to_nova_security_group_format( - port_sg) - instances_security_group_bindings.setdefault( - port['device_id'], []).append(sg_entry) - else: - # name is optional in neutron so if not specified - # return id - name = port_sg.get('name') - if not name: - name = port_sg.get('id') - sg_entry = {'name': name} - instances_security_group_bindings.setdefault( - port['device_id'], []).append(sg_entry) - - return instances_security_group_bindings - - def get_instance_security_groups(self, context, instance, detailed=False): - """Returns the security groups that are associated with an instance. - If detailed is True then it also returns the full details of the - security groups associated with an instance, otherwise just the - security group name. - """ - servers = [{'id': instance.uuid}] - sg_bindings = self.get_instances_security_groups_bindings( - context, servers, detailed) - return sg_bindings.get(instance.uuid, []) - - def _has_security_group_requirements(self, port): - port_security_enabled = port.get('port_security_enabled', True) - has_ip = port.get('fixed_ips') - deferred_ip = port.get('ip_allocation') == 'deferred' - if has_ip or deferred_ip: - return port_security_enabled - return False - - def add_to_instance(self, context, instance, security_group_name): - """Add security group to the instance.""" - - neutron = neutronapi.get_client(context) - try: - security_group_id = neutronv20.find_resourceid_by_name_or_id( - neutron, 'security_group', - 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: - msg = (_("Security group %(name)s is not found for " - "project %(project)s") % - {'name': security_group_name, - 'project': context.project_id}) - self.raise_not_found(msg) - else: - six.reraise(*exc_info) - params = {'device_id': instance.uuid} - try: - ports = neutron.list_ports(**params).get('ports') - except n_exc.NeutronClientException: - with excutils.save_and_reraise_exception(): - LOG.exception("Neutron Error:") - - if not ports: - msg = (_("instance_id %s could not be found as device id on" - " any ports") % instance.uuid) - self.raise_not_found(msg) - - for port in ports: - if not self._has_security_group_requirements(port): - LOG.warning("Cannot add security group %(name)s to " - "%(instance)s since the port %(port_id)s " - "does not meet security requirements", - {'name': security_group_name, - 'instance': instance.uuid, - 'port_id': port['id']}) - raise exception.SecurityGroupCannotBeApplied() - if 'security_groups' not in port: - port['security_groups'] = [] - port['security_groups'].append(security_group_id) - updated_port = {'security_groups': port['security_groups']} - try: - LOG.info("Adding security group %(security_group_id)s to " - "port %(port_id)s", - {'security_group_id': security_group_id, - 'port_id': port['id']}) - neutron.update_port(port['id'], {'port': updated_port}) - except n_exc.NeutronClientException as e: - exc_info = sys.exc_info() - if e.status_code == 400: - raise exception.SecurityGroupCannotBeApplied( - six.text_type(e)) - else: - six.reraise(*exc_info) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.exception("Neutron Error:") - - def remove_from_instance(self, context, instance, security_group_name): - """Remove the security group associated with the instance.""" - neutron = neutronapi.get_client(context) - try: - security_group_id = neutronv20.find_resourceid_by_name_or_id( - neutron, 'security_group', - security_group_name, - context.project_id) - except n_exc.NeutronClientException as e: - exc_info = sys.exc_info() - if e.status_code == 404: - msg = (_("Security group %(name)s is not found for " - "project %(project)s") % - {'name': security_group_name, - 'project': context.project_id}) - self.raise_not_found(msg) - else: - six.reraise(*exc_info) - params = {'device_id': instance.uuid} - try: - ports = neutron.list_ports(**params).get('ports') - except n_exc.NeutronClientException: - with excutils.save_and_reraise_exception(): - LOG.exception("Neutron Error:") - - if not ports: - msg = (_("instance_id %s could not be found as device id on" - " any ports") % instance.uuid) - self.raise_not_found(msg) - - found_security_group = False - for port in ports: - try: - port.get('security_groups', []).remove(security_group_id) - except ValueError: - # When removing a security group from an instance the security - # group should be on both ports since it was added this way if - # done through the nova api. In case it is not a 404 is only - # raised if the security group is not found on any of the - # ports on the instance. - continue - - updated_port = {'security_groups': port['security_groups']} - try: - LOG.info("Removing security group %(security_group_id)s from " - "port %(port_id)s", - {'security_group_id': security_group_id, - 'port_id': port['id']}) - neutron.update_port(port['id'], {'port': updated_port}) - found_security_group = True - except Exception: - with excutils.save_and_reraise_exception(): - LOG.exception("Neutron Error:") - if not found_security_group: - msg = (_("Security group %(security_group_name)s not associated " - "with the instance %(instance)s") % - {'security_group_name': security_group_name, - 'instance': instance.uuid}) - self.raise_not_found(msg) diff --git a/nova/network/security_group/openstack_driver.py b/nova/network/security_group/openstack_driver.py deleted file mode 100644 index 060c3e1b5038..000000000000 --- a/nova/network/security_group/openstack_driver.py +++ /dev/null @@ -1,30 +0,0 @@ -# Copyright 2013 Nicira, Inc. -# All Rights Reserved -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from oslo_utils import importutils - - -NEUTRON_DRIVER = ('nova.network.security_group.neutron_driver.' - 'SecurityGroupAPI') -DRIVER_CACHE = None # singleton of the driver once loaded - - -# TODO(stephenfin): Remove this since it's not needed with nova-net no longer -# screwing things up -def get_openstack_security_group_driver(): - global DRIVER_CACHE - if DRIVER_CACHE is None: - DRIVER_CACHE = importutils.import_object(NEUTRON_DRIVER) - return DRIVER_CACHE diff --git a/nova/network/security_group/security_group_base.py b/nova/network/security_group/security_group_base.py deleted file mode 100644 index 5a0228bca09b..000000000000 --- a/nova/network/security_group/security_group_base.py +++ /dev/null @@ -1,253 +0,0 @@ -# Copyright 2010 United States Government as represented by the -# Administrator of the National Aeronautics and Space Administration. -# Copyright 2011 Piston Cloud Computing, Inc. -# Copyright 2012 Red Hat, Inc. -# Copyright 2013 Nicira, Inc. -# All Rights Reserved -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -from oslo_utils import encodeutils -from oslo_utils import netutils as utils -from six.moves import urllib - -from nova import exception -from nova.i18n import _ -from nova.objects import security_group as security_group_obj - - -# TODO(stephenfin): Merge this into the only implementation that exists now -class SecurityGroupBase(object): - - def parse_cidr(self, cidr): - if cidr: - try: - cidr = encodeutils.safe_decode(urllib.parse.unquote(cidr)) - except Exception as e: - self.raise_invalid_cidr(cidr, e) - - if not utils.is_valid_cidr(cidr): - self.raise_invalid_cidr(cidr) - - return cidr - else: - return '0.0.0.0/0' - - @staticmethod - def new_group_ingress_rule(grantee_group_id, protocol, from_port, - to_port): - return SecurityGroupBase._new_ingress_rule( - protocol, from_port, to_port, group_id=grantee_group_id) - - @staticmethod - def new_cidr_ingress_rule(grantee_cidr, protocol, from_port, to_port): - return SecurityGroupBase._new_ingress_rule( - protocol, from_port, to_port, cidr=grantee_cidr) - - @staticmethod - def _new_ingress_rule(ip_protocol, from_port, to_port, - group_id=None, cidr=None): - values = {} - - if group_id: - values['group_id'] = group_id - # Open everything if an explicit port range or type/code are not - # specified, but only if a source group was specified. - ip_proto_upper = ip_protocol.upper() if ip_protocol else '' - if (ip_proto_upper == 'ICMP' and - from_port is None and to_port is None): - from_port = -1 - to_port = -1 - elif (ip_proto_upper in ['TCP', 'UDP'] and from_port is None and - to_port is None): - from_port = 1 - to_port = 65535 - - elif cidr: - values['cidr'] = cidr - - if ip_protocol and from_port is not None and to_port is not None: - - ip_protocol = str(ip_protocol) - try: - # Verify integer conversions - from_port = int(from_port) - to_port = int(to_port) - except ValueError: - if ip_protocol.upper() == 'ICMP': - raise exception.InvalidInput(reason=_("Type and" - " Code must be integers for ICMP protocol type")) - else: - raise exception.InvalidInput(reason=_("To and From ports " - "must be integers")) - - if ip_protocol.upper() not in ['TCP', 'UDP', 'ICMP']: - raise exception.InvalidIpProtocol(protocol=ip_protocol) - - # Verify that from_port must always be less than - # or equal to to_port - if (ip_protocol.upper() in ['TCP', 'UDP'] and - (from_port > to_port)): - raise exception.InvalidPortRange(from_port=from_port, - to_port=to_port, msg="Former value cannot" - " be greater than the later") - - # Verify valid TCP, UDP port ranges - if (ip_protocol.upper() in ['TCP', 'UDP'] and - (from_port < 1 or to_port > 65535)): - raise exception.InvalidPortRange(from_port=from_port, - to_port=to_port, msg="Valid %s ports should" - " be between 1-65535" - % ip_protocol.upper()) - - # Verify ICMP type and code - if (ip_protocol.upper() == "ICMP" and - (from_port < -1 or from_port > 255 or - to_port < -1 or to_port > 255)): - raise exception.InvalidPortRange(from_port=from_port, - to_port=to_port, msg="For ICMP, the" - " type:code must be valid") - - values['protocol'] = ip_protocol - values['from_port'] = from_port - values['to_port'] = to_port - - else: - # If cidr based filtering, protocol and ports are mandatory - if cidr: - return None - - return values - - def create_security_group_rule(self, context, security_group, new_rule): - if self.rule_exists(security_group, new_rule): - msg = (_('This rule already exists in group %s') % - new_rule['parent_group_id']) - self.raise_group_already_exists(msg) - return self.add_rules(context, new_rule['parent_group_id'], - security_group['name'], - [new_rule])[0] - - def rule_exists(self, security_group, new_rule): - """Indicates whether the specified rule is already - defined in the given security group. - """ - for rule in security_group['rules']: - keys = ('group_id', 'cidr', 'from_port', 'to_port', 'protocol') - for key in keys: - if rule.get(key) != new_rule.get(key): - break - else: - return rule.get('id') or True - return False - - def validate_property(self, value, property, allowed): - pass - - def ensure_default(self, context): - pass - - def trigger_rules_refresh(self, context, id): - """Called when a rule is added to or removed from a security_group.""" - pass - - def trigger_members_refresh(self, context, group_ids): - """Called when a security group gains a new or loses a member. - - Sends an update request to each compute node for each instance for - which this is relevant. - """ - pass - - def populate_security_groups(self, security_groups): - """Build and return a SecurityGroupList. - - :param security_groups: list of requested security group names or uuids - :type security_groups: list - :returns: nova.objects.security_group.SecurityGroupList - """ - if not security_groups: - # Make sure it's an empty SecurityGroupList and not None - return security_group_obj.SecurityGroupList() - return security_group_obj.make_secgroup_list(security_groups) - - def create_security_group(self, context, name, description): - raise NotImplementedError() - - def update_security_group(self, context, security_group, - name, description): - raise NotImplementedError() - - def get(self, context, name=None, id=None, map_exception=False): - raise NotImplementedError() - - def list(self, context, names=None, ids=None, project=None, - search_opts=None): - raise NotImplementedError() - - def destroy(self, context, security_group): - raise NotImplementedError() - - def add_rules(self, context, id, name, vals): - raise NotImplementedError() - - def remove_rules(self, context, security_group, rule_ids): - raise NotImplementedError() - - def get_rule(self, context, id): - raise NotImplementedError() - - def get_instance_security_groups(self, context, instance, detailed=False): - raise NotImplementedError() - - def add_to_instance(self, context, instance, security_group_name): - """Add security group to the instance. - - :param context: The request context. - :param instance: nova.objects.instance.Instance object. - :param security_group_name: security group name to add - """ - raise NotImplementedError() - - def remove_from_instance(self, context, instance, security_group_name): - """Remove the security group associated with the instance. - - :param context: The request context. - :param instance: nova.objects.instance.Instance object. - :param security_group_name: security group name to remove - """ - raise NotImplementedError() - - @staticmethod - def raise_invalid_property(msg): - raise exception.Invalid(msg) - - @staticmethod - def raise_group_already_exists(msg): - raise exception.Invalid(msg) - - @staticmethod - def raise_invalid_group(msg): - raise exception.Invalid(msg) - - @staticmethod - def raise_invalid_cidr(cidr, decoding_exception=None): - raise exception.InvalidCidr(cidr=cidr) - - @staticmethod - def raise_over_quota(msg): - raise exception.SecurityGroupLimitExceeded(msg) - - @staticmethod - def raise_not_found(msg): - raise exception.SecurityGroupNotFound(msg) diff --git a/nova/network/security_group_api.py b/nova/network/security_group_api.py new file mode 100644 index 000000000000..8861b4439182 --- /dev/null +++ b/nova/network/security_group_api.py @@ -0,0 +1,717 @@ +# Copyright 2010 United States Government as represented by the +# Administrator of the National Aeronautics and Space Administration. +# Copyright 2011 Piston Cloud Computing, Inc. +# Copyright 2012 Red Hat, Inc. +# Copyright 2013 Nicira, Inc. +# All Rights Reserved +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import sys + +import netaddr +from neutronclient.common import exceptions as n_exc +from neutronclient.neutron import v2_0 as neutronv20 +from oslo_log import log as logging +from oslo_utils import encodeutils +from oslo_utils import excutils +from oslo_utils import netutils +from oslo_utils import uuidutils +import six +from six.moves import urllib +from webob import exc + +from nova import exception +from nova.i18n import _ +from nova.network import neutron as neutronapi +from nova.objects import security_group as security_group_obj +from nova import utils + + +LOG = logging.getLogger(__name__) + +# NOTE: Neutron client has a max URL length of 8192, so we have +# to limit the number of IDs we include in any single search. Really +# doesn't seem to be any point in making this a config value. +MAX_SEARCH_IDS = 150 + + +def validate_id(id): + if not uuidutils.is_uuid_like(id): + msg = _("Security group id should be uuid") + raise exception.Invalid(msg) + return id + + +def parse_cidr(cidr): + if not cidr: + return '0.0.0.0/0' + + try: + cidr = encodeutils.safe_decode(urllib.parse.unquote(cidr)) + except Exception: + raise exception.InvalidCidr(cidr=cidr) + + if not netutils.is_valid_cidr(cidr): + raise exception.InvalidCidr(cidr=cidr) + + return cidr + + +def new_group_ingress_rule(grantee_group_id, protocol, from_port, + to_port): + return _new_ingress_rule( + protocol, from_port, to_port, group_id=grantee_group_id) + + +def new_cidr_ingress_rule(grantee_cidr, protocol, from_port, to_port): + return _new_ingress_rule( + protocol, from_port, to_port, cidr=grantee_cidr) + + +def _new_ingress_rule(ip_protocol, from_port, to_port, + group_id=None, cidr=None): + values = {} + + if group_id: + values['group_id'] = group_id + # Open everything if an explicit port range or type/code are not + # specified, but only if a source group was specified. + ip_proto_upper = ip_protocol.upper() if ip_protocol else '' + if (ip_proto_upper == 'ICMP' and + from_port is None and to_port is None): + from_port = -1 + to_port = -1 + elif (ip_proto_upper in ['TCP', 'UDP'] and from_port is None and + to_port is None): + from_port = 1 + to_port = 65535 + + elif cidr: + values['cidr'] = cidr + + if ip_protocol and from_port is not None and to_port is not None: + + ip_protocol = str(ip_protocol) + try: + # Verify integer conversions + from_port = int(from_port) + to_port = int(to_port) + except ValueError: + if ip_protocol.upper() == 'ICMP': + raise exception.InvalidInput(reason=_("Type and" + " Code must be integers for ICMP protocol type")) + else: + raise exception.InvalidInput(reason=_("To and From ports " + "must be integers")) + + if ip_protocol.upper() not in ['TCP', 'UDP', 'ICMP']: + raise exception.InvalidIpProtocol(protocol=ip_protocol) + + # Verify that from_port must always be less than + # or equal to to_port + if (ip_protocol.upper() in ['TCP', 'UDP'] and + (from_port > to_port)): + raise exception.InvalidPortRange(from_port=from_port, + to_port=to_port, msg="Former value cannot" + " be greater than the later") + + # Verify valid TCP, UDP port ranges + if (ip_protocol.upper() in ['TCP', 'UDP'] and + (from_port < 1 or to_port > 65535)): + raise exception.InvalidPortRange(from_port=from_port, + to_port=to_port, msg="Valid %s ports should" + " be between 1-65535" + % ip_protocol.upper()) + + # Verify ICMP type and code + if (ip_protocol.upper() == "ICMP" and + (from_port < -1 or from_port > 255 or + to_port < -1 or to_port > 255)): + raise exception.InvalidPortRange(from_port=from_port, + to_port=to_port, msg="For ICMP, the" + " type:code must be valid") + + values['protocol'] = ip_protocol + values['from_port'] = from_port + values['to_port'] = to_port + + else: + # If cidr based filtering, protocol and ports are mandatory + if cidr: + return None + + return values + + +def create_security_group_rule(context, security_group, new_rule): + if _rule_exists(security_group, new_rule): + msg = (_('This rule already exists in group %s') % + new_rule['parent_group_id']) + raise exception.Invalid(msg) + + return add_rules(context, new_rule['parent_group_id'], + security_group['name'], + [new_rule])[0] + + +def _rule_exists(security_group, new_rule): + """Indicates whether the specified rule is already + defined in the given security group. + """ + for rule in security_group['rules']: + keys = ('group_id', 'cidr', 'from_port', 'to_port', 'protocol') + for key in keys: + if rule.get(key) != new_rule.get(key): + break + else: + return rule.get('id') or True + return False + + +def validate_property(value, property, allowed): + """Validate given security group property. + + :param value: the value to validate, as a string or unicode + :param property: the property, either 'name' or 'description' + :param allowed: the range of characters allowed, but not used because + Neutron is allowing any characters. + """ + utils.check_string_length(value, name=property, min_length=0, + max_length=255) + + +def populate_security_groups(security_groups): + """Build and return a SecurityGroupList. + + :param security_groups: list of requested security group names or uuids + :type security_groups: list + :returns: nova.objects.security_group.SecurityGroupList + """ + if not security_groups: + # Make sure it's an empty SecurityGroupList and not None + return security_group_obj.SecurityGroupList() + return security_group_obj.make_secgroup_list(security_groups) + + +def create_security_group(context, name, description): + neutron = neutronapi.get_client(context) + body = _make_neutron_security_group_dict(name, description) + try: + security_group = neutron.create_security_group( + body).get('security_group') + except n_exc.BadRequest as e: + raise exception.Invalid(six.text_type(e)) + except n_exc.NeutronClientException as e: + exc_info = sys.exc_info() + LOG.exception("Neutron Error creating security group %s", name) + if e.status_code == 401: + # TODO(arosen) Cannot raise generic response from neutron here + # as this error code could be related to bad input or over + # quota + raise exc.HTTPBadRequest() + elif e.status_code == 409: + raise exception.SecurityGroupLimitExceeded(six.text_type(e)) + six.reraise(*exc_info) + return _convert_to_nova_security_group_format(security_group) + + +def update_security_group(context, security_group, name, description): + neutron = neutronapi.get_client(context) + body = _make_neutron_security_group_dict(name, description) + try: + security_group = neutron.update_security_group( + security_group['id'], body).get('security_group') + except n_exc.NeutronClientException as e: + exc_info = sys.exc_info() + LOG.exception("Neutron Error updating security group %s", name) + if e.status_code == 401: + # TODO(arosen) Cannot raise generic response from neutron here + # as this error code could be related to bad input or over + # quota + raise exc.HTTPBadRequest() + six.reraise(*exc_info) + return _convert_to_nova_security_group_format(security_group) + + +def _convert_to_nova_security_group_format(security_group): + nova_group = {} + nova_group['id'] = security_group['id'] + nova_group['description'] = security_group['description'] + nova_group['name'] = security_group['name'] + nova_group['project_id'] = security_group['tenant_id'] + nova_group['rules'] = [] + for rule in security_group.get('security_group_rules', []): + if rule['direction'] == 'ingress': + nova_group['rules'].append( + _convert_to_nova_security_group_rule_format(rule)) + + return nova_group + + +def _convert_to_nova_security_group_rule_format(rule): + nova_rule = {} + nova_rule['id'] = rule['id'] + nova_rule['parent_group_id'] = rule['security_group_id'] + nova_rule['protocol'] = rule['protocol'] + if (nova_rule['protocol'] and rule.get('port_range_min') is None and + rule.get('port_range_max') is None): + if rule['protocol'].upper() in ['TCP', 'UDP']: + nova_rule['from_port'] = 1 + nova_rule['to_port'] = 65535 + else: + nova_rule['from_port'] = -1 + nova_rule['to_port'] = -1 + else: + nova_rule['from_port'] = rule.get('port_range_min') + nova_rule['to_port'] = rule.get('port_range_max') + nova_rule['group_id'] = rule['remote_group_id'] + nova_rule['cidr'] = parse_cidr(rule.get('remote_ip_prefix')) + return nova_rule + + +def get(context, name=None, id=None, map_exception=False): + 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) + 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): + """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 the security group will be returned if the project/tenant + # id is not passed. + if project and 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 + + +def destroy(context, security_group): + """This function deletes a security group.""" + + neutron = neutronapi.get_client(context) + try: + neutron.delete_security_group(security_group['id']) + except n_exc.NeutronClientException as e: + exc_info = sys.exc_info() + if e.status_code == 404: + raise exception.SecurityGroupNotFound(six.text_type(e)) + elif e.status_code == 409: + raise exception.Invalid(six.text_type(e)) + else: + LOG.error("Neutron Error: %s", e) + six.reraise(*exc_info) + + +def add_rules(context, id, name, vals): + """Add security group rule(s) to security group. + + Note: the Nova security group API doesn't support adding multiple + security group rules at once but the EC2 one does. Therefore, + this function is written to support both. Multiple rules are + installed to a security group in neutron using bulk support. + """ + + neutron = neutronapi.get_client(context) + body = _make_neutron_security_group_rules_list(vals) + try: + rules = neutron.create_security_group_rule( + body).get('security_group_rules') + except n_exc.NeutronClientException as e: + exc_info = sys.exc_info() + if e.status_code == 404: + LOG.exception("Neutron Error getting security group %s", name) + raise exception.SecurityGroupNotFound(six.text_type(e)) + elif e.status_code == 409: + LOG.exception("Neutron Error adding rules to security " + "group %s", name) + raise exception.SecurityGroupLimitExceeded(six.text_type(e)) + elif e.status_code == 400: + LOG.exception("Neutron Error: %s", e) + raise exception.Invalid(six.text_type(e)) + else: + six.reraise(*exc_info) + converted_rules = [] + for rule in rules: + converted_rules.append( + _convert_to_nova_security_group_rule_format(rule)) + return converted_rules + + +def _make_neutron_security_group_dict(name, description): + return {'security_group': {'name': name, + 'description': description}} + + +def _make_neutron_security_group_rules_list(rules): + new_rules = [] + for rule in rules: + new_rule = {} + # nova only supports ingress rules so all rules are ingress. + new_rule['direction'] = "ingress" + new_rule['protocol'] = rule.get('protocol') + + # FIXME(arosen) Nova does not expose ethertype on security group + # rules. Therefore, in the case of self referential rules we + # should probably assume they want to allow both IPv4 and IPv6. + # Unfortunately, this would require adding two rules in neutron. + # The reason we do not do this is because when the user using the + # nova api wants to remove the rule we'd have to have some way to + # know that we should delete both of these rules in neutron. + # For now, self referential rules only support IPv4. + if not rule.get('cidr'): + new_rule['ethertype'] = 'IPv4' + else: + version = netaddr.IPNetwork(rule.get('cidr')).version + new_rule['ethertype'] = 'IPv6' if version == 6 else 'IPv4' + new_rule['remote_ip_prefix'] = rule.get('cidr') + new_rule['security_group_id'] = rule.get('parent_group_id') + new_rule['remote_group_id'] = rule.get('group_id') + if 'from_port' in rule and rule['from_port'] != -1: + new_rule['port_range_min'] = rule['from_port'] + if 'to_port' in rule and rule['to_port'] != -1: + new_rule['port_range_max'] = rule['to_port'] + new_rules.append(new_rule) + return {'security_group_rules': new_rules} + + +def remove_rules(context, security_group, rule_ids): + neutron = neutronapi.get_client(context) + rule_ids = set(rule_ids) + try: + # The ec2 api allows one to delete multiple security group rules + # at once. Since there is no bulk delete for neutron the best + # thing we can do is delete the rules one by one and hope this + # works.... :/ + for rule_id in range(0, len(rule_ids)): + neutron.delete_security_group_rule(rule_ids.pop()) + except n_exc.NeutronClientException: + with excutils.save_and_reraise_exception(): + LOG.exception("Neutron Error unable to delete %s", rule_ids) + + +def get_rule(context, id): + neutron = neutronapi.get_client(context) + try: + rule = neutron.show_security_group_rule( + id).get('security_group_rule') + except n_exc.NeutronClientException as e: + exc_info = sys.exc_info() + if e.status_code == 404: + LOG.debug("Neutron security group rule %s not found", id) + raise exception.SecurityGroupNotFound(six.text_type(e)) + else: + LOG.error("Neutron Error: %s", e) + six.reraise(*exc_info) + return _convert_to_nova_security_group_rule_format(rule) + + +def _get_ports_from_server_list(servers, neutron): + """Returns a list of ports used by the servers.""" + + def _chunk_by_ids(servers, limit): + ids = [] + for server in servers: + ids.append(server['id']) + if len(ids) >= limit: + yield ids + ids = [] + if ids: + yield ids + + # Note: Have to split the query up as the search criteria + # form part of the URL, which has a fixed max size + ports = [] + for ids in _chunk_by_ids(servers, MAX_SEARCH_IDS): + search_opts = {'device_id': ids} + try: + ports.extend(neutron.list_ports(**search_opts).get('ports')) + except n_exc.PortNotFoundClient: + # There could be a race between deleting an instance and + # retrieving its port groups from Neutron. In this case + # PortNotFoundClient is raised and it can be safely ignored + LOG.debug("Port not found for device with id %s", ids) + + return ports + + +def _get_secgroups_from_port_list(ports, neutron, fields=None): + """Returns a dict of security groups keyed by their ids.""" + + def _chunk_by_ids(sg_ids, limit): + sg_id_list = [] + for sg_id in sg_ids: + sg_id_list.append(sg_id) + if len(sg_id_list) >= limit: + yield sg_id_list + sg_id_list = [] + if sg_id_list: + yield sg_id_list + + # Find the set of unique SecGroup IDs to search for + sg_ids = set() + for port in ports: + sg_ids.update(port.get('security_groups', [])) + + # Note: Have to split the query up as the search criteria + # form part of the URL, which has a fixed max size + security_groups = {} + for sg_id_list in _chunk_by_ids(sg_ids, MAX_SEARCH_IDS): + sg_search_opts = {'id': sg_id_list} + if fields: + sg_search_opts['fields'] = fields + search_results = neutron.list_security_groups(**sg_search_opts) + for sg in search_results.get('security_groups'): + security_groups[sg['id']] = sg + + return security_groups + + +def get_instances_security_groups_bindings(context, servers, + detailed=False): + """Returns a dict(instance_id, [security_groups]) to allow obtaining + all of the instances and their security groups in one shot. + If detailed is False only the security group name is returned. + """ + + neutron = neutronapi.get_client(context) + + ports = _get_ports_from_server_list(servers, neutron) + + # If detailed is True, we want all fields from the security groups + # including the potentially slow-to-join security_group_rules field. + # But if detailed is False, only get the id and name fields since + # that's all we'll use below. + fields = None if detailed else ['id', 'name'] + security_groups = _get_secgroups_from_port_list( + ports, neutron, fields=fields) + + instances_security_group_bindings = {} + for port in ports: + for port_sg_id in port.get('security_groups', []): + + # Note: have to check we found port_sg as its possible + # the port has an SG that this user doesn't have access to + port_sg = security_groups.get(port_sg_id) + if port_sg: + if detailed: + sg_entry = _convert_to_nova_security_group_format( + port_sg) + instances_security_group_bindings.setdefault( + port['device_id'], []).append(sg_entry) + else: + # name is optional in neutron so if not specified + # return id + name = port_sg.get('name') + if not name: + name = port_sg.get('id') + sg_entry = {'name': name} + instances_security_group_bindings.setdefault( + port['device_id'], []).append(sg_entry) + + return instances_security_group_bindings + + +def get_instance_security_groups(context, instance, detailed=False): + """Returns the security groups that are associated with an instance. + If detailed is True then it also returns the full details of the + security groups associated with an instance, otherwise just the + security group name. + """ + servers = [{'id': instance.uuid}] + sg_bindings = get_instances_security_groups_bindings( + context, servers, detailed) + return sg_bindings.get(instance.uuid, []) + + +def _has_security_group_requirements(port): + port_security_enabled = port.get('port_security_enabled', True) + has_ip = port.get('fixed_ips') + deferred_ip = port.get('ip_allocation') == 'deferred' + if has_ip or deferred_ip: + return port_security_enabled + return False + + +def add_to_instance(context, instance, security_group_name): + """Add security group to the instance.""" + + neutron = neutronapi.get_client(context) + try: + security_group_id = neutronv20.find_resourceid_by_name_or_id( + neutron, 'security_group', + 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: + msg = (_("Security group %(name)s is not found for " + "project %(project)s") % + {'name': security_group_name, + 'project': context.project_id}) + raise exception.SecurityGroupNotFound(msg) + else: + six.reraise(*exc_info) + params = {'device_id': instance.uuid} + try: + ports = neutron.list_ports(**params).get('ports') + except n_exc.NeutronClientException: + with excutils.save_and_reraise_exception(): + LOG.exception("Neutron Error:") + + if not ports: + msg = (_("instance_id %s could not be found as device id on" + " any ports") % instance.uuid) + raise exception.SecurityGroupNotFound(msg) + + for port in ports: + if not _has_security_group_requirements(port): + LOG.warning("Cannot add security group %(name)s to " + "%(instance)s since the port %(port_id)s " + "does not meet security requirements", + {'name': security_group_name, + 'instance': instance.uuid, + 'port_id': port['id']}) + raise exception.SecurityGroupCannotBeApplied() + if 'security_groups' not in port: + port['security_groups'] = [] + port['security_groups'].append(security_group_id) + updated_port = {'security_groups': port['security_groups']} + try: + LOG.info("Adding security group %(security_group_id)s to " + "port %(port_id)s", + {'security_group_id': security_group_id, + 'port_id': port['id']}) + neutron.update_port(port['id'], {'port': updated_port}) + except n_exc.NeutronClientException as e: + exc_info = sys.exc_info() + if e.status_code == 400: + raise exception.SecurityGroupCannotBeApplied( + six.text_type(e)) + else: + six.reraise(*exc_info) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.exception("Neutron Error:") + + +def remove_from_instance(context, instance, security_group_name): + """Remove the security group associated with the instance.""" + neutron = neutronapi.get_client(context) + try: + security_group_id = neutronv20.find_resourceid_by_name_or_id( + neutron, 'security_group', + security_group_name, + context.project_id) + except n_exc.NeutronClientException as e: + exc_info = sys.exc_info() + if e.status_code == 404: + msg = (_("Security group %(name)s is not found for " + "project %(project)s") % + {'name': security_group_name, + 'project': context.project_id}) + raise exception.SecurityGroupNotFound(msg) + else: + six.reraise(*exc_info) + params = {'device_id': instance.uuid} + try: + ports = neutron.list_ports(**params).get('ports') + except n_exc.NeutronClientException: + with excutils.save_and_reraise_exception(): + LOG.exception("Neutron Error:") + + if not ports: + msg = (_("instance_id %s could not be found as device id on" + " any ports") % instance.uuid) + raise exception.SecurityGroupNotFound(msg) + + found_security_group = False + for port in ports: + try: + port.get('security_groups', []).remove(security_group_id) + except ValueError: + # When removing a security group from an instance the security + # group should be on both ports since it was added this way if + # done through the nova api. In case it is not a 404 is only + # raised if the security group is not found on any of the + # ports on the instance. + continue + + updated_port = {'security_groups': port['security_groups']} + try: + LOG.info("Removing security group %(security_group_id)s from " + "port %(port_id)s", + {'security_group_id': security_group_id, + 'port_id': port['id']}) + neutron.update_port(port['id'], {'port': updated_port}) + found_security_group = True + except Exception: + with excutils.save_and_reraise_exception(): + LOG.exception("Neutron Error:") + if not found_security_group: + msg = (_("Security group %(security_group_name)s not associated " + "with the instance %(instance)s") % + {'security_group_name': security_group_name, + 'instance': instance.uuid}) + raise exception.SecurityGroupNotFound(msg) diff --git a/nova/test.py b/nova/test.py index 9b7e1d7db336..54c5dcf8e878 100644 --- a/nova/test.py +++ b/nova/test.py @@ -55,7 +55,6 @@ import testtools from nova.compute import rpcapi as compute_rpcapi from nova import context from nova import exception -from nova.network.security_group import openstack_driver from nova import objects from nova.objects import base as objects_base from nova import quota @@ -236,8 +235,6 @@ class TestCase(base.BaseTestCase): self.useFixture(nova_fixtures.PoisonFunctions()) - openstack_driver.DRIVER_CACHE = None - self.useFixture(nova_fixtures.ForbidNewLegacyNotificationFixture()) # NOTE(mikal): make sure we don't load a privsep helper accidentally diff --git a/nova/tests/functional/api_sample_tests/test_security_groups.py b/nova/tests/functional/api_sample_tests/test_security_groups.py index 7e34ba1d9f26..3b5ca55b5fe5 100644 --- a/nova/tests/functional/api_sample_tests/test_security_groups.py +++ b/nova/tests/functional/api_sample_tests/test_security_groups.py @@ -27,7 +27,7 @@ def fake_get(*args, **kwargs): return nova_group -def fake_get_instances_security_groups_bindings(self, context, servers, +def fake_get_instances_security_groups_bindings(context, servers, detailed=False): result = {} for s in servers: @@ -35,29 +35,28 @@ def fake_get_instances_security_groups_bindings(self, context, servers, return result -def fake_add_to_instance(self, context, instance, security_group_name): +def fake_add_to_instance(context, instance, security_group_name): pass -def fake_remove_from_instance(self, context, instance, security_group_name): +def fake_remove_from_instance(context, instance, security_group_name): pass -def fake_list(self, context, names=None, ids=None, project=None, - search_opts=None): +def fake_list(context, names=None, ids=None, project=None, search_opts=None): return [fake_get()] -def fake_get_instance_security_groups(self, context, instance_uuid, +def fake_get_instance_security_groups(context, instance_uuid, detailed=False): return [fake_get()] -def fake_create_security_group(self, context, name, description): +def fake_create_security_group(context, name, description): return fake_get() -def fake_create_security_group_rule(self, context, security_group, new_rule): +def fake_create_security_group_rule(context, security_group, new_rule): return { 'from_port': 22, 'to_port': 22, @@ -69,11 +68,11 @@ def fake_create_security_group_rule(self, context, security_group, new_rule): } -def fake_remove_rules(self, context, security_group, rule_ids): +def fake_remove_rules(context, security_group, rule_ids): pass -def fake_get_rule(self, context, id): +def fake_get_rule(context, id): return { 'id': id, 'parent_group_id': '11111111-1111-1111-1111-111111111111' @@ -85,7 +84,7 @@ class SecurityGroupsJsonTest(test_servers.ServersSampleBase): def setUp(self): super(SecurityGroupsJsonTest, self).setUp() - path = 'nova.network.security_group.neutron_driver.SecurityGroupAPI.' + path = 'nova.network.security_group_api.' self.stub_out(path + 'get', fake_get) self.stub_out(path + 'get_instances_security_groups_bindings', fake_get_instances_security_groups_bindings) diff --git a/nova/tests/unit/api/openstack/compute/test_security_groups.py b/nova/tests/unit/api/openstack/compute/test_security_groups.py index f6c485ef6e10..2b767cec832c 100644 --- a/nova/tests/unit/api/openstack/compute/test_security_groups.py +++ b/nova/tests/unit/api/openstack/compute/test_security_groups.py @@ -29,6 +29,7 @@ import nova.db.api from nova import exception from nova.network import model from nova.network import neutron as neutron_api +from nova.network import security_group_api from nova import objects from nova.objects import instance as instance_obj from nova import test @@ -532,8 +533,8 @@ class TestSecurityGroupsV21(test.TestCase): expected = {'security_groups': [expected]} - with mock.patch.object( - self.controller.security_group_api, 'list', + with mock.patch( + 'nova.network.security_group_api.list', return_value=[ security_group_db( secgroup) for secgroup in groups]) as mock_list: @@ -560,13 +561,12 @@ class TestSecurityGroupsV21(test.TestCase): req, UUID_SERVER)['security_groups'] self.assertEqual(expected, res_dict) - @mock.patch('nova.network.security_group.neutron_driver.SecurityGroupAPI.' + @mock.patch('nova.network.security_group_api.' 'get_instances_security_groups_bindings') def test_get_security_group_empty_for_instance(self, neutron_sg_bind_mock): servers = [{'id': FAKE_UUID1}] neutron_sg_bind_mock.return_value = {} - security_group_api = self.controller.security_group_api ctx = context_maker.get_admin_context() sgs = security_group_api.get_instance_security_groups(ctx, instance_obj.Instance(uuid=FAKE_UUID1)) @@ -890,10 +890,8 @@ class TestSecurityGroupsV21(test.TestCase): device_id=FAKE_UUID2) expected = {FAKE_UUID1: [{'name': sg1['name']}, {'name': sg2['name']}], FAKE_UUID2: [{'name': sg2['name']}, {'name': sg3['id']}]} - security_group_api = self.controller.security_group_api - bindings = ( - security_group_api.get_instances_security_groups_bindings( - context_maker.get_admin_context(), servers)) + bindings = security_group_api.get_instances_security_groups_bindings( + context_maker.get_admin_context(), servers) self.assertEqual(bindings, expected) def test_get_instance_security_groups(self): @@ -909,7 +907,6 @@ class TestSecurityGroupsV21(test.TestCase): expected = [{'name': sg1['name']}, {'name': sg2['name']}, {'name': sg3['id']}] - security_group_api = self.controller.security_group_api sgs = security_group_api.get_instance_security_groups( context_maker.get_admin_context(), instance_obj.Instance(uuid=FAKE_UUID1)) @@ -922,7 +919,6 @@ class TestSecurityGroupsV21(test.TestCase): network_id=net['network']['id'], security_groups=[sg1['id']], port_security_enabled=True, device_id=FAKE_UUID1) - security_group_api = self.controller.security_group_api sgs = security_group_api.get_instance_security_groups( context_maker.get_admin_context(), instance_obj.Instance(uuid=FAKE_UUID1)) @@ -1450,7 +1446,7 @@ class SecurityGroupsOutputTest(test.TestCase): self.stub_out('nova.compute.api.API.get_all', fake_compute_get_all) self.stub_out('nova.compute.api.API.create', fake_compute_create) self.stub_out( - 'nova.network.security_group.neutron_driver.SecurityGroupAPI.' + 'nova.network.security_group_api.' 'get_instances_security_groups_bindings', self._fake_get_instances_security_groups_bindings) @@ -1459,8 +1455,7 @@ class SecurityGroupsOutputTest(test.TestCase): get_client()._reset() super(SecurityGroupsOutputTest, self).tearDown() - def _fake_get_instances_security_groups_bindings(self, inst, context, - servers): + def _fake_get_instances_security_groups_bindings(inst, context, servers): groups = { '00000000-0000-0000-0000-000000000001': [{'name': 'fake-0-0'}, {'name': 'fake-0-1'}], @@ -1528,10 +1523,10 @@ class SecurityGroupsOutputTest(test.TestCase): self.assertEqual(group.get('name'), 'default') def test_show(self): - self.stub_out('nova.network.security_group.neutron_driver.' - 'SecurityGroupAPI.get_instance_security_groups', - lambda self, inst, context, id: - [{'name': 'fake-2-0'}, {'name': 'fake-2-1'}]) + self.stub_out( + 'nova.network.security_group_api.get_instance_security_groups', + lambda inst, context, id: [ + {'name': 'fake-2-0'}, {'name': 'fake-2-1'}]) url = '/v2/%s/servers' % fakes.FAKE_PROJECT_ID image_uuid = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' diff --git a/nova/tests/unit/api/openstack/fakes.py b/nova/tests/unit/api/openstack/fakes.py index e0bd765353d3..6c8e2a17f412 100644 --- a/nova/tests/unit/api/openstack/fakes.py +++ b/nova/tests/unit/api/openstack/fakes.py @@ -37,7 +37,6 @@ import nova.conf from nova import context from nova.db.sqlalchemy import models from nova import exception as exc -from nova.network.security_group import security_group_base from nova import objects from nova.objects import base from nova import quota @@ -210,25 +209,27 @@ def stub_out_nw_api(test, cls=None, private=None, publics=None): def stub_out_secgroup_api(test, security_groups=None): - class FakeSecurityGroupAPI(security_group_base.SecurityGroupBase): - def get_instances_security_groups_bindings( - self, context, servers, detailed=False): - instances_security_group_bindings = {} - if servers: - # we don't get security group information for down cells - instances_security_group_bindings = { - server['id']: security_groups or [] for server in servers - if server['status'] != 'UNKNOWN' - } - return instances_security_group_bindings + def get_instances_security_groups_bindings( + context, servers, detailed=False): + instances_security_group_bindings = {} + if servers: + # we don't get security group information for down cells + instances_security_group_bindings = { + server['id']: security_groups or [] for server in servers + if server['status'] != 'UNKNOWN' + } + return instances_security_group_bindings - def get_instance_security_groups( - self, context, instance, detailed=False): - return security_groups if security_groups is not None else [] + def get_instance_security_groups(context, instance, detailed=False): + return security_groups if security_groups is not None else [] test.stub_out( - 'nova.network.security_group.neutron_driver.SecurityGroupAPI', - FakeSecurityGroupAPI) + 'nova.network.security_group_api' + '.get_instances_security_groups_bindings', + get_instances_security_groups_bindings) + test.stub_out( + 'nova.network.security_group_api.get_instance_security_groups', + get_instance_security_groups) class FakeToken(object): diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index e6eb241f35b8..f2d6394bf5ea 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -8696,8 +8696,8 @@ class ComputeAPITestCase(BaseTestCase): with test.nested( mock.patch.object(self.compute_api.compute_task_api, 'schedule_and_build_instances'), - mock.patch.object(self.compute_api.security_group_api, 'get', - return_value=group), + mock.patch('nova.network.security_group_api.get', + return_value=group), ) as (mock_sbi, mock_secgroups): self.compute_api.create( self.context, @@ -8714,10 +8714,8 @@ class ComputeAPITestCase(BaseTestCase): def test_create_instance_with_invalid_security_group_raises(self): pre_build_len = len(db.instance_get_all(self.context)) - with test.nested( - mock.patch.object(self.compute_api.security_group_api, 'get', - return_value=None), - ) as (mock_secgroups, ): + with mock.patch('nova.network.security_group_api.get', + return_value=None) as mock_secgroups: self.assertRaises(exception.SecurityGroupNotFoundForProject, self.compute_api.create, self.context, diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 11688af60cbd..b6d3066abaae 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -4561,12 +4561,10 @@ class _ComputeAPIUnitTestMixIn(object): expected_exception): @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(objects.Instance, 'create') - @mock.patch.object(self.compute_api.security_group_api, - 'ensure_default') @mock.patch.object(objects.RequestSpec, 'from_components') def do_test( - mock_req_spec_from_components, _mock_ensure_default, - _mock_create, mock_check_num_inst_quota): + mock_req_spec_from_components, _mock_create, + mock_check_num_inst_quota): req_spec_mock = mock.MagicMock() mock_check_num_inst_quota.return_value = 1 @@ -4654,7 +4652,7 @@ class _ComputeAPIUnitTestMixIn(object): '_create_reqspec_buildreq_instmapping', new=mock.MagicMock()) @mock.patch('nova.compute.utils.check_num_instances_quota') - @mock.patch.object(self.compute_api, 'security_group_api') + @mock.patch('nova.network.security_group_api') @mock.patch.object(self.compute_api, 'create_db_entry_for_new_instance') @mock.patch.object(self.compute_api, @@ -4691,12 +4689,9 @@ class _ComputeAPIUnitTestMixIn(object): '_create_reqspec_buildreq_instmapping') @mock.patch.object(objects.Instance, 'create') @mock.patch('nova.compute.utils.check_num_instances_quota') - @mock.patch.object(self.compute_api.security_group_api, - 'ensure_default') @mock.patch.object(objects.RequestSpec, 'from_components') - def do_test(mock_req_spec_from_components, _mock_ensure_default, - mock_check_num_inst_quota, mock_inst_create, - mock_create_rs_br_im, mock_get_volumes): + def do_test(mock_req_spec_from_components, mock_check_num_inst_quota, + mock_inst_create, mock_create_rs_br_im, mock_get_volumes): min_count = 1 max_count = 2 @@ -4780,8 +4775,6 @@ class _ComputeAPIUnitTestMixIn(object): new=mock.MagicMock()) @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(objects.Instance, 'create', new=mock.MagicMock()) - @mock.patch.object(self.compute_api.security_group_api, - 'ensure_default', new=mock.MagicMock()) @mock.patch.object(self.compute_api, '_validate_bdm', new=mock.MagicMock()) @mock.patch.object(objects.RequestSpec, 'from_components', @@ -4871,14 +4864,12 @@ class _ComputeAPIUnitTestMixIn(object): '_create_reqspec_buildreq_instmapping') @mock.patch('nova.compute.utils.check_num_instances_quota') @mock.patch.object(objects, 'Instance') - @mock.patch.object(self.compute_api.security_group_api, - 'ensure_default') @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(objects, 'BuildRequest') @mock.patch.object(objects, 'InstanceMapping') def do_test(mock_inst_mapping, mock_build_req, - mock_req_spec_from_components, _mock_ensure_default, - mock_inst, mock_check_num_inst_quota, mock_create_rs_br_im): + mock_req_spec_from_components, mock_inst, + mock_check_num_inst_quota, mock_create_rs_br_im): min_count = 1 max_count = 2 @@ -4963,7 +4954,8 @@ class _ComputeAPIUnitTestMixIn(object): '_create_reqspec_buildreq_instmapping', new=mock.MagicMock()) @mock.patch('nova.compute.utils.check_num_instances_quota') - @mock.patch.object(self.compute_api, 'security_group_api') + @mock.patch('nova.network.security_group_api' + '.populate_security_groups') @mock.patch.object(compute_api, 'objects') @mock.patch.object(self.compute_api, 'create_db_entry_for_new_instance', @@ -4979,7 +4971,7 @@ class _ComputeAPIUnitTestMixIn(object): [], None, None, None, None, None, objects.TagList(), None, False) - secgroups = mock_secgroup.populate_security_groups.return_value + secgroups = mock_secgroup.return_value mock_objects.RequestSpec.from_components.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, mock.ANY, @@ -6560,8 +6552,8 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): objects.NetworkRequest(network_id='none')]) max_count = 1 supports_port_resource_request = False - with mock.patch.object( - self.compute_api.security_group_api, 'get', + with mock.patch( + 'nova.network.security_group_api.get', return_value={'id': uuids.secgroup_uuid}) as scget: base_options, max_network_count, key_pair, security_groups, \ network_metadata = ( diff --git a/nova/tests/unit/network/test_config.py b/nova/tests/unit/network/test_config.py deleted file mode 100644 index 6ac4a95f216d..000000000000 --- a/nova/tests/unit/network/test_config.py +++ /dev/null @@ -1,29 +0,0 @@ -# Copyright 2016 HPE, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import mock - -import nova.network -import nova.network.security_group.openstack_driver as sgapi -import nova.test - - -class SecurityGroupAPIConfigTest(nova.test.NoDBTestCase): - - @mock.patch('oslo_utils.importutils.import_object') - def test_caches(self, mock_import): - sgapi.DRIVER_CACHE = None - for _ in range(2): - self.assertIsNotNone(sgapi.get_openstack_security_group_driver()) - mock_import.assert_called_once_with(sgapi.NEUTRON_DRIVER) diff --git a/nova/tests/unit/network/security_group/test_neutron_driver.py b/nova/tests/unit/network/test_security_group.py similarity index 94% rename from nova/tests/unit/network/security_group/test_neutron_driver.py rename to nova/tests/unit/network/test_security_group.py index 0b7fc352ed51..08e8761c87fb 100644 --- a/nova/tests/unit/network/security_group/test_neutron_driver.py +++ b/nova/tests/unit/network/test_security_group.py @@ -22,7 +22,7 @@ from six.moves import range from nova import context from nova import exception -from nova.network.security_group import neutron_driver +from nova.network import security_group_api as sg_api from nova import objects from nova import test @@ -44,7 +44,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client.list_security_groups.return_value = ( security_groups_list) - sg_api = neutron_driver.SecurityGroupAPI() sg_api.list(self.context, project=project_id) self.mocked_client.list_security_groups.assert_called_once_with( @@ -60,7 +59,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client, 'list_security_groups', return_value=security_groups_list) as mock_list_secgroup: - sg_api = neutron_driver.SecurityGroupAPI() sg_api.list(admin_context, project=project_id, search_opts=search_opts) @@ -76,7 +74,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client, 'list_security_groups', return_value=security_groups_list) as mock_list_secgroup: - sg_api = neutron_driver.SecurityGroupAPI() sg_api.list(admin_context, project=project_id) mock_list_secgroup.assert_called_once_with(tenant_id=project_id) @@ -92,7 +89,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client, 'list_security_groups', return_value=security_groups_list) as mock_list_secgroup: - sg_api = neutron_driver.SecurityGroupAPI() sg_api.list(admin_context, project=project_id, names=security_group_names, search_opts=search_opts) @@ -113,7 +109,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client, 'list_security_groups', return_value=security_groups_list) as mock_list_secgroup: - sg_api = neutron_driver.SecurityGroupAPI() sg_api.list(admin_context, project=project_id, names=security_group_names, ids=security_group_ids, @@ -132,7 +127,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client, 'list_security_groups', return_value=security_groups_list) as mock_list_secgroup: - sg_api = neutron_driver.SecurityGroupAPI() sg_api.list(self.context, project=self.context.project_id, search_opts=search_opts) @@ -148,7 +142,6 @@ class TestNeutronDriver(test.NoDBTestCase): 'description': 'server', 'rules': []}} self.mocked_client.show_security_group.return_value = expected_sg - sg_api = neutron_driver.SecurityGroupAPI() 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) @@ -165,7 +158,6 @@ class TestNeutronDriver(test.NoDBTestCase): with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id', return_value=expected_sg_id): - sg_api = neutron_driver.SecurityGroupAPI() self.assertRaises(exception.SecurityGroupNotFound, sg_api.get, self.context, name=sg_name) self.mocked_client.show_security_group.assert_called_once_with( @@ -180,7 +172,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client.create_security_group.side_effect = ( n_exc.BadRequest(message=message)) - sg_api = neutron_driver.SecurityGroupAPI() self.assertRaises(exception.Invalid, sg_api.create_security_group, self.context, name, description) @@ -194,7 +185,6 @@ class TestNeutronDriver(test.NoDBTestCase): message = "Quota exceeded for resources: ['security_group']" self.mocked_client.create_security_group.side_effect = ( n_exc.NeutronClientException(status_code=409, message=message)) - sg_api = neutron_driver.SecurityGroupAPI() self.assertRaises(exception.SecurityGroupLimitExceeded, sg_api.create_security_group, self.context, name, description) @@ -213,7 +203,6 @@ class TestNeutronDriver(test.NoDBTestCase): message = "Quota exceeded for resources: ['security_group_rule']" self.mocked_client.create_security_group_rule.side_effect = ( n_exc.NeutronClientException(status_code=409, message=message)) - sg_api = neutron_driver.SecurityGroupAPI() self.assertRaises(exception.SecurityGroupLimitExceeded, sg_api.add_rules, self.context, None, name, [vals]) self.mocked_client.create_security_group_rule.assert_called_once_with( @@ -233,7 +222,6 @@ class TestNeutronDriver(test.NoDBTestCase): " (port-range-min) is missing" self.mocked_client.create_security_group_rule.side_effect = ( n_exc.NeutronClientException(status_code=400, message=message)) - sg_api = neutron_driver.SecurityGroupAPI() self.assertRaises(exception.Invalid, sg_api.add_rules, self.context, None, name, [vals]) self.mocked_client.create_security_group_rule.assert_called_once_with( @@ -259,7 +247,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client.list_security_groups.return_value = ( {'security_groups': [sg1]}) - sg_api = neutron_driver.SecurityGroupAPI() result = sg_api.list(self.context) expected = [{'rules': [{'from_port': -1, 'protocol': '51', 'to_port': -1, @@ -292,7 +279,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client.list_security_groups.return_value = ( security_groups_list) - sg_api = neutron_driver.SecurityGroupAPI() with mock.patch.object( sg_api, '_convert_to_nova_security_group_format') as convert: result = sg_api.get_instances_security_groups_bindings( @@ -326,7 +312,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client.list_ports.side_effect = n_exc.PortNotFoundClient() - sg_api = neutron_driver.SecurityGroupAPI() result = sg_api.get_instances_security_groups_bindings( self.context, servers) self.mocked_client.list_ports.assert_called_once_with( @@ -365,7 +350,6 @@ class TestNeutronDriver(test.NoDBTestCase): security_groups_list) self.mocked_client.list_ports.side_effect = return_values - sg_api = neutron_driver.SecurityGroupAPI() result = sg_api.get_instances_security_groups_bindings( self.context, servers) self.assertEqual(sg_bindings, result) @@ -400,7 +384,6 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client.list_security_groups.return_value = ( security_groups_list) - sg_api = neutron_driver.SecurityGroupAPI() result = sg_api.get_instances_security_groups_bindings( self.context, servers) self.assertEqual(sg_bindings, result) @@ -416,7 +399,6 @@ class TestNeutronDriver(test.NoDBTestCase): port_list = {'ports': [{'id': 1, 'device_id': uuids.instance, 'security_groups': []}]} self.mocked_client.list_ports.return_value = port_list - sg_api = neutron_driver.SecurityGroupAPI() result = sg_api.get_instance_security_groups( self.context, objects.Instance(uuid=uuids.instance)) self.assertEqual([], result) @@ -431,7 +413,6 @@ class TestNeutronDriver(test.NoDBTestCase): 'fixed_ips': [{'ip_address': '10.0.0.1'}], 'port_security_enabled': True, 'security_groups': []}]} self.mocked_client.list_ports.return_value = port_list - sg_api = neutron_driver.SecurityGroupAPI() with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id', return_value=sg_id): sg_api.add_to_instance( @@ -449,7 +430,6 @@ class TestNeutronDriver(test.NoDBTestCase): 'fixed_ips': [{'ip_address': '10.0.0.1'}], 'port_security_enabled': True, 'security_groups': []}]} self.mocked_client.list_ports.return_value = port_list - sg_api = neutron_driver.SecurityGroupAPI() self.mocked_client.update_port.side_effect = ( n_exc.BadRequest(message='error')) with mock.patch.object(neutronv20, 'find_resourceid_by_name_or_id', @@ -466,8 +446,6 @@ class TestNeutronDriver(test.NoDBTestCase): class TestNeutronDriverWithoutMock(test.NoDBTestCase): def test_validate_property(self): - sg_api = neutron_driver.SecurityGroupAPI() - sg_api.validate_property('foo', 'name', None) sg_api.validate_property('', 'name', None) self.assertRaises(exception.Invalid, sg_api.validate_property, @@ -476,7 +454,6 @@ class TestNeutronDriverWithoutMock(test.NoDBTestCase): None, 'name', None) def test_populate_security_groups(self): - sg_api = neutron_driver.SecurityGroupAPI() r = sg_api.populate_security_groups(['default', uuids.secgroup_uuid]) self.assertIsInstance(r, objects.SecurityGroupList) self.assertEqual(2, len(r))