Skip to content

Commit 80a5123

Browse files
YakDriverYakDriver
authored
Merge pull request #44542 from hashicorp/f-ec2-transit-inconsist
[bug] Fix inconsistent final plan for tgw association/propagation
2 parents 9dcdb0e + 145d3e9 commit 80a5123

10 files changed

+380
-87
lines changed

‎.changelog/44542.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
resource/aws_ec2_transit_gateway_route_table_propagation.test: Fix bug causing `inconsistent final plan` errors
3+
```

‎internal/service/ec2/transitgateway_route_table_association.go‎

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
awstypes "github.com/aws/aws-sdk-go-v2/service/ec2/types"
1515
"github.com/hashicorp/aws-sdk-go-base/v2/tfawserr"
1616
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
17-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
1817
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1918
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
2019
"github.com/hashicorp/terraform-provider-aws/internal/conns"
@@ -52,6 +51,7 @@ func resourceTransitGatewayRouteTableAssociation() *schema.Resource {
5251
names.AttrTransitGatewayAttachmentID: {
5352
Type: schema.TypeString,
5453
Required: true,
54+
ForceNew: true,
5555
ValidateFunc: validation.NoZeroValues,
5656
},
5757
"transit_gateway_route_table_id": {
@@ -61,43 +61,6 @@ func resourceTransitGatewayRouteTableAssociation() *schema.Resource {
6161
ValidateFunc: validation.NoZeroValues,
6262
},
6363
},
64-
65-
CustomizeDiff: customdiff.Sequence(
66-
func(_ context.Context, d *schema.ResourceDiff, meta any) error {
67-
if !d.HasChange(names.AttrTransitGatewayAttachmentID) {
68-
return nil
69-
}
70-
71-
// See https://github.com/hashicorp/terraform-provider-aws/issues/30085
72-
// In all cases, changes should force new except:
73-
// o is not empty string AND
74-
// n is empty string AND
75-
// plan is unknown AND
76-
// state is known
77-
o, n := d.GetChange(names.AttrTransitGatewayAttachmentID)
78-
if o.(string) == "" || n.(string) != "" {
79-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
80-
}
81-
82-
rawPlan := d.GetRawPlan()
83-
plan := rawPlan.GetAttr(names.AttrTransitGatewayAttachmentID)
84-
if plan.IsKnown() {
85-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
86-
}
87-
88-
rawState := d.GetRawState()
89-
if rawState.IsNull() || !rawState.IsKnown() {
90-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
91-
}
92-
93-
state := rawState.GetAttr(names.AttrTransitGatewayAttachmentID)
94-
if !state.IsKnown() {
95-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
96-
}
97-
98-
return nil
99-
},
100-
),
10164
}
10265
}
10366

‎internal/service/ec2/transitgateway_route_table_association_test.go‎

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,49 @@ func testAccTransitGatewayRouteTableAssociation_disappears(t *testing.T, semapho
9797
})
9898
}
9999

100+
func testAccTransitGatewayRouteTableAssociation_attachmentChange(t *testing.T, semaphore tfsync.Semaphore) {
101+
if testing.Short() {
102+
t.Skip("skipping long-running test in short mode")
103+
}
104+
105+
ctx := acctest.Context(t)
106+
var v awstypes.TransitGatewayRouteTableAssociation
107+
resourceName := "aws_ec2_transit_gateway_route_table_association.test"
108+
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
109+
110+
resource.ParallelTest(t, resource.TestCase{
111+
PreCheck: func() {
112+
testAccPreCheckTransitGatewaySynchronize(t, semaphore)
113+
acctest.PreCheck(ctx, t)
114+
testAccPreCheckTransitGateway(ctx, t)
115+
},
116+
ErrorCheck: acctest.ErrorCheck(t, names.EC2ServiceID),
117+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
118+
CheckDestroy: testAccCheckTransitGatewayRouteTableAssociationDestroy(ctx),
119+
Steps: []resource.TestStep{
120+
{
121+
Config: testAccTransitGatewayRouteTableAssociationConfig_attachmentChange(rName, 0),
122+
Check: resource.ComposeTestCheckFunc(
123+
testAccCheckTransitGatewayRouteTableAssociationExists(ctx, resourceName, &v),
124+
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.0", names.AttrID),
125+
),
126+
},
127+
{
128+
Config: testAccTransitGatewayRouteTableAssociationConfig_attachmentChange(rName, 1),
129+
Check: resource.ComposeTestCheckFunc(
130+
testAccCheckTransitGatewayRouteTableAssociationExists(ctx, resourceName, &v),
131+
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.1", names.AttrID),
132+
),
133+
ConfigPlanChecks: resource.ConfigPlanChecks{
134+
PreApply: []plancheck.PlanCheck{
135+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
136+
},
137+
},
138+
},
139+
},
140+
})
141+
}
142+
100143
func testAccTransitGatewayRouteTableAssociation_replaceExistingAssociation(t *testing.T, semaphore tfsync.Semaphore) {
101144
if testing.Short() {
102145
t.Skip("skipping long-running test in short mode")
@@ -142,7 +185,7 @@ func testAccTransitGatewayRouteTableAssociation_replaceExistingAssociation(t *te
142185
})
143186
}
144187

145-
func testAccTransitGatewayRouteTableAssociation_notRecreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
188+
func testAccTransitGatewayRouteTableAssociation_recreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
146189
if testing.Short() {
147190
t.Skip("skipping long-running test in short mode")
148191
}
@@ -174,11 +217,9 @@ func testAccTransitGatewayRouteTableAssociation_notRecreatedDXGateway(t *testing
174217
Check: resource.ComposeTestCheckFunc(
175218
testAccCheckTransitGatewayRouteTableAssociationExists(ctx, resourceName, &a),
176219
),
177-
// Calling a NotRecreated function, such as testAccCheckRouteTableAssociationNotRecreated, as is typical,
178-
// won't work here because the recreated resource ID will be the same, because it's two IDs pegged together.
179220
ConfigPlanChecks: resource.ConfigPlanChecks{
180221
PreApply: []plancheck.PlanCheck{
181-
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
222+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
182223
},
183224
},
184225
},
@@ -344,3 +385,70 @@ resource "aws_ec2_transit_gateway_route_table_propagation" "test" {
344385
}
345386
`, rName, rBGPASN, strings.Join(allowedPrefixes, `", "`))
346387
}
388+
389+
func testAccTransitGatewayRouteTableAssociationConfig_attachmentChange(rName string, attachmentIndex int) string {
390+
return fmt.Sprintf(`
391+
resource "aws_vpc" "test" {
392+
count = 2
393+
394+
cidr_block = "10.${count.index}.0.0/16"
395+
396+
tags = {
397+
Name = "%[1]s-${count.index}"
398+
}
399+
}
400+
401+
resource "aws_subnet" "test" {
402+
count = 2
403+
404+
vpc_id = aws_vpc.test[count.index].id
405+
cidr_block = "10.${count.index}.0.0/24"
406+
availability_zone = data.aws_availability_zones.available.names[0]
407+
408+
tags = {
409+
Name = "%[1]s-${count.index}"
410+
}
411+
}
412+
413+
data "aws_availability_zones" "available" {
414+
state = "available"
415+
416+
filter {
417+
name = "opt-in-status"
418+
values = ["opt-in-not-required"]
419+
}
420+
}
421+
422+
resource "aws_ec2_transit_gateway" "test" {
423+
tags = {
424+
Name = %[1]q
425+
}
426+
}
427+
428+
resource "aws_ec2_transit_gateway_vpc_attachment" "test" {
429+
count = 2
430+
431+
subnet_ids = [aws_subnet.test[count.index].id]
432+
transit_gateway_default_route_table_association = false
433+
transit_gateway_id = aws_ec2_transit_gateway.test.id
434+
vpc_id = aws_vpc.test[count.index].id
435+
436+
tags = {
437+
Name = "%[1]s-${count.index}"
438+
}
439+
}
440+
441+
resource "aws_ec2_transit_gateway_route_table" "test" {
442+
transit_gateway_id = aws_ec2_transit_gateway.test.id
443+
444+
tags = {
445+
Name = %[1]q
446+
}
447+
}
448+
449+
resource "aws_ec2_transit_gateway_route_table_association" "test" {
450+
transit_gateway_attachment_id = aws_ec2_transit_gateway_vpc_attachment.test[%[2]d].id
451+
transit_gateway_route_table_id = aws_ec2_transit_gateway_route_table.test.id
452+
}
453+
`, rName, attachmentIndex)
454+
}

