-
Notifications
You must be signed in to change notification settings - Fork 71
Add IPv6 Support #140
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
base: main
Are you sure you want to change the base?
Add IPv6 Support #140
Conversation
defined_tags = var.defined_tags | ||
|
||
route_rules { | ||
# * With this route table, NAT Gateway is always declared as the default gateway for IPv4 |
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.
In this comment, do you mean with this route rule?
destination = local.anywhere | ||
destination_type = "CIDR_BLOCK" | ||
network_entity_id = oci_core_nat_gateway.nat_gateway[0].id | ||
description = "Terraformed - Auto-generated at NAT Gateway creation: NAT Gateway as default gateway for IPv4" |
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.
In description, keep only "NAT Gateway as default gateway for IPv4", drop the rest
destination = local.anywhere_ipv6 | ||
destination_type = "CIDR_BLOCK" | ||
network_entity_id = oci_core_internet_gateway.ig[0].id | ||
description = "Terraformed - Auto-generated at NAT Gateway creation: IGW Gateway as default gateway for 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.
ditto
@@ -64,6 +64,27 @@ variable "enable_ipv6" { | |||
default = false | |||
} | |||
|
|||
variable "vcn_is_oracle_gua_allocation_enabled" { |
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.
does gua stand for something?
dns_label = var.vcn_dns_label | ||
is_ipv6enabled = var.enable_ipv6 | ||
is_oracle_gua_allocation_enabled = var.enable_ipv6 ? var.vcn_is_oracle_gua_allocation_enabled : null | ||
ipv6private_cidr_blocks = var.enable_ipv6 ? var.vcn_ipv6private_cidr_blocks : null |
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.
ipv6private_cidr_blocks
is a crappy and thoughtless variable name and is not consistent with what vcn uses for ipv4 (not your fault) in the oci provider.
I'm willing to make a breaking change and release a new major version so we can handle this better:
- instead of var.vcn_cidrs, we can use vcn_cidrs4 or vcn_cidrs_ipv4
- instead of var.vcn_ipv6private_cidr_blocks, we can use vcn_cidrs6 or vcn_cidrs_ipv6
Or something along that line i.e. don't make the parameters gouge the eyes of developers who are going to use them.
Since we are thinking of making a breaking change, consider renaming other variables accordingly.
5fd3628
to
c28f946
Compare
Proposed change
How has these changes been tested?
Manual testing
If no automated testing is run, please ensure that at least the three steps below are passing without any error.
terraform apply
on each example provided with this module provisions the intended resource(s) without any errors.terraform destroy
on each example provided with this module destroys all the resources created by this module and only the resources created by this module.Checklist before submitting your PR
Note: If you are not an Oracle employee, to contribute to an Oracle-sponsored open-source project, you need to sign the Oracle Contributor Agreement (OCA).