ipagroup: Properly support IPA versions 4.6 and RHEL-7

group_add_member is only supporting services in more recent IPA versions.
This is 4.7+. Code has been added to detect if services are supported by
the used IPA version and used in the parameters of the module. In this case
an error is printed.

Additionally all parameters will be get from the module using
module_params_get provided by ansible_freeipa_module. Additional to_text
conversions have been removed as they are not needed anymore with this.

README-group.md

@@ -142,7 +142,7 @@ Variable | Description | Required
 `nomembers` | Suppress processing of membership attributes. (bool) | no
 `user` | List of user name strings assigned to this group. | no
 `group` | List of group name strings assigned to this group. | no
-`service` | List of service name strings assigned to this group | no
+`service` | List of service name strings assigned to this group. Only usable with IPA versions 4.7 and up. | no
 `action` | Work on group or member level. It can be on of `member` or `group` and defaults to `group`. | no
 `state` | The state to ensure. It can be one of `present` or `absent`, default: `present`. | yes
 

plugins/modules/ipagroup.py

@@ -70,7 +70,9 @@ options:
     required: false
     type: list
   service:
-    description: List of service names assigned to this group.
+    description:
+    - List of service names assigned to this group.
+    - Only usable with IPA versions 4.7 and up.
     required: false
     type: list
   action:
@@ -137,18 +139,18 @@ RETURN = """
 """
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils._text import to_text
 from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
-    temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa
+    temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
+    api_check_param, module_params_get
 
 
 def find_group(module, name):
     _args = {
         "all": True,
-        "cn": to_text(name),
+        "cn": name,
     }
 
-    _result = api_command(module, "group_find", to_text(name), _args)
+    _result = api_command(module, "group_find", name, _args)
 
     if len(_result["result"]) > 1:
         module.fail_json(
@@ -164,7 +166,7 @@ def gen_args(description, gid, nonposix, external, nomembers):
     if description is not None:
         _args["description"] = description
     if gid is not None:
-        _args["gidnumber"] = str(gid)
+        _args["gidnumber"] = gid
     if nonposix is not None:
         _args["nonposix"] = nonposix
     if external is not None:
@@ -219,22 +221,22 @@ def main():
     # Get parameters
 
     # general
-    ipaadmin_principal = ansible_module.params.get("ipaadmin_principal")
-    ipaadmin_password = ansible_module.params.get("ipaadmin_password")
-    names = ansible_module.params.get("name")
+    ipaadmin_principal = module_params_get(ansible_module, "ipaadmin_principal")
+    ipaadmin_password = module_params_get(ansible_module, "ipaadmin_password")
+    names = module_params_get(ansible_module, "name")
 
     # present
-    description = ansible_module.params.get("description")
-    gid = ansible_module.params.get("gid")
-    nonposix = ansible_module.params.get("nonposix")
-    external = ansible_module.params.get("external")
-    nomembers = ansible_module.params.get("nomembers")
-    user = ansible_module.params.get("user")
-    group = ansible_module.params.get("group")
-    service = ansible_module.params.get("service")
-    action = ansible_module.params.get("action")
+    description = module_params_get(ansible_module, "description")
+    gid = module_params_get(ansible_module, "gid")
+    nonposix = module_params_get(ansible_module, "nonposix")
+    external = module_params_get(ansible_module, "external")
+    nomembers = module_params_get(ansible_module, "nomembers")
+    user = module_params_get(ansible_module, "user")
+    group = module_params_get(ansible_module, "group")
+    service = module_params_get(ansible_module, "service")
+    action = module_params_get(ansible_module, "action")
     # state
-    state = ansible_module.params.get("state")
+    state = module_params_get(ansible_module, "state")
 
     # Check parameters
 
@@ -276,6 +278,12 @@ def main():
                                                  ipaadmin_password)
         api_connect()
 
+        has_add_member_service = api_check_param("group_add_member", "service")
+        if service is not None and not has_add_member_service:
+            ansible_module.fail_json(
+                msg="Managing a service as part of a group is not supported "
+                "by your IPA version")
+
         commands = []
 
         for name in names:
@@ -325,34 +333,56 @@ def main():
                             set(res_find.get("member_service", [])) -
                             set(service or []))
 
-                        # Add members
-                        if len(user_add) > 0 or len(group_add) > 0 or \
-                           len(service_add) > 0:
-                            commands.append([name, "group_add_member",
-                                             {
-                                                 "user": user_add,
-                                                 "group": group_add,
-                                                 "service": service_add,
-                                             }])
-                        # Remove members
-                        if len(user_del) > 0 or len(group_del) > 0 or \
-                           len(service_del) > 0:
-                            commands.append([name, "group_remove_member",
-                                             {
-                                                 "user": user_del,
-                                                 "group": group_del,
-                                                 "service": service_del,
-                                             }])
+                        if has_add_member_service:
+                            # Add members
+                            if len(user_add) > 0 or len(group_add) > 0 or \
+                               len(service_add) > 0:
+                                commands.append([name, "group_add_member",
+                                                 {
+                                                     "user": user_add,
+                                                     "group": group_add,
+                                                     "service": service_add,
+                                                 }])
+                            # Remove members
+                            if len(user_del) > 0 or len(group_del) > 0 or \
+                               len(service_del) > 0:
+                                commands.append([name, "group_remove_member",
+                                                 {
+                                                     "user": user_del,
+                                                     "group": group_del,
+                                                     "service": service_del,
+                                                 }])
+                        else:
+                            # Add members
+                            if len(user_add) > 0 or len(group_add) > 0:
+                                commands.append([name, "group_add_member",
+                                                 {
+                                                     "user": user_add,
+                                                     "group": group_add,
+                                                 }])
+                            # Remove members
+                            if len(user_del) > 0 or len(group_del) > 0:
+                                commands.append([name, "group_remove_member",
+                                                 {
+                                                     "user": user_del,
+                                                     "group": group_del,
+                                                 }])
                 elif action == "member":
                     if res_find is None:
                         ansible_module.fail_json(msg="No group '%s'" % name)
-
-                    commands.append([name, "group_add_member",
-                                     {
-                                         "user": user,
-                                         "group": group,
-                                         "service": service,
-                                     }])
+                    if has_add_member_service:
+                        commands.append([name, "group_add_member",
+                                         {
+                                             "user": user,
+                                             "group": group,
+                                             "service": service,
+                                         }])
+                    else:
+                        commands.append([name, "group_add_member",
+                                         {
+                                             "user": user,
+                                             "group": group,
+                                         }])
 
             elif state == "absent":
                 if action == "group":
@@ -376,7 +406,7 @@ def main():
 
         for name, command, args in commands:
             try:
-                result = api_command(ansible_module, command, to_text(name),
+                result = api_command(ansible_module, command, name,
                                      args)
                 if "completed" in result and result["completed"] > 0:
                     changed = True