Merge pull request #1143 from rjeffman/global_handle_datatype

Handle data type or empty string in module_utils

plugins/module_utils/ansible_freeipa_module.py

@@ -470,12 +470,11 @@ def _afm_convert(value):
     return value
 
 
-def module_params_get(module, name, allow_empty_string=False):
+def module_params_get(module, name, allow_empty_list_item=False):
     value = _afm_convert(module.params.get(name))
 
-    # Fail on empty strings in the list or if allow_empty_string is True
+    # Fail on empty strings in the list or if allow_empty_list_item is True
-    # if there is another entry in the list together with the empty
+    # if there is another entry in the list together with the empty string.
-    # string.
     # Due to an issue in Ansible it is possible to use the empty string
     # "" for lists with choices, even if the empty list is not part of
     # the choices.
@@ -483,7 +482,7 @@ def module_params_get(module, name, allow_empty_string=False):
     if isinstance(value, list):
         for val in value:
             if isinstance(val, (str, unicode)) and not val:
-                if not allow_empty_string:
+                if not allow_empty_list_item:
                     module.fail_json(
                         msg="Parameter '%s' contains an empty string" %
                         name)
@@ -495,8 +494,8 @@ def module_params_get(module, name, allow_empty_string=False):
     return value
 
 
-def module_params_get_lowercase(module, name, allow_empty_string=False):
+def module_params_get_lowercase(module, name, allow_empty_list_item=False):
-    value = module_params_get(module, name, allow_empty_string)
+    value = module_params_get(module, name, allow_empty_list_item)
     if isinstance(value, list):
         value = [v.lower() for v in value]
     if isinstance(value, (str, unicode)):
@@ -504,6 +503,48 @@ def module_params_get_lowercase(module, name, allow_empty_string=False):
     return value
 
 
+def module_params_get_with_type_cast(
+    module, name, datatype, allow_empty=False
+):
+    """
+    Retrieve value set for module parameter as a specific data type.
+
+    Parameters
+    ----------
+    module: AnsibleModule
+        The module from where to get the parameter value from.
+    name: string
+        The name of the parameter to retrieve.
+    datatype: type
+        The type to convert the value to, if value is not empty.
+    allow_empty: bool
+        Allow an empty string for non list parameters or a list
+        containing (only) an empty string item. This is used for
+        resetting parameters to the default value.
+
+    """
+    value = module_params_get(module, name, allow_empty)
+    if not allow_empty and value == "":
+        module.fail_json(
+            msg="Argument '%s' must not be an empty string" % (name,)
+        )
+    if value is not None and value != "":
+        try:
+            if datatype is bool:
+                # We let Ansible handle bool values
+                value = boolean(value)
+            else:
+                value = datatype(value)
+        except ValueError:
+            module.fail_json(
+                msg="Invalid value '%s' for argument '%s'" % (value, name)
+            )
+        except TypeError as terr:
+            # If Ansible fails to parse a boolean, it will raise TypeError
+            module.fail_json(msg="Param '%s': %s" % (name, str(terr)))
+    return value
+
+
 def api_get_domain():
     return api.env.domain
 
@@ -1051,7 +1092,7 @@ class IPAAnsibleModule(AnsibleModule):
             finally:
                 temp_kdestroy(ccache_dir, ccache_name)
 
-    def params_get(self, name, allow_empty_string=False):
+    def params_get(self, name, allow_empty_list_item=False):
         """
         Retrieve value set for module parameter.
 
@@ -1059,13 +1100,13 @@ class IPAAnsibleModule(AnsibleModule):
         ----------
         name: string
             The name of the parameter to retrieve.
-        allow_empty_string: bool
+        allow_empty_list_item: bool
             The parameter allowes to have empty strings in a list
 
         """
-        return module_params_get(self, name, allow_empty_string)
+        return module_params_get(self, name, allow_empty_list_item)
 
-    def params_get_lowercase(self, name, allow_empty_string=False):
+    def params_get_lowercase(self, name, allow_empty_list_item=False):
         """
         Retrieve value set for module parameter as lowercase, if not None.
 
@@ -1073,11 +1114,34 @@ class IPAAnsibleModule(AnsibleModule):
         ----------
         name: string
             The name of the parameter to retrieve.
-        allow_empty_string: bool
+        allow_empty_list_item: bool
             The parameter allowes to have empty strings in a list
 
         """
