From f4c9e2871582ee5e37f5d51b36ff5861c310b961 Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Mon, 18 Sep 2023 11:27:46 -0300
Subject: [PATCH] Rename parameter 'allow_empty_string' to
 'allow_empty_list_item'

The parameter 'allow_empty_string' in 'module_params_get' is used to
allow an item in a list to be an empty string. The problem is that the
naming is misleading, as it is checking a list item rather than a
string.

This patch rename the parameter to 'allow_empty_list_item' so that it
more clearly refers to list itens instead of standalone strings, and do
not collide with future parameters that may test for empty strings which
are not part of lists.
---
 .../module_utils/ansible_freeipa_module.py    | 25 +++++++++----------
 plugins/modules/ipaconfig.py                  |  4 +--
 plugins/modules/ipahost.py                    |  7 +++---
 plugins/modules/ipaservice.py                 |  6 +++--
 plugins/modules/ipauser.py                    |  4 +--
 5 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py
index 190f585a..1c40fa2c 100644
--- a/plugins/module_utils/ansible_freeipa_module.py
+++ b/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
-    # if there is another entry in the list together with the empty
-    # string.
+    # 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 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):
-    value = module_params_get(module, name, allow_empty_string)
+def module_params_get_lowercase(module, name, allow_empty_list_item=False):
+    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)):
@@ -1051,7 +1050,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 +1058,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 +1072,11 @@ 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_fail_used_invalid(self, invalid_params, state, action=None):
         """
diff --git a/plugins/modules/ipaconfig.py b/plugins/modules/ipaconfig.py
index a32376cc..b94b3c72 100644
--- a/plugins/modules/ipaconfig.py
+++ b/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
diff --git a/plugins/modules/ipahost.py b/plugins/modules/ipahost.py
index e5e2f465..8daf1c42 100644
--- a/plugins/modules/ipahost.py
+++ b/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",
-                                          allow_empty_string=True)
+    sshpubkey = ansible_module.params_get(
+        "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(
diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py
index d0f56098..533eed36 100644
--- a/plugins/modules/ipaservice.py
+++ b/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)
-    auth_ind = ansible_module.params_get("auth_ind", allow_empty_string=True)
+    pac_type = ansible_module.params_get(
+        "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")
diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py
index dcea92f4..03bf8af7 100644
--- a/plugins/modules/ipauser.py
+++ b/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")
-- 
GitLab