From 2f36b9e5ce0ec41a822752845d3b7c4afdf7eee9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Moser?= <mail@renemoser.net>
Date: Thu, 8 Feb 2018 07:59:21 +0100
Subject: [PATCH] basic: allow one or more when param list having choices
 (#34537)

* basic: allow one or more when param list having choices

* add unit tests

* optimize a bit

* re-add get_exception import

* a number of existing modules expect to be able to get it from basic.py
---
 lib/ansible/module_utils/basic.py             | 13 ++++++++++--
 .../module_utils/basic/test_argument_spec.py  | 20 +++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py
index b6ee8880fbc..3a8fa29249a 100644
--- a/lib/ansible/module_utils/basic.py
+++ b/lib/ansible/module_utils/basic.py
@@ -170,7 +170,7 @@ from ansible.module_utils.six import (
 )
 from ansible.module_utils.six.moves import map, reduce, shlex_quote
 from ansible.module_utils._text import to_native, to_bytes, to_text
-from ansible.module_utils.parsing.convert_bool import BOOLEANS, BOOLEANS_FALSE, BOOLEANS_TRUE, boolean
+from ansible.module_utils.parsing.convert_bool import BOOLEANS_FALSE, BOOLEANS_TRUE, boolean
 
 
 PASSWORD_MATCH = re.compile(r'^(?:.+[-_\s])?pass(?:[-_\s]?(?:word|phrase|wrd|wd)?)(?:[-_\s].+)?$', re.I)
@@ -1778,7 +1778,16 @@ class AnsibleModule(object):
                 continue
             if isinstance(choices, SEQUENCETYPE) and not isinstance(choices, (binary_type, text_type)):
                 if k in param:
-                    if param[k] not in choices:
+                    # Allow one or more when type='list' param with choices
+                    if isinstance(param[k], list):
+                        diff_list = ", ".join([item for item in param[k] if item not in choices])
+                        if diff_list:
+                            choices_str = ", ".join([to_native(c) for c in choices])
+                            msg = "value of %s must be one or more of: %s. Got no match for: %s" % (k, choices_str, diff_list)
+                            if self._options_context:
+                                msg += " found in %s" % " -> ".join(self._options_context)
+                            self.fail_json(msg=msg)
+                    elif param[k] not in choices:
                         # PyYaml converts certain strings to bools.  If we can unambiguously convert back, do so before checking
                         # the value.  If we can't figure this out, module author is responsible.
                         lowered_choices = None
diff --git a/test/units/module_utils/basic/test_argument_spec.py b/test/units/module_utils/basic/test_argument_spec.py
index babca5a0744..368d71ad9ed 100644
--- a/test/units/module_utils/basic/test_argument_spec.py
+++ b/test/units/module_utils/basic/test_argument_spec.py
@@ -71,6 +71,7 @@ def complex_argspec():
         baz=dict(fallback=(basic.env_fallback, ['BAZ'])),
         bar1=dict(type='bool'),
         zardoz=dict(choices=['one', 'two']),
+        zardoz2=dict(type='list', choices=['one', 'two', 'three']),
     )
     mut_ex = (('bar', 'bam'),)
     req_to = (('bam', 'baz'),)
@@ -254,6 +255,25 @@ class TestComplexArgSpecs:
         assert results['failed']
         assert results['msg'] == "parameters are required together: bam, baz"
 
+    @pytest.mark.parametrize('stdin', [{'foo': 'hello', 'zardoz2': ['one', 'four', 'five']}], indirect=['stdin'])
+    def test_fail_list_with_choices(self, capfd, mocker, stdin, complex_argspec):
+        """Fail because one of the items is not in the choice"""
+        with pytest.raises(SystemExit):
+            basic.AnsibleModule(**complex_argspec)
+
+        out, err = capfd.readouterr()
+        results = json.loads(out)
+
+        assert results['failed']
+        assert results['msg'] == "value of zardoz2 must be one or more of: one, two, three. Got no match for: four, five"
+
+    @pytest.mark.parametrize('stdin', [{'foo': 'hello', 'zardoz2': ['one', 'three']}], indirect=['stdin'])
+    def test_list_with_choices(self, capfd, mocker, stdin, complex_argspec):
+        """Test choices with list"""
+        am = basic.AnsibleModule(**complex_argspec)
+        assert isinstance(am.params['zardoz2'], list)
+        assert am.params['zardoz2'] == ['one', 'three']
+
 
 class TestComplexOptions:
     """Test arg spec options"""