‎internal/service/ec2/transitgateway_route_table_propagation.go‎

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/aws/aws-sdk-go-v2/service/ec2"
1414
"github.com/hashicorp/aws-sdk-go-base/v2/tfawserr"
1515
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
16-
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
1716
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1817
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1918
"github.com/hashicorp/terraform-provider-aws/internal/conns"
@@ -45,6 +44,7 @@ func resourceTransitGatewayRouteTablePropagation() *schema.Resource {
4544
names.AttrTransitGatewayAttachmentID: {
4645
Type: schema.TypeString,
4746
Required: true,
47+
ForceNew: true,
4848
ValidateFunc: validation.NoZeroValues,
4949
},
5050
"transit_gateway_route_table_id": {
@@ -54,42 +54,6 @@ func resourceTransitGatewayRouteTablePropagation() *schema.Resource {
5454
ValidateFunc: validation.NoZeroValues,
5555
},
5656
},
57-
CustomizeDiff: customdiff.Sequence(
58-
func(_ context.Context, d *schema.ResourceDiff, meta any) error {
59-
if !d.HasChange(names.AttrTransitGatewayAttachmentID) {
60-
return nil
61-
}
62-
63-
// See https://github.com/hashicorp/terraform-provider-aws/issues/30085
64-
// In all cases, changes should force new except:
65-
// o is not empty string AND
66-
// n is empty string AND
67-
// plan is unknown AND
68-
// state is known
69-
o, n := d.GetChange(names.AttrTransitGatewayAttachmentID)
70-
if o.(string) == "" || n.(string) != "" {
71-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
72-
}
73-
74-
rawPlan := d.GetRawPlan()
75-
plan := rawPlan.GetAttr(names.AttrTransitGatewayAttachmentID)
76-
if plan.IsKnown() {
77-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
78-
}
79-
80-
rawState := d.GetRawState()
81-
if rawState.IsNull() || !rawState.IsKnown() {
82-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
83-
}
84-
85-
state := rawState.GetAttr(names.AttrTransitGatewayAttachmentID)
86-
if !state.IsKnown() {
87-
return d.ForceNew(names.AttrTransitGatewayAttachmentID)
88-
}
89-
90-
return nil
91-
},
92-
),
9357
}
9458
}
9559

‎internal/service/ec2/transitgateway_route_table_propagation_test.go‎

Lines changed: 107 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,46 @@ func testAccTransitGatewayRouteTablePropagation_disappears(t *testing.T, semapho
8787
})
8888
}
8989

90-
func testAccTransitGatewayRouteTablePropagtion_notRecreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
90+
func testAccTransitGatewayRouteTablePropagation_attachmentChange(t *testing.T, semaphore tfsync.Semaphore) {
91+
ctx := acctest.Context(t)
92+
var v awstypes.TransitGatewayRouteTablePropagation
93+
resourceName := "aws_ec2_transit_gateway_route_table_propagation.test"
94+
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
95+
96+
resource.ParallelTest(t, resource.TestCase{
97+
PreCheck: func() {
98+
testAccPreCheckTransitGatewaySynchronize(t, semaphore)
99+
acctest.PreCheck(ctx, t)
100+
testAccPreCheckTransitGateway(ctx, t)
101+
},
102+
ErrorCheck: acctest.ErrorCheck(t, names.EC2ServiceID),
103+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
104+
CheckDestroy: testAccCheckTransitGatewayRouteTablePropagationDestroy(ctx),
105+
Steps: []resource.TestStep{
106+
{
107+
Config: testAccTransitGatewayRouteTablePropagationConfig_attachmentChange(rName, 0),
108+
Check: resource.ComposeTestCheckFunc(
109+
testAccCheckTransitGatewayRouteTablePropagationExists(ctx, resourceName, &v),
110+
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.0", names.AttrID),
111+
),
112+
},
113+
{
114+
Config: testAccTransitGatewayRouteTablePropagationConfig_attachmentChange(rName, 1),
115+
Check: resource.ComposeTestCheckFunc(
116+
testAccCheckTransitGatewayRouteTablePropagationExists(ctx, resourceName, &v),
117+
resource.TestCheckResourceAttrPair(resourceName, names.AttrTransitGatewayAttachmentID, "aws_ec2_transit_gateway_vpc_attachment.test.1", names.AttrID),
118+
),
119+
ConfigPlanChecks: resource.ConfigPlanChecks{
120+
PreApply: []plancheck.PlanCheck{
121+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
122+
},
123+
},
124+
},
125+
},
126+
})
127+
}
128+
129+
func testAccTransitGatewayRouteTablePropagtion_recreatedDXGateway(t *testing.T, semaphore tfsync.Semaphore) {
91130
if testing.Short() {
92131
t.Skip("skipping long-running test in short mode")
93132
}
@@ -119,11 +158,9 @@ func testAccTransitGatewayRouteTablePropagtion_notRecreatedDXGateway(t *testing.
119158
Check: resource.ComposeTestCheckFunc(
120159
testAccCheckTransitGatewayRouteTablePropagationExists(ctx, resourceName, &a),
121160
),
122-
// Calling a NotRecreated function, such as testAccCheckRouteTableAssociationNotRecreated, as is typical,
123-
// won't work here because the recreated resource ID will be the same, because it's two IDs pegged together.
124161
ConfigPlanChecks: resource.ConfigPlanChecks{
125162
PreApply: []plancheck.PlanCheck{
126-
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
163+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
127164
},
128165
},
129166
},
@@ -262,3 +299,69 @@ resource "aws_ec2_transit_gateway_route_table_propagation" "test" {
262299
}
263300
`, rName, rBGPASN, strings.Join(allowedPrefixes, `", "`))
264301
}
302+
303+
func testAccTransitGatewayRouteTablePropagationConfig_attachmentChange(rName string, attachmentIndex int) string {
304+
return fmt.Sprintf(`
305+
resource "aws_vpc" "test" {
306+
count = 2
307+
308+
cidr_block = "10.${count.index}.0.0/16"
309+
310+
tags = {
311+
Name = "%[1]s-${count.index}"
312+
}
313+
}
314+
315+
resource "aws_subnet" "test" {
316+
count = 2
317+
318+
vpc_id = aws_vpc.test[count.index].id
319+
cidr_block = "10.${count.index}.0.0/24"
320+
availability_zone = data.aws_availability_zones.available.names[0]
321+
322+
tags = {
323+
Name = "%[1]s-${count.index}"
324+
}
325+
}
326+
327+
data "aws_availability_zones" "available" {
328+
state = "available"
329+
330+
filter {
331+
name = "opt-in-status"
332+
values = ["opt-in-not-required"]
333+
}
334+
}
335+
336+
resource "aws_ec2_transit_gateway" "test" {
337+
tags = {
338+
Name = %[1]q
339+
}
340+
}
341+
342+
resource "aws_ec2_transit_gateway_vpc_attachment" "test" {
343+
count = 2
344+
345+
subnet_ids = [aws_subnet.test[count.index].id]
346+
transit_gateway_id = aws_ec2_transit_gateway.test.id
347+
vpc_id = aws_vpc.test[count.index].id
348+
349+
tags = {
350+
Name = "%[1]s-${count.index}"
351+
}
352+
}
353+
354+
resource "aws_ec2_transit_gateway_route_table" "test" {
355+
transit_gateway_id = aws_ec2_transit_gateway.test.id
356+
357+
tags = {
358+
Name = %[1]q
359+
}
360+
}
361+
362+
resource "aws_ec2_transit_gateway_route_table_propagation" "test" {
363+
transit_gateway_attachment_id = aws_ec2_transit_gateway_vpc_attachment.test[%[2]d].id
364+
transit_gateway_route_table_id = aws_ec2_transit_gateway_route_table.test.id
365+
}
366+
`, rName, attachmentIndex)
367+
}

0 commit comments

Comments
 (0)