Skip to content

Commit

Permalink
Remove aws_route_table.route[].instance_id
Browse files Browse the repository at this point in the history 
Route table routes can be defined with instance_id XOR network_interface_id
and that is a problem because AWS returns both properties when routes are retrieved
and thus it indefinitely generates a terraform diff.

This change is supposed to happen in the v5.0.0 of the provider
according to hashicorp#14197 (comment)

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
  • Loading branch information
@sylr
sylr committed Jan 7, 2022
1 parent d14b20b commit e83067a
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 126 deletions.
10 changes: 0 additions & 10 deletions internal/service/ec2/route_data_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ func DataSourceRoute() *schema.Resource {
Optional: true,
Computed: true,
},
"instance_id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
},
"local_gateway_id": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -140,10 +135,6 @@ func dataSourceRouteRead(d *schema.ResourceData, meta interface{}) error {
continue
}

if v, ok := d.GetOk("instance_id"); ok && aws.StringValue(r.InstanceId) != v.(string) {
continue
}

if v, ok := d.GetOk("local_gateway_id"); ok && aws.StringValue(r.LocalGatewayId) != v.(string) {
continue
}
Expand Down Expand Up @@ -191,7 +182,6 @@ func dataSourceRouteRead(d *schema.ResourceData, meta interface{}) error {
d.Set("destination_prefix_list_id", route.DestinationPrefixListId)
d.Set("egress_only_gateway_id", route.EgressOnlyInternetGatewayId)
d.Set("gateway_id", route.GatewayId)
d.Set("instance_id", route.InstanceId)
d.Set("local_gateway_id", route.LocalGatewayId)
d.Set("nat_gateway_id", route.NatGatewayId)
d.Set("network_interface_id", route.NetworkInterfaceId)
Expand Down
26 changes: 4 additions & 22 deletions internal/service/ec2/route_data_source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@ func TestAccEC2RouteDataSource_basic(t *testing.T) {
instanceRouteResourceName := "aws_route.instance"
pcxRouteResourceName := "aws_route.vpc_peering_connection"
rtResourceName := "aws_route_table.test"
instanceResourceName := "aws_instance.test"
pcxResourceName := "aws_vpc_peering_connection.test"
datasource1Name := "data.aws_route.by_destination_cidr_block"
datasource2Name := "data.aws_route.by_instance_id"
datasource3Name := "data.aws_route.by_peering_connection_id"
datasource2Name := "data.aws_route.by_peering_connection_id"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
Expand All @@ -34,15 +32,10 @@ func TestAccEC2RouteDataSource_basic(t *testing.T) {
resource.TestCheckResourceAttrPair(datasource1Name, "destination_cidr_block", instanceRouteResourceName, "destination_cidr_block"),
resource.TestCheckResourceAttrPair(datasource1Name, "route_table_id", rtResourceName, "id"),

// By instance ID.
resource.TestCheckResourceAttrPair(datasource2Name, "destination_cidr_block", instanceRouteResourceName, "destination_cidr_block"),
resource.TestCheckResourceAttrPair(datasource2Name, "instance_id", instanceResourceName, "id"),
resource.TestCheckResourceAttrPair(datasource2Name, "route_table_id", rtResourceName, "id"),

// By VPC peering connection ID.
resource.TestCheckResourceAttrPair(datasource3Name, "destination_cidr_block", pcxRouteResourceName, "destination_cidr_block"),
resource.TestCheckResourceAttrPair(datasource3Name, "route_table_id", rtResourceName, "id"),
resource.TestCheckResourceAttrPair(datasource3Name, "vpc_peering_connection_id", pcxResourceName, "id"),
resource.TestCheckResourceAttrPair(datasource2Name, "destination_cidr_block", pcxRouteResourceName, "destination_cidr_block"),
resource.TestCheckResourceAttrPair(datasource2Name, "route_table_id", rtResourceName, "id"),
resource.TestCheckResourceAttrPair(datasource2Name, "vpc_peering_connection_id", pcxResourceName, "id"),
),
},
},
Expand Down Expand Up @@ -263,12 +256,6 @@ resource "aws_instance" "test" {
}
}
resource "aws_route" "instance" {
route_table_id = aws_route_table.test.id
destination_cidr_block = "10.0.1.0/24"
instance_id = aws_instance.test.id
}
data "aws_route" "by_peering_connection_id" {
route_table_id = aws_route_table.test.id
vpc_peering_connection_id = aws_route.vpc_peering_connection.vpc_peering_connection_id
Expand All @@ -278,11 +265,6 @@ data "aws_route" "by_destination_cidr_block" {
route_table_id = aws_route_table.test.id
destination_cidr_block = aws_route.instance.destination_cidr_block
}
data "aws_route" "by_instance_id" {
route_table_id = aws_route_table.test.id
instance_id = aws_route.instance.instance_id
}
`, rName))
}

Expand Down
25 changes: 1 addition & 24 deletions internal/service/ec2/route_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ var routeTableValidTargets = []string{
"carrier_gateway_id",
"egress_only_gateway_id",
"gateway_id",
"instance_id",
"local_gateway_id",
"nat_gateway_id",
"network_interface_id",
Expand Down Expand Up @@ -120,10 +119,6 @@ func ResourceRouteTable() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"instance_id": {
Type: schema.TypeString,
Optional: true,
},
"local_gateway_id": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -445,12 +440,6 @@ func resourceRouteTableHash(v interface{}) int {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}

instanceSet := false
if v, ok := m["instance_id"]; ok {
instanceSet = v.(string) != ""
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}

if v, ok := m["transit_gateway_id"]; ok {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}
Expand All @@ -467,7 +456,7 @@ func resourceRouteTableHash(v interface{}) int {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}

if v, ok := m["network_interface_id"]; ok && !(instanceSet || natGatewaySet) {
if v, ok := m["network_interface_id"]; ok && !(natGatewaySet) {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}

Expand Down Expand Up @@ -695,10 +684,6 @@ func expandEc2CreateRouteInput(tfMap map[string]interface{}) *ec2.CreateRouteInp
apiObject.GatewayId = aws.String(v)
}

if v, ok := tfMap["instance_id"].(string); ok && v != "" {
apiObject.InstanceId = aws.String(v)
}

if v, ok := tfMap["local_gateway_id"].(string); ok && v != "" {
apiObject.LocalGatewayId = aws.String(v)
}
Expand Down Expand Up @@ -757,10 +742,6 @@ func expandEc2ReplaceRouteInput(tfMap map[string]interface{}) *ec2.ReplaceRouteI
apiObject.GatewayId = aws.String(v)
}

if v, ok := tfMap["instance_id"].(string); ok && v != "" {
apiObject.InstanceId = aws.String(v)
}

if v, ok := tfMap["local_gateway_id"].(string); ok && v != "" {
apiObject.LocalGatewayId = aws.String(v)
}
Expand Down Expand Up @@ -823,10 +804,6 @@ func flattenEc2Route(apiObject *ec2.Route) map[string]interface{} {
}
}

if v := apiObject.InstanceId; v != nil {
tfMap["instance_id"] = aws.StringValue(v)
}

if v := apiObject.LocalGatewayId; v != nil {
tfMap["local_gateway_id"] = aws.StringValue(v)
}
Expand Down
8 changes: 0 additions & 8 deletions internal/service/ec2/route_table_data_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,6 @@ func DataSourceRouteTable() *schema.Resource {
Computed: true,
},

"instance_id": {
Type: schema.TypeString,
Computed: true,
},

"local_gateway_id": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -301,9 +296,6 @@ func dataSourceRoutesRead(conn *ec2.EC2, ec2Routes []*ec2.Route) []map[string]in
if r.LocalGatewayId != nil {
m["local_gateway_id"] = *r.LocalGatewayId
}
if r.InstanceId != nil {
m["instance_id"] = *r.InstanceId
}
if r.TransitGatewayId != nil {
m["transit_gateway_id"] = *r.TransitGatewayId
}
Expand Down
72 changes: 14 additions & 58 deletions internal/service/ec2/route_table_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ func TestAccEC2RouteTable_ipv4ToInternetGateway(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
destinationCidr1 := "10.2.0.0/16"
destinationCidr2 := "10.3.0.0/16"
destinationCidr3 := "10.4.0.0/16"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
Expand All @@ -126,7 +125,7 @@ func TestAccEC2RouteTable_ipv4ToInternetGateway(t *testing.T) {
),
},
{
Config: testAccRouteTableIPv4InternetGatewayConfig(rName, destinationCidr2, destinationCidr3),
Config: testAccRouteTableIPv4InternetGatewayConfig(rName, destinationCidr2, destinationCidr2),
Check: resource.ComposeTestCheckFunc(
testAccCheckRouteTableExists(resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 3),
Expand All @@ -135,7 +134,7 @@ func TestAccEC2RouteTable_ipv4ToInternetGateway(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"),
resource.TestCheckResourceAttr(resourceName, "route.#", "2"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr2, "gateway_id", igwResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr3, "gateway_id", igwResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr2, "gateway_id", igwResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
),
Expand All @@ -152,7 +151,6 @@ func TestAccEC2RouteTable_ipv4ToInternetGateway(t *testing.T) {
func TestAccEC2RouteTable_ipv4ToInstance(t *testing.T) {
var routeTable ec2.RouteTable
resourceName := "aws_route_table.test"
instanceResourceName := "aws_instance.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
destinationCidr := "10.2.0.0/16"

Expand All @@ -171,7 +169,6 @@ func TestAccEC2RouteTable_ipv4ToInstance(t *testing.T) {
acctest.CheckResourceAttrAccountID(resourceName, "owner_id"),
resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"),
resource.TestCheckResourceAttr(resourceName, "route.#", "1"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr, "instance_id", instanceResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
),
Expand Down Expand Up @@ -909,13 +906,11 @@ func TestAccEC2RouteTable_multipleRoutes(t *testing.T) {
resourceName := "aws_route_table.test"
eoigwResourceName := "aws_egress_only_internet_gateway.test"
igwResourceName := "aws_internet_gateway.test"
instanceResourceName := "aws_instance.test"
pcxResourceName := "aws_vpc_peering_connection.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
destinationCidr1 := "10.2.0.0/16"
destinationCidr2 := "10.3.0.0/16"
destinationCidr3 := "10.4.0.0/16"
destinationCidr4 := "2001:db8::/122"
destinationCidr2 := "10.4.0.0/16"
destinationCidr3 := "2001:db8::/122"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
Expand All @@ -926,8 +921,7 @@ func TestAccEC2RouteTable_multipleRoutes(t *testing.T) {
{
Config: testAccRouteTableMultipleRoutesConfig(rName,
"cidr_block", destinationCidr1, "gateway_id", igwResourceName,
"cidr_block", destinationCidr2, "instance_id", instanceResourceName,
"ipv6_cidr_block", destinationCidr4, "egress_only_gateway_id", eoigwResourceName),
"ipv6_cidr_block", destinationCidr3, "egress_only_gateway_id", eoigwResourceName),
Check: resource.ComposeTestCheckFunc(
testAccCheckRouteTableExists(resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 5),
Expand All @@ -936,17 +930,15 @@ func TestAccEC2RouteTable_multipleRoutes(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"),
resource.TestCheckResourceAttr(resourceName, "route.#", "3"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr1, "gateway_id", igwResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr2, "instance_id", instanceResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "ipv6_cidr_block", destinationCidr4, "egress_only_gateway_id", eoigwResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "ipv6_cidr_block", destinationCidr3, "egress_only_gateway_id", eoigwResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
),
},
{
Config: testAccRouteTableMultipleRoutesConfig(rName,
"cidr_block", destinationCidr1, "vpc_peering_connection_id", pcxResourceName,
"cidr_block", destinationCidr3, "instance_id", instanceResourceName,
"ipv6_cidr_block", destinationCidr4, "egress_only_gateway_id", eoigwResourceName),
"ipv6_cidr_block", destinationCidr3, "egress_only_gateway_id", eoigwResourceName),
Check: resource.ComposeTestCheckFunc(
testAccCheckRouteTableExists(resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 5),
Expand All @@ -955,27 +947,24 @@ func TestAccEC2RouteTable_multipleRoutes(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"),
resource.TestCheckResourceAttr(resourceName, "route.#", "3"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr1, "vpc_peering_connection_id", pcxResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr3, "instance_id", instanceResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "ipv6_cidr_block", destinationCidr4, "egress_only_gateway_id", eoigwResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "ipv6_cidr_block", destinationCidr3, "egress_only_gateway_id", eoigwResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
),
},
{
Config: testAccRouteTableMultipleRoutesConfig(rName,
"ipv6_cidr_block", destinationCidr4, "vpc_peering_connection_id", pcxResourceName,
"cidr_block", destinationCidr3, "gateway_id", igwResourceName,
"cidr_block", destinationCidr2, "instance_id", instanceResourceName),
"ipv6_cidr_block", destinationCidr3, "vpc_peering_connection_id", pcxResourceName,
"cidr_block", destinationCidr2, "gateway_id", igwResourceName),
Check: resource.ComposeTestCheckFunc(
testAccCheckRouteTableExists(resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 5),
acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`route-table/.+$`)),
acctest.CheckResourceAttrAccountID(resourceName, "owner_id"),
resource.TestCheckResourceAttr(resourceName, "propagating_vgws.#", "0"),
resource.TestCheckResourceAttr(resourceName, "route.#", "3"),
testAccCheckRouteTableRoute(resourceName, "ipv6_cidr_block", destinationCidr4, "vpc_peering_connection_id", pcxResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr3, "gateway_id", igwResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr2, "instance_id", instanceResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "ipv6_cidr_block", destinationCidr3, "vpc_peering_connection_id", pcxResourceName, "id"),
testAccCheckRouteTableRoute(resourceName, "cidr_block", destinationCidr2, "gateway_id", igwResourceName, "id"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
),
Expand Down Expand Up @@ -1333,19 +1322,6 @@ resource "aws_instance" "test" {
Name = %[1]q
}
}
resource "aws_route_table" "test" {
vpc_id = aws_vpc.test.id
route {
cidr_block = %[2]q
instance_id = aws_instance.test.id
}
tags = {
Name = %[1]q
}
}
`, rName, destinationCidr))
}

