openstack
/
ironic
Code Issues Proposed changes

Merge "Validate the kickstart template and file before use"

This commit is contained in:
Zuul 2021-03-29 15:13:31 +00:00 committed by Gerrit Code Review
parent 356734aaca 9d3de26fb1
commit 1caaa0c507
8 changed files with 192 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ironic/common/exception.py
View File

@ -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")

63
ironic/common/pxe_utils.py
View File

@ -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
@ -1064,6 +1067,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,

13
ironic/common/utils.py
View File

@ -468,13 +468,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)
@ -486,8 +488,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)

5
ironic/drivers/modules/pxe_base.py
View File

@ -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"):

22
ironic/tests/unit/common/test_pxe_utils.py
View File

@ -1411,6 +1411,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):

41
ironic/tests/unit/drivers/ks_extra_vars.tmpl Normal file
View File

@ -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

37
ironic/tests/unit/drivers/ks_missing_var.tmpl Normal file
View File

@ -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

9
ironic/tests/unit/drivers/modules/test_pxe.py
View File

@ -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(