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>

lib/ansible/constants.py

@@ -5,20 +5,23 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import os
 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, get_ini_config_value
+from ansible.config.manager import ConfigManager, ensure_type
 from ansible.module_utils._text import to_text
 from ansible.module_utils.common.collections import Sequence
-from ansible.module_utils.parsing.convert_bool import boolean, BOOLEANS_TRUE
+from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE
 from ansible.module_utils.six import string_types
+from ansible.release import __version__
 from ansible.utils.fqcn import add_internal_fqcns
 
+# 4 versions above current
+default_deprecated = to_text(float('.'.join(__version__.split('.')[0:2])) + 0.04)
+
 
 def _warning(msg):
     ''' display is not guaranteed here, nor it being the full class, but try anyways, fallback to sys.stderr.write '''
@@ -30,7 +33,7 @@ def _warning(msg):
         sys.stderr.write(' [WARNING] %s\n' % (msg))
 
 
-def _deprecated(msg, version='2.8'):
+def _deprecated(msg, version=default_deprecated):
     ''' display is not guaranteed here, nor it being the full class, but try anyways, fallback to sys.stderr.write '''
     try:
         from ansible.utils.display import Display

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
 

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

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

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') }}"
+    loop_ip: "{{ csvline[0] }}"
-    int_ip: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=2') }}"
+    int_ip: "{{ csvline[1] }}"
-    int_mask: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=3') }}"
+    int_mask: "{{ csvline[2] }}"
-    int_name: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=4') }}"
+    int_name: "{{ csvline[3] }}"
-    local_as: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=5') }}"
+    local_as: "{{ csvline[4] }}"
-    neighbor_as: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=6') }}"
+    neighbor_as: "{{ csvline[5] }}"
-    neigh_int_ip: "{{ lookup('csvfile', bgp_neighbor_ip +' file=bgp_neighbors.csv delimiter=, col=7') }}"
+    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 = {
+            # parameters override per term using k/v
-                'col': "1",          # column to return
-                'default': None,
-                'delimiter': "TAB",
-                'file': 'ansible.csv',
-                'encoding': 'utf-8',
-            }
-
-            # parameters specified?
             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

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]
 

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:

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):
+            if isinstance(term, Mapping):
-                anydict = True
+                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 = []
+            files = self.get_option('files')
-        if anydict:
+            paths = self.get_option('paths')
-            for term in terms:
+
-                if isinstance(term, dict):
+            # NOTE: this is used as 'global' but  can be set many times?!?!?
-
+            skip = self.get_option('skip')
-                    files = term.get('files', [])
+
-                    paths = term.get('paths', [])
+            # magic extra spliting to create lists
-                    skip = boolean(term.get('skip', False), strict=False)
+            filelist = _split_on(files, ',;')
-
+            pathlist = _split_on(paths, ',:;')
-                    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)
 
+            # 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 "
+        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")
-                                 "files are found")

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:
+  loop: "{{q('ini', '.*', section='section1', file='test.ini', re=True)}}"
-    - '.* 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']
+    # TODO: deprecate this method
-    params = {}
+    valid_keys = paramvals.keys()
-    for k in keys:
+    params = defaultdict(lambda: '')
-        params[k] = ''
 
-    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:
+        # update current key if used
-                thiskey = k
+        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 list of values
-    return rparams
+    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:
+            if '=' in term or ' ' in term.strip():
-                for param in params[1:]:
+                self._deprecate_inline_kv()
-                    name, value = param.split('=')
+                params = _parse_params(term, paramvals)
-                    if name not in paramvals:
+                try:
-                        raise AnsibleAssertionError('%s not in paramvals' %
+                    for param in params:
-                                                    name)
+                        if '=' in param:
-                    paramvals[name] = value
+                            name, value = param.split('=')
-            except (ValueError, AssertionError) as e:
+                            if name not in paramvals:
-                raise AnsibleError(e)
+                                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',
+            path = self.find_file_in_search_path(variables, 'files', paramvals['file'])
-                                                 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',
+            contents = to_text(contents, errors='surrogate_or_strict', encoding=paramvals['encoding'])
-                               encoding=paramvals['encoding'])
             config.write(contents)
             config.seek(0, os.SEEK_SET)
 
             self.cp.readfp(config)
-            var = self.get_value(key, paramvals['section'],
+            var = self.get_value(key, paramvals['section'], paramvals['default'], paramvals['re'])
-                                 paramvals['default'], paramvals['re'])
             if var is not None:
                 if isinstance(var, MutableSequence):
                     for v in var:

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)
+        self.set_options(var_options=variables, direct=kwargs)
-        variable_end_string = kwargs.get('variable_end_string', None)
+
+        # 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)

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)
 

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(var_options=variables, direct=kwargs)
-        # self.set_options(direct=kwargs)
 
         ret = []
         variable_names = list(variables.keys())

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 = []

