Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ update genie_spaces.foo.permissions
Plan: 1 to add, 1 to change, 0 to delete, 0 unchanged

>>> [CLI] bundle destroy --auto-approve
The following resources will be deleted:

All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default

Destroy: 0 deleted
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

>>> errcode as-test-sp [CLI] bundle destroy --auto-approve
Warn: planning resources.jobs.foo.permissions: reading resources.jobs.foo.permissions id="/jobs/[NUMID]": [TEST_SP_APPLICATION_ID] does not have Manage permissions on Job with ID: ElasticJobId([NUMID]). Please contact the owner or an administrator for access.
Warn: planning resources.jobs.foo: reading resources.jobs.foo id="[NUMID]": User [TEST_SP_APPLICATION_ID] does not have View or Admin or Manage Run or Owner permissions on job [NUMID]
The following resources will be deleted:
delete resources.jobs.foo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,7 @@
}
],
"action": "delete",
"remote_state": {
"object_id": "/jobs/[JOB_WITH_PERMISSIONS_ID]",
"__embed__": [
{
"level": "CAN_MANAGE",
"user_name": "viewer@example.com"
},
{
"level": "IS_OWNER",
"user_name": "[USERNAME]"
}
]
}
"state_only": true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"action": "skip"
},
"resources.jobs.job_with_permissions.permissions": {
"action": "delete"
"action": "delete",
"state_only": true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,7 @@
}
],
"action": "delete",
"remote_state": {
"object_id": "/jobs/[JOB_WITH_PERMISSIONS_ID]",
"__embed__": [
{
"level": "CAN_VIEW",
"user_name": "viewer@example.com"
},
{
"level": "CAN_MANAGE",
"group_name": "data-team"
},
{
"level": "IS_OWNER",
"user_name": "[USERNAME]"
}
]
}
"state_only": true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"action": "skip"
},
"resources.jobs.job_with_permissions.permissions": {
"action": "delete"
"action": "delete",
"state_only": true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,8 @@ Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/jobs-permissions-test/default/files...
Deleted jobs.job_with_permissions.permissions
Files: 5 uploaded, 0 deleted
Resources: 0 created, 0 changed, 1 deleted, 1 unchanged
Resources: 0 created, 0 changed, 0 deleted, 1 unchanged

>>> print_requests.py --nostamp //jobs/

Expand Down Expand Up @@ -149,9 +148,8 @@ Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/jobs-permissions-test/default/files...
Deleted jobs.job_with_permissions.permissions
Files: 5 uploaded, 0 deleted
Resources: 0 created, 0 changed, 1 deleted, 1 unchanged
Resources: 0 created, 0 changed, 0 deleted, 1 unchanged

>>> print_requests.py --nostamp //jobs/

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,7 @@
}
],
"action": "delete",
"remote_state": {
"object_id": "/pipelines/[FOO_ID]",
"__embed__": [
{
"level": "CAN_MANAGE",
"user_name": "viewer@example.com"
},
{
"level": "IS_OWNER",
"user_name": "[USERNAME]"
}
]
}
"state_only": true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"action": "skip"
},
"resources.pipelines.foo.permissions": {
"action": "delete"
"action": "delete",
"state_only": true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,8 @@ resources:

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/permissions-test/default/files...
Deleted pipelines.foo.permissions
Files: 5 uploaded, 0 deleted
Resources: 0 created, 0 changed, 1 deleted, 1 unchanged
Resources: 0 created, 0 changed, 0 deleted, 1 unchanged

>>> print_requests.py --nostamp //pipeline ^//api/2.0/bundle --sort

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

>>> [CLI] bundle plan
delete secret_scopes.second
delete secret_scopes.second.permissions

Plan: 0 to add, 0 to change, 2 to delete, 2 unchanged
Plan: 0 to add, 0 to change, 1 to delete, 2 unchanged
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

>>> [CLI] bundle plan
delete secret_scopes.second
delete secret_scopes.second.permissions

Plan: 0 to add, 0 to change, 2 to delete, 1 unchanged
Plan: 0 to add, 0 to change, 1 to delete, 1 unchanged
27 changes: 26 additions & 1 deletion bundle/deploy/terraform/showplanfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,37 @@ func populatePlan(ctx context.Context, plan *deployplan.Plan, changes []*tfjson.
} else {
existing.Action = deployplan.GetHigherAction(existing.Action, actionType)
}
existing.StateOnly = isStateOnlyDelete(group, existing.Action)
} else {
plan.Plan[key] = &deployplan.PlanEntry{Action: actionType}
plan.Plan[key] = &deployplan.PlanEntry{
Action: actionType,
StateOnly: isStateOnlyDelete(group, actionType),
}
}
}
}

