diff --git a/docs/source/release_notes.rst b/docs/source/release_notes.rst index 346bec6b3..4e74bf1ee 100644 --- a/docs/source/release_notes.rst +++ b/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. diff --git a/plugins/module_utils/common.py b/plugins/module_utils/common.py index ec51d77cb..901e9e88b 100644 --- a/plugins/module_utils/common.py +++ b/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 diff --git a/plugins/modules/zhmc_user.py b/plugins/modules/zhmc_user.py index f7434bd2f..a66b19d0c 100644 --- a/plugins/modules/zhmc_user.py +++ b/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 diff --git a/tests/end2end/test_zhmc_user.py b/tests/end2end/test_zhmc_user.py index 968028121..e613b33d2 100644 --- a/tests/end2end/test_zhmc_user.py +++ b/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 = \ diff --git a/tests/unit/test_common.py b/tests/unit/test_common.py index b2f684145..2ac248fce 100644 --- a/tests/unit/test_common.py +++ b/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)