lib/ansible/template/__init__.py

@@ -42,7 +42,15 @@ from jinja2.loaders import FileSystemLoader
 from jinja2.runtime import Context, StrictUndefined
 
 from ansible import constants as C
-from ansible.errors import AnsibleError, AnsibleFilterError, AnsiblePluginRemovedError, AnsibleUndefinedVariable, AnsibleAssertionError
+from ansible.errors import (
+    AnsibleAssertionError,
+    AnsibleError,
+    AnsibleFilterError,
+    AnsibleLookupError,
+    AnsibleOptionsError,
+    AnsiblePluginRemovedError,
+    AnsibleUndefinedVariable,
+)
 from ansible.module_utils.six import iteritems, string_types, text_type
 from ansible.module_utils.six.moves import range
 from ansible.module_utils._text import to_native, to_text, to_bytes
@@ -997,7 +1005,22 @@ class Templar:
             ran = instance.run(loop_terms, variables=self._available_variables, **kwargs)
         except (AnsibleUndefinedVariable, UndefinedError) as e:
             raise AnsibleUndefinedVariable(e)
+        except AnsibleOptionsError as e:
+            # invalid options given to lookup, just reraise
+            raise e
+        except AnsibleLookupError as e:
+            # lookup handled error but still decided to bail
+            if self._fail_on_lookup_errors:
+                msg = 'Lookup failed but the error is being ignored: %s' % to_native(e)
+                if errors == 'warn':
+                    display.warning(msg)
+                elif errors == 'ignore':
+                    display.display(msg, log_only=True)
+                else:
+                    raise e
+            return [] if wantlist else None
         except Exception as e:
+            # errors not handled by lookup
             if self._fail_on_lookup_errors:
                 msg = u"An unhandled exception occurred while running the lookup plugin '%s'. Error was a %s, original message: %s" % \
                       (name, type(e), to_text(e))
@@ -1006,7 +1029,8 @@ class Templar:
                 elif errors == 'ignore':
                     display.display(msg, log_only=True)
                 else:
-                    raise AnsibleError(to_native(msg))
+                    display.vvv('exception during Jinja2 execution: {0}'.format(format_exc()))
+                    raise AnsibleError(to_native(msg), orig_exc=e)
             return [] if wantlist else None
 
         if ran and allow_unsafe is False:

test/integration/targets/lookup_csvfile/tasks/main.yml

@@ -1,15 +1,23 @@
-- set_fact:
+- name: using deprecated syntax but missing keyword
-    this_will_error: "{{ lookup('csvfile', 'file=people.csv delimiter=, col=1') }}"
+  set_fact:
+    this_will_error: "{{ lookup('csvfile', 'file=people.csv, delimiter=, col=1') }}"
   ignore_errors: yes
   register: no_keyword
 
-- set_fact:
+- name: extra arg in k=v syntax (deprecated)
+  set_fact:
     this_will_error: "{{ lookup('csvfile', 'foo file=people.csv delimiter=, col=1 thisarg=doesnotexist') }}"
   ignore_errors: yes
   register: invalid_arg
 
+- name: extra arg in config syntax
+  set_fact:
+    this_will_error: "{{ lookup('csvfile', 'foo', file='people.csv', delimiter=',' col=1, thisarg='doesnotexist') }}"
+  ignore_errors: yes
+  register: invalid_arg2
+
 - set_fact:
-    this_will_error: "{{ lookup('csvfile', 'foo file=doesnotexist delimiter=, col=1') }}"
+    this_will_error: "{{ lookup('csvfile', 'foo',  file='doesnotexist', delimiter=',', col=1) }}"
   ignore_errors: yes
   register: missing_file
 
@@ -19,24 +27,30 @@
       - no_keyword is failed
       - >
         "Search key is required but was not found" in no_keyword.msg
+      - invalid_arg is failed
+      - invalid_arg2 is failed
       - >
-        "not in paramvals" in invalid_arg.msg
+        "is not a valid option" in invalid_arg.msg
       - missing_file is failed
       - >
-        "need string or buffer" in missing_file.msg or "expected str, bytes or os.PathLike object" in missing_file.msg
+        "need string or buffer" in missing_file.msg or
+        "expected str, bytes or os.PathLike object" in missing_file.msg or
+        "No such file or directory" in missing_file.msg
 
 - name: Check basic comma-separated file
   assert:
     that:
-      - lookup('csvfile', 'Smith file=people.csv delimiter=, col=1') == "Jane"
+      - lookup('csvfile', 'Smith', file='people.csv', delimiter=',', col=1) == "Jane"
       - lookup('csvfile', 'German von Lastname file=people.csv delimiter=, col=1') == "Demo"
 
 - name: Check tab-separated file
   assert:
     that:
       - lookup('csvfile', 'electronics file=tabs.csv delimiter=TAB col=1') == "tvs"
