From 6132a947e65fb9c3a1ec5c059aed34afb06a67df Mon Sep 17 00:00:00 2001
From: Thomas Woerner <twoerner@redhat.com>
Date: Mon, 29 Jun 2020 13:12:12 +0200
Subject: [PATCH] ipa[host]group: Fix membermanager unknow user issue

If a unknown membermanager user presence will be ensured, the unknown user
error was ignored. This has been fixed in ipagroup. The code for the error
handling in ipagroup and ipahostgroup has been adapted because of this.

New tests for tests/[host]group/test_[host]group_membermnager.yml have been
added.
---
 plugins/modules/ipagroup.py                   | 19 +++++++++----------
 plugins/modules/ipahostgroup.py               | 13 +++++++------
 tests/group/test_group_membermanager.yml      | 11 ++++++++++-
 .../test_hostgroup_membermanager.yml          | 11 ++++++++++-
 4 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py
index 915bc499..903c256d 100644
--- a/plugins/modules/ipagroup.py
+++ b/plugins/modules/ipagroup.py
@@ -507,16 +507,15 @@ def main():
             # All "already a member" and "not a member" failures in the
             # result are ignored. All others are reported.
             errors = []
-            if "failed" in result and len(result["failed"]) > 0:
-                for item in result["failed"]:
-                    failed_item = result["failed"][item]
-                    for member_type in failed_item:
-                        for member, failure in failed_item[member_type]:
-                            if "already a member" in failure \
-                               or "not a member" in failure:
-                                continue
-                            errors.append("%s: %s %s: %s" % (
-                                command, member_type, member, failure))
+            for failed_item in result.get("failed", []):
+                failed = result["failed"][failed_item]
+                for member_type in failed:
+                    for member, failure in failed[member_type]:
+                        if "already a member" in failure \
+                           or "not a member" in failure:
+                            continue
+                        errors.append("%s: %s %s: %s" % (
+                            command, member_type, member, failure))
             if len(errors) > 0:
                 ansible_module.fail_json(msg=", ".join(errors))
 
diff --git a/plugins/modules/ipahostgroup.py b/plugins/modules/ipahostgroup.py
index 4c18e940..5f615160 100644
--- a/plugins/modules/ipahostgroup.py
+++ b/plugins/modules/ipahostgroup.py
@@ -423,14 +423,15 @@ def main():
             # All "already a member" and "not a member" failures in the
             # result are ignored. All others are reported.
             errors = []
-            if "failed" in result and "member" in result["failed"]:
-                failed = result["failed"]["member"]
+            for failed_item in result.get("failed", []):
+                failed = result["failed"][failed_item]
                 for member_type in failed:
                     for member, failure in failed[member_type]:
-                        if "already a member" not in failure \
-                           and "not a member" not in failure:
-                            errors.append("%s: %s %s: %s" % (
-                                command, member_type, member, failure))
+                        if "already a member" in failure \
+                           or "not a member" in failure:
+                            continue
+                        errors.append("%s: %s %s: %s" % (
+                            command, member_type, member, failure))
             if len(errors) > 0:
                 ansible_module.fail_json(msg=", ".join(errors))
 
diff --git a/tests/group/test_group_membermanager.yml b/tests/group/test_group_membermanager.yml
index 1d38654f..661f26d6 100644
--- a/tests/group/test_group_membermanager.yml
+++ b/tests/group/test_group_membermanager.yml
@@ -8,7 +8,7 @@
   - name: Ensure user manangeruser1 and manageruser2 is absent
     ipauser:
       ipaadmin_password: SomeADMINpassword
-      name: manageruser1,manageruser2
+      name: manageruser1,manageruser2,unknown_user
       state: absent
 
   - name: Ensure group testgroup, managergroup1 and managergroup2 are absent
@@ -185,6 +185,15 @@
     register: result
     failed_when: not result.changed
 
+  - name: Ensure unknown membermanager_user member failure
+    ipagroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testgroup
+      membermanager_user: unknown_user
+      action: member
+    register: result
+    failed_when: result.changed or "no such entry" not in result.msg
+
   - name: Ensure group testgroup, managergroup1 and managergroup2 are absent
     ipagroup:
       ipaadmin_password: SomeADMINpassword
diff --git a/tests/hostgroup/test_hostgroup_membermanager.yml b/tests/hostgroup/test_hostgroup_membermanager.yml
index c32d1088..c0f65460 100644
--- a/tests/hostgroup/test_hostgroup_membermanager.yml
+++ b/tests/hostgroup/test_hostgroup_membermanager.yml
@@ -15,7 +15,7 @@
   - name: Ensure user manangeruser1 and manageruser2 is absent
     ipauser:
       ipaadmin_password: SomeADMINpassword
-      name: manageruser1,manageruser2
+      name: manageruser1,manageruser2,unknown_user
       state: absent
 
   - name: Ensure group managergroup1 and managergroup2 are absent
@@ -200,6 +200,15 @@
     register: result
     failed_when: not result.changed
 
+  - name: Ensure unknown membermanager_user member failure
+    ipahostgroup:
+      ipaadmin_password: SomeADMINpassword
+      name: testhostgroup
+      membermanager_user: unknown_user
+      action: member
+    register: result
+    failed_when: result.changed or "no such entry" not in result.msg
+
   - name: Ensure host-group testhostgroup is absent
     ipahostgroup:
       ipaadmin_password: SomeADMINpassword