feat: use security-group module to provision AWS SG (#82)

SweetOps · web-flow · commit b4482a174268 · 2021-02-02T23:46:02.000+02:00
diff --git a/README.md b/README.md
@@ -67,6 +67,25 @@ We literally have [*hundreds of terraform modules*][terraform_modules] that are
 
 
 
+## Security & Compliance [<img src="https://cloudposse.com/wp-content/uploads/2020/11/bridgecrew.svg" width="250" align="right" />](https://bridgecrew.io/)
+
+Security scanning is graciously provided by Bridgecrew. Bridgecrew is the leading fully hosted, cloud-native solution providing continuous Terraform security and compliance.
+
+| Benchmark | Description |
+|--------|---------------|
+| [![Infrastructure Security](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/general)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=INFRASTRUCTURE+SECURITY) | Infrastructure Security Compliance |
+| [![CIS KUBERNETES](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/cis_kubernetes)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=CIS+KUBERNETES+V1.5) | Center for Internet Security, KUBERNETES Compliance |
+| [![CIS AWS](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/cis_aws)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=CIS+AWS+V1.2) | Center for Internet Security, AWS Compliance |
+| [![CIS AZURE](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/cis_azure)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=CIS+AZURE+V1.1) | Center for Internet Security, AZURE Compliance |
+| [![PCI-DSS](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/pci)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=PCI-DSS+V3.2) | Payment Card Industry Data Security Standards Compliance |
+| [![NIST-800-53](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/nist)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=NIST-800-53) | National Institute of Standards and Technology Compliance |
+| [![ISO27001](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/iso)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=ISO27001) | Information Security Management System, ISO/IEC 27001 Compliance |
+| [![SOC2](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/soc2)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=SOC2)| Service Organization Control 2 Compliance |
+| [![CIS GCP](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/cis_gcp)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=CIS+GCP+V1.1) | Center for Internet Security, GCP Compliance |
+| [![HIPAA](https://www.bridgecrew.cloud/badges/github/cloudposse/terraform-aws-ec2-instance/hipaa)](https://www.bridgecrew.cloud/link/badge?vcs=github&fullRepo=cloudposse%2Fterraform-aws-ec2-instance&benchmark=HIPAA) | Health Insurance Portability and Accountability Compliance |
+
+
+
 ## Usage
 
 
@@ -120,7 +139,43 @@ module "kafka_instance" {
   stage                       = "dev"
   additional_ips_count        = 1
   ebs_volume_count            = 2
-  allowed_ports               = [22, 80, 443]
+  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"]
+    },
+    {
+      type        = "ingress"
+      from_port   = 53
+      to_port     = 53
+      protocol    = "udp"
+      cidr_blocks = ["0.0.0.0/0"]
+    },
+  ]
 }
 ```
 
@@ -163,8 +218,6 @@ Available targets:
 |------|-------------|------|---------|:--------:|
 | additional\_ips\_count | Count of additional EIPs | `number` | `0` | no |
 | additional\_tag\_map | Additional tags for appending to tags\_as\_list\_of\_maps. Not added to `tags`. | `map(string)` | `{}` | no |
-| allowed\_ports | List of allowed ingress TCP ports | `list(number)` | `[]` | no |
-| allowed\_ports\_udp | List of allowed ingress UDP ports | `list(number)` | `[]` | no |
 | ami | The AMI to use for the instance. By default it is the AMI provided by Amazon with Ubuntu 16.04 | `string` | `""` | no |
 | ami\_owner | Owner of the given AMI (ignored if `ami` unset) | `string` | `""` | no |
 | applying\_period | The period in seconds over which the specified statistic is applied | `number` | `60` | no |
@@ -212,6 +265,7 @@ Available targets:
 | root\_iops | Amount of provisioned IOPS. This must be set if root\_volume\_type is set to `io1` | `number` | `0` | no |
 | root\_volume\_size | Size of the root volume in gigabytes | `number` | `10` | no |
 | root\_volume\_type | Type of root volume. Can be standard, gp2 or io1 | `string` | `"gp2"` | no |
+| security\_group\_rules | A list of maps of Security Group rules. <br>The values of map is fully complated with `aws_security_group_rule` resource. <br>To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule . | `list(any)` | <pre>[<br>  {<br>    "cidr_blocks": [<br>      "0.0.0.0/0"<br>    ],<br>    "from_port": 0,<br>    "protocol": "-1",<br>    "to_port": 65535,<br>    "type": "egress"<br>  }<br>]</pre> | no |
 | security\_groups | List of Security Group IDs allowed to connect to the instance | `list(string)` | `[]` | no |
 | source\_dest\_check | Controls if traffic is routed to the instance when the destination address does not match the instance. Used for NAT or VPNs | `bool` | `true` | no |
 | ssh\_key\_pair | SSH key pair to be provisioned on the instance | `string` | n/a | yes |
diff --git a/README.yaml b/README.yaml
@@ -82,7 +82,43 @@ usage: |-
     stage                       = "dev"
     additional_ips_count        = 1
     ebs_volume_count            = 2
-    allowed_ports               = [22, 80, 443]
+    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"]
+      },
+      {
+        type        = "ingress"
+        from_port   = 53
+        to_port     = 53
+        protocol    = "udp"
+        cidr_blocks = ["0.0.0.0/0"]
+      },
+    ]
   }
   ```
 references:
diff --git a/docs/terraform.md b/docs/terraform.md
@@ -20,8 +20,6 @@
 |------|-------------|------|---------|:--------:|
 | additional\_ips\_count | Count of additional EIPs | `number` | `0` | no |
 | additional\_tag\_map | Additional tags for appending to tags\_as\_list\_of\_maps. Not added to `tags`. | `map(string)` | `{}` | no |
-| allowed\_ports | List of allowed ingress TCP ports | `list(number)` | `[]` | no |
-| allowed\_ports\_udp | List of allowed ingress UDP ports | `list(number)` | `[]` | no |
 | ami | The AMI to use for the instance. By default it is the AMI provided by Amazon with Ubuntu 16.04 | `string` | `""` | no |
 | ami\_owner | Owner of the given AMI (ignored if `ami` unset) | `string` | `""` | no |
 | applying\_period | The period in seconds over which the specified statistic is applied | `number` | `60` | no |
@@ -69,6 +67,7 @@
 | root\_iops | Amount of provisioned IOPS. This must be set if root\_volume\_type is set to `io1` | `number` | `0` | no |
 | root\_volume\_size | Size of the root volume in gigabytes | `number` | `10` | no |
 | root\_volume\_type | Type of root volume. Can be standard, gp2 or io1 | `string` | `"gp2"` | no |
+| security\_group\_rules | A list of maps of Security Group rules. <br>The values of map is fully complated with `aws_security_group_rule` resource. <br>To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule . | `list(any)` | <pre>[<br>  {<br>    "cidr_blocks": [<br>      "0.0.0.0/0"<br>    ],<br>    "from_port": 0,<br>    "protocol": "-1",<br>    "to_port": 65535,<br>    "type": "egress"<br>  }<br>]</pre> | no |
 | security\_groups | List of Security Group IDs allowed to connect to the instance | `list(string)` | `[]` | no |
 | source\_dest\_check | Controls if traffic is routed to the instance when the destination address does not match the instance. Used for NAT or VPNs | `bool` | `true` | no |
 | ssh\_key\_pair | SSH key pair to be provisioned on the instance | `string` | n/a | yes |
diff --git a/eni.tf b/eni.tf
@@ -8,9 +8,7 @@ resource "aws_network_interface" "additional" {
 
   security_groups = compact(
     concat(
-      [
-        var.create_default_security_group ? join("", aws_security_group.default.*.id) : ""
-      ],
+      formatlist("%s", module.default_sg.id),
       var.security_groups
     )
   )
diff --git a/examples/complete/fixtures.us-east-2.tfvars b/examples/complete/fixtures.us-east-2.tfvars
@@ -16,6 +16,42 @@ associate_public_ip_address = true
 
 instance_type = "t3.micro"
 
-allowed_ports = [22, 80, 443]
+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"]
+  },
+  {
+    type        = "ingress"
+    from_port   = 53
+    to_port     = 53
+    protocol    = "udp"
+    cidr_blocks = ["0.0.0.0/0"]
+  },
+]
 
 ssh_public_key_path = "/secrets"
