openstack/nova
Code Issues Proposed changes

Merge "Change force_format strategy to catch mismatches"

Browse Source
This commit is contained in:
Zuul 2024-07-23 23:39:57 +00:00 committed by Gerrit Code Review
commit df39222b10
3 changed files with 108 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
nova/tests/unit/virt/libvirt/test_utils.py
View File

@ -443,12 +443,12 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
_context, image_id, target, trusted_certs)
@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(format_inspector, 'get_inspector')
@mock.patch.object(format_inspector, 'detect_file_format')
@mock.patch.object(compute_utils, 'disk_ops_semaphore')
@mock.patch('nova.privsep.utils.supports_direct_io', return_value=True)
@mock.patch('nova.privsep.qemu.unprivileged_convert_image')
def test_fetch_raw_image(self, mock_convert_image, mock_direct_io,
mock_disk_op_sema, mock_gi, mock_glance):
mock_disk_op_sema, mock_detect, mock_glance):
def fake_rename(old, new):
self.executes.append(('mv', old, new))
@ -488,7 +488,7 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
self.stub_out('oslo_utils.fileutils.delete_if_exists',
fake_rm_on_error)
mock_inspector = mock_gi.return_value.from_file.return_value
mock_inspector = mock_detect.return_value
# Since the remove param of fileutils.remove_path_on_error()
# is initialized at load time, we must provide a wrapper
@ -502,6 +502,7 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
# Make sure qcow2 gets converted to raw
mock_inspector.safety_check.return_value = True
mock_inspector.__str__.return_value = 'qcow2'
mock_glance.get.return_value = {'disk_format': 'qcow2'}
target = 't.qcow2'
self.executes = []
@ -516,12 +517,13 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
dest_encryption=None)
mock_convert_image.reset_mock()
mock_inspector.safety_check.assert_called_once_with()
mock_gi.assert_called_once_with('qcow2')
mock_detect.assert_called_once_with('t.qcow2.part')
# Make sure raw does not get converted
mock_gi.reset_mock()
mock_detect.reset_mock()
mock_inspector.safety_check.reset_mock()
mock_inspector.safety_check.return_value = True
mock_inspector.__str__.return_value = 'raw'
mock_glance.get.return_value = {'disk_format': 'raw'}
target = 't.raw'
self.executes = []
@ -530,12 +532,13 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
self.assertEqual(self.executes, expected_commands)
mock_convert_image.assert_not_called()
mock_inspector.safety_check.assert_called_once_with()
mock_gi.assert_called_once_with('raw')
mock_detect.assert_called_once_with('t.raw.part')
# Make sure safety check failure prevents us from proceeding
mock_gi.reset_mock()
mock_detect.reset_mock()
mock_inspector.safety_check.reset_mock()
mock_inspector.safety_check.return_value = False
mock_inspector.__str__.return_value = 'qcow2'
mock_glance.get.return_value = {'disk_format': 'qcow2'}
target = 'backing.qcow2'
self.executes = []
@ -545,10 +548,10 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
self.assertEqual(self.executes, expected_commands)
mock_convert_image.assert_not_called()
mock_inspector.safety_check.assert_called_once_with()
mock_gi.assert_called_once_with('qcow2')
mock_detect.assert_called_once_with('backing.qcow2.part')
# Make sure a format mismatch prevents us from proceeding
mock_gi.reset_mock()
mock_detect.reset_mock()
mock_inspector.safety_check.reset_mock()
mock_inspector.safety_check.side_effect = (
format_inspector.ImageFormatError)
@ -561,7 +564,7 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
self.assertEqual(self.executes, expected_commands)
mock_convert_image.assert_not_called()
mock_inspector.safety_check.assert_called_once_with()
mock_gi.assert_called_once_with('qcow2')
mock_detect.assert_called_once_with('backing.qcow2.part')
del self.executes

94
nova/tests/unit/virt/test_images.py
View File

