Revert "feat: use security-group module instead of resource (#117)"

This reverts commit 4988650.

Author: nitrocode

README.md

@@ -124,7 +124,7 @@ usage: |2-
     ecs_cluster_arn                    = aws_ecs_cluster.default.arn
     launch_type                        = var.ecs_launch_type
     vpc_id                             = module.vpc.vpc_id
-    security_groups                    = [module.vpc.vpc_default_security_group_id]
+    security_group_ids                 = [module.vpc.vpc_default_security_group_id]
     subnet_ids                         = module.subnets.public_subnet_ids
     tags                               = var.tags
     ignore_changes_task_definition     = var.ignore_changes_task_definition
@@ -138,36 +138,6 @@ usage: |2-
     desired_count                      = var.desired_count
     task_memory                        = var.task_memory
     task_cpu                           = var.task_cpu
-
-    security_group_rules = [
-      {
-        type                     = "egress"
-        from_port                = 0
-        to_port                  = 0
-        protocol                 = -1
-        cidr_blocks              = ["0.0.0.0/0"]
-        source_security_group_id = null
-        description              = "Allow all outbound traffic"
-      },
-      {
-        type                     = "ingress"
-        from_port                = 8
-        to_port                  = 0
-        protocol                 = "icmp"
-        cidr_blocks              = ["0.0.0.0/0"]
-        source_security_group_id = null
-        description              = "Enables ping command from anywhere, see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/security-group-rules-reference.html#sg-rules-ping"
-      },
-      {
-        type                     = "ingress"
-        from_port                = 80
-        to_port                  = 80
-        protocol                 = "tcp"
-        cidr_blocks              = []
-        source_security_group_id = module.vpc.vpc_default_security_group_id
-        description              = "Allow inbound traffic to container port"
-      }
-    ]
   }
   ```
 
@@ -227,5 +197,3 @@ contributors:
   github: nitrocode
 - name: Maxim Mironenko
   github: maximmi
-- name: Vladimir Syromyatnikov
-  github: SweetOps

README.yaml

@@ -47,11 +47,12 @@ module "container_definition" {
 
 module "ecs_alb_service_task" {
   source                             = "../.."
+  alb_security_group                 = module.vpc.vpc_default_security_group_id
   container_definition_json          = module.container_definition.json_map_encoded_list
   ecs_cluster_arn                    = aws_ecs_cluster.default.arn
   launch_type                        = var.ecs_launch_type
   vpc_id                             = module.vpc.vpc_id
-  security_groups                    = [module.vpc.vpc_default_security_group_id]
+  security_group_ids                 = [module.vpc.vpc_default_security_group_id]
   subnet_ids                         = module.subnets.public_subnet_ids
   ignore_changes_task_definition     = var.ignore_changes_task_definition
   network_mode                       = var.network_mode
@@ -64,35 +65,5 @@ module "ecs_alb_service_task" {
   task_memory                        = var.task_memory
   task_cpu                           = var.task_cpu
 
-  security_group_rules = [
-    {
-      type                     = "egress"
-      from_port                = 0
-      to_port                  = 0
-      protocol                 = -1
-      cidr_blocks              = ["0.0.0.0/0"]
-      source_security_group_id = null
-      description              = "Allow all outbound traffic"
-    },
-    {
-      type                     = "ingress"
-      from_port                = 8
-      to_port                  = 0
-      protocol                 = "icmp"
-      cidr_blocks              = ["0.0.0.0/0"]
-      source_security_group_id = null
-      description              = "Enables ping command from anywhere, see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/security-group-rules-reference.html#sg-rules-ping"
-    },
-    {
-      type                     = "ingress"
-      from_port                = 80
-      to_port                  = 80
-      protocol                 = "tcp"
-      cidr_blocks              = []
-      source_security_group_id = module.vpc.vpc_default_security_group_id
-      description              = "Allow inbound traffic to container port"
-    }
-  ]
-
   context = module.this.context
 }

docs/terraform.md

@@ -79,18 +79,8 @@ output "task_role_id" {
 }
 
 output "service_security_group_id" {
-  value       = module.ecs_alb_service_task.security_group_id
   description = "Security Group ID of the ECS task"
-}
-
-output "service_security_group_arn" {
-  value       = module.ecs_alb_service_task.security_group_arn
-  description = "Security Group ARN of the ECS task"
-}
-
-output "service_security_group_name" {
-  value       = module.ecs_alb_service_task.security_group_name
-  description = "Security Group name of the ECS task"
+  value       = module.ecs_alb_service_task.service_security_group_id
 }
 
 output "task_definition_family" {

examples/complete/main.tf

@@ -1,7 +1,6 @@
 locals {
   enabled                 = module.this.enabled
   enable_ecs_service_role = module.this.enabled && var.network_mode != "awsvpc" && length(var.ecs_load_balancers) <= 1
-  security_group_enabled  = module.this.enabled && var.security_group_enabled && var.network_mode == "awsvpc"
 }
 
 module "task_label" {
@@ -259,19 +258,58 @@ resource "aws_iam_role_policy_attachment" "ecs_exec" {
 
 # Service
 ## Security Groups
-module "security_group" {
-  source  = "cloudposse/security-group/aws"
-  version = "0.3.1"
+resource "aws_security_group" "ecs_service" {
+  count       = local.enabled && var.network_mode == "awsvpc" ? 1 : 0
+  vpc_id      = var.vpc_id
+  name        = module.service_label.id
+  description = "Allow ALL egress from ECS service"
+  tags        = module.service_label.tags
 
-  use_name_prefix = var.security_group_use_name_prefix
-  rules           = var.security_group_rules
-  description     = var.security_group_description
-  vpc_id          = var.vpc_id
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+
+resource "aws_security_group_rule" "allow_all_egress" {
+  count             = local.enabled && var.enable_all_egress_rule ? 1 : 0
+  type              = "egress"
+  from_port         = 0
+  to_port           = 0
+  protocol          = "-1"
+  cidr_blocks       = ["0.0.0.0/0"]
+  security_group_id = join("", aws_security_group.ecs_service.*.id)
+}
 
-  enabled = local.security_group_enabled
-  context = module.service_label.context
+resource "aws_security_group_rule" "allow_icmp_ingress" {
+  count             = local.enabled && var.enable_icmp_rule ? 1 : 0
+  description       = "Enables ping command from anywhere, see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/security-group-rules-reference.html#sg-rules-ping"
+  type              = "ingress"
+  from_port         = 8
+  to_port           = 0
+  protocol          = "icmp"
+  cidr_blocks       = ["0.0.0.0/0"]
+  security_group_id = join("", aws_security_group.ecs_service.*.id)
 }
 
+resource "aws_security_group_rule" "alb" {
+  count                    = local.enabled && var.use_alb_security_group ? 1 : 0
+  type                     = "ingress"
+  from_port                = var.container_port
+  to_port                  = var.container_port
+  protocol                 = "tcp"
+  source_security_group_id = var.alb_security_group
+  security_group_id        = join("", aws_security_group.ecs_service.*.id)
+}
+
+resource "aws_security_group_rule" "nlb" {
+  count             = local.enabled && var.use_nlb_cidr_blocks ? 1 : 0
+  type              = "ingress"
+  from_port         = var.nlb_container_port
+  to_port           = var.nlb_container_port
+  protocol          = "tcp"
+  cidr_blocks       = var.nlb_cidr_blocks
+  security_group_id = join("", aws_security_group.ecs_service.*.id)
+}
 
 resource "aws_ecs_service" "ignore_changes_task_definition" {
   count                              = local.enabled && var.ignore_changes_task_definition && ! var.ignore_changes_desired_count ? 1 : 0
@@ -347,7 +385,7 @@ resource "aws_ecs_service" "ignore_changes_task_definition" {
   dynamic "network_configuration" {
     for_each = var.network_mode == "awsvpc" ? ["true"] : []
     content {
-      security_groups  = compact(concat(module.security_group.*.id, var.security_groups))
+      security_groups  = compact(concat(var.security_group_ids, aws_security_group.ecs_service.*.id))
       subnets          = var.subnet_ids
       assign_public_ip = var.assign_public_ip
     }
@@ -602,7 +640,7 @@ resource "aws_ecs_service" "default" {
   dynamic "network_configuration" {
     for_each = var.network_mode == "awsvpc" ? ["true"] : []
     content {
-      security_groups  = compact(concat(module.security_group.*.id, var.security_groups))
+      security_groups  = compact(concat(var.security_group_ids, aws_security_group.ecs_service.*.id))
       subnets          = var.subnet_ids
       assign_public_ip = var.assign_public_ip
     }

examples/complete/outputs.tf

@@ -48,19 +48,9 @@ output "task_role_id" {
   value       = join("", aws_iam_role.ecs_task.*.unique_id)
 }
 
-output "security_group_id" {
-  value       = module.security_group.id
+output "service_security_group_id" {
   description = "Security Group ID of the ECS task"
-}
-
-output "security_group_arn" {
-  value       = module.security_group.arn
-  description = "Security Group ARN of the ECS task"
-}
-
-output "security_group_name" {
-  value       = module.security_group.name
-  description = "Security Group name of the ECS task"
+  value       = join("", aws_security_group.ecs_service.*.id)
 }
 
 output "task_definition_family" {

main.tf

@@ -2,33 +2,21 @@ package test
 
 import (
 	"encoding/json"
-	"math/rand"
-	"strconv"
-	"testing"
-	"time"
-
 	"github.com/gruntwork-io/terratest/modules/terraform"
 	"github.com/stretchr/testify/assert"
+	"testing"
 )
 
 // Test the Terraform module in examples/complete using Terratest.
 func TestExamplesComplete(t *testing.T) {
 	t.Parallel()
 
-	rand.Seed(time.Now().UnixNano())
-
-	randId := strconv.Itoa(rand.Intn(100000))
-	attributes := []string{randId}
-
 	terraformOptions := &terraform.Options{
 		// The path to where our Terraform code is located
 		TerraformDir: "../../examples/complete",
 		Upgrade:      true,
 		// Variables to pass to our Terraform code using -var-file options
 		VarFiles: []string{"fixtures.us-east-2.tfvars"},
-		Vars: map[string]interface{}{
-			"attributes": attributes,
-		},
 	}
 
 	// At the end of the test, run `terraform destroy` to clean up any resources that were created
@@ -69,61 +57,45 @@ func TestExamplesComplete(t *testing.T) {
 	// Run `terraform output` to get the value of an output variable
 	ecsClusterId := terraform.Output(t, terraformOptions, "ecs_cluster_id")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "arn:aws:ecs:us-east-2:126450723953:cluster/eg-test-ecs-alb-service-task-"+randId, ecsClusterId)
+	assert.Equal(t, "arn:aws:ecs:us-east-2:126450723953:cluster/eg-test-ecs-alb-service-task", ecsClusterId)
 
 	// Run `terraform output` to get the value of an output variable
 	ecsClusterArn := terraform.Output(t, terraformOptions, "ecs_cluster_arn")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "arn:aws:ecs:us-east-2:126450723953:cluster/eg-test-ecs-alb-service-task-"+randId, ecsClusterArn)
+	assert.Equal(t, "arn:aws:ecs:us-east-2:126450723953:cluster/eg-test-ecs-alb-service-task", ecsClusterArn)
 
 	// Run `terraform output` to get the value of an output variable
 	ecsExecRolePolicyName := terraform.Output(t, terraformOptions, "ecs_exec_role_policy_name")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "eg-test-ecs-alb-service-task-"+randId+"-exec", ecsExecRolePolicyName)
+	assert.Equal(t, "eg-test-ecs-alb-service-task-exec", ecsExecRolePolicyName)
 
 	// Run `terraform output` to get the value of an output variable
 	serviceName := terraform.Output(t, terraformOptions, "service_name")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "eg-test-ecs-alb-service-task-"+randId, serviceName)
+	assert.Equal(t, "eg-test-ecs-alb-service-task", serviceName)
 
 	// Run `terraform output` to get the value of an output variable
 	taskDefinitionFamily := terraform.Output(t, terraformOptions, "task_definition_family")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "eg-test-ecs-alb-service-task-"+randId, taskDefinitionFamily)
+	assert.Equal(t, "eg-test-ecs-alb-service-task", taskDefinitionFamily)
 
 	// Run `terraform output` to get the value of an output variable
 	taskExecRoleName := terraform.Output(t, terraformOptions, "task_exec_role_name")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "eg-test-ecs-alb-service-task-"+randId+"-exec", taskExecRoleName)
+	assert.Equal(t, "eg-test-ecs-alb-service-task-exec", taskExecRoleName)
 
 	// Run `terraform output` to get the value of an output variable
 	taskExecRoleArn := terraform.Output(t, terraformOptions, "task_exec_role_arn")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "arn:aws:iam::126450723953:role/eg-test-ecs-alb-service-task-"+randId+"-exec", taskExecRoleArn)
+	assert.Equal(t, "arn:aws:iam::126450723953:role/eg-test-ecs-alb-service-task-exec", taskExecRoleArn)
 
 	// Run `terraform output` to get the value of an output variable
 	taskRoleName := terraform.Output(t, terraformOptions, "task_role_name")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "eg-test-ecs-alb-service-task-"+randId+"-task", taskRoleName)
+	assert.Equal(t, "eg-test-ecs-alb-service-task-task", taskRoleName)
 
 	// Run `terraform output` to get the value of an output variable
 	taskRoleArn := terraform.Output(t, terraformOptions, "task_role_arn")
 	// Verify we're getting back the outputs we expect
-	assert.Equal(t, "arn:aws:iam::126450723953:role/eg-test-ecs-alb-service-task-"+randId+"-task", taskRoleArn)
-
-	// Run `terraform output` to get the value of an output variable
-	securityGroupName := terraform.Output(t, terraformOptions, "service_security_group_name")
-	expectedSecurityGroupName := "eg-test-ecs-alb-service-task-" + randId + "-service"
-	// Verify we're getting back the outputs we expect
-	assert.Equal(t, expectedSecurityGroupName, securityGroupName)
-
-	// Run `terraform output` to get the value of an output variable
-	securityGroupID := terraform.Output(t, terraformOptions, "service_security_group_id")
-	// Verify we're getting back the outputs we expect
-	assert.Contains(t, securityGroupID, "sg-", "SG ID should contains substring 'sg-'")
-
-	// Run `terraform output` to get the value of an output variable
-	securityGroupARN := terraform.Output(t, terraformOptions, "service_security_group_arn")
-	// Verify we're getting back the outputs we expect
-	assert.Contains(t, securityGroupARN, "arn:aws:ec2", "SG ID should contains substring 'arn:aws:ec2'")
+	assert.Equal(t, "arn:aws:iam::126450723953:role/eg-test-ecs-alb-service-task-task", taskRoleArn)
 }