Skip to content

Commit

Permalink
Merge "Use format_inspector from oslo"
Browse files Browse the repository at this point in the history
  • Loading branch information
Zuul authored and openstack-gerrit committed Aug 27, 2024
2 parents 01b207e + d854e7c commit b3a2494
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 1,741 deletions.
1,038 changes: 0 additions & 1,038 deletions nova/image/format_inspector.py

This file was deleted.

666 changes: 0 additions & 666 deletions nova/tests/unit/image/test_format_inspector.py

This file was deleted.

4 changes: 2 additions & 2 deletions nova/tests/unit/virt/libvirt/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -14920,7 +14920,7 @@ def test_create_images_and_backing_images_exist(
'/fake/instance/dir', disk_info)
self.assertFalse(mock_fetch_image.called)

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch('nova.privsep.path.utime')
@mock.patch('nova.virt.libvirt.utils.create_image')
def test_create_images_and_backing_ephemeral_gets_created(
Expand Down Expand Up @@ -16664,7 +16664,7 @@ def test_create_ephemeral_specified_fs(self, fake_mkfs):
fake_mkfs.assert_has_calls([mock.call('ext4', '/dev/something',
'myVol')])

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch('nova.privsep.path.utime')
@mock.patch('nova.virt.libvirt.utils.fetch_image')
@mock.patch('nova.virt.libvirt.utils.create_image')
Expand Down
14 changes: 8 additions & 6 deletions nova/tests/unit/virt/libvirt/test_imagebackend.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from oslo_service import loopingcall
from oslo_utils.fixture import uuidsentinel as uuids
from oslo_utils import imageutils
from oslo_utils.imageutils import format_inspector
from oslo_utils import units
from oslo_utils import uuidutils

Expand Down Expand Up @@ -562,7 +563,7 @@ def test_cache_template_exists(self, mock_exists):

mock_exists.assert_has_calls(exist_calls)

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch('nova.virt.libvirt.utils.create_image')
@mock.patch.object(os.path, 'exists', side_effect=[])
Expand Down Expand Up @@ -594,7 +595,7 @@ def test_create_image(
mock_detect_format.assert_called_once()
mock_detect_format.return_value.safety_check.assert_called_once_with()

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch('nova.virt.libvirt.utils.create_image')
@mock.patch.object(imagebackend.disk, 'extend')
Expand Down Expand Up @@ -622,7 +623,7 @@ def test_create_image_too_small(self, mock_utime, mock_get, mock_exist,
self.assertFalse(mock_extend.called)
mock_detect_format.assert_called_once()

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch('nova.virt.libvirt.utils.create_image')
@mock.patch('nova.virt.libvirt.utils.get_disk_backing_file')
Expand Down Expand Up @@ -664,7 +665,7 @@ def test_generate_resized_backing_files(self, mock_utime, mock_copy,
mock_utime.assert_called()
mock_detect_format.assert_called_once()

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch('nova.virt.libvirt.utils.create_image')
@mock.patch('nova.virt.libvirt.utils.get_disk_backing_file')
Expand Down Expand Up @@ -699,7 +700,7 @@ def test_qcow2_exists_and_has_no_backing_file(self, mock_utime,
self.assertFalse(mock_extend.called)
mock_detect_format.assert_called_once()

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch('nova.virt.libvirt.utils.create_image')
@mock.patch('nova.virt.libvirt.utils.get_disk_backing_file')
Expand All @@ -711,7 +712,8 @@ def test_qcow2_exists_and_fails_safety_check(self,
mock_extend, mock_get,
mock_create, mock_sync,
mock_detect_format):
mock_detect_format.return_value.safety_check.return_value = False
mock_detect_format.return_value.safety_check.side_effect = (
format_inspector.SafetyCheckFailed({}))
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
mock_get.return_value = None
fn = mock.MagicMock()
Expand Down
4 changes: 2 additions & 2 deletions nova/tests/unit/virt/libvirt/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@
from oslo_config import cfg
from oslo_utils import fileutils
from oslo_utils.fixture import uuidsentinel as uuids
from oslo_utils.imageutils import format_inspector

from nova.compute import utils as compute_utils
from nova import context
from nova import exception
from nova.image import format_inspector
from nova import objects
from nova.objects import fields as obj_fields
import nova.privsep.fs
Expand Down Expand Up @@ -107,7 +107,7 @@ def test_valid_hostname_bad(self):
@mock.patch('tempfile.NamedTemporaryFile')
@mock.patch('oslo_concurrency.processutils.execute')
@mock.patch('nova.virt.images.qemu_img_info')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
def _test_create_image(
self, path, disk_format, disk_size, mock_detect, mock_info,
mock_execute, mock_ntf, backing_file=None, encryption=None,
Expand Down
30 changes: 16 additions & 14 deletions nova/tests/unit/virt/test_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from oslo_concurrency import processutils
from oslo_serialization import jsonutils
from oslo_utils import imageutils
from oslo_utils.imageutils import format_inspector

from nova.compute import utils as compute_utils
from nova import exception
Expand Down Expand Up @@ -100,7 +101,7 @@ def test_qemu_img_info_with_disk_not_found(self, exists, mocked_execute):
mocked_execute.assert_called_once()

@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(images, 'convert_image',
side_effect=exception.ImageUnacceptable)
@mock.patch.object(images, 'qemu_img_info')
Expand All @@ -120,7 +121,7 @@ def test_fetch_to_raw_errors(self, convert_image, qemu_img_info, fetch,
None, 'href123', '/no/path')

@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(images, 'convert_image',
side_effect=exception.ImageUnacceptable)
@mock.patch.object(images, 'qemu_img_info')
Expand All @@ -143,7 +144,7 @@ def test_fetch_to_raw_data_file(self, convert_image, qemu_img_info_fn,
images.fetch_to_raw,
None, 'href123', '/no/path')

@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
@mock.patch.object(images, 'IMAGE_API')
@mock.patch('os.rename')
@mock.patch.object(images, 'qemu_img_info')
Expand Down Expand Up @@ -217,7 +218,7 @@ def test_convert_image_vmdk_allowed_list_checking(self):
format='json'))

@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.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_detect,
Expand All @@ -240,8 +241,8 @@ def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_detect,

@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('oslo_utils.imageutils.format_inspector.get_inspector')
@mock.patch('oslo_utils.imageutils.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(
Expand Down Expand Up @@ -271,7 +272,7 @@ def test_fetch_iso_is_raw(
mock_rename.assert_called_once_with('anypath.part', 'anypath')

@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.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_detect,
Expand All @@ -280,7 +281,8 @@ def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_detect,
# abort before qemu-img-info
mock_glance.get.return_value = {'disk_format': 'qcow2'}
inspector = mock_detect.return_value
inspector.safety_check.return_value = False
inspector.safety_check.side_effect = (
format_inspector.SafetyCheckFailed({}))
inspector.__str__.return_value = 'qcow2'
self.assertRaises(exception.ImageUnacceptable,
images.fetch_to_raw, None, 'href123', '/no.path')
Expand All @@ -291,7 +293,7 @@ def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_detect,

# Image claims to be qcow2, is qcow2, passes safety check, so we make
# it all the way to qemu-img-info
inspector.safety_check.return_value = True
inspector.safety_check.side_effect = None
qemu_img_info.side_effect = test.TestingException
self.assertRaises(test.TestingException,
images.fetch_to_raw, None, 'href123', '/no.path')
Expand All @@ -309,7 +311,7 @@ def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_detect,
qemu_img_info.assert_not_called()

@mock.patch.object(images, 'IMAGE_API')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.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,
Expand All @@ -327,7 +329,7 @@ def test_fetch_to_raw_inspector_disabled(self, fetch, qemu_img_info,

@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.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'
Expand All @@ -338,7 +340,7 @@ def test_fetch_inspect_ami(self, detect, imginfo, glance):

@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.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'
Expand All @@ -349,7 +351,7 @@ def test_fetch_inspect_aki(self, detect, imginfo, glance):

@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.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'
Expand All @@ -369,7 +371,7 @@ def test_fetch_inspect_unknown_format(self, imginfo, glance):

@mock.patch.object(images, 'IMAGE_API')
@mock.patch.object(images, 'qemu_img_info')
@mock.patch('nova.image.format_inspector.detect_file_format')
@mock.patch('oslo_utils.imageutils.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'
Expand Down
21 changes: 15 additions & 6 deletions nova/virt/images.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
from oslo_log import log as logging
from oslo_utils import fileutils
from oslo_utils import imageutils
from oslo_utils.imageutils import format_inspector

from nova.compute import utils as compute_utils
import nova.conf
from nova import exception
from nova.i18n import _
from nova.image import format_inspector
from nova.image import glance
import nova.privsep.qemu

Expand Down Expand Up @@ -159,20 +159,29 @@ def do_image_deep_inspection(img, image_href, path):
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')))
inspector.safety_check()

# Images detected as gpt but registered as raw are legacy "whole disk"
# formats, which we continue to allow for now.
# 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 disk_format == 'raw' and str(inspector) == 'gpt':
LOG.debug('Image %s registered as raw, but detected as gpt',
image_href)
elif disk_format not in ami_formats and str(inspector) != disk_format:
# If we detected the image as something other than glance claimed,
# we abort.
LOG.warning('Image %s expected to be %s but detected as %s',
image_href, disk_format, str(inspector))
raise exception.ImageUnacceptable(
image_id=image_href,
reason=_('Image content does not match disk_format'))
except format_inspector.SafetyCheckFailed as e:
LOG.error('Image %s failed safety check: %s', image_href, e)
raise exception.ImageUnacceptable(
image_id=image_href,
reason=(_('Image does not pass safety check')))
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.
Expand Down
8 changes: 5 additions & 3 deletions nova/virt/libvirt/imagebackend.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@
from oslo_service import loopingcall
from oslo_utils import excutils
from oslo_utils import fileutils
from oslo_utils.imageutils import format_inspector
from oslo_utils import strutils
from oslo_utils import units

import nova.conf
from nova import exception
from nova.i18n import _
from nova.image import format_inspector
from nova.image import glance
import nova.privsep.libvirt
import nova.privsep.path
Expand Down Expand Up @@ -681,8 +681,10 @@ def create_qcow2_image(base, target, size):
# downloaded the image.
if not CONF.workarounds.disable_deep_image_inspection:
inspector = format_inspector.detect_file_format(base)
if not inspector.safety_check():
LOG.warning('Base image %s failed safety check', base)
try:
inspector.safety_check()
except format_inspector.SafetyCheckFailed as e:
LOG.warning('Base image %s failed safety check: %s', base, e)
# NOTE(danms): This is the same exception as would be raised
# by qemu_img_info() if the disk format was unreadable or
# otherwise unsuitable.
Expand Down
9 changes: 6 additions & 3 deletions nova/virt/libvirt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@
from oslo_concurrency import processutils
from oslo_log import log as logging
from oslo_utils import fileutils
from oslo_utils.imageutils import format_inspector

import nova.conf
from nova import context as nova_context
from nova import exception
from nova.i18n import _
from nova.image import format_inspector
from nova import objects
from nova.objects import fields as obj_fields
import nova.privsep.fs
Expand Down Expand Up @@ -159,8 +159,11 @@ def create_image(
# downloaded the image.
if not CONF.workarounds.disable_deep_image_inspection:
inspector = format_inspector.detect_file_format(backing_file)
if not inspector.safety_check():
LOG.warning('Base image %s failed safety check', backing_file)
try:
inspector.safety_check()
except format_inspector.SafetyCheckFailed as e:
LOG.warning('Base image %s failed safety check: %s',
backing_file, e)
# NOTE(danms): This is the same exception as would be raised
# by qemu_img_info() if the disk format was unreadable or
# otherwise unsuitable.
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ oslo.limit>=1.5.0 # Apache-2.0
oslo.reports>=1.18.0 # Apache-2.0
oslo.serialization>=4.2.0 # Apache-2.0
oslo.upgradecheck>=1.3.0
oslo.utils>=4.12.1 # Apache-2.0
oslo.utils>=7.3.0 # Apache-2.0
oslo.db>=10.0.0 # Apache-2.0
oslo.rootwrap>=5.15.0 # Apache-2.0
oslo.messaging>=14.1.0 # Apache-2.0
Expand Down

0 comments on commit b3a2494

Please sign in to comment.