diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 17d02d0e2490125064fd6d371c857f850c998e73..88d12116110f934cb4d72785be4d25ed7b635239 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -310,15 +310,49 @@ else: raise ValueError("Invalid date '%s'" % value) def compare_args_ipa(module, args, ipa, ignore=None): # noqa - """Compare IPA obj attrs with the command args. + """Compare IPA object attributes against command arguments. + + This function compares 'ipa' attributes with the 'args' the module + is intending to use as parameters to an IPA API command. A list of + attribute names that should be ignored during comparison may be + provided. + + The comparison will be performed on every attribute provided in + 'args'. If the attribute in 'args' or 'ipa' is not a scalar value + (including strings) the comparison will be performed as if the + attribute is a set of values, so duplicate values will count as a + single one. If both values are scalar values, then a direct + comparison is performed. + + If an attribute is not available in 'ipa', its value is considered + to be a list with an empty string (['']), possibly forcing the + conversion of the 'args' attribute to a list for comparison. This + allows, for example, the usage of empty strings which should compare + as equals to inexistent attributes (None), as is done in IPA API. + + This function is mostly useful to evaluate the need of a call to + IPA server when provided arguments are equivalent to the existing + values for a given IPA object. - This function compares IPA objects attributes with the args the - module is intending to use to call a command. ignore can be a list - of attributes, that should be ignored in the comparison. - This is useful to know if a call to IPA server will be needed or not. - In order to compare we have to perform slight changes in data formats. + Parameters + ---------- + module: AnsibleModule + The AnsibleModule used to log debug messages. + + args: dict + The set of attributes provided by the playbook task. + + ipa: dict + The set of attributes from the IPA object retrieved. + + ignore: list + An optional list of attribute names that should be ignored and + not evaluated. - Returns True if they are the same and False otherwise. + Return + ------ + True is returned if all attribute values in 'args' are + equivalent to the corresponding attribute value in 'ipa'. """ base_debug_msg = "Ansible arguments and IPA commands differed. " @@ -338,52 +372,45 @@ else: filtered_args = [key for key in args if key not in ignore] for key in filtered_args: - if key not in ipa: # pylint: disable=no-else-return - module.debug( - base_debug_msg + "Command key not present in IPA: %s" % key - ) - return False + arg = args[key] + ipa_arg = ipa.get(key, [""]) + # If ipa_arg is a list and arg is not, replace arg + # with list containing arg. Most args in a find result + # are lists, but not all. + if isinstance(ipa_arg, (list, tuple)): + if not isinstance(arg, list): + arg = [arg] + if len(ipa_arg) != len(arg): + module.debug( + base_debug_msg + + "List length doesn't match for key %s: %d %d" + % (key, len(arg), len(ipa_arg),) + ) + return False + # ensure list elements types are the same. + if not ( + isinstance(ipa_arg[0], type(arg[0])) + or isinstance(arg[0], type(ipa_arg[0])) + ): + arg = [to_text(_arg) for _arg in arg] + try: + arg_set = set(arg) + ipa_arg_set = set(ipa_arg) + except TypeError: + if arg != ipa_arg: + module.debug( + base_debug_msg + + "Different values: %s %s" % (arg, ipa_arg) + ) + return False else: - arg = args[key] - ipa_arg = ipa[key] - # If ipa_arg is a list and arg is not, replace arg - # with list containing arg. Most args in a find result - # are lists, but not all. - if isinstance(ipa_arg, tuple): - ipa_arg = list(ipa_arg) - if isinstance(ipa_arg, list): - if not isinstance(arg, list): - arg = [arg] - if len(ipa_arg) != len(arg): - module.debug( - base_debug_msg - + "List length doesn't match for key %s: %d %d" - % (key, len(arg), len(ipa_arg),) - ) - return False - if isinstance(ipa_arg[0], str) and isinstance(arg[0], int): - arg = [to_text(_arg) for _arg in arg] - if isinstance(ipa_arg[0], unicode) \ - and isinstance(arg[0], int): - arg = [to_text(_arg) for _arg in arg] - try: - arg_set = set(arg) - ipa_arg_set = set(ipa_arg) - except TypeError: - if arg != ipa_arg: - module.debug( - base_debug_msg - + "Different values: %s %s" % (arg, ipa_arg) - ) - return False - else: - if arg_set != ipa_arg_set: - module.debug( - base_debug_msg - + "Different set content: %s %s" - % (arg_set, ipa_arg_set,) - ) - return False + if arg_set != ipa_arg_set: + module.debug( + base_debug_msg + + "Different set content: %s %s" + % (arg_set, ipa_arg_set,) + ) + return False return True def _afm_convert(value): diff --git a/plugins/modules/ipaautomountmap.py b/plugins/modules/ipaautomountmap.py index 79b341b99dcb43c4e41a695bee7f0928858ceb13..9cca0a9852fa98be8aa4553752f5ea995df8a47e 100644 --- a/plugins/modules/ipaautomountmap.py +++ b/plugins/modules/ipaautomountmap.py @@ -126,7 +126,8 @@ class AutomountMap(IPAAnsibleModule): _args = {} if mapname: _args["automountmapname"] = mapname - if desc: + # An empty string is valid and will clear the attribute. + if desc is not None: _args["description"] = desc return _args diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index fb0fbc47727176048867515ac6ae6d03a9c30f6e..739b27b560da88fe0f74216f0160d6473c21107e 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -1103,20 +1103,6 @@ def main(): if "noprivate" in args: del args["noprivate"] - # Ignore sshpubkey if it is empty (for resetting) - # and not set in for the user - if "ipasshpubkey" not in res_find and \ - "ipasshpubkey" in args and \ - args["ipasshpubkey"] == ['']: - del args["ipasshpubkey"] - - # Ignore userauthtype if it is empty (for resetting) - # and not set in for the user - if "ipauserauthtype" not in res_find and \ - "ipauserauthtype" in args and \ - args["ipauserauthtype"] == ['']: - del args["ipauserauthtype"] - # For all settings is args, check if there are # different settings in the find result. # If yes: modify diff --git a/tests/automount/test_automountmap.yml b/tests/automount/test_automountmap.yml index da542e89cb6a0cc16dde859c3811b4cfdbf85633..377b3a8270405d231345fdd451626f768bfc3338 100644 --- a/tests/automount/test_automountmap.yml +++ b/tests/automount/test_automountmap.yml @@ -71,6 +71,24 @@ register: result failed_when: result.failed or result.changed + - name: ensure map TestMap has an empty description + ipaautomountmap: + ipaadmin_password: SomeADMINpassword + name: TestMap + location: TestLocation + desc: "" + register: result + failed_when: result.failed or not result.changed + + - name: ensure map TestMap has an empty description, again + ipaautomountmap: + ipaadmin_password: SomeADMINpassword + name: TestMap + location: TestLocation + desc: "" + register: result + failed_when: result.failed or result.changed + - name: ensure map TestMap is removed ipaautomountmap: ipaadmin_password: SomeADMINpassword diff --git a/tests/user/test_user_empty_lists.yml b/tests/user/test_user_empty_lists.yml new file mode 100644 index 0000000000000000000000000000000000000000..45bb829a1939b67a95fa65969f3e974254feadbd --- /dev/null +++ b/tests/user/test_user_empty_lists.yml @@ -0,0 +1,71 @@ +--- +- name: Test users + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: no + gather_facts: no + + tasks: + # SETUP + - name: Remove test users + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + state: absent + + - name: Ensure user testuser is present + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + first: test + last: user + userauthtype: password,radius,otp + sshpubkey: + # yamllint disable-line rule:line-length + - ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCqmVDpEX5gnSjKuv97AyzOhaUMMKz8ahOA3GY77tVC4o68KNgMCmDSEG1/kOIaElngNLaCha3p/2iAcU9Bi1tLKUlm2bbO5NHNwHfRxY/3cJtq+/7D1vxJzqThYwI4F9vr1WxyY2+mMTv3pXbfAJoR8Mu06XaEY5PDetlDKjHLuNWF+/O7ZU8PsULTa1dJZFrtXeFpmUoLoGxQBvlrlcPI1zDciCSU24t27Zan5Py2l5QchyI7yhCyMM77KDtj5+AFVpmkb9+zq50rYJAyFVeyUvwjzErvQrKJzYpA0NyBp7vskWbt36M16/M/LxEK7HA6mkcakO3ESWx5MT1LAjvdlnxbWG3787MxweHXuB8CZU+9bZPFBaJ+VQtOfJ7I8eH0S16moPC4ak8FlcFvOH8ERDPWLFDqfy09yaZ7bVIF0//5ZI7Nf3YDe3S7GrBX5ieYuECyP6UNkTx9BRsAQeVvXEc6otzB7iCSnYBMGUGzCqeigoAWaVQUONsSR3Uatks= pinky@ipaserver.el81.local # noqa 204 + + # TESTS - action: user + - name: Ensure user testuser present with empty sshpubkey + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + sshpubkey: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure user testuser present with empty sshpubkey, again + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + sshpubkey: "" + register: result + failed_when: result.changed or result.failed + + - name: Ensure user testuser present with empty userauthtype + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + userauthtype: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure user testuser present with empty userauthtype, again + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + userauthtype: "" + register: result + failed_when: result.changed or result.failed + + # CLEANUP + - name: Remove test users + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + state: absent