Fix: Interrupted copy-image leaking data on subsequent operation

If copying existing image in other stores fails while staging the data to
staging directory due to power, network or any other reason. Then
subsequent try may lead to data leaks in stores.

To fix this, added check of the actual image size with the size
of image file present in the staging area. If it does not match
then delete the image file from staging area so that the entire
image will be staged again.

Change-Id: I44bfefb6eee421e18e5e95a0dafaef0ea4e170da
Closes-Bug: #1885003

glance/async_/flows/_internal_plugins/copy_image.py

@@ -59,7 +59,26 @@ class _CopyImage(task.Task):
             self.image_id)
 
         if os.path.exists(file_path):
-            return file_path, 0
+            # NOTE (abhishekk): If previous copy-image operation is failed
+            # due to power failure, network failure or any other reason and
+            # the image data here is partial then clear the staging area and
+            # re-stage the fresh image data.
+            # Ref: https://bugs.launchpad.net/glance/+bug/1885003
+            size_in_staging = os.path.getsize(file_path)
+            if image.size == size_in_staging:
+                return file_path, 0
+            else:
+                LOG.debug(("Found partial image data in staging "
+                           "%(fn)s, deleting it to re-stage "
+                           "again"), {'fn': file_path})
+                try:
+                    os.unlink(file_path)
+                except OSError as e:
+                    LOG.error(_LE("Deletion of staged "
+                                  "image data from %(fn)s has failed because "
+                                  "[Errno %(en)d]"), {'fn': file_path,
+                                                      'en': e.errno})
+                    raise
 
         # At first search image in default_backend
         default_store = CONF.glance_store.default_backend

glance/tests/unit/async_/flows/test_copy_image.py

@@ -14,6 +14,7 @@
 #    under the License.
 
 import datetime
+import os
 from unittest import mock
 
 import glance_store as store_api
@@ -139,6 +140,67 @@ class TestCopyImageTask(test_utils.BaseTestCase):
                 mock_store_api.assert_called_once_with(
                     "os_glance_staging_store")
 
+    @mock.patch.object(os, 'unlink')
+    @mock.patch.object(os.path, 'getsize')
+    @mock.patch.object(os.path, 'exists')
+    @mock.patch.object(store_api, 'get_store_from_store_identifier')
+    def test_copy_image_to_staging_store_partial_data_exists(
+            self, mock_store_api, mock_exists, mock_getsize, mock_unlink):
+        mock_store_api.return_value = self.staging_store
+        mock_exists.return_value = True
+        mock_getsize.return_value = 3
+
+        copy_image_task = copy_image._CopyImage(
+            self.task.task_id, self.task_type, self.image_repo,
+            self.image_id)
+        with mock.patch.object(self.image_repo, 'get') as get_mock:
+            get_mock.return_value = mock.MagicMock(
+                image_id=self.images[0]['id'],
+                locations=self.images[0]['locations'],
+                status=self.images[0]['status'],
+                size=4
+            )
+            with mock.patch.object(store_api, 'get') as get_data:
+                get_data.return_value = (b"dddd", 4)
+                copy_image_task.execute()
+                mock_exists.assert_called_once()
+                mock_getsize.assert_called_once()
+                mock_unlink.assert_called_once()
+                self.staging_store.add.assert_called_once()
+                mock_store_api.assert_called_once_with(
+                    "os_glance_staging_store")
+
+    @mock.patch.object(os, 'unlink')
+    @mock.patch.object(os.path, 'getsize')
+    @mock.patch.object(os.path, 'exists')
+    @mock.patch.object(store_api, 'get_store_from_store_identifier')
+    def test_copy_image_to_staging_store_data_exists(
+            self, mock_store_api, mock_exists, mock_getsize, mock_unlink):
+        mock_store_api.return_value = self.staging_store
+        mock_exists.return_value = True
+        mock_getsize.return_value = 4
+
+        copy_image_task = copy_image._CopyImage(
+            self.task.task_id, self.task_type, self.image_repo,
+            self.image_id)
+        with mock.patch.object(self.image_repo, 'get') as get_mock:
+            get_mock.return_value = mock.MagicMock(
+                image_id=self.images[0]['id'],
+                locations=self.images[0]['locations'],
+                status=self.images[0]['status'],
+                size=4
+            )
+            copy_image_task.execute()
+            mock_exists.assert_called_once()
+            mock_store_api.assert_called_once_with(
+                "os_glance_staging_store")
+            mock_getsize.assert_called_once()
+            # As valid image data already exists in staging area
+            # it does not remove it and also does not download
+            # it again to staging area
+            mock_unlink.assert_not_called()
+            self.staging_store.add.assert_not_called()
+
     @mock.patch.object(store_api, 'get_store_from_store_identifier')
     def test_copy_non_existing_image_to_staging_store_(self, mock_store_api):
         mock_store_api.return_value = self.staging_store