From 7b7d9c995755f7584f7cfd665439c0f9f856e99f Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Fri, 29 Dec 2023 14:30:07 -0300
Subject: [PATCH] ipagroup: Fix idempotence issues due to capitalization

Some attributes for ipagroup objects are stored using lower case letters
and should be converted upon retrieving parameter data.

This patch adds the missing conversion and provides a new test playbook:

    tests/group/test_group_case_insensitive.yml
---
 plugins/modules/ipagroup.py                 |  10 +-
 tests/group/test_group_case_insensitive.yml | 250 ++++++++++++++++++++
 2 files changed, 256 insertions(+), 4 deletions(-)
 create mode 100644 tests/group/test_group_case_insensitive.yml

diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py
index 70d85cb8..dcb1f3a3 100644
--- a/plugins/modules/ipagroup.py
+++ b/plugins/modules/ipagroup.py
@@ -520,12 +520,14 @@ def main():
     idoverrideuser = ansible_module.params_get("idoverrideuser")
     posix = ansible_module.params_get("posix")
     nomembers = ansible_module.params_get("nomembers")
-    user = ansible_module.params_get("user")
-    group = ansible_module.params_get("group")
+    user = ansible_module.params_get_lowercase("user")
+    group = ansible_module.params_get_lowercase("group")
     # Services are not case sensitive
     service = ansible_module.params_get_lowercase("service")
-    membermanager_user = ansible_module.params_get("membermanager_user")
-    membermanager_group = ansible_module.params_get("membermanager_group")
+    membermanager_user = (
+        ansible_module.params_get_lowercase("membermanager_user"))
+    membermanager_group = (
+        ansible_module.params_get_lowercase("membermanager_group"))
     externalmember = ansible_module.params_get("externalmember")
     # rename
     rename = ansible_module.params_get("rename")
