IPA: Remove duplicated code and fix a bug that occurs if empty lists were passed and IPA didn't know the value before (#19210)

* ipa: Add method get_diff and modify_if_diff to class IPAClient

* ipa_*: Use method get_diff and modify_if_diff from class IPAClient

lib/ansible/module_utils/ipa.py

@@ -32,11 +32,12 @@ try:
 except ImportError:
     import simplejson as json
 
+from ansible.module_utils._text import to_bytes, to_text
 from ansible.module_utils.pycompat24 import get_exception
-from ansible.module_utils.urls import fetch_url
-from ansible.module_utils.six.moves.urllib.parse import quote
 from ansible.module_utils.six import PY3
-from ansible.module_utils._text import to_bytes, to_text
+from ansible.module_utils.six.moves.urllib.parse import quote
+from ansible.module_utils.urls import fetch_url
+
 
 class IPAClient(object):
     def __init__(self, module, host, port, protocol):
@@ -117,3 +118,43 @@ class IPAClient(object):
                         return {}
             return result
         return None
+
+    def get_diff(self, ipa_data, module_data):
+        result = []
+        for key in module_data.keys():
+            mod_value = module_data.get(key, None)
+            if isinstance(mod_value, list):
+                default = []
+            else:
+                default = None
+            ipa_value = ipa_data.get(key, default)
+            if isinstance(ipa_value, list) and not isinstance(mod_value, list):
+                mod_value = [mod_value]
+            if isinstance(ipa_value, list) and isinstance(mod_value, list):
+                mod_value = sorted(mod_value)
+                ipa_value = sorted(ipa_value)
+            if mod_value != ipa_value:
+                result.append(key)
+        return result
+
+    def modify_if_diff(self, name, ipa_list, module_list, add_method, remove_method, item=None):
+        changed = False
+        diff = list(set(ipa_list) - set(module_list))
+        if len(diff) > 0:
+            changed = True
+            if not self.module.check_mode:
+                if item:
+                    remove_method(name=name, item={item: diff})
+                else:
+                    remove_method(name=name, item=diff)
+
+        diff = list(set(module_list) - set(ipa_list))
+        if len(diff) > 0:
+            changed = True
+            if not self.module.check_mode:
+                if item:
+                    add_method(name=name, item={item: diff})
+                else:
+                    add_method(name=name, item=diff)
+
+        return changed

lib/ansible/modules/identity/ipa/ipa_group.py

@@ -139,10 +139,12 @@ group:
   type: dict
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class GroupIPAClient(IPAClient):
 
+class GroupIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(GroupIPAClient, self).__init__(module, host, port, protocol)
 
@@ -190,7 +192,7 @@ def get_group_dict(description=None, external=None, gid=None, nonposix=None):
     return group
 
 
-def get_group_diff(ipa_group, module_group):
+def get_group_diff(client, ipa_group, module_group):
     data = []
     # With group_add attribute nonposix is passed, whereas with group_mod only posix can be passed.
     if 'nonposix' in module_group:
@@ -199,34 +201,7 @@ def get_group_diff(ipa_group, module_group):
             module_group['posix'] = True
         del module_group['nonposix']
 
