From f4fee7b968f25a175df346600e4e6ab57104c14e Mon Sep 17 00:00:00 2001
From: gregharvey <greg.harvey@gmail.com>
Date: Wed, 22 Nov 2023 19:53:33 +0100
Subject: [PATCH 1/2] Making SG handling consistent with a new role.

---
 .../defaults/main.yml                         | 11 ++++---
 .../aws_ec2_autoscale_cluster/tasks/main.yml  | 32 +++++++++++++++++--
 roles/aws/aws_ec2_with_eip/defaults/main.yml  |  2 +-
 roles/aws/aws_ec2_with_eip/tasks/main.yml     | 15 +++++++--
 roles/aws/aws_efs/defaults/main.yml           |  3 +-
 roles/aws/aws_efs/tasks/main.yml              | 19 +++++------
 roles/aws/aws_efs/tasks/security_group.yml    | 11 -------
 roles/aws/aws_efs/tasks/subnet.yml            |  1 +
 roles/aws/aws_efs/tasks/target.yml            |  4 ++-
 roles/aws/aws_elasticache/defaults/main.yml   |  2 +-
 roles/aws/aws_elasticache/tasks/main.yml      | 12 ++++++-
 roles/aws/aws_elb/defaults/main.yml           |  2 +-
 roles/aws/aws_elb/tasks/main.yml              | 12 ++++++-
 roles/aws/aws_opensearch/defaults/main.yml    |  2 +-
 roles/aws/aws_opensearch/tasks/main.yml       | 14 +++++++-
 roles/aws/aws_rds/defaults/main.yml           |  3 +-
 roles/aws/aws_rds/tasks/main.yml              | 22 ++++++-------
 roles/aws/aws_rds/tasks/security_group.yml    | 12 -------
 roles/aws/aws_security_groups/README.md       | 32 +++++++++++++++++++
 .../aws/aws_security_groups/defaults/main.yml |  8 +++++
 roles/aws/aws_security_groups/tasks/main.yml  | 30 +++++++++++++++++
 roles/debian/gitlab_runner/defaults/main.yml  |  2 +-
 roles/debian/gitlab_runner/tasks/main.yml     | 13 ++++++++
 .../gitlab_runner/templates/fargate.toml.j2   |  2 +-
 24 files changed, 199 insertions(+), 67 deletions(-)
 delete mode 100644 roles/aws/aws_efs/tasks/security_group.yml
 delete mode 100644 roles/aws/aws_rds/tasks/security_group.yml
 create mode 100644 roles/aws/aws_security_groups/README.md
 create mode 100644 roles/aws/aws_security_groups/defaults/main.yml
 create mode 100644 roles/aws/aws_security_groups/tasks/main.yml

diff --git a/roles/aws/aws_ec2_autoscale_cluster/defaults/main.yml b/roles/aws/aws_ec2_autoscale_cluster/defaults/main.yml
index 9af962f5c..5c4c4ace3 100644
--- a/roles/aws/aws_ec2_autoscale_cluster/defaults/main.yml
+++ b/roles/aws/aws_ec2_autoscale_cluster/defaults/main.yml
@@ -123,11 +123,12 @@ aws_ec2_autoscale_cluster:
   max_size: 8
   # Security groups for the instances cluster.
   # An internal one will be created automatically, use these vars to provide additional groups
-  cluster_security_groups: [] # used in launch template, must be SG IDs
-  alb_security_groups: [] # edge case, the module supports SG names or IDs
-  efs_security_groups: [] # must be SG names because the role uses the name to find the ID
-  rds_security_groups: [] # must be SG names because the role uses the name to find the ID
-  elasticache_security_groups: [] # must be SG IDs
+  # All security group lists should be a list of security group names, they will be automatically converted to IDs by the aws_security_groups as necessary
+  cluster_security_groups: []
+  alb_security_groups: []
+  efs_security_groups: []
+  rds_security_groups: []
+  elasticache_security_groups: []
   # ALB health checks - these are health check settings applied to the load balancer
   alb_health_check_type: ELB # Uses ALB health checks, set to EC2 to use default AWS instance status checks
   alb_health_check_period: 1200 # Length of time in seconds after a new EC2 instance comes into service that Auto Scaling starts checking its health
diff --git a/roles/aws/aws_ec2_autoscale_cluster/tasks/main.yml b/roles/aws/aws_ec2_autoscale_cluster/tasks/main.yml
index 62ac05cc5..d92e56a95 100644
--- a/roles/aws/aws_ec2_autoscale_cluster/tasks/main.yml
+++ b/roles/aws/aws_ec2_autoscale_cluster/tasks/main.yml
@@ -116,7 +116,7 @@
       elasticache_node_type: "{{ aws_ec2_autoscale_cluster.elasticache_node_type }}"
       elasticache_nodes: "{{ aws_ec2_autoscale_cluster.elasticache_nodes }}"
       elasticache_port: "{{ aws_ec2_autoscale_cluster.elasticache_port }}"
-      elasticache_security_groups: "{{ aws_ec2_autoscale_cluster.elasticache_security_groups + [_aws_ec2_autoscale_cluster_security_group.group_id] }}"
+      elasticache_security_groups: "{{ aws_ec2_autoscale_cluster.elasticache_security_groups + [_aws_ec2_autoscale_cluster_security_group.group_name] }}"
   when:
     - aws_ec2_autoscale_cluster.elasticache
 
@@ -376,6 +376,18 @@
     - aws_ec2_autoscale_cluster | length
     - aws_ec2_autoscale_cluster.create_elb
 
+- name: Generate security group information for the ALB.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_ec2_autoscale_cluster.aws_profile }}"
+    region: "{{ aws_ec2_autoscale_cluster.region }}"
+    group_names: "{{ aws_ec2_autoscale_cluster.alb_security_groups }}"
+    return_type: ids
+  when:
+    - aws_ec2_autoscale_cluster.alb_security_groups | length > 0
+    - aws_ec2_autoscale_cluster.create_elb
+
 - name: Create the ALB.
   amazon.aws.elb_application_lb:
     name: "{{ aws_ec2_autoscale_cluster.name }}"
