lambda_info and lambda_alias - avoid manipulating module.params (ansi…

…ble-collections#1336)

lambda_info and lambda_alias - avoid manipulating module.params

SUMMARY
Directly manipulating module.params results in the Ansible output incorrectly representing what was passed to the module, as such we shouldn't be doing this.

Refactors lambda_alias to avoid passing around "module" entirely
Updates lamba_info to avoid updating module.params

ISSUE TYPE

Feature Pull Request

COMPONENT NAME
lambda_info
lambda_alias
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Mark Chappell <None>

changelogs/fragments/1336-lambda-module_params.yml

@@ -0,0 +1,4 @@
+minor_changes:
+- lambda_info - updated to avoid manipulating ``module.params`` (https://github.com/ansible-collections/amazon.aws/pull/1336).
+- lambda_alias - updated to avoid manipulating ``module.params`` (https://github.com/ansible-collections/amazon.aws/pull/1336).
+- lambda_alias - refactored to avoid passing around the complex ``module`` resource (https://github.com/ansible-collections/amazon.aws/pull/1336).

plugins/modules/lambda_alias.py

@@ -154,94 +154,108 @@
 from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict
 from ansible.module_utils.common.dict_transformations import snake_dict_to_camel_dict
 
-from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule
 from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code
+from ansible_collections.amazon.aws.plugins.module_utils.exceptions import AnsibleAWSError
+from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule
 from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry
 
 
-def set_api_params(module, module_params):
+class LambdaAnsibleAWSError(AnsibleAWSError):
+    pass
+
+
+def set_api_params(module_params, param_names):
     """
     Sets non-None module parameters to those expected by the boto3 API.
 
-    :param module:
     :param module_params:
+    :param param_names:
     :return:
     """
 
     api_params = dict()
 
-    for param in module_params:
-        module_param = module.params.get(param, None)
+    for param in param_names:
+        module_param = module_params.get(param, None)
         if module_param:
             api_params[param] = module_param
 
     return snake_dict_to_camel_dict(api_params, capitalize_first=True)
 
 
-def validate_params(module):
+def validate_params(module_params):
     """
     Performs basic parameter validation.
 
-    :param module: AnsibleAWSModule reference
+    :param module_params: AnsibleAWSModule Parameters
     :return:
     """
 
-    function_name = module.params['function_name']
+    function_name = module_params['function_name']
 
     # validate function name
     if not re.search(r'^[\w\-:]+$', function_name):
-        module.fail_json(
-            msg='Function name {0} is invalid. Names must contain only alphanumeric characters and hyphens.'.format(function_name)
+        raise LambdaAnsibleAWSError(
+            f"Function name {function_name} is invalid. "
+            "Names must contain only alphanumeric characters and hyphens."
         )
     if len(function_name) > 64:
-        module.fail_json(msg='Function name "{0}" exceeds 64 character limit'.format(function_name))
+        raise LambdaAnsibleAWSError(
+            f"Function name '{function_name}' exceeds 64 character limit"
+        )
+    return
+
+
+def normalize_params(module_params):
+
+    params = dict(module_params)
 
     #  if parameter 'function_version' is zero, set it to $LATEST, else convert it to a string
-    if module.params['function_version'] == 0:
-        module.params['function_version'] = '$LATEST'
+    if params['function_version'] == 0:
+        params['function_version'] = '$LATEST'
     else:
-        module.params['function_version'] = str(module.params['function_version'])
+        params['function_version'] = str(params['function_version'])
 
-    return
+    return params
 
 
-def get_lambda_alias(module, client):
+def get_lambda_alias(module_params, client):
     """
     Returns the lambda function alias if it exists.
 
-    :param module: AnsibleAWSModule
+    :param module_params: AnsibleAWSModule parameters
     :param client: (wrapped) boto3 lambda client
     :return:
     """
 
     # set API parameters
-    api_params = set_api_params(module, ('function_name', 'name'))
+    api_params = set_api_params(module_params, ('function_name', 'name'))
 
     # check if alias exists and get facts
     try:
         results = client.get_alias(aws_retry=True, **api_params)
     except is_boto3_error_code('ResourceNotFoundException'):
         results = None
     except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:  # pylint: disable=duplicate-except
-        module.fail_json_aws(e, msg='Error retrieving function alias')
+        raise LambdaAnsibleAWSError("Error retrieving function alias", exception=e)
 
     return results
 
 
-def lambda_alias(module, client):
+def lambda_alias(module_params, client, check_mode):
     """
     Adds, updates or deletes lambda function aliases.
 
-    :param module: AnsibleAWSModule
+    :param module_params: AnsibleAWSModule parameters
     :param client: (wrapped) boto3 lambda client
     :return dict:
     """
     results = dict()
     changed = False
     current_state = 'absent'
-    state = module.params['state']
+    state = module_params['state']
 
-    facts = get_lambda_alias(module, client)
+    facts = get_lambda_alias(module_params, client)
     if facts:
         current_state = 'present'
 
@@ -252,44 +266,44 @@ def lambda_alias(module, client):
             # check if alias has changed -- only version and description can change
             alias_params = ('function_version', 'description')
             for param in alias_params:
-                if module.params.get(param) is None:
+                if module_params.get(param) is None:
                     continue
-                if module.params.get(param) != snake_facts.get(param):
+                if module_params.get(param) != snake_facts.get(param):
                     changed = True
                     break
 
             if changed:
-                api_params = set_api_params(module, ('function_name', 'name'))
-                api_params.update(set_api_params(module, alias_params))
+                api_params = set_api_params(module_params, ('function_name', 'name'))
+                api_params.update(set_api_params(module_params, alias_params))
 
-                if not module.check_mode:
+                if not check_mode:
                     try:
                         results = client.update_alias(aws_retry=True, **api_params)
                     except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                        module.fail_json_aws(e, msg='Error updating function alias')
+                        raise LambdaAnsibleAWSError("Error updating function alias", exception=e)
 
         else:
             # create new function alias
-            api_params = set_api_params(module, ('function_name', 'name', 'function_version', 'description'))
+            api_params = set_api_params(module_params, ('function_name', 'name', 'function_version', 'description'))
 
             try:
-                if not module.check_mode:
+                if not check_mode:
                     results = client.create_alias(aws_retry=True, **api_params)
                 changed = True
             except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                module.fail_json_aws(e, msg='Error creating function alias')
+                raise LambdaAnsibleAWSError("Error creating function alias", exception=e)
 
     else:  # state = 'absent'
         if current_state == 'present':
             # delete the function
-            api_params = set_api_params(module, ('function_name', 'name'))
+            api_params = set_api_params(module_params, ('function_name', 'name'))
 
             try:
-                if not module.check_mode:
+                if not check_mode:
                     results = client.delete_alias(aws_retry=True, **api_params)
                 changed = True
             except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
-                module.fail_json_aws(e, msg='Error deleting function alias')
+                raise LambdaAnsibleAWSError("Error deleting function alias", exception=e)
 
     return dict(changed=changed, **dict(results or facts or {}))
 
@@ -317,8 +331,14 @@ def main():
 
     client = module.client('lambda', retry_decorator=AWSRetry.jittered_backoff())
 
-    validate_params(module)
-    results = lambda_alias(module, client)
+    try:
+        validate_params(module.params)
+        module_params = normalize_params(module.params)
+        results = lambda_alias(module_params, client, module.check_mode)
+    except LambdaAnsibleAWSError as e:
+        if e.exception:
+            module.fail_json_aws(e.exception, msg=e.message)
+        module.fail_json(msg=e.message)
 
     module.exit_json(**camel_dict_to_snake_dict(results))
 

plugins/modules/lambda_info.py

@@ -274,8 +274,8 @@
 
 from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict
 
-from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule
 from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code
+from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule
 from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry
 
 
@@ -307,6 +307,17 @@ def alias_details(client, module, function_name):
     return camel_dict_to_snake_dict(lambda_info)
 
 
+def _get_query(query, function_name):
+    # create default values for query if not specified.
+    # if function name exists, query should default to 'all'.
+    # if function name does not exist, query should default to 'config' to limit the runtime when listing all lambdas.
+    if query:
+        return query
+    if function_name:
+        return "all"
+    return "config"
+
+
 def list_functions(client, module):
     """
     Returns queried facts for a specified function (or all functions).
@@ -325,7 +336,7 @@ def list_functions(client, module):
         all_function_info = _paginate(client, 'list_functions')['Functions']
         function_names = [function_info['FunctionName'] for function_info in all_function_info]
 
-    query = module.params['query']
+    query = _get_query(module.params['query'], function_name)
     functions = []
 
     # keep returning deprecated response (dict of dicts) until removed
@@ -508,15 +519,6 @@ def main():
         if len(function_name) > 64:
             module.fail_json(msg='Function name "{0}" exceeds 64 character limit'.format(function_name))
 
-    # create default values for query if not specified.
-    # if function name exists, query should default to 'all'.
-    # if function name does not exist, query should default to 'config' to limit the runtime when listing all lambdas.
-    if not module.params.get('query'):
-        if function_name:
-            module.params['query'] = 'all'
-        else:
-            module.params['query'] = 'config'
-
     client = module.client('lambda', retry_decorator=AWSRetry.jittered_backoff())
 
     # Deprecate previous return key of `function`, as it was a dict of dicts, as opposed to a list of dicts