Skip to content

Commit 414ba49

Browse files
YakDriverYakDriver
authored
Merge pull request #44518 from hashicorp/b-identity-for-framework
Fix framework identity interceptor
2 parents 65428fa + 4252f64 commit 414ba49

File tree

3 files changed

+235
-0
lines changed

3 files changed

+235
-0
lines changed

‎.changelog/44518.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
```release-note:bug
2+
provider: Fix `Missing Resource Identity After Update` errors for non-refreshed and failed updates of Plugin Framework based resources
3+
```
4+
```release-note:bug
5+
provider: Fix `Unexpected Identity Change` errors when fully-null identity values in state are updated to valid values for Plugin Framework based resources
6+
```

‎internal/provider/framework/identity_interceptor.go‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"github.com/hashicorp/terraform-plugin-framework/attr"
1010
"github.com/hashicorp/terraform-plugin-framework/path"
1111
"github.com/hashicorp/terraform-plugin-framework/resource"
12+
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
13+
"github.com/hashicorp/terraform-plugin-framework/types"
1214
inttypes "github.com/hashicorp/terraform-provider-aws/internal/types"
1315
"github.com/hashicorp/terraform-provider-aws/names"
1416
)
@@ -56,6 +58,41 @@ func (r identityInterceptor) create(ctx context.Context, opts interceptorOptions
5658
}
5759
}
5860
}
61+
case OnError:
62+
identity := response.Identity
63+
if identity == nil {
64+
break
65+
}
66+
67+
if identityIsFullyNull(ctx, identity, r.attributes) {
68+
for _, att := range r.attributes {
69+
switch att.Name() {
70+
case names.AttrAccountID:
71+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), awsClient.AccountID(ctx))...)
72+
if opts.response.Diagnostics.HasError() {
73+
return
74+
}
75+
76+
case names.AttrRegion:
77+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), awsClient.Region(ctx))...)
78+
if opts.response.Diagnostics.HasError() {
79+
return
80+
}
81+
82+
default:
83+
var attrVal attr.Value
84+
opts.response.Diagnostics.Append(response.State.GetAttribute(ctx, path.Root(att.ResourceAttributeName()), &attrVal)...)
85+
if opts.response.Diagnostics.HasError() {
86+
return
87+
}
88+
89+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), attrVal)...)
90+
if opts.response.Diagnostics.HasError() {
91+
return
92+
}
93+
}
94+
}
95+
}
5996
}
6097
}
6198

@@ -103,11 +140,109 @@ func (r identityInterceptor) read(ctx context.Context, opts interceptorOptions[r
103140
}
104141

105142
func (r identityInterceptor) update(ctx context.Context, opts interceptorOptions[resource.UpdateRequest, resource.UpdateResponse]) {
143+
awsClient := opts.c
144+
145+
switch response, when := opts.response, opts.when; when {
146+
case After:
147+
if response.State.Raw.IsNull() {
148+
break
149+
}
150+
identity := response.Identity
151+
if identity == nil {
152+
break
153+
}
154+
155+
for _, att := range r.attributes {
156+
switch att.Name() {
157+
case names.AttrAccountID:
158+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), awsClient.AccountID(ctx))...)
159+
if opts.response.Diagnostics.HasError() {
160+
return
161+
}
162+
163+
case names.AttrRegion:
164+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), awsClient.Region(ctx))...)
165+
if opts.response.Diagnostics.HasError() {
166+
return
167+
}
168+
169+
default:
170+
var attrVal attr.Value
171+
opts.response.Diagnostics.Append(response.State.GetAttribute(ctx, path.Root(att.ResourceAttributeName()), &attrVal)...)
172+
if opts.response.Diagnostics.HasError() {
173+
return
174+
}
175+
176+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), attrVal)...)
177+
if opts.response.Diagnostics.HasError() {
178+
return
179+
}
180+
}
181+
}
182+
case OnError:
183+
if response.State.Raw.IsNull() {
184+
break
185+
}
186+
identity := response.Identity
187+
if identity == nil {
188+
break
189+
}
190+
191+
if identityIsFullyNull(ctx, identity, r.attributes) {
192+
for _, att := range r.attributes {
193+
switch att.Name() {
194+
case names.AttrAccountID:
195+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), awsClient.AccountID(ctx))...)
196+
if opts.response.Diagnostics.HasError() {
197+
return
198+
}
199+
200+
case names.AttrRegion:
201+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), awsClient.Region(ctx))...)
202+
if opts.response.Diagnostics.HasError() {
203+
return
204+
}
205+
206+
default:
207+
var attrVal attr.Value
208+
opts.response.Diagnostics.Append(response.State.GetAttribute(ctx, path.Root(att.ResourceAttributeName()), &attrVal)...)
209+
if opts.response.Diagnostics.HasError() {
210+
return
211+
}
212+
213+
opts.response.Diagnostics.Append(identity.SetAttribute(ctx, path.Root(att.Name()), attrVal)...)
214+
if opts.response.Diagnostics.HasError() {
215+
return
216+
}
217+
}
218+
}
219+
}
220+
}
106221
}
107222

