Ensure the appropriate handlers are used for tags management

changelogs/fragments/531-use_tags_handlers.yml

@@ -0,0 +1,5 @@
+minor_changes:
+ - ec2_vpc_nat_gateway - use module_util helpers for tagging (https://github.com/ansible-collections/amazon.aws/pull/531).
+ - ec2_vpc_igw - use module_util helpers for tagging (https://github.com/ansible-collections/amazon.aws/pull/531).
+ - ec2_vpc_dhcp_option - use module_util helpers for tagging (https://github.com/ansible-collections/amazon.aws/pull/531).
+ - ec2_vpc_endpoint - use module_util helpers for tagging (https://github.com/ansible-collections/amazon.aws/pull/531).

plugins/modules/ec2_vpc_dhcp_option.py

@@ -250,46 +250,12 @@
 from ..module_utils.core import AnsibleAWSModule
 from ..module_utils.core import is_boto3_error_code
 from ..module_utils.ec2 import AWSRetry
-from ..module_utils.ec2 import ansible_dict_to_boto3_tag_list
-from ..module_utils.ec2 import boto3_tag_list_to_ansible_dict
 from ..module_utils.ec2 import camel_dict_to_snake_dict
-from ..module_utils.ec2 import compare_aws_tags
 from ..module_utils.ec2 import normalize_ec2_vpc_dhcp_config
-
-
-def ensure_tags(client, module, dhcp_options_id, tags, purge_tags):
-    changed = False
-    tags_to_unset = False
-    tags_to_set = False
-
-    if module.check_mode and dhcp_options_id is None:
-        # We can't describe tags without an option id, we might get here when creating a new option set in check_mode
-        return changed
-
-    current_tags = boto3_tag_list_to_ansible_dict(client.describe_tags(aws_retry=True, Filters=[{'Name': 'resource-id', 'Values': [dhcp_options_id]}])['Tags'])
-
-    if tags:
-        tags_to_set, tags_to_unset = compare_aws_tags(current_tags, tags, purge_tags=purge_tags)
-    if purge_tags and not tags:
-        tags_to_unset = current_tags
-
-    if tags_to_unset:
-        changed = True
-        if not module.check_mode:
-            try:
-                client.delete_tags(aws_retry=True, Resources=[dhcp_options_id], Tags=[dict(Key=tagkey) for tagkey in tags_to_unset])
-            except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
-                module.fail_json_aws(e, msg="Unable to delete tags {0}".format(tags_to_unset))
-
-    if tags_to_set:
-        changed = True
-        if not module.check_mode:
-            try:
-                client.create_tags(aws_retry=True, Resources=[dhcp_options_id], Tags=ansible_dict_to_boto3_tag_list(tags_to_set))
-            except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
-                module.fail_json_aws(e, msg="Unable to add tags {0}".format(tags_to_set))
-
-    return changed
+from ..module_utils.ec2 import ensure_ec2_tags
+from ..module_utils.tagging import boto3_tag_specifications
+from ..module_utils.tagging import ansible_dict_to_boto3_tag_list
+from ..module_utils.tagging import boto3_tag_list_to_ansible_dict
 
 
 def fetch_dhcp_options_for_vpc(client, module, vpc_id):
@@ -401,16 +367,21 @@ def create_dhcp_option_set(client, module, new_config):
     changed = True
     desired_config = normalize_ec2_vpc_dhcp_config(new_config)
     create_config = []
+    tags_list = []
+
     for option in ['domain-name', 'domain-name-servers', 'ntp-servers', 'netbios-name-servers']:
         if desired_config.get(option):
             create_config.append({'Key': option, 'Values': desired_config[option]})
     if desired_config.get('netbios-node-type'):
         # We need to listify this one
         create_config.append({'Key': 'netbios-node-type', 'Values': [desired_config['netbios-node-type']]})
 
+    if module.params.get('tags'):
+        tags_list = boto3_tag_specifications(module.params['tags'], ['dhcp-options'])
+
     try:
         if not module.check_mode:
-            dhcp_options = client.create_dhcp_options(aws_retry=True, DhcpConfigurations=create_config)
+            dhcp_options = client.create_dhcp_options(aws_retry=True, DhcpConfigurations=create_config, TagSpecifications=tags_list)
             return changed, dhcp_options['DhcpOptions']['DhcpOptionsId']
     except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
         module.fail_json_aws(e, msg="Unable to create dhcp option set")
@@ -530,8 +501,8 @@ def main():
                 if new_config == existing_config:
                     dhcp_options_id = existing_id
                     if tags or purge_tags:
-                        tags_changed = ensure_tags(client, module, dhcp_options_id, tags, purge_tags)
-                        changed = changed or tags_changed
+                        changed |= ensure_ec2_tags(client, module, dhcp_options_id, resource_type='dhcp-options',
+                                                   tags=tags, purge_tags=purge_tags)
                     return_config = normalize_ec2_vpc_dhcp_config(new_config)
                     results = get_dhcp_options_info(client, module, dhcp_options_id)
                     module.exit_json(changed=changed, new_options=return_config, dhcp_options_id=dhcp_options_id, dhcp_options=results)
@@ -553,11 +524,10 @@ def main():
     if not found:
         # If we still don't have an options ID, create it
         changed, dhcp_options_id = create_dhcp_option_set(client, module, new_config)
-
-    if tags or purge_tags:
-        # q('tags? ', module.params['dbg'])
-        tags_changed = ensure_tags(client, module, dhcp_options_id, tags, purge_tags)
-        changed = (changed or tags_changed)
+    else:
+        if tags or purge_tags:
+            changed |= ensure_ec2_tags(client, module, dhcp_options_id, resource_type='dhcp-options',
+                                       tags=tags, purge_tags=purge_tags)
 
     # If we were given a vpc_id, then attach the options we now have to that before we finish
     if vpc_id:

plugins/modules/ec2_vpc_endpoint.py

@@ -215,13 +215,8 @@
 from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry
 from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code
 from ansible_collections.amazon.aws.plugins.module_utils.waiters import get_waiter
-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 ensure_ec2_tags
-
-
-def _generate_tag_specifications(tags):
-    tag_list = ansible_dict_to_boto3_tag_list(tags)
-    return [dict(ResourceType="vpc-endpoint", Tags=tag_list)]
+from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications
 
 
 def get_endpoints(client, module, endpoint_id=None):
@@ -279,6 +274,7 @@ def setup_creation(client, module):
         # If we have an endpoint now, just ensure tags and exit
         if module.params.get('tags'):
             changed |= ensure_ec2_tags(client, module, endpoint_id,
+                                       resource_type='vpc-endpoint',
                                        tags=module.params.get('tags'),
                                        purge_tags=module.params.get('purge_tags'))
         normalized_result = get_endpoints(client, module, endpoint_id=endpoint_id)['VpcEndpoints'][0]
@@ -328,8 +324,9 @@ def create_vpc_endpoint(client, module):
 
     if policy:
         params['PolicyDocument'] = json.dumps(policy)
+
     if module.params.get('tags'):
-        params["TagSpecifications"] = _generate_tag_specifications(module.params.get('tags'))
+        params["TagSpecifications"] = boto3_tag_specifications(module.params.get('tags'), ['vpc-endpoint'])
 
     try:
         changed = True

plugins/modules/ec2_vpc_igw.py

@@ -106,10 +106,10 @@
 from ..module_utils.core import AnsibleAWSModule
 from ..module_utils.waiters import get_waiter
 from ..module_utils.ec2 import AWSRetry
-from ..module_utils.ec2 import ansible_dict_to_boto3_filter_list
-from ..module_utils.ec2 import boto3_tag_list_to_ansible_dict
 from ..module_utils.ec2 import camel_dict_to_snake_dict
 from ..module_utils.ec2 import ensure_ec2_tags
+from ..module_utils.ec2 import ansible_dict_to_boto3_filter_list
+from ..module_utils.tagging import boto3_tag_list_to_ansible_dict
 
 
 class AnsibleEc2Igw():

plugins/modules/ec2_vpc_nat_gateway.py

@@ -243,15 +243,14 @@
 except ImportError:
     pass  # Handled by AnsibleAWSModule
 
-from ..module_utils.ec2 import AWSRetry
 from ..module_utils.core import AnsibleAWSModule
 from ..module_utils.core import is_boto3_error_code
 from ..module_utils.waiters import get_waiter
+from ..module_utils.ec2 import AWSRetry
 from ..module_utils.ec2 import camel_dict_to_snake_dict
-from ..module_utils.ec2 import boto3_tag_list_to_ansible_dict
-from ..module_utils.ec2 import ansible_dict_to_boto3_tag_list
 from ..module_utils.ec2 import describe_ec2_tags
 from ..module_utils.ec2 import ensure_ec2_tags
+from ..module_utils.tagging import boto3_tag_specifications
 
 
 @AWSRetry.jittered_backoff(retries=10)
@@ -263,11 +262,6 @@ def _describe_nat_gateways(client, **params):
         return None
 
 
-def _generate_tag_specifications(tags):
-    tag_list = ansible_dict_to_boto3_tag_list(tags)
-    return [dict(ResourceType="natgateway", Tags=tag_list)]
-
-
 def wait_for_status(client, module, waiter_name, nat_gateway_id):
     wait_timeout = module.params.get('wait_timeout')
     try:
@@ -623,7 +617,7 @@ def create(client, module, subnet_id, allocation_id, tags, client_token=None,
         params['ClientToken'] = client_token
 
     if tags:
-        params["TagSpecifications"] = _generate_tag_specifications(tags)
+        params["TagSpecifications"] = boto3_tag_specifications(tags, ['natgateway'])
 
     if module.check_mode:
         changed = True

tests/integration/targets/ec2_vpc_nat_gateway/tasks/main.yml

@@ -468,7 +468,7 @@
       - '"nat_gateway_id" in delete_nat_gateway'
       - delete_nat_gateway.nat_gateway_id == nat_gateway_id
       - '"state" in delete_nat_gateway'
-      - delete_nat_gateway.state == 'deleted'
+      - delete_nat_gateway.state in ['deleted', 'deleting']
       - '"subnet_id" in delete_nat_gateway'
       - delete_nat_gateway.subnet_id == subnet_id
       - '"tags" in delete_nat_gateway'
@@ -539,7 +539,7 @@
     register: update_tags_ngw
     check_mode: yes
 
-  - name: assert tag update would do nothing (expected changed=false) - CHECK_MODE
+  - name: Assert tag update would do nothing (expected changed=false) - CHECK_MODE
     assert:
       that:
       - not update_tags_ngw.changed
@@ -564,7 +564,7 @@
       wait: yes
     register: update_tags_ngw
 
-  - name: assert tag update would do nothing (expected changed=false)
+  - name: Assert tag update would do nothing (expected changed=false)
     assert:
       that:
       - not update_tags_ngw.changed

tests/unit/plugins/modules/test_ec2_vpc_dhcp_option.py

@@ -63,17 +63,6 @@ def exit_json(self, *args, **kwargs):
 @patch.object(dhcp_module.AnsibleAWSModule, 'client')
 class TestDhcpModule(ModuleTestCase):
 
-    def test_create_dhcp_config(self, client_mock):
-        self.params = test_module_params
-        result = dhcp_module.create_dhcp_config(self)
-
-        assert result == test_create_config
-
-    def test_create_dhcp_option_set(self, client_mock):
-        self.check_mode = False
-        dhcp_module.create_dhcp_option_set(client_mock, self, test_create_config)
-        client_mock.create_dhcp_options.assert_called_once_with(DhcpConfigurations=test_create_option_set, aws_retry=True)
-
     def test_normalize_config(self, client_mock):
         result = dhcp_module.normalize_ec2_vpc_dhcp_config(test_create_config)