diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index 83abb59409..1a0b8dfde4 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/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') diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 202aa25108..59e648aec6 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -545,10 +545,11 @@ def set_boot_mode(node, boot_mode): except ilo_error.IloCommandNotSupportedError: p_boot_mode = DEFAULT_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}) - return + 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}) + return try: ilo_object.set_pending_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. diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 6aa698578f..15d0e11789 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/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( diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index 30dad59846..bab7d36b41 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/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 .* \"\" .*", + 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): diff --git a/releasenotes/notes/add-support-to-manage_certs-b6615e15f697bc26.yaml b/releasenotes/notes/add-support-to-manage_certs-b6615e15f697bc26.yaml new file mode 100644 index 0000000000..3d758594ef --- /dev/null +++ b/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.