108223
func (r identityInterceptor) delete(ctx context.Context, opts interceptorOptions[resource.DeleteRequest, resource.DeleteResponse]) {
109224
}
110225

226+
// identityIsFullyNull returns true if a resource supports identity and
227+
// all attributes are set to null values
228+
func identityIsFullyNull(ctx context.Context, identity *tfsdk.ResourceIdentity, attributes []inttypes.IdentityAttribute) bool {
229+
if identity == nil {
230+
return true
231+
}
232+
233+
for _, attr := range attributes {
234+
var attrVal types.String
235+
if diags := identity.GetAttribute(ctx, path.Root(attr.Name()), &attrVal); diags.HasError() {
236+
return false
237+
}
238+
if !attrVal.IsNull() && attrVal.ValueString() != "" {
239+
return false
240+
}
241+
}
242+
243+
return true
244+
}
245+
111246
func newIdentityInterceptor(attributes []inttypes.IdentityAttribute) identityInterceptor {
112247
return identityInterceptor{
113248
attributes: attributes,

‎internal/provider/framework/identity_interceptor_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,3 +269,97 @@ func (c mockClient) ValidateInContextRegionInPartition(ctx context.Context) erro
269269
func (c mockClient) AwsConfig(context.Context) aws.Config { // nosemgrep:ci.aws-in-func-name
270270
panic("not implemented") //lintignore:R009
271271
}
272+
273+
func TestIdentityIsFullyNull(t *testing.T) {
274+
t.Parallel()
275+
276+
attributes := []inttypes.IdentityAttribute{
277+
inttypes.StringIdentityAttribute("account_id", false),
278+
inttypes.StringIdentityAttribute("region", false),
279+
inttypes.StringIdentityAttribute("bucket", true),
280+
}
281+
282+
// Create identity schema once for all test cases
283+
identitySchema := &identityschema.Schema{
284+
Attributes: map[string]identityschema.Attribute{
285+
"account_id": identityschema.StringAttribute{},
286+
"region": identityschema.StringAttribute{},
287+
"bucket": identityschema.StringAttribute{},
288+
},
289+
}
290+
291+
ctx := context.Background()
292+
293+
// Helper function to create identity with values
294+
createIdentityWithValues := func(values map[string]string) *tfsdk.ResourceIdentity {
295+
if values == nil {
296+
return nil
297+
}
298+
identity := emtpyIdentityFromSchema(ctx, identitySchema)
299+
for attrName, value := range values {
300+
if value != "" {
301+
diags := identity.SetAttribute(ctx, path.Root(attrName), value)
302+
if diags.HasError() {
303+
t.Fatalf("unexpected error setting %s in identity: %s", attrName, fwdiag.DiagnosticsError(diags))
304+
}
305+
}
306+
}
307+
return identity
308+
}
309+
310+
testCases := map[string]struct {
311+
identity *tfsdk.ResourceIdentity
312+
expectNull bool
313+
description string
314+
}{
315+
"all_null": {
316+
identity: createIdentityWithValues(map[string]string{}),
317+
expectNull: true,
318+
description: "All attributes null should return true",
319+
},
320+
"some_null": {
321+
identity: createIdentityWithValues(map[string]string{
322+
"account_id": "123456789012",
323+
// region and bucket remain null
324+
}),
325+
expectNull: false,
326+
description: "Some attributes set should return false",
327+
},
328+
"all_set": {
329+
identity: createIdentityWithValues(map[string]string{
330+
"account_id": "123456789012",
331+
"region": "us-west-2", // lintignore:AWSAT003
332+
"bucket": "test-bucket",
333+
}),
334+
expectNull: false,
335+
description: "All attributes set should return false",
336+
},
337+
"empty_string_values": {
338+
identity: createIdentityWithValues(map[string]string{
339+
"account_id": "",
340+
"region": "",
341+
"bucket": "",
342+
}),
343+
expectNull: true,
344+
description: "Empty string values should be treated as null",
345+
},
346+
"nil_identity": {
347+
identity: createIdentityWithValues(nil),
348+
expectNull: true,
349+
description: "Nil identity should return true",
350+
},
351+
}
352+
353+
for name, tc := range testCases {
354+
t.Run(name, func(t *testing.T) {
355+
t.Parallel()
356+
ctx := context.Background()
357+
358+
result := identityIsFullyNull(ctx, tc.identity, attributes)
359+
if result != tc.expectNull {
360+
t.Errorf("%s: expected identityIsFullyNull to return %v, got %v",
361+
tc.description, tc.expectNull, result)
362+
}
363+
})
364+
}
365+
}

0 commit comments

Comments
 (0)