diff --git a/.nextchanges/bundles/6647.md b/.nextchanges/bundles/6647.md new file mode 100644 index 00000000000..0ca983be6bf --- /dev/null +++ b/.nextchanges/bundles/6647.md @@ -0,0 +1 @@ +* `bundle plan`, `deploy`, and `destroy` no longer report removing `permissions`, `grants`, or secret scope ACLs from a bundle as a deletion, since it leaves the resource untouched. ([#6647](https://github.com/databricks/cli/pull/6647)) diff --git a/acceptance/bundle/resources/cluster_policies/permissions/out_of_band_deletion/output.txt b/acceptance/bundle/resources/cluster_policies/permissions/out_of_band_deletion/output.txt index ac59fe14f24..7ef02fb0a88 100644 --- a/acceptance/bundle/resources/cluster_policies/permissions/out_of_band_deletion/output.txt +++ b/acceptance/bundle/resources/cluster_policies/permissions/out_of_band_deletion/output.txt @@ -51,8 +51,6 @@ update cluster_policies.test_cluster_policy.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_cluster_policy_permissions_recreate/default Destroy: 0 deleted diff --git a/acceptance/bundle/resources/permissions/genie_spaces/out_of_band_deletion/output.txt b/acceptance/bundle/resources/permissions/genie_spaces/out_of_band_deletion/output.txt index 187e679c824..0ae11ef832e 100644 --- a/acceptance/bundle/resources/permissions/genie_spaces/out_of_band_deletion/output.txt +++ b/acceptance/bundle/resources/permissions/genie_spaces/out_of_band_deletion/output.txt @@ -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 diff --git a/acceptance/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/out.destroy.direct.txt b/acceptance/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/out.destroy.direct.txt index 4e47b717bce..45b95472392 100644 --- a/acceptance/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/out.destroy.direct.txt +++ b/acceptance/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/out.destroy.direct.txt @@ -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 diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json index 802b24d2128..b6cfa725c53 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.direct.json @@ -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 } } } diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.terraform.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.terraform.json index dea726555ec..4910a177337 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.terraform.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_delete_all.terraform.json @@ -5,7 +5,8 @@ "action": "skip" }, "resources.jobs.job_with_permissions.permissions": { - "action": "delete" + "action": "delete", + "state_only": true } } } diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json index d9bf197e871..b01df3c6905 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.direct.json @@ -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 } } } diff --git a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json index dea726555ec..4910a177337 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json +++ b/acceptance/bundle/resources/permissions/jobs/update/out.plan_set_empty.terraform.json @@ -5,7 +5,8 @@ "action": "skip" }, "resources.jobs.job_with_permissions.permissions": { - "action": "delete" + "action": "delete", + "state_only": true } } } diff --git a/acceptance/bundle/resources/permissions/jobs/update/output.txt b/acceptance/bundle/resources/permissions/jobs/update/output.txt index 415de04d6b0..010de6f81c3 100644 --- a/acceptance/bundle/resources/permissions/jobs/update/output.txt +++ b/acceptance/bundle/resources/permissions/jobs/update/output.txt @@ -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: 3 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/ @@ -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: 3 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/ diff --git a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.direct.json b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.direct.json index bd9b344f45e..a68f285b9b5 100644 --- a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.direct.json +++ b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.direct.json @@ -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 } } } diff --git a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.terraform.json b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.terraform.json index a1177b2b9f5..fec4ffef802 100644 --- a/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.terraform.json +++ b/acceptance/bundle/resources/permissions/pipelines/update/out.plan_delete_all.terraform.json @@ -5,7 +5,8 @@ "action": "skip" }, "resources.pipelines.foo.permissions": { - "action": "delete" + "action": "delete", + "state_only": true } } } diff --git a/acceptance/bundle/resources/permissions/pipelines/update/output.txt b/acceptance/bundle/resources/permissions/pipelines/update/output.txt index c0c65518a1d..3b8b33221fb 100644 --- a/acceptance/bundle/resources/permissions/pipelines/update/output.txt +++ b/acceptance/bundle/resources/permissions/pipelines/update/output.txt @@ -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: 3 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 diff --git a/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.direct.txt b/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.direct.txt index 6270ea89c0b..be33e1f2267 100644 --- a/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.direct.txt +++ b/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.direct.txt @@ -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 diff --git a/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt b/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt index 1307df4e1a6..989400461fa 100644 --- a/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt +++ b/acceptance/bundle/resources/secret_scopes/delete_scope/out.plan.terraform.txt @@ -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 diff --git a/bundle/deploy/terraform/showplanfile.go b/bundle/deploy/terraform/showplanfile.go index c0cc7a4e0c3..5df56ff57b9 100644 --- a/bundle/deploy/terraform/showplanfile.go +++ b/bundle/deploy/terraform/showplanfile.go @@ -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) { diff --git a/bundle/deployplan/action.go b/bundle/deployplan/action.go index e855dbb2197..5f607201c4d 100644 --- a/bundle/deployplan/action.go +++ b/bundle/deployplan/action.go @@ -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 { diff --git a/bundle/deployplan/plan.go b/bundle/deployplan/plan.go index f4ecfaeea14..906d08542ea 100644 --- a/bundle/deployplan/plan.go +++ b/bundle/deployplan/plan.go @@ -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. @@ -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"` @@ -212,6 +225,7 @@ func (p *Plan) GetActions() []Action { ResourceKey: key, ActionType: entry.Action, Gone: entry.Gone, + StateOnly: entry.StateOnly, }) } diff --git a/bundle/direct/bundle_apply.go b/bundle/direct/bundle_apply.go index db0a7ab4b3c..a88a321a2a3 100644 --- a/bundle/direct/bundle_apply.go +++ b/bundle/direct/bundle_apply.go @@ -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) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index d286e892a9d..baa5aa8d1eb 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -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) }) diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index ec9697dc3f9..6210a952112 100644 --- a/bundle/direct/dresources/adapter.go +++ b/bundle/direct/dresources/adapter.go @@ -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 @@ -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 @@ -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 @@ -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. @@ -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 } diff --git a/bundle/direct/dresources/grants.go b/bundle/direct/dresources/grants.go index 5fd10f41046..f9c31eeb75f 100644 --- a/bundle/direct/dresources/grants.go +++ b/bundle/direct/dresources/grants.go @@ -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)) diff --git a/bundle/direct/dresources/permissions.go b/bundle/direct/dresources/permissions.go index 2ab265d29f1..c2aebb4d0ac 100644 --- a/bundle/direct/dresources/permissions.go +++ b/bundle/direct/dresources/permissions.go @@ -264,14 +264,11 @@ func (r *ResourcePermissions) DoUpdate(ctx context.Context, _ string, newState * return nil, err } -// DoDelete is activated in 2 distinct cases: +// ResourcePermissions intentionally implements no DoDelete (deleting is a state-only +// cleanup, see PlanEntry.StateOnly). A delete would otherwise be activated in 2 cases: // 1) 'permissions' field is deleted in DABs config. In that case terraform would restore the default permissions (IS_OWNER for current user). // 2) the parent resource is deleted; in that case there is no need to do anything; parent resource deletion is enough. -// Let's do nothing in both cases. If user no longer wishes to manage permissions with DABs they can go ahead and manage +// We do nothing in both cases. If the user no longer wishes to manage permissions with DABs they can go ahead and manage // it themselves. Trying to fix permissions back requires // - making assumptions on what it should look like // - storing current user somewhere or storing original permissions somewhere -func (r *ResourcePermissions) DoDelete(ctx context.Context, id string, _ *PermissionsState) error { - // intentional noop - return nil -} diff --git a/bundle/direct/dresources/secret_scope_acls.go b/bundle/direct/dresources/secret_scope_acls.go index e9a2375eb06..3e61a9d7801 100644 --- a/bundle/direct/dresources/secret_scope_acls.go +++ b/bundle/direct/dresources/secret_scope_acls.go @@ -114,10 +114,9 @@ func (r *ResourceSecretScopeAcls) DoUpdate(ctx context.Context, id string, state return nil, err } -// Removing ACLs is a no-op, to match the behavior for permissions and grants. -func (r *ResourceSecretScopeAcls) DoDelete(ctx context.Context, id string, _ *SecretScopeAclsState) error { - return nil -} +// ResourceSecretScopeAcls intentionally implements no DoDelete: removing ACLs is a +// no-op, to match the behavior for permissions and grants. Deleting the resource is a +// state-only cleanup (see PlanEntry.StateOnly). // setACLs reconciles the desired ACLs with the current state func (r *ResourceSecretScopeAcls) setACLs(ctx context.Context, scopeName string, desiredAcls []workspace.AclItem) error { diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 659537a54f9..296e117bc13 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -49,9 +49,10 @@ var deployApprovalGroups = []approvalGroup{ func approvalForDeploy(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan) (bool, error) { actions := plan.GetActions() - // Deletes of resources that are already gone remotely only clean up the state, - // so they don't count as destructive actions and need no approval. - actions = slices.DeleteFunc(actions, func(a deployplan.Action) bool { return a.Gone }) + // Deletes that only clean up the state — the resource is already gone remotely + // (Gone) or has no delete operation (StateOnly) — are not destructive and need + // no approval. + actions = slices.DeleteFunc(actions, func(a deployplan.Action) bool { return a.Gone || a.StateOnly }) err := checkForPreventDestroy(b, actions) if err != nil { @@ -145,6 +146,10 @@ func logDeploySummary(ctx context.Context, b *bundle.Bundle, plan *deployplan.Pl if action.ActionType == deployplan.Skip || action.ActionType == deployplan.Undefined { continue } + // A state-only delete performs no backend operation, so don't report it. + if action.StateOnly { + continue + } verb := action.ActionType.StringShort() + "d" cmdio.LogString(ctx, strings.ToUpper(verb[:1])+verb[1:]+" "+strings.TrimPrefix(action.ResourceKey, "resources.")) } diff --git a/bundle/phases/destroy.go b/bundle/phases/destroy.go index 2ac8c8482d5..cc1aabfb622 100644 --- a/bundle/phases/destroy.go +++ b/bundle/phases/destroy.go @@ -88,9 +88,11 @@ func logPipelineDeleteApproval(ctx context.Context, b *bundle.Bundle, actions [] func approvalForDestroy(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, engine engine.EngineType) (bool, error) { deleteActions := plan.GetActions() - // Deletes of resources that are already gone remotely only clean up the state, - // so they don't count as destructive actions and are not listed as deletions. - deleteActions = slices.DeleteFunc(deleteActions, func(a deployplan.Action) bool { return a.Gone }) + // Deletes that only clean up the state — the resource is already gone remotely + // (Gone) or has no delete operation (StateOnly) — are not destructive, so they + // are not listed as deletions and need no approval. In particular this makes + // prevent_destroy inert for state-only resources: nothing is destroyed. + deleteActions = slices.DeleteFunc(deleteActions, func(a deployplan.Action) bool { return a.Gone || a.StateOnly }) err := checkForPreventDestroy(b, deleteActions) if err != nil { @@ -186,7 +188,7 @@ func destroyCore(ctx context.Context, b *bundle.Bundle, plan *deployplan.Plan, e // a destruction to report. deleted := 0 for _, a := range plan.GetActions() { - if a.ActionType == deployplan.Delete && !a.IsChildResource() && !a.Gone { + if a.ActionType == deployplan.Delete && !a.IsChildResource() && !a.Gone && !a.StateOnly { deleted++ } } diff --git a/cmd/bundle/plan.go b/cmd/bundle/plan.go index 52327f385bc..5c1d790173d 100644 --- a/cmd/bundle/plan.go +++ b/cmd/bundle/plan.go @@ -89,6 +89,11 @@ It is useful for previewing changes before running 'bundle deploy'.`, if action.ActionType == deployplan.Skip { continue } + // A state-only delete has no backend effect; keep it in the JSON + // plan but omit it from the human-readable action list. + if action.StateOnly { + continue + } key := strings.TrimPrefix(action.ResourceKey, "resources.") fmt.Fprintf(out, "%s %s\n", action.ActionType.StringShort(), key) }