move extravars and option vars loading into VM (#51070)

* move extravars and option vars loading into VM

  also safedir setting, all these are intrinsic to VM
  avoid uneeded and inefectual shallow copy
  remove setters/getters as VM now does most of the work in init
  updated and added tests

* feedback + fixes

* keep extra_vars property for vars_prompt

* pass values not objects

changelogs/fragments/vm_updates.yml

@@ -0,0 +1,2 @@
+minor_changes:
+    - moved some operations to inside VariableManager to make using it simpler and slightly optimized, but creating API changes

lib/ansible/cli/__init__.py

@@ -27,7 +27,6 @@ from ansible.parsing.dataloader import DataLoader
 from ansible.release import __version__
 from ansible.utils.display import Display
 from ansible.utils.path import unfrackpath
-from ansible.utils.vars import load_extra_vars, load_options_vars
 from ansible.vars.manager import VariableManager
 from ansible.parsing.vault import PromptVaultSecret, get_file_vault_secret
 
@@ -520,19 +519,7 @@ class CLI(with_metaclass(ABCMeta, object)):
 
         # create the variable manager, which will be shared throughout
         # the code, ensuring a consistent view of global variables
-        variable_manager = VariableManager(loader=loader, inventory=inventory)
+        variable_manager = VariableManager(loader=loader, inventory=inventory, version_info=CLI.version_info(gitinfo=False))
-
-        # If the basedir is specified as the empty string then it results in cwd being used.  This
-        # is not a safe location to load vars from
-        if options.get('basedir', False) is not False:
-            if basedir:
-                variable_manager.safe_basedir = True
-        else:
-            variable_manager.safe_basedir = True
-
-        # load vars from cli options
-        variable_manager.extra_vars = load_extra_vars(loader=loader)
-        variable_manager.options_vars = load_options_vars(CLI.version_info(gitinfo=False))
 
         return loader, inventory, variable_manager
 

lib/ansible/utils/vars.py

@@ -145,6 +145,8 @@ def load_extra_vars(loader):
 
 def load_options_vars(version):
 
+    if version is None:
+        version = 'Unknown'
     options_vars = {'ansible_version': version}
     attrs = {'check': 'check_mode',
              'diff': 'diff_mode',

lib/ansible/vars/manager.py

@@ -43,7 +43,7 @@ from ansible.vars.fact_cache import FactCache
 from ansible.template import Templar
 from ansible.utils.display import Display
 from ansible.utils.listify import listify_lookup_plugin_terms
-from ansible.utils.vars import combine_vars
+from ansible.utils.vars import combine_vars, load_extra_vars, load_options_vars
 from ansible.utils.unsafe_proxy import wrap_var
 from ansible.vars.clean import namespace_facts, clean_facts
 
@@ -76,7 +76,7 @@ class VariableManager:
     _ALLOWED = frozenset(['plugins_by_group', 'groups_plugins_play', 'groups_plugins_inventory', 'groups_inventory',
                           'all_plugins_play', 'all_plugins_inventory', 'all_inventory'])
 
-    def __init__(self, loader=None, inventory=None):
+    def __init__(self, loader=None, inventory=None, version_info=None):
         self._nonpersistent_fact_cache = defaultdict(dict)
         self._vars_cache = defaultdict(dict)
         self._extra_vars = defaultdict(dict)
@@ -86,16 +86,25 @@ class VariableManager:
         self._loader = loader
         self._hostvars = None
         self._omit_token = '__omit_place_holder__%s' % sha1(os.urandom(64)).hexdigest()
-        self._options_vars = defaultdict(dict)
-        self.safe_basedir = False
         self._templar = Templar(loader=self._loader)
 
-        # bad cache plugin is not fatal error
+        self._options_vars = load_options_vars(version_info)
+
+        # If the basedir is specified as the empty string then it results in cwd being used.
+        # This is not a safe location to load vars from.
+        basedir = self._options_vars.get('basedir', False)
+        self.safe_basedir = bool(basedir is False or basedir)
+
+        # load extra vars
+        self._extra_vars = load_extra_vars(loader=self._loader)
+
+        # load fact cache
         try:
             self._fact_cache = FactCache()
         except AnsibleError as e:
-            display.warning(to_native(e))
+            # bad cache plugin is not fatal error
             # fallback to a dict as in memory cache
+            display.warning(to_native(e))
             self._fact_cache = {}
 
     def __getstate__(self):
@@ -127,31 +136,11 @@ class VariableManager:
 
     @property
     def extra_vars(self):
-        ''' ensures a clean copy of the extra_vars are made '''
+        return self._extra_vars
-        return self._extra_vars.copy()
-
-    @extra_vars.setter
-    def extra_vars(self, value):
-        ''' ensures a clean copy of the extra_vars are used to set the value '''
-        if not isinstance(value, Mapping):
-            raise AnsibleAssertionError("the type of 'value' for extra_vars should be a Mapping, but is a %s" % type(value))
-        self._extra_vars = value.copy()
 
     def set_inventory(self, inventory):
         self._inventory = inventory
 
-    @property
-    def options_vars(self):
-        ''' ensures a clean copy of the options_vars are made '''
-        return self._options_vars.copy()
-
-    @options_vars.setter
-    def options_vars(self, value):
-        ''' ensures a clean copy of the options_vars are used to set the value '''
-        if not isinstance(value, dict):
-            raise AnsibleAssertionError("the type of 'value' for options_vars should be a dict, but is a %s" % type(value))
-        self._options_vars = value.copy()
-
     def get_vars(self, play=None, host=None, task=None, include_hostvars=True, include_delegate_to=True, use_cache=True):
         '''
         Returns the variables, with optional "context" given via the parameters

test/units/vars/test_variable_manager.py

@@ -24,10 +24,9 @@ import os
 from collections import defaultdict
 
 from units.compat import unittest
-from units.compat.mock import MagicMock, mock_open, patch
+from units.compat.mock import MagicMock, patch
 from ansible.inventory.manager import InventoryManager
 from ansible.module_utils.six import iteritems
-from ansible.module_utils.six.moves import builtins
 from ansible.playbook.play import Play
 
 
@@ -63,14 +62,27 @@ class TestVariableManager(unittest.TestCase):
         extra_vars = dict(a=1, b=2, c=3)
         mock_inventory = MagicMock()
         v = VariableManager(loader=fake_loader, inventory=mock_inventory)
-        v.extra_vars = extra_vars
 
-        vars = v.get_vars(use_cache=False)
+        # override internal extra_vars loading
+        v._extra_vars = extra_vars
 
+        myvars = v.get_vars(use_cache=False)
         for (key, val) in iteritems(extra_vars):
-            self.assertEqual(vars.get(key), val)
+            self.assertEqual(myvars.get(key), val)
 
-        self.assertIsNot(v.extra_vars, extra_vars)
+    def test_variable_manager_options_vars(self):
+        fake_loader = DictDataLoader({})
+
+        options_vars = dict(a=1, b=2, c=3)
+        mock_inventory = MagicMock()
+        v = VariableManager(loader=fake_loader, inventory=mock_inventory)
+
+        # override internal options_vars loading
+        v._extra_vars = options_vars
+
+        myvars = v.get_vars(use_cache=False)
+        for (key, val) in iteritems(options_vars):
+            self.assertEqual(myvars.get(key), val)
 
     def test_variable_manager_play_vars(self):
         fake_loader = DictDataLoader({})