Reworked validation mechanism once more

Change-Id: I2d833fc4c3a1d4178075354eddccfe9f195d3e3a

ec2api/api/__init__.py

@@ -332,92 +332,6 @@ class Requestify(wsgi.Middleware):
 class Validator(wsgi.Middleware):
 
     validator.DEFAULT_VALIDATOR = {
-        'AllocationId': validator.validate_ec2_id(['eipalloc']),
-        'AllowReassignment': validator.validate_dummy,
-        'AllowReassociation': validator.validate_dummy,
-        'Architecture': validator.validate_dummy,
-        'AssociationId': validator.validate_ec2_association_id,
-        'AttachmentId': validator.validate_ec2_id(['eni-attach']),
-        'Attribute': validator.validate_dummy,
-        'AvailabilityZone': validator.validate_dummy,
-        'BlockDeviceMapping': validator.validate_dummy,
-        'CidrBlock': validator.validate_cidr_block,
-        'ClientToken': validator.validate_dummy,
-        'Description': validator.validate_dummy,
-        'DestinationCidrBlock': validator.validate_cidr_block,
-        'Device': validator.validate_dummy,
-        'DeviceIndex': validator.validate_dummy,
-        'DhcpConfiguration': validator.validate_dummy,
-        'Dhcp_optionsId': validator.validate_dummy,
-        'DisableApiTermination': validator.validate_dummy,
-        'Domain': validator.validate_dummy,
-        'Ebs_optimized': validator.validate_dummy,
-        'Encrypted': validator.validate_dummy,
-        'ExecutableBy': validator.validate_dummy,
-        'Filter': validator.validate_dummy,
-        'Force': validator.validate_dummy,
-        'GatewayId': validator.validate_dummy,
-        'GroupDescription': validator.validate_str(max_length=255),
-        'GroupId': validator.validate_ec2_id(['sg']),
-        'GroupName': validator.validate_str(max_length=255),
-        'IamInstanceProfile': validator.validate_dummy,
-        'ImageId': validator.validate_ec2_id(['ami', 'ari', 'aki']),
-        'ImageLocation': validator.validate_image_path,
-        'InstanceId': validator.validate_dummy,
-        'InstanceInitiatedShutdownBehavior': validator.validate_dummy,
-        'InstanceTenancy': validator.validate_dummy,
-        'InstanceType': validator.validate_dummy,
-        'InternetGatewayId': validator.validate_dummy,
-        'Iops': validator.validate_dummy,
-        'IpPermissions': validator.validate_dummy,
-        'KernelId': validator.validate_dummy,
-        'KeyName': validator.validate_dummy,
-        'KmsKeyId': validator.validate_dummy,
-        'LaunchPermission': validator.validate_dummy,
-        'MaxCount': validator.validate_dummy,
-        'MaxResults': validator.validate_dummy,
-        'Metadata': validator.validate_dummy,
-        'MinCount': validator.validate_dummy,
-        'Monitoring': validator.validate_dummy,
-        'Name': validator.validate_dummy,
-        'NetworkInterface': validator.validate_dummy,
-        'NetworkInterfaceId': validator.validate_dummy,
-        'NextToken': validator.validate_dummy,
-        'NoReboot': validator.validate_dummy,
-        'OperationType': validator.validate_dummy,
-        'Owner': validator.validate_dummy,
-        'Placement': validator.validate_dummy,
-        'PrivateIpAddress': validator.validate_dummy,
-        'PrivateIpAddresses': validator.validate_dummy,
-        'ProductCode': validator.validate_dummy,
-        'PublicIp': validator.validate_ipv4,
-        'PublicKey_material': validator.validate_dummy,
-        'RamdiskId': validator.validate_dummy,
-        'RemoteIpPrefix': validator.validate_dummy,
-        'RegionName': validator.validate_str(),
-        'ResourceId': validator.validate_dummy,
-        'RestorableBy': validator.validate_dummy,
-        'RootDeviceName': validator.validate_dummy,
-        'RouteTableId': validator.validate_dummy,
-        'SecondaryPrivateIpAddressCount': validator.validate_dummy,
-        'SecurityGroup': validator.validate_dummy,
-        'SecurityGroupId': validator.validate_dummy,
-        'Size': validator.validate_int(),
-        'SnapshotId': validator.validate_dummy,
-        'SourceDestCheck': validator.validate_dummy,
-        'SriovNetSupport': validator.validate_dummy,
-        'SubnetId': validator.validate_dummy,
-        'Tag': validator.validate_dummy,
-        'UserData': validator.validate_user_data,
-        'UserGroup': validator.validate_dummy,
-        'UserId': validator.validate_dummy,
-        'Value': validator.validate_dummy,
-        'VirtualizationType': validator.validate_dummy,
-        'VolumeId': validator.validate_dummy,
-        'VolumeType': validator.validate_dummy,
-        'VpcId': validator.validate_dummy,
-        'VpcPeeringConnectionId': validator.validate_dummy,
-        'ZoneName': validator.validate_dummy,
     }
 
     def __init__(self, application):

