diff --git a/doc/source/admin/drivers/ilo.rst b/doc/source/admin/drivers/ilo.rst index 56f1022877..65ff4f6dac 100644 --- a/doc/source/admin/drivers/ilo.rst +++ b/doc/source/admin/drivers/ilo.rst @@ -55,6 +55,8 @@ The hardware type ``ilo`` supports following HPE server features: * `Updating security parameters as manual clean step`_ * `Update Minimum Password Length security parameter as manual clean step`_ * `Update Authentication Failure Logging security parameter as manual clean step`_ +* `Create Certificate Signing Request(CSR) as manual clean step`_ +* `Add HTTPS Certificate as manual clean step`_ * `Activating iLO Advanced license as manual clean step`_ * `Removing CA certificates from iLO as manual clean step`_ * `Firmware based UEFI iSCSI boot from volume support`_ @@ -764,6 +766,12 @@ Supported **Manual** Cleaning Operations ``update_auth_failure_logging_threshold``: Updates the Authentication Failure Logging security parameter. See `Update Authentication Failure Logging security parameter as manual clean step`_ for user guidance on usage. + ``create_csr``: + Creates the certificate signing request. See `Create Certificate Signing Request(CSR) as manual clean step`_ + for user guidance on usage. + ``add_https_certificate``: + Adds the signed HTTPS certificate to the iLO. See `Add HTTPS Certificate as manual clean step`_ for user + guidance on usage. * iLO with firmware version 1.5 is minimally required to support all the operations. @@ -1661,6 +1669,54 @@ Both the arguments ``logging_threshold`` and ``ignore`` are optional. The accept value be False. If user passes the value of logging_threshold as 0, the Authentication Failure Logging security parameter will be disabled. +Create Certificate Signing Request(CSR) as manual clean step +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +iLO driver can invoke ``create_csr`` request as a manual clean step. This step is only supported for iLO5 based hardware. + +An example of a manual clean step with ``create_csr`` as the only clean step could be:: + + "clean_steps": [{ + "interface": "management", + "step": "create_csr", + "args": { + "csr_params": { + "City": "Bengaluru", + "CommonName": "1.1.1.1", + "Country": "India", + "OrgName": "HPE", + "State": "Karnataka" + } + } + }] + +The ``[ilo]cert_path`` option in ``ironic.conf`` is used as the directory path for +creating the CSR, which defaults to ``/var/lib/ironic/ilo``. The CSR is created in the directory location +given in ``[ilo]cert_path`` in ``node_uuid`` directory as .csr. + + +Add HTTPS Certificate as manual clean step +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +iLO driver can invoke ``add_https_certificate`` request as a manual clean step. This step is only supported for +iLO5 based hardware. + +An example of a manual clean step with ``add_https_certificate`` as the only clean step could be:: + + "clean_steps": [{ + "interface": "management", + "step": "add_https_certificate", + "args": { + "cert_file": "/test1/iLO.crt" + } + }] + +Argument ``cert_file`` is mandatory. The ``cert_file`` takes the path or url of the certificate file. +The url schemes supported are: ``file``, ``http`` and ``https``. +The CSR generated in step ``create_csr`` needs to be signed by a valid CA and the resultant HTTPS certificate should +be provided in ``cert_file``. It copies the ``cert_file`` to ``[ilo]cert_path`` under ``node.uuid`` as .crt +before adding it to iLO. + RAID Support ^^^^^^^^^^^^ diff --git a/driver-requirements.txt b/driver-requirements.txt index 5333dbd4f5..dc69eddc6a 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -4,7 +4,7 @@ # python projects they should package as optional dependencies for Ironic. # These are available on pypi -proliantutils>=2.13.0 +proliantutils>=2.14.0 pysnmp>=4.3.0,<5.0.0 python-scciclient>=0.12.2 python-dracclient>=5.1.0,<9.0.0 diff --git a/ironic/conf/ilo.py b/ironic/conf/ilo.py index 364c64c814..197378ce75 100644 --- a/ironic/conf/ilo.py +++ b/ironic/conf/ilo.py @@ -120,6 +120,11 @@ opts = [ '/proc/cmdline. Mind severe cmdline size limit! Can be ' 'overridden by `instance_info/kernel_append_params` ' 'property.')), + cfg.StrOpt('cert_path', + default='/var/lib/ironic/ilo/', + help=_('On the ironic-conductor node, directory where ilo ' + 'driver stores the CSR and the cert.')), + ] diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 6563d1e8c8..13f975c67c 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -32,6 +32,7 @@ from ironic.common import boot_devices from ironic.common import exception from ironic.common.glance_service import service_utils from ironic.common.i18n import _ +from ironic.common import image_service from ironic.common import images from ironic.common import swift from ironic.common import utils @@ -1147,3 +1148,23 @@ def setup_uefi_https(task, iso, persistent=False): except ilo_error.IloError as ilo_exception: raise exception.IloOperationError(operation=operation, error=ilo_exception) + + +def download(target_file, file_url): + """Downloads file based on the scheme. + + It downloads the file (url) to given location. + The supported url schemes are file, http, and https. + :param target_file: target file for copying the downloaded file. + :param file_url: source file url from where file needs to be downloaded. + :raises: ImageDownloadFailed, on failure to download the file. + """ + parsed_url = urlparse.urlparse(file_url) + if parsed_url.scheme == "file": + src_file = parsed_url.path + with open(target_file, 'wb') as fd: + image_service.FileImageService().download(src_file, fd) + elif parsed_url.scheme in ('http', 'https'): + src_file = parsed_url.geturl() + with open(target_file, 'wb') as fd: + image_service.HttpImageService().download(src_file, fd) diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index c9a8259e68..5c4f03fb60 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -14,7 +14,8 @@ """ iLO Management Interface """ - +import os +import shutil from urllib import parse as urlparse from ironic_lib import metrics_utils @@ -79,6 +80,27 @@ _RESET_ILO_CREDENTIALS_ARGSINFO = { } } +_CREATE_CSR_ARGSINFO = { + 'csr_params': { + 'description': ( + "This arguments represents the information needed " + "to create the CSR certificate. The keys to be provided are " + "City, CommonName, OrgName, State." + ), + 'required': True + } +} + +_ADD_HTTPS_CERT_ARGSINFO = { + 'cert_file': { + 'description': ( + "This argument represents the path to the signed HTTPS " + "certificate which will be added to the iLO." + ), + 'required': True + } +} + _SECURITY_PARAMETER_UPDATE_ARGSINFO = { 'security_parameters': { 'description': ( @@ -574,6 +596,61 @@ class IloManagement(base.ManagementInterface): "parameter for node %(node)s is updated", {'node': node.uuid}) + @METRICS.timer('IloManagement.create_csr') + @base.clean_step(priority=0, abortable=False, + argsinfo=_CREATE_CSR_ARGSINFO) + def create_csr(self, task, **kwargs): + """Creates the CSR. + + :param task: a TaskManager object. + """ + node = task.node + csr_params = kwargs.get('csr_params') + csr_path = CONF.ilo.cert_path + path = os.path.join(csr_path, task.node.uuid) + if not os.path.exists(path): + os.makedirs(path, 0o755) + + LOG.debug("Creating CSR for node %(node)s ..", + {'node': node.uuid}) + _execute_ilo_step(node, 'create_csr', path, csr_params) + LOG.info("Creation of CSR for node %(node)s is " + "completed.", {'node': node.uuid}) + + @METRICS.timer('IloManagement.add_https_certificate') + @base.clean_step(priority=0, abortable=False, + argsinfo=_ADD_HTTPS_CERT_ARGSINFO) + def add_https_certificate(self, task, **kwargs): + """Adds the signed HTTPS certificate to the iLO. + + :param task: a TaskManager object. + """ + node = task.node + csr_path = CONF.ilo.cert_path + path = os.path.join(csr_path, task.node.uuid) + if not os.path.exists(path): + os.makedirs(path, 0o755) + cert_file_name = node.uuid + ".crt" + cert_file_path = os.path.join(path, cert_file_name) + cert_file = kwargs.get('cert_file') + url_scheme = urlparse.urlparse(cert_file).scheme + if url_scheme == '': + shutil.copy(cert_file, cert_file_path) + elif url_scheme in ('http', 'https', 'file'): + ilo_common.download(cert_file_path, cert_file) + else: + msg = (_("The url scheme %(scheme)s not supported with clean step " + "%(step)s") % {'scheme': url_scheme, + 'step': 'add_https_certificate'}) + raise exception.IloOperationNotSupported(operation='clean step', + error=msg) + + LOG.debug("Adding the signed HTTPS certificate to the " + "node %(node)s ..", {'node': node.uuid}) + _execute_ilo_step(node, 'add_https_certificate', cert_file_path) + LOG.info("Adding of HTTPS certificate to the node %(node)s " + "is completed.", {'node': node.uuid}) + @METRICS.timer('IloManagement.update_firmware') @base.deploy_step(priority=0, argsinfo=_FIRMWARE_UPDATE_ARGSINFO) @base.clean_step(priority=0, abortable=False, diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index 41f2eb357d..c3e22453fe 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -31,6 +31,7 @@ from oslo_utils import uuidutils from ironic.common import boot_devices from ironic.common import exception +from ironic.common import image_service from ironic.common import images from ironic.common import swift from ironic.conductor import task_manager @@ -1521,3 +1522,37 @@ class IloCommonMethodsTestCase(BaseIloTest): self.assertRaises(exception.IloOperationError, ilo_common.setup_uefi_https, task, iso, True) + + @mock.patch.object(image_service, 'FileImageService', spec_set=True, + autospec=True) + @mock.patch.object(image_service, 'HttpImageService', spec_set=True, + autospec=True) + @mock.patch.object(builtins, 'open', autospec=True) + def test_download_file_url(self, open_mock, http_mock, file_mock): + url = "file:///test1/iLO.crt" + target_file = "/a/b/c" + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + ilo_common.download(target_file, url) + open_mock.assert_called_once_with(target_file, 'wb') + http_mock.assert_not_called() + file_mock.return_value.download.assert_called_once_with( + "/test1/iLO.crt", fd_mock) + + @mock.patch.object(image_service, 'FileImageService', spec_set=True, + autospec=True) + @mock.patch.object(image_service, 'HttpImageService', spec_set=True, + autospec=True) + @mock.patch.object(builtins, 'open', autospec=True) + def test_download_http_url(self, open_mock, http_mock, file_mock): + url = "http://1.1.1.1/iLO.crt" + target_file = "/a/b/c" + fd_mock = mock.MagicMock(spec=io.BytesIO) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + ilo_common.download(target_file, url) + http_mock.return_value.download.assert_called_once_with( + "http://1.1.1.1/iLO.crt", fd_mock) + file_mock.assert_not_called() + open_mock.assert_called_once_with(target_file, 'wb') diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index e4d891c3d0..f087c4d586 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -14,9 +14,12 @@ """Test class for Management Interface used by iLO modules.""" +import os +import shutil from unittest import mock import ddt +from oslo_config import cfg from oslo_utils import importutils from oslo_utils import uuidutils @@ -42,6 +45,8 @@ ilo_error = importutils.try_import('proliantutils.exception') INFO_DICT = db_utils.get_test_ilo_info() +CONF = cfg.CONF + @ddt.ddt class IloManagementTestCase(test_common.BaseIloTest): @@ -424,6 +429,116 @@ class IloManagementTestCase(test_common.BaseIloTest): step_mock.assert_called_once_with( task.node, 'update_authentication_failure_logging', '1', False) + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + @mock.patch.object(os, 'makedirs', spec_set=True, autospec=True) + def test_create_csr(self, os_mock, step_mock): + csr_params_args = { + "City": "Bangalore", + "CommonName": "1.1.1.1", + "Country": "ABC", + "OrgName": "DEF", + "State": "IJK" + } + csr_args = { + "csr_params": csr_params_args} + CONF.ilo.cert_path = "/var/lib/ironic/ilo" + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.management.create_csr(task, **csr_args) + cert_path = os.path.join(CONF.ilo.cert_path, self.node.uuid) + step_mock.assert_called_once_with(task.node, 'create_csr', + cert_path, csr_params_args) + os_mock.assert_called_once_with(cert_path, 0o755) + + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + @mock.patch.object(os, 'makedirs', spec_set=True, autospec=True) + @mock.patch.object(shutil, 'copy', spec_set=True, autospec=True) + def test_add_https_certificate(self, shutil_mock, os_mock, + step_mock): + CONF.ilo.cert_path = "/var/lib/ironic/ilo" + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + cert_file_args = {'cert_file': '/test1/cert'} + task.driver.management.add_https_certificate( + task, **cert_file_args) + cert_path = os.path.join(CONF.ilo.cert_path, self.node.uuid) + cert_path_name = os.path.join(cert_path, self.node.uuid) + filename = cert_path_name + ".crt" + step_mock.assert_called_once_with( + task.node, 'add_https_certificate', filename) + os_mock.assert_called_once_with(cert_path, 0o755) + shutil_mock.assert_called_once_with('/test1/cert', filename) + + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + @mock.patch.object(os, 'makedirs', spec_set=True, autospec=True) + @mock.patch.object(shutil, 'copy', spec_set=True, autospec=True) + @mock.patch.object(ilo_common, 'download', spec_set=True, autospec=True) + def test_add_https_certificate_fileurl(self, download_mock, shutil_mock, + os_mock, step_mock): + CONF.ilo.cert_path = "/var/lib/ironic/ilo" + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + cert_file_args = {'cert_file': 'file:///test1/cert'} + task.driver.management.add_https_certificate( + task, **cert_file_args) + cert_path = os.path.join(CONF.ilo.cert_path, self.node.uuid) + cert_path_name = os.path.join(cert_path, self.node.uuid) + fname = cert_path_name + ".crt" + step_mock.assert_called_once_with( + task.node, 'add_https_certificate', fname) + os_mock.assert_called_once_with(cert_path, 0o755) + shutil_mock.assert_not_called() + download_mock.assert_called_once_with(fname, 'file:///test1/cert') + + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + @mock.patch.object(os, 'makedirs', spec_set=True, autospec=True) + @mock.patch.object(shutil, 'copy', spec_set=True, autospec=True) + @mock.patch.object(ilo_common, 'download', spec_set=True, autospec=True) + def test_add_https_certificate_httpurl(self, download_mock, shutil_mock, + os_mock, step_mock): + CONF.ilo.cert_path = "/var/lib/ironic/ilo" + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + cert_file_args = {'cert_file': 'http://1.1.1.1/cert'} + task.driver.management.add_https_certificate( + task, **cert_file_args) + cert_path = os.path.join(CONF.ilo.cert_path, self.node.uuid) + cert_path_name = os.path.join(cert_path, self.node.uuid) + fname = cert_path_name + ".crt" + step_mock.assert_called_once_with( + task.node, 'add_https_certificate', fname) + os_mock.assert_called_once_with(cert_path, 0o755) + shutil_mock.assert_not_called() + download_mock.assert_called_once_with(fname, 'http://1.1.1.1/cert') + + @mock.patch.object(ilo_management, '_execute_ilo_step', + spec_set=True, autospec=True) + @mock.patch.object(os, 'makedirs', spec_set=True, autospec=True) + @mock.patch.object(shutil, 'copy', spec_set=True, autospec=True) + @mock.patch.object(ilo_common, 'download', spec_set=True, autospec=True) + def test_add_https_certificate_url_exception(self, download_mock, + shutil_mock, os_mock, + step_mock): + CONF.ilo.cert_path = "/var/lib/ironic/ilo" + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + cert_file_args = {'cert_file': 'swift://1.1.1.1/cert'} + self.assertRaises(exception.IloOperationNotSupported, + task.driver.management.add_https_certificate, + task, + **cert_file_args) + + cert_path = os.path.join(CONF.ilo.cert_path, self.node.uuid) + step_mock.assert_not_called() + os_mock.assert_called_once_with(cert_path, 0o755) + shutil_mock.assert_not_called() + download_mock.assert_not_called() + @mock.patch.object(deploy_utils, 'build_agent_options', spec_set=True, autospec=True) @mock.patch.object(ilo_boot.IloVirtualMediaBoot, 'clean_up_ramdisk', diff --git a/releasenotes/notes/create_csr_clean_step-a720932f61b42118.yaml b/releasenotes/notes/create_csr_clean_step-a720932f61b42118.yaml new file mode 100644 index 0000000000..1951245c10 --- /dev/null +++ b/releasenotes/notes/create_csr_clean_step-a720932f61b42118.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adds new clean steps ``create_csr`` and ``add_https_certificate`` + to ``ilo`` and ``ilo5`` hardware types which allows users to + create Certificate Signing Request(CSR) and adds signed HTTPS + certificate to the iLO.