// isStateOnlyDelete reports whether deleting a resource of this group has no
// backend effect, so a planned Delete is a state-only cleanup. The direct engine
// derives this from the resource omitting DoDelete (see dresources.Adapter.HasDoDelete
// and PlanEntry.StateOnly); the terraform engine has no adapters here, so the set is
// mirrored by group. TestStateOnlyGroupsMatchDirect guards the two against drift.
//
// Note this only suppresses these deletes from plan/deploy output and counts to keep
// the two engines consistent — it does not change what terraform applies (terraform
// still revokes grants / resets permissions on delete).
func isStateOnlyDelete(group string, action deployplan.ActionType) bool {
if action != deployplan.Delete {
return false
}
switch group {
case "permissions", "grants", "secret_acls":
return true
default:
return false
}
}

// ShowPlanFile reads a Terraform plan file located at planPath using the provided tfexec.Terraform handle
// and converts it into a deployplan.Plan.
func ShowPlanFile(ctx context.Context, tf *tfexec.Terraform, planPath string) (*deployplan.Plan, error) {
Expand Down
3 changes: 3 additions & 0 deletions bundle/deployplan/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ type Action struct {
// Gone mirrors PlanEntry.Gone: the delete is a state-only cleanup because the
// resource no longer exists remotely.
Gone bool
// StateOnly mirrors PlanEntry.StateOnly: the delete is a state-only cleanup
// because the resource implements no DoDelete (deleting it has no backend effect).
StateOnly bool
}

func (a Action) String() string {
Expand Down
16 changes: 15 additions & 1 deletion bundle/deployplan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ func (p *Plan) CountActions() ActionCounts {
case Update, UpdateWithID, Resize:
c.Change++
case Delete:
// A state-only delete touches nothing in the backend and only drops the
// state entry, so it is not a real action: leave it out of the tally
// entirely rather than misreport it as deleted or unchanged.
if entry.StateOnly {
continue
}
c.Delete++
case Recreate:
// A recreate counts as both a delete and a create.
Expand Down Expand Up @@ -122,7 +128,14 @@ type PlanEntry struct {
// Gone is set on Delete entries when planning confirmed the resource no longer
// exists remotely. Applying such an entry only removes it from the state, without
// calling the delete API, and approval prompts do not list it as a deletion.
Gone bool `json:"gone,omitempty"`
Gone bool `json:"gone,omitempty"`
// StateOnly is set on Delete entries for resources that implement no DoDelete:
// deleting them has no backend effect. Like Gone, applying such an entry only
// removes it from the state and it is excluded from destructive-action prompts,
// textual plan output and the deleted count — but unlike Gone it is a property of
// the resource type, not of the current remote state, so planning skips the
// remote read that Gone detection needs.
StateOnly bool `json:"state_only,omitempty"`
NewState *structvar.StructVarJSON `json:"new_state,omitempty"`
RemoteState any `json:"remote_state,omitempty"`
Changes Changes `json:"changes,omitempty"`
Expand Down Expand Up @@ -207,6 +220,7 @@ func (p *Plan) GetActions() []Action {
ResourceKey: key,
ActionType: entry.Action,
Gone: entry.Gone,
StateOnly: entry.StateOnly,
})
}

Expand Down
7 changes: 4 additions & 3 deletions bundle/direct/bundle_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,10 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
}

if action == deployplan.Delete {
if entry.Gone {
// Planning confirmed the resource is already deleted remotely; only
// remove it from the state, without calling the delete API.
if entry.Gone || entry.StateOnly {
// Either planning confirmed the resource is already deleted remotely
// (Gone), or the resource has no delete operation (StateOnly). Both
// cases only remove it from the state, without calling the delete API.
err = b.StateDB.DeleteState(ctx, resourceKey, false)
} else {
err = d.Destroy(ctx, &b.StateDB)
Expand Down
10 changes: 10 additions & 0 deletions bundle/direct/bundle_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,16 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
return false
}

if !adapter.HasDoDelete() {
// Deleting this resource has no backend effect, so applying the
// Delete only drops the state entry. The remote read below exists
// solely to detect an already-deleted/gone resource and skip the
// delete call — pointless when there is no delete call — so skip it
// and mark the entry state-only.
entry.StateOnly = true
return true
}

remoteState, err := retryOnTransient(ctx, func() (any, error) {
return adapter.DoRead(ctx, id)
})
Expand Down
35 changes: 28 additions & 7 deletions bundle/direct/dresources/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,14 @@ type IResource interface {
// Example: func (r *ResourceJob) DoRead(ctx context.Context, id string) (*jobs.Job, error)
DoRead(ctx context.Context, id string) (remoteState any, e error)

// DoDelete deletes the resource. The state argument is the last-persisted
// [Optional] DoDelete deletes the resource. The state argument is the last-persisted
// state for the resource; resources that don't need it should accept it as
// _ to satisfy the interface.
// Omitting DoDelete declares that deleting the resource has no backend effect
// (e.g. grants, permissions): the engine then treats a planned Delete as a
// state-only cleanup (see PlanEntry.StateOnly) — it skips the remote read at
// plan time, issues no delete call at apply, and omits the resource from
// textual plan output and destructive-action prompts.
// Example: func (r *ResourceJob) DoDelete(ctx context.Context, id string, _ *jobs.JobSettings) error
DoDelete(ctx context.Context, id string, state any) error

Expand Down Expand Up @@ -122,10 +127,10 @@ type Adapter struct {
prepareState *calladapt.BoundCaller
remapState *calladapt.BoundCaller
doRefresh *calladapt.BoundCaller
doDelete *calladapt.BoundCaller
doCreate *calladapt.BoundCaller

// Optional:
doDelete *calladapt.BoundCaller
prepareInputConfig *calladapt.BoundCaller
isEmptyState *calladapt.BoundCaller
doUpdate *calladapt.BoundCaller
Expand Down Expand Up @@ -239,18 +244,20 @@ func (a *Adapter) initMethods(resource any) error {
return err
}

a.doDelete, err = prepareCallRequired(resource, "DoDelete")
a.doCreate, err = prepareCallRequired(resource, "DoCreate")
if err != nil {
return err
}

a.doCreate, err = prepareCallRequired(resource, "DoCreate")
// Optional methods with varying signatures:

// DoDelete is optional: a resource that omits it declares that deleting it has
// no backend effect, and the engine handles the Delete as a state-only cleanup.
a.doDelete, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "DoDelete")
if err != nil {
return err
}

// Optional methods with varying signatures:

a.prepareInputConfig, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "PrepareInputConfig")
if err != nil {
return err
Expand Down Expand Up @@ -349,7 +356,11 @@ func (a *Adapter) validate() error {
validations := []any{
"PrepareState return", a.prepareState.OutTypes[0], stateType,
"DoCreate newState", a.doCreate.InTypes[1], stateType,
"DoDelete state", a.doDelete.InTypes[2], stateType,
}

// DoDelete is optional; validate its state argument only when implemented.
if a.doDelete != nil {
validations = append(validations, "DoDelete state", a.doDelete.InTypes[2], stateType)
}

// If RemapState is implemented, validate its signature.
Expand Down Expand Up @@ -525,7 +536,17 @@ func (a *Adapter) DoRead(ctx context.Context, id string) (any, error) {
return outs[0], nil
}

// HasDoDelete reports whether the resource implements DoDelete. When false, the
// resource's delete has no backend effect and the engine treats a planned Delete
// as a state-only cleanup.
func (a *Adapter) HasDoDelete() bool {
return a.doDelete != nil
}

func (a *Adapter) DoDelete(ctx context.Context, id string, state any) error {
if a.doDelete == nil {
return nil // no-op: deleting this resource has no backend effect
}
_, err := a.doDelete.Call(ctx, id, state)
return err
}
Expand Down
1 change: 1 addition & 0 deletions bundle/direct/dresources/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W
// (it is intrinsic to the branch), so DoRead still succeeds afterwards.
deleteIsNoop := strings.HasSuffix(group, "permissions") || strings.HasSuffix(group, "grants") || group == "postgres_snapshot_schedules"
isImmutable := strings.HasSuffix(group, "internal_immutable_snapshots")

// Apps DoDelete is fire-and-forget: the API returns success while the app
// sits in DELETING state for up to ~20 minutes before the record is removed.
// A GET on the DELETING app returns the app, not 404 -- the testserver
Expand Down
9 changes: 4 additions & 5 deletions bundle/direct/dresources/grants.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,10 @@ func (r *ResourceGrants) DoUpdate(ctx context.Context, _ string, state *GrantsSt
return nil, err
}

func (r *ResourceGrants) DoDelete(ctx context.Context, id string, _ *GrantsState) error {
// Similar to permissions, we do nothing there.
// We could delete all grants there, but it would be confusing to explain wrt permissions.
return nil
}
// ResourceGrants intentionally implements no DoDelete: removing grants from the
// bundle does nothing to the backend. We could revoke all grants here, but it would
// be confusing to explain wrt permissions. Deleting the resource is a state-only
// cleanup (see PlanEntry.StateOnly).

func buildGrantChanges(desiredAssignments []catalog.PrivilegeAssignment, removedPrincipals []string) []catalog.PermissionsChange {
changes := make([]catalog.PermissionsChange, 0, len(desiredAssignments)+len(removedPrincipals))
Expand Down
Loading