tf: create vpc and subnet instead of asking user for the details in tfvars #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…fvars Signed-off-by: Pratyush Singhal <psinghal20@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enables Terraform to create its own VPC and public subnet instead of requiring users to supply those IDs via tfvars.
- Added Terraform resources for VPC, Internet Gateway, public subnet, route table, and AZ data source.
- Removed
vpc_idandsubnet_idvariables and references, updating FRR instance and ASG to use the new resources. - Introduced outputs for the created VPC and subnet IDs and cleaned up stage.tfvars defaults.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cloud/aws/vpc.tf | New resources for VPC creation, IGW, public subnet, routing, AZs |
| cloud/aws/inputs.tf | Removed vpc_id/subnet_id variables; updated BGP default range |
| cloud/aws/outputs.tf | Added outputs for vpc_id and subnet_id |
| cloud/aws/frr.tf | Switched FRR router subnet reference to the created subnet |
| cloud/aws/asg.tf | Updated SG and ASG to use created VPC and subnet IDs |
| cloud/aws/tfvars/stage.tfvars | Removed user-supplied VPC/subnet IDs; adjusted BGP listen CIDR |
Comments suppressed due to low confidence (3)
cloud/aws/vpc.tf:3
- [nitpick] Consider parameterizing this CIDR block rather than hard-coding
10.0.0.0/16, so it can be customized per environment.
cidr_block = "10.0.0.0/16"
cloud/aws/vpc.tf:1
- [nitpick] New VPC and subnet resources lack accompanying Terraform tests or validations. Adding
terraform validateor unit tests (e.g., with terratest) could help catch misconfigurations early.
# Create VPC
cloud/aws/outputs.tf:25
- [nitpick] The generic output name
vpc_idmight collide when this module is consumed alongside others. Consider a more specific name likemitm_vpc_id.
output "vpc_id" {
| enable_dns_support = true | ||
|
|
||
| tags = { | ||
| Name = "mitm-vpc" |
There was a problem hiding this comment.
[nitpick] The resource tags are hard-coded. You may want to expose a variable for the VPC name tag to support different naming conventions or environments.
Copilot uses AI. Check for mistakes.
cloud/aws/vpc.tf
Outdated
Comment on lines
21
to
29
| # Create public subnet | ||
| resource "aws_subnet" "public" { | ||
| vpc_id = aws_vpc.main.id | ||
| cidr_block = "10.0.1.0/24" | ||
| availability_zone = data.aws_availability_zones.available.names[0] | ||
| map_public_ip_on_launch = true | ||
|
|
||
| tags = { | ||
| Name = "mitm-public-subnet" |
There was a problem hiding this comment.
Selecting only the first availability zone can limit resilience. Consider distributing subnets across multiple AZs or making this configurable.
Suggested change
| # Create public subnet | |
| resource "aws_subnet" "public" { | |
| vpc_id = aws_vpc.main.id | |
| cidr_block = "10.0.1.0/24" | |
| availability_zone = data.aws_availability_zones.available.names[0] | |
| map_public_ip_on_launch = true | |
| tags = { | |
| Name = "mitm-public-subnet" | |
| # Create public subnets across multiple availability zones | |
| resource "aws_subnet" "public" { | |
| count = var.num_azs | |
| vpc_id = aws_vpc.main.id | |
| cidr_block = cidrsubnet("10.0.0.0/16", 8, count.index) | |
| availability_zone = data.aws_availability_zones.available.names[count.index] | |
| map_public_ip_on_launch = true | |
| tags = { | |
| Name = "mitm-public-subnet-${count.index}" |
Copilot uses AI. Check for mistakes.
cidr block Signed-off-by: Pratyush Singhal <psinghal20@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change to allow Terraform to create VPC and subnet instead of asking users for specific VPC ID or subnet ID.
Purely vibe-coded 😛 so review carefully please