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

Adding flag to enable or disable the creation of the VPC Peering. #55

Closed albertocastro63 closed 3 years ago

albertocastro63 commented 4 years ago

Under certain conditions this feature is needed so different VPC peerings can be created with the same code in different environments.

For example when development is done in one region and in the other environments there are more than one regional deployments from different source vpcs to one target vpc. In this situation (and other similar situations) this flags allows for the conditional creation of peer connections depending on the environment of deployment.

grem11n commented 4 years ago

@albertocastro63 , I'm not sure if I quite understood the purpose. This module is designed specifically for creation of VPC peering connection. If you want to create one-to-many peering setup, it should be possible to use this module multiple times with the same this VPC (or peer VPC, depending on your setup)

Could you elaborate a bit more on your use-case?

albertocastro63 commented 4 years ago

Of course, thank you for your response.

The flag is motivated by similar flags one can find in terraform modules such as the aws vpc module https://registry.terraform.io/modules/terraform-aws-modules/vpc/aws, the aws security group module https://registry.terraform.io/modules/terraform-aws-modules/security-group/aws, etc. Both have a create input that if set to false it disables the creation of the resouce.

At first this seems superfluous but I have used it to make the code more general so when a resource is not needed then it can be skipped. In my case we have 1 VPC peering connection to create in development that is across accounts (the database for billing reasons was placed in a different account than the development code), while the VPC peering connection in staging and production are in the same account, the information that goes into the modules are different as well (not only the profiles). Having this "flag" would simplify the deployment process allowing me to use only one code base and not 2 as I need to do right now. There may be other work arounds that I have not thought about so if you have any ideas let me know.

Thanks.

grem11n commented 4 years ago

@albertocastro63 , Oki. I have no objections to this change. Your branch has conflicts with the current codebase, though. I'd be happy to merge this PR once these conflicts are solved and tests are successful (will need to run them locally, because GH doesn't inject secrets to run tests from forks).

Thank you for the contribution!

albertocastro63 commented 4 years ago

I will fix the conflicts over the weekend and test.

I also noted a problem with the documentation, there is an input variable that is no longer available: cross_region_peering. I will double check and send a PR if there's indeed a problem.

Thank you for the module, it is very helpful.

Alberto

On Thu, Jul 16, 2020 at 4:36 AM Yurii Rochniak notifications@github.com wrote:

@albertocastro63 https://github.com/albertocastro63 , Oki. I have no objections to this change. Your branch has conflicts with the current codebase, though. I'd be happy to merge this PR once these conflicts are solved and tests are successful (will need to run them locally, because GH doesn't inject secrets to run tests from forks).

Thank you for the contribution!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grem11n/terraform-aws-vpc-peering/pull/55#issuecomment-659251824, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQSDDQHMV2CUYGBCUAM6IDR3232XANCNFSM4OYU7UVQ .

albertocastro63 commented 3 years ago

I am closing this PR since now Terraform 0.13 and up provide the option to set a count parameter at the module level. So now just setting count=0 will have the same effect.