From d05ad6b1f27a4ce6be32c123e51ee9b978376fe2 Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Tue, 22 Feb 2022 14:27:16 +0100 Subject: [PATCH 1/5] module_params_get*: Fail on empty string in string list parameters So far it is possible to pass list parameters with empty strings to the modules. The use of empty strings in list does not make a lot of sense, though. The simple solution is to add a check to module_params_get for empty strings in returned lists. The option allow_empty_string can be set to True to allow an empty string in the list with a list len of 1. The option defaults to False. It is needed for some parameters the modules, like for example userauthtype in the user module. It is using "" to reset to the default value. module_params_get_lowercase has been changed to use module_params_get to have one place to add the check. 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. Ansible issue https://github.com/ansible/ansible/issues/77108 --- .../module_utils/ansible_freeipa_module.py | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 3c25c795..17d02d0e 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -398,11 +398,32 @@ else: return value - def module_params_get(module, name): - return _afm_convert(module.params.get(name)) - - def module_params_get_lowercase(module, name): + def module_params_get(module, name, allow_empty_string=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. + # 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. + # Ansible issue https://github.com/ansible/ansible/issues/77108 + if isinstance(value, list): + for val in value: + if isinstance(val, (str, unicode)) and not val: + if not allow_empty_string: + module.fail_json( + msg="Parameter '%s' contains an empty string" % + name) + elif len(value) > 1: + module.fail_json( + msg="Parameter '%s' may not contain another " + "entry together with an empty string" % name) + + return value + + def module_params_get_lowercase(module, name, allow_empty_string=False): + value = module_params_get(module, name, allow_empty_string) if isinstance(value, list): value = [v.lower() for v in value] if isinstance(value, (str, unicode)): @@ -897,7 +918,7 @@ else: finally: temp_kdestroy(ccache_dir, ccache_name) - def params_get(self, name): + def params_get(self, name, allow_empty_string=False): """ Retrieve value set for module parameter. @@ -905,11 +926,13 @@ else: ---------- name: string The name of the parameter to retrieve. + allow_empty_string: bool + The parameter allowes to have empty strings in a list """ - return module_params_get(self, name) + return module_params_get(self, name, allow_empty_string) - def params_get_lowercase(self, name): + def params_get_lowercase(self, name, allow_empty_string=False): """ Retrieve value set for module parameter as lowercase, if not None. @@ -917,9 +940,11 @@ else: ---------- name: string The name of the parameter to retrieve. + allow_empty_string: bool + The parameter allowes to have empty strings in a list """ - return module_params_get_lowercase(self, name) + return module_params_get_lowercase(self, name, allow_empty_string) def params_fail_used_invalid(self, invalid_params, state, action=None): """ From 03098c218d8cea0c69d67f26a4e786d9af91e945 Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Tue, 22 Feb 2022 15:04:00 +0100 Subject: [PATCH 2/5] ipauser: Set allow_empty_string for userauthtype and sshpubkey The parameters userauthtype and sshpubkey 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 sshpubkey has been found with the test and fixed additionally. --- plugins/modules/ipauser.py | 13 ++- tests/user/test_user_empty_string_params.yml | 115 +++++++++++++++++++ 2 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 tests/user/test_user_empty_string_params.yml diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index aee71cd2..fb0fbc47 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -892,8 +892,10 @@ def main(): title = ansible_module.params_get("title") manager = ansible_module.params_get("manager") carlicense = ansible_module.params_get("carlicense") - sshpubkey = ansible_module.params_get("sshpubkey") - userauthtype = ansible_module.params_get("userauthtype") + sshpubkey = ansible_module.params_get("sshpubkey", + allow_empty_string=True) + userauthtype = ansible_module.params_get("userauthtype", + allow_empty_string=True) userclass = ansible_module.params_get("userclass") radius = ansible_module.params_get("radius") radiususer = ansible_module.params_get("radiususer") @@ -1101,6 +1103,13 @@ def main(): if "noprivate" in args: del args["noprivate"] + # Ignore sshpubkey if it is empty (for resetting) + # and not set in for the user + if "ipasshpubkey" not in res_find and \ + "ipasshpubkey" in args and \ + args["ipasshpubkey"] == ['']: + del args["ipasshpubkey"] + # Ignore userauthtype if it is empty (for resetting) # and not set in for the user if "ipauserauthtype" not in res_find and \ diff --git a/tests/user/test_user_empty_string_params.yml b/tests/user/test_user_empty_string_params.yml new file mode 100644 index 00000000..ebece0c0 --- /dev/null +++ b/tests/user/test_user_empty_string_params.yml @@ -0,0 +1,115 @@ +--- +- name: Test user + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: true + gather_facts: false + + tasks: + + # CLEANUP TEST ITEMS + + - name: Ensure user testuser absent + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + state: absent + + # CREATE TEST ITEMS + + - name: Ensure user testuser present + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + first: test + last: user + register: result + failed_when: not result.changed or result.failed + + # TESTS + + - name: Ensure user testuser present with userauthtype password,radius,otp + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + userauthtype: password,radius,otp + register: result + failed_when: not result.changed or result.failed + + - name: Ensure user testuser present with userauthtype password,radius,otp, again + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + userauthtype: password,radius,otp + register: result + failed_when: result.changed or result.failed + + - name: Ensure user testuser present with empty userauthtype + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + userauthtype: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure user testuser present with empty userauthtype, again + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + userauthtype: "" + register: result + failed_when: result.changed or result.failed + + - name: Ensure user testuser present with sshpubkey + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + sshpubkey: + # yamllint disable-line rule:line-length + - ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCqmVDpEX5gnSjKuv97AyzOhaUMMKz8ahOA3GY77tVC4o68KNgMCmDSEG1/kOIaElngNLaCha3p/2iAcU9Bi1tLKUlm2bbO5NHNwHfRxY/3cJtq+/7D1vxJzqThYwI4F9vr1WxyY2+mMTv3pXbfAJoR8Mu06XaEY5PDetlDKjHLuNWF+/O7ZU8PsULTa1dJZFrtXeFpmUoLoGxQBvlrlcPI1zDciCSU24t27Zan5Py2l5QchyI7yhCyMM77KDtj5+AFVpmkb9+zq50rYJAyFVeyUvwjzErvQrKJzYpA0NyBp7vskWbt36M16/M/LxEK7HA6mkcakO3ESWx5MT1LAjvdlnxbWG3787MxweHXuB8CZU+9bZPFBaJ+VQtOfJ7I8eH0S16moPC4ak8FlcFvOH8ERDPWLFDqfy09yaZ7bVIF0//5ZI7Nf3YDe3S7GrBX5ieYuECyP6UNkTx9BRsAQeVvXEc6otzB7iCSnYBMGUGzCqeigoAWaVQUONsSR3Uatks= pinky@ipaserver.el81.local # noqa 204 + register: result + failed_when: not result.changed or result.failed + + - name: Ensure user testuser present with sshpubkey, again + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + sshpubkey: + # yamllint disable-line rule:line-length + - ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCqmVDpEX5gnSjKuv97AyzOhaUMMKz8ahOA3GY77tVC4o68KNgMCmDSEG1/kOIaElngNLaCha3p/2iAcU9Bi1tLKUlm2bbO5NHNwHfRxY/3cJtq+/7D1vxJzqThYwI4F9vr1WxyY2+mMTv3pXbfAJoR8Mu06XaEY5PDetlDKjHLuNWF+/O7ZU8PsULTa1dJZFrtXeFpmUoLoGxQBvlrlcPI1zDciCSU24t27Zan5Py2l5QchyI7yhCyMM77KDtj5+AFVpmkb9+zq50rYJAyFVeyUvwjzErvQrKJzYpA0NyBp7vskWbt36M16/M/LxEK7HA6mkcakO3ESWx5MT1LAjvdlnxbWG3787MxweHXuB8CZU+9bZPFBaJ+VQtOfJ7I8eH0S16moPC4ak8FlcFvOH8ERDPWLFDqfy09yaZ7bVIF0//5ZI7Nf3YDe3S7GrBX5ieYuECyP6UNkTx9BRsAQeVvXEc6otzB7iCSnYBMGUGzCqeigoAWaVQUONsSR3Uatks= pinky@ipaserver.el81.local # noqa 204 + register: result + failed_when: result.changed or result.failed + + - name: Ensure user testuser present with empty sshpubkey + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + sshpubkey: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure user testuser present with empty sshpubkey, again + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + sshpubkey: "" + register: result + failed_when: result.changed or result.failed + + # CLEANUP TEST ITEMS + + - name: Ensure user testuser absent + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testuser + state: absent From 9decad4e4f308e9d3e21f969adfed5518f48eab1 Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Tue, 22 Feb 2022 16:17:53 +0100 Subject: [PATCH 3/5] 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 From abf0cc325124d8603896e63f5652a838da102d3e Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Tue, 22 Feb 2022 16:22:14 +0100 Subject: [PATCH 4/5] ipahost: Set allow_empty_string for auth_ind The parameter auth_ind is 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. --- plugins/modules/ipahost.py | 2 +- tests/host/test_host_empty_string_params.yml | 86 ++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tests/host/test_host_empty_string_params.yml diff --git a/plugins/modules/ipahost.py b/plugins/modules/ipahost.py index 453200e6..39857bca 100644 --- a/plugins/modules/ipahost.py +++ b/plugins/modules/ipahost.py @@ -764,7 +764,7 @@ def main(): mac_address = ansible_module.params_get("mac_address") sshpubkey = ansible_module.params_get("sshpubkey") userclass = ansible_module.params_get("userclass") - auth_ind = ansible_module.params_get("auth_ind") + auth_ind = ansible_module.params_get("auth_ind", allow_empty_string=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/tests/host/test_host_empty_string_params.yml b/tests/host/test_host_empty_string_params.yml new file mode 100644 index 00000000..59481d11 --- /dev/null +++ b/tests/host/test_host_empty_string_params.yml @@ -0,0 +1,86 @@ +--- +- name: Test host + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: yes + gather_facts: yes + + tasks: + - name: Get Domain from server name + set_fact: + ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join ('.') }}" + when: ipaserver_domain is not defined + + - name: Set host1_fqdn .. host6_fqdn + set_fact: + host1_fqdn: "{{ 'host1.' + ipaserver_domain }}" + + # CLEANUP TEST ITEMS + + - name: Ensure host "{{ host1_fqdn }}" absent + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ host1_fqdn }}" + state: absent + + # CREATE TEST ITEMS + + - name: Ensure host "{{ host1_fqdn }}" present + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ host1_fqdn }}" + force: yes + register: result + failed_when: not result.changed or result.failed + + # TESTS + + - name: Ensure host "{{ host1_fqdn }}" present with auth_ind otp and radius + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ host1_fqdn }}" + auth_ind: + - otp + - radius + register: result + failed_when: not result.changed or result.failed + + - name: Ensure host "{{ host1_fqdn }}" present with auth_ind otp and radius, again + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ host1_fqdn }}" + auth_ind: + - otp + - radius + register: result + failed_when: result.changed or result.failed + + - name: Ensure host "{{ host1_fqdn }}" present with empty auth_ind + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ host1_fqdn }}" + auth_ind: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure host "{{ host1_fqdn }}" present with empty auth_ind, again + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ host1_fqdn }}" + auth_ind: "" + register: result + failed_when: result.changed or result.failed + + # CLEANUP TEST ITEMS + + - name: Ensure host "{{ host1_fqdn }}" absent + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ host1_fqdn }}" + state: absent From e30bcfd876bec39c8cd2c2fe0806c917eda04cfe Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Wed, 23 Feb 2022 15:17:08 +0100 Subject: [PATCH 5/5] ipaconfig: Set allow_empty_string for user_auth_type, pac_type, configstring The parameters user_auth_type, pac_type and configstring are allowing to use "" to reset to the default value or for configstring to set an empty list. 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. Additionally empty pac_type, user_auth_type and domain_resolution_order have been added to exit_args as if they have not been set. --- plugins/modules/ipaconfig.py | 15 +- .../test_config_empty_string_params.yml | 143 ++++++++++++++++++ 2 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 tests/config/test_config_empty_string_params.yml diff --git a/plugins/modules/ipaconfig.py b/plugins/modules/ipaconfig.py index 2a155bdf..f7901f2c 100644 --- a/plugins/modules/ipaconfig.py +++ b/plugins/modules/ipaconfig.py @@ -346,11 +346,13 @@ def main(): "ca_renewal_master_server": "ca_renewal_master_server", "domain_resolution_order": "ipadomainresolutionorder" } + allow_empty_string = ["pac_type", "user_auth_type", "configstring"] reverse_field_map = {v: k for k, v in field_map.items()} params = {} for x in field_map: - val = ansible_module.params_get(x) + val = ansible_module.params_get( + x, allow_empty_string=(x in allow_empty_string)) if val is not None: params[field_map.get(x, x)] = val @@ -401,6 +403,10 @@ def main(): k: v for k, v in params.items() if k not in result or result[k] != v } + # Remove empty string args from params if result arg is not set + for k in ["ipakrbauthzdata", "ipauserauthtype", "ipaconfigstring"]: + if k not in result and k in params and params[k] == [""]: + del params[k] if params \ and not compare_args_ipa(ansible_module, params, result): changed = True @@ -441,6 +447,13 @@ def main(): raise ValueError( "Unexpected attribute type: %s" % arg_type) exit_args[k] = type_map[arg_type](value) + # Add empty pac_type and user_auth_type if they are not set + for key in ["pac_type", "user_auth_type"]: + if key not in exit_args: + exit_args[key] = "" + # Add empty domain_resolution_order if it is not set + if "domain_resolution_order" not in exit_args: + exit_args["domain_resolution_order"] = [] # Done ansible_module.exit_json(changed=changed, config=exit_args) diff --git a/tests/config/test_config_empty_string_params.yml b/tests/config/test_config_empty_string_params.yml new file mode 100644 index 00000000..5329c203 --- /dev/null +++ b/tests/config/test_config_empty_string_params.yml @@ -0,0 +1,143 @@ +--- +- name: Test config + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: yes + gather_facts: no + + tasks: + + # GET CURRENT CONFIG + + - name: Return current values of the global configuration options + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + register: previousconfig + + - name: Ensure config with empty pac_type, user_auth_type and configstring + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + pac_type: "" + user_auth_type: "" + configstring: "" + + # TESTS + + - name: Ensure config with pac_type "nfs:NONE" and PAD + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + pac_type: + - "nfs:NONE" + - PAD + register: result + failed_when: not result.changed or result.failed + + - name: Ensure config with pac_type "nfs:NONE" and PAD, again + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + pac_type: + - "nfs:NONE" + - PAD + register: result + failed_when: result.changed or result.failed + + - name: Ensure config with empty pac_type + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + pac_type: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure config with empty pac_type, again + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + pac_type: "" + register: result + failed_when: result.changed or result.failed + + - name: Ensure config with user_auth_type otp and radius + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + user_auth_type: + - otp + - radius + register: result + failed_when: not result.changed or result.failed + + - name: Ensure config with user_auth_type otp and radius, again + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + user_auth_type: + - otp + - radius + register: result + failed_when: result.changed or result.failed + + - name: Ensure config with empty user_auth_type + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + user_auth_type: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure config with empty user_auth_type, again + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + user_auth_type: "" + register: result + failed_when: result.changed or result.failed + + - name: Ensure config with configstring AllowNThash and "KDC:Disable Lockout" + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + configstring: + - AllowNThash + - "KDC:Disable Lockout" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure config with configstring AllowNThash and "KDC:Disable Lockout", again + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + configstring: + - AllowNThash + - "KDC:Disable Lockout" + register: result + failed_when: result.changed or result.failed + + - name: Ensure config with empty configstring + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + configstring: "" + register: result + failed_when: not result.changed or result.failed + + - name: Ensure config with empty configstring, again + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + configstring: "" + register: result + failed_when: result.changed or result.failed + + # REVERT TO PREVIOUS CONFIG + + - name: Reset to previous pac_type and user_auth_type + ipaconfig: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + pac_type: '{{ previousconfig.config.pac_type }}' + user_auth_type: '{{ previousconfig.config.user_auth_type }}' + configstring: '{{ previousconfig.config.configstring }}'