exclude lookup_terms from config errors (#41740)

* exclude lookup_terms from config errors
* moved direct

(cherry picked from commit 0102e42272)

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

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'):
+            # direct setting via plugin arguments, can set to None so we bypass rest of processing/defaults
-                value, origin = self._loop_entries(variables, defs[config]['vars'])
+            if direct and config in direct:
-                origin = 'var: %s' % origin
+                value = direct[config]
-
+                origin = 'Direct'
-            # use playbook keywords if you have em
+
-            if value is None and keys:
+            else:
-                value, origin = self._loop_entries(keys, defs[config]['keywords'])
+                # Use 'variable overrides' if present, highest precedence, but only present when querying running play
-                origin = 'keyword: %s' % origin
+                if variables and defs[config].get('vars'):
-
+                    value, origin = self._loop_entries(variables, defs[config]['vars'])
-            # env vars are next precedence
+                    origin = 'var: %s' % origin
-            if value is None and defs[config].get('env'):
+
-                value, origin = self._loop_entries(os.environ, defs[config]['env'])
+                # use playbook keywords if you have em
-                origin = 'env: %s' % origin
+                if value is None and keys and defs[config].get('keywords'):
-
+                    value, origin = self._loop_entries(keys, defs[config]['keywords'])
-            # try config file entries next, if we have one
+                    origin = 'keyword: %s' % origin
-            if self._parsers.get(cfile, None) is None:
+
-                self._parse_config_file(cfile)
+                # env vars are next precedence
-
+                if value is None and defs[config].get('env'):
-            if value is None and cfile is not None:
+                    value, origin = self._loop_entries(os.environ, defs[config]['env'])
-                ftype = get_config_type(cfile)
+                    origin = 'env: %s' % origin
-                if ftype and defs[config].get(ftype):
+
-                    if ftype == 'ini':
+                # try config file entries next, if we have one
-                        # load from ini config
+                if self._parsers.get(cfile, None) is None:
-                        try:  # FIXME: generalize _loop_entries to allow for files also, most of this code is dupe
+                    self._parse_config_file(cfile)
-                            for ini_entry in defs[config]['ini']:
+
-                                temp_value = get_ini_config_value(self._parsers[cfile], ini_entry)
+                if value is None and cfile is not None:
-                                if temp_value is not None:
+                    ftype = get_config_type(cfile)
-                                    value = temp_value
+                    if ftype and defs[config].get(ftype):
-                                    origin = cfile
+                        if ftype == 'ini':
-                                    if 'deprecated' in ini_entry:
+                            # load from ini config
-                                        self.DEPRECATED.append(('[%s]%s' % (ini_entry['section'], ini_entry['key']), ini_entry['deprecated']))
+                            try:  # FIXME: generalize _loop_entries to allow for files also, most of this code is dupe
-                        except Exception as e:
+                                for ini_entry in defs[config]['ini']:
-                            sys.stderr.write("Error while loading ini config %s: %s" % (cfile, to_native(e)))
+                                    temp_value = get_ini_config_value(self._parsers[cfile], ini_entry)
-                    elif ftype == 'yaml':
+                                    if temp_value is not None:
-                        # FIXME: implement, also , break down key from defs (. notation???)
+                                        value = temp_value
-                        origin = cfile
+                                        origin = cfile
-
+                                        if 'deprecated' in ini_entry:
-            # set default if we got here w/o a value
+                                            self.DEPRECATED.append(('[%s]%s' % (ini_entry['section'], ini_entry['key']), ini_entry['deprecated']))
-            if value is None:
+                            except Exception as e:
-                if defs[config].get('required', False):
+                                sys.stderr.write("Error while loading ini config %s: %s" % (cfile, to_native(e)))
-                    entry = ''
+                        elif ftype == 'yaml':
-                    if plugin_type:
+                            # FIXME: implement, also , break down key from defs (. notation???)
-                        entry += 'plugin_type: %s ' % plugin_type
+                            origin = cfile
-                        if plugin_name:
+
-                            entry += 'plugin: %s ' % plugin_name
+                # set default if we got here w/o a value
-                    entry += 'setting: %s ' % config
+                if value is None:
-                    raise AnsibleError("No setting was provided for required configuration %s" % (entry))
+                    if defs[config].get('required', False):
-                else:
+                        entry = ''
-                    value = defs[config].get('default')
+                        if plugin_type:
-                    origin = 'default'
+                            entry += 'plugin_type: %s ' % plugin_type
-                    # skip typing as this is a temlated default that will be resolved later in constants, which has needed vars
+                            if plugin_name:
-                    if plugin_type is None and isinstance(value, string_types) and (value.startswith('{{') and value.endswith('}}')):
+                                entry += 'plugin: %s ' % plugin_name
-                        return value, origin
+                        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)

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)
+            self._options = C.config.get_plugin_options(get_plugin_class(self), self._load_name, keys=task_keys, variables=var_options, direct=direct)
-
-        # they can be direct options overriding config
-        if direct:
-            for k in self._options:
-                if k in direct:
-                    self.set_option(k, direct[k])
-
         # 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'])

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)
+        self._plugin_options = C.config.get_plugin_options(get_plugin_class(self), self._load_name, keys=task_keys, variables=var_options, direct=direct)
-
-        # or parse specific options
-        if direct:
-            for k in direct:
-                if k in self._plugin_options:
-                    self.set_option(k, direct[k])
 
     def _dump_results(self, result, indent=None, sort_keys=True, keep_invocation=False):
 

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')