-
Notifications
You must be signed in to change notification settings - Fork 40
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
Adv/feat/aws ecs fargate #14
base: main
Are you sure you want to change the base?
Conversation
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.
i think testing a clean install of this on our dev account is really important so we can figure out exactly what we want the interface to be (what has to be created first?).
to make this super easy would take a lot of work, but ideally someone can just come in with their retool license key and say 'i want a fargate deployment' and run this module to create one.
oh also, you should be able to request a quota increase through a support ticket with high urgency if you're hitting limits with eips or anything.
retention_in_days = var.log_retention_in_days | ||
} | ||
|
||
resource "aws_db_instance" "this" { |
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.
probably should allow people to opt-out of creating a DB with some var and then support the customer passing in the db information through env vars as normal
{"name": "POSTGRES_USER", "value": "${aws_secretsmanager_secret_version.rds_username.secret_string}"}, | ||
{"name": "POSTGRES_PASSWORD", "value": "${aws_secretsmanager_secret_version.rds_password.secret_string}"}, | ||
{"name": "JWT_SECRET", "value": "${random_string.jwt_secret.result}"}, | ||
{"name": "ENCRYPTION_KEY", "value": "${random_string.encryption_key.result}"}, |
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.
i always am weary about randomly generating keys like this because if a customer were to want to keep their db but for some reason re-create their retool containers, they may get a new encryption key generated and nothing will work right :')
typically this stuff should be customer-provided and referenced through aws secrets manager in this case
modules/aws_ecs_fargate/security.tf
Outdated
from_port = "5432" | ||
to_port = "5432" | ||
protocol = "tcp" | ||
cidr_blocks = ["0.0.0.0/0"] |
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.
this should probably be a var too - i doubt people would by default want everything to be able to reach their rds instance
modules/aws_ecs_fargate/main.tf
Outdated
vpc = true | ||
} | ||
|
||
resource "aws_nat_gateway" "retool_nat_gateway" { |
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.
so we expect the customer to come with a public subnet w/ an internet gateway inside of it? I wonder if we should just do this for them?
I recommend creating a networking.tf file that sets this stuff up if these variables are not provided. don't we also need route table rules to make sure traffic from our private subnet actually makes it to this NAT gateway?
i think this is a good example https://dev.betterdoc.org/infrastructure/2020/02/04/setting-up-a-nat-gateway-on-aws-using-terraform.html
Allow choice of subnets
Add new SG for tasks, predefine cross-SG rules, multiple public subnets, move LB to public subnets
…fargate Move RDS into VPC
Add ECS Fargate module