Open marcusit opened 2 years ago
Hi sorry for the delay, I've been crazy on work. I will review this all over the weekend and let you know if I have any questions!
This is great, I would suggesting adding a few additional variables so this is in parity with what's supported in the cdk. (see .env.sample )
For example, users may want to change the mineraft edition, task memory & cpu, and container environmental variables.
It looks like the handling of DNS in the PR differs from what the CDK version is doing.
The CDK version assumes that there is an existing Route 53 hosted zone and it adds the server name as a subdomain of that zone with an apex A record, along with NS glue records in the existing parent zone. The Terraform version is just creating a hosted zone directly along with a non-apex A record. Unless I'm just misunderstanding what's being done here, someone would have to manually add the NS glue records either to an existing (parent) zone of their own or with a registrar in order for this Terraform version to work.
In terms of maintainability, it also feels like the Terraform configuration should not have a different copy of the lambda code used to update desiredCount
, but should instead leverage the code that the CDK version uses so that any feature changes in that lambda would be reflected regardless of the chosen deployment method.
I ran through the step by step instructions and created Terraform code to perform a full deployment, including a separate VPC and subnets.
================== Running instructions:
You will need to have AWS authentication configured. The easiest way is to add your access key and secret into file ~/.aws/credentials https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html
Requires a recent Terraform version, tested on 1.2.8. CLI commands:
cd terraform terraform init terraform apply # <- you will be prompted to enter region, server name and DNS domain.