search

cloudposse / terraform-aws-vpc-peering-multi-account

Terraform module to provision a VPC peering across multiple VPCs in different accounts by using multiple providers
https://cloudposse.com/accelerate
Apache License 2.0
129 stars 92 forks source link

fix: use filtered subnet cidr blocks instead of the VPC cidr #83

Closed bmbferreira closed 3 days ago

bmbferreira commented 8 months ago

what

This PR changes the module to start using the filtered subnet cidrs to create the routes, instead of using the whole VPC cidr.

why

Currently even when filtering the accepter or requester subnets by tags, the route table on the requester/accepter side is always created to the vpc cidr and doesn't create individual routes for each filtered subnet. This is not flexible enough as we might need for example peering to two different VPCs with overlapping CIDRs and we might want to use the subnet cidrs to be able to route to individual cidrs within the subnet, dodging the VPC cidr overlapping.

references

hans-d commented 7 months ago

/terratest

bmbferreira commented 7 months ago

@hans-d any plans to merge this? the test failure doesn't seem to be related with my change, as I see the same test failin in other PRs as well: https://github.com/cloudposse/actions/actions/runs/8132588929/job/22223186662

hans-d commented 7 months ago

Unfortunately, I cannot bypass this check. Getting some internal visability on these failing tests as well, as they are now popping up in various places (for various different reasons). Once the blocking bits are gone (soon I hope), this will be revisited.

bmbferreira commented 2 months ago

@joe-niland @dotCipher Hi! What can I do to have this reviewed? Thank you

bmbferreira commented 2 weeks ago

@goruha just fixed format that was failing. Can you check if the PR is ok to merge? thank you

bmbferreira commented 2 weeks ago

@bmbferreira why do not we have ?

data "aws_subnet" "requester" {

Fixed. I applied the same logic to the requester as well. I also did an improvement to fallback to the VPC cidr if no tag filters are passed as inputs. I think it only makes sense to use the filtered subnet cidr blocks if we are actually filtering it. If not we can continue using the vpc cidr. WDYT?

goruha commented 1 week ago

@bmbferreira I have one request for change. Beside of that it looks fine

goruha commented 1 week ago

/terratest

bmbferreira commented 1 week ago

/terratest

@goruha done!

goruha commented 3 days ago

/terratest

goruha commented 3 days ago

@bmbferreira Thanks for your contribution

goruha commented 3 days ago

/terratest

github-actions[bot] commented 3 days ago

These changes were released in v1.0.0.