diff --git a/.changes/unreleased/BUG FIXES-20250922-175218.yaml b/.changes/unreleased/BUG FIXES-20250922-175218.yaml new file mode 100644 index 000000000..dbe5a3599 --- /dev/null +++ b/.changes/unreleased/BUG FIXES-20250922-175218.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'all: Prevent identity change validation from raising an error when prior identity is empty (all attributes are null)' +time: 2025-09-22T17:52:18.613034-04:00 +custom: + Issue: "1229" diff --git a/internal/fwserver/server_applyresourcechange_test.go b/internal/fwserver/server_applyresourcechange_test.go index 8ac63ca25..945c7e125 100644 --- a/internal/fwserver/server_applyresourcechange_test.go +++ b/internal/fwserver/server_applyresourcechange_test.go @@ -59,6 +59,24 @@ func TestServerApplyResourceChange(t *testing.T) { }, } + type testMultiIdentitySchemaData struct { + TestAttrA types.String `tfsdk:"test_attr_a"` + TestAttrB types.Int64 `tfsdk:"test_attr_b"` + } + + testMultiAttrIdentitySchema := identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_attr_a": identityschema.StringAttribute{ + RequiredForImport: true, + }, + "test_attr_b": identityschema.Int64Attribute{ + OptionalForImport: true, + }, + }, + } + + testMultiAttrIdentityType := testMultiAttrIdentitySchema.Type().TerraformType(context.Background()) + testEmptyPlan := &tfsdk.Plan{ Raw: tftypes.NewValue(testSchemaType, nil), Schema: testSchema, @@ -1719,6 +1737,76 @@ func TestServerApplyResourceChange(t *testing.T) { Private: testEmptyPrivate, }, }, + "update-response-newidentity-empty-plannedidentity": { + server: &fwserver.Server{ + Provider: &testprovider.Provider{}, + }, + request: &fwserver.ApplyResourceChangeRequest{ + Config: &tfsdk.Config{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, nil), + "test_required": tftypes.NewValue(tftypes.String, "test-new-value"), + }), + Schema: testSchema, + }, + PlannedState: &tfsdk.Plan{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"), + "test_required": tftypes.NewValue(tftypes.String, "test-new-value"), + }), + Schema: testSchema, + }, + PriorState: &tfsdk.State{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, nil), + "test_required": tftypes.NewValue(tftypes.String, "test-old-value"), + }), + Schema: testSchema, + }, + PlannedIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{ + "test_attr_a": tftypes.NewValue(tftypes.String, nil), + "test_attr_b": tftypes.NewValue(tftypes.Number, nil), + }), + Schema: testMultiAttrIdentitySchema, + }, + IdentitySchema: testMultiAttrIdentitySchema, + ResourceSchema: testSchema, + Resource: &testprovider.ResourceWithIdentity{ + Resource: &testprovider.Resource{ + CreateMethod: func(_ context.Context, _ resource.CreateRequest, resp *resource.CreateResponse) { + resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Create") + }, + DeleteMethod: func(_ context.Context, _ resource.DeleteRequest, resp *resource.DeleteResponse) { + resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Update, Got: Delete") + }, + UpdateMethod: func(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { + resp.Diagnostics.Append(resp.Identity.Set(ctx, testMultiIdentitySchemaData{ + TestAttrA: types.StringValue("new value"), + TestAttrB: types.Int64Value(20), + })...) + }, + }, + }, + }, + expectedResponse: &fwserver.ApplyResourceChangeResponse{ + NewIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{ + "test_attr_a": tftypes.NewValue(tftypes.String, "new value"), + "test_attr_b": tftypes.NewValue(tftypes.Number, 20), + }), + Schema: testMultiAttrIdentitySchema, + }, + NewState: &tfsdk.State{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, nil), + "test_required": tftypes.NewValue(tftypes.String, "test-old-value"), + }), + Schema: testSchema, + }, + Private: testEmptyPrivate, + }, + }, "update-response-newidentity-with-plannedidentity": { server: &fwserver.Server{ Provider: &testprovider.Provider{}, diff --git a/internal/fwserver/server_planresourcechange.go b/internal/fwserver/server_planresourcechange.go index 772c8c9bc..7aaf4ea3c 100644 --- a/internal/fwserver/server_planresourcechange.go +++ b/internal/fwserver/server_planresourcechange.go @@ -383,7 +383,7 @@ func (s *Server) PlanResourceChange(ctx context.Context, req *PlanResourceChange } // If we're updating or deleting and we already have an identity stored, validate that the planned identity isn't changing - if !req.ResourceBehavior.MutableIdentity && !req.PriorState.Raw.IsNull() && !req.PriorIdentity.Raw.IsNull() && !req.PriorIdentity.Raw.Equal(resp.PlannedIdentity.Raw) { + if !req.ResourceBehavior.MutableIdentity && !req.PriorState.Raw.IsNull() && !req.PriorIdentity.Raw.IsFullyNull() && !req.PriorIdentity.Raw.Equal(resp.PlannedIdentity.Raw) { resp.Diagnostics.AddError( "Unexpected Identity Change", "During the planning operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+ diff --git a/internal/fwserver/server_planresourcechange_test.go b/internal/fwserver/server_planresourcechange_test.go index bd757e02c..2dac98568 100644 --- a/internal/fwserver/server_planresourcechange_test.go +++ b/internal/fwserver/server_planresourcechange_test.go @@ -589,6 +589,24 @@ func TestServerPlanResourceChange(t *testing.T) { }, } + type testMultiIdentitySchemaData struct { + TestAttrA types.String `tfsdk:"test_attr_a"` + TestAttrB types.Int64 `tfsdk:"test_attr_b"` + } + + testMultiAttrIdentitySchema := identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_attr_a": identityschema.StringAttribute{ + RequiredForImport: true, + }, + "test_attr_b": identityschema.Int64Attribute{ + OptionalForImport: true, + }, + }, + } + + testMultiAttrIdentityType := testMultiAttrIdentitySchema.Type().TerraformType(context.Background()) + testSchemaWriteOnly := schema.Schema{ Attributes: map[string]schema.Attribute{ "test_computed": schema.StringAttribute{ @@ -6722,6 +6740,77 @@ func TestServerPlanResourceChange(t *testing.T) { PlannedPrivate: testEmptyPrivate, }, }, + "update-resourcewithmodifyplan-empty-prioridentity-plannedidentity-changed": { + server: &fwserver.Server{ + Provider: &testprovider.Provider{}, + }, + request: &fwserver.PlanResourceChangeRequest{ + Config: &tfsdk.Config{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, nil), + "test_required": tftypes.NewValue(tftypes.String, "test-new-value"), + }), + Schema: testSchema, + }, + ProposedNewState: &tfsdk.Plan{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, nil), + "test_required": tftypes.NewValue(tftypes.String, "test-new-value"), + }), + Schema: testSchema, + }, + PriorState: &tfsdk.State{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, nil), + "test_required": tftypes.NewValue(tftypes.String, "test-old-value"), + }), + Schema: testSchema, + }, + PriorIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{ + "test_attr_a": tftypes.NewValue(tftypes.String, nil), + "test_attr_b": tftypes.NewValue(tftypes.Number, nil), + }), + Schema: testMultiAttrIdentitySchema, + }, + IdentitySchema: testMultiAttrIdentitySchema, + ResourceSchema: testSchema, + Resource: &testprovider.ResourceWithIdentityAndModifyPlan{ + ModifyPlanMethod: func(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + var data testSchemaData + resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) + data.TestComputed = types.StringValue("test-plannedstate-value") + resp.Diagnostics.Append(resp.Plan.Set(ctx, &data)...) + + var identityData testMultiIdentitySchemaData + resp.Diagnostics.Append(req.Identity.Get(ctx, &identityData)...) + identityData.TestAttrA = types.StringValue("new value") + identityData.TestAttrB = types.Int64Value(20) + resp.Diagnostics.Append(resp.Identity.Set(ctx, &identityData)...) + }, + IdentitySchemaMethod: func(ctx context.Context, req resource.IdentitySchemaRequest, resp *resource.IdentitySchemaResponse) { + resp.IdentitySchema = testMultiAttrIdentitySchema + }, + }, + }, + expectedResponse: &fwserver.PlanResourceChangeResponse{ + PlannedIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{ + "test_attr_a": tftypes.NewValue(tftypes.String, "new value"), + "test_attr_b": tftypes.NewValue(tftypes.Number, 20), + }), + Schema: testMultiAttrIdentitySchema, + }, + PlannedState: &tfsdk.State{ + Raw: tftypes.NewValue(testSchemaType, map[string]tftypes.Value{ + "test_computed": tftypes.NewValue(tftypes.String, "test-plannedstate-value"), + "test_required": tftypes.NewValue(tftypes.String, "test-new-value"), + }), + Schema: testSchema, + }, + PlannedPrivate: testEmptyPrivate, + }, + }, "update-resourcewithmodifyplan-invalid-response-plannedidentity-changed": { server: &fwserver.Server{ Provider: &testprovider.Provider{}, diff --git a/internal/fwserver/server_readresource.go b/internal/fwserver/server_readresource.go index 504946cff..7f0a00423 100644 --- a/internal/fwserver/server_readresource.go +++ b/internal/fwserver/server_readresource.go @@ -185,7 +185,7 @@ func (s *Server) ReadResource(ctx context.Context, req *ReadResourceRequest, res } // If we're refreshing the resource state (excluding a recently imported resource), validate that the new identity isn't changing - if !req.ResourceBehavior.MutableIdentity && !readFollowingImport && !req.CurrentIdentity.Raw.IsNull() && !req.CurrentIdentity.Raw.Equal(resp.NewIdentity.Raw) { + if !req.ResourceBehavior.MutableIdentity && !readFollowingImport && !req.CurrentIdentity.Raw.IsFullyNull() && !req.CurrentIdentity.Raw.Equal(resp.NewIdentity.Raw) { resp.Diagnostics.AddError( "Unexpected Identity Change", "During the read operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+ diff --git a/internal/fwserver/server_readresource_test.go b/internal/fwserver/server_readresource_test.go index e639e23e0..b0b0ab127 100644 --- a/internal/fwserver/server_readresource_test.go +++ b/internal/fwserver/server_readresource_test.go @@ -90,6 +90,19 @@ func TestServerReadResource(t *testing.T) { }, } + testMultiAttrIdentitySchema := identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_attr_a": identityschema.StringAttribute{ + RequiredForImport: true, + }, + "test_attr_b": identityschema.Int64Attribute{ + OptionalForImport: true, + }, + }, + } + + testMultiAttrIdentityType := testMultiAttrIdentitySchema.Type().TerraformType(context.Background()) + testSchemaWriteOnly := schema.Schema{ Attributes: map[string]schema.Attribute{ "test_write_only": schema.StringAttribute{ @@ -663,6 +676,48 @@ func TestServerReadResource(t *testing.T) { Private: testEmptyPrivate, }, }, + "response-identity-valid-update-empty-currentidentity": { + server: &fwserver.Server{ + Provider: &testprovider.Provider{}, + }, + request: &fwserver.ReadResourceRequest{ + CurrentState: testCurrentState, + CurrentIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{ + "test_attr_a": tftypes.NewValue(tftypes.String, nil), + "test_attr_b": tftypes.NewValue(tftypes.Number, nil), + }), + Schema: testMultiAttrIdentitySchema, + }, + IdentitySchema: testMultiAttrIdentitySchema, + Resource: &testprovider.ResourceWithIdentity{ + Resource: &testprovider.Resource{ + ReadMethod: func(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { + identityData := struct { + TestAttrA types.String `tfsdk:"test_attr_a"` + TestAttrB types.Int64 `tfsdk:"test_attr_b"` + }{ + TestAttrA: types.StringValue("new value"), + TestAttrB: types.Int64Value(20), + } + + resp.Diagnostics.Append(resp.Identity.Set(ctx, &identityData)...) + }, + }, + }, + }, + expectedResponse: &fwserver.ReadResourceResponse{ + NewState: testCurrentState, + NewIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(testMultiAttrIdentityType, map[string]tftypes.Value{ + "test_attr_a": tftypes.NewValue(tftypes.String, "new value"), + "test_attr_b": tftypes.NewValue(tftypes.Number, 20), + }), + Schema: testMultiAttrIdentitySchema, + }, + Private: testEmptyPrivate, + }, + }, "response-identity-valid-update-after-import": { server: &fwserver.Server{ Provider: &testprovider.Provider{}, diff --git a/internal/fwserver/server_updateresource.go b/internal/fwserver/server_updateresource.go index 3ac99ccfd..0521aa80c 100644 --- a/internal/fwserver/server_updateresource.go +++ b/internal/fwserver/server_updateresource.go @@ -186,7 +186,7 @@ func (s *Server) UpdateResource(ctx context.Context, req *UpdateResourceRequest, } // If we already have an identity stored, validate that the new identity hasn't changing - if !req.ResourceBehavior.MutableIdentity && !req.PlannedIdentity.Raw.IsNull() && !req.PlannedIdentity.Raw.Equal(resp.NewIdentity.Raw) { + if !req.ResourceBehavior.MutableIdentity && !req.PlannedIdentity.Raw.IsFullyNull() && !req.PlannedIdentity.Raw.Equal(resp.NewIdentity.Raw) { resp.Diagnostics.AddError( "Unexpected Identity Change", "During the update operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.\n\n"+