From 9decad4e4f308e9d3e21f969adfed5518f48eab1 Mon Sep 17 00:00:00 2001
From: Thomas Woerner <twoerner@redhat.com>
Date: Tue, 22 Feb 2022 16:17:53 +0100
Subject: [PATCH] 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 | 4 +-
plugins/modules/ipaservice.py | 19 ++-
.../test_service_empty_string_params.yml | 110 ++++++++++++++++++
3 files changed, 126 insertions(+), 7 deletions(-)
create mode 100644 tests/service/test_service_empty_string_params.yml
diff --git a/README-service.md b/README-service.md
index 666c1893..c0cd3272 100644
--- a/README-service.md
+++ b/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
diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py
index bc737712..989ab9a0 100644
--- a/plugins/modules/ipaservice.py
+++ b/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 (
diff --git a/tests/service/test_service_empty_string_params.yml b/tests/service/test_service_empty_string_params.yml
new file mode 100644
index 00000000..1831e496
--- /dev/null
+++ b/tests/service/test_service_empty_string_params.yml
@@ -0,0 +1,110 @@
+---
+- 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
--
GitLab