Refactor security-group module (#57)

tedilabs · Nov 7, 2023 · 4aa87dc · 4aa87dc
1 parent bb619dd
commit 4aa87dc
Show file tree

Hide file tree

Showing 15 changed files with 534 additions and 116 deletions.
diff --git a/README.md b/README.md
@@ -43,6 +43,11 @@ Terraform Modules from [this package](https://github.com/tedilabs/terraform-aws-
 - [vpc-simple](./examples/vpc-simple)
 - [vpc-with-ipam](./examples/vpc-with-ipam)
 
+### Security Group
+
+- [security-group-simple](./examples/security-group-simple)
+- [security-group-with-ipv4-cidrs](./examples/security-group-with-ipv4-cidrs)
+
 ### NAT Gateway
 
 - [nat-gateway-public](./examples/nat-gateway-public/)

diff --git a/examples/security-group-simple/main.tf b/examples/security-group-simple/main.tf
@@ -0,0 +1,26 @@
+provider "aws" {
+  region = "us-east-1"
+}
+
+data "aws_vpc" "default" {
+  default = true
+}
+
+
+###################################################
+# Security Group
+###################################################
+
+module "security_group" {
+  source = "../../modules/security-group"
+  # source  = "tedilabs/ipam/aws//modules/security-group"
+  # version = "~> 0.30.0"
+
+  vpc_id = data.aws_vpc.default.id
+
+  name = "hello-world"
+
+  tags = {
+    "project" = "terraform-aws-network-examples"
+  }
+}
diff --git a/examples/security-group-simple/outputs.tf b/examples/security-group-simple/outputs.tf
@@ -0,0 +1,4 @@
+output "security_group" {
+  description = "The Security Group."
+  value       = module.security_group
+}
diff --git a/examples/security-group-simple/versions.tf b/examples/security-group-simple/versions.tf
@@ -0,0 +1,10 @@
+terraform {
+  required_version = "~> 1.5"
+
+  required_providers {
+    aws = {
+      source  = "hashicorp/aws"
+      version = "~> 5.0"
+    }
+  }
+}
diff --git a/examples/security-group-with-ipv4-cidrs/main.tf b/examples/security-group-with-ipv4-cidrs/main.tf
@@ -0,0 +1,50 @@
+provider "aws" {
+  region = "us-east-1"
+}
+
+data "aws_vpc" "default" {
+  default = true
+}
+
+
+###################################################
+# Security Group
+###################################################
+
+module "security_group" {
+  source = "../../modules/security-group"
+  # source  = "tedilabs/ipam/aws//modules/security-group"
+  # version = "~> 0.30.0"
+
+  vpc_id = data.aws_vpc.default.id
+
+  name        = "hello-world-ipv4-cidrs"
+  description = "Sample Security Group with IPv4 CIDRs."
+
+  revoke_rules_on_delete = true
+
+  ingress_rules = [
+    {
+      id          = "tcp/80"
+      description = "Allow HTTP from VPC"
+      from_port   = 80
+      to_port     = 80
+      protocol    = "tcp"
+      ipv4_cidrs  = ["192.168.0.0/16", "10.0.0.0/8", "172.168.0.0/24"]
+    },
+  ]
+  egress_rules = [
+    {
+      id          = "all/all"
+      description = "Allow all traffics to the internet"
+      from_port   = 0
+      to_port     = 0
+      protocol    = "-1"
+      ipv4_cidrs  = ["0.0.0.0/0"]
+    },
+  ]
+
+  tags = {
+    "project" = "terraform-aws-network-examples"
+  }
+}
diff --git a/examples/security-group-with-ipv4-cidrs/outputs.tf b/examples/security-group-with-ipv4-cidrs/outputs.tf
@@ -0,0 +1,4 @@
+output "security_group" {
+  description = "The Security Group."
+  value       = module.security_group
+}
diff --git a/examples/security-group-with-ipv4-cidrs/versions.tf b/examples/security-group-with-ipv4-cidrs/versions.tf
@@ -0,0 +1,10 @@
+terraform {
+  required_version = "~> 1.5"
+
+  required_providers {
+    aws = {
+      source  = "hashicorp/aws"
+      version = "~> 5.0"
+    }
+  }
+}
diff --git a/modules/security-group/README.md b/modules/security-group/README.md
@@ -3,21 +3,22 @@
 This module creates following resources.
 
 - `aws_security_group`
-- `aws_security_group_rule` (optional)
+- `aws_vpc_security_group_ingress_rule` (optional)
+- `aws_vpc_security_group_egress_rule` (optional)
 
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
 ## Requirements
 
 | Name | Version |
 |------|---------|
-| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.5 |
-| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 3.45 |
+| <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.6 |
+| <a name="requirement_aws"></a> [aws](#requirement\_aws) | >= 5.3 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
-| <a name="provider_aws"></a> [aws](#provider\_aws) | 5.19.0 |
+| <a name="provider_aws"></a> [aws](#provider\_aws) | 5.24.0 |
 
 ## Modules
 
@@ -30,8 +31,8 @@ This module creates following resources.
 | Name | Type |
 |------|------|
 | [aws_security_group.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource |
-| [aws_security_group_rule.egress](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource |
-| [aws_security_group_rule.ingress](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule) | resource |
+| [aws_vpc_security_group_egress_rule.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpc_security_group_egress_rule) | resource |
+| [aws_vpc_security_group_ingress_rule.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpc_security_group_ingress_rule) | resource |
 
 ## Inputs
 
@@ -40,10 +41,9 @@ This module creates following resources.
 | <a name="input_name"></a> [name](#input\_name) | (Required) The name of the security group. | `string` | n/a | yes |
 | <a name="input_vpc_id"></a> [vpc\_id](#input\_vpc\_id) | (Required) The ID of the associated VPC. | `string` | n/a | yes |
 | <a name="input_description"></a> [description](#input\_description) | (Optional) The security group description. This field maps to the AWS `GroupDescription` attribute, for which there is no Update API. | `string` | `"Managed by Terraform."` | no |
-| <a name="input_egress_rules"></a> [egress\_rules](#input\_egress\_rules) | (Optional) A list of egress rules in a security group. | `any` | `[]` | no |
-| <a name="input_ingress_rules"></a> [ingress\_rules](#input\_ingress\_rules) | (Optional) A list of ingress rules in a security group. | `any` | `[]` | no |
+| <a name="input_egress_rules"></a> [egress\_rules](#input\_egress\_rules) | (Optional) The configuration for egress rules of the security group. Each block of `egress_rules` as defined below.<br>    (Required) `id` - The ID of the egress rule. This value is only used internally within Terraform code.<br>    (Optional) `description` - The description of the rule.<br>    (Required) `protocol` - The protocol to match. Note that if `protocol` is set to `-1`, it translates to all protocols, all port ranges, and `from_port` and `to_port` values should not be defined.<br>    (Required) `from_port` - The start of port range for the TCP and UDP protocols, or an ICMP/ICMPv6 type.<br>    (Required) `to_port` - The end of port range for the TCP and UDP protocols, or an ICMP/ICMPv6 code.<br>    (Optional) `ipv4_cidrs` - The IPv4 network ranges to allow, in CIDR notation.<br>    (Optional) `ipv6_cidrs` - The IPv6 network ranges to allow, in CIDR notation.<br>    (Optional) `prefix_lists` - The prefix list IDs to allow.<br>    (Optional) `security_groups` - The source security group IDs to allow.<br>    (Optional) `self` - Whether the security group itself will be added as a source to this ingress rule. | <pre>list(object({<br>    id              = string<br>    description     = optional(string, "Managed by Terraform.")<br>    protocol        = string<br>    from_port       = number<br>    to_port         = number<br>    ipv4_cidrs      = optional(list(string), [])<br>    ipv6_cidrs      = optional(list(string), [])<br>    prefix_lists    = optional(list(string), [])<br>    security_groups = optional(list(string), [])<br>    self            = optional(bool, false)<br>  }))</pre> | `[]` | no |
+| <a name="input_ingress_rules"></a> [ingress\_rules](#input\_ingress\_rules) | (Optional) The configuration for ingress rules of the security group. Each block of `ingress_rules` as defined below.<br>    (Required) `id` - The ID of the ingress rule. This value is only used internally within Terraform code.<br>    (Optional) `description` - The description of the rule.<br>    (Required) `protocol` - The protocol to match. Note that if `protocol` is set to `-1`, it translates to all protocols, all port ranges, and `from_port` and `to_port` values should not be defined.<br>    (Required) `from_port` - The start of port range for the TCP and UDP protocols, or an ICMP/ICMPv6 type.<br>    (Required) `to_port` - The end of port range for the TCP and UDP protocols, or an ICMP/ICMPv6 code.<br>    (Optional) `ipv4_cidrs` - The IPv4 network ranges to allow, in CIDR notation.<br>    (Optional) `ipv6_cidrs` - The IPv6 network ranges to allow, in CIDR notation.<br>    (Optional) `prefix_lists` - The prefix list IDs to allow.<br>    (Optional) `security_groups` - The source security group IDs to allow.<br>    (Optional) `self` - Whether the security group itself will be added as a source to this ingress rule. | <pre>list(object({<br>    id              = string<br>    description     = optional(string, "Managed by Terraform.")<br>    protocol        = string<br>    from_port       = number<br>    to_port         = number<br>    ipv4_cidrs      = optional(list(string), [])<br>    ipv6_cidrs      = optional(list(string), [])<br>    prefix_lists    = optional(list(string), [])<br>    security_groups = optional(list(string), [])<br>    self            = optional(bool, false)<br>  }))</pre> | `[]` | no |
 | <a name="input_module_tags_enabled"></a> [module\_tags\_enabled](#input\_module\_tags\_enabled) | (Optional) Whether to create AWS Resource Tags for the module informations. | `bool` | `true` | no |
-| <a name="input_name_prefix"></a> [name\_prefix](#input\_name\_prefix) | (Optional) Creates a unique name beginning with the specified prefix. Conflicts with `name`. | `string` | `null` | no |
 | <a name="input_resource_group_description"></a> [resource\_group\_description](#input\_resource\_group\_description) | (Optional) The description of Resource Group. | `string` | `"Managed by Terraform."` | no |
 | <a name="input_resource_group_enabled"></a> [resource\_group\_enabled](#input\_resource\_group\_enabled) | (Optional) Whether to create Resource Group to find and group AWS resources which are created by this module. | `bool` | `true` | no |
 | <a name="input_resource_group_name"></a> [resource\_group\_name](#input\_resource\_group\_name) | (Optional) The name of Resource Group. A Resource Group name can have a maximum of 127 characters, including letters, numbers, hyphens, dots, and underscores. The name cannot start with `AWS` or `aws`. | `string` | `""` | no |
@@ -55,7 +55,10 @@ This module creates following resources.
 | Name | Description |
 |------|-------------|
 | <a name="output_arn"></a> [arn](#output\_arn) | The ARN of the security group. |
+| <a name="output_description"></a> [description](#output\_description) | The description of the security group. |
+| <a name="output_egress_rules"></a> [egress\_rules](#output\_egress\_rules) | The configuration of the security group egress rules. |
 | <a name="output_id"></a> [id](#output\_id) | The ID of the security group. |
+| <a name="output_ingress_rules"></a> [ingress\_rules](#output\_ingress\_rules) | The configuration of the security group ingress rules. |
 | <a name="output_name"></a> [name](#output\_name) | The name of the security group. |
 | <a name="output_owner_id"></a> [owner\_id](#output\_owner\_id) | The ID of the AWS account that owns the security group. |
 | <a name="output_vpc_id"></a> [vpc\_id](#output\_vpc\_id) | The ID of the associated VPC. |

diff --git a/modules/security-group/main.tf b/modules/security-group/main.tf
@@ -14,11 +14,21 @@ locals {
   } : {}
 }
 
+
+###################################################
+# Security Group
+###################################################
+
+# INFO: Not supported attributes
+# - `name_prefix`
+# INFO: Use a separate resource
+# - `egress`
+# - `ingress`
 resource "aws_security_group" "this" {
   vpc_id = var.vpc_id
 
-  name        = var.name
-  name_prefix = var.name_prefix
+  name = var.name
+  # name_prefix = var.name_prefix
   description = var.description
 
   revoke_rules_on_delete = var.revoke_rules_on_delete
@@ -31,85 +41,3 @@ resource "aws_security_group" "this" {
     var.tags,
   )
 }
-
-
-###################################################
-# Security Group Rules
-###################################################
-
-locals {
-  normalized_ingress_rules = [
-    for rule in var.ingress_rules : {
-      id          = rule.id
-      description = lookup(rule, "description", "Managed by Terraform")
-
-      protocol  = rule.protocol
-      from_port = rule.from_port
-      to_port   = rule.to_port
-
-      cidr_blocks              = try(sort(compact(rule.cidr_blocks)), null)
-      ipv6_cidr_blocks         = try(sort(compact(rule.ipv6_cidr_blocks)), null)
-      prefix_list_ids          = try(sort(compact(rule.prefix_list_ids)), null)
-      source_security_group_id = try(rule.source_security_group_id, null)
-      self                     = try(rule.self, false) ? true : null
-    }
-  ]
-  normalized_egress_rules = [
-    for rule in var.egress_rules : {
-      id          = rule.id
-      description = lookup(rule, "description", "Managed by Terraform")
-
-      protocol  = rule.protocol
-      from_port = rule.from_port
-      to_port   = rule.to_port
-
-      cidr_blocks              = try(sort(compact(rule.cidr_blocks)), null)
-      ipv6_cidr_blocks         = try(sort(compact(rule.ipv6_cidr_blocks)), null)
-      prefix_list_ids          = try(sort(compact(rule.prefix_list_ids)), null)
-      source_security_group_id = try(rule.source_security_group_id, null)
-      self                     = try(rule.self, false) ? true : null
-    }
-  ]
-}
-
-resource "aws_security_group_rule" "ingress" {
-  for_each = {
-    for rule in local.normalized_ingress_rules :
-    rule.id => rule
-  }
-
-  security_group_id = aws_security_group.this.id
-  type              = "ingress"
-  description       = each.value.description
-
-  protocol  = each.value.protocol
-  from_port = each.value.from_port
-  to_port   = each.value.to_port
-
-  cidr_blocks              = each.value.cidr_blocks
-  ipv6_cidr_blocks         = each.value.ipv6_cidr_blocks
-  prefix_list_ids          = each.value.prefix_list_ids
-  source_security_group_id = each.value.source_security_group_id
-  self                     = each.value.self
-}
-
-resource "aws_security_group_rule" "egress" {
-  for_each = {
-    for rule in local.normalized_egress_rules :
-    rule.id => rule
-  }
-
-  security_group_id = aws_security_group.this.id
-  type              = "egress"
-  description       = each.value.description
-
-  protocol  = each.value.protocol
-  from_port = each.value.from_port
-  to_port   = each.value.to_port
-
-  cidr_blocks              = each.value.cidr_blocks
-  ipv6_cidr_blocks         = each.value.ipv6_cidr_blocks
-  prefix_list_ids          = each.value.prefix_list_ids
-  source_security_group_id = each.value.source_security_group_id
-  self                     = each.value.self
-}
diff --git a/modules/security-group/outputs.tf b/modules/security-group/outputs.tf
@@ -13,6 +13,11 @@ output "name" {
   value       = aws_security_group.this.name
 }
 
+output "description" {
+  description = "The description of the security group."
+  value       = aws_security_group.this.description
+}
+
 output "owner_id" {
   description = "The ID of the AWS account that owns the security group."
   value       = aws_security_group.this.owner_id
@@ -22,3 +27,39 @@ output "vpc_id" {
   description = "The ID of the associated VPC."
   value       = aws_security_group.this.vpc_id
 }
+
+output "ingress_rules" {
+  description = <<EOF
+  The configuration of the security group ingress rules.
+  EOF
+  value = {
+    for name, rule in aws_vpc_security_group_ingress_rule.this :
+    name => {
+      id          = rule.id
+      arn         = rule.arn
+      description = rule.description
+
+      protocol  = rule.ip_protocol
+      from_port = rule.from_port
+      to_port   = rule.to_port
+    }
+  }
+}
+
+output "egress_rules" {
+  description = <<EOF
+  The configuration of the security group egress rules.
+  EOF
+  value = {
+    for name, rule in aws_vpc_security_group_egress_rule.this :
+    name => {
+      id          = rule.id
+      arn         = rule.arn
+      description = rule.description
+
+      protocol  = rule.ip_protocol
+      from_port = rule.from_port
+      to_port   = rule.to_port
+    }
+  }
+}