From 5b89be9c5f250be000e68db5dfaa4ef9b64fe0db Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Mon, 27 Jan 2025 20:51:50 -0800 Subject: [PATCH 1/6] Ensure module.params.get("vpc_subnet_id") is used when subnet_id is missing from network_interface" --- plugins/modules/ec2_instance.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index 809c2664f73..0f67027b981 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -1933,6 +1933,8 @@ def value_wrapper(v): if network_interfaces: subnet_id = network_interfaces[0].get("subnet_id") groups = network_interfaces[0].get("groups") + if not subnet_id and module.params.get("vpc_subnet_id"): + subnet_id = module.params.get("vpc_subnet_id") elif module.params.get("vpc_subnet_id"): subnet_id = module.params.get("vpc_subnet_id") else: From 29805c66ca3c21b0e5fcc8c370f0e7b16dde7fb5 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 28 Jan 2025 08:40:41 -0800 Subject: [PATCH 2/6] add changelog --- .../2488-ec2_instance-fix-security-group-attachment-issue.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/2488-ec2_instance-fix-security-group-attachment-issue.yml diff --git a/changelogs/fragments/2488-ec2_instance-fix-security-group-attachment-issue.yml b/changelogs/fragments/2488-ec2_instance-fix-security-group-attachment-issue.yml new file mode 100644 index 00000000000..82dadcfd061 --- /dev/null +++ b/changelogs/fragments/2488-ec2_instance-fix-security-group-attachment-issue.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - ec2_instance - Fix issue where EC2 instance module failed to apply security groups when both `network` and `vpc_subnet_id`` were specified, caused by passing `None` to discover_security_groups() (https://github.com/ansible-collections/amazon.aws/pull/2488). From 5ab1fda37c983778dea1b56f3e195afd421277dc Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 28 Jan 2025 08:52:22 -0800 Subject: [PATCH 3/6] add note in docs --- plugins/modules/ec2_instance.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index 0f67027b981..a00b3007ae7 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -160,6 +160,7 @@ description: - The subnet ID in which to launch the instance (VPC). - If none is provided, M(amazon.aws.ec2_instance) will chose the default zone of the default VPC. + - If used along with `network`, `vpc_subnet_id` is used as a fallback to prevent errors when no subnet ID is found in the `network`. aliases: ['subnet_id'] type: str network: @@ -170,6 +171,7 @@ - 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). + - If used along with `vpc_subnet_id`, `vpc_subnet_id` is used as a fallback to prevent errors when no subnet ID is found in the `network`. type: dict suboptions: interfaces: From e4eae82ce5765bfe88eb3398aa6ed31c71c1ef41 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 28 Jan 2025 09:01:49 -0800 Subject: [PATCH 4/6] add note in docs --- plugins/modules/ec2_instance.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index a00b3007ae7..ff50cf9213b 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -160,7 +160,7 @@ description: - The subnet ID in which to launch the instance (VPC). - If none is provided, M(amazon.aws.ec2_instance) will chose the default zone of the default VPC. - - If used along with `network`, `vpc_subnet_id` is used as a fallback to prevent errors when no subnet ID is found in the `network`. + - If used along with O("network"), O("vpc_subnet_id") is used as a fallback to prevent errors when O("network.subnet_id") is not specified. aliases: ['subnet_id'] type: str network: @@ -171,7 +171,7 @@ - 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). - - If used along with `vpc_subnet_id`, `vpc_subnet_id` is used as a fallback to prevent errors when no subnet ID is found in the `network`. + - If used along with O("vpc_subnet_id"), O("vpc_subnet_id") is used as a fallback to prevent errors when O("network.subnet_id") is not specified. type: dict suboptions: interfaces: From 9ad3ddedee5d1511f2c8c4a9d9c7bcb6b9ff770d Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 28 Jan 2025 09:51:36 -0800 Subject: [PATCH 5/6] sanity fix --- plugins/modules/ec2_instance.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py index ff50cf9213b..f7cfbde7353 100644 --- a/plugins/modules/ec2_instance.py +++ b/plugins/modules/ec2_instance.py @@ -160,7 +160,7 @@ description: - The subnet ID in which to launch the instance (VPC). - If none is provided, M(amazon.aws.ec2_instance) will chose the default zone of the default VPC. - - If used along with O("network"), O("vpc_subnet_id") is used as a fallback to prevent errors when O("network.subnet_id") is not specified. + - If used along with O(network), O(vpc_subnet_id) is used as a fallback to prevent errors when O(network.subnet_id) is not specified. aliases: ['subnet_id'] type: str network: @@ -171,7 +171,7 @@ - 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). - - If used along with O("vpc_subnet_id"), O("vpc_subnet_id") is used as a fallback to prevent errors when O("network.subnet_id") is not specified. + - If used along with O(vpc_subnet_id), O(vpc_subnet_id) is used as a fallback to prevent errors when O(network.subnet_id) is not specified. type: dict suboptions: interfaces: From f71fc7ff8c6ffa54b1ba7532d4ffcc5b71c4adbf Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Tue, 28 Jan 2025 14:26:37 -0800 Subject: [PATCH 6/6] add integration test --- .../defaults/main.yml | 5 + .../tasks/main.yml | 3 + .../tasks/test_default_sec_group.yml | 98 +++++++++++++++++++ 3 files changed, 106 insertions(+) create mode 100644 tests/integration/targets/ec2_instance_security_group/tasks/test_default_sec_group.yml diff --git a/tests/integration/targets/ec2_instance_security_group/defaults/main.yml b/tests/integration/targets/ec2_instance_security_group/defaults/main.yml index 886365e3076..17ecf1209ac 100644 --- a/tests/integration/targets/ec2_instance_security_group/defaults/main.yml +++ b/tests/integration/targets/ec2_instance_security_group/defaults/main.yml @@ -2,3 +2,8 @@ # defaults file for ec2_instance_security_group ec2_instance_type: t3a.micro ec2_instance_tag_testid: "{{ resource_prefix }}-instance-sg" + +vpc_cidr_1: "10.{{ 256 | random(seed=resource_prefix) }}.0.0/16" +vpc_cidr_2: "10.{{ 256 | random(seed=resource_prefix) }}.0.0/16" +subnet_cidr_1: "10.{{ 256 | random(seed=resource_prefix) }}.1.0/24" +subnet_cidr_2: "10.{{ 256 | random(seed=resource_prefix) }}.2.0/24" diff --git a/tests/integration/targets/ec2_instance_security_group/tasks/main.yml b/tests/integration/targets/ec2_instance_security_group/tasks/main.yml index fac2e1b7b6b..85a465b79c6 100644 --- a/tests/integration/targets/ec2_instance_security_group/tasks/main.yml +++ b/tests/integration/targets/ec2_instance_security_group/tasks/main.yml @@ -6,6 +6,9 @@ session_token: "{{ security_token | default(omit) }}" region: "{{ aws_region }}" block: + - name: Run tests for creating instance with vpc_subnet_id and network both specified + ansible.builtin.include_tasks: test_default_sec_group.yml + - name: New instance with 2 security groups amazon.aws.ec2_instance: state: running diff --git a/tests/integration/targets/ec2_instance_security_group/tasks/test_default_sec_group.yml b/tests/integration/targets/ec2_instance_security_group/tasks/test_default_sec_group.yml new file mode 100644 index 00000000000..04ea2eacc15 --- /dev/null +++ b/tests/integration/targets/ec2_instance_security_group/tasks/test_default_sec_group.yml @@ -0,0 +1,98 @@ +--- +- name: Test creating an instance attached to a security group "default" and network configuration missing subnet_id + block: + - name: Create VPC 1 + amazon.aws.ec2_vpc_net: + name: "vpc-1" + cidr_block: "{{ vpc_cidr_1 }}" + state: present + register: vpc_1 + + - name: Create VPC 2 + amazon.aws.ec2_vpc_net: + name: "vpc-2" + cidr_block: "{{ vpc_cidr_2 }}" + state: present + register: vpc_2 + + - name: Create Subnet 1 in VPC 1 + amazon.aws.ec2_vpc_subnet: + vpc_id: "{{ vpc_1.vpc.id }}" + cidr: "{{ subnet_cidr_1 }}" + state: present + register: subnet_1 + + - name: Create Subnet 2 in VPC 2 + amazon.aws.ec2_vpc_subnet: + vpc_id: "{{ vpc_2.vpc.id }}" + cidr: "{{ subnet_cidr_2 }}" + state: present + register: subnet_2 + + - name: Get security groups for vpc_1 + amazon.aws.ec2_security_group_info: + filters: + vpc-id: "{{ vpc_1.vpc.id }}" + register: security_group_vpc_1 + + - name: Create EC2 instance + amazon.aws.ec2_instance: + image_id: "{{ ec2_ami_id }}" + instance_type: "t2.micro" + network: + assign_public_ip: false + security_groups: "default" + tags: + Owner: Integration-Test + Persistent: false + Name: "{{ resource_prefix}}-test-instance" + vpc_subnet_id: "{{ subnet_1.subnet.id }}" + wait: true + state: present + register: ec2_instance + + - name: Assert create results + ansible.builtin.assert: + that: + - ec2_instance is not failed + - ec2_instance is changed + - ec2_instance.instances[0].security_groups[0].group_id == security_group_vpc_1.security_groups[0].group_id + - security_group_vpc_1.security_groups[0].group_name == "default" + + always: + - name: Destroy EC2 instance + amazon.aws.ec2_instance: + instance_ids: "{{ ec2_instance.instance_ids[0] }}" + state: absent + register: destroyed_instance + ignore_errors: true + + - name: Destroy Subnet 1 + amazon.aws.ec2_vpc_subnet: + vpc_id: "{{ vpc_1.vpc.id }}" + cidr: "{{ subnet_cidr_1 }}" + state: absent + register: destroyed_subnet_1 + ignore_errors: true + + - name: Destroy Subnet 2 + amazon.aws.ec2_vpc_subnet: + vpc_id: "{{ vpc_2.vpc.id }}" + cidr: "{{ subnet_cidr_2 }}" + state: absent + register: destroyed_subnet_2 + ignore_errors: true + + - name: Destroy VPC 1 + amazon.aws.ec2_vpc_net: + vpc_id: "{{ vpc_1.vpc.id }}" + state: absent + register: destroyed_vpc_1 + ignore_errors: true + + - name: Destroy VPC 2 + amazon.aws.ec2_vpc_net: + vpc_id: "{{ vpc_2.vpc.id }}" + state: absent + register: destroyed_vpc_2 + ignore_errors: true