Move utility functions out of basic.py (#51715)

Move the following methods to lib/anisble/module_utils/common/validation.py:

- _count_terms()
- _check_mutually_exclusive()
- _check_required_one_of()
- _check_required_together()
- _check_required_by()
- _check_required_arguments()
- _check_required_if
- fail_on_missing_params() --> create check_missing_parameters()

lib/ansible/module_utils/basic.py

@@ -173,6 +173,16 @@ from ansible.module_utils.six import (
     text_type,
 )
 from ansible.module_utils.six.moves import map, reduce, shlex_quote
+from ansible.module_utils.common.validation import (
+    check_missing_parameters,
+    check_mutually_exclusive,
+    check_required_arguments,
+    check_required_by,
+    check_required_if,
+    check_required_one_of,
+    check_required_together,
+    count_terms,
+)
 from ansible.module_utils._text import to_native, to_bytes, to_text
 from ansible.module_utils.common._utils import get_all_subclasses as _get_all_subclasses
 from ansible.module_utils.parsing.convert_bool import BOOLEANS, BOOLEANS_FALSE, BOOLEANS_TRUE, boolean
@@ -1593,21 +1603,18 @@ class AnsibleModule(object):
             self.exit_json(skipped=True, msg="remote module (%s) does not support check mode" % self._name)
 
     def _count_terms(self, check, param=None):
-        count = 0
         if param is None:
             param = self.params
-        for term in check:
+        return count_terms(check, param)
-            if term in param:
-                count += 1
-        return count
 
     def _check_mutually_exclusive(self, spec, param=None):
-        if spec is None:
+        if param is None:
-            return
+            param = self.params
-        for check in spec:
+
-            count = self._count_terms(check, param)
+        try:
-            if count > 1:
+            check_mutually_exclusive(spec, param)
-                msg = "parameters are mutually exclusive: %s" % ', '.join(check)
+        except TypeError as e:
+            msg = to_native(e)
             if self._options_context:
                 msg += " found in %s" % " -> ".join(self._options_context)
             self.fail_json(msg=msg)
@@ -1615,10 +1622,14 @@ class AnsibleModule(object):
     def _check_required_one_of(self, spec, param=None):
         if spec is None:
             return
-        for check in spec:
+
-            count = self._count_terms(check, param)
+        if param is None:
-            if count == 0:
+            param = self.params
-                msg = "one of the following is required: %s" % ', '.join(check)
+
+        try:
+            check_required_one_of(spec, param)
+        except TypeError as e:
+            msg = to_native(e)
             if self._options_context:
                 msg += " found in %s" % " -> ".join(self._options_context)
             self.fail_json(msg=msg)
@@ -1626,12 +1637,13 @@ class AnsibleModule(object):
     def _check_required_together(self, spec, param=None):
         if spec is None:
             return
-        for check in spec:
+        if param is None:
-            counts = [self._count_terms([field], param) for field in check]
+            param = self.params
-            non_zero = [c for c in counts if c > 0]
+
-            if len(non_zero) > 0:
+        try:
-                if 0 in counts:
+            check_required_together(spec, param)
-                    msg = "parameters are required together: %s" % ', '.join(check)
+        except TypeError as e:
+            msg = to_native(e)
             if self._options_context:
                 msg += " found in %s" % " -> ".join(self._options_context)
             self.fail_json(msg=msg)
@@ -1641,32 +1653,22 @@ class AnsibleModule(object):
             return
         if param is None:
             param = self.params
-        for (key, value) in spec.items():
+
-            if key not in param or param[key] is None:
+        try:
-                continue
+            check_required_by(spec, param)
-            missing = []
+        except TypeError as e:
-            # Support strings (single-item lists)
+            self.fail_json(msg=to_native(e))
-            if isinstance(value, string_types):
-                value = [value, ]
-            for required in value:
-                if required not in param or param[required] is None:
-                    missing.append(required)
-            if len(missing) > 0:
-                self.fail_json(msg="missing parameter(s) required by '%s': %s" % (key, ', '.join(missing)))
 
     def _check_required_arguments(self, spec=None, param=None):
-        ''' ensure all required arguments are present '''
-        missing = []
         if spec is None:
             spec = self.argument_spec
         if param is None:
             param = self.params
-        for (k, v) in spec.items():
+
-            required = v.get('required', False)
+        try:
-            if required and k not in param:
+            check_required_arguments(spec, param)
-                missing.append(k)
+        except TypeError as e:
-        if len(missing) > 0:
+            msg = to_native(e)
-            msg = "missing required arguments: %s" % ", ".join(missing)
             if self._options_context:
                 msg += " found in %s" % " -> ".join(self._options_context)
             self.fail_json(msg=msg)
@@ -1677,30 +1679,11 @@ class AnsibleModule(object):
             return
         if param is None:
             param = self.params
-        for sp in spec:
-            missing = []
-            max_missing_count = 0
-            is_one_of = False
-            if len(sp) == 4:
-                key, val, requirements, is_one_of = sp
-            else:
-                key, val, requirements = sp
 
-            # is_one_of is True at least one requirement should be
+        try:
-            # present, else all requirements should be present.
+            check_required_if(spec, param)
-            if is_one_of:
+        except TypeError as e:
-                max_missing_count = len(requirements)
+            msg = to_native(e)
-                term = 'any'
-            else:
-                term = 'all'
-
-            if key in param and param[key] == val:
-                for check in requirements:
-                    count = self._count_terms((check,), param)
-                    if count == 0:
-                        missing.append(check)
-            if len(missing) and len(missing) >= max_missing_count:
-                msg = "%s is %s but %s of the following are missing: %s" % (key, val, term, ', '.join(missing))
             if self._options_context:
                 msg += " found in %s" % " -> ".join(self._options_context)
             self.fail_json(msg=msg)
@@ -2315,17 +2298,12 @@ class AnsibleModule(object):
         sys.exit(1)
 
     def fail_on_missing_params(self, required_params=None):
-        ''' This is for checking for required params when we can not check via argspec because we
-        need more information than is simply given in the argspec.
-        '''
         if not required_params:
             return
-        missing_params = []
+        try:
-        for required_param in required_params:
+            check_missing_parameters(self.params, required_params)
-            if not self.params.get(required_param):
+        except TypeError as e:
-                missing_params.append(required_param)
+            self.fail_json(msg=to_native(e))
-        if missing_params:
-            self.fail_json(msg="missing required arguments: %s" % ', '.join(missing_params))
 
     def digest_from_file(self, filename, algorithm):
         ''' Return hex digest of local file for a digest_method specified by name, or None if file is not present. '''

lib/ansible/module_utils/common/validation.py

@@ -0,0 +1,283 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2018 Ansible Project
+# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+from ansible.module_utils._text import to_native
+from ansible.module_utils.common.collections import is_iterable
+from ansible.module_utils.six import string_types
+
+
+def count_terms(terms, module_parameters):
+    """Count the number of occurrences of a key in a given dictionary
+
+    :arg terms: String or iterable of values to check
+    :arg module_parameters: Dictionary of module parameters
+
+    :returns: An integer that is the number of occurrences of the terms values
+        in the provided dictionary.
+    """
+
+    if not is_iterable(terms):
+        terms = [terms]
+
+    return len(set(terms).intersection(module_parameters))
+
+
+def check_mutually_exclusive(terms, module_parameters):
+    """Check mutually exclusive terms against argument parameters. Accepts
+    a single list or list of lists that are groups of terms that should be
+    mutually exclusive with one another
+
+    :arg terms: List of mutually exclusive module parameters
+    :arg module_parameters: Dictionary of module parameters
+
+    :returns: Empty list or raises TypeError if the check fails.
+    """
+
+    results = []
+    if terms is None:
+        return results
+
+    for check in terms:
+        count = count_terms(check, module_parameters)
+        if count > 1:
+            results.append(check)
+
+    if results:
+        full_list = ['|'.join(check) for check in results]
+        msg = "parameters are mutually exclusive: %s" % ', '.join(full_list)
+        raise TypeError(to_native(msg))
+
+    return results
+
+
+def check_required_one_of(terms, module_parameters):
+    """Check each list of terms to ensure at least one exists in the given module
+    parameters. Accepts a list of lists or tuples.
+
+    :arg terms: List of lists of terms to check. For each list of terms, at
+        least one is required.
+    :arg module_parameters: Dictionary of module parameters
+
+    :returns: Empty list or raises TypeError if the check fails.
+    """
+
+    results = []
+    if terms is None:
+        return results
+
+    for term in terms:
+        count = count_terms(term, module_parameters)
+        if count == 0:
+            results.append(term)
+
+    if results:
+        for term in results:
+            msg = "one of the following is required: %s" % ', '.join(term)
+            raise TypeError(to_native(msg))
+
+    return results
+
+
+def check_required_together(terms, module_parameters):
+    """Check each list of terms to ensure every parameter in each list exists
+    in the given module parameters. Accepts a list of lists or tuples.
+
+    :arg terms: List of lists of terms to check. Each list should include
+        parameters that are all required when at least one is specified
+        in the module_parameters.
+    :arg module_parameters: Dictionary of module parameters
+
+    :returns: Empty list or raises TypeError if the check fails.
+    """
+
+    results = []
+    if terms is None:
+        return results
+
+    for term in terms:
+        counts = [count_terms(field, module_parameters) for field in term]
+        non_zero = [c for c in counts if c > 0]
+        if len(non_zero) > 0:
+            if 0 in counts:
+                results.append(term)
+    if results:
+        for term in results:
+            msg = "parameters are required together: %s" % ', '.join(term)
+            raise TypeError(to_native(msg))
+
+    return results
+
+
+def check_required_by(requirements, module_parameters):
+    """For each key in requirements, check the corresponding list to see if they
+    exist in module_parameters. Accepts a single string or list of values for
+    each key.
+
+    :arg requirements: Dictionary of requirements
+    :arg module_parameters: Dictionary of module parameters
+
+    :returns: Empty dictionary or raises TypeError if the
+    """
+
+    result = {}
+    if requirements is None:
+        return result
+
+    for (key, value) in requirements.items():
+        if key not in module_parameters or module_parameters[key] is None:
+            continue
+        result[key] = []
+        # Support strings (single-item lists)
+        if isinstance(value, string_types):
+            value = [value]
+        for required in value:
+            if required not in module_parameters or module_parameters[required] is None:
+                result[key].append(required)
+
+    if result:
+        for key, missing in result.items():
+            if len(missing) > 0:
+                msg = "missing parameter(s) required by '%s': %s" % (key, ', '.join(missing))
+                raise TypeError(to_native(msg))
+
+    return result
+
+
+def check_required_arguments(argument_spec, module_parameters):
+    """Check all paramaters in argument_spec and return a list of parameters
+    that are required by not present in module_parameters.
+
+    Raises AnsibleModuleParameterException if the check fails.
+
+    :arg argument_spec: Argument spec dicitionary containing all parameters
+        and their specification
+    :arg module_paramaters: Dictionary of module parameters
+
+    :returns: Empty list or raises TypeError if the check fails.
+    """
+
+    missing = []
+    if argument_spec is None:
+        return missing
+
+    for (k, v) in argument_spec.items():
+        required = v.get('required', False)
+        if required and k not in module_parameters:
+            missing.append(k)
+
+    if missing:
+        msg = "missing required arguments: %s" % ", ".join(missing)
+        raise TypeError(to_native(msg))
+
+    return missing
+
+
+def check_required_if(requirements, module_parameters):
+    """Check parameters that are conditionally required.
+
+    Raises TypeError if the check fails.
+
+    :arg requirements: List of lists specifying a parameter, value, parameters
+        required when the given parameter is the specified value, and optionally
+        a boolean indicating any or all parameters are required.
+
+        Example:
+            required_if=[
+                ['state', 'present', ('path',), True],
+                ['someint', 99, ('bool_param', 'string_param')],
+            ]
+
+    :arg module_paramaters: Dictionary of module parameters
+
+    :returns: Empty list or raises TypeError if the check fails.
+        The results attribute of the exception contains a list of dictionaries.
+        Each dictionary is the result of evaluting each item in requirements.
+        Each return dictionary contains the following keys:
+
+            :key missing: List of parameters that are required but missing
+            :key requires: 'any' or 'all'
+            :key paramater: Parameter name that has the requirement
+            :key value: Original value of the paramater
+            :key requirements: Original required parameters
+
+        Example:
+            [
+                {
+                    'parameter': 'someint',
+                    'value': 99
+                    'requirements': ('bool_param', 'string_param'),
+                    'missing': ['string_param'],
+                    'requires': 'all',
+                }
+            ]
+
+    """
+    results = []
+    if requirements is None:
+        return results
+
+    for req in requirements:
+        missing = {}
+        missing['missing'] = []
+        max_missing_count = 0
+        is_one_of = False
+        if len(req) == 4:
+            key, val, requirements, is_one_of = req
+        else:
+            key, val, requirements = req
+
+        # is_one_of is True at least one requirement should be
+        # present, else all requirements should be present.
+        if is_one_of:
+            max_missing_count = len(requirements)
+            missing['requires'] = 'any'
+        else:
+            missing['requires'] = 'all'
+
+        if key in module_parameters and module_parameters[key] == val:
+            for check in requirements:
+                count = count_terms(check, module_parameters)
+                if count == 0:
+                    missing['missing'].append(check)
+        if len(missing['missing']) and len(missing['missing']) >= max_missing_count:
+            missing['parameter'] = key
+            missing['value'] = val
+            missing['requirements'] = requirements
+            results.append(missing)
+
+    if results:
+        for missing in results:
+            msg = "%s is %s but %s of the following are missing: %s" % (
+                missing['parameter'], missing['value'], missing['requires'], ', '.join(missing['missing']))
+            raise TypeError(to_native(msg))
+
+    return results
+
+
+def check_missing_parameters(module_parameters, required_parameters=None):
+    """This is for checking for required params when we can not check via
+    argspec because we need more information than is simply given in the argspec.
+
+    :arg module_paramaters: Dictionary of module parameters
+    :arg required_parameters: List of parameters to look for in the given module
+        parameters
+
+    :returns: Empty list or raises TypeError if the check fails.
+    """
+    missing_params = []
+    if required_parameters is None:
+        return missing_params
+
+    for param in required_parameters:
+        if not module_parameters.get(param):
+            missing_params.append(param)
+
+    if missing_params:
+        msg = "missing required arguments: %s" % ', '.join(missing_params)
+        raise TypeError(to_native(msg))
+
+    return missing_params

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

@@ -10,7 +10,7 @@
   assert:
     that:
     - ip_address is failed
-    - 'ip_address.msg == "parameters are mutually exclusive: vpc, network"'
+    - 'ip_address.msg == "parameters are mutually exclusive: vpc|network"'
 
 - name: run test for network setup
   import_tasks: network.yml

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

@@ -250,7 +250,7 @@
   assert:
     that:
     - roleperm is failed
-    - 'roleperm.msg == "parameters are mutually exclusive: permission, parent"'
+    - 'roleperm.msg == "parameters are mutually exclusive: permission|parent"'
 
 - name: test fail if parent does not exist
   cs_role_permission:

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

@@ -57,7 +57,7 @@
     that:
     - ipr is not successful
     - ipr is not changed
-    - 'ipr.msg == "parameters are mutually exclusive: account, project"'
+    - 'ipr.msg == "parameters are mutually exclusive: account|project"'
 
 - name: test create a VLAN IP RANGE in check mode
   cs_vlan_ip_range:

test/integration/targets/docker_network/tasks/tests/ipam.yml

@@ -26,7 +26,7 @@
 - assert:
     that:
       - network is failed
-      - "network.msg == 'parameters are mutually exclusive: ipam_config, ipam_options'"
+      - "network.msg == 'parameters are mutually exclusive: ipam_config|ipam_options'"
 
 - name: Create network with deprecated custom IPAM config
   docker_network:

test/integration/targets/git_config/tasks/exclusion_state_list-all.yml

@@ -12,5 +12,5 @@
   assert:
     that:
       - result is failed
-      - "result.msg == 'parameters are mutually exclusive: list_all, state'"
+      - "result.msg == 'parameters are mutually exclusive: list_all|state'"
 ...

test/units/executor/module_common/test_recursive_finder.py

@@ -50,6 +50,7 @@ MODULE_UTILS_BASIC_IMPORTS = frozenset((('_text',),
                                         ('common', 'parameters'),
                                         ('common', 'process'),
                                         ('common', 'sys_info'),
+                                        ('common', 'validation'),
                                         ('common', '_utils'),
                                         ('distro', '__init__'),
                                         ('distro', '_distro'),
@@ -71,6 +72,7 @@ MODULE_UTILS_BASIC_FILES = frozenset(('ansible/module_utils/_text.py',
                                       'ansible/module_utils/common/file.py',
                                       'ansible/module_utils/common/process.py',
                                       'ansible/module_utils/common/sys_info.py',
+                                      'ansible/module_utils/common/validation.py',
                                       'ansible/module_utils/common/_utils.py',
                                       'ansible/module_utils/distro/__init__.py',
                                       'ansible/module_utils/distro/_distro.py',

test/units/module_utils/basic/test_argument_spec.py

@@ -94,13 +94,16 @@ def complex_argspec():
         foo=dict(required=True, aliases=['dup']),
         bar=dict(),
         bam=dict(),
+        bing=dict(),
+        bang=dict(),
+        bong=dict(),
         baz=dict(fallback=(basic.env_fallback, ['BAZ'])),
         bar1=dict(type='bool'),
         bar3=dict(type='list', elements='path'),
         zardoz=dict(choices=['one', 'two']),
         zardoz2=dict(type='list', choices=['one', 'two', 'three']),
     )
-    mut_ex = (('bar', 'bam'),)
+    mut_ex = (('bar', 'bam'), ('bing', 'bang', 'bong'))
     req_to = (('bam', 'baz'),)
 
     kwargs = dict(
@@ -137,7 +140,7 @@ def options_argspec_list():
             elements='dict',
             options=options_spec,
             mutually_exclusive=[
-                ['bam', 'bam1']
+                ['bam', 'bam1'],
             ],
             required_if=[
                 ['foo', 'hello', ['bam']],
@@ -241,7 +244,7 @@ class TestComplexArgSpecs:
         assert isinstance(am.params['baz'], str)
         assert am.params['baz'] == 'test data'
 
-    @pytest.mark.parametrize('stdin', [{'foo': 'hello', 'bar': 'bad', 'bam': 'bad2'}], indirect=['stdin'])
+    @pytest.mark.parametrize('stdin', [{'foo': 'hello', 'bar': 'bad', 'bam': 'bad2', 'bing': 'a', 'bang': 'b', 'bong': 'c'}], indirect=['stdin'])
     def test_fail_mutually_exclusive(self, capfd, stdin, complex_argspec):
         """Fail because of mutually exclusive parameters"""
         with pytest.raises(SystemExit):
@@ -251,7 +254,7 @@ class TestComplexArgSpecs:
         results = json.loads(out)
 
         assert results['failed']
-        assert results['msg'] == "parameters are mutually exclusive: bar, bam"
+        assert results['msg'] == "parameters are mutually exclusive: bar|bam, bing|bang|bong"
 
     @pytest.mark.parametrize('stdin', [{'foo': 'hello', 'bam': 'bad2'}], indirect=['stdin'])
     def test_fail_required_together(self, capfd, stdin, complex_argspec):
@@ -403,7 +406,7 @@ class TestComplexOptions:
         ({'foobar': [{"foo": "hello", "bam": "good", "invalid": "bad"}]}, 'module: invalid found in foobar. Supported parameters include'),
         # Mutually exclusive options found
         ({'foobar': [{"foo": "test", "bam": "bad", "bam1": "bad", "baz": "req_to"}]},
-         'parameters are mutually exclusive: bam, bam1 found in foobar'),
+         'parameters are mutually exclusive: bam|bam1 found in foobar'),
         # required_if fails
         ({'foobar': [{"foo": "hello", "bar": "bad"}]},
          'foo is hello but all of the following are missing: bam found in foobar'),
@@ -427,7 +430,7 @@ class TestComplexOptions:
          'module: invalid found in foobar. Supported parameters include'),
         # Mutually exclusive options found
         ({'foobar': {"foo": "test", "bam": "bad", "bam1": "bad", "baz": "req_to"}},
-         'parameters are mutually exclusive: bam, bam1 found in foobar'),
+         'parameters are mutually exclusive: bam|bam1 found in foobar'),
         # required_if fails
         ({'foobar': {"foo": "hello", "bar": "bad"}},
          'foo is hello but all of the following are missing: bam found in foobar'),

test/units/module_utils/common/validation/test_check_mutually_exclusive.py

@@ -0,0 +1,56 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2019 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
+
+import pytest
+
+from ansible.module_utils._text import to_native
+from ansible.module_utils.common.validation import check_mutually_exclusive
+
+
+@pytest.fixture
+def mutually_exclusive_terms():
+    return [
+        ('string1', 'string2',),
+        ('box', 'fox', 'socks'),
+    ]
+
+
+def test_check_mutually_exclusive(mutually_exclusive_terms):
+    params = {
+        'string1': 'cat',
+        'fox': 'hat',
+    }
+    assert check_mutually_exclusive(mutually_exclusive_terms, params) == []
+
+
+def test_check_mutually_exclusive_found(mutually_exclusive_terms):
+    params = {
+        'string1': 'cat',
+        'string2': 'hat',
+        'fox': 'red',
+        'socks': 'blue',
+    }
+    expected = "TypeError('parameters are mutually exclusive: string1|string2, box|fox|socks',)"
+
+    with pytest.raises(TypeError) as e:
+        check_mutually_exclusive(mutually_exclusive_terms, params)
+        assert e.value == expected
+
+
+def test_check_mutually_exclusive_none():
+    terms = None
+    params = {
+        'string1': 'cat',
+        'fox': 'hat',
+    }
+    assert check_mutually_exclusive(terms, params) == []
+
+
+def test_check_mutually_exclusive_no_params(mutually_exclusive_terms):
+    with pytest.raises(TypeError) as te:
+        check_mutually_exclusive(mutually_exclusive_terms, None)
+        assert "TypeError: 'NoneType' object is not iterable" in to_native(te.error)

test/units/module_utils/common/validation/test_count_terms.py

@@ -0,0 +1,40 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2019 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
+
+import pytest
+
+from ansible.module_utils.common.validation import count_terms
+
+
+@pytest.fixture
+def params():
+    return {
+        'name': 'bob',
+        'dest': '/etc/hosts',
+        'state': 'present',
+        'value': 5,
+    }
+
+
+def test_count_terms(params):
+    check = set(('name', 'dest'))
+    assert count_terms(check, params) == 2
+
+
+def test_count_terms_str_input(params):
+    check = 'name'
+    assert count_terms(check, params) == 1
+
+
+def test_count_terms_tuple_input(params):
+    check = ('name', 'dest')
+    assert count_terms(check, params) == 2
+
+
+def test_count_terms_list_input(params):
+    check = ['name', 'dest']
+    assert count_terms(check, params) == 2

test/units/modules/network/nxos/test_nxos_bgp_af.py

@@ -89,7 +89,9 @@ class TestNxosBgpAfModule(TestNxosModule):
                              dampening_half_time=5, dampening_suppress_time=2000,
                              dampening_reuse_time=1900, dampening_max_suppress_time=10))
         result = self.execute_module(failed=True)
-        self.assertEqual(result['msg'], 'parameters are mutually exclusive: dampening_routemap, dampening_half_time')
+        self.assertEqual(result['msg'], 'parameters are mutually exclusive: dampening_routemap|dampening_half_time, '
+                                        'dampening_routemap|dampening_suppress_time, dampening_routemap|dampening_reuse_time, '
+                                        'dampening_routemap|dampening_max_suppress_time')
 
     def test_nxos_bgp_af_client(self):
         set_module_args(dict(asn=65535, afi='ipv4', safi='unicast',