@@ -384,7 +396,7 @@
     state: "{{ aws_ec2_autoscale_cluster.state }}"
     tags: "{{ aws_ec2_autoscale_cluster.tags }}"
     subnets: "{{ _aws_ec2_autoscale_cluster_public_subnets_ids }}"
-    security_groups: "{{ aws_ec2_autoscale_cluster.alb_security_groups + [_aws_ec2_autoscale_cluster_security_group.group_id] }}"
+    security_groups: "{{ _aws_security_group_list + [_aws_ec2_autoscale_cluster_security_group.group_id] }}"
     listeners: "{{ _aws_ec2_autoscale_cluster_listeners }}"
     idle_timeout: "{{ aws_ec2_autoscale_cluster.alb_idle_timeout }}"
   register: _aws_ec2_autoscale_cluster_alb
@@ -410,6 +422,20 @@
   with_items: "{{ aws_ec2_autoscale_cluster.ssl_extra_certificate_ARNs }}"
 
 # EC2 - BUILD ASG
+- name: Generate security group information for the ASG.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_ec2_autoscale_cluster.aws_profile }}"
+    region: "{{ aws_ec2_autoscale_cluster.region }}"
+    group_names: "{{ aws_ec2_autoscale_cluster.cluster_security_groups }}"
+    return_type: ids
+  when:
+    - aws_ec2_autoscale_cluster.cluster_security_groups | length > 0
+    - aws_ec2_autoscale_cluster.asg_refresh
+    - aws_ec2_autoscale_cluster.type == "ec2"
+    - aws_ec2_autoscale_cluster.deploy_cluster
+
 - name: Create launch template.
   community.aws.ec2_launch_template:
     profile: "{{ aws_ec2_autoscale_cluster.aws_profile }}"
@@ -427,7 +453,7 @@
         delete_on_termination: "{{ aws_ec2_autoscale_cluster.instance_nic_delete_on_termination }}"
         subnet_id: "{{ subnet_id }}" # picked randomly from _aws_ec2_autoscale_cluster_subnets_ids, see with_random_choice
         device_index: 0 # must be 0 - see https://docs.aws.amazon.com/autoscaling/ec2/userguide/create-launch-template.html#change-network-interface
-        groups: "{{ aws_ec2_autoscale_cluster.cluster_security_groups + [_aws_ec2_autoscale_cluster_security_group.group_id] }}"
+        groups: "{{ _aws_security_group_list + [_aws_ec2_autoscale_cluster_security_group.group_id] }}"
     block_device_mappings:
       - ebs:
           delete_on_termination: "{{ aws_ec2_autoscale_cluster.root_volume_delete_on_termination }}"
diff --git a/roles/aws/aws_ec2_with_eip/defaults/main.yml b/roles/aws/aws_ec2_with_eip/defaults/main.yml
index fdfb30106..caf58f7a6 100644
--- a/roles/aws/aws_ec2_with_eip/defaults/main.yml
+++ b/roles/aws/aws_ec2_with_eip/defaults/main.yml
@@ -21,7 +21,7 @@ aws_ec2_with_eip:
   root_volume_encrypted: "{{ aws_ami.encrypt_boot }}" # in most cases this should match encrypt_boot in the aws_ami role
   root_volume_delete_on_termination: true
   ebs_optimized: true
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   tags:
     Name: "{{ _domain_name }}"
   # Add an A record tied to the EIP.
diff --git a/roles/aws/aws_ec2_with_eip/tasks/main.yml b/roles/aws/aws_ec2_with_eip/tasks/main.yml
index df46827d6..06aaec917 100644
--- a/roles/aws/aws_ec2_with_eip/tasks/main.yml
+++ b/roles/aws/aws_ec2_with_eip/tasks/main.yml
@@ -90,6 +90,17 @@
     dest: "{{ _ce_provision_data_dir + '/' + _aws_resource_name + '/ec2-subnet-id' }}"
     force: true
 
+# Convert security group names to IDs for consistency.
+- name: Generate security group information.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_ec2_with_eip.aws_profile }}"
+    region: "{{ aws_ec2_with_eip.region }}"
+    group_names: "{{ aws_ec2_with_eip.security_groups }}"
+    return_type: ids
+  when: aws_ec2_with_eip.security_groups | length > 0
+
 # Do not create an instance if _aws_hostname is not an EC2 generated address unless `force: true`
 - name: Create new EC2 instance.
   amazon.aws.ec2_instance:
@@ -105,7 +116,7 @@
     vpc_subnet_id: "{{ _aws_ec2_with_eip_vpc_subnet_id }}"
     name: "{{ aws_ec2_with_eip.instance_name }}"
     tags: "{{ aws_ec2_with_eip.tags }}"
-    security_groups: "{{ aws_ec2_with_eip.security_groups }}"
+    security_groups: "{{ _aws_security_group_list }}" # variable populated by the aws_security_groups role
     ebs_optimized: "{{ aws_ec2_with_eip.ebs_optimized }}"
     volumes:
       - device_name: /dev/xvda
@@ -131,7 +142,7 @@
     vpc_subnet_id: "{{ _aws_ec2_with_eip_vpc_subnet_id }}"
     name: "{{ aws_ec2_with_eip.instance_name }}"
     tags: "{{ aws_ec2_with_eip.tags }}"
-    security_groups: "{{ aws_ec2_with_eip.security_groups }}"
+    security_groups: "{{ _aws_security_group_list }}" # variable populated by the aws_security_groups role
     ebs_optimized: "{{ aws_ec2_with_eip.ebs_optimized }}"
     volumes:
       - device_name: /dev/xvda
diff --git a/roles/aws/aws_efs/defaults/main.yml b/roles/aws/aws_efs/defaults/main.yml
index 45ce6524d..0b9f700eb 100644
--- a/roles/aws/aws_efs/defaults/main.yml
+++ b/roles/aws/aws_efs/defaults/main.yml
@@ -9,8 +9,7 @@ aws_efs:
   wait: true
   # Subnets names.
   subnets: []
