From 209c6365ea654e49a204ea7628bb0ec394e971dc Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Thu, 2 Feb 2023 15:37:37 +0100 Subject: [PATCH 1/6] utils/new_module: Fix github_user test new_module was always failing with "github_user is not valid". The wrong variable was checked: $githubuser instead of $github_user. --- utils/new_module | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/new_module b/utils/new_module index 2b681966..60b50925 100755 --- a/utils/new_module +++ b/utils/new_module @@ -78,7 +78,7 @@ if [ -z "$name" ] || [ -z "$author" ] || [ -z "$email" ] || [ -z "$github_user" [ -z "$name" ] && echo "ERROR: name is not valid" [ -z "$author" ] && echo "ERROR: author is not valid" [ -z "$email" ] && echo "ERROR: email is not valid" - [ -z "$githubuser" ] && echo "ERROR: github_user is not valid" + [ -z "$github_user" ] && echo "ERROR: github_user is not valid" echo usage; exit 1; From 35ded3bf532a48f4db6475c107ac9bb90c766c96 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Wed, 8 Feb 2023 15:41:01 -0300 Subject: [PATCH 2/6] utils/new_module: Ensure correct number of parameters for new_module When testing the number parameters for new_module, the `github_user` was not being taken into account. --- utils/new_module | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/new_module b/utils/new_module index 60b50925..58e47d06 100755 --- a/utils/new_module +++ b/utils/new_module @@ -63,7 +63,7 @@ for (( i=0; i Date: Wed, 12 Apr 2023 12:50:55 +0200 Subject: [PATCH 3/6] utils/templates/ipamodule.py.in: Add missing bracket The parameter argument spec of name was missing the closing bracket. The bracket has been added. --- utils/templates/ipamodule.py.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/templates/ipamodule.py.in b/utils/templates/ipamodule.py.in index 833d5a88..aa52257b 100644 --- a/utils/templates/ipamodule.py.in +++ b/utils/templates/ipamodule.py.in @@ -121,7 +121,7 @@ def main(): argument_spec=dict( # general name=dict(type="list", elements="str", required=True, - aliases=["API_PARAMETER_NAME"], + aliases=["API_PARAMETER_NAME"]), # present PARAMETER1=dict(required=False, type='str', aliases=["API_PARAMETER_NAME"], default=None), From 4a18ad03c806f5dc506f028c821646f8d1c00c75 Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Wed, 12 Apr 2023 12:52:14 +0200 Subject: [PATCH 4/6] utils/templates/{README*.md.in,test_module*.yml.in}: Use true and false The values "yes" and "no" will not be valid in the future for bool parameters. Therefore "yes" and "no" have been replaced by "true" and "false". --- utils/templates/README-module+member.md.in | 10 +++++----- utils/templates/README-module.md.in | 6 +++--- utils/templates/test_module+member.yml.in | 4 ++-- utils/templates/test_module.yml.in | 4 ++-- utils/templates/test_module_client_context.yml.in | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/utils/templates/README-module+member.md.in b/utils/templates/README-module+member.md.in index 6fda8f94..7ee97d9c 100644 --- a/utils/templates/README-module+member.md.in +++ b/utils/templates/README-module+member.md.in @@ -45,7 +45,7 @@ Example playbook to make sure $name "NAME" is present: --- - name: Playbook to manage IPA $name. hosts: ipaserver - become: no + become: false tasks: - ipa$name: @@ -60,7 +60,7 @@ Example playbook to make sure $name "NAME" member PARAMETER2 VALUE is present: --- - name: Playbook to manage IPA $name PARAMETER2 member. hosts: ipaserver - become: no + become: false tasks: - ipa$name: @@ -78,7 +78,7 @@ Example playbook to make sure $name "NAME" member PARAMETER2 VALUE is absent: --- - name: Playbook to manage IPA $name PARAMETER2 member. hosts: ipaserver - become: no + become: false tasks: - ipa$name: @@ -96,7 +96,7 @@ Example playbook to make sure $name "NAME" is absent: --- - name: Playbook to manage IPA $name. hosts: ipaserver - become: no + become: false tasks: - ipa$name: @@ -117,7 +117,7 @@ Variable | Description | Required `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no `ipaadmin_password` | The admin password is a string and is required if there is no admin ticket available on the node | no `ipaapi_context` | The context in which the module will execute. Executing in a server context is preferred. If not provided context will be determined by the execution environment. Valid values are `server` and `client`. | no -`ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to yes. (bool) | no +`ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to true. (bool) | no `name` \| `ALIAS` | The list of $name name strings. | yes `PARAMETER1` \| `API_PARAMETER_NAME` | DESCRIPTION | TYPE `PARAMETER2` \| `API_PARAMETER_NAME` | DESCRIPTION | TYPE diff --git a/utils/templates/README-module.md.in b/utils/templates/README-module.md.in index 08faf505..1b8635cd 100644 --- a/utils/templates/README-module.md.in +++ b/utils/templates/README-module.md.in @@ -45,7 +45,7 @@ Example playbook to make sure $name "NAME" is present: --- - name: Playbook to manage IPA $name. hosts: ipaserver - become: no + become: false tasks: - ipa$name: @@ -61,7 +61,7 @@ Example playbook to make sure $name "NAME" is absent: --- - name: Playbook to manage IPA $name. hosts: ipaserver - become: no + become: false tasks: - ipa$name: @@ -82,7 +82,7 @@ Variable | Description | Required `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no `ipaadmin_password` | The admin password is a string and is required if there is no admin ticket available on the node | no `ipaapi_context` | The context in which the module will execute. Executing in a server context is preferred. If not provided context will be determined by the execution environment. Valid values are `server` and `client`. | no -`ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to yes. (bool) | no +`ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to true. (bool) | no `name` \| `ALIAS` | The list of $name name strings. | yes `PARAMETER1` \| `API_PARAMETER_NAME` | DESCRIPTION | TYPE `PARAMETER2` \| `API_PARAMETER_NAME` | DESCRIPTION | TYPE diff --git a/utils/templates/test_module+member.yml.in b/utils/templates/test_module+member.yml.in index 01012078..cafbbf64 100644 --- a/utils/templates/test_module+member.yml.in +++ b/utils/templates/test_module+member.yml.in @@ -3,8 +3,8 @@ hosts: "{{ ipa_test_host | default('ipaserver') }}" # Change "become" or "gather_facts" to "yes", # if you test playbook requires any. - become: no - gather_facts: no + become: false + gather_facts: false tasks: diff --git a/utils/templates/test_module.yml.in b/utils/templates/test_module.yml.in index 2ad53cc2..b7e82fbe 100644 --- a/utils/templates/test_module.yml.in +++ b/utils/templates/test_module.yml.in @@ -3,8 +3,8 @@ hosts: "{{ ipa_test_host | default('ipaserver') }}" # Change "become" or "gather_facts" to "yes", # if you test playbook requires any. - become: no - gather_facts: no + become: false + gather_facts: false tasks: diff --git a/utils/templates/test_module_client_context.yml.in b/utils/templates/test_module_client_context.yml.in index ee8c6789..38ac7ffb 100644 --- a/utils/templates/test_module_client_context.yml.in +++ b/utils/templates/test_module_client_context.yml.in @@ -3,8 +3,8 @@ hosts: ipaclients, ipaserver # Change "become" or "gather_facts" to "yes", # if you test playbook requires any. - become: no - gather_facts: no + become: false + gather_facts: false tasks: - name: Include FreeIPA facts. From 47d5211185691af7d423381d365b2dfa1cba2e75 Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Wed, 12 Apr 2023 13:08:12 +0200 Subject: [PATCH 5/6] utils/templates/test_module*.yml.in: Better docs for become and gather_facts The documentation for "become" and "gather_facts" has been updated to make sure that these parameters are enabled only in new tests if it is really needed. --- utils/templates/test_module+member.yml.in | 5 +++-- utils/templates/test_module.yml.in | 5 +++-- utils/templates/test_module_client_context.yml.in | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/utils/templates/test_module+member.yml.in b/utils/templates/test_module+member.yml.in index cafbbf64..e92ac9f4 100644 --- a/utils/templates/test_module+member.yml.in +++ b/utils/templates/test_module+member.yml.in @@ -1,9 +1,10 @@ --- - name: Test $name hosts: "{{ ipa_test_host | default('ipaserver') }}" - # Change "become" or "gather_facts" to "yes", - # if you test playbook requires any. + # It is normally not needed to set "become" to "true" for a module test. + # Only set it to true if it is needed to execute commands as root. become: false + # Enable "gather_facts" only if "ansible_facts" variable needs to be used. gather_facts: false tasks: diff --git a/utils/templates/test_module.yml.in b/utils/templates/test_module.yml.in index b7e82fbe..62e0a79b 100644 --- a/utils/templates/test_module.yml.in +++ b/utils/templates/test_module.yml.in @@ -1,9 +1,10 @@ --- - name: Test $name hosts: "{{ ipa_test_host | default('ipaserver') }}" - # Change "become" or "gather_facts" to "yes", - # if you test playbook requires any. + # It is normally not needed to set "become" to "true" for a module test. + # Only set it to true if it is needed to execute commands as root. become: false + # Enable "gather_facts" only if "ansible_facts" variable needs to be used. gather_facts: false tasks: diff --git a/utils/templates/test_module_client_context.yml.in b/utils/templates/test_module_client_context.yml.in index 38ac7ffb..193cd6f3 100644 --- a/utils/templates/test_module_client_context.yml.in +++ b/utils/templates/test_module_client_context.yml.in @@ -1,9 +1,10 @@ --- - name: Test ${name} hosts: ipaclients, ipaserver - # Change "become" or "gather_facts" to "yes", - # if you test playbook requires any. + # It is normally not needed to set "become" to "true" for a module test. + # Only set it to true if it is needed to execute commands as root. become: false + # Enable "gather_facts" only if "ansible_facts" variable needs to be used. gather_facts: false tasks: From 1c8f1c28e1409bb6a6c30ed52923fb54256cce5c Mon Sep 17 00:00:00 2001 From: Thomas Woerner Date: Wed, 12 Apr 2023 13:11:50 +0200 Subject: [PATCH 6/6] utils/templates/test_module*.yml.in: Use generic module_defaults The usage of module_defaults allows to reduce the size of the tests and to have the needed information in the tasks only. The default values for the parameters are automatically passed to the module by Ansible. It is not possible to use a module group for module_defaults as this could only be done with Ansible Collections. The tests are also used upstream and downstream without a collection. Without groups of a collection it is needed to add the defaults for all modules separately. Simple example: module_defaults: ipahost: ipaadmin_password: SomeADMINpassword ipaapi_context: "{{ ipa_context | default(omit) }}" Several module example using YAML anchors and aliases: module_defaults: ipahost: &ipa_module_defaults ipaadmin_password: SomeADMINpassword ipaapi_context: "{{ ipa_context | default(omit) }}" ipauser: *ipa_module_defaults ipagroup: *ipa_module_defaults --- utils/templates/test_module+member.yml.in | 23 ++++------------------- utils/templates/test_module.yml.in | 14 ++++---------- 2 files changed, 8 insertions(+), 29 deletions(-) diff --git a/utils/templates/test_module+member.yml.in b/utils/templates/test_module+member.yml.in index e92ac9f4..5517b6ae 100644 --- a/utils/templates/test_module+member.yml.in +++ b/utils/templates/test_module+member.yml.in @@ -6,6 +6,10 @@ become: false # Enable "gather_facts" only if "ansible_facts" variable needs to be used. gather_facts: false + module_defaults: + ipa$name: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" tasks: @@ -13,7 +17,6 @@ - name: Ensure $name NAME is absent ipa$name: - ipaadmin_password: SomeADMINpassword name: NAME state: absent @@ -23,8 +26,6 @@ - name: Ensure $name NAME is present ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME # Add needed parameters here register: result @@ -32,8 +33,6 @@ - name: Ensure $name NAME is present again ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME # Add needed parameters here register: result @@ -41,8 +40,6 @@ - name: Ensure $name NAME member PARAMETER2 VALUE is present ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME PARAMETER2: VALUE action: member @@ -51,8 +48,6 @@ - name: Ensure $name NAME member PARAMETER2 VALUE is present again ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME PARAMETER2: VALUE action: member @@ -61,8 +56,6 @@ - name: Ensure $name NAME member PARAMETER2 VALUE is absent ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME PARAMETER2: VALUE action: member @@ -72,8 +65,6 @@ - name: Ensure $name NAME member PARAMETER2 VALUE is absent again ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME PARAMETER2: VALUE action: member @@ -85,8 +76,6 @@ - name: Ensure $name NAME is absent ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME state: absent register: result @@ -94,8 +83,6 @@ - name: Ensure $name NAME is absent again ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME state: absent register: result @@ -105,7 +92,5 @@ - name: Ensure $name NAME is absent ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME state: absent diff --git a/utils/templates/test_module.yml.in b/utils/templates/test_module.yml.in index 62e0a79b..f135cfd0 100644 --- a/utils/templates/test_module.yml.in +++ b/utils/templates/test_module.yml.in @@ -6,6 +6,10 @@ become: false # Enable "gather_facts" only if "ansible_facts" variable needs to be used. gather_facts: false + module_defaults: + ipa$name: + ipaadmin_password: SomeADMINpassword + ipaapi_context: "{{ ipa_context | default(omit) }}" tasks: @@ -13,8 +17,6 @@ - name: Ensure $name NAME is absent ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME state: absent @@ -24,7 +26,6 @@ - name: Ensure $name NAME is present ipa$name: - ipaadmin_password: SomeADMINpassword name: NAME # Add needed parameters here register: result @@ -32,8 +33,6 @@ - name: Ensure $name NAME is present again ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME # Add needed parameters here register: result @@ -43,8 +42,6 @@ - name: Ensure $name NAME is absent ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME state: absent register: result @@ -52,8 +49,6 @@ - name: Ensure $name NAME is absent again ipa$name: - ipaadmin_password: SomeADMINpassword - ipaapi_context: "{{ ipa_context | default(omit) }}" name: NAME state: absent register: result @@ -63,6 +58,5 @@ - name: Ensure $name NAME is absent ipa$name: - ipaadmin_password: SomeADMINpassword name: NAME state: absent