From fc22a80c1d14a270206a5580bfac69b481fb6590 Mon Sep 17 00:00:00 2001
From: Alina Buzachis <49211501+alinabuzachis@users.noreply.github.com>
Date: Tue, 16 Mar 2021 06:45:22 +0100
Subject: [PATCH] ec2_vpc_igw: increase integration tests coverage and code
 cleaning (#386)

* Internet gateway: add missing integration tests and code check
 * Add CamelCase tags integration test
 * Removed check_input_tags() because useless as ansible_dict_to_boto3_tag_list() already performs a to_native() conversion to strings
 * Add additional integration tests for ec2_vpc_igw_info module

Signed-off-by: Alina Buzachis <abuzachis@redhat.com>

* Internet Gateway - integration tests
 * Fix Internet Gatways search by tags

* * Apply reviewer suggestions
        * Better error handling for the waiter

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: https://github.com/ansible-collections/community.aws/commit/cff6d0015d09307c5b6d430e6254db02e7df8f64
---
 plugins/modules/ec2_vpc_igw.py                |  15 +-
 .../targets/ec2_vpc_igw/tasks/main.yml        | 144 +++++++++++++++++-
 2 files changed, 145 insertions(+), 14 deletions(-)

diff --git a/plugins/modules/ec2_vpc_igw.py b/plugins/modules/ec2_vpc_igw.py
index 3d8d9f3bf25..bef92a71fcf 100644
--- a/plugins/modules/ec2_vpc_igw.py
+++ b/plugins/modules/ec2_vpc_igw.py
@@ -92,8 +92,6 @@
 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
@@ -141,13 +139,6 @@ 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, purge_tags):
         final_tags = []
 
@@ -167,7 +158,6 @@ def ensure_tags(self, igw_id, tags, purge_tags):
         if to_update:
             try:
                 if self._check_mode:
