From 17d52c8d647c4181922db42c91dc2828cdd79387 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Thu, 30 Jun 2022 21:05:39 +0200 Subject: [PATCH] Move undefined check from concat to finalize (#78165) * Move undefined check from concat to finalize In the classic Jinja2's Environment str() is called on the return value of the finalize method to potentially trigger the undefined error. That is not the case in NativeEnvironment where string conversion of the return value is not desired. We workaround that by checking for Undefined in all of our concat functions. It seems simpler to do it earlier in the finalize method(s) instead. As a side-effect it fixes an undefined variable detection in imported templates. Fixes #78156 ci_complete * Fix sanity * ... * sigh --- .../78156-undefined-check-in-finalize.yml | 2 ++ lib/ansible/template/__init__.py | 26 ++++++++++++-- lib/ansible/template/native_helpers.py | 36 +++---------------- test/integration/targets/template/runme.sh | 2 ++ .../template/undefined_in_import-import.j2 | 1 + .../targets/template/undefined_in_import.j2 | 1 + .../targets/template/undefined_in_import.yml | 11 ++++++ 7 files changed, 46 insertions(+), 33 deletions(-) create mode 100644 changelogs/fragments/78156-undefined-check-in-finalize.yml create mode 100644 test/integration/targets/template/undefined_in_import-import.j2 create mode 100644 test/integration/targets/template/undefined_in_import.j2 create mode 100644 test/integration/targets/template/undefined_in_import.yml diff --git a/changelogs/fragments/78156-undefined-check-in-finalize.yml b/changelogs/fragments/78156-undefined-check-in-finalize.yml new file mode 100644 index 00000000000..c4fb0030c96 --- /dev/null +++ b/changelogs/fragments/78156-undefined-check-in-finalize.yml @@ -0,0 +1,2 @@ +bugfixes: + - Move undefined check from concat to finalize (https://github.com/ansible/ansible/issues/78156) diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 31cc1fa4ec2..1809a696177 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -609,6 +609,28 @@ def get_fqcr_and_name(resource, collection='ansible.builtin'): return fqcr, name +def _fail_on_undefined(data): + """Recursively find an undefined value in a nested data structure + and properly raise the undefined exception. + """ + if isinstance(data, Mapping): + for value in data.values(): + _fail_on_undefined(value) + elif is_sequence(data): + for item in data: + _fail_on_undefined(item) + else: + if isinstance(data, StrictUndefined): + # To actually raise the undefined exception we need to + # access the undefined object otherwise the exception would + # be raised on the next access which might not be properly + # handled. + # See https://github.com/ansible/ansible/issues/52158 + # and StrictUndefined implementation in upstream Jinja2. + str(data) + return data + + @_unroll_iterator def _ansible_finalize(thing): """A custom finalize function for jinja2, which prevents None from being @@ -622,7 +644,7 @@ def _ansible_finalize(thing): which can produce a generator in the middle of a template are already wrapped with ``_unroll_generator`` in ``JinjaPluginIntercept``. """ - return thing if thing is not None else '' + return thing if _fail_on_undefined(thing) is not None else '' class AnsibleEnvironment(NativeEnvironment): @@ -651,7 +673,7 @@ class AnsibleNativeEnvironment(AnsibleEnvironment): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.finalize = _unroll_iterator(lambda thing: thing) + self.finalize = _unroll_iterator(_fail_on_undefined) class Templar: diff --git a/lib/ansible/template/native_helpers.py b/lib/ansible/template/native_helpers.py index e306ec32015..b6fc37b57dc 100644 --- a/lib/ansible/template/native_helpers.py +++ b/lib/ansible/template/native_helpers.py @@ -10,10 +10,7 @@ import ast from itertools import islice, chain from types import GeneratorType -from jinja2.runtime import StrictUndefined - from ansible.module_utils._text import to_text -from ansible.module_utils.common.collections import is_sequence, Mapping from ansible.module_utils.six import string_types from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode from ansible.utils.native_jinja import NativeJinjaText @@ -34,29 +31,6 @@ class Json2Python(ast.NodeTransformer): return ast.Constant(value=_JSON_MAP[node.id]) -def _fail_on_undefined(data): - """Recursively find an undefined value in a nested data structure - and properly raise the undefined exception. - """ - if isinstance(data, Mapping): - for value in data.values(): - _fail_on_undefined(value) - elif is_sequence(data): - for item in data: - _fail_on_undefined(item) - else: - if isinstance(data, StrictUndefined): - # To actually raise the undefined exception we need to - # access the undefined object otherwise the exception would - # be raised on the next access which might not be properly - # handled. - # See https://github.com/ansible/ansible/issues/52158 - # and StrictUndefined implementation in upstream Jinja2. - str(data) - - return data - - def ansible_eval_concat(nodes): """Return a string of concatenated compiled nodes. Throw an undefined error if any of the nodes is undefined. @@ -73,7 +47,7 @@ def ansible_eval_concat(nodes): return '' if len(head) == 1: - out = _fail_on_undefined(head[0]) + out = head[0] if isinstance(out, NativeJinjaText): return out @@ -82,7 +56,7 @@ def ansible_eval_concat(nodes): else: if isinstance(nodes, GeneratorType): nodes = chain(head, nodes) - out = ''.join([to_text(_fail_on_undefined(v)) for v in nodes]) + out = ''.join([to_text(v) for v in nodes]) # if this looks like a dictionary, list or bool, convert it to such if out.startswith(('{', '[')) or out in ('True', 'False'): @@ -111,7 +85,7 @@ def ansible_concat(nodes): Used in Templar.template() when jinja2_native=False and convert_data=False. """ - return ''.join([to_text(_fail_on_undefined(v)) for v in nodes]) + return ''.join([to_text(v) for v in nodes]) def ansible_native_concat(nodes): @@ -129,7 +103,7 @@ def ansible_native_concat(nodes): return None if len(head) == 1: - out = _fail_on_undefined(head[0]) + out = head[0] # TODO send unvaulted data to literal_eval? if isinstance(out, AnsibleVaultEncryptedUnicode): @@ -151,7 +125,7 @@ def ansible_native_concat(nodes): else: if isinstance(nodes, GeneratorType): nodes = chain(head, nodes) - out = ''.join([to_text(_fail_on_undefined(v)) for v in nodes]) + out = ''.join([to_text(v) for v in nodes]) try: return ast.literal_eval( diff --git a/test/integration/targets/template/runme.sh b/test/integration/targets/template/runme.sh index 14cfaf992bd..7a07c4fb021 100755 --- a/test/integration/targets/template/runme.sh +++ b/test/integration/targets/template/runme.sh @@ -42,3 +42,5 @@ ansible-playbook unsafe.yml -v "$@" ansible-playbook in_template_overrides.yml -v "$@" ansible-playbook lazy_eval.yml -i ../../inventory -v "$@" + +ansible-playbook undefined_in_import.yml -i ../../inventory -v "$@" diff --git a/test/integration/targets/template/undefined_in_import-import.j2 b/test/integration/targets/template/undefined_in_import-import.j2 new file mode 100644 index 00000000000..fbb97b0dd3e --- /dev/null +++ b/test/integration/targets/template/undefined_in_import-import.j2 @@ -0,0 +1 @@ +{{ undefined_variable }} diff --git a/test/integration/targets/template/undefined_in_import.j2 b/test/integration/targets/template/undefined_in_import.j2 new file mode 100644 index 00000000000..619e4f708b1 --- /dev/null +++ b/test/integration/targets/template/undefined_in_import.j2 @@ -0,0 +1 @@ +{% import 'undefined_in_import-import.j2' as t %} diff --git a/test/integration/targets/template/undefined_in_import.yml b/test/integration/targets/template/undefined_in_import.yml new file mode 100644 index 00000000000..62f60d6687c --- /dev/null +++ b/test/integration/targets/template/undefined_in_import.yml @@ -0,0 +1,11 @@ +- hosts: localhost + gather_facts: false + tasks: + - debug: + msg: "{{ lookup('template', 'undefined_in_import.j2') }}" + ignore_errors: true + register: res + + - assert: + that: + - "\"'undefined_variable' is undefined\" in res.msg"