Prevent taskflow creation in impossible import
This change prevents taskflow creation during image import in cases we know for sure that the image activation won't be successful. The change follows the Glance principles of failing early and tries to provide as meaningful error messages to the end user as possible. Co-authored-by: Abhishek Kekane <akekane@redhat.com> Closes-bug: 1758943 Change-Id: Iaee2781d07f4c0afd4f886f0b30b523bd47f1058
This commit is contained in:
Erno Kuvaja
parent
54329c6a21
commit
0376185fb3
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue