Move _handle_no_log_values() out of basic.py (#48628)

* Rename method and make private
* Use is_iterable, combine transformations
* Remove unused return_values from network modules
* Improve docstrings in new functions
* Add new PASS_VAR
* Add unit tests for list_no_log_values
* Fix unit tests for Python 2.6

lib/ansible/module_utils/basic.py

@@ -155,6 +155,8 @@ from ansible.module_utils.common.sys_info import (
 from ansible.module_utils.pycompat24 import get_exception, literal_eval
 from ansible.module_utils.common.parameters import (
     handle_aliases,
+    list_deprecations,
+    list_no_log_values,
     PASS_VARS,
 )
 
@@ -173,18 +175,13 @@ 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
 
+
 # Note: When getting Sequence from collections, it matches with strings. If
 # this matters, make sure to check for strings before checking for sequencetype
 SEQUENCETYPE = frozenset, KeysView, Sequence
+
 PASSWORD_MATCH = re.compile(r'^(?:.+[-_\s])?pass(?:[-_\s]?(?:word|phrase|wrd|wd)?)(?:[-_\s].+)?$', re.I)
 
-_NUMBERTYPES = tuple(list(integer_types) + [float])
-
-# Deprecated compat.  Only kept in case another module used these names  Using
-# ansible.module_utils.six is preferred
-
-NUMBERTYPES = _NUMBERTYPES
-
 imap = map
 
 try:
@@ -336,31 +333,6 @@ def json_dict_bytes_to_unicode(d, encoding='utf-8', errors='surrogate_or_strict'
         return d
 
 
-def return_values(obj):
-    """ Return native stringified values from datastructures.
-
-    For use with removing sensitive values pre-jsonification."""
-    if isinstance(obj, (text_type, binary_type)):
-        if obj:
-            yield to_native(obj, errors='surrogate_or_strict')
-        return
-    elif isinstance(obj, SEQUENCETYPE):
-        for element in obj:
-            for subelement in return_values(element):
-                yield subelement
-    elif isinstance(obj, Mapping):
-        for element in obj.items():
-            for subelement in return_values(element[1]):
-                yield subelement
-    elif isinstance(obj, (bool, NoneType)):
-        # This must come before int because bools are also ints
-        return
-    elif isinstance(obj, NUMBERTYPES):
-        yield to_native(obj, nonstring='simplerepr')
-    else:
-        raise TypeError('Unknown parameter type: %s, %s' % (type(obj), obj))
-
-
 def _remove_values_conditions(value, no_log_strings, deferred_removals):
     """
     Helper function for :meth:`remove_values`.
@@ -436,7 +408,7 @@ def _remove_values_conditions(value, no_log_strings, deferred_removals):
         deferred_removals.append((value, new_value))
         value = new_value
 
-    elif isinstance(value, tuple(chain(NUMBERTYPES, (bool, NoneType)))):
+    elif isinstance(value, tuple(chain(integer_types, (float, bool, NoneType)))):
         stringy_value = to_native(value, encoding='utf-8', errors='surrogate_or_strict')
         if stringy_value in no_log_strings:
             return 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER'
@@ -1581,19 +1553,8 @@ class AnsibleModule(object):
         if param is None:
             param = self.params
 
-        # Use the argspec to determine which args are no_log
+        self.no_log_values.update(list_no_log_values(spec, param))
-        for arg_name, arg_opts in spec.items():
+        self._deprecations.extend(list_deprecations(spec, param))
-            if arg_opts.get('no_log', False):
-                # Find the value for the no_log'd param
-                no_log_object = param.get(arg_name, None)
-                if no_log_object:
-                    self.no_log_values.update(return_values(no_log_object))
-
-            if arg_opts.get('removed_in_version') is not None and arg_name in param:
-                self._deprecations.append({
-                    'msg': "Param '%s' is deprecated. See the module docs for more information" % arg_name,
-                    'version': arg_opts.get('removed_in_version')
-                })
 
     def _check_arguments(self, check_invalid_arguments, spec=None, param=None, legal_inputs=None):
         self._syslog_facility = 'LOG_USER'

lib/ansible/module_utils/common/parameters.py

@@ -37,6 +37,77 @@ PASS_VARS = {
 }
 
 
+def _return_datastructure_name(obj):
+    """ Return native stringified values from datastructures.
+
+    For use with removing sensitive values pre-jsonification."""
+    if isinstance(obj, (text_type, binary_type)):
+        if obj:
+            yield to_native(obj, errors='surrogate_or_strict')
+        return
+    elif isinstance(obj, Mapping):
+        for element in obj.items():
+            for subelement in _return_datastructure_name(element[1]):
+                yield subelement
+    elif is_iterable(obj):
+        for element in obj:
+            for subelement in _return_datastructure_name(element):
+                yield subelement
+    elif isinstance(obj, (bool, NoneType)):
+        # This must come before int because bools are also ints
+        return
+    elif isinstance(obj, tuple(list(integer_types) + [float])):
+        yield to_native(obj, nonstring='simplerepr')
+    else:
+        raise TypeError('Unknown parameter type: %s, %s' % (type(obj), obj))
+
+
+def list_no_log_values(argument_spec, params):
+    """Return set of no log values
+
+    :arg argument_spec: An argument spec dictionary from a module
+    :arg params: Dictionary of all module parameters
+
+    :returns: Set of strings that should be hidden from output::
+
+        {'secret_dict_value', 'secret_list_item_one', 'secret_list_item_two', 'secret_string'}
+    """
+
+    no_log_values = set()
+    for arg_name, arg_opts in argument_spec.items():
+        if arg_opts.get('no_log', False):
+            # Find the value for the no_log'd param
+            no_log_object = params.get(arg_name, None)
+
+            if no_log_object:
+                no_log_values.update(_return_datastructure_name(no_log_object))
+
+    return no_log_values
+
+
+def list_deprecations(argument_spec, params):
+    """Return a list of deprecations
+
+    :arg argument_spec: An argument spec dictionary from a module
+    :arg params: Dictionary of all module parameters
+
+    :returns: List of dictionaries containing a message and version in which
+        the deprecated parameter will be removed, or an empty list::
+
+            [{'msg': "Param 'deptest' is deprecated. See the module docs for more information", 'version': '2.9'}]
+    """
+
+    deprecations = []
+    for arg_name, arg_opts in argument_spec.items():
+        if arg_opts.get('removed_in_version') is not None and arg_name in params:
+            deprecations.append({
+                'msg': "Param '%s' is deprecated. See the module docs for more information" % arg_name,
+                'version': arg_opts.get('removed_in_version')
+            })
+
+    return deprecations
+
+
 def handle_aliases(argument_spec, params):
     """Return a two item tuple. The first is a dictionary of aliases, the second is
     a list of legal inputs."""

lib/ansible/module_utils/network/aireos/aireos.py

@@ -26,7 +26,7 @@
 # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import exec_command
 

lib/ansible/module_utils/network/aruba/aruba.py

@@ -29,7 +29,7 @@
 import re
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import exec_command
 

lib/ansible/module_utils/network/asa/asa.py

@@ -26,7 +26,7 @@
 # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, EntityCollection
 from ansible.module_utils.connection import exec_command
 from ansible.module_utils.connection import Connection, ConnectionError

lib/ansible/module_utils/network/cnos/cnos.py

@@ -43,7 +43,7 @@ except Exception:
     HAS_LIB = False
 from distutils.cmd import Command
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, EntityCollection
 from ansible.module_utils.connection import Connection, exec_command
 from ansible.module_utils.connection import ConnectionError

lib/ansible/module_utils/network/dellos10/dellos10.py

@@ -32,7 +32,7 @@
 import re
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import exec_command
 from ansible.module_utils.network.common.config import NetworkConfig, ConfigLine

lib/ansible/module_utils/network/dellos6/dellos6.py

@@ -32,7 +32,7 @@
 import re
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import exec_command
 from ansible.module_utils.network.common.config import NetworkConfig, ConfigLine, ignore_line

lib/ansible/module_utils/network/dellos9/dellos9.py

@@ -32,7 +32,7 @@
 import re
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import exec_command
 from ansible.module_utils.network.common.config import NetworkConfig, ConfigLine

lib/ansible/module_utils/network/enos/enos.py

@@ -32,7 +32,7 @@
 # Lenovo Networking
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, EntityCollection
 from ansible.module_utils.connection import Connection, exec_command
 from ansible.module_utils.connection import ConnectionError

lib/ansible/module_utils/network/eos/eos.py

@@ -32,7 +32,7 @@ import os
 import time
 
 from ansible.module_utils._text import to_text, to_native
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.connection import Connection, ConnectionError
 from ansible.module_utils.network.common.config import NetworkConfig, dumps
 from ansible.module_utils.network.common.utils import to_list, ComplexList

lib/ansible/module_utils/network/exos/exos.py

@@ -27,7 +27,7 @@
 #
 import json
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list
 from ansible.module_utils.connection import Connection
 

lib/ansible/module_utils/network/ironware/ironware.py

@@ -20,7 +20,7 @@ from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, EntityCollection
 from ansible.module_utils.connection import Connection, exec_command
 

lib/ansible/module_utils/network/junos/junos.py

@@ -21,7 +21,7 @@ import json
 from contextlib import contextmanager
 from copy import deepcopy
 
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.connection import Connection, ConnectionError
 from ansible.module_utils.network.common.netconf import NetconfConnection
 from ansible.module_utils._text import to_text

lib/ansible/module_utils/network/nxos/nxos.py

@@ -33,7 +33,7 @@ import json
 import re
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values, get_timestamp
+from ansible.module_utils.basic import env_fallback, get_timestamp
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import Connection, ConnectionError
 from ansible.module_utils.common._collections_compat import Mapping

lib/ansible/module_utils/network/routeros/routeros.py

@@ -27,7 +27,7 @@
 #
 import json
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import Connection, ConnectionError
 

lib/ansible/module_utils/network/sros/sros.py

@@ -31,7 +31,7 @@
 import re
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.network.common.utils import to_list, ComplexList
 from ansible.module_utils.connection import exec_command
 

lib/ansible/module_utils/network/vyos/vyos.py

@@ -28,7 +28,7 @@
 import json
 
 from ansible.module_utils._text import to_text
-from ansible.module_utils.basic import env_fallback, return_values
+from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.connection import Connection, ConnectionError
 
 _DEVICE_CONFIGS = {}

test/units/executor/module_common/test_recursive_finder.py

@@ -46,6 +46,7 @@ MODULE_UTILS_BASIC_IMPORTS = frozenset((('_text',),
                                         ('common', '_collections_compat'),
                                         ('common', 'collections'),
                                         ('common', 'file'),
+                                        ('common', 'collections'),
                                         ('common', 'parameters'),
                                         ('common', 'process'),
                                         ('common', 'sys_info'),

test/units/module_utils/basic/test_no_log.py

@@ -8,7 +8,8 @@ __metaclass__ = type
 
 from units.compat import unittest
 
-from ansible.module_utils.basic import return_values, remove_values
+from ansible.module_utils.basic import remove_values
+from ansible.module_utils.common.parameters import _return_datastructure_name
 
 
 class TestReturnValues(unittest.TestCase):
@@ -40,12 +41,12 @@ class TestReturnValues(unittest.TestCase):
         ('Toshio くらとみ', frozenset(['Toshio くらとみ'])),
     )
 
-    def test_return_values(self):
+    def test_return_datastructure_name(self):
         for data, expected in self.dataset:
-            self.assertEquals(frozenset(return_values(data)), expected)
+            self.assertEquals(frozenset(_return_datastructure_name(data)), expected)
 
     def test_unknown_type(self):
-        self.assertRaises(TypeError, frozenset, return_values(object()))
+        self.assertRaises(TypeError, frozenset, _return_datastructure_name(object()))
 
 
 class TestRemoveValues(unittest.TestCase):

test/units/module_utils/common/parameters/test_list_deprecations.py

@@ -0,0 +1,35 @@
+# -*- 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.parameters import list_deprecations
+
+
+@pytest.fixture
+def params():
+    return {
+        'name': 'bob',
+        'dest': '/etc/hosts',
+        'state': 'present',
+        'value': 5,
+    }
+
+
+def test_list_deprecations():
+    argument_spec = {
+        'old': {'type': 'str', 'removed_in_version': '2.5'}
+    }
+
+    params = {
+        'name': 'rod',
+        'old': 'option',
+    }
+    result = list_deprecations(argument_spec, params)
+    for item in result:
+        assert item['msg'] == "Param 'old' is deprecated. See the module docs for more information"
+        assert item['version'] == '2.5'

test/units/module_utils/common/parameters/test_list_no_log_values.py

@@ -0,0 +1,41 @@
+# -*- 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.parameters import list_no_log_values
+
+
+@pytest.fixture
+def params():
+    return {
+        'secret': 'undercookwovennativity',
+        'other_secret': 'cautious-slate-makeshift',
+        'state': 'present',
+        'value': 5,
+    }
+
+
+def test_list_no_log_values(params):
+    argument_spec = {
+        'secret': {'type': 'str', 'no_log': True},
+        'other_secret': {'type': 'str', 'no_log': True},
+        'state': {'type': 'str'},
+        'value': {'type': 'int'},
+    }
+    result = set(('undercookwovennativity', 'cautious-slate-makeshift'))
+    assert result == list_no_log_values(argument_spec, params)
+
+
+def test_list_no_log_values_no_secrets(params):
+    argument_spec = {
+        'other_secret': {'type': 'str', 'no_log': False},
+        'state': {'type': 'str'},
+        'value': {'type': 'int'},
+    }
+    result = set()
+    assert result == list_no_log_values(argument_spec, params)