ec2_instance: Use "vpc_subnet_id" from module.params when subnet_id i…

…s missing from network interface (ansible-collections#2488)

SUMMARY

Fixes https://issues.redhat.com/browse/ACA-2123

This PR addresses an issue where module tries to attach all security groups in the region that default SGs for any VPC in the region. Causing error below

fatal: [localhost]: FAILED! => {"boto3_version": "1.34.144", "botocore_version": "1.34.144", "changed": false,
"msg": "Could not apply change {'Groups': ['sg-xxxx', 'sg-yyyy', 'sg-zzzz']} to existing instance.: Failed to modify instance attribute"}

The subnet_id was previously passed as None when not found in the network_interface, causing failures when applying security group.
The logic was updated to use module.params.get("vpc_subnet_id") when provided in task, as a fallback, preventing None from being passed to discover_security_groups() and ensuring the correct subnet is used when the default security group is specified.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ec2_instance
ADDITIONAL INFORMATION

According to jira, the issue was introduced after 8.1.0 onwards (worked fine until and including 8.1.0)
Issue was only seen (during testing/reproducing) when network and vpc_subnet_id were both specified in the task.

Playbook used for testing/reproducing the issue.
---
- name: EC2 instance
  hosts: localhost
  gather_facts: no
  vars:
    instance_type: "t2.micro"
    subnet_id: "subnet-xxxxxxx"
    region: "ap-northeast-2"
     image_id: "ami-xxxxxxx"
  tasks:
    - name: Create the EC2 instance with proper tags
      amazon.aws.ec2_instance:
        image_id: "{{ image_id }}"
        instance_type: "{{ instance_type }}"
        network:
          assign_public_ip: false
          private_ip_address: "{{ ec2_private_ip | default(omit) }}"
        purge_tags: false
        region: "{{ region }}"
        security_groups: "{{ security_group | default('default') }}"
        tags:
          Owner: mandkulk
          Persistent: False
          Name: xxxxx-test-instance
        vpc_subnet_id: "{{ subnet_id }}"
        wait: true
        state: present
      register: ec2

Reviewed-by: Mark Chappell
Reviewed-by: Alina Buzachis
(cherry picked from commit 2fb1661)

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).

plugins/modules/ec2_instance.py

@@ -158,6 +158,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.
     aliases: ['subnet_id']
     type: str
   network:
@@ -168,6 +169,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.
     type: dict
     suboptions:
       interfaces:
@@ -1881,8 +1883,10 @@ def value_wrapper(v):
             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")
+                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:
                 subnet_id = describe_default_subnet(module=module, use_availability_zone=False)
 

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"

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

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