search

grem11n / terraform-aws-vpc-peering

Terraform module to setup VPC peering connection
https://registry.terraform.io/modules/grem11n/vpc-peering/aws/latest
Apache License 2.0
126 stars 91 forks source link

Tags the same if overlapping resources #35

Closed tyrken closed 5 years ago

tyrken commented 5 years ago

If peering between two VPCs in the same account AND region, the aws_vpc_peering_connection_accepter and aws_vpc_peering_connection refer to the same thing.

I tried switching to skip the accepter resource in this case and had to use coalesce function everywhere - gave up as looked awful when I was only half way through!

Should supersede #33 and #34 in fixing #32

tyrken commented 5 years ago

Assuming the Travis failure is just as you (sensibly) don't allow arbitrary PRs access to your build secrets (AWS keys)

grem11n commented 5 years ago

Yes, this is exactly the case why tests are failing. I'll run them locally.

grem11n commented 5 years ago

Tests passed. All good.

--- PASS: TestPeeringActive (339.60s)
    --- PASS: TestPeeringActive/SingleAccountSingleRegion (74.98s)
    --- PASS: TestPeeringActive/SingleAccountSingleRegionWithOptions (75.57s)
    --- PASS: TestPeeringActive/SingleAccountMultiRegion (96.46s)
    --- PASS: TestPeeringActive/MultiAccountMultiRegion (92.59s)
PASS
ok      github.com/tyrken/terraform-aws-vpc-peering/test    339.613s

@tyrken, do you plan to implement it for Terraform <0.12 as well? Otherwise, I can do it.

Thank you for the contribution!

tyrken commented 5 years ago

@grem11n I had half a go at this today (but no longer have any TF 0.11 to test it on for real), but got confused by https://github.com/grem11n/terraform-aws-vpc-peering/blob/1e40a7436c35db0949b2efd1774ba9c0b8316ad2/main.tf#L18 from the terraform011 branch. Surely this should default to data.aws_region.peer.name , not ...this...?

Would then plan to introduce the same "Side":"Both" tag if locals calculated regions/accounts were the same, but feel free to go ahead yourself...

tyrken commented 5 years ago

Have added TF v0.11 PR #38 which also IMHO fixes the peer_region issue I commented about above