-    for key in module_group.keys():
-        module_value = module_group.get(key, None)
-        ipa_value = ipa_group.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(module_value, list):
-            module_value = [module_value]
-        if isinstance(ipa_value, list) and isinstance(module_value, list):
-            ipa_value = sorted(ipa_value)
-            module_value = sorted(module_value)
-        if ipa_value != module_value:
-            data.append(key)
-    return data
-
-
-def modify_if_diff(module, name, ipa_list, module_list, add_method, remove_method):
-    changed = False
-    diff = list(set(ipa_list) - set(module_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            remove_method(name=name, item=diff)
-
-    diff = list(set(module_list) - set(ipa_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            add_method(name=name, item=diff)
-
-    return changed
+    return client.get_diff(ipa_data=ipa_group, module_data=module_group)
 
 
 def ensure(module, client):
@@ -246,7 +221,7 @@ def ensure(module, client):
             if not module.check_mode:
                 ipa_group = client.group_add(name, item=module_group)
         else:
-            diff = get_group_diff(ipa_group, module_group)
+            diff = get_group_diff(client, ipa_group, module_group)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -256,14 +231,14 @@ def ensure(module, client):
                     client.group_mod(name=name, item=data)
 
         if group is not None:
-            changed = modify_if_diff(module, name, ipa_group.get('member_group', []), group,
-                                     client.group_add_member_group,
-                                     client.group_remove_member_group) or changed
+            changed = client.modify_if_diff(name, ipa_group.get('member_group', []), group,
+                                            client.group_add_member_group,
+                                            client.group_remove_member_group) or changed
 
         if user is not None:
-            changed = modify_if_diff(module, name, ipa_group.get('member_user', []), user,
-                                     client.group_add_member_user,
-                                     client.group_remove_member_user) or changed
+            changed = client.modify_if_diff(name, ipa_group.get('member_user', []), user,
+                                            client.group_add_member_user,
+                                            client.group_remove_member_user) or changed
 
     else:
         if ipa_group:
@@ -309,8 +284,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_hbacrule.py

@@ -170,10 +170,12 @@ hbacrule:
   type: dict
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class HBACRuleIPAClient(IPAClient):
 
+class HBACRuleIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(HBACRuleIPAClient, self).__init__(module, host, port, protocol)
 
@@ -233,36 +235,8 @@ def get_hbacrule_dict(description=None, hostcategory=None, ipaenabledflag=None,
     return data
 
 
-def get_hbcarule_diff(ipa_hbcarule, module_hbcarule):
-    data = []
-    for key in module_hbcarule.keys():
-        module_value = module_hbcarule.get(key, None)
-        ipa_value = ipa_hbcarule.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(module_value, list):
-            module_value = [module_value]
-        if isinstance(ipa_value, list) and isinstance(module_value, list):
-            ipa_value = sorted(ipa_value)
-            module_value = sorted(module_value)
-        if ipa_value != module_value:
-            data.append(key)
-    return data
-
-
-def modify_if_diff(module, name, ipa_list, module_list, add_method, remove_method, item):
-    changed = False
-    diff = list(set(ipa_list) - set(module_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            remove_method(name=name, item={item: diff})
-
-    diff = list(set(module_list) - set(ipa_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            add_method(name=name, item={item: diff})
-
-    return changed
+def get_hbcarule_diff(client, ipa_hbcarule, module_hbcarule):
+    return client.get_diff(ipa_data=ipa_hbcarule, module_data=module_hbcarule)
 
 
 def ensure(module, client):
@@ -302,7 +276,7 @@ def ensure(module, client):
             if not module.check_mode:
                 ipa_hbacrule = client.hbacrule_add(name=name, item=module_hbacrule)
         else:
-            diff = get_hbcarule_diff(ipa_hbacrule, module_hbacrule)
+            diff = get_hbcarule_diff(client, ipa_hbacrule, module_hbacrule)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -312,45 +286,45 @@ def ensure(module, client):
                     client.hbacrule_mod(name=name, item=data)
 
         if host is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('memberhost_host', []), host,
-                                     client.hbacrule_add_host,
-                                     client.hbacrule_remove_host, 'host') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('memberhost_host', []), host,
+                                            client.hbacrule_add_host,
+                                            client.hbacrule_remove_host, 'host') or changed
 
         if hostgroup is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('memberhost_hostgroup', []), hostgroup,
-                                     client.hbacrule_add_host,
-                                     client.hbacrule_remove_host, 'hostgroup') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('memberhost_hostgroup', []), hostgroup,
+                                            client.hbacrule_add_host,
+                                            client.hbacrule_remove_host, 'hostgroup') or changed
 
         if service is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('memberservice_hbacsvc', []), service,
-                                     client.hbacrule_add_service,
-                                     client.hbacrule_remove_service, 'hbacsvc') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('memberservice_hbacsvc', []), service,
+                                            client.hbacrule_add_service,
+                                            client.hbacrule_remove_service, 'hbacsvc') or changed
 
         if servicegroup is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('memberservice_hbacsvcgroup', []),
-                                     servicegroup,
-                                     client.hbacrule_add_service,
-                                     client.hbacrule_remove_service, 'hbacsvcgroup') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('memberservice_hbacsvcgroup', []),
+                                            servicegroup,
+                                            client.hbacrule_add_service,
+                                            client.hbacrule_remove_service, 'hbacsvcgroup') or changed
 
         if sourcehost is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('sourcehost_host', []), sourcehost,
-                                     client.hbacrule_add_sourcehost,
-                                     client.hbacrule_remove_sourcehost, 'host') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('sourcehost_host', []), sourcehost,
+                                            client.hbacrule_add_sourcehost,
+                                            client.hbacrule_remove_sourcehost, 'host') or changed
 
         if sourcehostgroup is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('sourcehost_group', []), sourcehostgroup,