-  # SG names - ID lookup is automatic. Note, the assumption is that all subnets have the same SGs.
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   # Whether to encrypt the volume or not.
   encrypt: false
   backup: "{{ _infra_name }}-{{ _env_type }}" # Name of the AWS Backup plan to use to backup the instance.
diff --git a/roles/aws/aws_efs/tasks/main.yml b/roles/aws/aws_efs/tasks/main.yml
index 105a65dff..6e07019b5 100644
--- a/roles/aws/aws_efs/tasks/main.yml
+++ b/roles/aws/aws_efs/tasks/main.yml
@@ -1,6 +1,13 @@
-- name: Reset security groups info
-  ansible.builtin.set_fact:
-    _aws_efs_security_group_ids: []
+---
+- name: Generate security group information.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_efs.aws_profile }}"
+    region: "{{ aws_efs.region }}"
+    group_names: "{{ aws_efs.security_groups }}"
+    return_type: ids
+  when: aws_efs.security_groups | length > 0
 
 - name: Reset subnet ids info
   ansible.builtin.set_fact:
@@ -10,12 +17,6 @@
   ansible.builtin.set_fact:
     _aws_efs_targets: {}
 
-- name: Gather security groups info.
-  ansible.builtin.include_tasks: security_group.yml
-  with_items: "{{ aws_efs.security_groups }}"
-  loop_control:
-    loop_var: security_group
-
 - name: Gather subnets info.
   ansible.builtin.include_tasks: subnet.yml
   with_items: "{{ aws_efs.subnets }}"
diff --git a/roles/aws/aws_efs/tasks/security_group.yml b/roles/aws/aws_efs/tasks/security_group.yml
deleted file mode 100644
index 564708437..000000000
--- a/roles/aws/aws_efs/tasks/security_group.yml
+++ /dev/null
@@ -1,11 +0,0 @@
-- name: Gather security group info.
-  amazon.aws.ec2_security_group_info:
-    profile: "{{ aws_efs.aws_profile }}"
-    region: "{{ aws_efs.region }}"
-    filters:
-      group_name: "{{ security_group }}"
-  register: _aws_efs_security_group_info
-
-- name: Add security group id.
-  ansible.builtin.set_fact:
-    _aws_efs_security_group_ids: "{{ _aws_efs_security_group_ids + [_aws_efs_security_group_info.security_groups[0].group_id] }}"
diff --git a/roles/aws/aws_efs/tasks/subnet.yml b/roles/aws/aws_efs/tasks/subnet.yml
index 75eed3f5a..5daa24026 100644
--- a/roles/aws/aws_efs/tasks/subnet.yml
+++ b/roles/aws/aws_efs/tasks/subnet.yml
@@ -1,3 +1,4 @@
+---
 - name: Gather subnet information
   amazon.aws.ec2_vpc_subnet_info:
     profile: "{{ aws_efs.aws_profile }}"
diff --git a/roles/aws/aws_efs/tasks/target.yml b/roles/aws/aws_efs/tasks/target.yml
index 89278b7ec..117b9161f 100644
--- a/roles/aws/aws_efs/tasks/target.yml
+++ b/roles/aws/aws_efs/tasks/target.yml
@@ -1,3 +1,5 @@
+---
+# _aws_security_group_list variable populated by the aws_security_groups role.
 - name: Add subnet to the list of EFS targets.
   ansible.builtin.set_fact:
-    _aws_efs_targets: "{{ _aws_efs_targets | combine({target: {'subnet_id': target, 'security_groups': _aws_efs_security_group_ids }}) }}"
+    _aws_efs_targets: "{{ _aws_efs_targets | combine({target: {'subnet_id': target, 'security_groups': _aws_security_group_list }}) }}"
diff --git a/roles/aws/aws_elasticache/defaults/main.yml b/roles/aws/aws_elasticache/defaults/main.yml
index cc767ff3f..5eded26e6 100644
--- a/roles/aws/aws_elasticache/defaults/main.yml
+++ b/roles/aws/aws_elasticache/defaults/main.yml
@@ -14,5 +14,5 @@ aws_elasticache:
   elasticache_node_type: cache.t3.medium
   elasticache_nodes: 1
   elasticache_port: 11211
-  elasticache_security_groups: [] # list of security group IDs, @TODO - support names like the EFS role
+  elasticache_security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   #zone: eu-west-1a # AZ where the cluster will reside
\ No newline at end of file
diff --git a/roles/aws/aws_elasticache/tasks/main.yml b/roles/aws/aws_elasticache/tasks/main.yml
index 3bdd150a1..83c64efd8 100644
--- a/roles/aws/aws_elasticache/tasks/main.yml
+++ b/roles/aws/aws_elasticache/tasks/main.yml
@@ -1,4 +1,14 @@
 ---
+- name: Generate security group information.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_elasticache.aws_profile }}"
+    region: "{{ aws_elasticache.region }}"
+    group_names: "{{ aws_elasticache.elasticache_security_groups }}"
+    return_type: ids
+  when: aws_elasticache.elasticache_security_groups | length > 0
+
 - name: Create ElastiCache subnet group.
   community.aws.elasticache_subnet_group:
     name: "{{ aws_elasticache.name }}"
@@ -24,7 +34,7 @@
     node_type: "{{ aws_elasticache.elasticache_node_type }}"
     num_nodes: "{{ aws_elasticache.elasticache_nodes }}"
     cache_port: "{{ aws_elasticache.elasticache_port }}"
-    security_group_ids: "{{ aws_elasticache.elasticache_security_groups }}"
+    security_group_ids: "{{ _aws_security_group_list }}" # variable populated by the aws_security_groups role
     zone: "{{ aws_elasticache.zone | default(omit) }}"
 
 # @TODO grab the endpoint data for the memcached client role
\ No newline at end of file
diff --git a/roles/aws/aws_elb/defaults/main.yml b/roles/aws/aws_elb/defaults/main.yml
index c5bb2f2a6..ec551d142 100644
--- a/roles/aws/aws_elb/defaults/main.yml
+++ b/roles/aws/aws_elb/defaults/main.yml
@@ -23,7 +23,7 @@ aws_elb:
     zone: "example.com"
     record: "*.{{ _domain_name }}"
     aws_profile: "{{ _aws_profile }}" # Not necessarily the same as the "target" one.
