From 5d51329c34abf4312de89f74faa6a4e9dc224e7e Mon Sep 17 00:00:00 2001
From: Graham Lyons <graham@grahamlyons.com>
Date: Wed, 21 Mar 2018 15:55:58 +0000
Subject: [PATCH] Don't create volumes for AMIs (#1456)

* Delete the volume used during AMI creation

Creating an AMI doesn't actually result in the creation of an EBS
volume, although the associated snapshot does reference one. To that
end, delete the volume once we've used it.

* Add `owner_id` to `Snapshot`, verify AMI snapshots

The default AMIs which are created by moto have EBS volume mappings
but the snapshots associated with those don't have the correct
owners set.

This adds the owner to the snapshot model and passes it through from
the JSON data.
---
 moto/ec2/models.py                        | 14 ++++--
 moto/ec2/responses/elastic_block_store.py |  4 +-
 tests/test_ec2/test_amis.py               | 56 +++++++++++++++++------
 3 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/moto/ec2/models.py b/moto/ec2/models.py
index b1989d67..51ef2a12 100755
--- a/moto/ec2/models.py
+++ b/moto/ec2/models.py
@@ -1088,7 +1088,8 @@ class Ami(TaggedEC2Resource):
         # AWS auto-creates these, we should reflect the same.
         volume = self.ec2_backend.create_volume(15, region_name)
         self.ebs_snapshot = self.ec2_backend.create_snapshot(
-            volume.id, "Auto-created snapshot for AMI %s" % self.id)
+            volume.id, "Auto-created snapshot for AMI %s" % self.id, owner_id)
+        self.ec2_backend.delete_volume(volume.id)
 
     @property
     def is_public(self):
@@ -1840,7 +1841,7 @@ class Volume(TaggedEC2Resource):
 
 
 class Snapshot(TaggedEC2Resource):
-    def __init__(self, ec2_backend, snapshot_id, volume, description, encrypted=False):
+    def __init__(self, ec2_backend, snapshot_id, volume, description, encrypted=False, owner_id='123456789012'):
         self.id = snapshot_id
         self.volume = volume
         self.description = description
@@ -1849,6 +1850,7 @@ class Snapshot(TaggedEC2Resource):
         self.ec2_backend = ec2_backend
         self.status = 'completed'
         self.encrypted = encrypted
+        self.owner_id = owner_id
 
     def get_filter_value(self, filter_name):
         if filter_name == 'description':
@@ -1940,11 +1942,13 @@ class EBSBackend(object):
         volume.attachment = None
         return old_attachment
 
-    def create_snapshot(self, volume_id, description):
+    def create_snapshot(self, volume_id, description, owner_id=None):
         snapshot_id = random_snapshot_id()
         volume = self.get_volume(volume_id)
-        snapshot = Snapshot(self, snapshot_id, volume,
-                            description, volume.encrypted)
+        params = [self, snapshot_id, volume, description, volume.encrypted]
+        if owner_id:
+            params.append(owner_id)
+        snapshot = Snapshot(*params)
         self.snapshots[snapshot_id] = snapshot
         return snapshot
 
diff --git a/moto/ec2/responses/elastic_block_store.py b/moto/ec2/responses/elastic_block_store.py
index 31831c18..2d43f8ff 100644
--- a/moto/ec2/responses/elastic_block_store.py
+++ b/moto/ec2/responses/elastic_block_store.py
@@ -229,7 +229,7 @@ CREATE_SNAPSHOT_RESPONSE = """<CreateSnapshotResponse xmlns="http://ec2.amazonaw
   <status>pending</status>
   <startTime>{{ snapshot.start_time}}</startTime>
   <progress>60%</progress>
-  <ownerId>123456789012</ownerId>
+  <ownerId>{{ snapshot.owner_id }}</ownerId>
   <volumeSize>{{ snapshot.volume.size }}</volumeSize>
   <description>{{ snapshot.description }}</description>
   <encrypted>{{ snapshot.encrypted }}</encrypted>
@@ -245,7 +245,7 @@ DESCRIBE_SNAPSHOTS_RESPONSE = """<DescribeSnapshotsResponse xmlns="http://ec2.am
              <status>{{ snapshot.status }}</status>
              <startTime>{{ snapshot.start_time}}</startTime>
              <progress>100%</progress>
-             <ownerId>123456789012</ownerId>
+             <ownerId>{{ snapshot.owner_id }}</ownerId>
             <volumeSize>{{ snapshot.volume.size }}</volumeSize>
              <description>{{ snapshot.description }}</description>
              <encrypted>{{ snapshot.encrypted }}</encrypted>
diff --git a/tests/test_ec2/test_amis.py b/tests/test_ec2/test_amis.py
index 3f21ca20..a8d4d1b6 100644
--- a/tests/test_ec2/test_amis.py
+++ b/tests/test_ec2/test_amis.py
@@ -10,6 +10,7 @@ from nose.tools import assert_raises
 import sure  # noqa
 
 from moto import mock_ec2_deprecated, mock_ec2
