feat: use security-group module instead of resource (#31)

SweetOps · web-flow · commit 05370576dde5 · 2021-06-15T15:04:33.000-04:00
diff --git a/README.md b/README.md
diff --git a/README.yaml b/README.yaml
@@ -79,6 +79,7 @@ usage: |-
     source = "cloudposse/ec2-instance-group/aws"
     # Cloud Posse recommends pinning every module to a specific version
     # version = "x.x.x"
+
     namespace                   = "eg"
     stage                       = "prod"
     name                        = "app"
@@ -91,8 +92,38 @@ usage: |-
     associate_public_ip_address = true
     additional_ips_count        = 1
     ebs_volume_count            = 2
-    allowed_ports               = [22, 80, 443]
     instance_count              = 3
+
+    security_group_rules = [
+      {
+        type        = "egress"
+        from_port   = 0
+        to_port     = 65535
+        protocol    = "-1"
+        cidr_blocks = ["0.0.0.0/0"]
+      },
+      {
+        type        = "ingress"
+        from_port   = 22
+        to_port     = 22
+        protocol    = "tcp"
+        cidr_blocks = ["0.0.0.0/0"]
+      },
+      {
+        type        = "ingress"
+        from_port   = 80
+        to_port     = 80
+        protocol    = "tcp"
+        cidr_blocks = ["0.0.0.0/0"]
+      },
+      {
+        type        = "ingress"
+        from_port   = 443
+        to_port     = 443
+        protocol    = "tcp"
+        cidr_blocks = ["0.0.0.0/0"]
+      }
+    ]
   }
   ```
 
@@ -127,7 +158,7 @@ contributors:
     github: "osterman"
   - name: "Jamie Nelson"
     github: "Jamie-BitFlight"
-  - name: "Vladimir"
+  - name: "Vladimir Syromyatnikov"
     github: "SweetOps"
   - name: "Andriy Knysh"
     github: "aknysh"
diff --git a/docs/terraform.md b/docs/terraform.md
diff --git a/eni.tf b/eni.tf
@@ -1,19 +1,11 @@
 locals {
-  additional_ips_count = var.associate_public_ip_address && var.instance_enabled && var.additional_ips_count > 0 ? var.additional_ips_count : 0
+  additional_ips_count = var.associate_public_ip_address && module.this.enabled && var.additional_ips_count > 0 ? var.additional_ips_count : 0
 }
 
 resource "aws_network_interface" "additional" {
-  count     = local.additional_ips_count * var.instance_count
-  subnet_id = var.subnet
-
-  security_groups = compact(
-    concat(
-      [
-        var.create_default_security_group ? join("", aws_security_group.default.*.id) : ""
-      ],
-      var.security_groups
-    )
-  )
+  count           = local.additional_ips_count * var.instance_count
+  subnet_id       = var.subnet
+  security_groups = compact(concat(module.security_group.*.id, var.security_groups))
 
   tags       = module.label.tags
   depends_on = [aws_instance.default]
diff --git a/examples/complete/fixtures.us-west-1.tfvars b/examples/complete/fixtures.us-west-1.tfvars
@@ -12,8 +12,6 @@ instance_type = "t2.micro"
 
 instance_count = 2
 
-allowed_ports = [22, 80, 443]
-
 ssh_public_key_path = "/secrets"
 
 generate_ssh_key_pair = true
@@ -29,5 +27,3 @@ root_volume_type = "gp2"
 root_volume_size = 10
 
 delete_on_termination = true
-
-create_default_security_group = true
diff --git a/examples/complete/main.tf b/examples/complete/main.tf
@@ -40,23 +40,52 @@ data "aws_ami" "ubuntu" {
 }
 
 module "ec2_instance_group" {
-  source                        = "../../"
-  region                        = var.region
-  ami                           = data.aws_ami.ubuntu.id
-  ami_owner                     = var.ami_owner
-  vpc_id                        = module.vpc.vpc_id
-  subnet                        = module.subnets.private_subnet_ids[0]
-  security_groups               = [module.vpc.vpc_default_security_group_id]
-  assign_eip_address            = var.assign_eip_address
-  associate_public_ip_address   = var.associate_public_ip_address
-  instance_type                 = var.instance_type
-  instance_count                = var.instance_count
-  allowed_ports                 = var.allowed_ports
-  create_default_security_group = var.create_default_security_group
-  generate_ssh_key_pair         = var.generate_ssh_key_pair
-  root_volume_type              = var.root_volume_type
-  root_volume_size              = var.root_volume_size
-  delete_on_termination         = var.delete_on_termination
+  source                      = "../../"
+  region                      = var.region
+  ami                         = data.aws_ami.ubuntu.id
+  ami_owner                   = var.ami_owner
+  vpc_id                      = module.vpc.vpc_id
+  subnet                      = module.subnets.private_subnet_ids[0]
+  security_groups             = [module.vpc.vpc_default_security_group_id]
+  assign_eip_address          = var.assign_eip_address
+  associate_public_ip_address = var.associate_public_ip_address
+  instance_type               = var.instance_type
+  instance_count              = var.instance_count
+  generate_ssh_key_pair       = var.generate_ssh_key_pair
+  root_volume_type            = var.root_volume_type
+  root_volume_size            = var.root_volume_size
+  delete_on_termination       = var.delete_on_termination
+
+  security_group_rules = [
+    {
+      type        = "egress"
+      from_port   = 0
+      to_port     = 65535
+      protocol    = "-1"
+      cidr_blocks = ["0.0.0.0/0"]
+    },
+    {
+      type        = "ingress"
+      from_port   = 22
+      to_port     = 22
+      protocol    = "tcp"
+      cidr_blocks = ["0.0.0.0/0"]
+    },
+    {
+      type        = "ingress"
+      from_port   = 80
+      to_port     = 80
+      protocol    = "tcp"
+      cidr_blocks = ["0.0.0.0/0"]
+    },
+    {
+      type        = "ingress"
+      from_port   = 443
+      to_port     = 443
+      protocol    = "tcp"
+      cidr_blocks = ["0.0.0.0/0"]
+    }
+  ]
 
   context = module.this.context
 }
diff --git a/examples/complete/outputs.tf b/examples/complete/outputs.tf
@@ -92,3 +92,18 @@ output "instance_count" {
   value       = module.ec2_instance_group.instance_count
   description = "Total number of instances created"
 }
+
+output "security_group_id" {
+  value       = module.ec2_instance_group.security_group_id
+  description = "EC2 instances Security Group ID"
+}
+
+output "security_group_arn" {
+  value       = module.ec2_instance_group.security_group_arn
+  description = "EC2 instances Security Group ARN"
+}
+
+output "security_group_name" {
+  value       = module.ec2_instance_group.security_group_name
+  description = "EC2 instances Security Group name"
+}
diff --git a/examples/complete/variables.tf b/examples/complete/variables.tf
@@ -28,11 +28,6 @@ variable "instance_type" {
   description = "Type of the instance"
 }
 
-variable "allowed_ports" {
-  type        = list(number)
-  description = "List of allowed ingress ports"
-}
-
 variable "ami_owner" {
   type        = string
   description = "Owner of the given AMI"
@@ -53,11 +48,6 @@ variable "delete_on_termination" {
   description = "Whether the volume should be destroyed on instance termination"
 }
 
-variable "create_default_security_group" {
-  type        = bool
-  description = "Create default Security Group with only Egress traffic allowed"
-}
-
 variable "instance_count" {
   type        = number
   description = "Count of ec2 instances to create"
diff --git a/main.tf b/main.tf
@@ -1,13 +1,13 @@
 locals {
-  instance_count       = var.instance_enabled ? var.instance_count : 0
-  security_group_count = var.create_default_security_group ? 1 : 0
-  region               = var.region != "" ? var.region : data.aws_region.default.name
-  root_iops            = var.root_volume_type == "io1" ? var.root_iops : 0
-  ebs_iops             = var.ebs_volume_type == "io1" ? var.ebs_iops : 0
-  availability_zone    = var.availability_zone
-  root_volume_type     = var.root_volume_type != "" ? var.root_volume_type : data.aws_ami.info.root_device_type
-  count_default_ips    = var.associate_public_ip_address && var.assign_eip_address && var.instance_enabled ? var.instance_count : 0
-  ssh_key_pair_path    = var.ssh_key_pair_path == "" ? path.cwd : var.ssh_key_pair_path
+  instance_count         = module.this.enabled ? var.instance_count : 0
+  region                 = var.region != "" ? var.region : data.aws_region.default.name
+  root_iops              = var.root_volume_type == "io1" ? var.root_iops : 0
+  ebs_iops               = var.ebs_volume_type == "io1" ? var.ebs_iops : 0
+  availability_zone      = var.availability_zone
+  root_volume_type       = var.root_volume_type != "" ? var.root_volume_type : data.aws_ami.info.root_device_type
+  count_default_ips      = var.associate_public_ip_address && var.assign_eip_address && module.this.enabled ? var.instance_count : 0
+  ssh_key_pair_path      = var.ssh_key_pair_path == "" ? path.cwd : var.ssh_key_pair_path
+  security_group_enabled = module.this.enabled && var.security_group_enabled
 }
 
 locals {
@@ -98,15 +98,7 @@ resource "aws_instance" "default" {
   source_dest_check           = var.source_dest_check
   ipv6_address_count          = var.ipv6_address_count < 0 ? null : var.ipv6_address_count
   ipv6_addresses              = length(var.ipv6_addresses) > 0 ? var.ipv6_addresses : null
-
-  vpc_security_group_ids = compact(
-    concat(
-      [
-        var.create_default_security_group ? join("", aws_security_group.default.*.id) : ""
-      ],
-      var.security_groups
-    )
-  )
+  vpc_security_group_ids      = compact(concat(module.security_group.*.id, var.security_groups))
 
   root_block_device {
     volume_type           = local.root_volume_type
diff --git a/outputs.tf b/outputs.tf
@@ -45,14 +45,22 @@ output "ssh_key_pem_path" {
 
 output "security_group_ids" {
   description = "ID on the new AWS Security Group associated with creating instance"
-  value = compact(
-    concat(
-      [
-        var.create_default_security_group ? join("", aws_security_group.default.*.id) : ""
-      ],
-      var.security_groups
-    )
-  )
+  value       = compact(concat(module.security_group.*.id, var.security_groups))
+}
+
+output "security_group_id" {
+  value       = module.security_group.id
+  description = "EC2 instances Security Group ID"
+}
+
+output "security_group_arn" {
+  value       = module.security_group.arn
+  description = "EC2 instances Security Group ARN"
+}
+
+output "security_group_name" {
+  value       = module.security_group.name
+  description = "EC2 instances Security Group name"
 }
 
 output "role_names" {
diff --git a/security_group.tf b/security_group.tf
@@ -1,31 +1,12 @@
-resource "aws_security_group" "default" {
-  count       = local.security_group_count
-  name        = module.label.id
-  vpc_id      = var.vpc_id
-  description = "Instance default security group (only egress access is allowed)"
-  tags        = module.label.tags
+module "security_group" {
+  source  = "cloudposse/security-group/aws"
+  version = "0.3.1"
 
-  lifecycle {
-    create_before_destroy = true
-  }
-}
-
-resource "aws_security_group_rule" "egress" {
-  count             = var.create_default_security_group ? 1 : 0
-  type              = "egress"
-  from_port         = 0
-  to_port           = 65535
-  protocol          = "-1"
-  cidr_blocks       = ["0.0.0.0/0"]
-  security_group_id = join("", aws_security_group.default.*.id)
-}
+  use_name_prefix = var.security_group_use_name_prefix
+  rules           = var.security_group_rules
+  description     = var.security_group_description
+  vpc_id          = var.vpc_id
 
-resource "aws_security_group_rule" "ingress" {
-  count             = var.create_default_security_group ? length(compact(var.allowed_ports)) : 0
-  type              = "ingress"
-  from_port         = var.allowed_ports[count.index]
-  to_port           = var.allowed_ports[count.index]
-  protocol          = "tcp"
-  cidr_blocks       = ["0.0.0.0/0"]
-  security_group_id = join("", aws_security_group.default.*.id)
+  enabled = local.security_group_enabled
+  context = module.this.context
 }
diff --git a/test/src/examples_complete_test.go b/test/src/examples_complete_test.go
@@ -1,7 +1,10 @@
 package test
 
 import (
+	"math/rand"
+	"strconv"
 	"testing"
+	"time"
 
 	"github.com/gruntwork-io/terratest/modules/terraform"
 	"github.com/stretchr/testify/assert"
@@ -11,12 +14,20 @@ import (
 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-west-1.tfvars"},
+		Vars: map[string]interface{}{
+			"attributes": attributes,
+		},
 	}
 
 	// At the end of the test, run `terraform destroy` to clean up any resources that were created
@@ -39,4 +50,20 @@ func TestExamplesComplete(t *testing.T) {
 	publicSubnetCidrs := terraform.OutputList(t, terraformOptions, "public_subnet_cidrs")
 	// Verify we're getting back the outputs we expect
 	assert.Equal(t, []string{"172.16.128.0/18", "172.16.192.0/18"}, publicSubnetCidrs)
+
+	// Run `terraform output` to get the value of an output variable
+	securityGroupName := terraform.Output(t, terraformOptions, "security_group_name")
+	expectedSecurityGroupName := "eg-test-ec2-group-test-" + randId
+	// 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, "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, "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'")
 }
diff --git a/test/src/go.mod b/test/src/go.mod
@@ -3,6 +3,6 @@ module github.com/cloudposse/terraform-aws-ec2-instance-group
 go 1.14
 
 require (
-	github.com/gruntwork-io/terratest v0.31.1
+	github.com/gruntwork-io/terratest v0.34.7
 	github.com/stretchr/testify v1.5.1
 )
diff --git a/test/src/go.sum b/test/src/go.sum
diff --git a/variables.tf b/variables.tf

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,6 @@ module github.com/cloudposse/terraform-aws-ec2-instance-group`
`3`	`3`	`go 1.14`
`4`	`4`
`5`	`5`	`require (`
`6`		`- github.com/gruntwork-io/terratest v0.31.1`
	`6`	`+ github.com/gruntwork-io/terratest v0.34.7`
`7`	`7`	`github.com/stretchr/testify v1.5.1`
`8`	`8`	`)`