From 431dc8667ab7165f6480acea0434aa1b9366fd2e Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Wed, 4 Dec 2024 19:48:44 -0300
Subject: [PATCH] ipagroup: Correctly handle externalmember in member actions

When creating the lists of external members, the attribute
'ipaexternalmember' also needs to be added to the list of external
members that are part of the group object for external groups.

A test to verify the correct behavior was added and the test suite for
group external members have been cleaned up with 'yes' values changed to
'true' and the use of module_defaults.
---
 plugins/modules/ipagroup.py                 | 18 ++++-
 tests/group/test_group_external_members.yml | 90 +++++++++++++++------
 2 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py
index 09e90e09..7251fda2 100644
--- a/plugins/modules/ipagroup.py
+++ b/plugins/modules/ipagroup.py
@@ -746,7 +746,11 @@ def main():
 
                         (externalmember_add,
                          externalmember_del) = gen_add_del_lists(
-                            externalmember, res_find.get("member_external"))
+                            externalmember, (
+                                list(res_find.get("member_external", []))
+                                + list(res_find.get("ipaexternalmember", []))
+                            )
+                        )
 
                         (idoverrides_add,
                          idoverrides_del) = gen_add_del_lists(
@@ -780,7 +784,11 @@ def main():
                     service_add = gen_add_list(
                         service, res_find.get("member_service"))
                     externalmember_add = gen_add_list(
-                        externalmember, res_find.get("member_external"))
+                        externalmember, (
+                            list(res_find.get("member_external", []))
+                            + list(res_find.get("ipaexternalmember", []))
+                        )
+                    )
                     idoverrides_add = gen_add_list(
                         idoverrideuser, res_find.get("member_idoverrideuser"))
 
@@ -815,7 +823,11 @@ def main():
                     service_del = gen_intersection_list(
                         service, res_find.get("member_service"))
                     externalmember_del = gen_intersection_list(
-                        externalmember, res_find.get("member_external"))
+                        externalmember, (
+                            list(res_find.get("member_external", []))
+                            + list(res_find.get("ipaexternalmember", []))
+                        )
+                    )
                     idoverrides_del = gen_intersection_list(
                         idoverrideuser, res_find.get("member_idoverrideuser"))
 
diff --git a/tests/group/test_group_external_members.yml b/tests/group/test_group_external_members.yml
index 7541f876..f9214d3d 100644
--- a/tests/group/test_group_external_members.yml
+++ b/tests/group/test_group_external_members.yml
@@ -1,37 +1,45 @@
 ---
 - name: Find trust
   hosts: ipaserver
-  become: true
+  become: false
   gather_facts: false
+  module_defaults:
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
 
   tasks:
 
   - name: Include tasks ../env_freeipa_facts.yml
     ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
+  - name: Ensure tests groups are absent
+    ipagroup:
+      name:
+        - extgroup
+        - extgroup_members
+      state: absent
+
   - name: Execute group tests if trust test environment is supported
     when: trust_test_is_supported | default(false)
     block:
 
     - name: Add nonposix group.
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
-        nonposix: yes
+        nonposix: true
       register: result
       failed_when: result.failed or not result.changed
 
     - name: Set group to be external
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
-        external: yes
+        external: true
       register: result
       failed_when: result.failed or not result.changed
 
     - name: Add AD users to group
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
         external_member: "AD\\Domain Users"
       register: result
@@ -39,7 +47,6 @@
 
     - name: Add AD users to group, again
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
         external_member: "AD\\Domain Users"
       register: result
@@ -47,7 +54,6 @@
 
     - name: Remove external group
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
         state: absent
       register: result
@@ -55,27 +61,24 @@
 
     - name: Add nonposix, external group, with AD users.
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
-        nonposix: yes
-        external: yes
+        nonposix: true
+        external: true
         external_member: "AD\\Domain Users"
       register: result
       failed_when: result.failed or not result.changed
 
     - name: Add nonposix, external group, with AD users, again.
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
-        nonposix: yes
-        external: yes
+        nonposix: true
+        external: true
         external_member: "AD\\Domain Users"
       register: result
       failed_when: result.failed or result.changed
 
     - name: Remove group
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
         state: absent
       register: result
@@ -83,32 +86,71 @@
 
     - name: Add nonposix group.
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
-        nonposix: yes
+        nonposix: true
       register: result
       failed_when: result.failed or not result.changed
 
     - name: Set group to be external, and add users.
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
-        external: yes
+        external: true
         external_member: "AD\\Domain Users"
       register: result
       failed_when: result.failed or not result.changed
 
     - name: Set group to be external, and add users, again.
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
         name: extgroup
-        external: yes
+        external: true
         external_member: "AD\\Domain Users"
       register: result
       failed_when: result.failed or result.changed
 
-    - name: Cleanup environment.
+    - name: Ensure external group for external member exist
       ipagroup:
-        ipaadmin_password: SomeADMINpassword
-        name: extgroup
+        name: extgroup_members
+        external: true
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Ensure external group members are present
+      ipagroup:
+        name: extgroup_members
+        external_member: "AD\\Domain Users"
+        action: member
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Ensure external group members are present, again
+      ipagroup:
+        name: extgroup_members
+        external_member: "AD\\Domain Users"
+        action: member
+      register: result
+      failed_when: result.failed or result.changed
+
+    - name: Ensure external group members are absent
+      ipagroup:
+        name: extgroup_members
+        external_member: "AD\\Domain Users"
+        action: member
         state: absent
+      register: result
+      failed_when: result.failed or not result.changed
+
+    - name: Ensure external group members are absent, again
+      ipagroup:
+        name: extgroup_members
+        external_member: "AD\\Domain Users"
+        action: member
+        state: absent
+      register: result
+      failed_when: result.failed or result.changed
+
+  - name: Ensure tests groups are absent
+    ipagroup:
+      name:
+        - extgroup
+        - extgroup_members
+      state: absent
-- 
GitLab