All lookups ported to config system (#74108)

* all lookups to support config system

 - added get_options to get full dict with all opts
 - fixed tests to match new error messages
 - kept inline string k=v parsing methods for backwards compat
 - placeholder depredation for inline string k=v parsing
 - updated tests and examples to also show new way
 - refactored and added comments to most custom k=v parsing
 - added missing docs for template_vars to template
 - normalized error messages and exception types
 - fixed constants default
 - better details value errors

Co-authored-by: Felix Fontein <felix@fontein.de>

12 files changed, 191 insertions, 125 deletions

diff --git a/lib/ansible/plugins/__init__.py b/lib/ansible/plugins/__init__.py
index 73857f45c5..2bb1a4c9a2 100644
--- a/lib/ansible/plugins/__init__.py
+++ b/lib/ansible/plugins/__init__.py
@@ -61,6 +61,13 @@ class AnsiblePlugin(with_metaclass(ABCMeta, object)):
             self.set_option(option, option_value)
         return self._options.get(option)
 
+    def get_options(self, hostvars=None):
+        options = {}
+        defs = C.config.get_configuration_definitions(plugin_type=get_plugin_class(self), name=self._load_name)
+        for option in defs:
+            options[option] = self.get_option(option, hostvars=hostvars)
+        return options
+
     def set_option(self, option, value):
         self._options[option] = value
 
diff --git a/lib/ansible/plugins/lookup/__init__.py b/lib/ansible/plugins/lookup/__init__.py
index 42f0d1cc46..8a8fd8ed3a 100644
--- a/lib/ansible/plugins/lookup/__init__.py
+++ b/lib/ansible/plugins/lookup/__init__.py
@@ -123,3 +123,8 @@ class LookupBase(AnsiblePlugin):
                 self._display.warning("Unable to find '%s' in expected paths (use -vvvvv to see paths)" % needle)
 
         return result
+
+    def _deprecate_inline_kv(self):
+        # TODO: place holder to deprecate in future version allowing for long transition period
+        # self._display.deprecated('Passing inline k=v values embeded in a string to this lookup. Use direct ,k=v, k2=v2 syntax instead.', version='2.18')
+        pass
diff --git a/lib/ansible/plugins/lookup/config.py b/lib/ansible/plugins/lookup/config.py
index 2b66881358..81a961e670 100644
--- a/lib/ansible/plugins/lookup/config.py
+++ b/lib/ansible/plugins/lookup/config.py
@@ -132,6 +132,7 @@ class LookupModule(LookupBase):
             raise AnsibleOptionsError('"on_missing" must be a string and one of "error", "warn" or "skip", not %s' % missing)
 
         ret = []
+
         for term in terms:
             if not isinstance(term, string_types):
                 raise AnsibleOptionsError('Invalid setting identifier, "%s" is not a string, its a %s' % (term, type(term)))
diff --git a/lib/ansible/plugins/lookup/csvfile.py b/lib/ansible/plugins/lookup/csvfile.py
index 272b4a7767..7effbc869a 100644
--- a/lib/ansible/plugins/lookup/csvfile.py
+++ b/lib/ansible/plugins/lookup/csvfile.py
@@ -19,7 +19,6 @@ DOCUMENTATION = """
         default: "1"
       default:
         description: what to return if the value is not found in the file.
-        default: ''
       delimiter:
         description: field separator in the file, for a tab you can specify C(TAB) or C(\\t).
         default: TAB
@@ -40,20 +39,22 @@ DOCUMENTATION = """
 
 EXAMPLES = """
 - name:  Match 'Li' on the first column, return the second column (0 based index)
-  debug: msg="The atomic number of Lithium is {{ lookup('csvfile', 'Li file=elements.csv delimiter=,') }}"
+  debug: msg="The atomic number of Lithium is {{ lookup('csvfile', 'Li', file='elements.csv', delimiter=',') }}"
 
 - name: msg="Match 'Li' on the first column, but return the 3rd column (columns start counting after the match)"
-  debug: msg="The atomic mass of Lithium is {{ lookup('csvfile', 'Li file=elements.csv delimiter=, col=2') }}"
+  debug: msg="The atomic mass of Lithium is {{ lookup('csvfile', 'Li', file='elements.csv', delimiter=',', col=2) }}"
 
-- name: Define Values From CSV File
+- name: Define Values From CSV File, this reads file in one go, but you could also use col= to read each in it's own lookup.
   set_fact:
-    loop_ip: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=1') }}"
-    int_ip: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=2') }}"
-    int_mask: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=3') }}"
-    int_name: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=4') }}"
-    local_as: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=5') }}"
-    neighbor_as: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=6') }}"
-    neigh_int_ip: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=7') }}"
+    loop_ip: "{{ csvline[0] }}"
+    int_ip: "{{ csvline[1] }}"
+    int_mask: "{{ csvline[2] }}"
+    int_name: "{{ csvline[3] }}"
+    local_as: "{{ csvline[4] }}"
+    neighbor_as: "{{ csvline[5] }}"
+    neigh_int_ip: "{{ csvline[6] }}"
+  vars:
+    csvline = "{{ lookup('csvfile', bgp_neighbor_ip, file='bgp_neighbors.csv', delimiter=',') }}"
   delegate_to: localhost
 """
 
@@ -121,7 +122,7 @@ class LookupModule(LookupBase):
     def read_csv(self, filename, key, delimiter, encoding='utf-8', dflt=None, col=1):
 
         try:
-            f = open(filename, 'rb')
+            f = open(to_bytes(filename), 'rb')
             creader = CSVReader(f, delimiter=to_native(delimiter), encoding=encoding)
 
             for row in creader:
@@ -136,6 +137,11 @@ class LookupModule(LookupBase):
 
         ret = []
 
+        self.set_options(var_options=variables, direct=kwargs)
+
+        # populate options
+        paramvals = self.get_options()
+
         for term in terms:
             kv = parse_kv(term)
 
@@ -144,25 +150,21 @@ class LookupModule(LookupBase):
 
             key = kv['_raw_params']
 
-            paramvals = {
-                'col': "1",          # column to return
-                'default': None,
-                'delimiter': "TAB",
-                'file': 'ansible.csv',
-                'encoding': 'utf-8',
-            }
-
-            # parameters specified?
+            # parameters override per term using k/v
             try:
                 for name, value in kv.items():
                     if name == '_raw_params':
                         continue
                     if name not in paramvals:
-                        raise AnsibleAssertionError('%s not in paramvals' % name)
+                        raise AnsibleAssertionError('%s is not a valid option' % name)
+
+                    self._deprecate_inline_kv()
                     paramvals[name] = value
+
             except (ValueError, AssertionError) as e:
                 raise AnsibleError(e)
 
+            # default is just placeholder for real tab
             if paramvals['delimiter'] == 'TAB':
                 paramvals['delimiter'] = "\t"
 
@@ -174,4 +176,5 @@ class LookupModule(LookupBase):
                         ret.append(v)
                 else:
                     ret.append(var)
+
         return ret
diff --git a/lib/ansible/plugins/lookup/dict.py b/lib/ansible/plugins/lookup/dict.py
index 5a83d9e8a1..608d14d740 100644
--- a/lib/ansible/plugins/lookup/dict.py
+++ b/lib/ansible/plugins/lookup/dict.py
@@ -62,7 +62,7 @@ class LookupModule(LookupBase):
 
     def run(self, terms, variables=None, **kwargs):
 
-        # FIXME: can remove once with_ special case is removed
+        # NOTE: can remove if with_ is removed
         if not isinstance(terms, list):
             terms = [terms]
 
diff --git a/lib/ansible/plugins/lookup/file.py b/lib/ansible/plugins/lookup/file.py
index 04ddc4b1ea..9a58119466 100644
--- a/lib/ansible/plugins/lookup/file.py
+++ b/lib/ansible/plugins/lookup/file.py
@@ -62,6 +62,7 @@ class LookupModule(LookupBase):
     def run(self, terms, variables=None, **kwargs):
 
         ret = []
+        self.set_options(var_options=variables, direct=kwargs)
 
         for term in terms:
             display.debug("File lookup term: %s" % term)
@@ -73,9 +74,9 @@ class LookupModule(LookupBase):
                 if lookupfile:
                     b_contents, show_data = self._loader._get_file_contents(lookupfile)
                     contents = to_text(b_contents, errors='surrogate_or_strict')
-                    if kwargs.get('lstrip', False):
+                    if self.get_option('lstrip'):
                         contents = contents.lstrip()
-                    if kwargs.get('rstrip', True):
+                    if self.get_option('rstrip'):
                         contents = contents.rstrip()
                     ret.append(contents)
                 else:
diff --git a/lib/ansible/plugins/lookup/first_found.py b/lib/ansible/plugins/lookup/first_found.py
index ffe9fd30f6..896d7e2db3 100644
--- a/lib/ansible/plugins/lookup/first_found.py
+++ b/lib/ansible/plugins/lookup/first_found.py
@@ -23,8 +23,12 @@ DOCUMENTATION = """
         description: list of file names
       files:
         description: list of file names
+        type: list
+        default: []
       paths:
         description: list of paths in which to look for the files
+        type: list
+        default: []
       skip:
         type: boolean
         default: False
@@ -106,71 +110,100 @@ import os
 
 from jinja2.exceptions import UndefinedError
 
-from ansible.errors import AnsibleFileNotFound, AnsibleLookupError, AnsibleUndefinedVariable
+from ansible.errors import AnsibleLookupError, AnsibleUndefinedVariable
+from ansible.module_utils.common._collections_compat import Mapping, Sequence
 from ansible.module_utils.six import string_types
-from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.plugins.lookup import LookupBase
 
 
+def _split_on(terms, spliters=','):
+
+    # TODO: fix as it does not allow spaces in names
+    termlist = []
+    if isinstance(terms, string_types):
+        for spliter in spliters:
+            terms = terms.replace(spliter, ' ')
+        termlist = terms.split(' ')
+    else:
+        # added since options will already listify
+        for t in terms:
+            termlist.extend(_split_on(t, spliters))
+
+    return termlist
+
+
 class LookupModule(LookupBase):
 
-    def run(self, terms, variables, **kwargs):
+    def _process_terms(self, terms, variables, kwargs):
 
-        anydict = False
+        total_search = []
         skip = False
 
+        # can use a dict instead of list item to pass inline config
         for term in terms:
-            if isinstance(term, dict):
-                anydict = True
+            if isinstance(term, Mapping):
+                self.set_options(var_options=variables, direct=term)
+            elif isinstance(term, string_types):
+                self.set_options(var_options=variables, direct=kwargs)
+            elif isinstance(term, Sequence):
+                partial, skip = self._process_terms(term, variables, kwargs)
+                total_search.extend(partial)
+                continue
+            else:
+                raise AnsibleLookupError("Invalid term supplied, can handle string, mapping or list of strings but got: %s for %s" % (type(term), term))
 
-        total_search = []
-        if anydict:
-            for term in terms:
-                if isinstance(term, dict):
-
-                    files = term.get('files', [])
-                    paths = term.get('paths', [])
-                    skip = boolean(term.get('skip', False), strict=False)
-
-                    filelist = files
-                    if isinstance(files, string_types):
-                        files = files.replace(',', ' ')
-                        files = files.replace(';', ' ')
-                        filelist = files.split(' ')
-
-                    pathlist = paths
-                    if paths:
-                        if isinstance(paths, string_types):
-                            paths = paths.replace(',', ' ')
-                            paths = paths.replace(':', ' ')
-                            paths = paths.replace(';', ' ')
-                            pathlist = paths.split(' ')
-
-                    if not pathlist:
-                        total_search = filelist
-                    else:
-                        for path in pathlist:
-                            for fn in filelist:
-                                f = os.path.join(path, fn)
-                                total_search.append(f)
-                else:
-                    total_search.append(term)
-        else:
-            total_search = self._flatten(terms)
+            files = self.get_option('files')
+            paths = self.get_option('paths')
+
+            # NOTE: this is used as 'global' but  can be set many times?!?!?
+            skip = self.get_option('skip')
+
+            # magic extra spliting to create lists
+            filelist = _split_on(files, ',;')
+            pathlist = _split_on(paths, ',:;')
 
+            # create search structure
+            if pathlist:
+                for path in pathlist:
+                    for fn in filelist:
+                        f = os.path.join(path, fn)
+                        total_search.append(f)
+            elif filelist:
+                # NOTE: this seems wrong, should be 'extend' as any option/entry can clobber all
+                total_search = filelist
+            else:
+                total_search.append(term)
+
+        return total_search, skip
+
+    def run(self, terms, variables, **kwargs):
+
+        total_search, skip = self._process_terms(terms, variables, kwargs)
+
+        # NOTE: during refactor noticed that the 'using a dict' as term
+        # is designed to only work with 'one' otherwise inconsistencies will appear.
+        # see other notes below.
+
+        # actually search
+        subdir = getattr(self, '_subdir', 'files')
+
+        path = None
         for fn in total_search:
+
             try:
                 fn = self._templar.template(fn)
             except (AnsibleUndefinedVariable, UndefinedError):
                 continue
 
             # get subdir if set by task executor, default to files otherwise
-            subdir = getattr(self, '_subdir', 'files')
-            path = None
             path = self.find_file_in_search_path(variables, subdir, fn, ignore_missing=True)
+
+            # exit if we find one!
             if path is not None:
                 return [path]
+
+        # if we get here, no file was found
         if skip:
+            # NOTE: global skip wont matter, only last 'skip' value in dict term
             return []
-        raise AnsibleLookupError("No file was found when using first_found. Use errors='ignore' to allow this task to be skipped if no "
-                                 "files are found")
+        raise AnsibleLookupError("No file was found when using first_found. Use errors='ignore' to allow this task to be skipped if no files are found")
diff --git a/lib/ansible/plugins/lookup/ini.py b/lib/ansible/plugins/lookup/ini.py
index a2e1607032..b88d4e5adf 100644
--- a/lib/ansible/plugins/lookup/ini.py
+++ b/lib/ansible/plugins/lookup/ini.py
@@ -23,7 +23,7 @@ DOCUMENTATION = """
         choices: ['ini', 'properties']
       file:
         description: Name of the file to load.
-        default: ansible.ini
+        default: 'ansible.ini'
       section:
         default: global
         description: Section where to lookup the key.
@@ -40,16 +40,15 @@ DOCUMENTATION = """
 """
 
 EXAMPLES = """
-- debug: msg="User in integration is {{ lookup('ini', 'user section=integration file=users.ini') }}"
+- debug: msg="User in integration is {{ lookup('ini', 'user', section='integration', file='users.ini') }}"
 
-- debug: msg="User in production  is {{ lookup('ini', 'user section=production  file=users.ini') }}"
+- debug: msg="User in production  is {{ lookup('ini', 'user', section='production',  file='users.ini') }}"
 
-- debug: msg="user.name is {{ lookup('ini', 'user.name type=properties file=user.properties') }}"
+- debug: msg="user.name is {{ lookup('ini', 'user.name', type='properties', file='user.properties') }}"
 
 - debug:
     msg: "{{ item }}"
-  with_ini:
-    - '.* section=section1 file=test.ini re=True'
+  loop: "{{q('ini', '.*', section='section1', file='test.ini', re=True)}}"
 """
 
 RETURN = """
@@ -59,37 +58,48 @@ _raw:
   type: list
   elements: str
 """
+
 import os
 import re
+
 from io import StringIO
+from collections import defaultdict
 
-from ansible.errors import AnsibleError, AnsibleAssertionError
+from ansible.errors import AnsibleLookupError
 from ansible.module_utils.six.moves import configparser
-from ansible.module_utils._text import to_bytes, to_text
+from ansible.module_utils._text import to_bytes, to_text, to_native
 from ansible.module_utils.common._collections_compat import MutableSequence
 from ansible.plugins.lookup import LookupBase
 
 
-def _parse_params(term):
+def _parse_params(term, paramvals):
     '''Safely split parameter term to preserve spaces'''
 
-    keys = ['key', 'type', 'section', 'file', 're', 'default', 'encoding']
-    params = {}
-    for k in keys:
-        params[k] = ''
+    # TODO: deprecate this method
+    valid_keys = paramvals.keys()
+    params = defaultdict(lambda: '')
 
-    thiskey = 'key'
+    # TODO: check kv_parser to see if it can handle spaces this same way
+    keys = []
+    thiskey = 'key'  # initialize for 'lookup item'
     for idp, phrase in enumerate(term.split()):
-        for k in keys:
-            if ('%s=' % k) in phrase:
-                thiskey = k
+
+        # update current key if used
+        if '=' in phrase:
+            for k in valid_keys:
+                if ('%s=' % k) in phrase:
+                    thiskey = k
+
+        # if first term or key does not exist
         if idp == 0 or not params[thiskey]:
             params[thiskey] = phrase
+            keys.append(thiskey)
         else:
+            # append to existing key
             params[thiskey] += ' ' + phrase
 
-    rparams = [params[x] for x in keys if params[x]]
-    return rparams
+    # return list of values
+    return [params[x] for x in keys]
 
 
 class LookupModule(LookupBase):
@@ -108,36 +118,36 @@ class LookupModule(LookupBase):
 
     def run(self, terms, variables=None, **kwargs):
 
+        self.set_options(var_options=variables, direct=kwargs)
+        paramvals = self.get_options()
+
         self.cp = configparser.ConfigParser()
 
         ret = []
         for term in terms:
-            params = _parse_params(term)
-            key = params[0]
-
-            paramvals = {
-                'file': 'ansible.ini',
-                're': False,
-                'default': None,
-                'section': "global",
-                'type': "ini",
-                'encoding': 'utf-8',
-            }
 
+            key = term
             # parameters specified?
-            try:
-                for param in params[1:]:
-                    name, value = param.split('=')
-                    if name not in paramvals:
-                        raise AnsibleAssertionError('%s not in paramvals' %
-                                                    name)
-                    paramvals[name] = value
-            except (ValueError, AssertionError) as e:
-                raise AnsibleError(e)
-
+            if '=' in term or ' ' in term.strip():
+                self._deprecate_inline_kv()
+                params = _parse_params(term, paramvals)
+                try:
+                    for param in params:
+                        if '=' in param:
+                            name, value = param.split('=')
+                            if name not in paramvals:
+                                raise AnsibleLookupError('%s is not a valid option.' % name)
+                            paramvals[name] = value
+                        elif key == term:
+                            # only take first, this format never supported multiple keys inline
+                            key = param
+                except ValueError as e:
+                    # bad params passed
+                    raise AnsibleLookupError("Could not use '%s' from '%s': %s" % (param, params, to_native(e)), orig_exc=e)
+
+            # TODO: look to use cache to avoid redoing this for every term if they use same file
             # Retrieve file path
-            path = self.find_file_in_search_path(variables, 'files',
-                                                 paramvals['file'])
+            path = self.find_file_in_search_path(variables, 'files', paramvals['file'])
 
             # Create StringIO later used to parse ini
             config = StringIO()
@@ -148,14 +158,12 @@ class LookupModule(LookupBase):
 
             # Open file using encoding
             contents, show_data = self._loader._get_file_contents(path)
-            contents = to_text(contents, errors='surrogate_or_strict',
-                               encoding=paramvals['encoding'])
+            contents = to_text(contents, errors='surrogate_or_strict', encoding=paramvals['encoding'])
             config.write(contents)
             config.seek(0, os.SEEK_SET)
 
             self.cp.readfp(config)
-            var = self.get_value(key, paramvals['section'],
-                                 paramvals['default'], paramvals['re'])
+            var = self.get_value(key, paramvals['section'], paramvals['default'], paramvals['re'])
             if var is not None:
                 if isinstance(var, MutableSequence):
                     for v in var:
diff --git a/lib/ansible/plugins/lookup/template.py b/lib/ansible/plugins/lookup/template.py
index f1cbe85a92..6f56933382 100644
--- a/lib/ansible/plugins/lookup/template.py
+++ b/lib/ansible/plugins/lookup/template.py
@@ -40,6 +40,11 @@ DOCUMENTATION = """
         default: False
         version_added: '2.11'
         type: bool
+      template_vars:
+        description: A dictionary, the keys become additional variables available for templating.
+        default: {}
+        version_added: '2.3'
+        type: dict
 """
 
 EXAMPLES = """
@@ -78,13 +83,17 @@ display = Display()
 class LookupModule(LookupBase):
 
     def run(self, terms, variables, **kwargs):
-        convert_data_p = kwargs.get('convert_data', True)
-        lookup_template_vars = kwargs.get('template_vars', {})
-        jinja2_native = kwargs.get('jinja2_native', False)
+
         ret = []
 
-        variable_start_string = kwargs.get('variable_start_string', None)
-        variable_end_string = kwargs.get('variable_end_string', None)
+        self.set_options(var_options=variables, direct=kwargs)
+
+        # capture options
+        convert_data_p = self.get_option('convert_data')
+        lookup_template_vars = self.get_option('template_vars')
+        jinja2_native = self.get_option('jinja2_native')
+        variable_start_string = self.get_option('variable_start_string')
+        variable_end_string = self.get_option('variable_end_string')
 
         if USE_JINJA2_NATIVE and not jinja2_native:
             templar = self._templar.copy_with_new_env(environment_class=AnsibleEnvironment)
diff --git a/lib/ansible/plugins/lookup/unvault.py b/lib/ansible/plugins/lookup/unvault.py
index 3712ba5be4..a6bddc7eda 100644
--- a/lib/ansible/plugins/lookup/unvault.py
+++ b/lib/ansible/plugins/lookup/unvault.py
@@ -42,10 +42,10 @@ class LookupModule(LookupBase):
 
     def run(self, terms, variables=None, **kwargs):
 
-        self.set_options(direct=kwargs)
-
         ret = []
 
+        self.set_options(var_options=variables, direct=kwargs)
+
         for term in terms:
             display.debug("Unvault lookup term: %s" % term)
 
diff --git a/lib/ansible/plugins/lookup/varnames.py b/lib/ansible/plugins/lookup/varnames.py
index 6a3def37b5..eba7de6eda 100644
--- a/lib/ansible/plugins/lookup/varnames.py
+++ b/lib/ansible/plugins/lookup/varnames.py
@@ -58,8 +58,7 @@ class LookupModule(LookupBase):
         if variables is None:
             raise AnsibleError('No variables available to search')
 
-        # no options, yet
-        # self.set_options(direct=kwargs)
+        self.set_options(var_options=variables, direct=kwargs)
 
         ret = []
         variable_names = list(variables.keys())
diff --git a/lib/ansible/plugins/lookup/vars.py b/lib/ansible/plugins/lookup/vars.py
index 9e14735219..3af5838a4b 100644
--- a/lib/ansible/plugins/lookup/vars.py
+++ b/lib/ansible/plugins/lookup/vars.py
@@ -79,7 +79,7 @@ class LookupModule(LookupBase):
             self._templar.available_variables = variables
         myvars = getattr(self._templar, '_available_variables', {})
 
-        self.set_options(direct=kwargs)
+        self.set_options(var_options=variables, direct=kwargs)
         default = self.get_option('default')
 
         ret = []