openstack/ec2-api
Code Issues Proposed changes

Fixes in security_groups.

Browse Source 
Fixed exception for orphan "vpc-..."-named groups in describe.
Added check for not deleting default VPC group.
Added check for not deleting VPC if non-default groups exist.
Added check for not allowing creation of groups with existing names.

Change-Id: Ia205ff65409577d148c08b40a297d521d8a8acb1
This commit is contained in:
Alexandre Levine 2015-03-30 21:50:34 +04:00
parent 46edb3ccd1
commit 5332325708
5 changed files with 70 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
ec2api/api/security_group.py
View File

@ -59,6 +59,15 @@ def get_security_group_engine():
def create_security_group(context, group_name, group_description,
vpc_id=None):
nova = clients.nova(context)
if vpc_id:
security_groups = describe_security_groups(
context,
filter=[{'name': 'vpc-id',
'value': [vpc_id]},
{'name': 'group-name',
'value': [group_name]}])['securityGroupInfo']
if security_groups:
raise exception.InvalidGroupDuplicate(name=group_name)
with common.OnCrashCleaner() as cleaner:
try:
# TODO(Alex): Shouldn't allow creation of groups with existing
@ -87,10 +96,12 @@ def _create_default_security_group(context, vpc):
'Default VPC security group', vpc['id'])
def delete_security_group(context, group_name=None, group_id=None):
def delete_security_group(context, group_name=None, group_id=None,
delete_default=False):
if group_name is None and group_id is None:
raise exception.MissingParameter(param='group id or name')
security_group_engine.delete_group(context, group_name, group_id)
security_group_engine.delete_group(context, group_name, group_id,
delete_default)
return True
@ -291,7 +302,7 @@ def _translate_group_name(context, os_group, db_groups):
# in all of the subsequent handling (filtering, using in parameters...)
if (os_group['name'].startswith('vpc-') and db_groups and
next((g for g in db_groups
if g['os_id'] == os_group['id']))):
if g['os_id'] == os_group['id']), None)):
return 'default'
return os_group['name']
@ -379,7 +390,8 @@ def _format_security_group(security_group, os_security_group,
class SecurityGroupEngineNeutron(object):
def delete_group(self, context, group_name=None, group_id=None):
def delete_group(self, context, group_name=None, group_id=None,
delete_default=False):
neutron = clients.neutron(context)
if group_id is None or not group_id.startswith('sg-'):
return SecurityGroupEngineNova().delete_group(context,
@ -387,6 +399,13 @@ class SecurityGroupEngineNeutron(object):
group_id)
security_group = ec2utils.get_db_item(context, group_id)
try:
if not delete_default:
os_security_group = neutron.show_security_group(
security_group['os_id'])
if (os_security_group and
os_security_group['security_group']['name'] ==
security_group['vpc_id']):
raise exception.CannotDelete()
neutron.delete_security_group(security_group['os_id'])
except neutron_exception.Conflict as ex:
# TODO(Alex): Instance ID is unknown here, report exception message
@ -435,7 +454,8 @@ class SecurityGroupEngineNeutron(object):
class SecurityGroupEngineNova(object):
def delete_group(self, context, group_name=None, group_id=None):
def delete_group(self, context, group_name=None, group_id=None,
delete_default=False):
nova = clients.nova(context)
os_id = self.get_group_os_id(context, group_id, group_name)
try:

18
ec2api/api/vpc.py
View File

@ -75,7 +75,12 @@ def delete_vpc(context, vpc_id):
route_tables = route_table_api.describe_route_tables(
context,
filter=[{'name': 'vpc-id', 'value': [vpc['id']]}])['routeTableSet']
if subnets or internet_gateways or len(route_tables) > 1:
security_groups = security_group_api.describe_security_groups(
context,
filter=[{'name': 'vpc-id',
'value': [vpc['id']]}])['securityGroupInfo']
if (subnets or internet_gateways or len(route_tables) > 1 or
len(security_groups) > 1):
msg = _("The vpc '%(vpc_id)s' has dependencies and "
"cannot be deleted.")
msg = msg % {'vpc_id': vpc['id']}
@ -87,15 +92,10 @@ def delete_vpc(context, vpc_id):
cleaner.addCleanup(db_api.restore_item, context, 'vpc', vpc)
route_table_api._delete_route_table(context, vpc['route_table_id'],
cleaner=cleaner)
# TODO(Alex): Check that only the default security group is left
# in this VPC, otherwise DependencyViolation.
security_groups = security_group_api.describe_security_groups(
context,
filter=[{'name': 'vpc-id',
'value': [vpc['id']]}])['securityGroupInfo']
for security_group in security_groups:
if len(security_groups) > 0:
security_group_api.delete_security_group(
context, group_id=security_group['groupId'])
context, group_id=security_groups[0]['groupId'],
delete_default=True)
try:
neutron.delete_router(vpc['os_id'])
except neutron_exception.Conflict as ex:

9
ec2api/exception.py
View File

@ -243,6 +243,10 @@ class DependencyViolation(EC2IncorrectStateException):
msg_fmt = _('Object %(obj1_id)s has dependent resource %(obj2_id)s')
class CannotDelete(EC2IncorrectStateException):
msg_fmt = _('Cannot delete the default VPC security group')
class ResourceAlreadyAssociated(EC2IncorrectStateException):
ec2_code = 'Resource.AlreadyAssociated'
@ -283,6 +287,11 @@ class InvalidPermissionDuplicate(EC2DuplicateException):
msg_fmt = _('The specified rule already exists for that security group.')
class InvalidGroupDuplicate(EC2DuplicateException):
ec2_code = 'InvalidGroup.Duplicate'
msg_fmt = _("Security group '%(name)s' already exists.")
class RouteAlreadyExists(EC2DuplicateException):
msg_fmt = _('The route identified by %(destination_cidr_block)s '
'already exists.')

17
ec2api/tests/unit/test_security_group.py
View File

@ -104,6 +104,14 @@ class SecurityGroupTestCase(base.ApiTestCase):
do_check({'GroupDescription': 'description'},
'MissingParameter')
self.set_mock_db_items(fakes.DB_SECURITY_GROUP_2, fakes.DB_VPC_1)
self.neutron.list_security_groups.return_value = (
{'security_groups': [fakes.OS_SECURITY_GROUP_2]})
do_check({'VpcId': fakes.ID_EC2_VPC_1,
'GroupName': fakes.OS_SECURITY_GROUP_2['name'],
'GroupDescription': 'description'},
'InvalidGroup.Duplicate')
def test_create_security_group_over_quota(self):
security_group.security_group_engine = (
security_group.SecurityGroupEngineNeutron())
@ -184,10 +192,15 @@ class SecurityGroupTestCase(base.ApiTestCase):
def test_delete_security_group_invalid(self):
security_group.security_group_engine = (
security_group.SecurityGroupEngineNeutron())
self.set_mock_db_items()
self.set_mock_db_items(fakes.DB_SECURITY_GROUP_1, fakes.DB_VPC_1)
self.neutron.show_security_group.return_value = (
{'security_group': fakes.OS_SECURITY_GROUP_1})
self.assert_execution_error(
'CannotDelete', 'DeleteSecurityGroup',
{'GroupId': fakes.ID_EC2_SECURITY_GROUP_1})
self.assert_execution_error(
'InvalidGroup.NotFound', 'DeleteSecurityGroup',
{'GroupId': fakes.ID_EC2_SECURITY_GROUP_1})
{'GroupId': fakes.ID_EC2_SECURITY_GROUP_2})
self.assertEqual(0, self.neutron.delete_port.call_count)
self.assert_execution_error(
'InvalidGroup.NotFound', 'DeleteSecurityGroup',

13
ec2api/tests/unit/test_vpc.py
View File

@ -114,7 +114,8 @@ class VpcTestCase(base.ApiTestCase):
fakes.ID_EC2_ROUTE_TABLE_1)
def test_delete_vpc(self):
self.set_mock_db_items(fakes.DB_VPC_1, fakes.DB_ROUTE_TABLE_1)
self.set_mock_db_items(fakes.DB_VPC_1, fakes.DB_ROUTE_TABLE_1,
fakes.DB_SECURITY_GROUP_1)
resp = self.execute('DeleteVpc', {'VpcId': fakes.ID_EC2_VPC_1})
@ -127,6 +128,9 @@ class VpcTestCase(base.ApiTestCase):
self.db_api.delete_item.assert_any_call(
mock.ANY,
fakes.ID_EC2_ROUTE_TABLE_1)
self.db_api.delete_item.assert_any_call(
mock.ANY,
fakes.ID_EC2_SECURITY_GROUP_1)
def test_delete_vpc_not_found(self):
self.set_mock_db_items()
@ -153,6 +157,13 @@ class VpcTestCase(base.ApiTestCase):
fakes.DB_VPC_1)
do_check()
self.set_mock_db_items(fakes.DB_SECURITY_GROUP_1,
fakes.DB_SECURITY_GROUP_2, fakes.DB_VPC_1)
self.neutron.list_security_groups.return_value = (
{'security_groups': [fakes.OS_SECURITY_GROUP_1,
fakes.OS_SECURITY_GROUP_2]})
do_check()
def test_delete_vpc_not_conststent_os_vpc(self):
self.set_mock_db_items(fakes.DB_VPC_1, fakes.DB_ROUTE_TABLE_1)