openstack
/
nova
Code Issues Proposed changes

Fix security group race condition while listing and deleting rules 

Previously, there was a race condition that could occur if one was listing
a security group which contained a rule which referenced another security
group as an additional api call is used to look up the security group's
name which is returned (rather than id) via the API. The problem occurs if
this security group is deleted in which case a 404 was raised.
This patch fixes this issue by catching the 404 and ignoring the rule as
its already deleted as there is a constraint enforced that you cannot
delete a security group if another security group has a rule that references
that security group.

Change-Id: I3855b8d3a1bd2f7c88901da071f9bd5581bd56e4
Closes-bug: 1262566
(cherry picked from commit eb84cfb4ef)
This commit is contained in:
Aaron Rosen 2014-05-08 11:25:02 -07:00 committed by Sean Dague
parent 7431cb9272
commit 867341ff27
2 changed files with 73 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
nova/api/openstack/compute/contrib/security_groups.py
View File

@ -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):

41
nova/tests/api/openstack/compute/contrib/test_security_groups.py
View File

@ -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 = []