From a3eacfa48bb2dbcfe1b563bad04fcf4950fd9f22 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 19 Aug 2026 09:46:29 +0000 Subject: [PATCH 1/6] Remove the empty .bundle/ directory on destroy `bundle destroy` deleted `~/.bundle//` but never the `` parent, so every destroy left an empty directory behind. Deployments that use a fresh bundle name each time never reuse those directories, so they accumulate and count against the workspace child-node limit; the CLI's own CI workspace reached ~27k of them per day. Delete the parent too, non-recursively so it survives while another target of the bundle is still deployed there. The delete is restricted to the `~/.bundle//` layout the CLI generates, since `root_path` is user-configurable and the parent of an arbitrary path is not ours to remove. The fake workspace accepted a non-recursive delete of a non-empty directory, which the real API rejects with DIRECTORY_NOT_EMPTY. Model that, so the sibling-target case is covered by the local suite. Co-authored-by: Isaac --- .../destroy-removes-bundle-directory.md | 1 + .../bundle/destroy/all-resources/output.txt | 6 ++++ .../bundle/destroy/all-resources/script | 5 +++ .../destroy/sibling-target/databricks.yml | 7 ++++ .../destroy/sibling-target/out.test.toml | 2 ++ .../bundle/destroy/sibling-target/output.txt | 35 +++++++++++++++++++ .../bundle/destroy/sibling-target/script | 17 +++++++++ .../resource_deps/remote_app_url/output.txt | 7 ++++ .../simple/out.requests.destroy.direct.json | 12 +++++++ .../out.requests.destroy.terraform.json | 12 +++++++ bundle/deploy/files/delete.go | 26 ++++++++++++++ libs/testserver/fake_workspace.go | 32 ++++++++++++++++- libs/testserver/handlers.go | 3 +- libs/testserver/workspace_test.go | 28 +++++++++++++++ 14 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 .nextchanges/bundles/destroy-removes-bundle-directory.md create mode 100644 acceptance/bundle/destroy/sibling-target/databricks.yml create mode 100644 acceptance/bundle/destroy/sibling-target/out.test.toml create mode 100644 acceptance/bundle/destroy/sibling-target/output.txt create mode 100644 acceptance/bundle/destroy/sibling-target/script diff --git a/.nextchanges/bundles/destroy-removes-bundle-directory.md b/.nextchanges/bundles/destroy-removes-bundle-directory.md new file mode 100644 index 00000000000..fca3bc5db04 --- /dev/null +++ b/.nextchanges/bundles/destroy-removes-bundle-directory.md @@ -0,0 +1 @@ +`bundle destroy` now also removes the `~/.bundle/` directory the deployment lived under, instead of only `~/.bundle//`. The directory is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit. diff --git a/acceptance/bundle/destroy/all-resources/output.txt b/acceptance/bundle/destroy/all-resources/output.txt index f8eb4d90e0d..207a85f8c8b 100644 --- a/acceptance/bundle/destroy/all-resources/output.txt +++ b/acceptance/bundle/destroy/all-resources/output.txt @@ -21,3 +21,9 @@ Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascad All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default Destroy: 2 deleted + +=== Assert the bundle directory is deleted +>>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle) doesn't exist. + +Exit code: 1 diff --git a/acceptance/bundle/destroy/all-resources/script b/acceptance/bundle/destroy/all-resources/script index ceb8acee91a..71160f7c3f7 100644 --- a/acceptance/bundle/destroy/all-resources/script +++ b/acceptance/bundle/destroy/all-resources/script @@ -1,2 +1,7 @@ trace $CLI bundle deploy trace $CLI bundle destroy --auto-approve + +# Destroy removes the target directory, and the .bundle/ parent goes with it once +# nothing else is deployed under it. Double slash keeps Windows from rewriting the path. +title "Assert the bundle directory is deleted" +trace errcode $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" diff --git a/acceptance/bundle/destroy/sibling-target/databricks.yml b/acceptance/bundle/destroy/sibling-target/databricks.yml new file mode 100644 index 00000000000..a12a0457944 --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/databricks.yml @@ -0,0 +1,7 @@ +bundle: + name: test-bundle + +targets: + dev: + default: true + prod: diff --git a/acceptance/bundle/destroy/sibling-target/out.test.toml b/acceptance/bundle/destroy/sibling-target/out.test.toml new file mode 100644 index 00000000000..98ea5040486 --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/destroy/sibling-target/output.txt b/acceptance/bundle/destroy/sibling-target/output.txt new file mode 100644 index 00000000000..48cbff888ce --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/output.txt @@ -0,0 +1,35 @@ + +>>> [CLI] bundle deploy -t dev +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/dev/files... +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 0 unchanged + +>>> [CLI] bundle deploy -t prod +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/prod/files... +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 0 unchanged + +=== Destroy one target while the other is still deployed +>>> [CLI] bundle destroy -t dev --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/dev + +Destroy: 0 deleted + +=== Assert the bundle directory is kept for the remaining target +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +{ + "path": "/Users/[USERNAME]/.bundle/test-bundle", + "object_type": "DIRECTORY" +} + +=== Destroy the remaining target +>>> [CLI] bundle destroy -t prod --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/prod + +Destroy: 0 deleted + +=== Assert the bundle directory is deleted +>>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle) doesn't exist. + +Exit code: 1 diff --git a/acceptance/bundle/destroy/sibling-target/script b/acceptance/bundle/destroy/sibling-target/script new file mode 100644 index 00000000000..1f2db9fec6c --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/script @@ -0,0 +1,17 @@ +# Double slash keeps Windows from rewriting the path. +BUNDLE_DIR="//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" + +trace $CLI bundle deploy -t dev +trace $CLI bundle deploy -t prod + +title "Destroy one target while the other is still deployed" +trace $CLI bundle destroy -t dev --auto-approve + +title "Assert the bundle directory is kept for the remaining target" +trace $CLI workspace get-status "${BUNDLE_DIR}" | jq '{path, object_type}' + +title "Destroy the remaining target" +trace $CLI bundle destroy -t prod --auto-approve + +title "Assert the bundle directory is deleted" +trace errcode $CLI workspace get-status "${BUNDLE_DIR}" diff --git a/acceptance/bundle/resource_deps/remote_app_url/output.txt b/acceptance/bundle/resource_deps/remote_app_url/output.txt index 84d8da92e35..9301329af12 100644 --- a/acceptance/bundle/resource_deps/remote_app_url/output.txt +++ b/acceptance/bundle/resource_deps/remote_app_url/output.txt @@ -107,6 +107,13 @@ Destroy: 2 deleted "method": "DELETE", "path": "/api/2.0/pipelines/[UUID]" } +{ + "method": "POST", + "path": "/api/2.0/workspace/delete", + "body": { + "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle" + } +} { "method": "POST", "path": "/api/2.0/workspace/delete", diff --git a/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json b/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json index c67f22e3730..1e97537e4d6 100644 --- a/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json +++ b/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json @@ -132,6 +132,18 @@ "User": "[USERNAME]" } } +{ + "headers": { + "User-Agent": [ + "cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_destroy cmd-exec-id/[UUID] interactive/none engine/direct auth/pat" + ] + }, + "method": "POST", + "path": "/api/2.0/workspace/delete", + "body": { + "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle" + } +} { "headers": { "User-Agent": [ diff --git a/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json b/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json index 09b923be4dd..325ff5b60aa 100644 --- a/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json +++ b/acceptance/bundle/user_agent/simple/out.requests.destroy.terraform.json @@ -111,6 +111,18 @@ "User": "[USERNAME]" } } +{ + "headers": { + "User-Agent": [ + "cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_destroy cmd-exec-id/[UUID] interactive/none engine/terraform auth/pat" + ] + }, + "method": "POST", + "path": "/api/2.0/workspace/delete", + "body": { + "path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle" + } +} { "headers": { "User-Agent": [ diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index 562437e5432..7f3502cd061 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -6,13 +6,17 @@ import ( "fmt" "io/fs" "os" + "path" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/sync" "github.com/databricks/databricks-sdk-go/service/workspace" ) +const bundleDirName = ".bundle" + type delete struct{} func (m *delete) Name() string { @@ -28,6 +32,8 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return diag.FromErr(err) } + removeEmptyBundleDir(ctx, b) + // Clean up sync snapshot file err = deleteSnapshotFile(ctx, b) if err != nil { @@ -36,6 +42,26 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return nil } +// removeEmptyBundleDir removes the ~/.bundle/ directory left behind once the +// target subdirectory under it is gone. The delete is not recursive, so it only +// succeeds while no other target of the bundle is still deployed there; both a +// remaining sibling target and an already-removed directory surface as an error that +// is expected and ignored. +func removeEmptyBundleDir(ctx context.Context, b *bundle.Bundle) { + dir := path.Dir(b.Config.Workspace.RootPath) + + // root_path is user-configurable, so only clean up the layout the CLI generates + // (~/.bundle//) instead of deleting the parent of an arbitrary path. + if path.Base(path.Dir(dir)) != bundleDirName { + return + } + + err := b.WorkspaceClient(ctx).Workspace.Delete(ctx, workspace.Delete{Path: dir}) + if err != nil { + log.Debugf(ctx, "Leaving %s in place: %s", dir, err) + } +} + func deleteSnapshotFile(ctx context.Context, b *bundle.Bundle) error { opts, err := GetSyncOptions(ctx, b) if err != nil { diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 1caec84d1e4..7625c4d8546 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -698,9 +698,21 @@ func (s *FakeWorkspace) WorkspaceExport(path string) []byte { return s.files[path].Data } -func (s *FakeWorkspace) WorkspaceDelete(path string, recursive bool) { +// WorkspaceDelete implements POST /api/2.0/workspace/delete. As in the real API, a +// non-recursive delete of a directory that still has children fails instead of removing +// it, which is what lets a caller delete a directory only if it is empty. +func (s *FakeWorkspace) WorkspaceDelete(path string, recursive bool) Response { defer s.LockUnlock()() if !recursive { + if _, isDir := s.directories[path]; isDir && s.hasChildren(path) { + return Response{ + StatusCode: 400, + Body: map[string]string{ + "error_code": "DIRECTORY_NOT_EMPTY", + "message": "Directory " + path + " is not empty", + }, + } + } delete(s.files, path) delete(s.directories, path) } else { @@ -715,6 +727,24 @@ func (s *FakeWorkspace) WorkspaceDelete(path string, recursive bool) { } } } + return Response{} +} + +// hasChildren reports whether any file or directory lives under dirPath. Callers must +// hold the lock. +func (s *FakeWorkspace) hasChildren(dirPath string) bool { + prefix := dirPath + "/" + for key := range s.files { + if strings.HasPrefix(key, prefix) { + return true + } + } + for key := range s.directories { + if strings.HasPrefix(key, prefix) { + return true + } + } + return false } func (s *FakeWorkspace) WorkspaceFilesImportFile(filePath string, body []byte, overwrite bool) Response { diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index 099230ad94e..1cc037dcb6b 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -143,8 +143,7 @@ func AddDefaultHandlers(server *Server) { StatusCode: 500, } } - req.Workspace.WorkspaceDelete(request.Path, request.Recursive) - return "" + return req.Workspace.WorkspaceDelete(request.Path, request.Recursive) }) server.Handle("POST", "/api/2.0/workspace-files/import-file/{path...}", func(req Request) any { diff --git a/libs/testserver/workspace_test.go b/libs/testserver/workspace_test.go index 5e4753246cf..e8d8c4a12a8 100644 --- a/libs/testserver/workspace_test.go +++ b/libs/testserver/workspace_test.go @@ -31,6 +31,18 @@ func mkdirs(t *testing.T, baseURL, path string) { require.Equal(t, 200, resp.StatusCode) } +func workspaceDelete(t *testing.T, baseURL, path string, recursive bool) int { + t.Helper() + body, err := json.Marshal(map[string]any{"path": path, "recursive": recursive}) + require.NoError(t, err) + req, _ := http.NewRequest(http.MethodPost, baseURL+"/api/2.0/workspace/delete", strings.NewReader(string(body))) + req.Header.Set("Authorization", "Bearer test-token") + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + return resp.StatusCode +} + func getStatus(t *testing.T, baseURL, path string) int { t.Helper() req, _ := http.NewRequest(http.MethodGet, baseURL+"/api/2.0/workspace/get-status?path="+path, nil) @@ -53,6 +65,22 @@ func TestWorkspaceImportRejectsMissingParent(t *testing.T) { assert.Equal(t, 200, importFile(t, server.URL, "/test-dir/file.py", "content")) } +// A non-recursive delete only removes an empty directory, so a caller can use it to +// clean up a parent directory without touching one that still holds a sibling. +func TestWorkspaceDeleteNonRecursiveRequiresEmptyDirectory(t *testing.T) { + server := testserver.New(t) + testserver.AddDefaultHandlers(server) + + mkdirs(t, server.URL, "/a/b/c") + + assert.Equal(t, 400, workspaceDelete(t, server.URL, "/a/b", false)) + assert.Equal(t, 200, getStatus(t, server.URL, "/a/b")) + + assert.Equal(t, 200, workspaceDelete(t, server.URL, "/a/b/c", false)) + assert.Equal(t, 200, workspaceDelete(t, server.URL, "/a/b", false)) + assert.Equal(t, 404, getStatus(t, server.URL, "/a/b")) +} + // mkdirs creates all intermediate directories, matching "mkdir -p". func TestWorkspaceMkdirsRecursive(t *testing.T) { server := testserver.New(t) From ccf0ff7510f124e6327532b9ebf1d02803aaa725 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 19 Aug 2026 09:55:09 +0000 Subject: [PATCH 2/6] Link the changelog fragment to the PR Co-authored-by: Isaac --- .nextchanges/bundles/destroy-removes-bundle-directory.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.nextchanges/bundles/destroy-removes-bundle-directory.md b/.nextchanges/bundles/destroy-removes-bundle-directory.md index fca3bc5db04..5bbba58514e 100644 --- a/.nextchanges/bundles/destroy-removes-bundle-directory.md +++ b/.nextchanges/bundles/destroy-removes-bundle-directory.md @@ -1 +1 @@ -`bundle destroy` now also removes the `~/.bundle/` directory the deployment lived under, instead of only `~/.bundle//`. The directory is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit. +`bundle destroy` now also removes the `~/.bundle/` directory the deployment lived under, instead of only `~/.bundle//`. The directory is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit ([#6317](https://github.com/databricks/cli/pull/6317)). From a9106f67683a5080e040eec8cdd8ed5f196b402a Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 24 Aug 2026 09:01:17 +0000 Subject: [PATCH 3/6] Gate the bundle-directory cleanup on a name/target scoped root_path The first version keyed off the path shape (a parent directly under .bundle), which both missed a root_path pointing elsewhere and could not tell a configured path from the generated one. Record instead, in DefineDefaultWorkspaceRoot, whether root_path ends in the bundle name and target. That mutator runs before variable resolution, so ${bundle.name} and ${bundle.target} are still literal there and a path that merely happens to end in the same two segments does not qualify. Applying the default sets the flag too, since the default has exactly that shape. Destroy removes the directory above the deployment root only when the flag is set, so it works for any prefix and never touches a directory the configuration did not scope to this bundle. Co-authored-by: Isaac --- .../destroy-removes-bundle-directory.md | 2 +- .../root-path-name-target/databricks.yml | 5 +++ .../root-path-name-target/out.test.toml | 2 ++ .../destroy/root-path-name-target/output.txt | 23 ++++++++++++ .../destroy/root-path-name-target/script | 10 ++++++ .../root-path-not-scoped/databricks.yml | 5 +++ .../root-path-not-scoped/out.test.toml | 2 ++ .../destroy/root-path-not-scoped/output.txt | 17 +++++++++ .../destroy/root-path-not-scoped/script | 7 ++++ acceptance/bundle/user_agent/output.txt | 2 ++ bundle/bundle.go | 7 ++++ .../config/mutator/default_workspace_root.go | 18 ++++++++++ .../mutator/default_workspace_root_test.go | 35 +++++++++++++++++++ bundle/deploy/files/delete.go | 23 +++++------- libs/testserver/workspace_test.go | 13 +++++-- 15 files changed, 153 insertions(+), 18 deletions(-) create mode 100644 acceptance/bundle/destroy/root-path-name-target/databricks.yml create mode 100644 acceptance/bundle/destroy/root-path-name-target/out.test.toml create mode 100644 acceptance/bundle/destroy/root-path-name-target/output.txt create mode 100644 acceptance/bundle/destroy/root-path-name-target/script create mode 100644 acceptance/bundle/destroy/root-path-not-scoped/databricks.yml create mode 100644 acceptance/bundle/destroy/root-path-not-scoped/out.test.toml create mode 100644 acceptance/bundle/destroy/root-path-not-scoped/output.txt create mode 100644 acceptance/bundle/destroy/root-path-not-scoped/script diff --git a/.nextchanges/bundles/destroy-removes-bundle-directory.md b/.nextchanges/bundles/destroy-removes-bundle-directory.md index 5bbba58514e..db057020a4b 100644 --- a/.nextchanges/bundles/destroy-removes-bundle-directory.md +++ b/.nextchanges/bundles/destroy-removes-bundle-directory.md @@ -1 +1 @@ -`bundle destroy` now also removes the `~/.bundle/` directory the deployment lived under, instead of only `~/.bundle//`. The directory is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit ([#6317](https://github.com/databricks/cli/pull/6317)). +`bundle destroy` now also removes the directory named after the bundle, not just the target directory beneath it, when `workspace.root_path` ends in `${bundle.name}/${bundle.target}` — which includes the default root path. It is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit ([#6317](https://github.com/databricks/cli/pull/6317)). diff --git a/acceptance/bundle/destroy/root-path-name-target/databricks.yml b/acceptance/bundle/destroy/root-path-name-target/databricks.yml new file mode 100644 index 00000000000..05d25995653 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/databricks.yml @@ -0,0 +1,5 @@ +bundle: + name: test-bundle + +workspace: + root_path: ~/.bundle/explicit/${bundle.name}/${bundle.target} diff --git a/acceptance/bundle/destroy/root-path-name-target/out.test.toml b/acceptance/bundle/destroy/root-path-name-target/out.test.toml new file mode 100644 index 00000000000..98ea5040486 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/destroy/root-path-name-target/output.txt b/acceptance/bundle/destroy/root-path-name-target/output.txt new file mode 100644 index 00000000000..98390afdd52 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/output.txt @@ -0,0 +1,23 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle/default/files... +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 0 unchanged + +>>> [CLI] bundle destroy --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle/default + +Destroy: 0 deleted + +=== Assert the bundle directory is deleted +>>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle +Error: Path (//Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle) doesn't exist. + +Exit code: 1 + +=== Assert the prefix above it is kept +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit +{ + "path": "/Users/[USERNAME]/.bundle/explicit", + "object_type": "DIRECTORY" +} diff --git a/acceptance/bundle/destroy/root-path-name-target/script b/acceptance/bundle/destroy/root-path-name-target/script new file mode 100644 index 00000000000..73a6b65db44 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/script @@ -0,0 +1,10 @@ +trace $CLI bundle deploy +trace $CLI bundle destroy --auto-approve + +# root_path spells out ${bundle.name}/${bundle.target}, so the directory named after the +# bundle goes with the deployment even though it is not under the default location. +title "Assert the bundle directory is deleted" +trace errcode $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/explicit/test-bundle" + +title "Assert the prefix above it is kept" +trace $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/explicit" | jq '{path, object_type}' diff --git a/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml b/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml new file mode 100644 index 00000000000..e480d162dda --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml @@ -0,0 +1,5 @@ +bundle: + name: test-bundle + +workspace: + root_path: ~/.bundle/custom-root/inner diff --git a/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml b/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml new file mode 100644 index 00000000000..98ea5040486 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/destroy/root-path-not-scoped/output.txt b/acceptance/bundle/destroy/root-path-not-scoped/output.txt new file mode 100644 index 00000000000..f535c80b5a5 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/output.txt @@ -0,0 +1,17 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/custom-root/inner/files... +Files: 3 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 0 unchanged + +>>> [CLI] bundle destroy --auto-approve +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/custom-root/inner + +Destroy: 0 deleted + +=== Assert the parent directory is kept +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/custom-root +{ + "path": "/Users/[USERNAME]/.bundle/custom-root", + "object_type": "DIRECTORY" +} diff --git a/acceptance/bundle/destroy/root-path-not-scoped/script b/acceptance/bundle/destroy/root-path-not-scoped/script new file mode 100644 index 00000000000..3817c6b46f1 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/script @@ -0,0 +1,7 @@ +trace $CLI bundle deploy +trace $CLI bundle destroy --auto-approve + +# root_path does not end in ${bundle.name}/${bundle.target}, so the directory above it +# is not this bundle's and is left alone. Double slash keeps Windows from rewriting it. +title "Assert the parent directory is kept" +trace $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/custom-root" | jq '{path, object_type}' diff --git a/acceptance/bundle/user_agent/output.txt b/acceptance/bundle/user_agent/output.txt index 2107e7e2b1f..809eb5b1de1 100644 --- a/acceptance/bundle/user_agent/output.txt +++ b/acceptance/bundle/user_agent/output.txt @@ -59,6 +59,7 @@ OK destroy.direct /api/2.0/workspace/get-status engine/direct OK destroy.direct /api/2.1/unity-catalog/schemas/mycatalog.myschema engine/direct OK destroy.direct /api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/deploy.lock engine/direct OK destroy.direct /api/2.0/workspace/delete engine/direct +OK destroy.direct /api/2.0/workspace/delete engine/direct MISS destroy.direct /.well-known/databricks-config 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS]' MISS destroy.terraform /api/2.0/preview/scim/v2/Me 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_destroy cmd-exec-id/[UUID] interactive/none auth/pat' MISS destroy.terraform /api/2.0/workspace-files/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/terraform.tfstate 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/bundle_destroy cmd-exec-id/[UUID] interactive/none auth/pat' @@ -70,6 +71,7 @@ OK destroy.terraform /api/2.0/workspace/get-status engine/terraform OK destroy.terraform /api/2.0/workspace/get-status engine/terraform OK destroy.terraform /api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/deploy.lock engine/terraform OK destroy.terraform /api/2.0/workspace/delete engine/terraform +OK destroy.terraform /api/2.0/workspace/delete engine/terraform MISS destroy.terraform /.well-known/databricks-config 'cli/[CLI_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS]' MISS destroy.terraform /api/2.1/unity-catalog/schemas/mycatalog.myschema 'databricks-tf-provider/[TF_PROVIDER_VERSION] databricks-sdk-go/[SDK_VERSION] go/1.25.8 os/[OS] cli/[CLI_VERSION] terraform/1.5.5 sdk/sdkv2 resource/schema auth/pat' MISS destroy.terraform /api/2.1/unity-catalog/current-metastore-assignment 'databricks-tf-provider/[TF_PROVIDER_VERSION] databricks-sdk-go/[SDK_VERSION] go/1.25.8 os/[OS] cli/[CLI_VERSION] terraform/1.5.5 sdk/sdkv2 resource/schema auth/pat' diff --git a/bundle/bundle.go b/bundle/bundle.go index bcceb752088..bca44c53842 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -157,6 +157,13 @@ type Bundle struct { // Target stores a snapshot of the Root.Bundle.Target configuration when it was selected by SelectTarget. Target *config.Target `json:"target_config,omitempty" bundle:"internal"` + // RootPathIsNameTargetScoped reports whether workspace.root_path ends in the bundle + // name and target. It is recorded before variable resolution, while ${bundle.name} + // and ${bundle.target} are still literal, so a path that only happens to end in + // those two segments does not count. Destroy uses it to tell whether the directory + // above the root path belongs to this bundle alone. + RootPathIsNameTargetScoped bool + // Metadata about the bundle deployment. This is the interface Databricks services // rely on to integrate with bundles when they need additional information about // a bundle deployment. diff --git a/bundle/config/mutator/default_workspace_root.go b/bundle/config/mutator/default_workspace_root.go index d7c24a5b557..0c6ee14277f 100644 --- a/bundle/config/mutator/default_workspace_root.go +++ b/bundle/config/mutator/default_workspace_root.go @@ -3,11 +3,20 @@ package mutator import ( "context" "fmt" + "path" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" ) +// How the bundle name and target appear in a configured root_path, which this mutator +// sees before variable resolution replaces them. +const ( + bundleNameRef = "${bundle.name}" + bundleTargetRef = "${bundle.target}" +) + type defineDefaultWorkspaceRoot struct{} // DefineDefaultWorkspaceRoot defines the default workspace root path. @@ -21,6 +30,7 @@ func (m *defineDefaultWorkspaceRoot) Name() string { func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { if b.Config.Workspace.RootPath != "" { + b.RootPathIsNameTargetScoped = endsWithNameAndTarget(b.Config.Workspace.RootPath) return nil } @@ -37,5 +47,13 @@ func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle b.Config.Bundle.Name, b.Config.Bundle.Target, ) + b.RootPathIsNameTargetScoped = true return nil } + +// endsWithNameAndTarget reports whether the last two segments of rootPath are the +// bundle name and target references. +func endsWithNameAndTarget(rootPath string) bool { + rootPath = strings.TrimSuffix(rootPath, "/") + return path.Base(rootPath) == bundleTargetRef && path.Base(path.Dir(rootPath)) == bundleNameRef +} diff --git a/bundle/config/mutator/default_workspace_root_test.go b/bundle/config/mutator/default_workspace_root_test.go index ad5dac4e42c..0a26c7ad596 100644 --- a/bundle/config/mutator/default_workspace_root_test.go +++ b/bundle/config/mutator/default_workspace_root_test.go @@ -24,3 +24,38 @@ func TestDefaultWorkspaceRoot(t *testing.T) { assert.Equal(t, "~/.bundle/name/environment", b.Config.Workspace.RootPath) } + +func TestDefaultWorkspaceRootIsNameTargetScoped(t *testing.T) { + tcases := []struct { + name string + rootPath string + scoped bool + }{ + {"defaulted", "", true}, + {"name and target references", "~/.bundle/${bundle.name}/${bundle.target}", true}, + {"references under another prefix", "/Workspace/Shared/${bundle.name}/${bundle.target}", true}, + {"trailing slash", "~/.bundle/${bundle.name}/${bundle.target}/", true}, + // Already-resolved segments are indistinguishable from a literal path that + // happens to match, so they do not count. + {"resolved values", "~/.bundle/name/environment", false}, + {"target only", "~/.bundle/${bundle.target}", false}, + {"name only", "~/.bundle/${bundle.name}", false}, + {"extra segment below", "~/.bundle/${bundle.name}/${bundle.target}/inner", false}, + {"unrelated path", "/Workspace/Shared/some/path", false}, + } + + for _, tc := range tcases { + t.Run(tc.name, func(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Bundle: config.Bundle{Name: "name", Target: "environment"}, + Workspace: config.Workspace{RootPath: tc.rootPath}, + }, + } + diags := bundle.Apply(t.Context(), b, mutator.DefineDefaultWorkspaceRoot()) + require.NoError(t, diags.Error()) + + assert.Equal(t, tc.scoped, b.RootPathIsNameTargetScoped) + }) + } +} diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index 7f3502cd061..a2e120a7c6a 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -15,8 +15,6 @@ import ( "github.com/databricks/databricks-sdk-go/service/workspace" ) -const bundleDirName = ".bundle" - type delete struct{} func (m *delete) Name() string { @@ -32,7 +30,7 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return diag.FromErr(err) } - removeEmptyBundleDir(ctx, b) + removeBundleNameDir(ctx, b) // Clean up sync snapshot file err = deleteSnapshotFile(ctx, b) @@ -42,20 +40,17 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return nil } -// removeEmptyBundleDir removes the ~/.bundle/ directory left behind once the -// target subdirectory under it is gone. The delete is not recursive, so it only -// succeeds while no other target of the bundle is still deployed there; both a -// remaining sibling target and an already-removed directory surface as an error that -// is expected and ignored. -func removeEmptyBundleDir(ctx context.Context, b *bundle.Bundle) { - dir := path.Dir(b.Config.Workspace.RootPath) - - // root_path is user-configurable, so only clean up the layout the CLI generates - // (~/.bundle//) instead of deleting the parent of an arbitrary path. - if path.Base(path.Dir(dir)) != bundleDirName { +// removeBundleNameDir removes the directory above the deployment root, the one named +// after the bundle, now that the deployment under it is gone. It runs only when +// root_path is scoped by bundle name and target, so the directory holds nothing but +// this bundle. The delete is not recursive, so it fails while another target is still +// deployed there; that and an already-removed directory are both ignored. +func removeBundleNameDir(ctx context.Context, b *bundle.Bundle) { + if !b.RootPathIsNameTargetScoped { return } + dir := path.Dir(b.Config.Workspace.RootPath) err := b.WorkspaceClient(ctx).Workspace.Delete(ctx, workspace.Delete{Path: dir}) if err != nil { log.Debugf(ctx, "Leaving %s in place: %s", dir, err) diff --git a/libs/testserver/workspace_test.go b/libs/testserver/workspace_test.go index e8d8c4a12a8..cde1c9741c6 100644 --- a/libs/testserver/workspace_test.go +++ b/libs/testserver/workspace_test.go @@ -65,17 +65,24 @@ func TestWorkspaceImportRejectsMissingParent(t *testing.T) { assert.Equal(t, 200, importFile(t, server.URL, "/test-dir/file.py", "content")) } -// A non-recursive delete only removes an empty directory, so a caller can use it to -// clean up a parent directory without touching one that still holds a sibling. +// A non-recursive delete only removes a directory that is truly empty, which is what +// lets a caller clean up a parent directory without touching one still in use. func TestWorkspaceDeleteNonRecursiveRequiresEmptyDirectory(t *testing.T) { server := testserver.New(t) testserver.AddDefaultHandlers(server) + // A subdirectory keeps the parent. mkdirs(t, server.URL, "/a/b/c") - assert.Equal(t, 400, workspaceDelete(t, server.URL, "/a/b", false)) assert.Equal(t, 200, getStatus(t, server.URL, "/a/b")) + // So does a file. + mkdirs(t, server.URL, "/f/dir") + require.Equal(t, 200, importFile(t, server.URL, "/f/dir/file.py", "content")) + assert.Equal(t, 400, workspaceDelete(t, server.URL, "/f/dir", false)) + assert.Equal(t, 200, getStatus(t, server.URL, "/f/dir")) + + // Emptied out, both go. assert.Equal(t, 200, workspaceDelete(t, server.URL, "/a/b/c", false)) assert.Equal(t, 200, workspaceDelete(t, server.URL, "/a/b", false)) assert.Equal(t, 404, getStatus(t, server.URL, "/a/b")) From bd9a063f1eba096f62846a56c9bb74bc26ddcc09 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 24 Aug 2026 09:16:39 +0000 Subject: [PATCH 4/6] Match the real not-empty error and trim the changelog entry Verified against a live workspace: a non-recursive delete of a directory holding a file or a subdirectory returns 400 DIRECTORY_NOT_EMPTY with "Folder () is not empty". The status and code already matched; align the message too. Co-authored-by: Isaac --- .nextchanges/bundles/destroy-removes-bundle-directory.md | 2 +- libs/testserver/fake_workspace.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.nextchanges/bundles/destroy-removes-bundle-directory.md b/.nextchanges/bundles/destroy-removes-bundle-directory.md index db057020a4b..41bed1e4c1c 100644 --- a/.nextchanges/bundles/destroy-removes-bundle-directory.md +++ b/.nextchanges/bundles/destroy-removes-bundle-directory.md @@ -1 +1 @@ -`bundle destroy` now also removes the directory named after the bundle, not just the target directory beneath it, when `workspace.root_path` ends in `${bundle.name}/${bundle.target}` — which includes the default root path. It is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind, which accumulated in the workspace and counted against its child-node limit ([#6317](https://github.com/databricks/cli/pull/6317)). +`bundle destroy` now also removes the directory named after the bundle, not just the target directory beneath it, when `workspace.root_path` ends in `${bundle.name}/${bundle.target}` — which includes the default root path. It is removed non-recursively, so it stays in place while another target of the same bundle is still deployed there. Previously every destroy left an empty directory behind ([#6317](https://github.com/databricks/cli/pull/6317)). diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 7625c4d8546..64fafb322aa 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -709,7 +709,7 @@ func (s *FakeWorkspace) WorkspaceDelete(path string, recursive bool) Response { StatusCode: 400, Body: map[string]string{ "error_code": "DIRECTORY_NOT_EMPTY", - "message": "Directory " + path + " is not empty", + "message": "Folder (" + path + ") is not empty", }, } } From 7ef818411e716ad61064b0f6ae456cee3c9348e3 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 24 Aug 2026 09:22:23 +0000 Subject: [PATCH 5/6] Run the destroy directory tests on cloud too Names and paths now carry $UNIQUE_NAME so concurrent legs do not collide, and each test cleans up the prefix it created. Also trims two comments to the 1-3 line cap. Co-authored-by: Isaac --- .../root-path-name-target/databricks.yml | 5 ----- .../root-path-name-target/databricks.yml.tmpl | 5 +++++ .../root-path-name-target/out.test.toml | 2 +- .../destroy/root-path-name-target/output.txt | 16 +++++++------- .../destroy/root-path-name-target/script | 13 +++++++++--- .../destroy/root-path-name-target/test.toml | 6 ++++++ .../root-path-not-scoped/databricks.yml | 5 ----- .../root-path-not-scoped/databricks.yml.tmpl | 5 +++++ .../root-path-not-scoped/out.test.toml | 2 +- .../destroy/root-path-not-scoped/output.txt | 12 ++++++----- .../destroy/root-path-not-scoped/script | 13 +++++++++--- .../destroy/root-path-not-scoped/test.toml | 6 ++++++ .../{databricks.yml => databricks.yml.tmpl} | 2 +- .../destroy/sibling-target/out.test.toml | 2 +- .../bundle/destroy/sibling-target/output.txt | 21 +++++++++---------- .../bundle/destroy/sibling-target/script | 8 ++++--- .../bundle/destroy/sibling-target/test.toml | 6 ++++++ bundle/bundle.go | 6 ++---- bundle/deploy/files/delete.go | 8 +++---- 19 files changed, 88 insertions(+), 55 deletions(-) delete mode 100644 acceptance/bundle/destroy/root-path-name-target/databricks.yml create mode 100644 acceptance/bundle/destroy/root-path-name-target/databricks.yml.tmpl create mode 100644 acceptance/bundle/destroy/root-path-name-target/test.toml delete mode 100644 acceptance/bundle/destroy/root-path-not-scoped/databricks.yml create mode 100644 acceptance/bundle/destroy/root-path-not-scoped/databricks.yml.tmpl create mode 100644 acceptance/bundle/destroy/root-path-not-scoped/test.toml rename acceptance/bundle/destroy/sibling-target/{databricks.yml => databricks.yml.tmpl} (60%) create mode 100644 acceptance/bundle/destroy/sibling-target/test.toml diff --git a/acceptance/bundle/destroy/root-path-name-target/databricks.yml b/acceptance/bundle/destroy/root-path-name-target/databricks.yml deleted file mode 100644 index 05d25995653..00000000000 --- a/acceptance/bundle/destroy/root-path-name-target/databricks.yml +++ /dev/null @@ -1,5 +0,0 @@ -bundle: - name: test-bundle - -workspace: - root_path: ~/.bundle/explicit/${bundle.name}/${bundle.target} diff --git a/acceptance/bundle/destroy/root-path-name-target/databricks.yml.tmpl b/acceptance/bundle/destroy/root-path-name-target/databricks.yml.tmpl new file mode 100644 index 00000000000..0c0aa94f179 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/databricks.yml.tmpl @@ -0,0 +1,5 @@ +bundle: + name: test-bundle + +workspace: + root_path: ~/.bundle/explicit-$UNIQUE_NAME/${bundle.name}/${bundle.target} diff --git a/acceptance/bundle/destroy/root-path-name-target/out.test.toml b/acceptance/bundle/destroy/root-path-name-target/out.test.toml index 98ea5040486..2a13818c13f 100644 --- a/acceptance/bundle/destroy/root-path-name-target/out.test.toml +++ b/acceptance/bundle/destroy/root-path-name-target/out.test.toml @@ -1,2 +1,2 @@ -Cloud = false +Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/destroy/root-path-name-target/output.txt b/acceptance/bundle/destroy/root-path-name-target/output.txt index 98390afdd52..dc6cd4287a8 100644 --- a/acceptance/bundle/destroy/root-path-name-target/output.txt +++ b/acceptance/bundle/destroy/root-path-name-target/output.txt @@ -1,23 +1,25 @@ >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle/default/files... -Files: 3 uploaded, 0 deleted +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME]/test-bundle/default/files... +Files: 5 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 0 unchanged >>> [CLI] bundle destroy --auto-approve -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle/default +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME]/test-bundle/default Destroy: 0 deleted === Assert the bundle directory is deleted ->>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle -Error: Path (//Workspace/Users/[USERNAME]/.bundle/explicit/test-bundle) doesn't exist. +>>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME]/test-bundle +Error: Path (//Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME]/test-bundle) doesn't exist. Exit code: 1 === Assert the prefix above it is kept ->>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME] { - "path": "/Users/[USERNAME]/.bundle/explicit", "object_type": "DIRECTORY" } + +=== Clean up +>>> [CLI] workspace delete //Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME] --recursive diff --git a/acceptance/bundle/destroy/root-path-name-target/script b/acceptance/bundle/destroy/root-path-name-target/script index 73a6b65db44..c4a11c3d984 100644 --- a/acceptance/bundle/destroy/root-path-name-target/script +++ b/acceptance/bundle/destroy/root-path-name-target/script @@ -1,10 +1,17 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +PREFIX="//Workspace/Users/${CURRENT_USER_NAME}/.bundle/explicit-${UNIQUE_NAME}" + trace $CLI bundle deploy trace $CLI bundle destroy --auto-approve # root_path spells out ${bundle.name}/${bundle.target}, so the directory named after the -# bundle goes with the deployment even though it is not under the default location. +# bundle goes with the deployment even outside the default location. title "Assert the bundle directory is deleted" -trace errcode $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/explicit/test-bundle" +trace errcode $CLI workspace get-status "${PREFIX}/test-bundle" title "Assert the prefix above it is kept" -trace $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/explicit" | jq '{path, object_type}' +trace $CLI workspace get-status "${PREFIX}" | jq '{object_type}' + +title "Clean up" +trace $CLI workspace delete "${PREFIX}" --recursive diff --git a/acceptance/bundle/destroy/root-path-name-target/test.toml b/acceptance/bundle/destroy/root-path-name-target/test.toml new file mode 100644 index 00000000000..606a2ff06ff --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/test.toml @@ -0,0 +1,6 @@ +Cloud = true + +Ignore = [ + "databricks.yml", + ".databricks/" +] diff --git a/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml b/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml deleted file mode 100644 index e480d162dda..00000000000 --- a/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml +++ /dev/null @@ -1,5 +0,0 @@ -bundle: - name: test-bundle - -workspace: - root_path: ~/.bundle/custom-root/inner diff --git a/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml.tmpl b/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml.tmpl new file mode 100644 index 00000000000..8e8273a8d21 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml.tmpl @@ -0,0 +1,5 @@ +bundle: + name: test-bundle + +workspace: + root_path: ~/.bundle/custom-root-$UNIQUE_NAME/inner diff --git a/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml b/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml index 98ea5040486..2a13818c13f 100644 --- a/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml +++ b/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml @@ -1,2 +1,2 @@ -Cloud = false +Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/destroy/root-path-not-scoped/output.txt b/acceptance/bundle/destroy/root-path-not-scoped/output.txt index f535c80b5a5..539e7d5d5e4 100644 --- a/acceptance/bundle/destroy/root-path-not-scoped/output.txt +++ b/acceptance/bundle/destroy/root-path-not-scoped/output.txt @@ -1,17 +1,19 @@ >>> [CLI] bundle deploy -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/custom-root/inner/files... -Files: 3 uploaded, 0 deleted +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/custom-root-[UNIQUE_NAME]/inner/files... +Files: 5 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 0 unchanged >>> [CLI] bundle destroy --auto-approve -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/custom-root/inner +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/custom-root-[UNIQUE_NAME]/inner Destroy: 0 deleted === Assert the parent directory is kept ->>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/custom-root +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/custom-root-[UNIQUE_NAME] { - "path": "/Users/[USERNAME]/.bundle/custom-root", "object_type": "DIRECTORY" } + +=== Clean up +>>> [CLI] workspace delete //Workspace/Users/[USERNAME]/.bundle/custom-root-[UNIQUE_NAME] --recursive diff --git a/acceptance/bundle/destroy/root-path-not-scoped/script b/acceptance/bundle/destroy/root-path-not-scoped/script index 3817c6b46f1..f53ddf23cdb 100644 --- a/acceptance/bundle/destroy/root-path-not-scoped/script +++ b/acceptance/bundle/destroy/root-path-not-scoped/script @@ -1,7 +1,14 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +PARENT="//Workspace/Users/${CURRENT_USER_NAME}/.bundle/custom-root-${UNIQUE_NAME}" + trace $CLI bundle deploy trace $CLI bundle destroy --auto-approve -# root_path does not end in ${bundle.name}/${bundle.target}, so the directory above it -# is not this bundle's and is left alone. Double slash keeps Windows from rewriting it. +# root_path does not end in ${bundle.name}/${bundle.target}, so the directory above it is +# not this bundle's and is left alone. title "Assert the parent directory is kept" -trace $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/custom-root" | jq '{path, object_type}' +trace $CLI workspace get-status "${PARENT}" | jq '{object_type}' + +title "Clean up" +trace $CLI workspace delete "${PARENT}" --recursive diff --git a/acceptance/bundle/destroy/root-path-not-scoped/test.toml b/acceptance/bundle/destroy/root-path-not-scoped/test.toml new file mode 100644 index 00000000000..606a2ff06ff --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/test.toml @@ -0,0 +1,6 @@ +Cloud = true + +Ignore = [ + "databricks.yml", + ".databricks/" +] diff --git a/acceptance/bundle/destroy/sibling-target/databricks.yml b/acceptance/bundle/destroy/sibling-target/databricks.yml.tmpl similarity index 60% rename from acceptance/bundle/destroy/sibling-target/databricks.yml rename to acceptance/bundle/destroy/sibling-target/databricks.yml.tmpl index a12a0457944..6ae23ac0937 100644 --- a/acceptance/bundle/destroy/sibling-target/databricks.yml +++ b/acceptance/bundle/destroy/sibling-target/databricks.yml.tmpl @@ -1,5 +1,5 @@ bundle: - name: test-bundle + name: test-bundle-$UNIQUE_NAME targets: dev: diff --git a/acceptance/bundle/destroy/sibling-target/out.test.toml b/acceptance/bundle/destroy/sibling-target/out.test.toml index 98ea5040486..2a13818c13f 100644 --- a/acceptance/bundle/destroy/sibling-target/out.test.toml +++ b/acceptance/bundle/destroy/sibling-target/out.test.toml @@ -1,2 +1,2 @@ -Cloud = false +Cloud = true EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/destroy/sibling-target/output.txt b/acceptance/bundle/destroy/sibling-target/output.txt index 48cbff888ce..ca1e6991d02 100644 --- a/acceptance/bundle/destroy/sibling-target/output.txt +++ b/acceptance/bundle/destroy/sibling-target/output.txt @@ -1,35 +1,34 @@ >>> [CLI] bundle deploy -t dev -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/dev/files... -Files: 3 uploaded, 0 deleted +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/dev/files... +Files: 5 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 0 unchanged >>> [CLI] bundle deploy -t prod -Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/prod/files... -Files: 3 uploaded, 0 deleted +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/prod/files... +Files: 5 uploaded, 0 deleted Resources: 0 created, 0 changed, 0 deleted, 0 unchanged === Destroy one target while the other is still deployed >>> [CLI] bundle destroy -t dev --auto-approve -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/dev +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/dev Destroy: 0 deleted -=== Assert the bundle directory is kept for the remaining target ->>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +=== Assert the bundle directory is kept: the sibling target is still under it +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME] { - "path": "/Users/[USERNAME]/.bundle/test-bundle", "object_type": "DIRECTORY" } === Destroy the remaining target >>> [CLI] bundle destroy -t prod --auto-approve -All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/prod +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/prod Destroy: 0 deleted === Assert the bundle directory is deleted ->>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle -Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle) doesn't exist. +>>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME] +Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]) doesn't exist. Exit code: 1 diff --git a/acceptance/bundle/destroy/sibling-target/script b/acceptance/bundle/destroy/sibling-target/script index 1f2db9fec6c..a12ebe32188 100644 --- a/acceptance/bundle/destroy/sibling-target/script +++ b/acceptance/bundle/destroy/sibling-target/script @@ -1,5 +1,7 @@ +envsubst < databricks.yml.tmpl > databricks.yml + # Double slash keeps Windows from rewriting the path. -BUNDLE_DIR="//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" +BUNDLE_DIR="//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle-${UNIQUE_NAME}" trace $CLI bundle deploy -t dev trace $CLI bundle deploy -t prod @@ -7,8 +9,8 @@ trace $CLI bundle deploy -t prod title "Destroy one target while the other is still deployed" trace $CLI bundle destroy -t dev --auto-approve -title "Assert the bundle directory is kept for the remaining target" -trace $CLI workspace get-status "${BUNDLE_DIR}" | jq '{path, object_type}' +title "Assert the bundle directory is kept: the sibling target is still under it" +trace $CLI workspace get-status "${BUNDLE_DIR}" | jq '{object_type}' title "Destroy the remaining target" trace $CLI bundle destroy -t prod --auto-approve diff --git a/acceptance/bundle/destroy/sibling-target/test.toml b/acceptance/bundle/destroy/sibling-target/test.toml new file mode 100644 index 00000000000..606a2ff06ff --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/test.toml @@ -0,0 +1,6 @@ +Cloud = true + +Ignore = [ + "databricks.yml", + ".databricks/" +] diff --git a/bundle/bundle.go b/bundle/bundle.go index bca44c53842..c58fba10d77 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -158,10 +158,8 @@ type Bundle struct { Target *config.Target `json:"target_config,omitempty" bundle:"internal"` // RootPathIsNameTargetScoped reports whether workspace.root_path ends in the bundle - // name and target. It is recorded before variable resolution, while ${bundle.name} - // and ${bundle.target} are still literal, so a path that only happens to end in - // those two segments does not count. Destroy uses it to tell whether the directory - // above the root path belongs to this bundle alone. + // name and target. Recorded before variable resolution, so a path that only happens + // to end in those two segments does not count. RootPathIsNameTargetScoped bool // Metadata about the bundle deployment. This is the interface Databricks services diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index a2e120a7c6a..1baa3457e26 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -40,11 +40,9 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return nil } -// removeBundleNameDir removes the directory above the deployment root, the one named -// after the bundle, now that the deployment under it is gone. It runs only when -// root_path is scoped by bundle name and target, so the directory holds nothing but -// this bundle. The delete is not recursive, so it fails while another target is still -// deployed there; that and an already-removed directory are both ignored. +// removeBundleNameDir removes the directory named after the bundle now that the +// deployment under it is gone. Not recursive, so it fails harmlessly while another +// target is still deployed there. func removeBundleNameDir(ctx context.Context, b *bundle.Bundle) { if !b.RootPathIsNameTargetScoped { return From f1f3a7cf3838e5899196621d689d3b8aaa0ccbb4 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 24 Aug 2026 09:55:53 +0000 Subject: [PATCH 6/6] Address review: Infof, musterr, and a plain suffix match - Ignored release of the bundle directory logs at Info, the highest level that stays hidden, so it lines up with -q/-v once log levels are wired up. - The "directory is gone" assertions use musterr, so the test fails if get-status ever succeeds instead of merely tolerating the error. - endsWithNameAndTarget matches the suffix as a plain string. root_path is not interpolated yet, so path.Base/path.Dir on it are not dependable. Co-authored-by: Isaac --- acceptance/bundle/destroy/all-resources/output.txt | 4 +--- acceptance/bundle/destroy/all-resources/script | 2 +- .../destroy/root-path-name-target/output.txt | 4 +--- .../bundle/destroy/root-path-name-target/script | 2 +- .../bundle/destroy/sibling-target/output.txt | 4 +--- acceptance/bundle/destroy/sibling-target/script | 2 +- bundle/config/mutator/default_workspace_root.go | 14 +++++--------- bundle/deploy/files/delete.go | 2 +- 8 files changed, 12 insertions(+), 22 deletions(-) diff --git a/acceptance/bundle/destroy/all-resources/output.txt b/acceptance/bundle/destroy/all-resources/output.txt index 207a85f8c8b..eed7625730b 100644 --- a/acceptance/bundle/destroy/all-resources/output.txt +++ b/acceptance/bundle/destroy/all-resources/output.txt @@ -23,7 +23,5 @@ All files and directories at the following location will be deleted: /Workspace/ Destroy: 2 deleted === Assert the bundle directory is deleted ->>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +>>> musterr [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle) doesn't exist. - -Exit code: 1 diff --git a/acceptance/bundle/destroy/all-resources/script b/acceptance/bundle/destroy/all-resources/script index 71160f7c3f7..c9aa66b07f3 100644 --- a/acceptance/bundle/destroy/all-resources/script +++ b/acceptance/bundle/destroy/all-resources/script @@ -4,4 +4,4 @@ trace $CLI bundle destroy --auto-approve # Destroy removes the target directory, and the .bundle/ parent goes with it once # nothing else is deployed under it. Double slash keeps Windows from rewriting the path. title "Assert the bundle directory is deleted" -trace errcode $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" +trace musterr $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" diff --git a/acceptance/bundle/destroy/root-path-name-target/output.txt b/acceptance/bundle/destroy/root-path-name-target/output.txt index dc6cd4287a8..b880722ba81 100644 --- a/acceptance/bundle/destroy/root-path-name-target/output.txt +++ b/acceptance/bundle/destroy/root-path-name-target/output.txt @@ -10,11 +10,9 @@ All files and directories at the following location will be deleted: /Workspace/ Destroy: 0 deleted === Assert the bundle directory is deleted ->>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME]/test-bundle +>>> musterr [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME]/test-bundle Error: Path (//Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME]/test-bundle) doesn't exist. -Exit code: 1 - === Assert the prefix above it is kept >>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME] { diff --git a/acceptance/bundle/destroy/root-path-name-target/script b/acceptance/bundle/destroy/root-path-name-target/script index c4a11c3d984..414a7ab4ba8 100644 --- a/acceptance/bundle/destroy/root-path-name-target/script +++ b/acceptance/bundle/destroy/root-path-name-target/script @@ -8,7 +8,7 @@ trace $CLI bundle destroy --auto-approve # root_path spells out ${bundle.name}/${bundle.target}, so the directory named after the # bundle goes with the deployment even outside the default location. title "Assert the bundle directory is deleted" -trace errcode $CLI workspace get-status "${PREFIX}/test-bundle" +trace musterr $CLI workspace get-status "${PREFIX}/test-bundle" title "Assert the prefix above it is kept" trace $CLI workspace get-status "${PREFIX}" | jq '{object_type}' diff --git a/acceptance/bundle/destroy/sibling-target/output.txt b/acceptance/bundle/destroy/sibling-target/output.txt index ca1e6991d02..53cfe9c37e1 100644 --- a/acceptance/bundle/destroy/sibling-target/output.txt +++ b/acceptance/bundle/destroy/sibling-target/output.txt @@ -28,7 +28,5 @@ All files and directories at the following location will be deleted: /Workspace/ Destroy: 0 deleted === Assert the bundle directory is deleted ->>> errcode [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME] +>>> musterr [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME] Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]) doesn't exist. - -Exit code: 1 diff --git a/acceptance/bundle/destroy/sibling-target/script b/acceptance/bundle/destroy/sibling-target/script index a12ebe32188..c77c90aa470 100644 --- a/acceptance/bundle/destroy/sibling-target/script +++ b/acceptance/bundle/destroy/sibling-target/script @@ -16,4 +16,4 @@ title "Destroy the remaining target" trace $CLI bundle destroy -t prod --auto-approve title "Assert the bundle directory is deleted" -trace errcode $CLI workspace get-status "${BUNDLE_DIR}" +trace musterr $CLI workspace get-status "${BUNDLE_DIR}" diff --git a/bundle/config/mutator/default_workspace_root.go b/bundle/config/mutator/default_workspace_root.go index 0c6ee14277f..b7571afbb85 100644 --- a/bundle/config/mutator/default_workspace_root.go +++ b/bundle/config/mutator/default_workspace_root.go @@ -3,7 +3,6 @@ package mutator import ( "context" "fmt" - "path" "strings" "github.com/databricks/cli/bundle" @@ -12,10 +11,7 @@ import ( // How the bundle name and target appear in a configured root_path, which this mutator // sees before variable resolution replaces them. -const ( - bundleNameRef = "${bundle.name}" - bundleTargetRef = "${bundle.target}" -) +const nameTargetSuffix = "${bundle.name}/${bundle.target}" type defineDefaultWorkspaceRoot struct{} @@ -51,9 +47,9 @@ func (m *defineDefaultWorkspaceRoot) Apply(ctx context.Context, b *bundle.Bundle return nil } -// endsWithNameAndTarget reports whether the last two segments of rootPath are the -// bundle name and target references. +// endsWithNameAndTarget reports whether rootPath ends in the bundle name and target +// references. Matched as a plain string: rootPath is not interpolated yet, so path +// operations on it are not reliable. func endsWithNameAndTarget(rootPath string) bool { - rootPath = strings.TrimSuffix(rootPath, "/") - return path.Base(rootPath) == bundleTargetRef && path.Base(path.Dir(rootPath)) == bundleNameRef + return strings.HasSuffix(strings.TrimSuffix(rootPath, "/"), nameTargetSuffix) } diff --git a/bundle/deploy/files/delete.go b/bundle/deploy/files/delete.go index 1baa3457e26..c30262f79ce 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -51,7 +51,7 @@ func removeBundleNameDir(ctx context.Context, b *bundle.Bundle) { dir := path.Dir(b.Config.Workspace.RootPath) err := b.WorkspaceClient(ctx).Workspace.Delete(ctx, workspace.Delete{Path: dir}) if err != nil { - log.Debugf(ctx, "Leaving %s in place: %s", dir, err) + log.Infof(ctx, "Leaving %s in place: %s", dir, err) } }