-      - lookup('csvfile', 'fruit file=tabs.csv delimiter=TAB col=1') == "bananas"
+      - "lookup('csvfile', 'fruit', file='tabs.csv', delimiter='TAB', col=1) == 'bananas'"
       - lookup('csvfile', 'fruit file=tabs.csv delimiter="\t" col=1') == "bananas"
+      - lookup('csvfile', 'electronics', 'fruit', file='tabs.csv', delimiter='\t', col=1) == "tvs,bananas"
+      - lookup('csvfile', 'electronics', 'fruit', file='tabs.csv', delimiter='\t', col=1, wantlist=True) == ["tvs", "bananas"]
 
 - name: Check \x1a-separated file
   assert:

test/integration/targets/lookup_first_found/tasks/main.yml

@@ -1,10 +1,9 @@
 - name: test with_first_found
-  #shell: echo {{ item }}
   set_fact: "first_found={{ item }}"
   with_first_found:
-    - "{{ role_path + '/files/does_not_exist' }}"
+    - "does_not_exist"
-    - "{{ role_path + '/files/foo1' }}"
+    - "foo1"
-    - "{{ role_path + '/files/bar1' }}"
+    - "{{ role_path + '/files/bar1' }}"   # will only hit this if dwim search is broken
 
 - name: set expected
   set_fact: first_expected="{{ role_path + '/files/foo1' }}"
@@ -24,6 +23,7 @@
   vars:
     params:
       files: "not_a_file.yaml"
+      skip: True
 
 - name: verify q(first_found) result
   assert:
@@ -71,3 +71,16 @@
   assert:
     that:
       - "this_not_set is not defined"
+
+- name: test legacy formats
+  set_fact: hatethisformat={{item}}
+  vars:
+      params:
+        files: not/a/file.yaml;hosts
+        paths: not/a/path:/etc
+  loop: "{{ q('first_found', params) }}"
+
+- name: verify /etc/hosts was found
+  assert:
+    that:
+      - "hatethisformat == '/etc/hosts'"

test/integration/targets/lookup_ini/test_lookup_properties.yml

@@ -6,18 +6,18 @@
     - name: "read properties value"
       set_fact:
         test1:            "{{lookup('ini', 'value1            type=properties  file=lookup.properties')}}"
-        test2:            "{{lookup('ini', 'value2            type=properties  file=lookup.properties')}}"
+        test2:            "{{lookup('ini', 'value2', type='properties',  file='lookup.properties')}}"
-        test_dot:         "{{lookup('ini', 'value.dot         type=properties  file=lookup.properties')}}"
+        test_dot:         "{{lookup('ini', 'value.dot', type='properties',  file='lookup.properties')}}"
         field_with_space: "{{lookup('ini', 'field.with.space  type=properties  file=lookup.properties')}}"
     - assert:
         that: "{{item}} is defined"
       with_items: [ 'test1', 'test2', 'test_dot', 'field_with_space' ]
     - name: "read ini value"
       set_fact:
-        value1_global:    "{{lookup('ini', 'value1            section=global   file=lookup.ini')}}"
+        value1_global:    "{{lookup('ini', 'value1', section='global', file='lookup.ini')}}"
-        value2_global:    "{{lookup('ini', 'value2            section=global   file=lookup.ini')}}"
+        value2_global:    "{{lookup('ini', 'value2', section='global', file='lookup.ini')}}"
-        value1_section1:  "{{lookup('ini', 'value1            section=section1 file=lookup.ini')}}"
+        value1_section1:  "{{lookup('ini', 'value1', section='section1', file='lookup.ini')}}"
-        field_with_unicode: "{{lookup('ini', 'unicode         section=global   file=lookup.ini')}}"
+        field_with_unicode: "{{lookup('ini', 'unicode', section='global', file='lookup.ini')}}"
     - debug: var={{item}}
       with_items: [ 'value1_global', 'value2_global', 'value1_section1', 'field_with_unicode' ]
     - assert:

test/units/plugins/lookup/test_ini.py

@@ -56,8 +56,9 @@ class TestINILookup(unittest.TestCase):
     )
 
     def test_parse_parameters(self):
+        pvals = {'file': '', 'section': '', 'key': '', 'type': '', 're': '', 'default': '', 'encoding': ''}
         for testcase in self.old_style_params_data:
             # print(testcase)
-            params = _parse_params(testcase['term'])
+            params = _parse_params(testcase['term'], pvals)
             params.sort()
             self.assertEqual(params, testcase['expected'])