From 046399ee6358084585264588df318bb0b26e50a8 Mon Sep 17 00:00:00 2001 From: Ilya Etingof Date: Tue, 16 Jul 2019 16:44:25 +0200 Subject: [PATCH] Add `filename` parameter to Redfish virtual media boot URL Some BMCs seem to validate virtual media boot image URL requiring that URL to end with something resembling ISO image file name (perhaps, its suffix i.e. extension). This patch tries to add, hopefully, meaningless `filename` parameter to boot URL's query string in hope to make the entire boot image URL looking more convincing to the BMC. Change-Id: I316712b38d10c0e801f7fd96a584074c8918b46b Story: 1526753 Task: 10389 --- ironic/drivers/modules/redfish/boot.py | 36 +++++++++++++++++ .../unit/drivers/modules/redfish/test_boot.py | 40 ++++++++++++++++--- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index 2d5295b2ef..a6c7162953 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -16,6 +16,7 @@ import tempfile from oslo_log import log from oslo_utils import importutils +from six.moves.urllib import parse as urlparse from ironic.common import boot_devices from ironic.common import exception @@ -218,6 +219,36 @@ class RedfishVirtualMediaBoot(base.BootInterface): {'node': task.node.uuid, 'image': object_name, 'error': e}) + @staticmethod + def _append_filename_param(url, filename): + """Append 'filename=' parameter to given URL. + + Some BMCs seem to validate boot image URL requiring the URL to end + with something resembling ISO image file name. + + This function tries to add, hopefully, meaningless 'filename' + parameter to URL's query string in hope to make the entire boot image + URL looking more convincing to the BMC. + + However, `url` with fragments might not get cured by this hack. + + :param url: a URL to work on + :param filename: name of the file to append to the URL + :returns: original URL with 'filename' parameter appended + """ + parsed_url = urlparse.urlparse(url) + parsed_qs = urlparse.parse_qsl(parsed_url.query) + + has_filename = [x for x in parsed_qs if x[0].lower() == 'filename'] + if has_filename: + return url + + parsed_qs.append(('filename', filename)) + parsed_url = list(parsed_url) + parsed_url[4] = urlparse.urlencode(parsed_qs) + + return urlparse.urlunparse(parsed_url) + @staticmethod def _get_floppy_image_name(node): """Returns the floppy image name for a given node. @@ -279,6 +310,8 @@ class RedfishVirtualMediaBoot(base.BootInterface): image_url = swift_api.get_temp_url(container, object_name, timeout) + image_url = cls._append_filename_param(image_url, 'bootme.img') + LOG.debug("Created floppy image %(name)s in Swift for node %(node)s, " "exposed as temporary URL " "%(url)s", {'node': task.node.uuid, @@ -388,6 +421,9 @@ class RedfishVirtualMediaBoot(base.BootInterface): boot_iso_url = swift_api.get_temp_url( container, iso_object_name, timeout) + boot_iso_url = cls._append_filename_param( + boot_iso_url, 'bootme.iso') + LOG.debug("Created ISO %(name)s in Swift for node %(node)s, exposed " "as temporary URL %(url)s", {'node': task.node.uuid, 'name': iso_object_name, diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 82aba08494..61033c5c64 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -175,6 +175,30 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot._parse_deploy_info, task.node) + def test__append_filename_param_without_qs(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + res = task.driver.boot._append_filename_param( + 'http://a.b/c', 'b.img') + expected = 'http://a.b/c?filename=b.img' + self.assertEqual(expected, res) + + def test__append_filename_param_with_qs(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + res = task.driver.boot._append_filename_param( + 'http://a.b/c?d=e&f=g', 'b.img') + expected = 'http://a.b/c?d=e&f=g&filename=b.img' + self.assertEqual(expected, res) + + def test__append_filename_param_with_filename(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + res = task.driver.boot._append_filename_param( + 'http://a.b/c?filename=bootme.img', 'b.img') + expected = 'http://a.b/c?filename=bootme.img' + self.assertEqual(expected, res) + @mock.patch.object(redfish_boot, 'swift', autospec=True) def test__cleanup_floppy_image(self, mock_swift): with task_manager.acquire(self.context, self.node.uuid, @@ -193,14 +217,18 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test__prepare_floppy_image(self, mock_create_vfat_image, mock_swift): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - task.driver.boot._prepare_floppy_image(task) + mock_swift_api = mock_swift.SwiftAPI.return_value + mock_swift_api.get_temp_url.return_value = 'https://a.b/c.f?e=f' + + url = task.driver.boot._prepare_floppy_image(task) + + self.assertIn('filename=bootme.img', url) + + mock_swift.SwiftAPI.assert_called_once_with() mock_create_vfat_image.assert_called_once_with( mock.ANY, parameters=mock.ANY) - mock_swift.SwiftAPI.assert_called_once_with() - mock_swift_api = mock_swift.SwiftAPI.return_value - mock_swift_api.create_object.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, mock.ANY) @@ -234,7 +262,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task, 'http://kernel/img', 'http://ramdisk/img', 'http://bootloader/img', root_uuid=task.node.uuid) - self.assertTrue(url) + self.assertIn('filename=bootme.iso', url) mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', @@ -263,7 +291,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task, 'http://kernel/img', 'http://ramdisk/img', bootloader_href=None, root_uuid=task.node.uuid) - self.assertTrue(url) + self.assertIn('filename=bootme.iso', url) mock_create_boot_iso.assert_called_once_with( mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img',