Fixed password blanked in zhmc_user module

Details:

* Fixed that the zhmc_user module blanked out the 'password' property in
  the input params before passing them on to the zhmcclient
  'User.update_properties()' method. (issue #1081)

Signed-off-by: Andreas Maier <[email protected]>

docs/source/release_notes.rst

@@ -54,6 +54,10 @@ Availability: `AutomationHub`_, `Galaxy`_, `GitHub`_
 
 * Dev: Fixed new issue 'too-many-positional-arguments' reported by Pylint 3.3.0.
 
+* Fixed that the zhmc_user module blanked out the 'password' property in
+  the input params before passing them on to the zhmcclient
+  'User.update_properties()' method. (issue #1081)
+
 **Enhancements:**
 
 * Support for ansible-core 2.18, by adding an ignore file for the sanity tests.

plugins/module_utils/common.py

@@ -26,6 +26,7 @@
 import sys
 import re
 from collections.abc import Mapping
+from copy import deepcopy
 
 try:
     from zhmcclient import Session, ClientAuthError
@@ -1408,3 +1409,29 @@ def wait_ready(self, timeout=None):
         The timeout is an int or float in seconds.
         """
         return self._ready_event.wait(timeout)
+
+
+def params_deepcopy(params):
+    """
+    Return a deep copy of the module input parameters, for dict items where
+    a deep copy is possible.
+
+    For items where a deep copy is not possible, it keeps the original value.
+
+    Reason for this function (instead of simply using `copy.deepcopy()`) is
+    the fact that in this collection, the module input params may contain
+    an optional '_faked_session' item with a value that cannot be copied.
+
+    Parameters:
+      params (dict): Module input parameters.
+
+    Returns:
+      dict: Deep copy of params, where possible.
+    """
+    copy_params = {}
+    for key, value in params.items():
+        try:
+            copy_params[key] = deepcopy(value)
+        except TypeError:
+            copy_params[key] = value
+    return copy_params

plugins/modules/zhmc_user.py

@@ -380,7 +380,8 @@
 from ..module_utils.common import log_init, open_session, close_session, \
     hmc_auth_parameter, Error, ParameterError, to_unicode, \
     process_normal_property, missing_required_lib, \
-    common_fail_on_import_errors, parse_hmc_host, BLANKED_OUT  # noqa: E402
+    common_fail_on_import_errors, parse_hmc_host, BLANKED_OUT, \
+    params_deepcopy  # noqa: E402
 
 try:
     import urllib3
@@ -1167,7 +1168,10 @@ def main():
 
     module.params['hmc_host'] = parse_hmc_host(module.params['hmc_host'])
 
-    _params = dict(module.params)
+    # We need to deepcopy the input parameters, because the dict in which we
+    # blank out the password is at the second level. With a shallow copy,
+    # that would blank out the password in the original params.
+    _params = params_deepcopy(module.params)
     del _params['hmc_auth']
     if _params['properties'] and 'password' in _params['properties']:
         # This is not a hard-coded password. Added # nosec to avoid

tests/end2end/test_zhmc_user.py

@@ -31,6 +31,7 @@
 from zhmcclient.testutils import hmc_definition, hmc_session  # noqa: F401, E501
 # pylint: enable=line-too-long,unused-import
 
+from plugins.module_utils.common import params_deepcopy
 from plugins.modules import zhmc_user
 from .utils import mock_ansible_module, get_failure_msg
 
@@ -460,6 +461,9 @@ def test_zhmc_user_absent_present(
 
         mod_obj = mock_ansible_module(ansible_mod_cls, params, check_mode)
 
+        # Save the input params
+        saved_params = params_deepcopy(params)
+
         # Exercise the code to be tested
         with pytest.raises(SystemExit) as exc_info:
             zhmc_user.main()
@@ -469,6 +473,9 @@ def test_zhmc_user_absent_present(
             f"{where}: Module failed with exit code {exit_code} and " \
             f"message:\n{get_failure_msg(mod_obj)}"
 
+        # Check that the input params have not been changed
+        assert params == saved_params
+
         changed, output_props = get_module_output(mod_obj)
         if changed != exp_changed:
             user_props_sorted = \

tests/unit/test_common.py

@@ -21,6 +21,8 @@
 __metaclass__ = type
 
 import re
+from collections.abc import Sequence, Mapping, Set
+from types import ModuleType
 import pytest
 from immutable_views import DictView
 
@@ -444,3 +446,105 @@ def test_common_hyphen_props(desc, input, exp_result, exp_exc_type):
         result = common.hyphen_properties(input)
 
         assert result == exp_result
+
+
+COMMON_PARAMS_DEEPCOPY_TESTCASES = [
+    # Testcases for test_common_params_deepcopy()
+    # The list items are tuples with the following items:
+    # - desc (string): description of the testcase.
+    # - in_params (dict): Input params
+
+    (
+        "Empty dict",
+        {}
+    ),
+    (
+        "Dict with string keys and immutable values",
+        {
+            'int': 1,
+            'float': 1.1,
+            'complex': (1.1 + 1.1j),
+            'short str': 'foo',
+            'long str': 'foo' * 100,
+            'tuple int': (1, 2),
+            'short bytes': b'foo',
+            'long bytes': b'foo' * 100,
+            'bool': True,
+            'frozenset': frozenset({1, 2, 3}),
+            'none': None,
+        }
+    ),
+    (
+        "Dict with string keys and mutable values",
+        {
+            'dict': {'a': 1},
+            'list': ['a', 'b'],
+            'set': ('a', 'b'),
+        }
+    ),
+    (
+        "Dict with string keys and values that cannot be deepcopy()'ed",
+        {
+            'module': re,
+        }
+    ),
+]
+
+# Types used in the testcases that are mutable
+MUTABLE_TYPES = (list, dict, set)
+
+# Types used in the testcases on which deepcopy() fails
+DEEPCOPY_FAILS_TYPES = (ModuleType, )
+
+
+def assert_disparate_equal(obj_a, obj_b):
+    """
+    Assert that obj_a and obj_b are equal but not identical (except when
+    immutable).
+
+    obj_a and obj_b must not have dependency loops.
+
+    In other words, assert that the two objects are separate deep copies.
+    """
+
+    # Depending on the type, this checks for value equality or just same object
+    assert obj_a == obj_b
+
+    # Immutable types may be identical, mutable types must be disparate.
+    if isinstance(obj_a, MUTABLE_TYPES):
+        assert id(obj_a) != id(obj_b)
+
+    # params_deepcopy() does not copy types where deepcopy() fails.
+    if isinstance(obj_a, DEEPCOPY_FAILS_TYPES):
+        assert id(obj_a) == id(obj_b)
+
+    # For collections, check the items recursively
+    if isinstance(obj_a, Sequence) and not isinstance(obj_a, (str, bytes)):
+        for i, value_a in enumerate(obj_a):
+            value_b = obj_b[i]
+            assert_disparate_equal(value_a, value_b)
+    elif isinstance(obj_a, Mapping):
+        for key, value_a in obj_a.items():
+            value_b = obj_b[key]
+            assert_disparate_equal(value_a, value_b)
+    elif isinstance(obj_a, Set):
+        sorted_a = sorted(obj_a)
+        sorted_b = sorted(obj_b)
+        for i, value_a in enumerate(sorted_a):
+            value_b = sorted_b[i]
+            assert_disparate_equal(value_a, value_b)
+
+
+@pytest.mark.parametrize(
+    "desc, in_params",
+    COMMON_PARAMS_DEEPCOPY_TESTCASES)
+def test_common_params_deepcopy(desc, in_params):
+    # pylint: disable=unused-argument
+    """
+    Test the params_deepcopy() function.
+    """
+
+    # The code to be tested
+    params = common.params_deepcopy(in_params)
+
+    assert_disparate_equal(params, in_params)