From d2408432ad0082035040ef6af4271129c4578a4d Mon Sep 17 00:00:00 2001 From: Ben Whaley Date: Mon, 13 May 2024 15:19:36 -0700 Subject: [PATCH] Updates repo structure to conform to the Terraform Registry requirements (#98) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Updates repo structure to conform to the Terraform Registry requirements * fix path * update readme * Test public ip * Instance state must be running 🤦‍♂️ * Explain name collisions --- LICENSE.txt => LICENSE | 0 README.md | 12 ++++------- ...alternat.conf.tftpl => alternat.conf.tftpl | 0 examples/main.tf | 4 +++- .../lambda.tf => lambda.tf | 2 +- .../terraform-aws-alternat/main.tf => main.tf | 2 +- .../outputs.tf => outputs.tf | 0 test/alternat_test.go | 20 +++++++++++++++---- .../variables.tf => variables.tf | 8 ++++---- .../versions.tf => versions.tf | 0 10 files changed, 29 insertions(+), 19 deletions(-) rename LICENSE.txt => LICENSE (100%) rename modules/terraform-aws-alternat/alternat.conf.tftpl => alternat.conf.tftpl (100%) rename modules/terraform-aws-alternat/lambda.tf => lambda.tf (99%) rename modules/terraform-aws-alternat/main.tf => main.tf (99%) rename modules/terraform-aws-alternat/outputs.tf => outputs.tf (100%) rename modules/terraform-aws-alternat/variables.tf => variables.tf (96%) rename modules/terraform-aws-alternat/versions.tf => versions.tf (100%) diff --git a/LICENSE.txt b/LICENSE similarity index 100% rename from LICENSE.txt rename to LICENSE diff --git a/README.md b/README.md index fb4c9f8..a3dd8e3 100644 --- a/README.md +++ b/README.md @@ -51,11 +51,11 @@ The two main elements of the NAT instance solution are: 1. The NAT instance Auto Scaling Groups, one per zone, with a corresponding standby NAT Gateway 1. The replace-route Lambda function -Both are deployed by the Terraform module located in [`modules/terraform-aws-alternat`](modules/terraform-aws-alternat). +Both are deployed by the Terraform module. ### NAT Instance Auto Scaling Group and Standby NAT Gateway -The solution deploys an Auto Scaling Group (ASG) for each provided public subnet. Each ASG contains a single instance. When the instance boots, the [user data](modules/terraform-aws-alternat/alternat.sh.tftpl) initializes the instance to do the NAT stuff. +The solution deploys an Auto Scaling Group (ASG) for each provided public subnet. Each ASG contains a single instance. When the instance boots, the [user data](alternat.sh.tftpl) initializes the instance to do the NAT stuff. By default, the ASGs are configured with a [maximum instance lifetime](https://docs.aws.amazon.com/autoscaling/ec2/userguide/asg-max-instance-lifetime.html). This is to facilitate periodic replacement of the instance to automate patching. When the maximum instance lifetime is reached (14 days by default), the following occurs: @@ -123,7 +123,7 @@ docker push / ### Use the Terraform Module -Start by reviewing the available [input variables](modules/terraform-aws-alternat/variables.tf). Example usage: +Start by reviewing the available [input variables](variables.tf). Example usage: ```hcl locals { @@ -144,7 +144,7 @@ data "aws_subnet" "subnet" { } module "alternat_instances" { - source = "git::https://github.com/chime/terraform-aws-alternat.git//modules/terraform-aws-alternat?ref=v0.3.3" + source = "chime/alternat/aws?ref=v0.3.3" alternat_image_uri = "0123456789012.dkr.ecr.us-east-1.amazonaws.com/alternat-functions-lambda" alternat_image_tag = "v0.3.3" @@ -204,10 +204,6 @@ If you are using the open source terraform-aws-vpc module, you can set `nat_gate AlterNATively, you can remove the NAT Gateways and their EIPs from your existing configuration and then `terraform import` them to allow alterNAT to manage them. -#### Why isn't this module published on the Terraform registry? - -While we'd like for this to be available on the Terraform Registry, it requires a specific repo naming convention and folder structure that we do not want to adopt. - ### Other Considerations - Read [the Amazon EC2 instance network bandwidth page](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-network-bandwidth.html) carefully. In particular: diff --git a/modules/terraform-aws-alternat/alternat.conf.tftpl b/alternat.conf.tftpl similarity index 100% rename from modules/terraform-aws-alternat/alternat.conf.tftpl rename to alternat.conf.tftpl diff --git a/examples/main.tf b/examples/main.tf index 6459049..1018a77 100644 --- a/examples/main.tf +++ b/examples/main.tf @@ -34,7 +34,9 @@ locals { } module "alternat" { - source = "../modules/terraform-aws-alternat" + # To use Alternat from the Terraform Registry: + # source = "chime/alternat/aws" + source = "./.." create_nat_gateways = var.create_nat_gateways ingress_security_group_cidr_blocks = var.private_subnets diff --git a/modules/terraform-aws-alternat/lambda.tf b/lambda.tf similarity index 99% rename from modules/terraform-aws-alternat/lambda.tf rename to lambda.tf index 166f354..07878e9 100644 --- a/modules/terraform-aws-alternat/lambda.tf +++ b/lambda.tf @@ -11,7 +11,7 @@ locals { data "archive_file" "lambda" { count = var.lambda_package_type == "Zip" ? 1 : 0 type = "zip" - source_dir = "${path.module}/../../functions/replace-route" + source_dir = "${path.module}/functions/replace-route" excludes = ["__pycache__"] output_path = var.lambda_zip_path } diff --git a/modules/terraform-aws-alternat/main.tf b/main.tf similarity index 99% rename from modules/terraform-aws-alternat/main.tf rename to main.tf index dd5529b..8ecea0d 100644 --- a/modules/terraform-aws-alternat/main.tf +++ b/main.tf @@ -193,7 +193,7 @@ data "cloudinit_config" "config" { part { content_type = "text/x-shellscript" - content = file("${path.module}/../../scripts/alternat.sh") + content = file("${path.module}/scripts/alternat.sh") } dynamic "part" { diff --git a/modules/terraform-aws-alternat/outputs.tf b/outputs.tf similarity index 100% rename from modules/terraform-aws-alternat/outputs.tf rename to outputs.tf diff --git a/test/alternat_test.go b/test/alternat_test.go index c8dc317..aa6f33d 100644 --- a/test/alternat_test.go +++ b/test/alternat_test.go @@ -15,6 +15,7 @@ import ( ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" terraws "github.com/gruntwork-io/terratest/modules/aws" + "github.com/gruntwork-io/terratest/modules/logger" "github.com/gruntwork-io/terratest/modules/random" "github.com/gruntwork-io/terratest/modules/retry" "github.com/gruntwork-io/terratest/modules/ssh" @@ -25,6 +26,11 @@ import ( "github.com/stretchr/testify/require" ) +// Maintainer's note: This test will currently cause name collisions if multiple tests run in parallel +// in the same account. This is because the test uses a fixed name prefix for resources. This could be fixed +// by using GetRandomStableRegion and updating some resources (such as IAM role and CloudWatch event name) +// to use a random suffix. + func TestAlternat(t *testing.T) { // Uncomment any of the following lines to skip that part of the test. // This is useful for iterating during test development. @@ -169,7 +175,7 @@ net.ipv4.ip_local_port_range = 1024 65535 // Validate that private route tables have routes to the Internet via NAT Gateway maxRetries := 12 waitTime := 10 * time.Second - retry.DoWithRetry(t, "Validating route through NAT Gateway", maxRetries, waitTime, func() (string, error) { + output := retry.DoWithRetry(t, "Validating route through NAT Gateway", maxRetries, waitTime, func() (string, error) { routeTables, err := getRouteTables(t, ec2Client, vpcID) require.NoError(t, err) for _, rt := range routeTables { @@ -181,6 +187,8 @@ net.ipv4.ip_local_port_range = 1024 65535 } return "All private route tables route through NAT Gateway", nil }) + logger := logger.Logger{} + logger.Logf(t, output) updateEgress(t, ec2Client, sgId, false) }) } @@ -250,6 +258,10 @@ func getNatInstancePublicIp(t *testing.T, ec2Client *ec2.Client) (string, error) Name: aws.String("tag:Name"), Values: []string{namePrefix + "*"}, }, + { + Name: aws.String("instance-state-name"), + Values: []string{"running"}, + }, }, } maxRetries := 6 @@ -260,11 +272,11 @@ func getNatInstancePublicIp(t *testing.T, ec2Client *ec2.Client) (string, error) return "", err } - ip := aws.ToString(result.Reservations[0].Instances[0].PublicIpAddress) - if ip == "" { + publicIp := aws.ToString(result.Reservations[0].Instances[0].PublicIpAddress) + if publicIp == "" { return "", fmt.Errorf("Public IP not found") } - return ip, nil + return publicIp, nil }) return ip, nil diff --git a/modules/terraform-aws-alternat/variables.tf b/variables.tf similarity index 96% rename from modules/terraform-aws-alternat/variables.tf rename to variables.tf index d7130c1..a08639d 100644 --- a/modules/terraform-aws-alternat/variables.tf +++ b/variables.tf @@ -1,5 +1,5 @@ variable "additional_instance_policies" { - description = "Additional policies for the HA NAT instance IAM role." + description = "Additional policies for the Alternat instance IAM role." type = list(object({ policy_name = string policy_json = string @@ -8,13 +8,13 @@ variable "additional_instance_policies" { } variable "alternat_image_tag" { - description = "The tag of the container image for the HA NAT Lambda functions." + description = "The tag of the container image for the Alternat Lambda functions." type = string default = "latest" } variable "alternat_image_uri" { - description = "The URI of the container image for the HA NAT Lambda functions." + description = "The URI of the container image for the Alternat Lambda functions." type = string default = "" } @@ -68,7 +68,7 @@ variable "enable_lambda_endpoint" { } variable "enable_ssm" { - description = "Whether to enable SSM on the HA NAT instances." + description = "Whether to enable SSM on the Alternat instances." type = bool default = true } diff --git a/modules/terraform-aws-alternat/versions.tf b/versions.tf similarity index 100% rename from modules/terraform-aws-alternat/versions.tf rename to versions.tf