From 1a4cd1d25e8f4196ef95a2fafb178540b4748377 Mon Sep 17 00:00:00 2001 From: abikouo Date: Tue, 4 Jun 2024 19:14:59 +0200 Subject: [PATCH 1/6] ec2_instance - update network interface parameters --- ...4-ec2_instance-refactor-network-option.yml | 5 + plugins/modules/ec2_instance.py | 643 ++++++++++++------ .../targets/ec2_eni/tasks/main.yaml | 4 +- .../tasks/main.yml | 24 +- .../targets/ec2_instance_network/aliases | 6 + .../ec2_instance_network/defaults/main.yml | 2 + .../ec2_instance_network/meta/main.yml | 6 + .../ec2_instance_network/tasks/enis.yml | 118 ++++ .../ec2_instance_network/tasks/failures.yml | 115 ++++ .../ec2_instance_network/tasks/main.yml | 258 +++++++ .../tasks/main.yml | 77 ++- .../ec2_metadata_facts/playbooks/setup.yml | 12 +- .../setup_ec2_instance_env/tasks/main.yml | 2 + 13 files changed, 1047 insertions(+), 225 deletions(-) create mode 100644 changelogs/fragments/20240604-ec2_instance-refactor-network-option.yml create mode 100644 tests/integration/targets/ec2_instance_network/aliases create mode 100644 tests/integration/targets/ec2_instance_network/defaults/main.yml create mode 100644 tests/integration/targets/ec2_instance_network/meta/main.yml create mode 100644 tests/integration/targets/ec2_instance_network/tasks/enis.yml create mode 100644 tests/integration/targets/ec2_instance_network/tasks/failures.yml create mode 100644 tests/integration/targets/ec2_instance_network/tasks/main.yml diff --git a/changelogs/fragments/20240604-ec2_instance-refactor-network-option.yml b/changelogs/fragments/20240604-ec2_instance-refactor-network-option.yml new file mode 100644 index 00000000000..3d46d31680f --- /dev/null +++ b/changelogs/fragments/20240604-ec2_instance-refactor-network-option.yml @@ -0,0 +1,5 @@ +--- +minor_changes: + - ec2_instance - Add support for ``network_interfaces`` and ``network_interfaces_ids`` options replacing deprecated option ``network`` (https://github.com/ansible-collections/amazon.aws/pull/2123). + - ec2_instance - ``network.source_dest_check`` option has been deprecated and replaced by new option ``source_dest_check`` (https://github.com/ansible-collections/amazon.aws/pull/2123). + - ec2_instance - add the possibility to create instance with multiple network interfaces (https://github.com/ansible-collections/amazon.aws/pull/2123). diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index b853dc544d5..c12a71289b1 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -140,6 +140,7 @@ description: - A list of security group IDs or names (strings). - Mutually exclusive with O(security_group). + - Mutually exclusive with O(network_interfaces_ids). type: list elements: str default: [] @@ -147,6 +148,7 @@ description: - A security group ID or name. - Mutually exclusive with O(security_groups). + - Mutually exclusive with O(network_interfaces_ids). type: str name: description: @@ -163,6 +165,9 @@ - Either a dictionary containing the key C(interfaces) corresponding to a list of network interface IDs or containing specifications for a single network interface. - Use the M(amazon.aws.ec2_eni) module to create ENIs with special settings. + - This field is deprecated and will be removed in a release after 2026-12-01, use O(network_interfaces) or O(network_interfaces_ids) instead. + - Mutually exclusive with O(network_interfaces). + - Mutually exclusive with O(network_interfaces_ids). type: dict suboptions: interfaces: @@ -186,6 +191,7 @@ source_dest_check: description: - Controls whether source/destination checking is enabled on the interface. + - This field with be ignored when O(source_dest_check) is provided. type: bool description: description: @@ -214,6 +220,95 @@ - A list of security group IDs to attach to the interface. type: list elements: str + source_dest_check: + description: + - Controls whether source/destination checking is enabled on the interface. + type: bool + version_added: 8.1.0 + network_interfaces: + description: + - A dictionary containing specifications for a single network interface. + - Use the M(amazon.aws.ec2_eni) module to create ENIs with special settings. + - Mutually exclusive with O(network). + type: list + elements: dict + version_added: 8.1.0 + suboptions: + assign_public_ip: + description: + - When V(true) assigns a public IP address to the interface. + type: bool + private_ip_address: + description: + - An IPv4 address to assign to the interface. + type: str + ipv6_addresses: + description: + - A list of IPv6 addresses to assign to the network interface. + type: list + elements: str + description: + description: + - A description for the network interface. + type: str + subnet_id: + description: + - The subnet to connect the network interface to. + type: str + delete_on_termination: + description: + - Delete the interface when the instance it is attached to is terminated. + type: bool + default: True + device_index: + description: + - The position of the network interface in the attachment order. + - Use device index V(0) for a primary network interface. + type: int + default: 0 + groups: + description: + - A list of security group IDs or names to attach to the interface. + type: list + elements: str + private_ip_addresses: + description: + - A list of private IPv4 addresses to assign to the network interface. + - Only one private IPv4 address can be designated as primary. + - You cannot specify this option if you're launching more than one instance. + type: list + elements: dict + suboptions: + private_ip_address: + description: + - The private IPv4 address. + type: str + required: true + primary: + description: + - Indicates whether the private IPv4 address is the primary private IPv4 address. + - Only one IPv4 address can be designated as primary. + type: bool + network_interfaces_ids: + description: + - A list of ENI ids to attach to the instance. + - Mutually exclusive with O(network). + - Mutually exclusive with O(security_group). + - Mutually exclusive with O(security_groups). + type: list + elements: dict + version_added: 8.1.0 + suboptions: + id: + description: + - The ID of the network interface. + type: str + required: True + device_index: + description: + - The position of the network interface in the attachment order. + type: int + default: 0 volumes: description: - A list of block device mappings, by default this will always use the AMI root device so the volumes option is primarily for adding more storage. @@ -454,8 +549,8 @@ vpc_subnet_id: subnet-5ca1ab1e instance_type: c5.large security_group: default - network: - assign_public_ip: true + network_interfaces: + - assign_public_ip: true image_id: ami-123456 tags: Environment: Testing @@ -510,8 +605,8 @@ tower_address: 1.2.3.4 job_template_id: 876 host_config_key: '[secret config key goes here]' - network: - assign_public_ip: true + network_interfaces: + - assign_public_ip: true image_id: ami-123456 cpu_credit_specification: unlimited tags: @@ -522,9 +617,9 @@ name: "public-eni-instance" key_name: "prod-ssh-key" vpc_subnet_id: subnet-5ca1ab1e - network: - interfaces: - - id: "eni-12345" + network_interfaces_ids: + - id: "eni-12345" + device_index: 0 tags: Env: "eni_on" volumes: @@ -537,10 +632,11 @@ - name: add second ENI interface amazon.aws.ec2_instance: name: "public-eni-instance" - network: - interfaces: + network_interfaces_ids: - id: "eni-12345" + device_index: 0 - id: "eni-67890" + device_index: 1 image_id: ami-123456 tags: Env: "eni_on" @@ -566,9 +662,10 @@ exact_count: 5 region: us-east-2 vpc_subnet_id: subnet-0123456 - network: - assign_public_ip: true - security_group: default + network_interfaces: + - assign_public_ip: true + groups: + - default tags: foo: bar @@ -579,10 +676,31 @@ image_id: ami-123456 count: 3 region: us-east-2 - network: - assign_public_ip: true - security_group: default - vpc_subnet_id: subnet-0123456 + network_interfaces: + - assign_public_ip: true + groups: + - default + subnet_id: subnet-0123456 + state: present + tags: + foo: bar + +# launches an instance with a primary and a secondary network interfaces +- name: start an instance with a primary and secondary network interfaces + amazon.aws.ec2_instance: + instance_type: t2.large + image_id: ami-123456 + region: us-east-2 + network_interfaces: + - assign_public_ip: true + groups: + - default + subnet_id: subnet-0123456 + private_ip_addresses: + - primary: true + private_ip_address: 168.50.4.239 + - primary: false + private_ip_address: 168.50.4.237 state: present tags: foo: bar @@ -1190,6 +1308,11 @@ import time import uuid from collections import namedtuple +from typing import Any +from typing import Dict +from typing import List +from typing import Optional +from typing import Union try: import botocore @@ -1284,122 +1407,164 @@ def add_or_update_instance_profile(instance, desired_profile_name): return False -def build_network_spec(params): +def validate_assign_public_ip(params: Dict[str, Any]) -> None: """ - Returns list of interfaces [complex] - Interface type: { - 'AssociatePublicIpAddress': True|False, - 'DeleteOnTermination': True|False, - 'Description': 'string', - 'DeviceIndex': 123, - 'Groups': [ - 'string', - ], - 'Ipv6AddressCount': 123, - 'Ipv6Addresses': [ - { - 'Ipv6Address': 'string' - }, - ], - 'NetworkInterfaceId': 'string', - 'PrivateIpAddress': 'string', - 'PrivateIpAddresses': [ - { - 'Primary': True|False, - 'PrivateIpAddress': 'string' - }, - ], - 'SecondaryPrivateIpAddressCount': 123, - 'SubnetId': 'string' - }, + Validate Network interface public IP configuration + Parameters: + params: module parameters. """ + network_interfaces = params.get("network_interfaces") or [] + network_interfaces_ids = params.get("network_interfaces_ids") or [] + if len(network_interfaces + network_interfaces_ids) > 1 and any( + i.get("assign_public_ip") for i in network_interfaces if i.get("assign_public_ip") is not None + ): + module.fail_json(msg="The option 'assign_public_ip' cannot be set to true with multiple network interfaces.") - interfaces = [] - network = params.get("network") or {} - if not network.get("interfaces"): - # they only specified one interface - spec = { - "DeviceIndex": 0, - } - if network.get("assign_public_ip") is not None: - spec["AssociatePublicIpAddress"] = network["assign_public_ip"] - - if params.get("vpc_subnet_id"): - spec["SubnetId"] = params["vpc_subnet_id"] - else: - default_vpc = get_default_vpc() - if default_vpc is None: + +def validate_network_params(params: Dict[str, Any], nb_instances: int) -> None: + """ + This function is used to validate network specifications with the following constraints + - 'assign_public_ip' cannot set to True when multiple network interfaces are specified + - 'private_ip_addresses' only one IP can be set as primary + - 'private_ip_addresses' or 'private_ip_address' can't be specified if launching more than one instance + Parameters: + params: module parameters. + nb_instances: number of instance to create. + """ + validate_assign_public_ip(params) + + network_interfaces = params.get("network_interfaces") + if network_interfaces: + # private_ip_addresses: ensure only one private ip is set as primary + for inty in network_interfaces: + if len([i for i in inty.get("private_ip_addresses") or [] if i.get("primary")]) > 1: module.fail_json( - msg=( - "No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter)" - " to create an instance" - ) + msg="Only one primary private IP address can be specified when creating network interface." ) - else: - sub = get_default_subnet(default_vpc, availability_zone=module.params.get("availability_zone")) - spec["SubnetId"] = sub["SubnetId"] - - if network.get("ipv6_addresses"): - spec["Ipv6Addresses"] = [{"Ipv6Address": a} for a in network.get("ipv6_addresses", [])] - if network.get("private_ip_address"): - spec["PrivateIpAddress"] = network["private_ip_address"] + # Ensure none of 'private_ip_address', 'private_ip_addresses', 'ipv6_addresses' were provided + # when launching more than one instance + if nb_instances > 1: + if any(True for inty in network_interfaces if inty.get("private_ip_address")): + module.fail_json( + msg="The option 'private_ip_address' cannot be specified when launching more than one instance." + ) + if any(True for inty in network_interfaces if inty.get("private_ip_addresses")): + module.fail_json( + msg="The option 'private_ip_addresses' cannot be specified when launching more than one instance." + ) + if any(True for inty in network_interfaces if inty.get("ipv6_addresses")): + module.fail_json( + msg="The option 'ipv6_addresses' cannot be specified when launching more than one instance." + ) - if params.get("security_group") or params.get("security_groups"): - groups = discover_security_groups( - group=params.get("security_group"), - groups=params.get("security_groups"), - subnet_id=spec["SubnetId"], - ) - spec["Groups"] = groups - if network.get("description") is not None: - spec["Description"] = network["description"] - # TODO more special snowflake network things - - return [spec] - - # handle list of `network.interfaces` options - for idx, interface_params in enumerate(network.get("interfaces", [])): - spec = { - "DeviceIndex": idx, - } - - if isinstance(interface_params, string_types): - # naive case where user gave - # network_interfaces: [eni-1234, eni-4567, ....] - # put into normal data structure so we don't dupe code - interface_params = {"id": interface_params} - - if interface_params.get("id") is not None: - # if an ID is provided, we don't want to set any other parameters. - spec["NetworkInterfaceId"] = interface_params["id"] - interfaces.append(spec) - continue - spec["DeleteOnTermination"] = interface_params.get("delete_on_termination", True) +def ansible_to_boto3_eni_specification( + interface: Dict[str, Any], subnet_id: Optional[str], groups: Optional[Union[List[str], str]] +) -> Dict[str, Any]: + """ + Converts Ansible network interface specifications into AWS network interface spec + Parameters: + interface: Ansible network interface specification + subnet_id: Subnet Id + security_groups: Optional list of security groups. + security_group: Optional security group. + Returns: + AWS network interface specification. + """ + spec = { + "DeviceIndex": interface.get("device_index") or 0, + } + if interface.get("assign_public_ip") is not None: + spec["AssociatePublicIpAddress"] = interface["assign_public_ip"] - if interface_params.get("ipv6_addresses"): - spec["Ipv6Addresses"] = [{"Ipv6Address": a} for a in interface_params.get("ipv6_addresses", [])] + if interface.get("subnet_id"): + spec["SubnetId"] = interface["subnet_id"] + elif subnet_id: + spec["SubnetId"] = subnet_id + else: + spec["SubnetId"] = describe_default_subnet(module) + + if interface.get("ipv6_addresses"): + spec["Ipv6Addresses"] = [{"Ipv6Address": a} for a in interface.get("ipv6_addresses")] + + if interface.get("private_ip_address"): + spec["PrivateIpAddress"] = interface["private_ip_address"] + + if interface.get("private_ip_addresses"): + spec["PrivateIpAddresses"] = [] + for addr in interface.get("private_ip_addresses"): + d = {"PrivateIpAddress": addr} + if isinstance(addr, dict): + d = {"PrivateIpAddress": addr.get("private_ip_address")} + if addr.get("primary") is not None: + d.update({"Primary": addr.get("primary")}) + spec["PrivateIpAddresses"].append(d) + + spec["DeleteOnTermination"] = interface.get("delete_on_termination", True) + + interface_groups = interface.get("groups") or groups + if interface_groups: + spec["Groups"] = discover_security_groups(groups=interface_groups, subnet_id=spec["SubnetId"]) + if interface.get("description") is not None: + spec["Description"] = interface["description"] + return spec - if interface_params.get("private_ip_address"): - spec["PrivateIpAddress"] = interface_params.get("private_ip_address") - if interface_params.get("description"): - spec["Description"] = interface_params.get("description") +def build_network_spec(params: Dict[str, Any]) -> List[Dict[str, Any]]: + """ + Returns network interface specifications for instance to be created. + Parameters: + params: module parameters + Returns: + network specs (list): List of network interfaces specifications + """ - if interface_params.get("subnet_id", params.get("vpc_subnet_id")): - spec["SubnetId"] = interface_params.get("subnet_id", params.get("vpc_subnet_id")) - elif not spec.get("SubnetId") and not interface_params["id"]: - # TODO grab a subnet from default VPC - raise ValueError(f"Failed to assign subnet to interface {interface_params}") + # They specified network_interfaces_ids (mutually exclusive with security_group(s) options) + interfaces = [] + network_interfaces_ids = params.get("network_interfaces_ids") + if network_interfaces_ids: + interfaces = [ + {"NetworkInterfaceId": eni.get("id"), "DeviceIndex": eni.get("device_index")} + for eni in network_interfaces_ids + ] - interfaces.append(spec) + network = params.get("network") + groups = params.get("security_groups") or params.get("security_group") + vpc_subnet_id = params.get("vpc_subnet_id") + network_interfaces = params.get("network_interfaces") + if (network is not None and not network.get("interfaces")) or network_interfaces: + # They specified network interfaces using `network` or `network_interfaces` options + if network is not None and not network.get("interfaces"): + network_interfaces = [network] + interfaces.extend( + [ansible_to_boto3_eni_specification(inty, vpc_subnet_id, groups) for inty in network_interfaces] + ) + elif not network and not network_interfaces_ids: + # They do not specified any network interface configuration + # build network interface using subnet_id and security group(s) defined on the module + interfaces.append(ansible_to_boto3_eni_specification({}, vpc_subnet_id, groups)) + elif network is not None: + # handle list of `network.interfaces` options + interfaces.extend( + [ + {"DeviceIndex": idx, "NetworkInterfaceId": inty if isinstance(inty, string_types) else inty.get("id")} + for idx, inty in enumerate(network.get("interfaces", [])) + ] + ) return interfaces -def warn_if_public_ip_assignment_changed(instance): +def warn_if_public_ip_assignment_changed(instance: Dict[str, Any]) -> None: # This is a non-modifiable attribute. assign_public_ip = (module.params.get("network") or {}).get("assign_public_ip") + validate_assign_public_ip(module.params) + network_interfaces = module.params.get("network_interfaces") + if network_interfaces: + # Either we only have one network interface or multiple network interface with 'assign_public_ip=false' + # Anyways the value 'assign_public_ip' in the first item should be enough to determine whether + # the user wants to update the public IP or not + assign_public_ip = network_interfaces[0].get("assign_public_ip") if assign_public_ip is None: return @@ -1436,7 +1601,9 @@ def warn_if_cpu_options_changed(instance): ) -def discover_security_groups(group, groups, parent_vpc_id=None, subnet_id=None): +def discover_security_groups( + groups: Union[str, List[str]], parent_vpc_id: Optional[str] = None, subnet_id: Optional[str] = None +) -> List[str]: if subnet_id is not None: try: sub = client.describe_subnets(aws_retry=True, SubnetIds=[subnet_id]) @@ -1452,11 +1619,7 @@ def discover_security_groups(group, groups, parent_vpc_id=None, subnet_id=None): module.fail_json_aws(e, msg=f"Error while searching for subnet {subnet_id} parent VPC.") parent_vpc_id = sub["Subnets"][0]["VpcId"] - if group: - return get_ec2_security_group_ids_from_names(group, client, vpc_id=parent_vpc_id) - if groups: - return get_ec2_security_group_ids_from_names(groups, client, vpc_id=parent_vpc_id) - return [] + return get_ec2_security_group_ids_from_names(groups, client, vpc_id=parent_vpc_id) def build_userdata(params): @@ -1591,11 +1754,19 @@ def build_instance_tags(params): def build_run_instance_spec(params, current_count=0): spec = dict( ClientToken=uuid.uuid4().hex, - MaxCount=1, - MinCount=1, ) spec.update(**build_top_level_options(params)) + nb_instances = params.get("count") or 1 + if params.get("exact_count"): + nb_instances = params.get("exact_count") - current_count + + spec["MinCount"] = nb_instances + spec["MaxCount"] = nb_instances + + # Validate network parameters + validate_network_params(params, nb_instances) + spec["NetworkInterfaces"] = build_network_spec(params) spec["BlockDeviceMappings"] = build_volume_spec(params) @@ -1607,14 +1778,6 @@ def build_run_instance_spec(params, current_count=0): if params.get("iam_instance_profile"): spec["IamInstanceProfile"] = dict(Arn=determine_iam_role(params.get("iam_instance_profile"))) - if params.get("exact_count"): - spec["MaxCount"] = params.get("exact_count") - current_count - spec["MinCount"] = params.get("exact_count") - current_count - - if params.get("count"): - spec["MaxCount"] = params.get("count") - spec["MinCount"] = params.get("count") - if params.get("instance_type"): spec["InstanceType"] = params["instance_type"] @@ -1707,47 +1870,50 @@ def value_wrapper(v): arguments[mapping.instance_key] = mapping.add_value(params.get(mapping.param_key)) changes_to_apply.append(arguments) - if params.get("security_group") or params.get("security_groups"): - try: - value = client.describe_instance_attribute(aws_retry=True, Attribute="groupSet", InstanceId=id_) - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: - module.fail_json_aws(e, msg=f"Could not describe attribute groupSet for instance {id_}") - # managing security groups - if params.get("vpc_subnet_id"): - subnet_id = params.get("vpc_subnet_id") + network_interfaces = params.get("network_interfaces") + if not network_interfaces and params.get("network") and not params.get("network").get("interfaces"): + network_interfaces = [params.get("network")] + if network_interfaces or params.get("security_groups") or params.get("security_group"): + if len(network_interfaces or []) > 1 or len(instance["NetworkInterfaces"]) > 1: + module.warn("Skipping group modification because instance contains mutiple network interfaces.") else: - default_vpc = get_default_vpc() - if default_vpc is None: - module.fail_json( - msg=( - "No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter)" - " to modify security groups." - ) - ) + try: + value = client.describe_instance_attribute(aws_retry=True, Attribute="groupSet", InstanceId=id_) + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + module.fail_json_aws(e, msg=f"Could not describe attribute groupSet for instance {id_}") + + # Read interface subnet + subnet_id = None + groups = None + if network_interfaces: + subnet_id = network_interfaces[0].get("subnet_id") + groups = network_interfaces[0].get("groups") + elif params.get("vpc_subnet_id"): + subnet_id = params.get("vpc_subnet_id") else: - sub = get_default_subnet(default_vpc) - subnet_id = sub["SubnetId"] - - groups = discover_security_groups( - group=params.get("security_group"), - groups=params.get("security_groups"), - subnet_id=subnet_id, - ) - expected_groups = groups - instance_groups = [g["GroupId"] for g in value["Groups"]] - if set(instance_groups) != set(expected_groups): - changes_to_apply.append(dict(Groups=expected_groups, InstanceId=instance["InstanceId"])) - - if (params.get("network") or {}).get("source_dest_check") is not None: + subnet_id = describe_default_subnet(module=module, use_availability_zone=False) + + # Read groups + groups = groups or params.get("security_groups") or params.get("security_group") + if groups: + groups = discover_security_groups(groups=groups, subnet_id=subnet_id) + instance_groups = [g["GroupId"] for g in value["Groups"]] + if set(instance_groups) != set(groups): + changes_to_apply.append(dict(Groups=groups, InstanceId=instance["InstanceId"])) + + source_dest_check = params.get("source_dest_check") + if source_dest_check is None: + source_dest_check = (params.get("network") or {}).get("source_dest_check") # network.source_dest_check is nested, so needs to be treated separately - check = bool(params.get("network").get("source_dest_check")) - if instance["SourceDestCheck"] != check: - changes_to_apply.append( - dict( - InstanceId=instance["InstanceId"], - SourceDestCheck={"Value": check}, - ) + if source_dest_check is not None: + source_dest_check = bool(source_dest_check) + if source_dest_check is not None and instance["SourceDestCheck"] != source_dest_check: + changes_to_apply.append( + dict( + InstanceId=instance["InstanceId"], + SourceDestCheck={"Value": source_dest_check}, ) + ) return changes_to_apply @@ -1792,23 +1958,37 @@ def change_instance_metadata_options(instance, params): return True -def change_network_attachments(instance, params): - if (params.get("network") or {}).get("interfaces") is not None: - new_ids = [] - for inty in params.get("network").get("interfaces"): - if isinstance(inty, dict) and "id" in inty: - new_ids.append(inty["id"]) - elif isinstance(inty, string_types): - new_ids.append(inty) - # network.interfaces can create the need to attach new interfaces - old_ids = [inty["NetworkInterfaceId"] for inty in instance["NetworkInterfaces"]] - to_attach = set(new_ids) - set(old_ids) - if not module.check_mode: - for eni_id in to_attach: +def change_network_attachments(instance: Dict[str, Any], params: Dict[str, Any]) -> bool: + """ + Attach Network interfaces to the instance + Parameters: + instance: A dictionnary describing the instance. + params: Ansible module parameters. + Returns: + A boolean set to True if changes have been done. + """ + new_enis = [ + eni.get("id") + for eni in params.get("network_interfaces_ids") or (params.get("network") or {}).get("interfaces") or [] + ] + if not new_enis: + return False + + new_ids = map(lambda x: x.get("id") if isinstance(x, dict) else x, new_enis) + old_ids = [inty["NetworkInterfaceId"] for inty in instance["NetworkInterfaces"]] + to_attach = set(new_ids) - set(old_ids) + if not module.check_mode: + for idx, eni in enumerate(new_enis): + eni_id = eni + device_index = idx + if isinstance(eni, dict): + eni_id = eni.get("id") + device_index = eni.get("device_index") or idx + if eni_id in to_attach: try: client.attach_network_interface( aws_retry=True, - DeviceIndex=new_ids.index(eni_id), + DeviceIndex=device_index, InstanceId=instance["InstanceId"], NetworkInterfaceId=eni_id, ) @@ -1816,8 +1996,7 @@ def change_network_attachments(instance, params): module.fail_json_aws( e, msg=f"Could not attach interface {eni_id} to instance {instance['InstanceId']}" ) - return bool(len(to_attach)) - return False + return bool(to_attach) def find_instances(ids=None, filters=None): @@ -2308,6 +2487,45 @@ def run_instances(**instance_spec): return client.run_instances(aws_retry=True, **instance_spec) +def describe_default_subnet(module: AnsibleAWSModule, use_availability_zone: bool = True) -> str: + """ + Read default subnet associated to the AWS account + Parameters: + module: The Ansible AWS module + use_availability_zone: Whether to use availability zone to find the default subnet. + Returns: + The default subnet id. + """ + default_vpc = get_default_vpc() + if default_vpc is None: + module.fail_json( + msg=("No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter).") + ) + availability_zone = module.params.get("availability_zone") if use_availability_zone else None + return get_default_subnet(default_vpc, availability_zone)["SubnetId"] + + +def build_network_filters() -> Dict[str, List[str]]: + """ + Create Network filters for the DescribeInstances API + Returns: + Dictionnary of network filters + """ + filters = {} + network_interfaces_ids = module.params.get("network_interfaces_ids") or (module.params.get("network") or {}).get( + "interfaces" + ) + if module.params.get("vpc_subnet_id"): + filters["subnet-id"] = [module.params.get("vpc_subnet_id")] + elif network_interfaces_ids: + filters["network-interface.network-interface-id"] = [ + eni["id"] if isinstance(eni, dict) else eni for eni in network_interfaces_ids + ] + else: + filters["subnet-id"] = describe_default_subnet(module) + return filters + + def build_filters(): filters = { # all states except shutting-down and terminated @@ -2318,22 +2536,8 @@ def build_filters(): elif isinstance(module.params.get("instance_ids"), list) and len(module.params.get("instance_ids")): filters["instance-id"] = module.params.get("instance_ids") else: - if not module.params.get("vpc_subnet_id"): - if module.params.get("network"): - # grab AZ from one of the ENIs - ints = module.params.get("network").get("interfaces") - if ints: - filters["network-interface.network-interface-id"] = [] - for i in ints: - if isinstance(i, dict): - i = i["id"] - filters["network-interface.network-interface-id"].append(i) - else: - sub = get_default_subnet(get_default_vpc(), availability_zone=module.params.get("availability_zone")) - filters["subnet-id"] = sub["SubnetId"] - else: - filters["subnet-id"] = [module.params.get("vpc_subnet_id")] - + # Network filters + filters.update(build_network_filters()) if module.params.get("name"): filters["tag:Name"] = [module.params.get("name")] elif module.params.get("tags"): @@ -2447,6 +2651,34 @@ def main(): ), ), additional_info=dict(type="str"), + network_interfaces_ids=dict( + type="list", + elements="dict", + options=dict( + id=dict(required=True), + device_index=dict(type="int", default=0), + ), + ), + network_interfaces=dict( + type="list", + elements="dict", + options=dict( + assign_public_ip=dict(type="bool"), + private_ip_address=dict(), + ipv6_addresses=dict(type="list", elements="str"), + description=dict(), + private_ip_addresses=dict( + type="list", + elements="dict", + options=dict(private_ip_address=dict(required=True), primary=dict(type="bool")), + ), + subnet_id=dict(), + delete_on_termination=dict(type="bool", default=True), + device_index=dict(type="int", default=0), + groups=dict(type="list", elements="str"), + ), + ), + source_dest_check=dict(type="bool"), ) # running/present are synonyms # as are terminated/absent @@ -2461,6 +2693,10 @@ def main(): ["exact_count", "instance_ids"], ["tenancy", "placement"], ["placement_group", "placement"], + ["network", "network_interfaces"], + ["network", "network_interfaces_ids"], + ["security_group", "network_interfaces_ids"], + ["security_groups", "network_interfaces_ids"], ], supports_check_mode=True, ) @@ -2468,11 +2704,20 @@ def main(): result = dict() if module.params.get("network"): + module.deprecate( + "The network parameter has been deprecated, please use network_interfaces and/or network_interfaces_ids instead.", + date="2026-12-01", + collection_name="amazon.aws", + ) if module.params.get("network").get("interfaces"): if module.params.get("security_group"): module.fail_json(msg="Parameter network.interfaces can't be used with security_group") if module.params.get("security_groups"): module.fail_json(msg="Parameter network.interfaces can't be used with security_groups") + if module.params.get("network").get("source_dest_check") is not None and module.params.get("source_dest_check"): + module.warn( + "the source_dest_check option has been set therefore network.source_dest_check will be ignored." + ) if module.params.get("placement_group"): module.deprecate( diff --git a/tests/integration/targets/ec2_eni/tasks/main.yaml b/tests/integration/targets/ec2_eni/tasks/main.yaml index 450a2a75df9..40bb443a8b9 100644 --- a/tests/integration/targets/ec2_eni/tasks/main.yaml +++ b/tests/integration/targets/ec2_eni/tasks/main.yaml @@ -59,8 +59,8 @@ instance_type: t2.micro wait: false security_group: "{{ vpc_sg_id }}" - network: - private_ip_address: "{{ ec2_ips[item] }}" + network_interfaces: + - private_ip_address: "{{ ec2_ips[item] }}" register: ec2_instances loop: - 0 diff --git a/tests/integration/targets/ec2_instance_external_resource_attach/tasks/main.yml b/tests/integration/targets/ec2_instance_external_resource_attach/tasks/main.yml index 1cfe2cc85c8..24195e63f36 100644 --- a/tests/integration/targets/ec2_instance_external_resource_attach/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_external_resource_attach/tasks/main.yml @@ -39,9 +39,8 @@ state: present name: "{{ resource_prefix }}-test-eni-vpc" key_name: "{{ resource_prefix }}_test_key" - network: - interfaces: - - id: "{{ eni_a.interface.id }}" + network_interfaces_ids: + - id: "{{ eni_a.interface.id }}" image_id: "{{ ec2_ami_id }}" availability_zone: "{{ subnet_b_az }}" tags: @@ -65,10 +64,9 @@ amazon.aws.ec2_instance: state: present name: "{{ resource_prefix }}-test-eni-vpc" - network: - interfaces: - - id: "{{ eni_a.interface.id }}" - - id: "{{ eni_b.interface.id }}" + network_interfaces_ids: + - id: "{{ eni_a.interface.id }}" + - id: "{{ eni_b.interface.id }}" image_id: "{{ ec2_ami_id }}" tags: TestId: "{{ ec2_instance_tag_TestId }}" @@ -98,10 +96,9 @@ amazon.aws.ec2_instance: state: present name: "{{ resource_prefix }}-test-eni-vpc" - network: - interfaces: - - id: "{{ eni_a.interface.id }}" - - id: "{{ eni_b.interface.id }}" + network_interfaces_ids: + - id: "{{ eni_a.interface.id }}" + - id: "{{ eni_b.interface.id }}" image_id: "{{ ec2_ami_id }}" tags: TestId: "{{ ec2_instance_tag_TestId }}" @@ -133,9 +130,8 @@ state: present name: "{{ resource_prefix }}-test-eni-vpc-checkmode" key_name: "{{ resource_prefix }}_test_key" - network: - interfaces: - - id: "{{ eni_c.interface.id }}" + network_interfaces_ids: + - id: "{{ eni_c.interface.id }}" image_id: "{{ ec2_ami_id }}" availability_zone: "{{ subnet_b_az }}" tags: diff --git a/tests/integration/targets/ec2_instance_network/aliases b/tests/integration/targets/ec2_instance_network/aliases new file mode 100644 index 00000000000..daa489fdab9 --- /dev/null +++ b/tests/integration/targets/ec2_instance_network/aliases @@ -0,0 +1,6 @@ +time=xxm + +cloud/aws + +ec2_instance_info +ec2_instance diff --git a/tests/integration/targets/ec2_instance_network/defaults/main.yml b/tests/integration/targets/ec2_instance_network/defaults/main.yml new file mode 100644 index 00000000000..464199033f6 --- /dev/null +++ b/tests/integration/targets/ec2_instance_network/defaults/main.yml @@ -0,0 +1,2 @@ +--- +ec2_instance_name: "{{ resource_prefix }}-ec2-instance-networking" diff --git a/tests/integration/targets/ec2_instance_network/meta/main.yml b/tests/integration/targets/ec2_instance_network/meta/main.yml new file mode 100644 index 00000000000..5cfaf566c4c --- /dev/null +++ b/tests/integration/targets/ec2_instance_network/meta/main.yml @@ -0,0 +1,6 @@ +--- +dependencies: + - role: setup_ec2_facts + - role: setup_ec2_instance_env + vars: + ec2_instance_test_name: networking diff --git a/tests/integration/targets/ec2_instance_network/tasks/enis.yml b/tests/integration/targets/ec2_instance_network/tasks/enis.yml new file mode 100644 index 00000000000..8bdcee66257 --- /dev/null +++ b/tests/integration/targets/ec2_instance_network/tasks/enis.yml @@ -0,0 +1,118 @@ +--- +- name: Create ENI a + amazon.aws.ec2_eni: + name: "{{ resource_prefix }}-eni-a" + delete_on_termination: true + subnet_id: "{{ testing_subnet_a.subnet.id }}" + security_groups: + - "{{ sg.group_id }}" + register: eni_a + +- name: Create ENI b + amazon.aws.ec2_eni: + name: "{{ resource_prefix }}-eni-b" + delete_on_termination: true + subnet_id: "{{ testing_subnet_a.subnet.id }}" + security_groups: + - "{{ sg.group_id }}" + register: eni_b + +- name: Make instance with 1 existing ENI + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}-a" + image_id: "{{ ec2_ami_id }}" + network_interfaces_ids: + - id: "{{ eni_a.interface.id }}" + device_index: 0 + instance_type: "t2.micro" + wait: false + register: create_instance + +- name: Set instance Id + ansible.builtin.set_fact: + ec2_instance_id: "{{ create_instance.instance_ids }}" + +- name: Get instance info + ec2_instance_info: + instance_ids: "{{ ec2_instance_id }}" + register: _instances + +- name: Ensure instance has one ENI attached + assert: + that: + - _instances.instances.0.network_interfaces | map(attribute='network_interface_id') | list == [eni_a.interface.id] + - _instances.instances.0.network_interfaces.0.attachment.device_index == 0 + +- name: Attach a new ENI to existing instance + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}-a" + image_id: "{{ ec2_ami_id }}" + network_interfaces_ids: + - id: "{{ eni_a.interface.id }}" + device_index: 0 + - id: "{{ eni_b.interface.id }}" + device_index: 1 + instance_type: "t2.micro" + wait: false + register: attach_eni + +- name: Get instance info + ec2_instance_info: + instance_ids: "{{ ec2_instance_id }}" + register: _instances + +- name: Ensure instance has 2 ENIs attached + assert: + that: + - attach_eni is changed + - _instances.instances.0.network_interfaces | length == 2 + - _instances.instances.0.network_interfaces | selectattr('attachment.device_index', 'equalto', 0) | map(attribute='network_interface_id') | list == [eni_a.interface.id] + - _instances.instances.0.network_interfaces | selectattr('attachment.device_index', 'equalto', 1) | map(attribute='network_interface_id') | list == [eni_b.interface.id] + +- name: Attach a new ENI to existing instance once again (idempotency) + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}-a" + image_id: "{{ ec2_ami_id }}" + network_interfaces_ids: + - id: "{{ eni_a.interface.id }}" + device_index: 0 + - id: "{{ eni_b.interface.id }}" + device_index: 1 + instance_type: "t2.micro" + wait: false + register: attach_idempotency + +- name: Ensure module did not reported change + ansible.builtin.assert: + that: + - attach_idempotency is not changed + +- name: Detach one ENI from the instance (should not report change) + amazon.aws.ec2_instance: + state: present + name: "{{ ec2_instance_name }}-a" + image_id: "{{ ec2_ami_id }}" + network_interfaces_ids: + - id: "{{ eni_a.interface.id }}" + device_index: 0 + - id: "{{ eni_b.interface.id }}" + device_index: 1 + instance_type: "t2.micro" + wait: false + register: detach_eni + +- name: Get instance info + ec2_instance_info: + instance_ids: "{{ ec2_instance_id }}" + register: _instances + +- name: Ensure instance has 2 ENIs attached + assert: + that: + - detach_eni is not changed + - _instances.instances.0.network_interfaces | length == 2 + - _instances.instances.0.network_interfaces | selectattr('attachment.device_index', 'equalto', 0) | map(attribute='network_interface_id') | list == [eni_a.interface.id] + - _instances.instances.0.network_interfaces | selectattr('attachment.device_index', 'equalto', 1) | map(attribute='network_interface_id') | list == [eni_b.interface.id] diff --git a/tests/integration/targets/ec2_instance_network/tasks/failures.yml b/tests/integration/targets/ec2_instance_network/tasks/failures.yml new file mode 100644 index 00000000000..0a2d23f27eb --- /dev/null +++ b/tests/integration/targets/ec2_instance_network/tasks/failures.yml @@ -0,0 +1,115 @@ +--- +# Instance with multiple network interfaces with 'assign_public_ip=true' +- name: Make instance with multiple network interfaces with 'assign_public_ip=true' + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + network_interfaces: + - assign_public_ip: true + subnet_id: "{{ testing_subnet_a.subnet.id }}" + device_index: 0 + - assign_public_ip: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + device_index: 1 + wait: false + register: make_instance + ignore_errors: true + +- name: Ensure module failed with proper message + ansible.builtin.assert: + that: + - make_instance is failed + - make_instance.msg == "The option 'assign_public_ip' cannot be set to true with multiple network interfaces." + + +# Network interface with mutiple primary private ip address +- name: Make instance with network interfaces with mutiple primary private ip address + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + network_interfaces: + - assign_public_ip: true + subnet_id: "{{ testing_subnet_a.subnet.id }}" + private_ip_addresses: + - primary: true + private_ip_address: 168.50.4.239 + - primary: true + private_ip_address: 168.50.4.237 + - primary: false + private_ip_address: 168.50.4.238 + wait: false + register: make_instance + ignore_errors: true + +- name: Ensure module failed with proper message + ansible.builtin.assert: + that: + - make_instance is failed + - make_instance.msg == "Only one primary private IP address can be specified when creating network interface." + +# Mutiple instances with private_ip_addresses specified +- name: Make multiple instances with private_ip_addresses provided + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + count: 2 + network_interfaces: + - assign_public_ip: true + subnet_id: "{{ testing_subnet_a.subnet.id }}" + private_ip_addresses: + - private_ip_address: 168.50.4.239 + wait: false + register: make_instance + ignore_errors: true + +- name: Ensure module failed with proper message + ansible.builtin.assert: + that: + - make_instance is failed + - make_instance.msg == "The option 'private_ip_addresses' cannot be specified when launching more than one instance." + +# Mutiple instances with private_ip_address specified +- name: Make multiple instances with private_ip_address provided + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + count: 2 + network_interfaces: + - assign_public_ip: true + subnet_id: "{{ testing_subnet_a.subnet.id }}" + private_ip_address: 168.50.4.239 + wait: false + register: make_instance + ignore_errors: true + +- name: Ensure module failed with proper message + ansible.builtin.assert: + that: + - make_instance is failed + - make_instance.msg == "The option 'private_ip_address' cannot be specified when launching more than one instance." + +# Mutiple instances with ipv6_addresses specified +- name: Make multiple instances with ipv6_addresses provided + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + count: 2 + network_interfaces: + - assign_public_ip: true + subnet_id: "{{ testing_subnet_a.subnet.id }}" + ipv6_addresses: + - "{{ testing_vpc.vpc.ipv6_cidr_block_association_set.0.ipv6_cidr_block | replace('/56', '122') }}" + wait: false + register: make_instance + ignore_errors: true + +- name: Ensure module failed with proper message + ansible.builtin.assert: + that: + - make_instance is failed + - make_instance.msg == "The option 'ipv6_addresses' cannot be specified when launching more than one instance." \ No newline at end of file diff --git a/tests/integration/targets/ec2_instance_network/tasks/main.yml b/tests/integration/targets/ec2_instance_network/tasks/main.yml new file mode 100644 index 00000000000..fcedd03fae7 --- /dev/null +++ b/tests/integration/targets/ec2_instance_network/tasks/main.yml @@ -0,0 +1,258 @@ +--- +- module_defaults: + group/aws: + access_key: "{{ aws_access_key }}" + secret_key: "{{ aws_secret_key }}" + session_token: "{{ security_token | default(omit) }}" + region: "{{ aws_region }}" + block: + - ansible.builtin.include_tasks: failures.yml + - ansible.builtin.include_tasks: enis.yml + + - name: Make instance with multiple private ip addresses + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-multiple-addresses" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + network_interfaces: + - assign_public_ip: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + private_ip_addresses: + - private_ip_address: "{{ subnet_a_startswith }}120" + primary: true + - private_ip_address: "{{ subnet_a_startswith }}121" + primary: false + register: multiple_ips + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ multiple_ips.instance_ids }}" + register: _instances + + - name: Ensure instance has ENI attached with 2 Private IP addresses + assert: + that: + - multiple_ips is changed + - _instances.instances.0.network_interfaces | length == 1 + - _instances.instances.0.network_interfaces.0.private_ip_addresses | length == 2 + - _instances.instances.0.network_interfaces.0.private_ip_addresses | selectattr('primary', 'equalto', true) | map(attribute='private_ip_address') | list | first == subnet_a_startswith+'120' + - _instances.instances.0.network_interfaces.0.private_ip_addresses | selectattr('primary', 'equalto', false) | map(attribute='private_ip_address') | list | first == subnet_a_startswith+'121' + + + - name: Make instance with single private ip addresses + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-single-address" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + network_interfaces: + - assign_public_ip: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + private_ip_address: "{{ subnet_a_startswith }}122" + register: single_ip + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ single_ip.instance_ids }}" + register: _instances + + - name: Ensure instance has one ENI attached to it + assert: + that: + - multiple_ips is changed + - _instances.instances.0.network_interfaces | length == 1 + - _instances.instances.0.network_interfaces.0.private_ip_addresses | length == 1 + - _instances.instances.0.network_interfaces.0.private_ip_addresses.0.private_ip_address == subnet_a_startswith+'122' + + - ansible.builtin.set_fact: + instance_ipv6_addr: "{{ testing_vpc.vpc.ipv6_cidr_block_association_set.0.ipv6_cidr_block | replace('/56', '125') }}" + + - name: Make instance with IPv6 address + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-ipv6" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + network_interfaces: + - assign_public_ip: false + delete_on_termination: true + device_index: 0 + subnet_id: "{{ testing_subnet_a.subnet.id }}" + ipv6_addresses: + - "{{ instance_ipv6_addr }}" + register: ipv6_instance + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ ipv6_instance.instance_ids }}" + register: _instances + + - name: Ensure instance has one ENI attached to it + assert: + that: + - ipv6_instance is changed + - _instances.instances.0.network_interfaces | length == 1 + - _instances.instances.0.network_interfaces.0.ipv6_addresses | length == 1 + - _instances.instances.0.network_interfaces.0.ipv6_addresses.0.ipv6_address == instance_ipv6_addr + + # Testing instance security groups + - name: Make instance with 1 Security Group + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-group" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + network_interfaces: + - assign_public_ip: false + groups: + - "{{ security_group_name_1 }}" + register: create_with_group + + - ansible.builtin.set_fact: + created_instance_id: "{{ create_with_group.instance_ids }}" + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ created_instance_id }}" + register: _instances + + - name: Ensure instance has been created with one security group + assert: + that: + - create_with_group is changed + - _instances.instances.0.security_groups | map(attribute='group_name') | list == [security_group_name_1] + + - name: Update instance Security Group from network interface (check_mode=true) + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-group" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + network_interfaces: + - assign_public_ip: false + groups: + - "{{ security_group_name_1 }}" + - "{{ sg2.group_id }}" + register: add_group_checkmode + check_mode: true + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ created_instance_id }}" + register: _instances + + - name: Ensure instance has still one Security group + assert: + that: + - add_group_checkmode is changed + - _instances.instances.0.security_groups | map(attribute='group_name') | list == [security_group_name_1] + + - name: Update instance Security Group from network interface + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-group" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + network_interfaces: + - assign_public_ip: false + groups: + - "{{ security_group_name_1 }}" + - "{{ sg2.group_id }}" + register: add_group + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ created_instance_id }}" + register: _instances + + - name: Ensure instance has 1 additional Security group + assert: + that: + - add_group is changed + - _instances.instances.0.security_groups | length == 2 + - security_group_name_1 in _instances.instances.0.security_groups | map(attribute='group_name') | list + - security_group_name_2 in _instances.instances.0.security_groups | map(attribute='group_name') | list + + - name: Update instance Security Group from network interface (idempotency) + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-group" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + network_interfaces: + - assign_public_ip: false + groups: + - "{{ security_group_name_1 }}" + - "{{ sg2.group_id }}" + register: add_group_idempotency + + - name: Ensure no change was made on the instance + assert: + that: + - add_group_idempotency is not changed + + - name: Remove one security group from the instance (check_mode=true) + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-group" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + security_group: "{{ security_group_name_2 }}" + register: remove_group_checkmode + check_mode: true + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ created_instance_id }}" + register: _instances + + - name: Ensure instance has still 2 Security groups + assert: + that: + - remove_group_checkmode is changed + - _instances.instances.0.security_groups | length == 2 + - security_group_name_1 in _instances.instances.0.security_groups | map(attribute='group_name') | list + - security_group_name_2 in _instances.instances.0.security_groups | map(attribute='group_name') | list + + - name: Remove one security group from the instance + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-group" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + security_group: "{{ security_group_name_2 }}" + register: remove_group + + - name: Fact presented instance + ec2_instance_info: + instance_ids: "{{ created_instance_id }}" + register: _instances + + - name: Ensure instance has 1 less Security group + assert: + that: + - remove_group is changed + - _instances.instances.0.security_groups | length == 1 + - _instances.instances.0.security_groups | map(attribute='group_name') == [security_group_name_2] + + - name: Remove one security group from the instance (idempotency) + amazon.aws.ec2_instance: + name: "{{ ec2_instance_name }}-group" + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + wait: false + subnet_id: "{{ testing_subnet_a.subnet.id }}" + security_group: "{{ security_group_name_2 }}" + register: remove_group_idempotency + + - name: Ensure module did not reported change + assert: + that: + - remove_group_idempotency is not changed diff --git a/tests/integration/targets/ec2_instance_tags_and_vpc_settings/tasks/main.yml b/tests/integration/targets/ec2_instance_tags_and_vpc_settings/tasks/main.yml index be05184f8f9..3ede6475d3c 100644 --- a/tests/integration/targets/ec2_instance_tags_and_vpc_settings/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_tags_and_vpc_settings/tasks/main.yml @@ -19,8 +19,10 @@ TestId: "{{ ec2_instance_tag_TestId }}" Something: else security_groups: "{{ sg.group_id }}" - network: - source_dest_check: false + network_interfaces: + - device_index: 0 + assign_public_ip: false + source_dest_check: false vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}" instance_type: "{{ ec2_instance_type }}" wait: false @@ -39,8 +41,10 @@ TestId: "{{ ec2_instance_tag_TestId }}" Something: else security_groups: "{{ sg.group_id }}" - network: - source_dest_check: false + network_interfaces: + - device_index: 0 + assign_public_ip: false + source_dest_check: false vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}" instance_type: "{{ ec2_instance_type }}" check_mode: true @@ -178,3 +182,68 @@ that: - _purge_tags.instances[0].tags | length == 1 - _purge_tags.instances[0].tags.Name.startswith(resource_prefix) + + - name: Update source_dest_check to true (check_mode) + amazon.aws.ec2_instance: + state: present + name: "{{ resource_prefix }}-test-basic-vpc-create" + image_id: "{{ ec2_ami_id }}" + tags: + TestId: "{{ ec2_instance_tag_TestId }}" + Something: else + security_groups: "{{ sg.group_id }}" + source_dest_check: true + vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}" + instance_type: "{{ ec2_instance_type }}" + check_mode: true + register: update_source_dest_check_checkmode + + - name: Ensure module reported change when running in check_mode + ansible.builtin.assert: + that: + - update_source_dest_check_checkmode is changed + + - name: Update source_dest_check to true + amazon.aws.ec2_instance: + state: present + name: "{{ resource_prefix }}-test-basic-vpc-create" + image_id: "{{ ec2_ami_id }}" + tags: + TestId: "{{ ec2_instance_tag_TestId }}" + Something: else + security_groups: "{{ sg.group_id }}" + source_dest_check: true + vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}" + instance_type: "{{ ec2_instance_type }}" + register: update_source_dest_check + + - name: fact presented ec2 instance + amazon.aws.ec2_instance_info: + filters: + tag:Name: "{{ resource_prefix }}-test-basic-vpc-create" + register: presented_instance_fact + + - name: Ensure module reported change and instance was updated + ansible.builtin.assert: + that: + - update_source_dest_check is changed + - presented_instance_fact.instances.0.source_dest_check + + - name: Update source_dest_check to true (idempotency) + amazon.aws.ec2_instance: + state: present + name: "{{ resource_prefix }}-test-basic-vpc-create" + image_id: "{{ ec2_ami_id }}" + tags: + TestId: "{{ ec2_instance_tag_TestId }}" + Something: else + security_groups: "{{ sg.group_id }}" + source_dest_check: true + vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}" + instance_type: "{{ ec2_instance_type }}" + register: update_source_dest_check_idempotency + + - name: Ensure module did not reported change + ansible.builtin.assert: + that: + - update_source_dest_check_idempotency is not changed diff --git a/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml b/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml index de112a3d48f..ed46e77396d 100644 --- a/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml +++ b/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml @@ -120,9 +120,9 @@ security_group: "{{ vpc_sg_id }}" instance_type: t2.micro key_name: "{{ ec2_key_name }}" - network: - assign_public_ip: true - delete_on_termination: true + network_interfaces: + - assign_public_ip: true + delete_on_termination: true metadata_options: instance_metadata_tags: enabled tags: @@ -143,9 +143,9 @@ security_group: "{{ vpc_sg_id }}" instance_type: t2.micro key_name: "{{ ec2_key_name }}" - network: - assign_public_ip: true - delete_on_termination: true + network_interfaces: + - assign_public_ip: true + delete_on_termination: true metadata_options: instance_metadata_tags: enabled tags: diff --git a/tests/integration/targets/setup_ec2_instance_env/tasks/main.yml b/tests/integration/targets/setup_ec2_instance_env/tasks/main.yml index 5b41b639620..b084e3d0dbd 100644 --- a/tests/integration/targets/setup_ec2_instance_env/tasks/main.yml +++ b/tests/integration/targets/setup_ec2_instance_env/tasks/main.yml @@ -15,6 +15,7 @@ tags: Name: "{{ vpc_name }}" tenancy: default + ipv6_cidr: true register: testing_vpc notify: - Delete ec2_instance environment @@ -27,6 +28,7 @@ az: "{{ subnet_a_az }}" resource_tags: Name: "{{ subnet_a_name }}" + ipv6_cidr: "{{ testing_vpc.vpc.ipv6_cidr_block_association_set.0.ipv6_cidr_block | replace('/56', '/64') }}" register: testing_subnet_a - name: Create secondary subnet in zone B From c4201518f13ff8f542f346f20f428a7d7e966981 Mon Sep 17 00:00:00 2001 From: abikouo Date: Tue, 11 Jun 2024 11:27:53 +0200 Subject: [PATCH 2/6] add time --- tests/integration/targets/ec2_instance_network/aliases | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/targets/ec2_instance_network/aliases b/tests/integration/targets/ec2_instance_network/aliases index daa489fdab9..e65383d9a85 100644 --- a/tests/integration/targets/ec2_instance_network/aliases +++ b/tests/integration/targets/ec2_instance_network/aliases @@ -1,4 +1,4 @@ -time=xxm +time=4m cloud/aws From 378ea1d0cf99943e65eccc00b4aa16c97e9b49f6 Mon Sep 17 00:00:00 2001 From: abikouo Date: Tue, 11 Jun 2024 15:15:37 +0200 Subject: [PATCH 3/6] fix ansible-lint issues --- plugins/modules/ec2_instance.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index c12a71289b1..476fe44015c 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -633,10 +633,10 @@ amazon.aws.ec2_instance: name: "public-eni-instance" network_interfaces_ids: - - id: "eni-12345" - device_index: 0 - - id: "eni-67890" - device_index: 1 + - id: "eni-12345" + device_index: 0 + - id: "eni-67890" + device_index: 1 image_id: ami-123456 tags: Env: "eni_on" From beea86f91556e7622da13d4a83f8792ff63e3dc4 Mon Sep 17 00:00:00 2001 From: abikouo Date: Wed, 26 Jun 2024 13:09:50 +0200 Subject: [PATCH 4/6] Code review updates --- plugins/modules/ec2_instance.py | 48 +++++++++++++++------------------ 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index 476fe44015c..d84dbe5a03e 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -227,7 +227,7 @@ version_added: 8.1.0 network_interfaces: description: - - A dictionary containing specifications for a single network interface. + - A list of dictionaries containing specifications for network interfaces. - Use the M(amazon.aws.ec2_eni) module to create ENIs with special settings. - Mutually exclusive with O(network). type: list @@ -1445,18 +1445,11 @@ def validate_network_params(params: Dict[str, Any], nb_instances: int) -> None: # Ensure none of 'private_ip_address', 'private_ip_addresses', 'ipv6_addresses' were provided # when launching more than one instance if nb_instances > 1: - if any(True for inty in network_interfaces if inty.get("private_ip_address")): - module.fail_json( - msg="The option 'private_ip_address' cannot be specified when launching more than one instance." - ) - if any(True for inty in network_interfaces if inty.get("private_ip_addresses")): - module.fail_json( - msg="The option 'private_ip_addresses' cannot be specified when launching more than one instance." - ) - if any(True for inty in network_interfaces if inty.get("ipv6_addresses")): - module.fail_json( - msg="The option 'ipv6_addresses' cannot be specified when launching more than one instance." - ) + for opt in ("private_ip_address", "private_ip_addresses", "ipv6_addresses"): + if any(True for inty in network_interfaces if inty.get(opt)): + module.fail_json( + msg=f"The option '{opt}' cannot be specified when launching more than one instance." + ) def ansible_to_boto3_eni_specification( @@ -1473,9 +1466,9 @@ def ansible_to_boto3_eni_specification( AWS network interface specification. """ spec = { - "DeviceIndex": interface.get("device_index") or 0, + "DeviceIndex": interface.get("device_index", 0), } - if interface.get("assign_public_ip") is not None: + if interface.get("assign_public_ip"): spec["AssociatePublicIpAddress"] = interface["assign_public_ip"] if interface.get("subnet_id"): @@ -1486,14 +1479,14 @@ def ansible_to_boto3_eni_specification( spec["SubnetId"] = describe_default_subnet(module) if interface.get("ipv6_addresses"): - spec["Ipv6Addresses"] = [{"Ipv6Address": a} for a in interface.get("ipv6_addresses")] + spec["Ipv6Addresses"] = [{"Ipv6Address": a} for a in interface["ipv6_addresses"]] if interface.get("private_ip_address"): spec["PrivateIpAddress"] = interface["private_ip_address"] if interface.get("private_ip_addresses"): spec["PrivateIpAddresses"] = [] - for addr in interface.get("private_ip_addresses"): + for addr in interface["private_ip_addresses"]: d = {"PrivateIpAddress": addr} if isinstance(addr, dict): d = {"PrivateIpAddress": addr.get("private_ip_address")} @@ -1533,18 +1526,18 @@ def build_network_spec(params: Dict[str, Any]) -> List[Dict[str, Any]]: groups = params.get("security_groups") or params.get("security_group") vpc_subnet_id = params.get("vpc_subnet_id") network_interfaces = params.get("network_interfaces") - if (network is not None and not network.get("interfaces")) or network_interfaces: + if (network and not network.get("interfaces")) or network_interfaces: # They specified network interfaces using `network` or `network_interfaces` options - if network is not None and not network.get("interfaces"): + if network and not network.get("interfaces"): network_interfaces = [network] interfaces.extend( [ansible_to_boto3_eni_specification(inty, vpc_subnet_id, groups) for inty in network_interfaces] ) elif not network and not network_interfaces_ids: - # They do not specified any network interface configuration + # They did not specify any network interface configuration # build network interface using subnet_id and security group(s) defined on the module interfaces.append(ansible_to_boto3_eni_specification({}, vpc_subnet_id, groups)) - elif network is not None: + elif network: # handle list of `network.interfaces` options interfaces.extend( [ @@ -1871,8 +1864,8 @@ def value_wrapper(v): changes_to_apply.append(arguments) network_interfaces = params.get("network_interfaces") - if not network_interfaces and params.get("network") and not params.get("network").get("interfaces"): - network_interfaces = [params.get("network")] + if not network_interfaces and params.get("network") and not params["network"].get("interfaces"): + network_interfaces = [params["network"]] if network_interfaces or params.get("security_groups") or params.get("security_group"): if len(network_interfaces or []) > 1 or len(instance["NetworkInterfaces"]) > 1: module.warn("Skipping group modification because instance contains mutiple network interfaces.") @@ -2497,7 +2490,7 @@ def describe_default_subnet(module: AnsibleAWSModule, use_availability_zone: boo The default subnet id. """ default_vpc = get_default_vpc() - if default_vpc is None: + if not default_vpc: module.fail_json( msg=("No default subnet could be found - you must include a VPC subnet ID (vpc_subnet_id parameter).") ) @@ -2709,12 +2702,15 @@ def main(): date="2026-12-01", collection_name="amazon.aws", ) - if module.params.get("network").get("interfaces"): + if module.params["network"].get("interfaces"): if module.params.get("security_group"): module.fail_json(msg="Parameter network.interfaces can't be used with security_group") if module.params.get("security_groups"): module.fail_json(msg="Parameter network.interfaces can't be used with security_groups") - if module.params.get("network").get("source_dest_check") is not None and module.params.get("source_dest_check"): + if ( + module.params["network"].get("source_dest_check") is not None + and module.params.get("source_dest_check") is not None + ): module.warn( "the source_dest_check option has been set therefore network.source_dest_check will be ignored." ) From 6b246b94f5edc895909be046ab8e935850a8fcca Mon Sep 17 00:00:00 2001 From: abikouo Date: Fri, 28 Jun 2024 17:58:17 +0200 Subject: [PATCH 5/6] update inconsistent failing test cases - ec2_metadata_facts and ec2_placement_groups --- .../targets/ec2_instance_placement_options/tasks/main.yml | 1 + tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/integration/targets/ec2_instance_placement_options/tasks/main.yml b/tests/integration/targets/ec2_instance_placement_options/tasks/main.yml index 48289425816..3b5eb5dff73 100644 --- a/tests/integration/targets/ec2_instance_placement_options/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_placement_options/tasks/main.yml @@ -58,6 +58,7 @@ TestId: "{{ ec2_instance_tag_TestId }}" security_group: default instance_type: "{{ ec2_instance_type }}" + availability_zone: us-east-1c wait: true ignore_errors: true register: instance_creation diff --git a/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml b/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml index ed46e77396d..aa83255d444 100644 --- a/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml +++ b/tests/integration/targets/ec2_metadata_facts/playbooks/setup.yml @@ -128,6 +128,7 @@ tags: snake_case_key: a_snake_case_value camelCaseKey: aCamelCaseValue + wait: true register: ec2_instance - ansible.builtin.set_fact: From 5f9042625c990f60b05a0511c4a4837041b65112 Mon Sep 17 00:00:00 2001 From: abikouo Date: Mon, 1 Jul 2024 10:01:56 +0200 Subject: [PATCH 6/6] Set correct python version --- .../targets/ec2_metadata_facts/templates/inventory.j2 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/targets/ec2_metadata_facts/templates/inventory.j2 b/tests/integration/targets/ec2_metadata_facts/templates/inventory.j2 index e79b2f243bf..59254bc398f 100644 --- a/tests/integration/targets/ec2_metadata_facts/templates/inventory.j2 +++ b/tests/integration/targets/ec2_metadata_facts/templates/inventory.j2 @@ -12,15 +12,16 @@ testhost_py2 [testhost:vars] ansible_ssh_private_key_file="{{ sshkey }}" -ansible_python_interpreter=/usr/bin/env python [testhost_py3:vars] ansible_user="{{ ec2_ami_ssh_user }}" image_id="{{ ec2_ami_id }}" +ansible_python_interpreter=python3 [testhost_py2:vars] ansible_user="{{ ec2_ami_ssh_user_py2 }}" image_id="{{ ec2_ami_id_py2 }}" +ansible_python_interpreter=python2 [all:vars] # Template vars that will need to be used in used in tests and teardown