diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index b4cdf613f9fdd458261460d4d39d153a83ed64c1..b9e32369b594c9d32bb1d5dccc453cec495abbfc 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -875,10 +875,11 @@ else: """ return api_command_no_name(self, command, args) - @staticmethod - def ipa_get_domain(): + def ipa_get_domain(self): """Retrieve IPA API domain.""" - return api_get_domain() + if not hasattr(self, "__ipa_api_domain"): + setattr(self, "__ipa_api_domain", api_get_domain()) + return getattr(self, "__ipa_api_domain") @staticmethod def ipa_get_realm(): diff --git a/plugins/modules/iparole.py b/plugins/modules/iparole.py index 75500d3cc87b88faefb7efab6957495e2a039057..61e4d3d4ed3aecfc7920f04c51d389cc2b9746b6 100644 --- a/plugins/modules/iparole.py +++ b/plugins/modules/iparole.py @@ -103,10 +103,10 @@ EXAMPLES = """ # pylint: disable=no-name-in-module from ansible.module_utils._text import to_text from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, gen_add_del_lists, compare_args_ipa + IPAAnsibleModule, gen_add_del_lists, compare_args_ipa, \ + gen_intersection_list, ensure_fqdn from ansible.module_utils import six - if six.PY3: unicode = str @@ -170,28 +170,15 @@ def check_parameters(module): module.params_fail_used_invalid(invalid, state, action) -def member_intersect(module, attr, memberof, res_find): - """Filter member arguments from role found by intersection.""" - params = module.params_get(attr) - if not res_find: - return params - filtered = [] - if params: - existing = res_find.get(memberof, []) - filtered = list(set(params) & set(existing)) - return filtered - - -def member_difference(module, attr, memberof, res_find): - """Filter member arguments from role found by difference.""" - params = module.params_get(attr) - if not res_find: - return params - filtered = [] - if params: - existing = res_find.get(memberof, []) - filtered = list(set(params) - set(existing)) - return filtered +def get_member_host_with_fqdn_lowercase(module, mod_member): + """Retrieve host members from module, as FQDN, lowercase.""" + default_domain = module.ipa_get_domain() + hosts = module.params_get(mod_member) + return ( + [ensure_fqdn(host, default_domain).lower() for host in hosts] + if hosts + else hosts + ) def ensure_absent_state(module, name, action, res_find): @@ -203,23 +190,41 @@ def ensure_absent_state(module, name, action, res_find): if action == "member": - members = member_intersect( - module, 'privilege', 'memberof_privilege', res_find) - if members: - commands.append([name, "role_remove_privilege", - {"privilege": members}]) + _members = module.params_get_lowercase("privilege") + if _members is not None: + del_list = gen_intersection_list( + _members, + result_get_value_lowercase(res_find, "memberof_privilege") + ) + if del_list: + commands.append([name, "role_remove_privilege", + {"privilege": del_list}]) member_args = {} - for key in ['user', 'group', 'host', 'hostgroup']: - items = member_intersect( - module, key, 'member_%s' % key, res_find) - if items: - member_args[key] = items - - _services = filter_service(module, res_find, - lambda res, svc: res.startswith(svc)) + for key in ['user', 'group', 'hostgroup']: + _members = module.params_get_lowercase(key) + if _members: + del_list = gen_intersection_list( + _members, + result_get_value_lowercase(res_find, "member_%s" % key) + ) + if del_list: + member_args[key] = del_list + + # ensure hosts are FQDN. + _members = get_member_host_with_fqdn_lowercase(module, "host") + if _members: + del_list = gen_intersection_list( + _members, res_find.get('member_host')) + if del_list: + member_args["host"] = del_list + + _services = get_service_param(module, "service") if _services: - member_args['service'] = _services + _existing = result_get_value_lowercase(res_find, "member_service") + items = gen_intersection_list(_services.keys(), _existing) + if items: + member_args["service"] = [_services[key] for key in items] # Only add remove command if there's at least one member no manage. if member_args: @@ -228,66 +233,112 @@ def ensure_absent_state(module, name, action, res_find): return commands -def filter_service(module, res_find, predicate): +def get_service_param(module, key): """ - Filter service based on predicate. + Retrieve dict of services, with realm, from the module parameters. - Compare service name with existing ones matching - at least until `@` from principal name. - - Predicate is a callable that accepts the existing service, and the - modified service to be compared to. + As the services are compared in a case insensitive manner, but + are recorded in a case preserving way, a dict mapping the services + in lowercase to the provided module parameter is generated, so + that dict keys can be used for comparison and the values are used + with IPA API. """ - _services = [] - service = module.params_get('service') - if service: - existing = [to_text(x) for x in res_find.get('member_service', [])] - for svc in service: - svc = svc if '@' in svc else ('%s@' % svc) - found = [x for x in existing if predicate(x, svc)] - _services.extend(found) + _services = module.params_get(key) + if _services is not None: + ipa_realm = module.ipa_get_realm() + _services = [ + to_text(svc) if '@' in svc else ('%s@%s' % (svc, ipa_realm)) + for svc in _services + ] + if _services: + _services = {svc.lower(): svc for svc in _services} return _services +def result_get_value_lowercase(res_find, key, default=None): + """ + Retrieve a member of a dictionary converted to lowercase. + + If field data is a string it is returned in lowercase. If + field data is a list or tuple, it is assumed that all values + are strings and the result is a list of strings in lowercase. + + If 'key' is not found in the dictionary, returns 'default'. + """ + existing = res_find.get(key) + if existing is not None: + if isinstance(existing, (list, tuple)): + existing = [to_text(item).lower() for item in existing] + if isinstance(existing, (str, unicode)): + existing = existing.lower() + else: + existing = default + return existing + + +def gen_services_add_del_lists(module, mod_member, res_find, res_member): + """Generate add/del lists for service principals.""" + add_list, del_list = None, None + _services = get_service_param(module, mod_member) + if _services is not None: + _existing = result_get_value_lowercase(res_find, res_member) + add_list, del_list = gen_add_del_lists(_services.keys(), _existing) + if add_list: + add_list = [_services[key] for key in add_list] + if del_list: + del_list = [to_text(item) for item in del_list] + return add_list, del_list + + def ensure_role_with_members_is_present(module, name, res_find, action): """Define commands to ensure member are present for action `role`.""" commands = [] - privilege_add, privilege_del = gen_add_del_lists( - module.params_get("privilege"), - res_find.get('memberof_privilege', [])) - if privilege_add: - commands.append([name, "role_add_privilege", - {"privilege": privilege_add}]) - if action == "role" and privilege_del: - commands.append([name, "role_remove_privilege", - {"privilege": privilege_del}]) + _members = module.params_get_lowercase("privilege") + if _members: + add_list, del_list = gen_add_del_lists( + _members, + result_get_value_lowercase(res_find, "memberof_privilege") + ) + + if add_list: + commands.append([name, "role_add_privilege", + {"privilege": add_list}]) + if action == "role" and del_list: + commands.append([name, "role_remove_privilege", + {"privilege": del_list}]) add_members = {} del_members = {} - for key in ["user", "group", "host", "hostgroup"]: + for key in ["user", "group", "hostgroup"]: + _members = module.params_get_lowercase(key) + if _members is not None: + add_list, del_list = gen_add_del_lists( + _members, + result_get_value_lowercase(res_find, "member_%s" % key) + ) + if add_list: + add_members[key] = add_list + if del_list: + del_members[key] = del_list + + # ensure hosts are FQDN. + _members = get_member_host_with_fqdn_lowercase(module, "host") + if _members: add_list, del_list = gen_add_del_lists( - module.params_get(key), - res_find.get('member_%s' % key, []) - ) + _members, res_find.get('member_host')) if add_list: - add_members[key] = add_list + add_members["host"] = add_list if del_list: - del_members[key] = [to_text(item) for item in del_list] - - service = [ - to_text(svc) - if '@' in svc - else ('%s@%s' % (svc, module.ipa_get_realm())) - for svc in (module.params_get('service') or []) - ] - existing = [str(svc) for svc in res_find.get('member_service', [])] - add_list, del_list = gen_add_del_lists(service, existing) - if add_list: - add_members['service'] = add_list - if del_list: - del_members['service'] = [to_text(item) for item in del_list] + del_members["host"] = del_list + + (add_services, del_services) = gen_services_add_del_lists( + module, "service", res_find, "member_service") + if add_services: + add_members["service"] = add_services + if del_services: + del_members["service"] = del_services if add_members: commands.append([name, "role_add_member", add_members]) @@ -298,52 +349,6 @@ def ensure_role_with_members_is_present(module, name, res_find, action): return commands -def ensure_members_are_present(module, name, res_find): - """Define commands to ensure members are present for action `member`.""" - commands = [] - - members = member_difference( - module, 'privilege', 'memberof_privilege', res_find) - if members: - commands.append([name, "role_add_privilege", - {"privilege": members}]) - - member_args = {} - for key in ['user', 'group', 'host', 'hostgroup']: - items = member_difference( - module, key, 'member_%s' % key, res_find) - if items: - member_args[key] = items - - _services = filter_service(module, res_find, - lambda res, svc: not res.startswith(svc)) - if _services: - member_args['service'] = _services - - if member_args: - commands.append([name, "role_add_member", member_args]) - - return commands - - -# pylint: disable=unused-argument -def result_handler(module, result, command, name, args, errors): - """Process the result of a command, looking for errors.""" - # Get all errors - # All "already a member" and "not a member" failures in the - # result are ignored. All others are reported. - if "failed" in result and len(result["failed"]) > 0: - for item in result["failed"]: - failed_item = result["failed"][item] - for member_type in failed_item: - for member, failure in failed_item[member_type]: - if "already a member" in failure \ - or "not a member" in failure: - continue - errors.append("%s: %s %s: %s" % ( - command, member_type, member, failure)) - - def role_commands_for_name(module, state, action, name): """Define commands for the Role module.""" commands = [] @@ -442,7 +447,8 @@ def main(): # Execute commands - changed = ansible_module.execute_ipa_commands(commands, result_handler) + changed = ansible_module.execute_ipa_commands( + commands, fail_on_member_errors=True) # Done ansible_module.exit_json(changed=changed, **exit_args) diff --git a/tests/role/test_role_member_case_insensitive.yml b/tests/role/test_role_member_case_insensitive.yml new file mode 100644 index 0000000000000000000000000000000000000000..3f386fc3eaaf1b44a6b4fe03475f3543dbd63a08 --- /dev/null +++ b/tests/role/test_role_member_case_insensitive.yml @@ -0,0 +1,450 @@ +--- +- name: Test role members + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: no + gather_facts: no + + vars: + user_list: + - User1 + - uSer2 + - usEr3 + group_list: + - Group1 + - gRoup2 + - grOup3 + host_list: + - HoSt01 + - hOsT02 + hostgroup_list: + - TestHostGroup + service_list: + - MySVC/host01 + + tasks: + - include_tasks: ../env_freeipa_facts.yml + + - block: + # setup + + - name: Ensure test role is absent + iparole: + ipaadmin_password: SomeADMINpassword + name: testrule + state: absent + + - name: Ensure test users are present + ipauser: + ipaadmin_password: SomeADMINpassword + users: + - name: "{{ item }}" + first: First + last: Last + with_items: "{{ user_list }}" + + - name: Ensure test groups are present + ipagroup: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}" + with_items: "{{ group_list }}" + + - name: Ensure test hosts are present + ipahost: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}.{{ ipaserver_domain }}" + ip_address: 192.168.122.101 + force: yes + with_items: "{{ host_list }}" + + - name: Ensure test hostgroups are present + ipahostgroup: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}" + with_items: "{{ hostgroup_list }}" + + - name: Ensure test services are present + ipaservice: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}.{{ ipaserver_domain }}" + with_items: "{{ service_list }}" + + # Test with action: hbacrule + + - name: Check role present with members would trigger a change, mixed case + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] }}" + - "{{ user_list[2] }}" + group: + - "{{ group_list[1] }}" + - "{{ group_list[2] }}" + host: + - "{{ host_list[0] }}" + - "{{ host_list[1] }}" + hostgroup: + - "{{ hostgroup_list[0] }}" + service: + - "{{ service_list[0] }}.{{ ipaserver_domain }}" + check_mode: yes + register: result + failed_when: not result.changed or result.failed + + - name: Ensure role is present with members, mixed case + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] }}" + - "{{ user_list[2] }}" + group: + - "{{ group_list[1] }}" + - "{{ group_list[2] }}" + host: + - "{{ host_list[0] }}" + - "{{ host_list[1] }}" + hostgroup: + - "{{ hostgroup_list[0] }}" + service: + - "{{ service_list[0] }}.{{ ipaserver_domain }}" + register: result + failed_when: not result.changed or result.failed + + - name: Check role present with members would not trigger a change, mixed case + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] }}" + - "{{ user_list[2] }}" + group: + - "{{ group_list[1] }}" + - "{{ group_list[2] }}" + host: + - "{{ host_list[0] }}" + - "{{ host_list[1] }}" + hostgroup: + - "{{ hostgroup_list[0] }}" + service: + - "{{ service_list[0] }}.{{ ipaserver_domain }}" + check_mode: yes + register: result + failed_when: result.changed or result.failed + + - name: Ensure role is present with members, lowercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | lower }}" + - "{{ user_list[2] | lower }}" + group: + - "{{ group_list[1] | lower }}" + - "{{ group_list[2] | lower }}" + host: + - "{{ host_list[0] | lower }}" + - "{{ host_list[1] | lower }}" + hostgroup: + - "{{ hostgroup_list[0] | lower }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | lower }}" + register: result + failed_when: result.changed or result.failed + + - name: Ensure role is present with members, upercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | upper }}" + - "{{ user_list[2] | upper }}" + group: + - "{{ group_list[1] | upper }}" + - "{{ group_list[2] | upper }}" + host: + - "{{ host_list[0] | upper }}" + - "{{ host_list[1] | upper }}" + hostgroup: + - "{{ hostgroup_list[0] | upper }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}" + register: result + failed_when: result.changed or result.failed + + - name: Ensure test role is absent + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + state: absent + + # Test with action: members + + - name: Ensure test role is present + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + + - name: Check role members present would trigger a change, mixed case + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] }}" + - "{{ user_list[2] }}" + group: + - "{{ group_list[1] }}" + - "{{ group_list[2] }}" + host: + - "{{ host_list[0] }}" + - "{{ host_list[1] }}" + hostgroup: + - "{{ hostgroup_list[0] }}" + service: + - "{{ service_list[0] }}.{{ ipaserver_domain }}" + action: member + check_mode: yes + register: result + failed_when: not result.changed or result.failed + + - name: Ensure role is present with members, mixed case + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] }}" + - "{{ user_list[2] }}" + group: + - "{{ group_list[1] }}" + - "{{ group_list[2] }}" + host: + - "{{ host_list[0] }}" + - "{{ host_list[1] }}" + hostgroup: + - "{{ hostgroup_list[0] }}" + service: + - "{{ service_list[0] }}.{{ ipaserver_domain }}" + action: member + register: result + failed_when: not result.changed or result.failed + + - name: Check role members present would not trigger a change, mixed case + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] }}" + - "{{ user_list[2] }}" + group: + - "{{ group_list[1] }}" + - "{{ group_list[2] }}" + host: + - "{{ host_list[0] }}" + - "{{ host_list[1] }}" + hostgroup: + - "{{ hostgroup_list[0] }}" + service: + - "{{ service_list[0] }}.{{ ipaserver_domain }}" + action: member + check_mode: yes + register: result + failed_when: result.changed or result.failed + + - name: Ensure role is present with members, lowercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | lower }}" + - "{{ user_list[2] | lower }}" + group: + - "{{ group_list[1] | lower }}" + - "{{ group_list[2] | lower }}" + host: + - "{{ host_list[0] | lower }}" + - "{{ host_list[1] | lower }}" + hostgroup: + - "{{ hostgroup_list[0] | lower }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | lower }}" + action: member + register: result + failed_when: result.changed or result.failed + + - name: Ensure role is present with members, upercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | upper }}" + - "{{ user_list[2] | upper }}" + group: + - "{{ group_list[1] | upper }}" + - "{{ group_list[2] | upper }}" + host: + - "{{ host_list[0] | upper }}" + - "{{ host_list[1] | upper }}" + hostgroup: + - "{{ hostgroup_list[0] | upper }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}" + action: member + register: result + failed_when: result.changed or result.failed + + # # Test absent members + - name: Check role members absent would trigger a change, upercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | upper }}" + - "{{ user_list[2] | upper }}" + group: + - "{{ group_list[1] | upper }}" + - "{{ group_list[2] | upper }}" + host: + - "{{ host_list[0] | upper }}" + - "{{ host_list[1] | upper }}" + hostgroup: + - "{{ hostgroup_list[0] | upper }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}" + action: member + state: absent + check_mode: yes + register: result + failed_when: not result.changed or result.failed + + - name: Ensure role members are absent, upercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | upper }}" + - "{{ user_list[2] | upper }}" + group: + - "{{ group_list[1] | upper }}" + - "{{ group_list[2] | upper }}" + host: + - "{{ host_list[0] | upper }}" + - "{{ host_list[1] | upper }}" + hostgroup: + - "{{ hostgroup_list[0] | upper }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}" + action: member + state: absent + register: result + failed_when: not result.changed or result.failed + + - name: Check role members absent would not trigger a change, upercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | upper }}" + - "{{ user_list[2] | upper }}" + group: + - "{{ group_list[1] | upper }}" + - "{{ group_list[2] | upper }}" + host: + - "{{ host_list[0] | upper }}" + - "{{ host_list[1] | upper }}" + hostgroup: + - "{{ hostgroup_list[0] | upper }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | upper }}" + action: member + state: absent + check_mode: yes + register: result + failed_when: result.changed or result.failed + + - name: Ensure role members are absent, mixed case + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] }}" + - "{{ user_list[2] }}" + group: + - "{{ group_list[1] }}" + - "{{ group_list[2] }}" + host: + - "{{ host_list[0] }}" + - "{{ host_list[1] }}" + hostgroup: + - "{{ hostgroup_list[0] }}" + service: + - "{{ service_list[0] }}.{{ ipaserver_domain }}" + action: member + state: absent + register: result + failed_when: result.changed or result.failed + + - name: Ensure role members are absent, lowercase + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + user: + - "{{ user_list[1] | lower }}" + - "{{ user_list[2] | lower }}" + group: + - "{{ group_list[1] | lower }}" + - "{{ group_list[2] | lower }}" + host: + - "{{ host_list[0] | lower }}" + - "{{ host_list[1] | lower }}" + hostgroup: + - "{{ hostgroup_list[0] | lower }}" + service: + - "{{ (service_list[0] + '.' + ipaserver_domain) | lower }}" + action: member + state: absent + register: result + failed_when: result.changed or result.failed + + always: + - name: Ensure test role is absent + iparole: + ipaadmin_password: SomeADMINpassword + name: testrole + state: absent + + - name: Ensure test users are absent + ipauser: + ipaadmin_password: SomeADMINpassword + users: + - name: "{{ item }}" + state: absent + with_items: "{{ user_list }}" + + - name: Ensure test groups are absent + ipagroup: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}" + state: absent + with_items: "{{ group_list }}" + + - name: Ensure test hosts are absent + ipahost: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}.{{ ipaserver_domain }}" + state: absent + with_items: "{{ host_list }}" + + - name: Ensure test hostgroups are absent + ipahostgroup: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}" + state: absent + with_items: "{{ hostgroup_list }}" + + - name: Ensure test services are absent + ipaservice: + ipaadmin_password: SomeADMINpassword + name: "{{ item }}.{{ ipaserver_domain }}" + continue: yes + state: absent + with_items: "{{ service_list }}" diff --git a/tests/sanity/ignore-2.12.txt b/tests/sanity/ignore-2.12.txt index 33b1770e1fafd5df2926b867d9d8e4b63fea6a80..b08a6ffa83adedcaf6155f07f8cc19105c2b32c9 100644 --- a/tests/sanity/ignore-2.12.txt +++ b/tests/sanity/ignore-2.12.txt @@ -18,6 +18,8 @@ plugins/modules/ipadnsrecord.py pylint:use-maxsplit-arg plugins/modules/ipareplica_enable_ipa.py pylint:ansible-format-automatic-specification plugins/modules/ipareplica_prepare.py pylint:ansible-format-automatic-specification plugins/modules/ipareplica_test.py pylint:ansible-format-automatic-specification +plugins/modules/iparole.py compile-2.6!skip +plugins/modules/iparole.py import-2.6!skip plugins/modules/ipaserver_setup_ca.py compile-2.6!skip plugins/modules/ipaserver_setup_ca.py import-2.6!skip plugins/modules/ipaserver_test.py pylint:ansible-format-automatic-specification