From 457050c6ac79c008d495d72af0dc91b03e998bb1 Mon Sep 17 00:00:00 2001
From: Thomas Woerner <twoerner@redhat.com>
Date: Wed, 6 May 2020 16:28:02 +0200
Subject: [PATCH] Do not remove member attributes while updating others
Because of a missing check member attributes (for use with action: member)
are cleared when a non-member attribute is changed. The fix simply adds a
check for None (parameter not set) to gen_add_del_lists in
ansible_freeipa_module to make sure that the parameter is only changed if
it should be changed.
All places where the add and removal lists have been generated manually
have been changed to also use gen_add_del_lists.
Resolves: #252 (The "Manager" attribute is removed when updating any user
attribute)
---
.../module_utils/ansible_freeipa_module.py | 4 ++
plugins/modules/ipagroup.py | 28 +++------
plugins/modules/ipahbacrule.py | 58 ++++++-------------
plugins/modules/ipahbacsvcgroup.py | 11 ++--
plugins/modules/ipahostgroup.py | 19 ++----
plugins/modules/ipasudocmdgroup.py | 13 ++---
plugins/modules/ipauser.py | 35 ++++-------
tests/user/test_user.yml | 4 +-
tests/user/test_users.yml | 4 +-
9 files changed, 62 insertions(+), 114 deletions(-)
diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py
index bf9eca82..af45a6cb 100644
--- a/plugins/module_utils/ansible_freeipa_module.py
+++ b/plugins/module_utils/ansible_freeipa_module.py
@@ -298,6 +298,10 @@ def api_get_realm():
def gen_add_del_lists(user_list, res_list):
"""Generate the lists for the addition and removal of members."""
+ # The user list is None, therefore the parameter should not be touched
+ if user_list is None:
+ return [], []
+
add_list = list(set(user_list or []) - set(res_list or []))
del_list = list(set(res_list or []) - set(user_list or []))
diff --git a/plugins/modules/ipagroup.py b/plugins/modules/ipagroup.py
index 10a61144..c1ffea3e 100644
--- a/plugins/modules/ipagroup.py
+++ b/plugins/modules/ipagroup.py
@@ -141,7 +141,7 @@ RETURN = """
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
- api_check_param, module_params_get
+ api_check_param, module_params_get, gen_add_del_lists
def find_group(module, name):
@@ -317,24 +317,14 @@ def main():
if not compare_args_ipa(ansible_module, member_args,
res_find):
# Generate addition and removal lists
- user_add = list(
- set(user or []) -
- set(res_find.get("member_user", [])))
- user_del = list(
- set(res_find.get("member_user", [])) -
- set(user or []))
- group_add = list(
- set(group or []) -
- set(res_find.get("member_group", [])))
- group_del = list(
- set(res_find.get("member_group", [])) -
- set(group or []))
- service_add = list(
- set(service or []) -
- set(res_find.get("member_service", [])))
- service_del = list(
- set(res_find.get("member_service", [])) -
- set(service or []))
+ user_add, user_del = gen_add_del_lists(
+ user, res_find.get("member_user"))
+
+ group_add, group_del = gen_add_del_lists(
+ group, res_find.get("member_group"))
+
+ service_add, service_del = gen_add_del_lists(
+ service, res_find.get("member_service"))
if has_add_member_service:
# Add members
diff --git a/plugins/modules/ipahbacrule.py b/plugins/modules/ipahbacrule.py
index fd0ce238..452f9f8c 100644
--- a/plugins/modules/ipahbacrule.py
+++ b/plugins/modules/ipahbacrule.py
@@ -159,7 +159,7 @@ RETURN = """
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
- module_params_get
+ module_params_get, gen_add_del_lists
def find_hbacrule(module, name):
@@ -342,44 +342,24 @@ def main():
res_find = {}
# Generate addition and removal lists
- host_add = list(
- set(host or []) -
- set(res_find.get("memberhost_host", [])))
- host_del = list(
- set(res_find.get("memberhost_host", [])) -
- set(host or []))
- hostgroup_add = list(
- set(hostgroup or []) -
- set(res_find.get("memberhost_hostgroup", [])))
- hostgroup_del = list(
- set(res_find.get("memberhost_hostgroup", [])) -
- set(hostgroup or []))
-
- hbacsvc_add = list(
- set(hbacsvc or []) -
- set(res_find.get("memberservice_hbacsvc", [])))
- hbacsvc_del = list(
- set(res_find.get("memberservice_hbacsvc", [])) -
- set(hbacsvc or []))
- hbacsvcgroup_add = list(
- set(hbacsvcgroup or []) -
- set(res_find.get("memberservice_hbacsvcgroup", [])))
- hbacsvcgroup_del = list(
- set(res_find.get("memberservice_hbacsvcgroup", [])) -
- set(hbacsvcgroup or []))
-
- user_add = list(
- set(user or []) -
- set(res_find.get("memberuser_user", [])))
- user_del = list(
- set(res_find.get("memberuser_user", [])) -
- set(user or []))
- group_add = list(
- set(group or []) -
- set(res_find.get("memberuser_group", [])))
- group_del = list(
- set(res_find.get("memberuser_group", [])) -
- set(group or []))
+ host_add, host_del = gen_add_del_lists(
+ host, res_find.get("memberhost_host"))
+
+ hostgroup_add, hostgroup_del = gen_add_del_lists(
+ hostgroup, res_find.get("memberhost_hostgroup"))
+
+ hbacsvc_add, hbacsvc_del = gen_add_del_lists(
+ hbacsvc, res_find.get("memberservice_hbacsvc"))
+
+ hbacsvcgroup_add, hbacsvcgroup_del = gen_add_del_lists(
+ hbacsvcgroup,
+ res_find.get("memberservice_hbacsvcgroup"))
+
+ user_add, user_del = gen_add_del_lists(
+ user, res_find.get("memberuser_user"))
+
+ group_add, group_del = gen_add_del_lists(
+ group, res_find.get("memberuser_group"))
# Add hosts and hostgroups
if len(host_add) > 0 or len(hostgroup_add) > 0:
diff --git a/plugins/modules/ipahbacsvcgroup.py b/plugins/modules/ipahbacsvcgroup.py
index 3b9132a0..d55dc138 100644
--- a/plugins/modules/ipahbacsvcgroup.py
+++ b/plugins/modules/ipahbacsvcgroup.py
@@ -104,7 +104,8 @@ RETURN = """
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils._text import to_text
from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
- temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa
+ temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
+ gen_add_del_lists
def find_hbacsvcgroup(module, name):
@@ -249,12 +250,8 @@ def main():
if not compare_args_ipa(ansible_module, member_args,
res_find):
# Generate addition and removal lists
- hbacsvc_add = list(
- set(hbacsvc or []) -
- set(res_find.get("member_hbacsvc", [])))
- hbacsvc_del = list(
- set(res_find.get("member_hbacsvc", [])) -
- set(hbacsvc or []))
+ hbacsvc_add, hbacsvc_del = gen_add_del_lists(
+ hbacsvc, res_find.get("member_hbacsvc"))
# Add members
if len(hbacsvc_add) > 0:
diff --git a/plugins/modules/ipahostgroup.py b/plugins/modules/ipahostgroup.py
index 5fcca1d6..7e1891d3 100644
--- a/plugins/modules/ipahostgroup.py
+++ b/plugins/modules/ipahostgroup.py
@@ -117,7 +117,7 @@ RETURN = """
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
- module_params_get
+ module_params_get, gen_add_del_lists
def find_hostgroup(module, name):
@@ -268,18 +268,11 @@ def main():
if not compare_args_ipa(ansible_module, member_args,
res_find):
# Generate addition and removal lists
- host_add = list(
- set(host or []) -
- set(res_find.get("member_host", [])))
- host_del = list(
- set(res_find.get("member_host", [])) -
- set(host or []))
- hostgroup_add = list(
- set(hostgroup or []) -
- set(res_find.get("member_hostgroup", [])))
- hostgroup_del = list(
- set(res_find.get("member_hostgroup", [])) -
- set(hostgroup or []))
+ host_add, host_del = gen_add_del_lists(
+ host, res_find.get("member_host"))
+
+ hostgroup_add, hostgroup_del = gen_add_del_lists(
+ hostgroup, res_find.get("member_hostgroup"))
# Add members
if len(host_add) > 0 or len(hostgroup_add) > 0:
diff --git a/plugins/modules/ipasudocmdgroup.py b/plugins/modules/ipasudocmdgroup.py
index 3cbb2803..a5b0e4e4 100644
--- a/plugins/modules/ipasudocmdgroup.py
+++ b/plugins/modules/ipasudocmdgroup.py
@@ -110,7 +110,8 @@ RETURN = """
from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils._text import to_text
from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
- temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa
+ temp_kdestroy, valid_creds, api_connect, api_command, compare_args_ipa, \
+ gen_add_del_lists
def find_sudocmdgroup(module, name):
@@ -257,12 +258,10 @@ def main():
if not compare_args_ipa(ansible_module, member_args,
res_find):
# Generate addition and removal lists
- sudocmdgroup_add = list(
- set(sudocmdgroup or []) -
- set(res_find.get("member_sudocmdgroup", [])))
- sudocmdgroup_del = list(
- set(res_find.get("member_sudocmdgroup", [])) -
- set(sudocmdgroup or []))
+ sudocmdgroup_add, sudocmdgroup_del = \
+ gen_add_del_lists(
+ sudocmdgroup,
+ res_find.get("member_sudocmdgroup"))
# Add members
if len(sudocmdgroup_add) > 0:
diff --git a/plugins/modules/ipauser.py b/plugins/modules/ipauser.py
index 1867ea7b..d854ba5a 100644
--- a/plugins/modules/ipauser.py
+++ b/plugins/modules/ipauser.py
@@ -467,7 +467,7 @@ from ansible.module_utils._text import to_text
from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
temp_kdestroy, valid_creds, api_connect, api_command, date_format, \
compare_args_ipa, module_params_get, api_check_param, api_get_realm, \
- api_command_no_name
+ api_command_no_name, gen_add_del_lists
import six
@@ -1063,36 +1063,21 @@ def main():
# certmapdata
if res_find is not None:
# Generate addition and removal lists
- manager_add = list(
- set(manager or []) -
- set(res_find.get("manager", [])))
- manager_del = list(
- set(res_find.get("manager", [])) -
- set(manager or []))
- principal_add = list(
- set(principal or []) -
- set(res_find.get("krbprincipalname", [])))
- principal_del = list(
- set(res_find.get("krbprincipalname", [])) -
- set(principal or []))
+ manager_add, manager_del = gen_add_del_lists(
+ manager, res_find.get("manager"))
+ principal_add, principal_del = gen_add_del_lists(
+ principal, res_find.get("krbprincipalname"))
# Principals are not returned as utf8 for IPA using
# python2 using user_find, therefore we need to
# convert the principals that we should remove.
principal_del = [to_text(x) for x in principal_del]
- certificate_add = list(
- set(certificate or []) -
- set(res_find.get("certificate", [])))
- certificate_del = list(
- set(res_find.get("certificate", [])) -
- set(certificate or []))
- certmapdata_add = list(
- set(certmapdata or []) -
- set(res_find.get("ipaCertMapData", [])))
- certmapdata_del = list(
- set(res_find.get("ipaCertMapData", [])) -
- set(certmapdata or []))
+ certificate_add, certificate_del = gen_add_del_lists(
+ certificate, res_find.get("usercertificate"))
+
+ certmapdata_add, certmapdata_del = gen_add_del_lists(
+ certmapdata, res_find.get("ipaCertMapData"))
else:
# Use given managers and principals
diff --git a/tests/user/test_user.yml b/tests/user/test_user.yml
index 541ea7cf..cca6b489 100644
--- a/tests/user/test_user.yml
+++ b/tests/user/test_user.yml
@@ -87,8 +87,8 @@
name: pinky
first: pinky
last: Acme
- #manager: manager1,manager2,manager3
- #principal: pa,pa1,pa3
+ manager: []
+ principal: []
sshpubkey:
- ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCqmVDpEX5gnSjKuv97AyzOhaUMMKz8ahOA3GY77tVC4o68KNgMCmDSEG1/kOIaElngNLaCha3p/2iAcU9Bi1tLKUlm2bbO5NHNwHfRxY/3cJtq+/7D1vxJzqThYwI4F9vr1WxyY2+mMTv3pXbfAJoR8Mu06XaEY5PDetlDKjHLuNWF+/O7ZU8PsULTa1dJZFrtXeFpmUoLoGxQBvlrlcPI1zDciCSU24t27Zan5Py2l5QchyI7yhCyMM77KDtj5+AFVpmkb9+zq50rYJAyFVeyUvwjzErvQrKJzYpA0NyBp7vskWbt36M16/M/LxEK7HA6mkcakO3ESWx5MT1LAjvdlnxbWG3787MxweHXuB8CZU+9bZPFBaJ+VQtOfJ7I8eH0S16moPC4ak8FlcFvOH8ERDPWLFDqfy09yaZ7bVIF0//5ZI7Nf3YDe3S7GrBX5ieYuECyP6UNkTx9BRsAQeVvXEc6otzB7iCSnYBMGUGzCqeigoAWaVQUONsSR3Uatks= pinky@ipaserver.el81.local
- ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDc8MIjaSrxLYHvu+hduoF4m6NUFSlXZWzYbd3BK4L47/U4eiXoOS6dcfuZJDjmLfOipc7XVp7NADwAgA1yBOAjbeVpXr2tC8w8saZibl75WBOEjDfNroiOh/f/ojrwwHg05QTVSZHs27sU1HBPyCQM/FHVM6EnRfmyiBkEBA/3ca0PJ9UJhWb2XisCaz6y6QcTh4gQnvHzgmEmK31GwiKnmBSEQuj8P5NGCO8RlN3cq3zpRpMDEoBRCjQYicllf/5P43r5OGvS1LhTiAMfyqE37URezNQa7aozBpH1GhIwAmjAtm84jXQjxUgZPYC0aSLuADYErScOP4792r6koH9t/DM5/M+jG2c4PNWynDczUw6Eaxl5E3hU0Ee9UN0Oee7iBnVenS/QMeZNyo5lMA/HXT5lrYiJGTYM0shRjGXXYBbJZhWerguSWDAdUd1gvuGP1nb7/+/Cvb46+HX7zYouS5Ojo0yPzMZ07X142jnKAfx9LnKdMUCwBJzbtoJ91Zc= pinky@ipaserver.el81.local
diff --git a/tests/user/test_users.yml b/tests/user/test_users.yml
index d66115cb..5b5d4538 100644
--- a/tests/user/test_users.yml
+++ b/tests/user/test_users.yml
@@ -205,8 +205,8 @@
name: pinky
first: pinky
last: Acme
- #manager: manager1,manager2,manager3
- #principal: pa,pa1,pa3
+ manager: []
+ principal: []
sshpubkey:
- ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCqmVDpEX5gnSjKuv97AyzOhaUMMKz8ahOA3GY77tVC4o68KNgMCmDSEG1/kOIaElngNLaCha3p/2iAcU9Bi1tLKUlm2bbO5NHNwHfRxY/3cJtq+/7D1vxJzqThYwI4F9vr1WxyY2+mMTv3pXbfAJoR8Mu06XaEY5PDetlDKjHLuNWF+/O7ZU8PsULTa1dJZFrtXeFpmUoLoGxQBvlrlcPI1zDciCSU24t27Zan5Py2l5QchyI7yhCyMM77KDtj5+AFVpmkb9+zq50rYJAyFVeyUvwjzErvQrKJzYpA0NyBp7vskWbt36M16/M/LxEK7HA6mkcakO3ESWx5MT1LAjvdlnxbWG3787MxweHXuB8CZU+9bZPFBaJ+VQtOfJ7I8eH0S16moPC4ak8FlcFvOH8ERDPWLFDqfy09yaZ7bVIF0//5ZI7Nf3YDe3S7GrBX5ieYuECyP6UNkTx9BRsAQeVvXEc6otzB7iCSnYBMGUGzCqeigoAWaVQUONsSR3Uatks= pinky@ipaserver.el81.local
- ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDc8MIjaSrxLYHvu+hduoF4m6NUFSlXZWzYbd3BK4L47/U4eiXoOS6dcfuZJDjmLfOipc7XVp7NADwAgA1yBOAjbeVpXr2tC8w8saZibl75WBOEjDfNroiOh/f/ojrwwHg05QTVSZHs27sU1HBPyCQM/FHVM6EnRfmyiBkEBA/3ca0PJ9UJhWb2XisCaz6y6QcTh4gQnvHzgmEmK31GwiKnmBSEQuj8P5NGCO8RlN3cq3zpRpMDEoBRCjQYicllf/5P43r5OGvS1LhTiAMfyqE37URezNQa7aozBpH1GhIwAmjAtm84jXQjxUgZPYC0aSLuADYErScOP4792r6koH9t/DM5/M+jG2c4PNWynDczUw6Eaxl5E3hU0Ee9UN0Oee7iBnVenS/QMeZNyo5lMA/HXT5lrYiJGTYM0shRjGXXYBbJZhWerguSWDAdUd1gvuGP1nb7/+/Cvb46+HX7zYouS5Ojo0yPzMZ07X142jnKAfx9LnKdMUCwBJzbtoJ91Zc= pinky@ipaserver.el81.local
--
GitLab