Skip to content
Open
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
3 changes: 3 additions & 0 deletions .changelog/44542.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ec2_transit_gateway_route_table_propagation.test: Fix bug causing `inconsistent final plan` errors
```
39 changes: 1 addition & 38 deletions internal/service/ec2/transitgateway_route_table_association.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
awstypes "github.com/aws/aws-sdk-go-v2/service/ec2/types"
"github.com/hashicorp/aws-sdk-go-base/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
Expand Down Expand Up @@ -52,6 +51,7 @@ func resourceTransitGatewayRouteTableAssociation() *schema.Resource {
names.AttrTransitGatewayAttachmentID: {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.NoZeroValues,
},
"transit_gateway_route_table_id": {
Expand All @@ -61,43 +61,6 @@ func resourceTransitGatewayRouteTableAssociation() *schema.Resource {
ValidateFunc: validation.NoZeroValues,
},
},

CustomizeDiff: customdiff.Sequence(
func(_ context.Context, d *schema.ResourceDiff, meta any) error {
if !d.HasChange(names.AttrTransitGatewayAttachmentID) {
return nil
}

// See https://github.com/hashicorp/terraform-provider-aws/issues/30085
// In all cases, changes should force new except:
// o is not empty string AND
// n is empty string AND
// plan is unknown AND
// state is known
o, n := d.GetChange(names.AttrTransitGatewayAttachmentID)
if o.(string) == "" || n.(string) != "" {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

rawPlan := d.GetRawPlan()
plan := rawPlan.GetAttr(names.AttrTransitGatewayAttachmentID)
if plan.IsKnown() {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

rawState := d.GetRawState()
if rawState.IsNull() || !rawState.IsKnown() {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

state := rawState.GetAttr(names.AttrTransitGatewayAttachmentID)
if !state.IsKnown() {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

return nil
},
),
}
}

Expand Down
116 changes: 112 additions & 4 deletions internal/service/ec2/transitgateway_route_table_association_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,49 @@ func testAccTransitGatewayRouteTableAssociation_disappears(t *testing.T, semapho
})
}

func testAccTransitGatewayRouteTableAssociation_attachmentChange(t *testing.T, semaphore tfsync.Semaphore) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

ctx := acctest.Context(t)
var v awstypes.TransitGatewayRouteTableAssociation
resourceName := "aws_ec2_transit_gateway_route_table_association.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
testAccPreCheckTransitGatewaySynchronize(t, semaphore)
acctest.PreCheck(ctx, t)
testAccPreCheckTransitGateway(ctx, t)
},
ErrorCheck: acctest.ErrorCheck(t, names.EC2ServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckTransitGatewayRouteTableAssociationDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccTransitGatewayRouteTableAssociationConfig_attachmentChange(rName, 0),
Check: resource.ComposeTestCheckFunc(
testAccCheckTransitGatewayRouteTableAssociationExists(ctx, resourceName, &v),
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.0", names.AttrID),
),
},
{
Config: testAccTransitGatewayRouteTableAssociationConfig_attachmentChange(rName, 1),
Check: resource.ComposeTestCheckFunc(
testAccCheckTransitGatewayRouteTableAssociationExists(ctx, resourceName, &v),
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.1", names.AttrID),
),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
},
},
},
},
})
}

func testAccTransitGatewayRouteTableAssociation_replaceExistingAssociation(t *testing.T, semaphore tfsync.Semaphore) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
Expand Down Expand Up @@ -142,7 +185,7 @@ func testAccTransitGatewayRouteTableAssociation_replaceExistingAssociation(t *te
})
}

func testAccTransitGatewayRouteTableAssociation_notRecreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
func testAccTransitGatewayRouteTableAssociation_recreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}
Expand Down Expand Up @@ -174,11 +217,9 @@ func testAccTransitGatewayRouteTableAssociation_notRecreatedDXGateway(t *testing
Check: resource.ComposeTestCheckFunc(
testAccCheckTransitGatewayRouteTableAssociationExists(ctx, resourceName, &a),
),
// Calling a NotRecreated function, such as testAccCheckRouteTableAssociationNotRecreated, as is typical,
// won't work here because the recreated resource ID will be the same, because it's two IDs pegged together.
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
},
},
},
Expand Down Expand Up @@ -344,3 +385,70 @@ resource "aws_ec2_transit_gateway_route_table_propagation" "test" {
}
`, rName, rBGPASN, strings.Join(allowedPrefixes, `", "`))
}