-                    # update tags
                     final_tags.update(to_update)
                 else:
                     self._connection.create_tags(
@@ -183,7 +173,6 @@ def ensure_tags(self, igw_id, tags, purge_tags):
         if to_delete:
             try:
                 if self._check_mode:
-                    # update tags
                     for key in to_delete:
                         del final_tags[key]
                 else:
@@ -233,8 +222,6 @@ def ensure_igw_absent(self, vpc_id):
         return self._results
 
     def ensure_igw_present(self, vpc_id, tags, purge_tags):
-        self.check_input_tags(tags)
-
         igw = self.get_matching_igw(vpc_id)
 
         if igw is None:
@@ -253,6 +240,8 @@ def ensure_igw_present(self, vpc_id, tags, purge_tags):
                 igw = camel_dict_to_snake_dict(response['InternetGateway'])
                 self._connection.attach_internet_gateway(aws_retry=True, InternetGatewayId=igw['internet_gateway_id'], VpcId=vpc_id)
                 self._results['changed'] = True
+            except botocore.exceptions.WaiterError as e:
+                self._module.fail_json_aws(e, msg="No Internet Gateway exists.")
             except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
                 self._module.fail_json_aws(e, msg='Unable to create Internet Gateway')
 
diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml
index 634438c0875..6697493127d 100644
--- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml
+++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml
@@ -37,6 +37,13 @@
     assert:
       that:
           - vpc_result is successful
+          - '"vpc" in vpc_result'
+          - '"id" in vpc_result.vpc'
+          - vpc_result.vpc.state == 'available'
+          - '"tags" in vpc_result.vpc'
+          - vpc_result.vpc.tags | length == 2
+          - vpc_result.vpc.tags["Name"] == "{{ resource_prefix }}-vpc"
+          - vpc_result.vpc.tags["Description"] == "Created by ansible-test"
 
   # ============================================================
   - name: Search for internet gateway by VPC - no matches
@@ -50,6 +57,7 @@
       that:
           - igw_info is successful
           - '"internet_gateways" in igw_info'
+          - (igw_info.internet_gateways | length) == 0
 
   # ============================================================
   - name: create internet gateway (expected changed=true) - CHECK_MODE
@@ -194,6 +202,12 @@
     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_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: attempt to recreate internet gateway on VPC (expected changed=false)
     ec2_vpc_igw:
@@ -227,6 +241,12 @@
     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 (no change)
     ec2_vpc_igw:
@@ -263,6 +283,13 @@
     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 - remove and add
     ec2_vpc_igw:
@@ -299,6 +326,13 @@
     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: Update the tags add without purge
     ec2_vpc_igw:
@@ -321,6 +355,114 @@
           - vpc_igw_update.tags["tag_three"] == '{{ resource_prefix }} Three'
           - vpc_igw_update.tags["Tag Two"] == 'two {{ resource_prefix }}'
 
+
+  # ============================================================
+  - name: Update with CamelCase tags - CHECK_MODE
+    ec2_vpc_igw:
+      state: present
+      vpc_id: "{{ vpc_result.vpc.id }}"
+      tags:
+        "lowercase spaced": 'hello cruel world ❤️'
+        "Title Case": 'Hello Cruel World ❤️'
+        CamelCase: 'SimpleCamelCase ❤️'
+        snake_case: 'simple_snake_case ❤️'
+    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
+          - vpc_igw_update.gateway_id == igw_id
+          - vpc_igw_update.vpc_id == vpc_id
+          - '"tags" in vpc_igw_update'
+          - vpc_igw_update.tags | length == 4
+          - vpc_igw_update.tags["lowercase spaced"] == 'hello cruel world ❤️'
+          - vpc_igw_update.tags["Title Case"] == 'Hello Cruel World ❤️'
+          - vpc_igw_update.tags["CamelCase"] == 'SimpleCamelCase ❤️'
+          - vpc_igw_update.tags["snake_case"] == 'simple_snake_case ❤️'
+  
+
+  - name: Update the tags - remove and add
+    ec2_vpc_igw:
+      state: present
+      vpc_id: "{{ vpc_result.vpc.id }}"
+      tags:
+        "lowercase spaced": 'hello cruel world ❤️'
+        "Title Case": 'Hello Cruel World ❤️'
+        CamelCase: 'SimpleCamelCase ❤️'
+        snake_case: 'simple_snake_case ❤️'
+    register: vpc_igw_update
+
+  - name: assert tags are updated (expected changed=true)
+    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 == 4
+          - vpc_igw_update.tags["lowercase spaced"] == 'hello cruel world ❤️'
+          - vpc_igw_update.tags["Title Case"] == 'Hello Cruel World ❤️'
+          - vpc_igw_update.tags["CamelCase"] == 'SimpleCamelCase ❤️'
+          - vpc_igw_update.tags["snake_case"] == 'simple_snake_case ❤️'
+  
+  # ============================================================
+  - name: Gather information about a filtered list of Internet Gateways using tags
+    ec2_vpc_igw_info:
+      filters:
+        "tag:Title Case": 'Hello Cruel World ❤️'
+    register: igw_info
+
+  - name: Assert success
+    assert:
+      that:
+          - igw_info is successful
+          - '"internet_gateways" in igw_info'
+          - igw_info.internet_gateways | selectattr("internet_gateway_id",'equalto',"{{ igw_id }}")
+  
+  - name: Gather information about a filtered list of Internet Gateways using tags - CHECK_MODE
+    ec2_vpc_igw_info:
+      filters:
+        "tag:Title Case": 'Hello Cruel World ❤️'
+    register: igw_info
+    check_mode: yes
+
+  - name: Assert success - CHECK_MODE
+    assert:
+      that:
+          - igw_info is successful
+          - '"internet_gateways" in igw_info'
+          - igw_info.internet_gateways | selectattr("internet_gateway_id",'equalto',"{{ igw_id }}")
+
+  # ============================================================
+  - name: Gather information about a filtered list of Internet Gateways using tags (no match)
+    ec2_vpc_igw_info:
+      filters:
+        "tag:tag_one": '{{ resource_prefix }} One'
+    register: igw_info
+
+  - name: Assert success
+    assert:
+      that:
+          - igw_info is successful
+          - '"internet_gateways" in igw_info'
+          - igw_info.internet_gateways | length == 0
+
+  - name: Gather information about a filtered list of Internet Gateways using tags (no match) - CHECK_MODE
+    ec2_vpc_igw_info:
+      filters:
+        "tag:tag_one": '{{ resource_prefix }} One'
+    register: igw_info
+    check_mode: yes
+
+  - name: Assert success - CHECK_MODE
+    assert:
+      that:
+          - igw_info is successful
+          - '"internet_gateways" in igw_info'
+          - igw_info.internet_gateways | length == 0
+
   # ============================================================
   - name: Remove all tags - CHECK_MODE
     ec2_vpc_igw:
@@ -350,7 +492,7 @@
           - 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) - CHECK_MODE
     ec2_vpc_igw: