diff options
| -rw-r--r-- | nova/api/openstack/compute/contrib/security_groups.py | 40 | ||||
| -rw-r--r-- | nova/tests/api/openstack/compute/contrib/test_security_groups.py | 41 |
2 files changed, 73 insertions, 8 deletions
diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py index 496c4dc962..73d0f4b64d 100644 --- a/nova/api/openstack/compute/contrib/security_groups.py +++ b/nova/api/openstack/compute/contrib/security_groups.py @@ -31,10 +31,12 @@ from nova import exception from nova.network.security_group import neutron_driver from nova.network.security_group import openstack_driver from nova.openstack.common.gettextutils import _ +from nova.openstack.common import log as logging from nova.openstack.common import xmlutils from nova.virt import netutils +LOG = logging.getLogger(__name__) authorize = extensions.extension_authorizer('compute', 'security_groups') softauth = extensions.soft_extension_authorizer('compute', 'security_groups') @@ -217,10 +219,22 @@ class SecurityGroupControllerBase(object): sg_rule['ip_range'] = {} if rule['group_id']: with translate_exceptions(): - source_group = self.security_group_api.get(context, - id=rule['group_id']) + try: + source_group = self.security_group_api.get( + context, id=rule['group_id']) + except exception.SecurityGroupNotFound: + # NOTE(arosen): There is a possible race condition that can + # occur here if two api calls occur concurrently: one that + # lists the security groups and another one that deletes a + # security group rule that has a group_id before the + # group_id is fetched. To handle this if + # SecurityGroupNotFound is raised we return None instead + # of the rule and the caller should ignore the rule. + LOG.debug("Security Group ID %s does not exist", + rule['group_id']) + return sg_rule['group'] = {'name': source_group.get('name'), - 'tenant_id': source_group.get('project_id')} + 'tenant_id': source_group.get('project_id')} else: sg_rule['ip_range'] = {'cidr': rule['cidr']} return sg_rule @@ -233,8 +247,9 @@ class SecurityGroupControllerBase(object): security_group['tenant_id'] = group['project_id'] security_group['rules'] = [] for rule in group['rules']: - security_group['rules'] += [self._format_security_group_rule( - context, rule)] + formatted_rule = self._format_security_group_rule(context, rule) + if formatted_rule: + security_group['rules'] += [formatted_rule] return security_group def _from_body(self, body, key): @@ -384,9 +399,18 @@ class SecurityGroupRulesController(SecurityGroupControllerBase): self.security_group_api.create_security_group_rule( context, security_group, new_rule)) - return {"security_group_rule": self._format_security_group_rule( - context, - security_group_rule)} + formatted_rule = self._format_security_group_rule(context, + security_group_rule) + if formatted_rule: + return {"security_group_rule": formatted_rule} + + # TODO(arosen): if we first look up the security group information for + # the group_id before creating the rule we can avoid the case that + # the remote group (group_id) has been deleted when we go to look + # up it's name. + with translate_exceptions(): + raise exception.SecurityGroupNotFound( + security_group_id=sg_rule['group_id']) def _rule_args_to_dict(self, context, to_port=None, from_port=None, ip_protocol=None, cidr=None, group_id=None): diff --git a/nova/tests/api/openstack/compute/contrib/test_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_security_groups.py index 5577df238e..da2c06dfce 100644 --- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py +++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py @@ -310,6 +310,47 @@ class TestSecurityGroups(test.TestCase): self.assertEqual(res_dict, expected) + def test_get_security_group_list_missing_group_id_rule(self): + groups = [] + rule1 = security_group_rule_template(cidr='10.2.3.124/24', + parent_group_id=1, + group_id={}, id=88, + protocol='TCP') + rule2 = security_group_rule_template(cidr='10.2.3.125/24', + parent_group_id=1, + id=99, protocol=88, + group_id='HAS_BEEN_DELETED') + sg = security_group_template(id=1, + name='test', + description='test-desc', + rules=[rule1, rule2]) + + groups.append(sg) + # An expected rule here needs to be created as the api returns + # different attributes on the rule for a response than what was + # passed in. For exmaple: + # "cidr": "0.0.0.0/0" ->"ip_range": {"cidr": "0.0.0.0/0"} + expected_rule = security_group_rule_template( + ip_range={'cidr': '10.2.3.124/24'}, parent_group_id=1, + group={}, id=88, ip_protocol='TCP') + expected = security_group_template(id=1, + name='test', + description='test-desc', + rules=[expected_rule]) + + expected = {'security_groups': [expected]} + + def return_security_groups(context, project, search_opts): + return [security_group_db(sg) for sg in groups] + + self.stubs.Set(self.controller.security_group_api, 'list', + return_security_groups) + + req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups') + res_dict = self.controller.index(req) + + self.assertEqual(res_dict, expected) + def test_get_security_group_list_all_tenants(self): all_groups = [] tenant_groups = [] |