ec2api/api/cloud.py

@@ -19,6 +19,8 @@ dispatched to other nodes via AMQP RPC. State is via distributed
 datastore.
 """
 
+import itertools
+
 from oslo.config import cfg
 
 from ec2api.api import address
@@ -42,6 +44,29 @@ CONF = cfg.CONF
 LOG = logging.getLogger(__name__)
 
 
+def module_and_param_types(module, *args, **kwargs):
+    """Decorator to check types and call function."""
+
+    param_types = args
+
+    def wrapped(func):
+
+        def func_wrapped(*args, **kwargs):
+            impl_func = getattr(module, func.func_name)
+            context = args[1]
+            for param_name, param_type in itertools.izip(
+                    func.func_code.co_varnames[2:], param_types):
+                param_value = kwargs.get(param_name)
+                if param_value:
+                    validator = module.Validator(param_name, func.func_name)
+                    validation_func = getattr(validator, param_type)
+                    is_valid = validation_func(param_value)
+            return impl_func(context, **kwargs)
+        return func_wrapped
+
+    return wrapped
+
+
 class CloudController(object):
 
     """Cloud Controller
@@ -305,6 +330,16 @@ class CloudController(object):
                                                 context, group_id,
                                                 group_name, ip_permissions)
 
+    @module_and_param_types(instance, 'ami_id', 'dummy', 'dummy',
+                            'str255', 'sg_ids',
+                            'str255s', 'dummy', 'dummy',
+                            'dummy', 'ami_id', 'ami_id',
+                            'dummy', 'dummy',
+                            'subnet_id', 'dummy',
+                            'dummy',
+                            'dummy', 'dummy',
+                            'dummy', 'dummy',
+                            'dummy')
     def run_instances(self, context, image_id, min_count, max_count,
                       key_name=None, security_group_id=None,
                       security_group=None, user_data=None, instance_type=None,
@@ -418,17 +453,8 @@ class CloudController(object):
         If you don't specify a security group when launching an instance, EC2
         uses the default security group.
         """
-        return instance.run_instances(context, image_id, min_count, max_count,
-                                      key_name, security_group_id,
-                                      security_group, user_data, instance_type,
-                                      placement, kernel_id, ramdisk_id,
-                                      block_device_mapping, monitoring,
-                                      subnet_id, disable_api_termination,
-                                      instance_initiated_shutdown_behavior,
-                                      private_ip_address, client_token,
-                                      network_interface, iam_instance_profile,
-                                      ebs_optimized)
 
+    @module_and_param_types(instance, 'i_ids')
     def terminate_instances(self, context, instance_id):
         """Shuts down one or more instances.
 
@@ -442,8 +468,8 @@ class CloudController(object):
         This operation is idempotent; if you terminate an instance more than
         once, each call succeeds.
         """
-        return instance.terminate_instances(context, instance_id)
 
+    @module_and_param_types(instance, 'i_ids', 'dummy', 'dummy', 'dummy')
     def describe_instances(self, context, instance_id=None, filter=None,
                            max_results=None, next_token=None):
         """Describes one or more of your instances.
@@ -467,9 +493,8 @@ class CloudController(object):
         instance ID, you receive an error. If you specify an instance that you
         don't own, we don't include it in the results.
         """
-        return instance.describe_instances(context, instance_id, filter,
-                                           max_results, next_token)
 
+    @module_and_param_types(instance, 'i_ids')
     def reboot_instances(self, context, instance_id):
         """Requests a reboot of one or more instances.
 
@@ -480,8 +505,8 @@ class CloudController(object):
         Returns:
             true if the request succeeds.
         """
-        return instance.reboot_instances(context, instance_id)
 
+    @module_and_param_types(instance, 'i_ids', 'dummy')
     def stop_instances(self, context, instance_id, force=False):
         """Stops one or more instances.
 
@@ -496,8 +521,8 @@ class CloudController(object):
         Returns:
             true if the request succeeds.
         """
-        return instance.stop_instances(context, instance_id, force)
 
+    @module_and_param_types(instance, 'i_ids')
     def start_instances(self, context, instance_id):
         """Starts one or more instances.
 
@@ -508,8 +533,8 @@ class CloudController(object):
         Returns:
             true if the request succeeds.
         """
-        return instance.start_instances(context, instance_id)
 
+    @module_and_param_types(instance, 'i_id', 'dummy')
     def describe_instance_attribute(self, context, instance_id, attribute):
         """Describes the specified attribute of the specified instance.
 
@@ -527,8 +552,6 @@ class CloudController(object):
         Returns:
             Specified attribute.
         """
-        return instance.describe_instance_attribute(context, instance_id,
-                                                    attribute)
 
     def describe_key_pairs(self, context, key_name=None, filter=None):
         return key_pair.describe_key_pairs(context, key_name, filter)
@@ -950,6 +973,7 @@ class VpcCloudController(CloudController):
         Adds full VPC functionality which requires Neutron to work.
     """
 
+    @module_and_param_types(vpc, 'vpc_cidr', 'str255')
     def create_vpc(self, context, cidr_block, instance_tenancy='default'):
         """Creates a VPC with the specified CIDR block.
 
@@ -968,7 +992,6 @@ class VpcCloudController(CloudController):
         The smallest VPC you can create uses a /28 netmask (16 IP addresses),
         and the largest uses a /16 netmask.
         """
-        return vpc.create_vpc(context, cidr_block, instance_tenancy)
 
     def delete_vpc(self, context, vpc_id):
         """Deletes the specified VPC.
@@ -1087,6 +1110,7 @@ class VpcCloudController(CloudController):
                                                           internet_gateway_id,
                                                           filter)
 
+    @module_and_param_types(subnet, 'vpc_id', 'subnet_cidr', 'str255')
     def create_subnet(self, context, vpc_id, cidr_block,
                       availability_zone=None):
         """Creates a subnet in an existing VPC.
@@ -1114,8 +1138,6 @@ class VpcCloudController(CloudController):
         If you add more than one subnet to a VPC, they're set up
         in a star topology with a logical router in the middle.
         """
-        return subnet.create_subnet(context, vpc_id,
-                                    cidr_block, availability_zone)
 
     def delete_subnet(self, context, subnet_id):
         """Deletes the specified subnet.
@@ -1163,6 +1185,8 @@ class VpcCloudController(CloudController):
         """
         return route_table.create_route_table(context, vpc_id)
 
+    @module_and_param_types(route_table, 'rtb_id', 'cidr',
+                            'igw_id', 'i_id', 'eni_id', 'dummy')
     def create_route(self, context, route_table_id, destination_cidr_block,
                      gateway_id=None, instance_id=None,
                      network_interface_id=None,
@@ -1191,10 +1215,6 @@ class VpcCloudController(CloudController):
         gateway attached to the VPC, a VPC peering connection, or a NAT
         instance in the VPC.
         """
-        return route_table.create_route(context, route_table_id,
-                                        destination_cidr_block, gateway_id,
-                                        instance_id, network_interface_id,
-                                        vpc_peering_connection_id)
 
     def replace_route(self, context, route_table_id, destination_cidr_block,
                       gateway_id=None, instance_id=None,

ec2api/api/common.py

@@ -18,6 +18,7 @@ import fnmatch
 from oslo.config import cfg
 
 from ec2api.api import ec2utils
+from ec2api.api import validator
 from ec2api.db import api as db_api
 from ec2api import exception
 
@@ -31,6 +32,88 @@ ec2_opts = [
 CONF = cfg.CONF
 CONF.register_opts(ec2_opts)
 
+
+class Validator(object):
+
+    def __init__(self, param_name="", action=""):
+        self.param_name = param_name
+        self.action = action
+
+    def dummy(self, value):
+        pass
+
+    def str255(self, value):
+        validator.validate_str(value, self.param_name, 255)
+
+    def multi(self, items, validation_func):
+        validator.validate_list(items, self.param_name)
+        for item in items:
+            validation_func(item)
+
+    def str255s(self, values):
+        self.multi(values, self.str_255)
+
+    def cidr(self, cidr):
+        validator.validate_cidr(cidr, self.param_name)
+
+    def subnet_cidr(self, cidr):
+        validator.validate_subnet_cidr(cidr)
+
+    def vpc_cidr(self, cidr):
+        validator.validate_vpc_cidr(cidr)
+
+    def ec2_id(self, id, prefices):
+        validator.validate_ec2_id(id, self.param_name, prefices)
+
+    def i_id(self, id):
+        self.ec2_id(id, ['i'])
+
+    def i_ids(self, ids):
+        self.multi(ids, self.i_id)
+
+    def ami_id(self, id):
+        self.ec2_id(id, ['ami', 'ari', 'aki'])
+
+    def ami_ids(self, ids):
+        self.multi(ids, self.aki_id)
+
+    def sg_id(self, id):
+        self.ec2_id(id, ['sg'])
+
+    def sg_ids(self, ids):
+        self.multi(ids, self.sg_id)
+
+    def subnet_id(self, id):
+        self.ec2_id(id, ['subnet'])
+
+    def subnet_ids(self, ids):
+        self.multi(ids, self.subnet_id)
+
+    def igw_id(self, id):
+        self.ec2_id(id, ['igw'])
+
+    def igw_ids(self, ids):
+        self.multi(ids, self.igw_id)
+
+    def rtb_id(self, id):
+        self.ec2_id(id, ['rtb'])
+
+    def rtb_ids(self, ids):
+        self.multi(ids, self.rtb_id)
+
+    def eni_id(self, id):
+        self.ec2_id(id, ['eni'])
+
+    def eni_ids(self, ids):
+        self.multi(ids, self.eni_id)
+
+    def vpc_id(self, id):
+        self.ec2_id(id, ['vpc'])
+
+    def vpc_ids(self, ids):
+        self.multi(ids, self.vpc_id)
+
+
 VPC_KINDS = ['vpc', 'igw', 'subnet', 'eni', 'dopt', 'eipalloc', 'sg', 'rtb']
 
 

ec2api/api/instance.py

@@ -51,6 +51,10 @@ CONF.register_opts(ec2_opts)
 """Instance related API implementation
 """
 
+
+Validator = common.Validator
+
+
 # TODO(ft): implement DeviceIndex
 
 

ec2api/api/route_table.py

@@ -27,6 +27,9 @@ from ec2api import exception
 from ec2api.openstack.common.gettextutils import _
 
 
+Validator = common.Validator
+
+
 def create_route_table(context, vpc_id):
     vpc = ec2utils.get_db_item(context, 'vpc', vpc_id)
     route_table = _create_route_table(context, vpc)

ec2api/api/subnet.py

@@ -37,6 +37,9 @@ LOG = logging.getLogger(__name__)
 """
 
 
+Validator = common.Validator
+
+
 def create_subnet(context, vpc_id, cidr_block,
                   availability_zone=None):
     vpc = ec2utils.get_db_item(context, 'vpc', vpc_id)

ec2api/api/validator.py

@@ -44,17 +44,13 @@ def validate_dummy(val, **kwargs):
     return True
 
 
-def validate_str(max_length=None):
+def validate_str(val, parameter_name, max_length=None):
-
+    if (isinstance(val, basestring) and
-    def _do(val, parameter_name, **kwargs):
+            (max_length is None or max_length and len(val) <= max_length)):
-        if (isinstance(val, basestring) and
+        return True
-                (max_length is None or max_length and len(val) <= max_length)):
+    raise exception.ValidationError(
-            return True
+        reason=_("%s should not be greater "
-        raise exception.ValidationError(
+                 "than 255 characters.") % parameter_name)
-            reason=_("%s should not be greater "
-                     "than 255 characters.") % parameter_name)
-
-    return _do
 
 
 def validate_int(max_value=None):
@@ -98,6 +94,14 @@ def validate_image_path(val, parameter_name=None, **kwargs):
     return True
 
 
+def validate_list(items, parameter_name):
+    if not isinstance(items, list):
+        raise exception.InvalidParameterValue(
+            value=items,
+            parameter=parameter_name,
+            reason='Expected a list here')
+
+
 def validate_user_data(user_data, **kwargs):
     """Check if the user_data is encoded properly."""
     try:
@@ -164,38 +168,37 @@ def validate_cidr(cidr, parameter_name, **kwargs):
     return True
 
 
-def validate_cidr_block(cidr, action, **kwargs):
+def _validate_cidr_block(cidr):
     validate_cidr(cidr, 'cidrBlock')
     size = int(cidr.split("/")[-1])
-    if size > 28 or size < 16:
+    return size >= 16 and size <= 28
-        if action == 'CreateVpc':
+
-            raise exception.InvalidVpcRange(cidr_block=cidr)
+
-        elif action == 'CreateSubnet':
+def validate_vpc_cidr(cidr):
-            raise exception.InvalidSubnetRange(cidr_block=cidr)
+    if not _validate_cidr_block(cidr):
-    return True
+        raise exception.InvalidVpcRange(cidr_block=cidr)
+
+
+def validate_subnet_cidr(cidr):
+    if not _validate_cidr_block(cidr):
+        raise exception.InvalidSubnetRange(cidr_block=cidr)
 
 
 # NOTE(Alex) Unfortunately Amazon returns various kinds of error for invalid
 # IDs (...ID.Malformed, ...Id.Malformed, ...ID.NotFound, InvalidParameterValue)
 # So we decided here to commonize invalid IDs to InvalidParameterValue error.
 
-def validate_ec2_id(prefices):
+def validate_ec2_id(val, parameter_name, prefices):
-
+    try:
-    def _do(val, parameter_name, **kwargs):
+        prefix, value = val.rsplit('-', 1)
-        if not validate_str()(val, parameter_name, **kwargs):
+        int(value, 16)
-            return False
+        if prefix in prefices:
-        try:
+            return True
-            prefix, value = val.rsplit('-', 1)
+    except Exception:
-            int(value, 16)
+        pass
-            if prefix in prefices:
+    raise exception.InvalidParameterValue(
-                return True
+        value=val, parameter=parameter_name,
-        except Exception:
+        reason=_('Expected: %(prefix)s-...') % {'prefix': prefices[0]})
-            pass
-        raise exception.InvalidParameterValue(
-            value=val, parameter=parameter_name,
-            reason=_('Expected: %(prefix)s-...') % {'prefix': prefices[0]})
-
-    return _do
 
 
 def validate_ec2_association_id(id, parameter_name, action):

ec2api/api/vpc.py

@@ -38,6 +38,9 @@ LOG = logging.getLogger(__name__)
 """
 
 
+Validator = common.Validator
+
+
 def create_vpc(context, cidr_block, instance_tenancy='default'):
     neutron = clients.neutron(context)
     # TODO(Alex): Handle errors like overlimit

ec2api/tests/test_db_api.py

@@ -87,7 +87,7 @@ class DbApiTestCase(test_base.BaseTestCase):
         self.assertIn('id', item)
         self.assertIsNotNone(item['id'])
         item_id = item.pop('id')
-        self.assertTrue(validator.validate_ec2_id(('fake',))(item_id, ''))
+        self.assertTrue(validator.validate_ec2_id(item_id, '', ['fake']))
         self.assertThat(item, matchers.DictMatches(new_item,
                                                    orderless_lists=True))
 
@@ -146,7 +146,7 @@ class DbApiTestCase(test_base.BaseTestCase):
     def test_add_item_id(self):
         os_id = fakes.random_os_id()
         item_id = db_api.add_item_id(self.context, 'fake', os_id)
-        self.assertTrue(validator.validate_ec2_id(('fake',))(item_id, ''))
+        self.assertTrue(validator.validate_ec2_id(item_id, '', ['fake']))
         item = db_api.get_item_by_id(self.context, 'fake', item_id)
         self.assertIsNone(item)
         item = db_api.add_item(self.context, 'fake', {'os_id': os_id})

ec2api/tests/test_ec2_validate.py

@@ -46,17 +46,11 @@ class EC2ValidationTestCase(testtools.TestCase):
         check_raise_invalid_parameter('10.10.0.0/33')
         check_raise_invalid_parameter('10.10.0.0/-1')
 
-        def check_raise_invalid_vpc_range(cidr, ex_class, action):
+        self.assertRaises(exception.InvalidSubnetRange,
-            self.assertRaises(ex_class,
+                          validator.validate_subnet_cidr, '10.10.0.0/15')
-                              validator.validate_cidr_block, cidr,
-                              action)
 
-        check_raise_invalid_vpc_range('10.10.0.0/15',
+        self.assertRaises(exception.InvalidVpcRange,
-                                      exception.InvalidSubnetRange,
+                          validator.validate_vpc_cidr, '10.10.0.0/29')
-                                      'CreateSubnet')
-        check_raise_invalid_vpc_range('10.10.0.0/29',
-                                      exception.InvalidVpcRange,
-                                      'CreateVpc')
 
 
 class EC2TimestampValidationTestCase(testtools.TestCase):

ec2api/tests/test_instance.py

@@ -508,6 +508,16 @@ class InstanceTestCase(base.ApiTestCase):
                                 fakes.EC2_RESERVATION_2]},
             orderless_lists=True))
 
