From 2dcd35b692af37b9fec0d13dacb9e7bf85a8d32d Mon Sep 17 00:00:00 2001 From: Matt Davis <6775756+nitzmahone@users.noreply.github.com> Date: Mon, 18 Aug 2025 09:58:32 -0700 Subject: [PATCH] [stable-2.19] Backward-compatible None handling in template concat and argspec str (#85652) (#85663) * Backward-compatible None handling in template concat and argspec str (#85652) * templating coerces None to empty string on multi-node result * avoid simple cases of embedded `None` in multi-node string concatenated template results ala <=2.18 * single-node template results preserve NoneType * add None->empty str equivalency to argspec validation * fix integration tests * remove conversion error message check from apt_repository test * remove error message check on `None` value for required str argspec in roles_arg_spec test (now logically-equivalent to empty string) * explanatory comment for None->empty str coalesce (cherry picked from commit e3c990867950f983463d5caa435668887ecc04d7) * eliminate None template nodes in _flatten_nodes (#85676) * defers value or concat choice until Nones are gone * fixes None -> empty string for TemplateModule cases * add tests (cherry picked from commit 5345ac99118d0035a853dd50093b9e81ee0b8ea9) --- .../fragments/concat_coerce_none_to_empty.yml | 3 +++ .../_internal/_templating/_jinja_bits.py | 2 ++ lib/ansible/module_utils/common/validation.py | 5 +++- .../targets/apt_repository/tasks/apt.yml | 2 +- .../targets/roles_arg_spec/test.yml | 23 ------------------- .../_internal/templating/test_templar.py | 18 +++++++++++++++ .../common/validation/test_check_type_str.py | 3 ++- 7 files changed, 30 insertions(+), 26 deletions(-) create mode 100644 changelogs/fragments/concat_coerce_none_to_empty.yml diff --git a/changelogs/fragments/concat_coerce_none_to_empty.yml b/changelogs/fragments/concat_coerce_none_to_empty.yml new file mode 100644 index 00000000000..9fea388973a --- /dev/null +++ b/changelogs/fragments/concat_coerce_none_to_empty.yml @@ -0,0 +1,3 @@ +bugfixes: + - templating - Multi-node template results coerce embedded ``None`` nodes to empty string (instead of rendering literal ``None`` to the output). + - argspec validation - The ``str`` argspec type treats ``None`` values as empty string for better consistency with pre-2.19 templating conversions. diff --git a/lib/ansible/_internal/_templating/_jinja_bits.py b/lib/ansible/_internal/_templating/_jinja_bits.py index 1190bbef60f..6c2166fa3c3 100644 --- a/lib/ansible/_internal/_templating/_jinja_bits.py +++ b/lib/ansible/_internal/_templating/_jinja_bits.py @@ -891,6 +891,8 @@ def _flatten_nodes(nodes: t.Iterable[t.Any]) -> t.Iterable[t.Any]: else: if type(node) is TemplateModule: # pylint: disable=unidiomatic-typecheck yield from _flatten_nodes(node._body_stream) + elif node is None: + continue # avoid yielding `None`-valued nodes to avoid literal "None" in stringified template results else: yield node diff --git a/lib/ansible/module_utils/common/validation.py b/lib/ansible/module_utils/common/validation.py index f5d5f5a061f..85e49465e57 100644 --- a/lib/ansible/module_utils/common/validation.py +++ b/lib/ansible/module_utils/common/validation.py @@ -376,7 +376,10 @@ def check_type_str(value, allow_conversion=True, param=None, prefix=''): if isinstance(value, string_types): return value - if allow_conversion and value is not None: + if value is None: + return '' # approximate pre-2.19 templating None->empty str equivalency here for backward compatibility + + if allow_conversion: return to_native(value, errors='surrogate_or_strict') msg = "'{0!r}' is not a string and conversion is not allowed".format(value) diff --git a/test/integration/targets/apt_repository/tasks/apt.yml b/test/integration/targets/apt_repository/tasks/apt.yml index 9d51e16e4bd..f1706ea0302 100644 --- a/test/integration/targets/apt_repository/tasks/apt.yml +++ b/test/integration/targets/apt_repository/tasks/apt.yml @@ -301,7 +301,7 @@ - assert: that: - result is failed - - result.msg.startswith("argument 'repo' is of type NoneType and we were unable to convert to str") + - result.msg == 'Please set argument \'repo\' to a non-empty value' - name: Test apt_repository with an empty value for repo apt_repository: diff --git a/test/integration/targets/roles_arg_spec/test.yml b/test/integration/targets/roles_arg_spec/test.yml index 73f797140e4..2c24fc481d3 100644 --- a/test/integration/targets/roles_arg_spec/test.yml +++ b/test/integration/targets/roles_arg_spec/test.yml @@ -188,29 +188,6 @@ c_list: [] c_raw: ~ tasks: - - name: test type coercion fails on None for required str - block: - - name: "Test import_role of role C (missing a_str)" - import_role: - name: c - vars: - a_str: ~ - - fail: - msg: "Should not get here" - rescue: - - debug: - var: ansible_failed_result - - name: "Validate import_role failure" - assert: - that: - # NOTE: a bug here that prevents us from getting ansible_failed_task - - ansible_failed_result.argument_errors == [error] - - ansible_failed_result.argument_spec_data == a_main_spec - vars: - error: >- - argument 'a_str' is of type NoneType and we were unable to convert to str: - 'None' is not a string and conversion is not allowed - - name: test type coercion fails on None for required int block: - name: "Test import_role of role C (missing c_int)" diff --git a/test/units/_internal/templating/test_templar.py b/test/units/_internal/templating/test_templar.py index c062d3f6b53..8da9bfc0a7d 100644 --- a/test/units/_internal/templating/test_templar.py +++ b/test/units/_internal/templating/test_templar.py @@ -27,6 +27,7 @@ import typing as t import pytest_mock from jinja2.runtime import Context +from jinja2.loaders import DictLoader import unittest @@ -1080,6 +1081,23 @@ def test_marker_from_test_plugin() -> None: assert TemplateEngine(variables=dict(something=TRUST.tag("{{ nope }}"))).template(TRUST.tag("{{ (something is eq {}) is undefined }}")) +@pytest.mark.parametrize("template,expected", ( + ("{{ none }}", None), # concat sees one node, NoneType result is preserved + ("{% if False %}{% endif %}", None), # concat sees one node, NoneType result is preserved + ("{{''}}{% if False %}{% endif %}", ""), # multiple blocks with an embedded None result, concat is in play, the result is an empty string + ("hey {{ none }}", "hey "), # composite template, the result is an empty string + ("{% import 'importme' as imported %}{{ imported }}", "imported template result"), +)) +def test_none_concat(template: str, expected: object) -> None: + """Validate that None values are omitted from composite template concat.""" + te = TemplateEngine() + + # set up an importable template to exercise TemplateModule code paths + te.environment.loader = DictLoader(dict(importme=TRUST.tag("{{ none }}{{ 'imported template result' }}{{ none }}"))) + + assert te.template(TRUST.tag(template)) == expected + + def test_filter_generator() -> None: """Verify that filters which return a generator are converted to a list while under the filter's JinjaCallContext.""" variables = dict( diff --git a/test/units/module_utils/common/validation/test_check_type_str.py b/test/units/module_utils/common/validation/test_check_type_str.py index 4381ad1fd04..8ea8b23a0e0 100644 --- a/test/units/module_utils/common/validation/test_check_type_str.py +++ b/test/units/module_utils/common/validation/test_check_type_str.py @@ -12,6 +12,7 @@ from ansible.module_utils.common.validation import check_type_str, _check_type_s TEST_CASES = ( ('string', 'string'), + (None, '',), # 2.19+ relaxed restriction on None<->empty for backward compatibility (100, '100'), (1.5, '1.5'), ({'k1': 'v1'}, "{'k1': 'v1'}"), @@ -25,7 +26,7 @@ def test_check_type_str(value, expected): assert expected == check_type_str(value) -@pytest.mark.parametrize('value, expected', TEST_CASES[1:]) +@pytest.mark.parametrize('value, expected', TEST_CASES[2:]) def test_check_type_str_no_conversion(value, expected): with pytest.raises(TypeError) as e: _check_type_str_no_conversion(value)