diff --git a/changelogs/fragments/1843-ec2_eip.yml b/changelogs/fragments/1843-ec2_eip.yml new file mode 100644 index 00000000000..10de3bec3a4 --- /dev/null +++ b/changelogs/fragments/1843-ec2_eip.yml @@ -0,0 +1,2 @@ +minor_changes: +- ec2_eip - use ``ResourceTags`` to set initial tags upon creation (https://github.com/ansible-collections/amazon.aws/issues/1843) diff --git a/changelogs/fragments/1843-ec2_vpc_igw.yml b/changelogs/fragments/1843-ec2_vpc_igw.yml new file mode 100644 index 00000000000..247effda99e --- /dev/null +++ b/changelogs/fragments/1843-ec2_vpc_igw.yml @@ -0,0 +1,2 @@ +minor_changes: +- ec2_vpc_igw - use ``ResourceTags`` to set initial tags upon creation (https://github.com/ansible-collections/amazon.aws/issues/1843) diff --git a/changelogs/fragments/1843-ec2_vpc_route_table.yml b/changelogs/fragments/1843-ec2_vpc_route_table.yml new file mode 100644 index 00000000000..431ba4ea55f --- /dev/null +++ b/changelogs/fragments/1843-ec2_vpc_route_table.yml @@ -0,0 +1,2 @@ +minor_changes: +- ec2_vpc_route_table - use ``ResourceTags`` to set initial tags upon creation (https://github.com/ansible-collections/amazon.aws/issues/1843) diff --git a/changelogs/fragments/1843-ec2_vpc_subnet.yml b/changelogs/fragments/1843-ec2_vpc_subnet.yml new file mode 100644 index 00000000000..2fce4878c19 --- /dev/null +++ b/changelogs/fragments/1843-ec2_vpc_subnet.yml @@ -0,0 +1,4 @@ +minor_changes: +- ec2_vpc_subnet - use ``ResourceTags`` to set initial tags upon creation (https://github.com/ansible-collections/amazon.aws/issues/1843) +- ec2_vpc_subnet - the default value for ``tags`` has been changed from ``{}`` to ``None``, to remove tags from a subnet an empty map must + be explicitly passed to the module (https://github.com/ansible-collections/amazon.aws/pull/1876). diff --git a/plugins/module_utils/tagging.py b/plugins/module_utils/tagging.py index 2bcf0692c64..9201c8979c0 100644 --- a/plugins/module_utils/tagging.py +++ b/plugins/module_utils/tagging.py @@ -71,6 +71,12 @@ def boto3_tag_list_to_ansible_dict(tags_list, tag_name_key_name=None, tag_value_ def ansible_dict_to_boto3_tag_list(tags_dict, tag_name_key_name="Key", tag_value_key_name="Value"): """Convert a flat dict of key:value pairs representing AWS resource tags to a boto3 list of dicts + + Note: booleans are converted to their Capitalized text form ("True" and "False"), this is + different to ansible_dict_to_boto3_filter_list because historically we've used "to_text()" and + AWS stores tags as strings, whereas for things which are actually booleans in AWS are returned + as lowercase strings in filters. + Args: tags_dict (dict): Dict representing AWS resource tags. tag_name_key_name (str): Value to use as the key for all tag keys (useful because boto3 doesn't always use "Key") @@ -101,6 +107,34 @@ def ansible_dict_to_boto3_tag_list(tags_dict, tag_name_key_name="Key", tag_value return tags_list +def _tag_name_to_filter_key(tag_name): + return f"tag:{tag_name}" + + +def ansible_dict_to_tag_filter_dict(tags_dict): + """Prepends "tag:" to all of the keys (not the values) in a dict + This is useful when you're then going to build a filter including the tags. + + Note: booleans are converted to their Capitalized text form ("True" and "False"), this is + different to ansible_dict_to_boto3_filter_list because historically we've used "to_text()" and + AWS stores tags as strings, whereas for things which are actually booleans in AWS are returned + as lowercase strings in filters. + + Args: + tags_dict (dict): Dict representing AWS resource tags. + + Basic Usage: + >>> filters = ansible_dict_to_boto3_filter_list(ansible_dict_to_tag_filter_dict(tags)) + + Returns: + Dict: A dictionary suitable for passing to ansible_dict_to_boto3_filter_list which can + also be combined with other common filter parameters. + """ + if not tags_dict: + return {} + return {_tag_name_to_filter_key(k): to_native(v) for k, v in tags_dict.items()} + + def boto3_tag_specifications(tags_dict, types=None): """Converts a list of resource types and a flat dictionary of key:value pairs representing AWS resource tags to a TagSpecification object. diff --git a/plugins/modules/ec2_eip.py b/plugins/modules/ec2_eip.py index 87f67bdd546..38bf32c87a3 100644 --- a/plugins/modules/ec2_eip.py +++ b/plugins/modules/ec2_eip.py @@ -225,6 +225,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ensure_ec2_tags from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications from ansible_collections.amazon.aws.plugins.module_utils.transformation import ansible_dict_to_boto3_filter_list @@ -342,7 +343,16 @@ def address_is_associated_with_device(ec2, module, address, device_id, is_instan return False -def allocate_address(ec2, module, domain, reuse_existing_ip_allowed, check_mode, tag_dict=None, public_ipv4_pool=None): +def allocate_address( + ec2, + module, + domain, + reuse_existing_ip_allowed, + check_mode, + tags, + search_tags=None, + public_ipv4_pool=None, +): """Allocate a new elastic IP address (when needed) and return it""" if not domain: domain = "standard" @@ -351,8 +361,8 @@ def allocate_address(ec2, module, domain, reuse_existing_ip_allowed, check_mode, filters = [] filters.append({"Name": "domain", "Values": [domain]}) - if tag_dict is not None: - filters += ansible_dict_to_boto3_filter_list(tag_dict) + if search_tags is not None: + filters += ansible_dict_to_boto3_filter_list(search_tags) try: all_addresses = ec2.describe_addresses(Filters=filters, aws_retry=True) @@ -369,12 +379,26 @@ def allocate_address(ec2, module, domain, reuse_existing_ip_allowed, check_mode, return unassociated_addresses[0], False if public_ipv4_pool: - return allocate_address_from_pool(ec2, module, domain, check_mode, public_ipv4_pool), True + return ( + allocate_address_from_pool( + ec2, + module, + domain, + check_mode, + public_ipv4_pool, + tags, + ), + True, + ) + + params = {"Domain": domain} + if tags: + params["TagSpecifications"] = boto3_tag_specifications(tags, types="elastic-ip") try: if check_mode: return None, True - result = ec2.allocate_address(Domain=domain, aws_retry=True), True + result = ec2.allocate_address(aws_retry=True, **params), True except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: module.fail_json_aws(e, msg="Couldn't allocate Elastic IP address") return result @@ -434,6 +458,7 @@ def ensure_present( reuse_existing_ip_allowed, allow_reassociation, check_mode, + tags, is_instance=True, ): changed = False @@ -443,7 +468,14 @@ def ensure_present( if check_mode: return {"changed": True} - address, changed = allocate_address(ec2, module, domain, reuse_existing_ip_allowed, check_mode) + address, changed = allocate_address( + ec2, + module, + domain, + reuse_existing_ip_allowed, + check_mode, + tags, + ) if device_id: # Allocate an IP for instance since no public_ip was provided @@ -485,7 +517,14 @@ def ensure_absent(ec2, module, address, device_id, check_mode, is_instance=True) return release_address(ec2, module, address, check_mode) -def allocate_address_from_pool(ec2, module, domain, check_mode, public_ipv4_pool): +def allocate_address_from_pool( + ec2, + module, + domain, + check_mode, + public_ipv4_pool, + tags, +): # type: (EC2Connection, AnsibleAWSModule, str, bool, str) -> Address """Overrides botocore's allocate_address function to support BYOIP""" if check_mode: @@ -499,6 +538,9 @@ def allocate_address_from_pool(ec2, module, domain, check_mode, public_ipv4_pool if public_ipv4_pool is not None: params["PublicIpv4Pool"] = public_ipv4_pool + if tags: + params["TagSpecifications"] = boto3_tag_specifications(tags, types="elastic-ip") + try: result = ec2.allocate_address(aws_retry=True, **params) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: @@ -583,7 +625,7 @@ def main(): module.fail_json(msg=str(e)) # Tags for *searching* for an EIP. - tag_dict = generate_tag_dict(module, tag_name, tag_value) + search_tags = generate_tag_dict(module, tag_name, tag_value) try: if device_id: @@ -603,6 +645,7 @@ def main(): reuse_existing_ip_allowed, allow_reassociation, module.check_mode, + tags, is_instance=is_instance, ) if "allocation_id" not in result: @@ -617,7 +660,14 @@ def main(): } else: address, changed = allocate_address( - ec2, module, domain, reuse_existing_ip_allowed, module.check_mode, tag_dict, public_ipv4_pool + ec2, + module, + domain, + reuse_existing_ip_allowed, + module.check_mode, + tags, + search_tags, + public_ipv4_pool, ) if address: result = { diff --git a/plugins/modules/ec2_vpc_igw.py b/plugins/modules/ec2_vpc_igw.py index 5e14716ea08..428de396e1f 100644 --- a/plugins/modules/ec2_vpc_igw.py +++ b/plugins/modules/ec2_vpc_igw.py @@ -148,6 +148,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications from ansible_collections.amazon.aws.plugins.module_utils.transformation import ansible_dict_to_boto3_filter_list from ansible_collections.amazon.aws.plugins.module_utils.waiters import get_waiter @@ -311,7 +312,10 @@ def ensure_igw_present(self, igw_id, vpc_id, tags, purge_tags, force_attach, det self.get_matching_vpc(vpc_id) try: - response = self._connection.create_internet_gateway(aws_retry=True) + create_params = {} + if tags: + create_params["TagSpecifications"] = boto3_tag_specifications(tags, types="internet-gateway") + response = self._connection.create_internet_gateway(aws_retry=True, **create_params) # Ensure the gateway exists before trying to attach it or add tags waiter = get_waiter(self._connection, "internet_gateway_exists") diff --git a/plugins/modules/ec2_vpc_route_table.py b/plugins/modules/ec2_vpc_route_table.py index c380c01bf14..34f12e789ca 100644 --- a/plugins/modules/ec2_vpc_route_table.py +++ b/plugins/modules/ec2_vpc_route_table.py @@ -303,6 +303,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ensure_ec2_tags from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications from ansible_collections.amazon.aws.plugins.module_utils.transformation import ansible_dict_to_boto3_filter_list from ansible_collections.amazon.aws.plugins.module_utils.waiters import get_waiter @@ -778,7 +779,10 @@ def ensure_route_table_present(connection, module): changed = True if not module.check_mode: try: - route_table = connection.create_route_table(aws_retry=True, VpcId=vpc_id)["RouteTable"] + create_params = {"VpcId": vpc_id} + if tags: + create_params["TagSpecifications"] = boto3_tag_specifications(tags, types="route-table") + route_table = connection.create_route_table(aws_retry=True, **create_params)["RouteTable"] # try to wait for route table to be present before moving on get_waiter(connection, "route_table_exists").wait( RouteTableIds=[route_table["RouteTableId"]], diff --git a/plugins/modules/ec2_vpc_subnet.py b/plugins/modules/ec2_vpc_subnet.py index 43471df001a..29c7c75f226 100644 --- a/plugins/modules/ec2_vpc_subnet.py +++ b/plugins/modules/ec2_vpc_subnet.py @@ -72,8 +72,6 @@ - Ignored unless I(wait=True). default: 300 type: int - tags: - default: {} extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules @@ -212,14 +210,15 @@ except ImportError: pass # caught by AnsibleAWSModule -from ansible.module_utils._text import to_text from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.arn import is_outpost_arn from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ensure_ec2_tags from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry +from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_tag_filter_dict from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications from ansible_collections.amazon.aws.plugins.module_utils.transformation import ansible_dict_to_boto3_filter_list from ansible_collections.amazon.aws.plugins.module_utils.waiters import get_waiter @@ -268,7 +267,7 @@ def handle_waiter(conn, module, waiter_name, params, start_time): module.fail_json_aws(e, "An exception happened while trying to wait for updates") -def create_subnet(conn, module, vpc_id, cidr, ipv6_cidr=None, outpost_arn=None, az=None, start_time=None): +def create_subnet(conn, module, vpc_id, cidr, tags, ipv6_cidr=None, outpost_arn=None, az=None, start_time=None): wait = module.params["wait"] params = dict(VpcId=vpc_id, CidrBlock=cidr) @@ -279,6 +278,9 @@ def create_subnet(conn, module, vpc_id, cidr, ipv6_cidr=None, outpost_arn=None, if az: params["AvailabilityZone"] = az + if tags: + params["TagSpecifications"] = boto3_tag_specifications(tags, types="subnet") + if outpost_arn: if is_outpost_arn(outpost_arn): params["OutpostArn"] = outpost_arn @@ -312,9 +314,12 @@ def ensure_tags(conn, module, subnet, tags, purge_tags, start_time): retry_codes=["InvalidSubnetID.NotFound"], ) + if not changed: + return changed + if module.params["wait"] and not module.check_mode: # Wait for tags to be updated - filters = [{"Name": f"tag:{k}", "Values": [v]} for k, v in tags.items()] + filters = ansible_dict_to_boto3_filter_list(ansible_dict_to_tag_filter_dict(tags)) handle_waiter(conn, module, "subnet_exists", {"SubnetIds": [subnet["id"]], "Filters": filters}, start_time) return changed @@ -452,6 +457,7 @@ def ensure_subnet_present(conn, module): module, module.params["vpc_id"], module.params["cidr"], + module.params["tags"], ipv6_cidr=module.params["ipv6_cidr"], outpost_arn=module.params["outpost_arn"], az=module.params["az"], @@ -478,10 +484,8 @@ def ensure_subnet_present(conn, module): ) changed = True - if module.params["tags"] != subnet["tags"]: - stringified_tags_dict = dict((to_text(k), to_text(v)) for k, v in module.params["tags"].items()) - if ensure_tags(conn, module, subnet, stringified_tags_dict, module.params["purge_tags"], start_time): - changed = True + if ensure_tags(conn, module, subnet, module.params["tags"], module.params["purge_tags"], start_time): + changed = True subnet = get_matching_subnet(conn, module, module.params["vpc_id"], module.params["cidr"]) if not module.check_mode and module.params["wait"]: @@ -549,7 +553,7 @@ def main(): ipv6_cidr=dict(default="", required=False), outpost_arn=dict(default="", type="str", required=False), state=dict(default="present", choices=["present", "absent"]), - tags=dict(default={}, required=False, type="dict", aliases=["resource_tags"]), + tags=dict(required=False, type="dict", aliases=["resource_tags"]), vpc_id=dict(required=True), map_public=dict(default=False, required=False, type="bool"), assign_instances_ipv6=dict(default=False, required=False, type="bool"), diff --git a/tests/integration/targets/ec2_eip/tasks/main.yml b/tests/integration/targets/ec2_eip/tasks/main.yml index 72ef028e664..a8c3b53718d 100644 --- a/tests/integration/targets/ec2_eip/tasks/main.yml +++ b/tests/integration/targets/ec2_eip/tasks/main.yml @@ -143,6 +143,8 @@ - assert: that: - eip is changed + - "'ec2:CreateTags' not in eip.resource_actions" + - "'ec2:DeleteTags' not in eip.resource_actions" - eip.public_ip is defined and ( eip.public_ip | ansible.utils.ipaddr ) - eip.allocation_id is defined and eip.allocation_id.startswith("eipalloc-") - ( eip_info_start.addresses | length ) + 1 == ( eip_info.addresses | length diff --git a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml index ed70119f88f..326a19d4a98 100644 --- a/tests/integration/targets/ec2_vpc_igw/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_igw/tasks/main.yml @@ -48,7 +48,7 @@ tags: Description: Created by ansible-test register: vpc_2_result - + # ============================================================ - name: Search for internet gateway by VPC - no matches ec2_vpc_igw_info: @@ -91,6 +91,8 @@ - name: Assert creation happened (expected changed=true) assert: that: + - "'ec2:CreateTags' not in vpc_igw_create.resource_actions" + - "'ec2:DeleteTags' not in vpc_igw_create.resource_actions" - vpc_igw_create is changed - vpc_igw_create.gateway_id.startswith("igw-") - vpc_igw_create.vpc_id == vpc_result.vpc.id diff --git a/tests/integration/targets/ec2_vpc_route_table/tasks/main.yml b/tests/integration/targets/ec2_vpc_route_table/tasks/main.yml index 886f53dfd30..0592310a41c 100644 --- a/tests/integration/targets/ec2_vpc_route_table/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_route_table/tasks/main.yml @@ -151,6 +151,8 @@ assert: that: - create_public_table.changed + - "'ec2:CreateTags' not in create_public_table.resource_actions" + - "'ec2:DeleteTags' not in create_public_table.resource_actions" - create_public_table.route_table.id.startswith('rtb-') - "'Public' in create_public_table.route_table.tags" - create_public_table.route_table.tags['Public'] == 'true' diff --git a/tests/integration/targets/ec2_vpc_subnet/tasks/main.yml b/tests/integration/targets/ec2_vpc_subnet/tasks/main.yml index 1837eec9c71..08327626734 100644 --- a/tests/integration/targets/ec2_vpc_subnet/tasks/main.yml +++ b/tests/integration/targets/ec2_vpc_subnet/tasks/main.yml @@ -68,6 +68,8 @@ assert: that: - 'vpc_subnet_create' + - "'ec2:CreateTags' not in vpc_subnet_create.resource_actions" + - "'ec2:DeleteTags' not in vpc_subnet_create.resource_actions" - 'vpc_subnet_create.subnet.id.startswith("subnet-")' - '"Name" in vpc_subnet_create.subnet.tags and vpc_subnet_create.subnet.tags["Name"] == ec2_vpc_subnet_name' - '"Description" in vpc_subnet_create.subnet.tags and vpc_subnet_create.subnet.tags["Description"] == ec2_vpc_subnet_description' diff --git a/tests/unit/module_utils/test_tagging.py b/tests/unit/module_utils/test_tagging.py index 8ac69f09972..edeb7dabd84 100644 --- a/tests/unit/module_utils/test_tagging.py +++ b/tests/unit/module_utils/test_tagging.py @@ -6,6 +6,7 @@ import pytest from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list +from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_tag_filter_dict from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications from ansible_collections.amazon.aws.plugins.module_utils.tagging import compare_aws_tags @@ -37,6 +38,13 @@ def setup_method(self): "lower case": "lower case value", } + self.tag_filter_dict = { + "tag:lowerCamel": "lowerCamelValue", + "tag:UpperCamel": "upperCamelValue", + "tag:Normal case": "Normal Value", + "tag:lower case": "lower case value", + } + self.tag_minimal_boto3_list = [ {"Key": "mykey", "Value": "myvalue"}, ] @@ -60,6 +68,14 @@ def test_ansible_dict_to_boto3_tag_list_empty(self): assert ansible_dict_to_boto3_tag_list({}) == [] assert ansible_dict_to_boto3_tag_list(None) == [] + def test_ansible_dict_to_boto3_tag_list_boolean(self): + dict_with_bool = dict(boolean=True) + list_with_bool = [{"Key": "boolean", "Value": "True"}] + assert ansible_dict_to_boto3_tag_list(dict_with_bool) == list_with_bool + dict_with_bool = dict(boolean=False) + list_with_bool = [{"Key": "boolean", "Value": "False"}] + assert ansible_dict_to_boto3_tag_list(dict_with_bool) == list_with_bool + # ======================================================== # tagging.boto3_tag_list_to_ansible_dict # ======================================================== @@ -140,6 +156,20 @@ def test_compare_aws_tags_changed(self): assert new_keys == keys_to_set assert [] == keys_to_unset + def test_compare_aws_tags_boolean(self): + dict_with_bool = dict(boolean=True) + dict_with_text_bool = dict(boolean="True") + # AWS always returns tag values as strings, so we only test this way around + keys_to_set, keys_to_unset = compare_aws_tags(dict_with_text_bool, dict_with_bool) + assert {} == keys_to_set + assert [] == keys_to_unset + keys_to_set, keys_to_unset = compare_aws_tags(dict_with_text_bool, dict_with_bool, purge_tags=False) + assert {} == keys_to_set + assert [] == keys_to_unset + keys_to_set, keys_to_unset = compare_aws_tags(dict_with_text_bool, dict_with_bool, purge_tags=True) + assert {} == keys_to_set + assert [] == keys_to_unset + def test_compare_aws_tags_complex_update(self): # Adds 'Me too!', Changes 'UpperCamel' and removes 'Normal case' new_dict = dict(self.tag_example_dict) @@ -221,3 +251,15 @@ def test_boto3_tag_specifications_multipe_types(self): sorted_tag_spec = sorted(tag_specification, key=lambda i: (i["ResourceType"])) sorted_expected = sorted(expected_specification, key=lambda i: (i["ResourceType"])) assert sorted_tag_spec == sorted_expected + + def test_ansible_dict_to_tag_filter_dict_empty(self): + assert ansible_dict_to_tag_filter_dict(None) == {} + assert ansible_dict_to_tag_filter_dict({}) == {} + + def test_ansible_dict_to_tag_filter_dict_example(self): + assert ansible_dict_to_tag_filter_dict(self.tag_example_dict) == self.tag_filter_dict + + def test_ansible_dict_to_tag_filter_dict_boolean(self): + dict_with_bool = {"boolean": True} + filter_dict_with_bool = {"tag:boolean": "True"} + assert ansible_dict_to_tag_filter_dict(dict_with_bool) == filter_dict_with_bool