Merge "Fix validating input for redfish update_firmware"

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})

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 '

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)

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 = {

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').