Don't use an unset playbook basedir when searching for hostgroup varsissue_16953

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

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