diff --git a/changelogs/fragments/config_manager_changes.yml b/changelogs/fragments/config_manager_changes.yml new file mode 100644 index 00000000000..0402b9e5bfe --- /dev/null +++ b/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 diff --git a/changelogs/fragments/ensure_config_always_templated.yml b/changelogs/fragments/ensure_config_always_templated.yml new file mode 100644 index 00000000000..1d91fa1e431 --- /dev/null +++ b/changelogs/fragments/ensure_config_always_templated.yml @@ -0,0 +1,2 @@ +bugfixes: + - config, ensure that pulling values from configmanager are templated if possible. diff --git a/lib/ansible/cli/config.py b/lib/ansible/cli/config.py index 75f345e0ca9..352529fc2bf 100755 --- a/lib/ansible/cli/config.py +++ b/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 diff --git a/lib/ansible/config/data.py b/lib/ansible/config/data.py deleted file mode 100644 index 6a5bb391ede..00000000000 --- a/lib/ansible/config/data.py +++ /dev/null @@ -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 diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index a28a84a528b..178b3f937b4 100644 --- a/lib/ansible/config/manager.py +++ b/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'))) diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py index afe3936baf5..36646fbef9c 100644 --- a/lib/ansible/constants.py +++ b/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) diff --git a/test/units/config/test_data.py b/test/units/config/test_data.py deleted file mode 100644 index da043e7b923..00000000000 --- a/test/units/config/test_data.py +++ /dev/null @@ -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) diff --git a/test/units/config/test_manager.py b/test/units/config/test_manager.py index a957e397491..8ef40437421 100644 --- a/test/units/config/test_manager.py +++ b/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)