Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update TaskInclude _raw_params with the expanded/templated path to file #39365

Merged
merged 2 commits into from Apr 26, 2018
Merged

Update TaskInclude _raw_params with the expanded/templated path to file #39365

merged 2 commits into from Apr 26, 2018

Conversation

sivel
Copy link
Member

@sivel sivel commented Apr 26, 2018

SUMMARY

Update TaskInclude _raw_params with the expanded/templated path to file

Fixes #34665

Before this change, if a parent include used a hostvar in the path for the included file, an AnsibleUndefinedVariable error would happen, due to ansible.playbook.helpers.load_list_of_tasks not having host context.

This PR resolves this by updating _raw_params of TaskIncludes as their results are processed, so that load_list_of_tasks has an expanded/templated path.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/playbook/included_file.py

ANSIBLE VERSION
2.5
2.6
ADDITIONAL INFORMATION

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels Apr 26, 2018
@ansibot
Copy link
Contributor

ansibot commented Apr 26, 2018

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

.github/BOTMETA.yml:437:3: key-duplicates duplication of key "$modules/net_tools/nios/" in mapping

click here for bot help

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Apr 26, 2018
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Apr 26, 2018
@sivel
Copy link
Member Author

sivel commented Apr 26, 2018 •
edited

Additional context about this change: #34665 (comment)

The problem is that, in order for a relatively pathed included file to be found, we look at the file of the parent include, to ensure we start with the correct directory.

In this case, the path of the parent include is defined as {{ ansible_system }}.yml. Because that contains a variable, we then try to template it.

When we try to template it in ansible.playbook.helpers.load_list_of_tasks we no longer have host context, and as such, we cannot template ansible_system as we have no record of that variable.

If we were to have defined ansible_system in another way, we would have gotten an error indicating that a_file_that_does_not_exist.yml does not exist.

jimi-c
jimi-c approved these changes Apr 26, 2018
View reviewed changes
Copy link
Member

@jimi-c jimi-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sivel sivel merged commit 4b01b92 into ansible:devel Apr 26, 2018
@sivel sivel mentioned this pull request Apr 26, 2018
Closed
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 12, 2018
@sivel
Update TaskInclude _raw_params with the expanded/templated path to f…
f4737db 
…ile (ansible#39365)

* Update TaskInclude _raw_params with the expanded/templated path to file

* Add tests to validate host vars include paths
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 14, 2018
@sivel
Update TaskInclude _raw_params with the expanded/templated path to f…
cf5a0cc 
…ile (ansible#39365)

* Update TaskInclude _raw_params with the expanded/templated path to file

* Add tests to validate host vars include paths
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 14, 2018
@sivel
Update TaskInclude _raw_params with the expanded/templated path to f…
72028ec 
…ile (ansible#39365)

* Update TaskInclude _raw_params with the expanded/templated path to file

* Add tests to validate host vars include paths
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 15, 2018
@sivel
Update TaskInclude _raw_params with the expanded/templated path to f…
36a538c 
…ile (ansible#39365)

* Update TaskInclude _raw_params with the expanded/templated path to file

* Add tests to validate host vars include paths
oolongbrothers pushed a commit to oolongbrothers/ansible that referenced this pull request May 15, 2018
@sivel
Update TaskInclude _raw_params with the expanded/templated path to f…
98abb1b 
…ile (ansible#39365)

* Update TaskInclude _raw_params with the expanded/templated path to file

* Add tests to validate host vars include paths
@sivel sivel mentioned this pull request May 30, 2018
Closed
sivel added a commit to sivel/ansible that referenced this pull request Jun 6, 2018
@sivel
Revert " Update TaskInclude _raw_params with the expanded/templated p…
f7a9d9a 
…ath to file (ansible#39365)"

This reverts commit 4b01b92.
@sivel sivel mentioned this pull request Jun 6, 2018
Merged
sivel added a commit to sivel/ansible that referenced this pull request Jun 7, 2018
@sivel
Revert ansible#39365, improve error messaging (ansible#41208)
72b1808 
* Revert " Update TaskInclude _raw_params with the expanded/templated path to file (ansible#39365)"

This reverts commit 4b01b92.

* Improve error messaging, catch error templating parent path

(cherry picked from commit c403f01)
sivel added a commit to sivel/ansible that referenced this pull request Jun 7, 2018
@sivel
Remove changelog entry for reverted ansible#39365
47fb238
mattclay pushed a commit that referenced this pull request Jun 7, 2018
@sivel @mattclay
Revert #39365, improve error messaging (#41208)
094c9e7 
* Revert " Update TaskInclude _raw_params with the expanded/templated path to file (#39365)"

This reverts commit 4b01b92.

* Improve error messaging, catch error templating parent path

(cherry picked from commit c403f01)
mattclay pushed a commit that referenced this pull request Jun 7, 2018
@sivel @mattclay
Remove changelog entry for reverted #39365
f74312e
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
@sivel
Revert ansible#39365, improve error messaging (ansible#41208)
db44c8f 
* Revert " Update TaskInclude _raw_params with the expanded/templated path to file (ansible#39365)"

This reverts commit 4b01b92.

* Improve error messaging, catch error templating parent path
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
@sivel @ilicmilan
Update TaskInclude _raw_params with the expanded/templated path to f…
5330d4d 
…ile (ansible#39365)

* Update TaskInclude _raw_params with the expanded/templated path to file

* Add tests to validate host vars include paths
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jimi-c jimi-c jimi-c approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Errors can be wrongly replaced on dynamically imported static include
4 participants
@sivel @ansibot @jimi-c @samdoran