From e05f74c623d53aecc4a2e097595da8cdbd2607ef Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Thu, 7 Oct 2021 15:19:56 +1000 Subject: [PATCH] Do not append filename parameter to image URL when using local file Currently Ironic always adds a filename parameter to virtual media image URL, for example: http://localhost/redfish/boot.iso?filename=file.iso. This is useful on certain BMCs which require a ".iso" suffix in the image URL especially when using swift as the image source. This should not be neccessary with file based virtual media images and adding this parameter can cause issues in BMCs which do not accept special characters such as "=" and "?" in virtual image URL. This change alters Ironic's behaviour to only add the filename parameter if swift backing store is used for the image. Story: 2009278 Task: 43547 Change-Id: I3338bdb6a24d9695ce32d205221183b9b876bcc8 --- ironic/drivers/modules/image_utils.py | 5 ++--- ironic/tests/unit/drivers/modules/test_image_utils.py | 6 +++--- ...ilename-param-from-vmedia-url-bf4773ede44f2206.yaml | 10 ++++++++++ 3 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/remove-filename-param-from-vmedia-url-bf4773ede44f2206.yaml diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index e42a645aee..bb0dfa166e 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/ironic/drivers/modules/image_utils.py @@ -193,6 +193,8 @@ class ImageHandler(object): object_headers=object_headers) image_url = swift_api.get_temp_url(container, object_name, timeout) + image_url = self._append_filename_param( + image_url, os.path.basename(image_file)) else: public_dir = os.path.join(CONF.deploy.http_root, @@ -221,9 +223,6 @@ class ImageHandler(object): http_url = CONF.deploy.external_http_url or CONF.deploy.http_url image_url = os.path.join(http_url, self._image_subdir, object_name) - image_url = self._append_filename_param( - image_url, os.path.basename(image_file)) - return image_url diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index ff061277a9..f008bcacf9 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -118,7 +118,7 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase): url = img_handler_obj.publish_image('file.iso', 'boot.iso') self.assertEqual( - 'http://localhost/redfish/boot.iso?filename=file.iso', url) + 'http://localhost/redfish/boot.iso', url) mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( @@ -140,7 +140,7 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase): url = img_handler_obj.publish_image('file.iso', 'boot.iso') self.assertEqual( - 'http://non-local.host/redfish/boot.iso?filename=file.iso', url) + 'http://non-local.host/redfish/boot.iso', url) mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( @@ -162,7 +162,7 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase): url = img_handler_obj.publish_image('file.iso', 'boot.iso') self.assertEqual( - 'http://localhost/redfish/boot.iso?filename=file.iso', url) + 'http://localhost/redfish/boot.iso', url) mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) diff --git a/releasenotes/notes/remove-filename-param-from-vmedia-url-bf4773ede44f2206.yaml b/releasenotes/notes/remove-filename-param-from-vmedia-url-bf4773ede44f2206.yaml new file mode 100644 index 0000000000..685f98a699 --- /dev/null +++ b/releasenotes/notes/remove-filename-param-from-vmedia-url-bf4773ede44f2206.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Removing `?filename=file.iso` suffix from the virtual media image URL + when the image is a regular file due to incompatibility with SuperMicro + X12 machines which do not accept special characters such as `=` or `?` + in the URL. Historically, this suffix was being added to improve + compatibility with those BMCs which require `.iso` suffix in the URL + while using swift as the image store. Old behaviour will remain for swift + backed images.