From 6a0b22360d27ce6c80671fe9f7e8507dc94045e1 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Sun, 20 Sep 2015 17:01:22 +0300 Subject: [PATCH] Cleanup chunks for deleted image if token expired In patch I47229b366c25367ec1bd48aec684e0880f3dfe60 it was introduced the logic that if image was deleted during file upload when we want to update image status from 'saving' to 'active' it's expected to get Duplicate error and delete stale chunks after that. But if user's token is expired there will be Unathorized exception and chunks will stay in store and clog it. And when, the upload operation for such an image is completed the operator configured quota can be exceeded. This patch fixes the issue of left over chunks for an image which was deleted from saving status, by correcly handle auth exceptions from registry server. Change-Id: I17a66eca55bfb83107046910e69c4da01415deec --- glance/api/v1/upload_utils.py | 7 ++++++ glance/api/v2/image_data.py | 14 +++++++++++- glance/common/scripts/image_import/main.py | 3 ++- .../unit/common/scripts/image_import/test_main.py | 26 ++++++++++++++++++++++ glance/tests/unit/v1/test_upload_utils.py | 26 ++++++++++++++++++++++ glance/tests/unit/v2/test_image_data_resource.py | 17 ++++++++++++++ 6 files changed, 91 insertions(+), 2 deletions(-) diff --git a/glance/api/v1/upload_utils.py b/glance/api/v1/upload_utils.py index db1eb54..9e5b251 100644 --- a/glance/api/v1/upload_utils.py +++ b/glance/api/v1/upload_utils.py @@ -171,6 +171,13 @@ def upload_data_to_store(req, image_meta, image_data, store, notifier): raise exception.ImageNotFound() else: raise + except exception.NotAuthenticated as e: + # Delete image data due to possible token expiration. + LOG.debug("Authorization problem - your token may have been " + "expired during file upload. Deleting image data for " + " %s " % image_id) + initiate_deletion(req, location_data, image_id) + raise webob.exc.HTTPUnauthorized(explanation=e.msg, request=req) except exception.ImageNotFound: msg = _LI("Image %s could not be found after upload. The image may" " have been deleted during the upload.") % image_id diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py index 317c7aa..a4290ae 100644 --- a/glance/api/v2/image_data.py +++ b/glance/api/v2/image_data.py @@ -105,7 +105,19 @@ class ImageDataController(object): raise webob.exc.HTTPGone(explanation=msg, request=req, content_type='text/plain') - + except exception.NotAuthenticated: + msg = (_("Authorization problem - your token may have been " + "expired during file upload. Deleting image data for " + "%s.") % image_id) + LOG.warn(msg) + try: + image.delete() + except exception.NotAuthenticated: + # NOTE: Ignore this exception + pass + raise webob.exc.HTTPUnauthorized(explanation=msg, + request=req, + content_type='text/plain') except ValueError as e: LOG.debug("Cannot save data for image %(id)s: %(e)s", {'id': image_id, diff --git a/glance/common/scripts/image_import/main.py b/glance/common/scripts/image_import/main.py index b3f82ed..e654095 100644 --- a/glance/common/scripts/image_import/main.py +++ b/glance/common/scripts/image_import/main.py @@ -110,7 +110,8 @@ def import_image(image_repo, image_factory, task_input, task_id, uri): "processing.") % {"image_id": image_id, "task_id": task_id} raise exception.Conflict(msg) - except (exception.Conflict, exception.NotFound): + except (exception.Conflict, exception.NotFound, + exception.NotAuthenticated): with excutils.save_and_reraise_exception(): if new_image.locations: for location in new_image.locations: diff --git a/glance/tests/unit/common/scripts/image_import/test_main.py b/glance/tests/unit/common/scripts/image_import/test_main.py index 924756a..b4b14a4 100644 --- a/glance/tests/unit/common/scripts/image_import/test_main.py +++ b/glance/tests/unit/common/scripts/image_import/test_main.py @@ -16,7 +16,10 @@ import mock from six.moves import urllib +import glance.common.exception as exception from glance.common.scripts.image_import import main as image_import_script +from glance.common import store_utils + import glance.tests.utils as test_utils @@ -91,3 +94,26 @@ class TestImageImport(test_utils.BaseTestCase): image = mock.Mock() self.assertRaises(urllib.error.URLError, image_import_script.set_image_data, image, uri, None) + + @mock.patch.object(image_import_script, 'create_image') + @mock.patch.object(image_import_script, 'set_image_data') + @mock.patch.object(store_utils, 'delete_image_location_from_backend') + def test_import_image_failed_with_expired_token( + self, mock_delete_data, mock_set_img_data, mock_create_image): + image_id = mock.ANY + locations = ['location'] + image = mock.Mock(image_id=image_id, locations=locations) + image_repo = mock.Mock() + image_repo.get.side_effect = [image, exception.NotAuthenticated] + image_factory = mock.ANY + task_input = mock.Mock(image_properties=mock.ANY) + uri = mock.ANY + + mock_create_image.return_value = image + self.assertRaises(exception.NotAuthenticated, + image_import_script.import_image, + image_repo, image_factory, + task_input, None, uri) + self.assertEqual(1, mock_set_img_data.call_count) + mock_delete_data.assert_called_once_with( + mock_create_image().context, image_id, 'location') diff --git a/glance/tests/unit/v1/test_upload_utils.py b/glance/tests/unit/v1/test_upload_utils.py index e1c4d4f..20d9c24 100644 --- a/glance/tests/unit/v1/test_upload_utils.py +++ b/glance/tests/unit/v1/test_upload_utils.py @@ -323,3 +323,29 @@ class TestUploadUtils(base.StoreClearingUnitTest): 'metadata': {}}, image_meta['id']) mock_safe_kill.assert_called_once_with( req, image_meta['id'], 'saving') + + @mock.patch.object(registry, 'update_image_metadata', + side_effect=exception.NotAuthenticated) + @mock.patch.object(upload_utils, 'initiate_deletion') + def test_activate_image_with_expired_token( + self, mocked_delete, mocked_update): + """Test token expiration during image upload. + + If users token expired before image was uploaded then if auth error + was caught from registry during changing image status from 'saving' + to 'active' then it's required to delete all image data. + """ + context = mock.Mock() + req = mock.Mock() + req.context = context + with self._get_store_and_notifier() as (location, checksum, image_meta, + image_data, store, notifier, + update_data): + self.assertRaises(webob.exc.HTTPUnauthorized, + upload_utils.upload_data_to_store, + req, image_meta, image_data, store, notifier) + self.assertEqual(2, mocked_update.call_count) + mocked_delete.assert_called_once_with( + req, + {'url': 'file://foo/bar', 'status': 'active', 'metadata': {}}, + 'c80a1a6c-bd1f-41c5-90ee-81afedb1d58d') diff --git a/glance/tests/unit/v2/test_image_data_resource.py b/glance/tests/unit/v2/test_image_data_resource.py index 2e5da38..8aaf7a5 100644 --- a/glance/tests/unit/v2/test_image_data_resource.py +++ b/glance/tests/unit/v2/test_image_data_resource.py @@ -192,6 +192,23 @@ class TestImagesController(base.StoreClearingUnitTest): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.upload, request, unit_test_utils.UUID1, 'YYYY', 4) + def test_upload_with_expired_token(self): + def side_effect(image, from_state=None): + if from_state == 'saving': + raise exception.NotAuthenticated() + + mocked_save = mock.Mock(side_effect=side_effect) + mocked_delete = mock.Mock() + request = unit_test_utils.get_fake_request() + image = FakeImage('abcd') + image.delete = mocked_delete + self.image_repo.result = image + self.image_repo.save = mocked_save + self.assertRaises(webob.exc.HTTPUnauthorized, self.controller.upload, + request, unit_test_utils.UUID1, 'YYYY', 4) + self.assertEqual(3, mocked_save.call_count) + mocked_delete.assert_called_once_with() + def test_upload_non_existent_image_during_save_initiates_deletion(self): def fake_save_not_found(self): raise exception.ImageNotFound() -- 2.5.3