From 82b0ee21f7d84f41efccfbe69bf988af599d7ce0 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 21 Jun 2018 17:50:24 -0400 Subject: [PATCH] exclude lookup_terms from config errors (#41740) * exclude lookup_terms from config errors * moved direct (cherry picked from commit 0102e422722afbadb65e93bcdd1b8f2fe80794b4) --- changelogs/fragments/fix_config_required.yml | 2 + lib/ansible/config/manager.py | 133 +++++++++++-------- lib/ansible/plugins/__init__.py | 9 +- lib/ansible/plugins/callback/__init__.py | 8 +- lib/ansible/plugins/lookup/vars.py | 1 - 5 files changed, 78 insertions(+), 75 deletions(-) create mode 100644 changelogs/fragments/fix_config_required.yml diff --git a/changelogs/fragments/fix_config_required.yml b/changelogs/fragments/fix_config_required.yml new file mode 100644 index 00000000000..10773157532 --- /dev/null +++ b/changelogs/fragments/fix_config_required.yml @@ -0,0 +1,2 @@ +bugfixes: + - fixed config required handling, specifically for _terms in lookups https://github.com/ansible/ansible/pull/41740 diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index 36f8a6a3c51..df82b4d854e 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -30,6 +30,8 @@ from ansible.utils.path import makedirs_safe Plugin = namedtuple('Plugin', 'name type') Setting = namedtuple('Setting', 'name value origin type') +INTERNAL_DEFS = {'lookup': ('_terms',)} + # FIXME: see if we can unify in module_utils with similar function used by argspec def ensure_type(value, value_type, origin=None): @@ -215,6 +217,11 @@ class ConfigManager(object): if cfile is not None: if ftype == 'ini': self._parsers[cfile] = configparser.ConfigParser() + with open(cfile, 'rb') as f: + try: + cfg_text = to_text(f.read(), errors='surrogate_or_strict') + except UnicodeError as e: + raise AnsibleOptionsError("Error reading config file(%s) because the config file was not utf8 encoded: %s" % (cfile, to_native(e))) try: self._parsers[cfile].read(cfile) except configparser.Error as e: @@ -230,12 +237,12 @@ class ConfigManager(object): ''' Load YAML Config Files in order, check merge flags, keep origin of settings''' pass - def get_plugin_options(self, plugin_type, name, keys=None, variables=None): + def get_plugin_options(self, plugin_type, name, keys=None, variables=None, direct=None): options = {} defs = self.get_configuration_definitions(plugin_type, name) for option in defs: - options[option] = self.get_config_value(option, plugin_type=plugin_type, plugin_name=name, keys=keys, variables=variables) + options[option] = self.get_config_value(option, plugin_type=plugin_type, plugin_name=name, keys=keys, variables=variables, direct=direct) return options @@ -279,17 +286,17 @@ class ConfigManager(object): return value, origin - def get_config_value(self, config, cfile=None, plugin_type=None, plugin_name=None, keys=None, variables=None): + def get_config_value(self, config, cfile=None, plugin_type=None, plugin_name=None, keys=None, variables=None, direct=None): ''' wrapper ''' try: value, _drop = self.get_config_value_and_origin(config, cfile=cfile, plugin_type=plugin_type, plugin_name=plugin_name, - keys=keys, variables=variables) + keys=keys, variables=variables, direct=direct) except Exception as e: raise AnsibleError("Invalid settings supplied for %s: %s" % (config, to_native(e))) return value - def get_config_value_and_origin(self, config, cfile=None, plugin_type=None, plugin_name=None, keys=None, variables=None): + def get_config_value_and_origin(self, config, cfile=None, plugin_type=None, plugin_name=None, keys=None, variables=None, direct=None): ''' Given a config key figure out the actual value and report on the origin of the settings ''' if cfile is None: @@ -308,60 +315,68 @@ class ConfigManager(object): defs = self._plugins[plugin_type][plugin_name] if config in defs: - # 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 keys: - value, origin = self._loop_entries(keys, defs[config]['keywords']) - origin = 'keyword: %s' % origin - - # env vars are next precedence - if value is None and defs[config].get('env'): - value, origin = self._loop_entries(os.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): - entry = '' - if plugin_type: - entry += 'plugin_type: %s ' % plugin_type - if plugin_name: - entry += 'plugin: %s ' % plugin_name - entry += 'setting: %s ' % config - raise AnsibleError("No setting was provided for required configuration %s" % (entry)) - else: - value = defs[config].get('default') - origin = 'default' - # skip typing as this is a temlated 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 + + # direct setting via plugin arguments, can set to None so we bypass rest of processing/defaults + if direct and config in direct: + value = direct[config] + origin = 'Direct' + + else: + # 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 keys and defs[config].get('keywords'): + value, origin = self._loop_entries(keys, defs[config]['keywords']) + origin = 'keyword: %s' % origin + + # env vars are next precedence + if value is None and defs[config].get('env'): + value, origin = self._loop_entries(os.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): + entry = '' + if plugin_type: + entry += 'plugin_type: %s ' % plugin_type + if plugin_name: + entry += 'plugin: %s ' % plugin_name + entry += 'setting: %s ' % config + if not plugin_type or config not in INTERNAL_DEFS.get(plugin_type, {}): + raise AnsibleError("No setting was provided for required configuration %s" % (entry)) + else: + value = defs[config].get('default') + origin = 'default' + # skip typing as this is a temlated 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 # ensure correct type, can raise exceptoins on mismatched types value = ensure_type(value, defs[config].get('type'), origin=origin) diff --git a/lib/ansible/plugins/__init__.py b/lib/ansible/plugins/__init__.py index 8eefd0bf5a5..dc629effb7e 100644 --- a/lib/ansible/plugins/__init__.py +++ b/lib/ansible/plugins/__init__.py @@ -73,14 +73,7 @@ class AnsiblePlugin(with_metaclass(ABCMeta, object)): if not self._options: # load config options if we have not done so already, if vars provided we should be mostly done - self._options = C.config.get_plugin_options(get_plugin_class(self), self._load_name, keys=task_keys, variables=var_options) - - # they can be direct options overriding config - if direct: - for k in self._options: - if k in direct: - self.set_option(k, direct[k]) - + self._options = C.config.get_plugin_options(get_plugin_class(self), self._load_name, keys=task_keys, variables=var_options, direct=direct) # allow extras/wildcards from vars that are not directly consumed in configuration if self.allow_extras and var_options and '_extras' in var_options: self.set_option('_extras', var_options['_extras']) diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py index 2a56911b6bb..e56bf8b702b 100644 --- a/lib/ansible/plugins/callback/__init__.py +++ b/lib/ansible/plugins/callback/__init__.py @@ -98,13 +98,7 @@ class CallbackBase(AnsiblePlugin): ''' # load from config - self._plugin_options = C.config.get_plugin_options(get_plugin_class(self), self._load_name, keys=task_keys, variables=var_options) - - # or parse specific options - if direct: - for k in direct: - if k in self._plugin_options: - self.set_option(k, direct[k]) + self._plugin_options = C.config.get_plugin_options(get_plugin_class(self), self._load_name, keys=task_keys, variables=var_options, direct=direct) def _dump_results(self, result, indent=None, sort_keys=True, keep_invocation=False): diff --git a/lib/ansible/plugins/lookup/vars.py b/lib/ansible/plugins/lookup/vars.py index c238cf57219..fd6d23b5b64 100644 --- a/lib/ansible/plugins/lookup/vars.py +++ b/lib/ansible/plugins/lookup/vars.py @@ -69,7 +69,6 @@ class LookupModule(LookupBase): self._templar.set_available_variables(variables) myvars = getattr(self._templar, '_available_variables', {}) - self.set_option('_terms', terms) self.set_options(direct=kwargs) default = self.get_option('default')