From bd98a2a93d4953f3008592b3e2eb73b4d1a865c2 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 10:57:24 +0100 Subject: [PATCH 01/13] import order --- plugins/modules/ec2_vpc_igw.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/plugins/modules/ec2_vpc_igw.py b/plugins/modules/ec2_vpc_igw.py index b920682b76c..50c2c52adf4 100644 --- a/plugins/modules/ec2_vpc_igw.py +++ b/plugins/modules/ec2_vpc_igw.py @@ -85,17 +85,16 @@ except ImportError: pass # caught by AnsibleAWSModule +from ansible.module_utils.six import string_types + from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.waiters import get_waiter -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ( - AWSRetry, - camel_dict_to_snake_dict, - boto3_tag_list_to_ansible_dict, - ansible_dict_to_boto3_filter_list, - ansible_dict_to_boto3_tag_list, - compare_aws_tags -) -from ansible.module_utils.six import string_types +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list +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 boto3_tag_list_to_ansible_dict +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_aws_tags class AnsibleEc2Igw(object): From e2051ae4d54f03c4a97f53b8c6d90eb9b1711c1f Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 10:57:43 +0100 Subject: [PATCH 02/13] Add retry decorators --- plugins/modules/ec2_vpc_igw.py | 21 +++++++++++---------- plugins/modules/ec2_vpc_igw_info.py | 7 ++++--- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/plugins/modules/ec2_vpc_igw.py b/plugins/modules/ec2_vpc_igw.py index 50c2c52adf4..11a062423b9 100644 --- a/plugins/modules/ec2_vpc_igw.py +++ b/plugins/modules/ec2_vpc_igw.py @@ -102,7 +102,7 @@ class AnsibleEc2Igw(object): def __init__(self, module, results): self._module = module self._results = results - self._connection = self._module.client('ec2') + self._connection = self._module.client('ec2', retry_decorator=AWSRetry.jittered_backoff()) self._check_mode = self._module.check_mode def process(self): @@ -119,7 +119,7 @@ def get_matching_igw(self, vpc_id): filters = ansible_dict_to_boto3_filter_list({'attachment.vpc-id': vpc_id}) igws = [] try: - response = self._connection.describe_internet_gateways(Filters=filters) + response = self._connection.describe_internet_gateways(aws_retry=True, Filters=filters) igws = response.get('InternetGateways', []) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: self._module.fail_json_aws(e) @@ -144,7 +144,7 @@ def ensure_tags(self, igw_id, tags, add_only): filters = ansible_dict_to_boto3_filter_list({'resource-id': igw_id, 'resource-type': 'internet-gateway'}) cur_tags = None try: - cur_tags = self._connection.describe_tags(Filters=filters) + cur_tags = self._connection.describe_tags(aws_retry=True, Filters=filters) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: self._module.fail_json_aws(e, msg="Couldn't describe tags") @@ -158,7 +158,8 @@ def ensure_tags(self, igw_id, tags, add_only): # update tags final_tags.update(to_update) else: - AWSRetry.exponential_backoff()(self._connection.create_tags)( + self._connection.create_tags( + aws_retry=True, Resources=[igw_id], Tags=ansible_dict_to_boto3_tag_list(to_update) ) @@ -178,7 +179,7 @@ def ensure_tags(self, igw_id, tags, add_only): for key in to_delete: tags_list.append({'Key': key}) - AWSRetry.exponential_backoff()(self._connection.delete_tags)(Resources=[igw_id], Tags=tags_list) + self._connection.delete_tags(aws_retry=True, Resources=[igw_id], Tags=tags_list) self._results['changed'] = True except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: @@ -186,7 +187,7 @@ def ensure_tags(self, igw_id, tags, add_only): if not self._check_mode and (to_update or to_delete): try: - response = self._connection.describe_tags(Filters=filters) + response = self._connection.describe_tags(aws_retry=True, Filters=filters) final_tags = boto3_tag_list_to_ansible_dict(response.get('Tags')) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: self._module.fail_json_aws(e, msg="Couldn't describe tags") @@ -212,8 +213,8 @@ def ensure_igw_absent(self, vpc_id): try: self._results['changed'] = True - self._connection.detach_internet_gateway(InternetGatewayId=igw['internet_gateway_id'], VpcId=vpc_id) - self._connection.delete_internet_gateway(InternetGatewayId=igw['internet_gateway_id']) + self._connection.detach_internet_gateway(aws_retry=True, InternetGatewayId=igw['internet_gateway_id'], VpcId=vpc_id) + self._connection.delete_internet_gateway(aws_retry=True, InternetGatewayId=igw['internet_gateway_id']) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: self._module.fail_json_aws(e, msg="Unable to delete Internet Gateway") @@ -231,14 +232,14 @@ def ensure_igw_present(self, vpc_id, tags): return self._results try: - response = self._connection.create_internet_gateway() + response = self._connection.create_internet_gateway(aws_retry=True) # Ensure the gateway exists before trying to attach it or add tags waiter = get_waiter(self._connection, 'internet_gateway_exists') waiter.wait(InternetGatewayIds=[response['InternetGateway']['InternetGatewayId']]) igw = camel_dict_to_snake_dict(response['InternetGateway']) - self._connection.attach_internet_gateway(InternetGatewayId=igw['internet_gateway_id'], VpcId=vpc_id) + self._connection.attach_internet_gateway(aws_retry=True, InternetGatewayId=igw['internet_gateway_id'], VpcId=vpc_id) self._results['changed'] = True except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: self._module.fail_json_aws(e, msg='Unable to create Internet Gateway') diff --git a/plugins/modules/ec2_vpc_igw_info.py b/plugins/modules/ec2_vpc_igw_info.py index 4719d495fd8..c80daf44d45 100644 --- a/plugins/modules/ec2_vpc_igw_info.py +++ b/plugins/modules/ec2_vpc_igw_info.py @@ -94,6 +94,7 @@ pass # Handled by AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list @@ -105,7 +106,7 @@ def get_internet_gateway_info(internet_gateway): return internet_gateway_info -def list_internet_gateways(client, module): +def list_internet_gateways(connection, module): params = dict() params['Filters'] = ansible_dict_to_boto3_filter_list(module.params.get('filters')) @@ -114,7 +115,7 @@ def list_internet_gateways(client, module): params['InternetGatewayIds'] = module.params.get("internet_gateway_ids") try: - all_internet_gateways = client.describe_internet_gateways(**params) + all_internet_gateways = connection.describe_internet_gateways(aws_retry=True, **params) except botocore.exceptions.ClientError as e: module.fail_json(msg=str(e)) @@ -134,7 +135,7 @@ def main(): # Validate Requirements try: - connection = module.client('ec2') + connection = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff()) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg='Failed to connect to AWS') From c2206673b928f56ee27b521e223f2f7f9dea6675 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 10:58:26 +0100 Subject: [PATCH 03/13] Switch tests to using module_defaults --- .../targets/ec2_vpc_igw/tasks/main.yml | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index 56da84772a2..ba5dccdabe2 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -3,23 +3,19 @@ collections: - amazon.aws + module_defaults: + group/aws: + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token | default(omit) }}" + region: "{{ aws_region }}" block: - - name: set up aws connection info - set_fact: - aws_connection_info: &aws_connection_info - aws_access_key: "{{ aws_access_key }}" - aws_secret_key: "{{ aws_secret_key }}" - security_token: "{{ security_token }}" - region: "{{ aws_region }}" - no_log: yes - # ============================================================ - name: create a VPC ec2_vpc_net: name: "{{ resource_prefix }}-vpc" state: present cidr_block: "10.232.232.128/26" - <<: *aws_connection_info tags: Name: "{{ resource_prefix }}-vpc" Description: "Created by ansible-test" @@ -47,7 +43,6 @@ ec2_vpc_igw: state: present vpc_id: "{{ vpc_result.vpc.id }}" - <<: *aws_connection_info register: vpc_igw_recreate - name: assert recreation did nothing (expected changed=false) @@ -62,7 +57,6 @@ ec2_vpc_igw: state: absent vpc_id: "{{ vpc_result.vpc.id }}" - <<: *aws_connection_info register: vpc_igw_delete - name: assert state=absent (expected changed=true) @@ -76,7 +70,6 @@ ec2_vpc_igw: state: absent vpc_id: "{{ vpc_result.vpc.id }}" - <<: *aws_connection_info ignore_errors: true - name: tidy up VPC @@ -84,5 +77,4 @@ name: "{{ resource_prefix }}-vpc" state: absent cidr_block: "10.232.232.128/26" - <<: *aws_connection_info ignore_errors: true From 1d4dff1d85c3769e208652556d83bc690c4b9dd5 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 10:58:38 +0100 Subject: [PATCH 04/13] module_defaults --- tests/integration/targets/ec2_vpc_igw/tasks/main.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index ba5dccdabe2..4c66d432f2c 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -26,7 +26,6 @@ ec2_vpc_igw: state: present vpc_id: "{{ vpc_result.vpc.id }}" - <<: *aws_connection_info register: vpc_igw_create - name: assert creation happened (expected changed=true) From f1d561e9cb5218671d7512a3099e0542235fba65 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 13:43:52 +0100 Subject: [PATCH 05/13] Add initial _info tests --- tests/integration/targets/ec2_vpc_igw/aliases | 1 + .../targets/ec2_vpc_igw/tasks/main.yml | 108 +++++++++++++++++- 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/tests/integration/targets/ec2_vpc_igw/aliases b/tests/integration/targets/ec2_vpc_igw/aliases index 6e3860bee23..6b8a2ae5af7 100644 --- a/tests/integration/targets/ec2_vpc_igw/aliases +++ b/tests/integration/targets/ec2_vpc_igw/aliases @@ -1,2 +1,3 @@ cloud/aws shippable/aws/group2 +ec2_vpc_igw_info diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index 4c66d432f2c..db4f119559a 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -10,6 +10,18 @@ security_token: "{{ security_token | default(omit) }}" region: "{{ aws_region }}" block: + # ============================================================ + - name: Fetch IGWs in check_mode + ec2_vpc_igw_info: + register: igw_info + check_mode: True + + - name: Assert success + assert: + that: + - igw_info is successful + - '"internet_gateways" in igw_info' + # ============================================================ - name: create a VPC ec2_vpc_net: @@ -37,6 +49,83 @@ - '"tags" in vpc_igw_create' - '"gateway_id" in vpc_igw_create' + # ============================================================ + - name: Save IDs for later + set_fact: + igw_id: '{{ vpc_igw_create.gateway_id }}' + vpc_id: '{{ vpc_result.vpc.id }}' + + # ============================================================ + - name: Search for internet gateway by VPC + ec2_vpc_igw_info: + filters: + attachment.vpc-id: '{{ vpc_id }}' + register: igw_info + + - name: 'Check standard IGW details' + assert: + that: + - '"internet_gateways" in igw_info' + - igw_info.internet_gateways | length == 1 + - '"attachments" in current_igw' + - current_igw.attachments | length == 1 + - '"state" in current_igw.attachments[0]' + - current_igw.attachments[0].state == "available" + - '"vpc_id" in current_igw.attachments[0]' + - current_igw.attachments[0].vpc_id == vpc_id + - '"internet_gateway_id" in current_igw' + - current_igw.internet_gateway_id == igw_id + - '"tags" in current_igw' + vars: + current_igw: '{{ igw_info.internet_gateways[0] }}' + + # ============================================================ + - name: Fetch IGW by ID + ec2_vpc_igw_info: + internet_gateway_ids: '{{ igw_id }}' + register: igw_info + + - name: 'Check standard IGW details' + assert: + that: + - '"internet_gateways" in igw_info' + - igw_info.internet_gateways | length == 1 + - '"attachments" in current_igw' + - current_igw.attachments | length == 1 + - '"state" in current_igw.attachments[0]' + - current_igw.attachments[0].state == "available" + - '"vpc_id" in current_igw.attachments[0]' + - current_igw.attachments[0].vpc_id == vpc_id + - '"internet_gateway_id" in current_igw' + - current_igw.internet_gateway_id == igw_id + - '"tags" in current_igw' + vars: + current_igw: '{{ igw_info.internet_gateways[0] }}' + + # ============================================================ + - name: Fetch IGW by ID (list) + ec2_vpc_igw_info: + internet_gateway_ids: + - '{{ igw_id }}' + register: igw_info + + - name: 'Check standard IGW details' + assert: + that: + - '"internet_gateways" in igw_info' + - igw_info.internet_gateways | length == 1 + - '"attachments" in current_igw' + - current_igw.attachments | length == 1 + - '"state" in current_igw.attachments[0]' + - current_igw.attachments[0].state == "available" + - '"vpc_id" in current_igw.attachments[0]' + - current_igw.attachments[0].vpc_id == vpc_id + - '"internet_gateway_id" in current_igw' + - current_igw.internet_gateway_id == igw_id + - '"tags" in current_igw' + vars: + current_igw: '{{ igw_info.internet_gateways[0] }}' + # ============================================================ - name: attempt to recreate internet gateway on VPC (expected changed=false) ec2_vpc_igw: @@ -47,9 +136,9 @@ - name: assert recreation did nothing (expected changed=false) assert: that: - - 'vpc_igw_recreate.changed == False' - - 'vpc_igw_recreate.gateway_id == vpc_igw_create.gateway_id' - - 'vpc_igw_recreate.vpc_id == vpc_igw_create.vpc_id' + - vpc_igw_recreate is not changed + - vpc_igw_recreate.gateway_id == igw_id + - vpc_igw_recreate.vpc_id == vpc_id # ============================================================ - name: test state=absent (expected changed=true) @@ -63,6 +152,19 @@ that: - 'vpc_igw_delete.changed' + # ============================================================ + - name: Fetch IGW by ID (list) + ec2_vpc_igw_info: + internet_gateway_ids: + - '{{ igw_id }}' + register: igw_info + ignore_errors: True + + - name: 'Check IGW does not exist' + assert: + that: + - igw_info is failed + always: # ============================================================ - name: tidy up IGW From c3d5dc6f38caf799c81eb87b39e61b2159ec041f Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 13:50:32 +0100 Subject: [PATCH 06/13] Handle Boto Errors with fail_json_aws --- plugins/modules/ec2_vpc_igw_info.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ec2_vpc_igw_info.py b/plugins/modules/ec2_vpc_igw_info.py index c80daf44d45..c117e580be6 100644 --- a/plugins/modules/ec2_vpc_igw_info.py +++ b/plugins/modules/ec2_vpc_igw_info.py @@ -94,6 +94,7 @@ pass # Handled by AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule +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 camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list @@ -116,8 +117,10 @@ def list_internet_gateways(connection, module): try: all_internet_gateways = connection.describe_internet_gateways(aws_retry=True, **params) - except botocore.exceptions.ClientError as e: - module.fail_json(msg=str(e)) + except is_boto3_error_code('InvalidInternetGatewayID.NotFound'): + module.fail_json('InternetGateway not found') + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: # pylint: disable=duplicate-except + module.fail_json_aws(e, 'Unable to describe internet gateways') return [camel_dict_to_snake_dict(get_internet_gateway_info(igw)) for igw in all_internet_gateways['InternetGateways']] From e9a3e9502872298f771611f36f513c92bb11b089 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 14:17:06 +0100 Subject: [PATCH 07/13] Test state=absent when IGW missing --- tests/integration/targets/ec2_vpc_igw/tasks/main.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index db4f119559a..fff4cc5debe 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -165,6 +165,18 @@ that: - igw_info is failed + # ============================================================ + - name: test state=absent when already deleted (expected changed=false) + ec2_vpc_igw: + state: absent + vpc_id: "{{ vpc_result.vpc.id }}" + register: vpc_igw_delete + + - name: assert state=absent (expected changed=false) + assert: + that: + - vpc_igw_delete is not changed + always: # ============================================================ - name: tidy up IGW From 9dbbfedf34cfd694c2486f26ac591015ac9463aa Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 15:04:13 +0100 Subject: [PATCH 08/13] Support not purging tags --- plugins/modules/ec2_vpc_igw.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/plugins/modules/ec2_vpc_igw.py b/plugins/modules/ec2_vpc_igw.py index 11a062423b9..3d8d9f3bf25 100644 --- a/plugins/modules/ec2_vpc_igw.py +++ b/plugins/modules/ec2_vpc_igw.py @@ -22,9 +22,16 @@ type: str tags: description: - - "A dict of tags to apply to the internet gateway. Any tags currently applied to the internet gateway and not present here will be removed." + - A dict of tags to apply to the internet gateway. + - To remove all tags set I(tags={}) and I(purge_tags=true). aliases: [ 'resource_tags' ] type: dict + purge_tags: + description: + - Remove tags not listed in I(tags). + type: bool + default: true + version_added: 1.3.0 state: description: - Create or terminate the IGW @@ -109,9 +116,10 @@ def process(self): vpc_id = self._module.params.get('vpc_id') state = self._module.params.get('state', 'present') tags = self._module.params.get('tags') + purge_tags = self._module.params.get('purge_tags') if state == 'present': - self.ensure_igw_present(vpc_id, tags) + self.ensure_igw_present(vpc_id, tags, purge_tags) elif state == 'absent': self.ensure_igw_absent(vpc_id) @@ -134,11 +142,13 @@ def get_matching_igw(self, vpc_id): return igw def check_input_tags(self, tags): + if tags is None: + return nonstring_tags = [k for k, v in tags.items() if not isinstance(v, string_types)] if nonstring_tags: self._module.fail_json(msg='One or more tags contain non-string values: {0}'.format(nonstring_tags)) - def ensure_tags(self, igw_id, tags, add_only): + def ensure_tags(self, igw_id, tags, purge_tags): final_tags = [] filters = ansible_dict_to_boto3_filter_list({'resource-id': igw_id, 'resource-type': 'internet-gateway'}) @@ -148,7 +158,9 @@ def ensure_tags(self, igw_id, tags, add_only): except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: self._module.fail_json_aws(e, msg="Couldn't describe tags") - purge_tags = bool(not add_only) + if tags is None: + return boto3_tag_list_to_ansible_dict(cur_tags.get('Tags')) + to_update, to_delete = compare_aws_tags(boto3_tag_list_to_ansible_dict(cur_tags.get('Tags')), tags, purge_tags) final_tags = boto3_tag_list_to_ansible_dict(cur_tags.get('Tags')) @@ -220,7 +232,7 @@ def ensure_igw_absent(self, vpc_id): return self._results - def ensure_igw_present(self, vpc_id, tags): + def ensure_igw_present(self, vpc_id, tags, purge_tags): self.check_input_tags(tags) igw = self.get_matching_igw(vpc_id) @@ -246,7 +258,7 @@ def ensure_igw_present(self, vpc_id, tags): igw['vpc_id'] = vpc_id - igw['tags'] = self.ensure_tags(igw_id=igw['internet_gateway_id'], tags=tags, add_only=False) + igw['tags'] = self.ensure_tags(igw_id=igw['internet_gateway_id'], tags=tags, purge_tags=purge_tags) igw_info = self.get_igw_info(igw) self._results.update(igw_info) @@ -258,7 +270,8 @@ def main(): argument_spec = dict( vpc_id=dict(required=True), state=dict(default='present', choices=['present', 'absent']), - tags=dict(default=dict(), required=False, type='dict', aliases=['resource_tags']) + tags=dict(required=False, type='dict', aliases=['resource_tags']), + purge_tags=dict(default=True, type='bool'), ) module = AnsibleAWSModule( From 73258a245ca70419275415554ccb6a82de094f79 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sat, 5 Dec 2020 15:04:38 +0100 Subject: [PATCH 09/13] Support converting Tags from boto to dict --- plugins/modules/ec2_vpc_igw_info.py | 31 +++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/plugins/modules/ec2_vpc_igw_info.py b/plugins/modules/ec2_vpc_igw_info.py index c117e580be6..ab7d26a80b4 100644 --- a/plugins/modules/ec2_vpc_igw_info.py +++ b/plugins/modules/ec2_vpc_igw_info.py @@ -27,6 +27,12 @@ - Get details of specific Internet Gateway ID. Provide this value as a list. type: list elements: str + convert_tags: + description: + - Convert tags from boto3 format (list of dictionaries) to the standard dictionary format. + - This currently defaults to C(False). The default will be changed to C(True) after 2022-06-22. + type: bool + version_added: 1.3.0 extends_documentation_fragment: - amazon.aws.aws - amazon.aws.ec2 @@ -96,14 +102,23 @@ from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule 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 camel_dict_to_snake_dict +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict -def get_internet_gateway_info(internet_gateway): +def get_internet_gateway_info(internet_gateway, convert_tags): + if convert_tags: + tags = boto3_tag_list_to_ansible_dict(internet_gateway['Tags']) + ignore_list = ["Tags"] + else: + tags = internet_gateway['Tags'] + ignore_list = [] internet_gateway_info = {'InternetGatewayId': internet_gateway['InternetGatewayId'], 'Attachments': internet_gateway['Attachments'], - 'Tags': internet_gateway['Tags']} + 'Tags': tags} + + internet_gateway_info = camel_dict_to_snake_dict(internet_gateway_info, ignore_list=ignore_list) return internet_gateway_info @@ -111,6 +126,7 @@ def list_internet_gateways(connection, module): params = dict() params['Filters'] = ansible_dict_to_boto3_filter_list(module.params.get('filters')) + convert_tags = module.params.get('convert_tags') if module.params.get("internet_gateway_ids"): params['InternetGatewayIds'] = module.params.get("internet_gateway_ids") @@ -122,7 +138,7 @@ def list_internet_gateways(connection, module): except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: # pylint: disable=duplicate-except module.fail_json_aws(e, 'Unable to describe internet gateways') - return [camel_dict_to_snake_dict(get_internet_gateway_info(igw)) + return [get_internet_gateway_info(igw, convert_tags) for igw in all_internet_gateways['InternetGateways']] @@ -130,12 +146,19 @@ def main(): argument_spec = dict( filters=dict(type='dict', default=dict()), internet_gateway_ids=dict(type='list', default=None, elements='str'), + convert_tags=dict(type='bool'), ) module = AnsibleAWSModule(argument_spec=argument_spec, supports_check_mode=True) if module._name == 'ec2_vpc_igw_facts': module.deprecate("The 'ec2_vpc_igw_facts' module has been renamed to 'ec2_vpc_igw_info'", date='2021-12-01', collection_name='community.aws') + if module.params.get('convert_tags') is None: + module.deprecate('This module currently returns boto3 style tags by default. ' + 'This default has been deprecated and the module will return a simple dictionary in future. ' + 'This behaviour can be controlled through the convert_tags parameter.', + date='2021-12-01', collection_name='community.aws') + # Validate Requirements try: connection = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff()) From 8124fb90edf828eb33f59d98a922030d44d9db57 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sun, 6 Dec 2020 10:23:11 +0100 Subject: [PATCH 10/13] Add tagging tests --- .../targets/ec2_vpc_igw/tasks/main.yml | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index fff4cc5debe..9f6aacbc0b6 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -38,6 +38,9 @@ ec2_vpc_igw: state: present vpc_id: "{{ vpc_result.vpc.id }}" + tags: + tag_one: '{{ resource_prefix }} One' + "Tag Two": 'two {{ resource_prefix }}' register: vpc_igw_create - name: assert creation happened (expected changed=true) @@ -47,6 +50,9 @@ - 'vpc_igw_create.gateway_id.startswith("igw-")' - 'vpc_igw_create.vpc_id == vpc_result.vpc.id' - '"tags" in vpc_igw_create' + - vpc_igw_create.tags | length == 2 + - vpc_igw_create.tags["tag_one"] == '{{ resource_prefix }} One' + - vpc_igw_create.tags["Tag Two"] == 'two {{ resource_prefix }}' - '"gateway_id" in vpc_igw_create' # ============================================================ @@ -76,6 +82,17 @@ - '"internet_gateway_id" in current_igw' - current_igw.internet_gateway_id == igw_id - '"tags" in current_igw' + - current_igw.tags | length == 2 + - '"key" in current_igw.tags[0]' + - '"value" in current_igw.tags[0]' + - '"key" in current_igw.tags[1]' + - '"value" in current_igw.tags[1]' + # Order isn't guaranteed in boto3 style, so just check the keys and + # values we expect are in there. + - current_igw.tags[0].key in ["tag_one", "Tag Two"] + - current_igw.tags[1].key in ["tag_one", "Tag Two"] + - current_igw.tags[0].value in [resource_prefix + " One", "two " + resource_prefix] + - current_igw.tags[1].value in [resource_prefix + " One", "two " + resource_prefix] vars: current_igw: '{{ igw_info.internet_gateways[0] }}' @@ -83,6 +100,7 @@ - name: Fetch IGW by ID ec2_vpc_igw_info: internet_gateway_ids: '{{ igw_id }}' + convert_tags: yes register: igw_info - name: 'Check standard IGW details' @@ -99,6 +117,11 @@ - '"internet_gateway_id" in current_igw' - current_igw.internet_gateway_id == igw_id - '"tags" in current_igw' + - current_igw.tags | length == 2 + - '"tag_one" in current_igw.tags' + - '"Tag Two" in current_igw.tags' + - current_igw.tags["tag_one"] == '{{ resource_prefix }} One' + - current_igw.tags["Tag Two"] == 'two {{ resource_prefix }}' vars: current_igw: '{{ igw_info.internet_gateways[0] }}' @@ -139,6 +162,91 @@ - vpc_igw_recreate is not changed - vpc_igw_recreate.gateway_id == igw_id - vpc_igw_recreate.vpc_id == vpc_id + - '"tags" in vpc_igw_create' + - vpc_igw_create.tags | length == 2 + - vpc_igw_create.tags["tag_one"] == '{{ resource_prefix }} One' + - vpc_igw_create.tags["Tag Two"] == 'two {{ resource_prefix }}' + + # ============================================================ + - name: Update the tags (no change) + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + tags: + tag_one: '{{ resource_prefix }} One' + "Tag Two": 'two {{ resource_prefix }}' + register: vpc_igw_recreate + + - name: assert recreation did nothing (expected changed=false) + assert: + that: + - vpc_igw_recreate is not changed + - vpc_igw_recreate.gateway_id == igw_id + - vpc_igw_recreate.vpc_id == vpc_id + - '"tags" in vpc_igw_recreate' + - vpc_igw_recreate.tags | length == 2 + - vpc_igw_recreate.tags["tag_one"] == '{{ resource_prefix }} One' + - vpc_igw_recreate.tags["Tag Two"] == 'two {{ resource_prefix }}' + + # ============================================================ + - name: Update the tags - remove and add + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + tags: + tag_three: '{{ resource_prefix }} Three' + "Tag Two": 'two {{ resource_prefix }}' + register: vpc_igw_update + + - name: assert recreation did nothing (expected changed=false) + assert: + that: + - vpc_igw_update is changed + - vpc_igw_update.gateway_id == igw_id + - vpc_igw_update.vpc_id == vpc_id + - '"tags" in vpc_igw_update' + - vpc_igw_update.tags | length == 2 + - vpc_igw_update.tags["tag_three"] == '{{ resource_prefix }} Three' + - vpc_igw_update.tags["Tag Two"] == 'two {{ resource_prefix }}' + + # ============================================================ + - name: Update the tags add without purge + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + purge_tags: no + tags: + tag_one: '{{ resource_prefix }} One' + register: vpc_igw_update + + - name: assert tags added + assert: + that: + - vpc_igw_update is changed + - vpc_igw_update.gateway_id == igw_id + - vpc_igw_update.vpc_id == vpc_id + - '"tags" in vpc_igw_update' + - vpc_igw_update.tags | length == 3 + - vpc_igw_update.tags["tag_one"] == '{{ resource_prefix }} One' + - vpc_igw_update.tags["tag_three"] == '{{ resource_prefix }} Three' + - vpc_igw_update.tags["Tag Two"] == 'two {{ resource_prefix }}' + + # ============================================================ + - name: Remove all tags + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + tags: {} + register: vpc_igw_update + + - name: assert tags added + assert: + that: + - vpc_igw_update is changed + - vpc_igw_update.gateway_id == igw_id + - vpc_igw_update.vpc_id == vpc_id + - '"tags" in vpc_igw_update' + - vpc_igw_update.tags | length == 0 # ============================================================ - name: test state=absent (expected changed=true) From 64b0cb5749a08e91c66bd7470d29c030a082c050 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Sun, 6 Dec 2020 19:55:24 +0100 Subject: [PATCH 11/13] Use random CIDR for VPC --- tests/integration/targets/ec2_vpc_igw/defaults/main.yml | 4 ++++ tests/integration/targets/ec2_vpc_igw/tasks/main.yml | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) create mode 100644 tests/integration/targets/ec2_vpc_igw/defaults/main.yml diff --git a/tests/integration/targets/ec2_vpc_igw/defaults/main.yml b/tests/integration/targets/ec2_vpc_igw/defaults/main.yml new file mode 100644 index 00000000000..eeda091c81c --- /dev/null +++ b/tests/integration/targets/ec2_vpc_igw/defaults/main.yml @@ -0,0 +1,4 @@ +--- +vpc_name: '{{ resource_prefix }}-vpc' +vpc_seed: '{{ resource_prefix }}' +vpc_cidr: '10.{{ 256 | random(seed=vpc_seed) }}.0.0/16' diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index 9f6aacbc0b6..a5961a715ed 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -25,9 +25,9 @@ # ============================================================ - name: create a VPC ec2_vpc_net: - name: "{{ resource_prefix }}-vpc" + name: "{{ vpc_name }}" state: present - cidr_block: "10.232.232.128/26" + cidr_block: "{{ vpc_cidr }}" tags: Name: "{{ resource_prefix }}-vpc" Description: "Created by ansible-test" @@ -295,7 +295,7 @@ - name: tidy up VPC ec2_vpc_net: - name: "{{ resource_prefix }}-vpc" + name: "{{ vpc_name }}" state: absent - cidr_block: "10.232.232.128/26" + cidr_block: "{{ vpc_cidr }}" ignore_errors: true From 8b0417b46d1087fc2f3c373eec2dcd1a02fd32b9 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 8 Dec 2020 12:35:33 +0100 Subject: [PATCH 12/13] Add check_mode tests --- .../targets/ec2_vpc_igw/tasks/main.yml | 138 +++++++++++++++++- 1 file changed, 133 insertions(+), 5 deletions(-) diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index a5961a715ed..634438c0875 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -33,7 +33,40 @@ Description: "Created by ansible-test" register: vpc_result + - name: Assert success + assert: + that: + - vpc_result is successful + + # ============================================================ + - name: Search for internet gateway by VPC - no matches + ec2_vpc_igw_info: + filters: + attachment.vpc-id: '{{ vpc_result.vpc.id }}' + register: igw_info + + - name: Assert success + assert: + that: + - igw_info is successful + - '"internet_gateways" in igw_info' + # ============================================================ + - name: create internet gateway (expected changed=true) - CHECK_MODE + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + tags: + tag_one: '{{ resource_prefix }} One' + "Tag Two": 'two {{ resource_prefix }}' + register: vpc_igw_create + check_mode: yes + + - name: assert creation would happen (expected changed=true) - CHECK_MODE + assert: + that: + - vpc_igw_create is changed + - name: create internet gateway (expected changed=true) ec2_vpc_igw: state: present @@ -46,7 +79,7 @@ - name: assert creation happened (expected changed=true) assert: that: - - 'vpc_igw_create' + - vpc_igw_create is changed - 'vpc_igw_create.gateway_id.startswith("igw-")' - 'vpc_igw_create.vpc_id == vpc_result.vpc.id' - '"tags" in vpc_igw_create' @@ -150,6 +183,18 @@ current_igw: '{{ igw_info.internet_gateways[0] }}' # ============================================================ + - name: attempt to recreate internet gateway on VPC (expected changed=false) - CHECK_MODE + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + register: vpc_igw_recreate + check_mode: yes + + - name: assert recreation would do nothing (expected changed=false) - CHECK_MODE + assert: + that: + - vpc_igw_recreate is not changed + - name: attempt to recreate internet gateway on VPC (expected changed=false) ec2_vpc_igw: state: present @@ -168,6 +213,21 @@ - vpc_igw_create.tags["Tag Two"] == 'two {{ resource_prefix }}' # ============================================================ + - name: Update the tags (no change) - CHECK_MODE + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + tags: + tag_one: '{{ resource_prefix }} One' + "Tag Two": 'two {{ resource_prefix }}' + register: vpc_igw_recreate + check_mode: yes + + - name: assert tag update would do nothing (expected changed=false) - CHECK_MODE + assert: + that: + - vpc_igw_recreate is not changed + - name: Update the tags (no change) ec2_vpc_igw: state: present @@ -177,7 +237,7 @@ "Tag Two": 'two {{ resource_prefix }}' register: vpc_igw_recreate - - name: assert recreation did nothing (expected changed=false) + - name: assert tag update did nothing (expected changed=false) assert: that: - vpc_igw_recreate is not changed @@ -189,6 +249,21 @@ - vpc_igw_recreate.tags["Tag Two"] == 'two {{ resource_prefix }}' # ============================================================ + - name: Update the tags - remove and add - CHECK_MODE + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + tags: + tag_three: '{{ resource_prefix }} Three' + "Tag Two": 'two {{ resource_prefix }}' + register: vpc_igw_update + check_mode: yes + + - name: assert tag update would happen (expected changed=true) - CHECK_MODE + assert: + that: + - vpc_igw_update is changed + - name: Update the tags - remove and add ec2_vpc_igw: state: present @@ -198,7 +273,7 @@ "Tag Two": 'two {{ resource_prefix }}' register: vpc_igw_update - - name: assert recreation did nothing (expected changed=false) + - name: assert tags are updated (expected changed=true) assert: that: - vpc_igw_update is changed @@ -210,6 +285,21 @@ - vpc_igw_update.tags["Tag Two"] == 'two {{ resource_prefix }}' # ============================================================ + - name: Update the tags add without purge - CHECK_MODE + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + purge_tags: no + tags: + tag_one: '{{ resource_prefix }} One' + register: vpc_igw_update + check_mode: yes + + - name: assert tags would be added - CHECK_MODE + assert: + that: + - vpc_igw_update is changed + - name: Update the tags add without purge ec2_vpc_igw: state: present @@ -232,6 +322,19 @@ - vpc_igw_update.tags["Tag Two"] == 'two {{ resource_prefix }}' # ============================================================ + - name: Remove all tags - CHECK_MODE + ec2_vpc_igw: + state: present + vpc_id: "{{ vpc_result.vpc.id }}" + tags: {} + register: vpc_igw_update + check_mode: yes + + - name: assert tags would be removed - CHECK_MODE + assert: + that: + - vpc_igw_update is changed + - name: Remove all tags ec2_vpc_igw: state: present @@ -239,7 +342,7 @@ tags: {} register: vpc_igw_update - - name: assert tags added + - name: assert tags removed assert: that: - vpc_igw_update is changed @@ -249,6 +352,18 @@ - vpc_igw_update.tags | length == 0 # ============================================================ + - name: test state=absent (expected changed=true) - CHECK_MODE + ec2_vpc_igw: + state: absent + vpc_id: "{{ vpc_result.vpc.id }}" + register: vpc_igw_delete + check_mode: yes + + - name: assert state=absent (expected changed=true) - CHECK_MODE + assert: + that: + - vpc_igw_delete is changed + - name: test state=absent (expected changed=true) ec2_vpc_igw: state: absent @@ -258,7 +373,7 @@ - name: assert state=absent (expected changed=true) assert: that: - - 'vpc_igw_delete.changed' + - vpc_igw_delete is changed # ============================================================ - name: Fetch IGW by ID (list) @@ -271,9 +386,22 @@ - name: 'Check IGW does not exist' assert: that: + # Deliberate choice not to change bevahiour when searching by ID - igw_info is failed # ============================================================ + - name: test state=absent when already deleted (expected changed=false) - CHECK_MODE + ec2_vpc_igw: + state: absent + vpc_id: "{{ vpc_result.vpc.id }}" + register: vpc_igw_delete + check_mode: yes + + - name: assert state=absent (expected changed=false) - CHECK_MODE + assert: + that: + - vpc_igw_delete is not changed + - name: test state=absent when already deleted (expected changed=false) ec2_vpc_igw: state: absent From f8772807085fe1fab07917a93f20f837faef92e0 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 9 Dec 2020 19:46:04 +0100 Subject: [PATCH 13/13] changelog --- changelogs/fragments/318-cleanup-vpc_igw.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelogs/fragments/318-cleanup-vpc_igw.yml diff --git a/changelogs/fragments/318-cleanup-vpc_igw.yml b/changelogs/fragments/318-cleanup-vpc_igw.yml new file mode 100644 index 00000000000..58dc4f50ea0 --- /dev/null +++ b/changelogs/fragments/318-cleanup-vpc_igw.yml @@ -0,0 +1,7 @@ +minor_changes: +- ec2_vpc_igw - Add AWSRetry decorators to improve reliability (https://github.com/ansible-collections/community.aws/pull/318). +- ec2_vpc_igw_info - Add AWSRetry decorators to improve reliability (https://github.com/ansible-collections/community.aws/pull/318). +- ec2_vpc_igw - Add ``purge_tags`` parameter so that tags can be added without purging existing tags to match the collection standard tagging behaviour (https://github.com/ansible-collections/community.aws/pull/318). +- ec2_vpc_igw_info - Add ``convert_tags`` parameter so that tags can be returned in standard dict format rather than the both list of dict format (https://github.com/ansible-collections/community.aws/pull/318). +deprecated_features: +- ec2_vpc_igw_info - After 2022-06-22 the ``convert_tags`` parameter default value will change from ``False`` to ``True`` to match the collection standard behavior (https://github.com/ansible-collections/community.aws/pull/318).