diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 4d1eb582c2..b0666748e4 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -711,6 +711,14 @@ class InvalidDeployTemplate(Invalid): _msg_fmt = _("Deploy template invalid: %(err)s.") +class InvalidKickstartTemplate(Invalid): + _msg_fmt = _("The kickstart template is missing required variables") + + +class InvalidKickstartFile(Invalid): + _msg_fmt = _("The kickstart file is not valid.") + + class IBMCError(DriverOperationError): _msg_fmt = _("IBMC exception occurred on node %(node)s. Error: %(error)s") diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index b670d983de..e37a3fcbc4 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -16,8 +16,11 @@ import copy import os +import tempfile from ironic_lib import utils as ironic_utils +import jinja2 +from oslo_concurrency import processutils from oslo_log import log as logging from oslo_utils import excutils from oslo_utils import fileutils @@ -1060,6 +1063,66 @@ def validate_boot_parameters_for_trusted_boot(node): raise exception.InvalidParameterValue(msg) +def validate_kickstart_template(ks_template): + """Validate the kickstart template + + :param ks_template: Path to the kickstart template + :raises: InvalidKickstartTemplate + """ + ks_options = {'liveimg_url': 'fake_image_url', + 'agent_token': 'fake_token', + 'heartbeat_url': 'fake_heartbeat_url'} + params = {'ks_options': ks_options} + try: + rendered_tmpl = utils.render_template(ks_template, params, strict=True) + except jinja2.exceptions.UndefinedError as exc: + msg = (_("The kickstart template includes a variable that is not " + "a valid kickstart option. Rendering the template returned " + " %(msg)s. The valid options are %(valid_options)s.") % + {'msg': exc.message, + 'valid_options': ','.join(ks_options.keys())}) + raise exception.InvalidKickstartTemplate(msg) + + missing_required_options = [] + for var, value in ks_options.items(): + if rendered_tmpl.find(value) == -1: + missing_required_options.append(var) + if missing_required_options: + msg = (_("Following required kickstart option variables are missing " + "from the kickstart template: %(missing_opts)s.") % + {'missing_opts': ','.join(missing_required_options)}) + raise exception.InvalidKickstartTemplate(msg) + return rendered_tmpl + + +def validate_kickstart_file(ks_cfg): + """Check if the kickstart file is valid + + :param ks_cfg: Contents of kickstart file to validate + :raises: InvalidKickstartFile + """ + if not os.path.isfile('/usr/bin/ksvalidator'): + LOG.warning( + "Unable to validate the kickstart file as ksvalidator binary is " + "missing. Please install pykickstart package to enable " + "validation of kickstart file." + ) + return + + with tempfile.NamedTemporaryFile( + dir=CONF.tempdir, suffix='.cfg') as ks_file: + ks_file.writelines(ks_cfg) + try: + result = utils.execute( + 'ksvalidator', ks_file.name, check_on_exit=[0], attempts=1 + ) + except processutils.ProcessExecutionError: + msg = _(("The kickstart file generated does not pass validation. " + "The ksvalidator tool returned following error(s): %s") % + (result)) + raise exception.InvalidKickstartFile(msg) + + def prepare_instance_pxe_config(task, image_info, iscsi_boot=False, ramdisk_boot=False, diff --git a/ironic/common/utils.py b/ironic/common/utils.py index cd323af364..eee581da63 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -470,13 +470,15 @@ def validate_network_port(port, port_name="Port"): {'port_name': port_name, 'port': port}) -def render_template(template, params, is_file=True): +def render_template(template, params, is_file=True, strict=False): """Renders Jinja2 template file with given parameters. :param template: full path to the Jinja2 template file :param params: dictionary with parameters to use when rendering :param is_file: whether template is file or string with template itself - :returns: the rendered template as a string + :param strict: Enable strict template rendering. Default is False + :returns: Rendered template + :raises: jinja2.exceptions.UndefinedError """ if is_file: tmpl_path, tmpl_name = os.path.split(template) @@ -488,8 +490,11 @@ def render_template(template, params, is_file=True): # and still complains with B701 for that line # NOTE(pas-ha) not using default_for_string=False as we set the name # of the template above for strings too. - env = jinja2.Environment(loader=loader, # nosec B701 - autoescape=jinja2.select_autoescape()) + env = jinja2.Environment( + loader=loader, + autoescape=jinja2.select_autoescape(), # nosec B701 + undefined=jinja2.StrictUndefined if strict else jinja2.Undefined + ) tmpl = env.get_template(tmpl_name) return tmpl.render(params, enumerate=enumerate) diff --git a/ironic/drivers/modules/pxe_base.py b/ironic/drivers/modules/pxe_base.py index d0c3a5e4ac..eb30f4846e 100644 --- a/ironic/drivers/modules/pxe_base.py +++ b/ironic/drivers/modules/pxe_base.py @@ -239,6 +239,11 @@ class PXEBaseMixin(object): task, ipxe_enabled=self.ipxe_enabled) pxe_utils.cache_ramdisk_kernel(task, instance_image_info, ipxe_enabled=self.ipxe_enabled) + if 'ks_template' in instance_image_info: + ks_cfg = pxe_utils.validate_kickstart_template( + instance_image_info['ks_template'][1] + ) + pxe_utils.validate_kickstart_file(ks_cfg) if (deploy_utils.is_iscsi_boot(task) or boot_option == "ramdisk" or boot_option == "kickstart"): diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index c7e5d76304..7dd28d61f6 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -1380,6 +1380,28 @@ class PXEBuildKickstartConfigOptionsTestCase(db_base.DbTestCase): write_mock.assert_called_with(image_info['ks_cfg'][1], render_mock.return_value) + def test_validate_kickstart_template(self): + self.config_temp_dir('http_root', group='deploy') + ks_template_path = 'ironic/drivers/modules/ks.cfg.template' + pxe_utils.validate_kickstart_template(ks_template_path) + + def test_validate_kickstart_template_missing_variable(self): + self.config_temp_dir('http_root', group='deploy') + # required variable is missing from the template + ks_template_path = 'ironic/tests/unit/drivers/ks_missing_var.tmpl' + self.assertRaises( + exception.InvalidKickstartTemplate, + pxe_utils.validate_kickstart_template, + ks_template_path) + + def test_validate_kickstart_template_has_additional_variables(self): + self.config_temp_dir('http_root', group='deploy') + ks_template_path = 'ironic/tests/unit/drivers/ks_extra_vars.tmpl' + self.assertRaises( + exception.InvalidKickstartTemplate, + pxe_utils.validate_kickstart_template, + ks_template_path) + @mock.patch.object(pxe.PXEBoot, '__init__', lambda self: None) class PXEBuildConfigOptionsTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/ks_extra_vars.tmpl b/ironic/tests/unit/drivers/ks_extra_vars.tmpl new file mode 100644 index 0000000000..37f2e2d194 --- /dev/null +++ b/ironic/tests/unit/drivers/ks_extra_vars.tmpl @@ -0,0 +1,41 @@ +lang en_US +keyboard us +timezone UTC --utc +#platform x86, AMD64, or Intel EM64T +text +cmdline +reboot +selinux --enforcing +firewall --enabled +firstboot --disabled + +bootloader --location=mbr --append="rhgb quiet crashkernel=auto" +zerombr +clearpart --all --initlabel +autopart + +# Downloading and installing OS image using liveimg section is mandatory +liveimg --url {{ ks_options.liveimg_url }} + +# Following %pre, %onerror and %trackback sections are mandatory +%pre +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "start"}' {{ ks_options.heartbeat_url }} +%end + +%onerror +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "Error: Deploying using anaconda. Check console for more information."}' {{ ks_options.heartbeat_url }} +%end + +%traceback +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "Error: Installer crashed unexpectedly."}' {{ ks_options.heartbeat_url }} +%end + +# Sending callback after the installation is mandatory +%post +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "success"}' {{ ks_options.heartbeat_url }} +%end + +# config_drive is an extra variable and should raise an exception +%post +{{ config_drive }} +%end diff --git a/ironic/tests/unit/drivers/ks_missing_var.tmpl b/ironic/tests/unit/drivers/ks_missing_var.tmpl new file mode 100644 index 0000000000..ad160fb8f9 --- /dev/null +++ b/ironic/tests/unit/drivers/ks_missing_var.tmpl @@ -0,0 +1,37 @@ +lang en_US +keyboard us +timezone UTC --utc +#platform x86, AMD64, or Intel EM64T +text +cmdline +reboot +selinux --enforcing +firewall --enabled +firstboot --disabled + +bootloader --location=mbr --append="rhgb quiet crashkernel=auto" +zerombr +clearpart --all --initlabel +autopart + +# Downloading and installing OS image using liveimg section is mandatory +liveimg --url http://this_should_raise_an_exception + +# Following %pre, %onerror and %trackback sections are mandatory +%pre +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "start"}' {{ ks_options.heartbeat_url }} +%end + +%onerror +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "Error: Deploying using anaconda. Check console for more information."}' {{ ks_options.heartbeat_url }} +%end + +%traceback +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "Error: Installer crashed unexpectedly."}' {{ ks_options.heartbeat_url }} +%end + +# Sending callback after the installation is mandatory +%post +/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept:application/json' -d '{"agent_token": {{ ks_options.agent_token }}, "agent_status": "success"}' {{ ks_options.heartbeat_url }} +%end + diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index a204f6954b..3f7d9e4b73 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -755,9 +755,10 @@ class PXEBootTestCase(db_base.DbTestCase): return_value='http://fakeserver/api', autospec=True) @mock.patch('ironic.common.utils.render_template', autospec=True) @mock.patch('ironic.common.utils.write_to_file', autospec=True) + @mock.patch('ironic.common.utils.execute', autospec=True) def test_prepare_instance_kickstart( - self, write_file_mock, render_mock, api_url_mock, boot_opt_mock, - get_image_info_mock, cache_mock, dhcp_factory_mock, + self, exec_mock, write_file_mock, render_mock, api_url_mock, + boot_opt_mock, get_image_info_mock, cache_mock, dhcp_factory_mock, create_pxe_config_mock, switch_pxe_config_mock, set_boot_device_mock): image_info = {'kernel': ['ins_kernel_id', '/path/to/kernel'], @@ -784,6 +785,10 @@ class PXEBootTestCase(db_base.DbTestCase): ipxe_enabled=False) cache_mock.assert_called_once_with( task, image_info, False) + if os.path.isfile('/usr/bin/ksvalidator'): + exec_mock.assert_called_once_with( + 'ksvalidator', mock.ANY, check_on_exit=[0], attempts=1 + ) provider_mock.update_dhcp.assert_called_once_with(task, dhcp_opts) render_mock.assert_called() write_file_mock.assert_called_with(