-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Adds ipv6 support with user-specified prefixes #218
Conversation
Based on work done in terraform-aws-modules#21.
… with ivp6 enabled
Leaving this for posterity. I initially reported IPv6 did not work for me though I felt I had implemented the module correctly. I dug into it today and it appears to be an issue with CentOS (7.6?) that Router Advertisements are not enabled. The default RA-issue route was not set. Enabling the sysctl setting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! LGTM
We could add one example to make it also easier to test.. like the following:
provider "aws" {
region = "${var.region}"
}
data "aws_availability_zones" "available" {}
module "vpc" {
source = "../.."
name = "ipv6"
cidr = "10.0.0.0/16"
azs = ["${data.aws_availability_zones.available.names[0]}", "${data.aws_availability_zones.available.names[1]}"]
private_subnets = ["10.0.1.0/24", "10.0.2.0/24"]
public_subnets = ["10.0.101.0/24", "10.0.102.0/24"]
enable_nat_gateway = false
enable_ipv6 = true
assign_generated_ipv6_cidr_block = true
assign_ipv6_address_on_creation = true
public_subnet_ipv6_prefixes = [0, 1]
private_subnet_ipv6_prefixes = [2, 3]
tags = {
Owner = "user"
Environment = "dev"
}
}
@@ -18,6 +18,40 @@ variable "assign_generated_ipv6_cidr_block" { | |||
default = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to remove this variable and set this assign_generated_ipv6_cidr_block = "${var.enable_ipv6}"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah.. now I see that you actually mentioned this in the PR description..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, I didn't want to remove the option until I had a chance to hear how you wanted to handle it. I wanted to remove the assign_generated_ipv6_cidr_block
variable entirely and just set have the new variable name be enable_ipv6
. Alternatively, keeping the assign name and making it actually set up the ipv6 cidr associations. Do you have any thoughts on how you'd like it to be handled?
Co-Authored-By: Steffen Gebert <st+github@st-g.de>
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. |
Based on work done in #21. There didn't seem to be much going on that pull request. I apologize if this is undesirable.
Adds support for what I think are all the necessary components.
enable_ipv6
,assign_generated_ipv6_cidr_block
), but I don't think that's good. Since they both must be set, and it doesn't seem to make sense to enable one without the other.* aws_route...._ipv6_egress.2: InvalidRoute.NotFound: no route with destination-cidr-block ::/0 in route table rtb-0be2b413b6e783fc7
error messages. Not sure why, ipv4 route entries clean up correctly. Bug in provider?I took a slightly different approach particularly on the prefix assignments as I found changing the assignments can cause an error so I didn't want to use any
+length()
or hard-coded subnet spacing.Usage: