Skip to content

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

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 70 additions & 8 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
# VPC
######
resource "aws_vpc" "this" {
cidr_block = "${var.cidr}"
instance_tenancy = "${var.instance_tenancy}"
enable_dns_hostnames = "${var.enable_dns_hostnames}"
enable_dns_support = "${var.enable_dns_support}"
cidr_block = "${var.cidr}"
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}"
Copy link
Member

Choose a reason for hiding this comment

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

Keep empty line before tags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest commit


tags = "${merge(var.tags, map("Name", format("%s", var.name)))}"
}
Expand Down Expand Up @@ -41,6 +42,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}"

route_table_id = "${aws_route_table.public.id}"
destination_ipv6_cidr_block = "::/0"
gateway_id = "${aws_internet_gateway.this.id}"
}

#################
# Private routes
#################
Expand All @@ -57,7 +66,7 @@ resource "aws_route_table" "private" {
# Public subnet
################
resource "aws_subnet" "public" {
count = "${length(var.public_subnets)}"
count = "${!var.enable_ipv6 ? length(var.public_subnets) : 0}"

vpc_id = "${aws_vpc.this.id}"
cidr_block = "${var.public_subnets[count.index]}"
Expand All @@ -67,11 +76,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" {
Copy link
Member

Choose a reason for hiding this comment

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

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:

  1. Will ipv6_cidr_block work with empty value?
  2. What will the value of aws_vpc.this.ipv6_cidr_block be when VPC has not enabled IPv6?
  3. Why did you specify the number of bits as "8" as an argument of cidrsubnet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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:

  1. 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
  2. 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'
  3. 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)

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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 :)

Copy link
Member

Choose a reason for hiding this comment

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

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

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

vpc_id = "${aws_vpc.this.id}"
cidr_block = "${var.public_subnets[count.index]}"
ipv6_cidr_block = "${cidrsubnet(aws_vpc.this.ipv6_cidr_block,8,count.index)}"
availability_zone = "${element(var.azs, count.index)}"
map_public_ip_on_launch = "${var.map_public_ip_on_launch}"
assign_ipv6_address_on_creation = "${var.assign_ipv6_address_on_creation}"

tags = "${merge(var.tags, var.public_subnet_tags, map("Name", format("%s-public-%s", var.name, element(var.azs, count.index))))}"
}

