elb_application_lb - treat empty security group as VPC default (ansib…

…le-collections#971) elb_application_lb - treat empty security group as VPC default SUMMARY Fixes idempotency issue when security_groups = [] by treating [] as using the VPC's default security group (like it does on creation). Fixes ansible-collections#28 Used same logic as amazon.aws.ec2_vpc_route_table does for using default igw Added integration tests ISSUE TYPE Bugfix Pull Request COMPONENT NAME elb_application_lb Reviewed-by: Jill R <None> Reviewed-by: Mark Woolley <[email protected]> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@20b726a
GomathiselviS · Sep 20, 2022 · e89eac8 · e89eac8
1 parent 5d4a479
commit e89eac8
Show file tree

Hide file tree

Showing 2 changed files with 128 additions and 5 deletions.
diff --git a/plugins/modules/elb_application_lb.py b/plugins/modules/elb_application_lb.py
@@ -144,7 +144,7 @@
     description:
       - A list of the names or IDs of the security groups to assign to the load balancer.
       - Required if I(state=present).
-    default: []
+      - If C([]), the VPC's default security group will be used.
     type: list
     elements: str
   scheme:
@@ -494,10 +494,16 @@
     type: bool
     sample: false
 '''
+try:
+    import botocore
+except ImportError:
+    pass  # caught by AnsibleAWSModule
 
 from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
-from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_filter_list
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry
 from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict
+from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict
 from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_aws_tags
 from ansible_collections.amazon.aws.plugins.module_utils.elbv2 import (
     ApplicationLoadBalancer,
@@ -509,6 +515,29 @@
 from ansible_collections.amazon.aws.plugins.module_utils.elb_utils import get_elb_listener_rules
 
 
+@AWSRetry.jittered_backoff()
+def describe_sgs_with_backoff(connection, **params):
+    paginator = connection.get_paginator('describe_security_groups')
+    return paginator.paginate(**params).build_full_result()['SecurityGroups']
+
+
+def find_default_sg(connection, module, vpc_id):
+    """
+    Finds the default security group for the given VPC ID.
+    """
+    filters = ansible_dict_to_boto3_filter_list({'vpc-id': vpc_id, 'group-name': 'default'})
+    try:
+        sg = describe_sgs_with_backoff(connection, Filters=filters)
+    except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+        module.fail_json_aws(e, msg='No default security group found for VPC {0}'.format(vpc_id))
+    if len(sg) == 1:
+        return sg[0]['GroupId']
+    elif len(sg) == 0:
+        module.fail_json(msg='No default security group found for VPC {0}'.format(vpc_id))
+    else:
+        module.fail_json(msg='Multiple security groups named "default" found for VPC {0}'.format(vpc_id))
+
+
 def create_or_update_alb(alb_obj):
     """Create ALB or modify main attributes. json_exit here"""
     if alb_obj.elb:
@@ -738,6 +767,11 @@ def main():
 
     alb = ApplicationLoadBalancer(connection, connection_ec2, module)
 
+    # Update security group if default is specified
+    if alb.elb and module.params.get('security_groups') == []:
+        module.params['security_groups'] = [find_default_sg(connection_ec2, module, alb.elb['VpcId'])]
+        alb = ApplicationLoadBalancer(connection, connection_ec2, module)
+
     if state == 'present':
         create_or_update_alb(alb)
     elif state == 'absent':

diff --git a/tests/integration/targets/elb_application_lb/tasks/main.yml b/tests/integration/targets/elb_application_lb/tasks/main.yml
@@ -25,6 +25,12 @@
       set_fact:
         vpc_id: "{{ vpc.vpc.id }}"
 
+    - name: Get VPC's default security group
+      ec2_group_info:
+        filters:
+          vpc-id: "{{ vpc_id }}"
+      register: default_sg
+
     - name: Create an internet gateway
       ec2_vpc_igw:
         vpc_id: '{{ vpc_id }}'
@@ -200,7 +206,90 @@
 
     # ------------------------------------------------------------------------------------------
 
-    - name: Create an ALB with ip address type - check_mode
+    - name: Create an ALB with defaults - check_mode
+      elb_application_lb:
+        name: "{{ alb_name }}"
+        subnets: "{{ public_subnets }}"
+        security_groups: []
+        state: present
+        listeners:
+          - Protocol: HTTP
+            Port: 80
+            DefaultActions:
+              - Type: forward
+                TargetGroupName: "{{ tg_name }}"
+      register: alb
+      check_mode: yes
+
+    - assert:
+        that:
+          - alb is changed
+          - alb.msg is match('Would have created ALB if not in check mode.')
+
+    - name: Create an ALB with defaults
+      elb_application_lb:
+        name: "{{ alb_name }}"
+        subnets: "{{ public_subnets }}"
+        security_groups: []
+        state: present
+        listeners:
+          - Protocol: HTTP
+            Port: 80
+            DefaultActions:
+              - Type: forward
+                TargetGroupName: "{{ tg_name }}"
+      register: alb
+
+    - assert:
+        that:
+          - alb is changed
+          - alb.listeners[0].rules | length == 1
+          - alb.security_groups | length == 1
+          - alb.security_groups[0] == default_sg.security_groups[0].group_id
+
+    - name: Create an ALB with defaults (idempotence) - check_mode
+      elb_application_lb:
+        name: "{{ alb_name }}"
+        subnets: "{{ public_subnets }}"
+        security_groups: []
+        state: present
+        listeners:
+          - Protocol: HTTP
+            Port: 80
+            DefaultActions:
+              - Type: forward
+                TargetGroupName: "{{ tg_name }}"
+      register: alb
+      check_mode: yes
+
+    - assert:
+        that:
+          - alb is not changed
+          - alb.msg is match('IN CHECK MODE - no changes to make to ALB specified.')
+
+    - name: Create an ALB with defaults (idempotence)
+      elb_application_lb:
+        name: "{{ alb_name }}"
+        subnets: "{{ public_subnets }}"
+        security_groups: []
+        state: present
+        listeners:
+          - Protocol: HTTP
+            Port: 80
+            DefaultActions:
+              - Type: forward
+                TargetGroupName: "{{ tg_name }}"
+      register: alb
+
+    - assert:
+        that:
+          - alb is not changed
+          - alb.listeners[0].rules | length == 1
+          - alb.security_groups[0] == default_sg.security_groups[0].group_id
+
+    # ------------------------------------------------------------------------------------------
+
+    - name: Update an ALB with ip address type - check_mode
       elb_application_lb:
         name: "{{ alb_name }}"
         subnets: "{{ public_subnets }}"
@@ -219,9 +308,9 @@
     - assert:
         that:
           - alb is changed
-          - alb.msg is match('Would have created ALB if not in check mode.')
+          - alb.msg is match('Would have updated ALB if not in check mode.')
 
-    - name: Create an ALB with ip address type
+    - name: Update an ALB with ip address type
       elb_application_lb:
         name: "{{ alb_name }}"
         subnets: "{{ public_subnets }}"