Handle iso+gpt detections

Some bootable CD images have an MBR or GPT boot record in the system area of the ISO format. That causes us to detect both ISO and GPT formats, which are currently rejected (intentionally). Since that special case is legitimate and we need to treat those as ISO files, this adds handling to make that determination. It does so by handling the detection pipeline within nova so we have access to the multiple matching inspectors (similar to what glance does). Depends-On: https://review.opendev.org/c/openstack/requirements/+/934176 Change-Id: I01e4f1bd74c9535f1e588159fd5e91c9b8bc60d4
2024-10-08 10:00:02 -07:00
parent 110849f7f9
commit 507c6c1113
8 changed files with 48 additions and 24 deletions
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@ -15006,7 +15006,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
                                            '/fake/instance/dir', disk_info)
        self.assertFalse(mock_fetch_image.called)

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch('nova.privsep.path.utime')
    @mock.patch('nova.virt.libvirt.utils.create_image')
    def test_create_images_and_backing_ephemeral_gets_created(
@ -16753,7 +16753,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
        fake_mkfs.assert_has_calls([mock.call('ext4', '/dev/something',
                                              'myVol')])

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch('nova.privsep.path.utime')
    @mock.patch('nova.virt.libvirt.utils.fetch_image')
    @mock.patch('nova.virt.libvirt.utils.create_image')
--- a/nova/tests/unit/virt/libvirt/test_imagebackend.py
+++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py
@ -563,7 +563,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):

        mock_exists.assert_has_calls(exist_calls)

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch.object(imagebackend.utils, 'synchronized')
    @mock.patch('nova.virt.libvirt.utils.create_image')
    @mock.patch.object(os.path, 'exists', side_effect=[])
@ -596,7 +596,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
        mock_detect_format.assert_called_once()
        mock_detect_format.return_value.safety_check.assert_called_once_with()

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch.object(imagebackend.utils, 'synchronized')
    @mock.patch('nova.virt.libvirt.utils.create_image')
    @mock.patch.object(imagebackend.disk, 'extend')
@ -624,7 +624,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
        self.assertFalse(mock_extend.called)
        mock_detect_format.assert_called_once()

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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')
@ -666,7 +666,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
        mock_utime.assert_called()
        mock_detect_format.assert_called_once()

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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')
@ -701,7 +701,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
        self.assertFalse(mock_extend.called)
        mock_detect_format.assert_called_once()

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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')
--- a/nova/tests/unit/virt/libvirt/test_utils.py
+++ b/nova/tests/unit/virt/libvirt/test_utils.py
@ -107,7 +107,7 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
    @mock.patch('tempfile.NamedTemporaryFile')
    @mock.patch('oslo_concurrency.processutils.execute')
    @mock.patch('nova.virt.images.qemu_img_info')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    def _test_create_image(
        self, path, disk_format, disk_size, mock_detect, mock_info,
        mock_execute, mock_ntf, backing_file=None, encryption=None,
@ -443,7 +443,7 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
            _context, image_id, target, trusted_certs)

    @mock.patch.object(images, 'IMAGE_API')
-    @mock.patch.object(format_inspector, 'detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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')
--- a/nova/tests/unit/virt/test_images.py
+++ b/nova/tests/unit/virt/test_images.py
@ -101,7 +101,7 @@ class QemuTestCase(test.NoDBTestCase):
        mocked_execute.assert_called_once()

    @mock.patch.object(images, 'IMAGE_API')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch.object(images, 'convert_image',
                       side_effect=exception.ImageUnacceptable)
    @mock.patch.object(images, 'qemu_img_info')
@ -121,7 +121,7 @@ class QemuTestCase(test.NoDBTestCase):
                               None, 'href123', '/no/path')

    @mock.patch.object(images, 'IMAGE_API')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch.object(images, 'convert_image',
                       side_effect=exception.ImageUnacceptable)
    @mock.patch.object(images, 'qemu_img_info')
@ -144,7 +144,7 @@ class QemuTestCase(test.NoDBTestCase):
                               images.fetch_to_raw,
                               None, 'href123', '/no/path')

-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch.object(images, 'IMAGE_API')
    @mock.patch('os.rename')
    @mock.patch.object(images, 'qemu_img_info')
@ -218,7 +218,7 @@ class QemuTestCase(test.NoDBTestCase):
                                                 format='json'))

    @mock.patch.object(images, 'IMAGE_API')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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,
