diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cb5c1cc..0760af28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.32.10] - 2024-11-04 + +### Fixed +- Fix jinja2 rendering of `boot_sets` data in `sat bootprep`. + ## [3.32.9] - 2024-10-29 ### Fixed diff --git a/sat/cli/bootprep/input/base.py b/sat/cli/bootprep/input/base.py index 3b73516d..1ee242e1 100644 --- a/sat/cli/bootprep/input/base.py +++ b/sat/cli/bootprep/input/base.py @@ -108,14 +108,25 @@ def wrapper(self, *args, **kwargs): # Default to no additional context if not set context = getattr(self, 'jinja_context', {}) - try: - return self.jinja_env.from_string(unrendered_result).render(context) - except SecurityError as err: - raise error_cls(f'Jinja2 template {unrendered_result} for value ' - f'{func.__name__} tried to access unsafe attributes.') from err - except TemplateError as err: - raise error_cls(f'Failed to render Jinja2 template {unrendered_result} ' - f'for value {func.__name__}: {err}') from err + def render_object(obj): + """Render an object with the given context.""" + if isinstance(obj, str): + try: + return self.jinja_env.from_string(obj).render(context) + except SecurityError as err: + raise error_cls(f'Jinja2 template {obj} for value ' + f'{func.__name__} tried to access unsafe attributes.') from err + except TemplateError as err: + raise error_cls(f'Failed to render Jinja2 template {obj} ' + f'for value {func.__name__}: {err}') from err + elif isinstance(obj, dict): + return {key: render_object(value) for key, value in obj.items()} + elif isinstance(obj, list): + return [render_object(item) for item in obj] + else: + return obj + + return render_object(unrendered_result) return wrapper @@ -242,11 +253,6 @@ def name(self): # items that inherit from BaseInputItem. return self.data['name'] - @property - def boot_set(self): - """Return the full boot_sets dictionary.""" - return self.data.get('bos_parameters', {}).get('boot_sets', {}) - def __str__(self): # Since the name can be rendered, and when unrendered, it does not need # to be unique, just refer to this item by its index in the instance. diff --git a/sat/cli/bootprep/input/session_template.py b/sat/cli/bootprep/input/session_template.py index 7febcb02..4fc48353 100644 --- a/sat/cli/bootprep/input/session_template.py +++ b/sat/cli/bootprep/input/session_template.py @@ -98,6 +98,7 @@ def configuration(self): return self.data['configuration'] @property + @jinja_rendered def boot_sets(self): """dict: the boot sets specified for the session template""" # the 'bos_parameters' property is required, and 'boot_sets' is required within that @@ -186,12 +187,6 @@ def get_create_item_data(self): boot_set_api_data.update(boot_set_data) api_data['boot_sets'][boot_set_name] = boot_set_api_data - # Fetch full boot sets - boot_sets = self.boot_set - # Render the rootfs_provider_passthrough using Jinja2 - rendered_rootfs = self.jinja_env.from_string( - boot_sets[boot_set_name]['rootfs_provider_passthrough']).render(self.data) - api_data['boot_sets'][boot_set_name]['rootfs_provider_passthrough'] = rendered_rootfs return api_data def get_image_record_by_id(self, ims_image_id): diff --git a/tests/cli/bootprep/input/test_base.py b/tests/cli/bootprep/input/test_base.py index 3600db63..3ef87dcf 100644 --- a/tests/cli/bootprep/input/test_base.py +++ b/tests/cli/bootprep/input/test_base.py @@ -1,7 +1,7 @@ # # MIT License # -# (C) Copyright 2022-2023 Hewlett Packard Enterprise Development LP +# (C) Copyright 2022-2024 Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -26,61 +26,163 @@ """ import unittest +from unittest.mock import MagicMock -from sat.cli.bootprep.input.base import BaseInputItem +from jinja2.sandbox import SandboxedEnvironment + +from sat.cli.bootprep.input.base import BaseInputItem, jinja_rendered +from sat.cli.bootprep.input.instance import InputInstance from sat.constants import MISSING_VALUE -def create_input_item_cls(cls_report_attrs): +def create_input_item_cls(): class SomeTestItem(BaseInputItem): """A mock concrete BaseInputItem class""" - description = 'some input item' - report_attrs = cls_report_attrs - - def __init__(self, data): - # Since the test input item class doesn't need access - # to the input instance, we can just ignore it here. - # As a consequence, other methods on this class won't - # work as expected. - self.data = data - - def __getattr__(self, item): - if item in cls_report_attrs: - try: - return self.data[item] - except KeyError as err: - raise AttributeError(str(err)) + description = 'some test item' + report_attrs = ['name', 'type', 'complex_data'] def get_create_item_data(self): return def create(self, payload): return + + @property + @jinja_rendered + def name(self): + return self.data.get('name') + + @property + @jinja_rendered + def type(self): + return self.data.get('type') + + @property + @jinja_rendered + def complex_data(self): + return self.data.get('complex_data') + return SomeTestItem class TestBaseInputItem(unittest.TestCase): + + def setUp(self): + self.mock_instance = MagicMock(spec=InputInstance) + self.jinja_env = SandboxedEnvironment() + self.jinja_env.globals = { + 'default': { + 'note': 'foo', + 'suffix': '-bar', + 'system_name': 'ryan', + 'site-domain': 'example.com' + } + } + def test_report_rows_have_correct_attrs(self): """Test that report rows are generated for input items""" - row_headings = ['name', 'type'] - row_vals = ['foo', 'configuration'] - item_cls = create_input_item_cls(row_headings) - self.assertEqual(item_cls(dict(zip(row_headings, row_vals))).report_row(), - row_vals) + data = {'name': 'foo', 'type': 'configuration', 'complex_data': 'columbo'} + item_cls = create_input_item_cls() + self.assertEqual(['foo', 'configuration', 'columbo'], + item_cls(data, self.mock_instance, 0, self.jinja_env).report_row()) def test_report_rows_excluded(self): """Test that report rows are not included if not present in report_attrs""" - item_cls = create_input_item_cls(['name']) - self.assertEqual(item_cls({'name': 'foo', 'something_else': 'another_thing'}).report_row(), ['foo']) + data = {'name': 'foo', 'something_else': 'another_thing'} + item_cls = create_input_item_cls() + item_cls.report_attrs = ['name'] + self.assertEqual(item_cls(data, self.mock_instance, 0, self.jinja_env).report_row(), ['foo']) def test_missing_report_attrs_are_missing(self): """Test that missing report attrs are reported as MISSING""" - row_headings = ['name', 'type', 'one_more_thing'] - row_vals = ['foo', 'configuration', 'columbo'] - item_cls = create_input_item_cls(row_headings) # Cut off last report attr - data = dict(zip(row_headings[:-1], row_vals[:-1])) - - report_row = item_cls(data).report_row() - self.assertEqual(len(report_row), len(row_headings)) - self.assertEqual(report_row[-1], MISSING_VALUE) + # This data is missing a value for the 'complex_data' key + data = {'name': 'foo', 'type': 'configuration'} + item_cls = create_input_item_cls() + item_cls.report_attrs = item_cls.report_attrs + ['nonexistent_property'] + report_row = item_cls(data, self.mock_instance, 0, self.jinja_env).report_row() + self.assertEqual(['foo', 'configuration', None, MISSING_VALUE], report_row) + + def test_jinja_rendered_string_value(self): + """Test that jinja_rendered works on a string value""" + data = {'name': 'foo{{default.suffix}}', 'type': 'configuration-{{default.system_name}}'} + item_cls = create_input_item_cls() + item = item_cls(data, self.mock_instance, 0, self.jinja_env) + self.assertEqual('foo-bar', item.name) + self.assertEqual('configuration-ryan', item.type) + + def test_jinja_rendered_complex_value(self): + """Test that jinja_rendered works on a more complex value""" + data = { + 'name': 'foo', + 'type': 'configuration', + 'complex_data': { + 'kitchen': { + 'furniture': ['{{furniture.brand}} table', '{{furniture.brand}} chair'], + 'appliances': ['oven', 'fridge'], + 'color': '{{paint.primary}}' + }, + 'living_room': { + 'furniture': ['couch', 'chair', 'rug'], + 'appliances': ['lamp', 'tv'], + 'color': '{{paint.secondary}}' + } + } + } + self.jinja_env.globals = { + 'furniture': { + 'brand': 'IKEA' + }, + 'paint': { + 'primary': 'blue', + 'secondary': 'red' + } + } + item_cls = create_input_item_cls() + item = item_cls(data, self.mock_instance, 0, self.jinja_env) + self.assertEqual( + { + 'kitchen': { + 'furniture': ['IKEA table', 'IKEA chair'], + 'appliances': ['oven', 'fridge'], + 'color': 'blue' + }, + 'living_room': { + 'furniture': ['couch', 'chair', 'rug'], + 'appliances': ['lamp', 'tv'], + 'color': 'red' + } + }, + item.complex_data + ) + + def test_jinja_template_rendering_undefined_variable(self): + """Test jinja_rendered property that uses an undefined variable.""" + item_cls = create_input_item_cls() + data = {'name': '{{undefined_variable}}', 'type': '{{nested.undefined_variable}}'} + item = item_cls(data, self.mock_instance, 0, self.jinja_env) + + self.assertEqual('', item.name) + err_regex = (r"Failed to render Jinja2 template {{nested\.undefined_variable}} " + r"for value type: 'nested' is undefined") + with self.assertRaisesRegex(item_cls.template_render_err, err_regex): + item.type + + def test_jinja_template_rendering_undefined_variable_nested(self): + """Test jinja_rendered property that uses an undefined variable in a nested field.""" + item_cls = create_input_item_cls() + data = { + 'name': 'foo', + 'type': 'configuration', + 'complex_data': { + 'kitchen': { + 'color': '{{colors.primary}}' + } + } + } + item = item_cls(data, self.mock_instance, 0, self.jinja_env) + + err_regex = (r"Failed to render Jinja2 template {{colors.primary}} " + r"for value complex_data: 'colors' is undefined") + with self.assertRaisesRegex(item_cls.template_render_err, err_regex): + item.complex_data diff --git a/tests/cli/bootprep/input/test_session_template.py b/tests/cli/bootprep/input/test_session_template.py index e97fbe22..26916867 100644 --- a/tests/cli/bootprep/input/test_session_template.py +++ b/tests/cli/bootprep/input/test_session_template.py @@ -38,6 +38,12 @@ def setUp(self): self.input_instance = Mock() self.index = 0 self.jinja_env = SandboxedEnvironment() + self.jinja_env.globals = { + 'default': { + 'system_name': 'drax', + 'site_domain': 'example.com', + } + } self.bos_client = Mock() self.mock_base_boot_set_data = { 'rootfs_provider': 'cpss3' @@ -93,16 +99,20 @@ def get_input_and_expected_bos_data(self, name='my-session-template', for boot_set_name, boot_set_arch in arch_by_bootset.items(): input_bootsets[boot_set_name] = { 'node_roles_groups': ['Compute'], - 'rootfs_provider_passthrough': "dvs:api-gw-service-nmn.local:300:hsn0,nmn0:0" + 'rootfs_provider': 'sbps', + 'rootfs_provider_passthrough': ('sbps:v1:iqn.2023-06.csm.iscsi:_sbps-hsn._tcp.' + '{{default.system_name}}.{{default.site_domain}}:300') } - bos_payload_bootsets[boot_set_name] = { + bos_payload_bootsets[boot_set_name] = self.mock_base_boot_set_data.copy() + bos_payload_bootsets[boot_set_name].update({ 'node_roles_groups': ['Compute'], - 'rootfs_provider_passthrough': "dvs:api-gw-service-nmn.local:300:hsn0,nmn0:0", + 'rootfs_provider': 'sbps', + 'rootfs_provider_passthrough': ('sbps:v1:iqn.2023-06.csm.iscsi:_sbps-hsn._tcp.' + 'drax.example.com:300'), 'path': self.mock_path, 'etag': self.mock_etag, 'type': self.mock_image_type - } - bos_payload_bootsets[boot_set_name].update(self.mock_base_boot_set_data) + }) if boot_set_arch: input_bootsets[boot_set_name]['arch'] = boot_set_arch bos_payload_bootsets[boot_set_name]['arch'] = boot_set_arch