-        return module_params_get_lowercase(self, name, allow_empty_string)
+        return module_params_get_lowercase(self, name, allow_empty_list_item)
+
+    def params_get_with_type_cast(
+        self, name, datatype, allow_empty=True
+    ):
+        """
+        Retrieve value set for module parameter as a specific data type.
+
+        Parameters
+        ----------
+        name: string
+            The name of the parameter to retrieve.
+        datatype: type
+            The type to convert the value to, if not empty.
+        datatype: type
+            The type to convert the value to, if value is not empty.
+        allow_empty: bool
+            Allow an empty string for non list parameters or a list
+            containing (only) an empty string item. This is used for
+            resetting parameters to the default value.
+
+        """
+        return module_params_get_with_type_cast(
+            self, name, datatype, allow_empty)
 
     def params_fail_used_invalid(self, invalid_params, state, action=None):
         """

plugins/modules/ipaconfig.py

@@ -470,13 +470,13 @@ def main():
         "netbios_name": "netbios_name",
         "add_sids": "add_sids",
     }
-    allow_empty_string = ["pac_type", "user_auth_type", "configstring"]
     reverse_field_map = {v: k for k, v in field_map.items()}
+    allow_empty_list_item = ["pac_type", "user_auth_type", "configstring"]
 
     params = {}
     for x in field_map:
         val = ansible_module.params_get(
-            x, allow_empty_string=x in allow_empty_string)
+            x, allow_empty_list_item=x in allow_empty_list_item)
 
         if val is not None:
             params[field_map.get(x, x)] = val

plugins/modules/ipahost.py

@@ -876,10 +876,11 @@ def main():
     allow_retrieve_keytab_hostgroup = ansible_module.params_get(
         "allow_retrieve_keytab_hostgroup")
     mac_address = ansible_module.params_get("mac_address")
-    sshpubkey = ansible_module.params_get("sshpubkey",
+    sshpubkey = ansible_module.params_get(
-                                          allow_empty_string=True)
+        "sshpubkey", allow_empty_list_item=True)
     userclass = ansible_module.params_get("userclass")
-    auth_ind = ansible_module.params_get("auth_ind", allow_empty_string=True)
+    auth_ind = ansible_module.params_get(
+        "auth_ind", allow_empty_list_item=True)
     requires_pre_auth = ansible_module.params_get("requires_pre_auth")
     ok_as_delegate = ansible_module.params_get("ok_as_delegate")
     ok_to_auth_as_delegate = ansible_module.params_get(

plugins/modules/ipaidoverridegroup.py

@@ -243,7 +243,7 @@ def main():
     # present
     description = ansible_module.params_get("description")
     name = ansible_module.params_get("name")
-    gid = ansible_module.params_get("gid")
+    gid = ansible_module.params_get_with_type_cast("gid", int)
 
     # runtime flags
     fallback_to_ldap = ansible_module.params_get("fallback_to_ldap")
@@ -271,19 +271,6 @@ def main():
 
     ansible_module.params_fail_used_invalid(invalid, state)
 
-    # Ensure parameter values are valid and have proper type.
-    def int_or_empty_param(value, param):
-        if value is not None and value != "":
-            try:
-                value = int(value)
-            except ValueError:
-                ansible_module.fail_json(
-                    msg="Invalid value '%s' for argument '%s'" % (value, param)
-                )
-        return value
-
-    gid = int_or_empty_param(gid, "gid")
-
     # Init
 
     changed = False

plugins/modules/ipaidoverrideuser.py

@@ -439,9 +439,9 @@ def main():
     # present
     description = ansible_module.params_get("description")
     name = ansible_module.params_get("name")
-    uid = ansible_module.params_get("uid")
+    uid = ansible_module.params_get_with_type_cast("uid", int)
     gecos = ansible_module.params_get("gecos")
-    gidnumber = ansible_module.params_get("gidnumber")
+    gidnumber = ansible_module.params_get_with_type_cast("gidnumber", int)
     homedir = ansible_module.params_get("homedir")
     shell = ansible_module.params_get("shell")
     sshpubkey = ansible_module.params_get("sshpubkey")
@@ -479,20 +479,6 @@ def main():
 
     ansible_module.params_fail_used_invalid(invalid, state, action)
 
-    # Ensure parameter values are valid and have proper type.
-    def int_or_empty_param(value, param):
-        if value is not None and value != "":
-            try:
-                value = int(value)
-            except ValueError:
-                ansible_module.fail_json(
-                    msg="Invalid value '%s' for argument '%s'" % (value, param)
-                )
-        return value
-
-    uid = int_or_empty_param(uid, "uid")
-    gidnumber = int_or_empty_param(gidnumber, "gidnumber")
-
     if certificate is not None:
         certificate = [cert.strip() for cert in certificate]
 

plugins/modules/ipapwpolicy.py

@@ -153,7 +153,7 @@ RETURN = """
 """
 
 from ansible.module_utils.ansible_freeipa_module import \
-    IPAAnsibleModule, compare_args_ipa, boolean
+    IPAAnsibleModule, compare_args_ipa
 
 
 def find_pwpolicy(module, name):
@@ -294,20 +294,34 @@ def main():
     names = ansible_module.params_get("name")
 
     # present
-    maxlife = ansible_module.params_get("maxlife")
+    maxlife = ansible_module.params_get_with_type_cast(
-    minlife = ansible_module.params_get("minlife")
+        "maxlife", int, allow_empty=True)
-    history = ansible_module.params_get("history")
+    minlife = ansible_module.params_get_with_type_cast(
-    minclasses = ansible_module.params_get("minclasses")
+        "minlife", int, allow_empty=True)
-    minlength = ansible_module.params_get("minlength")
+    history = ansible_module.params_get_with_type_cast(
-    priority = ansible_module.params_get("priority")
+        "history", int, allow_empty=True)
-    maxfail = ansible_module.params_get("maxfail")
+    minclasses = ansible_module.params_get_with_type_cast(
-    failinterval = ansible_module.params_get("failinterval")
+        "minclasses", int, allow_empty=True)
-    lockouttime = ansible_module.params_get("lockouttime")
+    minlength = ansible_module.params_get_with_type_cast(
-    maxrepeat = ansible_module.params_get("maxrepeat")
+        "minlength", int, allow_empty=True)
-    maxsequence = ansible_module.params_get("maxsequence")
+    priority = ansible_module.params_get_with_type_cast(
-    dictcheck = ansible_module.params_get("dictcheck")
+        "priority", int, allow_empty=True)
-    usercheck = ansible_module.params_get("usercheck")
+    maxfail = ansible_module.params_get_with_type_cast(
-    gracelimit = ansible_module.params_get("gracelimit")
+        "maxfail", int, allow_empty=True)
+    failinterval = ansible_module.params_get_with_type_cast(
+        "failinterval", int, allow_empty=True)
+    lockouttime = ansible_module.params_get_with_type_cast(
+        "lockouttime", int, allow_empty=True)
+    maxrepeat = ansible_module.params_get_with_type_cast(
+        "maxrepeat", int, allow_empty=True)
+    maxsequence = ansible_module.params_get_with_type_cast(
+        "maxsequence", int, allow_empty=True)
+    dictcheck = ansible_module.params_get_with_type_cast(
+        "dictcheck", bool, allow_empty=True)
+    usercheck = ansible_module.params_get_with_type_cast(
+        "usercheck", bool, allow_empty=True)
+    gracelimit = ansible_module.params_get_with_type_cast(
+        "gracelimit", int, allow_empty=True)
 
     # state
     state = ansible_module.params_get("state")
@@ -336,41 +350,6 @@ def main():
 
     ansible_module.params_fail_used_invalid(invalid, state)
 
-    # Ensure parameter values are valid and have proper type.
-    def int_or_empty_param(value, param):
-        if value is not None and value != "":
-            try:
-                value = int(value)
-            except ValueError:
-                ansible_module.fail_json(
-                    msg="Invalid value '%s' for argument '%s'" % (value, param)
-                )
-        return value
-
-    maxlife = int_or_empty_param(maxlife, "maxlife")
-    minlife = int_or_empty_param(minlife, "minlife")
-    history = int_or_empty_param(history, "history")
-    minclasses = int_or_empty_param(minclasses, "minclasses")
-    minlength = int_or_empty_param(minlength, "minlength")
-    priority = int_or_empty_param(priority, "priority")
-    maxfail = int_or_empty_param(maxfail, "maxfail")
-    failinterval = int_or_empty_param(failinterval, "failinterval")
-    lockouttime = int_or_empty_param(lockouttime, "lockouttime")
-    maxrepeat = int_or_empty_param(maxrepeat, "maxrepeat")
-    maxsequence = int_or_empty_param(maxsequence, "maxsequence")
-    gracelimit = int_or_empty_param(gracelimit, "gracelimit")
-
-    def bool_or_empty_param(value, param):  # pylint: disable=R1710
-        if value is None or value == "":
-            return value
-        try:
-            return boolean(value)
-        except TypeError as terr:
-            ansible_module.fail_json(msg="Param '%s': %s" % (param, str(terr)))
-
-    dictcheck = bool_or_empty_param(dictcheck, "dictcheck")
-    usercheck = bool_or_empty_param(usercheck, "usercheck")
-
     # Ensure gracelimit has proper limit.
     if gracelimit:
         if gracelimit < -1:

plugins/modules/ipaservice.py

@@ -607,8 +607,10 @@ def main():
     # white space also.
     if certificate is not None:
         certificate = [cert.strip() for cert in certificate]
-    pac_type = ansible_module.params_get("pac_type", allow_empty_string=True)
+    pac_type = ansible_module.params_get(
-    auth_ind = ansible_module.params_get("auth_ind", allow_empty_string=True)
+        "pac_type", allow_empty_list_item=True)
+    auth_ind = ansible_module.params_get(
+        "auth_ind", allow_empty_list_item=True)
     skip_host_check = ansible_module.params_get("skip_host_check")
     force = ansible_module.params_get("force")
     requires_pre_auth = ansible_module.params_get("requires_pre_auth")

plugins/modules/ipauser.py

@@ -1185,9 +1185,9 @@ def main():
     manager = ansible_module.params_get("manager")
     carlicense = ansible_module.params_get("carlicense")
     sshpubkey = ansible_module.params_get("sshpubkey",
-                                          allow_empty_string=True)
+                                          allow_empty_list_item=True)
     userauthtype = ansible_module.params_get("userauthtype",
-                                             allow_empty_string=True)
+                                             allow_empty_list_item=True)
     userclass = ansible_module.params_get("userclass")
     radius = ansible_module.params_get("radius")
     radiususer = ansible_module.params_get("radiususer")