Skip to content

Commit

Permalink
ipagroup: Fix idempotence issues due to capitalization
Browse files Browse the repository at this point in the history 
Some attributes for ipagroup objects are stored using lower case letters
and should be converted upon retrieving parameter data.

This patch adds the missing conversion and provides a new test playbook:

    tests/group/test_group_case_insensitive.yml
  • Loading branch information
@rjeffman
rjeffman committed Dec 29, 2023
1 parent cf4a72d commit c174a36
Show file tree
Hide file tree
Showing 2 changed files with 310 additions and 4 deletions.
10 changes: 6 additions & 4 deletions plugins/modules/ipagroup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,12 +520,14 @@ def main():
idoverrideuser = ansible_module.params_get("idoverrideuser")
posix = ansible_module.params_get("posix")
nomembers = ansible_module.params_get("nomembers")
user = ansible_module.params_get("user")
group = ansible_module.params_get("group")
user = ansible_module.params_get_lowercase("user")
group = ansible_module.params_get_lowercase("group")
# Services are not case sensitive
service = ansible_module.params_get_lowercase("service")
membermanager_user = ansible_module.params_get("membermanager_user")
membermanager_group = ansible_module.params_get("membermanager_group")
membermanager_user = (
ansible_module.params_get_lowercase("membermanager_user"))
membermanager_group = (
ansible_module.params_get_lowercase("membermanager_group"))
externalmember = ansible_module.params_get("externalmember")
# rename
rename = ansible_module.params_get("rename")
Expand Down
304 changes: 304 additions & 0 deletions tests/group/test_group_case_insensitive.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,304 @@
---
- name: Test group members case insensitive
hosts: ipaserver
become: true
gather_facts: false
module_defaults:
ipagroup:
ipaadmin_password: SomeADMINpassword
ipaapi_context: "{{ ipa_context | default(omit) }}"
ipauser:
ipaadmin_password: SomeADMINpassword
ipaapi_context: "{{ ipa_context | default(omit) }}"

vars:
test_users:
- { name: useR1, first: user1, last: last }
- { name: User2, first: user2, last: last }
user_names: "{{ test_users | map(attribute='name') }}"
test_groups:
- name: Group1
- name: Group2
group_names: "{{ test_groups | map(attribute='name') }}"

tasks:
- name: Include tasks ../env_freeipa_facts.yml
ansible.builtin.include_tasks: ../env_freeipa_facts.yml

- name: Test in all supported versions of IPA
block:
# setup environment
- name: Ensure test groups are absent
ipagroup:
name: "{{ test_groups + ['testgroup'] }}"
state: absent

- name: Ensure test users are present
ipauser:
users: "{{ test_users }}"

- name: Ensure test groups are present
ipagroup:
groups: "{{ test_groups }}"

# tests
- name: Test group presence with user members
vars:
test_cases:
- { id: 1, value: "{{ user_names[0] | lower }}", expected: true }
- { id: 2, value: "{{ user_names[0] | upper }}", expected: false }
- { id: 3, value: "{{ user_names[0] }}", expected: false }
- { id: 4, value: "{{ user_names }}", expected: true }
- { id: 5, value: "{{ user_names | upper }}", expected: false }
- { id: 6, value: "{{ user_names | lower }}", expected: false }
- { id: 7, value: "{{ user_names[1] }}", expected: true }
- { id: 8, value: "{{ user_names[1] | upper }}", expected: false }
- { id: 9, value: "{{ user_names[1] | lower }}", expected: false }
- { id: 10, value: [], expected: true }
block:
- name: Ensure group with user parameter present
ipagroup:
name: testgroup
user: "{{ item.value }}"
register: output
loop: "{{ test_cases }}"
loop_control:
index_var: index
label: "{{ item }}"
- name: "Verify results"
ansible.builtin.assert:
that: run.changed == run.item.expected or run.failed
fail_msg: "{{ run.msg | default('Failed condition: expected=' ~ run.item.expected ~ ', observed=' ~ run.changed) }}"
quiet: true
loop: "{{ output.results }}"
loop_control:
loop_var: run

Check warning on line 75 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} } -> {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed }} }
label: "{{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} }"

- name: Test group presence with group parameter
vars:
test_cases:
- { id: 1, value: "{{ group_names[0] | lower }}", expected: true }
- { id: 2, value: "{{ group_names[0] | upper }}", expected: false }
- { id: 3, value: "{{ group_names[0] }}", expected: false }
- { id: 4, value: "{{ group_names }}", expected: true }
- { id: 5, value: "{{ group_names | upper }}", expected: false }
- { id: 6, value: "{{ group_names | lower }}", expected: false }
- { id: 7, value: "{{ group_names[1] }}", expected: true }
- { id: 8, value: "{{ group_names[1] | upper }}", expected: false }
- { id: 9, value: "{{ group_names[1] | lower }}", expected: false }
- { id: 10, value: [], expected: true }
block:
- name: Ensure group with group present
ipagroup:
name: testgroup
group: "{{ item.value }}"
register: output
loop: "{{ test_cases }}"
loop_control:
index_var: index
label: "{{ item }}"
- name: "Verify results"
ansible.builtin.assert:
that: run.changed == run.item.expected or run.failed
fail_msg: "{{ run.msg | default('Failed condition: expected=' ~ run.item.expected ~ ', observed=' ~ run.changed) }}"
quiet: true
loop: "{{ output.results }}"
loop_control:
loop_var: run

