Removed bare default security group resource creation in favor of the…

main.tf

@@ -15,85 +15,77 @@ locals {
   ignore_admin_credentials = var.replication_source_identifier != "" || var.snapshot_identifier != null
   reserved_instance_engine = var.engine
   use_reserved_instances   = var.use_reserved_instances && !local.is_serverless
+
+  create_security_group = local.enabled && var.create_security_group
+
+  # Allow from CIDR blocks on the DB port
+  cidr_rules = length(var.allowed_cidr_blocks) == 0 ? [] : [
+    {
+      key                      = "db-cidrs"
+      type                     = "ingress"
+      from_port                = var.db_port
+      to_port                  = var.db_port
+      protocol                 = "tcp"
+      self                     = false
+      source_security_group_id = null
+      cidr_blocks              = var.allowed_cidr_blocks
+      description              = "Allow DB access from CIDR blocks"
+    }
+  ]
+  # Allow from security groups on the DB port
+  sg_rules = [
+    for sg in var.allowed_security_groups : {
+      key                      = "db-sg-${sg}"
+      type                     = "ingress"
+      from_port                = var.db_port
+      to_port                  = var.db_port
+      protocol                 = "tcp"
+      self                     = false
+      source_security_group_id = sg
+      cidr_blocks              = []
+      description              = "Allow DB access from SG ${sg}"
+    }
+  ]
+  # Allow intra-cluster traffic on DB port (self-referencing rule)
+  self_rule = var.intra_security_group_traffic_enabled == false ? [] : [
+   {
+     key                      = "db-self"
+     type                     = "ingress"
+     from_port                = var.db_port
+     to_port                  = var.db_port
+     protocol                 = "tcp"
+     self                     = true
+     source_security_group_id = null
+     cidr_blocks              = []
+     description              = "Allow intra-cluster DB traffic"
+   }
+ ]
+
+ security_group_rules = concat(local.self_rule, local.cidr_rules, local.sg_rules)
 }
 
 data "aws_partition" "current" {
   count = local.enabled ? 1 : 0
 }
 