-  elb_security_groups: []
+  elb_security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   target_groups: # list of target groups
     - name: example-target-group
       protocol: http
diff --git a/roles/aws/aws_elb/tasks/main.yml b/roles/aws/aws_elb/tasks/main.yml
index 1fc867bf1..4c4416338 100644
--- a/roles/aws/aws_elb/tasks/main.yml
+++ b/roles/aws/aws_elb/tasks/main.yml
@@ -109,6 +109,16 @@
     - aws_elb is defined
     - aws_elb | length
 
+- name: Generate security group information.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_elb.aws_profile }}"
+    region: "{{ aws_elb.region }}"
+    group_names: "{{ aws_elb.elb_security_groups }}"
+    return_type: ids
+  when: aws_elb.elb_security_groups | length > 0
+
 - name: Create the ALB.
   amazon.aws.elb_application_lb:
     name: "{{ aws_elb.name }}"
@@ -117,7 +127,7 @@
     state: "{{ aws_elb.state }}"
     tags: "{{ aws_elb.tags }}"
     subnets: "{{ _aws_ec2_elb_public_subnets_ids }}"
-    security_groups: "{{ aws_elb.elb_security_groups }}"
+    security_groups: "{{ _aws_security_group_list }}" # accepts names or IDs, variable populated by the aws_security_groups role
     listeners: "{{ _aws_ec2_listeners }}"
     idle_timeout: "{{ aws_elb.idle_timeout }}"
     ip_address_type: "{{ aws_elb.ip_address_type }}"
diff --git a/roles/aws/aws_opensearch/defaults/main.yml b/roles/aws/aws_opensearch/defaults/main.yml
index 52d27b043..4ade9b84f 100644
--- a/roles/aws/aws_opensearch/defaults/main.yml
+++ b/roles/aws/aws_opensearch/defaults/main.yml
@@ -16,7 +16,7 @@ aws_opensearch:
   subnets:
     - "subnet-aaaaaaaa"
     - "subnet-bbbbbbbb"
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   automated_snapshot_start_hour: 3
   auto_tune: "DISABLED" # not supported when t3's are used
   # List of maintenance schedules to use if auto_tune is set to "ENABLED":
diff --git a/roles/aws/aws_opensearch/tasks/main.yml b/roles/aws/aws_opensearch/tasks/main.yml
index 7c3023790..f43b79bb0 100644
--- a/roles/aws/aws_opensearch/tasks/main.yml
+++ b/roles/aws/aws_opensearch/tasks/main.yml
@@ -1,3 +1,15 @@
+---
+# Convert security group names to IDs for consistency.
+- name: Generate security group information.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_opensearch.aws_profile }}"
+    region: "{{ aws_opensearch.region }}"
+    group_names: "{{ aws_opensearch.security_groups }}"
+    return_type: ids
+  when: aws_opensearch.security_groups | length > 0
+
 - name: Get the current AWS account caller identity information
   amazon.aws.aws_caller_info:
     profile: "{{ aws_opensearch.aws_profile }}"
@@ -30,7 +42,7 @@
     access_policies: "{{ lookup('template', 'policy.json.j2') }}"
     vpc_options:
       subnets: "{{ aws_opensearch.subnets }}"
-      security_groups: "{{ aws_opensearch.security_groups }}"
+      security_groups: "{{ _aws_security_group_list }}" # variable populated by the aws_security_groups role
     snapshot_options:
       automated_snapshot_start_hour: "{{ aws_opensearch.automated_snapshot_start_hour }}"
     auto_tune_options:
diff --git a/roles/aws/aws_rds/defaults/main.yml b/roles/aws/aws_rds/defaults/main.yml
index faa3ee5bb..81f4214d3 100644
--- a/roles/aws/aws_rds/defaults/main.yml
+++ b/roles/aws/aws_rds/defaults/main.yml
@@ -25,8 +25,7 @@ aws_rds:
   master_username: hello # The name of the master user for the DB cluster. Must be 1-16 letters or numbers and begin with a letter.
   master_user_password: hellothere
   publicly_accessible: false # Wether to allocate an IP address
-  # SG names - ID lookup is automatic.
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   rds_cloudwatch_alarms: # name will have the RDS identifier prepended.
     - name: "example_free_storage_space_threshold_{{ _env_type }}_asg"
       description: "Average database free storage space over the last 10 minutes too low."
diff --git a/roles/aws/aws_rds/tasks/main.yml b/roles/aws/aws_rds/tasks/main.yml
index 9353f16d7..416dd30b4 100644
--- a/roles/aws/aws_rds/tasks/main.yml
+++ b/roles/aws/aws_rds/tasks/main.yml
@@ -8,16 +8,6 @@
     description: "{{ aws_rds.description }}"
   register: _aws_rds_rds_subnet_group
 
-- name: Reset security groups info.
-  ansible.builtin.set_fact:
-    _aws_rds_security_group_ids: []
-
-- name: Gather security groups info.
-  ansible.builtin.include_tasks: security_group.yml
-  with_items: "{{ aws_rds.security_groups }}"
-  loop_control:
-    loop_var: security_group
-
 - name: Create RDS parameter group
   amazon.aws.rds_param_group:
     state: present
@@ -73,6 +63,16 @@
     - "'aurora' in aws_rds.engine"
     - aws_rds.aurora_reader
 
+- name: Generate security group information.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_rds.aws_profile }}"
+    region: "{{ aws_rds.region }}"
+    group_names: "{{ aws_rds.security_groups }}"
+    return_type: ids
+  when: aws_rds.security_groups | length > 0
+
 - name: Create RDS instance
   amazon.aws.rds_instance:
     db_instance_identifier: "{{ aws_rds.name }}"
