Cleanup IGW modules (ansible-collections#318)

* import order

* Add retry decorators

* Switch tests to using module_defaults

* module_defaults

* Add initial _info tests

* Handle Boto Errors with fail_json_aws

* Test state=absent when IGW missing

* Support not purging tags

* Support converting Tags from boto to dict

* Add tagging tests

* Use random CIDR for VPC

* Add check_mode tests

* changelog

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).

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
@@ -85,42 +92,42 @@
 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):
 
     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):
         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)
 
     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)
@@ -135,21 +142,25 @@ 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'})
         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")
 
-        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'))
 
@@ -159,7 +170,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)
                     )
@@ -179,15 +191,15 @@ 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:
                 self._module.fail_json_aws(e, msg="Couldn't delete tags")
 
         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")
@@ -213,14 +225,14 @@ 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")
 
         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)
@@ -232,21 +244,21 @@ 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')
 
         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(

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
@@ -94,47 +100,68 @@
     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 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 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
 
 
-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'))
+    convert_tags = module.params.get('convert_tags')
 
     if module.params.get("internet_gateway_ids"):
         params['InternetGatewayIds'] = module.params.get("internet_gateway_ids")
 
     try:
-        all_internet_gateways = client.describe_internet_gateways(**params)
-    except botocore.exceptions.ClientError as e:
-        module.fail_json(msg=str(e))
+        all_internet_gateways = connection.describe_internet_gateways(aws_retry=True, **params)
+    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))
+    return [get_internet_gateway_info(igw, convert_tags)
             for igw in all_internet_gateways['InternetGateways']]
 
 
 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')
+        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')
 

tests/integration/targets/ec2_vpc_igw/aliases

@@ -1,2 +1,3 @@
 cloud/aws
 shippable/aws/group2
+ec2_vpc_igw_info

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'