-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Issue #2472] Terraform in ECS #2480
Conversation
node_modules | ||
**/.terraform/** | ||
.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build was pulling in all the .terraform files, which made the build huge. Like 10 GB
COPY --from=top-level-directory bin /app/bin | ||
COPY --from=top-level-directory infra /app/infra | ||
COPY --from=top-level-directory Makefile /app/Makefile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fancy new docker features here, ability to pull from a context above your current folder
@@ -0,0 +1,20 @@ | |||
data "external" "account_ids_by_name" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste
@@ -0,0 +1,7 @@ | |||
module "dev_config" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste and snipped
@@ -0,0 +1,7 @@ | |||
module "staging_config" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste and snipped
@@ -0,0 +1,7 @@ | |||
module "prod_config" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste and snipped
@@ -0,0 +1,31 @@ | |||
output "app_name" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste
@@ -0,0 +1,60 @@ | |||
locals { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste with slight modifications
@@ -0,0 +1,22 @@ | |||
variable "app_name" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste and snipped
@@ -0,0 +1,11 @@ | |||
output "service_config" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste and snipped
@@ -0,0 +1,17 @@ | |||
locals { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste
@@ -0,0 +1,59 @@ | |||
data "aws_iam_role" "github_actions" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste
@@ -0,0 +1,56 @@ | |||
# Make the "image_tag" variable optional so that "terraform plan" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste
@@ -0,0 +1,107 @@ | |||
# docs: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste from the analytics module, which also is "triggered only" (eg. no web server)
@@ -0,0 +1,24 @@ | |||
output "service_endpoint" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste
@@ -0,0 +1,16 @@ | |||
module "secrets" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy paste
build: | ||
docker buildx build \ | ||
--build-context top-level-directory=../ \ | ||
--tag $(notdir $(shell pwd)):latest \ | ||
. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fancy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files you had us focus on look good.
## Summary Relates to #2472, undos #2480 ### Time to review: __0.1 mins__ ## Changes proposed Removes all the ecs-terraform stuff ## Context for reviewers I spent more time on this than my (self imposed) timebox, so I'm swapping to the VPN route instead. If nothing else, it was in fact fun to work on. What is engineering if not the PRs we made along the way.
Summary
Contributes to, but does not solve, #2472
Time to review: 10 mins
Changes proposed
Adds a new ECS service that contains:
I plan on using this ECS service to deploy terraform from inside of our VPC
Context for reviewers
95% of this PR is copy paste, which is just how it is.
The parts that are specific to this PR are:
.dockerignore
ecs-terraform/Dockerfile
ecs-terraform/Makefile