Check warning on line 108 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} } -> {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed }} }
label: "{{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} }"

- name: Test group with group and user parameters, action member
vars:
test_cases:
- { id: 1, user: "{{ user_names }}", group: "{{ group_names }}", expected: true }
- { id: 2, user: "{{ user_names[0] }}", state: "absent", expected: true }
- { id: 3, user: "{{ user_names[1] }}", state: "present", expected: false }
- { id: 4, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
- { id: 5, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
- { id: 6, group: "{{ group_names[0] | upper }}", state: "present", expected: false }
- { id: 7, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
- { id: 8, group: "{{ group_names[0] }}", state: "present", expected: false }
- { id: 9, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: true }
- { id: 10, user: "{{ user_names[0] | lower }}", group: "{{ group_names[0] | upper}}", state: "absent", expected: false }

Check warning on line 123 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ group_names[0] | upper}} -> {{ group_names[0] | upper }}
- { id: 11, user: "{{ user_names[0] | upper }}", group: "{{ group_names[0] | lower }}", state: "absent", expected: false }
- { id: 12, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: false }
- { id: 13, group: "{{ group_names[0] | upper }}", state: "present", expected: true }
- { id: 14, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
- { id: 15, group: "{{ group_names[0] }}", state: "present", expected: false }
- { id: 16, group: "{{ group_names[1] | upper }}", state: "present", expected: false }
- { id: 17, group: "{{ group_names[1] | lower }}", state: "present", expected: false }
- { id: 18, group: "{{ group_names[1] }}", state: "present", expected: false }
- { id: 19, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
- { id: 20, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
- { id: 21, user: "{{ user_names[1] }}", state: "present", expected: false }
block:
- name: Ensure group works with group/user attributes and action member
ipagroup:
name: testgroup
user: "{{ item.user | default(omit) }}"
group: "{{ item.group | default(omit) }}"
action: member
state: "{{ item.state | default('present') }}"
register: output
loop: "{{ test_cases }}"
loop_control:
label: "{{ item }}"
- name: "Verify results"
ansible.builtin.assert:
that: run.changed == run.item.expected or run.failed
fail_msg: "{{ run.msg | default('Failed condition: expected=' ~ run.item.expected ~ ', observed=' ~ run.changed) }}"
quiet: true
loop: "{{ output.results }}"
loop_control:
loop_var: run

Check warning on line 154 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} } -> {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed }} }
label: "{{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} }"

always:
# cleanup
- name: "Ensure test groups test_groups are absent"
ipagroup:
name: "{{ group_names + ['testgroup'] }}"
state: absent

- name: "Ensure test users test_users are absent"
ipauser:
users: "{{ test_users }}"
state: absent

- name: Test in all IPA versions 8.4.4+
when: ipa_version is version('4.8.4', '>=')
block:
# setup environment
- name: Ensure test users are present
ipauser:
users: "{{ test_users }}"

- name: Ensure test groups are present
ipagroup:
groups: "{{ test_groups }}"

# tests
- name: Test group presence with memembermanager_user members
vars:
test_cases:
- { id: 1, value: "{{ user_names[0] | lower }}", expected: true }
- { id: 2, value: "{{ user_names[0] | upper }}", expected: false }
- { id: 3, value: "{{ user_names[0] }}", expected: false }
- { id: 4, value: "{{ user_names }}", expected: true }
- { id: 5, value: "{{ user_names | upper }}", expected: false }
- { id: 6, value: "{{ user_names | lower }}", expected: false }
- { id: 7, value: "{{ user_names[1] }}", expected: true }
- { id: 8, value: "{{ user_names[1] | upper }}", expected: false }
- { id: 9, value: "{{ user_names[1] | lower }}", expected: false }
- { id: 10, value: [], expected: true }
block:
- name: Ensure group with membermanager_user parameter present
ipagroup:
name: testgroup
membermanager_user: "{{ item.value }}"
register: output
loop: "{{ test_cases }}"
loop_control:
index_var: index
label: "{{ item }}"
- name: "Verify results"
ansible.builtin.assert:
that: run.changed == run.item.expected or run.failed
fail_msg: "{{ run.msg | default('Failed condition: expected=' ~ run.item.expected ~ ', observed=' ~ run.changed) }}"
quiet: true
loop: "{{ output.results }}"
loop_control:
loop_var: run

