From bf0b1ed75f6b56895a6402daabba6fa2a499456b Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 28 May 2020 12:28:38 -0300 Subject: [PATCH 1/8] Fixes no_log warning for `update_password`. This patch explicitly set `no_log` option for `update_password` attribute to `False`, so that the warning on `no_log` not being set is not issued anymore. Ansible incorrectly issued the warning, as `update_password` does not carry sensitive information. --- plugins/modules/ipauser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py index c7ea3f93..b8152ee4 100644 --- a/plugins/modules/ipauser.py +++ b/plugins/modules/ipauser.py @@ -817,7 +817,7 @@ def main(): preserve=dict(required=False, type='bool', default=None), # mod - update_password=dict(type='str', default=None, + update_password=dict(type='str', default=None, no_log=False, choices=['always', 'on_create']), # general From 6b5f03491205c7ea7738f531c4425629d450ffd0 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 1 Jun 2020 10:22:35 -0300 Subject: [PATCH 2/8] Fixes message when variable cannot be used in a given state action. When using a variable that is invalid for a given action, the `action` was not being displayed in the error message, leading to a poor user experience. --- plugins/modules/ipaservice.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index e0da817c..9468e9dd 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -304,8 +304,8 @@ def check_parameters(module, state, action, names, parameters): for _invalid in invalid: if parameters[_invalid] is not None: module.fail_json( - msg="Argument '%s' can not be used with state '%s'" % - (_invalid, state)) + msg="Argument '%s' can not be used with state '%s', " + "action '%s'" % (_invalid, state, action)) def init_ansible_module(): From f44e33c6b31168091e17c33fa379a258acca44e9 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 1 Jun 2020 14:40:53 -0300 Subject: [PATCH 3/8] Allow the use of multiple values with auth_ind variable. This patch changes auth_ind variable to receive a list of values instead of a single one, so that more than one value can be set at once. Tests have been updated to reflect the change. --- plugins/modules/ipaservice.py | 2 +- tests/service/test_service.yml | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 9468e9dd..f5cf5ddc 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -324,7 +324,7 @@ def init_ansible_module(): default=None), pac_type=dict(type="list", aliases=["ipakrbauthzdata"], choices=["MS-PAC", "PAD", "NONE"]), - auth_ind=dict(type="str", + auth_ind=dict(type="list", aliases=["krbprincipalauthind"], choices=["otp", "radius", "pkinit", "hardened"]), skip_host_check=dict(type="bool"), diff --git a/tests/service/test_service.yml b/tests/service/test_service.yml index 10d1285a..3c518055 100644 --- a/tests/service/test_service.yml +++ b/tests/service/test_service.yml @@ -113,7 +113,7 @@ - PAD auth_ind: otp skip_host_check: no - force: no + force: yes requires_pre_auth: yes ok_as_delegate: no ok_to_auth_as_delegate: no @@ -475,6 +475,26 @@ register: result failed_when: result.changed + - name: Ensure service is present, with multiple auth_ind values. + ipaservice: + ipaadmin_password: SomeADMINpassword + name: "HTTP/{{ svc_fqdn }}" + auth_ind: otp,radius + skip_host_check: no + force: yes + register: result + failed_when: not result.changed + + - name: Ensure service is present, with multiple auth_ind values, again. + ipaservice: + ipaadmin_password: SomeADMINpassword + name: "HTTP/{{ svc_fqdn }}" + auth_ind: otp,radius + skip_host_check: no + force: yes + register: result + failed_when: result.changed + # cleanup - name: Ensure services are absent. From bf9024f79fa6a8b63c8b150de07239966cfccef4 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 1 Jun 2020 15:46:07 -0300 Subject: [PATCH 4/8] Fix error message when adding a service without principal. --- plugins/modules/ipaservice.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index f5cf5ddc..34ba7db7 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -217,6 +217,7 @@ from ansible.module_utils.ansible_freeipa_module import temp_kinit, \ temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \ encode_certificate, gen_add_del_lists, module_params_get, to_text, \ api_check_param +import ipalib.errors def find_service(module, name): @@ -224,13 +225,13 @@ def find_service(module, name): "all": True, } - _result = api_command(module, "service_find", to_text(name), _args) + try: + _result = api_command(module, "service_show", to_text(name), _args) + except ipalib.errors.NotFound: + return None - if len(_result["result"]) > 1: - module.fail_json( - msg="There is more than one service '%s'" % (name)) - elif len(_result["result"]) == 1: - _res = _result["result"][0] + if "result" in _result: + _res = _result["result"] certs = _res.get("usercertificate") if certs is not None: _res["usercertificate"] = [encode_certificate(cert) for From cf0b7100470902d055fd2f454b1c811f94565a41 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 1 Jun 2020 16:07:37 -0300 Subject: [PATCH 5/8] Allow clearing auth_ind by using "" as input value. --- plugins/modules/ipaservice.py | 2 +- tests/service/test_service.yml | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 34ba7db7..157aee37 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -327,7 +327,7 @@ def init_ansible_module(): choices=["MS-PAC", "PAD", "NONE"]), auth_ind=dict(type="list", aliases=["krbprincipalauthind"], - choices=["otp", "radius", "pkinit", "hardened"]), + choices=["otp", "radius", "pkinit", "hardened", ""]), skip_host_check=dict(type="bool"), force=dict(type="bool"), requires_pre_auth=dict( diff --git a/tests/service/test_service.yml b/tests/service/test_service.yml index 3c518055..a1216aa8 100644 --- a/tests/service/test_service.yml +++ b/tests/service/test_service.yml @@ -495,6 +495,26 @@ register: result failed_when: result.changed + - name: Clear auth_ind. + ipaservice: + ipaadmin_password: SomeADMINpassword + name: "HTTP/{{ svc_fqdn }}" + auth_ind: "" + skip_host_check: no + force: yes + register: result + failed_when: not result.changed + + - name: Clear auth_ind, again. + ipaservice: + ipaadmin_password: SomeADMINpassword + name: "HTTP/{{ svc_fqdn }}" + auth_ind: "" + skip_host_check: no + force: yes + register: result + failed_when: result.changed + # cleanup - name: Ensure services are absent. From 95d90ef31fa27fd53069b51275f7dec31edd805a Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Mon, 1 Jun 2020 18:20:48 -0300 Subject: [PATCH 6/8] Removed invalid state `enabled` from available choices. --- plugins/modules/ipaservice.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 157aee37..5b668abe 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -142,7 +142,7 @@ options: state: description: State to ensure default: present - choices: ["present", "absent", "enabled", "disabled"] + choices: ["present", "absent", "disabled"] author: - Rafael Jeffman """ @@ -365,8 +365,7 @@ def init_ansible_module(): choices=["member", "service"]), # state state=dict(type="str", default="present", - choices=["present", "absent", - "enabled", "disabled"]), + choices=["present", "absent", "disabled"]), ), supports_check_mode=True, ) From 341078ed5d10b2b5a3ea28c45b2b7d2e0208a388 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 2 Jun 2020 15:13:16 -0300 Subject: [PATCH 7/8] Add support for FreeIPA API service_del `continue` option. --- README-service.md | 1 + plugins/modules/ipaservice.py | 21 ++++++++++++++++++--- tests/service/test_service.yml | 26 ++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/README-service.md b/README-service.md index da0c692c..28e834d2 100644 --- a/README-service.md +++ b/README-service.md @@ -310,6 +310,7 @@ Variable | Description | Required `allow_retrieve_keytab_group` \| `ipaallowedtoperform_read_keys_group` | Groups allowed to retrieve a keytab of this host. | no `allow_retrieve_keytab_host` \| `ipaallowedtoperform_read_keys_host` | Hosts allowed to retrieve a keytab from of host. | no `allow_retrieve_keytab_hostgroup` \| `ipaallowedtoperform_read_keys_hostgroup` | Host groups allowed to retrieve a keytab of this host. | no +`continue` | Continuous mode: don't stop on errors. Valid only if `state` is `absent`. Default: `no` (bool) | no `action` | Work on service or member level. It can be on of `member` or `service` and defaults to `service`. | no `state` | The state to ensure. It can be one of `present`, `absent`, or `disabled`, default: `present`. | no diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 5b668abe..941b9033 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -135,6 +135,12 @@ options: required: false type: list aliases: ["ipaallowedtoperform_read_keys_hostgroup"] + continue: + description: + Continuous mode. Don't stop on errors. Valid only if `state` is `absent`. + required: false + default: True + type: bool action: description: Work on service or member level default: service @@ -284,7 +290,9 @@ def check_parameters(module, state, action, names, parameters): module.fail_json(msg="Only one service can be added at a time.") if action == 'service': - invalid = [] + invalid = ['delete_continue'] + else: + invalid.append('delete_continue') elif state == 'absent': if len(names) < 1: @@ -292,9 +300,12 @@ def check_parameters(module, state, action, names, parameters): if action == "service": invalid.extend(invalid_not_member) + else: + invalid.extend('delete_continue') elif state == 'disabled': invalid.extend(invalid_not_member) + invalid.append('delete_continue') if action != "service": module.fail_json( msg="Invalid action '%s' for state '%s'" % (action, state)) @@ -303,7 +314,7 @@ def check_parameters(module, state, action, names, parameters): module.fail_json(msg="Invalid state '%s'" % (state)) for _invalid in invalid: - if parameters[_invalid] is not None: + if _invalid in parameters and parameters[_invalid] is not None: module.fail_json( msg="Argument '%s' can not be used with state '%s', " "action '%s'" % (_invalid, state, action)) @@ -360,6 +371,8 @@ def init_ansible_module(): allow_retrieve_keytab_hostgroup=dict( type="list", required=False, aliases=['ipaallowedtoperform_read_keys_hostgroup']), + delete_continue=dict(type="bool", required=False, + aliases=['continue']), # action action=dict(type="str", default="service", choices=["member", "service"]), @@ -417,6 +430,7 @@ def main(): ansible_module, "allow_create_keytab_host") allow_retrieve_keytab_hostgroup = module_params_get( ansible_module, "allow_retrieve_keytab_hostgroup") + delete_continue = module_params_get(ansible_module, "delete_continue") # action action = module_params_get(ansible_module, "action") @@ -699,7 +713,8 @@ def main(): elif state == "absent": if action == "service": if res_find is not None: - commands.append([name, 'service_del', {}]) + args = {'continue': True if delete_continue else False} + commands.append([name, 'service_del', args]) elif action == "member": if res_find is None: diff --git a/tests/service/test_service.yml b/tests/service/test_service.yml index a1216aa8..780499e7 100644 --- a/tests/service/test_service.yml +++ b/tests/service/test_service.yml @@ -515,6 +515,32 @@ register: result failed_when: result.changed + - name: Ensure services are absent. + ipaservice: + ipaadmin_password: SomeADMINpassword + name: + - "HTTP/{{ svc_fqdn }}" + - HTTP/www.ansible.com + - HTTP/svc.ihavenodns.info + - HTTP/no.idontexist.local + continue: yes + state: absent + register: result + failed_when: not result.changed + + - name: Ensure services are absent. + ipaservice: + ipaadmin_password: SomeADMINpassword + name: + - "HTTP/{{ svc_fqdn }}" + - HTTP/www.ansible.com + - HTTP/svc.ihavenodns.info + - HTTP/no.idontexist.local + continue: yes + state: absent + register: result + failed_when: result.changed + # cleanup - name: Ensure services are absent. From 5406c60157c00cc6c62dfd53ebb1c942c45c0a3d Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 2 Jun 2020 18:41:43 -0300 Subject: [PATCH 8/8] Add support for service-add-smb. This patch adds variable `smb`, that can be used when adding a new service, and creates a SMB service (cifs) with an optional `netbiosname`. --- plugins/modules/ipaservice.py | 48 +++++++++++++++++++++++++++++++--- tests/service/test_service.yml | 35 +++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py index 941b9033..23a0d6b3 100644 --- a/plugins/modules/ipaservice.py +++ b/plugins/modules/ipaservice.py @@ -90,6 +90,14 @@ options: required: false type: list aliases: ["krbprincipalname"] + smb: + description: Add a SMB service. Can only be used with new services. + required: false + type: bool + netbiosname: + description: NETBIOS name for the SMB service. + required: false + type: str host: description: Host that can manage the service. required: false @@ -226,11 +234,21 @@ from ansible.module_utils.ansible_freeipa_module import temp_kinit, \ import ipalib.errors -def find_service(module, name): +def find_service(module, name, netbiosname): _args = { "all": True, } + # Search for a SMB/cifs service. + if netbiosname is not None: + _result = api_command( + module, "service_find", to_text(netbiosname), _args) + + for _res_find in _result.get('result', []): + for uid in _res_find.get('uid', []): + if uid.startswith("%s$@" % netbiosname): + return _res_find + try: _result = api_command(module, "service_show", to_text(name), _args) except ipalib.errors.NotFound: @@ -275,7 +293,7 @@ def check_parameters(module, state, action, names, parameters): # invalid parameters for everything but state 'present', action 'service'. invalid = ['pac_type', 'auth_ind', 'skip_host_check', 'force', 'requires_pre_auth', 'ok_as_delegate', - 'ok_to_auth_as_delegate'] + 'ok_to_auth_as_delegate', 'smb', 'netbiosname'] # invalid parameters when not handling service members. invalid_not_member = \ @@ -291,6 +309,16 @@ def check_parameters(module, state, action, names, parameters): if action == 'service': invalid = ['delete_continue'] + + if parameters.get('smb', False): + invalid.extend(['force', 'auth_ind', 'skip_host_check', + 'requires_pre_auth', 'auth_ind', 'pac_type']) + + for _invalid in invalid: + if parameters.get(_invalid, False): + module.fail_json( + msg="Argument '%s' can not be used with SMB " + "service." % _invalid) else: invalid.append('delete_continue') @@ -334,6 +362,8 @@ def init_ansible_module(): default=None, required=False), principal=dict(type="list", aliases=["krbprincipalname"], default=None), + smb=dict(type="bool", required=False), + netbiosname=dict(type="str", required=False), pac_type=dict(type="list", aliases=["ipakrbauthzdata"], choices=["MS-PAC", "PAD", "NONE"]), auth_ind=dict(type="list", @@ -411,6 +441,9 @@ def main(): ok_to_auth_as_delegate = module_params_get(ansible_module, "ok_to_auth_as_delegate") + smb = module_params_get(ansible_module, "smb") + netbiosname = module_params_get(ansible_module, "netbiosname") + host = module_params_get(ansible_module, "host") allow_create_keytab_user = module_params_get( @@ -461,9 +494,11 @@ def main(): commands = [] for name in names: - res_find = find_service(ansible_module, name) + res_find = find_service(ansible_module, name, netbiosname) if state == "present": + # if service exists, 'smb' cannot be used. + if action == "service": args = gen_args( pac_type, auth_ind, skip_host_check, force, @@ -473,7 +508,12 @@ def main(): del args['skip_host_check'] if res_find is None: - commands.append([name, 'service_add', args]) + if smb: + if netbiosname is not None: + args['ipantflatname'] = netbiosname + commands.append([name, 'service_add_smb', args]) + else: + commands.append([name, 'service_add', args]) certificate_add = certificate or [] certificate_del = [] diff --git a/tests/service/test_service.yml b/tests/service/test_service.yml index 780499e7..25d66c69 100644 --- a/tests/service/test_service.yml +++ b/tests/service/test_service.yml @@ -541,6 +541,40 @@ register: result failed_when: result.changed + - name: Ensure SMB service is present. + ipaservice: + ipaadmin_password: MyPassword123 + name: "{{ host1_fqdn }}" + smb: yes + netbiosname: SAMBASVC + register: result + failed_when: not result.changed + + - name: Ensure SMB service is again. + ipaservice: + ipaadmin_password: MyPassword123 + name: "{{ host1_fqdn }}" + smb: yes + netbiosname: SAMBASVC + register: result + failed_when: result.changed + + - name: Ensure SMB service is absent. + ipaservice: + ipaadmin_password: MyPassword123 + name: "cifs/{{ host1_fqdn }}" + state: absent + register: result + failed_when: not result.changed + + - name: Ensure SMB service is absent, again. + ipaservice: + ipaadmin_password: MyPassword123 + name: "cifs/{{ host1_fqdn }}" + state: absent + register: result + failed_when: result.changed + # cleanup - name: Ensure services are absent. @@ -551,6 +585,7 @@ - HTTP/www.ansible.com - HTTP/svc.ihavenodns.info - HTTP/no.idontexist.local + - "cifs/{{ host1_fqdn }}" state: absent - name: Ensure host "{{ svc_fqdn }}" is absent