From 173acf282b83a710a40c554d3b57028444d0e477 Mon Sep 17 00:00:00 2001
From: Thomas Woerner <twoerner@redhat.com>
Date: Wed, 26 Jun 2024 14:44:44 +0200
Subject: [PATCH] permission: Fix idempotency issues for DN parameters

The parameters

- subtree (ipapermlocation)
- target (ipapermtarget)
- targetto (ipapermtargetto)
- targetfrom (ipapermtargetfrom)

have not been idempotent as the result returned from permission_show was
a DN and not a string.

The find_permission function has been exetended to convert the values
for these parameters to strings.

Fixes: #1257
---
 plugins/modules/ipapermission.py     |  9 ++-
 tests/permission/test_permission.yml | 98 ++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/plugins/modules/ipapermission.py b/plugins/modules/ipapermission.py
index 1ed25722..9d3d122d 100644
--- a/plugins/modules/ipapermission.py
+++ b/plugins/modules/ipapermission.py
@@ -154,7 +154,7 @@ RETURN = """
 
 
 from ansible.module_utils.ansible_freeipa_module import \
-    IPAAnsibleModule, compare_args_ipa
+    IPAAnsibleModule, compare_args_ipa, to_text
 
 
 def find_permission(module, name):
@@ -164,7 +164,12 @@ def find_permission(module, name):
     except Exception:  # pylint: disable=broad-except
         # An exception is raised if permission name is not found.
         return None
-    return _result["result"]
+    _res = _result["result"]
+    for param in ["ipapermlocation", "ipapermtarget", "ipapermtargetto",
+                  "ipapermtargetfrom"]:
+        if param in _res:
+            _res[param] = [to_text(elem) for elem in _res[param]]
+    return _res
 
 
 def gen_args(right, attrs, bindtype, subtree,
diff --git a/tests/permission/test_permission.yml b/tests/permission/test_permission.yml
index 5b27a769..0f78f55f 100644
--- a/tests/permission/test_permission.yml
+++ b/tests/permission/test_permission.yml
@@ -247,6 +247,104 @@
     register: result
     failed_when: result.changed or result.failed
 
+  - name: Ensure permission perm-test-1 is present with subtree
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      subtree: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 is present with subtree again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      subtree: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 with target is present
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      target: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 with target is present, again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      target: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 with targetto is present
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      targetto: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 with targetto is present, again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      targetto: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 with targetfrom is present
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      targetfrom: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 with targetfrom is present, again
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      right: write
+      targetfrom: "cn=computers,cn=accounts,dc={{ ipaserver_domain | replace('.', ',dc=') }}"
+      attrs: locality
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure permission perm-test-1 with object_type and right is present
+    ipapermission:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: perm-test-1
+      object_type: host
+      right: all
+    register: result
+    failed_when: not result.changed or result.failed
+
   - name: Ensure attributes carlicense and displayname are present in permission "System{{ ':' }} Update DNS Entries"
     ipapermission:
       ipaadmin_password: SomeADMINpassword
-- 
GitLab