+from moto.ec2.models import AMIS
 from tests.helpers import requires_boto_gte
 
 
@@ -17,9 +18,9 @@ from tests.helpers import requires_boto_gte
 def test_ami_create_and_delete():
     conn = boto.connect_ec2('the_key', 'the_secret')
 
-    initial_volume_count = 34
-    conn.get_all_volumes().should.have.length_of(initial_volume_count)
-    conn.get_all_snapshots().should.have.length_of(initial_volume_count)
+    initial_ami_count = len(AMIS)
+    conn.get_all_volumes().should.have.length_of(0)
+    conn.get_all_snapshots().should.have.length_of(initial_ami_count)
 
     reservation = conn.run_instances('ami-1234abcd')
     instance = reservation.instances[0]
@@ -47,19 +48,19 @@ def test_ami_create_and_delete():
     retrieved_image.creationDate.should_not.be.none
     instance.terminate()
 
-    # Validate auto-created volume and snapshot
+    # Ensure we're no longer creating a volume
     volumes = conn.get_all_volumes()
-    volumes.should.have.length_of(initial_volume_count + 1)
+    volumes.should.have.length_of(0)
 
+    # Validate auto-created snapshot
     snapshots = conn.get_all_snapshots()
-    snapshots.should.have.length_of(initial_volume_count + 1)
+    snapshots.should.have.length_of(initial_ami_count + 1)
 
     retrieved_image_snapshot_id = retrieved_image.block_device_mapping.current_value.snapshot_id
     [s.id for s in snapshots].should.contain(retrieved_image_snapshot_id)
     snapshot = [s for s in snapshots if s.id == retrieved_image_snapshot_id][0]
     snapshot.description.should.equal(
         "Auto-created snapshot for AMI {0}".format(retrieved_image.id))
-    [v.id for v in volumes].should.contain(snapshot.volume_id)
 
     # root device should be in AMI's block device mappings
     root_mapping = retrieved_image.block_device_mapping.get(retrieved_image.root_device_name)
@@ -88,9 +89,9 @@ def test_ami_create_and_delete():
 def test_ami_copy():
     conn = boto.ec2.connect_to_region("us-west-1")
 
-    initial_volume_count = 34
-    conn.get_all_volumes().should.have.length_of(initial_volume_count)
-    conn.get_all_snapshots().should.have.length_of(initial_volume_count)
+    initial_ami_count = len(AMIS)
+    conn.get_all_volumes().should.have.length_of(0)
+    conn.get_all_snapshots().should.have.length_of(initial_ami_count)
 
     reservation = conn.run_instances('ami-1234abcd')
     instance = reservation.instances[0]
@@ -123,9 +124,11 @@ def test_ami_copy():
     copy_image.kernel_id.should.equal(source_image.kernel_id)
     copy_image.platform.should.equal(source_image.platform)
 
-    # Validate auto-created volume and snapshot
-    conn.get_all_volumes().should.have.length_of(initial_volume_count + 2)
-    conn.get_all_snapshots().should.have.length_of(initial_volume_count + 2)
+    # Ensure we're no longer creating a volume
+    conn.get_all_volumes().should.have.length_of(0)
+
+    # Validate auto-created snapshot
+    conn.get_all_snapshots().should.have.length_of(initial_ami_count + 2)
 
     copy_image.block_device_mapping.current_value.snapshot_id.should_not.equal(
         source_image.block_device_mapping.current_value.snapshot_id)
@@ -744,3 +747,30 @@ def test_ami_filter_by_self():
 
     my_images = ec2_client.describe_images(Owners=['self'])['Images']
     my_images.should.have.length_of(1)
+
+
+@mock_ec2
+def test_ami_snapshots_have_correct_owner():
+    ec2_client = boto3.client('ec2', region_name='us-west-1')
+
+    images_response = ec2_client.describe_images()
+
+    owner_id_to_snapshot_ids = {}
+    for image in images_response['Images']:
+        owner_id = image['OwnerId']
+        snapshot_ids = [
+            block_device_mapping['Ebs']['SnapshotId']
+            for block_device_mapping in image['BlockDeviceMappings']
+        ]
+        existing_snapshot_ids = owner_id_to_snapshot_ids.get(owner_id, [])
+        owner_id_to_snapshot_ids[owner_id] = (
+            existing_snapshot_ids + snapshot_ids
+        )
+
+    for owner_id in owner_id_to_snapshot_ids:
+        snapshots_rseponse = ec2_client.describe_snapshots(
+            SnapshotIds=owner_id_to_snapshot_ids[owner_id]
+        )
+
+        for snapshot in snapshots_rseponse['Snapshots']:
+            assert owner_id == snapshot['OwnerId']