#################
# Private subnet
#################
resource "aws_subnet" "private" {
count = "${length(var.private_subnets)}"
count = "${!var.enable_ipv6 ? length(var.private_subnets) : 0}"

vpc_id = "${aws_vpc.this.id}"
cidr_block = "${var.private_subnets[count.index]}"
Expand All @@ -80,6 +102,18 @@ resource "aws_subnet" "private" {
tags = "${merge(var.tags, var.private_subnet_tags, map("Name", format("%s-private-%s", var.name, element(var.azs, count.index))))}"
}

resource "aws_subnet" "private_ipv6" {
count = "${var.enable_ipv6 ? length(var.private_subnets) : 0}"

vpc_id = "${aws_vpc.this.id}"
cidr_block = "${var.private_subnets[count.index]}"
ipv6_cidr_block = "${cidrsubnet(aws_vpc.this.ipv6_cidr_block,8,count.index+32)}"
availability_zone = "${element(var.azs, count.index)}"
assign_ipv6_address_on_creation = "${var.assign_ipv6_address_on_creation}"

tags = "${merge(var.tags, var.private_subnet_tags, map("Name", format("%s-private-%s", var.name, element(var.azs, count.index))))}"
}

##################
# Database subnet
##################
Expand Down Expand Up @@ -144,6 +178,12 @@ resource "aws_nat_gateway" "this" {
depends_on = ["aws_internet_gateway.this"]
}

resource "aws_egress_only_internet_gateway" "this" {
count = "${var.enable_ipv6 ? 1 : 0}"

vpc_id = "${aws_vpc.this.id}"
}

resource "aws_route" "private_nat_gateway" {
count = "${var.enable_nat_gateway ? length(var.azs) : 0}"

Expand All @@ -152,6 +192,14 @@ resource "aws_route" "private_nat_gateway" {
nat_gateway_id = "${element(aws_nat_gateway.this.*.id, count.index)}"
}

resource "aws_route" "private_nat_gateway_ipv6" {
count = "${var.enable_ipv6 ? length(var.azs) : 0}"

route_table_id = "${element(aws_route_table.private.*.id, count.index)}"
destination_ipv6_cidr_block = "::/0"
egress_only_gateway_id = "${aws_egress_only_internet_gateway.this.id}"
}

######################
# VPC Endpoint for S3
######################
Expand Down Expand Up @@ -216,12 +264,19 @@ resource "aws_vpc_endpoint_route_table_association" "public_dynamodb" {
# Route table association
##########################
resource "aws_route_table_association" "private" {
count = "${length(var.private_subnets)}"
count = "${!var.enable_ipv6 ? length(var.private_subnets) : 0}"

subnet_id = "${element(aws_subnet.private.*.id, count.index)}"
route_table_id = "${element(aws_route_table.private.*.id, count.index)}"
}

resource "aws_route_table_association" "private_ipv6" {
count = "${var.enable_ipv6 ? length(var.private_subnets) : 0}"

subnet_id = "${element(aws_subnet.private_ipv6.*.id, count.index)}"
route_table_id = "${element(aws_route_table.private.*.id, count.index)}"
}

resource "aws_route_table_association" "database" {
count = "${length(var.database_subnets)}"

Expand All @@ -237,8 +292,15 @@ resource "aws_route_table_association" "elasticache" {
}

resource "aws_route_table_association" "public" {
count = "${length(var.public_subnets)}"
count = "${!var.enable_ipv6 ? length(var.public_subnets) : 0}"

subnet_id = "${element(aws_subnet.public.*.id, count.index)}"
route_table_id = "${aws_route_table.public.id}"
}

resource "aws_route_table_association" "public_ipv6" {
count = "${var.enable_ipv6 ? length(var.public_subnets) : 0}"

subnet_id = "${element(aws_subnet.public_ipv6.*.id, count.index)}"
route_table_id = "${aws_route_table.public.id}"
}
35 changes: 35 additions & 0 deletions outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,21 @@ output "private_subnets_cidr_blocks" {
value = ["${aws_subnet.private.*.cidr_block}"]
}

output "ipv6_enabled_private_subnets" {
description = "List of IDs of private subnets in an ipv6 enabled VPC"
value = ["${aws_subnet.private_ipv6.*.id}"]
}

output "ipv6_enabled_private_subnets_ipv4_cidr_blocks" {
description = "List of ipv4 cidr_blocks of private subnets in an ipv6 enabled VPC"
value = ["${aws_subnet.private_ipv6.*.cidr_block}"]
}

output "ipv6_enabled_private_subnets_ipv6_cidr_blocks" {
description = "List of ipv6 cidr_blocks of private subnets in an ipv6 enabled VPC"
value = ["${aws_subnet.private_ipv6.*.ipv6_cidr_block}"]
}

output "public_subnets" {
description = "List of IDs of public subnets"
value = ["${aws_subnet.public.*.id}"]
Expand All @@ -40,6 +55,21 @@ output "public_subnets_cidr_blocks" {
value = ["${aws_subnet.public.*.cidr_block}"]
}

output "ipv6_enabled_public_subnets" {
description = "List of IDs of public subnets in an ipv6 enabled VPC"
value = ["${aws_subnet.public_ipv6.*.id}"]
}

output "ipv6_enabled_public_subnets_ipv4_cidr_blocks" {
description = "List of ipv4 cidr_blocks of public subnets in an ipv6 enabled VPC"
value = ["${aws_subnet.public_ipv6.*.cidr_block}"]
}

output "ipv6_enabled_public_subnets_ipv6_cidr_blocks" {
description = "List of ipv6 cidr_blocks of public subnets in an ipv6 enabled VPC"
value = ["${aws_subnet.public_ipv6.*.ipv6_cidr_block}"]
}

output "database_subnets" {
description = "List of IDs of database subnets"
value = ["${aws_subnet.database.*.id}"]
Expand Down Expand Up @@ -102,6 +132,11 @@ output "igw_id" {
value = "${aws_internet_gateway.this.id}"
}

output "ipv6_egress_only_igw_id" {
description = "The ID of the egress only Internet Gateway"
value = "${aws_egress_only_internet_gateway.this.id}"
}

# VPC Endpoints
output "vpc_endpoint_s3_id" {
description = "The ID of VPC endpoint for S3"
Expand Down
10 changes: 10 additions & 0 deletions variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,13 @@ variable "elasticache_subnet_tags" {
description = "Additional tags for the elasticache subnets"
default = {}
}

variable "enable_ipv6" {
description = "Enable ipv6 in this VPC"
default = false
}

variable "assign_ipv6_address_on_creation" {
description = "True to assign ipv6 dynamically, this is the ipv6 equivalent of map_public_ip_on_launch"
default = true
}