-# TODO: Use cloudposse/security-group module
-resource "aws_security_group" "default" {
-  count       = local.enabled ? 1 : 0
-  name        = module.this.id
-  description = "Allow inbound traffic from Security Groups and CIDRs"
-  vpc_id      = var.vpc_id
-  tags        = module.this.tags
-}
+module "security_group" {
+  source  = "cloudposse/security-group/aws"
+  version = "2.3.0"
 
-resource "aws_security_group_rule" "ingress_security_groups" {
-  count                    = local.enabled ? length(var.security_groups) : 0
-  description              = "Allow inbound traffic from existing security groups"
-  type                     = "ingress"
-  from_port                = var.db_port
-  to_port                  = var.db_port
-  protocol                 = "tcp"
-  source_security_group_id = var.security_groups[count.index]
-  security_group_id        = join("", aws_security_group.default[*].id)
-}
+  enabled                    = local.create_security_group
+  name                       = module.this.id
+  security_group_description = "Security group for ${module.this.id} RDS cluster"
+  vpc_id                     = var.vpc_id
 
-resource "aws_security_group_rule" "traffic_inside_security_group" {
-  count             = local.enabled && var.intra_security_group_traffic_enabled ? 1 : 0
-  description       = "Allow traffic between members of the database security group"
-  type              = "ingress"
-  from_port         = var.db_port
-  to_port           = var.db_port
-  protocol          = "tcp"
-  self              = true
-  security_group_id = join("", aws_security_group.default[*].id)
-}
-
-resource "aws_security_group_rule" "ingress_cidr_blocks" {
-  count             = local.enabled && length(var.allowed_cidr_blocks) > 0 ? 1 : 0
-  description       = "Allow inbound traffic from existing CIDR blocks"
-  type              = "ingress"
-  from_port         = var.db_port
-  to_port           = var.db_port
-  protocol          = "tcp"
-  cidr_blocks       = var.allowed_cidr_blocks
-  security_group_id = join("", aws_security_group.default[*].id)
-}
-
-resource "aws_security_group_rule" "ingress_ipv6_cidr_blocks" {
-  count             = local.enabled && length(var.allowed_ipv6_cidr_blocks) > 0 ? 1 : 0
-  description       = "Allow inbound traffic from existing CIDR blocks"
-  type              = "ingress"
-  from_port         = var.db_port
-  to_port           = var.db_port
-  protocol          = "tcp"
-  ipv6_cidr_blocks  = var.allowed_ipv6_cidr_blocks
-  security_group_id = join("", aws_security_group.default[*].id)
-}
-
-resource "aws_security_group_rule" "egress" {
-  count             = local.enabled && var.egress_enabled ? 1 : 0
-  description       = "Allow outbound traffic"
-  type              = "egress"
-  from_port         = 0
-  to_port           = 0
-  protocol          = "-1"
-  cidr_blocks       = ["0.0.0.0/0"]
-  security_group_id = join("", aws_security_group.default[*].id)
-}
+  # Supply rules built from existing inputs
+  rules                         = local.security_group_rules
+  allow_all_egress              = var.egress_enabled
+  preserve_security_group_id    = false
+  create_before_destroy         = var.security_group_create_before_destroy
+  security_group_create_timeout = var.security_group_create_timeout
+  security_group_delete_timeout = var.security_group_delete_timeout
 
-resource "aws_security_group_rule" "egress_ipv6" {
-  count             = local.enabled && var.egress_enabled ? 1 : 0
-  description       = "Allow outbound ipv6 traffic"
-  type              = "egress"
-  from_port         = 0
-  to_port           = 0
-  protocol          = "-1"
-  ipv6_cidr_blocks  = ["::/0"]
-  security_group_id = join("", aws_security_group.default[*].id)
+  context = module.this.context
 }
 
 data "aws_rds_reserved_instance_offering" "default" {
@@ -145,7 +137,7 @@ resource "aws_rds_cluster" "primary" {
   kms_key_id                            = var.kms_key_arn
   source_region                         = var.source_region
   snapshot_identifier                   = var.snapshot_identifier
-  vpc_security_group_ids                = compact(flatten([join("", aws_security_group.default[*].id), var.vpc_security_group_ids]))
+  vpc_security_group_ids                = compact(concat([module.security_group.id], var.vpc_security_group_ids))
   preferred_maintenance_window          = var.maintenance_window
   network_type                          = var.network_type
   db_subnet_group_name                  = join("", aws_db_subnet_group.default[*].name)
@@ -171,7 +163,7 @@ resource "aws_rds_cluster" "primary" {
   depends_on = [
     aws_db_subnet_group.default,
     aws_rds_cluster_parameter_group.default,
-    aws_security_group.default,
+    # aws_security_group.default,
   ]
 
   dynamic "s3_import" {
@@ -253,7 +245,7 @@ resource "aws_rds_cluster" "secondary" {
   kms_key_id                          = var.kms_key_arn
   source_region                       = var.source_region
   snapshot_identifier                 = var.snapshot_identifier
-  vpc_security_group_ids              = compact(flatten([join("", aws_security_group.default[*].id), var.vpc_security_group_ids]))
+  vpc_security_group_ids              = compact(concat([module.security_group.id], var.vpc_security_group_ids))
   preferred_maintenance_window        = var.maintenance_window
   network_type                        = var.network_type
   db_subnet_group_name                = join("", aws_db_subnet_group.default[*].name)
@@ -276,7 +268,7 @@ resource "aws_rds_cluster" "secondary" {
     aws_db_subnet_group.default,
     aws_db_parameter_group.default,
     aws_rds_cluster_parameter_group.default,
-    aws_security_group.default,
+    # aws_security_group.default,
   ]
 
   dynamic "scaling_configuration" {

outputs.tf

@@ -71,17 +71,17 @@ output "cluster_security_groups" {
 }
 
 output "security_group_id" {
-  value       = join("", aws_security_group.default[*].id)
+  value       = module.security_group.id
   description = "Security Group ID"
 }
 
 output "security_group_arn" {
-  value       = join("", aws_security_group.default[*].arn)
+  value       = module.security_group.arn
   description = "Security Group ARN"
 }
 
 output "security_group_name" {
-  value       = join("", aws_security_group.default[*].name)
+  value       = module.security_group.name
   description = "Security Group name"
 }
 

variables.tf

@@ -8,7 +8,7 @@ variable "zone_id" {
     EOT
 }
 
-variable "security_groups" {
+variable "allowed_security_groups" {
   type        = list(string)
   default     = []
   description = "List of security groups to be allowed to connect to the DB instance"
@@ -505,11 +505,19 @@ variable "enable_http_endpoint" {
   default     = false
 }
 
+variable "create_security_group" {
+  type        = bool
+  default     = true
+  description = "Set `true` to create and configure a new security group. If false, `vpc_security_group_ids` must be provided."
+}
+
 variable "vpc_security_group_ids" {
   type        = list(string)
-  description = "Additional security group IDs to apply to the cluster, in addition to the provisioned default security group with ingress traffic from existing CIDR blocks and existing security groups"
-
-  default = []
+  description = <<-EOT
+    Additional security group IDs to apply to the cluster, in addition to the provisioned default security group with ingress traffic from existing CIDR blocks and existing security groups.
+    These security groups will not be modified and, if `create_security_group` is `false`, must have rules providing the desired access.
+  EOT
+  default     = []
 }
 
 variable "ca_cert_identifier" {
@@ -613,3 +621,46 @@ variable "rds_ri_reservation_id" {
   default     = null
   description = "Customer-specified identifier to track the reservation of the reserved DB instance."
 }
+
+variable "security_group_create_before_destroy" {
+  type        = bool
+  default     = true
+  description = <<-EOT
+    Set `true` to enable Terraform `create_before_destroy` behavior on the created security group.
+    We only recommend setting this `false` if you are upgrading this module and need to keep
+    the existing security group from being replaced.
+    Note that changing this value will always cause the security group to be replaced.
+    EOT
+}
+variable "security_group_create_timeout" {
+  type        = string
+  default     = "10m"
+  description = "How long to wait for the security group to be created."
+}
+
+variable "security_group_delete_timeout" {
+  type        = string
+  default     = "15m"
+  description = <<-EOT
+    How long to retry on `DependencyViolation` errors during security group deletion.
+    EOT
+}
+
+variable "allowed_ipv6_prefix_list_ids" {
+  type        = list(string)
+  default     = []
+  description = <<-EOT
+    A list of IPv6 Prefix Lists IDs to allow access to the security group created by this module.
+    The length of this list must be known at "plan" time.
+    EOT
+}
+
+variable "security_group_name" {
+  type        = list(string)
+  default     = []
+  description = <<-EOT
+    The name to assign to the created security group. Must be unique within the VPC.
+    If not provided, will be derived from the `null-label.context` passed in.
+    If `create_before_destroy` is true, will be used as a name prefix.
+    EOT
+}