Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support AWS Direct Connect Gateway with option to create #17

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nickzxcv
Copy link

@nickzxcv nickzxcv commented Nov 7, 2023

Fixes #16

@nickzxcv
Copy link
Author

nickzxcv commented Nov 7, 2023

One thing this doesn't do, is support creating 1 DX Gateway with multiple connections attached, if there is a count for the connections it will create separate DX Gateways for each connection which isn't really what's wanted I think for adjusting capacity of connections. This behavior is consistent though with the create vgw option.

@displague
Copy link
Member

Thanks for the issue and

@displague displague closed this Nov 14, 2023
@displague displague reopened this Nov 14, 2023
@displague
Copy link
Member

displague commented Nov 14, 2023

Sorry - my corgi is trying to play, mouthed my typing hand, and accidentally closed the PR 😅 🐕

Thanks for the issue and PR! This looks good overall (plus terraform init and ... validate pass ✔️ ). We'll need to verify the functionality ahead of a merge which may take some time.

I should also mention that this module is currently based on the v3 ECX API Terraform resources, which are deprecated: equinix/terraform-provider-equinix#445 (cc @thogarty)

There has been some conversation about moving the examples/ from the Terraform provider repo into modules such as this one or refactoring modules such as this one to take advantage of a new central module (or revising https://github.com/equinix-labs/terraform-equinix-fabric-connection, which this module depends on).

Here is the AWS example based on the v4 Fabric API Terraform resources:
https://github.com/equinix/terraform-provider-equinix/tree/main/examples/fabric/v4/portConnectivity/aws

Copy link
Contributor

@ocobles ocobles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much @nickzxcv for your contribution with this PR 👏🏼 I requested some changes that I found by testing it myself. It seems amazon_side_asn is mandatory in aws_dx_gateway, so I set a default value 64512, but please change it If you think there is a better default value.

variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
nickzxcv and others added 3 commits November 15, 2023 19:22
Co-authored-by: Óscar Cobles <68897552+ocobles@users.noreply.github.com>
Co-authored-by: Óscar Cobles <68897552+ocobles@users.noreply.github.com>
Co-authored-by: Óscar Cobles <68897552+ocobles@users.noreply.github.com>
@nickzxcv
Copy link
Author

Thanks for the feedback! Let me know if we should make any more changes or if it should all be applied somehow differently to the newer API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Direct Connect Gateway support
3 participants