From 884e290a6f919efa50dd08797d9a127ba9f61369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aija=20Jaunt=C4=93va?= Date: Thu, 6 Jan 2022 06:34:19 -0500 Subject: Fix validating input for redfish update_firmware Story: 2009772 Task: 44249 Change-Id: I8e559b3c7e833c361e12d01d744510ac5c8d8cf6 (cherry picked from commit b824ea7fa8874e63cfe11bc82ce0dc049680344f) --- ironic/drivers/modules/redfish/firmware_utils.py | 58 ++++++++++++++ ironic/drivers/modules/redfish/management.py | 2 + .../drivers/modules/redfish/test_firmware_utils.py | 88 ++++++++++++++++++++++ .../drivers/modules/redfish/test_management.py | 8 ++ ...irmware-images-validation-9b5b2fd28314ce66.yaml | 8 ++ 5 files changed, 164 insertions(+) create mode 100644 ironic/drivers/modules/redfish/firmware_utils.py create mode 100644 ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py create mode 100644 releasenotes/notes/fix-redfish-firmware-images-validation-9b5b2fd28314ce66.yaml diff --git a/ironic/drivers/modules/redfish/firmware_utils.py b/ironic/drivers/modules/redfish/firmware_utils.py new file mode 100644 index 000000000..35e4bb1f2 --- /dev/null +++ b/ironic/drivers/modules/redfish/firmware_utils.py @@ -0,0 +1,58 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import jsonschema +from oslo_log import log + +from ironic.common import exception +from ironic.common.i18n import _ + +LOG = log.getLogger(__name__) + +_UPDATE_FIRMWARE_SCHEMA = { + "$schema": "http://json-schema.org/schema#", + "title": "update_firmware clean step schema", + "type": "array", + # list of firmware update images + "items": { + "type": "object", + "required": ["url"], + "properties": { + "url": { + "description": "URL for firmware file", + "type": "string", + "minLength": 1 + }, + "wait": { + "description": "optional wait time for firmware update", + "type": "integer", + "minimum": 1 + } + }, + "additionalProperties": False + } +} + + +def validate_update_firmware_args(firmware_images): + """Validate ``update_firmware`` step input argument + + :param firmware_images: args to validate. + :raises: InvalidParameterValue When argument is not valid + """ + try: + jsonschema.validate(firmware_images, _UPDATE_FIRMWARE_SCHEMA) + except jsonschema.ValidationError as err: + raise exception.InvalidParameterValue( + _('Invalid firmware update %(firmware_images)s. Errors: %(err)s') + % {'firmware_images': firmware_images, 'err': err}) diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index 6e8031c26..03153e4fb 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -35,6 +35,7 @@ from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils +from ironic.drivers.modules.redfish import firmware_utils from ironic.drivers.modules.redfish import utils as redfish_utils LOG = log.getLogger(__name__) @@ -759,6 +760,7 @@ class RedfishManagement(base.ManagementInterface): :returns: None if it is completed. :raises: RedfishError on an error from the Sushy library. """ + firmware_utils.validate_update_firmware_args(firmware_images) node = task.node LOG.debug('Updating firmware on node %(node_uuid)s with firmware ' diff --git a/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py new file mode 100644 index 000000000..60c66c024 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py @@ -0,0 +1,88 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from ironic.common import exception +from ironic.drivers.modules.redfish import firmware_utils +from ironic.tests import base + + +class FirmwareUtilsTestCase(base.TestCase): + + def test_validate_update_firmware_args(self): + firmware_images = [ + { + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "wait": 300 + }, + { + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE" + } + ] + firmware_utils.validate_update_firmware_args(firmware_images) + + def test_validate_update_firmware_args_not_list(self): + firmware_images = { + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "wait": 300 + } + self.assertRaisesRegex( + exception.InvalidParameterValue, "is not of type 'array'", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_unknown_key(self): + firmware_images = [ + { + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "wait": 300, + }, + { + "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE", + "something": "unknown" + } + ] + self.assertRaisesRegex( + exception.InvalidParameterValue, "'something' was unexpected", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_url_missing(self): + firmware_images = [ + { + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "wait": 300, + }, + { + "wait": 300 + } + ] + self.assertRaisesRegex( + exception.InvalidParameterValue, + "'url' is a required property", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_url_not_string(self): + firmware_images = [{ + "url": 123, + "wait": 300 + }] + self.assertRaisesRegex( + exception.InvalidParameterValue, "123 is not of type 'string'", + firmware_utils.validate_update_firmware_args, firmware_images) + + def test_validate_update_firmware_args_wait_not_int(self): + firmware_images = [{ + "url": "http://192.0.2.10/BMC_4_22_00_00.EXE", + "wait": 'abc' + }] + self.assertRaisesRegex( + exception.InvalidParameterValue, "'abc' is not of type 'integer'", + firmware_utils.validate_update_firmware_args, firmware_images) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index 72a355072..d6d532327 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -856,6 +856,14 @@ class RedfishManagementTestCase(db_base.DbTestCase): task.node) mock_node_power_action.assert_called_once_with(task, states.REBOOT) + def test_update_firmware_invalid_args(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises( + exception.InvalidParameterValue, + task.driver.management.update_firmware, + task, [{'urlX': 'test1'}, {'url': 'test2'}]) + @mock.patch.object(task_manager, 'acquire', autospec=True) def test__query_firmware_update_failed(self, mock_acquire): driver_internal_info = { diff --git a/releasenotes/notes/fix-redfish-firmware-images-validation-9b5b2fd28314ce66.yaml b/releasenotes/notes/fix-redfish-firmware-images-validation-9b5b2fd28314ce66.yaml new file mode 100644 index 000000000..21ee1b949 --- /dev/null +++ b/releasenotes/notes/fix-redfish-firmware-images-validation-9b5b2fd28314ce66.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes validation of input argument ``firmware_images`` of ``redfish`` + hardware type clean step ``update_firmware``. Now it validates the argument + at the beginning of clean step. Prior to this fix issues were determined + at the time of executing firmware update or not at all (for example, + mistyping optional field 'wait'). -- cgit v1.2.1