diff --git a/.nextchanges/bundles/destroy-removes-bundle-directory.md b/.nextchanges/bundles/destroy-removes-bundle-directory.md new file mode 100644 index 0000000000..41bed1e4c1 --- /dev/null +++ b/.nextchanges/bundles/destroy-removes-bundle-directory.md @@ -0,0 +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 ([#6317](https://github.com/databricks/cli/pull/6317)). diff --git a/acceptance/bundle/destroy/all-resources/output.txt b/acceptance/bundle/destroy/all-resources/output.txt index f8eb4d90e0..eed7625730 100644 --- a/acceptance/bundle/destroy/all-resources/output.txt +++ b/acceptance/bundle/destroy/all-resources/output.txt @@ -21,3 +21,7 @@ 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 +>>> musterr [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/test-bundle +Error: Path (//Workspace/Users/[USERNAME]/.bundle/test-bundle) doesn't exist. diff --git a/acceptance/bundle/destroy/all-resources/script b/acceptance/bundle/destroy/all-resources/script index ceb8acee91..c9aa66b07f 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 musterr $CLI workspace get-status "//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle" 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 0000000000..0c0aa94f17 --- /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 new file mode 100644 index 0000000000..2a13818c13 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/out.test.toml @@ -0,0 +1,2 @@ +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 new file mode 100644 index 0000000000..b880722ba8 --- /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-[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-[UNIQUE_NAME]/test-bundle/default + +Destroy: 0 deleted + +=== Assert the bundle directory is deleted +>>> 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. + +=== Assert the prefix above it is kept +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/explicit-[UNIQUE_NAME] +{ + "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 new file mode 100644 index 0000000000..414a7ab4ba --- /dev/null +++ b/acceptance/bundle/destroy/root-path-name-target/script @@ -0,0 +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 outside the default location. +title "Assert the bundle directory is deleted" +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}' + +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 0000000000..606a2ff06f --- /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.tmpl b/acceptance/bundle/destroy/root-path-not-scoped/databricks.yml.tmpl new file mode 100644 index 0000000000..8e8273a8d2 --- /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 new file mode 100644 index 0000000000..2a13818c13 --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/out.test.toml @@ -0,0 +1,2 @@ +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 new file mode 100644 index 0000000000..539e7d5d5e --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/output.txt @@ -0,0 +1,19 @@ + +>>> [CLI] bundle deploy +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-[UNIQUE_NAME]/inner + +Destroy: 0 deleted + +=== Assert the parent directory is kept +>>> [CLI] workspace get-status //Workspace/Users/[USERNAME]/.bundle/custom-root-[UNIQUE_NAME] +{ + "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 new file mode 100644 index 0000000000..f53ddf23cd --- /dev/null +++ b/acceptance/bundle/destroy/root-path-not-scoped/script @@ -0,0 +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. +title "Assert the parent directory is kept" +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 0000000000..606a2ff06f --- /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.tmpl b/acceptance/bundle/destroy/sibling-target/databricks.yml.tmpl new file mode 100644 index 0000000000..6ae23ac093 --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/databricks.yml.tmpl @@ -0,0 +1,7 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +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 0000000000..2a13818c13 --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/out.test.toml @@ -0,0 +1,2 @@ +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 new file mode 100644 index 0000000000..53cfe9c37e --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/output.txt @@ -0,0 +1,32 @@ + +>>> [CLI] bundle deploy -t dev +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-[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-[UNIQUE_NAME]/dev + +Destroy: 0 deleted + +=== 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] +{ + "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-[UNIQUE_NAME]/prod + +Destroy: 0 deleted + +=== Assert the bundle directory is deleted +>>> 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. diff --git a/acceptance/bundle/destroy/sibling-target/script b/acceptance/bundle/destroy/sibling-target/script new file mode 100644 index 0000000000..c77c90aa47 --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/script @@ -0,0 +1,19 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +# Double slash keeps Windows from rewriting the path. +BUNDLE_DIR="//Workspace/Users/${CURRENT_USER_NAME}/.bundle/test-bundle-${UNIQUE_NAME}" + +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: 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 + +title "Assert the bundle directory is deleted" +trace musterr $CLI workspace get-status "${BUNDLE_DIR}" diff --git a/acceptance/bundle/destroy/sibling-target/test.toml b/acceptance/bundle/destroy/sibling-target/test.toml new file mode 100644 index 0000000000..606a2ff06f --- /dev/null +++ b/acceptance/bundle/destroy/sibling-target/test.toml @@ -0,0 +1,6 @@ +Cloud = true + +Ignore = [ + "databricks.yml", + ".databricks/" +] diff --git a/acceptance/bundle/resource_deps/remote_app_url/output.txt b/acceptance/bundle/resource_deps/remote_app_url/output.txt index 84d8da92e3..9301329af1 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/output.txt b/acceptance/bundle/user_agent/output.txt index 2107e7e2b1..809eb5b1de 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/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json b/acceptance/bundle/user_agent/simple/out.requests.destroy.direct.json index c67f22e373..1e97537e4d 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 09b923be4d..325ff5b60a 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/bundle.go b/bundle/bundle.go index bcceb75208..c58fba10d7 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -157,6 +157,11 @@ 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. 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 // 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 d7c24a5b55..b7571afbb8 100644 --- a/bundle/config/mutator/default_workspace_root.go +++ b/bundle/config/mutator/default_workspace_root.go @@ -3,11 +3,16 @@ package mutator import ( "context" "fmt" + "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 nameTargetSuffix = "${bundle.name}/${bundle.target}" + type defineDefaultWorkspaceRoot struct{} // DefineDefaultWorkspaceRoot defines the default workspace root path. @@ -21,6 +26,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 +43,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 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 { + return strings.HasSuffix(strings.TrimSuffix(rootPath, "/"), nameTargetSuffix) +} diff --git a/bundle/config/mutator/default_workspace_root_test.go b/bundle/config/mutator/default_workspace_root_test.go index ad5dac4e42..0a26c7ad59 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 562437e543..c30262f79c 100644 --- a/bundle/deploy/files/delete.go +++ b/bundle/deploy/files/delete.go @@ -6,9 +6,11 @@ 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" ) @@ -28,6 +30,8 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return diag.FromErr(err) } + removeBundleNameDir(ctx, b) + // Clean up sync snapshot file err = deleteSnapshotFile(ctx, b) if err != nil { @@ -36,6 +40,21 @@ func (m *delete) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return nil } +// 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 + } + + dir := path.Dir(b.Config.Workspace.RootPath) + err := b.WorkspaceClient(ctx).Workspace.Delete(ctx, workspace.Delete{Path: dir}) + if err != nil { + log.Infof(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 1caec84d1e..64fafb322a 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": "Folder (" + 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 099230ad94..1cc037dcb6 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 5e4753246c..cde1c9741c 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,29 @@ func TestWorkspaceImportRejectsMissingParent(t *testing.T) { assert.Equal(t, 200, importFile(t, server.URL, "/test-dir/file.py", "content")) } +// 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")) +} + // mkdirs creates all intermediate directories, matching "mkdir -p". func TestWorkspaceMkdirsRecursive(t *testing.T) { server := testserver.New(t)