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

vpc/route_table: Allow local inline routes #32794

Merged
merged 12 commits into from
Aug 3, 2023
3 changes: 3 additions & 0 deletions .changelog/32794.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_route_table: Allow an existing route to be changed to/from a `local` target such as `gateway_id = "local"` and `network_interface_id = "eni-0bf6fcb204c94231e"`
```
2 changes: 1 addition & 1 deletion internal/service/ec2/vpc_route_data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ 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
network_interface_id = aws_instance.test.primary_network_interface_id
}

data "aws_route" "by_peering_connection_id" {
Expand Down
52 changes: 48 additions & 4 deletions internal/service/ec2/vpc_route_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func resourceRouteTableRead(ctx context.Context, d *schema.ResourceData, meta in
if err := d.Set("propagating_vgws", propagatingVGWs); err != nil {
return sdkdiag.AppendErrorf(diags, "setting propagating_vgws: %s", err)
}
if err := d.Set("route", flattenRoutes(ctx, conn, routeTable.Routes)); err != nil {
if err := d.Set("route", flattenRoutes(ctx, d, conn, routeTable.Routes)); err != nil {
return sdkdiag.AppendErrorf(diags, "setting route: %s", err)
}
d.Set("vpc_id", routeTable.VpcId)
Expand Down Expand Up @@ -488,6 +488,11 @@ func routeTableAddRoute(ctx context.Context, conn *ec2.EC2, routeTableID string,
errCodeInvalidTransitGatewayIDNotFound,
)

// Local routes cannot be created manually.
if tfawserr.ErrMessageContains(err, errCodeInvalidGatewayIDNotFound, "The gateway ID 'local' does not exist") {
return fmt.Errorf("cannot create local Route, use `terraform import` to manage existing local Routes")
}

if err != nil {
return fmt.Errorf("creating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err)
}
Expand Down Expand Up @@ -719,7 +724,11 @@ func expandReplaceRouteInput(tfMap map[string]interface{}) *ec2.ReplaceRouteInpu
}

if v, ok := tfMap["gateway_id"].(string); ok && v != "" {
apiObject.GatewayId = aws.String(v)
if v == gatewayIDLocal {
apiObject.LocalTarget = aws.Bool(true)
} else {
apiObject.GatewayId = aws.String(v)
}
}

if v, ok := tfMap["local_gateway_id"].(string); ok && v != "" {
Expand Down Expand Up @@ -811,7 +820,7 @@ func flattenRoute(apiObject *ec2.Route) map[string]interface{} {
return tfMap
}

func flattenRoutes(ctx context.Context, conn *ec2.EC2, apiObjects []*ec2.Route) []interface{} {
func flattenRoutes(ctx context.Context, d *schema.ResourceData, conn *ec2.EC2, apiObjects []*ec2.Route) []interface{} {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Swap *schema.ResourceData and *ec2.EC2 arguments as the most common pattern is f(ctx, conn, other-args...).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, absolutely! Careless...

if len(apiObjects) == 0 {
return nil
}
Expand All @@ -823,7 +832,13 @@ func flattenRoutes(ctx context.Context, conn *ec2.EC2, apiObjects []*ec2.Route)
continue
}

if gatewayID := aws.StringValue(apiObject.GatewayId); gatewayID == gatewayIDLocal || gatewayID == gatewayIDVPCLattice {
if gatewayID := aws.StringValue(apiObject.GatewayId); gatewayID == gatewayIDVPCLattice {
continue
}

// local routes from config need to be included but not default local routes, as determined by hasLocalConfig
// see local route tests
if gatewayID := aws.StringValue(apiObject.GatewayId); gatewayID == gatewayIDLocal && !hasLocalConfig(d, apiObject) {
continue
}

Expand Down Expand Up @@ -854,6 +869,35 @@ func flattenRoutes(ctx context.Context, conn *ec2.EC2, apiObjects []*ec2.Route)
return tfList
}

// hasLocalConfig along with flattenRoutes prevents default local routes from
// being stored in state but allows configured local routes to be stored in
// state. hasLocalConfig checks the ResourceData and flattenRoutes skips or
// includes the route. Normally, you can't count on ResourceData to represent
// config. However, in this case, a local gateway route in ResourceData must
// come from config because of the gatekeeping done by hasLocalConfig and
// flattenRoutes.
func hasLocalConfig(d *schema.ResourceData, apiObject *ec2.Route) bool {
if apiObject.GatewayId == nil {
return false
}
if v, ok := d.GetOk("route"); ok && v.(*schema.Set).Len() > 0 {
for _, v := range v.(*schema.Set).List() {
v := v.(map[string]interface{})
if v["cidr_block"].(string) != aws.StringValue(apiObject.DestinationCidrBlock) &&
v["destination_prefix_list_id"] != aws.StringValue(apiObject.DestinationPrefixListId) &&
v["ipv6_cidr_block"] != aws.StringValue(apiObject.DestinationIpv6CidrBlock) {
continue
}

if v["gateway_id"].(string) == gatewayIDLocal {
return true
}
}
}

return false
}

// routeTableRouteDestinationAttribute returns the attribute key and value of the route table route's destination.
func routeTableRouteDestinationAttribute(m map[string]interface{}) (string, string) {
for _, key := range routeTableValidDestinations {
Expand Down
236 changes: 235 additions & 1 deletion internal/service/ec2/vpc_route_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,131 @@ func TestAccVPCRouteTable_prefixListToInternetGateway(t *testing.T) {
})
}

func TestAccVPCRouteTable_localRoute(t *testing.T) {
ctx := acctest.Context(t)
var routeTable ec2.RouteTable
var vpc ec2.Vpc
resourceName := "aws_route_table.test"
vpcResourceName := "aws_vpc.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckRouteDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccVPCRouteTableConfig_basic(rName),
Check: resource.ComposeTestCheckFunc(
acctest.CheckVPCExists(ctx, vpcResourceName, &vpc),
testAccCheckRouteTableExists(ctx, resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 1),
),
},
{
Config: testAccVPCRouteTableConfig_ipv4Local(rName),
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) {
ctx := acctest.Context(t)
var routeTable ec2.RouteTable
var vpc ec2.Vpc
resourceName := "aws_route_table.test"
rteResourceName := "aws_route.test"
vpcResourceName := "aws_vpc.test"
eniResourceName := "aws_network_interface.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
vpcCIDR := "10.1.0.0/16"
localGatewayCIDR := "10.1.0.0/16"
eniCIDR := "10.1.0.0/16"
subnetCIDR := "10.1.1.0/24"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckRouteDestroy(ctx),
Steps: []resource.TestStep{
// This test is a little wonky. Because there's no way (that I
// could figure anyway) to use aws_route_table to import a local
// route and then persist it to the next step since the route is
// inline rather than a separate resource. Instead, it uses
// aws_route config rather than aws_route_table w/ inline routes
// for steps 1-3 and then does slight of hand, switching
// to aws_route_table to finish the test.
{
Config: testAccVPCRouteConfig_ipv4NoRoute(rName),
Check: resource.ComposeTestCheckFunc(
acctest.CheckVPCExists(ctx, vpcResourceName, &vpc),
testAccCheckRouteTableExists(ctx, resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 1),
),
},
{
Config: testAccVPCRouteConfig_ipv4Local(rName),
ResourceName: rteResourceName,
ImportState: true,
ImportStateIdFunc: func(rt *ec2.RouteTable, v *ec2.Vpc) resource.ImportStateIdFunc {
return func(s *terraform.State) (string, error) {
return fmt.Sprintf("%s_%s", aws.StringValue(rt.RouteTableId), aws.StringValue(v.CidrBlock)), nil
}
}(&routeTable, &vpc),
ImportStatePersist: true,
// Don't verify the state as the local route isn't actually in the pre-import state.
// Just running ImportState verifies that we can import a local route.
ImportStateVerify: false,
},
{
Config: testAccVPCRouteConfig_ipv4LocalToNetworkInterface(rName),
Check: resource.ComposeAggregateTestCheckFunc(
acctest.CheckVPCExists(ctx, vpcResourceName, &vpc),
testAccCheckRouteTableExists(ctx, resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 1),
resource.TestCheckResourceAttr(rteResourceName, "gateway_id", ""),
resource.TestCheckResourceAttrPair(rteResourceName, "network_interface_id", eniResourceName, "id"),
),
},
{
Config: testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, eniCIDR, subnetCIDR),
Check: resource.ComposeAggregateTestCheckFunc(
acctest.CheckVPCExists(ctx, vpcResourceName, &vpc),
testAccCheckRouteTableExists(ctx, resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 1),
testAccCheckRouteTableRoute(resourceName, "cidr_block", vpcCIDR, "network_interface_id", eniResourceName, "id"),
),
},
{
Config: testAccVPCRouteTableConfig_ipv4NetworkInterfaceToLocal(rName, vpcCIDR, localGatewayCIDR, subnetCIDR),
Check: resource.ComposeAggregateTestCheckFunc(
acctest.CheckVPCExists(ctx, vpcResourceName, &vpc),
testAccCheckRouteTableExists(ctx, resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 1),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "route.*", map[string]string{
"gateway_id": "local",
"cidr_block": localGatewayCIDR,
}),
),
},
{
Config: testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, eniCIDR, subnetCIDR),
Check: resource.ComposeAggregateTestCheckFunc(
acctest.CheckVPCExists(ctx, vpcResourceName, &vpc),
testAccCheckRouteTableExists(ctx, resourceName, &routeTable),
testAccCheckRouteTableNumberOfRoutes(&routeTable, 1),
testAccCheckRouteTableRoute(resourceName, "cidr_block", vpcCIDR, "network_interface_id", eniResourceName, "id"),
),
},
},
})
}

func testAccCheckRouteTableExists(ctx context.Context, n string, v *ec2.RouteTable) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
Expand Down Expand Up @@ -1669,6 +1794,10 @@ func testAccVPCRouteTableConfig_ipv4EndpointID(rName, destinationCidr string) st
fmt.Sprintf(`
data "aws_caller_identity" "current" {}

