Yolo (#77554)

* Revert "Revert "Config, ensure templating happens at functions (#77483)""

This reverts commit 94c9106153.

* removed update configdata, which is unused
* removed test for action we don't perform anymore
* removed unused configdata

changelogs/fragments/config_manager_changes.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - config manager, move templating into main query function in config instead of constants
+  - config manager, remove updates to configdata as it is mostly unused

changelogs/fragments/ensure_config_always_templated.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - config, ensure that pulling values from configmanager are templated if possible.

lib/ansible/cli/config.py

@@ -35,6 +35,13 @@ from ansible.utils.path import unfrackpath
 display = Display()
 
 
+def get_constants():
+    ''' helper method to ensure we can template based on existing constants '''
+    if not hasattr(get_constants, 'cvars'):
+        get_constants.cvars = {k: getattr(C, k) for k in dir(C) if not k.startswith('__')}
+    return get_constants.cvars
+
+
 class ConfigCLI(CLI):
     """ Config command line class """
 
@@ -395,9 +402,9 @@ class ConfigCLI(CLI):
 
     def _get_global_configs(self):
         config = self.config.get_configuration_definitions(ignore_private=True).copy()
-        for setting in self.config.data.get_settings():
-            if setting.name in config:
-                config[setting.name] = setting
+        for setting in config.keys():
+            v, o = C.config.get_config_value_and_origin(setting, cfile=self.config_file, variables=get_constants())
+            config[setting] = Setting(setting, v, o, None)
 
         return self._render_settings(config)
 
@@ -445,7 +452,7 @@ class ConfigCLI(CLI):
             # actually get the values
             for setting in config_entries[finalname].keys():
                 try:
-                    v, o = C.config.get_config_value_and_origin(setting, plugin_type=ptype, plugin_name=name)
+                    v, o = C.config.get_config_value_and_origin(setting, cfile=self.config_file, plugin_type=ptype, plugin_name=name, variables=get_constants())
                 except AnsibleError as e:
                     if to_text(e).startswith('No setting was provided for required configuration'):
                         v = None

lib/ansible/config/data.py

@@ -1,43 +0,0 @@
-# Copyright: (c) 2017, Ansible Project
-# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
-
-from __future__ import (absolute_import, division, print_function)
-__metaclass__ = type
-
-
-class ConfigData(object):
-
-    def __init__(self):
-        self._global_settings = {}
-        self._plugins = {}
-
-    def get_setting(self, name, plugin=None):
-
-        setting = None
-        if plugin is None:
-            setting = self._global_settings.get(name)
-        elif plugin.type in self._plugins and plugin.name in self._plugins[plugin.type]:
-            setting = self._plugins[plugin.type][plugin.name].get(name)
-
-        return setting
-
-    def get_settings(self, plugin=None):
-
-        settings = []
-        if plugin is None:
-            settings = [self._global_settings[k] for k in self._global_settings]
-        elif plugin.type in self._plugins and plugin.name in self._plugins[plugin.type]:
-            settings = [self._plugins[plugin.type][plugin.name][k] for k in self._plugins[plugin.type][plugin.name]]
-
-        return settings
-
-    def update_setting(self, setting, plugin=None):
-
-        if plugin is None:
-            self._global_settings[setting.name] = setting
-        else:
-            if plugin.type not in self._plugins:
-                self._plugins[plugin.type] = {}
-            if plugin.name not in self._plugins[plugin.type]:
-                self._plugins[plugin.type][plugin.name] = {}
-            self._plugins[plugin.type][plugin.name][setting.name] = setting

lib/ansible/config/manager.py

@@ -13,11 +13,10 @@ import stat
 import tempfile
 import traceback
 
-from collections.abc import Mapping, Sequence
-
 from collections import namedtuple
+from collections.abc import Mapping, Sequence
+from jinja2.nativetypes import NativeEnvironment
 
-from ansible.config.data import ConfigData
 from ansible.errors import AnsibleOptionsError, AnsibleError
 from ansible.module_utils._text import to_text, to_bytes, to_native
 from ansible.module_utils.common.yaml import yaml_load
@@ -287,7 +286,6 @@ class ConfigManager(object):
         self._parsers = {}
 
         self._config_file = conf_file
-        self.data = ConfigData()
 
         self._base_defs = self._read_config_yaml_file(defs_file or ('%s/base.yml' % os.path.dirname(__file__)))
         _add_base_defs_deprecations(self._base_defs)
@@ -301,8 +299,8 @@ class ConfigManager(object):
             # initialize parser and read config
             self._parse_config_file()
 
-        # update constants
-        self.update_config_data()
+        # ensure we always have config def entry
+        self._base_defs['CONFIG_FILE'] = {'default': None, 'type': 'path'}
 
     def _read_config_yaml_file(self, yml_file):
         # TODO: handle relative paths as relative to the directory containing the current playbook instead of CWD
@@ -447,6 +445,9 @@ class ConfigManager(object):
             # use default config
             cfile = self._config_file
 
+        if config == 'CONFIG_FILE':
+            return cfile, ''
+
         # Note: sources that are lists listed in low to high precedence (last one wins)
         value = None
         origin = None
@@ -457,90 +458,94 @@ class ConfigManager(object):
             aliases = defs[config].get('aliases', [])
 
             # direct setting via plugin arguments, can set to None so we bypass rest of processing/defaults
-            direct_aliases = []
             if direct:
-                direct_aliases = [direct[alias] for alias in aliases if alias in direct]
-            if direct and config in direct:
-                value = direct[config]
-                origin = 'Direct'
-            elif direct and direct_aliases:
-                value = direct_aliases[0]
-                origin = 'Direct'
+                if config in direct:
+                    value = direct[config]
+                    origin = 'Direct'
+                else:
+                    direct_aliases = [direct[alias] for alias in aliases if alias in direct]
+                    if direct_aliases:
+                        value = direct_aliases[0]
+                        origin = 'Direct'
 
-            else:
+            if value is None and variables and defs[config].get('vars'):
                 # Use 'variable overrides' if present, highest precedence, but only present when querying running play
-                if variables and defs[config].get('vars'):
-                    value, origin = self._loop_entries(variables, defs[config]['vars'])
-                    origin = 'var: %s' % origin
-
-                # use playbook keywords if you have em
-                if value is None and defs[config].get('keyword') and keys:
-                    value, origin = self._loop_entries(keys, defs[config]['keyword'])
-                    origin = 'keyword: %s' % origin
-
-                # automap to keywords
-                # TODO: deprecate these in favor of explicit keyword above
-                if value is None and keys:
-                    if config in keys:
-                        value = keys[config]
-                        keyword = config
-
-                    elif aliases:
-                        for alias in aliases:
-                            if alias in keys:
-                                value = keys[alias]
-                                keyword = alias
-                                break
-
-                    if value is not None:
-                        origin = 'keyword: %s' % keyword
-
-                if value is None and 'cli' in defs[config]:
-                    # avoid circular import .. until valid
-                    from ansible import context
-                    value, origin = self._loop_entries(context.CLIARGS, defs[config]['cli'])
-                    origin = 'cli: %s' % origin
-
-                # env vars are next precedence
-                if value is None and defs[config].get('env'):
-                    value, origin = self._loop_entries(py3compat.environ, defs[config]['env'])
-                    origin = 'env: %s' % origin
-
-                # try config file entries next, if we have one
-                if self._parsers.get(cfile, None) is None:
-                    self._parse_config_file(cfile)
-
-                if value is None and cfile is not None:
-                    ftype = get_config_type(cfile)
-                    if ftype and defs[config].get(ftype):
-                        if ftype == 'ini':
-                            # load from ini config
-                            try:  # FIXME: generalize _loop_entries to allow for files also, most of this code is dupe
-                                for ini_entry in defs[config]['ini']:
-                                    temp_value = get_ini_config_value(self._parsers[cfile], ini_entry)
-                                    if temp_value is not None:
-                                        value = temp_value
-                                        origin = cfile
-                                        if 'deprecated' in ini_entry:
-                                            self.DEPRECATED.append(('[%s]%s' % (ini_entry['section'], ini_entry['key']), ini_entry['deprecated']))
-                            except Exception as e:
-                                sys.stderr.write("Error while loading ini config %s: %s" % (cfile, to_native(e)))
-                        elif ftype == 'yaml':
-                            # FIXME: implement, also , break down key from defs (. notation???)
-                            origin = cfile
-
-                # set default if we got here w/o a value
-                if value is None:
-                    if defs[config].get('required', False):
-                        if not plugin_type or config not in INTERNAL_DEFS.get(plugin_type, {}):
-                            raise AnsibleError("No setting was provided for required configuration %s" %
-                                               to_native(_get_entry(plugin_type, plugin_name, config)))
-                    else:
-                        value = defs[config].get('default')
-                        origin = 'default'
-                        # skip typing as this is a templated default that will be resolved later in constants, which has needed vars
-                        if plugin_type is None and isinstance(value, string_types) and (value.startswith('{{') and value.endswith('}}')):
-                            return value, origin
+                value, origin = self._loop_entries(variables, defs[config]['vars'])
+                origin = 'var: %s' % origin
+
+            # use playbook keywords if you have em
+            if value is None and defs[config].get('keyword') and keys:
+                value, origin = self._loop_entries(keys, defs[config]['keyword'])
+                origin = 'keyword: %s' % origin
+
+            # automap to keywords
+            # TODO: deprecate these in favor of explicit keyword above
+            if value is None and keys:
+                if config in keys:
+                    value = keys[config]
+                    keyword = config
+
+                elif aliases:
+                    for alias in aliases:
+                        if alias in keys:
+                            value = keys[alias]
+                            keyword = alias
+                            break
+
+                if value is not None:
+                    origin = 'keyword: %s' % keyword
+
+            if value is None and 'cli' in defs[config]:
+                # avoid circular import .. until valid
+                from ansible import context
+                value, origin = self._loop_entries(context.CLIARGS, defs[config]['cli'])
+                origin = 'cli: %s' % origin
+
+            # env vars are next precedence
+            if value is None and defs[config].get('env'):
+                value, origin = self._loop_entries(py3compat.environ, defs[config]['env'])
+                origin = 'env: %s' % origin
+
+            # try config file entries next, if we have one
+            if self._parsers.get(cfile, None) is None:
+                self._parse_config_file(cfile)
+
+            if value is None and cfile is not None:
+                ftype = get_config_type(cfile)
+                if ftype and defs[config].get(ftype):
+                    if ftype == 'ini':
+                        # load from ini config
+                        try:  # FIXME: generalize _loop_entries to allow for files also, most of this code is dupe
+                            for ini_entry in defs[config]['ini']:
+                                temp_value = get_ini_config_value(self._parsers[cfile], ini_entry)
+                                if temp_value is not None:
+                                    value = temp_value
+                                    origin = cfile
+                                    if 'deprecated' in ini_entry:
+                                        self.DEPRECATED.append(('[%s]%s' % (ini_entry['section'], ini_entry['key']), ini_entry['deprecated']))
+                        except Exception as e:
+                            sys.stderr.write("Error while loading ini config %s: %s" % (cfile, to_native(e)))
+                    elif ftype == 'yaml':
+                        # FIXME: implement, also , break down key from defs (. notation???)
+                        origin = cfile
+
+            # set default if we got here w/o a value
+            if value is None:
+                if defs[config].get('required', False):
+                    if not plugin_type or config not in INTERNAL_DEFS.get(plugin_type, {}):
+                        raise AnsibleError("No setting was provided for required configuration %s" %
+                                           to_native(_get_entry(plugin_type, plugin_name, config)))
+                else:
+                    origin = 'default'
+                    value = defs[config].get('default')
+                    if isinstance(value, string_types) and (value.startswith('{{') and value.endswith('}}')) and variables is not None:
+                        # template default values if possible
+                        # NOTE: cannot use is_template due to circular dep
+                        try:
+                            t = NativeEnvironment().from_string(value)
+                            value = t.render(variables)
+                        except Exception:
+                            pass  # not templatable
 
             # ensure correct type, can raise exceptions on mismatched types
             try:
@@ -592,43 +597,3 @@ class ConfigManager(object):
             self._plugins[plugin_type] = {}
 
         self._plugins[plugin_type][name] = defs
-
-    def update_config_data(self, defs=None, configfile=None):
-        ''' really: update constants '''
-
-        if defs is None:
-            defs = self._base_defs
-
-        if configfile is None:
-            configfile = self._config_file
-
-        if not isinstance(defs, dict):
-            raise AnsibleOptionsError("Invalid configuration definition type: %s for %s" % (type(defs), defs))
-
-        # update the constant for config file
-        self.data.update_setting(Setting('CONFIG_FILE', configfile, '', 'string'))
-
-        origin = None
-        # env and config defs can have several entries, ordered in list from lowest to highest precedence
-        for config in defs:
-            if not isinstance(defs[config], dict):
-                raise AnsibleOptionsError("Invalid configuration definition '%s': type is %s" % (to_native(config), type(defs[config])))
-
-            # get value and origin
-            try:
-                value, origin = self.get_config_value_and_origin(config, configfile)
-            except Exception as e:
-                # Printing the problem here because, in the current code:
-                # (1) we can't reach the error handler for AnsibleError before we
-                #     hit a different error due to lack of working config.
-                # (2) We don't have access to display yet because display depends on config
-                #     being properly loaded.
-                #
-                # If we start getting double errors printed from this section of code, then the
-                # above problem #1 has been fixed.  Revamp this to be more like the try: except
-                # in get_config_value() at that time.
-                sys.stderr.write("Unhandled error:\n %s\n\n" % traceback.format_exc())
-                raise AnsibleError("Invalid settings supplied for %s: %s\n" % (config, to_native(e)), orig_exc=e)
-
-            # set the constant
-            self.data.update_setting(Setting(config, value, origin, defs[config].get('type', 'string')))

lib/ansible/constants.py

@@ -7,11 +7,9 @@ __metaclass__ = type
 
 import re
 
-from ast import literal_eval
-from jinja2 import Template
 from string import ascii_letters, digits
 
-from ansible.config.manager import ConfigManager, ensure_type
+from ansible.config.manager import ConfigManager
 from ansible.module_utils._text import to_text
 from ansible.module_utils.common.collections import Sequence
 from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE
@@ -181,25 +179,8 @@ MAGIC_VARIABLE_MAPPING = dict(
 config = ConfigManager()
 
 # Generate constants from config
-for setting in config.data.get_settings():
-
-    value = setting.value
-    if setting.origin == 'default' and \
-       isinstance(setting.value, string_types) and \
-       (setting.value.startswith('{{') and setting.value.endswith('}}')):
-        try:
-            t = Template(setting.value)
-            value = t.render(vars())
-            try:
-                value = literal_eval(value)
-            except ValueError:
-                pass  # not a python data structure
-        except Exception:
-            pass  # not templatable
-
-        value = ensure_type(value, setting.type)
-
-    set_constant(setting.name, value)
+for setting in config.get_configuration_definitions():
+    set_constant(setting, config.get_config_value(setting, variables=vars()))
 
 for warn in config.WARNINGS:
     _warning(warn)

test/units/config/test_data.py

@@ -1,41 +0,0 @@
-# Make coding more python3-ish
-from __future__ import (absolute_import, division, print_function)
-__metaclass__ = type
-
-from units.compat import unittest
-
-from ansible.config.data import ConfigData
-from ansible.config.manager import Setting
-
-
-mykey = Setting('mykey', 'myvalue', 'test', 'string')
-mykey2 = Setting('mykey2', 'myvalue2', ['test', 'test2'], 'list')
-mykey3 = Setting('mykey3', 'myvalue3', 11111111111, 'integer')
-
-
-class TestConfigData(unittest.TestCase):
-
-    def setUp(self):
-        self.cdata = ConfigData()
-
-    def tearDown(self):
-        self.cdata = None
-
-    def test_update_setting(self):
-        for setting in [mykey, mykey2, mykey3]:
-            self.cdata.update_setting(setting)
-            self.assertEqual(setting, self.cdata._global_settings.get(setting.name))
-
-    def test_update_setting_with_plugin(self):
-        pass
-
-    def test_get_setting(self):
-        self.cdata._global_settings = {'mykey': mykey}
-        self.assertEqual(mykey, self.cdata.get_setting('mykey'))
-
-    def test_get_settings(self):
-        all_settings = {'mykey': mykey, 'mykey2': mykey2}
-        self.cdata._global_settings = all_settings
-
-        for setting in self.cdata.get_settings():
-            self.assertEqual(all_settings[setting.name], setting)

test/units/config/test_manager.py

@@ -19,16 +19,6 @@ curdir = os.path.dirname(__file__)
 cfg_file = os.path.join(curdir, 'test.cfg')
 cfg_file2 = os.path.join(curdir, 'test2.cfg')
 
-expected_ini = {'CONFIG_FILE': Setting(name='CONFIG_FILE', value=cfg_file, origin='', type='string'),
-                'config_entry': Setting(name='config_entry', value=u'fromini', origin=cfg_file, type='string'),
-                'config_entry_bool': Setting(name='config_entry_bool', value=False, origin=cfg_file, type='bool'),
-                'config_entry_list': Setting(name='config_entry_list', value=['fromini'], origin=cfg_file, type='list'),
-                'config_entry_deprecated': Setting(name='config_entry_deprecated', value=u'fromini', origin=cfg_file, type='string'),
-                'config_entry_multi': Setting(name='config_entry_multi', value=u'morefromini', origin=cfg_file, type='string'),
-                'config_entry_multi_deprecated': Setting(name='config_entry_multi_deprecated', value=u'morefromini', origin=cfg_file, type='string'),
-                'config_entry_multi_deprecated_source': Setting(name='config_entry_multi_deprecated_source', value=u'morefromini',
-                                                                origin=cfg_file, type='string')}
-
 ensure_test_data = [
     ('a,b', 'list', list),
     (['a', 'b'], 'list', list),
@@ -85,9 +75,6 @@ class TestConfigManager:
     def teardown_class(cls):
         cls.manager = None
 
-    def test_initial_load(self):
-        assert self.manager.data._global_settings == expected_ini
-
     @pytest.mark.parametrize("value, expected_type, python_type", ensure_test_data)
     def test_ensure_type(self, value, expected_type, python_type):
         assert isinstance(ensure_type(value, expected_type), python_type)