From ba7eec311b2135ae1ca6e72f16ed6284704b7e36 Mon Sep 17 00:00:00 2001 From: Denis Karpelevich Date: Wed, 11 Jan 2023 17:24:11 +0100 Subject: [PATCH] New utils method that returns the lists of members. Extend netgroup and sudorule modules to support external users and hosts wherever possible. Add tests for ipanetgroup and ipasudorule. Problem statement: ``` - name: Ensure sudorule is present with users and hosts (action member) ipasudorule: name: testrule2 user: - external-user action: member - name: Ensure sudorule is present with users and hosts (action member) again ipasudorule: name: testrule2 user: - external-user action: member ``` After execution of the first task with external users ansible returns changed as expected, after second it still returns changed - it's a bug. This PR fixes it. After the second task ansible will return ok. "External" entities are: for `ipasudorule`: `externalhost, externaluser, ipasudorunasextuser, ipasudorunasextgroup` for `ipanetgroup`: `externalhost` Signed-off-by: Denis Karpelevich --- .../module_utils/ansible_freeipa_module.py | 15 ++ plugins/modules/ipanetgroup.py | 17 +- plugins/modules/ipasudorule.py | 76 +++--- tests/netgroup/test_netgroup_ext_member.yml | 131 ++++++++++ ...est_netgroup_ext_member_client_context.yml | 40 +++ tests/sudorule/test_sudorule_ext_member.yml | 235 ++++++++++++++++++ ...est_sudorule_ext_member_client_context.yml | 40 +++ 7 files changed, 520 insertions(+), 34 deletions(-) create mode 100644 tests/netgroup/test_netgroup_ext_member.yml create mode 100644 tests/netgroup/test_netgroup_ext_member_client_context.yml create mode 100644 tests/sudorule/test_sudorule_ext_member.yml create mode 100644 tests/sudorule/test_sudorule_ext_member_client_context.yml diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index ff7b06dc35..bf09e28436 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -566,6 +566,21 @@ def gen_intersection_list(user_list, res_list): return list(set(res_list or []).intersection(set(user_list or []))) +def concat_attr_list(res, *args): + """ + Get the lists of members to pass on `gen_*` methods as a res_list argument. + + This function should be used to get members (usually users, + external users, hosts, external hosts, etc.) with any action and any state. + + It is returning the concatenation of all attributes provided by user. + """ + res_list = [] + for attribute in args: + res_list += res.get(attribute, []) + return list(set(res_list)) + + def encode_certificate(cert): """ Encode a certificate using base64. diff --git a/plugins/modules/ipanetgroup.py b/plugins/modules/ipanetgroup.py index e07893236a..dbd462cf27 100644 --- a/plugins/modules/ipanetgroup.py +++ b/plugins/modules/ipanetgroup.py @@ -157,7 +157,7 @@ from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, \ - gen_add_list, gen_intersection_list, ensure_fqdn + gen_add_list, gen_intersection_list, concat_attr_list, ensure_fqdn def find_netgroup(module, name): @@ -339,8 +339,13 @@ def main(): group_add, group_del = gen_add_del_lists( group, res_find.get("memberuser_group")) + # `externalhost` adds an entity to the "External host" + # list for `ipanetgroup`. Hosts enrolled to IPA are in + # "Member Host" list. host_add, host_del = gen_add_del_lists( - host, res_find.get("memberhost_host")) + host, concat_attr_list(res_find, + "memberhost_host", + "externalhost")) hostgroup_add, hostgroup_del = gen_add_del_lists( hostgroup, res_find.get("memberhost_hostgroup")) @@ -360,7 +365,9 @@ def main(): group_add = gen_add_list( group, res_find.get("memberuser_group")) host_add = gen_add_list( - host, res_find.get("memberhost_host")) + host, concat_attr_list(res_find, + "memberhost_host", + "externalhost")) hostgroup_add = gen_add_list( hostgroup, res_find.get("memberhost_hostgroup")) netgroup_add = gen_add_list( @@ -379,7 +386,9 @@ def main(): group_del = gen_intersection_list( group, res_find.get("memberuser_group")) host_del = gen_intersection_list( - host, res_find.get("memberhost_host")) + host, concat_attr_list(res_find, + "memberhost_host", + "externalhost")) hostgroup_del = gen_intersection_list( hostgroup, res_find.get("memberhost_hostgroup")) netgroup_del = gen_intersection_list( diff --git a/plugins/modules/ipasudorule.py b/plugins/modules/ipasudorule.py index 2be49c2244..abdb6e724a 100644 --- a/plugins/modules/ipasudorule.py +++ b/plugins/modules/ipasudorule.py @@ -228,7 +228,8 @@ from ansible.module_utils.ansible_freeipa_module import \ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \ - gen_intersection_list, api_get_domain, ensure_fqdn, netaddr, to_text + gen_intersection_list, concat_attr_list, api_get_domain, ensure_fqdn, \ + netaddr, to_text def find_sudorule(module, name): @@ -505,9 +506,14 @@ def main(): # Set res_find to empty dict for next step res_find = {} - # Generate addition and removal lists + # Generate addition and removal lists. + # `externalhost` adds an entity to the "External host" + # list for `ipasudorule`. Hosts enrolled to IPA are in + # "Hosts" list. host_add, host_del = gen_add_del_lists( - host, res_find.get('memberhost_host', [])) + host, concat_attr_list(res_find, + "memberhost_host", + "externalhost")) hostgroup_add, hostgroup_del = gen_add_del_lists( hostgroup, res_find.get('memberhost_hostgroup', [])) @@ -515,8 +521,13 @@ def main(): hostmask_add, hostmask_del = gen_add_del_lists( hostmask, res_find.get('hostmask', [])) + # `externaluser` adds an entity to the "External user" + # (non-IPA users) list for `ipasudorule`. Users enrolled to + # IPA are in "Users" list. user_add, user_del = gen_add_del_lists( - user, res_find.get('memberuser_user', [])) + user, concat_attr_list(res_find, + "memberuser_user", + "externaluser")) group_add, group_del = gen_add_del_lists( group, res_find.get('memberuser_group', [])) @@ -547,10 +558,9 @@ def main(): # users list. runasuser_add, runasuser_del = gen_add_del_lists( runasuser, - ( - res_find.get('ipasudorunas_user', []) - + res_find.get('ipasudorunasextuser', []) - ) + concat_attr_list(res_find, + 'ipasudorunas_user', + 'ipasudorunasextuser') ) # runasgroup attribute can be used with both IPA and @@ -560,10 +570,9 @@ def main(): # groups list. runasgroup_add, runasgroup_del = gen_add_del_lists( runasgroup, - ( - res_find.get('ipasudorunasgroup_group', []) - + res_find.get('ipasudorunasextgroup', []) - ) + concat_attr_list(res_find, + 'ipasudorunasgroup_group', + 'ipasudorunasextgroup') ) elif action == "member": @@ -577,7 +586,9 @@ def main(): # the sudorule already if host is not None: host_add = gen_add_list( - host, res_find.get("memberhost_host")) + host, concat_attr_list(res_find, + "memberhost_host", + "externalhost")) if hostgroup is not None: hostgroup_add = gen_add_list( hostgroup, res_find.get("memberhost_hostgroup")) @@ -586,7 +597,9 @@ def main(): hostmask, res_find.get("hostmask")) if user is not None: user_add = gen_add_list( - user, res_find.get("memberuser_user")) + user, concat_attr_list(res_find, + "memberuser_user", + "externaluser")) if group is not None: group_add = gen_add_list( group, res_find.get("memberuser_group")) @@ -620,8 +633,9 @@ def main(): if runasuser is not None: runasuser_add = gen_add_list( runasuser, - (list(res_find.get('ipasudorunas_user', [])) - + list(res_find.get('ipasudorunasextuser', []))) + concat_attr_list(res_find, + 'ipasudorunas_user', + 'ipasudorunasextuser') ) # runasgroup attribute can be used with both IPA and # non-IPA (external) groups, so we need to compare @@ -630,8 +644,9 @@ def main(): if runasgroup is not None: runasgroup_add = gen_add_list( runasgroup, - (list(res_find.get("ipasudorunasgroup_group", [])) - + list(res_find.get("ipasudorunasextgroup", []))) + concat_attr_list(res_find, + 'ipasudorunasgroup_group', + 'ipasudorunasextgroup') ) elif state == "absent": @@ -650,7 +665,9 @@ def main(): # in sudorule if host is not None: host_del = gen_intersection_list( - host, res_find.get("memberhost_host")) + host, concat_attr_list(res_find, + "memberhost_host", + "externalhost")) if hostgroup is not None: hostgroup_del = gen_intersection_list( @@ -662,7 +679,9 @@ def main(): if user is not None: user_del = gen_intersection_list( - user, res_find.get("memberuser_user")) + user, concat_attr_list(res_find, + "memberuser_user", + "externaluser")) if group is not None: group_del = gen_intersection_list( @@ -698,10 +717,10 @@ def main(): if runasuser is not None: runasuser_del = gen_intersection_list( runasuser, - ( - list(res_find.get('ipasudorunas_user', [])) - + list(res_find.get('ipasudorunasextuser', [])) - ) + concat_attr_list(res_find, + 'ipasudorunas_user', + 'ipasudorunasextuser') + ) # runasgroup attribute can be used with both IPA and # non-IPA (external) groups, so we need to compare @@ -710,12 +729,9 @@ def main(): if runasgroup is not None: runasgroup_del = gen_intersection_list( runasgroup, - ( - list(res_find.get( - "ipasudorunasgroup_group", [])) - + list(res_find.get( - "ipasudorunasextgroup", [])) - ) + concat_attr_list(res_find, + 'ipasudorunasgroup_group', + 'ipasudorunasextgroup') ) elif state == "enabled": diff --git a/tests/netgroup/test_netgroup_ext_member.yml b/tests/netgroup/test_netgroup_ext_member.yml new file mode 100644 index 0000000000..0f80183d43 --- /dev/null +++ b/tests/netgroup/test_netgroup_ext_member.yml @@ -0,0 +1,131 @@ +--- + +- name: Test netgroup with external members + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: true + gather_facts: true + + tasks: + - name: Test netgroup with external members + block: + # setup + - name: Ensure netgroups are absent + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - testnetgroup1 + - testnetgroup2 + state: absent + + - name: Ensure external host is absent + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external.host + state: absent + + - name: Ensure host is present + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ ansible_facts['fqdn'] }}" + + - name: Ensure netgroup testnetgroup2 is present + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testnetgroup2 + + # tests + - name: Ensure netgroup is present with hosts (action netgroup) + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testnetgroup1 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + register: result + failed_when: not result.changed or result.failed + + - name: Ensure netgroup is present with hosts (action netgroup) again + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testnetgroup1 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + register: result + failed_when: result.changed or result.failed + + - name: Ensure netgroup is present with hosts (action member) + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testnetgroup2 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + action: member + register: result + failed_when: not result.changed or result.failed + + - name: Ensure netgroup is present with hosts (action member) again + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testnetgroup2 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + action: member + register: result + failed_when: result.changed or result.failed + + - name: Ensure hosts are absent in netgroup (action member) + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testnetgroup2 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + action: member + state: absent + register: result + failed_when: not result.changed or result.failed + + - name: Ensure hosts are absent in netgroup (action member) again + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testnetgroup2 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + action: member + state: absent + register: result + failed_when: result.changed or result.failed + + always: + # cleanup + - name: Ensure netgroups are absent + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - testnetgroup1 + - testnetgroup2 + state: absent + + - name: Ensure external host is absent + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external.host + state: absent diff --git a/tests/netgroup/test_netgroup_ext_member_client_context.yml b/tests/netgroup/test_netgroup_ext_member_client_context.yml new file mode 100644 index 0000000000..46cec77002 --- /dev/null +++ b/tests/netgroup/test_netgroup_ext_member_client_context.yml @@ -0,0 +1,40 @@ +--- + +- name: Test netgroup with external members, client context + hosts: ipaclients, ipaserver + become: false + gather_facts: false + + tasks: + - name: Include FreeIPA facts. + ansible.builtin.include_tasks: ../env_freeipa_facts.yml + + # Test will only be executed if host is not a server. + - name: Execute with server context in the client. + ipanetgroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: server + name: ThisShouldNotWork + register: result + failed_when: not (result.failed and result.msg is regex("No module named '*ipaserver'*")) + when: ipa_host_is_client + +# Import basic module tests, and execute with ipa_context set to 'client'. +# If ipaclients is set, it will be executed using the client, if not, +# ipaserver will be used. +# +# With this setup, tests can be executed against an IPA client, against +# an IPA server using "client" context, and ensure that tests are executed +# in upstream CI. + +- name: Test netgroup with external members using client context, in client host. + ansible.builtin.import_playbook: test_netgroup_ext_member.yml + when: groups['ipaclients'] + vars: + ipa_test_host: ipaclients + +- name: Test netgroup with external members using client context, in server host. + ansible.builtin.import_playbook: test_netgroup_ext_member.yml + when: groups['ipaclients'] is not defined or not groups['ipaclients'] + vars: + ipa_context: client diff --git a/tests/sudorule/test_sudorule_ext_member.yml b/tests/sudorule/test_sudorule_ext_member.yml new file mode 100644 index 0000000000..410bcad594 --- /dev/null +++ b/tests/sudorule/test_sudorule_ext_member.yml @@ -0,0 +1,235 @@ +--- + +- name: Test sudorule with external members + hosts: "{{ ipa_test_host | default('ipaserver') }}" + become: true + gather_facts: true + + tasks: + - name: Test sudorule with external members + block: + # setup + - name: Ensure sudorules are absent + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - testrule1 + - testrule2 + state: absent + + - name: Ensure external user and user are absent + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external-user + - user01 + state: absent + + - name: Ensure external group and group are absent + ipagroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external-group + - group01 + state: absent + + - name: Ensure external host is absent + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external.host + state: absent + + - name: Ensure user is present + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: user01 + first: user + last: zeroone + + - name: Ensure group is present + ipagroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: group01 + + - name: Ensure host is present + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: "{{ ansible_facts['fqdn'] }}" + + - name: Ensure sudorule testrule2 is present + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testrule2 + + # tests + - name: Ensure sudorule is present with users and hosts (action sudorule) + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testrule1 + user: + - external-user + - user01 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + runasuser: + - user01 + - external-user + runasgroup: + - group01 + - external-group + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorule is present with users and hosts (action sudorule) again + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testrule1 + user: + - external-user + - user01 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + runasuser: + - user01 + - external-user + runasgroup: + - group01 + - external-group + register: result + failed_when: result.changed or result.failed + + - name: Ensure sudorule is present with users and hosts (action member) + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testrule2 + user: + - external-user + - user01 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + runasuser: + - user01 + - external-user + runasgroup: + - group01 + - external-group + action: member + register: result + failed_when: not result.changed or result.failed + + - name: Ensure sudorule is present with users and hosts (action member) again + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testrule2 + user: + - external-user + - user01 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + runasuser: + - user01 + - external-user + runasgroup: + - group01 + - external-group + action: member + register: result + failed_when: result.changed or result.failed + + - name: Ensure users and hosts are absent in sudorule (action member) + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testrule2 + user: + - external-user + - user01 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + runasuser: + - user01 + - external-user + runasgroup: + - group01 + - external-group + action: member + state: absent + register: result + failed_when: not result.changed or result.failed + + - name: Ensure users and hosts are absent in sudorule (action member) again + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: testrule2 + user: + - external-user + - user01 + host: + - "{{ ansible_facts['fqdn'] }}" + - external.host + runasuser: + - user01 + - external-user + runasgroup: + - group01 + - external-group + action: member + state: absent + register: result + failed_when: result.changed or result.failed + + always: + # cleanup + - name: Ensure sudorules are absent + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - testrule1 + - testrule2 + state: absent + + - name: Ensure external user and user are absent + ipauser: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external-user + - user01 + state: absent + + - name: Ensure external group and group are absent + ipagroup: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external-group + - group01 + state: absent + + - name: Ensure external host is absent + ipahost: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" + name: + - external.host + state: absent diff --git a/tests/sudorule/test_sudorule_ext_member_client_context.yml b/tests/sudorule/test_sudorule_ext_member_client_context.yml new file mode 100644 index 0000000000..6dc6619e04 --- /dev/null +++ b/tests/sudorule/test_sudorule_ext_member_client_context.yml @@ -0,0 +1,40 @@ +--- + +- name: Test sudorule with external members, client context + hosts: ipaclients, ipaserver + become: false + gather_facts: false + + tasks: + - name: Include FreeIPA facts. + ansible.builtin.include_tasks: ../env_freeipa_facts.yml + + # Test will only be executed if host is not a server. + - name: Execute with server context in the client. + ipasudorule: + ipaadmin_password: SomeADMINpassword + ipaapi_context: server + name: ThisShouldNotWork + register: result + failed_when: not (result.failed and result.msg is regex("No module named '*ipaserver'*")) + when: ipa_host_is_client + +# Import basic module tests, and execute with ipa_context set to 'client'. +# If ipaclients is set, it will be executed using the client, if not, +# ipaserver will be used. +# +# With this setup, tests can be executed against an IPA client, against +# an IPA server using "client" context, and ensure that tests are executed +# in upstream CI. + +- name: Test sudorule with external members using client context, in client host. + ansible.builtin.import_playbook: test_sudorule_ext_member.yml + when: groups['ipaclients'] + vars: + ipa_test_host: ipaclients + +- name: Test sudorule with external members using client context, in server host. + ansible.builtin.import_playbook: test_sudorule_ext_member.yml + when: groups['ipaclients'] is not defined or not groups['ipaclients'] + vars: + ipa_context: client