From 30adca7d511547f32bba420016e1346388466b4d Mon Sep 17 00:00:00 2001 From: Anders Rex Date: Thu, 15 Oct 2026 12:45:55 +0300 Subject: [PATCH 1/2] Support selecting resources for deletion with bundle deploy --select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve --select selectors against the computed plan (Plan.FilterToSelected) instead of the config, so resources removed from config and planned for deletion can be selected like any other. The old ResolveSelect mutator is reduced to recording the select_used telemetry flag (renamed MarkSelectUsed). When a selected resource is being deleted, the plan's closure follows reverse dependency edges: its dependents are deleted too so none are left dangling, while its own dependencies are left untouched — mirroring bundle destroy. A deployed selection still follows forward edges to pull in its dependencies. Grants and permissions sub-nodes move with their parent in both directions. Deleting a resource whose dependent is still live (still in config, not itself being deleted) is an error before any API delete requests are sent. An invalid --select (a name in neither config nor state) is rejected before the predeploy script runs or any files are uploaded. When --plan is used alongside a --select that doesn't match the selectors the plan was built with, a warning is emitted. The plan serializes was_filtered and selectors so a pre-filtered plan file can be passed to bundle deploy --plan without re-specifying --select. Co-authored-by: Isaac --- .nextchanges/bundles/select-deletions.md | 1 + .../select/deletion/databricks.yml.after.tmpl | 7 + .../select/deletion/databricks.yml.tmpl | 10 + .../bundle/select/deletion/out.test.toml | 2 + acceptance/bundle/select/deletion/output.txt | 43 ++++ acceptance/bundle/select/deletion/script | 31 +++ acceptance/bundle/select/deletion/test.toml | 4 + .../databricks.yml.after.tmpl | 7 + .../databricks.yml.live_consumer.tmpl | 14 ++ .../deletion_dependents/databricks.yml.tmpl | 17 ++ .../select/deletion_dependents/out.test.toml | 2 + .../select/deletion_dependents/output.txt | 71 ++++++ .../bundle/select/deletion_dependents/script | 48 ++++ .../select/deletion_dependents/test.toml | 4 + .../databricks.yml.after.tmpl | 7 + .../deletion_saved_plan/databricks.yml.tmpl | 10 + .../select/deletion_saved_plan/out.test.toml | 2 + .../select/deletion_saved_plan/output.txt | 38 +++ .../bundle/select/deletion_saved_plan/script | 34 +++ .../select/deletion_saved_plan/test.toml | 5 + .../invalid_no_upload/databricks.yml.tmpl | 12 + .../select/invalid_no_upload/out.test.toml | 2 + .../select/invalid_no_upload/output.txt | 6 + .../bundle/select/invalid_no_upload/script | 13 ++ .../bundle/select/invalid_no_upload/test.toml | 4 + acceptance/bundle/select/missing/script | 4 +- .../redeploy_unqualified/databricks.yml.tmpl | 7 + .../select/redeploy_unqualified/out.test.toml | 2 + .../select/redeploy_unqualified/output.txt | 24 ++ .../bundle/select/redeploy_unqualified/script | 15 ++ .../select/redeploy_unqualified/test.toml | 1 + bundle/config/mutator/mark_select_used.go | 32 +++ .../config/mutator/mark_select_used_test.go | 40 ++++ bundle/config/mutator/resolve_select.go | 76 ------ bundle/config/mutator/resolve_select_test.go | 53 ----- bundle/deployplan/plan.go | 152 +++++++++++- bundle/deployplan/plan_filter_test.go | 218 +++++++++++++++++- bundle/phases/deploy.go | 5 +- bundle/phases/initialize.go | 10 +- cmd/bundle/utils/process.go | 45 ++++ 40 files changed, 926 insertions(+), 152 deletions(-) create mode 100644 .nextchanges/bundles/select-deletions.md create mode 100644 acceptance/bundle/select/deletion/databricks.yml.after.tmpl create mode 100644 acceptance/bundle/select/deletion/databricks.yml.tmpl create mode 100644 acceptance/bundle/select/deletion/out.test.toml create mode 100644 acceptance/bundle/select/deletion/output.txt create mode 100644 acceptance/bundle/select/deletion/script create mode 100644 acceptance/bundle/select/deletion/test.toml create mode 100644 acceptance/bundle/select/deletion_dependents/databricks.yml.after.tmpl create mode 100644 acceptance/bundle/select/deletion_dependents/databricks.yml.live_consumer.tmpl create mode 100644 acceptance/bundle/select/deletion_dependents/databricks.yml.tmpl create mode 100644 acceptance/bundle/select/deletion_dependents/out.test.toml create mode 100644 acceptance/bundle/select/deletion_dependents/output.txt create mode 100644 acceptance/bundle/select/deletion_dependents/script create mode 100644 acceptance/bundle/select/deletion_dependents/test.toml create mode 100644 acceptance/bundle/select/deletion_saved_plan/databricks.yml.after.tmpl create mode 100644 acceptance/bundle/select/deletion_saved_plan/databricks.yml.tmpl create mode 100644 acceptance/bundle/select/deletion_saved_plan/out.test.toml create mode 100644 acceptance/bundle/select/deletion_saved_plan/output.txt create mode 100644 acceptance/bundle/select/deletion_saved_plan/script create mode 100644 acceptance/bundle/select/deletion_saved_plan/test.toml create mode 100644 acceptance/bundle/select/invalid_no_upload/databricks.yml.tmpl create mode 100644 acceptance/bundle/select/invalid_no_upload/out.test.toml create mode 100644 acceptance/bundle/select/invalid_no_upload/output.txt create mode 100644 acceptance/bundle/select/invalid_no_upload/script create mode 100644 acceptance/bundle/select/invalid_no_upload/test.toml create mode 100644 acceptance/bundle/select/redeploy_unqualified/databricks.yml.tmpl create mode 100644 acceptance/bundle/select/redeploy_unqualified/out.test.toml create mode 100644 acceptance/bundle/select/redeploy_unqualified/output.txt create mode 100644 acceptance/bundle/select/redeploy_unqualified/script create mode 100644 acceptance/bundle/select/redeploy_unqualified/test.toml create mode 100644 bundle/config/mutator/mark_select_used.go create mode 100644 bundle/config/mutator/mark_select_used_test.go delete mode 100644 bundle/config/mutator/resolve_select.go delete mode 100644 bundle/config/mutator/resolve_select_test.go diff --git a/.nextchanges/bundles/select-deletions.md b/.nextchanges/bundles/select-deletions.md new file mode 100644 index 00000000000..8a32d564bf9 --- /dev/null +++ b/.nextchanges/bundles/select-deletions.md @@ -0,0 +1 @@ +* `databricks bundle deploy --select` and `bundle plan --select` can now select resources removed from config, so a selective deploy applies their deletion. Deleting a selected resource also deletes its dependents that are also being removed, mirroring `bundle destroy`. Selecting a resource for deletion errors if a live resource still references it, preventing a broken bundle. An unqualified `--select ` is ambiguous when a resource of the same name exists in both config and state as different types. ([#7023](https://github.com/databricks/cli/pull/7023)) diff --git a/acceptance/bundle/select/deletion/databricks.yml.after.tmpl b/acceptance/bundle/select/deletion/databricks.yml.after.tmpl new file mode 100644 index 00000000000..1f769c007ef --- /dev/null +++ b/acceptance/bundle/select/deletion/databricks.yml.after.tmpl @@ -0,0 +1,7 @@ +bundle: + name: select-del-$UNIQUE_NAME + +resources: + jobs: + keep: + name: keep-$UNIQUE_NAME diff --git a/acceptance/bundle/select/deletion/databricks.yml.tmpl b/acceptance/bundle/select/deletion/databricks.yml.tmpl new file mode 100644 index 00000000000..2157a17bc39 --- /dev/null +++ b/acceptance/bundle/select/deletion/databricks.yml.tmpl @@ -0,0 +1,10 @@ +bundle: + name: select-del-$UNIQUE_NAME + +resources: + jobs: + keep: + name: keep-$UNIQUE_NAME + + gone: + name: gone-$UNIQUE_NAME diff --git a/acceptance/bundle/select/deletion/out.test.toml b/acceptance/bundle/select/deletion/out.test.toml new file mode 100644 index 00000000000..0f167effef1 --- /dev/null +++ b/acceptance/bundle/select/deletion/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/select/deletion/output.txt b/acceptance/bundle/select/deletion/output.txt new file mode 100644 index 00000000000..d39a3f3d392 --- /dev/null +++ b/acceptance/bundle/select/deletion/output.txt @@ -0,0 +1,43 @@ + +=== Initial deploy + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-del-[UNIQUE_NAME]/default/files... +Created jobs.gone +Created jobs.keep +Files: 3 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +>>> [CLI] bundle plan --select jobs.gone +delete jobs.gone + +Plan: 0 to add, 0 to change, 1 to delete, 0 unchanged, 1 not selected + +=== bundle deploy --select jobs.gone + +>>> [CLI] bundle deploy --select jobs.gone +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-del-[UNIQUE_NAME]/default/files... +Deleted jobs.gone +Files: 1 uploaded, 0 deleted +Resources: 0 created, 0 changed, 1 deleted, 0 unchanged, 1 not selected + +>>> print_requests.py --sort //jobs/delete +{ + "method": "POST", + "path": "/api/2.2/jobs/delete", + "body": { + "job_id": [GONE_ID] + } +} + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + +=== Destroy +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.jobs.keep + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/select-del-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/select/deletion/script b/acceptance/bundle/select/deletion/script new file mode 100644 index 00000000000..cf1bb396299 --- /dev/null +++ b/acceptance/bundle/select/deletion/script @@ -0,0 +1,31 @@ +envsubst '$UNIQUE_NAME' < databricks.yml.tmpl > databricks.yml + +cleanup() { + title "Destroy" + trace $CLI bundle destroy --auto-approve + rm -f "$OUT_REQUESTS" +} +trap cleanup EXIT + +# Deploy both jobs. +title "Initial deploy\n" +trace $CLI bundle deploy +# Register [KEEP_ID]/[GONE_ID] from state while both still exist, so the delete +# request body below is masked deterministically. +replace_ids.py + +# Remove 'gone' from config. It is now orphaned in state and planned for deletion. +envsubst '$UNIQUE_NAME' < databricks.yml.after.tmpl > databricks.yml + +# A resource removed from config is selectable by name, like any created or updated +# one. The plan is filtered to just its deletion. +trace $CLI bundle plan --select jobs.gone + +title "bundle deploy --select jobs.gone\n" +trace $CLI bundle deploy --select jobs.gone + +# The delete call targeted 'gone' only. +trace print_requests.py --sort //jobs/delete + +# A full plan now has nothing to delete and 'keep' is unchanged. +trace $CLI bundle plan diff --git a/acceptance/bundle/select/deletion/test.toml b/acceptance/bundle/select/deletion/test.toml new file mode 100644 index 00000000000..aa2338b15ea --- /dev/null +++ b/acceptance/bundle/select/deletion/test.toml @@ -0,0 +1,4 @@ +Cloud = true +RecordRequests = true +# databricks.yml is generated at runtime from the committed templates. +Ignore = [".databricks", ".gitignore", "databricks.yml"] diff --git a/acceptance/bundle/select/deletion_dependents/databricks.yml.after.tmpl b/acceptance/bundle/select/deletion_dependents/databricks.yml.after.tmpl new file mode 100644 index 00000000000..664ca80fb20 --- /dev/null +++ b/acceptance/bundle/select/deletion_dependents/databricks.yml.after.tmpl @@ -0,0 +1,7 @@ +bundle: + name: select-deldep-$UNIQUE_NAME + +resources: + jobs: + keep: + name: keep-$UNIQUE_NAME diff --git a/acceptance/bundle/select/deletion_dependents/databricks.yml.live_consumer.tmpl b/acceptance/bundle/select/deletion_dependents/databricks.yml.live_consumer.tmpl new file mode 100644 index 00000000000..2ea4e6e5749 --- /dev/null +++ b/acceptance/bundle/select/deletion_dependents/databricks.yml.live_consumer.tmpl @@ -0,0 +1,14 @@ +bundle: + name: select-deldep-$UNIQUE_NAME + +resources: + jobs: + keep: + name: keep-$UNIQUE_NAME + + consumer: + name: consumer-$UNIQUE_NAME + tasks: + - task_key: run_producer + run_job_task: + job_id: ${resources.jobs.producer.id} diff --git a/acceptance/bundle/select/deletion_dependents/databricks.yml.tmpl b/acceptance/bundle/select/deletion_dependents/databricks.yml.tmpl new file mode 100644 index 00000000000..e796e69d643 --- /dev/null +++ b/acceptance/bundle/select/deletion_dependents/databricks.yml.tmpl @@ -0,0 +1,17 @@ +bundle: + name: select-deldep-$UNIQUE_NAME + +resources: + jobs: + keep: + name: keep-$UNIQUE_NAME + + producer: + name: producer-$UNIQUE_NAME + + consumer: + name: consumer-$UNIQUE_NAME + tasks: + - task_key: run_producer + run_job_task: + job_id: ${resources.jobs.producer.id} diff --git a/acceptance/bundle/select/deletion_dependents/out.test.toml b/acceptance/bundle/select/deletion_dependents/out.test.toml new file mode 100644 index 00000000000..0f167effef1 --- /dev/null +++ b/acceptance/bundle/select/deletion_dependents/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/select/deletion_dependents/output.txt b/acceptance/bundle/select/deletion_dependents/output.txt new file mode 100644 index 00000000000..e4f9490f3b7 --- /dev/null +++ b/acceptance/bundle/select/deletion_dependents/output.txt @@ -0,0 +1,71 @@ + +=== Initial deploy + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-deldep-[UNIQUE_NAME]/default/files... +Created jobs.consumer +Created jobs.keep +Created jobs.producer +Files: 4 uploaded, 0 deleted +Resources: 3 created, 0 changed, 0 deleted, 0 unchanged + +>>> [CLI] bundle plan --select jobs.producer +Error: cannot delete jobs.producer: jobs.consumer still references it; remove the reference or also remove jobs.consumer from the config + + +>>> [CLI] bundle deploy --select jobs.producer +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-deldep-[UNIQUE_NAME]/default/files... +Error: cannot delete jobs.producer: jobs.consumer still references it; remove the reference or also remove jobs.consumer from the config + +Files: 1 uploaded, 0 deleted + +>>> print_requests.py --sort //jobs/delete --keep + +>>> [CLI] bundle plan +update jobs.consumer +delete jobs.producer + +Plan: 0 to add, 1 to change, 1 to delete, 1 unchanged + +>>> [CLI] bundle plan --select jobs.producer +delete jobs.consumer +delete jobs.producer + +Plan: 0 to add, 0 to change, 2 to delete, 0 unchanged, 1 not selected + +=== bundle deploy --select jobs.producer + +>>> [CLI] bundle deploy --select jobs.producer +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-deldep-[UNIQUE_NAME]/default/files... +Deleted jobs.consumer +Deleted jobs.producer +Files: 1 uploaded, 0 deleted +Resources: 0 created, 0 changed, 2 deleted, 0 unchanged, 1 not selected + +>>> print_requests.py --sort //jobs/delete +{ + "method": "POST", + "path": "/api/2.2/jobs/delete", + "body": { + "job_id": [PRODUCER_ID] + } +} +{ + "method": "POST", + "path": "/api/2.2/jobs/delete", + "body": { + "job_id": [CONSUMER_ID] + } +} + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + +=== Destroy +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.jobs.keep + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/select-deldep-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/select/deletion_dependents/script b/acceptance/bundle/select/deletion_dependents/script new file mode 100644 index 00000000000..4f70a2c27f0 --- /dev/null +++ b/acceptance/bundle/select/deletion_dependents/script @@ -0,0 +1,48 @@ +envsubst '$UNIQUE_NAME' < databricks.yml.tmpl > databricks.yml + +cleanup() { + title "Destroy" + trace $CLI bundle destroy --auto-approve + rm -f "$OUT_REQUESTS" +} +trap cleanup EXIT + +# Deploy three jobs; consumer references producer, so state records consumer's +# dependency on producer. +title "Initial deploy\n" +trace $CLI bundle deploy +replace_ids.py + +# --- error case: consumer is still live and references producer --- +# Remove only producer from config; consumer stays with its reference intact. +# Selecting producer for deletion must error: it would leave consumer broken. +envsubst '$UNIQUE_NAME' < databricks.yml.live_consumer.tmpl > databricks.yml + +musterr trace $CLI bundle plan --select jobs.producer + +# bundle deploy --select also errors before sending any delete request. +musterr trace $CLI bundle deploy --select jobs.producer + +# No delete request was sent by either command. +trace print_requests.py --sort //jobs/delete --keep + +# A full plan still works and shows producer as a pending delete. +trace $CLI bundle plan + +# --- success case: both producer and consumer are removed from config --- +# Remove producer and consumer from config; both are now planned for deletion. +envsubst '$UNIQUE_NAME' < databricks.yml.after.tmpl > databricks.yml + +# Selecting the depended-upon producer for deletion also deletes its dependent +# consumer (which would otherwise be left referencing a deleted job). 'keep' is +# untouched. The dependency direction is reversed for deletes. +trace $CLI bundle plan --select jobs.producer + +title "bundle deploy --select jobs.producer\n" +trace $CLI bundle deploy --select jobs.producer + +# Both producer and consumer were deleted; keep was not. +trace print_requests.py --sort //jobs/delete + +# A full plan now has nothing to delete and 'keep' is unchanged. +trace $CLI bundle plan diff --git a/acceptance/bundle/select/deletion_dependents/test.toml b/acceptance/bundle/select/deletion_dependents/test.toml new file mode 100644 index 00000000000..aa2338b15ea --- /dev/null +++ b/acceptance/bundle/select/deletion_dependents/test.toml @@ -0,0 +1,4 @@ +Cloud = true +RecordRequests = true +# databricks.yml is generated at runtime from the committed templates. +Ignore = [".databricks", ".gitignore", "databricks.yml"] diff --git a/acceptance/bundle/select/deletion_saved_plan/databricks.yml.after.tmpl b/acceptance/bundle/select/deletion_saved_plan/databricks.yml.after.tmpl new file mode 100644 index 00000000000..04a7399dcc9 --- /dev/null +++ b/acceptance/bundle/select/deletion_saved_plan/databricks.yml.after.tmpl @@ -0,0 +1,7 @@ +bundle: + name: select-del-plan-$UNIQUE_NAME + +resources: + jobs: + keep: + name: keep-$UNIQUE_NAME diff --git a/acceptance/bundle/select/deletion_saved_plan/databricks.yml.tmpl b/acceptance/bundle/select/deletion_saved_plan/databricks.yml.tmpl new file mode 100644 index 00000000000..b8637ee0c37 --- /dev/null +++ b/acceptance/bundle/select/deletion_saved_plan/databricks.yml.tmpl @@ -0,0 +1,10 @@ +bundle: + name: select-del-plan-$UNIQUE_NAME + +resources: + jobs: + keep: + name: keep-$UNIQUE_NAME + + gone: + name: gone-$UNIQUE_NAME diff --git a/acceptance/bundle/select/deletion_saved_plan/out.test.toml b/acceptance/bundle/select/deletion_saved_plan/out.test.toml new file mode 100644 index 00000000000..0f167effef1 --- /dev/null +++ b/acceptance/bundle/select/deletion_saved_plan/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/select/deletion_saved_plan/output.txt b/acceptance/bundle/select/deletion_saved_plan/output.txt new file mode 100644 index 00000000000..026d4672a6b --- /dev/null +++ b/acceptance/bundle/select/deletion_saved_plan/output.txt @@ -0,0 +1,38 @@ + +=== Initial deploy + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-del-plan-[UNIQUE_NAME]/default/files... +Created jobs.gone +Created jobs.keep +Files: 3 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== bundle deploy --plan (no --select) + +>>> [CLI] bundle deploy --plan plan.json +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-del-plan-[UNIQUE_NAME]/default/files... +Deleted jobs.gone +Files: 2 uploaded, 0 deleted +Resources: 0 created, 0 changed, 1 deleted, 0 unchanged, 1 not selected + +>>> print_requests.py --sort //jobs/delete +{ + "method": "POST", + "path": "/api/2.2/jobs/delete", + "body": { + "job_id": [GONE_ID] + } +} + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + +=== Destroy +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.jobs.keep + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/select-del-plan-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/select/deletion_saved_plan/script b/acceptance/bundle/select/deletion_saved_plan/script new file mode 100644 index 00000000000..8575df90011 --- /dev/null +++ b/acceptance/bundle/select/deletion_saved_plan/script @@ -0,0 +1,34 @@ +envsubst '$UNIQUE_NAME' < databricks.yml.tmpl > databricks.yml + +cleanup() { + title "Destroy" + trace $CLI bundle destroy --auto-approve + rm -f "$OUT_REQUESTS" +} +trap cleanup EXIT + +# Deploy both jobs. +title "Initial deploy\n" +trace $CLI bundle deploy +# Register [KEEP_ID]/[GONE_ID] from state while both still exist, so the delete +# request body below is masked deterministically. +replace_ids.py + +# Remove 'gone' from config. It is now orphaned in state and planned for deletion. +envsubst '$UNIQUE_NAME' < databricks.yml.after.tmpl > databricks.yml + +# Filter the plan to the deletion and serialize it. (Kept out of recorded output: +# the JSON embeds remote state that differs between local and cloud.) +$CLI bundle plan --select jobs.gone -o json > plan.json + +# Deploy from the plan file WITHOUT re-passing --select: the deletion was selected +# at plan time and survives serialization, so applying the saved plan deletes 'gone' +# and reports the ", 1 not selected" suffix from plan.NotSelected, not the flag. +title "bundle deploy --plan (no --select)\n" +trace $CLI bundle deploy --plan plan.json + +# The delete call targeted 'gone' only. +trace print_requests.py --sort //jobs/delete + +# A full plan now has nothing to delete and 'keep' is unchanged. +trace $CLI bundle plan diff --git a/acceptance/bundle/select/deletion_saved_plan/test.toml b/acceptance/bundle/select/deletion_saved_plan/test.toml new file mode 100644 index 00000000000..468ad526604 --- /dev/null +++ b/acceptance/bundle/select/deletion_saved_plan/test.toml @@ -0,0 +1,5 @@ +Cloud = true +RecordRequests = true +# databricks.yml is generated at runtime from the committed templates; the serialized +# plan embeds local/cloud-specific state, so it is kept out of the recorded output. +Ignore = [".databricks", ".gitignore", "databricks.yml", "plan.json"] diff --git a/acceptance/bundle/select/invalid_no_upload/databricks.yml.tmpl b/acceptance/bundle/select/invalid_no_upload/databricks.yml.tmpl new file mode 100644 index 00000000000..d7b416f590a --- /dev/null +++ b/acceptance/bundle/select/invalid_no_upload/databricks.yml.tmpl @@ -0,0 +1,12 @@ +bundle: + name: select-invalid-$UNIQUE_NAME + +experimental: + scripts: + # Must NOT run: the invalid selector is rejected before the predeploy script. + predeploy: touch predeploy_ran.marker + +resources: + jobs: + real: + name: real-$UNIQUE_NAME diff --git a/acceptance/bundle/select/invalid_no_upload/out.test.toml b/acceptance/bundle/select/invalid_no_upload/out.test.toml new file mode 100644 index 00000000000..a927a5fbc06 --- /dev/null +++ b/acceptance/bundle/select/invalid_no_upload/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/select/invalid_no_upload/output.txt b/acceptance/bundle/select/invalid_no_upload/output.txt new file mode 100644 index 00000000000..f643c71056f --- /dev/null +++ b/acceptance/bundle/select/invalid_no_upload/output.txt @@ -0,0 +1,6 @@ + +>>> [CLI] bundle deploy --select jobs.nonexistent +Error: no such resource: jobs.nonexistent + + +>>> print_requests.py --dms //workspace //api/2.0/bundle diff --git a/acceptance/bundle/select/invalid_no_upload/script b/acceptance/bundle/select/invalid_no_upload/script new file mode 100644 index 00000000000..8440a70c060 --- /dev/null +++ b/acceptance/bundle/select/invalid_no_upload/script @@ -0,0 +1,13 @@ +envsubst '$UNIQUE_NAME' < databricks.yml.tmpl > databricks.yml + +# A --select selector that matches no resource is rejected before the predeploy script +# runs and before any files are uploaded or state is pushed. +musterr trace $CLI bundle deploy --select jobs.nonexistent + +# The predeploy script never ran: its marker file was not created. +find.py predeploy_ran.marker --expect 0 + +# Nothing was written: no non-GET requests to the workspace (file upload / state push) +# or to the bundle deployment API (DMS state). --dms includes the latter, which is +# excluded by default. +trace print_requests.py --dms //workspace //api/2.0/bundle diff --git a/acceptance/bundle/select/invalid_no_upload/test.toml b/acceptance/bundle/select/invalid_no_upload/test.toml new file mode 100644 index 00000000000..4757a899c96 --- /dev/null +++ b/acceptance/bundle/select/invalid_no_upload/test.toml @@ -0,0 +1,4 @@ +RecordRequests = true +# databricks.yml is generated at runtime from the committed template. predeploy_ran.marker +# is a guard artifact the predeploy script would create if it wrongly ran. +Ignore = [".databricks", ".gitignore", "databricks.yml", "predeploy_ran.marker"] diff --git a/acceptance/bundle/select/missing/script b/acceptance/bundle/select/missing/script index 878e547cec7..e4dca420aac 100644 --- a/acceptance/bundle/select/missing/script +++ b/acceptance/bundle/select/missing/script @@ -1,5 +1,5 @@ -# Selectors are resolved by the ResolveSelect mutator during initialize, before the -# engine is known, so these errors are identical on both engines. +# An unknown resource is rejected early, once state is open (process.go), before the +# plan is built; FilterToSelected also validates it when the plan is computed. # Unqualified name that matches no resource. musterr trace $CLI bundle plan --select no_such_resource diff --git a/acceptance/bundle/select/redeploy_unqualified/databricks.yml.tmpl b/acceptance/bundle/select/redeploy_unqualified/databricks.yml.tmpl new file mode 100644 index 00000000000..5cb9c243dcc --- /dev/null +++ b/acceptance/bundle/select/redeploy_unqualified/databricks.yml.tmpl @@ -0,0 +1,7 @@ +bundle: + name: select-redeploy-$UNIQUE_NAME + +resources: + jobs: + my_job: + name: my-job-$UNIQUE_NAME diff --git a/acceptance/bundle/select/redeploy_unqualified/out.test.toml b/acceptance/bundle/select/redeploy_unqualified/out.test.toml new file mode 100644 index 00000000000..a927a5fbc06 --- /dev/null +++ b/acceptance/bundle/select/redeploy_unqualified/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/select/redeploy_unqualified/output.txt b/acceptance/bundle/select/redeploy_unqualified/output.txt new file mode 100644 index 00000000000..b09c02e30e2 --- /dev/null +++ b/acceptance/bundle/select/redeploy_unqualified/output.txt @@ -0,0 +1,24 @@ + +=== Initial deploy + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-redeploy-[UNIQUE_NAME]/default/files... +Created jobs.my_job +Files: 2 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 0 unchanged + +=== Redeploy with unqualified --select + +>>> [CLI] bundle deploy --select my_job +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-redeploy-[UNIQUE_NAME]/default/files... +Files: 0 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 1 unchanged + +=== Destroy +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.jobs.my_job + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/select-redeploy-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/select/redeploy_unqualified/script b/acceptance/bundle/select/redeploy_unqualified/script new file mode 100644 index 00000000000..f99a7543e4d --- /dev/null +++ b/acceptance/bundle/select/redeploy_unqualified/script @@ -0,0 +1,15 @@ +envsubst '$UNIQUE_NAME' < databricks.yml.tmpl > databricks.yml + +cleanup() { + title "Destroy" + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +title "Initial deploy\n" +trace $CLI bundle deploy + +# Redeploy selecting the resource by its unqualified name. It exists in both config +# and state now; that must not read as an ambiguous match. +title "Redeploy with unqualified --select\n" +trace $CLI bundle deploy --select my_job diff --git a/acceptance/bundle/select/redeploy_unqualified/test.toml b/acceptance/bundle/select/redeploy_unqualified/test.toml new file mode 100644 index 00000000000..20c63f73703 --- /dev/null +++ b/acceptance/bundle/select/redeploy_unqualified/test.toml @@ -0,0 +1 @@ +Ignore = [".databricks", ".gitignore", "databricks.yml"] diff --git a/bundle/config/mutator/mark_select_used.go b/bundle/config/mutator/mark_select_used.go new file mode 100644 index 00000000000..168eb59f396 --- /dev/null +++ b/bundle/config/mutator/mark_select_used.go @@ -0,0 +1,32 @@ +package mutator + +import ( + "context" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/metrics" + "github.com/databricks/cli/libs/diag" +) + +type markSelectUsed struct{} + +// MarkSelectUsed records in telemetry whether the --select flag was used. It runs +// during initialize so the metric is set even for a deploy that loads a prebuilt +// plan and therefore skips RunPlan. Selectors are validated once state is open (an +// early check in process.go) and resolved against the computed plan in +// Plan.FilterToSelected, so resources removed from config (planned for deletion) +// can be selected too. +func MarkSelectUsed() bundle.Mutator { + return &markSelectUsed{} +} + +func (m *markSelectUsed) Name() string { + return "MarkSelectUsed" +} + +func (m *markSelectUsed) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { + if len(b.Select) > 0 { + b.Metrics.SetBoolValue(metrics.SelectUsed, true) + } + return nil +} diff --git a/bundle/config/mutator/mark_select_used_test.go b/bundle/config/mutator/mark_select_used_test.go new file mode 100644 index 00000000000..b2da87c681d --- /dev/null +++ b/bundle/config/mutator/mark_select_used_test.go @@ -0,0 +1,40 @@ +package mutator_test + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/metrics" + "github.com/databricks/cli/libs/telemetry/protos" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMarkSelectUsed_Set checks that the telemetry flag is recorded when --select +// is used. Selector resolution is covered by TestFilterToSelected_* and the +// acceptance tests under acceptance/bundle/select. +func TestMarkSelectUsed_Set(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{"my_job": {}}, + }, + }, + } + b.Select = []string{"my_job"} + diags := bundle.Apply(t.Context(), b, mutator.MarkSelectUsed()) + require.NoError(t, diags.Error()) + assert.Contains(t, b.Metrics.BoolValues, protos.BoolMapEntry{Key: metrics.SelectUsed, Value: true}) +} + +// TestMarkSelectUsed_Unset checks that the flag is left unset when --select is not +// used, so bundles that never select keep the metric out of their telemetry. +func TestMarkSelectUsed_Unset(t *testing.T) { + b := &bundle.Bundle{} + diags := bundle.Apply(t.Context(), b, mutator.MarkSelectUsed()) + require.NoError(t, diags.Error()) + assert.Empty(t, b.Metrics.BoolValues) +} diff --git a/bundle/config/mutator/resolve_select.go b/bundle/config/mutator/resolve_select.go deleted file mode 100644 index 28d9751c322..00000000000 --- a/bundle/config/mutator/resolve_select.go +++ /dev/null @@ -1,76 +0,0 @@ -package mutator - -import ( - "context" - "strings" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/metrics" - "github.com/databricks/cli/libs/diag" -) - -type resolveSelect struct{} - -// ResolveSelect returns a mutator that resolves and validates the selectors in -// b.Select, normalizing each to its qualified "type.name" form. Selectors may be -// "type.name" (e.g. "jobs.myjob") or just "name" if unique across all resource -// types. The mutator does not filter the config; the direct engine selects against -// the resolved keys later via plan.FilterToSelected. -// If b.Select is empty, this is a no-op. -func ResolveSelect() bundle.Mutator { - return &resolveSelect{} -} - -func (m *resolveSelect) Name() string { - return "ResolveSelect" -} - -func (m *resolveSelect) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { - if len(b.Select) == 0 { - return nil - } - - b.Metrics.SetBoolValue(metrics.SelectUsed, true) - - // Build reverse index: unqualified name → []"type.name" matches. - byName := map[string][]string{} - for _, group := range b.Config.Resources.AllResources() { - typeName := group.Description.PluralName - for name := range group.Resources { - byName[name] = append(byName[name], typeName+"."+name) - } - } - - resolved := make([]string, 0, len(b.Select)) - for _, selector := range b.Select { - if strings.Contains(selector, ".") { - typeName, name, _ := strings.Cut(selector, ".") - found := false - for _, group := range b.Config.Resources.AllResources() { - if group.Description.PluralName == typeName { - if _, ok := group.Resources[name]; ok { - found = true - } - break - } - } - if !found { - return diag.Errorf("no such resource: %s", selector) - } - resolved = append(resolved, selector) - } else { - matches := byName[selector] - switch len(matches) { - case 0: - return diag.Errorf("no such resource: %s", selector) - case 1: - resolved = append(resolved, matches[0]) - default: - return diag.Errorf("ambiguous resource: %s (can resolve to %s); use a qualified name to disambiguate", selector, strings.Join(matches, ", ")) - } - } - } - - b.Select = resolved - return nil -} diff --git a/bundle/config/mutator/resolve_select_test.go b/bundle/config/mutator/resolve_select_test.go deleted file mode 100644 index d116d9ca601..00000000000 --- a/bundle/config/mutator/resolve_select_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package mutator_test - -import ( - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/mutator" - "github.com/databricks/cli/bundle/config/resources" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// TestResolveSelect_Normalizes checks that selectors are normalized to their -// qualified "type.name" form in place, without filtering the config. End-to-end -// selection behavior is covered by acceptance/bundle/select. -func TestResolveSelect_Normalizes(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{"my_job": {}}, - Pipelines: map[string]*resources.Pipeline{"my_pipeline": {}}, - }, - }, - } - b.Select = []string{"my_job", "pipelines.my_pipeline"} - diags := bundle.Apply(t.Context(), b, mutator.ResolveSelect()) - require.NoError(t, diags.Error()) - assert.Equal(t, []string{"jobs.my_job", "pipelines.my_pipeline"}, b.Select) - // Config is left untouched; the mutator only resolves selectors. - assert.Len(t, b.Config.Resources.Jobs, 1) - assert.Len(t, b.Config.Resources.Pipelines, 1) -} - -// TestResolveSelect_Ambiguous covers the ambiguous-selector error in isolation. -// In a real bundle this is unreachable — UniqueResourceKeys rejects two resources -// sharing a key across types before this mutator runs (see the acceptance test -// bundle/select_errors/ambiguous for the real, load-time error). -func TestResolveSelect_Ambiguous(t *testing.T) { - b := &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{"thing": {}}, - Pipelines: map[string]*resources.Pipeline{"thing": {}}, - }, - }, - } - b.Select = []string{"thing"} - diags := bundle.Apply(t.Context(), b, mutator.ResolveSelect()) - require.Error(t, diags.Error()) - assert.ErrorContains(t, diags.Error(), "ambiguous resource: thing") - assert.ErrorContains(t, diags.Error(), "use a qualified name to disambiguate") -} diff --git a/bundle/deployplan/plan.go b/bundle/deployplan/plan.go index 874479db43d..a6bbefceb03 100644 --- a/bundle/deployplan/plan.go +++ b/bundle/deployplan/plan.go @@ -2,10 +2,15 @@ package deployplan import ( "cmp" + "context" "encoding/json" + "errors" "fmt" + "iter" + "maps" "os" "slices" + "strings" "sync" "github.com/databricks/cli/internal/build" @@ -33,6 +38,17 @@ type Plan struct { // (--plan); used only for summary reporting. NotSelected int `json:"not_selected,omitempty"` + // WasFiltered is true when FilterToSelected was called on this plan, regardless + // of how many resources were excluded. Serialized so process.go can distinguish + // a plan that was pre-filtered at plan time (suppress the "--select ignored" + // warning) from one that was not (the flag has no effect and should warn). + WasFiltered bool `json:"was_filtered,omitempty"` + + // Selectors holds the raw --select values used to produce this filtered plan. + // Serialized alongside WasFiltered so process.go can warn when --select at + // deploy time differs from the selectors baked into the plan file. + Selectors []string `json:"selectors,omitempty"` + mutex sync.Mutex `json:"-"` lockmap lockmap `json:"-"` } @@ -277,17 +293,87 @@ func (p *Plan) ReadUnlockEntry(resourceKey string) { p.lockmap.RUnlock(resourceKey) } -// FilterToSelected reduces the plan to the nodes in selected (format "type.name", -// e.g. "jobs.my_job") plus their transitive dependencies as recorded in each -// entry's DependsOn field. Nodes not reachable from the selected set are removed. -func (p *Plan) FilterToSelected(selected []string) { +// ResolveSelectors maps each --select selector to its plan key ("resources.type.name") +// against resourceKeys of that same form. A selector is a qualified "type.name" or an +// unqualified "name"; it errors on no match or, when unqualified, more than one. +// Duplicate resourceKeys (a resource present in both config and state) collapse, so an +// unqualified selector is never falsely ambiguous against itself. +func ResolveSelectors(resourceKeys iter.Seq[string], selectors []string) ([]string, error) { + // Index top-level resource keys by name and as qualified "type.name". Keys look like + // "resources.." (names have no dots, see Action.IsChildResource), so + // sub-nodes (4 parts: grants/permissions) are skipped as not --select-addressable. + byName := map[string][]string{} + known := map[string]struct{}{} + for key := range resourceKeys { + parts := strings.Split(key, ".") + if len(parts) != 3 || parts[0] != "resources" { + continue + } + typeName, name := parts[1], parts[2] + // Internal resource types (e.g. internal_immutable_snapshots) are not user- + // addressable; skip them so they neither resolve a selector nor make an + // unqualified one ambiguous (nor leak an internal type into the error). + if strings.HasPrefix(typeName, "internal_") { + continue + } + qualified := typeName + "." + name + if _, alreadySeen := known[qualified]; !alreadySeen { + known[qualified] = struct{}{} + byName[name] = append(byName[name], qualified) + } + } + + resolved := make([]string, 0, len(selectors)) + for _, s := range selectors { + var qualified string + if strings.Contains(s, ".") { + if _, ok := known[s]; !ok { + return nil, fmt.Errorf("no such resource: %s", s) + } + qualified = s + } else { + switch matches := byName[s]; len(matches) { + case 0: + return nil, fmt.Errorf("no such resource: %s", s) + case 1: + qualified = matches[0] + default: + slices.Sort(matches) + return nil, fmt.Errorf("ambiguous resource: %s (can resolve to %s); use a qualified name to disambiguate", s, strings.Join(matches, ", ")) + } + } + resolved = append(resolved, "resources."+qualified) + } + return resolved, nil +} + +// FilterToSelected reduces the plan to the resources named in selected (resolved via +// ResolveSelectors) plus the others that must change with them. The closure follows +// edges by action, like the apply graph (makeGraph): a deployed resource pulls in its +// dependencies (forward), a deleted one its dependents (reverse) so they are removed +// together. A delete selection errors if a live resource (not itself being deleted) +// still references the resource to be deleted; removing it would break the live resource. +func (p *Plan) FilterToSelected(_ context.Context, selected []string) error { before := len(p.Plan) - // Convert "type.name" → "resources.type.name" (plan key format). + resolved, err := ResolveSelectors(maps.Keys(p.Plan), selected) + if err != nil { + return err + } + + // Only build reverse edges when at least one selected resource is being deleted, + // since non-delete selections only follow forward DependsOn edges. + var dependents map[string][]string + for _, key := range resolved { + if e, ok := p.Plan[key]; ok && e.Action == Delete { + dependents = p.reverseEdges() + break + } + } + queue := make([]string, 0, len(selected)) reachable := make(map[string]struct{}, len(selected)) - for _, s := range selected { - key := "resources." + s + for _, key := range resolved { p.enqueueReachable(reachable, &queue, key) // Grants and permissions are modeled as separate plan nodes for internal // reasons, but the user cannot address them via --select. Pull them in as @@ -298,14 +384,45 @@ func (p *Plan) FilterToSelected(selected []string) { p.enqueueReachable(reachable, &queue, key+".permissions") } - // BFS following DependsOn edges to include transitive dependencies. + // BFS: deleted nodes pull in their dependents (also being deleted); deployed + // nodes pull in their dependencies. Collect errors for live dependents of a + // delete selection before returning, so the user sees all conflicts at once. + var errs []string for len(queue) > 0 { key := queue[0] queue = queue[1:] - for _, dep := range p.Plan[key].DependsOn { - p.enqueueReachable(reachable, &queue, dep.Node) + if p.Plan[key].Action == Delete { + for _, dependent := range dependents[key] { + // Sub-nodes (grants/permissions, 4-part key) are not user-facing dependents + // and never trigger the live-dependent error. If they are themselves being + // deleted, they must be pulled in alongside their parent. + if strings.Count(dependent, ".") >= 3 { + if p.Plan[dependent].Action == Delete { + p.enqueueReachable(reachable, &queue, dependent) + } + continue + } + if p.Plan[dependent].Action == Delete { + // Dependent is also being deleted: pull it in. + p.enqueueReachable(reachable, &queue, dependent) + } else { + // Dependent is live (still in config): deleting key would leave + // it broken. Report a user-facing error with both names. + deleted := strings.TrimPrefix(key, "resources.") + live := strings.TrimPrefix(dependent, "resources.") + errs = append(errs, fmt.Sprintf("cannot delete %s: %s still references it; remove the reference or also remove %s from the config", deleted, live, live)) + } + } + } else { + for _, dep := range p.Plan[key].DependsOn { + p.enqueueReachable(reachable, &queue, dep.Node) + } } } + if len(errs) > 0 { + slices.Sort(errs) + return errors.New(strings.Join(errs, "\n")) + } for key := range p.Plan { if _, ok := reachable[key]; !ok { @@ -314,6 +431,21 @@ func (p *Plan) FilterToSelected(selected []string) { } p.NotSelected = before - len(p.Plan) + p.WasFiltered = true + p.Selectors = slices.Clone(selected) + return nil +} + +// reverseEdges returns the reverse of the plan's DependsOn edges: for each node, the +// nodes that depend on it. +func (p *Plan) reverseEdges() map[string][]string { + dependents := map[string][]string{} + for key, entry := range p.Plan { + for _, dep := range entry.DependsOn { + dependents[dep.Node] = append(dependents[dep.Node], key) + } + } + return dependents } // enqueueReachable marks key as reachable and appends it to queue, if key exists diff --git a/bundle/deployplan/plan_filter_test.go b/bundle/deployplan/plan_filter_test.go index c5451f470b1..53f6b133200 100644 --- a/bundle/deployplan/plan_filter_test.go +++ b/bundle/deployplan/plan_filter_test.go @@ -1,10 +1,12 @@ package deployplan_test import ( + "slices" "testing" "github.com/databricks/cli/bundle/deployplan" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func planWithDeps() *deployplan.Plan { @@ -39,7 +41,7 @@ func planWithGrants() *deployplan.Plan { func TestFilterToSelected_Direct(t *testing.T) { p := planWithDeps() - p.FilterToSelected([]string{"jobs.foo"}) + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.foo"})) assert.Contains(t, p.Plan, "resources.jobs.foo") assert.Contains(t, p.Plan, "resources.jobs.bar") assert.Contains(t, p.Plan, "resources.jobs.baz") @@ -49,7 +51,7 @@ func TestFilterToSelected_Direct(t *testing.T) { func TestFilterToSelected_NoDeps(t *testing.T) { p := planWithDeps() - p.FilterToSelected([]string{"jobs.baz"}) + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.baz"})) assert.Contains(t, p.Plan, "resources.jobs.baz") assert.NotContains(t, p.Plan, "resources.jobs.foo") assert.NotContains(t, p.Plan, "resources.jobs.bar") @@ -59,7 +61,7 @@ func TestFilterToSelected_NoDeps(t *testing.T) { func TestFilterToSelected_Multiple(t *testing.T) { p := planWithDeps() - p.FilterToSelected([]string{"jobs.baz", "jobs.independent"}) + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.baz", "jobs.independent"})) assert.Contains(t, p.Plan, "resources.jobs.baz") assert.Contains(t, p.Plan, "resources.jobs.independent") assert.NotContains(t, p.Plan, "resources.jobs.foo") @@ -67,9 +69,17 @@ func TestFilterToSelected_Multiple(t *testing.T) { assert.Equal(t, 2, p.NotSelected) } +func TestFilterToSelected_Unqualified(t *testing.T) { + p := planWithDeps() + require.NoError(t, p.FilterToSelected(t.Context(), []string{"baz"})) + assert.Contains(t, p.Plan, "resources.jobs.baz") + assert.NotContains(t, p.Plan, "resources.jobs.foo") + assert.Equal(t, 3, p.NotSelected) +} + func TestFilterToSelected_IncludesGrantsAndPermissions(t *testing.T) { p := planWithGrants() - p.FilterToSelected([]string{"schemas.bronze"}) + require.NoError(t, p.FilterToSelected(t.Context(), []string{"schemas.bronze"})) assert.Contains(t, p.Plan, "resources.schemas.bronze") assert.Contains(t, p.Plan, "resources.schemas.bronze.grants") assert.Contains(t, p.Plan, "resources.schemas.bronze.permissions") @@ -80,9 +90,207 @@ func TestFilterToSelected_IncludesGrantsAndPermissions(t *testing.T) { func TestFilterToSelected_MissingSubNodesAreSkipped(t *testing.T) { p := planWithGrants() // silver has grants but no permissions node; selecting it must not fail. - p.FilterToSelected([]string{"schemas.silver"}) + require.NoError(t, p.FilterToSelected(t.Context(), []string{"schemas.silver"})) assert.Contains(t, p.Plan, "resources.schemas.silver") assert.Contains(t, p.Plan, "resources.schemas.silver.grants") assert.NotContains(t, p.Plan, "resources.schemas.silver.permissions") assert.NotContains(t, p.Plan, "resources.schemas.bronze") } + +// A grants/permissions sub-node is not addressable via --select. +func TestFilterToSelected_SubNodeNotSelectable(t *testing.T) { + p := planWithGrants() + err := p.FilterToSelected(t.Context(), []string{"schemas.bronze.grants"}) + assert.ErrorContains(t, err, "no such resource: schemas.bronze.grants") +} + +func TestFilterToSelected_Missing(t *testing.T) { + p := planWithDeps() + assert.ErrorContains(t, p.FilterToSelected(t.Context(), []string{"nope"}), "no such resource: nope") + assert.ErrorContains(t, p.FilterToSelected(t.Context(), []string{"jobs.nope"}), "no such resource: jobs.nope") +} + +// A resource planned for deletion (removed from config, present in state) is +// selectable just like a created or updated one. +func TestFilterToSelected_Deletion(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.keep"] = &deployplan.PlanEntry{} + p.Plan["resources.jobs.gone"] = &deployplan.PlanEntry{Action: deployplan.Delete} + + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.gone"})) + assert.Contains(t, p.Plan, "resources.jobs.gone") + assert.NotContains(t, p.Plan, "resources.jobs.keep") + assert.Equal(t, 1, p.NotSelected) +} + +// An unqualified selector matching a created resource of one type and a deleted +// resource of another is ambiguous and must be reported, not silently resolved. +func TestFilterToSelected_AmbiguousAcrossCreateAndDelete(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.thing"] = &deployplan.PlanEntry{} + p.Plan["resources.pipelines.thing"] = &deployplan.PlanEntry{Action: deployplan.Delete} + + err := p.FilterToSelected(t.Context(), []string{"thing"}) + assert.ErrorContains(t, err, "ambiguous resource: thing") + assert.ErrorContains(t, err, "jobs.thing, pipelines.thing") + assert.ErrorContains(t, err, "use a qualified name to disambiguate") +} + +// A resource present in both config and state (any redeploy) appears as a duplicate +// key; ResolveSelectors collapses it so an unqualified selector is not falsely +// ambiguous against itself. +func TestResolveSelectors_DeduplicatesKeys(t *testing.T) { + keys := []string{"resources.jobs.foo", "resources.jobs.foo"} + resolved, err := deployplan.ResolveSelectors(slices.Values(keys), []string{"foo"}) + require.NoError(t, err) + assert.Equal(t, []string{"resources.jobs.foo"}, resolved) +} + +// Internal resource types (internal_immutable_snapshots) are not --select-addressable +// and must not resolve a selector, make an unqualified name ambiguous, or leak into an +// error — even when a user resource shares the internal resource's name. +func TestResolveSelectors_IgnoresInternalTypes(t *testing.T) { + keys := []string{"resources.jobs.immutable", "resources.internal_immutable_snapshots.immutable"} + + // Unqualified: resolves to the user resource, not ambiguous. + resolved, err := deployplan.ResolveSelectors(slices.Values(keys), []string{"immutable"}) + require.NoError(t, err) + assert.Equal(t, []string{"resources.jobs.immutable"}, resolved) + + // Qualified internal type: not selectable. + _, err = deployplan.ResolveSelectors(slices.Values(keys), []string{"internal_immutable_snapshots.immutable"}) + assert.ErrorContains(t, err, "no such resource") + + // Ambiguity error must not mention the internal type. + keys2 := []string{"resources.jobs.immutable", "resources.pipelines.immutable", "resources.internal_immutable_snapshots.immutable"} + _, err = deployplan.ResolveSelectors(slices.Values(keys2), []string{"immutable"}) + require.Error(t, err) + assert.ErrorContains(t, err, "jobs.immutable") + assert.NotContains(t, err.Error(), "internal_immutable_snapshots") +} + +// Multiple live dependents of the same deleted resource are all reported in one error. +func TestFilterToSelected_DeletionErrorsOnMultipleLiveDependents(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.gone"] = &deployplan.PlanEntry{Action: deployplan.Delete} + p.Plan["resources.jobs.consumer1"] = &deployplan.PlanEntry{ + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.gone"}}, + } + p.Plan["resources.jobs.consumer2"] = &deployplan.PlanEntry{ + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.gone"}}, + } + + err := p.FilterToSelected(t.Context(), []string{"jobs.gone"}) + require.Error(t, err) + assert.ErrorContains(t, err, "jobs.consumer1") + assert.ErrorContains(t, err, "jobs.consumer2") +} + +// FilterToSelected sets WasFiltered and Selectors regardless of how many resources were +// excluded, including when the selection covers all resources (NotSelected == 0). +func TestFilterToSelected_SetsWasFiltered(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.only"] = &deployplan.PlanEntry{} + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.only"})) + assert.True(t, p.WasFiltered) + assert.Equal(t, 0, p.NotSelected) + assert.Equal(t, []string{"jobs.only"}, p.Selectors) +} + +// When a resource is pulled in by the reverse-edge BFS as a Delete dependent, its own +// grants/permissions sub-nodes must also be included in the filtered plan. +func TestFilterToSelected_DeletionIncludesTransitiveDependentSubNodes(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.schemas.gone"] = &deployplan.PlanEntry{Action: deployplan.Delete} + // consumer depends on gone and is also being deleted. + p.Plan["resources.schemas.consumer"] = &deployplan.PlanEntry{ + Action: deployplan.Delete, + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.schemas.gone"}}, + } + // consumer's grants sub-node also depends on consumer (sub-node → parent). + p.Plan["resources.schemas.consumer.grants"] = &deployplan.PlanEntry{ + Action: deployplan.Delete, + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.schemas.consumer"}}, + } + + require.NoError(t, p.FilterToSelected(t.Context(), []string{"schemas.gone"})) + assert.Contains(t, p.Plan, "resources.schemas.gone") + assert.Contains(t, p.Plan, "resources.schemas.consumer") + assert.Contains(t, p.Plan, "resources.schemas.consumer.grants") +} + +// Selecting a deployed resource drags in its dependencies so its references +// resolve; the dependency keeps its planned action. +func TestFilterToSelected_DependencyIncluded(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.foo"] = &deployplan.PlanEntry{ + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.bar"}}, + } + p.Plan["resources.jobs.bar"] = &deployplan.PlanEntry{Action: deployplan.Create} + + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.foo"})) + assert.Contains(t, p.Plan, "resources.jobs.bar") + assert.Equal(t, deployplan.Create, p.Plan["resources.jobs.bar"].Action) +} + +// Explicitly selecting a resource planned for deletion still deletes it. +func TestFilterToSelected_ExplicitDeleteKept(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.gone"] = &deployplan.PlanEntry{Action: deployplan.Delete} + + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.gone"})) + assert.Equal(t, deployplan.Delete, p.Plan["resources.jobs.gone"].Action) +} + +// Selecting a resource for deletion drags in and deletes its dependents (resources +// that reference it), since leaving them would break their configuration. The +// dependency direction is reversed for deletes, mirroring `bundle destroy`. +func TestFilterToSelected_DeletionIncludesDependents(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.gone"] = &deployplan.PlanEntry{Action: deployplan.Delete} + // consumer referenced gone, so its state records a dependency on gone; both were + // removed from config and are planned for deletion. + p.Plan["resources.jobs.consumer"] = &deployplan.PlanEntry{ + Action: deployplan.Delete, + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.gone"}}, + } + p.Plan["resources.jobs.unrelated"] = &deployplan.PlanEntry{Action: deployplan.Delete} + + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.gone"})) + assert.Equal(t, deployplan.Delete, p.Plan["resources.jobs.gone"].Action) + assert.Equal(t, deployplan.Delete, p.Plan["resources.jobs.consumer"].Action) + assert.NotContains(t, p.Plan, "resources.jobs.unrelated") +} + +// Deleting a resource that a live (non-delete) dependent still references is an error: +// proceeding would leave the dependent broken. Both the deleted and the live resource +// are named in the message without the "resources." prefix. +func TestFilterToSelected_DeletionErrorsOnLiveDependent(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.gone"] = &deployplan.PlanEntry{Action: deployplan.Delete} + // live still references gone and has a pending update. + p.Plan["resources.jobs.live"] = &deployplan.PlanEntry{ + Action: deployplan.Update, + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.gone"}}, + } + + err := p.FilterToSelected(t.Context(), []string{"jobs.gone"}) + require.Error(t, err) + assert.ErrorContains(t, err, "cannot delete jobs.gone") + assert.ErrorContains(t, err, "jobs.live still references it") +} + +// Selecting a resource for deletion leaves its own dependencies alone: they are not +// pulled into the plan, so they stay deployed untouched. +func TestFilterToSelected_DeletionLeavesDependencies(t *testing.T) { + p := deployplan.NewPlanDirect() + p.Plan["resources.jobs.gone"] = &deployplan.PlanEntry{ + Action: deployplan.Delete, + DependsOn: []deployplan.DependsOnEntry{{Node: "resources.jobs.lib"}}, + } + p.Plan["resources.jobs.lib"] = &deployplan.PlanEntry{Action: deployplan.Skip} + + require.NoError(t, p.FilterToSelected(t.Context(), []string{"jobs.gone"})) + assert.Contains(t, p.Plan, "resources.jobs.gone") + assert.NotContains(t, p.Plan, "resources.jobs.lib") +} diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 13c5121f569..35a2c9c0294 100644 --- a/bundle/phases/deploy.go +++ b/bundle/phases/deploy.go @@ -375,7 +375,10 @@ func RunPlan(ctx context.Context, b *bundle.Bundle) *deployplan.Plan { return nil } if len(b.Select) > 0 { - plan.FilterToSelected(b.Select) + if err := plan.FilterToSelected(ctx, b.Select); err != nil { + logdiag.LogError(ctx, err) + return nil + } } return plan } diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 902a277b018..6711d45f184 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -175,11 +175,11 @@ func Initialize(ctx context.Context, b *bundle.Bundle) { // required fields (no omitempty) still error if empty. mutator.DropEmptyStrings(), - // Resolve --select selectors against the materialized resources: normalize - // each to its "type.name" form and validate it exists. Runs after all resource - // mutations so that dynamically added resources are visible. This does not - // filter resources; the direct engine selects against the resolved keys later. - mutator.ResolveSelect(), + // Record in telemetry whether --select was used. Selectors are validated once + // state is open (process.go) and resolved against the computed plan + // (Plan.FilterToSelected), so resources removed from config and planned for + // deletion are selectable too. + mutator.MarkSelectUsed(), // Validate all required fields are set. This is run after variable interpolation and PyDABs mutators // since they can also set and modify resources. diff --git a/cmd/bundle/utils/process.go b/cmd/bundle/utils/process.go index 7e60716ad6c..e5124328305 100644 --- a/cmd/bundle/utils/process.go +++ b/cmd/bundle/utils/process.go @@ -6,6 +6,7 @@ import ( "fmt" "path" "path/filepath" + "slices" "strconv" "time" @@ -105,6 +106,23 @@ func ProcessBundle(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle, err return b, err } +// validateSelectedResources rejects a --select selector that names no resource in +// config or state. Deletion targets live only in state, so it must run after the +// state is open; ResolveSelectors collapses the config/state key overlap. +func validateSelectedResources(ctx context.Context, b *bundle.Bundle) error { + var keys []string + for _, group := range b.Config.Resources.AllResources() { + for name := range group.Resources { + keys = append(keys, "resources."+group.Description.PluralName+"."+name) + } + } + for key := range b.DeploymentBundle.ExportState(ctx) { + keys = append(keys, key) + } + _, err := deployplan.ResolveSelectors(slices.Values(keys), b.Select) + return err +} + func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle, stateDesc *statemgmt.StateDesc, retErr error) { var err error ctx := cmd.Context() @@ -458,6 +476,33 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (b *bundle.Bundle logdiag.LogError(ctx, err) return b, stateDesc, root.ErrAlreadyPrinted } + + // Warn when --select is passed alongside a plan that was either not + // pre-filtered, or was pre-filtered with different selectors. In both cases + // the current --select has no effect: the plan's resource set is fixed. + if len(b.Select) > 0 { + currentSorted := slices.Clone(b.Select) + slices.Sort(currentSorted) + planSorted := slices.Clone(plan.Selectors) + slices.Sort(planSorted) + if !plan.WasFiltered || !slices.Equal(currentSorted, planSorted) { + log.Warnf(ctx, "--select is ignored when --plan is used; re-run bundle plan --select to produce a pre-filtered plan") + } + } + } + + // Reject an unknown --select before PreDeployChecks and the predeploy script + // (and well before deploy uploads anything). Deletion targets live only in state, + // so this is the earliest point a selector can be checked. Note: for a valid + // deletion target whose live dependent makes it unsafe to delete, the predeploy + // script still runs before FilterToSelected (in RunPlan) catches the conflict; + // that case requires the plan to detect it. Skipped for a saved plan (plan != nil); + // for a fresh plan, FilterToSelected re-validates and filters it. + if (opts.Deploy || opts.PreDeployChecks) && plan == nil && len(b.Select) > 0 { + if err := validateSelectedResources(ctx, b); err != nil { + logdiag.LogError(ctx, err) + return b, stateDesc, root.ErrAlreadyPrinted + } } } From 3f151b0b8250033206c31fec676cd4c5b50a49a1 Mon Sep 17 00:00:00 2001 From: Anders Rex Date: Wed, 21 Oct 2026 10:47:44 +0300 Subject: [PATCH 2/2] Shorten select-deletions changelog and cover unqualified deletion select Co-authored-by: Isaac --- .nextchanges/bundles/select-deletions.md | 2 +- acceptance/bundle/select/deletion/output.txt | 33 ++++++++++++++++++++ acceptance/bundle/select/deletion/script | 17 ++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/.nextchanges/bundles/select-deletions.md b/.nextchanges/bundles/select-deletions.md index 8a32d564bf9..5f4bc00b924 100644 --- a/.nextchanges/bundles/select-deletions.md +++ b/.nextchanges/bundles/select-deletions.md @@ -1 +1 @@ -* `databricks bundle deploy --select` and `bundle plan --select` can now select resources removed from config, so a selective deploy applies their deletion. Deleting a selected resource also deletes its dependents that are also being removed, mirroring `bundle destroy`. Selecting a resource for deletion errors if a live resource still references it, preventing a broken bundle. An unqualified `--select ` is ambiguous when a resource of the same name exists in both config and state as different types. ([#7023](https://github.com/databricks/cli/pull/7023)) +* Support selecting resources for deletion with `bundle deploy --select`. ([#7023](https://github.com/databricks/cli/pull/7023)) diff --git a/acceptance/bundle/select/deletion/output.txt b/acceptance/bundle/select/deletion/output.txt index d39a3f3d392..a1083051d83 100644 --- a/acceptance/bundle/select/deletion/output.txt +++ b/acceptance/bundle/select/deletion/output.txt @@ -33,6 +33,39 @@ Resources: 0 created, 0 changed, 1 deleted, 0 unchanged, 1 not selected >>> [CLI] bundle plan Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged +=== Redeploy gone, then select it by unqualified name + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-del-[UNIQUE_NAME]/default/files... +Created jobs.gone +Files: 1 uploaded, 0 deleted +Resources: 1 created, 0 changed, 0 deleted, 1 unchanged + +>>> [CLI] bundle plan --select gone +delete jobs.gone + +Plan: 0 to add, 0 to change, 1 to delete, 0 unchanged, 1 not selected + +=== bundle deploy --select gone + +>>> [CLI] bundle deploy --select gone +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/select-del-[UNIQUE_NAME]/default/files... +Deleted jobs.gone +Files: 1 uploaded, 0 deleted +Resources: 0 created, 0 changed, 1 deleted, 0 unchanged, 1 not selected + +>>> print_requests.py --sort //jobs/delete +{ + "method": "POST", + "path": "/api/2.2/jobs/delete", + "body": { + "job_id": [GONE_ID_2] + } +} + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged + === Destroy >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: diff --git a/acceptance/bundle/select/deletion/script b/acceptance/bundle/select/deletion/script index cf1bb396299..c495b8dac39 100644 --- a/acceptance/bundle/select/deletion/script +++ b/acceptance/bundle/select/deletion/script @@ -29,3 +29,20 @@ trace print_requests.py --sort //jobs/delete # A full plan now has nothing to delete and 'keep' is unchanged. trace $CLI bundle plan + +# The same works with an unqualified name. Deploy 'gone' again, then remove it from +# config so it is once more orphaned in state. +title "Redeploy gone, then select it by unqualified name\n" +envsubst '$UNIQUE_NAME' < databricks.yml.tmpl > databricks.yml +trace $CLI bundle deploy +replace_ids.py +envsubst '$UNIQUE_NAME' < databricks.yml.after.tmpl > databricks.yml + +trace $CLI bundle plan --select gone + +title "bundle deploy --select gone\n" +trace $CLI bundle deploy --select gone + +trace print_requests.py --sort //jobs/delete + +trace $CLI bundle plan