Terraform changes to use existing VPC #22
Conversation
| name = var.ecr_repository_name | ||
| } | ||
| # data "aws_ecr_repository" "service" { | ||
| # count = var.use_ecr ? 1 : 0 |
There was a problem hiding this comment.
This is a good change IMHO 🥳 But let's remove the use_ecs in the variables.tf also then delete this block and keep your other change in this module. Nice work
There was a problem hiding this comment.
We still need use_ecr in other parts of main. For example:
There was a problem hiding this comment.
Doh, I didn't check that. I don't think we need it. What we're basically doing here is saying, "let's just create the role with all the permissions we think it needs for ECR too" instead of having the user tell us at the function call time whether they need it or not. I think that's fine.
But are you comfortable changing all those things? If not just abort all changes and I can handle it in the future
| @@ -0,0 +1,13 @@ | |||
| # region = "us-west-2" | |||
There was a problem hiding this comment.
you shouldn't have to worry about commenting var files out. They won't be used b/c you should be specifically passing them in plan/apply via the -var-file=./vars/<name>.tf argument. Maybe take a look at this document and the last command:
https://github.com/NASA-IMPACT/veda-features-api/blob/main/docs/IACHOWTO.md
terraform/features-api/vars/dev.tf
Outdated
| availability_zones = ["us-west-2a", "us-west-2b"] | ||
| service_port = 8080 | ||
| dns_zone_name = "delta-backend.com" | ||
| dns_subdomain = "firenrt" |
There was a problem hiding this comment.
as we talked about you'll probably want to change the dns_subdomain to ghg or something and check what dns_zone_name is available in MCP for ghg
terraform/features-api/init.tf
Outdated
| } | ||
| } | ||
| backend "s3" { | ||
| bucket = "veda-wfs3-tf-state-bucket" |
There was a problem hiding this comment.
you'll need to manually create a new S3 bucket in this region and change this name so we don't clobber this state bucket
There was a problem hiding this comment.
Nice work duder 🎉
I left specific comments below. Here are some general comments.
-
I'd rename
features-apifolder to something more specific such asghg-features-api-shared-vpcsince your variables will probably change and require us to pass in VPC ids, subnet ids, security group ids, etc, etc -
I'd advise reading over https://github.com/NASA-IMPACT/veda-features-api/blob/main/docs/IACHOWTO.md and especially using
tfenvandworkspacesin your flows. You'll need to change the bucket name ininit.tfbefore you do anything else in that workflow or it won't work
These terraform change allow using an existing VPC instead of creating a new one.