ipadelegation: Fix idempotence issues due to capitalization.

This patch force processing of permission, attribute and group attributes in lower case, to match behavior of IPA CLI, transforming all of them into lowercase characters. The new behavior fixes idempotence issues when mixing different capitalization in different tasks for the same attribute. A new test playbook is avaiable at: tests/delegation/test_delegation_member_case_insensitive.yml
rjeffman · Jan 24, 2024 · 5ea2ce9 · 5ea2ce9
1 parent 4321478
commit 5ea2ce9
Show file tree

Hide file tree

Showing 2 changed files with 240 additions and 19 deletions.
diff --git a/plugins/modules/ipadelegation.py b/plugins/modules/ipadelegation.py
@@ -180,10 +180,10 @@ def main():
     names = ansible_module.params_get("name")
 
     # present
-    permission = ansible_module.params_get("permission")
-    attribute = ansible_module.params_get("attribute")
+    permission = ansible_module.params_get_lowercase("permission")
+    attribute = ansible_module.params_get_lowercase("attribute")
     membergroup = ansible_module.params_get("membergroup")
-    group = ansible_module.params_get("group")
+    group = ansible_module.params_get_lowercase("group")
     action = ansible_module.params_get("action")
     # state
     state = ansible_module.params_get("state")
@@ -233,6 +233,7 @@ def main():
 
         commands = []
         for name in names:
+            args = {}
             # Make sure delegation exists
             res_find = find_delegation(ansible_module, name)
 
@@ -244,14 +245,7 @@ def main():
 
                 if action == "delegation":
                     # Found the delegation
-                    if res_find is not None:
-                        # For all settings is args, check if there are
-                        # different settings in the find result.
-                        # If yes: modify
-                        if not compare_args_ipa(ansible_module, args,
-                                                res_find):
-                            commands.append([name, "delegation_mod", args])
-                    else:
+                    if res_find is None:
                         commands.append([name, "delegation_add", args])
 
                 elif action == "member":
@@ -265,9 +259,7 @@ def main():
                     # New attribute list (add given ones to find result)
                     # Make list with unique entries
                     attrs = list(set(list(res_find["attrs"]) + attribute))
-                    if len(attrs) > len(res_find["attrs"]):
-                        commands.append([name, "delegation_mod",
-                                         {"attrs": attrs}])
+                    args["attrs"] = attrs
 
             elif state == "absent":
                 if action == "delegation":
@@ -288,15 +280,18 @@ def main():
                     if len(attrs) < 1:
                         ansible_module.fail_json(
                             msg="At minimum one attribute is needed.")
-
-                    # Entries New number of attributes is smaller
-                    if len(attrs) < len(res_find["attrs"]):
-                        commands.append([name, "delegation_mod",
-                                         {"attrs": attrs}])
+                    args["attrs"] = attrs
 
             else:
                 ansible_module.fail_json(msg="Unkown state '%s'" % state)
 
+            # Manage members
+            if (
+                args and res_find and
+                not compare_args_ipa(ansible_module, args, res_find)
+            ):
+                commands.append([name, "delegation_mod", args])
+
         # Execute commands
 
         changed = ansible_module.execute_ipa_commands(commands)

diff --git a/tests/delegation/test_delegation_member_case_insensitive.yml b/tests/delegation/test_delegation_member_case_insensitive.yml
@@ -0,0 +1,226 @@
+---
+- name: Test delegation
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: no
+  gather_facts: no
+
+  tasks:
+  - block:
+      # CLEANUP TEST ITEMS
+
+      - name: Ensure delegation "basic manager attributes" is absent
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          state: absent
+
+      # CREATE TEST ITEMS
+
+      - name: Ensure test group managers is present
+        ipagroup:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: managers
+
+      - name: Ensure test group employees is present
+        ipagroup:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: employees
+
+      # TESTS
+
+      - name: Ensure delegation "basic manager attributes" is present, group/membergroup mixed case
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - businesscategory
+          group: Managers
+          membergroup: Employees
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, group lowercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - businesscategory
+          group: "{{ 'Managers' | lower }}"
+          membergroup: Employees
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, group uppercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - businesscategory
+          group: "{{ 'Managers' | upper }}"
+          membergroup: Employees
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, permission upercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: "{{ 'read' | upper }}"
+          attribute:
+          - businesscategory
+          group: managers
+          membergroup: Employees
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, permission mixed case
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: Read
+          attribute:
+          - businesscategory
+          group: managers
+          membergroup: Employees
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, attribute upercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - "{{ 'businesscategory' | upper }}"
+          group: managers
+          membergroup: Employees
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, attribute mixed case
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - BusinessCategory
+          group: managers
+          membergroup: Employees
+        register: result
+        failed_when: result.changed or result.failed
+
+      # membergroup does not use case insensitive comparison
+
+      - name: Ensure delegation "basic manager attributes" is present, membergroup lowercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - businesscategory
+          group: managers
+          membergroup: "{{ 'Employees' | lower }}"
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, membergroup uppercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - businesscategory
+          group: managers
+          membergroup: "{{ 'Employees' | upper }}"
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" is present, group/membergroup mixed case
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - businesscategory
+          group: Managers
+          membergroup: Employees
+        register: result
+        failed_when: not result.changed or result.failed
+
+      # tests for action: member
+      - name: Ensure delegation "basic manager attributes" is present, with group and attribute in mixed case
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - BusinessCategory
+          group: Managers
+          membergroup: Employees
+
+      - name: Ensure delegation "basic manager attributes" is present, attribute mixed case
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          permission: read
+          attribute:
+          - BusinessCategory
+          group: managers
+          membergroup: employees
+
+      - name: Ensure delegation "basic manager attributes" member is present, attribute upercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          attribute:
+          - "{{ 'BusinessCategory' | upper }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure delegation "basic manager attributes" member is present, attribute lowercase
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          attribute:
+          - "{{ 'BusinessCategory' | lower }}"
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+    always:
+      # CLEANUP TEST ITEMS
+
+      - name: Ensure delegation "basic manager attributes" is absent
+        ipadelegation:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: "basic manager attributes"
+          state: absent
+
+      - name: Ensure test groups are absent
+        ipagroup:
+          ipaadmin_password: SomeADMINpassword
+          ipaapi_context: "{{ ipa_context | default(omit) }}"
+          name: managers,employees
+          state: absent