From 960f10a902a9286b93ab953d96023b8c1d978fe3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Aija=20Jaunt=C4=93va?= <aija.jaunteva@dell.com>
Date: Thu, 9 Dec 2021 09:54:41 -0500
Subject: [PATCH] Add more sources to redfish firmware upgrade

Adds swift and file support for `redfish` management interface
`firmware_update` step.
Adds `source` to step and `[redfish]firmware_source` to config
for setting up if and how files are staged. Support `http`, `local`
and `swift` for staging.
Adds `checksum` to step for checksum verification when file is
staged.

Story: 2008723
Task: 42067
Change-Id: Ibcc7815b32344d67f912d7dcda7283bac3582316
---
 doc/source/admin/drivers/redfish.rst          |  51 ++-
 ironic/conf/redfish.py                        |  15 +
 .../drivers/modules/redfish/firmware_utils.py | 201 +++++++++-
 ironic/drivers/modules/redfish/management.py  |  72 +++-
 .../modules/redfish/test_firmware_utils.py    | 375 +++++++++++++++++-
 .../modules/redfish/test_management.py        | 219 +++++++++-
 ...fish-firmware-update-3da89f10dc0f8d21.yaml |  14 +
 7 files changed, 913 insertions(+), 34 deletions(-)
 create mode 100644 releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml

diff --git a/doc/source/admin/drivers/redfish.rst b/doc/source/admin/drivers/redfish.rst
index 0878b08bfc..d2d93d9ffb 100644
--- a/doc/source/admin/drivers/redfish.rst
+++ b/doc/source/admin/drivers/redfish.rst
@@ -385,6 +385,8 @@ The ``update_firmware`` cleaning step accepts JSON in the following format::
             "firmware_images":[
                 {
                     "url": "<url_to_firmware_image1>",
+                    "checksum": "<checksum for image, uses SHA1>",
+                    "source": "<optional override source setting for image>",
                     "wait": <number_of_seconds_to_wait>
                 },
                 {
@@ -410,16 +412,21 @@ Each firmware image dictionary, is of the form::
 
     {
       "url": "<URL of firmware image file>",
+      "checksum": "<checksum for image, uses SHA1>",
+      "source": "<Optional override source setting for image>",
       "wait": <Optional time in seconds to wait after applying update>
     }
 
-The ``url`` argument in the firmware image dictionary is mandatory, while the
-``wait`` argument is optional.
+The ``url``and ``checksum`` arguments in the firmware image dictionary are
+mandatory, while the ``source`` and ``wait`` arguments are optional.
 
+For ``url`` currently ``http``, ``https``, ``swift`` and ``file`` schemes are
+supported.
+
+``source`` corresponds to ``[redfish]firmware_source`` and by setting it here,
+it is possible to override global setting per firmware image in clean step
+arguments.
 
-.. note::
-   Only ``http`` and ``https`` URLs are currently supported in the ``url``
-   argument.
 
 .. note::
    At the present time, targets for the firmware update cannot be specified.
@@ -427,19 +434,20 @@ The ``url`` argument in the firmware image dictionary is mandatory, while the
    node. It is assumed that the BMC knows what components a given firmware
    image is applicable to.
 
-To perform a firmware update, first download the firmware to a web server that
-the BMC has network access to. This could be the ironic conductor web server
-or another web server on the BMC network. Using a web browser, curl, or similar
-tool on a server that has network access to the BMC, try downloading
-the firmware to verify that the URLs are correct and that the web server is
-configured properly.
+To perform a firmware update, first download the firmware to a web server,
+Swift or filesystem that the Ironic conductor or BMC has network access to.
+This could be the ironic conductor web server or another web server on the BMC
+network. Using a web browser, curl, or similar tool on a server that has
+network access to the BMC or Ironic conductor, try downloading the firmware to
+verify that the URLs are correct and that the web server is configured
+properly.
 
 Next, construct the JSON for the firmware update cleaning step to be executed.
 When launching the firmware update, the JSON may be specified on the command
-line directly or in a file. The following
-example shows one cleaning step that installs two firmware updates. The first
-updates the BMC firmware followed by a five minute wait to allow the BMC time
-to start back up. The second updates the firmware on all applicable NICs.::
+line directly or in a file. The following example shows one cleaning step that
+installs four firmware updates. All except 3rd entry that has explicit
+``source`` added, uses setting from ``[redfish]firmware_source`` to determine
+if and where to stage the files::
 
     [{
         "interface": "management",
@@ -448,10 +456,21 @@ to start back up. The second updates the firmware on all applicable NICs.::
             "firmware_images":[
                 {
                     "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+                    "checksum": "<sha1-checksum-of-the-file>",
                     "wait": 300
                 },
                 {
-                    "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE"
+                    "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE",
+                    "checksum": "<sha1-checksum-of-the-file>"
+                },
+                {
+                    "url": "file:///firmware_images/idrac/9/PERC_WN64_6.65.65.65_A00.EXE",
+                    "checksum": "<sha1-checksum-of-the-file>",
+                    "source": "http"
+                },
+                {
+                    "url": "swift://firmware_container/BIOS_W8Y0W_WN64_2.1.7.EXE",
+                    "checksum": "<sha1-checksum-of-the-file>"
                 }
             ]
         }
diff --git a/ironic/conf/redfish.py b/ironic/conf/redfish.py
index eddf3e0134..3cc9fe0159 100644
--- a/ironic/conf/redfish.py
+++ b/ironic/conf/redfish.py
@@ -90,6 +90,21 @@ opts = [
                default=60,
                help=_('Number of seconds to wait between checking for '
                       'failed firmware update tasks')),
+    cfg.StrOpt('firmware_source',
+               choices=[('http', _('If firmware source URL is also HTTP, then '
+                                   'serve from original location, otherwise '
+                                   'copy to ironic\'s HTTP server. Default.')),
+                        ('local', _('Download from original location and '
+                                    'server from ironic\'s HTTP server.')),
+                        ('swift', _('If firmware source URL is also Swift, '
+                                    'serve from original location, otherwise '
+                                    'copy to ironic\'s Swift server.'))],
+               default='http',
+               mutable=True,
+               help=_('Specifies how firmware image should be served. Whether '
+                      'from its original location using the firmware source '
+                      'URL directly, or should serve it from ironic\'s Swift '
+                      'or HTTP server.')),
     cfg.IntOpt('raid_config_status_interval',
                min=0,
                default=60,
diff --git a/ironic/drivers/modules/redfish/firmware_utils.py b/ironic/drivers/modules/redfish/firmware_utils.py
index 35e4bb1f29..c73cb80dd6 100644
--- a/ironic/drivers/modules/redfish/firmware_utils.py
+++ b/ironic/drivers/modules/redfish/firmware_utils.py
@@ -11,11 +11,20 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import os
+import shutil
+import tempfile
+from urllib import parse as urlparse
+
 import jsonschema
 from oslo_log import log
+from oslo_utils import fileutils
 
 from ironic.common import exception
 from ironic.common.i18n import _
+from ironic.common import image_service
+from ironic.common import swift
+from ironic.conf import CONF
 
 LOG = log.getLogger(__name__)
 
@@ -26,22 +35,35 @@ _UPDATE_FIRMWARE_SCHEMA = {
     # list of firmware update images
     "items": {
         "type": "object",
-        "required": ["url"],
+        "required": ["url", "checksum"],
         "properties": {
             "url": {
                 "description": "URL for firmware file",
                 "type": "string",
                 "minLength": 1
             },
+            "checksum": {
+                "description": "SHA1 checksum for firmware file",
+                "type": "string",
+                "minLength": 1
+            },
             "wait": {
                 "description": "optional wait time for firmware update",
                 "type": "integer",
                 "minimum": 1
+            },
+            "source":
+            {
+                "description": "optional firmware_source to override global "
+                "setting for firmware file",
+                "type": "string",
+                "enum": ["http", "local", "swift"]
             }
         },
         "additionalProperties": False
     }
 }
+_FIRMWARE_SUBDIR = 'firmware'
 
 
 def validate_update_firmware_args(firmware_images):
@@ -56,3 +78,180 @@ def validate_update_firmware_args(firmware_images):
         raise exception.InvalidParameterValue(
             _('Invalid firmware update %(firmware_images)s. Errors: %(err)s')
             % {'firmware_images': firmware_images, 'err': err})
+
+
+def get_swift_temp_url(parsed_url):
+    """Gets Swift temporary URL
+
+    :param parsed_url: Parsed URL from URL in format
+        swift://container/[sub-folder/]file
+    :returns: Swift temporary URL
+    """
+    return swift.SwiftAPI().get_temp_url(
+        parsed_url.netloc, parsed_url.path.lstrip('/'),
+        CONF.redfish.swift_object_expiry_timeout)
+
+
+def download_to_temp(node, url):
+    """Downloads to temporary location from given URL
+
+    :param node: Node for which to download to temporary location
+    :param url: URL to download from
+    :returns: File path of temporary location file is downloaded to
+    """
+    parsed_url = urlparse.urlparse(url)
+    scheme = parsed_url.scheme.lower()
+    if scheme not in ('http', 'swift', 'file'):
+        raise exception.InvalidParameterValue(
+            _('%(scheme)s is not supported for %(url)s.')
+            % {'scheme': scheme, 'url': parsed_url.geturl()})
+
+    tempdir = os.path.join(tempfile.gettempdir(), node.uuid)
+    os.makedirs(tempdir, exist_ok=True)
+    temp_file = os.path.join(
+        tempdir,
+        os.path.basename(parsed_url.path))
+    LOG.debug('For node %(node)s firmware at %(url)s will be downloaded to '
+              'temporary location at %(temp_file)s',
+              {'node': node.uuid, 'url': url, 'temp_file': temp_file})
+    if scheme == 'http':
+        with open(temp_file, 'wb') as tf:
+            image_service.HttpImageService().download(url, tf)
+    elif scheme == 'swift':
+        swift_url = get_swift_temp_url(parsed_url)
+        with open(temp_file, 'wb') as tf:
+            image_service.HttpImageService().download(swift_url, tf)
+    elif scheme == 'file':
+        with open(temp_file, 'wb') as tf:
+            image_service.FileImageService().download(
+                parsed_url.path, tf)
+
+    return temp_file
+
+
+def verify_checksum(node, checksum, file_path):
+    """Verify checksum.
+
+    :param node: Node for which file to verify checksum
+    :param checksum: Expected checksum value
+    :param file_path: File path for which to verify checksum
+    :raises RedfishError: When checksum does not match
+    """
+    calculated_checksum = fileutils.compute_file_checksum(
+        file_path, algorithm='sha1')
+    if checksum != calculated_checksum:
+        raise exception.RedfishError(
+            _('For node %(node)s firmware file %(temp_file)s checksums do not '
+              'match. Expected: %(checksum)s, calculated: '
+              '%(calculated_checksum)s.')
+            % {'node': node.uuid, 'temp_file': file_path, 'checksum': checksum,
+               'calculated_checksum': calculated_checksum})
+
+
+def stage(node, source, temp_file):
+    """Stage temporary file to configured location
+
+    :param node: Node for which to stage the file
+    :param source: Where to stage the file. Corresponds to
+        CONF.redfish.firmware_source.
+    :param temp_file: File path of temporary file to stage
+    :returns: Tuple of staged URL and source (http or swift) that needs
+        cleanup of staged files afterwards.
+    :raises RedfishError: If staging to HTTP server has failed.
+    """
+    staged_url = None
+    filename = os.path.basename(temp_file)
+    if source in ('http', 'local'):
+        http_url = CONF.deploy.external_http_url or CONF.deploy.http_url
+        staged_url = urlparse.urljoin(
+            http_url, "/".join([_FIRMWARE_SUBDIR, node.uuid, filename]))
+        staged_folder = os.path.join(
+            CONF.deploy.http_root, _FIRMWARE_SUBDIR, node.uuid)
+        staged_path = os.path.join(staged_folder, filename)
+        LOG.debug('For node %(node)s temporary file %(temp_file)s will be '
+                  'hard-linked or copied to %(staged_path)s and served over '
+                  '%(staged_url)s',
+                  {'node': node.uuid, 'temp_file': temp_file,
+                   'staged_path': staged_path, 'staged_url': staged_url})
+        os.makedirs(staged_folder, exist_ok=True)
+        try:
+            os.link(temp_file, staged_path)
+            os.chmod(temp_file, CONF.redfish.file_permission)
+        except OSError as oserror:
+            LOG.debug("Could not hardlink file %(temp_file)s to location "
+                      "%(staged_path)s. Will try to copy it. Error: %(error)s",
+                      {'temp_file': temp_file, 'staged_path': staged_path,
+                       'error': oserror})
+            try:
+                shutil.copyfile(temp_file, staged_path)
+                os.chmod(staged_path, CONF.redfish.file_permission)
+            except IOError as ioerror:
+                raise exception.RedfishError(
+                    _('For %(node)s failed to copy firmware file '
+                      '%(temp_file)s to HTTP server root. Error %(error)s')
+                    % {'node': node.uuid, 'temp_file': temp_file,
+                       'error': ioerror})
+
+    elif source == 'swift':
+        container = CONF.redfish.swift_container
+        timeout = CONF.redfish.swift_object_expiry_timeout
+        swift_api = swift.SwiftAPI()
+        object_name = "/".join([node.uuid, filename])
+        swift_api.create_object(
+            container,
+            object_name,
+            temp_file,
+            object_headers={'X-Delete-After': str(timeout)})
+        staged_url = swift_api.get_temp_url(
+            container, object_name, timeout)
+        LOG.debug('For node %(node)s temporary file at %(temp_file)s will be '
+                  'served from Swift temporary URL %(staged_url)s',
+                  {'node': node.uuid, 'temp_file': temp_file,
+                   'staged_url': staged_url})
+
+    need_cleanup = 'swift' if source == 'swift' else 'http'
+    return staged_url, need_cleanup
+
+
+def cleanup(node):
+    """Clean up staged files
+
+    :param node: Node for which to clean up. Should contain
+        'firmware_cleanup' entry in `driver_internal_info` to indicate
+        source(s) to be cleaned up.
+    """
+    # Cleaning up temporary just in case there is something when staging
+    # to http or swift has failed.
+    temp_dir = os.path.join(tempfile.gettempdir(), node.uuid)
+    LOG.debug('For node %(node)s cleaning up temporary files, if any, from '
+              '%(temp_dir)s.', {'node': node.uuid, 'temp_dir': temp_dir})
+    shutil.rmtree(temp_dir, ignore_errors=True)
+
+    cleanup = node.driver_internal_info.get('firmware_cleanup')
+    if not cleanup:
+        return
+
+    if 'http' in cleanup:
+        http_dir = os.path.join(
+            CONF.deploy.http_root, _FIRMWARE_SUBDIR, node.uuid)
+        LOG.debug('For node %(node)s cleaning up files from  %(http_dir)s.',
+                  {'node': node.uuid, 'http_dir': http_dir})
+        shutil.rmtree(http_dir, ignore_errors=True)
+
+    if 'swift' in cleanup:
+        swift_api = swift.SwiftAPI()
+        container = CONF.redfish.swift_container
+        LOG.debug('For node %(node)s cleaning up files from Swift container '
+                  '%(container)s.',
+                  {'node': node.uuid, 'container': container})
+        _, objects = swift_api.connection.get_container(container)
+        for o in objects:
+            name = o.get('name')
+            if name and name.startswith(node.uuid):
+                try:
+                    swift_api.delete_object(container, name)
+                except exception.SwiftOperationError as error:
+                    LOG.warning('For node %(node)s failed to clean up '
+                                '%(object)s. Error: %(error)s',
+                                {'node': node.uuid, 'object': name,
+                                 'error': error})
diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py
index cb56a821b3..a669d09bc8 100644
--- a/ironic/drivers/modules/redfish/management.py
+++ b/ironic/drivers/modules/redfish/management.py
@@ -14,6 +14,7 @@
 #    under the License.
 
 import collections
+from urllib.parse import urlparse
 
 from ironic_lib import metrics_utils
 from oslo_log import log
@@ -799,7 +800,8 @@ class RedfishManagement(base.ManagementInterface):
         """
 
         firmware_update = firmware_updates[0]
-        firmware_url = firmware_update['url']
+        firmware_url, need_cleanup = self._stage_firmware_file(
+            node, firmware_update)
 
         LOG.debug('Applying firmware %(firmware_image)s to node '
                   '%(node_uuid)s',
@@ -809,8 +811,15 @@ class RedfishManagement(base.ManagementInterface):
         task_monitor = update_service.simple_update(firmware_url)
 
         firmware_update['task_monitor'] = task_monitor.task_monitor_uri
-        node.set_driver_internal_info('firmware_updates',
-                                      firmware_updates)
+        node.set_driver_internal_info('firmware_updates', firmware_updates)
+
+        if need_cleanup:
+            fw_cleanup = node.driver_internal_info.get('firmware_cleanup')
+            if not fw_cleanup:
+                fw_cleanup = [need_cleanup]
+            elif need_cleanup not in fw_cleanup:
+                fw_cleanup.append(need_cleanup)
+            node.set_driver_internal_info('firmware_cleanup', fw_cleanup)
 
     def _continue_firmware_updates(self, task, update_service,
                                    firmware_updates):
@@ -860,13 +869,18 @@ class RedfishManagement(base.ManagementInterface):
             manager_utils.node_power_action(task, states.REBOOT)
 
     def _clear_firmware_updates(self, node):
-        """Clears firmware updates from driver_internal_info
+        """Clears firmware updates artifacts
+
+        Clears firmware updates from driver_internal_info and any files
+        that were staged.
 
         Note that the caller must have an exclusive lock on the node.
 
         :param node: the node to clear the firmware updates from
         """
+        firmware_utils.cleanup(node)
         node.del_driver_internal_info('firmware_updates')
+        node.del_driver_internal_info('firmware_cleanup')
         node.save()
 
     @METRICS.timer('RedfishManagement._query_firmware_update_failed')
@@ -1012,6 +1026,56 @@ class RedfishManagement(base.ManagementInterface):
                       {'node': node.uuid,
                        'firmware_image': current_update['url']})
 
+    def _stage_firmware_file(self, node, firmware_update):
+        """Stage firmware update according to configuration.
+
+        :param node: Node for which to stage the firmware file
+        :param firmware_update: Firmware update to stage
+        :returns: Tuple of staged URL and source that needs cleanup of
+            staged files afterwards. If not staging, then return
+            original URL and None for source that needs cleanup.
+        :raises IronicException: If something goes wrong with staging.
+        """
+        try:
+            url = firmware_update['url']
+            parsed_url = urlparse(url)
+            scheme = parsed_url.scheme.lower()
+            source = (firmware_update.get('source')
+                      or CONF.redfish.firmware_source).lower()
+
+            # Keep it simple, in further processing TLS does not matter
+            if scheme == 'https':
+                scheme = 'http'
+
+            # If source and scheme is HTTP, then no staging,
+            # returning original location
+            if scheme == 'http' and source == scheme:
+                LOG.debug('For node %(node)s serving firmware from original '
+                          'location %(url)s', {'node': node.uuid, 'url': url})
+                return url, None
+
+            # If source and scheme is Swift, then not moving, but
+            # returning Swift temp URL
+            if scheme == 'swift' and source == scheme:
+                temp_url = firmware_utils.get_swift_temp_url(parsed_url)
+                LOG.debug('For node %(node)s serving original firmware at '
+                          '%(url)s via Swift temporary url %(temp_url)s',
+                          {'node': node.uuid, 'url': url,
+                           'temp_url': temp_url})
+                return temp_url, None
+
+            # For remaining, download the image to temporary location
+            temp_file = firmware_utils.download_to_temp(node, url)
+
+            firmware_utils.verify_checksum(
+                node, firmware_update.get('checksum'), temp_file)
+
+            return firmware_utils.stage(node, source, temp_file)
+
+        except exception.IronicException as error:
+            firmware_utils.cleanup(node)
+            raise error
+
     def get_secure_boot_state(self, task):
         """Get the current secure boot state for the node.
 
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py
index 60c66c0249..e2c6e75b2d 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_firmware_utils.py
@@ -11,7 +11,18 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import os
+import shutil
+import tempfile
+from unittest import mock
+from urllib.parse import urlparse
+
+from oslo_utils import fileutils
+
 from ironic.common import exception
+from ironic.common import image_service
+from ironic.common import swift
+from ironic.conf import CONF
 from ironic.drivers.modules.redfish import firmware_utils
 from ironic.tests import base
 
@@ -22,10 +33,12 @@ class FirmwareUtilsTestCase(base.TestCase):
         firmware_images = [
             {
                 "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+                "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
                 "wait": 300
             },
             {
-                "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE"
+                "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE",
+                "checksum": "9f6227549221920e312fed2cfc6586ee832cc546"
             }
         ]
         firmware_utils.validate_update_firmware_args(firmware_images)
@@ -33,6 +46,7 @@ class FirmwareUtilsTestCase(base.TestCase):
     def test_validate_update_firmware_args_not_list(self):
         firmware_images = {
             "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+            "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
             "wait": 300
         }
         self.assertRaisesRegex(
@@ -43,10 +57,12 @@ class FirmwareUtilsTestCase(base.TestCase):
         firmware_images = [
             {
                 "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+                "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
                 "wait": 300,
             },
             {
                 "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE",
+                "checksum": "9f6227549221920e312fed2cfc6586ee832cc546",
                 "something": "unknown"
             }
         ]
@@ -58,9 +74,11 @@ class FirmwareUtilsTestCase(base.TestCase):
         firmware_images = [
             {
                 "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+                "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
                 "wait": 300,
             },
             {
+                "checksum": "9f6227549221920e312fed2cfc6586ee832cc546",
                 "wait": 300
             }
         ]
@@ -72,6 +90,34 @@ class FirmwareUtilsTestCase(base.TestCase):
     def test_validate_update_firmware_args_url_not_string(self):
         firmware_images = [{
             "url": 123,
+            "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
+            "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_checksum_missing(self):
+        firmware_images = [
+            {
+                "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+                "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
+                "wait": 300,
+            },
+            {
+                "url": "https://192.0.2.10/NIC_19.0.12_A00.EXE",
+                "wait": 300
+            }
+        ]
+        self.assertRaisesRegex(
+            exception.InvalidParameterValue,
+            "'checksum' is a required property",
+            firmware_utils.validate_update_firmware_args, firmware_images)
+
+    def test_validate_update_firmware_args_checksum_not_string(self):
+        firmware_images = [{
+            "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+            "checksum": 123,
             "wait": 300
         }]
         self.assertRaisesRegex(
@@ -81,8 +127,335 @@ class FirmwareUtilsTestCase(base.TestCase):
     def test_validate_update_firmware_args_wait_not_int(self):
         firmware_images = [{
             "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+            "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
             "wait": 'abc'
         }]
         self.assertRaisesRegex(
             exception.InvalidParameterValue, "'abc' is not of type 'integer'",
             firmware_utils.validate_update_firmware_args, firmware_images)
+
+    def test_validate_update_firmware_args_source_not_known(self):
+        firmware_images = [{
+            "url": "http://192.0.2.10/BMC_4_22_00_00.EXE",
+            "checksum": "aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d",
+            "source": "abc"
+        }]
+        self.assertRaisesRegex(
+            exception.InvalidParameterValue, "'abc' is not one of",
+            firmware_utils.validate_update_firmware_args, firmware_images)
+
+    @mock.patch.object(swift, 'SwiftAPI', autospec=True)
+    def test_get_swift_temp_url(self, mock_swift_api):
+        mock_swift_api.return_value.get_temp_url.return_value = 'http://temp'
+        parsed_url = urlparse("swift://firmware/sub/bios.exe")
+
+        result = firmware_utils.get_swift_temp_url(parsed_url)
+
+        self.assertEqual(result, 'http://temp')
+        mock_swift_api.return_value.get_temp_url.assert_called_with(
+            'firmware', 'sub/bios.exe',
+            CONF.redfish.swift_object_expiry_timeout)
+
+    @mock.patch.object(tempfile, 'gettempdir', autospec=True)
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(image_service, 'HttpImageService', autospec=True)
+    def test_download_to_temp_http(
+            self, mock_http_image_service, mock_makedirs, mock_gettempdir):
+        node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435')
+        mock_gettempdir.return_value = '/tmp'
+        http_url = 'http://example.com/bios.exe'
+
+        with mock.patch.object(firmware_utils, 'open', mock.mock_open(),
+                               create=True) as mock_open:
+            result = firmware_utils.download_to_temp(node, http_url)
+
+            exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe'
+            exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435'
+            mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True)
+            self.assertEqual(result, exp_result)
+            mock_http_image_service.return_value.download.assert_called_with(
+                http_url, mock_open.return_value)
+            mock_open.assert_has_calls([mock.call(exp_result, 'wb')])
+
+    @mock.patch.object(tempfile, 'gettempdir', autospec=True)
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(image_service, 'HttpImageService', autospec=True)
+    @mock.patch.object(swift, 'SwiftAPI', autospec=True)
+    def test_download_to_temp_swift(
+            self, mock_swift_api, mock_http_image_service, mock_makedirs,
+            mock_gettempdir):
+        node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435')
+        mock_gettempdir.return_value = '/tmp'
+        swift_url = 'swift://firmware/sub/bios.exe'
+        temp_swift_url = 'http://swift_temp'
+        mock_swift_api.return_value.get_temp_url.return_value = temp_swift_url
+
+        with mock.patch.object(firmware_utils, 'open', mock.mock_open(),
+                               create=True) as mock_open:
+            result = firmware_utils.download_to_temp(node, swift_url)
+
+            exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe'
+            exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435'
+            mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True)
+            self.assertEqual(result, exp_result)
+            mock_http_image_service.return_value.download.assert_called_with(
+                temp_swift_url, mock_open.return_value)
+            mock_open.assert_has_calls([mock.call(exp_result, 'wb')])
+
+    @mock.patch.object(tempfile, 'gettempdir', autospec=True)
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(image_service, 'FileImageService', autospec=True)
+    def test_download_to_temp_file(
+            self, mock_file_image_service, mock_makedirs,
+            mock_gettempdir):
+        node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435')
+        mock_gettempdir.return_value = '/tmp'
+        file_url = 'file:///firmware/bios.exe'
+
+        with mock.patch.object(firmware_utils, 'open', mock.mock_open(),
+                               create=True) as mock_open:
+            result = firmware_utils.download_to_temp(node, file_url)
+
+            exp_result = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435/bios.exe'
+            exp_temp_dir = '/tmp/9f0f6795-f74e-4b5a-850e-72f586a92435'
+            mock_makedirs.assert_called_with(exp_temp_dir, exist_ok=True)
+            self.assertEqual(result, exp_result)
+            mock_file_image_service.return_value.download.assert_called_with(
+                '/firmware/bios.exe', mock_open.return_value)
+            mock_open.assert_has_calls([mock.call(exp_result, 'wb')])
+
+    def test_download_to_temp_invalid(self):
+        node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435')
+        self.assertRaises(
+            exception.InvalidParameterValue,
+            firmware_utils.download_to_temp, node, 'ftp://firmware/bios.exe')
+
+    @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True)
+    def test_verify_checksum(self, mock_compute_file_checksum):
+        checksum = 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'
+        file_path = '/tmp/bios.exe'
+        mock_compute_file_checksum.return_value = checksum
+        node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435')
+
+        firmware_utils.verify_checksum(node, checksum, file_path)
+
+        mock_compute_file_checksum.assert_called_with(
+            file_path, algorithm='sha1')
+
+    @mock.patch.object(fileutils, 'compute_file_checksum', autospec=True)
+    def test_verify_checksum_mismatch(self, mock_compute_file_checksum):
+        checksum1 = 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'
+        checksum2 = '9f6227549221920e312fed2cfc6586ee832cc546'
+        file_path = '/tmp/bios.exe'
+        mock_compute_file_checksum.return_value = checksum1
+        node = mock.Mock(uuid='9f0f6795-f74e-4b5a-850e-72f586a92435')
+
+        self.assertRaises(
+            exception.RedfishError, firmware_utils.verify_checksum, node,
+            checksum2, file_path)
+        mock_compute_file_checksum.assert_called_with(
+            file_path, algorithm='sha1')
+
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(shutil, 'copyfile', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    def test_stage_http(self, mock_chmod, mock_link, mock_copyfile,
+                        mock_makedirs):
+        CONF.deploy.http_url = 'http://10.0.0.2'
+        CONF.deploy.external_http_url = None
+        CONF.deploy.http_root = '/httproot'
+        node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3')
+
+        staged_url, need_cleanup = firmware_utils.stage(
+            node, 'http', '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+
+        self.assertEqual(staged_url,
+                         'http://10.0.0.2/firmware/'
+                         '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        self.assertEqual(need_cleanup, 'http')
+        mock_makedirs.assert_called_with(
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            exist_ok=True)
+        mock_link.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        mock_chmod.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            CONF.redfish.file_permission)
+        mock_copyfile.assert_not_called()
+
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(shutil, 'copyfile', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    def test_stage_http_copyfile(self, mock_chmod, mock_link, mock_copyfile,
+                                 mock_makedirs):
+        CONF.deploy.http_url = 'http://10.0.0.2'
+        CONF.deploy.external_http_url = None
+        CONF.deploy.http_root = '/httproot'
+        node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3')
+        mock_link.side_effect = OSError
+
+        staged_url, need_cleanup = firmware_utils.stage(
+            node, 'http', '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+
+        self.assertEqual(staged_url,
+                         'http://10.0.0.2/firmware/'
+                         '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        self.assertEqual(need_cleanup, 'http')
+        mock_makedirs.assert_called_with(
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            exist_ok=True)
+        mock_link.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        mock_copyfile.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        mock_chmod.assert_called_with(
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            CONF.redfish.file_permission)
+
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(shutil, 'copyfile', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    def test_stage_http_copyfile_fails(self, mock_chmod, mock_link,
+                                       mock_copyfile, mock_makedirs):
+        CONF.deploy.http_url = 'http://10.0.0.2'
+        CONF.deploy.external_http_url = None
+        CONF.deploy.http_root = '/httproot'
+        node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3')
+        mock_link.side_effect = OSError
+        mock_copyfile.side_effect = IOError
+
+        self.assertRaises(exception.RedfishError, firmware_utils.stage,
+                          node, 'http',
+                          '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+
+        mock_makedirs.assert_called_with(
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            exist_ok=True)
+        mock_link.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        mock_copyfile.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        mock_chmod.assert_not_called()
+
+    @mock.patch.object(os, 'makedirs', autospec=True)
+    @mock.patch.object(shutil, 'copyfile', autospec=True)
+    @mock.patch.object(shutil, 'rmtree', autospec=True)
+    @mock.patch.object(os, 'link', autospec=True)
+    @mock.patch.object(os, 'chmod', autospec=True)
+    def test_stage_local_external(self, mock_chmod, mock_link, mock_rmtree,
+                                  mock_copyfile, mock_makedirs):
+        CONF.deploy.http_url = 'http://10.0.0.2'
+        CONF.deploy.external_http_url = 'http://90.0.0.9'
+        CONF.deploy.http_root = '/httproot'
+        node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3')
+
+        staged_url, need_cleanup = firmware_utils.stage(
+            node, 'local',
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+
+        self.assertEqual(staged_url,
+                         'http://90.0.0.9/firmware/'
+                         '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        self.assertEqual(need_cleanup, 'http')
+        mock_makedirs.assert_called_with(
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            exist_ok=True)
+        mock_link.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe')
+        mock_chmod.assert_called_with(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe',
+            CONF.redfish.file_permission)
+        mock_copyfile.assert_not_called()
+
+    @mock.patch.object(swift, 'SwiftAPI', autospec=True)
+    def test_stage_swift(self, mock_swift_api):
+        node = mock.Mock(uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3')
+        mock_swift_api.return_value.get_temp_url.return_value = 'http://temp'
+        temp_file = '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe'
+
+        staged_url, need_cleanup = firmware_utils.stage(
+            node, 'swift', temp_file)
+
+        self.assertEqual(staged_url, 'http://temp')
+        self.assertEqual(need_cleanup, 'swift')
+        exp_object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe'
+        mock_swift_api.return_value.create_object.assert_called_with(
+            CONF.redfish.swift_container,
+            exp_object_name, temp_file,
+            object_headers={'X-Delete-After':
+                            str(CONF.redfish.swift_object_expiry_timeout)})
+        mock_swift_api.return_value.get_temp_url.assert_called_with(
+            CONF.redfish.swift_container, exp_object_name,
+            CONF.redfish.swift_object_expiry_timeout)
+
+    @mock.patch.object(shutil, 'rmtree', autospec=True)
+    @mock.patch.object(tempfile, 'gettempdir', autospec=True)
+    @mock.patch.object(swift, 'SwiftAPI', autospec=True)
+    def test_cleanup(self, mock_swift_api, mock_gettempdir, mock_rmtree):
+        mock_gettempdir.return_value = '/tmp'
+        CONF.deploy.http_root = '/httproot'
+        node = mock.Mock(
+            uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            driver_internal_info={'firmware_cleanup': ['http', 'swift']})
+        object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe'
+        get_container = mock_swift_api.return_value.connection.get_container
+        get_container.return_value = (mock.Mock(), [{'name': object_name}])
+
+        firmware_utils.cleanup(node)
+
+        mock_rmtree.assert_any_call(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            ignore_errors=True)
+        mock_rmtree.assert_any_call(
+            '/httproot/firmware/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            ignore_errors=True)
+        mock_swift_api.return_value.delete_object.assert_called_with(
+            CONF.redfish.swift_container, object_name)
+
+    @mock.patch.object(shutil, 'rmtree', autospec=True)
+    @mock.patch.object(tempfile, 'gettempdir', autospec=True)
+    def test_cleanup_notstaged(self, mock_gettempdir, mock_rmtree):
+        mock_gettempdir.return_value = '/tmp'
+        node = mock.Mock(
+            uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            driver_internal_info={'something': 'else'})
+
+        firmware_utils.cleanup(node)
+
+        mock_rmtree.assert_any_call(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            ignore_errors=True)
+
+    @mock.patch.object(shutil, 'rmtree', autospec=True)
+    @mock.patch.object(tempfile, 'gettempdir', autospec=True)
+    @mock.patch.object(swift, 'SwiftAPI', autospec=True)
+    @mock.patch.object(firmware_utils.LOG, 'warning', autospec=True)
+    def test_cleanup_swift_fails(self, mock_warning, mock_swift_api,
+                                 mock_gettempdir, mock_rmtree):
+        mock_gettempdir.return_value = '/tmp'
+        node = mock.Mock(
+            uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            driver_internal_info={'firmware_cleanup': ['swift']})
+        object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe'
+        get_container = mock_swift_api.return_value.connection.get_container
+        get_container.return_value = (mock.Mock(), [{'name': object_name}])
+        mock_swift_api.return_value.delete_object.side_effect =\
+            exception.SwiftOperationError
+
+        firmware_utils.cleanup(node)
+
+        mock_rmtree.assert_any_call(
+            '/tmp/55cdaba0-1123-4622-8b37-bb52dd6285d3',
+            ignore_errors=True)
+        mock_swift_api.return_value.delete_object.assert_called_with(
+            CONF.redfish.swift_container, object_name)
+        mock_warning.assert_called_once()
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py
index b467006648..93aae5de84 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_management.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py
@@ -27,8 +27,10 @@ from ironic.common import indicator_states
 from ironic.common import states
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
+from ironic.conf import CONF
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.redfish import boot as redfish_boot
+from ironic.drivers.modules.redfish import firmware_utils
 from ironic.drivers.modules.redfish import management as redfish_mgmt
 from ironic.drivers.modules.redfish import utils as redfish_utils
 from ironic.tests.unit.db import base as db_base
@@ -834,22 +836,145 @@ class RedfishManagementTestCase(db_base.DbTestCase):
         mock_update_service = mock.Mock()
         mock_update_service.simple_update.return_value = mock_task_monitor
         mock_get_update_service.return_value = mock_update_service
+        CONF.redfish.firmware_source = 'http'
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.node.save = mock.Mock()
 
-            task.driver.management.update_firmware(task,
-                                                   [{'url': 'test1'},
-                                                    {'url': 'test2'}])
+            task.driver.management.update_firmware(
+                task,
+                [{'url': 'http://test1',
+                  'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'},
+                 {'url': 'http://test2',
+                  'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}])
 
             mock_get_update_service.assert_called_once_with(task.node)
-            mock_update_service.simple_update.assert_called_once_with('test1')
+            mock_update_service.simple_update.assert_called_once_with(
+                'http://test1')
             self.assertIsNotNone(task.node
                                  .driver_internal_info['firmware_updates'])
             self.assertEqual(
-                [{'task_monitor': '/task/123', 'url': 'test1'},
-                 {'url': 'test2'}],
+                [{'task_monitor': '/task/123', 'url': 'http://test1',
+                  'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'},
+                 {'url': 'http://test2',
+                  'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}],
                 task.node.driver_internal_info['firmware_updates'])
+            self.assertIsNone(
+                task.node.driver_internal_info.get('firmware_cleanup'))
+            mock_set_async_step_flags.assert_called_once_with(
+                task.node, reboot=True, skip_current_step=True, polling=True)
+            mock_get_async_step_return_state.assert_called_once_with(
+                task.node)
+            mock_node_power_action.assert_called_once_with(task, states.REBOOT)
+
+    @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file',
+                       autospec=True)
+    @mock.patch.object(deploy_utils, 'build_agent_options',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
+    @mock.patch.object(deploy_utils, 'get_async_step_return_state',
+                       autospec=True)
+    @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True)
+    @mock.patch.object(redfish_utils, 'get_update_service', autospec=True)
+    def test_update_firmware_stage(
+            self, mock_get_update_service, mock_set_async_step_flags,
+            mock_get_async_step_return_state, mock_node_power_action,
+            mock_prepare, build_mock, mock_stage):
+        build_mock.return_value = {'a': 'b'}
+        mock_task_monitor = mock.Mock()
+        mock_task_monitor.task_monitor_uri = '/task/123'
+        mock_update_service = mock.Mock()
+        mock_update_service.simple_update.return_value = mock_task_monitor
+        mock_get_update_service.return_value = mock_update_service
+        mock_stage.return_value = ('http://staged/test1', 'http')
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.save = mock.Mock()
+
+            task.driver.management.update_firmware(
+                task,
+                [{'url': 'http://test1',
+                  'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'},
+                 {'url': 'http://test2',
+                  'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}])
+
+            mock_get_update_service.assert_called_once_with(task.node)
+            mock_update_service.simple_update.assert_called_once_with(
+                'http://staged/test1')
+            self.assertIsNotNone(task.node
+                                 .driver_internal_info['firmware_updates'])
+            self.assertEqual(
+                [{'task_monitor': '/task/123', 'url': 'http://test1',
+                  'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'},
+                 {'url': 'http://test2',
+                  'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}],
+                task.node.driver_internal_info['firmware_updates'])
+            self.assertIsNotNone(
+                task.node.driver_internal_info['firmware_cleanup'])
+            self.assertEqual(
+                ['http'], task.node.driver_internal_info['firmware_cleanup'])
+            mock_set_async_step_flags.assert_called_once_with(
+                task.node, reboot=True, skip_current_step=True, polling=True)
+            mock_get_async_step_return_state.assert_called_once_with(
+                task.node)
+            mock_node_power_action.assert_called_once_with(task, states.REBOOT)
+
+    @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file',
+                       autospec=True)
+    @mock.patch.object(deploy_utils, 'build_agent_options',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, 'prepare_ramdisk',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
+    @mock.patch.object(deploy_utils, 'get_async_step_return_state',
+                       autospec=True)
+    @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True)
+    @mock.patch.object(redfish_utils, 'get_update_service', autospec=True)
+    def test_update_firmware_stage_both(
+            self, mock_get_update_service, mock_set_async_step_flags,
+            mock_get_async_step_return_state, mock_node_power_action,
+            mock_prepare, build_mock, mock_stage):
+        build_mock.return_value = {'a': 'b'}
+        mock_task_monitor = mock.Mock()
+        mock_task_monitor.task_monitor_uri = '/task/123'
+        mock_update_service = mock.Mock()
+        mock_update_service.simple_update.return_value = mock_task_monitor
+        mock_get_update_service.return_value = mock_update_service
+        mock_stage.return_value = ('http://staged/test1', 'http')
+        info = self.node.driver_internal_info
+        info['firmware_cleanup'] = ['swift']
+        self.node.driver_internal_info = info
+        self.node.save()
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.save = mock.Mock()
+
+            task.driver.management.update_firmware(
+                task,
+                [{'url': 'http://test1',
+                  'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'},
+                 {'url': 'http://test2',
+                  'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}])
+
+            mock_get_update_service.assert_called_once_with(task.node)
+            mock_update_service.simple_update.assert_called_once_with(
+                'http://staged/test1')
+            self.assertIsNotNone(task.node
+                                 .driver_internal_info['firmware_updates'])
+            self.assertEqual(
+                [{'task_monitor': '/task/123', 'url': 'http://test1',
+                  'checksum': 'aaf4c61ddcc5e8a2dabede0f3b482cd9aea9434d'},
+                 {'url': 'http://test2',
+                  'checksum': '9f6227549221920e312fed2cfc6586ee832cc546'}],
+                task.node.driver_internal_info['firmware_updates'])
+            self.assertIsNotNone(
+                task.node.driver_internal_info['firmware_cleanup'])
+            self.assertEqual(
+                ['swift', 'http'],
+                task.node.driver_internal_info['firmware_cleanup'])
             mock_set_async_step_flags.assert_called_once_with(
                 task.node, reboot=True, skip_current_step=True, polling=True)
             mock_get_async_step_return_state.assert_called_once_with(
@@ -1218,9 +1343,10 @@ class RedfishManagementTestCase(db_base.DbTestCase):
         driver_internal_info = {
             'something': 'else',
             'firmware_updates': [
-                {'task_monitor': '/task/123', 'url': 'test1'},
-                {'url': 'test2'}]}
+                {'task_monitor': '/task/123', 'url': 'http://test1'},
+                {'url': 'http://test2'}]}
         self.node.driver_internal_info = driver_internal_info
+        CONF.redfish.firmware_source = 'http'
 
         management = redfish_mgmt.RedfishManagement()
         with task_manager.acquire(self.context, self.node.uuid,
@@ -1230,19 +1356,88 @@ class RedfishManagementTestCase(db_base.DbTestCase):
             management._continue_firmware_updates(
                 task,
                 mock_update_service,
-                [{'task_monitor': '/task/123', 'url': 'test1'},
-                 {'url': 'test2'}])
+                [{'task_monitor': '/task/123', 'url': 'http://test1'},
+                 {'url': 'http://test2'}])
 
             self.assertTrue(mock_log.called)
-            mock_update_service.simple_update.assert_called_once_with('test2')
+            mock_update_service.simple_update.assert_called_once_with(
+                'http://test2')
             self.assertIsNotNone(
                 task.node.driver_internal_info['firmware_updates'])
             self.assertEqual(
-                [{'url': 'test2', 'task_monitor': '/task/987'}],
+                [{'url': 'http://test2', 'task_monitor': '/task/987'}],
                 task.node.driver_internal_info['firmware_updates'])
             task.node.save.assert_called_once_with()
             mock_node_power_action.assert_called_once_with(task, states.REBOOT)
 
+    @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True)
+    @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True)
+    @mock.patch.object(firmware_utils, 'stage', autospec=True)
+    def test__stage_firmware_file_https(self, mock_stage, mock_verify_checksum,
+                                        mock_download_to_temp):
+        CONF.redfish.firmware_source = 'local'
+        firmware_update = {'url': 'https://test1', 'checksum': 'abc'}
+        node = mock.Mock()
+        mock_download_to_temp.return_value = '/tmp/test1'
+        mock_stage.return_value = ('http://staged/test1', 'http')
+
+        management = redfish_mgmt.RedfishManagement()
+
+        staged_url, needs_cleanup = management._stage_firmware_file(
+            node, firmware_update)
+
+        self.assertEqual(staged_url, 'http://staged/test1')
+        self.assertEqual(needs_cleanup, 'http')
+        mock_download_to_temp.assert_called_with(node, 'https://test1')
+        mock_verify_checksum.assert_called_with(node, 'abc', '/tmp/test1')
+        mock_stage.assert_called_with(node, 'local', '/tmp/test1')
+
+    @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True)
+    @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True)
+    @mock.patch.object(firmware_utils, 'stage', autospec=True)
+    @mock.patch.object(firmware_utils, 'get_swift_temp_url', autospec=True)
+    def test__stage_firmware_file_swift(
+            self, mock_get_swift_temp_url, mock_stage, mock_verify_checksum,
+            mock_download_to_temp):
+        CONF.redfish.firmware_source = 'swift'
+        firmware_update = {'url': 'swift://container/bios.exe'}
+        node = mock.Mock()
+        mock_get_swift_temp_url.return_value = 'http://temp'
+
+        management = redfish_mgmt.RedfishManagement()
+
+        staged_url, needs_cleanup = management._stage_firmware_file(
+            node, firmware_update)
+
+        self.assertEqual(staged_url, 'http://temp')
+        self.assertIsNone(needs_cleanup)
+        mock_download_to_temp.assert_not_called()
+        mock_verify_checksum.assert_not_called()
+        mock_stage.assert_not_called()
+
+    @mock.patch.object(firmware_utils, 'cleanup', autospec=True)
+    @mock.patch.object(firmware_utils, 'download_to_temp', autospec=True)
+    @mock.patch.object(firmware_utils, 'verify_checksum', autospec=True)
+    @mock.patch.object(firmware_utils, 'stage', autospec=True)
+    def test__stage_firmware_file_error(self, mock_stage, mock_verify_checksum,
+                                        mock_download_to_temp, mock_cleanup):
+        node = mock.Mock()
+        firmware_update = {'url': 'https://test1'}
+        CONF.redfish.firmware_source = 'local'
+        firmware_update = {'url': 'https://test1'}
+        node = mock.Mock()
+        mock_download_to_temp.return_value = '/tmp/test1'
+        mock_stage.side_effect = exception.IronicException
+
+        management = redfish_mgmt.RedfishManagement()
+        self.assertRaises(exception.IronicException,
+                          management._stage_firmware_file, node,
+                          firmware_update)
+        mock_download_to_temp.assert_called_with(node, 'https://test1')
+        mock_verify_checksum.assert_called_with(node, None, '/tmp/test1')
+        mock_stage.assert_called_with(node, 'local', '/tmp/test1')
+        mock_cleanup.assert_called_with(node)
+
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_get_secure_boot_state(self, mock_get_system):
         fake_system = mock_get_system.return_value
diff --git a/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml b/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml
new file mode 100644
index 0000000000..559ae22710
--- /dev/null
+++ b/releasenotes/notes/add-more-sources-redfish-firmware-update-3da89f10dc0f8d21.yaml
@@ -0,0 +1,14 @@
+---
+features:
+  - |
+    For ``redfish`` and ``idrac-redfish`` management interface
+    ``firmware_update`` clean step adds Swift, HTTP service and file system
+    support to serve and Ironic's HTTP and Swift service to stage files. Also
+    adds mandatory parameter ``checksum`` for file checksum verification.
+
+upgrade:
+  - |
+    For ``redfish`` and ``idrac-redfish`` management interface
+    ``firmware_update`` clean step there is now mandatory ``checksum``
+    parameter necessary. Update existing clean steps to include it, otherwise
+    clean step will fail with error "'checksum' is a required property".