From 11279a909dc441dcef07e4104fd1320c4ba4fb22 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Tue, 30 Apr 2019 11:10:20 -0400 Subject: [PATCH] fix combine filter using undefined vars (#55840) * Check variables are defined before using combine filter * Add tests for the combine filter * Remove dependencies that should already be installed * relocate the function to recursively check for undefined vars add another test * changelog --- ...40-combine-filter-undefined-variables.yaml | 2 ++ lib/ansible/plugins/filter/core.py | 3 ++ lib/ansible/template/__init__.py | 14 ++++++++ .../targets/filters/tasks/main.yml | 34 +++++++++++++++++++ 4 files changed, 53 insertions(+) create mode 100644 changelogs/fragments/55840-combine-filter-undefined-variables.yaml diff --git a/changelogs/fragments/55840-combine-filter-undefined-variables.yaml b/changelogs/fragments/55840-combine-filter-undefined-variables.yaml new file mode 100644 index 0000000000..6b50d0904b --- /dev/null +++ b/changelogs/fragments/55840-combine-filter-undefined-variables.yaml @@ -0,0 +1,2 @@ +bugfixes: + - combine filter - Validate that undefined variables aren't used (https://github.com/ansible/ansible/issues/55810). diff --git a/lib/ansible/plugins/filter/core.py b/lib/ansible/plugins/filter/core.py index a739e2bd5f..4ceb4a6c81 100644 --- a/lib/ansible/plugins/filter/core.py +++ b/lib/ansible/plugins/filter/core.py @@ -48,6 +48,7 @@ from ansible.module_utils.common.collections import is_sequence from ansible.module_utils.common._collections_compat import Mapping, MutableMapping from ansible.parsing.ajson import AnsibleJSONEncoder from ansible.parsing.yaml.dumper import AnsibleDumper +from ansible.template import recursive_check_defined from ansible.utils.display import Display from ansible.utils.encrypt import passlib_or_crypt from ansible.utils.hashing import md5s, checksum_s @@ -300,8 +301,10 @@ def combine(*terms, **kwargs): dicts = [] for t in terms: if isinstance(t, MutableMapping): + recursive_check_defined(t) dicts.append(t) elif isinstance(t, list): + recursive_check_defined(t) dicts.append(combine(*t, **kwargs)) else: raise AnsibleFilterError("|combine expects dictionaries, got " + repr(t)) diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index d754335d5e..599d9edbd8 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -178,6 +178,20 @@ def _count_newlines_from_end(in_str): return i +def recursive_check_defined(item): + from jinja2.runtime import Undefined + + if isinstance(item, MutableMapping): + for key in item: + recursive_check_defined(item[key]) + elif isinstance(item, list): + for i in item: + recursive_check_defined(i) + else: + if isinstance(item, Undefined): + raise AnsibleFilterError("{0} is undefined".format(item)) + + class AnsibleUndefined(StrictUndefined): ''' A custom Undefined class, which returns further Undefined objects on access, diff --git a/test/integration/targets/filters/tasks/main.yml b/test/integration/targets/filters/tasks/main.yml index 2e17daaef7..b5b59a7691 100644 --- a/test/integration/targets/filters/tasks/main.yml +++ b/test/integration/targets/filters/tasks/main.yml @@ -277,3 +277,37 @@ loop: "{{ hostvars|dict2items }}" loop_control: label: "{{ item.key }}" + +- name: Ensure combining two dictionaries containing undefined variables provides a helpful error + block: + - set_fact: + foo: + key1: value1 + + - set_fact: + combined: "{{ foo | combine({'key2': undef_variable}) }}" + ignore_errors: yes + register: result + + - assert: + that: + - "result.msg.startswith('The task includes an option with an undefined variable')" + + - set_fact: + combined: "{{ foo | combine({'key2': {'nested': [undef_variable]}})}}" + ignore_errors: yes + register: result + + - assert: + that: + - "result.msg.startswith('The task includes an option with an undefined variable')" + + - set_fact: + key2: is_defined + + - set_fact: + combined: "{{ foo | combine({'key2': key2}) }}" + + - assert: + that: + - "combined.key2 == 'is_defined'"