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

aws_ec2_transit_gateway_route: cidr match is unreliable #14846

Merged
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
5 changes: 3 additions & 2 deletions aws/ec2_transit_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,9 @@ func ec2DescribeTransitGatewayRoute(conn *ec2.EC2, transitGatewayRouteTableID, d
if route == nil {
continue
}

if aws.StringValue(route.DestinationCidrBlock) == destination {
if cidrBlocksEqual(aws.StringValue(route.DestinationCidrBlock), destination) {
cidrString := canonicalCidrBlock(aws.StringValue(route.DestinationCidrBlock))
route.DestinationCidrBlock = aws.String(cidrString)
return route, nil
}
}
Expand Down
8 changes: 5 additions & 3 deletions aws/resource_aws_ec2_transit_gateway_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ func resourceAwsEc2TransitGatewayRoute() *schema.Resource {

Schema: map[string]*schema.Schema{
"destination_cidr_block": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateCIDRNetworkAddress,
DiffSuppressFunc: suppressEqualCIDRBlockDiffs,
},
"blackhole": {
Type: schema.TypeBool,
Expand Down
36 changes: 36 additions & 0 deletions aws/resource_aws_ec2_transit_gateway_route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,36 @@ func TestAccAWSEc2TransitGatewayRoute_basic(t *testing.T) {
})
}

func TestAccAWSEc2TransitGatewayRoute_basic_ipv6(t *testing.T) {
var transitGatewayRoute1 ec2.TransitGatewayRoute
resourceName := "aws_ec2_transit_gateway_route.test_ipv6"
transitGatewayResourceName := "aws_ec2_transit_gateway.test"
transitGatewayVpcAttachmentResourceName := "aws_ec2_transit_gateway_vpc_attachment.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSEc2TransitGateway(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEc2TransitGatewayRouteDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEc2TransitGatewayRouteConfigDestinationCidrBlock(),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEc2TransitGatewayRouteExists(resourceName, &transitGatewayRoute1),
resource.TestCheckResourceAttr(resourceName, "destination_cidr_block", "2001:db8::/56"),
resource.TestCheckResourceAttr(resourceName, "blackhole", "false"),
resource.TestCheckResourceAttrPair(resourceName, "transit_gateway_attachment_id", transitGatewayVpcAttachmentResourceName, "id"),
resource.TestCheckResourceAttrPair(resourceName, "transit_gateway_route_table_id", transitGatewayResourceName, "association_default_route_table_id"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAWSEc2TransitGatewayRoute_blackhole(t *testing.T) {
var transitGatewayRoute1 ec2.TransitGatewayRoute
resourceName := "aws_ec2_transit_gateway_route.test_blackhole"
Expand Down Expand Up @@ -248,6 +278,12 @@ resource "aws_ec2_transit_gateway_route" "test" {
transit_gateway_route_table_id = aws_ec2_transit_gateway.test.association_default_route_table_id
}

resource "aws_ec2_transit_gateway_route" "test_ipv6" {
destination_cidr_block = "2001:db8::/56"
transit_gateway_attachment_id = aws_ec2_transit_gateway_vpc_attachment.test.id
transit_gateway_route_table_id = aws_ec2_transit_gateway.test.association_default_route_table_id
}

resource "aws_ec2_transit_gateway_route" "test_blackhole" {
destination_cidr_block = "10.1.0.0/16"
blackhole = true
Expand Down
11 changes: 11 additions & 0 deletions aws/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,17 @@ func cidrBlocksEqual(cidr1, cidr2 string) bool {
return ip2.String() == ip1.String() && ipnet2.String() == ipnet1.String()
}

// canonicalCidrBlock returns the canonical representation of a CIDR block.
// This function is especially useful for hash functions for sets which include IPv6 CIDR blocks.
func canonicalCidrBlock(cidr string) string {
_, ipnet, err := net.ParseCIDR(cidr)
if err != nil {
return cidr
}

return ipnet.String()
}

func validateHTTPMethod() schema.SchemaValidateFunc {
return validation.StringInSlice([]string{
"ANY",
Expand Down
27 changes: 23 additions & 4 deletions aws/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ func TestValidateCIDRNetworkAddress(t *testing.T) {
}
}

func Test_validateCIDRBlock(t *testing.T) {
func TestValidateCIDRBlock(t *testing.T) {
for _, ts := range []struct {
cidr string
valid bool
Expand All @@ -477,7 +477,7 @@ func Test_validateCIDRBlock(t *testing.T) {
}
}

func Test_validateIpv4CIDRBlock(t *testing.T) {
func TestValidateIpv4CIDRBlock(t *testing.T) {
for _, ts := range []struct {
cidr string
valid bool
Expand All @@ -500,7 +500,7 @@ func Test_validateIpv4CIDRBlock(t *testing.T) {
}
}

func Test_validateIpv6CIDRBlock(t *testing.T) {
func TestValidateIpv6CIDRBlock(t *testing.T) {
for _, ts := range []struct {
cidr string
valid bool
Expand All @@ -524,7 +524,7 @@ func Test_validateIpv6CIDRBlock(t *testing.T) {
}
}

func Test_cidrBlocksEqual(t *testing.T) {
func TestCidrBlocksEqual(t *testing.T) {
for _, ts := range []struct {
cidr1 string
cidr2 string
Expand All @@ -544,6 +544,25 @@ func Test_cidrBlocksEqual(t *testing.T) {
}
}
}
func TestCanonicalCidrBlock(t *testing.T) {
for _, ts := range []struct {
cidr string
expected string
}{
{"10.2.2.0/24", "10.2.2.0/24"},
{"10.2.2.5/24", "10.2.2.0/24"},
{"::/0", "::/0"},
{"::0/0", "::/0"},
{"2001::/15", "2000::/15"},
{"2001:db8::1/120", "2001:db8::/120"},
{"", ""},
} {
got := canonicalCidrBlock(ts.cidr)
if ts.expected != got {
t.Fatalf("canonicalCidrBlock(%q) should be: %q, got: %q", ts.cidr, ts.expected, got)
}
}
}

func TestValidateLogMetricFilterName(t *testing.T) {
validNames := []string{
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/ec2_transit_gateway_route.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ resource "aws_ec2_transit_gateway_route" "example" {

The following arguments are supported:

* `destination_cidr_block` - (Required) IPv4 CIDR range used for destination matches. Routing decisions are based on the most specific match.
* `destination_cidr_block` - (Required) IPv4 or IPv6 RFC1924 CIDR used for destination matches. Routing decisions are based on the most specific match.
* `transit_gateway_attachment_id` - (Optional) Identifier of EC2 Transit Gateway Attachment (required if `blackhole` is set to false).
* `blackhole` - (Optional) Indicates whether to drop traffic that matches this route (default to `false`).
* `transit_gateway_route_table_id` - (Required) Identifier of EC2 Transit Gateway Route Table.
Expand Down