diff --git a/tests/group/test_group_case_insensitive.yml b/tests/group/test_group_case_insensitive.yml
new file mode 100644
index 00000000..098d77e6
--- /dev/null
+++ b/tests/group/test_group_case_insensitive.yml
@@ -0,0 +1,250 @@
+---
+- name: Test group members case insensitive
+  hosts: ipaserver
+  become: true
+  gather_facts: false
+  module_defaults:
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+    ipauser:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+
+  vars:
+    test_users:
+      - { name: useR1, first: user1, last: last }
+      - { name: User2, first: user2, last: last }
+    user_names: "{{ test_users | map(attribute='name') }}"
+    test_groups:
+      - name: Group1
+      - name: Group2
+    group_names: "{{ test_groups | map(attribute='name') }}"
+
+  tasks:
+  - name: Include tasks ../env_freeipa_facts.yml
+    ansible.builtin.include_tasks: ../env_freeipa_facts.yml
+
+  - name: Test in all supported versions of IPA
+    block:
+      # setup environment
+      - name: Ensure testgroup is absent
+        ipagroup:
+          name: testgroup
+          state: absent
+
+      - name: Ensure test users are present
+        ipauser:
+          users: "{{ test_users }}"
+
+      - name: Ensure test groups are present
+        ipagroup:
+          groups: "{{ test_groups }}"
+
+      # tests
+      - name: Test group presence with user members
+        vars:
+          test_cases:
+            - { id: 1, value: "{{ user_names[0] | lower }}", expected: true }
+            - { id: 2, value: "{{ user_names[0] | upper }}", expected: false }
+            - { id: 3, value: "{{ user_names[0] }}", expected: false }
+            - { id: 4, value: "{{ user_names }}", expected: true }
+            - { id: 5, value: "{{ user_names | upper }}", expected: false }
+            - { id: 6, value: "{{ user_names | lower }}", expected: false }
+            - { id: 7, value: "{{ user_names[1] }}", expected: true }
+            - { id: 8, value: "{{ user_names[1] | upper }}", expected: false }
+            - { id: 9, value: "{{ user_names[1] | lower }}", expected: false }
+            - { id: 10, value: [], expected: true }
+        block:
+          - name: Ensure group with user parameter present
+            ipagroup:
+              name: testgroup
+              user: "{{ item.value }}"
+            register: output
+            failed_when: output.changed != item.expected or output.failed
+            loop: "{{ test_cases }}"
+            loop_control:
+              label: "Test id: {{ item.id }}"
+
+      - name: Test group presence with group parameter
+        vars:
+          test_cases:
+            - { id: 1, value: "{{ group_names[0] | lower }}", expected: true }
+            - { id: 2, value: "{{ group_names[0] | upper }}", expected: false }
+            - { id: 3, value: "{{ group_names[0] }}", expected: false }
+            - { id: 4, value: "{{ group_names }}", expected: true }
+            - { id: 5, value: "{{ group_names | upper }}", expected: false }
+            - { id: 6, value: "{{ group_names | lower }}", expected: false }
+            - { id: 7, value: "{{ group_names[1] }}", expected: true }
+            - { id: 8, value: "{{ group_names[1] | upper }}", expected: false }
+            - { id: 9, value: "{{ group_names[1] | lower }}", expected: false }
+            - { id: 10, value: [], expected: true }
+        block:
+          - name: Ensure group with group present
+            ipagroup:
+              name: testgroup
+              group: "{{ item.value }}"
+            register: output
+            failed_when: output.changed != item.expected or output.failed
+            loop: "{{ test_cases }}"
+            loop_control:
+              label: "Test id: {{ item.id }}"
+
+      - name: Test group with group and user parameters, action member
+        vars:
+          test_cases:
+            - { id: 1, user: "{{ user_names }}", group: "{{ group_names }}", expected: true }
+            - { id: 2, user: "{{ user_names[0] }}", state: "absent", expected: true }
+            - { id: 3, user: "{{ user_names[1] }}", state: "present", expected: false }
+            - { id: 4, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
+            - { id: 5, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
+            - { id: 6, group: "{{ group_names[0] | upper }}", state: "present", expected: false }
+            - { id: 7, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
+            - { id: 8, group: "{{ group_names[0] }}", state: "present", expected: false }
+            - { id: 9, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: true }
+            - { id: 10, user: "{{ user_names[0] | lower }}", group: "{{ group_names[0] | upper }}", state: "absent", expected: false }
+            - { id: 11, user: "{{ user_names[0] | upper }}", group: "{{ group_names[0] | lower }}", state: "absent", expected: false }
+            - { id: 12, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: false }
+            - { id: 13, group: "{{ group_names[0] | upper }}", state: "present", expected: true }
+            - { id: 14, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
+            - { id: 15, group: "{{ group_names[0] }}", state: "present", expected: false }
+            - { id: 16, group: "{{ group_names[1] | upper }}", state: "present", expected: false }
+            - { id: 17, group: "{{ group_names[1] | lower }}", state: "present", expected: false }
+            - { id: 18, group: "{{ group_names[1] }}", state: "present", expected: false }
+            - { id: 19, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
+            - { id: 20, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
+            - { id: 21, user: "{{ user_names[1] }}", state: "present", expected: false }
+        block:
+          - name: Ensure group works with group/user attributes and action member
+            ipagroup:
+              name: testgroup
+              user: "{{ item.user | default(omit) }}"
+              group: "{{ item.group | default(omit) }}"
+              action: member
+              state: "{{ item.state | default('present') }}"
+            register: output
+            failed_when: output.changed != item.expected or output.failed
+            loop: "{{ test_cases }}"
+            loop_control:
+              label: "Test id: {{ item.id }}"
+    always:
+      # cleanup
+      - name: "Ensure test groups test_groups are absent"
+        ipagroup:
+          name: "{{ group_names + ['testgroup'] }}"
+          state: absent
+
+      - name: "Ensure test users test_users are absent"
+        ipauser:
+          users: "{{ test_users }}"
+          state: absent
+
+  - name: Test in all IPA versions 8.4.4+
+    when: ipa_version is version('4.8.4', '>=')
+    block:
+      # setup environment
+      - name: Ensure test users are present
+        ipauser:
+          users: "{{ test_users }}"
+
+      - name: Ensure test groups are present
+        ipagroup:
+          groups: "{{ test_groups }}"
+
+      # tests
+      - name: Test group presence with memembermanager_user members
+        vars:
+          test_cases:
+            - { id: 1, value: "{{ user_names[0] | lower }}", expected: true }
+            - { id: 2, value: "{{ user_names[0] | upper }}", expected: false }
+            - { id: 3, value: "{{ user_names[0] }}", expected: false }
+            - { id: 4, value: "{{ user_names }}", expected: true }
+            - { id: 5, value: "{{ user_names | upper }}", expected: false }
+            - { id: 6, value: "{{ user_names | lower }}", expected: false }
+            - { id: 7, value: "{{ user_names[1] }}", expected: true }
+            - { id: 8, value: "{{ user_names[1] | upper }}", expected: false }
+            - { id: 9, value: "{{ user_names[1] | lower }}", expected: false }
+            - { id: 10, value: [], expected: true }
+        block:
+          - name: Ensure group with membermanager_user parameter present
+            ipagroup:
+              name: testgroup
+              membermanager_user: "{{ item.value }}"
+            register: output
+            failed_when: output.changed != item.expected or output.failed
+            loop: "{{ test_cases }}"
+            loop_control:
+              label: "Test id: {{ item.id }}"
+
+      - name: Test group presence with membermanager_group parameter
+        vars:
+          test_cases:
+            - { id: 1, value: "{{ group_names[0] | lower }}", expected: true }
+            - { id: 2, value: "{{ group_names[0] | upper }}", expected: false }
+            - { id: 3, value: "{{ group_names[0] }}", expected: false }
+            - { id: 4, value: "{{ group_names }}", expected: true }
+            - { id: 5, value: "{{ group_names | upper }}", expected: false }
+            - { id: 6, value: "{{ group_names | lower }}", expected: false }
+            - { id: 7, value: "{{ group_names[1] }}", expected: true }
+            - { id: 8, value: "{{ group_names[1] | upper }}", expected: false }
+            - { id: 9, value: "{{ group_names[1] | lower }}", expected: false }
+            - { id: 10, value: [], expected: true }
+        block:
+          - name: Ensure group with membermanager_group present
+            ipagroup:
+              name: testgroup
+              membermanager_group: "{{ item.value }}"
+            register: output
+            failed_when: output.changed != item.expected or output.failed
+            loop: "{{ test_cases }}"
+            loop_control:
+              label: "Test id: {{ item.id }}"
+
+      - name: Test group with membermanager_group and membermanager_user parameters, action member
+        vars:
+          test_cases:
+            - { id: 1, user: "{{ user_names }}", group: "{{ group_names }}", expected: true }
+            - { id: 2, user: "{{ user_names[0] }}", state: "absent", expected: true }
+            - { id: 3, user: "{{ user_names[1] }}", state: "present", expected: false }
+            - { id: 4, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
+            - { id: 5, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
+            - { id: 6, group: "{{ group_names[0] | upper }}", state: "present", expected: false }
+            - { id: 7, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
+            - { id: 8, group: "{{ group_names[0] }}", state: "present", expected: false }
+            - { id: 9, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: true }
+            - { id: 10, user: "{{ user_names[0] | lower }}", group: "{{ group_names[0] | upper }}", state: "absent", expected: false }
+            - { id: 11, user: "{{ user_names[0] | upper }}", group: "{{ group_names[0] | lower }}", state: "absent", expected: false }
+            - { id: 12, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: false }
+            - { id: 13, group: "{{ group_names[0] | upper }}", state: "present", expected: true }
+            - { id: 14, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
+            - { id: 15, group: "{{ group_names[0] }}", state: "present", expected: false }
+            - { id: 16, group: "{{ group_names[1] | upper }}", state: "present", expected: false }
+            - { id: 17, group: "{{ group_names[1] | lower }}", state: "present", expected: false }
+            - { id: 18, group: "{{ group_names[1] }}", state: "present", expected: false }
+            - { id: 19, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
+            - { id: 20, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
+            - { id: 21, user: "{{ user_names[1] }}", state: "present", expected: false }
+        block:
+          - name: Ensure group works with group/user attributes and action member
+            ipagroup:
+              name: testgroup
+              membermanager_user: "{{ item.user | default(omit) }}"
+              membermanager_group: "{{ item.group | default(omit) }}"
+              action: member
+              state: "{{ item.state | default('present') }}"
+            register: output
+            failed_when: output.changed != item.expected or output.failed
+            loop: "{{ test_cases }}"
+            loop_control:
+              label: "Test id: {{ item.id }}"
+    always:
+      # cleanup
+      - name: "Ensure test groups test_groups are absent"
+        ipagroup:
+          name: "{{ group_names + ['testgroup'] }}"
+          state: absent
+
+      - name: "Ensure test users test_users are absent"
+        ipauser:
+          name: "{{ user_names }}"
+          state: absent
-- 
GitLab