From 627371e2d8cfdd9be43a00e697a0d631036bcb10 Mon Sep 17 00:00:00 2001 From: Jonathan Kamens Date: Sat, 4 Mar 2023 14:51:50 -0500 Subject: [PATCH] dconf: Check for changes properly despite style of quotes used by user (#6049) dconf: parse GVariant values to check for equality whenever possible Direct string comparisons are an inaccurate way to compare two GVariant representations. For example, 'foo' and "foo" (including the quote marks, which are part of the representation) are equal GVariants but if you just do a string compare (remember, including the quotes) they'll be interpreted. We therefore want to use the `gi.repository` Python library to parse GVariant representations before comparing them whenever possible. However, we don't want to assume that this library will always be available or require it for Ansible to function, so we use a straight string comparison as a fallback when the library isn't available. This may result in some false positives, i.e., Ansible thinking a value is changing when it actually isn't, but will not result in incorrect values being written into `dconf`. Co-authored-by: Jonathan Kamens --- changelogs/fragments/6049-dconf-strings.yml | 2 + plugins/modules/dconf.py | 45 ++++++++++++++++++++- tests/unit/plugins/modules/test_dconf.py | 44 ++++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/6049-dconf-strings.yml create mode 100644 tests/unit/plugins/modules/test_dconf.py diff --git a/changelogs/fragments/6049-dconf-strings.yml b/changelogs/fragments/6049-dconf-strings.yml new file mode 100644 index 0000000000..1d194b3389 --- /dev/null +++ b/changelogs/fragments/6049-dconf-strings.yml @@ -0,0 +1,2 @@ +minor_changes: + - dconf - parse GVariants for equality comparison when the Python module ``gi.repository`` is available (https://github.com/ansible-collections/community.general/pull/6049). diff --git a/plugins/modules/dconf.py b/plugins/modules/dconf.py index 4d7ed34bf9..b5ece96ec1 100644 --- a/plugins/modules/dconf.py +++ b/plugins/modules/dconf.py @@ -21,11 +21,23 @@ description: - Since C(dconf) requires a running D-Bus session to change values, the module will try to detect an existing session and reuse it, or run the tool via C(dbus-run-session). +requirements: + - Optionally the C(gi.repository) Python library (usually included in the OS + on hosts which have C(dconf)); this will become a non-optional requirement + in a future major release of community.general. notes: - This module depends on C(psutil) Python library (version 4.0.0 and upwards), C(dconf), C(dbus-send), and C(dbus-run-session) binaries. Depending on distribution you are using, you may need to install additional packages to have these available. + - This module uses the C(gi.repository) Python library when available for + accurate comparison of values in C(dconf) to values specified in Ansible + code. C(gi.repository) is likely to be present on most systems which have + C(dconf) but may not be present everywhere. When it is missing, a simple + string comparison between values is used, and there may be false positives, + that is, Ansible may think that a value is being changed when it is not. + This fallback will be removed in a future version of this module, at which + point the module will stop working on hosts without C(gi.repository). - Detection of existing, running D-Bus session, required to change settings via C(dconf), is not 100% reliable due to implementation details of D-Bus daemon itself. This might lead to running applications not picking-up @@ -128,6 +140,12 @@ EXAMPLES = r""" import os import traceback +try: + from gi.repository.GLib import Variant, GError +except ImportError: + Variant = None + GError = AttributeError + PSUTIL_IMP_ERR = None try: import psutil @@ -258,6 +276,25 @@ class DconfPreference(object): # Check if dconf binary exists self.dconf_bin = self.module.get_bin_path('dconf', required=True) + @staticmethod + def variants_are_equal(canonical_value, user_value): + """Compare two string GVariant representations for equality. + + Assumes `canonical_value` is "canonical" in the sense that the type of + the variant is specified explicitly if it cannot be inferred; this is + true for textual representations of variants generated by the `dconf` + command. The type of `canonical_value` is used to parse `user_value`, + so the latter does not need to be explicitly typed. + + Returns True if the two values are equal. + """ + try: + variant1 = Variant.parse(None, canonical_value) + variant2 = Variant.parse(variant1.get_type(), user_value) + return variant1 == variant2 + except GError: + return canonical_value == user_value + def read(self, key): """ Retrieves current value associated with the dconf key. @@ -298,7 +335,7 @@ class DconfPreference(object): """ # If no change is needed (or won't be done due to check_mode), notify # caller straight away. - if value == self.read(key): + if self.variants_are_equal(self.read(key), value): return False elif self.check_mode: return True @@ -369,6 +406,12 @@ def main(): supports_check_mode=True ) + if Variant is None: + module.warn( + 'WARNING: The gi.repository Python library is not available; ' + 'using string comparison to check value equality. This fallback ' + 'will be deprecated in a future version of community.general.') + if not HAS_PSUTIL: module.fail_json(msg=missing_required_lib("psutil"), exception=PSUTIL_IMP_ERR) diff --git a/tests/unit/plugins/modules/test_dconf.py b/tests/unit/plugins/modules/test_dconf.py new file mode 100644 index 0000000000..e0ea8195a1 --- /dev/null +++ b/tests/unit/plugins/modules/test_dconf.py @@ -0,0 +1,44 @@ +# Copyright (c) 2023 Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or +# https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import pytest + +from ansible_collections.community.general.plugins.modules import dconf + +try: + from gi.repository.GLib import Variant +except ImportError: + Variant = None + +DconfPreference = dconf.DconfPreference + + +@pytest.mark.parametrize( + "v1,v2,expected,fallback_expected", + (("'foo'", "'foo'", True, True), + ('"foo"', "'foo'", True, False), + ("'foo'", '"foo"', True, False), + ("'foo'", '"bar"', False, False), + ("[1, 2, 3]", "[1, 2, 3]", True, True), + ("[1, 2, 3]", "[3, 2, 1]", False, False), + ('1234', '1234', True, True), + ('1234', '1235', False, False), + ('1.0', '1.0', True, True), + ('1.000', '1.0', True, False), + ('2.0', '4.0', False, False), + # GVariants with different types aren't equal! + ('1', '1.0', False, False), + # Explicit types + ('@as []', '[]', True, False), + )) +def test_gvariant_equality(mocker, v1, v2, expected, fallback_expected): + assert DconfPreference.variants_are_equal(v1, v2) is \ + (expected if Variant else fallback_expected) + mocker.patch.object(dconf, 'Variant', None) + mocker.patch.object(dconf, "GError", AttributeError) + assert DconfPreference.variants_are_equal(v1, v2) is fallback_expected