@@ -94,7 +94,7 @@
     #enable_performance_insights: undefined # not required. Whether to enable Performance Insights for the DB instance.
     allocated_storage: "{{ aws_rds.allocated_storage }}"
     max_allocated_storage: "{{ aws_rds.max_allocated_storage }}"
-    vpc_security_group_ids: "{{ _aws_rds_security_group_ids }}"
+    vpc_security_group_ids: "{{ _aws_security_group_list }}" # variable populated by the aws_security_groups role
     profile: "{{ aws_rds.aws_profile }}"
     tags: "{{ aws_rds.tags | combine({ 'Name': aws_rds.name }) }}"
     allow_major_version_upgrade: false
diff --git a/roles/aws/aws_rds/tasks/security_group.yml b/roles/aws/aws_rds/tasks/security_group.yml
deleted file mode 100644
index 6359fc7cf..000000000
--- a/roles/aws/aws_rds/tasks/security_group.yml
+++ /dev/null
@@ -1,12 +0,0 @@
-- name: Gather security group info.
-  amazon.aws.ec2_security_group_info:
-    profile: "{{ aws_rds.aws_profile }}"
-    region: "{{ aws_rds.region }}"
-    filters:
-      group_name: "{{ security_group }}"
-      vpc_id: "{{ _aws_rds_rds_subnet_group.subnet_group.vpc_id }}"
-  register: _aws_rds_security_group_info
-
-- name: Add security group id.
-  ansible.builtin.set_fact:
-    _aws_rds_security_group_ids: "{{ _aws_rds_security_group_ids + [ _aws_rds_security_group_info.security_groups[0].group_id ] }}"
diff --git a/roles/aws/aws_security_groups/README.md b/roles/aws/aws_security_groups/README.md
new file mode 100644
index 000000000..f921b7528
--- /dev/null
+++ b/roles/aws/aws_security_groups/README.md
@@ -0,0 +1,32 @@
+# AWS Security Groups
+This is a helper role for managing AWS security groups. Security groups are created in the [`aws_vpc`](https://github.com/codeenigma/ce-provision/tree/2.x/roles/aws/aws_vpc) role, however different roles and modules require different data when working with security groups. This role allows you to feed in a list of security group names and get back, in private variables, the following data for later use in other places:
+
+```yaml
+- name: Set up lists.
+    ansible.builtin.set_fact:
+      _aws_security_groups: [] # a list of all the security group data requested
+      _aws_security_group_ids: [] # a list of security group IDs
+      _aws_security_group_names: "{{ aws_security_groups.group_names }}" # a list of security group names (in a new var for consistency)
+      _aws_security_group_list: [] # a list in the requested format, either a list of names or a list of IDs, so allow for calling a consistent variable name
+```
+
+This role is used by other roles to provide a consistent experience for developers, you can always provide a list of security group names, no matter what the module being called within a role requires.
+
+<!--TOC-->
+<!--ENDTOC-->
+
+<!--ROLEVARS-->
+## Default variables
+```yaml
+---
+aws_security_groups:
+  aws_profile: "{{ _aws_profile }}"
+  region: "{{ _aws_region }}"
+  group_names:
+    - ssh_open
+    - web_open
+  return_type: names # can be either 'names' or 'ids'
+
+```
+
+<!--ENDROLEVARS-->
diff --git a/roles/aws/aws_security_groups/defaults/main.yml b/roles/aws/aws_security_groups/defaults/main.yml
new file mode 100644
index 000000000..948f9f0ba
--- /dev/null
+++ b/roles/aws/aws_security_groups/defaults/main.yml
@@ -0,0 +1,8 @@
+---
+aws_security_groups:
+  aws_profile: "{{ _aws_profile }}"
+  region: "{{ _aws_region }}"
+  group_names:
+    - ssh_open
+    - web_open
+  return_type: ids # can be either 'names' or 'ids'
diff --git a/roles/aws/aws_security_groups/tasks/main.yml b/roles/aws/aws_security_groups/tasks/main.yml
new file mode 100644
index 000000000..1076d586f
--- /dev/null
+++ b/roles/aws/aws_security_groups/tasks/main.yml
@@ -0,0 +1,30 @@
+---
+- name: Set up lists.
+  ansible.builtin.set_fact:
+    _aws_security_groups: []
+    _aws_security_group_ids: []
+    _aws_security_group_names: "{{ aws_security_groups.group_names }}"
+    _aws_security_group_list: []
+
+- name: Fetch groups.
+  amazon.aws.ec2_security_group_info:
+    profile: "{{ aws_security_groups.aws_profile }}"
+    region: "{{ aws_security_groups.region }}"
+    filters:
+      group-name: "{{ _aws_security_group_names }}"
+  register: _aws_security_groups
+
+- name: Create list of security group IDs.
+  ansible.builtin.set_fact:
+    _aws_security_group_ids: "{{ _aws_security_group_ids + [ item.group_id ] }}"
+  with_items: "{{ _aws_security_groups.security_groups }}"
+
+- name: Set return variable to SG names.
+  ansible.builtin.set_fact:
+    _aws_security_group_list: "{{ _aws_security_group_names }}"
+  when: aws_security_groups.return_type == 'names'
+
+- name: Set return variable to SG IDs.
+  ansible.builtin.set_fact:
+    _aws_security_group_list: "{{ _aws_security_group_ids }}"
+  when: aws_security_groups.return_type == 'ids'
diff --git a/roles/debian/gitlab_runner/defaults/main.yml b/roles/debian/gitlab_runner/defaults/main.yml
index 538b7bcaa..845371447 100644
--- a/roles/debian/gitlab_runner/defaults/main.yml
+++ b/roles/debian/gitlab_runner/defaults/main.yml
@@ -38,7 +38,7 @@ gitlab_runner:
     cluster: "my-cluster" # ECS cluster name
     region: "eu-west-1" # AWS region name
     subnet: "subnet-abcdef123456" # subnet ID
-    security_group: "sg-abcdef123456" # SG ID
+    security_group: "my-security-group" # SG name
     task_definition: "my-task:1" # task definition in format name:revision, if revision is not provided ECS will use latest
     public_ip: "false" # if your containers need a public IP assigning
     version: "1.4.0" # Fargate platform version
diff --git a/roles/debian/gitlab_runner/tasks/main.yml b/roles/debian/gitlab_runner/tasks/main.yml
index a28cb1b59..a418bb001 100644
--- a/roles/debian/gitlab_runner/tasks/main.yml
+++ b/roles/debian/gitlab_runner/tasks/main.yml
@@ -35,6 +35,19 @@
 # gitlab-runner register --url https://gitlab.com/ --registration-token TOKEN_HERE --name fargate-test-runner --run-untagged --executor custom -n
 # This will automatically update /etc/gitlab-runner/config.toml
 
+# Populates the _aws_security_group_list variable used in fargate.toml.j2
+- name: Generate security group information.
+  ansible.builtin.include_role:
+    name: aws/aws_security_groups
+  vars:
+    profile: "{{ aws_elasticache.aws_profile }}"
+    region: "{{ aws_elasticache.region }}"
+    group_names:
+      - "{{ gitlab_runner.fargate.security_group }}"
+    return_type: ids
+  when: gitlab_runner.install_fargate
+  delegate_to: localhost
+
 - name: Create the Fargate driver directory if it does not exist.
   ansible.builtin.file:
     path: /opt/gitlab-runner
diff --git a/roles/debian/gitlab_runner/templates/fargate.toml.j2 b/roles/debian/gitlab_runner/templates/fargate.toml.j2
index c32c3633a..6af78fc5a 100644
--- a/roles/debian/gitlab_runner/templates/fargate.toml.j2
+++ b/roles/debian/gitlab_runner/templates/fargate.toml.j2
@@ -5,7 +5,7 @@ LogFormat = "text"
   Cluster = "{{ gitlab_runner.fargate.cluster }}"
   Region = "{{ gitlab_runner.fargate.region }}"
   Subnet = "{{ gitlab_runner.fargate.subnet }}"
-  SecurityGroup = "{{ gitlab_runner.fargate.security_group }}"
+  SecurityGroup = "{{ _aws_security_group_list[0] }}"
   TaskDefinition = "{{ gitlab_runner.fargate.task_definition }}"
   EnablePublicIP = {{ gitlab_runner.fargate.public_ip }}
   PlatformVersion = "{{ gitlab_runner.fargate.version }}"

From e98af0a0e688a0edb4daa3e24b534261705a6174 Mon Sep 17 00:00:00 2001
From: gregharvey <greg.harvey@gmail.com>
Date: Wed, 22 Nov 2023 19:54:38 +0100
Subject: [PATCH 2/2] Updating documentation.

---
 docs/_Sidebar.md                              |  1 +
 docs/roles/aws/aws_ec2_autoscale_cluster.md   | 11 ++++---
 docs/roles/aws/aws_ec2_with_eip.md            |  2 +-
 docs/roles/aws/aws_efs.md                     |  3 +-
 docs/roles/aws/aws_elasticache.md             |  2 +-
 docs/roles/aws/aws_opensearch.md              |  2 +-
 docs/roles/aws/aws_rds.md                     |  3 +-
 docs/roles/aws/aws_security_groups.md         | 32 +++++++++++++++++++
 docs/roles/debian/gitlab_runner.md            |  2 +-
 roles/aws/aws_ec2_autoscale_cluster/README.md | 11 ++++---
 roles/aws/aws_ec2_with_eip/README.md          |  2 +-
 roles/aws/aws_efs/README.md                   |  3 +-
 roles/aws/aws_elasticache/README.md           |  2 +-
 roles/aws/aws_opensearch/README.md            |  2 +-
 roles/aws/aws_rds/README.md                   |  3 +-
 roles/aws/aws_security_groups/README.md       |  2 +-
 roles/debian/gitlab_runner/README.md          |  2 +-
 17 files changed, 58 insertions(+), 27 deletions(-)
 create mode 100644 docs/roles/aws/aws_security_groups.md

diff --git a/docs/_Sidebar.md b/docs/_Sidebar.md
index 4f93d9c12..16c64086b 100644
--- a/docs/_Sidebar.md
+++ b/docs/_Sidebar.md
@@ -22,6 +22,7 @@
        - [AWS key pair.](/roles/aws/aws_provision_ec2_keypair)
        - [AWS RDS](/roles/aws/aws_rds)
        - [AWS S3 Bucket](/roles/aws/aws_s3_bucket)
+       - [AWS Security Groups](/roles/aws/aws_security_groups)
        - [AWS SNS](/roles/aws/aws_sns)
        - [VPC](/roles/aws/aws_vpc)
        - [Update main route for a given VPC](/roles/aws/aws_vpc_route)
diff --git a/docs/roles/aws/aws_ec2_autoscale_cluster.md b/docs/roles/aws/aws_ec2_autoscale_cluster.md
index 49f419391..00f0cc329 100644
--- a/docs/roles/aws/aws_ec2_autoscale_cluster.md
+++ b/docs/roles/aws/aws_ec2_autoscale_cluster.md
@@ -142,11 +142,12 @@ aws_ec2_autoscale_cluster:
   max_size: 8
   # Security groups for the instances cluster.
   # An internal one will be created automatically, use these vars to provide additional groups
-  cluster_security_groups: [] # used in launch template, must be SG IDs
-  alb_security_groups: [] # edge case, the module supports SG names or IDs
-  efs_security_groups: [] # must be SG names because the role uses the name to find the ID
-  rds_security_groups: [] # must be SG names because the role uses the name to find the ID
-  elasticache_security_groups: [] # must be SG IDs
+  # All security group lists should be a list of security group names, they will be automatically converted to IDs by the aws_security_groups as necessary
+  cluster_security_groups: []
+  alb_security_groups: []
+  efs_security_groups: []
+  rds_security_groups: []
+  elasticache_security_groups: []
   # ALB health checks - these are health check settings applied to the load balancer
   alb_health_check_type: ELB # Uses ALB health checks, set to EC2 to use default AWS instance status checks
   alb_health_check_period: 1200 # Length of time in seconds after a new EC2 instance comes into service that Auto Scaling starts checking its health
diff --git a/docs/roles/aws/aws_ec2_with_eip.md b/docs/roles/aws/aws_ec2_with_eip.md
index e8eedfb1a..e7dbd666d 100644
--- a/docs/roles/aws/aws_ec2_with_eip.md
+++ b/docs/roles/aws/aws_ec2_with_eip.md
@@ -31,7 +31,7 @@ aws_ec2_with_eip:
   root_volume_encrypted: "{{ aws_ami.encrypt_boot }}" # in most cases this should match encrypt_boot in the aws_ami role
   root_volume_delete_on_termination: true
   ebs_optimized: true
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   tags:
     Name: "{{ _domain_name }}"
   # Add an A record tied to the EIP.
diff --git a/docs/roles/aws/aws_efs.md b/docs/roles/aws/aws_efs.md
index 1ebdcbe4b..c197667b0 100644
--- a/docs/roles/aws/aws_efs.md
+++ b/docs/roles/aws/aws_efs.md
@@ -19,8 +19,7 @@ aws_efs:
   wait: true
   # Subnets names.
   subnets: []
-  # SG names - ID lookup is automatic. Note, the assumption is that all subnets have the same SGs.
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   # Whether to encrypt the volume or not.
   encrypt: false
   backup: "{{ _infra_name }}-{{ _env_type }}" # Name of the AWS Backup plan to use to backup the instance.
diff --git a/docs/roles/aws/aws_elasticache.md b/docs/roles/aws/aws_elasticache.md
index 64f1190c2..88b29713d 100644
--- a/docs/roles/aws/aws_elasticache.md
+++ b/docs/roles/aws/aws_elasticache.md
@@ -22,7 +22,7 @@ aws_elasticache:
   elasticache_node_type: cache.t3.medium
   elasticache_nodes: 1
   elasticache_port: 11211
-  elasticache_security_groups: [] # list of security group IDs, @TODO - support names like the EFS role
+  elasticache_security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   #zone: eu-west-1a # AZ where the cluster will reside
 ```
 
diff --git a/docs/roles/aws/aws_opensearch.md b/docs/roles/aws/aws_opensearch.md
index e5fcad7c2..c27085bfa 100644
--- a/docs/roles/aws/aws_opensearch.md
+++ b/docs/roles/aws/aws_opensearch.md
@@ -25,7 +25,7 @@ aws_opensearch:
   subnets:
     - "subnet-aaaaaaaa"
     - "subnet-bbbbbbbb"
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   automated_snapshot_start_hour: 3
   auto_tune: "DISABLED" # not supported when t3's are used
   # List of maintenance schedules to use if auto_tune is set to "ENABLED":
diff --git a/docs/roles/aws/aws_rds.md b/docs/roles/aws/aws_rds.md
index 1e5287a15..a1ce80f9c 100644
--- a/docs/roles/aws/aws_rds.md
+++ b/docs/roles/aws/aws_rds.md
@@ -42,8 +42,7 @@ aws_rds:
   master_username: hello # The name of the master user for the DB cluster. Must be 1-16 letters or numbers and begin with a letter.
   master_user_password: hellothere
   publicly_accessible: false # Wether to allocate an IP address
-  # SG names - ID lookup is automatic.
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   rds_cloudwatch_alarms: # name will have the RDS identifier prepended.
     - name: "example_free_storage_space_threshold_{{ _env_type }}_asg"
       description: "Average database free storage space over the last 10 minutes too low."
diff --git a/docs/roles/aws/aws_security_groups.md b/docs/roles/aws/aws_security_groups.md
new file mode 100644
index 000000000..a99623a50
--- /dev/null
+++ b/docs/roles/aws/aws_security_groups.md
@@ -0,0 +1,32 @@
+# AWS Security Groups
+This is a helper role for managing AWS security groups. Security groups are created in the [`aws_vpc`](https://github.com/codeenigma/ce-provision/tree/2.x/roles/aws/aws_vpc) role, however different roles and modules require different data when working with security groups. This role allows you to feed in a list of security group names and get back, in private variables, the following data for later use in other places:
+
+```yaml
+- name: Set up lists.
+    ansible.builtin.set_fact:
+      _aws_security_groups: [] # a list of all the security group data requested
+      _aws_security_group_ids: [] # a list of security group IDs
+      _aws_security_group_names: "{{ aws_security_groups.group_names }}" # a list of security group names (in a new var for consistency)
+      _aws_security_group_list: [] # a list in the requested format, either a list of names or a list of IDs, so allow for calling a consistent variable name
+```
+
+This role is used by other roles to provide a consistent experience for developers, you can always provide a list of security group names, no matter what the module being called within a role requires.
+
+<!--TOC-->
+<!--ENDTOC-->
+
+<!--ROLEVARS-->
+## Default variables
+```yaml
+---
+aws_security_groups:
+  aws_profile: "{{ _aws_profile }}"
+  region: "{{ _aws_region }}"
+  group_names:
+    - ssh_open
+    - web_open
+  return_type: ids # can be either 'names' or 'ids'
+
+```
+
+<!--ENDROLEVARS-->
diff --git a/docs/roles/debian/gitlab_runner.md b/docs/roles/debian/gitlab_runner.md
index 952f48f3e..52554ced5 100644
--- a/docs/roles/debian/gitlab_runner.md
+++ b/docs/roles/debian/gitlab_runner.md
@@ -50,7 +50,7 @@ gitlab_runner:
     cluster: "my-cluster" # ECS cluster name
     region: "eu-west-1" # AWS region name
     subnet: "subnet-abcdef123456" # subnet ID
-    security_group: "sg-abcdef123456" # SG ID
+    security_group: "my-security-group" # SG name
     task_definition: "my-task:1" # task definition in format name:revision, if revision is not provided ECS will use latest
     public_ip: "false" # if your containers need a public IP assigning
     version: "1.4.0" # Fargate platform version
diff --git a/roles/aws/aws_ec2_autoscale_cluster/README.md b/roles/aws/aws_ec2_autoscale_cluster/README.md
index 49f419391..00f0cc329 100644
--- a/roles/aws/aws_ec2_autoscale_cluster/README.md
+++ b/roles/aws/aws_ec2_autoscale_cluster/README.md
@@ -142,11 +142,12 @@ aws_ec2_autoscale_cluster:
   max_size: 8
   # Security groups for the instances cluster.
   # An internal one will be created automatically, use these vars to provide additional groups
-  cluster_security_groups: [] # used in launch template, must be SG IDs
-  alb_security_groups: [] # edge case, the module supports SG names or IDs
-  efs_security_groups: [] # must be SG names because the role uses the name to find the ID
-  rds_security_groups: [] # must be SG names because the role uses the name to find the ID
-  elasticache_security_groups: [] # must be SG IDs
+  # All security group lists should be a list of security group names, they will be automatically converted to IDs by the aws_security_groups as necessary
+  cluster_security_groups: []
+  alb_security_groups: []
+  efs_security_groups: []
+  rds_security_groups: []
+  elasticache_security_groups: []
   # ALB health checks - these are health check settings applied to the load balancer
   alb_health_check_type: ELB # Uses ALB health checks, set to EC2 to use default AWS instance status checks
   alb_health_check_period: 1200 # Length of time in seconds after a new EC2 instance comes into service that Auto Scaling starts checking its health
diff --git a/roles/aws/aws_ec2_with_eip/README.md b/roles/aws/aws_ec2_with_eip/README.md
index e8eedfb1a..e7dbd666d 100644
--- a/roles/aws/aws_ec2_with_eip/README.md
+++ b/roles/aws/aws_ec2_with_eip/README.md
@@ -31,7 +31,7 @@ aws_ec2_with_eip:
   root_volume_encrypted: "{{ aws_ami.encrypt_boot }}" # in most cases this should match encrypt_boot in the aws_ami role
   root_volume_delete_on_termination: true
   ebs_optimized: true
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   tags:
     Name: "{{ _domain_name }}"
   # Add an A record tied to the EIP.
diff --git a/roles/aws/aws_efs/README.md b/roles/aws/aws_efs/README.md
index 1ebdcbe4b..c197667b0 100644
--- a/roles/aws/aws_efs/README.md
+++ b/roles/aws/aws_efs/README.md
@@ -19,8 +19,7 @@ aws_efs:
   wait: true
   # Subnets names.
   subnets: []
-  # SG names - ID lookup is automatic. Note, the assumption is that all subnets have the same SGs.
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   # Whether to encrypt the volume or not.
   encrypt: false
   backup: "{{ _infra_name }}-{{ _env_type }}" # Name of the AWS Backup plan to use to backup the instance.
diff --git a/roles/aws/aws_elasticache/README.md b/roles/aws/aws_elasticache/README.md
index 64f1190c2..88b29713d 100644
--- a/roles/aws/aws_elasticache/README.md
+++ b/roles/aws/aws_elasticache/README.md
@@ -22,7 +22,7 @@ aws_elasticache:
   elasticache_node_type: cache.t3.medium
   elasticache_nodes: 1
   elasticache_port: 11211
-  elasticache_security_groups: [] # list of security group IDs, @TODO - support names like the EFS role
+  elasticache_security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   #zone: eu-west-1a # AZ where the cluster will reside
 ```
 
diff --git a/roles/aws/aws_opensearch/README.md b/roles/aws/aws_opensearch/README.md
index e5fcad7c2..c27085bfa 100644
--- a/roles/aws/aws_opensearch/README.md
+++ b/roles/aws/aws_opensearch/README.md
@@ -25,7 +25,7 @@ aws_opensearch:
   subnets:
     - "subnet-aaaaaaaa"
     - "subnet-bbbbbbbb"
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   automated_snapshot_start_hour: 3
   auto_tune: "DISABLED" # not supported when t3's are used
   # List of maintenance schedules to use if auto_tune is set to "ENABLED":
diff --git a/roles/aws/aws_rds/README.md b/roles/aws/aws_rds/README.md
index 1e5287a15..a1ce80f9c 100644
--- a/roles/aws/aws_rds/README.md
+++ b/roles/aws/aws_rds/README.md
@@ -42,8 +42,7 @@ aws_rds:
   master_username: hello # The name of the master user for the DB cluster. Must be 1-16 letters or numbers and begin with a letter.
   master_user_password: hellothere
   publicly_accessible: false # Wether to allocate an IP address
-  # SG names - ID lookup is automatic.
-  security_groups: []
+  security_groups: [] # list of security group names, converted to IDs by aws_security_groups role
   rds_cloudwatch_alarms: # name will have the RDS identifier prepended.
     - name: "example_free_storage_space_threshold_{{ _env_type }}_asg"
       description: "Average database free storage space over the last 10 minutes too low."
diff --git a/roles/aws/aws_security_groups/README.md b/roles/aws/aws_security_groups/README.md
index f921b7528..a99623a50 100644
--- a/roles/aws/aws_security_groups/README.md
+++ b/roles/aws/aws_security_groups/README.md
@@ -25,7 +25,7 @@ aws_security_groups:
   group_names:
     - ssh_open
     - web_open
-  return_type: names # can be either 'names' or 'ids'
+  return_type: ids # can be either 'names' or 'ids'
 
 ```
 
diff --git a/roles/debian/gitlab_runner/README.md b/roles/debian/gitlab_runner/README.md
index 952f48f3e..52554ced5 100644
--- a/roles/debian/gitlab_runner/README.md
+++ b/roles/debian/gitlab_runner/README.md
@@ -50,7 +50,7 @@ gitlab_runner:
     cluster: "my-cluster" # ECS cluster name
     region: "eu-west-1" # AWS region name
     subnet: "subnet-abcdef123456" # subnet ID
-    security_group: "sg-abcdef123456" # SG ID
+    security_group: "my-security-group" # SG name
     task_definition: "my-task:1" # task definition in format name:revision, if revision is not provided ECS will use latest
     public_ip: "false" # if your containers need a public IP assigning
     version: "1.4.0" # Fargate platform version