From a1144a98a105a9be8cfdb7bba60417013fa7d8d2 Mon Sep 17 00:00:00 2001 From: abikouo Date: Mon, 1 Jul 2024 13:31:30 +0200 Subject: [PATCH 1/6] Refactor ec2_vpc_net* modules --- plugins/modules/ec2_vpc_net.py | 338 ++++++++++++---------------- plugins/modules/ec2_vpc_net_info.py | 53 ++--- 2 files changed, 170 insertions(+), 221 deletions(-) diff --git a/plugins/modules/ec2_vpc_net.py b/plugins/modules/ec2_vpc_net.py index a3bc5aa9772..6c3b8773bb2 100644 --- a/plugins/modules/ec2_vpc_net.py +++ b/plugins/modules/ec2_vpc_net.py @@ -206,44 +206,56 @@ from time import sleep from time import time - -try: - import botocore -except ImportError: - pass # Handled by AnsibleAWSModule +from typing import Any +from typing import Dict +from typing import List +from typing import Optional from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible.module_utils.common.network import to_subnet +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AnsibleEC2Error +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import associate_dhcp_options +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import associate_vpc_cidr_block +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import create_vpc +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import delete_vpc +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_vpc_attribute +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_vpcs +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import disassociate_vpc_cidr_block from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ensure_ec2_tags +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import modify_vpc_attribute 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_boto3_tag_list 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 +from ansible_collections.amazon.aws.plugins.module_utils.waiters import wait_for_resource_state -def vpc_exists(module, vpc, name, cidr_block, multi): +def get_vpc_id(connection, module: AnsibleAWSModule) -> Optional[str]: """Returns None or a vpc object depending on the existence of a VPC. When supplied with a CIDR, it will check for matching tags to determine if it is a match otherwise it will assume the VPC does not exist and thus return None. """ - try: - vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": cidr_block}) - matching_vpcs = vpc.describe_vpcs(aws_retry=True, Filters=vpc_filters)["Vpcs"] - # If an exact matching using a list of CIDRs isn't found, check for a match with the first CIDR as is documented for C(cidr_block) - if not matching_vpcs: - vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": [cidr_block[0]]}) - matching_vpcs = vpc.describe_vpcs(aws_retry=True, Filters=vpc_filters)["Vpcs"] - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg="Failed to describe VPCs") + vpc_id = module.params.get("vpc_id") + if vpc_id is not None: + return vpc_id + name = module.params.get("name") + cidr_block = module.params.get("cidr_block") + multi = module.params.get("multi_ok") + vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": cidr_block}) + matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) + # If an exact matching using a list of CIDRs isn't found, check for a match with the first CIDR as is documented for C(cidr_block) + if not matching_vpcs: + vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": [cidr_block[0]]}) + matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) + + vpc_id = None if multi: - return None + vpc_id = None elif len(matching_vpcs) == 1: - return matching_vpcs[0]["VpcId"] + vpc_id = matching_vpcs[0]["VpcId"] elif len(matching_vpcs) > 1: module.fail_json( msg=( @@ -251,37 +263,24 @@ def vpc_exists(module, vpc, name, cidr_block, multi): " If you would like to create the VPC anyway please pass True to the multi_ok param." ) ) - return None - - -def wait_for_vpc_to_exist(module, connection, **params): - # wait for vpc to be available - try: - get_waiter(connection, "vpc_exists").wait(**params) - except botocore.exceptions.WaiterError as e: - module.fail_json_aws(e, msg="VPC failed to reach expected state (exists)") - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg="Unable to wait for VPC creation.") + return vpc_id -def wait_for_vpc(module, connection, **params): +def wait_for_vpc( + module: AnsibleAWSModule, + connection, + waiter_name: str, + delay: Optional[int] = None, + max_attempts: Optional[int] = None, + **params, +) -> None: # wait for vpc to be available - try: - get_waiter(connection, "vpc_available").wait(**params) - except botocore.exceptions.WaiterError as e: - module.fail_json_aws(e, msg="VPC failed to reach expected state (available)") - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg="Unable to wait for VPC state to update.") - + wait_for_resource_state(connection, module, waiter_name, delay=delay, max_attempts=max_attempts, **params) -def get_vpc(module, connection, vpc_id, wait=True): - wait_for_vpc(module, connection, VpcIds=[vpc_id]) - try: - vpc_obj = connection.describe_vpcs(VpcIds=[vpc_id], aws_retry=True)["Vpcs"][0] - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg="Failed to describe VPCs") - return vpc_obj +def get_vpc(module: AnsibleAWSModule, connection, vpc_id: str) -> Dict[str, Any]: + wait_for_vpc(module, connection, waiter_name="vpc_available", VpcIds=[vpc_id]) + return describe_vpcs(connection, VpcIds=[vpc_id])[0] def update_vpc_tags(connection, module, vpc_id, tags, name, purge_tags): @@ -304,7 +303,7 @@ def update_vpc_tags(connection, module, vpc_id, tags, name, purge_tags): return True -def update_dhcp_opts(connection, module, vpc_obj, dhcp_id): +def update_dhcp_opts(connection, module: AnsibleAWSModule, vpc_obj: Dict[str, Any], dhcp_id: Optional[str]) -> bool: if dhcp_id is None: return False if vpc_obj["DhcpOptionsId"] == dhcp_id: @@ -312,15 +311,18 @@ def update_dhcp_opts(connection, module, vpc_obj, dhcp_id): if module.check_mode: return True - try: - connection.associate_dhcp_options(DhcpOptionsId=dhcp_id, VpcId=vpc_obj["VpcId"], aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg=f"Failed to associate DhcpOptionsId {dhcp_id}") - - return True + return associate_dhcp_options(connection, vpc_id=vpc_obj["VpcId"], dhcp_options_id=dhcp_id) -def create_vpc(connection, module, cidr_block, tenancy, tags, ipv6_cidr, name): +def create_vpc_net( + connection, + module: AnsibleAWSModule, + cidr_block: List[str], + tenancy, + tags: Dict[str, str], + ipv6_cidr: bool, + name: Optional[str], +) -> str: if module.check_mode: module.exit_json(changed=True, msg="VPC would be created if not in check mode") @@ -339,27 +341,14 @@ def create_vpc(connection, module, cidr_block, tenancy, tags, ipv6_cidr, name): if ipv6_cidr: create_args["AmazonProvidedIpv6CidrBlock"] = True - try: - vpc_obj = connection.create_vpc(aws_retry=True, **create_args) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, "Failed to create the VPC") + vpc_obj = create_vpc(connection, **create_args) # wait up to 30 seconds for vpc to exist - wait_for_vpc_to_exist( - module, - connection, - VpcIds=[vpc_obj["Vpc"]["VpcId"]], - WaiterConfig=dict(MaxAttempts=30), - ) + wait_for_vpc(module, connection, waiter_name="vpc_exists", max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) # Wait for the VPC to enter an 'Available' State - wait_for_vpc( - module, - connection, - VpcIds=[vpc_obj["Vpc"]["VpcId"]], - WaiterConfig=dict(MaxAttempts=30), - ) + wait_for_vpc(module, connection, waiter_name="vpc_available", max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) - return vpc_obj["Vpc"]["VpcId"] + return vpc_obj["VpcId"] def wait_for_vpc_attribute(connection, module, vpc_id, attribute, expected_value): @@ -371,7 +360,7 @@ def wait_for_vpc_attribute(connection, module, vpc_id, attribute, expected_value start_time = time() updated = False while time() < start_time + 300: - current_value = connection.describe_vpc_attribute(Attribute=attribute, VpcId=vpc_id, aws_retry=True)[ + current_value = describe_vpc_attribute(connection, attribute=attribute, vpc_id=vpc_id)[ f"{attribute[0].upper()}{attribute[1:]}" ]["Value"] if current_value != expected_value: @@ -478,20 +467,14 @@ def update_ipv6_cidrs(connection, module, vpc_obj, vpc_id, ipv6_cidr): # There's no block associated, and we want one to be associated if ipv6_cidr: - try: - connection.associate_vpc_cidr_block(AmazonProvidedIpv6CidrBlock=ipv6_cidr, VpcId=vpc_id, aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, "Unable to associate IPv6 CIDR") + associate_vpc_cidr_block(connection, vpc_id=vpc_id, AmazonProvidedIpv6CidrBlock=ipv6_cidr) else: for ipv6_assoc in vpc_obj["Ipv6CidrBlockAssociationSet"]: if ipv6_assoc["Ipv6Pool"] == "Amazon" and ipv6_assoc["Ipv6CidrBlockState"]["State"] in [ "associated", "associating", ]: - try: - connection.disassociate_vpc_cidr_block(AssociationId=ipv6_assoc["AssociationId"], aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, f"Unable to disassociate IPv6 CIDR {ipv6_assoc['AssociationId']}.") + disassociate_vpc_cidr_block(connection, association_id=ipv6_assoc["AssociationId"]) return True @@ -521,19 +504,18 @@ def update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs): for cidr in cidrs_to_add: try: - connection.associate_vpc_cidr_block(CidrBlock=cidr, VpcId=vpc_id, aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + associate_vpc_cidr_block(connection, vpc_id=vpc_id, CidrBlock=cidr) + except AnsibleEC2Error as e: module.fail_json_aws(e, f"Unable to associate CIDR {cidr}.") for cidr in cidrs_to_remove: - association_id = associated_cidrs[cidr] try: - connection.disassociate_vpc_cidr_block(AssociationId=association_id, aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + disassociate_vpc_cidr_block(connection, associated_cidrs[cidr]) + except AnsibleEC2Error as e: module.fail_json_aws( e, ( - f"Unable to disassociate {association_id}. You must detach or delete all gateways and resources" + f"Unable to disassociate {associated_cidrs[cidr]}. You must detach or delete all gateways and resources" " that are associated with the CIDR block before you can disassociate it." ), ) @@ -544,7 +526,7 @@ def update_dns_enabled(connection, module, vpc_id, dns_support): if dns_support is None: return False - current_dns_enabled = connection.describe_vpc_attribute(Attribute="enableDnsSupport", VpcId=vpc_id, aws_retry=True)[ + current_dns_enabled = describe_vpc_attribute(connection, attribute="enableDnsSupport", vpc_id=vpc_id)[ "EnableDnsSupport" ]["Value"] if current_dns_enabled == dns_support: @@ -553,10 +535,7 @@ def update_dns_enabled(connection, module, vpc_id, dns_support): if module.check_mode: return True - try: - connection.modify_vpc_attribute(VpcId=vpc_id, EnableDnsSupport={"Value": dns_support}, aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, "Failed to update enabled dns support attribute") + modify_vpc_attribute(connection, vpc_id=vpc_id, EnableDnsSupport={"Value": dns_support}) return True @@ -564,39 +543,16 @@ def update_dns_hostnames(connection, module, vpc_id, dns_hostnames): if dns_hostnames is None: return False - current_dns_hostnames = connection.describe_vpc_attribute( - Attribute="enableDnsHostnames", VpcId=vpc_id, aws_retry=True - )["EnableDnsHostnames"]["Value"] + current_dns_hostnames = describe_vpc_attribute(connection, attribute="enableDnsHostnames", vpc_id=vpc_id)[ + "EnableDnsHostnames" + ]["Value"] if current_dns_hostnames == dns_hostnames: return False if module.check_mode: return True - try: - connection.modify_vpc_attribute(VpcId=vpc_id, EnableDnsHostnames={"Value": dns_hostnames}, aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, "Failed to update enabled dns hostnames attribute") - return True - - -def delete_vpc(connection, module, vpc_id): - if vpc_id is None: - return False - if module.check_mode: - return True - - try: - connection.delete_vpc(VpcId=vpc_id, aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws( - e, - msg=( - f"Failed to delete VPC {vpc_id} You may want to use the ec2_vpc_subnet, ec2_vpc_igw, and/or" - " ec2_vpc_route_table modules to ensure that all depenednt components are absent." - ), - ) - + modify_vpc_attribute(connection, vpc_id=vpc_id, EnableDnsHostnames={"Value": dns_hostnames}) return True @@ -608,6 +564,7 @@ def wait_for_updates(connection, module, vpc_id, ipv6_cidr, expected_cidrs, dns_ wait_for_vpc( module, connection, + waiter_name="vpc_available", VpcIds=[vpc_id], Filters=[{"Name": "cidr-block-association.cidr-block", "Values": expected_cidrs}], ) @@ -616,7 +573,7 @@ def wait_for_updates(connection, module, vpc_id, ipv6_cidr, expected_cidrs, dns_ if tags is not None: tag_list = ansible_dict_to_boto3_tag_list(tags) filters = [{"Name": f"tag:{t['Key']}", "Values": [t["Value"]]} for t in tag_list] - wait_for_vpc(module, connection, VpcIds=[vpc_id], Filters=filters) + wait_for_vpc(module, connection, waiter_name="vpc_available", VpcIds=[vpc_id], Filters=filters) wait_for_vpc_attribute(connection, module, vpc_id, "enableDnsSupport", dns_support) wait_for_vpc_attribute(connection, module, vpc_id, "enableDnsHostnames", dns_hostnames) @@ -624,11 +581,72 @@ def wait_for_updates(connection, module, vpc_id, ipv6_cidr, expected_cidrs, dns_ if dhcp_id is not None: # Wait for DhcpOptionsId to be updated filters = [{"Name": "dhcp-options-id", "Values": [dhcp_id]}] - wait_for_vpc(module, connection, VpcIds=[vpc_id], Filters=filters) + wait_for_vpc(module, connection, waiter_name="vpc_available", VpcIds=[vpc_id], Filters=filters) return +def ensure_present(connection, module: AnsibleAWSModule, vpc_id: str) -> None: + name = module.params.get("name") + cidr_block = module.params.get("cidr_block") + ipv6_cidr = module.params.get("ipv6_cidr") + purge_cidrs = module.params.get("purge_cidrs") + tenancy = module.params.get("tenancy") + dns_support = module.params.get("dns_support") + dns_hostnames = module.params.get("dns_hostnames") + dhcp_id = module.params.get("dhcp_opts_id") + tags = module.params.get("tags") + purge_tags = module.params.get("purge_tags") + + if vpc_id is None: + if name is None: + module.fail_json("The name parameter must be specified when creating a new VPC.") + vpc_id = create_vpc_net(connection, module, cidr_block[0], tenancy, tags, ipv6_cidr, name) + changed = True + vpc_obj = get_vpc(module, connection, vpc_id) + if len(cidr_block) > 1: + cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs) + changed |= cidrs_changed + else: + desired_cidrs = None + # Set on-creation defaults + if dns_hostnames is None: + dns_hostnames = True + if dns_support is None: + dns_support = True + else: + changed = False + vpc_obj = get_vpc(module, connection, vpc_id) + cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs) + changed |= cidrs_changed + changed |= update_ipv6_cidrs(connection, module, vpc_obj, vpc_id, ipv6_cidr) + changed |= update_vpc_tags(connection, module, vpc_id, tags, name, purge_tags) + + changed |= update_dhcp_opts(connection, module, vpc_obj, dhcp_id) + changed |= update_dns_enabled(connection, module, vpc_id, dns_support) + changed |= update_dns_hostnames(connection, module, vpc_id, dns_hostnames) + + wait_for_updates(connection, module, vpc_id, ipv6_cidr, desired_cidrs, dns_support, dns_hostnames, tags, dhcp_id) + + updated_obj = get_vpc(module, connection, vpc_id) + final_state = camel_dict_to_snake_dict(updated_obj) + final_state["tags"] = boto3_tag_list_to_ansible_dict(updated_obj.get("Tags", [])) + final_state["name"] = final_state["tags"].get("Name", None) + final_state["id"] = final_state.pop("vpc_id") + + module.exit_json(changed=changed, vpc=final_state) + + +def ensure_absent(connection, module: AnsibleAWSModule, vpc_id: str) -> None: + changed = False + if vpc_id: + if module.check_mode: + changed = True + else: + changed = delete_vpc(connection, vpc_id=vpc_id) + module.exit_json(changed=changed, vpc={}) + + def main(): argument_spec = dict( name=dict(required=False), @@ -652,88 +670,22 @@ def main(): module = AnsibleAWSModule(argument_spec=argument_spec, required_one_of=required_one_of, supports_check_mode=True) - name = module.params.get("name") - vpc_id = module.params.get("vpc_id") - cidr_block = module.params.get("cidr_block") - ipv6_cidr = module.params.get("ipv6_cidr") - purge_cidrs = module.params.get("purge_cidrs") - tenancy = module.params.get("tenancy") dns_support = module.params.get("dns_support") dns_hostnames = module.params.get("dns_hostnames") - dhcp_id = module.params.get("dhcp_opts_id") - tags = module.params.get("tags") - purge_tags = module.params.get("purge_tags") state = module.params.get("state") - multi = module.params.get("multi_ok") - - changed = False - - connection = module.client( - "ec2", - retry_decorator=AWSRetry.jittered_backoff( - retries=8, delay=3, catch_extra_error_codes=["InvalidVpcID.NotFound"] - ), - ) if dns_hostnames and not dns_support: module.fail_json(msg="In order to enable DNS Hostnames you must also enable DNS support") - cidr_block = get_cidr_network_bits(module, module.params.get("cidr_block")) - - if vpc_id is None: - vpc_id = vpc_exists(module, connection, name, cidr_block, multi) - - if state == "present": - # Check if VPC exists - if vpc_id is None: - if module.params.get("name") is None: - module.fail_json("The name parameter must be specified when creating a new VPC.") - vpc_id = create_vpc(connection, module, cidr_block[0], tenancy, tags, ipv6_cidr, name) - changed = True - vpc_obj = get_vpc(module, connection, vpc_id) - if len(cidr_block) > 1: - cidrs_changed, desired_cidrs = update_cidrs( - connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs - ) - changed |= cidrs_changed - else: - desired_cidrs = None - # Set on-creation defaults - if dns_hostnames is None: - dns_hostnames = True - if dns_support is None: - dns_support = True + connection = module.client("ec2") + try: + vpc_id = get_vpc_id(connection, module) + if state == "present": + ensure_present(connection, module, vpc_id) else: - vpc_obj = get_vpc(module, connection, vpc_id) - cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs) - changed |= cidrs_changed - ipv6_changed = update_ipv6_cidrs(connection, module, vpc_obj, vpc_id, ipv6_cidr) - changed |= ipv6_changed - tags_changed = update_vpc_tags(connection, module, vpc_id, tags, name, purge_tags) - changed |= tags_changed - - dhcp_changed = update_dhcp_opts(connection, module, vpc_obj, dhcp_id) - changed |= dhcp_changed - dns_changed = update_dns_enabled(connection, module, vpc_id, dns_support) - changed |= dns_changed - hostnames_changed = update_dns_hostnames(connection, module, vpc_id, dns_hostnames) - changed |= hostnames_changed - - wait_for_updates( - connection, module, vpc_id, ipv6_cidr, desired_cidrs, dns_support, dns_hostnames, tags, dhcp_id - ) - - updated_obj = get_vpc(module, connection, vpc_id) - final_state = camel_dict_to_snake_dict(updated_obj) - final_state["tags"] = boto3_tag_list_to_ansible_dict(updated_obj.get("Tags", [])) - final_state["name"] = final_state["tags"].get("Name", None) - final_state["id"] = final_state.pop("vpc_id") - - module.exit_json(changed=changed, vpc=final_state) - - elif state == "absent": - changed = delete_vpc(connection, module, vpc_id) - module.exit_json(changed=changed, vpc={}) + ensure_absent(connection, module, vpc_id) + except AnsibleEC2Error as e: + module.fail_json_aws_error(e) if __name__ == "__main__": diff --git a/plugins/modules/ec2_vpc_net_info.py b/plugins/modules/ec2_vpc_net_info.py index a9d357cfb86..995e6638781 100644 --- a/plugins/modules/ec2_vpc_net_info.py +++ b/plugins/modules/ec2_vpc_net_info.py @@ -145,21 +145,17 @@ sample: dopt-12345678 """ -try: - import botocore -except ImportError: - pass # Handled by AnsibleAWSModule - from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict -from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AnsibleEC2Error +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_vpc_attribute +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_vpcs 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.transformation import ansible_dict_to_boto3_filter_list -def describe_vpcs(connection, module): +def list_vpcs(connection, module: AnsibleAWSModule) -> None: """ Describe VPCs. @@ -175,18 +171,31 @@ def describe_vpcs(connection, module): # Get the basic VPC info try: - response = connection.describe_vpcs(VpcIds=vpc_ids, Filters=filters, aws_retry=True) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + vpcs = describe_vpcs(connection, VpcIds=vpc_ids, Filters=filters) + except AnsibleEC2Error as e: module.fail_json_aws(e, msg=f"Unable to describe VPCs {vpc_ids}") # We can get these results in bulk but still needs two separate calls to the API dns_support = {} dns_hostnames = {} # Loop through the results and add the other VPC attributes we gathered - for vpc in response["Vpcs"]: + for vpc in vpcs: error_message = "Unable to describe VPC attribute {0} on VPC {1}" - dns_support = describe_vpc_attribute(module, connection, vpc["VpcId"], "enableDnsSupport", error_message) - dns_hostnames = describe_vpc_attribute(module, connection, vpc["VpcId"], "enableDnsHostnames", error_message) + try: + attribute = "enableDnsSupport" + dns_support = describe_vpc_attribute(connection, vpc_id=vpc["VpcId"], attribute=attribute) + if not dns_support: + module.warn(error_message.format(attribute, vpc["VpcId"])) + except AnsibleEC2Error as e: + module.fail_json_aws(e, msg=error_message.format(attribute, vpc)) + + try: + attribute = "enableDnsHostnames" + dns_hostnames = describe_vpc_attribute(connection, vpc_id=vpc["VpcId"], attribute=attribute) + if not dns_hostnames: + module.warn(error_message.format(attribute, vpc["VpcId"])) + except AnsibleEC2Error as e: + module.fail_json_aws(e, msg=error_message.format(attribute, vpc)) # add the two DNS attributes if dns_support: @@ -202,28 +211,16 @@ def describe_vpcs(connection, module): module.exit_json(vpcs=vpc_info) -def describe_vpc_attribute(module, connection, vpc, attribute, error_message): - result = None - try: - return connection.describe_vpc_attribute(VpcId=vpc, Attribute=attribute, aws_retry=True) - except is_boto3_error_code("InvalidVpcID.NotFound"): - module.warn(error_message.format(attribute, vpc)) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg=error_message.format(attribute, vpc)) - return result - - -def main(): +def main() -> None: argument_spec = dict( vpc_ids=dict(type="list", elements="str", default=[]), filters=dict(type="dict", default={}), ) module = AnsibleAWSModule(argument_spec=argument_spec, supports_check_mode=True) + connection = module.client("ec2") - connection = module.client("ec2", retry_decorator=AWSRetry.jittered_backoff(retries=10)) - - describe_vpcs(connection, module) + list_vpcs(connection, module) if __name__ == "__main__": From ae0289e25f8e27db1a4428b03ebbbb4462b34eba Mon Sep 17 00:00:00 2001 From: abikouo Date: Mon, 1 Jul 2024 13:41:20 +0200 Subject: [PATCH 2/6] add changelog fragment --- changelogs/fragments/refactor_ec2_vpc_net-modules.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/fragments/refactor_ec2_vpc_net-modules.yml diff --git a/changelogs/fragments/refactor_ec2_vpc_net-modules.yml b/changelogs/fragments/refactor_ec2_vpc_net-modules.yml new file mode 100644 index 00000000000..d063eb36f9a --- /dev/null +++ b/changelogs/fragments/refactor_ec2_vpc_net-modules.yml @@ -0,0 +1,4 @@ +--- +minor_changes: + - ec2_vpc_net - refactored code to use ``AnsibleEC2Error`` as well as moving shared code into module_utils.ec2 (https://github.com/ansible-collections/amazon.aws/pull/2158). + - ec2_vpc_net_info - refactored code to use ``AnsibleEC2Error`` as well as moving shared code into module_utils.ec2 (https://github.com/ansible-collections/amazon.aws/pull/2158). \ No newline at end of file From 745d0eecba9cefbae4c7e8343d6c8323bdabd596 Mon Sep 17 00:00:00 2001 From: abikouo Date: Thu, 4 Jul 2024 17:49:17 +0200 Subject: [PATCH 3/6] code review updates --- plugins/modules/ec2_vpc_net.py | 63 ++++++++++++----------------- plugins/modules/ec2_vpc_net_info.py | 34 ++++++---------- 2 files changed, 38 insertions(+), 59 deletions(-) diff --git a/plugins/modules/ec2_vpc_net.py b/plugins/modules/ec2_vpc_net.py index 6c3b8773bb2..bdc70cbe923 100644 --- a/plugins/modules/ec2_vpc_net.py +++ b/plugins/modules/ec2_vpc_net.py @@ -238,31 +238,26 @@ def get_vpc_id(connection, module: AnsibleAWSModule) -> Optional[str]: otherwise it will assume the VPC does not exist and thus return None. """ vpc_id = module.params.get("vpc_id") - if vpc_id is not None: - return vpc_id - - name = module.params.get("name") - cidr_block = module.params.get("cidr_block") - multi = module.params.get("multi_ok") - vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": cidr_block}) - matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) - # If an exact matching using a list of CIDRs isn't found, check for a match with the first CIDR as is documented for C(cidr_block) - if not matching_vpcs: - vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": [cidr_block[0]]}) + if not vpc_id: + name = module.params.get("name") + cidr_block = module.params.get("cidr_block") + multi = module.params.get("multi_ok") + vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": cidr_block}) matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) - - vpc_id = None - if multi: - vpc_id = None - elif len(matching_vpcs) == 1: - vpc_id = matching_vpcs[0]["VpcId"] - elif len(matching_vpcs) > 1: - module.fail_json( - msg=( - f"Currently there are {len(matching_vpcs)} VPCs that have the same name and CIDR block you specified." - " If you would like to create the VPC anyway please pass True to the multi_ok param." + # If an exact matching using a list of CIDRs isn't found, check for a match with the first CIDR as is documented for C(cidr_block) + if not matching_vpcs: + vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": [cidr_block[0]]}) + matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) + + if len(matching_vpcs) == 1: + vpc_id = matching_vpcs[0]["VpcId"] + elif len(matching_vpcs) > 1 and not multi: + module.fail_json( + msg=( + f"Currently there are {len(matching_vpcs)} VPCs that have the same name and CIDR block you specified." + " If you would like to create the VPC anyway please pass True to the multi_ok param." + ) ) - ) return vpc_id @@ -322,7 +317,7 @@ def create_vpc_net( tags: Dict[str, str], ipv6_cidr: bool, name: Optional[str], -) -> str: +) -> Dict[str, Any]: if module.check_mode: module.exit_json(changed=True, msg="VPC would be created if not in check mode") @@ -348,7 +343,7 @@ def create_vpc_net( # Wait for the VPC to enter an 'Available' State wait_for_vpc(module, connection, waiter_name="vpc_available", max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) - return vpc_obj["VpcId"] + return vpc_obj def wait_for_vpc_attribute(connection, module, vpc_id, attribute, expected_value): @@ -586,7 +581,7 @@ def wait_for_updates(connection, module, vpc_id, ipv6_cidr, expected_cidrs, dns_ return -def ensure_present(connection, module: AnsibleAWSModule, vpc_id: str) -> None: +def ensure_present(connection, module: AnsibleAWSModule, vpc_id: Optional[str]) -> None: name = module.params.get("name") cidr_block = module.params.get("cidr_block") ipv6_cidr = module.params.get("ipv6_cidr") @@ -598,17 +593,11 @@ def ensure_present(connection, module: AnsibleAWSModule, vpc_id: str) -> None: tags = module.params.get("tags") purge_tags = module.params.get("purge_tags") - if vpc_id is None: - if name is None: + if not vpc_id: + if not name: module.fail_json("The name parameter must be specified when creating a new VPC.") - vpc_id = create_vpc_net(connection, module, cidr_block[0], tenancy, tags, ipv6_cidr, name) + vpc_obj = create_vpc_net(connection, module, cidr_block[0], tenancy, tags, ipv6_cidr, name) changed = True - vpc_obj = get_vpc(module, connection, vpc_id) - if len(cidr_block) > 1: - cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs) - changed |= cidrs_changed - else: - desired_cidrs = None # Set on-creation defaults if dns_hostnames is None: dns_hostnames = True @@ -617,11 +606,11 @@ def ensure_present(connection, module: AnsibleAWSModule, vpc_id: str) -> None: else: changed = False vpc_obj = get_vpc(module, connection, vpc_id) - cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs) - changed |= cidrs_changed changed |= update_ipv6_cidrs(connection, module, vpc_obj, vpc_id, ipv6_cidr) changed |= update_vpc_tags(connection, module, vpc_id, tags, name, purge_tags) + cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs) + changed |= cidrs_changed changed |= update_dhcp_opts(connection, module, vpc_obj, dhcp_id) changed |= update_dns_enabled(connection, module, vpc_id, dns_support) changed |= update_dns_hostnames(connection, module, vpc_id, dns_hostnames) diff --git a/plugins/modules/ec2_vpc_net_info.py b/plugins/modules/ec2_vpc_net_info.py index 995e6638781..5d56f502274 100644 --- a/plugins/modules/ec2_vpc_net_info.py +++ b/plugins/modules/ec2_vpc_net_info.py @@ -175,33 +175,23 @@ def list_vpcs(connection, module: AnsibleAWSModule) -> None: except AnsibleEC2Error as e: module.fail_json_aws(e, msg=f"Unable to describe VPCs {vpc_ids}") - # We can get these results in bulk but still needs two separate calls to the API - dns_support = {} - dns_hostnames = {} # Loop through the results and add the other VPC attributes we gathered for vpc in vpcs: error_message = "Unable to describe VPC attribute {0} on VPC {1}" - try: - attribute = "enableDnsSupport" - dns_support = describe_vpc_attribute(connection, vpc_id=vpc["VpcId"], attribute=attribute) - if not dns_support: - module.warn(error_message.format(attribute, vpc["VpcId"])) - except AnsibleEC2Error as e: - module.fail_json_aws(e, msg=error_message.format(attribute, vpc)) - - try: - attribute = "enableDnsHostnames" - dns_hostnames = describe_vpc_attribute(connection, vpc_id=vpc["VpcId"], attribute=attribute) - if not dns_hostnames: - module.warn(error_message.format(attribute, vpc["VpcId"])) - except AnsibleEC2Error as e: - module.fail_json_aws(e, msg=error_message.format(attribute, vpc)) + vpc_attributes = {} + for attribute in ("enableDnsSupport", "enableDnsHostnames"): + try: + vpc_attributes[attribute] = describe_vpc_attribute(connection, vpc_id=vpc["VpcId"], attribute=attribute) + if not vpc_attributes[attribute]: + module.warn(error_message.format(attribute, vpc["VpcId"])) + except AnsibleEC2Error as e: + module.fail_json_aws(e, msg=error_message.format(attribute, vpc)) # add the two DNS attributes - if dns_support: - vpc["EnableDnsSupport"] = dns_support["EnableDnsSupport"].get("Value") - if dns_hostnames: - vpc["EnableDnsHostnames"] = dns_hostnames["EnableDnsHostnames"].get("Value") + if vpc_attributes["enableDnsSupport"]: + vpc["EnableDnsSupport"] = vpc_attributes["enableDnsSupport"]["EnableDnsSupport"].get("Value") + if vpc_attributes["enableDnsHostnames"]: + vpc["EnableDnsHostnames"] = vpc_attributes["enableDnsHostnames"]["EnableDnsHostnames"].get("Value") # for backwards compatibility vpc["id"] = vpc["VpcId"] vpc_info.append(camel_dict_to_snake_dict(vpc)) From 3be7ce10d531f1d8156f978d73695dd73199ffbe Mon Sep 17 00:00:00 2001 From: abikouo Date: Fri, 5 Jul 2024 08:08:00 +0200 Subject: [PATCH 4/6] minor updates --- plugins/modules/ec2_vpc_net.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/ec2_vpc_net.py b/plugins/modules/ec2_vpc_net.py index bdc70cbe923..7a07934997c 100644 --- a/plugins/modules/ec2_vpc_net.py +++ b/plugins/modules/ec2_vpc_net.py @@ -343,7 +343,7 @@ def create_vpc_net( # Wait for the VPC to enter an 'Available' State wait_for_vpc(module, connection, waiter_name="vpc_available", max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) - return vpc_obj + return vpc_obj["Vpc"] def wait_for_vpc_attribute(connection, module, vpc_id, attribute, expected_value): @@ -597,6 +597,7 @@ def ensure_present(connection, module: AnsibleAWSModule, vpc_id: Optional[str]) if not name: module.fail_json("The name parameter must be specified when creating a new VPC.") vpc_obj = create_vpc_net(connection, module, cidr_block[0], tenancy, tags, ipv6_cidr, name) + vpc_id = vpc_obj["VpcId"] changed = True # Set on-creation defaults if dns_hostnames is None: From 3a6a6a485e221783ad66f43479f8b728a7de73da Mon Sep 17 00:00:00 2001 From: abikouo Date: Fri, 5 Jul 2024 13:42:42 +0200 Subject: [PATCH 5/6] add some unit tests --- plugins/modules/ec2_vpc_net.py | 75 +++-- .../unit/plugins/modules/test_ec2_vpc_net.py | 265 ++++++++++++++++++ 2 files changed, 301 insertions(+), 39 deletions(-) create mode 100644 tests/unit/plugins/modules/test_ec2_vpc_net.py diff --git a/plugins/modules/ec2_vpc_net.py b/plugins/modules/ec2_vpc_net.py index 7a07934997c..216ea8fad39 100644 --- a/plugins/modules/ec2_vpc_net.py +++ b/plugins/modules/ec2_vpc_net.py @@ -237,27 +237,26 @@ def get_vpc_id(connection, module: AnsibleAWSModule) -> Optional[str]: with a CIDR, it will check for matching tags to determine if it is a match otherwise it will assume the VPC does not exist and thus return None. """ - vpc_id = module.params.get("vpc_id") - if not vpc_id: - name = module.params.get("name") - cidr_block = module.params.get("cidr_block") - multi = module.params.get("multi_ok") - vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": cidr_block}) + name = module.params.get("name") + cidr_block = module.params.get("cidr_block") + multi_ok = module.params.get("multi_ok") + vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": cidr_block}) + matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) + # If an exact matching using a list of CIDRs isn't found, check for a match with the first CIDR as is documented for C(cidr_block) + if not matching_vpcs: + vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": [cidr_block[0]]}) matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) - # If an exact matching using a list of CIDRs isn't found, check for a match with the first CIDR as is documented for C(cidr_block) - if not matching_vpcs: - vpc_filters = ansible_dict_to_boto3_filter_list({"tag:Name": name, "cidr-block": [cidr_block[0]]}) - matching_vpcs = describe_vpcs(connection, Filters=vpc_filters) - - if len(matching_vpcs) == 1: - vpc_id = matching_vpcs[0]["VpcId"] - elif len(matching_vpcs) > 1 and not multi: + + vpc_id = None + if not multi_ok and matching_vpcs: + if len(matching_vpcs) > 1: module.fail_json( msg=( f"Currently there are {len(matching_vpcs)} VPCs that have the same name and CIDR block you specified." " If you would like to create the VPC anyway please pass True to the multi_ok param." ) ) + vpc_id = matching_vpcs[0]["VpcId"] return vpc_id @@ -343,7 +342,7 @@ def create_vpc_net( # Wait for the VPC to enter an 'Available' State wait_for_vpc(module, connection, waiter_name="vpc_available", max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) - return vpc_obj["Vpc"] + return vpc_obj def wait_for_vpc_attribute(connection, module, vpc_id, attribute, expected_value): @@ -473,8 +472,8 @@ def update_ipv6_cidrs(connection, module, vpc_obj, vpc_id, ipv6_cidr): return True -def update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs): - if cidr_block is None: +def update_cidrs(connection, module, vpc_obj, cidr_block, purge_cidrs): + if not cidr_block: return False, None associated_cidrs = dict( @@ -494,26 +493,24 @@ def update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs): if not cidrs_to_add and not cidrs_to_remove: return False, None - if module.check_mode: - return True, list(desired_cidrs) - - for cidr in cidrs_to_add: - try: - associate_vpc_cidr_block(connection, vpc_id=vpc_id, CidrBlock=cidr) - except AnsibleEC2Error as e: - module.fail_json_aws(e, f"Unable to associate CIDR {cidr}.") - - for cidr in cidrs_to_remove: - try: - disassociate_vpc_cidr_block(connection, associated_cidrs[cidr]) - except AnsibleEC2Error as e: - module.fail_json_aws( - e, - ( - f"Unable to disassociate {associated_cidrs[cidr]}. You must detach or delete all gateways and resources" - " that are associated with the CIDR block before you can disassociate it." - ), - ) + if not module.check_mode: + for cidr in cidrs_to_add: + try: + associate_vpc_cidr_block(connection, vpc_id=vpc_obj["VpcId"], CidrBlock=cidr) + except AnsibleEC2Error as e: + module.fail_json_aws(e, f"Unable to associate CIDR {cidr}.") + + for cidr in cidrs_to_remove: + try: + disassociate_vpc_cidr_block(connection, associated_cidrs[cidr]) + except AnsibleEC2Error as e: + module.fail_json_aws( + e, + ( + f"Unable to disassociate {associated_cidrs[cidr]}. You must detach or delete all gateways and resources" + " that are associated with the CIDR block before you can disassociate it." + ), + ) return True, list(desired_cidrs) @@ -610,7 +607,7 @@ def ensure_present(connection, module: AnsibleAWSModule, vpc_id: Optional[str]) changed |= update_ipv6_cidrs(connection, module, vpc_obj, vpc_id, ipv6_cidr) changed |= update_vpc_tags(connection, module, vpc_id, tags, name, purge_tags) - cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, vpc_id, cidr_block, purge_cidrs) + cidrs_changed, desired_cidrs = update_cidrs(connection, module, vpc_obj, cidr_block, purge_cidrs) changed |= cidrs_changed changed |= update_dhcp_opts(connection, module, vpc_obj, dhcp_id) changed |= update_dns_enabled(connection, module, vpc_id, dns_support) @@ -669,7 +666,7 @@ def main(): connection = module.client("ec2") try: - vpc_id = get_vpc_id(connection, module) + vpc_id = module.params.get("vpc_id") or get_vpc_id(connection, module) if state == "present": ensure_present(connection, module, vpc_id) else: diff --git a/tests/unit/plugins/modules/test_ec2_vpc_net.py b/tests/unit/plugins/modules/test_ec2_vpc_net.py new file mode 100644 index 00000000000..5ebfc5a225c --- /dev/null +++ b/tests/unit/plugins/modules/test_ec2_vpc_net.py @@ -0,0 +1,265 @@ +# (c) 2024 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from unittest.mock import ANY +from unittest.mock import MagicMock +from unittest.mock import call +from unittest.mock import patch + +import pytest + +from ansible_collections.amazon.aws.plugins.modules import ec2_vpc_net + +module_name = "ansible_collections.amazon.aws.plugins.modules.ec2_vpc_net" + + +@pytest.fixture +def ansible_module(): + module = MagicMock() + module.check_mode = False + module.params = {} + module.fail_json.side_effect = SystemExit(1) + module.fail_json_aws.side_effect = SystemExit(1) + + return module + + +@patch(module_name + ".ansible_dict_to_boto3_filter_list") +@patch(module_name + ".describe_vpcs") +def test_get_vpc_id_no_id(m_describe_vpcs, m_ansible_dict_to_boto3_filter_list, ansible_module): + connection = MagicMock() + cidr_block = MagicMock() + vpc_id = MagicMock() + + m_ansible_dict_to_boto3_filter_list.return_value = MagicMock() + ansible_module.params.update({"cidr_block": cidr_block}) + m_describe_vpcs.return_value = [{"VpcId": vpc_id}] + + assert ec2_vpc_net.get_vpc_id(connection, ansible_module) == vpc_id + m_ansible_dict_to_boto3_filter_list.assert_called_once_with( + {"tag:Name": ansible_module.params.get("name"), "cidr-block": cidr_block} + ) + m_describe_vpcs.assert_called_once_with(connection, Filters=m_ansible_dict_to_boto3_filter_list.return_value) + + +@pytest.mark.parametrize("multi_ok", [True, False]) +@patch(module_name + ".ansible_dict_to_boto3_filter_list") +@patch(module_name + ".describe_vpcs") +def test_get_vpc_id_multiple(m_describe_vpcs, m_ansible_dict_to_boto3_filter_list, ansible_module, multi_ok): + connection = MagicMock() + cidr_block = MagicMock() + vpc_id = MagicMock() + another_vpc_id = MagicMock() + + m_ansible_dict_to_boto3_filter_list.return_value = MagicMock() + ansible_module.params.update({"cidr_block": cidr_block, "multi_ok": multi_ok}) + m_describe_vpcs.return_value = [{"VpcId": vpc_id}, {"VpcId": another_vpc_id}] + + if multi_ok: + assert not ec2_vpc_net.get_vpc_id(connection, ansible_module) + else: + with pytest.raises(SystemExit): + ec2_vpc_net.get_vpc_id(connection, ansible_module) + + m_ansible_dict_to_boto3_filter_list.assert_called_once_with( + {"tag:Name": ansible_module.params.get("name"), "cidr-block": cidr_block} + ) + m_describe_vpcs.assert_called_once_with(connection, Filters=m_ansible_dict_to_boto3_filter_list.return_value) + + +@patch(module_name + ".ansible_dict_to_boto3_filter_list") +@patch(module_name + ".describe_vpcs") +def test_get_vpc_id_cidr_block_list(m_describe_vpcs, m_ansible_dict_to_boto3_filter_list, ansible_module): + connection = MagicMock() + cidr_block = [MagicMock(), MagicMock()] + vpc_id = MagicMock() + vpc_filters = [MagicMock(), MagicMock()] + + m_ansible_dict_to_boto3_filter_list.side_effect = vpc_filters + ansible_module.params.update({"cidr_block": cidr_block}) + m_describe_vpcs.side_effect = [[], [{"VpcId": vpc_id}]] + + assert ec2_vpc_net.get_vpc_id(connection, ansible_module) == vpc_id + m_ansible_dict_to_boto3_filter_list.assert_has_calls( + [ + call({"tag:Name": ansible_module.params.get("name"), "cidr-block": cidr_block}), + call({"tag:Name": ansible_module.params.get("name"), "cidr-block": [cidr_block[0]]}), + ] + ) + m_describe_vpcs.assert_has_calls( + [ + call(connection, Filters=vpc_filters[0]), + call(connection, Filters=vpc_filters[1]), + ] + ) + + +@pytest.mark.parametrize("cidr_block", [None, []]) +@patch(module_name + ".disassociate_vpc_cidr_block") +@patch(module_name + ".associate_vpc_cidr_block") +def test_update_cidrs_no_cidr_block( + m_associate_vpc_cidr_block, m_disassociate_vpc_cidr_block, ansible_module, cidr_block +): + connection = MagicMock() + vpc_obj = {} + cidr_block = [] + changed, desired_cidrs = ec2_vpc_net.update_cidrs(connection, ansible_module, vpc_obj, cidr_block, ANY) + assert not changed + assert desired_cidrs is None + + m_associate_vpc_cidr_block.assert_not_called() + m_disassociate_vpc_cidr_block.assert_not_called() + + +@pytest.mark.parametrize("check_mode", [True, False]) +@pytest.mark.parametrize( + "associated_cidrs, cidr_block, purge_cidrs, add, remove, expected", + [ + ( + [ + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.1.0.0/24", + "CidrBlockState": {"State": "associated"}, + }, + ], + ["10.1.0.0/24"], + True, + [], + [], + (False, None), + ), + ( + [ + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.1.0.0/24", + "CidrBlockState": {"State": "associated"}, + }, + ], + ["10.1.0.0/24"], + False, + [], + [], + (False, None), + ), + ( + [ + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.1.0.0/24", + "CidrBlockState": {"State": "associated"}, + }, + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.2.0.0/24", + "CidrBlockState": {"State": "failed"}, + }, + ], + ["10.1.0.0/24"], + True, + [], + [], + (True, ["10.1.0.0/24"]), + ), + ( + [ + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.1.0.0/24", + "CidrBlockState": {"State": "associated"}, + }, + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.2.0.0/24", + "CidrBlockState": {"State": "failed"}, + }, + ], + ["10.1.0.0/24"], + False, + [], + [], + (False, None), + ), + ( + [ + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.1.0.0/24", + "CidrBlockState": {"State": "associated"}, + }, + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.2.0.0/24", + "CidrBlockState": {"State": "disassociating"}, + }, + ], + ["10.2.0.0/24"], + False, + ["10.2.0.0/24"], + [], + (True, ["10.1.0.0/24", "10.2.0.0/24"]), + ), + ( + [ + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.1.0.0/24", + "CidrBlockState": {"State": "associated"}, + }, + { + "AssociationId": "vpc-cidr-assoc-001", + "CidrBlock": "10.2.0.0/24", + "CidrBlockState": {"State": "disassociating"}, + }, + ], + ["10.2.0.0/24"], + True, + ["10.2.0.0/24"], + ["vpc-cidr-assoc-001"], + (True, ["10.2.0.0/24"]), + ), + ], +) +@patch(module_name + ".disassociate_vpc_cidr_block") +@patch(module_name + ".associate_vpc_cidr_block") +def test_update_cidrs_no_purge( + m_associate_vpc_cidr_block, + m_disassociate_vpc_cidr_block, + ansible_module, + check_mode, + associated_cidrs, + cidr_block, + purge_cidrs, + add, + remove, + expected, +): + connection = MagicMock() + vpc_obj = {"VpcId": MagicMock(), "CidrBlockAssociationSet": associated_cidrs} + ansible_module.check_mode = check_mode + changed, desired_cidrs = ec2_vpc_net.update_cidrs(connection, ansible_module, vpc_obj, cidr_block, purge_cidrs) + assert expected[0] == changed + + def sorted_list(x): + return x if not x else sorted(x) + + assert sorted_list(expected[1]) == sorted_list(desired_cidrs) + + if not expected[0] or check_mode: + m_associate_vpc_cidr_block.assert_not_called() + m_disassociate_vpc_cidr_block.assert_not_called() + else: + if add: + m_associate_vpc_cidr_block.assert_has_calls( + [call(connection, vpc_id=vpc_obj["VpcId"], CidrBlock=cidr) for cidr in add], any_order=True + ) + else: + m_associate_vpc_cidr_block.assert_not_called() + + if remove: + m_disassociate_vpc_cidr_block.assert_has_calls( + [call(connection, association_id) for association_id in remove], any_order=False + ) From 790add335f10a569dd21a7c3e16da99554696601 Mon Sep 17 00:00:00 2001 From: abikouo Date: Mon, 8 Jul 2024 12:00:51 +0200 Subject: [PATCH 6/6] Minor code review updates --- plugins/modules/ec2_vpc_net.py | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/plugins/modules/ec2_vpc_net.py b/plugins/modules/ec2_vpc_net.py index 216ea8fad39..dbaf971784b 100644 --- a/plugins/modules/ec2_vpc_net.py +++ b/plugins/modules/ec2_vpc_net.py @@ -260,20 +260,8 @@ def get_vpc_id(connection, module: AnsibleAWSModule) -> Optional[str]: return vpc_id -def wait_for_vpc( - module: AnsibleAWSModule, - connection, - waiter_name: str, - delay: Optional[int] = None, - max_attempts: Optional[int] = None, - **params, -) -> None: - # wait for vpc to be available - wait_for_resource_state(connection, module, waiter_name, delay=delay, max_attempts=max_attempts, **params) - - def get_vpc(module: AnsibleAWSModule, connection, vpc_id: str) -> Dict[str, Any]: - wait_for_vpc(module, connection, waiter_name="vpc_available", VpcIds=[vpc_id]) + wait_for_resource_state(connection, module, "vpc_available", VpcIds=[vpc_id]) return describe_vpcs(connection, VpcIds=[vpc_id])[0] @@ -337,10 +325,9 @@ def create_vpc_net( vpc_obj = create_vpc(connection, **create_args) - # wait up to 30 seconds for vpc to exist - wait_for_vpc(module, connection, waiter_name="vpc_exists", max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) - # Wait for the VPC to enter an 'Available' State - wait_for_vpc(module, connection, waiter_name="vpc_available", max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) + # Wait up to 30 seconds for vpc to exist and for state 'available' + for waiter_name in ("vpc_exists", "vpc_available"): + wait_for_resource_state(connection, module, waiter_name, max_attempts=30, VpcIds=[vpc_obj["VpcId"]]) return vpc_obj @@ -553,10 +540,10 @@ def wait_for_updates(connection, module, vpc_id, ipv6_cidr, expected_cidrs, dns_ return if expected_cidrs: - wait_for_vpc( - module, + wait_for_resource_state( connection, - waiter_name="vpc_available", + module, + "vpc_available", VpcIds=[vpc_id], Filters=[{"Name": "cidr-block-association.cidr-block", "Values": expected_cidrs}], ) @@ -565,7 +552,7 @@ def wait_for_updates(connection, module, vpc_id, ipv6_cidr, expected_cidrs, dns_ if tags is not None: tag_list = ansible_dict_to_boto3_tag_list(tags) filters = [{"Name": f"tag:{t['Key']}", "Values": [t["Value"]]} for t in tag_list] - wait_for_vpc(module, connection, waiter_name="vpc_available", VpcIds=[vpc_id], Filters=filters) + wait_for_resource_state(connection, module, "vpc_available", VpcIds=[vpc_id], Filters=filters) wait_for_vpc_attribute(connection, module, vpc_id, "enableDnsSupport", dns_support) wait_for_vpc_attribute(connection, module, vpc_id, "enableDnsHostnames", dns_hostnames) @@ -573,7 +560,7 @@ def wait_for_updates(connection, module, vpc_id, ipv6_cidr, expected_cidrs, dns_ if dhcp_id is not None: # Wait for DhcpOptionsId to be updated filters = [{"Name": "dhcp-options-id", "Values": [dhcp_id]}] - wait_for_vpc(module, connection, waiter_name="vpc_available", VpcIds=[vpc_id], Filters=filters) + wait_for_resource_state(connection, module, "vpc_available", VpcIds=[vpc_id], Filters=filters) return