From c2cdd8e4033dfd1b655a20edb70a3251283b5aa6 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Fri, 21 Aug 2020 12:18:09 -0400 Subject: [PATCH 01/13] Initial commit of test_completeness --- awx_collection/test/awx/test_completeness.py | 158 +++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100755 awx_collection/test/awx/test_completeness.py diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py new file mode 100755 index 0000000000..21fc4a47de --- /dev/null +++ b/awx_collection/test/awx/test_completeness.py @@ -0,0 +1,158 @@ +#!/usr/bin/env python3 + +from awx.main.tests.functional.conftest import _request +import yaml +import os +import re + +# Analysis variables +no_module_for_endpoint = [ 'tower_me' ] +no_endpoint_for_module = [ 'tower_import', 'tower_meta', 'tower_export' ] +ignore_parameters = [ 'state', 'new_name' ] +no_api_parameter_ok = { + 'tower_credential': ['authorize', 'authorize_password', 'become_method', 'become_password', 'become_username', 'client', 'domain', 'host', 'kind', 'password', 'project', 'secret', 'security_token', 'ssh_key_data', 'ssh_key_unlock', 'subscription', 'tenant', 'username', 'vault_id', 'vault_password'], +} + +return_value = 0 + +def determine_state(module_id, endpoint, module, parameter, api_option, module_option): + global return_value + # This is a heiratchal list of things that are ok/failures based on conditions + if module_id in no_module_for_endpoint and module == 'N/A': + return "OK, this endpoing should not have a module" + if module_id in no_endpoint_for_module and endpoint == 'N/A': + return "OK, this module does not require an endpoint" + if module == 'N/A': + return_value = 255 + return 'Failed, missing module' + if endpoint == 'N/A': + return_value = 255 + return 'Failed, why does this module have no endpoint' + if parameter in ignore_parameters: + return "OK, globally ignored parameter" + if api_option == '' and parameter in no_api_parameter_ok.get(module, {}): + return 'OK, no api parameter is ok' + if api_option != module_option: + return_value = 255 + return 'Failed, option mismatch' + if api_option == module_option: + return 'OK' + return_value = 255 + return "Failed, unknown reason" + +def test_completeness(collection_import, request, admin_user): + option_comparison = {} + # Load a list of existing module files from disk + base_folder = os.path.abspath( + os.path.join(os.path.dirname(__file__), os.pardir, os.pardir) + ) + module_directory = os.path.join(base_folder, 'plugins', 'modules') + for root, dirs, files in os.walk(module_directory): + if root == module_directory: + for filename in files: + if re.match('^tower_.*\.py$', filename): + module_name = filename[:-3] + option_comparison[module_name] = { + 'endpoint': 'N/A', + 'api_options': {}, + 'module_options': {}, + 'module_name': module_name, + } + resource_module = collection_import('plugins.modules.{0}'.format(module_name)) + option_comparison[module_name]['module_options'] = yaml.load(resource_module.DOCUMENTATION, Loader=yaml.SafeLoader)['options'] + + endpoint_response = _request('get')( + url='/api/v2/', + user=admin_user, + expect=None, + ) + for endpoint in endpoint_response.data.keys(): + # Module names are singular and endpoints are plural so we need to convert to singular + singular_endpoint = '{}'.format(endpoint) + if singular_endpoint.endswith('ies'): + singular_endpoint = singular_endpoint[:-3] + if singular_endpoint != 'settings' and singular_endpoint.endswith('s'): + singular_endpoint = singular_endpoint[:-1] + module_name = 'tower_{}'.format(singular_endpoint) + + endpoint_url = endpoint_response.data.get(endpoint) + + # If we don't have a module for this endpoint then we can create an empty one + if module_name not in option_comparison: + option_comparison[module_name] = {} + option_comparison[module_name]['module_name'] = 'N/A' + option_comparison[module_name]['module_options'] = {} + + # Add in our endpoint and an empty api_options + option_comparison[module_name]['endpoint'] = endpoint_url + option_comparison[module_name]['api_options'] = {} + + # Get out the endpoint, load and parse its options page + options_response = _request('options')( + url=endpoint_url, + user=admin_user, + expect=None, + ) + if 'POST' in options_response.data.get('actions', {}): + option_comparison[module_name]['api_options'] = options_response.data.get('actions').get('POST') + + # Parse through our data to get string lengths to make a pretty report + longest_module_name = 0 + longest_option_name = 0 + longest_endpoint = 0 + for module in option_comparison: + if len(option_comparison[module]['module_name']) > longest_module_name: + longest_module_name = len(option_comparison[module]['module_name']) + if len(option_comparison[module]['endpoint']) > longest_endpoint: + longest_endpoint = len(option_comparison[module]['endpoint']) + for option in option_comparison[module]['api_options'], option_comparison[module]['module_options']: + if len(option) > longest_option_name: + longest_option_name = len(option) + + + # Print out some headers + print( + "End Point", " "*(longest_endpoint-len("End Point")), + " | Module Name", " "*(longest_module_name-len("Module Name")), + " | Option", " "*(longest_option_name-len("Option")), + " | API | Module | State", + sep="" + ) + print("-"*longest_endpoint, "-"*longest_module_name, "-"*longest_option_name, "---", "------", "---------------------------------------------", sep="-|-") + + # Print out all of our data + for module in sorted(option_comparison): + all_param_names = list(set(option_comparison[module]['api_options']) | set(option_comparison[module]['module_options'])) + for parameter in sorted(all_param_names): + print( + option_comparison[module]['endpoint'], " "*(longest_endpoint - len(option_comparison[module]['endpoint'])), " | ", + option_comparison[module]['module_name'], " "*(longest_module_name - len(option_comparison[module]['module_name'])), " | ", + parameter, " "*(longest_option_name - len(parameter)), " | ", + " X " if (parameter in option_comparison[module]['api_options']) else ' ', " | ", + ' X ' if (parameter in option_comparison[module]['module_options']) else ' ', " | ", + determine_state( + module, + option_comparison[module]['endpoint'], + option_comparison[module]['module_name'], + parameter, + 'X' if (parameter in option_comparison[module]['api_options']) else '', + 'X' if (parameter in option_comparison[module]['module_options']) else '', + ), + sep="" + ) + # This handles cases were we got no params from the options page nor from the modules + if len(all_param_names) == 0: + print( + option_comparison[module]['endpoint'], " "*(longest_endpoint - len(option_comparison[module]['endpoint'])), " | ", + option_comparison[module]['module_name'], " "*(longest_module_name - len(option_comparison[module]['module_name'])), " | ", + "N/A", " "*(longest_option_name - len("N/A")), " | ", + ' ', " | ", + ' ', " | ", + determine_state( + module, option_comparison[module]['endpoint'], option_comparison[module]['module_name'], 'N/A', '', '' + ), + sep="" + ) + + if return_value != 0: + raise Exception("One or more failures caused issues") From 85a123376436fe95ef5978e61d2f2fbce75365ac Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 10:16:24 -0400 Subject: [PATCH 02/13] Adding read-only endpoint check, fixing typo, adding needs_development check --- awx_collection/test/awx/test_completeness.py | 47 ++++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index 21fc4a47de..0d87f14025 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -6,20 +6,57 @@ import os import re # Analysis variables -no_module_for_endpoint = [ 'tower_me' ] -no_endpoint_for_module = [ 'tower_import', 'tower_meta', 'tower_export' ] +# ----------------------------------------------------------------------------------------------------------- + +# Read-only endpoints are dynamally created by an options page with no POST section +read_only_endpoint = [] + +# If a module should not be created for an endpoint and the endpoint is not read-only add it here +# THINK HARD ABOUT DOING THIS +no_module_for_endpoint = [] + +# Some modules work on the related fields of an endpoint. These modules will not have an auto-associated endpoint +no_endpoint_for_module = [ + 'tower_import', 'tower_meta', 'tower_export', 'tower_job_launch', 'tower_job_wait', 'tower_job_list', + 'tower_license', 'tower_ping', 'tower_receive', 'tower_send', 'tower_workflow_launch', 'tower_job_cancel', +] + +# Global module parameters we can ignore ignore_parameters = [ 'state', 'new_name' ] + +# Some modules take additional parameters that do not appear in the API +# Add the module name as the key with the value being the list of params to ignore no_api_parameter_ok = { - 'tower_credential': ['authorize', 'authorize_password', 'become_method', 'become_password', 'become_username', 'client', 'domain', 'host', 'kind', 'password', 'project', 'secret', 'security_token', 'ssh_key_data', 'ssh_key_unlock', 'subscription', 'tenant', 'username', 'vault_id', 'vault_password'], + 'tower_credential': [ + 'authorize', 'authorize_password', 'become_method', 'become_password', 'become_username', 'client', + 'domain', 'host', 'kind', 'password', 'project', 'secret', 'security_token', 'ssh_key_data', + 'ssh_key_unlock', 'subscription', 'tenant', 'username', 'vault_id', 'vault_password', + ], + 'tower_project': ['wait', 'timeout'], + 'tower_token': ['existing_token', 'existing_token_id'], + 'tower_settings': ['name', 'settings', 'value'], } +# When this tool was created we were not feature complete. Adding something in here indicates a module +# that needs to be developed. If the module is found on the file system it will auto-detect that the +# work is being done and will bypass this check. At some point this module should be removed from this list. +needs_development = [ + 'tower_ad_hoc_command', 'tower_application', 'tower_instance_group', 'tower_inventory_script', + 'tower_workflow_approval' +] +# ----------------------------------------------------------------------------------------------------------- + return_value = 0 def determine_state(module_id, endpoint, module, parameter, api_option, module_option): global return_value # This is a heiratchal list of things that are ok/failures based on conditions + if module_id in needs_development and module == 'N/A': + return "Failed (non-blocking), needs development" + if module_id in read_only_endpoint and module == 'N/A': + return "OK, this endpoint is read-only and should not have a module" if module_id in no_module_for_endpoint and module == 'N/A': - return "OK, this endpoing should not have a module" + return "OK, this endpoint should not have a module" if module_id in no_endpoint_for_module and endpoint == 'N/A': return "OK, this module does not require an endpoint" if module == 'N/A': @@ -95,6 +132,8 @@ def test_completeness(collection_import, request, admin_user): ) if 'POST' in options_response.data.get('actions', {}): option_comparison[module_name]['api_options'] = options_response.data.get('actions').get('POST') + else: + read_only_endpoint.append(module_name) # Parse through our data to get string lengths to make a pretty report longest_module_name = 0 From 9353e946298473fcfeabbd2573dc62805e13e1d2 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 10:17:26 -0400 Subject: [PATCH 03/13] Spell checking --- awx_collection/test/awx/test_completeness.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index 0d87f14025..f6ee213f53 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -8,7 +8,7 @@ import re # Analysis variables # ----------------------------------------------------------------------------------------------------------- -# Read-only endpoints are dynamally created by an options page with no POST section +# Read-only endpoints are dynamically created by an options page with no POST section read_only_endpoint = [] # If a module should not be created for an endpoint and the endpoint is not read-only add it here @@ -50,7 +50,7 @@ return_value = 0 def determine_state(module_id, endpoint, module, parameter, api_option, module_option): global return_value - # This is a heiratchal list of things that are ok/failures based on conditions + # This is a hierarchical list of things that are ok/failures based on conditions if module_id in needs_development and module == 'N/A': return "Failed (non-blocking), needs development" if module_id in read_only_endpoint and module == 'N/A': From 1ad4c4ab86b94070d39108b427a7ebfbb5fe6040 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 10:29:00 -0400 Subject: [PATCH 04/13] Fixing pep8 issues --- awx_collection/test/awx/test_completeness.py | 63 ++++++++++++-------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index f6ee213f53..9957435b66 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -22,13 +22,13 @@ no_endpoint_for_module = [ ] # Global module parameters we can ignore -ignore_parameters = [ 'state', 'new_name' ] +ignore_parameters = ['state', 'new_name'] # Some modules take additional parameters that do not appear in the API # Add the module name as the key with the value being the list of params to ignore no_api_parameter_ok = { 'tower_credential': [ - 'authorize', 'authorize_password', 'become_method', 'become_password', 'become_username', 'client', + 'authorize', 'authorize_password', 'become_method', 'become_password', 'become_username', 'client', 'domain', 'host', 'kind', 'password', 'project', 'secret', 'security_token', 'ssh_key_data', 'ssh_key_unlock', 'subscription', 'tenant', 'username', 'vault_id', 'vault_password', ], @@ -48,6 +48,7 @@ needs_development = [ return_value = 0 + def determine_state(module_id, endpoint, module, parameter, api_option, module_option): global return_value # This is a hierarchical list of things that are ok/failures based on conditions @@ -77,6 +78,7 @@ def determine_state(module_id, endpoint, module, parameter, api_option, module_o return_value = 255 return "Failed, unknown reason" + def test_completeness(collection_import, request, admin_user): option_comparison = {} # Load a list of existing module files from disk @@ -96,7 +98,10 @@ def test_completeness(collection_import, request, admin_user): 'module_name': module_name, } resource_module = collection_import('plugins.modules.{0}'.format(module_name)) - option_comparison[module_name]['module_options'] = yaml.load(resource_module.DOCUMENTATION, Loader=yaml.SafeLoader)['options'] + option_comparison[module_name]['module_options'] = yaml.load( + resource_module.DOCUMENTATION, + Loader=yaml.SafeLoader + )['options'] endpoint_response = _request('get')( url='/api/v2/', @@ -124,7 +129,7 @@ def test_completeness(collection_import, request, admin_user): option_comparison[module_name]['endpoint'] = endpoint_url option_comparison[module_name]['api_options'] = {} - # Get out the endpoint, load and parse its options page + # Get out the endpoint, load and parse its options page options_response = _request('options')( url=endpoint_url, user=admin_user, @@ -148,48 +153,54 @@ def test_completeness(collection_import, request, admin_user): if len(option) > longest_option_name: longest_option_name = len(option) - # Print out some headers print( - "End Point", " "*(longest_endpoint-len("End Point")), - " | Module Name", " "*(longest_module_name-len("Module Name")), - " | Option", " "*(longest_option_name-len("Option")), + "End Point", " " * (longest_endpoint - len("End Point")), + " | Module Name", " " * (longest_module_name - len("Module Name")), + " | Option", " " * (longest_option_name - len("Option")), " | API | Module | State", sep="" ) - print("-"*longest_endpoint, "-"*longest_module_name, "-"*longest_option_name, "---", "------", "---------------------------------------------", sep="-|-") + print( + "-" * longest_endpoint, + "-" * longest_module_name, + "-" * longest_option_name, + "---", + "------", + "---------------------------------------------", + sep="-|-" + ) # Print out all of our data for module in sorted(option_comparison): - all_param_names = list(set(option_comparison[module]['api_options']) | set(option_comparison[module]['module_options'])) + module_data = option_comparison[module] + all_param_names = list(set(module_data['api_options']) | set(module_data['module_options'])) for parameter in sorted(all_param_names): - print( - option_comparison[module]['endpoint'], " "*(longest_endpoint - len(option_comparison[module]['endpoint'])), " | ", - option_comparison[module]['module_name'], " "*(longest_module_name - len(option_comparison[module]['module_name'])), " | ", - parameter, " "*(longest_option_name - len(parameter)), " | ", - " X " if (parameter in option_comparison[module]['api_options']) else ' ', " | ", - ' X ' if (parameter in option_comparison[module]['module_options']) else ' ', " | ", + print( + module_data['endpoint'], " " * (longest_endpoint - len(module_data['endpoint'])), " | ", + module_data['module_name'], " " * (longest_module_name - len(module_data['module_name'])), " | ", + parameter, " " * (longest_option_name - len(parameter)), " | ", + " X " if (parameter in module_data['api_options']) else ' ', " | ", + ' X ' if (parameter in module_data['module_options']) else ' ', " | ", determine_state( module, - option_comparison[module]['endpoint'], - option_comparison[module]['module_name'], + module_data['endpoint'], + module_data['module_name'], parameter, - 'X' if (parameter in option_comparison[module]['api_options']) else '', - 'X' if (parameter in option_comparison[module]['module_options']) else '', + 'X' if (parameter in module_data['api_options']) else '', + 'X' if (parameter in module_data['module_options']) else '', ), sep="" ) # This handles cases were we got no params from the options page nor from the modules if len(all_param_names) == 0: print( - option_comparison[module]['endpoint'], " "*(longest_endpoint - len(option_comparison[module]['endpoint'])), " | ", - option_comparison[module]['module_name'], " "*(longest_module_name - len(option_comparison[module]['module_name'])), " | ", - "N/A", " "*(longest_option_name - len("N/A")), " | ", + module_data['endpoint'], " " * (longest_endpoint - len(module_data['endpoint'])), " | ", + module_data['module_name'], " " * (longest_module_name - len(module_data['module_name'])), " | ", + "N/A", " " * (longest_option_name - len("N/A")), " | ", ' ', " | ", ' ', " | ", - determine_state( - module, option_comparison[module]['endpoint'], option_comparison[module]['module_name'], 'N/A', '', '' - ), + determine_state(module, module_data['endpoint'], module_data['module_name'], 'N/A', '', ''), sep="" ) From 3b6152a380de91691e7bb584a45fc2b22ffbf2e3 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 11:26:28 -0400 Subject: [PATCH 05/13] Changing job_timeout to an aliase of timeout to match the API --- awx_collection/plugins/modules/tower_project.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/awx_collection/plugins/modules/tower_project.py b/awx_collection/plugins/modules/tower_project.py index 36a4f8666a..bf95e9c583 100644 --- a/awx_collection/plugins/modules/tower_project.py +++ b/awx_collection/plugins/modules/tower_project.py @@ -86,11 +86,13 @@ options: type: bool aliases: - scm_allow_override - job_timeout: + timeout: description: - The amount of time (in seconds) to run before the SCM Update is canceled. A value of 0 means no timeout. default: 0 type: int + aliases: + - job_timeout custom_virtualenv: description: - Local absolute file path containing a custom Python virtualenv to use @@ -194,7 +196,7 @@ def main(): scm_update_on_launch=dict(type='bool', default=False), scm_update_cache_timeout=dict(type='int', default=0), allow_override=dict(type='bool', aliases=['scm_allow_override']), - job_timeout=dict(type='int', default=0), + timeout=dict(type='int', default=0, aliases=['job_timeout']), custom_virtualenv=dict(), organization=dict(required=True), notification_templates_started=dict(type="list", elements='str'), @@ -223,7 +225,7 @@ def main(): scm_update_on_launch = module.params.get('scm_update_on_launch') scm_update_cache_timeout = module.params.get('scm_update_cache_timeout') allow_override = module.params.get('allow_override') - job_timeout = module.params.get('job_timeout') + timeout = module.params.get('timeout') custom_virtualenv = module.params.get('custom_virtualenv') organization = module.params.get('organization') state = module.params.get('state') @@ -276,7 +278,7 @@ def main(): 'scm_refspec': scm_refspec, 'scm_clean': scm_clean, 'scm_delete_on_update': scm_delete_on_update, - 'timeout': job_timeout, + 'timeout': timeout, 'organization': org_id, 'scm_update_on_launch': scm_update_on_launch, 'scm_update_cache_timeout': scm_update_cache_timeout, From 41ca20dc998882cdc4b77d107619e67e68cab2ea Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 11:28:02 -0400 Subject: [PATCH 06/13] Changing scm_credential to credential to match API --- awx_collection/plugins/modules/tower_project.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/awx_collection/plugins/modules/tower_project.py b/awx_collection/plugins/modules/tower_project.py index bf95e9c583..8662029f66 100644 --- a/awx_collection/plugins/modules/tower_project.py +++ b/awx_collection/plugins/modules/tower_project.py @@ -55,10 +55,12 @@ options: - The refspec to use for the SCM resource. type: str default: '' - scm_credential: + credential: description: - Name of the credential to use with this SCM resource. type: str + aliases: + - scm_credential scm_clean: description: - Remove local modifications before updating. @@ -190,7 +192,7 @@ def main(): local_path=dict(), scm_branch=dict(default=''), scm_refspec=dict(default=''), - scm_credential=dict(), + credential=dict(aliases=['scm_credential']), scm_clean=dict(type='bool', default=False), scm_delete_on_update=dict(type='bool', default=False), scm_update_on_launch=dict(type='bool', default=False), @@ -219,7 +221,7 @@ def main(): local_path = module.params.get('local_path') scm_branch = module.params.get('scm_branch') scm_refspec = module.params.get('scm_refspec') - scm_credential = module.params.get('scm_credential') + credential = module.params.get('credential') scm_clean = module.params.get('scm_clean') scm_delete_on_update = module.params.get('scm_delete_on_update') scm_update_on_launch = module.params.get('scm_update_on_launch') @@ -233,8 +235,8 @@ def main(): # Attempt to look up the related items the user specified (these will fail the module if not found) org_id = module.resolve_name_to_id('organizations', organization) - if scm_credential is not None: - scm_credential_id = module.resolve_name_to_id('credentials', scm_credential) + if credential is not None: + credential = module.resolve_name_to_id('credentials', credential) # Attempt to look up project based on the provided name and org ID project = module.get_one('projects', **{ @@ -286,8 +288,8 @@ def main(): } if description is not None: project_fields['description'] = description - if scm_credential is not None: - project_fields['credential'] = scm_credential_id + if credential is not None: + project_fields['credential'] = credential if allow_override is not None: project_fields['allow_override'] = allow_override if scm_type == '': From f8290f0ce38b3d280e558794282f5d55dffd7177 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 13:24:54 -0400 Subject: [PATCH 07/13] Fixing flake8 and adding more module OKs --- awx_collection/test/awx/test_completeness.py | 58 ++++++++++++++++---- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index 9957435b66..de953aac0f 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -8,8 +8,11 @@ import re # Analysis variables # ----------------------------------------------------------------------------------------------------------- -# Read-only endpoints are dynamically created by an options page with no POST section -read_only_endpoint = [] +# Read-only endpoints are dynamically created by an options page with no POST section. +# Noramlly a read-only endpoint should not have a module (i.e. /api/v2/me) but sometimes we reuse a name +# For example, we have a tower_role module but /api/v2/roles is a read only endpoint. +# Rhis list indicates which read-only endpoints have associated modules with them. +read_only_endpoints_with_modules = ['tower_settings', 'tower_role'] # If a module should not be created for an endpoint and the endpoint is not read-only add it here # THINK HARD ABOUT DOING THIS @@ -19,22 +22,43 @@ no_module_for_endpoint = [] no_endpoint_for_module = [ 'tower_import', 'tower_meta', 'tower_export', 'tower_job_launch', 'tower_job_wait', 'tower_job_list', 'tower_license', 'tower_ping', 'tower_receive', 'tower_send', 'tower_workflow_launch', 'tower_job_cancel', + 'tower_workflow_template', ] # Global module parameters we can ignore -ignore_parameters = ['state', 'new_name'] +ignore_parameters = [ + 'state', 'new_name', + # The way we implemented notifications was to add these fileds to modules which can notify + 'notification_templates_approvals', + 'notification_templates_error', + 'notification_templates_started', + 'notification_templates_success', +] # Some modules take additional parameters that do not appear in the API # Add the module name as the key with the value being the list of params to ignore no_api_parameter_ok = { + # The credential module used to take a lot of parameters which are now combined into "inputs" 'tower_credential': [ 'authorize', 'authorize_password', 'become_method', 'become_password', 'become_username', 'client', 'domain', 'host', 'kind', 'password', 'project', 'secret', 'security_token', 'ssh_key_data', 'ssh_key_unlock', 'subscription', 'tenant', 'username', 'vault_id', 'vault_password', ], - 'tower_project': ['wait', 'timeout'], + # The wait is for wheter or not to wait for a projec update on change + 'tower_project': ['wait'], + # Existing_token and id are for working with an exisitng tokens 'tower_token': ['existing_token', 'existing_token_id'], - 'tower_settings': ['name', 'settings', 'value'], + # Children and hosts are how tower_group deals with associations + 'tower_group': ['children', 'hosts'], + # Credential and Vault Credential are a legacy fields for old Towers + # Credentials/labels/survey spec is now how we handle associations + # We take an organization here to help with the lookups only + 'tower_job_template': ['credential', 'vault_credential', 'credentials', 'labels', 'survey_spec', 'organization'], + # Always_nodes, failure_nodes, success_nodes, credentials is how we handle associations + # Organization is how we looking job templates + 'tower_workflow_job_template_node': ['always_nodes', 'failure_nodes', 'success_nodes', 'credentials', 'organization'], + # Survey is how we handle associations + 'tower_workflow_job_template': ['survey'], } # When this tool was created we were not feature complete. Adding something in here indicates a module @@ -44,18 +68,30 @@ needs_development = [ 'tower_ad_hoc_command', 'tower_application', 'tower_instance_group', 'tower_inventory_script', 'tower_workflow_approval' ] +needs_param_development = { + 'tower_host': ['instance_id'], + 'tower_inventory': ['insights_credential'], +} # ----------------------------------------------------------------------------------------------------------- return_value = 0 +read_only_endpoint = [] def determine_state(module_id, endpoint, module, parameter, api_option, module_option): global return_value # This is a hierarchical list of things that are ok/failures based on conditions if module_id in needs_development and module == 'N/A': - return "Failed (non-blocking), needs development" - if module_id in read_only_endpoint and module == 'N/A': - return "OK, this endpoint is read-only and should not have a module" + return "Failed (non-blocking), module needs development" + if module_id in read_only_endpoint: + if module == 'N/A': + # There may be some cases where a read only endpoint has a module + return "OK, this endpoint is read-only and should not have a module" + elif module_id not in read_only_endpoints_with_modules: + return_value = 255 + return "Failed, read-only endpoint should not have an associated module" + else: + return "OK, module params can not be checked to read-only" if module_id in no_module_for_endpoint and module == 'N/A': return "OK, this endpoint should not have a module" if module_id in no_endpoint_for_module and endpoint == 'N/A': @@ -71,6 +107,8 @@ def determine_state(module_id, endpoint, module, parameter, api_option, module_o if api_option == '' and parameter in no_api_parameter_ok.get(module, {}): return 'OK, no api parameter is ok' if api_option != module_option: + if module_id in needs_param_development and parameter in needs_param_development[module_id]: + return "Failed (non-blocking), parameter needs development" return_value = 255 return 'Failed, option mismatch' if api_option == module_option: @@ -79,7 +117,7 @@ def determine_state(module_id, endpoint, module, parameter, api_option, module_o return "Failed, unknown reason" -def test_completeness(collection_import, request, admin_user): +def test_completeness(collection_import, request, admin_user, job_template): option_comparison = {} # Load a list of existing module files from disk base_folder = os.path.abspath( @@ -89,7 +127,7 @@ def test_completeness(collection_import, request, admin_user): for root, dirs, files in os.walk(module_directory): if root == module_directory: for filename in files: - if re.match('^tower_.*\.py$', filename): + if re.match('^tower_.*.py$', filename): module_name = filename[:-3] option_comparison[module_name] = { 'endpoint': 'N/A', From b1ffbf1e395937e8048cf67b3cdc8de25a5ff64d Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 13:59:05 -0400 Subject: [PATCH 08/13] Change module from tower_notification to tower_notification_template --- awx_collection/meta/runtime.yml | 2 + ...tion.py => tower_notification_template.py} | 16 ++++---- awx_collection/test/awx/test_completeness.py | 6 +++ .../tasks/main.yml | 38 +++++++++---------- 4 files changed, 35 insertions(+), 27 deletions(-) rename awx_collection/plugins/modules/{tower_notification.py => tower_notification_template.py} (98%) rename awx_collection/tests/integration/targets/{tower_notification => tower_notification_template}/tasks/main.yml (77%) diff --git a/awx_collection/meta/runtime.yml b/awx_collection/meta/runtime.yml index 3980ccc14c..d8c2535871 100644 --- a/awx_collection/meta/runtime.yml +++ b/awx_collection/meta/runtime.yml @@ -13,3 +13,5 @@ plugin_routing: deprecation: removal_date: TBD warning_text: see plugin documentation for details + tower_notifitcation: + redirect: tower_notification_template diff --git a/awx_collection/plugins/modules/tower_notification.py b/awx_collection/plugins/modules/tower_notification_template.py similarity index 98% rename from awx_collection/plugins/modules/tower_notification.py rename to awx_collection/plugins/modules/tower_notification_template.py index 12dd28ef31..ec25038e34 100644 --- a/awx_collection/plugins/modules/tower_notification.py +++ b/awx_collection/plugins/modules/tower_notification_template.py @@ -15,7 +15,7 @@ ANSIBLE_METADATA = {'metadata_version': '1.1', DOCUMENTATION = ''' --- -module: tower_notification +module: tower_notification_template author: "Samuel Carpentier (@samcarpentier)" short_description: create, update, or destroy Ansible Tower notification. description: @@ -203,7 +203,7 @@ extends_documentation_fragment: awx.awx.auth EXAMPLES = ''' - name: Add Slack notification with custom messages - tower_notification: + tower_notification_template: name: slack notification organization: Default notification_type: slack @@ -222,7 +222,7 @@ EXAMPLES = ''' tower_config_file: "~/tower_cli.cfg" - name: Add webhook notification - tower_notification: + tower_notification_template: name: webhook notification notification_type: webhook notification_configuration: @@ -233,7 +233,7 @@ EXAMPLES = ''' tower_config_file: "~/tower_cli.cfg" - name: Add email notification - tower_notification: + tower_notification_template: name: email notification notification_type: email notification_configuration: @@ -250,7 +250,7 @@ EXAMPLES = ''' tower_config_file: "~/tower_cli.cfg" - name: Add twilio notification - tower_notification: + tower_notification_template: name: twilio notification notification_type: twilio notification_configuration: @@ -263,7 +263,7 @@ EXAMPLES = ''' tower_config_file: "~/tower_cli.cfg" - name: Add PagerDuty notification - tower_notification: + tower_notification_template: name: pagerduty notification notification_type: pagerduty notification_configuration: @@ -275,7 +275,7 @@ EXAMPLES = ''' tower_config_file: "~/tower_cli.cfg" - name: Add IRC notification - tower_notification: + tower_notification_template: name: irc notification notification_type: irc notification_configuration: @@ -290,7 +290,7 @@ EXAMPLES = ''' tower_config_file: "~/tower_cli.cfg" - name: Delete notification - tower_notification: + tower_notification_template: name: old notification state: absent tower_config_file: "~/tower_cli.cfg" diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index de953aac0f..bbdc1a8c1d 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -59,6 +59,12 @@ no_api_parameter_ok = { 'tower_workflow_job_template_node': ['always_nodes', 'failure_nodes', 'success_nodes', 'credentials', 'organization'], # Survey is how we handle associations 'tower_workflow_job_template': ['survey'], + # These fields get wrapped into notification_configuration + 'tower_notification_template': [ + 'account_sid', 'account_token', 'channels', 'client_name', 'color', 'from_number', 'headers', 'host', + 'message_from', 'nickname', 'notify', 'password', 'port', 'recipients', 'sender', 'server', + 'service_key', 'subdomain', 'targets', 'to_numbers', 'token', 'url', 'use_ssl', 'use_tls', 'username', + ] } # When this tool was created we were not feature complete. Adding something in here indicates a module diff --git a/awx_collection/tests/integration/targets/tower_notification/tasks/main.yml b/awx_collection/tests/integration/targets/tower_notification_template/tasks/main.yml similarity index 77% rename from awx_collection/tests/integration/targets/tower_notification/tasks/main.yml rename to awx_collection/tests/integration/targets/tower_notification_template/tasks/main.yml index ea3e96b90a..a4d41571cf 100644 --- a/awx_collection/tests/integration/targets/tower_notification/tasks/main.yml +++ b/awx_collection/tests/integration/targets/tower_notification_template/tasks/main.yml @@ -1,14 +1,14 @@ --- - name: Generate names set_fact: - slack_not: "AWX-Collection-tests-tower_notification-slack-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - webhook_not: "AWX-Collection-tests-tower_notification-wehbook-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - email_not: "AWX-Collection-tests-tower_notification-email-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - twillo_not: "AWX-Collection-tests-tower_notification-twillo-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - pd_not: "AWX-Collection-tests-tower_notification-pd-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" - irc_not: "AWX-Collection-tests-tower_notification-irc-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + slack_not: "AWX-Collection-tests-tower_notification_template-slack-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + webhook_not: "AWX-Collection-tests-tower_notification_template-wehbook-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + email_not: "AWX-Collection-tests-tower_notification_template-email-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + twillo_not: "AWX-Collection-tests-tower_notification_template-twillo-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + pd_not: "AWX-Collection-tests-tower_notification_template-pd-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" + irc_not: "AWX-Collection-tests-tower_notification_template-irc-not-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}" -- name: Test deprecation warnings +- name: Test deprecation warnings with legacy name tower_notification: name: "{{ slack_not }}" organization: Default @@ -54,7 +54,7 @@ - result['deprecations'] | length() == 25 - name: Create Slack notification with custom messages - tower_notification: + tower_notification_template: name: "{{ slack_not }}" organization: Default notification_type: slack @@ -76,7 +76,7 @@ - result is changed - name: Delete Slack notification - tower_notification: + tower_notification_template: name: "{{ slack_not }}" organization: Default state: absent @@ -87,7 +87,7 @@ - result is changed - name: Add webhook notification - tower_notification: + tower_notification_template: name: "{{ webhook_not }}" organization: Default notification_type: webhook @@ -102,7 +102,7 @@ - result is changed - name: Delete webhook notification - tower_notification: + tower_notification_template: name: "{{ webhook_not }}" organization: Default state: absent @@ -113,7 +113,7 @@ - result is changed - name: Add email notification - tower_notification: + tower_notification_template: name: "{{ email_not }}" organization: Default notification_type: email @@ -134,7 +134,7 @@ - result is changed - name: Delete email notification - tower_notification: + tower_notification_template: name: "{{ email_not }}" organization: Default state: absent @@ -145,7 +145,7 @@ - result is changed - name: Add twilio notification - tower_notification: + tower_notification_template: name: "{{ twillo_not }}" organization: Default notification_type: twilio @@ -162,7 +162,7 @@ - result is changed - name: Delete twilio notification - tower_notification: + tower_notification_template: name: "{{ twillo_not }}" organization: Default state: absent @@ -173,7 +173,7 @@ - result is changed - name: Add PagerDuty notification - tower_notification: + tower_notification_template: name: "{{ pd_not }}" organization: Default notification_type: pagerduty @@ -189,7 +189,7 @@ - result is changed - name: Delete PagerDuty notification - tower_notification: + tower_notification_template: name: "{{ pd_not }}" organization: Default state: absent @@ -200,7 +200,7 @@ - result is changed - name: Add IRC notification - tower_notification: + tower_notification_template: name: "{{ irc_not }}" organization: Default notification_type: irc @@ -219,7 +219,7 @@ - result is changed - name: Delete IRC notification - tower_notification: + tower_notification_template: name: "{{ irc_not }}" organization: Default state: absent From a36d942f679ae14a674a5bb2984c5a1a046137a8 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Mon, 24 Aug 2020 14:14:15 -0400 Subject: [PATCH 09/13] Migrating old test_notification test to test_notification_template --- ...notification.py => test_notification_template.py} | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) rename awx_collection/test/awx/{test_notification.py => test_notification_template.py} (91%) diff --git a/awx_collection/test/awx/test_notification.py b/awx_collection/test/awx/test_notification_template.py similarity index 91% rename from awx_collection/test/awx/test_notification.py rename to awx_collection/test/awx/test_notification_template.py index 9d916d1dc9..28f7c4ecee 100644 --- a/awx_collection/test/awx/test_notification.py +++ b/awx_collection/test/awx/test_notification_template.py @@ -34,7 +34,7 @@ def test_create_modify_notification_template(run_module, admin_user, organizatio 'use_tls': False, 'use_ssl': False, 'timeout': 4 } - result = run_module('tower_notification', dict( + result = run_module('tower_notification_template', dict( name='foo-notification-template', organization=organization.name, notification_type='email', @@ -49,7 +49,7 @@ def test_create_modify_notification_template(run_module, admin_user, organizatio # Test no-op, this is impossible if the notification_configuration is given # because we cannot determine if password fields changed - result = run_module('tower_notification', dict( + result = run_module('tower_notification_template', dict( name='foo-notification-template', organization=organization.name, notification_type='email', @@ -59,7 +59,7 @@ def test_create_modify_notification_template(run_module, admin_user, organizatio # Test a change in the configuration nt_config['timeout'] = 12 - result = run_module('tower_notification', dict( + result = run_module('tower_notification_template', dict( name='foo-notification-template', organization=organization.name, notification_type='email', @@ -74,7 +74,7 @@ def test_create_modify_notification_template(run_module, admin_user, organizatio @pytest.mark.django_db def test_invalid_notification_configuration(run_module, admin_user, organization): - result = run_module('tower_notification', dict( + result = run_module('tower_notification_template', dict( name='foo-notification-template', organization=organization.name, notification_type='email', @@ -92,7 +92,7 @@ def test_deprecated_to_modern_no_op(run_module, admin_user, organization): 'X-Custom-Header': 'value123' } } - result = run_module('tower_notification', dict( + result = run_module('tower_notification_template', dict( name='foo-notification-template', organization=organization.name, notification_type='webhook', @@ -101,7 +101,7 @@ def test_deprecated_to_modern_no_op(run_module, admin_user, organization): assert not result.get('failed', False), result.get('msg', result) assert result.pop('changed', None), result - result = run_module('tower_notification', dict( + result = run_module('tower_notification_template', dict( name='foo-notification-template', organization=organization.name, notification_type='webhook', From 730a9b25ac04abf5d19813b569403d490f4ab3f5 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Tue, 25 Aug 2020 11:11:10 -0400 Subject: [PATCH 10/13] Fixing pep8 and other issues --- awx_collection/test/awx/test_completeness.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index bbdc1a8c1d..0c21ff98f2 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -1,4 +1,7 @@ -#!/usr/bin/env python3 +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import pytest from awx.main.tests.functional.conftest import _request import yaml @@ -91,7 +94,7 @@ def determine_state(module_id, endpoint, module, parameter, api_option, module_o return "Failed (non-blocking), module needs development" if module_id in read_only_endpoint: if module == 'N/A': - # There may be some cases where a read only endpoint has a module + # There may be some cases where a read only endpoint has a module return "OK, this endpoint is read-only and should not have a module" elif module_id not in read_only_endpoints_with_modules: return_value = 255 @@ -154,12 +157,12 @@ def test_completeness(collection_import, request, admin_user, job_template): ) for endpoint in endpoint_response.data.keys(): # Module names are singular and endpoints are plural so we need to convert to singular - singular_endpoint = '{}'.format(endpoint) + singular_endpoint = '{0}'.format(endpoint) if singular_endpoint.endswith('ies'): singular_endpoint = singular_endpoint[:-3] if singular_endpoint != 'settings' and singular_endpoint.endswith('s'): singular_endpoint = singular_endpoint[:-1] - module_name = 'tower_{}'.format(singular_endpoint) + module_name = 'tower_{0}'.format(singular_endpoint) endpoint_url = endpoint_response.data.get(endpoint) From 8f97109ac73067e691e438ce719dc8bc66fffac4 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Tue, 25 Aug 2020 11:25:11 -0400 Subject: [PATCH 11/13] Fixing PY2 compile issues --- awx_collection/test/awx/test_completeness.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index 0c21ff98f2..2a186d25db 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -201,29 +201,27 @@ def test_completeness(collection_import, request, admin_user, job_template): longest_option_name = len(option) # Print out some headers - print( + print("".join([ "End Point", " " * (longest_endpoint - len("End Point")), " | Module Name", " " * (longest_module_name - len("Module Name")), " | Option", " " * (longest_option_name - len("Option")), " | API | Module | State", - sep="" - ) - print( + ])) + print("-|-".join([ "-" * longest_endpoint, "-" * longest_module_name, "-" * longest_option_name, "---", "------", "---------------------------------------------", - sep="-|-" - ) + ])) # Print out all of our data for module in sorted(option_comparison): module_data = option_comparison[module] all_param_names = list(set(module_data['api_options']) | set(module_data['module_options'])) for parameter in sorted(all_param_names): - print( + print("".join([ module_data['endpoint'], " " * (longest_endpoint - len(module_data['endpoint'])), " | ", module_data['module_name'], " " * (longest_module_name - len(module_data['module_name'])), " | ", parameter, " " * (longest_option_name - len(parameter)), " | ", @@ -237,19 +235,17 @@ def test_completeness(collection_import, request, admin_user, job_template): 'X' if (parameter in module_data['api_options']) else '', 'X' if (parameter in module_data['module_options']) else '', ), - sep="" - ) + ])) # This handles cases were we got no params from the options page nor from the modules if len(all_param_names) == 0: - print( + print("".join([ module_data['endpoint'], " " * (longest_endpoint - len(module_data['endpoint'])), " | ", module_data['module_name'], " " * (longest_module_name - len(module_data['module_name'])), " | ", "N/A", " " * (longest_option_name - len("N/A")), " | ", ' ', " | ", ' ', " | ", determine_state(module, module_data['endpoint'], module_data['module_name'], 'N/A', '', ''), - sep="" - ) + ])) if return_value != 0: raise Exception("One or more failures caused issues") From b7b304eb846c07f832fad40e3a8a74b74c0eff25 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Tue, 25 Aug 2020 17:06:38 -0400 Subject: [PATCH 12/13] Changing how we check modules Options which are not in the API POST and are marked in the module as deprecated are ignored If an option is not in the API POST but is marked as a list we assume its a relation --- awx_collection/test/awx/test_completeness.py | 109 +++++++++++-------- 1 file changed, 63 insertions(+), 46 deletions(-) diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py index 2a186d25db..55a00f29b9 100755 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -4,6 +4,7 @@ __metaclass__ = type import pytest from awx.main.tests.functional.conftest import _request +from ansible.module_utils.six import PY2, string_types import yaml import os import re @@ -31,43 +32,22 @@ no_endpoint_for_module = [ # Global module parameters we can ignore ignore_parameters = [ 'state', 'new_name', - # The way we implemented notifications was to add these fileds to modules which can notify - 'notification_templates_approvals', - 'notification_templates_error', - 'notification_templates_started', - 'notification_templates_success', ] # Some modules take additional parameters that do not appear in the API # Add the module name as the key with the value being the list of params to ignore no_api_parameter_ok = { - # The credential module used to take a lot of parameters which are now combined into "inputs" - 'tower_credential': [ - 'authorize', 'authorize_password', 'become_method', 'become_password', 'become_username', 'client', - 'domain', 'host', 'kind', 'password', 'project', 'secret', 'security_token', 'ssh_key_data', - 'ssh_key_unlock', 'subscription', 'tenant', 'username', 'vault_id', 'vault_password', - ], # The wait is for wheter or not to wait for a projec update on change 'tower_project': ['wait'], # Existing_token and id are for working with an exisitng tokens 'tower_token': ['existing_token', 'existing_token_id'], - # Children and hosts are how tower_group deals with associations - 'tower_group': ['children', 'hosts'], - # Credential and Vault Credential are a legacy fields for old Towers - # Credentials/labels/survey spec is now how we handle associations + # /survey spec is now how we handle associations # We take an organization here to help with the lookups only - 'tower_job_template': ['credential', 'vault_credential', 'credentials', 'labels', 'survey_spec', 'organization'], - # Always_nodes, failure_nodes, success_nodes, credentials is how we handle associations + 'tower_job_template': ['survey_spec', 'organization'], # Organization is how we looking job templates - 'tower_workflow_job_template_node': ['always_nodes', 'failure_nodes', 'success_nodes', 'credentials', 'organization'], + 'tower_workflow_job_template_node': ['organization'], # Survey is how we handle associations 'tower_workflow_job_template': ['survey'], - # These fields get wrapped into notification_configuration - 'tower_notification_template': [ - 'account_sid', 'account_token', 'channels', 'client_name', 'color', 'from_number', 'headers', 'host', - 'message_from', 'nickname', 'notify', 'password', 'port', 'recipients', 'sender', 'server', - 'service_key', 'subdomain', 'targets', 'to_numbers', 'token', 'url', 'use_ssl', 'use_tls', 'username', - ] } # When this tool was created we were not feature complete. Adding something in here indicates a module @@ -87,43 +67,80 @@ return_value = 0 read_only_endpoint = [] -def determine_state(module_id, endpoint, module, parameter, api_option, module_option): +def cause_error(msg): global return_value + return_value = 255 + return(msg) + +def determine_state(module_id, endpoint, module, parameter, api_option, module_option): # This is a hierarchical list of things that are ok/failures based on conditions + + # If we know this module needs develpment this is a non-blocking failure if module_id in needs_development and module == 'N/A': return "Failed (non-blocking), module needs development" + + # If the module is a read only endpoint: + # If it has no module on disk that is ok. + # If it has a module on disk but its listed in read_only_endpoints_with_modules that is ok + # Else we have a module for a read only endpoint that should not exit if module_id in read_only_endpoint: if module == 'N/A': # There may be some cases where a read only endpoint has a module return "OK, this endpoint is read-only and should not have a module" - elif module_id not in read_only_endpoints_with_modules: - return_value = 255 - return "Failed, read-only endpoint should not have an associated module" - else: + elif module_id in read_only_endpoints_with_modules: return "OK, module params can not be checked to read-only" + else: + return cause_error("Failed, read-only endpoint should not have an associated module") + + # If the endpoint is listed as not needing a module and we don't have one we are ok if module_id in no_module_for_endpoint and module == 'N/A': return "OK, this endpoint should not have a module" + + # If module is listed as not needing an endpoint and we don't have one we are ok if module_id in no_endpoint_for_module and endpoint == 'N/A': return "OK, this module does not require an endpoint" + + # All of the end/point module conditionals are done so if we don't have a module or endpoint we have a problem if module == 'N/A': - return_value = 255 - return 'Failed, missing module' + return cause_error('Failed, missing module') if endpoint == 'N/A': - return_value = 255 - return 'Failed, why does this module have no endpoint' + return cause_error('Failed, why does this module have no endpoint') + + # Now perform parameter checks + + # First, if the patemrer is in the ignore_parameters list we are ok if parameter in ignore_parameters: return "OK, globally ignored parameter" - if api_option == '' and parameter in no_api_parameter_ok.get(module, {}): - return 'OK, no api parameter is ok' - if api_option != module_option: - if module_id in needs_param_development and parameter in needs_param_development[module_id]: + + # If both the api option and the module option are both either objects or none + if (api_option == None) ^ (module_option == None): + # If the API option is node and the parameter is in the no_api_parameter list we are ok + if api_option == None and parameter in no_api_parameter_ok.get(module, {}): + return 'OK, no api parameter is ok' + # If we know this parameter needs development and + if module_option == None and parameter in needs_param_development.get(module_id, {}): return "Failed (non-blocking), parameter needs development" - return_value = 255 - return 'Failed, option mismatch' - if api_option == module_option: - return 'OK' - return_value = 255 - return "Failed, unknown reason" + # Check for deprecated in the node, if its deprecated and has no api option we are ok, otherise we have a problem + if module_option and module_option.get('description'): + description = '' + if isinstance(module_option.get('description'), string_types): + description = module_option.get('description') + else: + description = " ".join(module_option.get('description')) + + if 'deprecated' in description.lower(): + if api_option == None: + return 'OK, depricated module option' + else: + return cause_error('Failed, module marks option as deprecated but option still exists in API') + # If we don't have a cooresponding API option but we are a list then we are likely a relation + if not api_option and module_option and module_option.get('type', 'str') == 'list': + return "OK, Field appears to be relation" + # TODO, at some point try and check the onbject model to confirm its actually a relation + return cause_error('Failed, option mismatch') + + # We made it through all of the checks so we are ok + return 'OK' def test_completeness(collection_import, request, admin_user, job_template): @@ -232,8 +249,8 @@ def test_completeness(collection_import, request, admin_user, job_template): module_data['endpoint'], module_data['module_name'], parameter, - 'X' if (parameter in module_data['api_options']) else '', - 'X' if (parameter in module_data['module_options']) else '', + module_data['api_options'][parameter] if (parameter in module_data['api_options']) else None, + module_data['module_options'][parameter] if (parameter in module_data['module_options']) else None, ), ])) # This handles cases were we got no params from the options page nor from the modules @@ -244,7 +261,7 @@ def test_completeness(collection_import, request, admin_user, job_template): "N/A", " " * (longest_option_name - len("N/A")), " | ", ' ', " | ", ' ', " | ", - determine_state(module, module_data['endpoint'], module_data['module_name'], 'N/A', '', ''), + determine_state(module, module_data['endpoint'], module_data['module_name'], 'N/A', None, None), ])) if return_value != 0: From 2b6e4fe3537ad69c5bbaf5568933e4a2af97dc17 Mon Sep 17 00:00:00 2001 From: John Westcott IV Date: Wed, 26 Aug 2020 10:14:29 -0400 Subject: [PATCH 13/13] Permissions change, spell check, pep8, etc --- awx_collection/test/awx/test_completeness.py | 34 ++++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) mode change 100755 => 100644 awx_collection/test/awx/test_completeness.py diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py old mode 100755 new mode 100644 index 55a00f29b9..a4ffad3c48 --- a/awx_collection/test/awx/test_completeness.py +++ b/awx_collection/test/awx/test_completeness.py @@ -2,7 +2,6 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import pytest - from awx.main.tests.functional.conftest import _request from ansible.module_utils.six import PY2, string_types import yaml @@ -13,9 +12,9 @@ import re # ----------------------------------------------------------------------------------------------------------- # Read-only endpoints are dynamically created by an options page with no POST section. -# Noramlly a read-only endpoint should not have a module (i.e. /api/v2/me) but sometimes we reuse a name +# Normally a read-only endpoint should not have a module (i.e. /api/v2/me) but sometimes we reuse a name # For example, we have a tower_role module but /api/v2/roles is a read only endpoint. -# Rhis list indicates which read-only endpoints have associated modules with them. +# This list indicates which read-only endpoints have associated modules with them. read_only_endpoints_with_modules = ['tower_settings', 'tower_role'] # If a module should not be created for an endpoint and the endpoint is not read-only add it here @@ -37,9 +36,9 @@ ignore_parameters = [ # Some modules take additional parameters that do not appear in the API # Add the module name as the key with the value being the list of params to ignore no_api_parameter_ok = { - # The wait is for wheter or not to wait for a projec update on change + # The wait is for whether or not to wait for a project update on change 'tower_project': ['wait'], - # Existing_token and id are for working with an exisitng tokens + # Existing_token and id are for working with an existing tokens 'tower_token': ['existing_token', 'existing_token_id'], # /survey spec is now how we handle associations # We take an organization here to help with the lookups only @@ -70,12 +69,13 @@ read_only_endpoint = [] def cause_error(msg): global return_value return_value = 255 - return(msg) + return msg + def determine_state(module_id, endpoint, module, parameter, api_option, module_option): # This is a hierarchical list of things that are ok/failures based on conditions - # If we know this module needs develpment this is a non-blocking failure + # If we know this module needs development this is a non-blocking failure if module_id in needs_development and module == 'N/A': return "Failed (non-blocking), module needs development" @@ -108,19 +108,19 @@ def determine_state(module_id, endpoint, module, parameter, api_option, module_o # Now perform parameter checks - # First, if the patemrer is in the ignore_parameters list we are ok + # First, if the parameter is in the ignore_parameters list we are ok if parameter in ignore_parameters: return "OK, globally ignored parameter" # If both the api option and the module option are both either objects or none - if (api_option == None) ^ (module_option == None): + if (api_option is None) ^ (module_option is None): # If the API option is node and the parameter is in the no_api_parameter list we are ok - if api_option == None and parameter in no_api_parameter_ok.get(module, {}): + if api_option is None and parameter in no_api_parameter_ok.get(module, {}): return 'OK, no api parameter is ok' - # If we know this parameter needs development and - if module_option == None and parameter in needs_param_development.get(module_id, {}): + # If we know this parameter needs development and we don't have a module option we are non-blocking + if module_option is None and parameter in needs_param_development.get(module_id, {}): return "Failed (non-blocking), parameter needs development" - # Check for deprecated in the node, if its deprecated and has no api option we are ok, otherise we have a problem + # Check for deprecated in the node, if its deprecated and has no api option we are ok, otherwise we have a problem if module_option and module_option.get('description'): description = '' if isinstance(module_option.get('description'), string_types): @@ -129,14 +129,14 @@ def determine_state(module_id, endpoint, module, parameter, api_option, module_o description = " ".join(module_option.get('description')) if 'deprecated' in description.lower(): - if api_option == None: - return 'OK, depricated module option' + if api_option is None: + return 'OK, deprecated module option' else: return cause_error('Failed, module marks option as deprecated but option still exists in API') - # If we don't have a cooresponding API option but we are a list then we are likely a relation + # If we don't have a corresponding API option but we are a list then we are likely a relation if not api_option and module_option and module_option.get('type', 'str') == 'list': return "OK, Field appears to be relation" - # TODO, at some point try and check the onbject model to confirm its actually a relation + # TODO, at some point try and check the object model to confirm its actually a relation return cause_error('Failed, option mismatch') # We made it through all of the checks so we are ok