Skip to content

Commit da315eb

Browse files
pieterndenik
authored andcommitted
direct: pass last-persisted state to DoDelete (#5279)
## Summary Extends `IResource.DoDelete` to take the resource's last-persisted state alongside `id`. The framework loads it from `dstate` in `DeploymentUnit.Delete` and `DeploymentUnit.Recreate`, and `Adapter.DoDelete` forwards it to the resource implementation. No behavioral change in this PR: every existing resource ignores the new argument (`_ *FooState`). Motivation: some delete APIs need values that come from local config (e.g. `postgres_project.purge_on_delete`, which is set on the user's resource and forwarded to the Lakebase DeleteProject RPC). Today there is nowhere to plumb those values because `DoDelete` only sees the resource id. This change keeps the call symmetric with `DoCreate`/`DoUpdate`, which already receive state. Adapter-level validation now type-checks `DoDelete state` the same way it already does for `DoCreate newState`. ## Test plan - [x] Unit + acceptance suites pass (`go test ./bundle/...`, `go test ./acceptance/...`); no fixture updates needed since behavior is unchanged. This pull request and its description were written by Isaac.
1 parent edc9c92 commit da315eb

32 files changed

Lines changed: 68 additions & 41 deletions

bundle/direct/apply.go

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,16 @@ func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState,
8080
}
8181

