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

vars_files depending on host vars no longer fails as of 2.15 with error_on_undefined_vars disabled #80483

Open
1 task done
sivel opened this issue Apr 11, 2023 · 3 comments · May be fixed by #80505
Open
1 task done
Labels
affects_2.16 affects_2.18 bug This issue/PR relates to a bug. data_tagging has_pr This issue has an associated PR.

Comments

@sivel
Copy link
Member

sivel commented Apr 11, 2023

Summary

As of #80171 VariableManager.get_vars is no longer responsible for calculating delegated_vars, and instead that is deferred to the TaskExecutor to call VariableManager.get_delegated_vars_and_hostname explicitly. As a result include_delegate_to changed it's default to False since it no longer handles this functionality.

However, when loading vars_files we have some logic that made a potential non-exact determination that the failure could be related to not having a host or needing a delegated host:

# if include_delegate_to is set to False or we don't have a host, we ignore the missing
# vars file here because we're working on a delegated host or require host vars, see NOTE above
if include_delegate_to and host:
raise AnsibleFileNotFound("vars file %s was not found" % vars_file_item)

I'm pretty sure that logic wasn't capable of making the real determination anyway that the missing var was due to missing host vars or facts, and it seems to have been more a guess.

As a result, we never fail, regardless of error_on_undefined_vars being disabled.

This code is also suspect, and I don't think capable of making a proper determination of whether it was host vars vs facts, or anything else pretty much:

except (UndefinedError, AnsibleUndefinedVariable):
if host is not None and self._fact_cache.get(host.name, dict()).get('module_setup') and task is not None:
raise AnsibleUndefinedVariable("an undefined variable was found when attempting to template the vars_files item '%s'"
% vars_file_item, obj=vars_file_item)

Honestly, I'm kind of sad I went looking for this, I've waffled about ignoring it, but the fact that we have code that I don't think is actually capable of doing what it was intended to do bothers me.

Maybe we just always allow it to skip (with a message), and never make it an error, and allow the fact that any vars from that file would be missing, and would cause a failure later. Also, for the record I'm also advocating that we deprecate and remove DEFAULT_UNDEFINED_VAR_BEHAVIOR.

Issue Type

Bug Report

Component Name

lib/ansible/vars/manager.py

Ansible Version

$ ansible --version
2.15

Configuration

# if using a version older than ansible-core 2.12 you should omit the '-t all'
$ ansible-config dump --only-changed -t all
DEFAULT_UNDEFINED_VAR_BEHAVIOR(.../ansible.cfg) = False

OS / Environment

N/A

Steps to Reproduce

- hosts: localhost
  gather_facts: false
  vars_files:
    - '{{ foo }}'

Expected Results

ERROR! vars file {{ foo }} was not found
Could not find file on the Ansible Controller.
If you are using a module and expect the file to exist on the remote, see the remote_src option

Actual Results

No failure, and no indication of skipping loading the file.

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibot
Copy link
Contributor

ansibot commented Apr 11, 2023

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.15 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. labels Apr 11, 2023
@sivel
Copy link
Member Author

sivel commented Apr 11, 2023

So I guess to extend this, a few options:

  1. Never fail, always skip, rely on a task later using a var from the vars file to fail due to missing var
  2. Skip only when get_vars is called for the play itself, and always fail if there is a task/host.
  3. Fail always, but this would prevent using host_vars for later templating

@sivel

This comment was marked as outdated.

sivel added a commit to sivel/ansible that referenced this issue Apr 12, 2023
@ansibot ansibot added the has_pr This issue has an associated PR. label Apr 12, 2023
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 13, 2023
sivel added a commit to sivel/ansible that referenced this issue Aug 3, 2023
sivel added a commit to sivel/ansible that referenced this issue Apr 17, 2024
sivel added a commit to sivel/ansible that referenced this issue Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.16 affects_2.18 bug This issue/PR relates to a bug. data_tagging has_pr This issue has an associated PR.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants