From 2d885126ec5b3cfbe548fa42325d53fbc47d8fd4 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 19 May 2022 18:32:18 +0200 Subject: [PATCH] Don't use URLs as part of temporary file names (part 2) First, it's leaking information. Second, the URLs can be really long and cause "OSError: [Errno 36] File name too long". The same issue has already been fixed in the image cache, this change updates building ISO. Since the files are created in a temporary location, just use constant names. Change-Id: Ie5e0523bb4a8941bcb8dc87c4a0b29cb28ddfe40 --- ironic/common/images.py | 10 ++-- ironic/tests/unit/common/test_images.py | 52 +++++++++---------- .../file-name-too-long-72265bb3fec704f8.yaml | 5 ++ 3 files changed, 35 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/file-name-too-long-72265bb3fec704f8.yaml diff --git a/ironic/common/images.py b/ironic/common/images.py index 8652ba1fb3..839faa0880 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -527,8 +527,8 @@ def create_boot_iso(context, output_filename, kernel_href, :raises: ImageCreationFailed, if creating boot ISO failed. """ with utils.tempdir() as tmpdir: - kernel_path = os.path.join(tmpdir, kernel_href.split('/')[-1]) - ramdisk_path = os.path.join(tmpdir, ramdisk_href.split('/')[-1]) + kernel_path = os.path.join(tmpdir, 'kernel') + ramdisk_path = os.path.join(tmpdir, 'ramdisk') fetch(context, kernel_href, kernel_path) fetch(context, ramdisk_href, ramdisk_path) @@ -543,13 +543,11 @@ def create_boot_iso(context, output_filename, kernel_href, deploy_iso_path = esp_image_path = None if deploy_iso_href: - deploy_iso_path = os.path.join( - tmpdir, deploy_iso_href.split('/')[-1]) + deploy_iso_path = os.path.join(tmpdir, 'iso') fetch(context, deploy_iso_href, deploy_iso_path) elif esp_image_href: - esp_image_path = os.path.join( - tmpdir, esp_image_href.split('/')[-1]) + esp_image_path = os.path.join(tmpdir, 'esp') fetch(context, esp_image_href, esp_image_path) elif CONF.esp_image: diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index e705373f2e..98cbedf275 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -740,16 +740,16 @@ class FsImageTestCase(base.TestCase): boot_mode='uefi') fetch_images_mock.assert_any_call( - 'ctx', 'kernel-uuid', 'tmpdir/kernel-uuid') + 'ctx', 'kernel-uuid', 'tmpdir/kernel') fetch_images_mock.assert_any_call( - 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk-uuid') + 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk') fetch_images_mock.assert_any_call( - 'ctx', 'deploy_iso-uuid', 'tmpdir/deploy_iso-uuid') + 'ctx', 'deploy_iso-uuid', 'tmpdir/iso') params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( - 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', - deploy_iso='tmpdir/deploy_iso-uuid', + 'output_file', 'tmpdir/kernel', 'tmpdir/ramdisk', + deploy_iso='tmpdir/iso', esp_image=None, kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @@ -768,16 +768,16 @@ class FsImageTestCase(base.TestCase): boot_mode='uefi') fetch_images_mock.assert_any_call( - 'ctx', 'kernel-uuid', 'tmpdir/kernel-uuid') + 'ctx', 'kernel-uuid', 'tmpdir/kernel') fetch_images_mock.assert_any_call( - 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk-uuid') + 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk') fetch_images_mock.assert_any_call( - 'ctx', 'efiboot-uuid', 'tmpdir/efiboot-uuid') + 'ctx', 'efiboot-uuid', 'tmpdir/esp') params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( - 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', - deploy_iso=None, esp_image='tmpdir/efiboot-uuid', + 'output_file', 'tmpdir/kernel', 'tmpdir/ramdisk', + deploy_iso=None, esp_image='tmpdir/esp', kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @@ -796,16 +796,16 @@ class FsImageTestCase(base.TestCase): boot_mode='uefi') expected_calls = [mock.call('ctx', 'http://kernel-href', - 'tmpdir/kernel-href'), + 'tmpdir/kernel'), mock.call('ctx', 'http://ramdisk-href', - 'tmpdir/ramdisk-href'), + 'tmpdir/ramdisk'), mock.call('ctx', 'http://deploy_iso-href', - 'tmpdir/deploy_iso-href')] + 'tmpdir/iso')] fetch_images_mock.assert_has_calls(expected_calls) params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( - 'output_file', 'tmpdir/kernel-href', 'tmpdir/ramdisk-href', - deploy_iso='tmpdir/deploy_iso-href', + 'output_file', 'tmpdir/kernel', 'tmpdir/ramdisk', + deploy_iso='tmpdir/iso', esp_image=None, kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @@ -824,16 +824,16 @@ class FsImageTestCase(base.TestCase): boot_mode='uefi') expected_calls = [mock.call('ctx', 'http://kernel-href', - 'tmpdir/kernel-href'), + 'tmpdir/kernel'), mock.call('ctx', 'http://ramdisk-href', - 'tmpdir/ramdisk-href'), + 'tmpdir/ramdisk'), mock.call('ctx', 'http://efiboot-href', - 'tmpdir/efiboot-href')] + 'tmpdir/esp')] fetch_images_mock.assert_has_calls(expected_calls) params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( - 'output_file', 'tmpdir/kernel-href', 'tmpdir/ramdisk-href', - deploy_iso=None, esp_image='tmpdir/efiboot-href', + 'output_file', 'tmpdir/kernel', 'tmpdir/ramdisk', + deploy_iso=None, esp_image='tmpdir/esp', kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_isolinux_image_for_bios', autospec=True) @@ -851,9 +851,9 @@ class FsImageTestCase(base.TestCase): 'kernel-params', 'bios') fetch_images_mock.assert_any_call( - 'ctx', 'kernel-uuid', 'tmpdir/kernel-uuid') + 'ctx', 'kernel-uuid', 'tmpdir/kernel') fetch_images_mock.assert_any_call( - 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk-uuid') + 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk') # Note (NobodyCam): the original assert asserted that fetch_images # was not called with parameters, this did not @@ -864,7 +864,7 @@ class FsImageTestCase(base.TestCase): params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( - 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', + 'output_file', 'tmpdir/kernel', 'tmpdir/ramdisk', kernel_params=params, inject_files=None) @mock.patch.object(images, 'create_isolinux_image_for_bios', autospec=True) @@ -883,13 +883,13 @@ class FsImageTestCase(base.TestCase): 'kernel-params', None) fetch_images_mock.assert_any_call( - 'ctx', 'kernel-uuid', 'tmpdir/kernel-uuid') + 'ctx', 'kernel-uuid', 'tmpdir/kernel') fetch_images_mock.assert_any_call( - 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk-uuid') + 'ctx', 'ramdisk-uuid', 'tmpdir/ramdisk') params = ['root=UUID=root-uuid', 'kernel-params'] create_isolinux_mock.assert_called_once_with( - 'output_file', 'tmpdir/kernel-uuid', 'tmpdir/ramdisk-uuid', + 'output_file', 'tmpdir/kernel', 'tmpdir/ramdisk', kernel_params=params, inject_files=None) @mock.patch.object(image_service, 'get_image_service', autospec=True) diff --git a/releasenotes/notes/file-name-too-long-72265bb3fec704f8.yaml b/releasenotes/notes/file-name-too-long-72265bb3fec704f8.yaml new file mode 100644 index 0000000000..9ccb07c3a0 --- /dev/null +++ b/releasenotes/notes/file-name-too-long-72265bb3fec704f8.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes ``OSError: [Errno 36] File name too long`` when building a virtual + media ISO from a long kernel, ramdisk or ESP URL.