func testAccTransitGatewayRouteTableAssociationConfig_attachmentChange(rName string, attachmentIndex int) string {
return fmt.Sprintf(`
resource "aws_vpc" "test" {
count = 2

cidr_block = "10.${count.index}.0.0/16"

tags = {
Name = "%[1]s-${count.index}"
}
}

resource "aws_subnet" "test" {
count = 2

vpc_id = aws_vpc.test[count.index].id
cidr_block = "10.${count.index}.0.0/24"
availability_zone = data.aws_availability_zones.available.names[0]

tags = {
Name = "%[1]s-${count.index}"
}
}

data "aws_availability_zones" "available" {
state = "available"

filter {
name = "opt-in-status"
values = ["opt-in-not-required"]
}
}

resource "aws_ec2_transit_gateway" "test" {
tags = {
Name = %[1]q
}
}

resource "aws_ec2_transit_gateway_vpc_attachment" "test" {
count = 2

subnet_ids = [aws_subnet.test[count.index].id]
transit_gateway_default_route_table_association = false
transit_gateway_id = aws_ec2_transit_gateway.test.id
vpc_id = aws_vpc.test[count.index].id

tags = {
Name = "%[1]s-${count.index}"
}
}

resource "aws_ec2_transit_gateway_route_table" "test" {
transit_gateway_id = aws_ec2_transit_gateway.test.id

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

resource "aws_ec2_transit_gateway_route_table_association" "test" {
transit_gateway_attachment_id = aws_ec2_transit_gateway_vpc_attachment.test[%[2]d].id
transit_gateway_route_table_id = aws_ec2_transit_gateway_route_table.test.id
}
`, rName, attachmentIndex)
}
38 changes: 1 addition & 37 deletions internal/service/ec2/transitgateway_route_table_propagation.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/aws/aws-sdk-go-v2/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
Expand Down Expand Up @@ -45,6 +44,7 @@ func resourceTransitGatewayRouteTablePropagation() *schema.Resource {
names.AttrTransitGatewayAttachmentID: {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.NoZeroValues,
},
"transit_gateway_route_table_id": {
Expand All @@ -54,42 +54,6 @@ func resourceTransitGatewayRouteTablePropagation() *schema.Resource {
ValidateFunc: validation.NoZeroValues,
},
},
CustomizeDiff: customdiff.Sequence(
func(_ context.Context, d *schema.ResourceDiff, meta any) error {
if !d.HasChange(names.AttrTransitGatewayAttachmentID) {
return nil
}

// See https://github.com/hashicorp/terraform-provider-aws/issues/30085
// In all cases, changes should force new except:
// o is not empty string AND
// n is empty string AND
// plan is unknown AND
// state is known
o, n := d.GetChange(names.AttrTransitGatewayAttachmentID)
if o.(string) == "" || n.(string) != "" {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

rawPlan := d.GetRawPlan()
plan := rawPlan.GetAttr(names.AttrTransitGatewayAttachmentID)
if plan.IsKnown() {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

rawState := d.GetRawState()
if rawState.IsNull() || !rawState.IsKnown() {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

state := rawState.GetAttr(names.AttrTransitGatewayAttachmentID)
if !state.IsKnown() {
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
}

return nil
},
),
}
}

Expand Down
111 changes: 107 additions & 4 deletions internal/service/ec2/transitgateway_route_table_propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,46 @@ func testAccTransitGatewayRouteTablePropagation_disappears(t *testing.T, semapho
})
}

