Merge "only safety check bootable files created from glance"

This commit is contained in:
Zuul
2024-09-19 04:56:06 +00:00
committed by Gerrit Code Review
7 changed files with 81 additions and 55 deletions
+3 -1
View File
@@ -215,7 +215,9 @@ class LibvirtImageBackendFixture(fixtures.Fixture):
return image_init
def _fake_cache(self, fetch_func, filename, size=None, *args, **kwargs):
def _fake_cache(
self, fetch_func, filename, size=None, safe=False, *args,
**kwargs):
# Execute the template function so we can test the arguments it was
# called with.
fetch_func(target=filename, *args, **kwargs)
@@ -3,6 +3,8 @@ import functools
import os
import shutil
from unittest import mock
import fixtures
from oslo_utils.fixture import uuidsentinel as uuids
@@ -11,7 +13,6 @@ from oslo_utils import units
import nova.conf
from nova import exception
from nova import objects
from nova import test
from nova.virt.libvirt import driver
@@ -53,17 +54,16 @@ class TestBugBackingFilePartitionTables(test.NoDBTestCase):
file_path = os.path.join(self.base_dir.path, 'test_file')
libvirt_utils.create_image(file_path, 'raw', '64M')
self.assertTrue(os.path.exists(file_path))
# nova should ensure that any file we create has a partition table
# inspector = format_inspector.GPTInspector.from_file(file_path)
# self.assertIsNotNone(inspector)
# inspector.safety_check()
# however the libvirt_utils.create_image method does not create a
# partition table so we should expect this to fail
# FIXME(sean-k-mooney): oslo currently detect vfat as an mbr partition
self.assertRaises(
format_inspector.ImageFormatError,
format_inspector.GPTInspector.from_file, file_path)
# nova files should pass the RawFileInspector safety check
inspector = format_inspector.RawFileInspector.from_file(file_path)
self.assertIsNotNone(inspector)
inspector.safety_check()
def test_cache_file(self):
"""Test the qcow2 cache interaction for ephemeral disks
@@ -85,8 +85,11 @@ class TestBugBackingFilePartitionTables(test.NoDBTestCase):
os_type=None, is_block_dev=False)
# this need to be multiples of 1G
size = 1 * units.Gi
fname = "ephemeral_%s_%s" % (size, ".qcow")
e = self.assertRaises(exception.InvalidDiskInfo,
image.cache, fetch_func=fn, context=None, filename=fname,
size=size, ephemeral_size=1)
self.assertIn("Base image failed safety check", str(e))
fname = "ephemeral_%s_%s" % (size, ".img")
with mock.patch.object(
imagebackend, '_update_utime_ignore_eacces') as m:
image.cache(
fetch_func=fn, context=None, filename=fname,
size=size, ephemeral_size=1, safe=True)
m.assert_called_once()
+6 -2
View File
@@ -14903,6 +14903,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
'qcow2',
virt_disk_size,
backing_file=backfile_path,
safe=False
)
@mock.patch('nova.virt.libvirt.imagebackend.Image.exists',
@@ -15054,12 +15055,14 @@ class LibvirtConnTestCase(test.NoDBTestCase,
'qcow2',
disk_info_byname['disk']['virt_disk_size'],
backing_file=root_backing,
safe=False
),
mock.call(
CONF.instances_path + '/disk.local',
'qcow2',
disk_info_byname['disk.local']['virt_disk_size'],
backing_file=ephemeral_backing,
safe=True
),
])
@@ -16626,7 +16629,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
context=self.context)
backend.disks['disk.swap'].cache.assert_called_once_with(
fetch_func=mock.ANY, filename='swap_%i' % expected,
size=expected * units.Mi, context=self.context, swap_mb=expected)
size=expected * units.Mi, context=self.context, swap_mb=expected,
safe=True)
@mock.patch.object(nova.virt.libvirt.imagebackend.Image, 'cache')
def test_create_vz_container_with_swap(self, mock_cache):
@@ -16711,7 +16715,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
backend.disks['disk.eph0'].cache.assert_called_once_with(
fetch_func=mock.ANY, context=self.context,
filename=filename, size=100 * units.Gi, ephemeral_size=mock.ANY,
specified_fs=None)
specified_fs=None, safe=True)
def test_create_image_resize_snap_backend(self):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@@ -514,7 +514,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
image.cache(fn, self.TEMPLATE)
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False)
mock_exists.assert_has_calls(exist_calls)
@mock.patch.object(os.path, 'exists')
@@ -545,7 +545,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
image.cache(fn, self.TEMPLATE)
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False)
mock_exists.assert_has_calls(exist_calls)
@mock.patch.object(os.path, 'exists')
@@ -587,7 +587,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
mock_verify.assert_called_once_with(self.TEMPLATE_PATH, self.SIZE)
mock_create.assert_called_once_with(
self.PATH, 'qcow2', self.SIZE, backing_file=self.TEMPLATE_PATH)
self.PATH, 'qcow2', self.SIZE, backing_file=self.TEMPLATE_PATH,
safe=False)
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
mock_exist.assert_has_calls(exist_calls)
self.assertTrue(mock_sync.called)
@@ -828,7 +829,7 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase):
image.cache(fn, self.TEMPLATE)
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False)
mock_ensure.assert_called_once_with(self.TEMPLATE_DIR)
mock_exists.assert_has_calls(exist_calls)
@@ -1369,7 +1370,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
image.cache(fn, self.TEMPLATE)
mock_ensure.assert_called_once_with(self.TEMPLATE_DIR)
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False)
mock_img_exist.assert_called_with()
mock_os_exist.assert_has_calls([
mock.call(self.TEMPLATE_DIR), mock.call(self.TEMPLATE_PATH)
@@ -1392,7 +1393,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
mock_os_exist.assert_has_calls([
mock.call(self.TEMPLATE_DIR), mock.call(self.TEMPLATE_PATH)
])
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False)
@mock.patch.object(os.path, 'exists', return_value=True)
@mock.patch.object(imagebackend.Rbd, 'exists', return_value=True)
@@ -2078,7 +2079,7 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase):
mock_ensure.assert_called_once_with(self.TEMPLATE_DIR)
mock_exists.assert_has_calls(exist_calls)
fn.assert_called_once_with(target=self.TEMPLATE_PATH)
fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False)
@mock.patch.object(imagebackend.Ploop, 'get_disk_size',
return_value=2048)
+17 -17
View File
@@ -4926,7 +4926,8 @@ class LibvirtDriver(driver.ComputeDriver):
'%dG' % ephemeral_size,
specified_fs)
return
libvirt_utils.create_image(target, 'raw', f'{ephemeral_size}G')
libvirt_utils.create_image(
target, 'raw', f'{ephemeral_size}G', safe=True)
# Run as root only for block devices.
disk_api.mkfs(os_type, fs_label, target, run_as_root=is_block_dev,
@@ -5169,11 +5170,9 @@ class LibvirtDriver(driver.ComputeDriver):
vm_mode=vm_mode)
fname = "ephemeral_%s_%s" % (ephemeral_gb, file_extension)
size = ephemeral_gb * units.Gi
disk_image.cache(fetch_func=fn,
context=context,
filename=fname,
size=size,
ephemeral_size=ephemeral_gb)
disk_image.cache(
fetch_func=fn, context=context, filename=fname, size=size,
ephemeral_size=ephemeral_gb, safe=True)
for idx, eph in enumerate(driver.block_device_info_get_ephemerals(
block_device_info)):
@@ -5195,12 +5194,10 @@ class LibvirtDriver(driver.ComputeDriver):
vm_mode=vm_mode)
size = eph['size'] * units.Gi
fname = "ephemeral_%s_%s" % (eph['size'], file_extension)
disk_image.cache(fetch_func=fn,
context=context,
filename=fname,
size=size,
ephemeral_size=eph['size'],
specified_fs=specified_fs)
disk_image.cache(
fetch_func=fn, context=context, filename=fname, size=size,
ephemeral_size=eph['size'], specified_fs=specified_fs,
safe=True)
if swap_mb > 0:
size = swap_mb * units.Mi
@@ -5208,9 +5205,10 @@ class LibvirtDriver(driver.ComputeDriver):
swap = image('disk.swap', disk_info_mapping=disk_info_mapping)
# Short circuit the exists() tests if we already created a disk
created_disks = created_disks or not swap.exists()
swap.cache(fetch_func=self._create_swap, context=context,
filename="swap_%s" % swap_mb,
size=size, swap_mb=swap_mb)
swap.cache(
fetch_func=self._create_swap, context=context,
filename="swap_%s" % swap_mb, size=size, swap_mb=swap_mb,
safe=True)
if created_disks:
LOG.debug('Created local disks', instance=instance)
@@ -11650,14 +11648,16 @@ class LibvirtDriver(driver.ComputeDriver):
os_type=instance.os_type,
filename=cache_name,
size=info['virt_disk_size'],
ephemeral_size=info['virt_disk_size'] / units.Gi)
ephemeral_size=info['virt_disk_size'] / units.Gi,
safe=True)
elif cache_name.startswith('swap'):
flavor = instance.get_flavor()
swap_mb = flavor.swap
disk.cache(fetch_func=self._create_swap,
filename="swap_%s" % swap_mb,
size=swap_mb * units.Mi,
swap_mb=swap_mb)
swap_mb=swap_mb,
safe=True)
else:
self._try_fetch_image_cache(disk,
libvirt_utils.fetch_image,
+24 -13
View File
@@ -135,7 +135,8 @@ class Image(metaclass=abc.ABCMeta):
return False
@abc.abstractmethod
def create_image(self, prepare_template, base, size, *args, **kwargs):
def create_image(
self, prepare_template, base, size, safe=False, *args, **kwargs):
"""Create image from template.
Contains specific behavior for each image type.
@@ -144,6 +145,7 @@ class Image(metaclass=abc.ABCMeta):
Should accept `target` argument.
:base: Template name
:size: Size of created image in bytes
:safe: True if image contains a safe filesystem
"""
pass
@@ -263,7 +265,8 @@ class Image(metaclass=abc.ABCMeta):
def exists(self):
return os.path.exists(self.path)
def cache(self, fetch_func, filename, size=None, *args, **kwargs):
def cache(self, fetch_func, filename, size=None, safe=False, *args,
**kwargs):
"""Creates image from template.
Ensures that template and image not already exists.
@@ -298,8 +301,9 @@ class Image(metaclass=abc.ABCMeta):
fetch_func(target=target, *args, **kwargs)
if not self.exists() or not os.path.exists(base):
self.create_image(fetch_func_sync, base, size,
*args, **kwargs)
self.create_image(
fetch_func_sync, base, size, safe=safe, *args,
**kwargs)
if size:
# create_image() only creates the base image if needed, so
@@ -593,7 +597,8 @@ class Flat(Image):
if os.path.exists(self.path):
self.driver_format = self.resolve_driver_format()
def create_image(self, prepare_template, base, size, *args, **kwargs):
def create_image(
self, prepare_template, base, size, safe=False, *args, **kwargs):
filename = self._get_lock_name(base)
@utils.synchronized(filename, external=True, lock_path=self.lock_path)
@@ -663,13 +668,14 @@ class Qcow2(Image):
self.disk_info_path = os.path.join(os.path.dirname(path), 'disk.info')
self.resolve_driver_format()
def create_image(self, prepare_template, base, size, *args, **kwargs):
def create_image(
self, prepare_template, base, size, safe=False, *args, **kwargs):
filename = self._get_lock_name(base)
@utils.synchronized(filename, external=True, lock_path=self.lock_path)
def create_qcow2_image(base, target, size):
def create_qcow2_image(base, target, size, safe=False):
libvirt_utils.create_image(
target, 'qcow2', size, backing_file=base)
target, 'qcow2', size, backing_file=base, safe=safe)
# Download the unmodified base image unless we already have a copy.
if not os.path.exists(base):
@@ -679,7 +685,9 @@ class Qcow2(Image):
# before we inspect it for other attributes. We do this each time
# because additional safety checks could have been added since we
# downloaded the image.
if not CONF.workarounds.disable_deep_image_inspection:
# 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)
try:
inspector.safety_check()
@@ -724,7 +732,7 @@ class Qcow2(Image):
if not os.path.exists(self.path):
with fileutils.remove_path_on_error(self.path):
create_qcow2_image(base, self.path, size)
create_qcow2_image(base, self.path, size, safe=safe)
def resize_image(self, size):
image = imgmodel.LocalFileImage(self.path, imgmodel.FORMAT_QCOW2)
@@ -800,7 +808,8 @@ class Lvm(Image):
def _can_fallocate(self):
return False
def create_image(self, prepare_template, base, size, *args, **kwargs):
def create_image(
self, prepare_template, base, size, safe=False, *args, **kwargs):
def encrypt_lvm_image():
dmcrypt.create_volume(self.path.rpartition('/')[2],
self.lv_path,
@@ -997,7 +1006,8 @@ class Rbd(Image):
LOG.warning("Ignoring failure to remove %(path)s: "
"%(error)s", {'path': base, 'error': e})
def create_image(self, prepare_template, base, size, *args, **kwargs):
def create_image(
self, prepare_template, base, size, safe=False, *args, **kwargs):
if not self.exists():
self._remove_non_raw_cache_image(base)
@@ -1272,7 +1282,8 @@ class Ploop(Image):
# Create new ploop disk (in case of epehemeral) or
# copy ploop disk from glance image
def create_image(self, prepare_template, base, size, *args, **kwargs):
def create_image(
self, prepare_template, base, size, safe=False, *args, **kwargs):
filename = os.path.basename(base)
# Copy main file of ploop disk, restore DiskDescriptor.xml for it
+7 -2
View File
@@ -133,7 +133,8 @@ def create_image(
disk_format: str,
disk_size: ty.Optional[ty.Union[str, int]],
backing_file: ty.Optional[str] = None,
encryption: ty.Optional[EncryptionOptions] = None
encryption: ty.Optional[EncryptionOptions] = None,
safe: bool = False,
) -> None:
"""Disk image creation with qemu-img
:param path: Desired location of the disk image
@@ -147,6 +148,7 @@ def create_image(
:param backing_file: (Optional) Backing file to use.
:param encryption: (Optional) Dict detailing various encryption attributes
such as the format and passphrase.
:param safe: If True, the image is know to be safe.
"""
cmd = [
'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'create', '-f', disk_format
@@ -157,7 +159,10 @@ def create_image(
# before we inspect it for other attributes. We do this each time
# because additional safety checks could have been added since we
# downloaded the image.
if not CONF.workarounds.disable_deep_image_inspection:
# Note(sean-k-mooney): We only need to perform the safety check for
# 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)
try:
inspector.safety_check()