Merge pull request #2281 from acsbendi/describe-instance-attribute-fix

Describe instance attribute fix
2019-07-06 15:39:32 -05:00 · 2019-07-06 15:39:32 -05:00 · fab6eb436e
commit fab6eb436e
parent f6dd3ab959 b0aab6fb15
4 changed files with 74 additions and 11 deletions
--- 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):
--- 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
@ -405,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:
@ -788,14 +795,22 @@ 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

-    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
--- 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)
@ -113,12 +114,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:
@ -597,7 +597,9 @@ EC2_DESCRIBE_INSTANCE_ATTRIBUTE = """<DescribeInstanceAttributeResponse xmlns="h
  <requestId>59dbff89-35bd-4eac-99ed-be587EXAMPLE</requestId>
  <instanceId>{{ instance.id }}</instanceId>
  <{{ attribute }}>
+    {% if value is not none %}
    <value>{{ value }}</value>
+    {% endif %}
  </{{ attribute }}>
 </DescribeInstanceAttributeResponse>"""

@ -605,9 +607,9 @@ EC2_DESCRIBE_INSTANCE_GROUPSET_ATTRIBUTE = """<DescribeInstanceAttributeResponse
  <requestId>59dbff89-35bd-4eac-99ed-be587EXAMPLE</requestId>
  <instanceId>{{ instance.id }}</instanceId>
  <{{ attribute }}>
-    {% for sg_id in value %}
+    {% for sg in value %}
      <item>
-        <groupId>{{ sg_id }}</groupId>
+        <groupId>{{ sg.id }}</groupId>
      </item>
    {% endfor %}
  </{{ attribute }}>
--- 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

@ -679,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)
@ -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,37 @@ 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')
+    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,
+                         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:
+        response = client.describe_instance_attribute(InstanceId=instance_id, Attribute=valid_instance_attribute)
+        if valid_instance_attribute == "groupSet":
+            response.should.have.key("Groups")
+            response["Groups"].should.have.length_of(1)
+            response["Groups"][0]["GroupId"].should.equal(security_group_id)
+        elif valid_instance_attribute == "userData":
+            response.should.have.key("UserData")
+            response["UserData"].should.be.empty
+
+    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)