diff --git a/examples/complete/main.tf b/examples/complete/main.tf
@@ -82,8 +82,7 @@ module "ec2_instance" {
   assign_eip_address          = var.assign_eip_address
   associate_public_ip_address = var.associate_public_ip_address
   instance_type               = var.instance_type
-  allowed_ports               = var.allowed_ports
-  allowed_ports_udp           = var.allowed_ports_udp
+  security_group_rules        = var.security_group_rules
   instance_profile            = aws_iam_instance_profile.test.name
 
   context = module.this.context
diff --git a/examples/complete/variables.tf b/examples/complete/variables.tf
@@ -23,19 +23,16 @@ variable "instance_type" {
   description = "The type of the instance"
 }
 
-variable "allowed_ports" {
-  type        = list(number)
-  description = "List of allowed ingress TCP ports"
-  default     = []
-}
-
-variable "allowed_ports_udp" {
-  type        = list(number)
-  description = "List of allowed ingress UDP ports"
-  default     = []
-}
-
 variable "ssh_public_key_path" {
   type        = string
   description = "Path to SSH public key directory (e.g. `/secrets`)"
 }
+
+variable "security_group_rules" {
+  type        = list(any)
+  description = <<-EOT
+    A list of maps of Security Group rules. 
+    The values of map is fully complated with `aws_security_group_rule` resource. 
+    To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule .
+  EOT
+}
diff --git a/main.tf b/main.tf
@@ -3,7 +3,7 @@ locals {
   # create an instance profile if the instance is enabled and we aren't given one to use
   instance_profile_count = module.this.enabled ? (length(var.instance_profile) > 0 ? 0 : 1) : 0
   instance_profile       = local.instance_profile_count == 0 ? var.instance_profile : join("", aws_iam_instance_profile.default.*.name)
-  security_group_count   = var.create_default_security_group ? 1 : 0
+  security_group_enabled = module.this.enabled && var.create_default_security_group ? true : false
   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"
@@ -121,9 +121,7 @@ resource "aws_instance" "default" {
 
   vpc_security_group_ids = compact(
     concat(
-      [
-        var.create_default_security_group ? join("", aws_security_group.default.*.id) : "",
-      ],
+      formatlist("%s", module.default_sg.id),
       var.security_groups
     )
   )
diff --git a/outputs.tf b/outputs.tf
@@ -42,7 +42,7 @@ output "security_group_ids" {
   description = "IDs on the AWS Security Groups associated with the instance"
   value = compact(
     concat(
-      [var.create_default_security_group == true ? join("", aws_security_group.default.*.id) : ""],
+      formatlist("%s", module.default_sg.id),
       var.security_groups
     )
   )
diff --git a/security_group.tf b/security_group.tf
@@ -1,41 +1,9 @@
-resource "aws_security_group" "default" {
-  count       = local.security_group_count
-  name        = module.this.id
-  vpc_id      = var.vpc_id
-  description = "Instance default security group (only egress access is allowed)"
-  tags        = module.this.tags
-
-  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)
-}
-
-resource "aws_security_group_rule" "ingress_tcp" {
-  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)
-}
-
-resource "aws_security_group_rule" "ingress_udp" {
-  count             = var.create_default_security_group ? length(compact(var.allowed_ports_udp)) : 0
-  type              = "ingress"
-  from_port         = var.allowed_ports_udp[count.index]
-  to_port           = var.allowed_ports_udp[count.index]
-  protocol          = "udp"
-  cidr_blocks       = ["0.0.0.0/0"]
-  security_group_id = join("", aws_security_group.default.*.id)
+module "default_sg" {
+  source  = "cloudposse/security-group/aws"
+  version = "0.1.0"
+  rules   = var.security_group_rules
+  vpc_id  = var.vpc_id
+
+  enabled = local.security_group_enabled
+  context = module.this.context
 }
diff --git a/variables.tf b/variables.tf
@@ -44,16 +44,22 @@ variable "security_groups" {
   default     = []
 }
 
-variable "allowed_ports" {
-  type        = list(number)
-  description = "List of allowed ingress TCP ports"
-  default     = []
-}
-
-variable "allowed_ports_udp" {
-  type        = list(number)
-  description = "List of allowed ingress UDP ports"
-  default     = []
+variable "security_group_rules" {
+  type = list(any)
+  default = [
+    {
+      type        = "egress"
+      from_port   = 0
+      to_port     = 65535
+      protocol    = "-1"
+      cidr_blocks = ["0.0.0.0/0"]
+    }
+  ]
+  description = <<-EOT
+    A list of maps of Security Group rules. 
+    The values of map is fully complated with `aws_security_group_rule` resource. 
+    To get more info see https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule .
+  EOT
 }
 
 variable "subnet" {
@@ -287,4 +293,4 @@ variable "kms_key_id" {
   type        = string
   default     = null
   description = "KMS key ID used to encrypt EBS volume. When specifying kms_key_id, ebs_volume_encrypted needs to be set to true"
-}
+}

Original file line number	Diff line number	Diff line change
`@@ -8,9 +8,7 @@ resource "aws_network_interface" "additional" {`
`8`	`8`
`9`	`9`	`security_groups = compact(`
`10`	`10`	`concat(`
`11`		`- [`
`12`		`- var.create_default_security_group ? join("", aws_security_group.default.*.id) : ""`
`13`		`- ],`
	`11`	`+ formatlist("%s", module.default_sg.id),`
`14`	`12`	`var.security_groups`
`15`	`13`	`)`
`16`	`14`	`)`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ output "security_group_ids" {`
`42`	`42`	`description = "IDs on the AWS Security Groups associated with the instance"`
`43`	`43`	`value = compact(`
`44`	`44`	`concat(`
`45`		`- [var.create_default_security_group == true ? join("", aws_security_group.default.*.id) : ""],`
	`45`	`+ formatlist("%s", module.default_sg.id),`
`46`	`46`	`var.security_groups`
`47`	`47`	`)`
`48`	`48`	`)`