diff --git a/changelogs/fragments/6270-gconftool2-changed.yml b/changelogs/fragments/6270-gconftool2-changed.yml new file mode 100644 index 0000000000..dff6d71748 --- /dev/null +++ b/changelogs/fragments/6270-gconftool2-changed.yml @@ -0,0 +1,5 @@ +bugfixes: + - gconftool2 - fix ``changed`` result always being ``true`` (https://github.com/ansible-collections/community.general/issues/6028). + - gconftool2 - remove requirement of parameter ``value`` when ``state=absent`` (https://github.com/ansible-collections/community.general/issues/6028). +breaking_changes: + - gconftool2 - fix processing of ``gconftool-2`` when ``key`` does not exist, returning ``null`` instead of empty string for both ``value`` and ``previous_value`` return values (https://github.com/ansible-collections/community.general/issues/6028). diff --git a/plugins/modules/gconftool2.py b/plugins/modules/gconftool2.py index 949e92b306..704bfb18d9 100644 --- a/plugins/modules/gconftool2.py +++ b/plugins/modules/gconftool2.py @@ -35,12 +35,13 @@ options: type: str description: - Preference keys typically have simple values such as strings, - integers, or lists of strings and integers. This is ignored if the state - is "get". See man gconftool-2(1). + integers, or lists of strings and integers. + This is ignored unless I(state=present). See man gconftool-2(1). value_type: type: str description: - - The type of value being set. This is ignored if the state is "get". + - The type of value being set. + This is ignored unless I(state=present). See man gconftool-2(1). choices: [ bool, float, int, string ] state: type: str @@ -56,8 +57,8 @@ options: See man gconftool-2(1). direct: description: - - Access the config database directly, bypassing server. If direct is - specified then the config_source must be specified as well. + - Access the config database directly, bypassing server. If I(direct) is + specified then the I(config_source) must be specified as well. See man gconftool-2(1). type: bool default: false @@ -73,17 +74,26 @@ EXAMPLES = """ RETURN = ''' key: - description: The key specified in the module parameters + description: The key specified in the module parameters. returned: success type: str sample: /desktop/gnome/interface/font_name value_type: - description: The type of the value that was changed + description: The type of the value that was changed. returned: success type: str sample: string value: - description: The value of the preference key after executing the module + description: + - The value of the preference key after executing the module or C(null) if key is removed. + - From community.general 7.0.0 onwards it returns C(null) for a non-existent I(key), and returns C("") before that. + returned: success + type: str + sample: "Serif 12" + previous_value: + description: + - The value of the preference key before executing the module. + - From community.general 7.0.0 onwards it returns C(null) for a non-existent I(key), and returns C("") before that. returned: success type: str sample: "Serif 12" @@ -95,7 +105,6 @@ from ansible_collections.community.general.plugins.module_utils.gconftool2 impor class GConftool(StateModuleHelper): - change_params = ('value', ) diff_params = ('value', ) output_params = ('key', 'value_type') facts_params = ('key', 'value_type') @@ -111,7 +120,6 @@ class GConftool(StateModuleHelper): ), required_if=[ ('state', 'present', ['value', 'value_type']), - ('state', 'absent', ['value']), ('direct', True, ['config_source']), ], supports_check_mode=True, @@ -125,6 +133,7 @@ class GConftool(StateModuleHelper): self.vars.set('previous_value', self._get(), fact=True) self.vars.set('value_type', self.vars.value_type) + self.vars.set('_value', self.vars.previous_value, output=False, change=True) self.vars.set_meta('value', initial_value=self.vars.previous_value) self.vars.set('playbook_value', self.vars.value, fact=True) @@ -132,7 +141,8 @@ class GConftool(StateModuleHelper): def process(rc, out, err): if err and fail_on_err: self.ansible.fail_json(msg='gconftool-2 failed with error: %s' % (str(err))) - self.vars.value = out.rstrip() + out = out.rstrip() + self.vars.value = None if out == "" else out return self.vars.value return process @@ -148,11 +158,18 @@ class GConftool(StateModuleHelper): def state_absent(self): with self.runner("state key", output_process=self._make_process(False)) as ctx: ctx.run() + if self.verbosity >= 4: + self.vars.run_info = ctx.run_info self.vars.set('new_value', None, fact=True) + self.vars._value = None def state_present(self): with self.runner("direct config_source value_type state key value", output_process=self._make_process(True)) as ctx: - self.vars.set('new_value', ctx.run(), fact=True) + ctx.run() + if self.verbosity >= 4: + self.vars.run_info = ctx.run_info + self.vars.set('new_value', self._get(), fact=True) + self.vars._value = self.vars.new_value def main(): diff --git a/tests/unit/plugins/modules/test_gconftool2.py b/tests/unit/plugins/modules/test_gconftool2.py index f01f15ef82..31d4df29f2 100644 --- a/tests/unit/plugins/modules/test_gconftool2.py +++ b/tests/unit/plugins/modules/test_gconftool2.py @@ -36,7 +36,6 @@ TEST_CASES = [ (0, '100\n', '',), ), ], - 'new_value': '100', } ], [ @@ -50,11 +49,10 @@ TEST_CASES = [ (0, '', "No value set for `/desktop/gnome/background/picture_filename'\n",), ), ], - 'new_value': None, } ], [ - {'state': 'present', 'key': '/desktop/gnome/background/picture_filename', 'value': '200', 'value_type': 'int'}, + {'state': 'present', 'key': '/desktop/gnome/background/picture_filename', 'value': 200, 'value_type': 'int'}, { 'id': 'test_simple_element_set', 'run_command.calls': [ @@ -66,10 +64,104 @@ TEST_CASES = [ ( ['/testbin/gconftool-2', '--type', 'int', '--set', '/desktop/gnome/background/picture_filename', '200'], {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '', '',), + ), + ( + ['/testbin/gconftool-2', '--get', '/desktop/gnome/background/picture_filename'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, (0, '200\n', '',), ), ], 'new_value': '200', + 'changed': True, + } + ], + [ + {'state': 'present', 'key': '/desktop/gnome/background/picture_filename', 'value': 200, 'value_type': 'int'}, + { + 'id': 'test_simple_element_set_idempotency_int', + 'run_command.calls': [ + ( + ['/testbin/gconftool-2', '--get', '/desktop/gnome/background/picture_filename'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '200\n', '',), + ), + ( + ['/testbin/gconftool-2', '--type', 'int', '--set', '/desktop/gnome/background/picture_filename', '200'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '', '',), + ), + ( + ['/testbin/gconftool-2', '--get', '/desktop/gnome/background/picture_filename'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '200\n', '',), + ), + ], + 'new_value': '200', + 'changed': False, + } + ], + [ + {'state': 'present', 'key': '/apps/gnome_settings_daemon/screensaver/start_screensaver', 'value': 'false', 'value_type': 'bool'}, + { + 'id': 'test_simple_element_set_idempotency_bool', + 'run_command.calls': [ + ( + ['/testbin/gconftool-2', '--get', '/apps/gnome_settings_daemon/screensaver/start_screensaver'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, 'false\n', '',), + ), + ( + ['/testbin/gconftool-2', '--type', 'bool', '--set', '/apps/gnome_settings_daemon/screensaver/start_screensaver', 'false'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '', '',), + ), + ( + ['/testbin/gconftool-2', '--get', '/apps/gnome_settings_daemon/screensaver/start_screensaver'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, 'false\n', '',), + ), + ], + 'new_value': 'false', + 'changed': False, + } + ], + [ + {'state': 'absent', 'key': '/desktop/gnome/background/picture_filename'}, + { + 'id': 'test_simple_element_unset', + 'run_command.calls': [ + ( + ['/testbin/gconftool-2', '--get', '/desktop/gnome/background/picture_filename'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '200\n', '',), + ), + ( + ['/testbin/gconftool-2', '--unset', '/desktop/gnome/background/picture_filename'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '', '',), + ), + ], + 'changed': True, + } + ], + [ + {'state': 'absent', 'key': '/apps/gnome_settings_daemon/screensaver/start_screensaver'}, + { + 'id': 'test_simple_element_unset_idempotency', + 'run_command.calls': [ + ( + ['/testbin/gconftool-2', '--get', '/apps/gnome_settings_daemon/screensaver/start_screensaver'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '', '',), + ), + ( + ['/testbin/gconftool-2', '--unset', '/apps/gnome_settings_daemon/screensaver/start_screensaver'], + {'environ_update': {'LANGUAGE': 'C', 'LC_ALL': 'C'}, 'check_rc': True}, + (0, '', '',), + ), + ], + 'changed': False, } ], ] @@ -101,6 +193,11 @@ def test_gconftool2(mocker, capfd, patch_gconftool2, testcase): print("testcase =\n%s" % testcase) print("results =\n%s" % results) + if 'changed' in testcase: + assert results.get('changed', False) == testcase['changed'] + if 'new_value' in testcase: + assert results.get('new_value', None) == testcase['new_value'] + for conditional_test_result in ('value',): if conditional_test_result in testcase: assert conditional_test_result in results, "'{0}' not found in {1}".format(conditional_test_result, results)