Migrations to AnsibleAWSModule (and related cleanup) (#38)

* Move last boto 2 modules to AnsibleAWSModule

* Move last boto3 modules to AnsibleAWSModule

* Remove last use of ec2_argument_spec in modules

* HAS_BOTO3 logic no longer required - Using AnsibleAWSModule

* Be more consistent in our use of HAS_BOTO

* Split imports by line - much easier to work with during refactoring/cleanup

* Use module.client() to get boto3 clients

* Shuffle import orders for consistency

* Pull camel_dict_to_snake_dict from its canonical source

* Use module.fail_json_aws and is_boto3_error_code to simplify error handling

* tweak integration tests to allow more information in the error messages

plugins/modules/cloudformation.py

@@ -330,19 +330,15 @@
 try:
     import boto3
     import botocore
-    HAS_BOTO3 = True
 except ImportError:
-    HAS_BOTO3 = False
+    pass  # Handled by AnsibleAWSModule
 
-from ansible_collections.amazon.aws.plugins.module_utils.ec2 import (ansible_dict_to_boto3_tag_list,
-                                                                     AWSRetry,
-                                                                     boto3_conn,
-                                                                     boto_exception,
-                                                                     ec2_argument_spec,
-                                                                     get_aws_connection_info,
-                                                                     )
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils._text import to_bytes, to_native
+from ansible.module_utils._text import to_bytes
+from ansible.module_utils._text import to_native
+from ansible_collections.amazon.aws.plugins.module_utils.aws.core import AnsibleAWSModule
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_tag_list
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto_exception
 
 
 def get_stack_events(cfn, stack_name, events_limit, token_filter=None):
@@ -406,8 +402,7 @@ def create_stack(module, stack_params, cfn, events_limit):
         # Use stack ID to follow stack state in case of on_create_failure = DELETE
         result = stack_operation(cfn, response['StackId'], 'CREATE', events_limit, stack_params.get('ClientRequestToken', None))
     except Exception as err:
-        error_msg = boto_exception(err)
-        module.fail_json(msg="Failed to create stack {0}: {1}.".format(stack_params.get('StackName'), error_msg), exception=traceback.format_exc())
+        module.fail_json_aws(err, msg="Failed to create stack {0}".format(stack_params.get('StackName')))
     if not result:
         module.fail_json(msg="empty result")
     return result
@@ -444,8 +439,7 @@ def create_changeset(module, stack_params, cfn, events_limit):
                 try:
                     newcs = cfn.describe_change_set(ChangeSetName=cs['Id'])
                 except botocore.exceptions.BotoCoreError as err:
-                    error_msg = boto_exception(err)
-                    module.fail_json(msg=error_msg)
+                    module.fail_json_aws(err)
                 if newcs['Status'] == 'CREATE_PENDING' or newcs['Status'] == 'CREATE_IN_PROGRESS':
                     time.sleep(1)
                 elif newcs['Status'] == 'FAILED' and "The submitted information didn't contain changes" in newcs['StatusReason']:
@@ -469,7 +463,7 @@ def create_changeset(module, stack_params, cfn, events_limit):
         if 'No updates are to be performed.' in error_msg:
             result = dict(changed=False, output='Stack is already up-to-date.')
         else:
-            module.fail_json(msg="Failed to create change set: {0}".format(error_msg), exception=traceback.format_exc())
+            module.fail_json_aws(err, msg='Failed to create change set')
 
     if not result:
         module.fail_json(msg="empty result")
@@ -491,7 +485,7 @@ def update_stack(module, stack_params, cfn, events_limit):
         if 'No updates are to be performed.' in error_msg:
             result = dict(changed=False, output='Stack is already up-to-date.')
         else:
-            module.fail_json(msg="Failed to update stack {0}: {1}".format(stack_params.get('StackName'), error_msg), exception=traceback.format_exc())
+            module.fail_json_aws(err, msg="Failed to update stack {0}".format(stack_params.get('StackName')))
     if not result:
         module.fail_json(msg="empty result")
     return result
@@ -509,7 +503,7 @@ def update_termination_protection(module, cfn, stack_name, desired_termination_p
                     EnableTerminationProtection=desired_termination_protection_state,
                     StackName=stack_name)
             except botocore.exceptions.ClientError as e:
-                module.fail_json(msg=boto_exception(e), exception=traceback.format_exc())
+                module.fail_json_aws(e)
 
 
 def boto_supports_termination_protection(cfn):
@@ -605,8 +599,7 @@ def check_mode_changeset(module, stack_params, cfn):
         return {'changed': True, 'msg': reason, 'meta': description['Changes']}
 
     except (botocore.exceptions.ValidationError, botocore.exceptions.ClientError) as err:
-        error_msg = boto_exception(err)
-        module.fail_json(msg=error_msg, exception=traceback.format_exc())
+        module.fail_json_aws(err)
 
 
 def get_stack_facts(cfn, stack_name):
@@ -631,8 +624,7 @@ def get_stack_facts(cfn, stack_name):
 
 
 def main():
-    argument_spec = ec2_argument_spec()
-    argument_spec.update(dict(
+    argument_spec = dict(
         stack_name=dict(required=True),
         template_parameters=dict(required=False, type='dict', default={}),
         state=dict(default='present', choices=['present', 'absent']),
@@ -656,16 +648,13 @@ def main():
         backoff_max_delay=dict(type='int', default=30, required=False),
         capabilities=dict(type='list', default=['CAPABILITY_IAM', 'CAPABILITY_NAMED_IAM'])
     )
-    )
 
-    module = AnsibleModule(
+    module = AnsibleAWSModule(
         argument_spec=argument_spec,
         mutually_exclusive=[['template_url', 'template', 'template_body'],
                             ['disable_rollback', 'on_create_failure']],
         supports_check_mode=True
     )
-    if not HAS_BOTO3:
-        module.fail_json(msg='boto3 and botocore are required for this module')
 
     invalid_capabilities = []
     user_capabilities = module.params.get('capabilities')
@@ -731,11 +720,7 @@ def main():
 
     result = {}
 
-    try:
-        region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True)
-        cfn = boto3_conn(module, conn_type='client', resource='cloudformation', region=region, endpoint=ec2_url, **aws_connect_kwargs)
-    except botocore.exceptions.NoCredentialsError as e:
-        module.fail_json(msg=boto_exception(e))
+    cfn = module.client('cloudformation')
 
     # Wrap the cloudformation client methods that this module uses with
     # automatic backoff / retry for throttling error codes
@@ -817,7 +802,7 @@ def main():
                 result = stack_operation(cfn, stack_params['StackName'], 'DELETE', module.params.get('events_limit'),
                                          stack_params.get('ClientRequestToken', None))
         except Exception as err:
-            module.fail_json(msg=boto_exception(err), exception=traceback.format_exc())
+            module.fail_json_aws(err)
 
     module.exit_json(**result)
 

plugins/modules/ec2.py

@@ -582,24 +582,27 @@
 
 import time
 import datetime
-import traceback
 from ast import literal_eval
 from distutils.version import LooseVersion
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible_collections.amazon.aws.plugins.module_utils.ec2 import get_aws_connection_info, ec2_argument_spec, ec2_connect
-from ansible.module_utils.six import get_function_code, string_types
-from ansible.module_utils._text import to_bytes, to_text
-
 try:
     import boto.ec2
-    from boto.ec2.blockdevicemapping import BlockDeviceType, BlockDeviceMapping
+    from boto.ec2.blockdevicemapping import BlockDeviceType
+    from boto.ec2.blockdevicemapping import BlockDeviceMapping
     from boto.exception import EC2ResponseError
     from boto import connect_ec2_endpoint
     from boto import connect_vpc
-    HAS_BOTO = True
 except ImportError:
-    HAS_BOTO = False
+    pass  # Taken care of by ec2.HAS_BOTO
+
+from ansible.module_utils.six import get_function_code
+from ansible.module_utils.six import string_types
+from ansible.module_utils._text import to_bytes
+from ansible.module_utils._text import to_text
+from ansible_collections.amazon.aws.plugins.module_utils.aws.core import AnsibleAWSModule
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import HAS_BOTO
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ec2_connect
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import get_aws_connection_info
 
 
 def find_running_instances_by_count_tag(module, ec2, vpc, count_tag, zone=None):
@@ -966,7 +969,7 @@ def create_instances(module, ec2, vpc, override_count=None):
     """
     Creates new instances
 
-    module : AnsibleModule object
+    module : AnsibleAWSModule object
     ec2: authenticated ec2 connection object
 
     Returns:
@@ -1041,7 +1044,7 @@ def create_instances(module, ec2, vpc, override_count=None):
             grp_details = ec2.get_all_security_groups(group_ids=group_id)
             group_name = [grp_item.name for grp_item in grp_details]
     except boto.exception.NoAuthHandlerFound as e:
-        module.fail_json(msg=str(e))
+        module.fail_json_aws(e, msg='Unable to authenticate to AWS')
 
     # Lookup any instances that much our run id.
 
@@ -1182,11 +1185,11 @@ def create_instances(module, ec2, vpc, override_count=None):
                         ec2.get_all_instances(instids)
                         break
                     except boto.exception.EC2ResponseError as e:
-                        if "<Code>InvalidInstanceID.NotFound</Code>" in str(e):
+                        if e.error_code == 'InvalidInstanceID.NotFound':
                             # there's a race between start and get an instance
                             continue
                         else:
-                            module.fail_json(msg=str(e))
+                            module.fail_json_aws(e)
 
                 # The instances returned through ec2.run_instances above can be in
                 # terminated state due to idempotency. See commit 7f11c3d for a complete
@@ -1239,7 +1242,7 @@ def create_instances(module, ec2, vpc, override_count=None):
                 else:
                     instids = []
         except boto.exception.BotoServerError as e:
-            module.fail_json(msg="Instance creation failed => %s: %s" % (e.error_code, e.error_message))
+            module.fail_json_aws(e, msg='Instance creation failed')
 
         # wait here until the instances are up
         num_running = 0
@@ -1291,7 +1294,7 @@ def create_instances(module, ec2, vpc, override_count=None):
             try:
                 ec2.create_tags(instids, instance_tags)
             except boto.exception.EC2ResponseError as e:
-                module.fail_json(msg="Instance tagging failed => %s: %s" % (e.error_code, e.error_message))
+                module.fail_json_aws(e, msg='Instance tagging failed')
 
     instance_dict_array = []
     created_instance_ids = []
@@ -1340,7 +1343,7 @@ def terminate_instances(module, ec2, instance_ids):
                 try:
                     ec2.terminate_instances([inst.id])
                 except EC2ResponseError as e:
-                    module.fail_json(msg='Unable to terminate instance {0}, error: {1}'.format(inst.id, e))
+                    module.fail_json_aws(e, msg='Unable to terminate instance {0}'.format(inst.id))
                 changed = True
 
     # wait here until the instances are 'terminated'
@@ -1456,7 +1459,7 @@ def startstop_instances(module, ec2, instance_ids, state, instance_tags):
                     else:
                         inst.stop()
                 except EC2ResponseError as e:
-                    module.fail_json(msg='Unable to change state for instance {0}, error: {1}'.format(inst.id, e))
+                    module.fail_json_aws(e, 'Unable to change state for instance {0}'.format(inst.id))
                 changed = True
             existing_instances_array.append(inst.id)
 
@@ -1542,7 +1545,7 @@ def restart_instances(module, ec2, instance_ids, state, instance_tags):
                 try:
                     inst.reboot()
                 except EC2ResponseError as e:
-                    module.fail_json(msg='Unable to change state for instance {0}, error: {1}'.format(inst.id, e))
+                    module.fail_json_aws(e, msg='Unable to change state for instance {0}'.format(inst.id))
                 changed = True
 
     return (changed, instance_dict_array, instance_ids)
@@ -1592,8 +1595,7 @@ def check_source_dest_attr(module, inst, ec2):
                         ec2.modify_network_interface_attribute(interface.id, "sourceDestCheck", source_dest_check)
                         return True
             else:
-                module.fail_json(msg='Failed to handle source_dest_check state for instance {0}, error: {1}'.format(inst.id, exc),
-                                 exception=traceback.format_exc())
+                module.fail_json_aws(exc, msg='Failed to handle source_dest_check state for instance {0}'.format(inst.id))
 
 
 def warn_if_public_ip_assignment_changed(module, instance):
@@ -1608,49 +1610,47 @@ def warn_if_public_ip_assignment_changed(module, instance):
 
 
 def main():
-    argument_spec = ec2_argument_spec()
-    argument_spec.update(
-        dict(
-            key_name=dict(aliases=['keypair']),
-            id=dict(),
-            group=dict(type='list', aliases=['groups']),
-            group_id=dict(type='list'),
-            zone=dict(aliases=['aws_zone', 'ec2_zone']),
-            instance_type=dict(aliases=['type']),
-            spot_price=dict(),
-            spot_type=dict(default='one-time', choices=["one-time", "persistent"]),
-            spot_launch_group=dict(),
-            image=dict(),
-            kernel=dict(),
-            count=dict(type='int', default='1'),
-            monitoring=dict(type='bool', default=False),
-            ramdisk=dict(),
-            wait=dict(type='bool', default=False),
-            wait_timeout=dict(type='int', default=300),
-            spot_wait_timeout=dict(type='int', default=600),
-            placement_group=dict(),
-            user_data=dict(),
-            instance_tags=dict(type='dict'),
-            vpc_subnet_id=dict(),
-            assign_public_ip=dict(type='bool'),
-            private_ip=dict(),
-            instance_profile_name=dict(),
-            instance_ids=dict(type='list', aliases=['instance_id']),
-            source_dest_check=dict(type='bool', default=None),
-            termination_protection=dict(type='bool', default=None),
-            state=dict(default='present', choices=['present', 'absent', 'running', 'restarted', 'stopped']),
-            instance_initiated_shutdown_behavior=dict(default='stop', choices=['stop', 'terminate']),
-            exact_count=dict(type='int', default=None),
-            count_tag=dict(type='raw'),
-            volumes=dict(type='list'),
-            ebs_optimized=dict(type='bool', default=False),
-            tenancy=dict(default='default', choices=['default', 'dedicated']),
-            network_interfaces=dict(type='list', aliases=['network_interface'])
-        )
+    argument_spec = dict(
+        key_name=dict(aliases=['keypair']),
+        id=dict(),
+        group=dict(type='list', aliases=['groups']),
+        group_id=dict(type='list'),
+        zone=dict(aliases=['aws_zone', 'ec2_zone']),
+        instance_type=dict(aliases=['type']),
+        spot_price=dict(),
+        spot_type=dict(default='one-time', choices=["one-time", "persistent"]),
+        spot_launch_group=dict(),
+        image=dict(),
+        kernel=dict(),
+        count=dict(type='int', default='1'),
+        monitoring=dict(type='bool', default=False),
+        ramdisk=dict(),
+        wait=dict(type='bool', default=False),
+        wait_timeout=dict(type='int', default=300),
+        spot_wait_timeout=dict(type='int', default=600),
+        placement_group=dict(),
+        user_data=dict(),
+        instance_tags=dict(type='dict'),
+        vpc_subnet_id=dict(),
+        assign_public_ip=dict(type='bool'),
+        private_ip=dict(),
+        instance_profile_name=dict(),
+        instance_ids=dict(type='list', aliases=['instance_id']),
+        source_dest_check=dict(type='bool', default=None),
+        termination_protection=dict(type='bool', default=None),
+        state=dict(default='present', choices=['present', 'absent', 'running', 'restarted', 'stopped']),
+        instance_initiated_shutdown_behavior=dict(default='stop', choices=['stop', 'terminate']),
+        exact_count=dict(type='int', default=None),
+        count_tag=dict(type='raw'),
+        volumes=dict(type='list'),
+        ebs_optimized=dict(type='bool', default=False),
+        tenancy=dict(default='default', choices=['default', 'dedicated']),
+        network_interfaces=dict(type='list', aliases=['network_interface'])
     )
 
-    module = AnsibleModule(
+    module = AnsibleAWSModule(
         argument_spec=argument_spec,
+        check_boto3=False,
         mutually_exclusive=[
             # Can be uncommented when we finish the deprecation cycle.
             # ['group', 'group_id'],
@@ -1686,7 +1686,7 @@ def main():
 
         vpc = connect_vpc(**aws_connect_kwargs)
     except boto.exception.NoAuthHandlerFound as e:
-        module.fail_json(msg="Failed to get connection: %s" % e.message, exception=traceback.format_exc())
+        module.fail_json_aws(e, msg='Failed to get connection')
 
     tagged_instances = []