From c93ddf5473f7604f48d34293ca77f37ff7183f47 Mon Sep 17 00:00:00 2001 From: Will Thames Date: Wed, 8 Nov 2017 04:56:17 +1000 Subject: [PATCH] Move profile and region checking to module_utils.ec2 (#31921) * Move profile and region checking to module_utils.ec2 Remove ProfileNotFound checking from individual modules There are plenty of `if not region:` checks that could be removed, once more thorough testing of this change has occured The ec2_asg, iam_managed_policy and ec2_vpc_subnet_facts modules would also benefit from this change but as they do not have tests and are marked stableinterface, they do not get this change. --- lib/ansible/module_utils/ec2.py | 11 ++++++++--- lib/ansible/modules/cloud/amazon/GUIDELINES.md | 12 ++++++------ .../modules/cloud/amazon/aws_acm_facts.py | 9 +++------ lib/ansible/modules/cloud/amazon/aws_s3.py | 5 +---- .../cloud/amazon/aws_s3_bucket_facts.py | 12 ++---------- .../cloud/amazon/cloudwatchevent_rule.py | 18 +++++------------- lib/ansible/modules/cloud/amazon/ec2_group.py | 7 ++----- .../modules/cloud/amazon/ec2_snapshot_copy.py | 10 +++------- .../cloud/amazon/ecs_taskdefinition_facts.py | 10 +++------- .../targets/aws_api_gateway/tasks/main.yml | 4 ++-- .../targets/ec2_group/tasks/main.yml | 4 ++-- 11 files changed, 37 insertions(+), 65 deletions(-) diff --git a/lib/ansible/module_utils/ec2.py b/lib/ansible/module_utils/ec2.py index 30d53881ad..2e45ef1b0e 100644 --- a/lib/ansible/module_utils/ec2.py +++ b/lib/ansible/module_utils/ec2.py @@ -104,9 +104,13 @@ class AWSRetry(CloudRetry): def boto3_conn(module, conn_type=None, resource=None, region=None, endpoint=None, **params): try: return _boto3_conn(conn_type=conn_type, resource=resource, region=region, endpoint=endpoint, **params) - except ValueError: - module.fail_json(msg='There is an issue in the code of the module. You must specify either both, resource or client to the conn_type ' - 'parameter in the boto3_conn function call') + except ValueError as e: + module.fail_json(msg="Couldn't connect to AWS: " % to_native(e)) + except (botocore.exceptions.ProfileNotFound, botocore.exceptions.PartialCredentialsError) as e: + module.fail_json(msg=to_native(e)) + except botocore.exceptions.NoRegionError as e: + module.fail_json(msg="The %s module requires a region and none was found in configuration, " + "environment variables or module parameters" % module._name) def _boto3_conn(conn_type=None, resource=None, region=None, endpoint=None, **params): @@ -129,6 +133,7 @@ def _boto3_conn(conn_type=None, resource=None, region=None, endpoint=None, **par resource = boto3.session.Session(profile_name=profile).resource(resource, region_name=region, endpoint_url=endpoint, **params) return client, resource + boto3_inventory_conn = _boto3_conn diff --git a/lib/ansible/modules/cloud/amazon/GUIDELINES.md b/lib/ansible/modules/cloud/amazon/GUIDELINES.md index a24777fd1d..8e725d23fe 100644 --- a/lib/ansible/modules/cloud/amazon/GUIDELINES.md +++ b/lib/ansible/modules/cloud/amazon/GUIDELINES.md @@ -129,13 +129,14 @@ To connect to AWS, you should use `get_aws_connection_info` and then `boto3_conn These functions handle some of the more esoteric connection options, such as security tokens and boto profiles. -Some boto services require that the region is specified. You should check for the region parameter -if required. #### boto An example of connecting to ec2: +Some boto services require that the region is specified. You should check for the region parameter +if required. + ```python region, ec2_url, aws_connect_params = get_aws_connection_info(module) if region: @@ -153,13 +154,12 @@ An example of connecting to ec2 is shown below. Note that there is no `NoAuthHa exception handling like in boto. Instead, an `AuthFailure` exception will be thrown when you use 'connection'. To ensure that authorization, parameter validation and permissions errors are all caught, you should catch `ClientError` and `BotoCoreError` exceptions with every boto3 connection call. +See exception handling. module_utils.ec2 checks for missing profiles or a region not set when it needs to be, +so you don't have to. ```python region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=True) -if region: - connection = boto3_conn(module, conn_type='client', resource='ec2', region=region, endpoint=ec2_url, **aws_connect_params) -else: - module.fail_json(msg="region must be specified") +connection = boto3_conn(module, conn_type='client', resource='ec2', region=region, endpoint=ec2_url, **aws_connect_params) ``` ### Exception Handling for boto diff --git a/lib/ansible/modules/cloud/amazon/aws_acm_facts.py b/lib/ansible/modules/cloud/amazon/aws_acm_facts.py index 6e435df532..c79bf230d6 100644 --- a/lib/ansible/modules/cloud/amazon/aws_acm_facts.py +++ b/lib/ansible/modules/cloud/amazon/aws_acm_facts.py @@ -315,12 +315,9 @@ def main(): if not HAS_BOTO3: module.fail_json('boto3 and botocore are required by this module') - try: - region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True) - client = boto3_conn(module, conn_type='client', resource='acm', - region=region, endpoint=ec2_url, **aws_connect_kwargs) - except (botocore.exceptions.NoCredentialsError, botocore.exceptions.ProfileNotFound) as e: - module.fail_json(msg="Can't authorize connection - " + str(e)) + region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True) + client = boto3_conn(module, conn_type='client', resource='acm', + region=region, endpoint=ec2_url, **aws_connect_kwargs) certificates = get_certificates(client, module, name=module.params['name'], statuses=module.params['statuses']) module.exit_json(certificates=certificates) diff --git a/lib/ansible/modules/cloud/amazon/aws_s3.py b/lib/ansible/modules/cloud/amazon/aws_s3.py index a57c383fae..b791028a7e 100644 --- a/lib/ansible/modules/cloud/amazon/aws_s3.py +++ b/lib/ansible/modules/cloud/amazon/aws_s3.py @@ -671,10 +671,7 @@ def main(): if s3_url: for key in ['validate_certs', 'security_token', 'profile_name']: aws_connect_kwargs.pop(key, None) - try: - s3 = get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url) - except botocore.exceptions.ProfileNotFound as e: - module.fail_json(msg=to_native(e)) + s3 = get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url) validate = not ignore_nonexistent_bucket diff --git a/lib/ansible/modules/cloud/amazon/aws_s3_bucket_facts.py b/lib/ansible/modules/cloud/amazon/aws_s3_bucket_facts.py index 0b5ae12cc1..ba1f277625 100644 --- a/lib/ansible/modules/cloud/amazon/aws_s3_bucket_facts.py +++ b/lib/ansible/modules/cloud/amazon/aws_s3_bucket_facts.py @@ -90,16 +90,8 @@ def main(): # Set up connection region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=HAS_BOTO3) - - # Set up connection - if region: - try: - connection = boto3_conn(module, conn_type='client', resource='s3', region=region, endpoint=ec2_url, - **aws_connect_params) - except (botocore.exceptions.NoCredentialsError, botocore.exceptions.ProfileNotFound) as e: - module.fail_json(msg=e.message, exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response)) - else: - module.fail_json(msg="AWS region must be specified (like: us-east-1)") + connection = boto3_conn(module, conn_type='client', resource='s3', region=region, endpoint=ec2_url, + **aws_connect_params) # Gather results result['buckets'] = get_bucket_list(module, connection) diff --git a/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py b/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py index bb16223524..05b9784be7 100644 --- a/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py +++ b/lib/ansible/modules/cloud/amazon/cloudwatchevent_rule.py @@ -395,19 +395,11 @@ class CloudWatchEventRuleManager(object): def get_cloudwatchevents_client(module): """Returns a boto3 client for accessing CloudWatch Events""" - try: - region, ec2_url, aws_conn_kwargs = get_aws_connection_info(module, - boto3=True) - if not region: - module.fail_json(msg="Region must be specified as a parameter, in \ - EC2_REGION or AWS_REGION environment variables \ - or in boto configuration file") - return boto3_conn(module, conn_type='client', - resource='events', - region=region, endpoint=ec2_url, - **aws_conn_kwargs) - except botocore.exceptions.ProfileNotFound as e: - module.fail_json(msg=str(e)) + region, ec2_url, aws_conn_kwargs = get_aws_connection_info(module, boto3=True) + return boto3_conn(module, conn_type='client', + resource='events', + region=region, endpoint=ec2_url, + **aws_conn_kwargs) def main(): diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index 877cc57de4..a7068aa65f 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -680,11 +680,8 @@ def main(): changed = False region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=True) - if not region: - module.fail_json(msg="The AWS region must be specified as an " - "environment variable or in the AWS credentials " - "profile.") - client = boto3_conn(module, conn_type='client', resource='ec2', endpoint=ec2_url, region=region, **aws_connect_params) + client = boto3_conn(module, conn_type='client', resource='ec2', + endpoint=ec2_url, region=region, **aws_connect_params) if not has_rule_description_attr(client): all_rules = rules if rules else [] + rules_egress if rules_egress else [] diff --git a/lib/ansible/modules/cloud/amazon/ec2_snapshot_copy.py b/lib/ansible/modules/cloud/amazon/ec2_snapshot_copy.py index b2e1082fd8..9cfc5bc3d8 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_snapshot_copy.py +++ b/lib/ansible/modules/cloud/amazon/ec2_snapshot_copy.py @@ -113,7 +113,7 @@ from ansible.module_utils._text import to_native try: import boto3 - from botocore.exceptions import ClientError, NoCredentialsError, ProfileNotFound, NoRegionError, WaiterError + from botocore.exceptions import ClientError, WaiterError HAS_BOTO3 = True except ImportError: HAS_BOTO3 = False @@ -174,12 +174,8 @@ def main(): module.fail_json(msg='botocore and boto3 are required.') region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True) - if not region: - module.fail_json(msg="Region must be provided.") - try: - client = boto3_conn(module, conn_type='client', resource='ec2', region=region, endpoint=ec2_url, **aws_connect_kwargs) - except (NoCredentialsError, ProfileNotFound) as e: - module.fail_json(msg="Can't authorize connection - %s" % to_native(e)) + client = boto3_conn(module, conn_type='client', resource='ec2', + region=region, endpoint=ec2_url, **aws_connect_kwargs) copy_snapshot(module, client) diff --git a/lib/ansible/modules/cloud/amazon/ecs_taskdefinition_facts.py b/lib/ansible/modules/cloud/amazon/ecs_taskdefinition_facts.py index 045e85862d..b29aa28fbb 100644 --- a/lib/ansible/modules/cloud/amazon/ecs_taskdefinition_facts.py +++ b/lib/ansible/modules/cloud/amazon/ecs_taskdefinition_facts.py @@ -321,13 +321,9 @@ def main(): if not HAS_BOTO3: module.fail_json(msg='boto3 is required.') - try: - region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True) - if not region: - module.fail_json(msg="Region must be specified as a parameter, in EC2_REGION or AWS_REGION environment variables or in boto configuration file") - ecs = boto3_conn(module, conn_type='client', resource='ecs', region=region, endpoint=ec2_url, **aws_connect_kwargs) - except botocore.exceptions.ProfileNotFound as e: - module.fail_json(msg=str(e)) + region, ec2_url, aws_connect_kwargs = get_aws_connection_info(module, boto3=True) + ecs = boto3_conn(module, conn_type='client', resource='ecs', + region=region, endpoint=ec2_url, **aws_connect_kwargs) ecs_td = ecs.describe_task_definition(taskDefinition=module.params['task_definition'])['taskDefinition'] ecs_td_snake = {} diff --git a/test/integration/targets/aws_api_gateway/tasks/main.yml b/test/integration/targets/aws_api_gateway/tasks/main.yml index 5b4c2691e2..cf09562346 100644 --- a/test/integration/targets/aws_api_gateway/tasks/main.yml +++ b/test/integration/targets/aws_api_gateway/tasks/main.yml @@ -10,7 +10,7 @@ assert: that: - 'result.failed' - - 'result.msg.startswith("Region must be specified")' + - 'result.msg.startswith("The aws_api_gateway module requires a region")' # ============================================================ - name: test with minimal parameters but no region @@ -23,7 +23,7 @@ assert: that: - 'result.failed' - - 'result.msg.startswith("Region must be specified")' + - 'result.msg.startswith("The aws_api_gateway module requires a region")' # ============================================================ - name: test disallow multiple swagger sources diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml index 6ec69601a4..f2b91b1770 100644 --- a/test/integration/targets/ec2_group/tasks/main.yml +++ b/test/integration/targets/ec2_group/tasks/main.yml @@ -108,7 +108,7 @@ assert: that: - 'result.failed' - - 'result.msg.startswith("The AWS region must be specified as an environment variable or in the AWS credentials profile")' + - 'result.msg.startswith("The ec2_group module requires a region")' # ============================================================ - name: test valid ec2_url parameter @@ -124,7 +124,7 @@ assert: that: - 'result.failed' - - 'result.msg.startswith("The AWS region must be specified as an environment variable or in the AWS credentials profile")' + - 'result.msg.startswith("The ec2_group module requires a region")' # ============================================================ - name: test credentials from environment