From b140f04a9da5cd966705c1ee49ca258cd7507762 Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Fri, 12 Nov 2021 15:01:33 -0300
Subject: [PATCH] hbacsvcgroup: Fix member management idempotence issues.
The hbacsvc members of hbacsvcgroup must be compared in a case
insensitive manner. This patch fixes comparation of member parameters
against existing members by converting parameters to lowercase, as it
is how the hbacsvc members are stored for hbacsvcgroups.
Also, there were some cases where a change with an empty set of members
was issued to IPA API, leading to a result of 'changed: yes' when
'changed: no' was expected. The fix involved a refactoring of the
hbacsvcgroup member management code.
---
plugins/modules/ipahbacsvcgroup.py | 56 +++--
...t_hbacsvcgroup_member_case_insensitive.yml | 233 ++++++++++++++++++
2 files changed, 263 insertions(+), 26 deletions(-)
create mode 100644 tests/hbacsvcgroup/test_hbacsvcgroup_member_case_insensitive.yml
diff --git a/plugins/modules/ipahbacsvcgroup.py b/plugins/modules/ipahbacsvcgroup.py
index 60f05d2d..530e1f1a 100644
--- a/plugins/modules/ipahbacsvcgroup.py
+++ b/plugins/modules/ipahbacsvcgroup.py
@@ -98,7 +98,8 @@ RETURN = """
"""
from ansible.module_utils.ansible_freeipa_module import \
- IPAAnsibleModule, compare_args_ipa, gen_add_del_lists
+ IPAAnsibleModule, compare_args_ipa, gen_add_del_lists, gen_add_list, \
+ gen_intersection_list
def find_hbacsvcgroup(module, name):
@@ -180,7 +181,7 @@ def main():
# present
description = ansible_module.params_get("description")
nomembers = ansible_module.params_get("nomembers")
- hbacsvc = ansible_module.params_get("hbacsvc")
+ hbacsvc = ansible_module.params_get_lowercase("hbacsvc")
action = ansible_module.params_get("action")
# state
state = ansible_module.params_get("state")
@@ -220,6 +221,8 @@ def main():
# Make sure hbacsvcgroup exists
res_find = find_hbacsvcgroup(ansible_module, name)
+ hbacsvc_add, hbacsvc_del = [], []
+
# Create command
if state == "present":
# Generate args
@@ -243,32 +246,20 @@ def main():
if not compare_args_ipa(ansible_module, member_args,
res_find):
# Generate addition and removal lists
- hbacsvc_add, hbacsvc_del = gen_add_del_lists(
- hbacsvc, res_find.get("member_hbacsvc"))
+ if hbacsvc is not None:
+ hbacsvc_add, hbacsvc_del = gen_add_del_lists(
+ hbacsvc, res_find.get("member_hbacsvc"))
- # Add members
- if len(hbacsvc_add) > 0:
- commands.append([name, "hbacsvcgroup_add_member",
- {
- "hbacsvc": hbacsvc_add
- }])
- # Remove members
- if len(hbacsvc_del) > 0:
- commands.append([name,
- "hbacsvcgroup_remove_member",
- {
- "hbacsvc": hbacsvc_del
- }])
elif action == "member":
if res_find is None:
ansible_module.fail_json(
msg="No hbacsvcgroup '%s'" % name)
# Ensure members are present
- commands.append([name, "hbacsvcgroup_add_member",
- {
- "hbacsvc": hbacsvc
- }])
+ if hbacsvc:
+ hbacsvc_add = gen_add_list(
+ hbacsvc, res_find.get("member_hbacsvc"))
+
elif state == "absent":
if action == "hbacsvcgroup":
if res_find is not None:
@@ -280,15 +271,28 @@ def main():
msg="No hbacsvcgroup '%s'" % name)
# Ensure members are absent
- commands.append([name, "hbacsvcgroup_remove_member",
- {
- "hbacsvc": hbacsvc
- }])
+ if hbacsvc:
+ hbacsvc_del = gen_intersection_list(
+ hbacsvc, res_find.get("member_hbacsvc"))
+
else:
ansible_module.fail_json(msg="Unkown state '%s'" % state)
- # Execute commands
+ # Manage members
+ if len(hbacsvc_add) > 0:
+ commands.append([name, "hbacsvcgroup_add_member",
+ {
+ "hbacsvc": hbacsvc_add
+ }])
+ # Remove members
+ if len(hbacsvc_del) > 0:
+ commands.append([name,
+ "hbacsvcgroup_remove_member",
+ {
+ "hbacsvc": hbacsvc_del
+ }])
+ # Execute commands
changed = ansible_module.execute_ipa_commands(commands, result_handler)
# Done
diff --git a/tests/hbacsvcgroup/test_hbacsvcgroup_member_case_insensitive.yml b/tests/hbacsvcgroup/test_hbacsvcgroup_member_case_insensitive.yml
new file mode 100644
index 00000000..ca16b4bc
--- /dev/null
+++ b/tests/hbacsvcgroup/test_hbacsvcgroup_member_case_insensitive.yml
@@ -0,0 +1,233 @@
+---
+- name: Test hbacsvcgroup member varying capitalization
+ hosts: "{{ ipa_test_host | default('ipaserver') }}"
+ become: no
+ gather_facts: no
+
+ vars:
+ hbacsvc_list:
+ - sVc1
+ - SvC2
+
+ tasks:
+ - block:
+ - name: Ensure test hbacsvcgroup is absent
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ state: absent
+
+ - name: Ensure test HBAC services are present
+ ipahbacsvc:
+ ipaadmin_password: SomeADMINpassword
+ name: "{{ item }}"
+ with_items: "{{ hbacsvc_list }}"
+
+ - name: Ensure test hbacsvcgroup is present with duplicate hbacsvc
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc:
+ - sVc1
+ - SvC1
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Ensure test hbacsvc is absent from hbacsvcgroup, with duplicate hbacsvc
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc:
+ - sVc1
+ - SvC1
+ action: member
+ state: absent
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Check if test hbacsvc absent, again, from hbacsvcgroup, with duplicate hbacsvc, would trigger changes
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc:
+ - svC1
+ - SVC1
+ action: member
+ state: absent
+ check_mode: yes
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure test hbacsvcgroup is absent
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ state: absent
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Check if hbacsvcgroup with members would trigger changes, mixed case
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list }}"
+ check_mode: yes
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup is present with members, mixed case
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list }}"
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Check if hbacsvcgroup with members would not trigger changes, mixed case
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list }}"
+ check_mode: yes
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup is present with members, lowercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | lower }}"
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup is present with members, uppercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | upper }}"
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure test hbacsvcgroup is absent
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ state: absent
+
+ - name: Ensure test hbacsvcgroup is present
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+
+ - name: Check if hbacsvcgroup members would trigger changes, mixed case
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list }}"
+ action: member
+ check_mode: yes
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup has members, mixed case
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list }}"
+ action: member
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Check if hbacsvcgroup members would not trigger changes, mixed case
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list }}"
+ action: member
+ check_mode: yes
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup has members, lowercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | lower }}"
+ action: member
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup has members, uppercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | upper }}"
+ action: member
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Check if hbacsvcgroup members absence would trigger changes, uppercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | upper }}"
+ action: member
+ state: absent
+ check_mode: yes
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup has members absent, uppercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | upper }}"
+ action: member
+ state: absent
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Check if hbacsvcgroup members absence would not trigger changes, uppercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | upper }}"
+ action: member
+ state: absent
+ check_mode: yes
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup has members absent, mixed case
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list }}"
+ action: member
+ state: absent
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure hbacsvcgroup has members absent, lowercase
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ hbacsvc: "{{ hbacsvc_list | lower }}"
+ action: member
+ state: absent
+ register: result
+ failed_when: result.changed or result.failed
+
+ always:
+ - name: Ensure test hbac service group is absent
+ ipahbacsvcgroup:
+ ipaadmin_password: SomeADMINpassword
+ name: testgroup
+ state: absent
+
+ - name: Ensure test hbac services are absent
+ ipahbacsvc:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: "{{ hbacsvc_list }}"
+ state: absent
--
GitLab