Add support to manage certificates in iLO

Adds methods to export and clear certificates on the node during node
provisioning. Also enables to implement clean/deploy steps for certificate
management through these APIs.

Change-Id: Idc86d7b78854e0618f62c7e4c1a88670dc4561b0
Story: #2008297
Task: #41182

ironic/drivers/modules/ilo/boot.py

@@ -21,6 +21,7 @@ from ironic_lib import metrics_utils
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
+from oslo_utils import strutils
 
 from ironic.common import boot_devices
 from ironic.common import exception
@@ -72,7 +73,10 @@ OPTIONAL_PROPERTIES = {
                         "image containing EFI boot loader. This image will "
                         "be used by ironic when building UEFI-bootable ISO "
                         "out of kernel and ramdisk. Required for UEFI "
-                        "boot from partition images.")
+                        "boot from partition images."),
+    'ilo_add_certificates': _("Boolean value that indicates whether the "
+                              "certificates require to be added to the "
+                              "iLO.")
 }
 COMMON_PROPERTIES = REQUIRED_PROPERTIES
 
@@ -124,6 +128,7 @@ def parse_driver_info(node, mode='deploy'):
     d_info.update(
         {k: info.get(k, getattr(CONF.conductor, k.replace('ilo_', ''), None))
          for k in OPTIONAL_PROPERTIES})
+    d_info.pop('ilo_add_certificates', None)
 
     return d_info
 
@@ -896,6 +901,8 @@ class IloUefiHttpsBoot(base.BootInterface):
         :returns: A dict with the driver_info values.
         :raises: MissingParameterValue, if any of the required parameters are
             missing.
