From e3c990867950f983463d5caa435668887ecc04d7 Mon Sep 17 00:00:00 2001 From: Matt Davis <6775756+nitzmahone@users.noreply.github.com> Date: Wed, 13 Aug 2025 15:05:01 -0700 Subject: [PATCH] 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 --- .../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 | 10 ++++++++ .../common/validation/test_check_type_str.py | 3 ++- 7 files changed, 21 insertions(+), 27 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 0000000000..9fea388973 --- /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 1190bbef60..54b1eef682 100644 --- a/lib/ansible/_internal/_templating/_jinja_bits.py +++ b/lib/ansible/_internal/_templating/_jinja_bits.py @@ -753,7 +753,7 @@ class AnsibleEnvironment(SandboxedEnvironment): except MarkerError as ex: return ex.source # return the first Marker encountered - return ''.join([to_text(v) for v in node_list]) + return ''.join([to_text(v) for v in node_list if v is not None]) # skip concat on `None`-valued nodes to avoid literal "None" in template results @staticmethod def _access_const(const_template: t.LiteralString) -> t.Any: diff --git a/lib/ansible/module_utils/common/validation.py b/lib/ansible/module_utils/common/validation.py index 498248c0ff..81100cd5bc 100644 --- a/lib/ansible/module_utils/common/validation.py +++ b/lib/ansible/module_utils/common/validation.py @@ -374,7 +374,10 @@ def check_type_str(value, allow_conversion=True, param=None, prefix=''): if isinstance(value, str): 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 9d51e16e4b..f1706ea030 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 73f797140e..2c24fc481d 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 c062d3f6b5..9d6a193c6f 100644 --- a/test/units/_internal/templating/test_templar.py +++ b/test/units/_internal/templating/test_templar.py @@ -1080,6 +1080,16 @@ 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 +)) +def test_none_concat(template: str, expected: object) -> None: + assert TemplateEngine().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 4381ad1fd0..8ea8b23a0e 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)