From b617d622038dcf0be593a9870f0e286a6c0ef31c Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 17 Aug 2016 14:44:06 -0500 Subject: [PATCH] Don't use an unset playbook basedir when searching for hostgroup vars The flag new_pb_basedir is not being utilized in Inventory._get_hostgroup_vars, leading to the situation where an inventory with no playbook basedir set will read host/group vars from the $CWD, regardless of the inventory and/or playbook relative location. This patch corrects that by not using the playbook basedir if it is unset (None). This patch also corrects a bug in which the VariableManager would accumulate host/group vars files, which could lead to incorrect vars files being used when playbooks are run from different directories containing their own group/host vars directories. Fixes #16953 --- lib/ansible/executor/playbook_executor.py | 2 +- lib/ansible/inventory/__init__.py | 10 ++++++++-- lib/ansible/vars/__init__.py | 17 +++++++++++++++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/ansible/executor/playbook_executor.py b/lib/ansible/executor/playbook_executor.py index 3d64407d07..03a551fb40 100644 --- a/lib/ansible/executor/playbook_executor.py +++ b/lib/ansible/executor/playbook_executor.py @@ -72,7 +72,7 @@ class PlaybookExecutor: try: for playbook_path in self._playbooks: pb = Playbook.load(playbook_path, variable_manager=self._variable_manager, loader=self._loader) - self._inventory.set_playbook_basedir(os.path.dirname(playbook_path)) + self._inventory.set_playbook_basedir(os.path.realpath(os.path.dirname(playbook_path))) if self._tqm is None: # we are doing a listing entry = {'playbook': playbook_path} diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 0e5324edd9..f76702133d 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -717,6 +717,12 @@ class Inventory(object): """ # Only update things if dir is a different playbook basedir if dir_name != self._playbook_basedir: + # we're changing the playbook basedir, so if we had set one previously + # clear the host/group vars entries from the VariableManager so they're + # not incorrectly used by playbooks from different directories + if self._playbook_basedir: + self._variable_manager.clear_playbook_hostgroup_vars_files(self._playbook_basedir) + self._playbook_basedir = dir_name # get group vars from group_vars/ files # TODO: excluding the new_pb_basedir directory may result in group_vars @@ -782,10 +788,10 @@ class Inventory(object): # look in both the inventory base directory and the playbook base directory # unless we do an update for a new playbook base dir - if not new_pb_basedir: + if not new_pb_basedir and _playbook_basedir: basedirs = [_basedir, _playbook_basedir] else: - basedirs = [_playbook_basedir] + basedirs = [_basedir] for basedir in basedirs: # this can happen from particular API usages, particularly if not run diff --git a/lib/ansible/vars/__init__.py b/lib/ansible/vars/__init__.py index 4975839d6b..793e346ada 100644 --- a/lib/ansible/vars/__init__.py +++ b/lib/ansible/vars/__init__.py @@ -21,8 +21,7 @@ __metaclass__ = type import os -from collections import defaultdict -from collections import MutableMapping +from collections import defaultdict, MutableMapping from ansible.compat.six import iteritems from jinja2.exceptions import UndefinedError @@ -619,6 +618,20 @@ class VariableManager: return data + def clear_playbook_hostgroup_vars_files(self, path): + for f in self._host_vars_files.keys(): + keepers = [] + for entry in self._host_vars_files[f]: + if os.path.dirname(entry.path) != os.path.join(path, 'host_vars'): + keepers.append(entry) + self._host_vars_files[f] = keepers + for f in self._group_vars_files.keys(): + keepers = [] + for entry in self._group_vars_files[f]: + if os.path.dirname(entry.path) != os.path.join(path, 'group_vars'): + keepers.append(entry) + self._group_vars_files[f] = keepers + def clear_facts(self, hostname): ''' Clears the facts for a host