diff --git a/plugins/modules/ipaprivilege.py b/plugins/modules/ipaprivilege.py
index 82d65c362f32df46f3409a8bf6065f3c73873db3..6856b1a625eea3d650f8973095191e0def2a4f3e 100644
--- a/plugins/modules/ipaprivilege.py
+++ b/plugins/modules/ipaprivilege.py
@@ -108,7 +108,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
import six
if six.PY3:
@@ -126,22 +127,6 @@ def find_privilege(module, name):
return _result["result"]
-# pylint: disable=unused-argument
-def result_handler(module, result, command, name, args, errors):
- # Get all errors
- # All "already a member" and "not a member" failures in the
- # result are ignored. All others are reported.
- for failed_item in result.get("failed", []):
- failed = result["failed"][failed_item]
- for member_type in failed:
- for member, failure in failed[member_type]:
- if "already a member" in failure \
- or "not a member" in failure:
- continue
- errors.append("%s: %s %s: %s" % (
- command, member_type, member, failure))
-
-
def main():
ansible_module = IPAAnsibleModule(
argument_spec=dict(
@@ -230,47 +215,31 @@ def main():
if action == "privilege":
# Found the privilege
if res_find is not None:
- res_cmp = {
- k: v for k, v in res_find.items()
- if k not in [
- "objectclass", "cn", "dn",
- "memberof_permisssion"
- ]
- }
- # For all settings is args, check if there are
- # different settings in the find result.
- # If yes: modify
- if args and not compare_args_ipa(ansible_module, args,
- res_cmp):
+ cmp = {"description": res_find.get("description")}
+ if not compare_args_ipa(ansible_module, args, cmp):
commands.append([name, "privilege_mod", args])
else:
commands.append([name, "privilege_add", args])
res_find = {}
- member_args = {}
- if permission:
- member_args['permission'] = permission
-
- if not compare_args_ipa(ansible_module, member_args,
- res_find):
-
- # Generate addition and removal lists
- permission_add, permission_del = gen_add_del_lists(
- permission, res_find.get("member_permission"))
-
- # Add members
- if len(permission_add) > 0:
- commands.append([name, "privilege_add_permission",
- {
- "permission": permission_add,
- }])
- # Remove members
- if len(permission_del) > 0:
- commands.append([
- name,
- "privilege_remove_permission",
- {"permission": permission_del}
- ])
+ # Generate addition and removal lists
+ permission_add, permission_del = gen_add_del_lists(
+ permission, res_find.get("memberof_permission")
+ )
+
+ # Add members
+ if len(permission_add) > 0:
+ commands.append([name, "privilege_add_permission",
+ {
+ "permission": permission_add,
+ }])
+ # Remove members
+ if len(permission_del) > 0:
+ commands.append([
+ name,
+ "privilege_remove_permission",
+ {"permission": permission_del}
+ ])
elif action == "member":
if res_find is None:
@@ -280,8 +249,11 @@ def main():
if permission is None:
ansible_module.fail_json(msg="No permission given")
- commands.append([name, "privilege_add_permission",
- {"permission": permission}])
+ permission = gen_add_list(
+ permission, res_find.get("memberof_permission"))
+ if permission:
+ commands.append([name, "privilege_add_permission",
+ {"permission": permission}])
elif state == "absent":
if action == "privilege":
@@ -296,10 +268,11 @@ def main():
if permission is None:
ansible_module.fail_json(msg="No permission given")
- commands.append([name, "privilege_remove_permission",
- {
- "permission": permission,
- }])
+ permission = gen_intersection_list(
+ permission, res_find.get("memberof_permission"))
+ if permission:
+ commands.append([name, "privilege_remove_permission",
+ {"permission": permission}])
elif state == "renamed":
if not rename:
@@ -318,7 +291,8 @@ def main():
# Execute commands
- changed = ansible_module.execute_ipa_commands(commands, result_handler)
+ changed = ansible_module.execute_ipa_commands(
+ commands, fail_on_member_errors=True)
# Done
diff --git a/tests/privilege/test_privilege.yml b/tests/privilege/test_privilege.yml
index 71c08d1617187789ff83b7a871dfc90c57ec63fd..bfb467299c6744ed4eeda1d2a1b04f2a8819eb60 100644
--- a/tests/privilege/test_privilege.yml
+++ b/tests/privilege/test_privilege.yml
@@ -47,6 +47,20 @@
register: result
failed_when: not result.changed or result.failed
+ - name: Check if adding member permissions would cause a change (issue 669).
+ ipaprivilege:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: Broad Privilege
+ permission:
+ - "Write IPA Configuration"
+ - "System: Write DNS Configuration"
+ - "System: Update DNS Entries"
+ action: member
+ check_mode: yes
+ register: result
+ failed_when: not result.changed or result.failed
+
- name: Ensure privilege Broad Privilege has permissions
ipaprivilege:
ipaadmin_password: SomeADMINpassword
@@ -73,6 +87,20 @@
register: result
failed_when: result.changed or result.failed
+ - name: Check if existing member permissions would not cause a change (issue 669).
+ ipaprivilege:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: Broad Privilege
+ permission:
+ - "Write IPA Configuration"
+ - "System: Write DNS Configuration"
+ - "System: Update DNS Entries"
+ action: member
+ check_mode: yes
+ register: result
+ failed_when: result.changed or result.failed
+
- name: Ensure privilege Broad Privilege member permission "Write IPA Configuration" is absent
ipaprivilege:
ipaadmin_password: SomeADMINpassword
@@ -161,6 +189,17 @@
name: Broad Privilege
state: absent
+ - name: Check if creating privilege Broad Privilege with permission would issue a change. (issue 669)
+ ipaprivilege:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: Broad Privilege
+ permission:
+ - "Write IPA Configuration"
+ check_mode: yes
+ register: result
+ failed_when: not result.changed or result.failed
+
- name: Ensure privilege Broad Privilege is created with permission. (issue 529)
ipaprivilege:
ipaadmin_password: SomeADMINpassword
@@ -181,6 +220,17 @@
register: result
failed_when: result.changed or result.failed
+ - name: Check if exisintg privilege Broad Privilege with permission would not issue a change. (issue 669)
+ ipaprivilege:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: Broad Privilege
+ permission:
+ - "Write IPA Configuration"
+ check_mode: yes
+ register: result
+ failed_when: result.changed or result.failed
+
# CLEANUP TEST ITEMS
- name: Ensure privilege testing privileges are absent