diff --git a/bindep.txt b/bindep.txt index 7f3c28f12a..2752e2dc82 100644 --- a/bindep.txt +++ b/bindep.txt @@ -94,3 +94,5 @@ unzip [imagebuild] sudo [imagebuild] gawk [imagebuild] mtools [imagebuild] +# For automatic artifact decompression +zstd [devstack] diff --git a/ironic/common/images.py b/ironic/common/images.py index df321219e8..f3193042c6 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -417,6 +417,34 @@ def fetch_into(context, image_href, image_file, return None +def _handle_zstd_compression(path): + zstd_comp = False + with open(path, 'rb') as comp_check: + # Check for zstd compression. Zstd has a variable window for streaming + # clients with transparent connections, and 128 byte blocks. + # Ultimately, requests can't support handling of such content without + # the zstandard library (bsd), but that is not available in global + # requirements. As such, and likely best complexity wise, if we find it + # we can handle it directly. + # https://github.com/facebook/zstd/blob/dev/doc/zstd_compression_format.md + # Ensure we're at the start of the file + comp_check.seek(0) + read = comp_check.read(4) + if read.startswith(b"\x28\xb5\x2f\xfd"): + zstd_comp = True + + if zstd_comp and not CONF.conductor.disable_zstandard_decompression: + temp_path = path + '.zstd' + shutil.move(path, temp_path) + try: + utils.execute('zstd', '-d', '--rm', temp_path) + except OSError as e: + LOG.error('Failed to decompress a zstd compressed file: %s', e) + # Restore the downloaded file... We might want to fail the + # entire process. + shutil.move(temp_path, path) + + def fetch(context, image_href, path, force_raw=False, checksum=None, checksum_algo=None, image_auth_data=None): @@ -428,8 +456,11 @@ def fetch(context, image_href, path, force_raw=False, and checksum): checksum_utils.validate_checksum(path, checksum, checksum_algo) - # FIXME(TheJulia): need to check if we need to extract the file - # i.e. zstd... before forcing raw. + # Check and decompress zstd files, since python-requests realistically + # can't do it for us as-is. Also, some OCI container registry artifacts + # may generally just be zstd compressed, regardless if it is a raw file + # or a qcow2 file. + _handle_zstd_compression(path) if force_raw: image_to_raw(image_href, path, "%s.part" % path) diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py index 189cec6060..43aab0fa37 100644 --- a/ironic/conf/conductor.py +++ b/ironic/conf/conductor.py @@ -522,6 +522,15 @@ opts = [ 'functionality by setting this option to True will ' 'create a more secure environment, however it may ' 'break users in an unexpected fashion.')), + cfg.BoolOpt('disable_zstandard_decompression', + default=False, + mutable=False, + help=_('Option to enable disabling transparent decompression ' + 'of files which are compressed with Zstandard ' + 'compression. This option is provided should operators ' + 'wish to disable this functionality, otherwise it is ' + 'automaticly applied by the conductor should a ' + 'compressed artifact be detected.')), ] diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index 4361233fd9..50a82e72e3 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -42,9 +42,11 @@ class IronicImagesTestCase(base.TestCase): class FakeImgInfo(object): pass + @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @mock.patch.object(image_service, 'get_image_service', autospec=True) @mock.patch.object(builtins, 'open', autospec=True) - def test_fetch_image_service(self, open_mock, image_service_mock): + def test_fetch_image_service(self, open_mock, image_service_mock, + mock_zstd): mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'file' open_mock.return_value = mock_file_handle @@ -56,12 +58,15 @@ class IronicImagesTestCase(base.TestCase): context='context') image_service_mock.return_value.download.assert_called_once_with( 'image_href', 'file') + mock_zstd.assert_called_once_with('path') + @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @mock.patch.object(image_service, 'get_image_service', autospec=True) @mock.patch.object(images, 'image_to_raw', autospec=True) @mock.patch.object(builtins, 'open', autospec=True) def test_fetch_image_service_force_raw(self, open_mock, image_to_raw_mock, - image_service_mock): + image_service_mock, + mock_zstd): image_service_mock.return_value.transfer_verified_checksum = None mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'file' @@ -74,7 +79,9 @@ class IronicImagesTestCase(base.TestCase): 'image_href', 'file') image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') + mock_zstd.assert_called_once_with('path') + @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) @mock.patch.object(image_service, 'get_image_service', autospec=True) @@ -82,7 +89,7 @@ class IronicImagesTestCase(base.TestCase): @mock.patch.object(builtins, 'open', autospec=True) def test_fetch_image_service_force_raw_with_checksum( self, open_mock, image_to_raw_mock, - image_service_mock, mock_checksum): + image_service_mock, mock_checksum, mock_zstd): image_service_mock.return_value.transfer_verified_checksum = None mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'file' @@ -98,7 +105,9 @@ class IronicImagesTestCase(base.TestCase): 'image_href', 'file') image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') + mock_zstd.assert_called_once_with('path') + @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) @mock.patch.object(image_service, 'get_image_service', autospec=True) @@ -106,7 +115,8 @@ class IronicImagesTestCase(base.TestCase): @mock.patch.object(builtins, 'open', autospec=True) def test_fetch_image_service_with_checksum_mismatch( self, open_mock, image_to_raw_mock, - image_service_mock, mock_checksum): + image_service_mock, mock_checksum, + mock_zstd): image_service_mock.return_value.transfer_verified_checksum = None mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'file' @@ -124,7 +134,9 @@ class IronicImagesTestCase(base.TestCase): 'image_href', 'file') # If the checksum fails, then we don't attempt to convert the image. image_to_raw_mock.assert_not_called() + mock_zstd.assert_not_called() + @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) @mock.patch.object(image_service, 'get_image_service', autospec=True) @@ -132,7 +144,8 @@ class IronicImagesTestCase(base.TestCase): @mock.patch.object(builtins, 'open', autospec=True) def test_fetch_image_service_force_raw_no_checksum_algo( self, open_mock, image_to_raw_mock, - image_service_mock, mock_checksum): + image_service_mock, mock_checksum, + mock_zstd): image_service_mock.return_value.transfer_verified_checksum = None mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'file' @@ -148,7 +161,9 @@ class IronicImagesTestCase(base.TestCase): 'image_href', 'file') image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') + mock_zstd.assert_called_once_with('path') + @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) @mock.patch.object(image_service, 'get_image_service', autospec=True) @@ -156,7 +171,8 @@ class IronicImagesTestCase(base.TestCase): @mock.patch.object(builtins, 'open', autospec=True) def test_fetch_image_service_force_raw_combined_algo( self, open_mock, image_to_raw_mock, - image_service_mock, mock_checksum): + image_service_mock, mock_checksum, + mock_zstd): image_service_mock.return_value.transfer_verified_checksum = None mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'file' @@ -172,7 +188,9 @@ class IronicImagesTestCase(base.TestCase): 'image_href', 'file') image_to_raw_mock.assert_called_once_with( 'image_href', 'path', 'path.part') + mock_zstd.assert_called_once_with('path') + @mock.patch.object(images, '_handle_zstd_compression', autospec=True) @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True) @mock.patch.object(image_service, 'get_image_service', autospec=True) @@ -180,7 +198,8 @@ class IronicImagesTestCase(base.TestCase): @mock.patch.object(builtins, 'open', autospec=True) def test_fetch_image_service_auth_data_checksum( self, open_mock, image_to_raw_mock, - svc_mock, mock_checksum): + svc_mock, mock_checksum, + mock_zstd): svc_mock.return_value.transfer_verified_checksum = 'f00' svc_mock.return_value.is_auth_set_needed = True mock_file_handle = mock.MagicMock(spec=io.BytesIO) @@ -201,6 +220,7 @@ class IronicImagesTestCase(base.TestCase): 'image_href', 'path', 'path.part') svc_mock.return_value.set_image_auth.assert_called_once_with( 'image_href', 'meow') + mock_zstd.assert_called_once_with('path') @mock.patch.object(image_format_inspector, 'detect_file_format', autospec=True) @@ -682,6 +702,32 @@ class IronicImagesTestCase(base.TestCase): validate_mock.side_effect = OSError self.assertIsNone(images.is_source_a_path('context', url)) + @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(shutil, 'move', autospec=True) + @mock.patch.object(builtins, 'open', autospec=True) + def test__hanlde_zstd_compression(self, mock_open, mock_move, + mock_exec): + mock_file_handle = mock.Mock() + mock_file_handle.read.return_value = b"\x28\xb5\x2f\xfd" + mock_open.return_value.__enter__.open = mock_file_handle + images._handle_zstd_compression('path') + mock_move.assert_called_once_with('path', 'path.zstd') + mock_exec.assert_called_once_with('zstd', '-d', '--rm', 'path.zstd') + + @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(shutil, 'move', autospec=True) + @mock.patch.object(builtins, 'open', autospec=True) + def test__hanlde_zstd_compression_disabled(self, mock_open, mock_move, + mock_exec): + mock_file_handle = mock.Mock() + mock_file_handle.read.return_value = b"\x28\xb5\x2f\xfd" + mock_open.return_value.__enter__.open = mock_file_handle + CONF.set_override('disable_zstandard_decompression', True, + group='conductor') + images._handle_zstd_compression('path') + mock_move.assert_not_called() + mock_exec.assert_not_called() + class FsImageTestCase(base.TestCase): diff --git a/releasenotes/notes/automatic-zstd-decompression-bf30cb99ebbb07f3.yaml b/releasenotes/notes/automatic-zstd-decompression-bf30cb99ebbb07f3.yaml new file mode 100644 index 0000000000..a393a4794e --- /dev/null +++ b/releasenotes/notes/automatic-zstd-decompression-bf30cb99ebbb07f3.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Adds the capability for Ironic's conductor to detect Zstandard + compressed content and to automatically decompress the files + to enable image format detection and conversion. + + This is due to use of Zstandard compression upon artifacts stored + in container registries is a popular practice, and can be disabled + using the ``[conductor]disable_zstandard_decompression`` + configuration option.