Make this module work with terraform 0.13

[eliranw]

Hi, we would like to use this module in TF 0.13 with an outer count module, outside providers, etc. I believe some small changes should be made to make this work

[grem11n]

Tried to run the code from examples with Terraform 0.13 here: https://github.com/grem11n/terraform-aws-vpc-peering/pull/60

There is an issue with one of the tests but seems to be unrelated

UPD: this was an interim issue indeed. Fixed after a re-run

[eliranw]

I'm not sure if the change I did can fix, but I need to work with this module using depends_on = [], and it currently doesn't work because of the inner providers.

[grem11n]

I see. Ok, this is something I need to investigate. Basically, it looks like an ability to specify AWS tokens explicitly as variables was lost somewhere. Therefore, the tests failed.

I need to check, how to move the providers outside of the module.

[grem11n]

@eliranw , please, check the configuration from https://github.com/grem11n/terraform-aws-vpc-peering/pull/60 Does it fix your issue? According to this discussion, proxy provider configuration is required to explicitly pass the providers to the module. Basically, I followed this doc

Also, could you elaborate with some code examples / workflows, which involve depends_on functionality? Unfortunately, I'm not using Terraform 0.13. Therefore, it's hard for me to come up with some test scenario, which could be another one test case.

[grem11n]

I've tried to investigate depends_on behavior a little bit. I tried to use a public VPC module to create VPCs and then this module to set up a peering connection. All the module definitions were a part of the same root module. However, I got an error:

The "count" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the count depends on.

A quick search brought me to this issue comment: https://github.com/hashicorp/terraform/issues/26078#issuecomment-687093646 It looks like depends_on doesn't work as expected

[Cajga]

@grem11n have you find a solution for this issue? We wanted to use the module from CI pipeline where using the recommended "-target" solution is not really an option...

[grem11n]

Hey @Cajga, well this module works with Terraform v0.13 per se. A behavior mentioned in this issue doesn't. Likely due to the bug I linked above. However, I haven't tried this with newer versions of Terraform. Perhaps, it's not a problem anymore. Do you have a prototype or some code examples I can use to test the behavior specific to your use case?

[Cajga]

Hi @grem11n,

I am using the latest terraform in the CI pipeline but the issue is still there. The problem with the module (as well as few other VPC modules in the registry) is that you have to create the VPC in a first run and then apply the peering module in a second. You will not be able to create the whole infra with one terraform run. This is due to the use of id in count. It is explained in the documentation as well as in the this comment from a developer. As the developer describes, this is a known behavior (that cannot be modified with depend_on) and not a bug. IMHO, this behavior will not change in the near future.

For a reproducer, create 2 VPCs with the https://github.com/terraform-aws-modules/terraform-aws-vpc module and use your module to try to peer them. It is important to try to do this from scratch with one terraform run. You will always receive the output that you mentioned above...

The solution is (as the developer recommends in his comment as well) is to use variables/locals like this module does: https://github.com/cornfeedhobo/terraform-aws-vpc-peering

[grem11n]

Thank you for the clarification!

I will add the test to reproduce it. Hopefully, I'll be able to figure out, how to apply everything in a single blow. I'm not sure if I can avoid using count, though. I need to somehow retrieve information regarding VPC setup and I don't want to handle VPC creation with this module for sure.

[grem11n]

Another thing I've noticed: my module doesn't work with the latest 0.14.6 version. Which is a problem indeed. I'm going to investigate it here: https://github.com/grem11n/terraform-aws-vpc-peering/issues/62

[Cajga]

Maybe you could move the use of ids down to a local variable and use that variable in the count. I believe that would work but I did not try it yet. Another way is receiving the ids in a variable (that is how the module that I linked does) and that works fine.

[grem11n]

I haven't created a test case, which covers the simultaneous creation of both VPCs and peerings yet, but you can check the code from this PR: https://github.com/grem11n/terraform-aws-vpc-peering/pull/61

I have removed count from the data sources. It's still present in main.tf, though. Not sure if I can remove count completely.

I've tested that code with Terraform 0.13 and it works fine (current test cases). I also tested it locally with 0.14 and it works. Apparently, there is something happening when I try to run tests with Terratest

[grem11n]

@eliranw , @Cajga ,

With https://github.com/grem11n/terraform-aws-vpc-peering/pull/61 merged, it should be possible now to use depends_on with this module from the master branch.

You can find an example here

Generally, this is recommended to always pin the module version. However, I want to make a few more cosmetic changes like README update and so on before creating a new tag. Hence, if you need this behavior right away, you can use unpinned version.

I will keep this ticket open not to forget to make the aforementioned changes and make a new release.

Also, please keep in mind that I only tested this module with Terraform 0.13.6. I'm not sure if it's going to work with 0.14. In theory, it should, but I had some problems with the Terratest run with TF 0.14