DNXLabs / terraform-aws-network

Terraform module for creating basic network resources: VPC, subnets and NAT.
https://modules.dnx.one
Apache License 2.0
5 stars 18 forks source link

Add capabilities to dynamically Bring Your Own Elastic IP for Nat Gateway #23

Closed wvxavier closed 2 years ago

wvxavier commented 2 years ago

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.

Types of changes

What types of changes does your code introduce to ? Put an x in the boxes that apply

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

adenot commented 2 years ago

Would be required to pass 3 allocation_ids when using multi-nat, right?

lzrocha commented 2 years ago

Would be required to pass 3 allocation_ids when using multi-nat, right?

My questions are related to this as well

adenot commented 2 years ago

I'm ok if we import the EIPs into the module when we need instead of passing as a parameter, I think this is adding unecessary complexity, or is there another reason?

wvxavier commented 2 years ago

That's true.

I've converted the variable eip_allocation_id to a list so one can add multiple allocation ids when multi_nat is used.

adenot commented 2 years ago

Looks good. Can you just rename to eip_allocation_ids?

wvxavier commented 2 years ago

Sure thing, it's been renamed. Also, I've tested the module using workspaces with and without EIP, looking good. ✅