lambda AnsibleAWSModule cleanup (also enable retries) (#5)

* Bomb out early if someone tries to set tags without the necessary botocore version

* Simplify some error handling by using fail_json_aws

* Use BotoCoreError rather than the sub-errors

We still bomb out, but fail_json_aws is more graceful and user friendly
than an uncaught Boto3 error.

* use is_boto3_error_code to limit what we catch rather than catching and re-raising.

* Cleanup get_account_info
- use module.client to avoid the mass of extra args
- use is_boto3_error_code('AccessDenied') to be a little cleaner
- fix text search (re.search(, mystring) rather than mystring.search())

* Use module.client helpers

* Delete dead code path - we test for having *both* vpc_subnet_ids and vpc_security_group_ids when we parse the arguments

* Enable basic AWS Retries

* Tweak integration test to allow for common 'missing region' error message

* Rename lambda tests from 'aws_lambda' to 'lambda' (matching the module name)

* Use omit rather than 'null' in the tests - ansible/ansible#69190

* Ignore duplicate-except warnings (it's caused by the way is_boto3_error works)

* change expected error messages now we're using an AnsibleAWSModule feature

plugins/modules/lambda.py

@@ -212,21 +212,24 @@
 '''
 
 from ansible.module_utils._text import to_native
+from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict
 from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
-from ansible_collections.amazon.aws.plugins.module_utils.ec2 import get_aws_connection_info, boto3_conn, camel_dict_to_snake_dict
+from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry
 from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_aws_tags
+
 import base64
 import hashlib
 import traceback
 import re
 
 try:
-    from botocore.exceptions import ClientError, BotoCoreError, ValidationError, ParamValidationError
+    from botocore.exceptions import ClientError, BotoCoreError
 except ImportError:
     pass  # protected by AnsibleAWSModule
 
 
-def get_account_info(module, region=None, endpoint=None, **aws_connect_kwargs):
+def get_account_info(module):
     """return the account information (account id and partition) we are currently working on
 
     get_account_info tries too find out the account that we are working
@@ -237,27 +240,25 @@ def get_account_info(module, region=None, endpoint=None, **aws_connect_kwargs):
     account_id = None
     partition = None
     try:
-        sts_client = boto3_conn(module, conn_type='client', resource='sts',
-                                region=region, endpoint=endpoint, **aws_connect_kwargs)
-        caller_id = sts_client.get_caller_identity()
+        sts_client = module.client('sts', retry_decorator=AWSRetry.jittered_backoff())
+        caller_id = sts_client.get_caller_identity(aws_retry=True)
         account_id = caller_id.get('Account')
         partition = caller_id.get('Arn').split(':')[1]
-    except ClientError:
+    except (BotoCoreError, ClientError):
         try:
-            iam_client = boto3_conn(module, conn_type='client', resource='iam',
-                                    region=region, endpoint=endpoint, **aws_connect_kwargs)
-            arn, partition, service, reg, account_id, resource = iam_client.get_user()['User']['Arn'].split(':')
-        except ClientError as e:
-            if (e.response['Error']['Code'] == 'AccessDenied'):
+            iam_client = module.client('iam', retry_decorator=AWSRetry.jittered_backoff())
+            arn, partition, service, reg, account_id, resource = iam_client.get_user(aws_retry=True)['User']['Arn'].split(':')
+        except is_boto3_error_code('AccessDenied') as e:
+            try:
                 except_msg = to_native(e.message)
-                m = except_msg.search(r"arn:(aws(-([a-z\-]+))?):iam::([0-9]{12,32}):\w+/")
-                account_id = m.group(4)
-                partition = m.group(1)
-            if account_id is None:
+            except AttributeError:
+                except_msg = to_native(e)
+            m = re.search(r"arn:(aws(-([a-z\-]+))?):iam::([0-9]{12,32}):\w+/", except_msg)
+            if m is None:
                 module.fail_json_aws(e, msg="getting account information")
-            if partition is None:
-                module.fail_json_aws(e, msg="getting account information: partition")
-        except Exception as e:
+            account_id = m.group(4)
+            partition = m.group(1)
+        except (BotoCoreError, ClientError) as e:  # pylint: disable=duplicate-except
             module.fail_json_aws(e, msg="getting account information")
 
     return account_id, partition
@@ -266,15 +267,10 @@ def get_account_info(module, region=None, endpoint=None, **aws_connect_kwargs):
 def get_current_function(connection, function_name, qualifier=None):
     try:
         if qualifier is not None:
-            return connection.get_function(FunctionName=function_name, Qualifier=qualifier)
-        return connection.get_function(FunctionName=function_name)
-    except ClientError as e:
-        try:
-            if e.response['Error']['Code'] == 'ResourceNotFoundException':
-                return None
-        except (KeyError, AttributeError):
-            pass
-        raise e
+            return connection.get_function(FunctionName=function_name, Qualifier=qualifier, aws_retry=True)
+        return connection.get_function(FunctionName=function_name, aws_retry=True)
+    except is_boto3_error_code('ResourceNotFoundException'):
+        return None
 
 
 def sha256sum(filename):
@@ -290,42 +286,36 @@ def sha256sum(filename):
 
 
 def set_tag(client, module, tags, function):
-    if not hasattr(client, "list_tags"):
-        module.fail_json(msg="Using tags requires botocore 1.5.40 or above")
 
     changed = False
     arn = function['Configuration']['FunctionArn']
 
     try:
-        current_tags = client.list_tags(Resource=arn).get('Tags', {})
-    except ClientError as e:
-        module.fail_json(msg="Unable to list tags: {0}".format(to_native(e)),
-                         exception=traceback.format_exc())
+        current_tags = client.list_tags(Resource=arn, aws_retry=True).get('Tags', {})
+    except (BotoCoreError, ClientError) as e:
+        module.fail_json_aws(e, msg="Unable to list tags")
 
     tags_to_add, tags_to_remove = compare_aws_tags(current_tags, tags, purge_tags=True)
 
     try:
         if tags_to_remove:
             client.untag_resource(
                 Resource=arn,
-                TagKeys=tags_to_remove
+                TagKeys=tags_to_remove,
+                aws_retry=True
             )
             changed = True
 
         if tags_to_add:
             client.tag_resource(
                 Resource=arn,
-                Tags=tags_to_add
+                Tags=tags_to_add,
+                aws_retry=True
             )
             changed = True
 
-    except ClientError as e:
-        module.fail_json(msg="Unable to tag resource {0}: {1}".format(arn,
-                         to_native(e)), exception=traceback.format_exc(),
-                         **camel_dict_to_snake_dict(e.response))
-    except BotoCoreError as e:
-        module.fail_json(msg="Unable to tag resource {0}: {1}".format(arn,
-                         to_native(e)), exception=traceback.format_exc())
+    except (BotoCoreError, ClientError) as e:
+        module.fail_json_aws(e, msg="Unable to tag resource {0}".format(arn))
 
     return changed
 
@@ -389,22 +379,21 @@ def main():
     check_mode = module.check_mode
     changed = False
 
-    region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True)
-    if not region:
-        module.fail_json(msg='region must be specified')
-
     try:
-        client = boto3_conn(module, conn_type='client', resource='lambda',
-                            region=region, endpoint=ec2_url, **aws_connect_kwargs)
-    except (ClientError, ValidationError) as e:
+        client = module.client('lambda', retry_decorator=AWSRetry.jittered_backoff())
+    except (ClientError, BotoCoreError) as e:
         module.fail_json_aws(e, msg="Trying to connect to AWS")
 
+    if tags is not None:
+        if not hasattr(client, "list_tags"):
+            module.fail_json(msg="Using tags requires botocore 1.5.40 or above")
+
     if state == 'present':
         if re.match(r'^arn:aws(-([a-z\-]+))?:iam', role):
             role_arn = role
         else:
             # get account ID and assemble ARN
-            account_id, partition = get_account_info(module, region=region, endpoint=ec2_url, **aws_connect_kwargs)
+            account_id, partition = get_account_info(module)
             role_arn = 'arn:{0}:iam::{1}:role/{2}'.format(partition, account_id, role)
 
     # Get function configuration if present, False otherwise
@@ -447,9 +436,7 @@ def main():
             func_kwargs.update({'TracingConfig': {'Mode': tracing_mode}})
 
         # If VPC configuration is desired
-        if vpc_subnet_ids or vpc_security_group_ids:
-            if not vpc_subnet_ids or not vpc_security_group_ids:
-                module.fail_json(msg='vpc connectivity requires at least one security group and one subnet')
+        if vpc_subnet_ids:
 
             if 'VpcConfig' in current_config:
                 # Compare VPC config with current config
@@ -472,10 +459,10 @@ def main():
         if len(func_kwargs) > 1:
             try:
                 if not check_mode:
-                    response = client.update_function_configuration(**func_kwargs)
+                    response = client.update_function_configuration(aws_retry=True, **func_kwargs)
                     current_version = response['Version']
                 changed = True
-            except (ParamValidationError, ClientError) as e:
+            except (BotoCoreError, ClientError) as e:
                 module.fail_json_aws(e, msg="Trying to update lambda configuration")
 
         # Update code configuration
@@ -513,10 +500,10 @@ def main():
         if len(code_kwargs) > 2:
             try:
                 if not check_mode:
-                    response = client.update_function_code(**code_kwargs)
+                    response = client.update_function_code(aws_retry=True, **code_kwargs)
                     current_version = response['Version']
                 changed = True
-            except (ParamValidationError, ClientError) as e:
+            except (BotoCoreError, ClientError) as e:
                 module.fail_json_aws(e, msg="Trying to upload new code")
 
         # Describe function code and configuration
@@ -573,21 +560,18 @@ def main():
             func_kwargs.update({'TracingConfig': {'Mode': tracing_mode}})
 
         # If VPC configuration is given
-        if vpc_subnet_ids or vpc_security_group_ids:
-            if not vpc_subnet_ids or not vpc_security_group_ids:
-                module.fail_json(msg='vpc connectivity requires at least one security group and one subnet')
-
+        if vpc_subnet_ids:
             func_kwargs.update({'VpcConfig': {'SubnetIds': vpc_subnet_ids,
                                               'SecurityGroupIds': vpc_security_group_ids}})
 
         # Finally try to create function
         current_version = None
         try:
             if not check_mode:
-                response = client.create_function(**func_kwargs)
+                response = client.create_function(aws_retry=True, **func_kwargs)
                 current_version = response['Version']
             changed = True
-        except (ParamValidationError, ClientError) as e:
+        except (BotoCoreError, ClientError) as e:
             module.fail_json_aws(e, msg="Trying to create function")
 
         # Tag Function
@@ -604,9 +588,9 @@ def main():
     if state == 'absent' and current_function:
         try:
             if not check_mode:
-                client.delete_function(FunctionName=name)
+                client.delete_function(FunctionName=name, aws_retry=True)
             changed = True
-        except (ParamValidationError, ClientError) as e:
+        except (BotoCoreError, ClientError) as e:
             module.fail_json_aws(e, msg="Trying to delete Lambda function")
 
         module.exit_json(changed=changed)

tests/integration/targets/aws_lambda/aliases → tests/integration/targets/lambda/aliases

@@ -1,5 +1,4 @@
 cloud/aws
 shippable/aws/group2
 execute_lambda
-lambda
 lambda_info

tests/integration/targets/aws_lambda/tasks/main.yml → tests/integration/targets/lambda/tasks/main.yml

@@ -9,7 +9,7 @@
     - community.general
   block:
   - name: test with no parameters
-    lambda: null
+    lambda:
     register: result
     ignore_errors: true
   - name: assert failure when called with no parameters
@@ -44,37 +44,14 @@
       runtime: python2.7
       handler: no-handler
       role: arn:fake-role-doesnt-exist
-      region: null
-    register: result
-    ignore_errors: true
-  - name: assert failure when called with only 'name'
-    assert:
-      that:
-      - result.failed
-      - result.msg == "region must be specified"
-  - name: test with all module required variables, no region and all possible variables set to blank
-    lambda:
-      name: ansible-testing-fake-should-not-be-created
-      state: present
-      runtime: python2.7
-      role: arn:fake-role-doesnt-exist
-      handler: null
-      s3_bucket: null
-      s3_key: null
-      s3_object_version: null
-      description: null
-      vpc_subnet_ids: null
-      vpc_security_group_ids: null
-      environment_variables: null
-      dead_letter_arn: null
-      region: null
+      region: '{{ omit }}'
     register: result
     ignore_errors: true
   - name: assert failure when called with only 'name'
     assert:
       that:
       - result.failed
-      - result.msg == "region must be specified"
+      - '"requires a region and none was found" in result.msg'
   - name: move lambda into place for archive module
     copy:
       src: mini_lambda.py
@@ -233,12 +210,12 @@
       runtime: python2.7
       role: ansible_lambda_role
       zip_file: '{{zip_res.dest}}'
-      handler: null
-      description: null
-      vpc_subnet_ids: null
+      handler: '{{ omit }}'
+      description: '{{ omit }}'
+      vpc_subnet_ids: '{{ omit }}'
       vpc_security_group_ids: sg-FA6E
-      environment_variables: null
-      dead_letter_arn: null
+      environment_variables: '{{ omit }}'
+      dead_letter_arn: '{{ omit }}'
     register: result
     ignore_errors: true
   - name: assert lambda fails with proper message
@@ -247,7 +224,7 @@
       - result is failed
       - result.msg != "MODULE FAILURE"
       - result.changed == False
-      - '"requires at least one security group and one subnet" in result.msg'
+      - '"parameters are required together" in result.msg'
   - name: test state=present with all nullable variables explicitly set to null
     lambda:
       name: '{{lambda_function_name}}'