From 7215b0046698aee0ed4db92d309742a46e3e809b Mon Sep 17 00:00:00 2001 From: acsbendi Date: Fri, 5 Jul 2019 13:43:47 +0200 Subject: [PATCH 1/6] Created test case for describe_instance_attribute. --- tests/test_ec2/test_instances.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_ec2/test_instances.py b/tests/test_ec2/test_instances.py index f14f85721..43bb1d561 100644 --- a/tests/test_ec2/test_instances.py +++ b/tests/test_ec2/test_instances.py @@ -1,5 +1,7 @@ from __future__ import unicode_literals # Ensure 'assert_raises' context manager support for Python 2.6 +from botocore.exceptions import ClientError + import tests.backport_assert_raises from nose.tools import assert_raises @@ -1255,6 +1257,7 @@ def test_create_instance_ebs_optimized(): instance.load() instance.ebs_optimized.should.be(False) + @mock_ec2 def test_run_multiple_instances_in_same_command(): instance_count = 4 @@ -1269,3 +1272,27 @@ def test_run_multiple_instances_in_same_command(): instances = reservations[0]['Instances'] for i in range(0, instance_count): instances[i]['AmiLaunchIndex'].should.be(i) + + +@mock_ec2 +def test_describe_instance_attribute(): + client = boto3.client('ec2', region_name='us-east-1') + client.run_instances(ImageId='ami-1234abcd', + MinCount=1, + MaxCount=1) + instance_id = client.describe_instances()['Reservations'][0]['Instances'][0]['InstanceId'] + + valid_instance_attributes = ['instanceType', 'kernel', 'ramdisk', 'userData', 'disableApiTermination', 'instanceInitiatedShutdownBehavior', 'rootDeviceName', 'blockDeviceMapping', 'productCodes', 'sourceDestCheck', 'groupSet', 'ebsOptimized', 'sriovNetSupport'] + + for valid_instance_attribute in valid_instance_attributes: + client.describe_instance_attribute(InstanceId=instance_id, Attribute=valid_instance_attribute) + + invalid_instance_attributes = ['abc', 'Kernel', 'RamDisk', 'userdata', 'iNsTaNcEtYpE'] + + for invalid_instance_attribute in invalid_instance_attributes: + with assert_raises(ClientError) as ex: + client.describe_instance_attribute(InstanceId=instance_id, Attribute=invalid_instance_attribute) + ex.exception.response['Error']['Code'].should.equal('InvalidParameterValue') + ex.exception.response['ResponseMetadata']['HTTPStatusCode'].should.equal(400) + message = 'Value ({invalid_instance_attribute}) for parameter attribute is invalid. Unknown attribute.'.format(invalid_instance_attribute=invalid_instance_attribute) + ex.exception.response['Error']['Message'].should.equal(message) From 9623e8a10c70967ad3e019138746af85ce603a36 Mon Sep 17 00:00:00 2001 From: acsbendi Date: Fri, 5 Jul 2019 14:09:58 +0200 Subject: [PATCH 2/6] Implemented raising error if the attribute is invalid. --- moto/ec2/exceptions.py | 9 +++++++++ moto/ec2/models.py | 14 ++++++++++++-- moto/ec2/responses/instances.py | 5 ++--- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/moto/ec2/exceptions.py b/moto/ec2/exceptions.py index 259e84bc3..5d5ccd844 100644 --- a/moto/ec2/exceptions.py +++ b/moto/ec2/exceptions.py @@ -332,6 +332,15 @@ class InvalidParameterValueErrorTagNull(EC2ClientError): "Tag value cannot be null. Use empty string instead.") +class InvalidParameterValueErrorUnknownAttribute(EC2ClientError): + + def __init__(self, parameter_value): + super(InvalidParameterValueErrorUnknownAttribute, self).__init__( + "InvalidParameterValue", + "Value ({0}) for parameter attribute is invalid. Unknown attribute." + .format(parameter_value)) + + class InvalidInternetGatewayIdError(EC2ClientError): def __init__(self, internet_gateway_id): diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 811283fe8..e2a834289 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -54,6 +54,7 @@ from .exceptions import ( InvalidNetworkInterfaceIdError, InvalidParameterValueError, InvalidParameterValueErrorTagNull, + InvalidParameterValueErrorUnknownAttribute, InvalidPermissionNotFoundError, InvalidPermissionDuplicateError, InvalidRouteTableIdError, @@ -383,6 +384,10 @@ class NetworkInterfaceBackend(object): class Instance(TaggedEC2Resource, BotoInstance): + VALID_ATTRIBUTES = {'instanceType', 'kernel', 'ramdisk', 'userData', 'disableApiTermination', + 'instanceInitiatedShutdownBehavior', 'rootDeviceName', 'blockDeviceMapping', + 'productCodes', 'sourceDestCheck', 'groupSet', 'ebsOptimized', 'sriovNetSupport'} + def __init__(self, ec2_backend, image_id, user_data, security_groups, **kwargs): super(Instance, self).__init__() self.ec2_backend = ec2_backend @@ -793,9 +798,14 @@ class InstanceBackend(object): setattr(instance, 'security_groups', new_group_list) return instance - def describe_instance_attribute(self, instance_id, key): - if key == 'group_set': + def describe_instance_attribute(self, instance_id, attribute): + if attribute not in Instance.VALID_ATTRIBUTES: + raise InvalidParameterValueErrorUnknownAttribute(attribute) + + if attribute == 'groupSet': key = 'security_groups' + else: + key = camelcase_to_underscores(attribute) instance = self.get_instance(instance_id) value = getattr(instance, key) return instance, value diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py index a5359daca..f82d0cc52 100644 --- a/moto/ec2/responses/instances.py +++ b/moto/ec2/responses/instances.py @@ -113,12 +113,11 @@ class InstanceResponse(BaseResponse): # TODO this and modify below should raise IncorrectInstanceState if # instance not in stopped state attribute = self._get_param('Attribute') - key = camelcase_to_underscores(attribute) instance_id = self._get_param('InstanceId') instance, value = self.ec2_backend.describe_instance_attribute( - instance_id, key) + instance_id, attribute) - if key == "group_set": + if attribute == "groupSet": template = self.response_template( EC2_DESCRIBE_INSTANCE_GROUPSET_ATTRIBUTE) else: From 9e6152588a10c4df076c583604122b618bee6826 Mon Sep 17 00:00:00 2001 From: acsbendi Date: Fri, 5 Jul 2019 14:31:46 +0200 Subject: [PATCH 3/6] Fixed attributes missing from Instance. --- moto/ec2/models.py | 2 ++ moto/ec2/responses/instances.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index e2a834289..d09cff9f5 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -410,6 +410,8 @@ class Instance(TaggedEC2Resource, BotoInstance): self.launch_time = utc_date_and_time() self.ami_launch_index = kwargs.get("ami_launch_index", 0) self.disable_api_termination = kwargs.get("disable_api_termination", False) + self.instance_initiated_shutdown_behavior = kwargs.get("instance_initiated_shutdown_behavior", "stop") + self.sriov_net_support = "simple" self._spot_fleet_id = kwargs.get("spot_fleet_id", None) associate_public_ip = kwargs.get("associate_public_ip", False) if in_ec2_classic: diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py index f82d0cc52..96fb554a0 100644 --- a/moto/ec2/responses/instances.py +++ b/moto/ec2/responses/instances.py @@ -46,6 +46,7 @@ class InstanceResponse(BaseResponse): associate_public_ip = self._get_param('AssociatePublicIpAddress') key_name = self._get_param('KeyName') ebs_optimized = self._get_param('EbsOptimized') + instance_initiated_shutdown_behavior = self._get_param("InstanceInitiatedShutdownBehavior") tags = self._parse_tag_specification("TagSpecification") region_name = self.region @@ -55,7 +56,7 @@ class InstanceResponse(BaseResponse): instance_type=instance_type, placement=placement, region_name=region_name, subnet_id=subnet_id, owner_id=owner_id, key_name=key_name, security_group_ids=security_group_ids, nics=nics, private_ip=private_ip, associate_public_ip=associate_public_ip, - tags=tags, ebs_optimized=ebs_optimized) + tags=tags, ebs_optimized=ebs_optimized, instance_initiated_shutdown_behavior=instance_initiated_shutdown_behavior) template = self.response_template(EC2_RUN_INSTANCES) return template.render(reservation=new_reservation) From 0b88dd1efb0795c52c846ee72ff5d578669532a1 Mon Sep 17 00:00:00 2001 From: acsbendi Date: Fri, 5 Jul 2019 15:12:38 +0200 Subject: [PATCH 4/6] Fixed security group IDs not returned correctly. --- moto/ec2/models.py | 5 ++++- moto/ec2/responses/instances.py | 4 ++-- tests/test_ec2/test_instances.py | 14 ++++++++++---- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index d09cff9f5..47f201888 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -795,8 +795,11 @@ class InstanceBackend(object): setattr(instance, key, value) return instance - def modify_instance_security_groups(self, instance_id, new_group_list): + def modify_instance_security_groups(self, instance_id, new_group_id_list): instance = self.get_instance(instance_id) + new_group_list = [] + for new_group_id in new_group_id_list: + new_group_list.append(self.get_security_group_from_id(new_group_id)) setattr(instance, 'security_groups', new_group_list) return instance diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py index 96fb554a0..996324ef6 100644 --- a/moto/ec2/responses/instances.py +++ b/moto/ec2/responses/instances.py @@ -605,9 +605,9 @@ EC2_DESCRIBE_INSTANCE_GROUPSET_ATTRIBUTE = """59dbff89-35bd-4eac-99ed-be587EXAMPLE {{ instance.id }} <{{ attribute }}> - {% for sg_id in value %} + {% for sg in value %} - {{ sg_id }} + {{ sg.id }} {% endfor %} diff --git a/tests/test_ec2/test_instances.py b/tests/test_ec2/test_instances.py index 43bb1d561..85dad28ae 100644 --- a/tests/test_ec2/test_instances.py +++ b/tests/test_ec2/test_instances.py @@ -681,8 +681,8 @@ def test_modify_instance_attribute_security_groups(): reservation = conn.run_instances('ami-1234abcd') instance = reservation.instances[0] - sg_id = 'sg-1234abcd' - sg_id2 = 'sg-abcd4321' + sg_id = conn.create_security_group('test security group', 'this is a test security group').id + sg_id2 = conn.create_security_group('test security group 2', 'this is a test security group 2').id with assert_raises(EC2ResponseError) as ex: instance.modify_attribute("groupSet", [sg_id, sg_id2], dry_run=True) @@ -1277,15 +1277,21 @@ def test_run_multiple_instances_in_same_command(): @mock_ec2 def test_describe_instance_attribute(): client = boto3.client('ec2', region_name='us-east-1') + security_group_id = client.create_security_group( + GroupName='test security group', Description='this is a test security group')['GroupId'] client.run_instances(ImageId='ami-1234abcd', MinCount=1, - MaxCount=1) + MaxCount=1, + SecurityGroupIds=[security_group_id]) instance_id = client.describe_instances()['Reservations'][0]['Instances'][0]['InstanceId'] valid_instance_attributes = ['instanceType', 'kernel', 'ramdisk', 'userData', 'disableApiTermination', 'instanceInitiatedShutdownBehavior', 'rootDeviceName', 'blockDeviceMapping', 'productCodes', 'sourceDestCheck', 'groupSet', 'ebsOptimized', 'sriovNetSupport'] for valid_instance_attribute in valid_instance_attributes: - client.describe_instance_attribute(InstanceId=instance_id, Attribute=valid_instance_attribute) + response = client.describe_instance_attribute(InstanceId=instance_id, Attribute=valid_instance_attribute) + if valid_instance_attribute == "groupSet": + response["Groups"].should.have.length_of(1) + response["Groups"][0]["GroupId"].should.equal(security_group_id) invalid_instance_attributes = ['abc', 'Kernel', 'RamDisk', 'userdata', 'iNsTaNcEtYpE'] From 7de0ef0f8b24f05580268324650b2d3cac600860 Mon Sep 17 00:00:00 2001 From: acsbendi Date: Fri, 5 Jul 2019 15:24:16 +0200 Subject: [PATCH 5/6] Fixed value is present in response even if it's None. --- moto/ec2/responses/instances.py | 2 ++ tests/test_ec2/test_instances.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py index 996324ef6..3bc8a44ac 100644 --- a/moto/ec2/responses/instances.py +++ b/moto/ec2/responses/instances.py @@ -597,7 +597,9 @@ EC2_DESCRIBE_INSTANCE_ATTRIBUTE = """ Date: Fri, 5 Jul 2019 15:32:22 +0200 Subject: [PATCH 6/6] Fixed a linting error. --- moto/ec2/responses/instances.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py index 3bc8a44ac..3f73d2e94 100644 --- a/moto/ec2/responses/instances.py +++ b/moto/ec2/responses/instances.py @@ -597,7 +597,7 @@ EC2_DESCRIBE_INSTANCE_ATTRIBUTE = """