From a649a8dfe18f45f9545e3a511361b8b1b513cb55 Mon Sep 17 00:00:00 2001 From: Denis Karpelevich Date: Thu, 30 Mar 2023 21:07:52 +0200 Subject: [PATCH 1/2] [RFE] Allow multiple groups creation. Adding an option `groups` to create multiple groups in one operation. Adding tests (present/absent/external/nonposix) with server and client context. Simple example of `groups` option: ``` tasks: - name: Ensure 2 groups are present ipagroup: ipaadmin_password: SomeADMINpassword groups: - name: group1 - name: group2 ``` Signed-off-by: Denis Karpelevich --- README-group.md | 105 ++++- playbooks/{user => group}/add-group.yml | 0 .../{user => group}/add-groups-to-group.yml | 0 playbooks/group/add-groups.yml | 32 ++ .../{user => group}/add-user-to-group.yml | 0 playbooks/{user => group}/delete-group.yml | 0 plugins/modules/ipagroup.py | 286 ++++++++++--- tests/group/create_groups_json.yml | 13 + tests/group/groups.sh | 25 ++ tests/group/test_group_client_context.yml | 24 ++ tests/group/test_groups.yml | 143 +++++++ tests/group/test_groups_absent.yml | 35 ++ tests/group/test_groups_external_nonposix.yml | 395 ++++++++++++++++++ tests/group/test_groups_present.yml | 40 ++ tests/group/test_groups_present_slice.yml | 47 +++ tests/sanity/ignore-2.12.txt | 1 + tests/sanity/ignore-2.13.txt | 1 + tests/sanity/ignore-2.14.txt | 1 + 18 files changed, 1095 insertions(+), 53 deletions(-) rename playbooks/{user => group}/add-group.yml (100%) rename playbooks/{user => group}/add-groups-to-group.yml (100%) create mode 100644 playbooks/group/add-groups.yml rename playbooks/{user => group}/add-user-to-group.yml (100%) rename playbooks/{user => group}/delete-group.yml (100%) create mode 100644 tests/group/create_groups_json.yml create mode 100644 tests/group/groups.sh create mode 100644 tests/group/test_groups.yml create mode 100644 tests/group/test_groups_absent.yml create mode 100644 tests/group/test_groups_external_nonposix.yml create mode 100644 tests/group/test_groups_present.yml create mode 100644 tests/group/test_groups_present_slice.yml diff --git a/README-group.md b/README-group.md index 021fc105..181e407c 100644 --- a/README-group.md +++ b/README-group.md @@ -8,6 +8,9 @@ The group module allows to ensure presence and absence of groups and members of The group module is as compatible as possible to the Ansible upstream `ipa_group` module, but additionally offers to add users to a group and also to remove users from a group. +## Note +Ensuring presence (adding) of several groups with mixed types (`external`, `nonposix` and `posix`) requires a fix in FreeIPA. The module implements a workaround to automatically use `client` context if the fix is not present in the target node FreeIPA and if more than one group is provided to the task using the `groups` parameter. If `ipaapi_context` is forced to be `server`, the module will fail in this case. + Features -------- @@ -71,6 +74,62 @@ Example playbook to add groups: name: appops ``` +These three `ipagroup` module calls can be combined into one with the `groups` variable: + +```yaml +--- +- name: Playbook to handle groups + hosts: ipaserver + + tasks: + - name: Ensure groups ops, sysops and appops are present + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: ops + gidnumber: 1234 + - name: sysops + user: + - pinky + - name: appops +``` + +You can also alternatively use a json file containing the groups, here `groups_present.json`: + +```json +{ + "groups": [ + { + "name": "group1", + "description": "description group1" + }, + { + "name": "group2", + "description": "description group2" + } + ] +} +``` + +And ensure the presence of the groups with this example playbook: + +```yaml +--- +- name: Tests + hosts: ipaserver + gather_facts: false + + tasks: + - name: Include groups_present.json + include_vars: + file: groups_present.json + + - name: Groups present + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: "{{ groups }}" +``` + Example playbook to add users to a group: ```yaml @@ -112,11 +171,11 @@ Example playbook to add group members to a group: Example playbook to add members from a trusted realm to an external group: ```yaml --- +--- - name: Playbook to handle groups. hosts: ipaserver - became: true - + + tasks: - name: Create an external group and add members from a trust to it. ipagroup: ipaadmin_password: SomeADMINpassword @@ -127,6 +186,24 @@ Example playbook to add members from a trusted realm to an external group: - WINIPA\\Developers ``` +Example playbook to add nonposix and external groups: + +```yaml +--- +- name: Playbook to add nonposix and external groups + hosts: ipaserver + + tasks: + - name: Add nonposix group sysops and external group appops + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: sysops + nonposix: true + - name: appops + external: true +``` + Example playbook to remove groups: ```yaml @@ -136,13 +213,29 @@ Example playbook to remove groups: become: true tasks: - # Remove goups sysops, appops and ops + # Remove groups sysops, appops and ops - ipagroup: ipaadmin_password: SomeADMINpassword name: sysops,appops,ops state: absent ``` +Example playbook to ensure groups are absent: + +```yaml +--- +- name: Playbook to handle groups + hosts: ipaserver + + tasks: + - name: Ensure groups ops and sysops are absent + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: ops + - name: sysops + state: absent +``` Variables ========= @@ -152,8 +245,10 @@ Variable | Description | Required `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no `ipaadmin_password` | The admin password is a string and is required if there is no admin ticket available on the node | no `ipaapi_context` | The context in which the module will execute. Executing in a server context is preferred. If not provided context will be determined by the execution environment. Valid values are `server` and `client`. | no -`ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to yes. (bool) | no +`ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to
. (bool) | no `name` \| `cn` | The list of group name strings. | no +`groups` | The list of group dicts. Each `groups` dict entry can contain group variables.
There is one required option in the `groups` dict:| no +  | `name` - The group name string of the entry. | yes `description` | The group description string. | no `gid` \| `gidnumber` | The GID integer. | no `posix` | Create a non-POSIX group or change a non-POSIX to a posix group. `nonposix`, `posix` and `external` are mutually exclusive. (bool) | no diff --git a/playbooks/user/add-group.yml b/playbooks/group/add-group.yml similarity index 100% rename from playbooks/user/add-group.yml rename to playbooks/group/add-group.yml diff --git a/playbooks/user/add-groups-to-group.yml b/playbooks/group/add-groups-to-group.yml similarity index 100% rename from playbooks/user/add-groups-to-group.yml rename to playbooks/group/add-groups-to-group.yml diff --git a/playbooks/group/add-groups.yml b/playbooks/group/add-groups.yml new file mode 100644 index 00000000..3b1802ff --- /dev/null +++ b/playbooks/group/add-groups.yml @@ -0,0 +1,32 @@ +--- +- name: Playbook to handle multiple groups + hosts: ipaserver + + tasks: + - name: Create multiple groups ops, sysops + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: ops + gidnumber: 1234 + - name: sysops + + - name: Add user and group members to groups sysops and appops + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: sysops + user: + - user1 + - name: appops + group: + - group2 + + - name: Create multiple non-POSIX and external groups + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nongroup + nonposix: true + - name: extgroup + external: true diff --git a/playbooks/user/add-user-to-group.yml b/playbooks/group/add-user-to-group.yml similarity index 100% rename from playbooks/user/add-user-to-group.yml rename to playbooks/group/add-user-to-group.yml diff --git a/playbooks/user/delete-group.yml b/playbooks/group/delete-group.yml similarity index 100% rename from playbooks/user/delete-group.yml rename to playbooks/group/delete-group.yml diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py index 2488f17f..e0d5bb12 100644 --- a/plugins/modules/ipagroup.py +++ b/plugins/modules/ipagroup.py @@ -41,8 +41,88 @@ options: description: The group name type: list elements: str - required: true + required: false aliases: ["cn"] + groups: + description: The list of group dicts (internally gid). + type: list + elements: dict + suboptions: + name: + description: The group (internally gid). + type: str + required: true + aliases: ["cn"] + description: + description: The group description + type: str + required: false + gid: + description: The GID + type: int + required: false + aliases: ["gidnumber"] + nonposix: + description: Create as a non-POSIX group + required: false + type: bool + external: + description: Allow adding external non-IPA members from trusted domains + required: false + type: bool + posix: + description: + Create a non-POSIX group or change a non-POSIX to a posix group. + required: false + type: bool + nomembers: + description: Suppress processing of membership attributes + required: false + type: bool + user: + description: List of user names assigned to this group. + required: false + type: list + elements: str + group: + description: List of group names assigned to this group. + required: false + type: list + elements: str + service: + description: + - List of service names assigned to this group. + - Only usable with IPA versions 4.7 and up. + required: false + type: list + elements: str + membermanager_user: + description: + - List of member manager users assigned to this group. + - Only usable with IPA versions 4.8.4 and up. + required: false + type: list + elements: str + membermanager_group: + description: + - List of member manager groups assigned to this group. + - Only usable with IPA versions 4.8.4 and up. + required: false + type: list + elements: str + externalmember: + description: + - List of members of a trusted domain in DOM\\name or name@domain form. + required: false + type: list + elements: str + aliases: ["ipaexternalmember", "external_member"] + idoverrideuser: + description: + - User ID overrides to add + required: false + type: list + elements: str description: description: The group description type: str @@ -144,6 +224,14 @@ EXAMPLES = """ ipaadmin_password: SomeADMINpassword name: appops +# Create multiple groups ops, sysops +- ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: ops + gidnumber: 1234 + - name: sysops + # Add user member pinky to group sysops - ipagroup: ipaadmin_password: SomeADMINpassword @@ -160,7 +248,7 @@ EXAMPLES = """ user: - brain -# Add group members sysops and appops to group sysops +# Add group members sysops and appops to group ops - ipagroup: ipaadmin_password: SomeADMINpassword name: ops @@ -168,6 +256,17 @@ EXAMPLES = """ - sysops - appops +# Add user and group members to groups sysops and appops +- ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: sysops + user: + - user1 + - name: appops + group: + - group2 + # Create a non-POSIX group - ipagroup: ipaadmin_password: SomeADMINpassword @@ -189,7 +288,16 @@ EXAMPLES = """ - WINIPA\\Web Users - WINIPA\\Developers -# Remove goups sysops, appops, ops and nongroup +# Create multiple non-POSIX and external groups +- ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nongroup + nonposix: true + - name: extgroup + external: true + +# Remove groups sysops, appops, ops and nongroup - ipagroup: ipaadmin_password: SomeADMINpassword name: sysops,appops,ops, nongroup @@ -203,6 +311,9 @@ from ansible.module_utils._text import to_text from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, \ gen_add_list, gen_intersection_list, api_check_param +from ansible.module_utils import six +if six.PY3: + unicode = str def find_group(module, name): @@ -257,6 +368,22 @@ def gen_member_args(user, group, service, externalmember, idoverrideuser): return _args +def check_parameters(module, state, action): + invalid = [] + if state == "present": + if action == "member": + invalid = ["description", "gid", "posix", "nonposix", "external", + "nomembers"] + + else: + invalid = ["description", "gid", "posix", "nonposix", "external", + "nomembers"] + if action == "group": + invalid.extend(["user", "group", "service", "externalmember"]) + + module.params_fail_used_invalid(invalid, state, action) + + def is_external_group(res_find): """Verify if the result group is an external group.""" return res_find and 'ipaexternalgroup' in res_find['objectclass'] @@ -285,45 +412,63 @@ def check_objectclass_args(module, res_find, posix, external): def main(): + group_spec = dict( + # present + description=dict(type="str", default=None), + gid=dict(type="int", aliases=["gidnumber"], default=None), + nonposix=dict(required=False, type='bool', default=None), + external=dict(required=False, type='bool', default=None), + posix=dict(required=False, type='bool', default=None), + nomembers=dict(required=False, type='bool', default=None), + user=dict(required=False, type='list', elements="str", + default=None), + group=dict(required=False, type='list', elements="str", + default=None), + service=dict(required=False, type='list', elements="str", + default=None), + idoverrideuser=dict(required=False, type='list', elements="str", + default=None), + membermanager_user=dict(required=False, type='list', + elements="str", default=None), + membermanager_group=dict(required=False, type='list', + elements="str", default=None), + externalmember=dict(required=False, type='list', elements="str", + default=None, + aliases=[ + "ipaexternalmember", + "external_member" + ]) + ) ansible_module = IPAAnsibleModule( argument_spec=dict( # general name=dict(type="list", elements="str", aliases=["cn"], - required=True), - # present - description=dict(type="str", default=None), - gid=dict(type="int", aliases=["gidnumber"], default=None), - nonposix=dict(required=False, type='bool', default=None), - external=dict(required=False, type='bool', default=None), - posix=dict(required=False, type='bool', default=None), - nomembers=dict(required=False, type='bool', default=None), - user=dict(required=False, type='list', elements="str", - default=None), - group=dict(required=False, type='list', elements="str", - default=None), - service=dict(required=False, type='list', elements="str", - default=None), - idoverrideuser=dict(required=False, type='list', elements="str", - default=None), - membermanager_user=dict(required=False, type='list', - elements="str", default=None), - membermanager_group=dict(required=False, type='list', - elements="str", default=None), - externalmember=dict(required=False, type='list', elements="str", - default=None, - aliases=[ - "ipaexternalmember", - "external_member" - ]), + default=None, required=False), + groups=dict(type="list", + default=None, + options=dict( + # Here name is a simple string + name=dict(type="str", required=True, + aliases=["cn"]), + # Add group specific parameters + **group_spec + ), + elements='dict', + required=False), + # general action=dict(type="str", default="group", choices=["member", "group"]), - # state state=dict(type="str", default="present", choices=["present", "absent"]), + + # Add group specific parameters for simple use case + **group_spec ), # It does not make sense to set posix, nonposix or external at the # same time - mutually_exclusive=[['posix', 'nonposix', 'external']], + mutually_exclusive=[['posix', 'nonposix', 'external'], + ["name", "groups"]], + required_one_of=[["name", "groups"]], supports_check_mode=True, ) @@ -333,6 +478,7 @@ def main(): # general names = ansible_module.params_get("name") + groups = ansible_module.params_get("groups") # present description = ansible_module.params_get("description") @@ -354,31 +500,26 @@ def main(): state = ansible_module.params_get("state") # Check parameters - invalid = [] + + if (names is None or len(names) < 1) and \ + (groups is None or len(groups) < 1): + ansible_module.fail_json(msg="At least one name or groups is required") if state == "present": - if len(names) != 1: + if names is not None and len(names) != 1: ansible_module.fail_json( - msg="Only one group can be added at a time.") - if action == "member": - invalid = ["description", "gid", "posix", "nonposix", "external", - "nomembers"] + msg="Only one group can be added at a time using 'name'.") - if state == "absent": - if len(names) < 1: - ansible_module.fail_json( - msg="No name given.") - invalid = ["description", "gid", "posix", "nonposix", "external", - "nomembers"] - if action == "group": - invalid.extend(["user", "group", "service", "externalmember"]) - - ansible_module.params_fail_used_invalid(invalid, state, action) + check_parameters(ansible_module, state, action) if external is False: ansible_module.fail_json( msg="group can not be non-external") + # Use groups if names is None + if groups is not None: + names = groups + # Init changed = False @@ -415,8 +556,57 @@ def main(): "supported by your IPA version") commands = [] + group_set = set() + + for group_name in names: + if isinstance(group_name, dict): + name = group_name.get("name") + if name in group_set: + ansible_module.fail_json( + msg="group '%s' is used more than once" % name) + group_set.add(name) + # present + description = group_name.get("description") + gid = group_name.get("gid") + nonposix = group_name.get("nonposix") + external = group_name.get("external") + idoverrideuser = group_name.get("idoverrideuser") + posix = group_name.get("posix") + # Check mutually exclusive condition for multiple groups + # creation. It's not possible to check it with + # `mutually_exclusive` argument in `IPAAnsibleModule` class + # because it accepts only (list[str] or list[list[str]]). Here + # we need to loop over all groups and fail on mutually + # exclusive ones. + if all((posix, nonposix)) or\ + all((posix, external)) or\ + all((nonposix, external)): + ansible_module.fail_json( + msg="parameters are mutually exclusive for group " + "`{0}`: posix|nonposix|external".format(name)) + # Duplicating the condition for multiple group creation + if external is False: + ansible_module.fail_json( + msg="group can not be non-external") + # If nonposix is used, set posix as not nonposix + if nonposix is not None: + posix = not nonposix + user = group_name.get("user") + group = group_name.get("group") + service = group_name.get("service") + membermanager_user = group_name.get("membermanager_user") + membermanager_group = group_name.get("membermanager_group") + externalmember = group_name.get("externalmember") + nomembers = group_name.get("nomembers") + + check_parameters(ansible_module, state, action) + + elif isinstance(group_name, (str, unicode)): + name = group_name + else: + ansible_module.fail_json(msg="Group '%s' is not valid" % + repr(group_name)) - for name in names: # Make sure group exists res_find = find_group(ansible_module, name) diff --git a/tests/group/create_groups_json.yml b/tests/group/create_groups_json.yml new file mode 100644 index 00000000..ef9dd393 --- /dev/null +++ b/tests/group/create_groups_json.yml @@ -0,0 +1,13 @@ +--- +- name: Create groups.json + hosts: localhost + + tasks: + - name: Check if groups.json exists + ansible.builtin.stat: + path: groups.json + register: register_stat_groups + + - name: Create groups.json + ansible.builtin.command: /bin/bash groups.sh 500 + when: not register_stat_groups.stat.exists diff --git a/tests/group/groups.sh b/tests/group/groups.sh new file mode 100644 index 00000000..63e36c01 --- /dev/null +++ b/tests/group/groups.sh @@ -0,0 +1,25 @@ +#!/bin/bash + +NUM=${1-1000} +FILE="groups.json" + +echo "{" > "$FILE" + +echo " \"group_list\": [" >> "$FILE" + +for i in $(seq 1 "$NUM"); do + { + echo " {" + echo " \"name\": \"group$i\"," + echo " \"description\": \"group description $i\"" + } >> "$FILE" + if [ "$i" -lt "$NUM" ]; then + echo " }," >> "$FILE" + else + echo " }" >> "$FILE" + fi +done + +echo " ]" >> "$FILE" + +echo "}" >> "$FILE" diff --git a/tests/group/test_group_client_context.yml b/tests/group/test_group_client_context.yml index 4a1d7ac9..8d0132de 100644 --- a/tests/group/test_group_client_context.yml +++ b/tests/group/test_group_client_context.yml @@ -37,3 +37,27 @@ when: groups['ipaclients'] is not defined or not groups['ipaclients'] vars: ipa_context: client + +- name: Test groups using client context, in client host. + ansible.builtin.import_playbook: test_groups.yml + when: groups['ipaclients'] + vars: + ipa_test_host: ipaclients + +- name: Test groups using client context, in server host. + ansible.builtin.import_playbook: test_groups.yml + when: groups['ipaclients'] is not defined or not groups['ipaclients'] + vars: + ipa_context: client + +- name: Test groups with mixed types using client context, in client host. + ansible.builtin.import_playbook: test_groups_external_nonposix.yml + when: groups['ipaclients'] + vars: + ipa_test_host: ipaclients + +- name: Test groups with mixed types using client context, in server host. + ansible.builtin.import_playbook: test_groups_external_nonposix.yml + when: groups['ipaclients'] is not defined or not groups['ipaclients'] + vars: + ipa_context: client diff --git a/tests/group/test_groups.yml b/tests/group/test_groups.yml new file mode 100644 index 00000000..648b35b8 --- /dev/null +++ b/tests/group/test_groups.yml @@ -0,0 +1,143 @@ +--- +- name: Test groups + hosts: "{{ ipa_test_host | default('ipaserver') }}" + gather_facts: true + + tasks: + # setup + - name: Include tasks ../env_freeipa_facts.yml + ansible.builtin.include_tasks: ../env_freeipa_facts.yml + + # GET FQDN_AT_DOMAIN + + - name: Get fqdn_at_domain + ansible.builtin.set_fact: + fqdn_at_domain: "{{ ansible_facts['fqdn'] + '@' + ipaserver_realm }}" + + # CLEANUP TEST ITEMS + + - name: Remove test groups + ipagroup: + ipaadmin_password: SomeADMINpassword + name: group1,group2,group3,group4,group5,group6,group7,group8,group9,group10 + state: absent + + - name: Remove test users + ipauser: + ipaadmin_password: SomeADMINpassword + name: user1,user2,user3 + state: absent + + # CREATE TEST ITEMS + + - name: Users user1..3 present + ipauser: + ipaadmin_password: SomeADMINpassword + users: + - name: user1 + first: user1 + last: Last + - name: user2 + first: user2 + last: Last + - name: user3 + first: user3 + last: Last + + # TESTS + + - name: Groups group1..10 present + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: group1 + - name: group2 + user: + - user1 + - user2 + - user3 + - name: group3 + group: + - group1 + - group2 + - name: group4 + - name: group5 + - name: group6 + - name: group7 + - name: group8 + - name: group9 + - name: group10 + register: result + failed_when: not result.changed or result.failed + + - name: Groups group1..10 present again + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: group1 + - name: group2 + - name: group3 + - name: group4 + - name: group5 + - name: group6 + - name: group7 + - name: group8 + - name: group9 + - name: group10 + register: result + failed_when: result.changed or result.failed + + # failed_when: not result.failed has been added as this test needs to + # fail because two groups with the same name should be added in the same + # task. + - name: Duplicate names in groups failure test + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: group1 + - name: group2 + - name: group3 + - name: group3 + register: result + failed_when: result.changed or not result.failed or "is used more than once" not in result.msg + + - name: Groups/name and name group11 present + ipagroup: + ipaadmin_password: SomeADMINpassword + name: group11 + groups: + - name: group11 + register: result + failed_when: result.changed or not result.failed or "parameters are mutually exclusive" not in result.msg + + - name: Groups/name and name are absent + ipagroup: + ipaadmin_password: SomeADMINpassword + register: result + failed_when: result.changed or not result.failed or "one of the following is required" not in result.msg + + - name: Name is absent + ipagroup: + ipaadmin_password: SomeADMINpassword + name: + register: result + failed_when: result.changed or not result.failed or "At least one name or groups is required" not in result.msg + + - name: Only one group can be added at a time using name. + ipagroup: + ipaadmin_password: SomeADMINpassword + name: group11,group12 + register: result + failed_when: result.changed or not result.failed or "Only one group can be added at a time using 'name'." not in result.msg + + - name: Remove test groups + ipagroup: + ipaadmin_password: SomeADMINpassword + name: group1,group2,group3,group4,group5,group6,group7,group8,group9,group10 + state: absent + + - name: Remove test users + ipauser: + ipaadmin_password: SomeADMINpassword + name: user1,user2,user3 + state: absent diff --git a/tests/group/test_groups_absent.yml b/tests/group/test_groups_absent.yml new file mode 100644 index 00000000..3238f4e2 --- /dev/null +++ b/tests/group/test_groups_absent.yml @@ -0,0 +1,35 @@ +--- +- name: Include create_groups_json.yml + ansible.builtin.import_playbook: create_groups_json.yml + +- name: Test groups absent + hosts: ipaserver + gather_facts: false + + tasks: + - name: Include groups.json + ansible.builtin.include_vars: + file: groups.json # noqa 505 + + - name: Initialize groups_names + ansible.builtin.set_fact: + groups_names: [] + + - name: Create dict with group names + ansible.builtin.set_fact: + groups_names: "{{ groups_names | default([]) + [{'name': item.name}] }}" + loop: "{{ group_list }}" + + - name: Groups absent len:{{ group_list | length }} + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: "{{ groups_names }}" + state: absent + +- name: Remove groups.json + hosts: localhost + tasks: + - name: Remove groups.json + ansible.builtin.file: + state: absent + path: groups.json diff --git a/tests/group/test_groups_external_nonposix.yml b/tests/group/test_groups_external_nonposix.yml new file mode 100644 index 00000000..5116b723 --- /dev/null +++ b/tests/group/test_groups_external_nonposix.yml @@ -0,0 +1,395 @@ +--- +- name: Test multiple external and nonposix groups + hosts: "{{ ipa_test_host | default('ipaserver') }}" + gather_facts: true + + tasks: + # setup + - name: Include tasks ../env_freeipa_facts.yml + ansible.builtin.include_tasks: ../env_freeipa_facts.yml + + # GET FQDN_AT_DOMAIN + + - name: Get fqdn_at_domain + ansible.builtin.set_fact: + fqdn_at_domain: "{{ ansible_facts['fqdn'] + '@' + ipaserver_realm }}" + + # CLEANUP TEST ITEMS + + - name: Remove testing groups. + ipagroup: + ipaadmin_password: SomeADMINpassword + name: + - extgroup + - nonposixgroup + - posixgroup + - fail_group + - group_1 + - posix_group_1 + - nonposix_group_1 + - external_group_1 + - external_group_2 + state: absent + + - name: Ensure test users testuser1, testuser2 and testuser3 are absent + ipauser: + ipaadmin_password: SomeADMINpassword + name: testuser1,testuser2,testuser3 + state: absent + + # CREATE TEST ITEMS + + - name: Ensure test users testuser1..testuser3 are present + ipauser: + ipaadmin_password: SomeADMINpassword + users: + - name: testuser1 + first: testuser1 + last: Last + - name: testuser2 + first: testuser2 + last: Last + - name: testuser3 + first: testuser3 + last: Last + register: result + failed_when: not result.changed or result.failed + + - name: Add nonposix group. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: extgroup + nonposix: true + register: result + failed_when: result.failed or not result.changed + + - name: Add nonposix group, again. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: extgroup + nonposix: true + register: result + failed_when: result.failed or result.changed + + - name: Set group to be external + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: extgroup + external: true + register: result + failed_when: result.failed or not result.changed + + - name: Set group to be external, again. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: extgroup + external: true + register: result + failed_when: result.failed or result.changed + + - name: Set external group to be non-external. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: extgroup + external: false + register: result + failed_when: not result.failed or "group can not be non-external" not in result.msg + + - name: Set external group to be posix. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: extgroup + posix: true + register: result + failed_when: not result.failed or "Cannot change `external` group" not in result.msg + + - name: Add nonposix group. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + nonposix: true + register: result + failed_when: result.failed or not result.changed + + - name: Set group to be posix + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + posix: true + register: result + failed_when: result.failed or not result.changed + + - name: Set group to be posix, again. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + posix: true + register: result + failed_when: result.failed or result.changed + + - name: Set posix group to be external. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + external: true + register: result + failed_when: not result.failed or "Cannot change `posix` group" not in result.msg + + - name: Set posix group to be non-posix. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + posix: false + register: result + failed_when: not result.failed or "Cannot change `posix` group" not in result.msg + + - name: Set posix group to be non-posix. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + nonposix: true + register: result + failed_when: not result.failed or "Cannot change `posix` group" not in result.msg + + - name: Add nonposix group. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nonposixgroup + posix: false + register: result + failed_when: result.failed or not result.changed + + - name: Add nonposix group, again. + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nonposixgroup + nonposix: true + register: result + failed_when: result.failed or result.changed + + + # NONPOSIX MEMBER TEST + + - name: Ensure users testuser1, testuser2 and testuser3 are present in group nonposixgroup + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nonposixgroup + nonposix: true + user: + - testuser1 + - testuser2 + - testuser3 + register: result + failed_when: not result.changed or result.failed + + - name: Ensure users testuser1, testuser2 and testuser3 are present in group nonposixgroup again + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nonposixgroup + nonposix: true + user: + - testuser1 + - testuser2 + - testuser3 + register: result + failed_when: result.changed or result.failed + + + # POSIX MEMBER TEST + + - name: Ensure users testuser1, testuser2 and testuser3 are present in group posixgroup + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + posix: true + user: + - testuser1 + - testuser2 + - testuser3 + register: result + failed_when: not result.changed or result.failed + + - name: Ensure users testuser1, testuser2 and testuser3 are present in group posixgroup again + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posixgroup + posix: true + user: + - testuser1 + - testuser2 + - testuser3 + register: result + failed_when: result.changed or result.failed + + # EXTERNAL MEMBER TEST (REQUIRES AD) + + - name: Execute group tests if trust test environment is supported + when: trust_test_is_supported | default(false) + block: + - name: Ensure users testuser1, testuser2 and testuser3 are present in group externalgroup + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: externalgroup + external: true + user: + - testuser1 + - testuser2 + - testuser3 + register: result + failed_when: not result.changed or result.failed + + - name: Ensure users testuser1, testuser2 and testuser3 are present in group externalgroup again + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: externalgroup + external: true + user: + - testuser1 + - testuser2 + - testuser3 + register: result + failed_when: result.changed or result.failed + + + # CONVERT NONPOSIX TO POSIX GROUP WITH USERS + + - name: Ensure nonposix group nonposixgroup as posix + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nonposixgroup + posix: true + register: result + failed_when: not result.changed or result.failed + + - name: Ensure nonposix group nonposixgroup as posix, again + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nonposixgroup + posix: true + register: result + failed_when: result.changed or result.failed + + - name: Ensure nonposix group nonposixgroup (now posix) has users still + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: nonposixgroup + posix: true + user: + - testuser1 + - testuser2 + - testuser3 + register: result + failed_when: result.changed or result.failed + + # FAIL ON COMBINATIONS OF NONPOSIX, POSIX AND EXTERNAL + + - name: Fail to ensure group as nonposix and posix + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: firstgroup + nonposix: true + posix: false + - name: fail_group + nonposix: true + posix: true + register: result + failed_when: not result.failed or "parameters are mutually exclusive for group `fail_group`" not in result.msg + + - name: Fail to ensure group as nonposix and external + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: firstgroup + nonposix: true + posix: false + - name: fail_group + nonposix: true + external: true + register: result + failed_when: not result.failed or "parameters are mutually exclusive for group `fail_group`" not in result.msg + + - name: Fail to ensure group as posix and external + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: firstgroup + nonposix: true + posix: false + - name: fail_group + posix: true + external: true + register: result + failed_when: not result.failed or "parameters are mutually exclusive for group `fail_group`" not in result.msg + + # GROUPS WITH MIXED TYPES + + - name: Adding posix, nonposix and external groups in one batch + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: posix_group_1 + posix: true + - name: nonposix_group_1 + nonposix: true + - name: external_group_1 + external: true + register: result + failed_when: not result.changed or result.failed + + - name: Adding non-external and external groups in one batch + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: + - name: non_external_group_2 + - name: external_group_2 + external: true + register: result + failed_when: not result.changed or result.failed + + # CLEANUP + + - name: Remove testing groups. + ipagroup: + ipaadmin_password: SomeADMINpassword + name: + - extgroup + - nonposixgroup + - posixgroup + - fail_group + - group_1 + - posix_group_1 + - nonposix_group_1 + - external_group_1 + - external_group_2 + - non_external_group_2 + state: absent + + - name: Ensure test users testuser1, testuser2 and testuser3 are absent + ipauser: + ipaadmin_password: SomeADMINpassword + name: testuser1,testuser2,testuser3 + state: absent diff --git a/tests/group/test_groups_present.yml b/tests/group/test_groups_present.yml new file mode 100644 index 00000000..71ed659d --- /dev/null +++ b/tests/group/test_groups_present.yml @@ -0,0 +1,40 @@ +--- +- name: Include create_groups_json.yml + ansible.builtin.import_playbook: create_groups_json.yml + +- name: Test groups present + hosts: ipaserver + gather_facts: false + + tasks: + - name: Include groups.json + ansible.builtin.include_vars: + file: groups.json # noqa 505 + + - name: Groups present len:{{ group_list | length }} + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: "{{ group_list }}" + + - name: Initialize groups_names + ansible.builtin.set_fact: + groups_names: [] + + - name: Create dict with group names + ansible.builtin.set_fact: + groups_names: "{{ groups_names | default([]) + [{'name': item.name}] }}" + loop: "{{ group_list }}" + + - name: Remove groups + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: "{{ groups_names }}" + state: absent + +- name: Remove groups.json + hosts: localhost + tasks: + - name: Remove groups.json + ansible.builtin.file: + state: absent + path: groups.json diff --git a/tests/group/test_groups_present_slice.yml b/tests/group/test_groups_present_slice.yml new file mode 100644 index 00000000..3a5593a2 --- /dev/null +++ b/tests/group/test_groups_present_slice.yml @@ -0,0 +1,47 @@ +--- +- name: Include create_groups_json.yml + ansible.builtin.import_playbook: create_groups_json.yml + +- name: Test groups present slice + hosts: ipaserver + gather_facts: false + + vars: + slice_size: 500 + tasks: + - name: Include groups.json + ansible.builtin.include_vars: + file: groups.json # noqa 505 + + - name: Size of groups slice. + ansible.builtin.debug: + msg: "{{ group_list | length }}" + + - name: Groups present + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: "{{ group_list[item : item + slice_size] }}" + loop: "{{ range(0, group_list | length, slice_size) | list }}" + + - name: Initialize groups_names + ansible.builtin.set_fact: + groups_names: [] + + - name: Create dict with group names + ansible.builtin.set_fact: + groups_names: "{{ groups_names | default([]) + [{'name': item.name}] }}" + loop: "{{ group_list }}" + + - name: Remove groups + ipagroup: + ipaadmin_password: SomeADMINpassword + groups: "{{ groups_names }}" + state: absent + +- name: Remove groups.json + hosts: localhost + tasks: + - name: Remove groups.json + ansible.builtin.file: + state: absent + path: groups.json diff --git a/tests/sanity/ignore-2.12.txt b/tests/sanity/ignore-2.12.txt index 364d9249..e639f478 100644 --- a/tests/sanity/ignore-2.12.txt +++ b/tests/sanity/ignore-2.12.txt @@ -39,6 +39,7 @@ tests/pytests/conftest.py pylint:ansible-format-automatic-specification tests/sanity/sanity.sh shebang!skip tests/user/users.sh shebang!skip tests/user/users_absent.sh shebang!skip +tests/group/groups.sh shebang!skip tests/utils.py pylint:ansible-format-automatic-specification utils/ansible-doc-test shebang!skip utils/build-galaxy-release.sh shebang!skip diff --git a/tests/sanity/ignore-2.13.txt b/tests/sanity/ignore-2.13.txt index 0709427e..d5525e7b 100644 --- a/tests/sanity/ignore-2.13.txt +++ b/tests/sanity/ignore-2.13.txt @@ -21,6 +21,7 @@ tests/pytests/conftest.py pylint:ansible-format-automatic-specification tests/sanity/sanity.sh shebang!skip tests/user/users.sh shebang!skip tests/user/users_absent.sh shebang!skip +tests/group/groups.sh shebang!skip tests/utils.py pylint:ansible-format-automatic-specification utils/ansible-doc-test shebang!skip utils/build-galaxy-release.sh shebang!skip diff --git a/tests/sanity/ignore-2.14.txt b/tests/sanity/ignore-2.14.txt index 0709427e..d5525e7b 100644 --- a/tests/sanity/ignore-2.14.txt +++ b/tests/sanity/ignore-2.14.txt @@ -21,6 +21,7 @@ tests/pytests/conftest.py pylint:ansible-format-automatic-specification tests/sanity/sanity.sh shebang!skip tests/user/users.sh shebang!skip tests/user/users_absent.sh shebang!skip +tests/group/groups.sh shebang!skip tests/utils.py pylint:ansible-format-automatic-specification utils/ansible-doc-test shebang!skip utils/build-galaxy-release.sh shebang!skip From ee92d99243f2c1d0d2dd8649a4c24f475eca5d19 Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Wed, 29 Mar 2023 15:52:44 +0200 Subject: [PATCH 2/2] ipagroup: Handle ensuring groups with mixed types without IPA fix 6741 Ensuring (adding) several groups with mixed types external, nonposix and posix require to have a fix in IPA: FreeIPA issue: https://pagure.io/freeipa/issue/9349 FreeIPA fix: https://github.com/freeipa/freeipa/pull/6741 The simple solution is to switch to client context for ensuring several groups simply if the user was not explicitly asking for the server context no matter if mixed types are used. --- plugins/modules/ipagroup.py | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py index e0d5bb12..0e54c02e 100644 --- a/plugins/modules/ipagroup.py +++ b/plugins/modules/ipagroup.py @@ -314,6 +314,17 @@ from ansible.module_utils.ansible_freeipa_module import \ from ansible.module_utils import six if six.PY3: unicode = str +# Ensuring (adding) several groups with mixed types external, nonposix +# and posix require to have a fix in IPA: +# FreeIPA issue: https://pagure.io/freeipa/issue/9349 +# FreeIPA fix: https://github.com/freeipa/freeipa/pull/6741 +try: + from ipaserver.plugins import baseldap +except ImportError: + FIX_6741_DEEPCOPY_OBJECTCLASSES = False +else: + FIX_6741_DEEPCOPY_OBJECTCLASSES = \ + "deepcopy" in baseldap.LDAPObject.__json__.__code__.co_names def find_group(module, name): @@ -516,6 +527,30 @@ def main(): ansible_module.fail_json( msg="group can not be non-external") + # Ensuring (adding) several groups with mixed types external, nonposix + # and posix require to have a fix in IPA: + # + # FreeIPA issue: https://pagure.io/freeipa/issue/9349 + # FreeIPA fix: https://github.com/freeipa/freeipa/pull/6741 + # + # The simple solution is to switch to client context for ensuring + # several groups simply if the user was not explicitly asking for + # the server context no matter if mixed types are used. + context = None + if state == "present" and groups is not None and len(groups) > 1 \ + and not FIX_6741_DEEPCOPY_OBJECTCLASSES: + _context = ansible_module.params_get("ipaapi_context") + if _context is None: + context = "client" + ansible_module.debug( + "Switching to client context due to an unfixed issue in " + "your IPA version: https://pagure.io/freeipa/issue/9349") + elif _context == "server": + ansible_module.fail_json( + msg="Ensuring several groups with server context is not " + "supported by your IPA version: " + "https://pagure.io/freeipa/issue/9349") + # Use groups if names is None if groups is not None: names = groups @@ -530,7 +565,7 @@ def main(): posix = not nonposix # Connect to IPA API - with ansible_module.ipa_connect(): + with ansible_module.ipa_connect(context=context): has_add_member_service = ansible_module.ipa_command_param_exists( "group_add_member", "service")