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

Fully enable IPv6 in the VPC (as new opt-in variable) #5

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

danopia
Copy link
Contributor

@danopia danopia commented Oct 15, 2020

Hallo from Berlin 👋

I noticed that the VPC resource always has IPv6 on, and there's a variable to set dualstack on the ALB, but there were some pieces missing in the middle. Specifically the default route and security group ingress for IPv6 traffic.

When I applied the module as-is, the ALB's AAAA records were created, but trying to connect to them just resulted in a timeout / no packets in response.

This PR adds a variable to preserve previous behavior by default, but any new users looking for dualstack will probably want to enable it. I'm not sure how dualstack would be used right now unless you update the routing manually after setting up this module.


In my fork I also added a couple ARN outputs and a variable for specifying the IPv4 newbits. I figured I'd submit this PR first and ask if the other variable and outputs make sense for a further PR.

Thanks for your module and time!

snovikov
snovikov previously approved these changes Oct 22, 2021
@snovikov
Copy link
Contributor

@danopia can you please resolve conflict and rebase your branch?

Dualstack ALB was already an option, but the VPC wasn't fully set up for
it. I made this a variable to preserve previous behavior, but any new
users looking for dualstack will probably want to enable it.
@danopia
Copy link
Contributor Author

danopia commented Oct 25, 2021

Hi, I've restated my changes as a fresh signed commit. The conflict was only in README.md so I regenerated using the new Makefile.

Also, as mentioned in my original comment I also have two other changes to this module which are included in this comparison: parameterizing ipv4_newbits as an input & adding several outputs. This is the module version I'm actually using so I have some interest in proposing these changes as follow-up if that seems reasonable.

@danvaida
Copy link
Contributor

Thanks @danopia for your contribution!
We'll create a new release shortly.

@danvaida danvaida merged commit 572f818 into Flaconi:master Oct 28, 2021
@danopia danopia deleted the upstream-ipv6 branch November 10, 2021 21:42
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.

None yet

3 participants