[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 e3c9908679)

* 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 5345ac9911)

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.

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
 

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)

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:

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

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(

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)