Skip to content

Commit 6553df9

Browse files
committed
Merge branch 'main' into b-generate-existing-resource-norefresh-nochange-test-v6-nulls
2 parents 6cb1e5c + 85bbaaa commit 6553df9

File tree

10 files changed

+153
-196
lines changed

10 files changed

+153
-196
lines changed

.changelog/44375.txt

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
```release-note:note
2+
provider: This release contains both internal provider fixes and a Terraform Plugin SDK V2 update related to a [regression](https://github.com/hashicorp/terraform-provider-aws/issues/44366) which may impact resources that support resource identity
3+
```
4+
5+
```release-note:bug
6+
provider: Fix `Missing Resource Identity After Update` errors for non-refreshed and failed updates
7+
```
8+
```release-note:bug
9+
provider: Fix `Unexpected Identity Change` errors when fully-null identity values in state are updated to valid values
10+
```

CHANGELOG.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
## 6.15.0 (Unreleased)
1+
## 6.14.1 (September 22, 2025)
2+
3+
NOTES:
4+
5+
* provider: This release contains both internal provider fixes and a Terraform Plugin SDK V2 update related to a [regression](https://github.com/hashicorp/terraform-provider-aws/issues/44366) which may impact resources that support resource identity ([#44375](https://github.com/hashicorp/terraform-provider-aws/issues/44375))
6+
7+
BUG FIXES:
8+
9+
* provider: Fix `Missing Resource Identity After Update` errors for non-refreshed and failed updates ([#44375](https://github.com/hashicorp/terraform-provider-aws/issues/44375))
10+
* provider: Fix `Unexpected Identity Change` errors when fully-null identity values in state are updated to valid values ([#44375](https://github.com/hashicorp/terraform-provider-aws/issues/44375))
211

312
## 6.14.0 (September 18, 2025)
413

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ require (
299299
github.com/hashicorp/terraform-plugin-go v0.29.0
300300
github.com/hashicorp/terraform-plugin-log v0.9.0
301301
github.com/hashicorp/terraform-plugin-mux v0.21.0
302-
github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.0
302+
github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.1
303303
github.com/hashicorp/terraform-plugin-testing v1.14.0-beta.1
304304
github.com/jaswdr/faker/v2 v2.8.0
305305
github.com/jmespath/go-jmespath v0.4.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -683,8 +683,8 @@ github.com/hashicorp/terraform-plugin-go v0.29.0 h1:1nXKl/nSpaYIUBU1IG/EsDOX0vv+
683683
github.com/hashicorp/terraform-plugin-go v0.29.0/go.mod h1:vYZbIyvxyy0FWSmDHChCqKvI40cFTDGSb3D8D70i9GM=
684684
github.com/hashicorp/terraform-plugin-mux v0.21.0 h1:QsEYnzSD2c3zT8zUrUGqaFGhV/Z8zRUlU7FY3ZPJFfw=
685685
github.com/hashicorp/terraform-plugin-mux v0.21.0/go.mod h1:Qpt8+6AD7NmL0DS7ASkN0EXpDQ2J/FnnIgeUr1tzr5A=
686-
github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.0 h1:PQP7Crrc7t/ozj+P9x0/lsTzGNy3lVppH8zAJylofaE=
687-
github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.0/go.mod h1:GQhpKVvvuwzD79e8/NZ+xzj+ZpWovdPAe8nfV/skwNU=
686+
github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.1 h1:mlAq/OrMlg04IuJT7NpefI1wwtdpWudnEmjuQs04t/4=
687+
github.com/hashicorp/terraform-plugin-sdk/v2 v2.38.1/go.mod h1:GQhpKVvvuwzD79e8/NZ+xzj+ZpWovdPAe8nfV/skwNU=
688688
github.com/hashicorp/terraform-plugin-testing v1.14.0-beta.1 h1:caWmY2Fv/KgDAXU7IVjcBDfIdmr/n6VRYhCLxNmlaXs=
689689
github.com/hashicorp/terraform-plugin-testing v1.14.0-beta.1/go.mod h1:jVm3pD9uQAT0X2RSEdcqjju2bCGv5f73DGZFU4v7EAU=
690690
github.com/hashicorp/terraform-registry-address v0.4.0 h1:S1yCGomj30Sao4l5BMPjTGZmCNzuv7/GDTDX99E9gTk=

internal/provider/sdkv2/identity_interceptor.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,42 @@ func (r identityInterceptor) run(ctx context.Context, opts crudInterceptorOption
6363
}
6464
}
6565
}
66+
case OnError:
67+
switch why {
68+
case Update:
69+
if identityIsFullyNull(d, r.identitySpec) {
70+
if d.Id() == "" {
71+
break
72+
}
73+
identity, err := d.Identity()
74+
if err != nil {
75+
return sdkdiag.AppendFromErr(diags, err)
76+
}
77+
78+
for _, attr := range r.identitySpec.Attributes {
79+
switch attr.Name() {
80+
case names.AttrAccountID:
81+
if err := identity.Set(attr.Name(), awsClient.AccountID(ctx)); err != nil {
82+
return sdkdiag.AppendFromErr(diags, err)
83+
}
84+
85+
case names.AttrRegion:
86+
if err := identity.Set(attr.Name(), awsClient.Region(ctx)); err != nil {
87+
return sdkdiag.AppendFromErr(diags, err)
88+
}
89+
90+
default:
91+
val, ok := getAttributeOk(d, attr.ResourceAttributeName())
92+
if !ok {
93+
continue
94+
}
95+
if err := identity.Set(attr.Name(), val); err != nil {
96+
return sdkdiag.AppendFromErr(diags, err)
97+
}
98+
}
99+
}
100+
}
101+
}
66102
}
67103

68104
return diags
@@ -99,7 +135,7 @@ func getAttributeOk(d schemaResourceData, name string) (string, bool) {
99135

100136
func newIdentityInterceptor(identitySpec *inttypes.Identity) interceptorInvocation {
101137
interceptor := interceptorInvocation{
102-
when: After,
138+
when: After | OnError,
103139
why: Create | Read | Update,
104140
interceptor: identityInterceptor{
105141
identitySpec: identitySpec,

internal/provider/sdkv2/identity_interceptor_test.go

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,32 +189,37 @@ func TestIdentityInterceptor_Update(t *testing.T) {
189189
attrName string
190190
identitySpec inttypes.Identity
191191
ExpectIdentity bool
192+
Description string
192193
}{
193-
"not mutable": {
194+
"not mutable - fresh resource": {
194195
attrName: "name",
195196
identitySpec: regionalSingleParameterizedIdentitySpec("name"),
196-
ExpectIdentity: false,
197+
ExpectIdentity: true,
198+
Description: "Immutable identity with all null attributes should get populated (bug fix scenario)",
197199
},
198200
"v6.0 SDK fix": {
199201
attrName: "name",
200202
identitySpec: regionalSingleParameterizedIdentitySpec("name",
201203
inttypes.WithV6_0SDKv2Fix(),
202204
),
203-
ExpectIdentity: false,
205+
ExpectIdentity: true,
206+
Description: "Mutable identity (v6.0 SDK fix) should always get populated on Update",
204207
},
205208
"identity fix": {
206209
attrName: "name",
207210
identitySpec: regionalSingleParameterizedIdentitySpec("name",
208211
inttypes.WithIdentityFix(),
209212
),
210-
ExpectIdentity: false,
213+
ExpectIdentity: true,
214+
Description: "Mutable identity (identity fix) should always get populated on Update",
211215
},
212216
"mutable": {
213217
attrName: "name",
214218
identitySpec: regionalSingleParameterizedIdentitySpec("name",
215219
inttypes.WithMutableIdentity(),
216220
),
217221
ExpectIdentity: true,
222+
Description: "Explicitly mutable identity should always get populated on Update",
218223
},
219224
}
220225

@@ -317,3 +322,82 @@ func (c mockClient) ValidateInContextRegionInPartition(ctx context.Context) erro
317322
func (c mockClient) AwsConfig(context.Context) aws.Config { // nosemgrep:ci.aws-in-func-name
318323
panic("not implemented") //lintignore:R009
319324
}
325+
326+
func TestIdentityIsFullyNull(t *testing.T) {
327+
t.Parallel()
328+
329+
identitySpec := &inttypes.Identity{
330+
Attributes: []inttypes.IdentityAttribute{
331+
inttypes.StringIdentityAttribute(names.AttrAccountID, false),
332+
inttypes.StringIdentityAttribute(names.AttrRegion, false),
333+
inttypes.StringIdentityAttribute(names.AttrBucket, true),
334+
},
335+
}
336+
337+
testCases := map[string]struct {
338+
identityValues map[string]string
339+
expectNull bool
340+
description string
341+
}{
342+
"all_null": {
343+
identityValues: map[string]string{},
344+
expectNull: true,
345+
description: "All attributes null should return true",
346+
},
347+
"some_null": {
348+
identityValues: map[string]string{
349+
names.AttrAccountID: "123456789012",
350+
// region and bucket remain null
351+
},
352+
expectNull: false,
353+
description: "Some attributes set should return false",
354+
},
355+
"all_set": {
356+
identityValues: map[string]string{
357+
names.AttrAccountID: "123456789012",
358+
names.AttrRegion: "us-west-2", // lintignore:AWSAT003
359+
names.AttrBucket: "test-bucket",
360+
},
361+
expectNull: false,
362+
description: "All attributes set should return false",
363+
},
364+
"empty_string_values": {
365+
identityValues: map[string]string{
366+
names.AttrAccountID: "",
367+
names.AttrRegion: "",
368+
names.AttrBucket: "",
369+
},
370+
expectNull: true,
371+
description: "Empty string values should be treated as null",
372+
},
373+
}
374+
375+
for name, tc := range testCases {
376+
t.Run(name, func(t *testing.T) {
377+
t.Parallel()
378+
379+
resourceSchema := map[string]*schema.Schema{
380+
names.AttrBucket: {Type: schema.TypeString, Required: true},
381+
}
382+
identitySchema := identity.NewIdentitySchema(*identitySpec)
383+
d := schema.TestResourceDataWithIdentityRaw(t, resourceSchema, identitySchema, nil)
384+
d.SetId("test-id")
385+
386+
identity, err := d.Identity()
387+
if err != nil {
388+
t.Fatalf("unexpected error getting identity: %v", err)
389+
}
390+
for attrName, value := range tc.identityValues {
391+
if err := identity.Set(attrName, value); err != nil {
392+
t.Fatalf("unexpected error setting %s in identity: %v", attrName, err)
393+
}
394+
}
395+
396+
result := identityIsFullyNull(d, identitySpec)
397+
if result != tc.expectNull {
398+
t.Errorf("%s: expected identityIsFullyNull to return %v, got %v",
399+
tc.description, tc.expectNull, result)
400+
}
401+
})
402+
}
403+
}

internal/service/iam/role_test.go

Lines changed: 2 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,9 @@ import (
1515
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/id"
1616
sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest"
1717
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
18-
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
1918
"github.com/hashicorp/terraform-plugin-testing/plancheck"
20-
"github.com/hashicorp/terraform-plugin-testing/statecheck"
2119
"github.com/hashicorp/terraform-plugin-testing/terraform"
22-
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
2320
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
24-
tfknownvalue "github.com/hashicorp/terraform-provider-aws/internal/acctest/knownvalue"
25-
tfstatecheck "github.com/hashicorp/terraform-provider-aws/internal/acctest/statecheck"
2621
"github.com/hashicorp/terraform-provider-aws/internal/conns"
2722
"github.com/hashicorp/terraform-provider-aws/internal/errs"
2823
tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam"
@@ -977,63 +972,6 @@ func TestAccIAMRole_ManagedPolicy_outOfBandAdditionRemovedEmpty(t *testing.T) {
977972
})
978973
}
979974

980-
func TestAccIAMRole_Identity_ExistingResource_NoRefresh_WithChange(t *testing.T) {
981-
ctx := acctest.Context(t)
982-
var conf awstypes.Role
983-
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
984-
resourceName := "aws_iam_role.test"
985-
986-
resource.ParallelTest(t, resource.TestCase{
987-
PreCheck: func() { acctest.PreCheck(ctx, t) },
988-
ErrorCheck: acctest.ErrorCheck(t, names.IAMServiceID),
989-
CheckDestroy: testAccCheckRoleDestroy(ctx),
990-
AdditionalCLIOptions: &resource.AdditionalCLIOptions{
991-
Plan: resource.PlanOptions{
992-
NoRefresh: true,
993-
},
994-
},
995-
Steps: []resource.TestStep{
996-
{
997-
ExternalProviders: map[string]resource.ExternalProvider{
998-
"aws": {
999-
Source: "hashicorp/aws",
1000-
VersionConstraint: "5.100.0",
1001-
},
1002-
},
1003-
Config: testAccRoleConfig_basic(rName),
1004-
Check: resource.ComposeAggregateTestCheckFunc(
1005-
testAccCheckRoleExists(ctx, resourceName, &conf),
1006-
),
1007-
ConfigStateChecks: []statecheck.StateCheck{
1008-
tfstatecheck.ExpectNoIdentity(resourceName),
1009-
},
1010-
},
1011-
{
1012-
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
1013-
Config: testAccRoleConfig_description(rName),
1014-
Check: resource.ComposeAggregateTestCheckFunc(
1015-
testAccCheckRoleExists(ctx, resourceName, &conf),
1016-
),
1017-
ConfigPlanChecks: resource.ConfigPlanChecks{
1018-
PreApply: []plancheck.PlanCheck{
1019-
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
1020-
},
1021-
PostApplyPostRefresh: []plancheck.PlanCheck{
1022-
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop),
1023-
},
1024-
},
1025-
ConfigStateChecks: []statecheck.StateCheck{
1026-
statecheck.ExpectIdentity(resourceName, map[string]knownvalue.Check{
1027-
names.AttrAccountID: tfknownvalue.AccountID(),
1028-
names.AttrName: knownvalue.NotNull(),
1029-
}),
1030-
statecheck.ExpectIdentityValueMatchesState(resourceName, tfjsonpath.New(names.AttrName)),
1031-
},
1032-
},
1033-
},
1034-
})
1035-
}
1036-
1037975
func TestAccIAMRole_Identity_ExistingResource_NoRefresh_OnError(t *testing.T) {
1038976
ctx := acctest.Context(t)
1039977
var conf awstypes.Role
@@ -1068,10 +1006,7 @@ func TestAccIAMRole_Identity_ExistingResource_NoRefresh_OnError(t *testing.T) {
10681006
Check: resource.ComposeAggregateTestCheckFunc(
10691007
testAccCheckRoleExists(ctx, resourceName, &conf),
10701008
),
1071-
// On an update failure, the identity interceptor is not executed and both
1072-
// the MalformedPolicyDocument error and a missing resource identity
1073-
// error will be present.
1074-
ExpectError: regexache.MustCompile(`Missing Resource Identity After Update`),
1009+
ExpectError: regexache.MustCompile(`MalformedPolicyDocument: Unknown field invalid`),
10751010
},
10761011
},
10771012
})
@@ -1106,9 +1041,7 @@ func TestAccIAMRole_Identity_ExistingResource_OnError(t *testing.T) {
11061041
Check: resource.ComposeAggregateTestCheckFunc(
11071042
testAccCheckRoleExists(ctx, resourceName, &conf),
11081043
),
1109-
// On an update failure, the identity interceptor is not executed and
1110-
// the MalformedPolicyDocument error will be present.
1111-
ExpectError: regexache.MustCompile(`MalformedPolicyDocument`),
1044+
ExpectError: regexache.MustCompile(`MalformedPolicyDocument: Unknown field invalid`),
11121045
},
11131046
},
11141047
})

0 commit comments

Comments
 (0)