diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index fedd53d82d..81510b0e39 100644 --- a/glance/api/v2/images.py +++ b/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", diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 2d6b8d0662..5d86907621 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/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 - # 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', - side_effect=exception.Conflict): + + with mock.patch.object( + glance.api.authorization.ImageRepoProxy, 'get') as mock_get: + mock_get.return_value = FakeImage(container_format=None) 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', - side_effect=exception.InvalidImageStatusTransition): + 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': 'glance-direct'}}) def test_image_import_raises_bad_request(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', - side_effect=ValueError): - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.import_image, request, UUID4, - {'method': {'name': 'glance-direct'}}) + with mock.patch.object( + glance.api.authorization.ImageRepoProxy, 'get') as mock_get: + mock_get.return_value = FakeImage(status='uploading') + # 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', + 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, - self.controller.import_image, request, UUID4, - {'method': {'name': 'web-download', - 'uri': 'fake_uri'}}) + with mock.patch.object( + glance.api.authorization.ImageRepoProxy, 'get') as mock_get: + mock_get.return_value = FakeImage(status='queued') + 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, - {'method': {'name': - 'glance-direct'}}) + with mock.patch.object( + glance.api.authorization.ImageRepoProxy, 'get') as mock_get: + 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, - self.controller.import_image, - request, UUID4, {'method': {'name': - 'glance-direct'}}) + with mock.patch.object( + glance.api.authorization.ImageRepoProxy, 'get') as mock_get: + mock_get.return_value = FakeImage(status='uploading') + self.assertRaises(webob.exc.HTTPForbidden, + self.controller.import_image, + request, UUID4, {'method': {'name': + 'glance-direct'}}) class TestImagesControllerPolicies(base.IsolatedUnitTest):