@ -242,7 +242,7 @@ class QemuTestCase(test.NoDBTestCase):
    @mock.patch('os.rename')
    @mock.patch.object(images, 'IMAGE_API')
    @mock.patch('oslo_utils.imageutils.format_inspector.get_inspector')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    @mock.patch.object(images, 'fetch')
    @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info')
    def test_fetch_iso_is_raw(
@ -272,7 +272,7 @@ class QemuTestCase(test.NoDBTestCase):
        mock_rename.assert_called_once_with('anypath.part', 'anypath')

    @mock.patch.object(images, 'IMAGE_API')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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,
@ -311,7 +311,7 @@ class QemuTestCase(test.NoDBTestCase):
        qemu_img_info.assert_not_called()

    @mock.patch.object(images, 'IMAGE_API')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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,
@ -329,7 +329,7 @@ class QemuTestCase(test.NoDBTestCase):

    @mock.patch.object(images, 'IMAGE_API')
    @mock.patch.object(images, 'qemu_img_info')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    def test_fetch_inspect_ami(self, detect, imginfo, glance):
        glance.get.return_value = {'disk_format': 'ami'}
        detect.return_value.__str__.return_value = 'raw'
@ -340,7 +340,7 @@ class QemuTestCase(test.NoDBTestCase):

    @mock.patch.object(images, 'IMAGE_API')
    @mock.patch.object(images, 'qemu_img_info')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    def test_fetch_inspect_aki(self, detect, imginfo, glance):
        glance.get.return_value = {'disk_format': 'aki'}
        detect.return_value.__str__.return_value = 'raw'
@ -351,7 +351,7 @@ class QemuTestCase(test.NoDBTestCase):

    @mock.patch.object(images, 'IMAGE_API')
    @mock.patch.object(images, 'qemu_img_info')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_format')
    def test_fetch_inspect_ari(self, detect, imginfo, glance):
        glance.get.return_value = {'disk_format': 'ari'}
        detect.return_value.__str__.return_value = 'raw'
@ -371,7 +371,7 @@ class QemuTestCase(test.NoDBTestCase):

    @mock.patch.object(images, 'IMAGE_API')
    @mock.patch.object(images, 'qemu_img_info')
-    @mock.patch('oslo_utils.imageutils.format_inspector.detect_file_format')
+    @mock.patch('nova.virt.images.get_image_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'
--- a/nova/virt/images.py
+++ b/nova/virt/images.py
@ -142,6 +142,30 @@ def check_vmdk_image(image_id, data):
        raise exception.ImageUnacceptable(image_id=image_id, reason=msg)


+def get_image_format(path):
+    with open(path, 'rb') as f:
+        wrapper = format_inspector.InspectWrapper(f)
+        try:
+            while f.peek():
+                wrapper.read(4096)
+                if wrapper.formats:
+                    break
+        finally:
+            wrapper.close()
+
+    try:
+        return wrapper.format
+    except format_inspector.ImageFormatError:
+        format_names = set(str(x) for x in wrapper.formats)
+        if format_names == {'iso', 'gpt'}:
+            # If iso+gpt, we choose the iso because bootable-as-block ISOs
+            # can legitimately have a GPT bootloader in front.
+            LOG.debug('Detected %s as ISO+GPT, allowing as ISO', path)
+            return [x for x in wrapper.formats if str(x) == 'iso'][0]
+        # Any other case of multiple formats is an error
+        raise
+
+
 def do_image_deep_inspection(img, image_href, path):
    ami_formats = ('ami', 'aki', 'ari')
    disk_format = img['disk_format']
@ -158,7 +182,7 @@ def do_image_deep_inspection(img, image_href, path):
                image_id=image_href,
                reason=_('Image not in a supported format'))

-        inspector = format_inspector.detect_file_format(path)
+        inspector = get_image_format(path)
        inspector.safety_check()

        # Images detected as gpt but registered as raw are legacy "whole disk"
--- a/nova/virt/libvirt/imagebackend.py
+++ b/nova/virt/libvirt/imagebackend.py
@ -688,7 +688,7 @@ class Qcow2(Image):
        # NOTE(sean-k-mooney) If the image was created by nova as a swap
        # or ephemeral disk it is safe to skip the deep inspection.
        if not CONF.workarounds.disable_deep_image_inspection and not safe:
-            inspector = format_inspector.detect_file_format(base)
+            inspector = images.get_image_format(base)
            try:
                inspector.safety_check()
            except format_inspector.SafetyCheckFailed as e:
--- a/nova/virt/libvirt/utils.py
+++ b/nova/virt/libvirt/utils.py
@ -163,7 +163,7 @@ def create_image(
        # the backing file if the image is not created by nova for swap or
        # ephemeral disks.
        if not CONF.workarounds.disable_deep_image_inspection and not safe:
-            inspector = format_inspector.detect_file_format(backing_file)
+            inspector = images.get_image_format(backing_file)
            try:
                inspector.safety_check()
            except format_inspector.SafetyCheckFailed as e:
--- a/requirements.txt
+++ b/requirements.txt
@ -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>=7.3.0 # Apache-2.0
+oslo.utils>=7.4.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