diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 15a7a3de2cd..c145a21d840 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -18,6 +18,8 @@ * `bundle generate dashboard` now honors the `--key` flag when naming the generated resource, and rejects combining `--existing-path`, `--existing-id`, and `--resource` instead of silently ignoring all but one ([#5492](https://github.com/databricks/cli/pull/5492)). * Fixed `bundle deployment migrate` failing on `model_serving_endpoints`/`database_instances` with permissions (regression since v1.5.0) ([#5775](https://github.com/databricks/cli/pull/5775)). * After a terraform deploy, the CLI now dry-runs a migration to the direct engine (writing nothing locally or remotely) and reports the outcome via telemetry, warning if the migration could not be completed ([#5797](https://github.com/databricks/cli/pull/5797)). + * direct: Fix deploy bug when a `postgres_projects`, `postgres_branches`, or `postgres_endpoints` field is set to its zero value (e.g. `enable_pg_native_login: false`, `replace_existing: false`). + * Support `purge_on_delete: true` on `postgres_branches` so bundles can hard-delete a Lakebase branch on destroy (skipping the soft-delete retention window) ([#5801](https://github.com/databricks/cli/pull/5801)). ### Dependency updates * Bump `github.com/databricks/databricks-sdk-go` from v0.147.0 to v0.152.0 ([#5773](https://github.com/databricks/cli/pull/5773)). diff --git a/acceptance/bundle/refschema/out.fields.txt b/acceptance/bundle/refschema/out.fields.txt index 7f9cf9c94f5..6ac7405243b 100644 --- a/acceptance/bundle/refschema/out.fields.txt +++ b/acceptance/bundle/refschema/out.fields.txt @@ -2892,6 +2892,7 @@ resources.postgres_branches.*.modified_status string INPUT resources.postgres_branches.*.name string REMOTE resources.postgres_branches.*.no_expiry bool ALL resources.postgres_branches.*.parent string ALL +resources.postgres_branches.*.purge_on_delete bool INPUT STATE resources.postgres_branches.*.replace_existing bool INPUT STATE resources.postgres_branches.*.source_branch string ALL resources.postgres_branches.*.source_branch_lsn string ALL diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/databricks.yml.tmpl b/acceptance/bundle/resources/postgres_branches/purge_on_delete/databricks.yml.tmpl new file mode 100644 index 00000000000..fe20913512e --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/databricks.yml.tmpl @@ -0,0 +1,28 @@ +bundle: + name: deploy-postgres-branch-purge-$UNIQUE_NAME + +sync: + paths: [] + +# A project with two branches side by side so the recorded destroy requests +# make the purge_on_delete contrast obvious: +# - hard_delete: purge_on_delete: true -> DELETE …?purge=true +# - soft_delete: field omitted (default) -> DELETE … (no purge query) +resources: + postgres_projects: + my_project: + project_id: test-pg-proj-$UNIQUE_NAME + display_name: "Test Project for Branch purge_on_delete" + pg_version: 16 + + postgres_branches: + hard_delete: + parent: ${resources.postgres_projects.my_project.id} + branch_id: test-pg-branch-hard-$UNIQUE_NAME + no_expiry: true + purge_on_delete: true + + soft_delete: + parent: ${resources.postgres_projects.my_project.id} + branch_id: test-pg-branch-soft-$UNIQUE_NAME + no_expiry: true diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.destroy.txt b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.destroy.txt new file mode 100644 index 00000000000..90c4cff4ada --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.destroy.txt @@ -0,0 +1,18 @@ +The following resources will be deleted: + delete resources.postgres_branches.hard_delete + delete resources.postgres_branches.soft_delete + delete resources.postgres_projects.my_project + +This action will result in the deletion of the following Lakebase projects along with +all their branches, databases, and endpoints. All data stored in them will be permanently lost: + delete resources.postgres_projects.my_project + +This action will result in the deletion of the following Lakebase branches. +All data stored in them will be permanently lost: + delete resources.postgres_branches.hard_delete + delete resources.postgres_branches.soft_delete + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/deploy-postgres-branch-purge-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.deploy.direct.json b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.deploy.direct.json new file mode 100644 index 00000000000..a2d9b93874c --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.deploy.direct.json @@ -0,0 +1,45 @@ +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-hard-[UNIQUE_NAME]" +} +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-soft-[UNIQUE_NAME]" +} +{ + "method": "POST", + "path": "/api/2.0/postgres/projects", + "q": { + "project_id": "test-pg-proj-[UNIQUE_NAME]" + }, + "body": { + "spec": { + "display_name": "Test Project for Branch purge_on_delete", + "pg_version": 16 + } + } +} +{ + "method": "POST", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches", + "q": { + "branch_id": "test-pg-branch-hard-[UNIQUE_NAME]" + }, + "body": { + "spec": { + "no_expiry": true + } + } +} +{ + "method": "POST", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches", + "q": { + "branch_id": "test-pg-branch-soft-[UNIQUE_NAME]" + }, + "body": { + "spec": { + "no_expiry": true + } + } +} diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.deploy.terraform.json b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.deploy.terraform.json new file mode 100644 index 00000000000..01ff62d44c0 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.deploy.terraform.json @@ -0,0 +1,47 @@ +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-hard-[UNIQUE_NAME]" +} +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-soft-[UNIQUE_NAME]" +} +{ + "method": "POST", + "path": "/api/2.0/postgres/projects", + "q": { + "project_id": "test-pg-proj-[UNIQUE_NAME]" + }, + "body": { + "spec": { + "display_name": "Test Project for Branch purge_on_delete", + "pg_version": 16 + } + } +} +{ + "method": "POST", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches", + "q": { + "branch_id": "test-pg-branch-hard-[UNIQUE_NAME]" + }, + "body": { + "parent": "projects/test-pg-proj-[UNIQUE_NAME]", + "spec": { + "no_expiry": true + } + } +} +{ + "method": "POST", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches", + "q": { + "branch_id": "test-pg-branch-soft-[UNIQUE_NAME]" + }, + "body": { + "parent": "projects/test-pg-proj-[UNIQUE_NAME]", + "spec": { + "no_expiry": true + } + } +} diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.destroy.json b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.destroy.json new file mode 100644 index 00000000000..f4296d06190 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.requests.destroy.json @@ -0,0 +1,27 @@ +{ + "method": "DELETE", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]" +} +{ + "method": "DELETE", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-hard-[UNIQUE_NAME]", + "q": { + "purge": "true" + } +} +{ + "method": "DELETE", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-soft-[UNIQUE_NAME]" +} +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]" +} +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-hard-[UNIQUE_NAME]" +} +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-soft-[UNIQUE_NAME]" +} diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.test.toml b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.test.toml new file mode 100644 index 00000000000..110f841fa05 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/out.test.toml @@ -0,0 +1,6 @@ +Local = true +Cloud = true +RequiresUnityCatalog = true +CloudEnvs.azure = false +CloudEnvs.gcp = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct", "terraform"] diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/output.txt b/acceptance/bundle/resources/postgres_branches/purge_on_delete/output.txt new file mode 100644 index 00000000000..6e5d1339e69 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/output.txt @@ -0,0 +1,56 @@ + +>>> [CLI] bundle validate +Name: deploy-postgres-branch-purge-[UNIQUE_NAME] +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/deploy-postgres-branch-purge-[UNIQUE_NAME]/default + +Validation OK! + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-postgres-branch-purge-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] postgres get-branch projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-hard-[UNIQUE_NAME] +{ + "branch_id": "test-pg-branch-hard-[UNIQUE_NAME]", + "name": "projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-hard-[UNIQUE_NAME]", + "parent": "projects/test-pg-proj-[UNIQUE_NAME]", + "status": { + "branch_id": "test-pg-branch-hard-[UNIQUE_NAME]", + "current_state": "READY", + "default": false, + "is_protected": false, + "source_branch": "projects/test-pg-proj-[UNIQUE_NAME]/branches/production", + "source_branch_lsn": "[LSN]", + "source_branch_time": "[TIMESTAMP]", + "state_change_time": "[TIMESTAMP]" + }, + "uid": "[BRANCH_UID]" +} + +>>> [CLI] postgres get-branch projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-soft-[UNIQUE_NAME] +{ + "branch_id": "test-pg-branch-soft-[UNIQUE_NAME]", + "name": "projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-soft-[UNIQUE_NAME]", + "parent": "projects/test-pg-proj-[UNIQUE_NAME]", + "status": { + "branch_id": "test-pg-branch-soft-[UNIQUE_NAME]", + "current_state": "READY", + "default": false, + "is_protected": false, + "source_branch": "projects/test-pg-proj-[UNIQUE_NAME]/branches/production", + "source_branch_lsn": "[LSN]", + "source_branch_time": "[TIMESTAMP]", + "state_change_time": "[TIMESTAMP]" + }, + "uid": "[BRANCH_UID]" +} + +>>> print_requests.py --del-body project_id,branch_id,endpoint_id,database_id,role_id,catalog_id,synced_table_id --sort --get //postgres ^//workspace-files/ ^//workspace/ ^//telemetry-ext ^//operations/ + +=== bundle destroy +>>> print_requests.py --del-body project_id,branch_id,endpoint_id,database_id,role_id,catalog_id,synced_table_id --sort --get //postgres ^//workspace-files/ ^//workspace/ ^//telemetry-ext ^//operations/ diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete/script b/acceptance/bundle/resources/postgres_branches/purge_on_delete/script new file mode 100644 index 00000000000..cff76451f29 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete/script @@ -0,0 +1,35 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +project_name="projects/test-pg-proj-${UNIQUE_NAME}" +hard_branch="${project_name}/branches/test-pg-branch-hard-${UNIQUE_NAME}" +soft_branch="${project_name}/branches/test-pg-branch-soft-${UNIQUE_NAME}" + +cleanup() { + # Belt-and-braces in case bundle destroy was skipped or partially failed. + # The soft-delete case leaves a record in the trash; --purge clears it. + $CLI postgres delete-branch --purge "${hard_branch}" 2>>LOG.delete-branch || true + $CLI postgres delete-branch --purge "${soft_branch}" 2>>LOG.delete-branch || true + $CLI postgres delete-project --purge "${project_name}" 2>>LOG.delete-project || true + rm -f out.requests.txt +} +trap cleanup EXIT + +trace $CLI bundle validate + +rm -f out.requests.txt +trace $CLI bundle deploy + +trace $CLI postgres get-branch "${hard_branch}" | jq 'del(.create_time, .update_time, .status.logical_size_bytes)' +trace $CLI postgres get-branch "${soft_branch}" | jq 'del(.create_time, .update_time, .status.logical_size_bytes)' + +# Deploy requests are split per-engine: Terraform sends "parent" in the branch +# create body, the direct engine sends it only as a query parameter. +trace print_requests.py --del-body project_id,branch_id,endpoint_id,database_id,role_id,catalog_id,synced_table_id --sort --get '//postgres' '^//workspace-files/' '^//workspace/' '^//telemetry-ext' '^//operations/' > out.requests.deploy.$DATABRICKS_BUNDLE_ENGINE.json + +# bundle destroy should send ?purge=true on the hard_delete branch and no +# purge query on the soft_delete branch. Both DELETEs land in the recorded +# requests so the contrast is visible in the diff. +title "bundle destroy" +$CLI bundle destroy --auto-approve > out.destroy.txt 2>&1 || true + +trace print_requests.py --del-body project_id,branch_id,endpoint_id,database_id,role_id,catalog_id,synced_table_id --sort --get '//postgres' '^//workspace-files/' '^//workspace/' '^//telemetry-ext' '^//operations/' > out.requests.destroy.json diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/databricks.yml.tmpl b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/databricks.yml.tmpl new file mode 100644 index 00000000000..a8bc2b191c9 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/databricks.yml.tmpl @@ -0,0 +1,23 @@ +bundle: + name: pg-branch-purge-transitions-$UNIQUE_NAME + +sync: + paths: [] + +# Walks purge_on_delete through unset -> true -> false -> unset, deploying +# at each step and inspecting the persisted state so reviewers can see that +# state tracks the user's latest intent. The final destroy is a soft delete +# (state has purge_on_delete unset), recorded for regression coverage. +resources: + postgres_projects: + proj: + project_id: test-pg-proj-$UNIQUE_NAME + display_name: "Transitions test" + pg_version: 16 + + postgres_branches: + branch: + parent: ${resources.postgres_projects.proj.id} + branch_id: test-pg-branch-$UNIQUE_NAME + no_expiry: true + # PURGE_ON_DELETE diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.destroy.txt b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.destroy.txt new file mode 100644 index 00000000000..fe24a0edf04 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.destroy.txt @@ -0,0 +1,16 @@ +The following resources will be deleted: + delete resources.postgres_branches.branch + delete resources.postgres_projects.proj + +This action will result in the deletion of the following Lakebase projects along with +all their branches, databases, and endpoints. All data stored in them will be permanently lost: + delete resources.postgres_projects.proj + +This action will result in the deletion of the following Lakebase branches. +All data stored in them will be permanently lost: + delete resources.postgres_branches.branch + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/pg-branch-purge-transitions-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.requests.destroy.json b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.requests.destroy.json new file mode 100644 index 00000000000..ebefc7fdaa8 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.requests.destroy.json @@ -0,0 +1,16 @@ +{ + "method": "DELETE", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]" +} +{ + "method": "DELETE", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-[UNIQUE_NAME]" +} +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]" +} +{ + "method": "GET", + "path": "/api/2.0/postgres/projects/test-pg-proj-[UNIQUE_NAME]/branches/test-pg-branch-[UNIQUE_NAME]" +} diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.test.toml b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.test.toml new file mode 100644 index 00000000000..4fe23e297fe --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/out.test.toml @@ -0,0 +1,6 @@ +Local = true +Cloud = true +RequiresUnityCatalog = true +CloudEnvs.azure = false +CloudEnvs.gcp = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/output.txt b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/output.txt new file mode 100644 index 00000000000..24b99e5a9a4 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/output.txt @@ -0,0 +1,43 @@ + +=== Step 1: deploy with purge_on_delete unset +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/pg-branch-purge-transitions-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> get_purge +(unset) + +=== Step 2: set purge_on_delete: true +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/pg-branch-purge-transitions-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> get_purge +json.state.resources.postgres_branches.branch.state.purge_on_delete = true; + +=== Step 3: flip to purge_on_delete: false +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/pg-branch-purge-transitions-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> get_purge +json.state.resources.postgres_branches.branch.state.purge_on_delete = false; + +=== Step 4: remove the line again +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/pg-branch-purge-transitions-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> get_purge +json.state.resources.postgres_branches.branch.state.purge_on_delete = false; + +=== bundle destroy (branch DELETE must be a plain DELETE, no ?purge=true) +>>> print_requests.py --del-body project_id,branch_id,endpoint_id,database_id,role_id,catalog_id,synced_table_id --sort --get //postgres ^//workspace-files/ ^//workspace/ ^//telemetry-ext ^//operations/ diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/script b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/script new file mode 100644 index 00000000000..919fe6e4f4f --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/script @@ -0,0 +1,45 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +project_name="projects/test-pg-proj-${UNIQUE_NAME}" +branch_name="${project_name}/branches/test-pg-branch-${UNIQUE_NAME}" + +cleanup() { + # After this test the branch destroy should be a soft delete; --purge here + # so the trash entry doesn't linger past the test. + $CLI postgres delete-branch --purge "${branch_name}" 2>>LOG.delete-branch || true + $CLI postgres delete-project --purge "${project_name}" 2>>LOG.delete-project || true + rm -f out.requests.txt +} +trap cleanup EXIT + +# Prints the persisted purge_on_delete assignment from the direct engine's +# resources.json. The field is omitted from JSON when unset (omitempty + +# ForceSendFields tracking), so an absent value falls back to "(unset)". +get_purge() { + gron.py < .databricks/bundle/default/resources.json | grep 'postgres_branches.branch.state.purge_on_delete' || echo '(unset)' +} + +title "Step 1: deploy with purge_on_delete unset" +trace $CLI bundle deploy +trace get_purge + +title "Step 2: set purge_on_delete: true" +update_file.py databricks.yml "# PURGE_ON_DELETE" "purge_on_delete: true" +trace $CLI bundle deploy +trace get_purge + +title "Step 3: flip to purge_on_delete: false" +update_file.py databricks.yml "purge_on_delete: true" "purge_on_delete: false" +trace $CLI bundle deploy +trace get_purge + +title "Step 4: remove the line again" +update_file.py databricks.yml "purge_on_delete: false" "# PURGE_ON_DELETE" +trace $CLI bundle deploy +trace get_purge + +rm -f out.requests.txt + +title "bundle destroy (branch DELETE must be a plain DELETE, no ?purge=true)" +$CLI bundle destroy --auto-approve > out.destroy.txt 2>&1 || true +trace print_requests.py --del-body project_id,branch_id,endpoint_id,database_id,role_id,catalog_id,synced_table_id --sort --get '//postgres' '^//workspace-files/' '^//workspace/' '^//telemetry-ext' '^//operations/' > out.requests.destroy.json diff --git a/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/test.toml b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/test.toml new file mode 100644 index 00000000000..797a2d57508 --- /dev/null +++ b/acceptance/bundle/resources/postgres_branches/purge_on_delete_transitions/test.toml @@ -0,0 +1,4 @@ +# Direct engine only: this test exercises the FSF preservation in +# PrepareState and direct's plan/diff classification when the user flips +# purge_on_delete. Terraform has its own provider-managed state lifecycle. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/postgres_projects/basic/databricks.yml.tmpl b/acceptance/bundle/resources/postgres_projects/basic/databricks.yml.tmpl index 140b9116c0e..c399622a738 100644 --- a/acceptance/bundle/resources/postgres_projects/basic/databricks.yml.tmpl +++ b/acceptance/bundle/resources/postgres_projects/basic/databricks.yml.tmpl @@ -9,6 +9,7 @@ resources: my_project: project_id: test-pg-proj-$UNIQUE_NAME display_name: "Test Postgres Project" + enable_pg_native_login: false pg_version: 16 history_retention_duration: "604800s" default_endpoint_settings: diff --git a/acceptance/bundle/resources/postgres_projects/basic/out.requests.json b/acceptance/bundle/resources/postgres_projects/basic/out.requests.json index 938ae44f313..3ab697ae02a 100644 --- a/acceptance/bundle/resources/postgres_projects/basic/out.requests.json +++ b/acceptance/bundle/resources/postgres_projects/basic/out.requests.json @@ -12,6 +12,7 @@ "suspend_timeout_duration": "300s" }, "display_name": "Test Postgres Project", + "enable_pg_native_login": false, "history_retention_duration": "604800s", "pg_version": 16 } diff --git a/bundle/config/resources/postgres_branch.go b/bundle/config/resources/postgres_branch.go index a5d093ca194..9ae7f5f9200 100644 --- a/bundle/config/resources/postgres_branch.go +++ b/bundle/config/resources/postgres_branch.go @@ -24,6 +24,18 @@ type PostgresBranchConfig struct { // instead of returning ALREADY_EXISTS. Used to manage the implicitly-created // production branch of a new project. Input-only: not returned by the GET API. ReplaceExisting bool `json:"replace_existing,omitempty"` + + // PurgeOnDelete, when true, hard-deletes the branch on destroy (Purge=true on + // DeleteBranch). When false or unset, the backend performs a soft delete that + // can be undone within the branch's retention window. Input-only: not + // returned by the GET API. + PurgeOnDelete bool `json:"purge_on_delete,omitempty"` + + // ForceSendFields shadows the embedded BranchSpec.ForceSendFields so the + // SDK's marshal package tracks zero-value top-level fields (branch_id, + // parent, replace_existing, purge_on_delete) here instead of polluting + // BranchSpec.ForceSendFields with names that don't exist in that struct. + ForceSendFields []string `json:"-" url:"-"` } func (c *PostgresBranchConfig) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources_types_test.go b/bundle/config/resources_types_test.go index 5d2a7298c6f..4dd0b18d26a 100644 --- a/bundle/config/resources_types_test.go +++ b/bundle/config/resources_types_test.go @@ -1,12 +1,18 @@ package config import ( + "encoding/json" "reflect" + "slices" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/structs/structtag" ) func TestResourcesTypesMap(t *testing.T) { @@ -20,3 +26,85 @@ func TestResourcesTypesMap(t *testing.T) { assert.True(t, ok, "resources type for 'jobs.permissions' not found in ResourcesTypes map") assert.Equal(t, reflect.TypeFor[[]resources.JobPermission](), typ, "resources type for 'jobs.permissions' mismatch") } + +// TestResourceTypesZeroValueFieldsSerialize guards against the ForceSendFields +// routing bug fixed in libs/dyn/convert: a field declared in a struct embedded +// more than one level deep (e.g. PostgresProject -> PostgresProjectConfig -> +// ProjectSpec) had its zero value recorded in the wrong struct's ForceSendFields, +// which the SDK marshaler rejects with "field X cannot be found in struct Y". +// The direct engine hits this path when it serializes planned state to JSON. +// +// For every registered resource type it sets every omitempty scalar field (at any +// depth) to its zero value, converts via ToTyped, and marshals - the same round +// trip the direct engine performs. Any newly added resource whose wrapper embeds +// an SDK spec is covered automatically. +func TestResourceTypesZeroValueFieldsSerialize(t *testing.T) { + names := make([]string, 0, len(ResourcesTypes)) + for name := range ResourcesTypes { + names = append(names, name) + } + slices.Sort(names) + + for _, name := range names { + t.Run(name, func(t *testing.T) { + typ := ResourcesTypes[name] + zeros := zeroValueScalars(typ, 0, map[reflect.Type]bool{}) + if zeros.Kind() != dyn.KindMap { + return + } + + ptr := reflect.New(typ) + require.NoError(t, convert.ToTyped(ptr.Interface(), zeros)) + + _, err := json.Marshal(ptr.Interface()) + require.NoError(t, err) + }) + } +} + +// zeroValueScalars builds a [dyn.Value] map that sets every omitempty scalar field +// reachable through embedded anonymous structs to its zero value. Those are exactly +// the fields the convert layer records in ForceSendFields, so they exercise the +// routing logic. depth and seen bound recursion against deep or recursive types. +func zeroValueScalars(t reflect.Type, depth int, seen map[reflect.Type]bool) dyn.Value { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + if t.Kind() != reflect.Struct || depth > 6 || seen[t] { + return dyn.NilValue + } + seen[t] = true + defer delete(seen, t) + + m := dyn.NewMapping() + for f := range t.Fields() { + if f.Anonymous { + if sub := zeroValueScalars(f.Type, depth+1, seen); sub.Kind() == dyn.KindMap { + for _, p := range sub.MustMap().Pairs() { + m.SetLoc(p.Key.MustString(), nil, p.Value) + } + } + continue + } + + tag := structtag.JSONTag(f.Tag.Get("json")) + name := tag.Name() + if name == "" || name == "-" || !f.IsExported() || !tag.OmitEmpty() { + continue + } + + switch f.Type.Kind() { + case reflect.Bool: + m.SetLoc(name, nil, dyn.V(false)) + case reflect.String: + m.SetLoc(name, nil, dyn.V("")) + case reflect.Int, reflect.Int32, reflect.Int64: + m.SetLoc(name, nil, dyn.V(int64(0))) + case reflect.Float32, reflect.Float64: + m.SetLoc(name, nil, dyn.V(float64(0))) + default: + // Only basic types are eligible for ForceSendFields; skip the rest. + } + } + return dyn.V(m) +} diff --git a/bundle/deploy/terraform/tfdyn/convert_postgres_branch.go b/bundle/deploy/terraform/tfdyn/convert_postgres_branch.go index 12914e93074..0867c392e0c 100644 --- a/bundle/deploy/terraform/tfdyn/convert_postgres_branch.go +++ b/bundle/deploy/terraform/tfdyn/convert_postgres_branch.go @@ -15,7 +15,7 @@ func (c postgresBranchConverter) Convert(ctx context.Context, key string, vin dy // The bundle config has flattened BranchSpec fields at the top level. // Terraform expects them nested in a "spec" block. specFields := specFieldNames(schema.ResourcePostgresBranchSpec{}) - topLevelFields := []string{"branch_id", "parent", "replace_existing"} + topLevelFields := []string{"branch_id", "parent", "replace_existing", "purge_on_delete"} // Build the spec block from the flattened fields specMap := make(map[string]dyn.Value) diff --git a/bundle/deploy/terraform/tfdyn/convert_postgres_branch_test.go b/bundle/deploy/terraform/tfdyn/convert_postgres_branch_test.go index 0f8040caf19..39902579cfe 100644 --- a/bundle/deploy/terraform/tfdyn/convert_postgres_branch_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_postgres_branch_test.go @@ -78,6 +78,37 @@ func TestConvertPostgresBranchWithSourceBranch(t *testing.T) { }, postgresBranch) } +func TestConvertPostgresBranchPurgeOnDelete(t *testing.T) { + src := resources.PostgresBranch{ + PostgresBranchConfig: resources.PostgresBranchConfig{ + BranchId: "my-branch", + Parent: "projects/my-project", + PurgeOnDelete: true, + BranchSpec: postgres.BranchSpec{ + IsProtected: true, + }, + }, + } + + vin, err := convert.FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + ctx := t.Context() + out := schema.NewResources() + err = postgresBranchConverter{}.Convert(ctx, "my_postgres_branch", vin, out) + require.NoError(t, err) + + postgresBranch := out.PostgresBranch["my_postgres_branch"] + assert.Equal(t, map[string]any{ + "branch_id": "my-branch", + "parent": "projects/my-project", + "purge_on_delete": true, + "spec": map[string]any{ + "is_protected": true, + }, + }, postgresBranch) +} + func TestConvertPostgresBranchMinimal(t *testing.T) { src := resources.PostgresBranch{ PostgresBranchConfig: resources.PostgresBranchConfig{ diff --git a/bundle/direct/dresources/postgres_branch.go b/bundle/direct/dresources/postgres_branch.go index 82b50f8a10c..b033b42a8f0 100644 --- a/bundle/direct/dresources/postgres_branch.go +++ b/bundle/direct/dresources/postgres_branch.go @@ -2,6 +2,7 @@ package dresources import ( "context" + "slices" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/databricks-sdk-go" @@ -52,7 +53,9 @@ func (*ResourcePostgresBranch) PrepareState(input *resources.PostgresBranch) *Po BranchId: input.BranchId, Parent: input.Parent, ReplaceExisting: input.ReplaceExisting, + PurgeOnDelete: input.PurgeOnDelete, BranchSpec: input.BranchSpec, + ForceSendFields: input.ForceSendFields, } } @@ -65,6 +68,11 @@ func (*ResourcePostgresBranch) RemapState(remote *PostgresBranchRemote) *Postgre // it, so RemapState leaves it false. ReplaceExisting: false, + // purge_on_delete is a delete-time query parameter; the GET API never + // returns it, so RemapState leaves it false. + PurgeOnDelete: false, + ForceSendFields: nil, + BranchSpec: remote.BranchSpec, } } @@ -133,12 +141,25 @@ func (r *ResourcePostgresBranch) DoCreate(ctx context.Context, config *PostgresB } func (r *ResourcePostgresBranch) DoUpdate(ctx context.Context, id string, config *PostgresBranchState, entry *PlanEntry) (*PostgresBranchRemote, error) { - // Build update mask from fields that have action="update" in the changes map. - // This excludes immutable fields and fields that haven't changed. - // Prefix with "spec." because the API expects paths relative to the Branch object, - // not relative to our flattened state type. + // Build the mask from the plan's change list and prefix with "spec." (the + // API expects paths relative to Branch). The API rejects mask entries + // that aren't also populated in the request body, and a wildcard "*" + // expands to nested attributes the body would have to set too — so we + // can't use a static all-fields mask. The change list naturally tracks + // what the user actually set, so the body and mask stay consistent. fieldPaths := collectUpdatePathsWithPrefix(entry.Changes, "spec.") + // purge_on_delete is an input-only flag consulted at delete time; it is + // not a spec field. Strip it from the mask so toggling it between deploys + // becomes a state-only refresh (the framework saves newState when this + // returns nil error). + fieldPaths = slices.DeleteFunc(fieldPaths, func(p string) bool { + return p == "spec.purge_on_delete" + }) + if len(fieldPaths) == 0 { + return nil, nil + } + waiter, err := r.client.Postgres.UpdateBranch(ctx, postgres.UpdateBranchRequest{ Branch: postgres.Branch{ Spec: &config.BranchSpec, @@ -170,10 +191,10 @@ func (r *ResourcePostgresBranch) DoUpdate(ctx context.Context, id string, config return makePostgresBranchRemote(result), nil } -func (r *ResourcePostgresBranch) DoDelete(ctx context.Context, id string, _ *PostgresBranchState) error { +func (r *ResourcePostgresBranch) DoDelete(ctx context.Context, id string, state *PostgresBranchState) error { waiter, err := r.client.Postgres.DeleteBranch(ctx, postgres.DeleteBranchRequest{ Name: id, - Purge: false, + Purge: state.PurgeOnDelete, ForceSendFields: nil, }) if err != nil { diff --git a/bundle/direct/dresources/type_test.go b/bundle/direct/dresources/type_test.go index 3474450ed3e..8123dc9f264 100644 --- a/bundle/direct/dresources/type_test.go +++ b/bundle/direct/dresources/type_test.go @@ -37,6 +37,7 @@ var knownMissingInRemoteType = map[string][]string{ }, "postgres_branches": { "replace_existing", + "purge_on_delete", }, "postgres_endpoints": { "replace_existing", diff --git a/bundle/internal/schema/annotations.yml b/bundle/internal/schema/annotations.yml index a868bdc98f7..bdd79b2074e 100644 --- a/bundle/internal/schema/annotations.yml +++ b/bundle/internal/schema/annotations.yml @@ -1357,6 +1357,9 @@ resources: "parent": "description": |- PLACEHOLDER + "purge_on_delete": + "description": |- + PLACEHOLDER "replace_existing": "description": |- PLACEHOLDER diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index 87154f7945f..4cbce09acf4 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -1561,6 +1561,9 @@ "parent": { "$ref": "#/$defs/string" }, + "purge_on_delete": { + "$ref": "#/$defs/bool" + }, "replace_existing": { "$ref": "#/$defs/bool" }, diff --git a/bundle/terraform_dabs_map/generated.go b/bundle/terraform_dabs_map/generated.go index 5a0cf360e9d..2b0584b9962 100644 --- a/bundle/terraform_dabs_map/generated.go +++ b/bundle/terraform_dabs_map/generated.go @@ -18,7 +18,6 @@ package terraform_dabs_map // pipelines / databricks_pipeline: 3 renames // pipelines / databricks_pipeline: 5 dabs-only // pipelines / databricks_pipeline: 2 tf-only -// postgres_branches / databricks_postgres_branch: 1 tf-only // postgres_branches / databricks_postgres_branch: 1 unwraps // postgres_catalogs / databricks_postgres_catalog: 1 unwraps // postgres_databases / databricks_postgres_database: 1 tf-only @@ -562,9 +561,6 @@ var TerraformOnlyFields = map[string]FieldSet{ "expected_last_modified": {}, "url": {}, }, - "postgres_branches": { - "purge_on_delete": {}, - }, "postgres_databases": { "replace_existing": {}, }, diff --git a/libs/dyn/convert/struct_info.go b/libs/dyn/convert/struct_info.go index 7e5b0bc741e..90b76e08b38 100644 --- a/libs/dyn/convert/struct_info.go +++ b/libs/dyn/convert/struct_info.go @@ -3,6 +3,8 @@ package convert import ( "reflect" "slices" + "strconv" + "strings" "sync" "github.com/databricks/cli/libs/dyn" @@ -28,9 +30,12 @@ type structInfo struct { // Maps JSON-name of the field to Golang struct name GolangNames map[string]string - // ForceSendFieldsStructKey maps the JSON-name of the field to which ForceSendFields slice it belongs to: - // -1 for direct fields, embedded struct index for embedded fields - ForceSendFieldsStructKey map[string]int + // ForceSendFieldsStructKey maps the JSON-name of the field to the struct that + // owns the ForceSendFields slice it belongs to. The value is the index path to + // that struct (see indexPathKey); "" is the top-level struct. The path can be + // more than one element deep because a field's declaring struct may be embedded + // several levels down (e.g. PostgresProject -> PostgresProjectConfig -> ProjectSpec). + ForceSendFieldsStructKey map[string]string } // structInfoCache caches type information. @@ -61,7 +66,7 @@ func buildStructInfo(typ reflect.Type) structInfo { Fields: make(map[string][]int), ForceEmpty: make(map[string]bool), GolangNames: make(map[string]string), - ForceSendFieldsStructKey: make(map[string]int), + ForceSendFieldsStructKey: make(map[string]string), } // Queue holds the indexes of the structs to visit. @@ -119,14 +124,9 @@ func buildStructInfo(typ reflect.Type) structInfo { } out.GolangNames[name] = sf.Name - // Determine which ForceSendFields this field belongs to - if len(prefix) == 0 { - // Direct field on the main struct - out.ForceSendFieldsStructKey[name] = -1 - } else { - // Field on embedded struct - out.ForceSendFieldsStructKey[name] = prefix[0] - } + // The field is declared directly in the struct reached by prefix, so + // its ForceSendFields lives there. prefix is empty for the top-level struct. + out.ForceSendFieldsStructKey[name] = indexPathKey(prefix) } } @@ -192,33 +192,45 @@ func (s *structInfo) FieldValues(v reflect.Value) []FieldValue { // Type of [dyn.Value]. var configValueType = reflect.TypeFor[dyn.Value]() -// getForceSendFieldsValues collects ForceSendFields reflect.Values -// Returns map[structKey]reflect.Value where structKey is -1 for direct fields, embedded index for embedded fields -func getForceSendFieldsValues(v reflect.Value) map[int]reflect.Value { - if !v.IsValid() || v.Type().Kind() != reflect.Struct { - return make(map[int]reflect.Value) - } +// getForceSendFieldsValues collects the ForceSendFields slice declared directly +// by the top-level struct and by every embedded struct reachable through anonymous +// fields, at any depth. The result is keyed by the index path to each owning struct +// (see indexPathKey), matching structInfo.ForceSendFieldsStructKey. Embedding can be +// arbitrarily deep (e.g. PostgresProject -> PostgresProjectConfig -> ProjectSpec), +// so each level is recorded under its own path rather than collapsed to one index. +func getForceSendFieldsValues(v reflect.Value) map[string]reflect.Value { + result := make(map[string]reflect.Value) + collectForceSendFieldsValues(v, nil, result) + return result +} - result := make(map[int]reflect.Value) +func collectForceSendFieldsValues(v reflect.Value, path []int, result map[string]reflect.Value) { + v = deref(v) + if !v.IsValid() || v.Kind() != reflect.Struct { + return + } for i := range v.Type().NumField() { field := v.Type().Field(i) fieldValue := v.Field(i) - if field.Name == "ForceSendFields" && !field.Anonymous { - // Direct ForceSendFields (structKey = -1) - result[-1] = fieldValue - } else if field.Anonymous { - // Embedded struct - check for ForceSendFields inside it - if embeddedStruct := deref(fieldValue); embeddedStruct.IsValid() { - if forceSendField := embeddedStruct.FieldByName("ForceSendFields"); forceSendField.IsValid() { - result[i] = forceSendField - } - } + switch { + case field.Name == "ForceSendFields" && !field.Anonymous: + result[indexPathKey(path)] = fieldValue + case field.Anonymous: + collectForceSendFieldsValues(fieldValue, append(path, i), result) } } +} - return result +// indexPathKey renders a reflect index path as a stable map key. The empty path +// (the top-level struct) renders as "". +func indexPathKey(path []int) string { + parts := make([]string, len(path)) + for i, x := range path { + parts[i] = strconv.Itoa(x) + } + return strings.Join(parts, ".") } // deref dereferences a pointer, returning invalid value if nil diff --git a/libs/dyn/convert/to_typed.go b/libs/dyn/convert/to_typed.go index 6fb63d1f0ba..491e325584f 100644 --- a/libs/dyn/convert/to_typed.go +++ b/libs/dyn/convert/to_typed.go @@ -76,7 +76,7 @@ func toTypedStruct(dst reflect.Value, src dyn.Value) error { info := getStructInfo(dst.Type()) forceSendFieldLocations := getForceSendFieldsValues(dst) - forceSendFieldsMap := make(map[int][]string) + forceSendFieldsMap := make(map[string][]string) for _, pair := range src.MustMap().Pairs() { pk := pair.Key @@ -111,14 +111,9 @@ func toTypedStruct(dst reflect.Value, src dyn.Value) error { } if pv.IsZero() { - // Use first index as key: -1 for direct fields, struct index for embedded fields - var structKey int - if len(index) == 1 { - structKey = -1 // Direct field - } else { - structKey = index[0] // Embedded struct index - } - + // The field's zero value must still serialize, so record its Go name + // in the ForceSendFields of the struct that declares it. + structKey := info.ForceSendFieldsStructKey[jsonKey] forceSendFieldsMap[structKey] = append(forceSendFieldsMap[structKey], info.GolangNames[jsonKey]) } } diff --git a/libs/dyn/convert/to_typed_test.go b/libs/dyn/convert/to_typed_test.go index 7b95d17056d..fd8213c16b1 100644 --- a/libs/dyn/convert/to_typed_test.go +++ b/libs/dyn/convert/to_typed_test.go @@ -754,3 +754,37 @@ func TestToTypedFieldByNameBugRegressionTest(t *testing.T) { assert.Equal(t, "test-job", out.Name) assert.Empty(t, out.Permissions) } + +func TestToTypedDeeplyEmbeddedStructForceSendFields(t *testing.T) { + // Mirrors resources.PostgresProject -> PostgresProjectConfig -> ProjectSpec: + // Spec is embedded two levels down and Wrapper shadows ForceSendFields to keep + // its own direct field out of Spec's ForceSendFields. A zero-value spec field + // must route to Spec.ForceSendFields, not Wrapper's, otherwise the SDK marshaler + // fails with "field ... cannot be found in struct". + type Spec struct { + SpecField bool `json:"spec_field,omitempty"` + ForceSendFields []string `json:"-"` + } + + type Wrapper struct { + Spec + WrapperField string `json:"wrapper_field,omitempty"` + ForceSendFields []string `json:"-"` + } + + type Outer struct { + Wrapper + } + + var out Outer + m := dyn.Mapping{} + m.SetLoc("spec_field", nil, dyn.V(false)) + m.SetLoc("wrapper_field", nil, dyn.V("")) + v := dyn.V(m) + + err := ToTyped(&out, v) + require.NoError(t, err) + + assert.Equal(t, []string{"SpecField"}, out.Spec.ForceSendFields) + assert.Equal(t, []string{"WrapperField"}, out.ForceSendFields) +} diff --git a/libs/testserver/postgres.go b/libs/testserver/postgres.go index 83b50213a30..7d34e806c98 100644 --- a/libs/testserver/postgres.go +++ b/libs/testserver/postgres.go @@ -452,7 +452,10 @@ func (s *FakeWorkspace) PostgresBranchUpdate(req Request, name string) Response } } -// PostgresBranchDelete deletes a postgres branch. +// PostgresBranchDelete deletes a postgres branch. The `purge` query parameter +// is ignored: acceptance tests assert on the recorded HTTP request rather than +// on retention semantics, so a single "remove from map" action serves both +// hard- and soft-delete paths. func (s *FakeWorkspace) PostgresBranchDelete(name string) Response { defer s.LockUnlock()()