From 6671fcdd57a240bda580f50673f828a28bf55867 Mon Sep 17 00:00:00 2001 From: alevine Date: Tue, 30 Dec 2014 14:02:31 +0400 Subject: [PATCH] Reworked validation mechanism once more Change-Id: I2d833fc4c3a1d4178075354eddccfe9f195d3e3a --- ec2api/api/__init__.py | 86 ------------------------------- ec2api/api/cloud.py | 70 ++++++++++++++++--------- ec2api/api/common.py | 83 +++++++++++++++++++++++++++++ ec2api/api/instance.py | 4 ++ ec2api/api/route_table.py | 3 ++ ec2api/api/subnet.py | 3 ++ ec2api/api/validator.py | 73 +++++++++++++------------- ec2api/api/vpc.py | 3 ++ ec2api/tests/test_db_api.py | 4 +- ec2api/tests/test_ec2_validate.py | 14 ++--- ec2api/tests/test_instance.py | 10 ++++ ec2api/tests/test_route_table.py | 2 +- 12 files changed, 196 insertions(+), 159 deletions(-) diff --git a/ec2api/api/__init__.py b/ec2api/api/__init__.py index 4c42dcce..80d30139 100644 --- a/ec2api/api/__init__.py +++ b/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): diff --git a/ec2api/api/cloud.py b/ec2api/api/cloud.py index f06749d2..2d6ed0a5 100644 --- a/ec2api/api/cloud.py +++ b/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, diff --git a/ec2api/api/common.py b/ec2api/api/common.py index e1f91ecc..41d66327 100644 --- a/ec2api/api/common.py +++ b/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'] diff --git a/ec2api/api/instance.py b/ec2api/api/instance.py index 5efd441a..587ee3f4 100644 --- a/ec2api/api/instance.py +++ b/ec2api/api/instance.py @@ -51,6 +51,10 @@ CONF.register_opts(ec2_opts) """Instance related API implementation """ + +Validator = common.Validator + + # TODO(ft): implement DeviceIndex diff --git a/ec2api/api/route_table.py b/ec2api/api/route_table.py index d48a9013..183be7a3 100644 --- a/ec2api/api/route_table.py +++ b/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) diff --git a/ec2api/api/subnet.py b/ec2api/api/subnet.py index 64f7886e..7226daaf 100644 --- a/ec2api/api/subnet.py +++ b/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) diff --git a/ec2api/api/validator.py b/ec2api/api/validator.py index 45f11602..22b3cc3d 100644 --- a/ec2api/api/validator.py +++ b/ec2api/api/validator.py @@ -44,17 +44,13 @@ def validate_dummy(val, **kwargs): return True -def validate_str(max_length=None): - - def _do(val, parameter_name, **kwargs): - if (isinstance(val, basestring) and - (max_length is None or max_length and len(val) <= max_length)): - return True - raise exception.ValidationError( - reason=_("%s should not be greater " - "than 255 characters.") % parameter_name) - - return _do +def validate_str(val, parameter_name, max_length=None): + if (isinstance(val, basestring) and + (max_length is None or max_length and len(val) <= max_length)): + return True + raise exception.ValidationError( + reason=_("%s should not be greater " + "than 255 characters.") % parameter_name) 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: - if action == 'CreateVpc': - raise exception.InvalidVpcRange(cidr_block=cidr) - elif action == 'CreateSubnet': - raise exception.InvalidSubnetRange(cidr_block=cidr) - return True + return size >= 16 and size <= 28 + + +def validate_vpc_cidr(cidr): + if not _validate_cidr_block(cidr): + 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 _do(val, parameter_name, **kwargs): - if not validate_str()(val, parameter_name, **kwargs): - return False - try: - prefix, value = val.rsplit('-', 1) - int(value, 16) - if prefix in prefices: - return True - except Exception: - pass - raise exception.InvalidParameterValue( - value=val, parameter=parameter_name, - reason=_('Expected: %(prefix)s-...') % {'prefix': prefices[0]}) - - return _do +def validate_ec2_id(val, parameter_name, prefices): + try: + prefix, value = val.rsplit('-', 1) + int(value, 16) + if prefix in prefices: + return True + except Exception: + pass + raise exception.InvalidParameterValue( + value=val, parameter=parameter_name, + reason=_('Expected: %(prefix)s-...') % {'prefix': prefices[0]}) def validate_ec2_association_id(id, parameter_name, action): diff --git a/ec2api/api/vpc.py b/ec2api/api/vpc.py index 7199731f..136ef15f 100644 --- a/ec2api/api/vpc.py +++ b/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 diff --git a/ec2api/tests/test_db_api.py b/ec2api/tests/test_db_api.py index 8f83ad84..27ececd2 100644 --- a/ec2api/tests/test_db_api.py +++ b/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}) diff --git a/ec2api/tests/test_ec2_validate.py b/ec2api/tests/test_ec2_validate.py index 6ff47f5c..784b3dce 100644 --- a/ec2api/tests/test_ec2_validate.py +++ b/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(ex_class, - validator.validate_cidr_block, cidr, - action) + self.assertRaises(exception.InvalidSubnetRange, + validator.validate_subnet_cidr, '10.10.0.0/15') - check_raise_invalid_vpc_range('10.10.0.0/15', - exception.InvalidSubnetRange, - 'CreateSubnet') - check_raise_invalid_vpc_range('10.10.0.0/29', - exception.InvalidVpcRange, - 'CreateVpc') + self.assertRaises(exception.InvalidVpcRange, + validator.validate_vpc_cidr, '10.10.0.0/29') class EC2TimestampValidationTestCase(testtools.TestCase): diff --git a/ec2api/tests/test_instance.py b/ec2api/tests/test_instance.py index 5cc95ced..3438f0ac 100644 --- a/ec2api/tests/test_instance.py +++ b/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.""" diff --git a/ec2api/tests/test_route_table.py b/ec2api/tests/test_route_table.py index c181a81a..ca8d1cc1 100644 --- a/ec2api/tests/test_route_table.py +++ b/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