diff --git a/.nextchanges/bundles/select-deletions.md b/.nextchanges/bundles/select-deletions.md new file mode 100644 index 00000000000..5f4bc00b924 --- /dev/null +++ b/.nextchanges/bundles/select-deletions.md @@ -0,0 +1 @@ +* Support selecting resources for deletion with `bundle deploy --select`. ([#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..a1083051d83 --- /dev/null +++ b/acceptance/bundle/select/deletion/output.txt @@ -0,0 +1,76 @@ + +=== 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 + +=== 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: + 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..c495b8dac39 --- /dev/null +++ b/acceptance/bundle/select/deletion/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 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 + +# 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 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 + } } }