Skip to content

resource: Add identity to DeleteRequest #1132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 2, 2025
Merged

resource: Add identity to DeleteRequest #1132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4020c19
resource: Add identity to DeleteRequest
austinvalle Apr 22, 2025
c3d709d
Merge branch 'main' into av/add-identity-to-delete
austinvalle Apr 24, 2025
780f076
Merge branch 'main' into av/add-identity-to-delete
austinvalle May 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions internal/fwserver/server_applyresourcechange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ func (s *Server) ApplyResourceChange(ctx context.Context, req *ApplyResourceChan
deleteReq := &DeleteResourceRequest{
PlannedPrivate: req.PlannedPrivate,
PriorState: req.PriorState,
// MAINTAINER NOTE: There isn't a separate data field for prior identity, like there is with prior_state and planned_state.
// Here the planned_identity field will contain what would be considered the prior identity (since the final identity value
// after deleting will be null).
PriorIdentity: req.PlannedIdentity,
ProviderMeta: req.ProviderMeta,
ResourceSchema: req.ResourceSchema,
IdentitySchema: req.IdentitySchema,
Expand Down
69 changes: 62 additions & 7 deletions internal/fwserver/server_applyresourcechange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ func TestServerApplyResourceChange(t *testing.T) {
Schema: testSchema,
}

testEmptyIdentity := &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, nil),
Schema: testIdentitySchema,
}