-                                     client.hbacrule_add_sourcehost,
-                                     client.hbacrule_remove_sourcehost, 'hostgroup') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('sourcehost_group', []), sourcehostgroup,
+                                            client.hbacrule_add_sourcehost,
+                                            client.hbacrule_remove_sourcehost, 'hostgroup') or changed
 
         if user is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('memberuser_user', []), user,
-                                     client.hbacrule_add_user,
-                                     client.hbacrule_remove_user, 'user') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('memberuser_user', []), user,
+                                            client.hbacrule_add_user,
+                                            client.hbacrule_remove_user, 'user') or changed
 
         if usergroup is not None:
-            changed = modify_if_diff(module, name, ipa_hbacrule.get('memberuser_group', []), usergroup,
-                                     client.hbacrule_add_user,
-                                     client.hbacrule_remove_user, 'group') or changed
+            changed = client.modify_if_diff(name, ipa_hbacrule.get('memberuser_group', []), usergroup,
+                                            client.hbacrule_add_user,
+                                            client.hbacrule_remove_user, 'group') or changed
     else:
         if ipa_hbacrule:
             changed = True
@@ -404,8 +378,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_host.py

@@ -164,10 +164,12 @@ host_diff:
   type: list
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class HostIPAClient(IPAClient):
 
+class HostIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(HostIPAClient, self).__init__(module, host, port, protocol)
 