@ -21,7 +21,6 @@ from oslo_utils import imageutils
from nova.compute import utils as compute_utils
from nova import exception
from nova.image import format_inspector
from nova import test
from nova.virt import images
@ -101,15 +100,16 @@ class QemuTestCase(test.NoDBTestCase):
mocked_execute.assert_called_once()
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.get_inspector')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch.object(images, 'convert_image',
side_effect=exception.ImageUnacceptable)
@mock.patch.object(images, 'qemu_img_info')
@mock.patch.object(images, 'fetch')
def test_fetch_to_raw_errors(self, convert_image, qemu_img_info, fetch,
get_inspector, glance):
inspector = get_inspector.return_value.from_file.return_value
mock_detect, glance):
inspector = mock_detect.return_value
inspector.safety_check.return_value = True
inspector.__str__.return_value = 'qcow2'
glance.get.return_value = {'disk_format': 'qcow2'}
qemu_img_info.backing_file = None
qemu_img_info.file_format = 'qcow2'
@ -120,16 +120,17 @@ class QemuTestCase(test.NoDBTestCase):
None, 'href123', '/no/path')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.get_inspector')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch.object(images, 'convert_image',
side_effect=exception.ImageUnacceptable)
@mock.patch.object(images, 'qemu_img_info')
@mock.patch.object(images, 'fetch')
def test_fetch_to_raw_data_file(self, convert_image, qemu_img_info_fn,
fetch, mock_gi, mock_glance):
fetch, mock_detect, mock_glance):
mock_glance.get.return_value = {'disk_format': 'qcow2'}
inspector = mock_gi.return_value.from_file.return_value
inspector = mock_detect.return_value
inspector.safety_check.return_value = True
inspector.__str__.return_value = 'qcow2'
# NOTE(danms): the above test needs the following line as well, as it
# is broken without it.
qemu_img_info = qemu_img_info_fn.return_value
@ -142,16 +143,17 @@ class QemuTestCase(test.NoDBTestCase):
images.fetch_to_raw,
None, 'href123', '/no/path')
@mock.patch('nova.image.format_inspector.get_inspector')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('os.rename')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch.object(images, 'fetch')
def test_fetch_to_raw_from_raw(self, fetch, qemu_img_info_fn, mock_rename,
mock_glance, mock_gi):
mock_glance, mock_detect):
# Make sure we support a case where we fetch an already-raw image and
# qemu-img returns None for "format_specific".
mock_glance.get.return_value = {'disk_format': 'raw'}
mock_detect.return_value.__str__.return_value = 'raw'
qemu_img_info = qemu_img_info_fn.return_value
qemu_img_info.file_format = 'raw'
qemu_img_info.backing_file = None
@ -215,14 +217,15 @@ class QemuTestCase(test.NoDBTestCase):
format='json'))
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.get_inspector')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch.object(images, 'fetch')
@mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info')
def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_gi,
def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_detect,
mock_glance):
mock_glance.get.return_value = {'disk_format': 'vmdk'}
inspector = mock_gi.return_value.from_file.return_value
inspector = mock_detect.return_value
inspector.safety_check.return_value = True
inspector.__str__.return_value = 'vmdk'
info = {'format': 'vmdk',
'format-specific': {
'type': 'vmdk',
@ -238,13 +241,17 @@ class QemuTestCase(test.NoDBTestCase):
@mock.patch('os.rename')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.get_inspector')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch.object(images, 'fetch')
@mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info')
def test_fetch_iso_is_raw(self, mock_info, mock_fetch, mock_gi,
def test_fetch_iso_is_raw(
self, mock_info, mock_fetch, mock_detect_file_format, mock_gi,
mock_glance, mock_rename):
mock_glance.get.return_value = {'disk_format': 'iso'}
inspector = mock_gi.return_value.from_file.return_value
inspector.safety_check.return_value = True
inspector.__str__.return_value = 'iso'
mock_detect_file_format.return_value = inspector
# qemu-img does not have a parser for iso so it is treated as raw
info = {
"virtual-size": 356352,
@ -258,27 +265,27 @@ class QemuTestCase(test.NoDBTestCase):
images.fetch_to_raw(None, 'foo', 'anypath')
# Make sure we called info with -f raw for an iso, since qemu-img does
# not support iso
mock_info.assert_called_once_with('anypath.part', format='raw')
mock_info.assert_called_once_with('anypath.part', format=None)
# Make sure that since we considered this to be a raw file, we did the
# just-rename-don't-convert path
mock_rename.assert_called_once_with('anypath.part', 'anypath')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.get_inspector')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch.object(images, 'fetch')
def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_gi,
def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_detect,
mock_glance):
# Image claims to be qcow2, is qcow2, but fails safety check, so we
# abort before qemu-img-info
mock_glance.get.return_value = {'disk_format': 'qcow2'}
inspector = mock_gi.return_value.from_file.return_value
inspector = mock_detect.return_value
inspector.safety_check.return_value = False
inspector.__str__.return_value = 'qcow2'
self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw, None, 'href123', '/no.path')
qemu_img_info.assert_not_called()
mock_gi.assert_called_once_with('qcow2')
mock_gi.return_value.from_file.assert_called_once_with('/no.path.part')
mock_detect.assert_called_once_with('/no.path.part')
inspector.safety_check.assert_called_once_with()
mock_glance.get.assert_called_once_with(None, 'href123')
@ -292,18 +299,17 @@ class QemuTestCase(test.NoDBTestCase):
# Image claims to be qcow2 in glance, but the image is something else,
# so we abort before qemu-img-info
qemu_img_info.reset_mock()
mock_gi.reset_mock()
mock_detect.reset_mock()
inspector.safety_check.reset_mock()
mock_gi.return_value.from_file.side_effect = (
format_inspector.ImageFormatError)
mock_detect.return_value.__str__.return_value = 'vmdk'
self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw, None, 'href123', '/no.path')
mock_gi.assert_called_once_with('qcow2')
inspector.safety_check.assert_not_called()
mock_detect.assert_called_once_with('/no.path.part')
inspector.safety_check.assert_called_once_with()
qemu_img_info.assert_not_called()
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.get_inspector')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch.object(images, 'fetch')
def test_fetch_to_raw_inspector_disabled(self, fetch, qemu_img_info,
@ -316,36 +322,41 @@ class QemuTestCase(test.NoDBTestCase):
# If deep inspection is disabled, we should never call the inspector
mock_gi.assert_not_called()
# ... and we let qemu-img detect the format itself.
qemu_img_info.assert_called_once_with('/no.path.part',
format=None)
qemu_img_info.assert_called_once_with('/no.path.part')
mock_glance.get.assert_not_called()
@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
def test_fetch_inspect_ami(self, imginfo, glance):
@mock.patch('nova.image.format_inspector.detect_file_format')
def test_fetch_inspect_ami(self, detect, imginfo, glance):
glance.get.return_value = {'disk_format': 'ami'}
detect.return_value.__str__.return_value = 'raw'
self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw, None, 'href123', '/no.path')
# Make sure 'ami was translated into 'raw' before we call qemu-img
imginfo.assert_called_once_with('/no.path.part', format='raw')
imginfo.assert_called_once_with('/no.path.part')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
def test_fetch_inspect_aki(self, imginfo, glance):
@mock.patch('nova.image.format_inspector.detect_file_format')
def test_fetch_inspect_aki(self, detect, imginfo, glance):
glance.get.return_value = {'disk_format': 'aki'}
detect.return_value.__str__.return_value = 'raw'
self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw, None, 'href123', '/no.path')
# Make sure 'aki was translated into 'raw' before we call qemu-img
imginfo.assert_called_once_with('/no.path.part', format='raw')
imginfo.assert_called_once_with('/no.path.part')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
def test_fetch_inspect_ari(self, imginfo, glance):
@mock.patch('nova.image.format_inspector.detect_file_format')
def test_fetch_inspect_ari(self, detect, imginfo, glance):
glance.get.return_value = {'disk_format': 'ari'}
detect.return_value.__str__.return_value = 'raw'
self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw, None, 'href123', '/no.path')
# Make sure 'aki was translated into 'raw' before we call qemu-img
imginfo.assert_called_once_with('/no.path.part', format='raw')
imginfo.assert_called_once_with('/no.path.part')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
@ -358,13 +369,16 @@ class QemuTestCase(test.NoDBTestCase):
@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch('nova.image.format_inspector.get_inspector')
def test_fetch_inspect_disagrees_qemu(self, mock_gi, imginfo, glance):
@mock.patch('nova.image.format_inspector.detect_file_format')
def test_fetch_inspect_disagrees_qemu(self, mock_detect, imginfo, glance):
glance.get.return_value = {'disk_format': 'qcow2'}
mock_detect.return_value.__str__.return_value = 'qcow2'
# Glance and inspector think it is a qcow2 file, but qemu-img does not
# agree. It was forced to interpret as a qcow2, but returned no
# format information as a result.
# agree.
imginfo.return_value.data_file = None
self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw, None, 'href123', '/no.path')
imginfo.assert_called_once_with('/no.path.part', format='qcow2')
imginfo.return_value.file_format = 'vmdk'
ex = self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw,
None, 'href123', '/no.path')
self.assertIn('content does not match disk_format', str(ex))
imginfo.assert_called_once_with('/no.path.part')

