hbacsvcgroup: Fix member management idempotence issues.

The hbacsvc members of hbacsvcgroup must be compared in a case
insensitive manner. This patch fixes comparation of member parameters
against existing members by converting parameters to lowercase, as it
is how the hbacsvc members are stored for hbacsvcgroups.

Also, there were some cases where a change with an empty set of members
was issued to IPA API, leading to a result of 'changed: yes' when
'changed: no' was expected. The fix involved a refactoring of the
hbacsvcgroup member management code.

plugins/modules/ipahbacsvcgroup.py

@@ -98,7 +98,8 @@ RETURN = """
 """
 
 from ansible.module_utils.ansible_freeipa_module import \
-    IPAAnsibleModule, compare_args_ipa, gen_add_del_lists
+    IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \
+    gen_intersection_list
 
 
 def find_hbacsvcgroup(module, name):
@@ -180,7 +181,7 @@ def main():
     # present
     description = ansible_module.params_get("description")
     nomembers = ansible_module.params_get("nomembers")
-    hbacsvc = ansible_module.params_get("hbacsvc")
+    hbacsvc = ansible_module.params_get_lowercase("hbacsvc")
     action = ansible_module.params_get("action")
     # state
     state = ansible_module.params_get("state")
@@ -220,6 +221,8 @@ def main():
             # Make sure hbacsvcgroup exists
             res_find = find_hbacsvcgroup(ansible_module, name)
 
+            hbacsvc_add, hbacsvc_del = [], []
+
             # Create command
             if state == "present":
                 # Generate args
@@ -243,32 +246,20 @@ def main():
                     if not compare_args_ipa(ansible_module, member_args,
                                             res_find):
                         # Generate addition and removal lists
+                        if hbacsvc is not None:
                             hbacsvc_add, hbacsvc_del = gen_add_del_lists(
                                 hbacsvc, res_find.get("member_hbacsvc"))
 
-                        # Add members
-                        if len(hbacsvc_add) > 0:
-                            commands.append([name, "hbacsvcgroup_add_member",
-                                             {
-                                                 "hbacsvc": hbacsvc_add
-                                             }])
-                        # Remove members
-                        if len(hbacsvc_del) > 0:
-                            commands.append([name,
-                                             "hbacsvcgroup_remove_member",
-                                             {
-                                                 "hbacsvc": hbacsvc_del
-                                             }])
                 elif action == "member":
                     if res_find is None:
                         ansible_module.fail_json(
                             msg="No hbacsvcgroup '%s'" % name)
 
                     # Ensure members are present
-                    commands.append([name, "hbacsvcgroup_add_member",
-                                     {
-                                         "hbacsvc": hbacsvc
-                                     }])
+                    if hbacsvc:
+                        hbacsvc_add = gen_add_list(
+                            hbacsvc, res_find.get("member_hbacsvc"))
+
             elif state == "absent":
                 if action == "hbacsvcgroup":
                     if res_find is not None:
@@ -280,15 +271,28 @@ def main():
                             msg="No hbacsvcgroup '%s'" % name)
 
                     # Ensure members are absent
-                    commands.append([name, "hbacsvcgroup_remove_member",
-                                     {
-                                         "hbacsvc": hbacsvc
-                                     }])
+                    if hbacsvc:
+                        hbacsvc_del = gen_intersection_list(
+                            hbacsvc, res_find.get("member_hbacsvc"))
+
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
-        # Execute commands
+            # Manage members
+            if len(hbacsvc_add) > 0:
+                commands.append([name, "hbacsvcgroup_add_member",
+                                 {
+                                     "hbacsvc": hbacsvc_add
+                                 }])
+            # Remove members
+            if len(hbacsvc_del) > 0:
+                commands.append([name,
+                                 "hbacsvcgroup_remove_member",
+                                 {
+                                     "hbacsvc": hbacsvc_del
+                                 }])
 
+        # Execute commands
         changed = ansible_module.execute_ipa_commands(commands, result_handler)
 
     # Done

tests/hbacsvcgroup/test_hbacsvcgroup_member_case_insensitive.yml

+---
+- name: Test hbacsvcgroup member varying capitalization
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: no
+  gather_facts: no
+
+  vars:
+    hbacsvc_list:
+      - sVc1
+      - SvC2
+
+  tasks:
+  - block:
+      - name: Ensure test hbacsvcgroup is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+
+      - name: Ensure test HBAC services are present
+        ipahbacsvc:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ item }}"
+        with_items: "{{ hbacsvc_list }}"
+
+      - name: Ensure test hbacsvcgroup is present with duplicate hbacsvc
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc:
+          - sVc1
+          - SvC1
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure test hbacsvc is absent from hbacsvcgroup, with duplicate hbacsvc
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc:
+          - sVc1
+          - SvC1
+          action: member
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if test hbacsvc absent, again, from hbacsvcgroup, with duplicate hbacsvc, would trigger changes
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc:
+          - svC1
+          - SVC1
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure test hbacsvcgroup is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup with members would trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup is present with members, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup with members would not trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup is present with members, lowercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | lower }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup is present with members, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure test hbacsvcgroup is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+
+      - name: Ensure test hbacsvcgroup is present
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+
+      - name: Check if hbacsvcgroup members would trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup members would not trigger changes, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members, lowercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | lower }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Check if hbacsvcgroup members absence would trigger changes, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members absent, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Check if hbacsvcgroup members absence would not trigger changes, uppercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | upper }}"
+          action: member
+          state: absent
+        check_mode: yes
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members absent, mixed case
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure hbacsvcgroup has members absent, lowercase
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          hbacsvc: "{{ hbacsvc_list | lower }}"
+          action: member
+          state: absent
+        register: result
+        failed_when: result.changed or result.failed
+
+    always:
+      - name: Ensure test hbac service group is absent
+        ipahbacsvcgroup:
+          ipaadmin_password: SomeADMINpassword
+          name: testgroup
+          state: absent
+
+      - name: Ensure test hbac services are absent
+        ipahbacsvc:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "{{ hbacsvc_list }}"
+          state: absent