@@ -209,23 +211,13 @@ def get_host_dict(description=None, force=None, ip_address=None, ns_host_locatio
     return data
 
 
-def get_host_diff(ipa_host, module_host):
+def get_host_diff(client, ipa_host, module_host):
     non_updateable_keys = ['force', 'ip_address']
-    data = []
     for key in non_updateable_keys:
         if key in module_host:
             del module_host[key]
-    for key in module_host.keys():
-        ipa_value = ipa_host.get(key, None)
-        module_value = module_host.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(module_value, list):
-            module_value = [module_value]
-        if isinstance(ipa_value, list) and isinstance(module_value, list):
-            ipa_value = sorted(ipa_value)
-            module_value = sorted(module_value)
-        if ipa_value != module_value:
-            data.append(key)
-    return data
+
+    return client.get_diff(ipa_data=ipa_host, module_data=module_host)
 
 
 def ensure(module, client):
@@ -247,7 +239,7 @@ def ensure(module, client):
             if not module.check_mode:
                 client.host_add(name=name, host=module_host)
         else:
-            diff = get_host_diff(ipa_host, module_host)
+            diff = get_host_diff(client, ipa_host, module_host)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -304,8 +296,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_hostgroup.py

@@ -117,10 +117,12 @@ hostgroup:
   type: dict
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class HostGroupIPAClient(IPAClient):
 
+class HostGroupIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(HostGroupIPAClient, self).__init__(module, host, port, protocol)
 
@@ -162,35 +164,8 @@ def get_hostgroup_dict(description=None):
     return data
 
 
-def get_hostgroup_diff(ipa_hostgroup, module_hostgroup):
-    data = []
-    for key in module_hostgroup.keys():
-        ipa_value = ipa_hostgroup.get(key, None)
-        module_value = module_hostgroup.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(module_value, list):
-            module_value = [module_value]
-        if isinstance(ipa_value, list) and isinstance(module_value, list):
-            ipa_value = sorted(ipa_value)
-            module_value = sorted(module_value)
-        if ipa_value != module_value:
-            data.append(key)
-    return data
-
-
-def modify_if_diff(module, name, ipa_list, module_list, add_method, remove_method):
-    changed = False
-    diff = list(set(ipa_list) - set(module_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            remove_method(name=name, item=diff)
-
-    diff = list(set(module_list) - set(ipa_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            add_method(name=name, item=diff)
-    return changed
+def get_hostgroup_diff(client, ipa_hostgroup, module_hostgroup):
+    return client.get_diff(ipa_data=ipa_hostgroup, module_data=module_hostgroup)
 
 
 def ensure(module, client):
@@ -209,7 +184,7 @@ def ensure(module, client):
             if not module.check_mode:
                 ipa_hostgroup = client.hostgroup_add(name=name, item=module_hostgroup)
         else:
-            diff = get_hostgroup_diff(ipa_hostgroup, module_hostgroup)
+            diff = get_hostgroup_diff(client, ipa_hostgroup, module_hostgroup)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -219,14 +194,14 @@ def ensure(module, client):
                     client.hostgroup_mod(name=name, item=data)
 
         if host is not None:
-            changed = modify_if_diff(module, name, ipa_hostgroup.get('member_host', []),
-                                     [item.lower() for item in host],
-                                     client.hostgroup_add_host, client.hostgroup_remove_host) or changed
+            changed = client.modify_if_diff(name, ipa_hostgroup.get('member_host', []), [item.lower() for item in host],
+                                            client.hostgroup_add_host, client.hostgroup_remove_host) or changed
 
         if hostgroup is not None:
-            changed = modify_if_diff(module, name, ipa_hostgroup.get('member_hostgroup', []),
-                                     [item.lower() for item in hostgroup],
-                                     client.hostgroup_add_hostgroup, client.hostgroup_remove_hostgroup) or changed
+            changed = client.modify_if_diff(name, ipa_hostgroup.get('member_hostgroup', []),
+                                            [item.lower() for item in hostgroup],
+                                            client.hostgroup_add_hostgroup,
+                                            client.hostgroup_remove_hostgroup) or changed
 
     else:
         if ipa_hostgroup:
@@ -271,8 +246,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_role.py

@@ -147,10 +147,12 @@ role:
   type: dict
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class RoleIPAClient(IPAClient):
 
+class RoleIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(RoleIPAClient, self).__init__(module, host, port, protocol)
 
@@ -210,35 +212,8 @@ def get_role_dict(description=None):
     return data
 
 
-def get_role_diff(ipa_role, module_role):
-    data = []
-    for key in module_role.keys():
-        module_value = module_role.get(key, None)
-        ipa_value = ipa_role.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(module_value, list):
-            module_value = [module_value]
-        if isinstance(ipa_value, list) and isinstance(module_value, list):
-            ipa_value = sorted(ipa_value)
-            module_value = sorted(module_value)
-        if ipa_value != module_value:
-            data.append(key)
-    return data
-
-
-def modify_if_diff(module, name, ipa_list, module_list, add_method, remove_method):
-    changed = False
-    diff = list(set(ipa_list) - set(module_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            remove_method(name=name, item=diff)
-
-    diff = list(set(module_list) - set(ipa_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            add_method(name=name, item=diff)
-    return changed
+def get_role_diff(client, ipa_role, module_role):
+    return client.get_diff(ipa_data=ipa_role, module_data=module_role)
 
 
 def ensure(module, client):
@@ -260,7 +235,7 @@ def ensure(module, client):
             if not module.check_mode:
                 ipa_role = client.role_add(name=name, item=module_role)
         else:
-            diff = get_role_diff(ipa_role=ipa_role, module_role=module_role)
+            diff = get_role_diff(client, ipa_role, module_role)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -270,28 +245,28 @@ def ensure(module, client):
                     client.role_mod(name=name, item=data)
 
         if group is not None:
-            changed = modify_if_diff(module, name, ipa_role.get('member_group', []), group,
-                                     client.role_add_group,
-                                     client.role_remove_group) or changed
+            changed = client.modify_if_diff(name, ipa_role.get('member_group', []), group,
+                                            client.role_add_group,
+                                            client.role_remove_group) or changed
 
         if host is not None:
-            changed = modify_if_diff(module, name, ipa_role.get('member_host', []), host,
-                                     client.role_add_host,
-                                     client.role_remove_host) or changed
+            changed = client.modify_if_diff(name, ipa_role.get('member_host', []), host,
+                                            client.role_add_host,
+                                            client.role_remove_host) or changed
 
         if hostgroup is not None:
-            changed = modify_if_diff(module, name, ipa_role.get('member_hostgroup', []), hostgroup,
-                                     client.role_add_hostgroup,
-                                     client.role_remove_hostgroup) or changed
+            changed = client.modify_if_diff(name, ipa_role.get('member_hostgroup', []), hostgroup,
+                                            client.role_add_hostgroup,
+                                            client.role_remove_hostgroup) or changed
 
         if service is not None:
-            changed = modify_if_diff(module, name, ipa_role.get('member_service', []), service,
-                                     client.role_add_service,
-                                     client.role_remove_service) or changed
+            changed = client.modify_if_diff(name, ipa_role.get('member_service', []), service,
+                                            client.role_add_service,
+                                            client.role_remove_service) or changed
         if user is not None:
-            changed = modify_if_diff(module, name, ipa_role.get('member_user', []), user,
-                                     client.role_add_user,
-                                     client.role_remove_user) or changed
+            changed = client.modify_if_diff(name, ipa_role.get('member_user', []), user,
+                                            client.role_add_user,
+                                            client.role_remove_user) or changed
     else:
         if ipa_role:
             changed = True
@@ -337,8 +312,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_sudocmd.py

@@ -96,10 +96,12 @@ sudocmd:
   type: dict
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class SudoCmdIPAClient(IPAClient):
 
+class SudoCmdIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(SudoCmdIPAClient, self).__init__(module, host, port, protocol)
 
@@ -123,19 +125,8 @@ def get_sudocmd_dict(description=None):
     return data
 
 
-def get_sudocmd_diff(ipa_sudocmd, module_sudocmd):
-    data = []
-    for key in module_sudocmd.keys():
-        module_value = module_sudocmd.get(key, None)
-        ipa_value = ipa_sudocmd.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(module_value, list):
-            module_value = [module_value]
-        if isinstance(ipa_value, list) and isinstance(module_value, list):
-            ipa_value = sorted(ipa_value)
-            module_value = sorted(module_value)
-        if ipa_value != module_value:
-            data.append(key)
-    return data
+def get_sudocmd_diff(client, ipa_sudocmd, module_sudocmd):
+    return client.get_diff(ipa_data=ipa_sudocmd, module_data=module_sudocmd)
 
 
 def ensure(module, client):
@@ -152,7 +143,7 @@ def ensure(module, client):
             if not module.check_mode:
                 client.sudocmd_add(name=name, item=module_sudocmd)
         else:
-            diff = get_sudocmd_diff(ipa_sudocmd, module_sudocmd)
+            diff = get_sudocmd_diff(client, ipa_sudocmd, module_sudocmd)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -200,8 +191,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_sudocmdgroup.py

@@ -103,10 +103,12 @@ sudocmdgroup:
   type: dict
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class SudoCmdGroupIPAClient(IPAClient):
 
+class SudoCmdGroupIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(SudoCmdGroupIPAClient, self).__init__(module, host, port, protocol)
 
@@ -142,35 +144,8 @@ def get_sudocmdgroup_dict(description=None):
     return data
 
 
-def modify_if_diff(module, name, ipa_list, module_list, add_method, remove_method):
-    changed = False
-    diff = list(set(ipa_list) - set(module_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            remove_method(name=name, item=diff)
-
-    diff = list(set(module_list) - set(ipa_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            add_method(name=name, item=diff)
-    return changed
-
-
-def get_sudocmdgroup_diff(ipa_sudocmdgroup, module_sudocmdgroup):
-    data = []
-    for key in module_sudocmdgroup.keys():
-        module_value = module_sudocmdgroup.get(key, None)
-        ipa_value = ipa_sudocmdgroup.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(module_value, list):
-            module_value = [module_value]
-        if isinstance(ipa_value, list) and isinstance(module_value, list):
-            ipa_value = sorted(ipa_value)
-            module_value = sorted(module_value)
-        if ipa_value != module_value:
-            data.append(key)
-    return data
+def get_sudocmdgroup_diff(client, ipa_sudocmdgroup, module_sudocmdgroup):
+    return client.get_diff(ipa_data=ipa_sudocmdgroup, module_data=module_sudocmdgroup)
 
 
 def ensure(module, client):
@@ -188,7 +163,7 @@ def ensure(module, client):
             if not module.check_mode:
                 ipa_sudocmdgroup = client.sudocmdgroup_add(name=name, item=module_sudocmdgroup)
         else:
-            diff = get_sudocmdgroup_diff(ipa_sudocmdgroup, module_sudocmdgroup)
+            diff = get_sudocmdgroup_diff(client, ipa_sudocmdgroup, module_sudocmdgroup)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -198,9 +173,9 @@ def ensure(module, client):
                     client.sudocmdgroup_mod(name=name, item=data)
 
         if sudocmd is not None:
-            changed = modify_if_diff(module, name, ipa_sudocmdgroup.get('member_sudocmd', []), sudocmd,
-                                     client.sudocmdgroup_add_member_sudocmd,
-                                     client.sudocmdgroup_remove_member_sudocmd)
+            changed = client.modify_if_diff(name, ipa_sudocmdgroup.get('member_sudocmd', []), sudocmd,
+                                            client.sudocmdgroup_add_member_sudocmd,
+                                            client.sudocmdgroup_remove_member_sudocmd)
     else:
         if ipa_sudocmdgroup:
             changed = True
@@ -242,8 +217,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_sudorule.py

@@ -155,10 +155,12 @@ sudorule:
   type: dict
 '''
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class SudoRuleIPAClient(IPAClient):
 
+class SudoRuleIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(SudoRuleIPAClient, self).__init__(module, host, port, protocol)
 
@@ -259,25 +261,6 @@ def get_sudorule_diff(ipa_sudorule, module_sudorule):
     return data
 
 
-def modify_if_diff(module, name, ipa_list, module_list, add_method, remove_method):
-    changed = False
-    diff = list(set(ipa_list) - set(module_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            for item in diff:
-                remove_method(name=name, item=item)
-
-    diff = list(set(module_list) - set(ipa_list))
-    if len(diff) > 0:
-        changed = True
-        if not module.check_mode:
-            for item in diff:
-                add_method(name=name, item=item)
-
-    return changed
-
-
 def category_changed(module, client, category_name, ipa_sudorule):
     if ipa_sudorule.get(category_name, None) == ['all']:
         if not module.check_mode:
@@ -320,7 +303,7 @@ def ensure(module, client):
             if not module.check_mode:
                 ipa_sudorule = client.sudorule_add(name=name, item=module_sudorule)
         else:
-            diff = get_sudorule_diff(ipa_sudorule, module_sudorule)
+            diff = get_sudorule_diff(client, ipa_sudorule, module_sudorule)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -340,29 +323,29 @@ def ensure(module, client):
 
         if host is not None:
             changed = category_changed(module, client, 'hostcategory', ipa_sudorule) or changed
-            changed = modify_if_diff(module, name, ipa_sudorule.get('memberhost_host', []), host,
-                                     client.sudorule_add_host_host,
-                                     client.sudorule_remove_host_host) or changed
+            changed = client.modify_if_diff(name, ipa_sudorule.get('memberhost_host', []), host,
+                                            client.sudorule_add_host_host,
+                                            client.sudorule_remove_host_host) or changed
 
         if hostgroup is not None:
             changed = category_changed(module, client, 'hostcategory', ipa_sudorule) or changed
-            changed = modify_if_diff(module, name, ipa_sudorule.get('memberhost_hostgroup', []), hostgroup,
-                                     client.sudorule_add_host_hostgroup,
-                                     client.sudorule_remove_host_hostgroup) or changed
+            changed = client.modify_if_diff(name, ipa_sudorule.get('memberhost_hostgroup', []), hostgroup,
+                                            client.sudorule_add_host_hostgroup,
+                                            client.sudorule_remove_host_hostgroup) or changed
         if sudoopt is not None:
-            changed = modify_if_diff(module, name, ipa_sudorule.get('ipasudoopt', []), sudoopt,
-                                     client.sudorule_add_option_ipasudoopt,
-                                     client.sudorule_remove_option_ipasudoopt) or changed
+            changed = client.modify_if_diff(name, ipa_sudorule.get('ipasudoopt', []), sudoopt,
+                                            client.sudorule_add_option_ipasudoopt,
+                                            client.sudorule_remove_option_ipasudoopt) or changed
         if user is not None:
             changed = category_changed(module, client, 'usercategory', ipa_sudorule) or changed
-            changed = modify_if_diff(module, name, ipa_sudorule.get('memberuser_user', []), user,
-                                     client.sudorule_add_user_user,
-                                     client.sudorule_remove_user_user) or changed
+            changed = client.modify_if_diff(name, ipa_sudorule.get('memberuser_user', []), user,
+                                            client.sudorule_add_user_user,
+                                            client.sudorule_remove_user_user) or changed
         if usergroup is not None:
             changed = category_changed(module, client, 'usercategory', ipa_sudorule) or changed
-            changed = modify_if_diff(module, name, ipa_sudorule.get('memberuser_group', []), usergroup,
-                                     client.sudorule_add_user_group,
-                                     client.sudorule_remove_user_group) or changed
+            changed = client.modify_if_diff(name, ipa_sudorule.get('memberuser_group', []), usergroup,
+                                            client.sudorule_add_user_group,
+                                            client.sudorule_remove_user_group) or changed
     else:
         if ipa_sudorule:
             changed = True
@@ -417,8 +400,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()

lib/ansible/modules/identity/ipa/ipa_user.py

@@ -143,10 +143,12 @@ user:
 import base64
 import hashlib
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.pycompat24 import get_exception
 from ansible.module_utils.ipa import IPAClient
 
-class UserIPAClient(IPAClient):
 
+class UserIPAClient(IPAClient):
     def __init__(self, module, host, port, protocol):
         super(UserIPAClient, self).__init__(module, host, port, protocol)
 
@@ -195,7 +197,7 @@ def get_user_dict(displayname=None, givenname=None, loginshell=None, mail=None,
     return user
 
 
-def get_user_diff(ipa_user, module_user):
+def get_user_diff(client, ipa_user, module_user):
     """
         Return the keys of each dict whereas values are different. Unfortunately the IPA
         API returns everything as a list even if only a single value is possible.
@@ -207,8 +209,6 @@ def get_user_diff(ipa_user, module_user):
     :param module_user:
     :return:
     """
-    #    return [item for item in module_user.keys() if module_user.get(item, None) != ipa_user.get(item, None)]
-    result = []
     # sshpubkeyfp is the list of ssh key fingerprints. IPA doesn't return the keys itself but instead the fingerprints.
     # These are used for comparison.
     sshpubkey = None
@@ -217,16 +217,9 @@ def get_user_diff(ipa_user, module_user):
         # Remove the ipasshpubkey element as it is not returned from IPA but save it's value to be used later on
         sshpubkey = module_user['ipasshpubkey']
         del module_user['ipasshpubkey']
-    for key in module_user.keys():
-        mod_value = module_user.get(key, None)
-        ipa_value = ipa_user.get(key, None)
-        if isinstance(ipa_value, list) and not isinstance(mod_value, list):
-            mod_value = [mod_value]
-        if isinstance(ipa_value, list) and isinstance(mod_value, list):
-            mod_value = sorted(mod_value)
-            ipa_value = sorted(ipa_value)
-        if mod_value != ipa_value:
-            result.append(key)
+
+    result = client.get_diff(ipa_data=ipa_user, module_data=module_user)
+
     # If there are public keys, remove the fingerprints and add them back to the dict
     if sshpubkey is not None:
         del module_user['sshpubkeyfp']
@@ -278,7 +271,7 @@ def ensure(module, client):
             if not module.check_mode:
                 ipa_user = client.user_add(name=name, item=module_user)
         else:
-            diff = get_user_diff(ipa_user, module_user)
+            diff = get_user_diff(client, ipa_user, module_user)
             if len(diff) > 0:
                 changed = True
                 if not module.check_mode:
@@ -339,8 +332,5 @@ def main():
         module.fail_json(msg=str(e))
 
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
-
 if __name__ == '__main__':
     main()