56
nova/virt/images.py
View File

@ -143,42 +143,50 @@ def check_vmdk_image(image_id, data):
def do_image_deep_inspection(img, image_href, path):
ami_formats = ('ami', 'aki', 'ari')
disk_format = img['disk_format']
try:
# NOTE(danms): Use our own cautious inspector module to make sure
# the image file passes safety checks.
# See https://bugs.launchpad.net/nova/+bug/2059809 for details.
inspector_cls = format_inspector.get_inspector(disk_format)
if not inspector_cls.from_file(path).safety_check():
# Make sure we have a format inspector for the claimed format, else
# it is something we do not support and must reject. AMI is excluded.
if (disk_format not in ami_formats and
not format_inspector.get_inspector(disk_format)):
raise exception.ImageUnacceptable(
image_id=image_href,
reason=_('Image not in a supported format'))
inspector = format_inspector.detect_file_format(path)
if not inspector.safety_check():
raise exception.ImageUnacceptable(
image_id=image_href,
reason=(_('Image does not pass safety check')))
# AMI formats can be other things, so don't obsess over this
# requirement for them. Otherwise, make sure our detection agrees
# with glance.
if disk_format not in ami_formats and str(inspector) != disk_format:
# If we detected the image as something other than glance claimed,
# we abort.
raise exception.ImageUnacceptable(
image_id=image_href,
reason=_('Image content does not match disk_format'))
except format_inspector.ImageFormatError:
# If the inspector we chose based on the image's metadata does not
# think the image is the proper format, we refuse to use it.
raise exception.ImageUnacceptable(
image_id=image_href,
reason=_('Image content does not match disk_format'))
except AttributeError:
# No inspector was found
LOG.warning('Unable to perform deep image inspection on type %r',
img['disk_format'])
if disk_format in ('ami', 'aki', 'ari'):
# A lot of things can be in a UEC, although it is typically a raw
# filesystem. We really have nothing we can do other than treat it
# like a 'raw', which is what qemu-img will detect a filesystem as
# anyway. If someone puts a qcow2 inside, we should fail because
# we won't do our inspection.
disk_format = 'raw'
else:
except Exception:
raise exception.ImageUnacceptable(
image_id=image_href,
reason=_('Image not in a supported format'))
if disk_format == 'iso':
# ISO image passed safety check; qemu will treat this as raw from here
if disk_format in ('iso',) + ami_formats:
# ISO or AMI image passed safety check; qemu will treat this as raw
# from here so return the expected formats it will find.
disk_format = 'raw'
return disk_format
@ -197,12 +205,22 @@ def fetch_to_raw(context, image_href, path, trusted_certs=None):
# Only run qemu-img after we have done deep inspection (if enabled).
# If it was not enabled, we will let it detect the format.
data = qemu_img_info(path_tmp, format=force_format)
data = qemu_img_info(path_tmp)
fmt = data.file_format
if fmt is None:
raise exception.ImageUnacceptable(
reason=_("'qemu-img info' parsing failed."),
image_id=image_href)
elif force_format is not None and fmt != force_format:
# Format inspector and qemu-img must agree on the format, else
# we reject. This will catch VMDK some variants that we don't
# explicitly support because qemu will identify them as such
# and we will not.
LOG.warning('Image %s detected by qemu as %s but we expected %s',
image_href, fmt, force_format)
raise exception.ImageUnacceptable(
image_id=image_href,
reason=_('Image content does not match disk_format'))
backing_file = data.backing_file
if backing_file is not None: