describe-security-groups by name in default VPC mode describes

security groups in default vpc only

changed check_and_create_default_vpc function to return default vpc

fix unit test of describe_security_groups

Change-Id: I50bad4a6a7152eb03599e3985a43162c81500049

ec2api/api/common.py

@@ -387,6 +387,10 @@ class UniversalDescriber(object):
 
         return formatted_items
 
+    def is_selected_item(self, context, os_item_name, item):
+        return (os_item_name in self.names or
+                (item and item['id'] in self.ids))
+
     def handle_unpaired_item(self, item):
         self.delete_obsolete_item(item)
 
@@ -415,8 +419,7 @@ class UniversalDescriber(object):
                 paired_items_ids.add(item['id'])
             # NOTE(Alex): Filter out items not requested in names or ids
             if (self.selective_describe and
-                    not (os_item_name in self.names or
+                    not self.is_selected_item(context, os_item_name, item)):
-                         (item and item['id'] in self.ids))):
                 continue
             # NOTE(Alex): Autoupdate DB for autoupdatable items
             item = self.auto_update_db(item, os_item)

ec2api/api/ec2utils.py

@@ -495,9 +495,7 @@ def set_check_and_create_default_vpc(check_and_create_default_vpc):
 
 
 def get_default_vpc(context):
-    check_and_create_default_vpc(context)
+    default_vpc = check_and_create_default_vpc(context)
-    default_vpc = next((vpc for vpc in db_api.get_items(context, 'vpc')
-                        if vpc.get('is_default')), None)
     if not default_vpc:
         raise exception.VPCIdNotSpecified()
     return default_vpc

ec2api/api/security_group.py

@@ -82,7 +82,6 @@ def create_security_group(context, group_name, group_description,
                            if sg.get('vpcId') is None]
     if security_groups:
         raise exception.InvalidGroupDuplicate(name=group_name)
-
     return _create_security_group(context, group_name, group_description,
                                   vpc_id)
 
@@ -158,9 +157,10 @@ class SecurityGroupDescriber(common.TaggableItemsDescriber):
                   'vpc-id': 'vpcId',
     }
 
-    def __init__(self):
+    def __init__(self, default_vpc_id):
         super(SecurityGroupDescriber, self).__init__()
         self.all_db_items = None
