From 9437c6f465815ca30de3974d430030aca35704e7 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 2 Sep 2021 16:59:47 -0400 Subject: [PATCH] Avoid accessing delegated vars when you dont have a host (#75524) (#75534) * avoid getting delegated vars w/o inventory host fixes #75512 In the case of imports, we don't have a host, so getting host vars for the delegated host makes no sense and should be avoided. * also avoid error on vars_files with per host vars * test * testing given case * oops (cherry picked from commit 91319c5cfc523fb9dfb343be81ff373ec394818a) --- changelogs/fragments/fix_syntax_check.yml | 2 ++ lib/ansible/vars/manager.py | 18 ++++++++++-------- test/integration/targets/cli/runme.sh | 2 ++ .../targets/cli/test_syntax/files/vaultsecret | 1 + .../test_syntax/group_vars/all/testvault.yml | 6 ++++++ .../test_syntax/roles/some_role/tasks/main.yml | 1 + .../targets/cli/test_syntax/syntax_check.yml | 7 +++++++ 7 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/fix_syntax_check.yml create mode 100644 test/integration/targets/cli/test_syntax/files/vaultsecret create mode 100644 test/integration/targets/cli/test_syntax/group_vars/all/testvault.yml create mode 100644 test/integration/targets/cli/test_syntax/roles/some_role/tasks/main.yml create mode 100644 test/integration/targets/cli/test_syntax/syntax_check.yml diff --git a/changelogs/fragments/fix_syntax_check.yml b/changelogs/fragments/fix_syntax_check.yml new file mode 100644 index 00000000000..4a2a0d4fc1f --- /dev/null +++ b/changelogs/fragments/fix_syntax_check.yml @@ -0,0 +1,2 @@ +bugfixes: + - variable manager, avoid sourcing delegated variables when no inventory hostname is present. This affects scenarios like syntax check and imports. diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py index 8b2a151dde9..2c22ad93d2b 100644 --- a/lib/ansible/vars/manager.py +++ b/lib/ansible/vars/manager.py @@ -328,6 +328,9 @@ class VariableManager: for vars_file_item in vars_files: # create a set of temporary vars here, which incorporate the extra # and magic vars so we can properly template the vars_files entries + # NOTE: this makes them depend on host vars/facts so things like + # ansible_facts['os_distribution'] can be used, ala include_vars. + # Consider DEPRECATING this in the future, since we have include_vars ... temp_vars = combine_vars(all_vars, self._extra_vars) temp_vars = combine_vars(temp_vars, magic_variables) templar = Templar(loader=self._loader, variables=temp_vars) @@ -363,9 +366,9 @@ class VariableManager: except AnsibleParserError: raise else: - # if include_delegate_to is set to False, we ignore the missing - # vars file here because we're working on a delegated host - if include_delegate_to: + # 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) except (UndefinedError, AnsibleUndefinedVariable): if host is not None and self._fact_cache.get(host.name, dict()).get('module_setup') and task is not None: @@ -431,9 +434,9 @@ class VariableManager: # has to be copy, otherwise recursive ref all_vars['vars'] = all_vars.copy() - # if we have a task and we're delegating to another host, figure out the - # variables for that host now so we don't have to rely on hostvars later - if task and task.delegate_to is not None and include_delegate_to: + # if we have a host and task and we're delegating to another host, + # figure out the variables for that host now so we don't have to rely on host vars later + if task and host and task.delegate_to is not None and include_delegate_to: all_vars['ansible_delegated_vars'], all_vars['_ansible_loop_cache'] = self._get_delegated_vars(play, task, all_vars) display.debug("done with get_vars()") @@ -443,8 +446,7 @@ class VariableManager: else: return all_vars - def _get_magic_variables(self, play, host, task, include_hostvars, include_delegate_to, - _hosts=None, _hosts_all=None): + def _get_magic_variables(self, play, host, task, include_hostvars, include_delegate_to, _hosts=None, _hosts_all=None): ''' Returns a dictionary of so-called "magic" variables in Ansible, which are special variables we set internally for use. diff --git a/test/integration/targets/cli/runme.sh b/test/integration/targets/cli/runme.sh index d9e846256ff..c4f0867a1fa 100755 --- a/test/integration/targets/cli/runme.sh +++ b/test/integration/targets/cli/runme.sh @@ -5,3 +5,5 @@ set -eux ANSIBLE_ROLES_PATH=../ ansible-playbook setup.yml python test-cli.py + +ansible-playbook test_syntax/syntax_check.yml --syntax-check -i ../../inventory -v "$@" diff --git a/test/integration/targets/cli/test_syntax/files/vaultsecret b/test/integration/targets/cli/test_syntax/files/vaultsecret new file mode 100644 index 00000000000..9daeafb9864 --- /dev/null +++ b/test/integration/targets/cli/test_syntax/files/vaultsecret @@ -0,0 +1 @@ +test diff --git a/test/integration/targets/cli/test_syntax/group_vars/all/testvault.yml b/test/integration/targets/cli/test_syntax/group_vars/all/testvault.yml new file mode 100644 index 00000000000..dab41f81565 --- /dev/null +++ b/test/integration/targets/cli/test_syntax/group_vars/all/testvault.yml @@ -0,0 +1,6 @@ +$ANSIBLE_VAULT;1.1;AES256 +63666636353064303132316633383734303731353066643832633666616162373531306563616139 +3038343765633138376561336530366162646332353132620a643661663366336237636562393662 +61656465393864613832383565306133656332656534326530346638336165346264386666343266 +3066336331313830310a666265653532643434303233306635366635616261373166613564326238 +62306134303765306537396162623232396639316239616534613631336166616561 diff --git a/test/integration/targets/cli/test_syntax/roles/some_role/tasks/main.yml b/test/integration/targets/cli/test_syntax/roles/some_role/tasks/main.yml new file mode 100644 index 00000000000..307927cc4cd --- /dev/null +++ b/test/integration/targets/cli/test_syntax/roles/some_role/tasks/main.yml @@ -0,0 +1 @@ +- debug: msg='in role' diff --git a/test/integration/targets/cli/test_syntax/syntax_check.yml b/test/integration/targets/cli/test_syntax/syntax_check.yml new file mode 100644 index 00000000000..8e26cf021d7 --- /dev/null +++ b/test/integration/targets/cli/test_syntax/syntax_check.yml @@ -0,0 +1,7 @@ +- name: "main" + hosts: all + gather_facts: false + tasks: + - import_role: + name: some_role + delegate_to: testhost