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.

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

TaskExecutor: update_vars (add magic vars) before post_validate #22156

Closed

Conversation

fullyint
Copy link
Contributor

@fullyint fullyint commented Mar 1, 2017

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

TaskExecutor

ANSIBLE VERSION
ansible 2.3.0 (devel c26cac2a53) last updated 2017/03/01 10:55:21 (GMT -600)
  config file =
  configured module search path = Default w/o overrides
SUMMARY

Fixes #21639

Concept. Magic vars are not available for consumption (are undefined) in the explicit redefinition of other magic vars.

Example. Suppose a user's definition of ansible_become_pass includes ansible_user. Despite passing the --user CLI option, the definition below will produce 'ansible_user' is undefined.

ansible_become_pass: "{{ passwords[ansible_user] }}"

Proposed solution. This PR avoids the error by restoring the sequence of update_vars() executing before post_validate(), as existed prior to 479cbfc. This in unfamiliar code for me, so a team member familiar with TaskExecutor may want to review/confirm this revised execution sequence.

Aside: It appears good to keep the remote_addr update before the update_vars() so that the latter can add the ansible_ssh_host and ansible_host magic vars.

Steps to reproduce
Test playbook:

---
- name: No problem if NON-magic var is defined using magic var
  hosts: test # NOT localhost
  gather_facts: false
  vars:
    passwords: # normally in vault-encrypted file
      admin: strong_pass
    non_magic_var: "{{ passwords[ansible_user] }}"
  tasks:
    - debug: var=non_magic_var

- name: Failure when magic var is redefined using magic var
  hosts: test # NOT localhost
  gather_facts: false
  vars:
    passwords: # normally in vault-encrypted file
      admin: strong_pass
    ansible_become_pass: "{{ passwords[ansible_user] }}"
  tasks:
    - debug: var=ansible_become_pass

Test command:

ansible-playbook debug.yml -i hosts -u admin

Output from current devel:

PLAY [No problem if NON-magic var is defined using magic var] ******************

TASK [debug] *******************************************************************
ok: [test] => {
    "changed": false,
    "non_magic_var": "strong_pass"
}

PLAY [Failure when magic var is redefined using magic var] *********************

TASK [debug] *******************************************************************
fatal: [test]: FAILED! => {"failed": true, "msg": "the field 'become_pass' has an invalid value, which appears to include a variable that is undefined. The error was: 'ansible_user' is undefined"}
	to retry, use: --limit @/Users/philip/projects/ansible/ansible/debug.retry

PLAY RECAP *********************************************************************
test                       : ok=1    changed=0    unreachable=0    failed=1

Output after PR:

PLAY [No problem if NON-magic var is defined using magic var] ******************

TASK [debug] *******************************************************************
ok: [test] => {
    "changed": false,
    "non_magic_var": "strong_pass"
}

PLAY [Failure when magic var is redefined using magic var] *********************

TASK [debug] *******************************************************************
ok: [test] => {
    "ansible_become_pass": "strong_pass",
    "changed": false
}

PLAY RECAP *********************************************************************
test                       : ok=2    changed=0    unreachable=0    failed=0

* Fixes ansible#21639 - a failure of "'varname' is undefined" resulting from redefining
  a magic var with a value that includes another magic var.
* Avoids the error by restoring the sequence of `update_vars()` executing before
  `post_validate()`, as existed prior to 479cbfc.
@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 bugfix_pull_request c:executor/task_executor needs_triage Needs a first human triage before being processed. labels Mar 1, 2017
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Mar 3, 2017
@ansibot ansibot added the affects_2.4 This issue/PR affects Ansible v2.4 label Mar 23, 2017
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 11, 2017
@ansibot ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jun 29, 2017
self._play_context.post_validate(templar=templar)

# now that the play context is finalized, if the remote_addr is not set
# default to using the host's address field as the remote address
Copy link
Member

Choose a reason for hiding this comment

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

we cannot set remote_addr w/o post_validating first

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 17, 2017

# now that the play context is finalized, if the remote_addr is not set
# default to using the host's address field as the remote address
# if the remote_addr is not set, default to using the host's address field
if not self._play_context.remote_addr:
self._play_context.remote_addr = self._host.address

# We also add "magic" variables back into the variables dict to make sure
# a certain subset of variables exist.
self._play_context.update_vars(variables)
Copy link
Member

Choose a reason for hiding this comment

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

some of the magic vars rely on post_validate already being run to have the correct value

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Sep 10, 2017
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 2, 2018
@bcoca
Copy link
Member

bcoca commented Nov 30, 2018

alternate fix #38861

@samdoran
Copy link
Contributor

Closing since this was resolved by #38861.

@samdoran samdoran closed this Jun 26, 2020
@ansible ansible locked and limited conversation to collaborators Jul 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. c:executor/task_executor executor/task_executor needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot override built-in connection variable ansible_become_pass
6 participants