data "aws_iam_session_context" "current" {
arn = data.aws_caller_identity.current.arn
}

resource "aws_vpc" "test" {
cidr_block = "10.10.10.0/25"

Expand Down Expand Up @@ -1698,7 +1827,7 @@ resource "aws_lb" "test" {

resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
allowed_principals = [data.aws_caller_identity.current.arn]
allowed_principals = [data.aws_iam_session_context.current.issuer_arn]
gateway_load_balancer_arns = [aws_lb.test.arn]

tags = {
Expand Down Expand Up @@ -2260,3 +2389,108 @@ resource "aws_route_table" "test" {
}
`, rName)
}

func testAccVPCRouteTableConfig_ipv4Local(rName string) string {
return fmt.Sprintf(`
resource "aws_vpc" "test" {
cidr_block = "10.1.0.0/16"

tags = {
Name = %[1]q
}
}

resource "aws_route_table" "test" {
vpc_id = aws_vpc.test.id

route {
cidr_block = aws_vpc.test.cidr_block
gateway_id = "local"
}
}
`, rName)
}

func testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, eniCIDR, subnetCIDR string) string {
return fmt.Sprintf(`
resource "aws_vpc" "test" {
cidr_block = %[2]q

tags = {
Name = %[1]q
}
}

resource "aws_route_table" "test" {
vpc_id = aws_vpc.test.id

route {
cidr_block = aws_vpc.test.cidr_block
network_interface_id = aws_network_interface.test.id
}

tags = {
Name = %[1]q
}
}

resource "aws_subnet" "test" {
cidr_block = %[4]q
vpc_id = aws_vpc.test.id

tags = {
Name = %[1]q
}
}

resource "aws_network_interface" "test" {
subnet_id = aws_subnet.test.id

tags = {
Name = %[1]q
}
}
`, rName, vpcCIDR, eniCIDR, subnetCIDR)
}

func testAccVPCRouteTableConfig_ipv4NetworkInterfaceToLocal(rName, vpcCIDR, gatewayCIDR, subnetCIDR string) string {
return fmt.Sprintf(`
resource "aws_vpc" "test" {
cidr_block = %[2]q

tags = {
Name = %[1]q
}
}

resource "aws_route_table" "test" {
vpc_id = aws_vpc.test.id

route {
cidr_block = %[3]q
gateway_id = "local"
}

tags = {
Name = %[1]q
}
}

resource "aws_subnet" "test" {
cidr_block = %[4]q
vpc_id = aws_vpc.test.id

tags = {
Name = %[1]q
}
}

resource "aws_network_interface" "test" {
subnet_id = aws_subnet.test.id

tags = {
Name = %[1]q
}
}
`, rName, vpcCIDR, gatewayCIDR, subnetCIDR)
}
Loading
Loading