Issue #16: ipv6 support - add ipv6 support #21

bcenker · 2017-11-05T04:05:29Z

There are several resources which should be updated to have IPv6 support:

aws_vpc (done in v1.39.0)
aws_subnet
aws_egress_only_internet_gateway
aws_route...

antonbabenko

Good work. I have left a few comments with the main idea to make this code smaller and easier to maintain in long run.

antonbabenko · 2017-11-11T17:36:24Z

main.tf

@@ -41,6 +41,14 @@ resource "aws_route" "public_internet_gateway" {
  gateway_id             = "${aws_internet_gateway.this.id}"
 }

+resource "aws_route" "public_internet_gateway_ipv6" {
+  count = "${var.enable_ipv6 ? (length(var.public_subnets) > 0 ? 1 : 0) : 0}"


count = "${var.enable_ipv6 && length(var.public_subnets) > 0 ? 1 : 0}"

oops 🤦‍♂️ :) Fixed in latest commit

antonbabenko · 2017-11-11T17:39:03Z

main.tf

+  instance_tenancy                 = "${var.instance_tenancy}"
+  enable_dns_hostnames             = "${var.enable_dns_hostnames}"
+  enable_dns_support               = "${var.enable_dns_support}"
+  assign_generated_ipv6_cidr_block = "${var.enable_ipv6}"


Keep empty line before tags

Fixed in latest commit

antonbabenko · 2017-11-11T18:00:51Z

outputs.tf

+}
+
+output "ipv6_enabled_private_subnets_ipv4_cidr_blocks" {
+  description = "Map of List of ipv4 cidr_blocks of private subnets in an ipv6 enabled VPC"


The description is a dangerous nice mix of ipv4 and ipv6 even if it makes sense :)

Also, this is not map of list, but just a list

Fixed in latest commit

antonbabenko · 2017-11-11T18:04:33Z

main.tf

@@ -67,11 +75,24 @@ resource "aws_subnet" "public" {
  tags = "${merge(var.tags, var.public_subnet_tags, map("Name", format("%s-public-%s", var.name, element(var.azs, count.index))))}"
 }

