openstack
/
nova
Code Issues Proposed changes

Merge "Do not close returned image-chunk iterator & get_verifier early"

This commit is contained in:
Zuul 2024-05-06 17:57:32 +00:00 committed by Gerrit Code Review
parent 07f05add31 198805c7c5
commit 428990e7b7
2 changed files with 91 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
nova/image/glance.py
View File

@ -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

71
nova/tests/unit/image/test_glance.py
View File

@ -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)