+        :raises: InvalidParameterValue, if any of the required parameters are
+            invalid.
         """
         info = node.driver_info
 
@@ -919,6 +926,20 @@ class IloUefiHttpsBoot(base.BootInterface):
                                     k.replace('ilo_', ''), None))
              for k in OPTIONAL_PROPERTIES})
 
+        should_add_certs = deploy_info.pop('ilo_add_certificates', True)
+
+        if should_add_certs is not None:
+            try:
+                should_add_certs = strutils.bool_from_string(should_add_certs,
+                                                             strict=True)
+            except ValueError:
+                raise exception.InvalidParameterValue(
+                    _('Invalid value type set in driver_info/'
+                      'ilo_add_certificates on node %(node)s. '
+                      'The value should be a Boolean '
+                      ' not "%(value)s"'
+                      ) % {'value': should_add_certs, 'node': node.uuid})
+
         self._validate_hrefs(deploy_info)
 
         error_msg = (_("Error validating %s for iLO UEFI HTTPS boot. Some "
@@ -1095,6 +1116,7 @@ class IloUefiHttpsBoot(base.BootInterface):
                   "%(node)s to boot from URL %(iso_ref)s.",
                   {'node': node.uuid, 'iso_ref': iso_ref})
 
+        ilo_common.add_certificates(task)
         ilo_common.setup_uefi_https(task, iso_ref)
 
     @METRICS.timer('IloUefiHttpsBoot.clean_up_ramdisk')

ironic/drivers/modules/ilo/common.py

@@ -545,6 +545,7 @@ def set_boot_mode(node, boot_mode):
     except ilo_error.IloCommandNotSupportedError:
         p_boot_mode = DEFAULT_BOOT_MODE
 
+    if p_boot_mode:
         if BOOT_MODE_ILO_TO_GENERIC[p_boot_mode.lower()] == boot_mode:
             LOG.info("Node %(uuid)s pending boot mode is %(boot_mode)s.",
                      {'uuid': node.uuid, 'boot_mode': boot_mode})
@@ -924,6 +925,147 @@ def get_server_post_state(node):
                                           error=ilo_exception)
 
 
+def _get_certificate_file_list(cert_file_list):
+    """Get the list of certificates to use.
+
+    :param cert_file_list: certificates file list.
+    :returns: cert_file_list if it's not empty. If empty or None,
+        returns the list of path configured for "webserver_verify_ca"
+        configuration option if the path exists. If the path does not
+        exist, returns empty list.
+    :raises: InvalidParameterValue if argument provided is other than
+        a list.
+    """
+
+    cfl = cert_file_list
+
+    if not cfl:
+        try:
+            verify = strutils.bool_from_string(CONF.webserver_verify_ca,
+                                               strict=True)
+        except ValueError:
+            verify = CONF.webserver_verify_ca
+
+        if isinstance(verify, bool):
+            return []
+
+        if not os.path.exists(verify):
+            LOG.error("Path to the certificate file %(path)s "
+                      "does not exist.", {'path': verify})
+            return []
+
+        cfl = [verify]
+
+    if not isinstance(cfl, list):
+        raise exception.InvalidParameterValue(_(
+            'List of files is expected whereas "%(atype)s" type '
+            'is provided.') % {'atype': type(cfl)})
+
+    return cfl
+
+
+def add_certificates(task, cert_file_list=None):
+    """Adds certificates to the node.
+
+    Adds certificates to the node based on the driver info
+    provided.
+
+    :param task: a TaskManager instance containing the node to act on.
+    :param cert_file_list: List of certificates to be added to the node.
+        If None, certificates from path configured in 'webserver_verify_ca'
+        will be added to the node.
+    :raises: IloOperationError on an error from IloClient library.
+    :raises: IloOperationNotSupported if retrieving post state is not
+        supported on the server.
+    :raises: InvalidParameterValue, if any of the required parameters are
+            invalid.
+    """
+
+    node = task.node
+    ilo_object = get_ilo_object(node)
+    d_info = node.driver_info
+
+    export_certs = d_info.get('ilo_add_certificates', True)
+
+    if export_certs is None:
+        export_certs = True
+    else:
+        try:
+            export_certs = strutils.bool_from_string(export_certs,
+                                                     strict=True)
+        except ValueError:
+            raise exception.InvalidParameterValue(
+                _('Invalid value type set in driver_info/'
+                  'ilo_add_certificates on node %(node)s. '
+                  'The value should be a Boolean '
+                  ' not "%(value)s"'
+                  ) % {'value': export_certs, 'node': node.uuid})
+
+    if not export_certs:
+        LOG.info("Adding of certificates to the node %(node)s is not "
+                 "requested. Assuming required certificates are available "
+                 "on the node.", {'node': node.uuid})
+        return
+
+    cfl = _get_certificate_file_list(cert_file_list)
+
+    if not cfl:
+        LOG.debug("Not adding any certificate to the node %(node)s "
+                  "as no certificates are provided", {'node': node.uuid})
+        return
+
+    try:
+        # NOTE(vmud213): Add the certificates to the node which are
+        # eventually being used for TLS verification by the node before
+        # downloading the deploy/instance images during HTTPS boot from
+        # URL.
+        operation = (_("Add certificates to %(node)s from paths "
+                       "%(cpath)s.") % {'cpath': cfl, 'node': node.uuid})
+
+        ilo_object.add_tls_certificate(cfl)
+
+        LOG.info("Successfully added certificates to the node %(node)s from "
+                 "paths %(cpath)s.", {'cpath': cfl, 'node': node.uuid})
+    except ilo_error.IloCommandNotSupportedInBiosError as ilo_exception:
+        raise exception.IloOperationNotSupported(operation=operation,
+                                                 error=ilo_exception)
+    except ilo_error.IloError as ilo_exception:
+        raise exception.IloOperationError(operation=operation,
+                                          error=ilo_exception)
+
+
+def clear_certificates(task, cert_file_list=None):
+    """Clears any certificates added to the node.
+
+    Clears the certificates added to the node as part of any Ironic
+    operation
+
+    :param task: a TaskManager instance containing the node to act on.
+    :param cert_file_list: List of certificates to be removed from node.
+        If None, all the certificates present on the node will be removed.
+    :raises: IloOperationError on an error from IloClient library.
+    :raises: IloOperationNotSupported if retrieving post state is not
+        supported on the server.
+    """
+
+    node = task.node
+    operation = (_("Clearing certificates from node %(node)s.") %
+                 {'node': node.uuid})
+
+    try:
+        ilo_object = get_ilo_object(node)
+        ilo_object.remove_tls_certificate(cert_file_list)
+    except ilo_error.IloCommandNotSupportedInBiosError as ilo_exception:
+        raise exception.IloOperationNotSupported(operation=operation,
+                                                 error=ilo_exception)
+    except ilo_error.IloError as ilo_exception:
+        raise exception.IloOperationError(operation=operation,
+                                          error=ilo_exception)
+    LOG.info("Cleared TLS certificates from the node %(node)s "
+             "successfully from paths %(cpath)s.",
+             {'node': node.uuid, 'cpath': cert_file_list})
+
+
 def setup_uefi_https(task, iso, persistent=False):
     """Sets up system to boot from UEFIHTTP boot device.
 

ironic/tests/unit/drivers/modules/ilo/test_boot.py

@@ -1557,6 +1557,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
         driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk'
         driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk'
         driver_info['ilo_bootloader'] = 'bootloader'
+        driver_info['ilo_add_certificates'] = True
         driver_info['dummy_key'] = 'dummy-value'
         self.node.driver_info = driver_info
         self.node.save()
@@ -1568,15 +1569,15 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
                 'ilo_deploy_ramdisk': 'deploy-ramdisk',
                 'ilo_bootloader': 'bootloader'
             }
-            actual_info = deploy_info
-            actual_info.update({'ilo_username': 'admin',
+
+            deploy_info.update({'ilo_username': 'admin',
                                 'ilo_password': 'admin'})
 
             expected_info = task.driver.boot._parse_driver_info(task.node)
             validate_href_mock.assert_called_once_with(mock.ANY, deploy_info)
             check_missing_mock.assert_called_once_with(deploy_info, mock.ANY)
             parse_driver_mock.assert_called_once_with(task.node)
-            self.assertEqual(actual_info, expected_info)
+            self.assertEqual(deploy_info, expected_info)
 
     @mock.patch.object(ilo_boot.IloUefiHttpsBoot, '_validate_hrefs',
                        autospec=True)
@@ -1596,6 +1597,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
         driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk'
         driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk'
         driver_info['ilo_bootloader'] = 'bootloader'
+        driver_info['ilo_add_certificates'] = 'false'
         driver_info['dummy_key'] = 'dummy-value'
         self.node.driver_info = driver_info
         self.node.save()
@@ -1607,8 +1609,8 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
                 'ilo_rescue_ramdisk': 'rescue-ramdisk',
                 'ilo_bootloader': 'bootloader'
             }
-            actual_info = deploy_info
-            actual_info.update({'ilo_username': 'admin',
+
+            deploy_info.update({'ilo_username': 'admin',
                                 'ilo_password': 'admin'})
 
             expected_info = task.driver.boot._parse_driver_info(
@@ -1616,7 +1618,47 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
             check_missing_mock.assert_called_once_with(deploy_info, mock.ANY)
             validate_href_mock.assert_called_once_with(mock.ANY, deploy_info)
             parse_driver_mock.assert_called_once_with(task.node)
-            self.assertEqual(actual_info, expected_info)
+            self.assertEqual(deploy_info, expected_info)
+
+    @mock.patch.object(ilo_boot.IloUefiHttpsBoot, '_validate_hrefs',
+                       autospec=True)
+    @mock.patch.object(deploy_utils, 'check_for_missing_params',
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'parse_driver_info', autospec=True)
+    def test__parse_driver_info_invalid_params(
+            self, parse_driver_mock, check_missing_mock, validate_href_mock):
+        parse_driver_mock.return_value = {
+            'ilo_username': 'admin',
+            'ilo_password': 'admin'
+        }
+        driver_info = self.node.driver_info
+        driver_info['ilo_deploy_kernel'] = 'deploy-kernel'
+        driver_info['ilo_rescue_kernel'] = 'rescue-kernel'
+        driver_info['ilo_deploy_ramdisk'] = 'deploy-ramdisk'
+        driver_info['ilo_rescue_ramdisk'] = 'rescue-ramdisk'
+        driver_info['ilo_bootloader'] = 'bootloader'
+        driver_info['dummy_key'] = 'dummy-value'
+        driver_info['ilo_add_certificates'] = 'xyz'
+        self.node.driver_info = driver_info
+        self.node.save()
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            deploy_info = {
+                'ilo_deploy_kernel': 'deploy-kernel',
+                'ilo_deploy_ramdisk': 'deploy-ramdisk',
+                'ilo_bootloader': 'bootloader'
+            }
+
+            deploy_info.update({'ilo_username': 'admin',
+                                'ilo_password': 'admin'})
+            self.assertRaisesRegex(exception.InvalidParameterValue,
+                                   "Invalid value type set in driver_info.*",
+                                   task.driver.boot._parse_driver_info,
+                                   task.node)
+            validate_href_mock.assert_not_called()
+            check_missing_mock.assert_not_called()
+            parse_driver_mock.assert_not_called()
 
     @mock.patch.object(ilo_boot.IloUefiHttpsBoot, '_validate_hrefs',
                        autospec=True)
@@ -1882,6 +1924,8 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
             self.assertRaises(exception.UnsupportedDriverExtension,
                               task.driver.boot.validate_inspection, task)
 
+    @mock.patch.object(ilo_common, 'add_certificates',
+                       spec_set=True, autospec=True)
     @mock.patch.object(ilo_common, 'setup_uefi_https',
                        spec_set=True, autospec=True)
     @mock.patch.object(image_utils, 'prepare_deploy_iso',
@@ -1897,6 +1941,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
                               prepare_node_for_deploy_mock,
                               prepare_deploy_iso_mock,
                               setup_uefi_https_mock,
+                              add_mock,
                               ilo_boot_iso, image_source,
                               ramdisk_params={'a': 'b'},
                               mode='deploy', state=states.DEPLOYING):
@@ -1936,6 +1981,7 @@ class IloUefiHttpsBootTestCase(db_base.DbTestCase):
                 task, ramdisk_params, mode, d_info)
             setup_uefi_https_mock.assert_called_once_with(task,
                                                           'recreated-iso')
+            add_mock.assert_called_once_with(task)
 
     def test_prepare_ramdisk_rescue_glance_image(self):
         self._test_prepare_ramdisk(

ironic/tests/unit/drivers/modules/ilo/test_common.py

@@ -1221,6 +1221,201 @@ class IloCommonMethodsTestCase(BaseIloTest):
                               task.node)
         ilo_mock_object.get_host_post_state.assert_called_once_with()
 
+    @mock.patch.object(os.path, 'exists', spec_set=True,
+                       autospec=True)
+    def test__get_certificate_file_list_none(self, path_exists_mock):
+        cl = None
+        CONF.webserver_verify_ca = '/file/path'
+        path_exists_mock.return_value = True
+        expected = ['/file/path']
+        actual = ilo_common._get_certificate_file_list(cl)
+        self.assertEqual(expected, actual)
+
+    @mock.patch.object(os.path, 'exists', spec_set=True,
+                       autospec=True)
+    def test__get_certificate_file_list_empty(self, path_exists_mock):
+        cl = []
+        CONF.webserver_verify_ca = '/file/path'
+        path_exists_mock.return_value = True
+        expected = ['/file/path']
+        actual = ilo_common._get_certificate_file_list(cl)
+        self.assertEqual(expected, actual)
+
+    @mock.patch.object(os.path, 'exists', spec_set=True,
+                       autospec=True)
+    def test__get_certificate_file_list_empty_no_path(self, path_exists_mock):
+        cl = []
+        CONF.webserver_verify_ca = '/file/path'
+        path_exists_mock.return_value = False
+        expected = []
+        actual = ilo_common._get_certificate_file_list(cl)
+        self.assertEqual(expected, actual)
+
+    def test__get_certificate_file_list(self):
+        cl = ['file/path/a', 'file/path/b']
+        CONF.webserver_verify_ca = '/file/path/c'
+        expected = cl
+        actual = ilo_common._get_certificate_file_list(cl)
+        self.assertEqual(expected, actual)
+
+    def test__get_certificate_file_list_string_type(self):
+        cl = 'file/path/a'
+        CONF.webserver_verify_ca = '/file/path/c'
+        self.assertRaisesRegex(exception.InvalidParameterValue,
+                               "List of files is .* \"<class 'str'>\" .*",
+                               ilo_common._get_certificate_file_list, cl)
+
+    @mock.patch.object(ilo_common, '_get_certificate_file_list', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_add_certificates_false(self, get_ilo_object_mock, get_cl_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/a', '/file/path/b']
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.driver_info['ilo_add_certificates'] = "false"
+            ilo_common.add_certificates(task, c_l)
+
+        get_cl_mock.assert_not_called()
+        ilo_mock_object.add_tls_certificate.assert_not_called()
+
+    @mock.patch.object(ilo_common, '_get_certificate_file_list', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_add_certificates_true(self, get_ilo_object_mock, get_cl_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/a', '/file/path/b']
+        get_cl_mock.return_value = c_l
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.driver_info['ilo_add_certificates'] = "true"
+            ilo_common.add_certificates(task, c_l)
+
+        get_cl_mock.assert_called_once_with(c_l)
+        ilo_mock_object.add_tls_certificate.assert_called_once_with(c_l)
+
+    @mock.patch.object(ilo_common, '_get_certificate_file_list', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_add_certificates_None(self, get_ilo_object_mock, get_cl_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/a', '/file/path/b']
+        get_cl_mock.return_value = c_l
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.driver_info['ilo_add_certificates'] = None
+            ilo_common.add_certificates(task, c_l)
+
+        get_cl_mock.assert_called_once_with(c_l)
+        ilo_mock_object.add_tls_certificate.assert_called_once_with(c_l)
+
+    @mock.patch.object(ilo_common, '_get_certificate_file_list', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_add_certificates_invalid(self, get_ilo_object_mock, get_cl_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/a', '/file/path/b']
+        get_cl_mock.return_value = c_l
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.driver_info['ilo_add_certificates'] = "xyz"
+            self.assertRaisesRegex(exception.InvalidParameterValue,
+                                   "Invalid value type set in driver_info.*",
+                                   ilo_common.add_certificates,
+                                   task, c_l)
+
+        get_cl_mock.assert_not_called()
+        ilo_mock_object.add_tls_certificate.assert_not_called()
+
+    @mock.patch.object(ilo_common, '_get_certificate_file_list', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_add_certificates_true_default(self, get_ilo_object_mock,
+                                           get_cl_mock):
+
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/A']
+        get_cl_mock.return_value = c_l
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.driver_info['ilo_add_certificates'] = "true"
+            ilo_common.add_certificates(task)
+
+        get_cl_mock.assert_called_once_with(None)
+        ilo_mock_object.add_tls_certificate.assert_called_once_with(c_l)
+
+    @mock.patch.object(ilo_common, '_get_certificate_file_list', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_add_certificates_raises_ilo_error(self, get_ilo_object_mock,
+                                               get_cl_mock):
+        CONF.webserver_verify_ca = False
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/a', '/file/path/b']
+        get_cl_mock.return_value = c_l
+        exc = ilo_error.IloError('error')
+        ilo_mock_object.add_tls_certificate.side_effect = exc
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.driver_info['ilo_add_certificates'] = "true"
+            self.assertRaises(exception.IloOperationError,
+                              ilo_common.add_certificates,
+                              task, c_l)
+
+        get_cl_mock.assert_called_once_with(c_l)
+        ilo_mock_object.add_tls_certificate.assert_called_once_with(c_l)
+
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_clear_certificates(self, get_ilo_object_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/a', '/file/path/b']
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            ilo_common.clear_certificates(task, c_l)
+
+        ilo_mock_object.remove_tls_certificate.assert_called_once_with(c_l)
+
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_clear_certificates_default(self, get_ilo_object_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            ilo_common.clear_certificates(task)
+
+        ilo_mock_object.remove_tls_certificate.assert_called_once_with(None)
+
+    @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
+                       autospec=True)
+    def test_clear_certificates_raises_ilo_error(self, get_ilo_object_mock):
+        ilo_mock_object = get_ilo_object_mock.return_value
+        c_l = ['/file/path/a', '/file/path/b']
+        exc = ilo_error.IloError('error')
+        ilo_mock_object.remove_tls_certificate.side_effect = exc
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            self.assertRaises(exception.IloOperationError,
+                              ilo_common.clear_certificates,
+                              task, c_l)
+
+        ilo_mock_object.remove_tls_certificate.assert_called_once_with(c_l)
+
     @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True,
                        autospec=True)
     def test_setup_uefi_https_scheme_http(self, get_ilo_object_mock):

releasenotes/notes/add-support-to-manage_certs-b6615e15f697bc26.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Adds support to manage certificates to the ``ilo5`` hardware type. A new
+    optional boolean driver_info parameter ``ilo_add_certificates`` is
+    introduced which can be used by the user to request addition of
+    certificates to the iLO with ``ilo-uefi-https`` boot interface.