From 0b88dd1efb0795c52c846ee72ff5d578669532a1 Mon Sep 17 00:00:00 2001
From: acsbendi <acsbendi28@gmail.com>
Date: Fri, 5 Jul 2019 15:12:38 +0200
Subject: [PATCH] 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 = """<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 }}>
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']