diff --git a/nova/image/glance.py b/nova/image/glance.py index f524618a8beb..df6d537a9a4c 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -341,34 +341,49 @@ class GlanceImageServiceV2(object): if not any(check(mode) for check in (stat.S_ISFIFO, stat.S_ISSOCK)): os.fsync(fileno) + def _try_special_handlers(self, context, image_id, dst_path, verifier): + image = self.show(context, image_id, include_locations=True) + for entry in image.get('locations', []): + loc_url = entry['url'] + loc_meta = entry['metadata'] + o = urlparse.urlparse(loc_url) + xfer_method = self._get_transfer_method(o.scheme) + if not xfer_method: + continue + + try: + xfer_method(context, o, dst_path, loc_meta) + LOG.info("Successfully transferred using %s", o.scheme) + + if not verifier: + return True + + # Load chunks from the downloaded image file + # for verification + with open(dst_path, 'rb') as fh: + downloaded_length = os.path.getsize(dst_path) + image_chunks = glance_utils.IterableWithLength(fh, + downloaded_length) + self._verify_and_write(context, image_id, verifier, + image_chunks, None, None) + return True + except Exception: + LOG.exception("Download image error") + + return False + def download(self, context, image_id, data=None, dst_path=None, trusted_certs=None): """Calls out to Glance for data and writes data.""" + # First, try to get the verifier, so we do not even start to download + # the image and then fail on the metadata + verifier = self._get_verifier(context, image_id, trusted_certs) - # First, check if image could be directly downloaded by special handler + # Second, try to delegate image download to a special handler if (self._download_handlers and dst_path is not None): - image = self.show(context, image_id, include_locations=True) - for entry in image.get('locations', []): - loc_url = entry['url'] - loc_meta = entry['metadata'] - o = urlparse.urlparse(loc_url) - xfer_method = self._get_transfer_method(o.scheme) - if xfer_method: - try: - xfer_method(context, o, dst_path, loc_meta) - LOG.info("Successfully transferred using %s", o.scheme) - - # Load chunks from the downloaded image file - # for verification (if required) - with open(dst_path, 'rb') as fh: - downloaded_length = os.path.getsize(dst_path) - image_chunks = glance_utils.IterableWithLength(fh, - downloaded_length) - self._verify_and_write(context, image_id, - trusted_certs, image_chunks, None, None) - return - except Exception: - LOG.exception("Download image error") + if self._try_special_handlers(context, image_id, dst_path, + verifier): + return # By default (or if direct download has failed), use glance client call # to fetch the image and fill image_chunks @@ -384,10 +399,10 @@ class GlanceImageServiceV2(object): raise exception.ImageUnacceptable(image_id=image_id, reason='Image has no associated data') - return self._verify_and_write(context, image_id, trusted_certs, + return self._verify_and_write(context, image_id, verifier, image_chunks, data, dst_path) - def _verify_and_write(self, context, image_id, trusted_certs, + def _verify_and_write(self, context, image_id, verifier, image_chunks, data, dst_path): """Perform image signature verification and save the image file if needed. @@ -398,9 +413,7 @@ class GlanceImageServiceV2(object): be written out but instead image_chunks iterator is returned. :param image_id: The UUID of the image - :param trusted_certs: A 'nova.objects.trusted_certs.TrustedCerts' - object with a list of trusted image certificate - IDs. + :param verifier: An instance of a 'cursive.verifier' :param image_chunks An iterator pointing to the image data :param data: File object to use when writing the image. If passed as None and dst_path is provided, new file is opened. @@ -421,15 +434,9 @@ class GlanceImageServiceV2(object): write_image = False try: - # Retrieve properties for verification of Glance image signature - verifier = self._get_verifier(context, image_id, trusted_certs) - # Exit early if we do not need write nor verify - if verifier is None and write_image is False: - if data is None: - return image_chunks - else: - return + if verifier is None and not write_image: + return image_chunks for chunk in image_chunks: if verifier: @@ -464,8 +471,6 @@ class GlanceImageServiceV2(object): data.flush() self._safe_fsync(data) data.close() - if isinstance(image_chunks, glance_utils.IterableWithLength): - image_chunks.iterable.close() if data is None: return image_chunks diff --git a/nova/tests/unit/image/test_glance.py b/nova/tests/unit/image/test_glance.py index 200602b0575e..1623dc85f9ea 100644 --- a/nova/tests/unit/image/test_glance.py +++ b/nova/tests/unit/image/test_glance.py @@ -587,6 +587,25 @@ class TestDownloadNoDirectUri(test.NoDBTestCase): ctx, 2, 'data', args=(mock.sentinel.image_id,)) self.assertEqual(mock.sentinel.image_chunks, res) + @mock.patch('builtins.open') + @mock.patch('nova.image.glance.GlanceImageServiceV2.show') + def test_download_no_data_no_dest_path_iterator_v2( + self, show_mock, open_mock): + glance_iterable = mock.MagicMock(spec=io.BytesIO) + fake_img_data = ['A', 'B', 'C'] + glance_iterable.__iter__.return_value = fake_img_data + iterable = glanceclient.common.utils.IterableWithLength( + iterable=glance_iterable, length=len(fake_img_data)) + client = mock.MagicMock() + client.call.return_value = fake_glance_response(iterable) + ctx = mock.sentinel.ctx + service = glance.GlanceImageServiceV2(client) + res = service.download(ctx, mock.sentinel.image_id) + self.assertFalse(show_mock.called) + self.assertFalse(open_mock.called) + self.assertEqual(iterable, res) + self.assertFalse(glance_iterable.close.called) + @mock.patch('builtins.open') @mock.patch('nova.image.glance.GlanceImageServiceV2.show') def test_download_data_no_dest_path_v2(self, show_mock, open_mock): @@ -709,9 +728,9 @@ class TestDownloadNoDirectUri(test.NoDBTestCase): @mock.patch('nova.image.glance.GlanceImageServiceV2._get_verifier') @mock.patch('nova.image.glance.GlanceImageServiceV2._get_transfer_method') @mock.patch('nova.image.glance.GlanceImageServiceV2.show') - def test_download_direct_rbd_uri_v2( + def _test_download_direct_rbd_uri_v2( self, show_mock, get_tran_mock, get_verifier_mock, log_mock, - open_mock, getsize_mock): + open_mock, getsize_mock, with_verifier=True): self.flags(enable_rbd_download=True, group='glance') show_mock.return_value = { 'locations': [ @@ -734,7 +753,7 @@ class TestDownloadNoDirectUri(test.NoDBTestCase): open_mock.return_value = writer service = glance.GlanceImageServiceV2(client) - verifier = mock.MagicMock() + verifier = mock.MagicMock() if with_verifier else None get_verifier_mock.return_value = verifier res = service.download(ctx, mock.sentinel.image_id, @@ -748,34 +767,41 @@ class TestDownloadNoDirectUri(test.NoDBTestCase): tran_mod.assert_called_once_with(ctx, mock.ANY, mock.sentinel.dst_path, mock.sentinel.loc_meta) - open_mock.assert_called_once_with(mock.sentinel.dst_path, 'rb') - get_tran_mock.assert_called_once_with('rbd') # no client call, chunks were read right after xfer_mod.download: client.call.assert_not_called() - # verifier called with the value we got from rbd download - verifier.update.assert_has_calls( + if verifier: + open_mock.assert_called_once_with(mock.sentinel.dst_path, 'rb') + + # verifier called with the value we got from rbd download + verifier.update.assert_has_calls( + [ + mock.call("rbd1"), + mock.call("rbd2") + ] + ) + verifier.verify.assert_called() + log_mock.info.assert_has_calls( [ - mock.call("rbd1"), - mock.call("rbd2") + mock.call('Successfully transferred using %s', 'rbd'), + mock.call( + 'Image signature verification succeeded for image %s', + mock.sentinel.image_id) ] - ) - verifier.verify.assert_called() - log_mock.info.assert_has_calls( - [ - mock.call('Successfully transferred using %s', 'rbd'), - mock.call( - 'Image signature verification succeeded for image %s', - mock.sentinel.image_id) - ] - ) + ) # not opened for writing (already written) self.assertFalse(open_mock(mock.sentinel.dst_path, 'rw').called) # write not called (written by rbd download) writer.write.assert_not_called() + def test_download_direct_rbd_uri_v2(self): + self._test_download_direct_rbd_uri_v2() + + def test_download_direct_rbd_uri_without_verifier_v2(self): + self._test_download_direct_rbd_uri_v2(with_verifier=False) + @mock.patch('nova.image.glance.GlanceImageServiceV2._get_transfer_method') @mock.patch('nova.image.glance.GlanceImageServiceV2.show') @mock.patch('nova.image.glance.GlanceImageServiceV2._safe_fsync') @@ -999,7 +1025,7 @@ class TestDownloadSignatureVerification(test.NoDBTestCase): service.download, context=None, image_id=None, data=None, dst_path=None) - self.assertEqual(mock_log.error.call_count, 2) + self.assertEqual(mock_log.error.call_count, 1) @mock.patch('nova.image.glance.LOG') @mock.patch('nova.image.glance.GlanceImageServiceV2.show') @@ -1069,13 +1095,12 @@ class TestDownloadSignatureVerification(test.NoDBTestCase): glanceclient.common.utils.IterableWithLength( iterable=glance_iterable, length=len(self.fake_img_data))) service = glance.GlanceImageServiceV2(self.client) - mock_get_verifier.side_effect = \ - cursive_exception.SignatureVerificationError + mock_get_verifier.return_value = self.BadVerifier() mock_dest = mock.MagicMock() mock_open.return_value = mock_dest mock_show.return_value = self.fake_img_props fake_path = 'FAKE_PATH' - self.assertRaises(cursive_exception.SignatureVerificationError, + self.assertRaises(cryptography.exceptions.InvalidSignature, service.download, context=None, image_id=None, data=None, dst_path=fake_path)