ipaservice: Set allow_empty_string for auth_ind and pac_type

The parameters auth_ind and pac_type are allowing to use "" to reset to
the default value.

The new check in params_get is not allowing to use empty strings in lists,
therefore allow_empty_string=True had to be added to the call.

A test has been added to verify that the empty strings are supported and
working. An idempotency issue with pac_type has been found with the test
and fixed additionally.

README-service.md

@@ -293,8 +293,8 @@ Variable | Description | Required
 `ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to yes. (bool) | no
 `name` \| `service` | The list of service name strings. | yes
 `certificate` \| `usercertificate` | Base-64 encoded service certificate. | no
-`pac_type` \| `ipakrbauthzdata` | Supported PAC type. It can be one of `MS-PAC`, `PAD`, or `NONE`. | no
-`auth_ind` \| `krbprincipalauthind` | Defines an allow list for Authentication Indicators. It can be any of `otp`, `radius`, `pkinit`, or `hardened`. | no
+`pac_type` \| `ipakrbauthzdata` | Supported PAC type. It can be one of `MS-PAC`, `PAD`, or `NONE`. Use empty string to reset pac_type to the initial value. | no
+`auth_ind` \| `krbprincipalauthind` | Defines an allow list for Authentication Indicators. It can be any of `otp`, `radius`, `pkinit` or `hardened`.  Use empty string to reset auth_ind to the initial value. | no
 `requires_pre_auth` \| `ipakrbrequirespreauth` | Pre-authentication is required for the service. Default to true. (bool) | no
 `ok_as_delegate` \|  `ipakrbokasdelegate` | Client credentials may be delegated to the service. Default to false. (bool) | no
 `ok_to_auth_as_delegate` \|  `ipakrboktoauthasdelegate` | The service is allowed to authenticate on behalf of a client. Default to false. (bool) | no

plugins/modules/ipaservice.py

@@ -50,13 +50,13 @@ options:
   pac_type:
     description: Supported PAC type.
     required: false
-    choices: ["MS-PAC", "PAD", "NONE"]
+    choices: ["MS-PAC", "PAD", "NONE", ""]
     type: list
     aliases: ["pac_type", "ipakrbauthzdata"]
   auth_ind:
     description: Defines a whitelist for Authentication Indicators.
     required: false
-    choices: ["otp", "radius", "pkinit", "hardened"]
+    choices: ["otp", "radius", "pkinit", "hardened", ""]
     aliases: ["krbprincipalauthind"]
   skip_host_check:
     description: Skip checking if host object exists.
@@ -356,7 +356,7 @@ def init_ansible_module():
             smb=dict(type="bool", required=False),
             netbiosname=dict(type="str", required=False),
             pac_type=dict(type="list", aliases=["ipakrbauthzdata"],
-                          choices=["MS-PAC", "PAD", "NONE"]),
+                          choices=["MS-PAC", "PAD", "NONE", ""]),
             auth_ind=dict(type="list",
                           aliases=["krbprincipalauthind"],
                           choices=["otp", "radius", "pkinit", "hardened", ""]),
@@ -420,8 +420,8 @@ def main():
     # service attributes
     principal = ansible_module.params_get("principal")
     certificate = ansible_module.params_get("certificate")
-    pac_type = ansible_module.params_get("pac_type")
-    auth_ind = ansible_module.params_get("auth_ind")
+    pac_type = ansible_module.params_get("pac_type", allow_empty_string=True)
+    auth_ind = ansible_module.params_get("auth_ind", allow_empty_string=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")
@@ -537,6 +537,15 @@ def main():
                             if remove in args:
                                 del args[remove]
 
+                        if (
+                            "ipakrbauthzdata" in args
+                            and (
+                                args.get("ipakrbauthzdata", [""]) ==
+                                res_find.get("ipakrbauthzdata", [""])
+                            )
+                        ):
+                            del args["ipakrbauthzdata"]
+
                         if (
                             "krbprincipalauthind" in args
                             and (

tests/service/test_service_empty_string_params.yml

+---
+- name: Test service
+  hosts: "{{ ipa_test_host | default('ipaserver') }}"
+  become: yes
+  gather_facts: yes
+
+  tasks:
+
+  # CLEANUP TEST ITEMS
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is absent.
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      continue: yes
+      state: absent
+
+  # CREATE TEST ITEMS
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+    register: result
+    failed_when: not result.changed or result.failed
+
+  # TESTS
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with pac_type MS-PAC and PAD
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      pac_type:
+        - MS-PAC
+        - PAD
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with pac_type MS-PAC and PAD, again
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      pac_type:
+        - MS-PAC
+        - PAD
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with empty pac_type
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      pac_type: ""
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with empty pac_type, again
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      pac_type: ""
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with auth_ind otp and radius
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      auth_ind:
+        - otp
+        - radius
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with auth_ind otp and radius, again
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      auth_ind:
+        - otp
+        - radius
+    register: result
+    failed_when: result.changed or result.failed
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with empty auth_ind
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      auth_ind: ""
+    register: result
+    failed_when: not result.changed or result.failed
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is present with empty auth_ind, again
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      auth_ind: ""
+    register: result
+    failed_when: result.changed or result.failed
+
+  # CLEANUP TEST ITEMS
+
+  - name: Ensure service "test-service/{{ ansible_facts['fqdn'] }}" is absent.
+    ipaservice:
+      ipaadmin_password: SomeADMINpassword
+      ipaapi_context: "{{ ipa_context | default(omit) }}"
+      name: "test-service/{{ ansible_facts['fqdn'] }}"
+      continue: yes
+      state: absent