func testAccTransitGatewayRouteTablePropagtion_notRecreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
func testAccTransitGatewayRouteTablePropagation_attachmentChange(t *testing.T, semaphore tfsync.Semaphore) {
ctx := acctest.Context(t)
var v awstypes.TransitGatewayRouteTablePropagation
resourceName := "aws_ec2_transit_gateway_route_table_propagation.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
testAccPreCheckTransitGatewaySynchronize(t, semaphore)
acctest.PreCheck(ctx, t)
testAccPreCheckTransitGateway(ctx, t)
},
ErrorCheck: acctest.ErrorCheck(t, names.EC2ServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckTransitGatewayRouteTablePropagationDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccTransitGatewayRouteTablePropagationConfig_attachmentChange(rName, 0),
Check: resource.ComposeTestCheckFunc(
testAccCheckTransitGatewayRouteTablePropagationExists(ctx, resourceName, &v),
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.0", names.AttrID),
),
},
{
Config: testAccTransitGatewayRouteTablePropagationConfig_attachmentChange(rName, 1),
Check: resource.ComposeTestCheckFunc(
testAccCheckTransitGatewayRouteTablePropagationExists(ctx, resourceName, &v),
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.1", names.AttrID),
),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
},
},
},
},
})
}

func testAccTransitGatewayRouteTablePropagtion_recreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}
Expand Down Expand Up @@ -119,11 +158,9 @@ func testAccTransitGatewayRouteTablePropagtion_notRecreatedDXGateway(t *testing.
Check: resource.ComposeTestCheckFunc(
testAccCheckTransitGatewayRouteTablePropagationExists(ctx, resourceName, &a),
),
// Calling a NotRecreated function, such as testAccCheckRouteTableAssociationNotRecreated, as is typical,
// won't work here because the recreated resource ID will be the same, because it's two IDs pegged together.
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
},
},
},
Expand Down Expand Up @@ -262,3 +299,69 @@ resource "aws_ec2_transit_gateway_route_table_propagation" "test" {
}
`, rName, rBGPASN, strings.Join(allowedPrefixes, `", "`))
}

func testAccTransitGatewayRouteTablePropagationConfig_attachmentChange(rName string, attachmentIndex int) string {
return fmt.Sprintf(`
resource "aws_vpc" "test" {
count = 2

cidr_block = "10.${count.index}.0.0/16"

tags = {
Name = "%[1]s-${count.index}"
}
}

resource "aws_subnet" "test" {
count = 2

vpc_id = aws_vpc.test[count.index].id
cidr_block = "10.${count.index}.0.0/24"
availability_zone = data.aws_availability_zones.available.names[0]

tags = {
Name = "%[1]s-${count.index}"
}
}

data "aws_availability_zones" "available" {
state = "available"

filter {
name = "opt-in-status"
values = ["opt-in-not-required"]
}
}

resource "aws_ec2_transit_gateway" "test" {
tags = {
Name = %[1]q
}
}

resource "aws_ec2_transit_gateway_vpc_attachment" "test" {
count = 2

subnet_ids = [aws_subnet.test[count.index].id]
transit_gateway_id = aws_ec2_transit_gateway.test.id
vpc_id = aws_vpc.test[count.index].id

tags = {
Name = "%[1]s-${count.index}"
}
}

resource "aws_ec2_transit_gateway_route_table" "test" {
transit_gateway_id = aws_ec2_transit_gateway.test.id

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

resource "aws_ec2_transit_gateway_route_table_propagation" "test" {
transit_gateway_attachment_id = aws_ec2_transit_gateway_vpc_attachment.test[%[2]d].id
transit_gateway_route_table_id = aws_ec2_transit_gateway_route_table.test.id
}
`, rName, attachmentIndex)
}
Loading
Loading