Merge pull request #280 from Cray-HPE/CRAYSAT-1917-fix-jinja2-rendering

CRAYSAT-1917: Fix jinja2 rendering of boot_sets data in `sat bootprep`

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

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.

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

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

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