From c403f01971bc305ee1c9b47b5b65bc14b37dabd2 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 7 Jun 2018 09:00:50 -0500 Subject: [PATCH] Revert #39365, improve error messaging (#41208) * Revert " Update TaskInclude _raw_params with the expanded/templated path to file (#39365)" This reverts commit 4b01b92cfefd2179d7b601a30e60a3fdf45fa35d. * Improve error messaging, catch error templating parent path --- lib/ansible/playbook/helpers.py | 20 +++++++++++++++---- lib/ansible/playbook/included_file.py | 2 -- .../one/include_me.yml | 2 -- .../include_path_inheritance/playbook.yml | 14 ------------- .../two/include_me.yml | 2 -- .../targets/include_import/runme.sh | 4 ---- 6 files changed, 16 insertions(+), 28 deletions(-) delete mode 100644 test/integration/targets/include_import/include_path_inheritance/one/include_me.yml delete mode 100644 test/integration/targets/include_import/include_path_inheritance/playbook.yml delete mode 100644 test/integration/targets/include_import/include_path_inheritance/two/include_me.yml diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index 14b4f480d53..1667f5c42e0 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -189,7 +189,19 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h if not isinstance(parent_include, TaskInclude): parent_include = parent_include._parent continue - parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params'))) + try: + parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params'))) + except AnsibleUndefinedVariable as e: + if not parent_include.statically_loaded: + raise AnsibleParserError( + "Error when evaluating variable in dynamic parent include path: %s. " + "When using static imports, the parent dynamic include cannot utilize host facts " + "or variables from inventory" % parent_include.args.get('_raw_params'), + obj=task_ds, + suppress_extended_error=True, + orig_exc=e + ) + raise if cumulative_path is None: cumulative_path = parent_include_dir elif not os.path.isabs(cumulative_path): @@ -212,9 +224,9 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h include_target = templar.template(t.args['_raw_params']) except AnsibleUndefinedVariable as e: raise AnsibleParserError( - "Error when evaluating variable in include name: %s.\n\n" - "When using static includes, ensure that any variables used in their names are defined in vars/vars_files\n" - "or extra-vars passed in from the command line. Static includes cannot use variables from inventory\n" + "Error when evaluating variable in import path: %s.\n\n" + "When using static imports, ensure that any variables used in their names are defined in vars/vars_files\n" + "or extra-vars passed in from the command line. Static imports cannot use variables from facts or inventory\n" "sources like group or host vars." % t.args['_raw_params'], obj=task_ds, suppress_extended_error=True, diff --git a/lib/ansible/playbook/included_file.py b/lib/ansible/playbook/included_file.py index 7681ba65740..db4f89eae3d 100644 --- a/lib/ansible/playbook/included_file.py +++ b/lib/ansible/playbook/included_file.py @@ -144,8 +144,6 @@ class IncludedFile: include_file = loader.path_dwim(include_result['include']) include_file = templar.template(include_file) - # Update the task args to reflect the expanded/templated path - original_task.args['_raw_params'] = include_file inc_file = IncludedFile(include_file, include_variables, original_task) else: # template the included role's name here diff --git a/test/integration/targets/include_import/include_path_inheritance/one/include_me.yml b/test/integration/targets/include_import/include_path_inheritance/one/include_me.yml deleted file mode 100644 index ea345d28527..00000000000 --- a/test/integration/targets/include_import/include_path_inheritance/one/include_me.yml +++ /dev/null @@ -1,2 +0,0 @@ -- debug: - msg: one diff --git a/test/integration/targets/include_import/include_path_inheritance/playbook.yml b/test/integration/targets/include_import/include_path_inheritance/playbook.yml deleted file mode 100644 index b92e0f5d588..00000000000 --- a/test/integration/targets/include_import/include_path_inheritance/playbook.yml +++ /dev/null @@ -1,14 +0,0 @@ -- hosts: testhost:testhost2 - tasks: - - set_fact: - include_me: one - when: inventory_hostname == ansible_play_hosts[0] - - - set_fact: - include_me: two - when: inventory_hostname == ansible_play_hosts[1] - - - debug: - var: include_me - - - include_tasks: '{{ include_me }}/include_me.yml' diff --git a/test/integration/targets/include_import/include_path_inheritance/two/include_me.yml b/test/integration/targets/include_import/include_path_inheritance/two/include_me.yml deleted file mode 100644 index 96855323a0e..00000000000 --- a/test/integration/targets/include_import/include_path_inheritance/two/include_me.yml +++ /dev/null @@ -1,2 +0,0 @@ -- debug: - msg: two diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index b8a8a4ce9b3..af1abb15634 100755 --- a/test/integration/targets/include_import/runme.sh +++ b/test/integration/targets/include_import/runme.sh @@ -64,10 +64,6 @@ ANSIBLE_STRATEGY='linear' ansible-playbook test_grandparent_inheritance.yml -i . ANSIBLE_STRATEGY='linear' ansible-playbook undefined_var/playbook.yml -i ../../inventory "$@" ANSIBLE_STRATEGY='free' ansible-playbook undefined_var/playbook.yml -i ../../inventory "$@" -# Include path inheritance using host var for include file path -ANSIBLE_STRATEGY='linear' ansible-playbook include_path_inheritance/playbook.yml -i ../../inventory "$@" -ANSIBLE_STRATEGY='free' ansible-playbook include_path_inheritance/playbook.yml -i ../../inventory "$@" - # include_ + apply (explicit inheritance) ANSIBLE_STRATEGY='linear' ansible-playbook apply/include_apply.yml -i ../../inventory "$@" --tags foo set +e