8282
func (d *DeploymentUnit) Recreate(ctx context.Context, db *dstate.DeploymentState, oldID string, newState any) error {
83+
oldState, err := d.loadPersistedState(db)
84+
if err != nil {
85+
return err
86+
}
87+
8388
// Note, unlike Delete(), we hard error on 403 here intentionally.
8489
// MANAGED_BY_PARENT is still disregarded — the subsequent Create with
8590
// replace_existing=true will reconfigure the parent-managed resource in
8691
// place, matching the Terraform provider's recreate behaviour.
87-
err := d.Adapter.DoDelete(ctx, oldID)
92+
err = d.Adapter.DoDelete(ctx, oldID, oldState)
8893
if err != nil && !isResourceGone(err) && !isManagedByParent(err) {
8994
return fmt.Errorf("deleting old id=%s: %w", oldID, err)
9095
}
@@ -170,7 +175,12 @@ func (d *DeploymentUnit) UpdateWithID(ctx context.Context, db *dstate.Deployment
170175
}
171176

172177
func (d *DeploymentUnit) Delete(ctx context.Context, db *dstate.DeploymentState, oldID string) error {
173-
err := d.Adapter.DoDelete(ctx, oldID)
178+
oldState, err := d.loadPersistedState(db)
179+
if err != nil {
180+
return err
181+
}
182+
183+
err = d.Adapter.DoDelete(ctx, oldID, oldState)
174184
if err != nil && !isResourceGone(err) && !isManagedByParent(err) {
175185
// Rather than failing delete and requiring user to unbind, we perform unbind automatically there.
176186
// Some services, e.g. jobs, return 403 for missing resources if caller did not have permissions to it when job existed.
@@ -215,6 +225,23 @@ func parseState(destType reflect.Type, raw json.RawMessage) (any, error) {
215225
return reflect.ValueOf(destPtr).Elem().Interface(), nil
216226
}
217227

228+
// loadPersistedState reads and parses the resource's last-persisted state for
229+
// the DoDelete call. Returns a zero-value state pointer when nothing has been
230+
// persisted yet (e.g. delete after a partial-create failure), so the call site
231+
// always passes a typed value.
232+
func (d *DeploymentUnit) loadPersistedState(db *dstate.DeploymentState) (any, error) {
233+
stateType := d.Adapter.StateType()
234+
dbentry, ok := db.GetResourceEntry(d.ResourceKey)
235+
if !ok || len(dbentry.State) == 0 {
236+
return reflect.New(stateType.Elem()).Interface(), nil
237+
}
238+
state, err := parseState(stateType, dbentry.State)
239+
if err != nil {
240+
return nil, fmt.Errorf("parsing persisted state: %w", err)
241+
}
242+
return state, nil
243+
}
244+
218245
func (d *DeploymentUnit) refreshRemoteState(ctx context.Context, id string) error {
219246
if d.RemoteState != nil {
220247
return nil

bundle/direct/dresources/adapter.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,11 @@ type IResource interface {
4242
// Example: func (r *ResourceJob) DoRead(ctx context.Context, id string) (*jobs.Job, error)
4343
DoRead(ctx context.Context, id string) (remoteState any, e error)
4444

45-
// DoDelete deletes the resource.
46-
// Example: func (r *ResourceJob) DoDelete(ctx context.Context, id string) error
47-
DoDelete(ctx context.Context, id string) error
45+
// DoDelete deletes the resource. The state argument is the last-persisted
46+
// state for the resource; resources that don't need it should accept it as
47+
// _ to satisfy the interface.
48+
// Example: func (r *ResourceJob) DoDelete(ctx context.Context, id string, _ *jobs.JobSettings) error
49+
DoDelete(ctx context.Context, id string, state any) error
4850

4951
// [Optional] OverrideChangeDesc can implement custom logic to update a given ChangeDesc; it is run last after built-in classifiers and field triggers.
5052
OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, changedesc *ChangeDesc, remoteState any) error
@@ -264,6 +266,7 @@ func (a *Adapter) validate() error {
264266
validations := []any{
265267
"PrepareState return", a.prepareState.OutTypes[0], stateType,
266268
"DoCreate newState", a.doCreate.InTypes[1], stateType,
269+
"DoDelete state", a.doDelete.InTypes[2], stateType,
267270
}
268271

269272
// If RemapState is implemented, validate its signature.
@@ -399,12 +402,9 @@ func (a *Adapter) DoRead(ctx context.Context, id string) (any, error) {
399402
return outs[0], nil
400403
}
401404

402-
func (a *Adapter) DoDelete(ctx context.Context, id string) error {
403-
_, err := a.doDelete.Call(ctx, id)
404-
if err != nil {
405-
return err
406-
}
407-
return nil
405+
func (a *Adapter) DoDelete(ctx context.Context, id string, state any) error {
406+
_, err := a.doDelete.Call(ctx, id, state)
407+
return err
408408
}
409409

410410
// normalizeNilPointer converts a nil pointer wrapped in an interface to a nil interface.

bundle/direct/dresources/alert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func (r *ResourceAlert) DoUpdate(ctx context.Context, id string, config *sql.Ale
5959
}
6060

6161
// DoDelete deletes the alert by id.
62-
func (r *ResourceAlert) DoDelete(ctx context.Context, id string) error {
62+
func (r *ResourceAlert) DoDelete(ctx context.Context, id string, _ *sql.AlertV2) error {
6363
return r.client.AlertsV2.TrashAlert(ctx, sql.TrashAlertV2Request{
6464
Id: id,
6565
Purge: true,

bundle/direct/dresources/all_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
860860
assert.Equal(t, val, remoteValue, "path=%q\nnewState=%s\nremappedState=%s", path.String(), jsonDump(newState), jsonDump(remappedState))
861861
}))
862862

863-
err = adapter.DoDelete(ctx, createdID)
863+
err = adapter.DoDelete(ctx, createdID, newState)
864864
require.NoError(t, err)
865865

866866
p, err := structpath.ParsePath("name")

bundle/direct/dresources/app.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ func deploymentToAppConfig(d *apps.AppDeployment) *resources.AppConfig {
316316
return config
317317
}
318318

319-
func (r *ResourceApp) DoDelete(ctx context.Context, id string) error {
319+
func (r *ResourceApp) DoDelete(ctx context.Context, id string, _ *AppState) error {
320320
_, err := r.client.Apps.DeleteByName(ctx, id)
321321
return err
322322
}

bundle/direct/dresources/catalog.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (r *ResourceCatalog) DoUpdateWithID(ctx context.Context, id string, config
104104
return newID, response, nil
105105
}
106106

107-
func (r *ResourceCatalog) DoDelete(ctx context.Context, id string) error {
107+
func (r *ResourceCatalog) DoDelete(ctx context.Context, id string, _ *catalog.CreateCatalog) error {
108108
return r.client.Catalogs.Delete(ctx, catalog.DeleteCatalogRequest{
109109
Name: id,
110110
Force: true,

bundle/direct/dresources/cluster.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (r *ResourceCluster) DoResize(ctx context.Context, id string, config *compu
117117
return err
118118
}
119119

120-
func (r *ResourceCluster) DoDelete(ctx context.Context, id string) error {
120+
func (r *ResourceCluster) DoDelete(ctx context.Context, id string, _ *compute.ClusterSpec) error {
121121
return r.client.Clusters.PermanentDeleteByClusterId(ctx, id)
122122
}
123123

bundle/direct/dresources/dashboard.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ func (r *ResourceDashboard) DoUpdate(ctx context.Context, id string, config *Das
357357
return responseToState(updateResp, publishResp, dashboard.SerializedDashboard, config.Published), nil
358358
}
359359

360-
func (r *ResourceDashboard) DoDelete(ctx context.Context, id string) error {
360+
func (r *ResourceDashboard) DoDelete(ctx context.Context, id string, _ *DashboardState) error {
361361
return r.client.Lakeview.Trash(ctx, dashboards.TrashDashboardRequest{
362362
DashboardId: id,
363363
})

bundle/direct/dresources/database_catalog.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (r *ResourceDatabaseCatalog) DoUpdate(ctx context.Context, id string, confi
4545
return nil, err
4646
}
4747

48-
func (r *ResourceDatabaseCatalog) DoDelete(ctx context.Context, id string) error {
48+
func (r *ResourceDatabaseCatalog) DoDelete(ctx context.Context, id string, _ *database.DatabaseCatalog) error {
4949
return r.client.Database.DeleteDatabaseCatalog(ctx, database.DeleteDatabaseCatalogRequest{
5050
Name: id,
5151
})

bundle/direct/dresources/database_instance.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (d *ResourceDatabaseInstance) WaitAfterCreate(ctx context.Context, id strin
6060
return nil, err
6161
}
6262

63-
func (d *ResourceDatabaseInstance) DoDelete(ctx context.Context, name string) error {
63+
func (d *ResourceDatabaseInstance) DoDelete(ctx context.Context, name string, _ *database.DatabaseInstance) error {
6464
return d.client.Database.DeleteDatabaseInstance(ctx, database.DeleteDatabaseInstanceRequest{
6565
Name: name,
6666
Purge: true,

0 commit comments

Comments
 (0)