diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d79d89d..e0bfe3ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +* **plugin:nextcloud_occ_app_config**: An `array` config value is now compared as JSON, so a key whose stored value already matches the desired one no longer reports a change (and re-runs `occ config:app:set`) on every run. * **plugin:bitwarden_item**: The module no longer writes to the Bitwarden vault when run in check mode (`--check`); it reports the would-be change instead. * **plugin:bitwarden_item**: A run without `password` (the default `None`) no longer overwrites an existing item's password; the current password is preserved, matching the documented behavior. * **plugin:sqlite_query**: A failed query now fails the task instead of reporting success with the error text in `query_result`. Playbooks that relied on the previous silent success will now correctly fail. diff --git a/plugins/modules/nextcloud_occ_app_config.py b/plugins/modules/nextcloud_occ_app_config.py index ffd583c5..269f4cb9 100644 --- a/plugins/modules/nextcloud_occ_app_config.py +++ b/plugins/modules/nextcloud_occ_app_config.py @@ -117,6 +117,23 @@ from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.common.text.converters import to_native + +def values_match(current_value, value, value_type): + """Decide whether the stored value already matches the desired one. + + For C(array) values both sides are compared as parsed JSON, so an + array stored by Nextcloud (returned as a JSON array, e.g. via + C(config:list)) compares equal to the user's array literal regardless + of whitespace or key ordering. All other types compare as strings. + """ + if value_type == 'array': + try: + return json.loads(current_value) == json.loads(value) + except (json.JSONDecodeError, ValueError, TypeError): + return False + return current_value == value + + def main(): # define available arguments/parameters a user can pass to this module module_args = dict( @@ -187,7 +204,9 @@ def main(): current_value = str(raw) current_type = 'float' elif isinstance(raw, list): - current_value = str(raw) + # store canonical JSON so it can be compared as JSON against the + # user's array literal (config:list returns an already-parsed list) + current_value = json.dumps(raw) current_type = 'array' else: current_value = str(raw) @@ -225,7 +244,7 @@ def main(): if state == 'present': # check if the current value and type match the desired settings - if current_value == value and current_type == value_type: + if current_type == value_type and values_match(current_value, value, value_type): module.exit_json(**result) # else, the value will be changed diff --git a/tests/unit/plugins/modules/test_nextcloud_occ_app_config.py b/tests/unit/plugins/modules/test_nextcloud_occ_app_config.py new file mode 100644 index 00000000..9871eb69 --- /dev/null +++ b/tests/unit/plugins/modules/test_nextcloud_occ_app_config.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8; py-indent-offset: 4 -*- +# +# Author: Linuxfabrik GmbH, Zurich, Switzerland +# Contact: info (at) linuxfabrik (dot) ch +# https://www.linuxfabrik.ch/ +# License: The Unlicense, see LICENSE file. + +"""Unit tests for nextcloud_occ_app_config array idempotency. + +Nextcloud stores an C(array) value and returns it as a parsed JSON array +via C(config:list) (verified against Nextcloud 33: the value comes back +as C(["alpha", "beta"])). Comparing that as a Python repr string against +the user's array literal never matched, so the module reported a change +on every run. values_match() now compares array values as parsed JSON. +The collection import is wired up by tests/conftest.py. +""" + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +import unittest + +import ansible_harness + +from ansible_collections.linuxfabrik.lfops.plugins.modules import nextcloud_occ_app_config as mod + + +class TestValuesMatch(unittest.TestCase): + + def test_array_equal_ignoring_whitespace(self): + # occ returns '["alpha","beta"]'; user passes a spaced literal + self.assertTrue(mod.values_match('["alpha","beta"]', '["alpha", "beta"]', 'array')) + + def test_array_canonical_vs_user(self): + # cached path stores json.dumps(list) -> '["alpha", "beta"]' + self.assertTrue(mod.values_match('["alpha", "beta"]', '["alpha","beta"]', 'array')) + + def test_array_different(self): + self.assertFalse(mod.values_match('["alpha", "beta"]', '["alpha","gamma"]', 'array')) + + def test_array_invalid_json_is_not_a_match(self): + self.assertFalse(mod.values_match("['alpha', 'beta']", '["alpha","beta"]', 'array')) + + def test_non_array_string_compare(self): + self.assertTrue(mod.values_match('90', '90', 'integer')) + self.assertFalse(mod.values_match('90', '91', 'integer')) + + +class TestMainCachedArray(unittest.TestCase): + """Exercise main() via the installed_config_json (cache) path, no occ needed.""" + + def setUp(self): + self._patch = ansible_harness.patch_module() + self._patch.start() + + def tearDown(self): + self._patch.stop() + + def _run(self, args): + ansible_harness.set_module_args(args) + try: + mod.main() + except ansible_harness.AnsibleExitJson as exc: + return exc.args[0] + raise AssertionError('module did not call exit_json') + + def test_array_already_set_is_idempotent(self): + result = self._run({ + 'app': 'core', + 'name': 'test_array', + 'value': '["alpha","beta"]', + 'type': 'array', + 'installed_config_json': {'apps': {'core': {'test_array': ['alpha', 'beta']}}}, + }) + self.assertFalse(result['changed']) + + def test_array_differs_reports_change(self): + result = self._run({ + 'app': 'core', + 'name': 'test_array', + 'value': '["alpha","beta"]', + 'type': 'array', + 'installed_config_json': {'apps': {'core': {'test_array': ['alpha', 'gamma']}}}, + '_ansible_check_mode': True, # avoid the real occ config:app:set call + }) + self.assertTrue(result['changed']) + + +if __name__ == '__main__': + unittest.main()