From 8772379dccccc7919d5756c0bb96aa53752e99c9 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 3 Feb 2022 15:34:17 -0300 Subject: [PATCH 1/3] module templates: Refactor member management. This patch refactors the module template for modules with member management, in a way that the addition of member management command logic is not duplicated in different states or actions. This idiom has been applied recently along with other fixes to modules with idempotence issues reducing the modules code size and centering code logic in specific blocks. --- utils/templates/ipamodule+member.py.in | 53 +++++++++++--------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/utils/templates/ipamodule+member.py.in b/utils/templates/ipamodule+member.py.in index 1515ccc1..f0c2edb9 100644 --- a/utils/templates/ipamodule+member.py.in +++ b/utils/templates/ipamodule+member.py.in @@ -202,6 +202,9 @@ def main(): # Make sure $name exists res_find = find_$name(ansible_module, name) + # add/del lists + PARAMETER2_add, PARAMETER2_del = [], [] + # Create command if state == "present": @@ -228,19 +231,6 @@ def main(): PARAMETER2_add, PARAMETER2_del = gen_add_del_lists( PARAMETER2, res_find.get("member_PARAMETER2")) - # Add members - if len(PARAMETER2_add) > 0: - commands.append([name, "$name_add_member", - { - "PARAMETER2": PARAMETER2_add, - }]) - # Remove members - if len(PARAMETER2_del) > 0: - commands.append([name, "$name_remove_member", - { - "PARAMETER2": PARAMETER2_del, - }]) - elif action == "member": if res_find is None: ansible_module.fail_json( @@ -248,16 +238,10 @@ def main(): # Reduce add lists for PARAMETER2 # to new entries only that are not in res_find. - if PARAMETER2 is not None and \ - "API_PARAMETER2_NAME" in res_find: - PARAMETER2 = gen_add_list( - PARAMETER2, res_find["API_PARAMETER2_NAME"]) - if PARAMETER2 is not None: - commands.append([name, "$name_add_member", - { - "PARAMETER2": PARAMETER2, - }]) + PARAMETER2_add = gen_add_list( + PARAMETER2, res_find.get("member_PARAMETER2")) + elif state == "absent": if action == "$name": @@ -272,18 +256,27 @@ def main(): # Reduce del lists of member_host and member_hostgroup, # to the entries only that are in res_find. if PARAMETER2 is not None: - PARAMETER2 = gen_intersection_list( - PARAMETER2, res_find.get("API_PARAMETER2_NAME")) - - if PARAMETER2 is not None: - commands.append([name, "$name_remove_member", - { - "PARAMETER2": PARAMETER2, - }]) + PARAMETER2_del = gen_intersection_list( + PARAMETER2, res_find.get("member_PARAMETER2")) else: ansible_module.fail_json(msg="Unkown state '%s'" % state) + # Member management + + # Add members + if PARAMETER2_add: + commands.append([name, "$name_add_member", + { + "PARAMETER2": PARAMETER2_add, + }]) + + # Remove members + if PARAMETER2_del: + commands.append([name, "$name_remove_member", + { + "PARAMETER2": PARAMETER2_del, + }]) # Execute commands From 5d6324e2daa6488894f51c8f222130e5ba1451d2 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 3 Feb 2022 15:42:04 -0300 Subject: [PATCH 2/3] module templates: Add example and note for case insensitive members. Some modules should be compared in a case insensitive manner, and this patch adds an example of a call to IPAAnsibleModule.params_get_lowercase and a note on its usage. --- utils/templates/ipamodule+member.py.in | 5 ++++- utils/templates/ipamodule.py.in | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/utils/templates/ipamodule+member.py.in b/utils/templates/ipamodule+member.py.in index f0c2edb9..90152502 100644 --- a/utils/templates/ipamodule+member.py.in +++ b/utils/templates/ipamodule+member.py.in @@ -163,7 +163,10 @@ def main(): # present PARAMETER1 = ansible_module.params_get("PARAMETER1") - PARAMETER2 = ansible_module.params_get("PARAMETER2") + # Note: some parameters must be compared in a case insensitive way, + # or are transliterated into its lowercase version by IPA API. For + # these parameters, use IPAAnsibleModule.params_get_lowercase. + PARAMETER2 = ansible_module.params_get_lowercase("PARAMETER2") action = ansible_module.params_get("action") # state diff --git a/utils/templates/ipamodule.py.in b/utils/templates/ipamodule.py.in index 00750f83..b66be523 100644 --- a/utils/templates/ipamodule.py.in +++ b/utils/templates/ipamodule.py.in @@ -135,7 +135,10 @@ def main(): # present PARAMETER1 = ansible_module.params_get("PARAMETER1") - PARAMETER2 = ansible_module.params_get("PARAMETER2") + # Note: some parameters must be compared in a case insensitive way, + # or are transliterated into its lowercase version by IPA API. For + # these parameters, use IPAAnsibleModule.params_get_lowercase. + PARAMETER2 = ansible_module.params_get_lowercase("PARAMETER2") # state state = ansible_module.params_get("state") From 4df2cab42a40e0663d2cfdab297117e96d78e49a Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Thu, 17 Feb 2022 15:22:18 -0300 Subject: [PATCH 3/3] module templates: Add delete_commit code template. This patch add the lines necessary to allow the use of the attribute `delete_continue`, as it is a commom attribute, and if newer commom attributes are added to IPAAnsibleModule in the future, the usage will be similar. --- utils/templates/ipamodule+member.py.in | 10 +++++++++- utils/templates/ipamodule.py.in | 8 +++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/utils/templates/ipamodule+member.py.in b/utils/templates/ipamodule+member.py.in index 90152502..dc682cf9 100644 --- a/utils/templates/ipamodule+member.py.in +++ b/utils/templates/ipamodule+member.py.in @@ -36,6 +36,7 @@ short description: Manage FreeIPA $name description: Manage FreeIPA $name and $name members extends_documentation_fragment: - ipamodule_base_docs + - ipamoudle_base_docs.delete_continue options: name: description: The list of $name name strings. @@ -152,6 +153,7 @@ def main(): choices=["present", "absent"]), ), supports_check_mode=True, + ipa_module_options=["delete_continue"] ) ansible_module._ansible_debug = True @@ -169,6 +171,8 @@ def main(): PARAMETER2 = ansible_module.params_get_lowercase("PARAMETER2") action = ansible_module.params_get("action") + delete_continue = ansible_module.params_get("delete_continue") + # state state = ansible_module.params_get("state") @@ -249,7 +253,9 @@ def main(): elif state == "absent": if action == "$name": if res_find is not None: - commands.append([name, "$name_del", {}]) + commands.append( + [name, "$name_del", {"continue": delete_continue}] + ) elif action == "member": if res_find is None: @@ -275,10 +281,12 @@ def main(): }]) # Remove members + if PARAMETER2_del: commands.append([name, "$name_remove_member", { "PARAMETER2": PARAMETER2_del, + "continue": delete_continue, }]) # Execute commands diff --git a/utils/templates/ipamodule.py.in b/utils/templates/ipamodule.py.in index b66be523..07fe2ac4 100644 --- a/utils/templates/ipamodule.py.in +++ b/utils/templates/ipamodule.py.in @@ -36,6 +36,7 @@ short description: Manage FreeIPA $name description: Manage FreeIPA $name extends_documentation_fragment: - ipamodule_base_docs + - ipamodule_base_docs.delete_continue options: name: description: The list of $name name strings. @@ -124,6 +125,7 @@ def main(): choices=["present", "absent"]), ), supports_check_mode=True, + ipa_module_options=["delete_continue"], ) ansible_module._ansible_debug = True @@ -140,6 +142,8 @@ def main(): # these parameters, use IPAAnsibleModule.params_get_lowercase. PARAMETER2 = ansible_module.params_get_lowercase("PARAMETER2") + delete_continue = ansible_module.params_get("delete_continue") + # state state = ansible_module.params_get("state") @@ -191,7 +195,9 @@ def main(): elif state == "absent": if res_find is not None: - commands.append([name, "$name_del", {}]) + commands.append( + [name, "$name_del", {"continue": delete_continue}] + ) else: ansible_module.fail_json(msg="Unkown state '%s'" % state)