+        self.db_api.get_items_by_ids.return_value = [fakes.DB_INSTANCE_2]
+        resp = self.execute('DescribeInstances', {'InstanceId.1':
+                                                  fakes.ID_EC2_INSTANCE_2})
+
+        self.assertEqual(200, resp['status'])
+        resp.pop('status')
+        self.assertThat(resp, matchers.DictMatches(
+            {'reservationSet': [fakes.EC2_RESERVATION_2]},
+            orderless_lists=True))
+
     # TODO(ft): restore test after finish extraction of Nova EC2 API
     def _test_describe_instances_mutliple_networks(self):
         """Describe 2 instances with various combinations of network."""

ec2api/tests/test_route_table.py

@@ -189,7 +189,7 @@ class RouteTableTestCase(base.ApiTestCase):
         do_check({'RouteTableId': fakes.ID_EC2_ROUTE_TABLE_2,
                   'DestinationCidrBlock': '0.0.0.0/0',
                   'NetworkInterfaceId': fakes.ID_EC2_NETWORK_INTERFACE_1,
-                  'GatewayId': fakes.ID_EC2_NETWORK_INTERFACE_1},
+                  'GatewayId': fakes.ID_EC2_IGW_1},
                  'InvalidParameterCombination')
 
         # NOTE(ft): gateway from different vpc