+        self.default_vpc_id = default_vpc_id
 
     def format(self, item=None, os_item=None):
         return _format_security_group(item, os_item,
@@ -196,12 +196,26 @@ class SecurityGroupDescriber(common.TaggableItemsDescriber):
                 had_to_repair = True
         return had_to_repair
 
+    def is_selected_item(self, context, os_item_name, item):
+        if item and item['id'] in self.ids:
+            return True
+        if os_item_name in self.names:
+            if not CONF.disable_ec2_classic:
+                return (not item or not item['vpc_id'])
+            else:
+                return (self.default_vpc_id and item and
+                        item['vpc_id'] == self.default_vpc_id)
+        return False
+
 
 def describe_security_groups(context, group_name=None, group_id=None,
                              filter=None):
-    ec2utils.check_and_create_default_vpc(context)
+    default_vpc_id = None
-    formatted_security_groups = SecurityGroupDescriber().describe(
+    default_vpc = ec2utils.check_and_create_default_vpc(context)
-        context, group_id, group_name, filter)
+    if default_vpc:
+        default_vpc_id = default_vpc['id']
+    formatted_security_groups = SecurityGroupDescriber(
+        default_vpc_id).describe(context, group_id, group_name, filter)
     return {'securityGroupInfo': formatted_security_groups}
 
 
@@ -648,7 +662,7 @@ class SecurityGroupEngineNova(object):
         nova_group = next((g for g in nova_security_groups
                            if g.name == group_name), None)
         if nova_group is None:
-            raise exception.InvalidGroupNotFound(sg_id=group_name)
+            raise exception.InvalidGroupNotFound(id=group_name)
         return nova_group
 
 

ec2api/api/vpc.py

@@ -164,13 +164,17 @@ def _create_vpc(context, cidr_block, is_default=False):
 
 
 def _check_and_create_default_vpc(context):
-    if (CONF.disable_ec2_classic and
+    if (CONF.disable_ec2_classic):
-            not any(vpc.get('is_default')
+        for vpc in db_api.get_items(context, 'vpc'):
-                    for vpc in db_api.get_items(context, 'vpc'))):
+            if vpc.get('is_default'):
+                return vpc
         try:
-            _create_vpc(context, DEFAULT_VPC_CIDR_BLOCK, is_default=True)
+            default_vpc = _create_vpc(context, DEFAULT_VPC_CIDR_BLOCK,
+                                      is_default=True)
+            return default_vpc
         except Exception:
             LOG.exception('Failed to create default vpc')
+        return None
 
 ec2utils.set_check_and_create_default_vpc(_check_and_create_default_vpc)
 

ec2api/exception.py

@@ -387,7 +387,7 @@ class InvalidSecurityGroupIDNotFound(EC2NotFoundException):
 
 class InvalidGroupNotFound(EC2NotFoundException):
     ec2_code = 'InvalidGroup.NotFound'
-    msg_fmg = _("The security group ID '%(id)s' does not exist")
+    msg_fmt = _("The security group ID '%(id)s' does not exist")
 
 
 class InvalidPermissionNotFound(EC2NotFoundException):

ec2api/tests/unit/fakes.py

@@ -162,10 +162,15 @@ ID_EC2_SECURITY_GROUP_DEFAULT = random_ec2_id('sg')
 ID_EC2_SECURITY_GROUP_1 = random_ec2_id('sg')
 ID_EC2_SECURITY_GROUP_2 = random_ec2_id('sg')
 ID_EC2_SECURITY_GROUP_3 = random_ec2_id('sg')
+ID_EC2_SECURITY_GROUP_4 = random_ec2_id('sg')
+ID_EC2_SECURITY_GROUP_5 = random_ec2_id('sg')
+ID_EC2_SECURITY_GROUP_6 = random_ec2_id('sg')
 ID_OS_SECURITY_GROUP_DEFAULT = random_os_id()
 ID_OS_SECURITY_GROUP_1 = random_os_id()
 ID_OS_SECURITY_GROUP_2 = random_os_id()
 ID_OS_SECURITY_GROUP_3 = random_os_id()
+ID_OS_SECURITY_GROUP_4 = random_os_id()
+ID_OS_SECURITY_GROUP_5 = random_os_id()
 
 ID_NOVA_OS_SECURITY_GROUP_1 = 1
 ID_NOVA_OS_SECURITY_GROUP_2 = 2
@@ -1137,6 +1142,21 @@ DB_SECURITY_GROUP_3 = {
     'os_id': ID_OS_SECURITY_GROUP_3,
     'vpc_id': None,
 }
+DB_SECURITY_GROUP_4 = {
+    'id': ID_EC2_SECURITY_GROUP_4,
+    'os_id': ID_OS_SECURITY_GROUP_4,
+    'vpc_id': None,
+}
+DB_SECURITY_GROUP_5 = {
+    'id': ID_EC2_SECURITY_GROUP_5,
+    'os_id': ID_OS_SECURITY_GROUP_5,
+    'vpc_id': ID_EC2_VPC_DEFAULT,
+}
+DB_SECURITY_GROUP_6 = {
+    'id': ID_EC2_SECURITY_GROUP_6,
+    'os_id': None,
+    'vpc_id': None,
+}
 OS_SECURITY_GROUP_RULE_1 = {
     'direction': 'ingress',
     'ethertype': 'IPv4',
@@ -1216,6 +1236,38 @@ OS_SECURITY_GROUP_3 = {
     'description': 'Group description',
     'tenant_id': ID_OS_PROJECT
 }
+OS_SECURITY_GROUP_4 = {
+    'id': ID_OS_SECURITY_GROUP_4,
+    'name': 'groupname2',
+    'security_group_rules': [
+        {'direction': 'ingress',
+         'ethertype': 'IPv4',
+         'id': random_os_id(),
+         'port_range_min': 10,
+         'port_range_max': 10,
+         'protocol': 'tcp',
+         'remote_group_id': None,
+         'remote_ip_prefix': '192.168.1.0/24',
+         'security_group_id': ID_OS_SECURITY_GROUP_4},
+        {'direction': 'egress',
+         'ethertype': 'IPv4',
+         'id': random_os_id(),
+         'port_range_min': 10,
+         'port_range_max': None,
+         'protocol': 100,
+         'remote_group_id': ID_OS_SECURITY_GROUP_1,
+         'remote_ip_prefix': None,
+         'security_group_id': ID_OS_SECURITY_GROUP_4}
+    ],
+    'description': 'Group description',
+    'tenant_id': ID_OS_PROJECT
+}
+OS_SECURITY_GROUP_5 = {
+    'id': ID_OS_SECURITY_GROUP_5,
+    'name': 'groupname2',
+    'description': 'Group description',
+    'tenant_id': ID_OS_PROJECT
+}
 EC2_SECURITY_GROUP_DEFAULT = {
     'vpcId': ID_EC2_VPC_DEFAULT,
     'groupDescription': 'Group description',
@@ -1271,6 +1323,37 @@ EC2_SECURITY_GROUP_3 = {
     'ownerId': ID_OS_PROJECT,
     'groupId': ID_EC2_SECURITY_GROUP_3
 }
+EC2_SECURITY_GROUP_4 = {
+    'groupDescription': 'Group description',
+    'ipPermissions':
+    [{'toPort': 10,
+      'ipProtocol': 'tcp',
+      'fromPort': 10,
+      'ipRanges':
+      [{'cidrIp': '192.168.1.0/24'}]
+      }],
+    'groupName': 'groupname2',
+    'ipPermissionsEgress':
+    [{'toPort': -1,
+      'ipProtocol': 100,
+      'fromPort': 10,
+      'groups':
+      [{'groupId': ID_EC2_SECURITY_GROUP_1,
+        'groupName': NAME_DEFAULT_OS_SECURITY_GROUP,
+        'userId': ID_OS_PROJECT}]
+      }],
+    'ownerId': ID_OS_PROJECT,
+    'groupId': ID_EC2_SECURITY_GROUP_4
+}
+EC2_SECURITY_GROUP_5 = {
+    'vpcId': ID_EC2_VPC_DEFAULT,
+    'groupDescription': 'Group description',
+    'ipPermissions': None,
+    'ipPermissionsEgress': None,
+    'groupName': 'groupname2',
+    'ownerId': ID_OS_PROJECT,
+    'groupId': ID_EC2_SECURITY_GROUP_5
+}
 
 NOVA_DB_SECURITY_GROUP_1 = {
     'id': ID_EC2_SECURITY_GROUP_1,

ec2api/tests/unit/test_instance.py

@@ -526,12 +526,10 @@ class InstanceTestCase(base.ApiTestCase):
         instance_api.instance_engine = (
             instance_api.InstanceEngineNeutron())
         self.set_mock_db_items(fakes.DB_IMAGE_2,
+                               fakes.DB_SUBNET_DEFAULT,
                                fakes.DB_NETWORK_INTERFACE_DEFAULT)
 
-        def mock_check_and_create(context):
+        check_and_create.return_value = fakes.DB_VPC_DEFAULT
-            self.add_mock_db_items(fakes.DB_VPC_DEFAULT,
-                                   fakes.DB_SUBNET_DEFAULT)
-        check_and_create.side_effect = mock_check_and_create
 
         self.glance.images.get.return_value = fakes.OSImage(fakes.OS_IMAGE_2)
         self.network_interface_api.create_network_interface.return_value = (
@@ -592,7 +590,9 @@ class InstanceTestCase(base.ApiTestCase):
                   'InstanceType': 'fake_flavor',
                   'MinCount': '1', 'MaxCount': '1'}
 
-        with mock.patch('ec2api.api.ec2utils.check_and_create_default_vpc'):
+        with mock.patch('ec2api.api.ec2utils.check_and_create_default_vpc'
+                        ) as check_and_create:
+            check_and_create.return_value = None
             self.assert_execution_error('VPCIdNotSpecified',
                                         'RunInstances', params)
 

ec2api/tests/unit/test_security_group.py

@@ -245,38 +245,45 @@ class SecurityGroupTestCase(base.ApiTestCase):
             security_group.SecurityGroupEngineNeutron())
         self.set_mock_db_items(fakes.DB_SECURITY_GROUP_1,
                                fakes.DB_SECURITY_GROUP_2,
-                               fakes.DB_SECURITY_GROUP_3)
+                               fakes.DB_SECURITY_GROUP_3,
+                               fakes.DB_SECURITY_GROUP_4,
+                               fakes.DB_SECURITY_GROUP_5,)
         self.neutron.list_security_groups.return_value = (
             {'security_groups': [copy.deepcopy(fakes.OS_SECURITY_GROUP_1),
                                  fakes.OS_SECURITY_GROUP_2,
-                                 fakes.OS_SECURITY_GROUP_3]})
+                                 fakes.OS_SECURITY_GROUP_3,
+                                 fakes.OS_SECURITY_GROUP_4,
+                                 fakes.OS_SECURITY_GROUP_5]})
 
         resp = self.execute('DescribeSecurityGroups', {})
         self.assertThat(resp['securityGroupInfo'],
                         matchers.ListMatches(
                             [fakes.EC2_SECURITY_GROUP_1,
                              fakes.EC2_SECURITY_GROUP_2,
-                             fakes.EC2_SECURITY_GROUP_3],
+                             fakes.EC2_SECURITY_GROUP_3,
+                             fakes.EC2_SECURITY_GROUP_4,
+                             fakes.EC2_SECURITY_GROUP_5],
                             orderless_lists=True))
 
         resp = self.execute('DescribeSecurityGroups',
                             {'GroupName.1': 'groupname2'})
         self.assertThat(resp['securityGroupInfo'],
                         matchers.ListMatches(
-                            [fakes.EC2_SECURITY_GROUP_2],
+                            [fakes.EC2_SECURITY_GROUP_4],
                             orderless_lists=True))
         self.assertEqual(0, self.db_api.delete_item.call_count)
 
         self.db_api.get_items_by_ids = tools.CopyingMock(
-            return_value=[fakes.DB_SECURITY_GROUP_2])
+            return_value=[fakes.DB_SECURITY_GROUP_4])
+
         resp = self.execute('DescribeSecurityGroups',
-                            {'GroupId.1': fakes.ID_EC2_SECURITY_GROUP_2})
+                            {'GroupId.1': fakes.ID_EC2_SECURITY_GROUP_4})
         self.assertThat(resp['securityGroupInfo'],
                         matchers.ListMatches(
-                            [fakes.EC2_SECURITY_GROUP_2],
+                            [fakes.EC2_SECURITY_GROUP_4],
                             orderless_lists=True))
         self.db_api.get_items_by_ids.assert_called_once_with(
-            mock.ANY, set([fakes.ID_EC2_SECURITY_GROUP_2]))
+            mock.ANY, set([fakes.ID_EC2_SECURITY_GROUP_4]))
         self.assertEqual(0, self.db_api.delete_item.call_count)
 
         self.check_filtering(
@@ -296,7 +303,17 @@ class SecurityGroupTestCase(base.ApiTestCase):
              ('owner-id', fakes.ID_OS_PROJECT)])
         self.check_tag_support(
             'DescribeSecurityGroups', 'securityGroupInfo',
-            fakes.ID_EC2_SECURITY_GROUP_2, 'groupId')
+            fakes.ID_EC2_SECURITY_GROUP_4, 'groupId')
+
+        self.configure(disable_ec2_classic=True)
+        self.add_mock_db_items(fakes.DB_VPC_DEFAULT,
+                               fakes.DB_SECURITY_GROUP_6)
+        resp = self.execute('DescribeSecurityGroups',
+                            {'GroupName.1': 'groupname2'})
+        self.assertThat(resp['securityGroupInfo'],
+                        matchers.ListMatches(
+                            [fakes.EC2_SECURITY_GROUP_5],
+                            orderless_lists=True))
 
     def test_describe_security_groups_nova(self):
         security_group.security_group_engine = (