ec2 api _get_image method logic flaw that strips the hex16 digit off of the image name, and does a search against the db for it and ignores that it may not be the correct image, such as if doing a search for aki-0000009, yet that image name doesn't exist, it strips off aki- and looks for any image_id 0000009 and if there was an image match that happens to be an ami instead of aki, it will go ahead and deregister that. That behavior is unintended, so added logic to ensure that the original request image_id matches the type of image being returned from database by matching against container_format attr

2 files changed, 28 insertions, 8 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 9be30cf75..a4f4dae4f 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -1088,12 +1088,16 @@ class CloudController(object):
     def _get_image(self, context, ec2_id):
         try:
             internal_id = ec2utils.ec2_id_to_id(ec2_id)
-            return self.image_service.show(context, internal_id)
+            image = self.image_service.show(context, internal_id)
         except (exception.InvalidEc2Id, exception.ImageNotFound):
             try:
                 return self.image_service.show_by_name(context, ec2_id)
             except exception.NotFound:
                 raise exception.ImageNotFound(image_id=ec2_id)
+        image_type = ec2_id.split('-')[0]
+        if image.get('container_format') != image_type:
+            raise exception.ImageNotFound(image_id=ec2_id)
+        return image
 
     def _format_image(self, image):
         """Convert from format defined by BaseImageService to S3 format."""
diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py
index bf7a2b7ca..d71a03aff 100644
--- a/nova/tests/test_cloud.py
+++ b/nova/tests/test_cloud.py
@@ -67,7 +67,8 @@ class CloudTestCase(test.TestCase):
         host = self.network.host
 
         def fake_show(meh, context, id):
-            return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1,
+            return {'id': 1, 'container_format': 'ami',
+                    'properties': {'kernel_id': 1, 'ramdisk_id': 1,
                     'type': 'machine', 'image_state': 'available'}}
 
         self.stubs.Set(fake._FakeImageService, 'show', fake_show)
@@ -418,7 +419,8 @@ class CloudTestCase(test.TestCase):
         describe_images = self.cloud.describe_images
 
         def fake_detail(meh, context):
-            return [{'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1,
+            return [{'id': 1, 'container_format': 'ami',
+                     'properties': {'kernel_id': 1, 'ramdisk_id': 1,
                     'type': 'machine'}}]
 
         def fake_show_none(meh, context, id):
@@ -448,7 +450,8 @@ class CloudTestCase(test.TestCase):
 
         def fake_show(meh, context, id):
             return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1,
-                    'type': 'machine'}, 'is_public': True}
+                    'type': 'machine'}, 'container_format': 'ami',
+                    'is_public': True}
 
         self.stubs.Set(fake._FakeImageService, 'show', fake_show)
         self.stubs.Set(fake._FakeImageService, 'show_by_name', fake_show)
@@ -460,7 +463,8 @@ class CloudTestCase(test.TestCase):
         modify_image_attribute = self.cloud.modify_image_attribute
 
         def fake_show(meh, context, id):
-            return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1,
+            return {'id': 1, 'container_format': 'ami',
+                    'properties': {'kernel_id': 1, 'ramdisk_id': 1,
                     'type': 'machine'}, 'is_public': False}
 
         def fake_update(meh, context, image_id, metadata, data=None):
@@ -494,6 +498,16 @@ class CloudTestCase(test.TestCase):
         self.assertRaises(exception.ImageNotFound, deregister_image,
                           self.context, 'ami-bad001')
 
+    def test_deregister_image_wrong_container_type(self):
+        deregister_image = self.cloud.deregister_image
+
+        def fake_delete(self, context, id):
+            return None
+
+        self.stubs.Set(fake._FakeImageService, 'delete', fake_delete)
+        self.assertRaises(exception.NotFound, deregister_image, self.context,
+                          'aki-00000001')
+
     def _run_instance(self, **kwargs):
         rv = self.cloud.run_instances(self.context, **kwargs)
         instance_id = rv['instancesSet'][0]['instanceId']
@@ -609,7 +623,7 @@ class CloudTestCase(test.TestCase):
 
         def fake_show_no_state(self, context, id):
             return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1,
-                    'type': 'machine'}}
+                    'type': 'machine'}, 'container_format': 'ami'}
 
         self.stubs.UnsetAll()
         self.stubs.Set(fake._FakeImageService, 'show', fake_show_no_state)
@@ -623,7 +637,8 @@ class CloudTestCase(test.TestCase):
         run_instances = self.cloud.run_instances
 
         def fake_show_decrypt(self, context, id):
-            return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1,
+            return {'id': 1, 'container_format': 'ami',
+                    'properties': {'kernel_id': 1, 'ramdisk_id': 1,
                     'type': 'machine', 'image_state': 'decrypting'}}
 
         self.stubs.UnsetAll()
@@ -638,7 +653,8 @@ class CloudTestCase(test.TestCase):
         run_instances = self.cloud.run_instances
 
         def fake_show_stat_active(self, context, id):
-            return {'id': 1, 'properties': {'kernel_id': 1, 'ramdisk_id': 1,
+            return {'id': 1, 'container_format': 'ami',
+                    'properties': {'kernel_id': 1, 'ramdisk_id': 1,
                     'type': 'machine'}, 'status': 'active'}
 
         self.stubs.Set(fake._FakeImageService, 'show', fake_show_stat_active)