From c480de6ee1e4bef3bd690ec4c0ccec1e900b74d9 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 1 Aug 2023 14:43:21 -0400 Subject: [PATCH 01/12] vpc/route_table: Allow local inline routes --- internal/service/ec2/vpc_route_table.go | 15 +- internal/service/ec2/vpc_route_table_test.go | 246 +++++++++++++++++++ 2 files changed, 259 insertions(+), 2 deletions(-) diff --git a/internal/service/ec2/vpc_route_table.go b/internal/service/ec2/vpc_route_table.go index b848dc5e83c..0df34424b07 100644 --- a/internal/service/ec2/vpc_route_table.go +++ b/internal/service/ec2/vpc_route_table.go @@ -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) } @@ -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 != "" { @@ -823,7 +832,9 @@ func flattenRoutes(ctx context.Context, conn *ec2.EC2, apiObjects []*ec2.Route) continue } - if gatewayID := aws.StringValue(apiObject.GatewayId); gatewayID == gatewayIDLocal || gatewayID == gatewayIDVPCLattice { + // not continuing for local so that it can be set without a diff + // see local route tests + if gatewayID := aws.StringValue(apiObject.GatewayId); gatewayID == gatewayIDVPCLattice { continue } diff --git a/internal/service/ec2/vpc_route_table_test.go b/internal/service/ec2/vpc_route_table_test.go index 33bbfb496c6..bb54c7a0c65 100644 --- a/internal/service/ec2/vpc_route_table_test.go +++ b/internal/service/ec2/vpc_route_table_test.go @@ -1033,6 +1033,147 @@ 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" + 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, eniCIDR, 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", + }), + ), + }, + }, + }) +} + +func testAccCreateRouteTable(ctx context.Context, v *ec2.Vpc) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn(ctx) + if v.VpcId == nil { + return fmt.Errorf("no VPC ID") + } + input := &ec2.CreateRouteTableInput{ + VpcId: v.VpcId, + } + + output, err := conn.CreateRouteTableWithContext(ctx, input) + + if err != nil { + return fmt.Errorf("test creating route table: %w", err) + } + + if output == nil || output.RouteTable == nil { + return fmt.Errorf("test creating route table: %s", "nil output") + } + + if _, err := tfec2.WaitRouteTableReady(ctx, conn, aws.StringValue(output.RouteTable.RouteTableId), time.Minute*2); err != nil { + return fmt.Errorf("waiting for test creating route table: %s", err) + } + return nil + } +} + func testAccCheckRouteTableExists(ctx context.Context, n string, v *ec2.RouteTable) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -2260,3 +2401,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) +} From 15ba47341a5c792fd3e3ab1f37c086ea2fca2c3d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 2 Aug 2023 14:30:29 -0400 Subject: [PATCH 02/12] Distinguish between default local and config local --- internal/service/ec2/vpc_route_table.go | 39 ++++++++++++++++++-- internal/service/ec2/vpc_route_table_test.go | 6 ++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/internal/service/ec2/vpc_route_table.go b/internal/service/ec2/vpc_route_table.go index 0df34424b07..db627ead4a0 100644 --- a/internal/service/ec2/vpc_route_table.go +++ b/internal/service/ec2/vpc_route_table.go @@ -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) @@ -820,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{} { if len(apiObjects) == 0 { return nil } @@ -832,12 +832,16 @@ func flattenRoutes(ctx context.Context, conn *ec2.EC2, apiObjects []*ec2.Route) continue } - // not continuing for local so that it can be set without a diff - // see local route tests 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 + } + if aws.StringValue(apiObject.Origin) == ec2.RouteOriginEnableVgwRoutePropagation { continue } @@ -865,6 +869,33 @@ func flattenRoutes(ctx context.Context, conn *ec2.EC2, apiObjects []*ec2.Route) return tfList } +// hasLocalConfig along with flattenRoutes prevents default "local" routes made +// by AWS from getting stored in state. They do this by checking the +// ResourceData. 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 { diff --git a/internal/service/ec2/vpc_route_table_test.go b/internal/service/ec2/vpc_route_table_test.go index bb54c7a0c65..96d7fd550d5 100644 --- a/internal/service/ec2/vpc_route_table_test.go +++ b/internal/service/ec2/vpc_route_table_test.go @@ -1810,6 +1810,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" @@ -1839,7 +1843,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 = { From 57fb3ddb243061ecc5dcc223a91d0aa5c15845e1 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 2 Aug 2023 14:32:22 -0400 Subject: [PATCH 03/12] Remove unused --- internal/service/ec2/vpc_route_table_test.go | 27 -------------------- 1 file changed, 27 deletions(-) diff --git a/internal/service/ec2/vpc_route_table_test.go b/internal/service/ec2/vpc_route_table_test.go index 96d7fd550d5..cba89ebecd5 100644 --- a/internal/service/ec2/vpc_route_table_test.go +++ b/internal/service/ec2/vpc_route_table_test.go @@ -1147,33 +1147,6 @@ func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) { }) } -func testAccCreateRouteTable(ctx context.Context, v *ec2.Vpc) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn(ctx) - if v.VpcId == nil { - return fmt.Errorf("no VPC ID") - } - input := &ec2.CreateRouteTableInput{ - VpcId: v.VpcId, - } - - output, err := conn.CreateRouteTableWithContext(ctx, input) - - if err != nil { - return fmt.Errorf("test creating route table: %w", err) - } - - if output == nil || output.RouteTable == nil { - return fmt.Errorf("test creating route table: %s", "nil output") - } - - if _, err := tfec2.WaitRouteTableReady(ctx, conn, aws.StringValue(output.RouteTable.RouteTableId), time.Minute*2); err != nil { - return fmt.Errorf("waiting for test creating route table: %s", err) - } - return nil - } -} - func testAccCheckRouteTableExists(ctx context.Context, n string, v *ec2.RouteTable) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] From a077071e9e282e157a029120d4ce5f93bca15c74 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 2 Aug 2023 15:11:38 -0400 Subject: [PATCH 04/12] Add changelog --- .changelog/32794.txt | 3 +++ internal/service/ec2/vpc_route_table.go | 12 +++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 .changelog/32794.txt diff --git a/.changelog/32794.txt b/.changelog/32794.txt new file mode 100644 index 00000000000..8742f489a9f --- /dev/null +++ b/.changelog/32794.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_route_table: Allow a `route` with `gateway_id = "local"` to be imported and modified +``` \ No newline at end of file diff --git a/internal/service/ec2/vpc_route_table.go b/internal/service/ec2/vpc_route_table.go index db627ead4a0..df11a49cd08 100644 --- a/internal/service/ec2/vpc_route_table.go +++ b/internal/service/ec2/vpc_route_table.go @@ -869,11 +869,13 @@ func flattenRoutes(ctx context.Context, d *schema.ResourceData, conn *ec2.EC2, a return tfList } -// hasLocalConfig along with flattenRoutes prevents default "local" routes made -// by AWS from getting stored in state. They do this by checking the -// ResourceData. 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. +// 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 From 0992c87e5b38d3fcedc1fdc88cb7cf7c08a4a956 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 2 Aug 2023 16:59:36 -0400 Subject: [PATCH 05/12] tests/route_table: Clarify variables --- internal/service/ec2/vpc_route_table_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/service/ec2/vpc_route_table_test.go b/internal/service/ec2/vpc_route_table_test.go index cba89ebecd5..5463bfe486b 100644 --- a/internal/service/ec2/vpc_route_table_test.go +++ b/internal/service/ec2/vpc_route_table_test.go @@ -1075,6 +1075,7 @@ func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) { 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" @@ -1133,16 +1134,26 @@ func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) { ), }, { - Config: testAccVPCRouteTableConfig_ipv4NetworkInterfaceToLocal(rName, vpcCIDR, eniCIDR, subnetCIDR), + 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"), + ), + }, }, }) } From bf00b1f24b3e3d2f8e973f42c4a00e595117a147 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 2 Aug 2023 17:15:24 -0400 Subject: [PATCH 06/12] Update changelog --- .changelog/32794.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/32794.txt b/.changelog/32794.txt index 8742f489a9f..f2f3c0e28bd 100644 --- a/.changelog/32794.txt +++ b/.changelog/32794.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -resource/aws_route_table: Allow a `route` with `gateway_id = "local"` to be imported and modified +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"` ``` \ No newline at end of file From 3ad454532411b9fdb26fbb11c1f33c5a6d09d212 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 2 Aug 2023 18:18:43 -0400 Subject: [PATCH 07/12] tests/route: Fix configuration --- .../service/ec2/vpc_route_data_source_test.go | 2 +- internal/service/ec2/vpc_route_test.go | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/internal/service/ec2/vpc_route_data_source_test.go b/internal/service/ec2/vpc_route_data_source_test.go index fe19f41bb34..d49063cfd51 100644 --- a/internal/service/ec2/vpc_route_data_source_test.go +++ b/internal/service/ec2/vpc_route_data_source_test.go @@ -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" { diff --git a/internal/service/ec2/vpc_route_test.go b/internal/service/ec2/vpc_route_test.go index 12923400853..d3be532c38e 100644 --- a/internal/service/ec2/vpc_route_test.go +++ b/internal/service/ec2/vpc_route_test.go @@ -3316,6 +3316,10 @@ func testAccVPCRouteConfig_resourceIPv4Endpoint(rName, destinationCidr string) s 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" @@ -3345,7 +3349,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 = { @@ -3386,6 +3390,10 @@ func testAccVPCRouteConfig_resourceIPv6Endpoint(rName, destinationIpv6Cidr strin 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" @@ -3415,7 +3423,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 = { @@ -3570,6 +3578,10 @@ resource "aws_nat_gateway" "test" { data "aws_caller_identity" "current" {} +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + resource "aws_lb" "test" { load_balancer_type = "gateway" name = %[1]q @@ -3581,7 +3593,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 = { From ee43b12032e59a6aae77820cc1b4c258f13f8667 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 3 Aug 2023 12:15:26 -0400 Subject: [PATCH 08/12] route_table: Allow adopting routes --- .changelog/32794.txt | 2 +- internal/service/ec2/vpc_route_table.go | 25 +++++-- internal/service/ec2/vpc_route_table_test.go | 73 ++++++++++++++++-- website/docs/r/route_table.html.markdown | 78 +++++++++++++++++++- 4 files changed, 164 insertions(+), 14 deletions(-) diff --git a/.changelog/32794.txt b/.changelog/32794.txt index f2f3c0e28bd..088ea4117ca 100644 --- a/.changelog/32794.txt +++ b/.changelog/32794.txt @@ -1,3 +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"` +resource/aws_route_table: Allow an existing local route to be adopted or imported and the target to be updated ``` \ No newline at end of file diff --git a/internal/service/ec2/vpc_route_table.go b/internal/service/ec2/vpc_route_table.go index df11a49cd08..65d65cf4b36 100644 --- a/internal/service/ec2/vpc_route_table.go +++ b/internal/service/ec2/vpc_route_table.go @@ -480,6 +480,25 @@ func routeTableAddRoute(ctx context.Context, conn *ec2.EC2, routeTableID string, input.RouteTableId = aws.String(routeTableID) + _, target := routeTableRouteTargetAttribute(tfMap) + + if target == gatewayIDLocal { + // created by AWS so probably doesn't need a retry but just to be sure + // we provide a small one + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, time.Second*15, + func() (interface{}, error) { + return routeFinder(ctx, conn, routeTableID, destination) + }, + errCodeInvalidRouteNotFound, + ) + + if tfresource.NotFound(err) { + return fmt.Errorf("local route cannot be created but must exist to be adopted, %s %s does not exist", target, destination) + } + + return nil + } + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, timeout, func() (interface{}, error) { return conn.CreateRouteWithContext(ctx, input) @@ -488,11 +507,7 @@ 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") - } - + // local routes can be adopted if err != nil { return fmt.Errorf("creating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } diff --git a/internal/service/ec2/vpc_route_table_test.go b/internal/service/ec2/vpc_route_table_test.go index 5463bfe486b..64cfb7e9fba 100644 --- a/internal/service/ec2/vpc_route_table_test.go +++ b/internal/service/ec2/vpc_route_table_test.go @@ -1065,7 +1065,67 @@ func TestAccVPCRouteTable_localRoute(t *testing.T) { }) } -func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) { +func TestAccVPCRouteTable_localRouteAdoptUpdate(t *testing.T) { + ctx := acctest.Context(t) + var routeTable ec2.RouteTable + var vpc ec2.Vpc + resourceName := "aws_route_table.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" + localGatewayCIDRBad := "10.2.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{ + { + Config: testAccVPCRouteTableConfig_ipv4NetworkInterfaceToLocal(rName, vpcCIDR, localGatewayCIDRBad, subnetCIDR), + ExpectError: regexp.MustCompile("must exist to be adopted"), + }, + { + 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, 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, + }), + ), + }, + }, + }) +} + +func TestAccVPCRouteTable_localRouteImportUpdate(t *testing.T) { ctx := acctest.Context(t) var routeTable ec2.RouteTable var vpc ec2.Vpc @@ -1076,7 +1136,6 @@ func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) { 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{ @@ -1125,7 +1184,7 @@ func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) { ), }, { - Config: testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, eniCIDR, subnetCIDR), + Config: testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, subnetCIDR), Check: resource.ComposeAggregateTestCheckFunc( acctest.CheckVPCExists(ctx, vpcResourceName, &vpc), testAccCheckRouteTableExists(ctx, resourceName, &routeTable), @@ -1146,7 +1205,7 @@ func TestAccVPCRouteTable_localRouteUpdate(t *testing.T) { ), }, { - Config: testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, eniCIDR, subnetCIDR), + Config: testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, subnetCIDR), Check: resource.ComposeAggregateTestCheckFunc( acctest.CheckVPCExists(ctx, vpcResourceName, &vpc), testAccCheckRouteTableExists(ctx, resourceName, &routeTable), @@ -2411,7 +2470,7 @@ resource "aws_route_table" "test" { `, rName) } -func testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, eniCIDR, subnetCIDR string) string { +func testAccVPCRouteTableConfig_ipv4LocalNetworkInterface(rName, vpcCIDR, subnetCIDR string) string { return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = %[2]q @@ -2435,7 +2494,7 @@ resource "aws_route_table" "test" { } resource "aws_subnet" "test" { - cidr_block = %[4]q + cidr_block = %[3]q vpc_id = aws_vpc.test.id tags = { @@ -2450,7 +2509,7 @@ resource "aws_network_interface" "test" { Name = %[1]q } } -`, rName, vpcCIDR, eniCIDR, subnetCIDR) +`, rName, vpcCIDR, subnetCIDR) } func testAccVPCRouteTableConfig_ipv4NetworkInterfaceToLocal(rName, vpcCIDR, gatewayCIDR, subnetCIDR string) string { diff --git a/website/docs/r/route_table.html.markdown b/website/docs/r/route_table.html.markdown index 34237f80aa8..911269e4fee 100644 --- a/website/docs/r/route_table.html.markdown +++ b/website/docs/r/route_table.html.markdown @@ -31,6 +31,8 @@ the separate resource. ## Example Usage +### Basic example + ```terraform resource "aws_route_table" "example" { vpc_id = aws_vpc.example.id @@ -65,6 +67,80 @@ resource "aws_route_table" "example" { } ``` +### Adopting an existing local route + +AWS creates certain routes that the AWS provider mostly ignores. You can manage them by importing or adopting them. See [Import](#import) below for information on importing. This example shows adopting a route and then updating its target. + +First, adopt an existing AWS-created route: + +```terraform +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = "leibovitz" + } +} + +resource "aws_route_table" "test" { + vpc_id = aws_vpc.test.id + + # since this is exactly the route AWS will create, the route will be adopted + route { + cidr_block = "10.1.0.0/16" + gateway_id = "local" + } + + tags = { + Name = "leibovitz" + } +} +``` + +Next, update the target of the route: + +```terraform +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = "lihaspoj" + } +} + +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 = "lihaspoj" + } +} + +resource "aws_subnet" "test" { + cidr_block = "10.1.1.0/24" + vpc_id = aws_vpc.test.id + + tags = { + Name = "lihaspoj" + } +} + +resource "aws_network_interface" "test" { + subnet_id = aws_subnet.test.id + + tags = { + Name = "lihaspoj" + } +} +``` + +The target could then be updated again back to `local`. + ## Argument Reference This resource supports the following arguments: @@ -90,7 +166,7 @@ One of the following target arguments must be supplied: * `carrier_gateway_id` - (Optional) Identifier of a carrier gateway. This attribute can only be used when the VPC contains a subnet which is associated with a Wavelength Zone. * `core_network_arn` - (Optional) The Amazon Resource Name (ARN) of a core network. * `egress_only_gateway_id` - (Optional) Identifier of a VPC Egress Only Internet Gateway. -* `gateway_id` - (Optional) Identifier of a VPC internet gateway or a virtual private gateway. +* `gateway_id` - (Optional) Identifier of a VPC internet gateway, virtual private gateway, or `local`. `local` routes cannot be created but can be adopted or imported. See the [example](#adopting-an-existing-local-route) above. * `local_gateway_id` - (Optional) Identifier of a Outpost local gateway. * `nat_gateway_id` - (Optional) Identifier of a VPC NAT gateway. * `network_interface_id` - (Optional) Identifier of an EC2 network interface. From a6fb8cce54fcfc9add79e13d23402af18b32379b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 3 Aug 2023 12:18:38 -0400 Subject: [PATCH 09/12] Trailing space --- website/docs/r/route_table.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/route_table.html.markdown b/website/docs/r/route_table.html.markdown index 911269e4fee..bcd68b867ee 100644 --- a/website/docs/r/route_table.html.markdown +++ b/website/docs/r/route_table.html.markdown @@ -86,7 +86,7 @@ resource "aws_route_table" "test" { vpc_id = aws_vpc.test.id # since this is exactly the route AWS will create, the route will be adopted - route { + route { cidr_block = "10.1.0.0/16" gateway_id = "local" } From 74aa9cf3cce79d0d76be708b7692a054e501412c Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 3 Aug 2023 12:20:57 -0400 Subject: [PATCH 10/12] route_table: Follow convention --- internal/service/ec2/vpc_route_table.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/ec2/vpc_route_table.go b/internal/service/ec2/vpc_route_table.go index 65d65cf4b36..a8f97078b1e 100644 --- a/internal/service/ec2/vpc_route_table.go +++ b/internal/service/ec2/vpc_route_table.go @@ -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, d, conn, routeTable.Routes)); err != nil { + if err := d.Set("route", flattenRoutes(ctx, conn, d, routeTable.Routes)); err != nil { return sdkdiag.AppendErrorf(diags, "setting route: %s", err) } d.Set("vpc_id", routeTable.VpcId) @@ -835,7 +835,7 @@ func flattenRoute(apiObject *ec2.Route) map[string]interface{} { return tfMap } -func flattenRoutes(ctx context.Context, d *schema.ResourceData, conn *ec2.EC2, apiObjects []*ec2.Route) []interface{} { +func flattenRoutes(ctx context.Context, conn *ec2.EC2, d *schema.ResourceData, apiObjects []*ec2.Route) []interface{} { if len(apiObjects) == 0 { return nil } From 0c1db21675d8b9326fdce2ceedf9d0ce11a6011f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 3 Aug 2023 12:26:01 -0400 Subject: [PATCH 11/12] route_table: Review items --- internal/service/ec2/vpc_route_table.go | 1 - website/docs/r/route_table.html.markdown | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/service/ec2/vpc_route_table.go b/internal/service/ec2/vpc_route_table.go index a8f97078b1e..e8ebb5a92ec 100644 --- a/internal/service/ec2/vpc_route_table.go +++ b/internal/service/ec2/vpc_route_table.go @@ -507,7 +507,6 @@ func routeTableAddRoute(ctx context.Context, conn *ec2.EC2, routeTableID string, errCodeInvalidTransitGatewayIDNotFound, ) - // local routes can be adopted if err != nil { return fmt.Errorf("creating Route in Route Table (%s) with destination (%s): %w", routeTableID, destination, err) } diff --git a/website/docs/r/route_table.html.markdown b/website/docs/r/route_table.html.markdown index bcd68b867ee..da9da1aaa8b 100644 --- a/website/docs/r/route_table.html.markdown +++ b/website/docs/r/route_table.html.markdown @@ -104,7 +104,7 @@ resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "lihaspoj" + Name = "leibovitz" } } @@ -117,7 +117,7 @@ resource "aws_route_table" "test" { } tags = { - Name = "lihaspoj" + Name = "leibovitz" } } @@ -126,7 +126,7 @@ resource "aws_subnet" "test" { vpc_id = aws_vpc.test.id tags = { - Name = "lihaspoj" + Name = "leibovitz" } } @@ -134,7 +134,7 @@ resource "aws_network_interface" "test" { subnet_id = aws_subnet.test.id tags = { - Name = "lihaspoj" + Name = "leibovitz" } } ``` From fc41f7a51138b02ec9fe19610ea5d12b8f9ad640 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 3 Aug 2023 12:27:33 -0400 Subject: [PATCH 12/12] docs/route_table: Trim down example --- website/docs/r/route_table.html.markdown | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/website/docs/r/route_table.html.markdown b/website/docs/r/route_table.html.markdown index da9da1aaa8b..546df7a7804 100644 --- a/website/docs/r/route_table.html.markdown +++ b/website/docs/r/route_table.html.markdown @@ -76,10 +76,6 @@ First, adopt an existing AWS-created route: ```terraform resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" - - tags = { - Name = "leibovitz" - } } resource "aws_route_table" "test" { @@ -90,10 +86,6 @@ resource "aws_route_table" "test" { cidr_block = "10.1.0.0/16" gateway_id = "local" } - - tags = { - Name = "leibovitz" - } } ``` @@ -102,10 +94,6 @@ Next, update the target of the route: ```terraform resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" - - tags = { - Name = "leibovitz" - } } resource "aws_route_table" "test" { @@ -115,27 +103,15 @@ resource "aws_route_table" "test" { cidr_block = aws_vpc.test.cidr_block network_interface_id = aws_network_interface.test.id } - - tags = { - Name = "leibovitz" - } } resource "aws_subnet" "test" { cidr_block = "10.1.1.0/24" vpc_id = aws_vpc.test.id - - tags = { - Name = "leibovitz" - } } resource "aws_network_interface" "test" { subnet_id = aws_subnet.test.id - - tags = { - Name = "leibovitz" - } } ```