Merge "Prevent taskflow creation in impossible import"

glance/api/v2/images.py

@@ -90,6 +90,7 @@ class ImagesController(object):
 
     @utils.mutating
     def import_image(self, req, image_id, body):
+        image_repo = self.gateway.get_repo(req.context)
         task_factory = self.gateway.get_task_factory(req.context)
         executor_factory = self.gateway.get_task_executor_factory(req.context)
         task_repo = self.gateway.get_task_repo(req.context)
@@ -99,6 +100,32 @@ class ImagesController(object):
 
         import_method = body.get('method').get('name')
         uri = body.get('method').get('uri')
+
+        try:
+            image = image_repo.get(image_id)
+            if image.status == 'active':
+                msg = _("Image with status active cannot be target for import")
+                raise exception.Conflict(msg)
+            if image.status != 'queued' and import_method == 'web-download':
+                msg = _("Image needs to be in 'queued' state to use "
+                        "'web-download' method")
+                raise exception.Conflict(msg)
+            if (image.status != 'uploading' and
+                    import_method == 'glance-direct'):
+                msg = _("Image needs to be staged before 'glance-direct' "
+                        "method can be used")
+                raise exception.Conflict(msg)
+            if not getattr(image, 'container_format', None):
+                msg = _("'container_format' needs to be set before import")
+                raise exception.Conflict(msg)
+            if not getattr(image, 'disk_format', None):
+                msg = _("'disk_format' needs to be set before import")
+                raise exception.Conflict(msg)
+        except exception.Conflict as e:
+            raise webob.exc.HTTPConflict(explanation=e.msg)
+        except exception.NotFound as e:
+            raise webob.exc.HTTPNotFound(explanation=e.msg)
+
         if (import_method == 'web-download' and
            not utils.validate_import_uri(uri)):
                 LOG.debug("URI for web-download does not pass filtering: %s",

glance/tests/unit/v2/test_images_resource.py

@@ -112,6 +112,15 @@ def _db_image_member_fixture(image_id, member_id, **kwargs):
     return obj
 
 
+class FakeImage(object):
+    def __init__(self, status='active', container_format='ami',
+                 disk_format='ami'):
+        self.id = UUID4
+        self.status = status
+        self.container_format = container_format
+        self.disk_format = disk_format
+
+
 class TestImagesController(base.IsolatedUnitTest):
 
     def setUp(self):
@@ -600,49 +609,80 @@ class TestImagesController(base.IsolatedUnitTest):
         self.assertRaises(webob.exc.HTTPNotFound,
                           self.controller.show, request, UUID4)
 
-    def test_image_import_raises_conflict(self):
+    def test_image_import_raises_conflict_if_container_format_is_none(self):
         request = unit_test_utils.get_fake_request()
-        # NOTE(abhishekk): Due to
+
-        # https://bugs.launchpad.net/glance/+bug/1712463 taskflow is not
+        with mock.patch.object(
-        # executing. Once it is fixed instead of mocking spawn_n method
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
-        # we should mock execute method of _ImportToStore task.
+            mock_get.return_value = FakeImage(container_format=None)
-        with mock.patch.object(eventlet.GreenPool, 'spawn_n',
-                               side_effect=exception.Conflict):
             self.assertRaises(webob.exc.HTTPConflict,
                               self.controller.import_image, request, UUID4,
                               {'method': {'name': 'glance-direct'}})
 
+    def test_image_import_raises_conflict_if_disk_format_is_none(self):
+        request = unit_test_utils.get_fake_request()
+
+        with mock.patch.object(
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
+            mock_get.return_value = FakeImage(disk_format=None)
+            self.assertRaises(webob.exc.HTTPConflict,
+                              self.controller.import_image, request, UUID4,
+                              {'method': {'name': 'glance-direct'}})
+
+    def test_image_import_raises_conflict(self):
+        request = unit_test_utils.get_fake_request()
+
+        with mock.patch.object(
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
+            mock_get.return_value = FakeImage(status='queued')
+            self.assertRaises(webob.exc.HTTPConflict,
+                              self.controller.import_image, request, UUID4,
+                              {'method': {'name': 'glance-direct'}})
+
+    def test_image_import_raises_conflict_for_web_download(self):
+        request = unit_test_utils.get_fake_request()
+
+        with mock.patch.object(
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
+            mock_get.return_value = FakeImage()
+            self.assertRaises(webob.exc.HTTPConflict,
+                              self.controller.import_image, request, UUID4,
+                              {'method': {'name': 'web-download'}})
+
     def test_image_import_raises_conflict_for_invalid_status_change(self):
         request = unit_test_utils.get_fake_request()
-        # NOTE(abhishekk): Due to
+
-        # https://bugs.launchpad.net/glance/+bug/1712463 taskflow is not
-        # executing. Once it is fixed instead of mocking spawn_n method
-        # we should mock execute method of _ImportToStore task.
         with mock.patch.object(
-                eventlet.GreenPool, 'spawn_n',
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
-                side_effect=exception.InvalidImageStatusTransition):
+            mock_get.return_value = FakeImage()
             self.assertRaises(webob.exc.HTTPConflict,
                               self.controller.import_image, request, UUID4,
                               {'method': {'name': 'glance-direct'}})
 
     def test_image_import_raises_bad_request(self):
         request = unit_test_utils.get_fake_request()
-        # NOTE(abhishekk): Due to
+        with mock.patch.object(
-        # https://bugs.launchpad.net/glance/+bug/1712463 taskflow is not
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
-        # executing. Once it is fixed instead of mocking spawn_n method
+            mock_get.return_value = FakeImage(status='uploading')
-        # we should mock execute method of _ImportToStore task.
+            # NOTE(abhishekk): Due to
-        with mock.patch.object(eventlet.GreenPool, 'spawn_n',
+            # https://bugs.launchpad.net/glance/+bug/1712463 taskflow is not
-                               side_effect=ValueError):
+            # executing. Once it is fixed instead of mocking spawn_n method
-            self.assertRaises(webob.exc.HTTPBadRequest,
+            # we should mock execute method of _ImportToStore task.
-                              self.controller.import_image, request, UUID4,
+            with mock.patch.object(eventlet.GreenPool, 'spawn_n',
-                              {'method': {'name': 'glance-direct'}})
+                                   side_effect=ValueError):
+                self.assertRaises(webob.exc.HTTPBadRequest,
+                                  self.controller.import_image, request, UUID4,
+                                  {'method': {'name': 'glance-direct'}})
 
     def test_image_import_invalid_uri_filtering(self):
         request = unit_test_utils.get_fake_request()
-        self.assertRaises(webob.exc.HTTPBadRequest,
+        with mock.patch.object(
-                          self.controller.import_image, request, UUID4,
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
-                          {'method': {'name': 'web-download',
+            mock_get.return_value = FakeImage(status='queued')
-                                      'uri': 'fake_uri'}})
+            self.assertRaises(webob.exc.HTTPBadRequest,
+                              self.controller.import_image, request, UUID4,
+                              {'method': {'name': 'web-download',
+                                          'uri': 'fake_uri'}})
 
     def test_create(self):
         request = unit_test_utils.get_fake_request()
@@ -2251,9 +2291,12 @@ class TestImagesController(base.IsolatedUnitTest):
 
     def test_image_import(self):
         request = unit_test_utils.get_fake_request()
-        output = self.controller.import_image(request, UUID4,
+        with mock.patch.object(
-                                              {'method': {'name':
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
-                                                          'glance-direct'}})
+            mock_get.return_value = FakeImage(status='uploading')
+            output = self.controller.import_image(
+                request, UUID4, {'method': {'name': 'glance-direct'}})
+
         self.assertEqual(UUID4, output)
 
     def test_image_import_not_allowed(self):
@@ -2261,10 +2304,13 @@ class TestImagesController(base.IsolatedUnitTest):
         # NOTE(abhishekk): For coverage purpose setting tenant to
         # None. It is not expected to do in normal scenarios.
         request.context.tenant = None
-        self.assertRaises(webob.exc.HTTPForbidden,
+        with mock.patch.object(
-                          self.controller.import_image,
+                glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
-                          request, UUID4, {'method': {'name':
+            mock_get.return_value = FakeImage(status='uploading')
-                                                      'glance-direct'}})
+            self.assertRaises(webob.exc.HTTPForbidden,
+                              self.controller.import_image,
+                              request, UUID4, {'method': {'name':
+                                                          'glance-direct'}})
 
 
 class TestImagesControllerPolicies(base.IsolatedUnitTest):