Expand Down Expand Up @@ -1478,18 +1454,6 @@ func testAccRouteTableConfigNoDestination(rName string) string {
acctest.AvailableEC2InstanceTypeForAvailabilityZone("data.aws_availability_zones.available.names[0]", "t3.micro", "t2.micro"),
acctest.ConfigLatestAmazonLinuxHvmEbsAmi(),
fmt.Sprintf(`
resource "aws_route_table" "test" {
vpc_id = aws_vpc.test.id
route {
instance_id = aws_instance.test.id
}
tags = {
Name = %[1]q
}
}
resource "aws_vpc" "test" {
cidr_block = "10.1.0.0/16"
Expand Down Expand Up @@ -2092,8 +2056,7 @@ resource "aws_vpc_endpoint" "test" {

func testAccRouteTableMultipleRoutesConfig(rName,
destinationAttr1, destinationValue1, targetAttribute1, targetValue1,
destinationAttr2, destinationValue2, targetAttribute2, targetValue2,
destinationAttr3, destinationValue3, targetAttribute3, targetValue3 string) string {
destinationAttr2, destinationValue2, targetAttribute2, targetValue2 string) string {
return acctest.ConfigCompose(
testAccLatestAmazonNatInstanceAmiConfig(),
acctest.ConfigAvailableAZsNoOptInDefaultExclude(),
Expand Down Expand Up @@ -2176,12 +2139,6 @@ locals {
target_attr = %[8]q
target_value = %[9]s.id
},
{
destination_attr = %[10]q
destination_value = %[11]q
target_attr = %[12]q
target_value = %[13]s.id
}
]
}
Expand All @@ -2199,7 +2156,6 @@ resource "aws_route_table" "test" {
carrier_gateway_id = (route.value["target_attr"] == "carrier_gateway_id") ? route.value["target_value"] : null
egress_only_gateway_id = (route.value["target_attr"] == "egress_only_gateway_id") ? route.value["target_value"] : null
gateway_id = (route.value["target_attr"] == "gateway_id") ? route.value["target_value"] : null
instance_id = (route.value["target_attr"] == "instance_id") ? route.value["target_value"] : null
local_gateway_id = (route.value["target_attr"] == "local_gateway_id") ? route.value["target_value"] : null
nat_gateway_id = (route.value["target_attr"] == "nat_gateway_id") ? route.value["target_value"] : null
network_interface_id = (route.value["target_attr"] == "network_interface_id") ? route.value["target_value"] : null
Expand All @@ -2213,7 +2169,7 @@ resource "aws_route_table" "test" {
Name = %[1]q
}
}
`, rName, destinationAttr1, destinationValue1, targetAttribute1, targetValue1, destinationAttr2, destinationValue2, targetAttribute2, targetValue2, destinationAttr3, destinationValue3, targetAttribute3, targetValue3))
`, rName, destinationAttr1, destinationValue1, targetAttribute1, targetValue1, destinationAttr2, destinationValue2, targetAttribute2, targetValue2))
}

// testAccLatestAmazonNatInstanceAmiConfig returns the configuration for a data source that
Expand Down
1 change: 0 additions & 1 deletion website/docs/d/route.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ The following arguments are optional:
* `destination_prefix_list_id` - (Optional) The ID of a [managed prefix list](ec2_managed_prefix_list.html) destination of the Route belonging to the Route Table.
* `egress_only_gateway_id` - (Optional) Egress Only Gateway ID of the Route belonging to the Route Table.
* `gateway_id` - (Optional) Gateway ID of the Route belonging to the Route Table.
* `instance_id` - (Optional) Instance ID of the Route belonging to the Route Table.
* `local_gateway_id` - (Optional) Local Gateway ID of the Route belonging to the Route Table.
* `nat_gateway_id` - (Optional) NAT Gateway ID of the Route belonging to the Route Table.
* `network_interface_id` - (Optional) Network Interface ID of the Route belonging to the Route Table.
Expand Down
1 change: 0 additions & 1 deletion website/docs/d/route_table.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ For targets:
* `carrier_gateway_id` - ID of the Carrier Gateway.
* `egress_only_gateway_id` - ID of the Egress Only Internet Gateway.
* `gateway_id` - Internet Gateway ID.
* `instance_id` - EC2 instance ID.
* `local_gateway_id` - Local Gateway ID.
* `nat_gateway_id` - NAT Gateway ID.
* `network_interface_id` - ID of the elastic network interface (eni) to use.
Expand Down

0 comments on commit e83067a

Please sign in to comment.