Skip to content

Commit bd26b78

Browse files
austinvalleaustinvalle
authored
identity: Update change validation to ensure empty identities (all null attributes) are not validated (#1229)
1 parent 7ab0d80 commit bd26b78

File tree

7 files changed

+240
-3
lines changed

7 files changed

+240
-3
lines changed

‎.changes/unreleased/BUG FIXES-20250922-175218.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
kind: BUG FIXES
2+
body: 'all: Prevent identity change validation from raising an error when prior identity is empty (all attributes are null)'
3+
time: 2025-09-22T17:52:18.613034-04:00
4+
custom:
5+
Issue: "1229"

‎internal/fwserver/server_applyresourcechange_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,24 @@ func TestServerApplyResourceChange(t *testing.T) {
5959
},
6060
}
6161

62+
type testMultiIdentitySchemaData struct {
63+
TestAttrA types.String `tfsdk:"test_attr_a"`
64+
TestAttrB types.Int64 `tfsdk:"test_attr_b"`
65+
}
66+
67+
testMultiAttrIdentitySchema := identityschema.Schema{
68+
Attributes: map[string]identityschema.Attribute{
69+
"test_attr_a": identityschema.StringAttribute{
70+
RequiredForImport: true,
71+
},
72+
"test_attr_b": identityschema.Int64Attribute{
73+
OptionalForImport: true,
74+
},
75+
},
76+
}
77+
78+
testMultiAttrIdentityType := testMultiAttrIdentitySchema.Type().TerraformType(context.Background())
79+
6280
testEmptyPlan := &tfsdk.Plan{
6381
Raw: tftypes.NewValue(testSchemaType, nil),
6482
Schema: testSchema,
@@ -1719,6 +1737,76 @@ func TestServerApplyResourceChange(t *testing.T) {
17191737
Private: testEmptyPrivate,
17201738
},
17211739
},
1740+
"update-response-newidentity-empty-plannedidentity": {
1741+
server: &fwserver.Server{
1742+
Provider: &testprovider.Provider{},
1743+
},
1744+
request: &fwserver.ApplyResourceChangeRequest{
1745+
Config: &tfsdk.Config{
1746+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1747+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1748+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
1749+
}),
1750+
Schema: testSchema,
1751+
},
1752+
PlannedState: &tfsdk.Plan{
1753+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1754+
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
1755+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
1756+
}),
1757+
Schema: testSchema,
1758+
},
1759+
PriorState: &tfsdk.State{
1760+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1761+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1762+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
1763+
}),
1764+
Schema: testSchema,
1765+
},
1766+
PlannedIdentity: &tfsdk.ResourceIdentity{
1767+
Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{
1768+
"test_attr_a": tftypes.NewValue(tftypes.String, nil),
1769+
"test_attr_b": tftypes.NewValue(tftypes.Number, nil),
1770+
}),
1771+
Schema: testMultiAttrIdentitySchema,
1772+
},
1773+
IdentitySchema: testMultiAttrIdentitySchema,
1774+
ResourceSchema: testSchema,
1775+
Resource: &testprovider.ResourceWithIdentity{
1776+
Resource: &testprovider.Resource{
1777+
CreateMethod: func(_ context.Context, _ resource.CreateRequest, resp *resource.CreateResponse) {
1778+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Create")
1779+
},
1780+
DeleteMethod: func(_ context.Context, _ resource.DeleteRequest, resp *resource.DeleteResponse) {
1781+
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Delete")
1782+
},
1783+
UpdateMethod: func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
1784+
resp.Diagnostics.Append(resp.Identity.Set(ctx, testMultiIdentitySchemaData{
1785+
TestAttrA: types.StringValue("new value"),
1786+
TestAttrB: types.Int64Value(20),
1787+
})...)
1788+
},
1789+
},
1790+
},
1791+
},
1792+
expectedResponse: &fwserver.ApplyResourceChangeResponse{
1793+
NewIdentity: &tfsdk.ResourceIdentity{
1794+
Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{
1795+
"test_attr_a": tftypes.NewValue(tftypes.String, "new value"),
1796+
"test_attr_b": tftypes.NewValue(tftypes.Number, 20),
1797+
}),
1798+
Schema: testMultiAttrIdentitySchema,
1799+
},
1800+
NewState: &tfsdk.State{
1801+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
1802+
"test_computed": tftypes.NewValue(tftypes.String, nil),
1803+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
1804+
}),
1805+
Schema: testSchema,
1806+
},
1807+
Private: testEmptyPrivate,
1808+
},
1809+
},
17221810
"update-response-newidentity-with-plannedidentity": {
17231811
server: &fwserver.Server{
17241812
Provider: &testprovider.Provider{},

‎internal/fwserver/server_planresourcechange.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ func (s *Server) PlanResourceChange(ctx context.Context, req *PlanResourceChange
383383
}
384384

385385
// If we're updating or deleting and we already have an identity stored, validate that the planned identity isn't changing
386-
if !req.ResourceBehavior.MutableIdentity && !req.PriorState.Raw.IsNull() && !req.PriorIdentity.Raw.IsNull() && !req.PriorIdentity.Raw.Equal(resp.PlannedIdentity.Raw) {
386+
if !req.ResourceBehavior.MutableIdentity && !req.PriorState.Raw.IsNull() && !req.PriorIdentity.Raw.IsFullyNull() && !req.PriorIdentity.Raw.Equal(resp.PlannedIdentity.Raw) {
387387
resp.Diagnostics.AddError(
388388
"Unexpected Identity Change",
389389
"During the planning operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+

‎internal/fwserver/server_planresourcechange_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,24 @@ func TestServerPlanResourceChange(t *testing.T) {
589589
},
590590
}
591591

592+
type testMultiIdentitySchemaData struct {
593+
TestAttrA types.String `tfsdk:"test_attr_a"`
594+
TestAttrB types.Int64 `tfsdk:"test_attr_b"`
595+
}
596+
597+
testMultiAttrIdentitySchema := identityschema.Schema{
598+
Attributes: map[string]identityschema.Attribute{
599+
"test_attr_a": identityschema.StringAttribute{
600+
RequiredForImport: true,
601+
},
602+
"test_attr_b": identityschema.Int64Attribute{
603+
OptionalForImport: true,
604+
},
605+
},
606+
}
607+
608+
testMultiAttrIdentityType := testMultiAttrIdentitySchema.Type().TerraformType(context.Background())
609+
592610
testSchemaWriteOnly := schema.Schema{
593611
Attributes: map[string]schema.Attribute{
594612
"test_computed": schema.StringAttribute{
@@ -6722,6 +6740,77 @@ func TestServerPlanResourceChange(t *testing.T) {
67226740
PlannedPrivate: testEmptyPrivate,
67236741
},
67246742
},
6743+
"update-resourcewithmodifyplan-empty-prioridentity-plannedidentity-changed": {
6744+
server: &fwserver.Server{
6745+
Provider: &testprovider.Provider{},
6746+
},
6747+
request: &fwserver.PlanResourceChangeRequest{
6748+
Config: &tfsdk.Config{
6749+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
6750+
"test_computed": tftypes.NewValue(tftypes.String, nil),
6751+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
6752+
}),
6753+
Schema: testSchema,
6754+
},
6755+
ProposedNewState: &tfsdk.Plan{
6756+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
6757+
"test_computed": tftypes.NewValue(tftypes.String, nil),
6758+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
6759+
}),
6760+
Schema: testSchema,
6761+
},
6762+
PriorState: &tfsdk.State{
6763+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
6764+
"test_computed": tftypes.NewValue(tftypes.String, nil),
6765+
"test_required": tftypes.NewValue(tftypes.String, "test-old-value"),
6766+
}),
6767+
Schema: testSchema,
6768+
},
6769+
PriorIdentity: &tfsdk.ResourceIdentity{
6770+
Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{
6771+
"test_attr_a": tftypes.NewValue(tftypes.String, nil),
6772+
"test_attr_b": tftypes.NewValue(tftypes.Number, nil),
6773+
}),
6774+
Schema: testMultiAttrIdentitySchema,
6775+
},
6776+
IdentitySchema: testMultiAttrIdentitySchema,
6777+
ResourceSchema: testSchema,
6778+
Resource: &testprovider.ResourceWithIdentityAndModifyPlan{
6779+
ModifyPlanMethod: func(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
6780+
var data testSchemaData
6781+
resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
6782+
data.TestComputed = types.StringValue("test-plannedstate-value")
6783+
resp.Diagnostics.Append(resp.Plan.Set(ctx, &data)...)
6784+
6785+
var identityData testMultiIdentitySchemaData
6786+
resp.Diagnostics.Append(req.Identity.Get(ctx, &identityData)...)
6787+
identityData.TestAttrA = types.StringValue("new value")
6788+
identityData.TestAttrB = types.Int64Value(20)
6789+
resp.Diagnostics.Append(resp.Identity.Set(ctx, &identityData)...)
6790+
},
6791+
IdentitySchemaMethod: func(ctx context.Context, req resource.IdentitySchemaRequest, resp *resource.IdentitySchemaResponse) {
6792+
resp.IdentitySchema = testMultiAttrIdentitySchema
6793+
},
6794+
},
6795+
},
6796+
expectedResponse: &fwserver.PlanResourceChangeResponse{
6797+
PlannedIdentity: &tfsdk.ResourceIdentity{
6798+
Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{
6799+
"test_attr_a": tftypes.NewValue(tftypes.String, "new value"),
6800+
"test_attr_b": tftypes.NewValue(tftypes.Number, 20),
6801+
}),
6802+
Schema: testMultiAttrIdentitySchema,
6803+
},
6804+
PlannedState: &tfsdk.State{
6805+
Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{
6806+
"test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"),
6807+
"test_required": tftypes.NewValue(tftypes.String, "test-new-value"),
6808+
}),
6809+
Schema: testSchema,
6810+
},
6811+
PlannedPrivate: testEmptyPrivate,
6812+
},
6813+
},
67256814
"update-resourcewithmodifyplan-invalid-response-plannedidentity-changed": {
67266815
server: &fwserver.Server{
67276816
Provider: &testprovider.Provider{},

‎internal/fwserver/server_readresource.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ func (s *Server) ReadResource(ctx context.Context, req *ReadResourceRequest, res
185185
}
186186

187187
// If we're refreshing the resource state (excluding a recently imported resource), validate that the new identity isn't changing
188-
if !req.ResourceBehavior.MutableIdentity && !readFollowingImport && !req.CurrentIdentity.Raw.IsNull() && !req.CurrentIdentity.Raw.Equal(resp.NewIdentity.Raw) {
188+
if !req.ResourceBehavior.MutableIdentity && !readFollowingImport && !req.CurrentIdentity.Raw.IsFullyNull() && !req.CurrentIdentity.Raw.Equal(resp.NewIdentity.Raw) {
189189
resp.Diagnostics.AddError(
190190
"Unexpected Identity Change",
191191
"During the read operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+

‎internal/fwserver/server_readresource_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,19 @@ func TestServerReadResource(t *testing.T) {
9090
},
9191
}
9292

93+
testMultiAttrIdentitySchema := identityschema.Schema{
94+
Attributes: map[string]identityschema.Attribute{
95+
"test_attr_a": identityschema.StringAttribute{
96+
RequiredForImport: true,
97+
},
98+
"test_attr_b": identityschema.Int64Attribute{
99+
OptionalForImport: true,
100+
},
101+
},
102+
}
103+
104+
testMultiAttrIdentityType := testMultiAttrIdentitySchema.Type().TerraformType(context.Background())
105+
93106
testSchemaWriteOnly := schema.Schema{
94107
Attributes: map[string]schema.Attribute{
95108
"test_write_only": schema.StringAttribute{
@@ -663,6 +676,48 @@ func TestServerReadResource(t *testing.T) {
663676
Private: testEmptyPrivate,
664677
},
665678
},
679+
"response-identity-valid-update-empty-currentidentity": {
680+
server: &fwserver.Server{
681+
Provider: &testprovider.Provider{},
682+
},
683+
request: &fwserver.ReadResourceRequest{
684+
CurrentState: testCurrentState,
685+
CurrentIdentity: &tfsdk.ResourceIdentity{
686+
Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{
687+
"test_attr_a": tftypes.NewValue(tftypes.String, nil),
688+
"test_attr_b": tftypes.NewValue(tftypes.Number, nil),
689+
}),
690+
Schema: testMultiAttrIdentitySchema,
691+
},
692+
IdentitySchema: testMultiAttrIdentitySchema,
693+
Resource: &testprovider.ResourceWithIdentity{
694+
Resource: &testprovider.Resource{
695+
ReadMethod: func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
696+
identityData := struct {
697+
TestAttrA types.String `tfsdk:"test_attr_a"`
698+
TestAttrB types.Int64 `tfsdk:"test_attr_b"`
699+
}{
700+
TestAttrA: types.StringValue("new value"),
701+
TestAttrB: types.Int64Value(20),
702+
}
703+
704+
resp.Diagnostics.Append(resp.Identity.Set(ctx, &identityData)...)
705+
},
706+
},
707+
},
708+
},
709+
expectedResponse: &fwserver.ReadResourceResponse{
710+
NewState: testCurrentState,
711+
NewIdentity: &tfsdk.ResourceIdentity{
712+
Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{
713+
"test_attr_a": tftypes.NewValue(tftypes.String, "new value"),
714+
"test_attr_b": tftypes.NewValue(tftypes.Number, 20),
715+
}),
716+
Schema: testMultiAttrIdentitySchema,
717+
},
718+
Private: testEmptyPrivate,
719+
},
720+
},
666721
"response-identity-valid-update-after-import": {
667722
server: &fwserver.Server{
668723
Provider: &testprovider.Provider{},

‎internal/fwserver/server_updateresource.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func (s *Server) UpdateResource(ctx context.Context, req *UpdateResourceRequest,
186186
}
187187

188188
// If we already have an identity stored, validate that the new identity hasn't changing
189-
if !req.ResourceBehavior.MutableIdentity && !req.PlannedIdentity.Raw.IsNull() && !req.PlannedIdentity.Raw.Equal(resp.NewIdentity.Raw) {
189+
if !req.ResourceBehavior.MutableIdentity && !req.PlannedIdentity.Raw.IsFullyNull() && !req.PlannedIdentity.Raw.Equal(resp.NewIdentity.Raw) {
190190
resp.Diagnostics.AddError(
191191
"Unexpected Identity Change",
192192
"During the update operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+

0 commit comments

Comments
 (0)