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

@@ -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
 
 

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)
 

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.