From 95ec1618ef8d066607fcd5d3c02b95eef58c86c2 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Fri, 17 Jul 2020 13:53:56 -0400 Subject: [PATCH] [stable-2.10] Only pass kwargs to our string checker not callable checkers (#70151) (#70170) Since only check_type_str() accepts extra param, only pass to our checker and do not pass kwargs to custom checkers. * Add unit tests (cherry picked from commit bc05415109) Co-authored-by: Sam Doran --- ...0017-avoid-params-to-callable-checkers.yml | 4 ++++ lib/ansible/module_utils/basic.py | 6 +++-- .../module_utils/basic/test_argument_spec.py | 23 ++++++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/70017-avoid-params-to-callable-checkers.yml diff --git a/changelogs/fragments/70017-avoid-params-to-callable-checkers.yml b/changelogs/fragments/70017-avoid-params-to-callable-checkers.yml new file mode 100644 index 00000000000..2b9b50ec588 --- /dev/null +++ b/changelogs/fragments/70017-avoid-params-to-callable-checkers.yml @@ -0,0 +1,4 @@ +bugfixes: + - >- + if the ``type`` for a module parameter in the argument spec is callable, + do not pass ``kwargs`` to avoid errors (https://github.com/ansible/ansible/issues/70017) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index b160cecce0d..db90dafd13b 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1765,8 +1765,9 @@ class AnsibleModule(object): type_checker, wanted_name = self._get_wanted_type(wanted, param) validated_params = [] # Get param name for strings so we can later display this value in a useful error message if needed + # Only pass 'kwargs' to our checkers and ignore custom callable checkers kwargs = {} - if wanted_name == 'str': + if wanted_name == 'str' and isinstance(wanted, string_types): if isinstance(param, string_types): kwargs['param'] = param elif isinstance(param, dict): @@ -1801,8 +1802,9 @@ class AnsibleModule(object): type_checker, wanted_name = self._get_wanted_type(wanted, k) # Get param name for strings so we can later display this value in a useful error message if needed + # Only pass 'kwargs' to our checkers and ignore custom callable checkers kwargs = {} - if wanted_name == 'str': + if wanted_name == 'str' and isinstance(type_checker, string_types): kwargs['param'] = list(param.keys())[0] # Get the name of the parent key if this is a nested option diff --git a/test/units/module_utils/basic/test_argument_spec.py b/test/units/module_utils/basic/test_argument_spec.py index 4caf7f62f85..b0d4f1a8baf 100644 --- a/test/units/module_utils/basic/test_argument_spec.py +++ b/test/units/module_utils/basic/test_argument_spec.py @@ -15,7 +15,7 @@ import pytest from units.compat.mock import MagicMock from ansible.module_utils import basic from ansible.module_utils.common.warnings import get_deprecation_messages, get_warning_messages -from ansible.module_utils.six import integer_types +from ansible.module_utils.six import integer_types, string_types from ansible.module_utils.six.moves import builtins @@ -101,6 +101,7 @@ def complex_argspec(): baz=dict(fallback=(basic.env_fallback, ['BAZ'])), bar1=dict(type='bool'), bar3=dict(type='list', elements='path'), + bar_str=dict(type='list', elements=str), zardoz=dict(choices=['one', 'two']), zardoz2=dict(type='list', choices=['one', 'two', 'three']), zardoz3=dict(type='str', aliases=['zodraz'], deprecated_aliases=[dict(name='zodraz', version='9.99')]), @@ -212,6 +213,16 @@ def test_validator_function(mocker, stdin): assert am.params['arg'] == 27 +@pytest.mark.parametrize('stdin', [{'arg': '123'}, {'arg': 123}], indirect=['stdin']) +def test_validator_string_type(mocker, stdin): + # Custom callable that is 'str' + argspec = {'arg': {'type': str}} + am = basic.AnsibleModule(argspec) + + assert isinstance(am.params['arg'], string_types) + assert am.params['arg'] == '123' + + @pytest.mark.parametrize('argspec, expected, stdin', [(s[0], s[2], s[1]) for s in INVALID_SPECS], indirect=['stdin']) def test_validator_fail(stdin, capfd, argspec, expected): @@ -342,6 +353,16 @@ class TestComplexArgSpecs: assert "Alias 'zodraz' is deprecated." in get_deprecation_messages()[0]['msg'] assert get_deprecation_messages()[0]['version'] == '9.99' + @pytest.mark.parametrize('stdin', [{'foo': 'hello', 'bar_str': [867, '5309']}], indirect=['stdin']) + def test_list_with_elements_callable_str(self, capfd, mocker, stdin, complex_argspec): + """Test choices with list""" + am = basic.AnsibleModule(**complex_argspec) + assert isinstance(am.params['bar_str'], list) + assert isinstance(am.params['bar_str'][0], string_types) + assert isinstance(am.params['bar_str'][1], string_types) + assert am.params['bar_str'][0] == '867' + assert am.params['bar_str'][1] == '5309' + class TestComplexOptions: """Test arg spec options"""