Check warning on line 212 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} } -> {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed }} }
label: "{{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} }"

- name: Test group presence with membermanager_group parameter
vars:
test_cases:
- { id: 1, value: "{{ group_names[0] | lower }}", expected: true }
- { id: 2, value: "{{ group_names[0] | upper }}", expected: false }
- { id: 3, value: "{{ group_names[0] }}", expected: false }
- { id: 4, value: "{{ group_names }}", expected: true }
- { id: 5, value: "{{ group_names | upper }}", expected: false }
- { id: 6, value: "{{ group_names | lower }}", expected: false }
- { id: 7, value: "{{ group_names[1] }}", expected: true }
- { id: 8, value: "{{ group_names[1] | upper }}", expected: false }
- { id: 9, value: "{{ group_names[1] | lower }}", expected: false }
- { id: 10, value: [], expected: true }
block:
- name: Ensure group with membermanager_group present
ipagroup:
name: testgroup
membermanager_group: "{{ item.value }}"
register: output
loop: "{{ test_cases }}"
loop_control:
index_var: index
label: "{{ item }}"
- name: "Verify results"
ansible.builtin.assert:
that: run.changed == run.item.expected or run.failed
fail_msg: "{{ run.msg | default('Failed condition: expected=' ~ run.item.expected ~ ', observed=' ~ run.changed) }}"
quiet: true
loop: "{{ output.results }}"
loop_control:
loop_var: run

Check warning on line 245 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} } -> {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed }} }
label: "{{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} }"

- name: Test group with membermanager_group and membermanager_user parameters, action member
vars:
test_cases:
- { id: 1, user: "{{ user_names }}", group: "{{ group_names }}", expected: true }
- { id: 2, user: "{{ user_names[0] }}", state: "absent", expected: true }
- { id: 3, user: "{{ user_names[1] }}", state: "present", expected: false }
- { id: 4, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
- { id: 5, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
- { id: 6, group: "{{ group_names[0] | upper }}", state: "present", expected: false }
- { id: 7, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
- { id: 8, group: "{{ group_names[0] }}", state: "present", expected: false }
- { id: 9, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: true }
- { id: 10, user: "{{ user_names[0] | lower }}", group: "{{ group_names[0] | upper}}", state: "absent", expected: false }

Check warning on line 260 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ group_names[0] | upper}} -> {{ group_names[0] | upper }}
- { id: 11, user: "{{ user_names[0] | upper }}", group: "{{ group_names[0] | lower }}", state: "absent", expected: false }
- { id: 12, user: "{{ user_names[0] }}", group: "{{ group_names[0] }}", state: "absent", expected: false }
- { id: 13, group: "{{ group_names[0] | upper }}", state: "present", expected: true }
- { id: 14, group: "{{ group_names[0] | lower }}", state: "present", expected: false }
- { id: 15, group: "{{ group_names[0] }}", state: "present", expected: false }
- { id: 16, group: "{{ group_names[1] | upper }}", state: "present", expected: false }
- { id: 17, group: "{{ group_names[1] | lower }}", state: "present", expected: false }
- { id: 18, group: "{{ group_names[1] }}", state: "present", expected: false }
- { id: 19, user: "{{ user_names[1] | upper }}", state: "present", expected: false }
- { id: 20, user: "{{ user_names[1] | lower }}", state: "present", expected: false }
- { id: 21, user: "{{ user_names[1] }}", state: "present", expected: false }
block:
- name: Ensure group works with group/user attributes and action member
ipagroup:
name: testgroup
membermanager_user: "{{ item.user | default(omit) }}"
membermanager_group: "{{ item.group | default(omit) }}"
action: member
state: "{{ item.state | default('present') }}"
register: output
loop: "{{ test_cases }}"
loop_control:
label: "{{ item }}"
- name: "Verify results"
ansible.builtin.assert:
that: run.changed == run.item.expected or run.failed
fail_msg: "{{ run.msg | default('Failed condition: expected=' ~ run.item.expected ~ ', observed=' ~ run.changed) }}"
quiet: true
loop: "{{ output.results }}"
loop_control:
loop_var: run

Check warning on line 291 in tests/group/test_group_case_insensitive.yml

View workflow job for this annotation

GitHub Actions / Verify ansible-lint

jinja[spacing] 

Jinja2 spacing could be improved: {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} } -> {{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed }} }
label: "{{ run.item }}, output={'changed': {{ run.changed }}, 'failed': {{ run.failed}} }"

always:
# cleanup
- name: "Ensure test groups test_groups are absent"
ipagroup:
name: "{{ group_names + ['testgroup'] }}"
state: absent

- name: "Ensure test users test_users are absent"
ipauser:
users: "{{ test_users }}"
state: absent

0 comments on commit c174a36

Please sign in to comment.