+resource "aws_subnet" "public_ipv6" {


Making aws_subnet resources conditionally is a bad idea, because it require twice more code for some things. Instead, we should always use the same aws_subnet resource (public and private). I think it should be possible, because ipv6-related features are the same as for ipv4 (with the exception of aws_egress_only_internet_gateway which is only for ipv6).

Follow up questions:

Will ipv6_cidr_block work with empty value?

What will the value of aws_vpc.this.ipv6_cidr_block be when VPC has not enabled IPv6?

Why did you specify the number of bits as "8" as an argument of cidrsubnet?

Hi Anton,
I agree that making aws_subnet resources conditionally is not ideal - if you had a chance to review the discussion in the related issue you'll see I originally went down the path of using the same private and public subnet resources but ran into some issues due to the ipv6_cidr_block behavior.

To address your questions directly:

ipv6_cidr_block subnet argument will work with an empty value, but the following
ipv6_cidr_block = "${var.enable_ipv6 ? cidrsubnet(aws_vpc.this.ipv6_cidr_block,8,count.index) : ""}"
won't work due to the answer to 2. below

aws_vpc.this.ipv6_cidr_block is undefined when the assign_generated_ipv6_cidr_block argument is false in vpc resource definition, which resulted in the conditional referenced in 1. failing with the following error:
* module.vpc.aws_subnet.public[0]: Resource 'aws_vpc.this' does not have attribute 'ipv6_cidr_block' for variable 'aws_vpc.this.ipv6_cidr_block'

I specified 8 bits in cidrsubnet because AWS ipv6 blocks are autoassigned a /56 mask and the AWS ipv6 subnets require a /64 mask, so this should always be 8 (i think)

Thanks for the clarification. I want to try the code in this PR and see if I can make something around 1 and 2 during Monday evening.

@antonbabenko looks like there was a recently submitted PR that would fix the issue we are seeing.

If we wait until this is fixed we won't need to create conditional subnet resources. Additionally, it looks like there is a "fix" coming in 0.11.x (see 0.11.0-beta1 changelog) that may cause this PR code to break because of the new output validation (I assume that if our conditionally created subnet resources are not created, terraform would throw an error due to the exception in the ipv6_* output interpolations).

I'm thinking we can sit on this until that PR is merged and I can refactor the ipv6 support into the existing the resources, and hopefully at the same time provide forward compatibility. Let me know your thoughts :)

Great finding! Absolutely, let's wait for this PR. Terraform AWS provider is the best place to fix this kind of issues.

nelg · 2018-01-08T22:59:05Z

+1 for this feature

nicolas-g · 2018-02-16T21:56:06Z

+1

cornfeedhobo · 2018-03-03T19:23:06Z

Has work on this halted? It's pretty much the last thing in the way of me switching over from my own vpc module ...

antonbabenko · 2018-03-03T20:39:24Z

Please thumb-up the related issue in the AWS provider - hashicorp/terraform-provider-aws#2103.

This should indicate to the maintainers that this feature is not very large and it is still important for a lot of users.

OneBadSanta · 2018-07-24T01:46:16Z

+1 Still not available in v1.37.0

antonbabenko · 2018-08-19T12:45:53Z

IPv6 is a good feature to have, but it is not so easy to make it supported in this module.

I have updated this module by adding support forassign_generated_ipv6_cidr_block to the VPC resource - v1.39.0 has been released.

Note, that this adds IPv6 support only to VPC and not to other resources. Adding IPv6 support to aws_subnet, aws_egress_only_internet_gateway and others requires some time.

rsrchboy · 2019-01-19T20:52:24Z

It looks like the upstream provider PR has been merged in; are we ready to move forward with this PR again?

antonbabenko · 2019-01-19T21:13:19Z

Yes, at least we can try once more. Last time I looked at it there were some bugs which prevented me from making it. I am not entirely sure what was it.

miry · 2019-01-25T07:22:04Z

I suggest to split this PR to small parts. Example:

ipv6 for subnets
ipv6 for internet gateways
...

Based on work done in terraform-aws-modules#21.

dekimsey · 2019-02-26T15:57:27Z

I re-implemented most of this in #218 with success to test it. I think all the terraform components are in place.

I did notice some issues with the deletion of the ipv6 route table when the vpc is destroyed.

One last thing, this PR makes an assumption about the spread of the ipv6 subnets. I think it'd be better to have the user specify the prefixes, they already are doing so for ipv4 anyway.

antonbabenko · 2019-09-02T13:16:24Z

v2.10.0 has been released (see #317).

github-actions · 2022-11-04T02:38:43Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Issue terraform-aws-modules#16: ipv6 support - add ipv6 support

4444f99

antonbabenko reviewed Nov 11, 2017

View reviewed changes

code cleanup

b5ab74e

antonbabenko added the requires terraform update label Nov 13, 2017

antonbabenko mentioned this pull request Nov 13, 2017

ipv6 support #16

Closed

antonbabenko mentioned this pull request Nov 23, 2017

resource/aws_vpc: Set ipv6 properties even if no ipv6 is enabled to allow usage in conditionals (fixes #688) hashicorp/terraform-provider-aws#2103

Merged

dekimsey added a commit to dekimsey/terraform-aws-vpc that referenced this pull request Feb 14, 2019

Adds ipv6 support

ec3d1bf

Based on work done in terraform-aws-modules#21.

dekimsey mentioned this pull request Feb 14, 2019

Adds ipv6 support with user-specified prefixes #218

Closed

9 tasks

antonbabenko mentioned this pull request Sep 2, 2019

Add IPv6 support #317

Merged

antonbabenko closed this Sep 2, 2019

github-actions bot locked as resolved and limited conversation to collaborators Nov 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #16: ipv6 support - add ipv6 support #21

Issue #16: ipv6 support - add ipv6 support #21

bcenker commented Nov 5, 2017 •

edited by antonbabenko

Loading

antonbabenko left a comment

antonbabenko Nov 11, 2017

bcenker Nov 11, 2017

antonbabenko Nov 11, 2017

bcenker Nov 11, 2017

antonbabenko Nov 11, 2017

antonbabenko Nov 11, 2017

bcenker Nov 11, 2017

antonbabenko Nov 11, 2017

bcenker Nov 11, 2017

antonbabenko Nov 11, 2017

bcenker Nov 12, 2017

antonbabenko Nov 13, 2017

nelg commented Jan 8, 2018

nicolas-g commented Feb 16, 2018

cornfeedhobo commented Mar 3, 2018

antonbabenko commented Mar 3, 2018

OneBadSanta commented Jul 24, 2018

antonbabenko commented Aug 19, 2018

rsrchboy commented Jan 19, 2019

antonbabenko commented Jan 19, 2019

miry commented Jan 25, 2019 •

edited

Loading

dekimsey commented Feb 26, 2019

antonbabenko commented Sep 2, 2019

github-actions bot commented Nov 4, 2022

Issue #16: ipv6 support - add ipv6 support #21

Issue #16: ipv6 support - add ipv6 support #21

Conversation

bcenker commented Nov 5, 2017 • edited by antonbabenko Loading

antonbabenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nelg commented Jan 8, 2018

nicolas-g commented Feb 16, 2018

cornfeedhobo commented Mar 3, 2018

antonbabenko commented Mar 3, 2018

OneBadSanta commented Jul 24, 2018

antonbabenko commented Aug 19, 2018

rsrchboy commented Jan 19, 2019

antonbabenko commented Jan 19, 2019

miry commented Jan 25, 2019 • edited Loading

dekimsey commented Feb 26, 2019

antonbabenko commented Sep 2, 2019

github-actions bot commented Nov 4, 2022

bcenker commented Nov 5, 2017 •

edited by antonbabenko

Loading

miry commented Jan 25, 2019 •

edited

Loading