diff --git a/changelogs/fragments/64379-no-loop-unsafe.yml b/changelogs/fragments/64379-no-loop-unsafe.yml new file mode 100644 index 00000000000..5b00b0915b4 --- /dev/null +++ b/changelogs/fragments/64379-no-loop-unsafe.yml @@ -0,0 +1,5 @@ +bugfixes: +- loops - Do not indiscriminately mark loop items as unsafe, only apply unsafe + to ``with_`` style loops. The items from ``loop`` should not be explicitly + wrapped in unsafe. The underlying templating mechanism should dictate this. + (https://github.com/ansible/ansible/issues/64379) diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.9.rst b/docs/docsite/rst/porting_guides/porting_guide_2.9.rst index fcc555290a6..105dc3ea1cd 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.9.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.9.rst @@ -19,8 +19,19 @@ This document is part of a collection on porting. The complete list of porting g Playbook ======== +Inventory +--------- + * ``hash_behaviour`` now affects inventory sources. If you have it set to ``merge``, the data you get from inventory might change and you will have to update playbooks accordingly. If you're using the default setting (``overwrite``), you will see no changes. Inventory was ignoring this setting. +Loops +----- + +Ansible 2.9 handles "unsafe" data more robustly, ensuring that data marked "unsafe" is not templated. In previous versions, Ansible recursively marked all data returned by the direct use of ``lookup()`` as "unsafe", but only marked structured data returned by indirect lookups using ``with_X`` style loops as "unsafe" if the returned elements were strings. Ansible 2.9 treats these two approaches consistently. + +As a result, if you use ``with_dict`` to return keys with templatable values, your templates may no longer work as expected in Ansible 2.9. + +To allow the old behavior, switch from using ``with_X`` to using ``loop`` with a filter as described at :ref:`migrating_to_loop`. Command Line ============ diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index bcf919a1968..0516479adc2 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -242,7 +242,7 @@ class TaskExecutor: setattr(mylookup, '_subdir', subdir + 's') # run lookup - items = mylookup.run(terms=loop_terms, variables=self._job_vars, wantlist=True) + items = wrap_var(mylookup.run(terms=loop_terms, variables=self._job_vars, wantlist=True)) else: raise AnsibleError("Unexpected failure in finding the lookup named '%s' in the available lookup plugins" % self._task.loop_with) @@ -264,11 +264,6 @@ class TaskExecutor: else: del self._job_vars[k] - if items: - for idx, item in enumerate(items): - if item is not None and not isinstance(item, AnsibleUnsafe): - items[idx] = wrap_var(item) - return items def _run_loop(self, items): diff --git a/lib/ansible/utils/unsafe_proxy.py b/lib/ansible/utils/unsafe_proxy.py index 3c74ea83699..28fb10c5cb0 100644 --- a/lib/ansible/utils/unsafe_proxy.py +++ b/lib/ansible/utils/unsafe_proxy.py @@ -115,7 +115,7 @@ def _wrap_set(v): def wrap_var(v): - if isinstance(v, AnsibleUnsafe): + if v is None or isinstance(v, AnsibleUnsafe): return v if isinstance(v, Mapping): diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py index 21fd5814ee5..3cf485c07ea 100644 --- a/lib/ansible/vars/manager.py +++ b/lib/ansible/vars/manager.py @@ -509,7 +509,7 @@ class VariableManager: try: loop_terms = listify_lookup_plugin_terms(terms=task.loop, templar=templar, loader=self._loader, fail_on_undefined=True, convert_bare=False) - items = lookup_loader.get(task.loop_with, loader=self._loader, templar=templar).run(terms=loop_terms, variables=vars_copy) + items = wrap_var(lookup_loader.get(task.loop_with, loader=self._loader, templar=templar).run(terms=loop_terms, variables=vars_copy)) except AnsibleTemplateError: # This task will be skipped later due to this, so we just setup # a dummy array for the later code so it doesn't fail diff --git a/test/integration/targets/loops/tasks/main.yml b/test/integration/targets/loops/tasks/main.yml index 5dd7d26fe95..5575dd36497 100644 --- a/test/integration/targets/loops/tasks/main.yml +++ b/test/integration/targets/loops/tasks/main.yml @@ -359,3 +359,33 @@ - assert: that: - loop_out['results'][1]['ansible_remote_tmp'] == '/tmp/test1' + +# https://github.com/ansible/ansible/issues/64169 +- include_vars: 64169.yml + +- set_fact: "{{ item.key }}={{ hostvars[inventory_hostname][item.value] }}" + with_dict: + foo: __foo + +- debug: + var: foo + +- assert: + that: + - foo[0] != 'foo1.0' + - foo[0] == unsafe_value + vars: + unsafe_value: !unsafe 'foo{{ version_64169 }}' + +- set_fact: "{{ item.key }}={{ hostvars[inventory_hostname][item.value] }}" + loop: "{{ dicty_dict|dict2items }}" + vars: + dicty_dict: + foo: __foo + +- debug: + var: foo + +- assert: + that: + - foo[0] == 'foo1.0' diff --git a/test/integration/targets/loops/vars/64169.yml b/test/integration/targets/loops/vars/64169.yml new file mode 100644 index 00000000000..f48d616ac79 --- /dev/null +++ b/test/integration/targets/loops/vars/64169.yml @@ -0,0 +1,2 @@ +__foo: + - "foo{{ version_64169 }}" diff --git a/test/integration/targets/loops/vars/main.yml b/test/integration/targets/loops/vars/main.yml index 2d85bf824bb..5d85370d4cc 100644 --- a/test/integration/targets/loops/vars/main.yml +++ b/test/integration/targets/loops/vars/main.yml @@ -5,3 +5,4 @@ phrases: filenames: - 'data1.txt' - 'data2.txt' +version_64169: '1.0'