type testSchemaData struct {
TestComputed types.String `tfsdk:"test_computed"`
TestRequired types.String `tfsdk:"test_required"`
Expand Down Expand Up @@ -689,6 +694,52 @@ func TestServerApplyResourceChange(t *testing.T) {
NewState: testEmptyState,
},
},
"delete-request-prioridentity": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.ApplyResourceChangeRequest{
PlannedState: testEmptyPlan,
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-priorstate-value"),
}),
Schema: testSchema,
},
PlannedIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
}),
Schema: testIdentitySchema,
},
IdentitySchema: testIdentitySchema,
ResourceSchema: testSchema,
Resource: &testprovider.ResourceWithIdentity{
Resource: &testprovider.Resource{
CreateMethod: func(_ context.Context, _ resource.CreateRequest, resp *resource.CreateResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Delete, Got: Create")
},
DeleteMethod: func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
var identityData testIdentitySchemaData

resp.Diagnostics.Append(req.Identity.Get(ctx, &identityData)...)

if identityData.TestID.ValueString() != "id-123" {
resp.Diagnostics.AddError("Unexpected req.Identity Value", "Got: "+identityData.TestID.ValueString())
}
},
UpdateMethod: func(_ context.Context, _ resource.UpdateRequest, resp *resource.UpdateResponse) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Delete, Got: Update")
},
},
},
},
expectedResponse: &fwserver.ApplyResourceChangeResponse{
NewIdentity: testEmptyIdentity,
NewState: testEmptyState,
},
},
"delete-request-providermeta": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
Expand Down Expand Up @@ -982,6 +1033,12 @@ func TestServerApplyResourceChange(t *testing.T) {
}),
Schema: testSchema,
},
PlannedIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
}),
Schema: testIdentitySchema,
},
IdentitySchema: testIdentitySchema,
ResourceSchema: testSchema,
Resource: &testprovider.ResourceWithIdentity{
Expand All @@ -990,10 +1047,11 @@ func TestServerApplyResourceChange(t *testing.T) {
resp.Diagnostics.AddError("Unexpected Method Call", "Expected: Delete, Got: Create")
},
DeleteMethod: func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
if resp.Identity == nil || !resp.Identity.Raw.IsNull() {
// The identity is automatically set to null in the response after the Delete method is called
if resp.Identity == nil || resp.Identity.Raw.IsNull() {
resp.Diagnostics.AddError(
"Unexpected resp.Identity",
"expected resp.Identity to be a null object of the schema type.",
"expected resp.Identity to be a known non-null object of the schema type.",
)
}
},
Expand All @@ -1004,11 +1062,8 @@ func TestServerApplyResourceChange(t *testing.T) {
},
},
expectedResponse: &fwserver.ApplyResourceChangeResponse{
NewIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, nil),
Schema: testIdentitySchema,
},
NewState: testEmptyState,
NewIdentity: testEmptyIdentity,
NewState: testEmptyState,
},
},
"update-request-config": {
Expand Down
17 changes: 15 additions & 2 deletions internal/fwserver/server_deleteresource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
type DeleteResourceRequest struct {
PlannedPrivate *privatestate.Data
PriorState *tfsdk.State
PriorIdentity *tfsdk.ResourceIdentity
ProviderMeta *tfsdk.Config
ResourceSchema fwschema.Schema
IdentitySchema fwschema.Schema
Expand Down Expand Up @@ -98,15 +99,27 @@ func (s *Server) DeleteResource(ctx context.Context, req *DeleteResourceRequest,
resp.Private = req.PlannedPrivate
}

if req.IdentitySchema != nil {
if req.PriorIdentity == nil && req.IdentitySchema != nil {
nullIdentityTfValue := tftypes.NewValue(req.IdentitySchema.Type().TerraformType(ctx), nil)

deleteResp.Identity = &tfsdk.ResourceIdentity{
req.PriorIdentity = &tfsdk.ResourceIdentity{
Schema: req.IdentitySchema,
Raw: nullIdentityTfValue.Copy(),
}
}

if req.PriorIdentity != nil {
deleteReq.Identity = &tfsdk.ResourceIdentity{
Schema: req.PriorIdentity.Schema,
Raw: req.PriorIdentity.Raw.Copy(),
}

deleteResp.Identity = &tfsdk.ResourceIdentity{
Schema: req.PriorIdentity.Schema,
Raw: req.PriorIdentity.Raw.Copy(),
}
}

logging.FrameworkTrace(ctx, "Calling provider defined Resource Delete")
req.Resource.Delete(ctx, deleteReq, &deleteResp)
logging.FrameworkTrace(ctx, "Called provider defined Resource Delete")
Expand Down
75 changes: 63 additions & 12 deletions internal/fwserver/server_deleteresource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ func TestServerDeleteResource(t *testing.T) {
Schema: testSchema,
}

testEmptyIdentity := &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, nil),
Schema: testIdentitySchema,
}

type testSchemaData struct {
TestComputed types.String `tfsdk:"test_computed"`
TestRequired types.String `tfsdk:"test_required"`
Expand Down Expand Up @@ -163,6 +168,45 @@ func TestServerDeleteResource(t *testing.T) {
NewState: testEmptyState,
},
},
"request-prioridentity": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
},
request: &fwserver.DeleteResourceRequest{
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-priorstate-value"),
}),
Schema: testSchema,
},
PriorIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
}),
Schema: testIdentitySchema,
},
IdentitySchema: testIdentitySchema,
ResourceSchema: testSchema,
Resource: &testprovider.ResourceWithIdentity{
Resource: &testprovider.Resource{
DeleteMethod: func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
var identityData testIdentitySchemaData

resp.Diagnostics.Append(req.Identity.Get(ctx, &identityData)...)

if identityData.TestID.ValueString() != "id-123" {
resp.Diagnostics.AddError("Unexpected req.Identity Value", "Got: "+identityData.TestID.ValueString())
}
},
},
},
},
expectedResponse: &fwserver.DeleteResourceResponse{
NewState: testEmptyState,
NewIdentity: testEmptyIdentity,
},
},
"request-providermeta": {
server: &fwserver.Server{
Provider: &testprovider.Provider{},
Expand Down Expand Up @@ -365,25 +409,29 @@ func TestServerDeleteResource(t *testing.T) {
}),
Schema: testSchema,
},
PriorIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
}),
Schema: testIdentitySchema,
},
IdentitySchema: testIdentitySchema,
ResourceSchema: testSchema,
Resource: &testprovider.Resource{
DeleteMethod: func(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
if resp.Identity == nil || !resp.Identity.Raw.IsNull() {
// The identity is automatically set to null in the response after the Delete method is called
if resp.Identity == nil || resp.Identity.Raw.IsNull() {
resp.Diagnostics.AddError(
"Unexpected resp.Identity",
"expected resp.Identity to be a null object of the schema type.",
"expected resp.Identity to be a known non-null object of the schema type.",
)
}
},
},
},
expectedResponse: &fwserver.DeleteResourceResponse{
NewIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, nil),
Schema: testIdentitySchema,
},
NewState: testEmptyState,
NewIdentity: testEmptyIdentity,
NewState: testEmptyState,
},
},
"response-newidentity-set-to-null": {
Expand All @@ -398,6 +446,12 @@ func TestServerDeleteResource(t *testing.T) {
}),
Schema: testSchema,
},
PriorIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, map[string]tftypes.Value{
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
}),
Schema: testIdentitySchema,
},
IdentitySchema: testIdentitySchema,
ResourceSchema: testSchema,
Resource: &testprovider.Resource{
Expand All @@ -410,11 +464,8 @@ func TestServerDeleteResource(t *testing.T) {
},
},
expectedResponse: &fwserver.DeleteResourceResponse{
NewIdentity: &tfsdk.ResourceIdentity{
Raw: tftypes.NewValue(testIdentityType, nil),
Schema: testIdentitySchema,
},
NewState: testEmptyState,
NewIdentity: testEmptyIdentity,
NewState: testEmptyState,
},
},
"response-invalid-newidentity": {
Expand Down
4 changes: 4 additions & 0 deletions resource/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ type DeleteRequest struct {
// operation.
State tfsdk.State

// Identity is the current identity of the resource prior to the Delete
// operation. If the resource does not support identity, this value will not be set.
Identity *tfsdk.ResourceIdentity

// ProviderMeta is metadata from the provider_meta block of the module.
ProviderMeta tfsdk.Config

Expand Down