Fixed immutable folder failing with DMS or top level permissions

.nextchanges/bundles/immutable-folder-paths.md

@@ -0,0 +1 @@
+* Fixed `experimental.immutable_folder` failing with `experimental.deployment_history` ("workspace_info.file_path must be an absolute workspace path") and failing `bundle validate` when top-level `permissions` are set. ([#7001](https://github.com/databricks/cli/pull/7001))

acceptance/bundle/dms/immutable-folder/databricks.yml.tmpl

@@ -0,0 +1,19 @@
+bundle:
+  name: dms-immutable-folder-$UNIQUE_NAME
+experimental:
+  immutable_folder: true
+  deployment_history: true
+
+resources:
+  jobs:
+    foo:
+      name: foo
+      tasks:
+        - task_key: main
+          spark_python_task:
+            python_file: ./src/main.py
+          environment_key: env
+      environments:
+        - environment_key: env
+          spec:
+            environment_version: "4"

acceptance/bundle/dms/immutable-folder/out.test.toml

@@ -0,0 +1,3 @@
+Cloud = false
+EnvMatrix.DMS = ["true"]
+EnvMatrix.READPLAN = ["", "1"]

acceptance/bundle/dms/immutable-folder/output.txt

@@ -0,0 +1,43 @@
+
+=== Deploying with an immutable folder records the resolved snapshot path as the deployment's file_path
+>>> [CLI] bundle deploy
+Created internal_immutable_snapshots.immutable
+Created jobs.foo
+Files: 0 uploaded, 0 deleted
+Resources: 2 created, 0 changed, 0 deleted, 0 unchanged
+
+>>> MSYS_NO_PATHCONV=1 [CLI] api get /api/2.0/bundle/deployments/[DEPLOYMENT_ID]
+{
+  "workspace_info": {
+    "root_path": "/Workspace/Users/[USERNAME]/.bundle/dms-immutable-folder-[UNIQUE_NAME]/default",
+    "file_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files"
+  }
+}
+
+=== A new snapshot moves file_path, which is then updated on the deployment
+>>> [CLI] bundle deploy
+Recreated internal_immutable_snapshots.immutable
+Updated jobs.foo
+Files: 0 uploaded, 0 deleted
+Resources: 1 created, 1 changed, 1 deleted, 0 unchanged
+
+>>> MSYS_NO_PATHCONV=1 [CLI] api get /api/2.0/bundle/deployments/[DEPLOYMENT_ID]
+{
+  "workspace_info": {
+    "root_path": "/Workspace/Users/[USERNAME]/.bundle/dms-immutable-folder-[UNIQUE_NAME]/default",
+    "file_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files"
+  }
+}
+
+=== An unchanged snapshot keeps the recorded file_path
+>>> [CLI] bundle deploy
+Files: 0 uploaded, 0 deleted
+Resources: 0 created, 0 changed, 0 deleted, 2 unchanged
+
+>>> MSYS_NO_PATHCONV=1 [CLI] api get /api/2.0/bundle/deployments/[DEPLOYMENT_ID]
+{
+  "workspace_info": {
+    "root_path": "/Workspace/Users/[USERNAME]/.bundle/dms-immutable-folder-[UNIQUE_NAME]/default",
+    "file_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files"
+  }
+}

acceptance/bundle/dms/immutable-folder/script

@@ -0,0 +1,30 @@
+envsubst < databricks.yml.tmpl > databricks.yml
+
+deployment_workspace_info() {
+    deployment_id=$(MSYS_NO_PATHCONV=1 $CLI workspace get-status "/Workspace/Users/${CURRENT_USER_NAME}/.bundle/dms-immutable-folder-${UNIQUE_NAME}/default/state/resources.deployment.json" -o json | python3 -c 'import sys,json; print(json.load(sys.stdin)["object_id"])')
+    add_repl "$deployment_id" DEPLOYMENT_ID
+    # MSYS_NO_PATHCONV: Git Bash on Windows would rewrite the leading-'/' API path.
+    trace MSYS_NO_PATHCONV=1 $CLI api get "/api/2.0/bundle/deployments/${deployment_id}" | jq '{workspace_info}'
+}
+
+deploy() {
+    mkdir -p .databricks
+    $CLI bundle plan -o json > .databricks/plan.json
+    trace $CLI bundle deploy $(readplanarg .databricks/plan.json)
+    rm .databricks/plan.json
+}
+
+title "Deploying with an immutable folder records the resolved snapshot path as the deployment's file_path"
+deploy
+deployment_workspace_info
+
+title "A new snapshot moves file_path, which is then updated on the deployment"
+echo 'print("changed")' > src/main.py
+deploy
+deployment_workspace_info
+
+title "An unchanged snapshot keeps the recorded file_path"
+deploy
+deployment_workspace_info
+
+rm -f "$OUT_REQUESTS"

acceptance/bundle/dms/immutable-folder/src/main.py

@@ -0,0 +1 @@
+print("hello")

acceptance/bundle/dms/immutable-folder/test.toml

@@ -0,0 +1,20 @@
+# The immutable folder API is not available against a real workspace yet.
+Cloud = false
+
+EnvMatrix.READPLAN = ["", "1"]
+
+Ignore = [
+    '.databricks',
+    'databricks.yml',
+]
+
+# The snapshot path is content-addressed.
+[[Repls]]
+Old = '[0-9a-f]{64}'
+New = '[SNAPSHOT_HASH]'
+
+# When READPLAN=1, "bundle deploy" is called as "bundle deploy --plan .databricks/plan.json".
+# Normalize so both variants produce identical output.
+[[Repls]]
+Old = ' --plan .databricks/plan.json'
+New = ''

acceptance/bundle/validate/immutable_folder_permissions/databricks.yml

@@ -0,0 +1,9 @@
+bundle:
+  name: my-bundle
+
+experimental:
+  immutable_folder: true
+
+permissions:
+  - level: CAN_VIEW
+    user_name: viewer@example.com

acceptance/bundle/validate/immutable_folder_permissions/out.test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+EnvMatrix.DMS = ["", "true"]

acceptance/bundle/validate/immutable_folder_permissions/output.txt

@@ -0,0 +1,21 @@
+
+>>> [CLI] bundle validate
+Recommendation: permissions section should explicitly include the current deployment identity '[USERNAME]' or one of its groups
+If it is not included, CAN_MANAGE permissions are only applied if the present identity is used to deploy.
+
+Consider using a adding a top-level permissions section such as the following:
+
+  permissions:
+    - user_name: [USERNAME]
+      level: CAN_MANAGE
+
+See https://docs.databricks.com/dev-tools/bundles/permissions.html to learn more about permission configuration.
+  in databricks.yml:8:3
+
+Name: my-bundle
+Target: default
+Workspace:
+  User: [USERNAME]
+  Path: /Workspace/Users/[USERNAME]/.bundle/my-bundle/default
+
+Found 1 recommendation

acceptance/bundle/validate/immutable_folder_permissions/script

@@ -0,0 +1,3 @@
+# file_path and artifact_path point into the snapshot, which does not exist until deploy,
+# so validate must not check their folder permissions.
+trace $CLI bundle validate

acceptance/bundle/validate/immutable_folder_permissions/test.toml

@@ -0,0 +1,2 @@
+Cloud = false
+Ignore = [".databricks"]

acceptance/bundle/variables/resolve-builtin/databricks.yml

@@ -2,5 +2,5 @@ bundle:
   name: TestResolveVariableReferences
 
 workspace:
-  root_path: "${bundle.name}/bar"
+  root_path: "/${bundle.name}/bar"
   file_path: "${workspace.root_path}/baz"

acceptance/bundle/variables/resolve-builtin/output.txt

@@ -1,7 +1,7 @@
 {
-  "artifact_path": "TestResolveVariableReferences/bar/artifacts",
-  "file_path": "TestResolveVariableReferences/bar/baz",
-  "resource_path": "TestResolveVariableReferences/bar/resources",
-  "root_path": "TestResolveVariableReferences/bar",
-  "state_path": "TestResolveVariableReferences/bar/state"
+  "artifact_path": "/Workspace/TestResolveVariableReferences/bar/artifacts",
+  "file_path": "/Workspace/TestResolveVariableReferences/bar/baz",
+  "resource_path": "/Workspace/TestResolveVariableReferences/bar/resources",
+  "root_path": "/Workspace/TestResolveVariableReferences/bar",
+  "state_path": "/Workspace/TestResolveVariableReferences/bar/state"
 }

acceptance/bundle/variables/resolve-vars-in-root-path/databricks.yml

@@ -2,7 +2,7 @@ bundle:
   name: TestResolveVariableReferencesToBundleVariables
 
 workspace:
-  root_path: "${bundle.name}/${var.foo}"
+  root_path: "/${bundle.name}/${var.foo}"
 
 variables:
   foo:

acceptance/bundle/variables/resolve-vars-in-root-path/output.txt

@@ -1,7 +1,7 @@
 {
-  "artifact_path": "TestResolveVariableReferencesToBundleVariables/bar/artifacts",
-  "file_path": "TestResolveVariableReferencesToBundleVariables/bar/files",
-  "resource_path": "TestResolveVariableReferencesToBundleVariables/bar/resources",
-  "root_path": "TestResolveVariableReferencesToBundleVariables/bar",
-  "state_path": "TestResolveVariableReferencesToBundleVariables/bar/state"
+  "artifact_path": "/Workspace/TestResolveVariableReferencesToBundleVariables/bar/artifacts",
+  "file_path": "/Workspace/TestResolveVariableReferencesToBundleVariables/bar/files",
+  "resource_path": "/Workspace/TestResolveVariableReferencesToBundleVariables/bar/resources",
+  "root_path": "/Workspace/TestResolveVariableReferencesToBundleVariables/bar",
+  "state_path": "/Workspace/TestResolveVariableReferencesToBundleVariables/bar/state"
 }

bundle/config/validate/folder_permissions.go

@@ -18,7 +18,14 @@ func (f *folderPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Di
 		return nil
 	}
 
-	bundlePaths := paths.CollectUniqueWorkspacePathPrefixes(b.Config.Workspace).Paths
+	workspace := b.Config.Workspace
+	if b.IsImmutableFolder() {
+		// file_path and artifact_path reference the snapshot, which does not exist until
+		// deploy, so there is no folder to check. See permissions.ApplyWorkspaceRootPermissions.
+		workspace.FilePath = ""
+		workspace.ArtifactPath = ""
+	}
+	bundlePaths := paths.CollectUniqueWorkspacePathPrefixes(workspace).Paths
 
 	var diags diag.Diagnostics
 	g, ctx := errgroup.WithContext(ctx)

bundle/phases/dms.go

@@ -2,14 +2,18 @@ package phases
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"net/url"
+	"path"
 	"strconv"
 	"strings"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/bundle/config/resources"
 	"github.com/databricks/cli/bundle/deployplan"
+	"github.com/databricks/cli/bundle/direct/dresources"
 	"github.com/databricks/cli/internal/build"
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/dms"
@@ -71,7 +75,11 @@ func actionToSDK(a deployplan.ActionType) (bundledeployments.OperationActionType
 func createOrUpdateDeployment(ctx context.Context, b *bundle.Bundle, current *bundledeployments.Deployment) {
 	db := &b.DeploymentBundle
 	w := b.WorkspaceClient(ctx)
-	metadata := deploymentMetadata(b)
+	metadata, err := deploymentMetadata(b)
+	if err != nil {
+		logdiag.LogError(ctx, fmt.Errorf("failed to compute deployment metadata: %w", err))
+		return
+	}
 	deploymentID := db.StateDB.DeploymentID
 	if deploymentID == "" {
 		dep := metadata.Deployment()
@@ -180,7 +188,7 @@ func logDeploymentVersion(ctx context.Context, b *bundle.Bundle) {
 
 // deploymentMetadata describes the bundle this deploy came from and where it
 // landed, mirroring what bundle/deploy/metadata computes for the metadata file.
-func deploymentMetadata(b *bundle.Bundle) dms.Metadata {
+func deploymentMetadata(b *bundle.Bundle) (dms.Metadata, error) {
 	p := dms.Metadata{
 		DisplayName: b.Config.Bundle.Name,
 		TargetName:  b.Config.Bundle.Target,
@@ -191,6 +199,15 @@ func deploymentMetadata(b *bundle.Bundle) dms.Metadata {
 		RootPath: b.Config.Workspace.RootPath,
 		FilePath: b.Config.Workspace.FilePath,
 	}
+	// With an immutable folder, file_path is a reference to the snapshot, which only
+	// resolves once the snapshot is uploaded. Its path is already known from the plan.
+	if b.IsImmutableFolder() {
+		snapshotPath, err := immutableSnapshotPath(b)
+		if err != nil {
+			return dms.Metadata{}, err
+		}
+		ws.FilePath = path.Join(snapshotPath, "files")
+	}
 	// In a source-linked deployment files are not copied, so resources read them
 	// from the sync root instead of file_path (see bundle/deploy/metadata.Compute).
 	if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
@@ -204,7 +221,31 @@ func deploymentMetadata(b *bundle.Bundle) dms.Metadata {
 		ws.BundleRootPath = b.Config.Bundle.Git.BundleRootPath
 	}
 	p.Workspace = ws
-	return p
+	return p, nil
+}
+
+// immutableSnapshotPath returns the workspace path of the immutable folder snapshot. The
+// snapshot is not uploaded yet at this point, but its content-addressed path is already known.
+func immutableSnapshotPath(b *bundle.Bundle) (string, error) {
+	if sv, ok := b.DeploymentBundle.StateCache.Load(resources.SnapshotKey); ok {
+		state, ok := sv.Value.(*dresources.SnapshotState)
+		if !ok {
+			return "", fmt.Errorf("unexpected state type %T for %s", sv.Value, resources.SnapshotKey)
+		}
+		return state.FullPath, nil
+	}
+
+	// An unchanged snapshot has no planned state when deploying from a saved plan. Its
+	// recorded state is what the plan would have held.
+	entry, ok := b.DeploymentBundle.StateDB.GetResourceEntry(resources.SnapshotKey)
+	if !ok {
+		return "", fmt.Errorf("no state for %s", resources.SnapshotKey)
+	}
+	var state dresources.SnapshotState
+	if err := json.Unmarshal(entry.State, &state); err != nil {
+		return "", fmt.Errorf("failed to read state of %s: %w", resources.SnapshotKey, err)
+	}
+	return state.FullPath, nil
 }
 
 // deploymentModeToSDK maps the bundle target's mode to the DMS enum. An unset mode

libs/testserver/bundledeployments.go

@@ -178,8 +178,16 @@ type dmsWorkspaceInfo struct {
 var dmsUpdatableDeploymentFields = []string{"display_name", "target_name", "deployment_mode", "workspace_info"}
 
 // checkWorkspaceInfo rejects a bundle_root_path without the git_folder_path it is relative to,
-// which is what the service does.
+// and a root_path or file_path that is not absolute, which is what the service does.
 func checkWorkspaceInfo(ws *bundledeployments.WorkspaceInfo) (Response, bool) {
+	if ws != nil {
+		if ws.RootPath != "" && !strings.HasPrefix(ws.RootPath, "/") {
+			return dmsInvalidArgument("workspace_info.root_path must be an absolute workspace path"), false
+		}
+		if ws.FilePath != "" && !strings.HasPrefix(ws.FilePath, "/") {
+			return dmsInvalidArgument("workspace_info.file_path must be an absolute workspace path"), false
+		}
+	}
 	if ws != nil && (ws.GitFolderPath == "") != (ws.BundleRootPath == "") {
 		return dmsInvalidArgument("workspace_info.git_folder_path and workspace_info.bundle_root_path must be set together"), false
 	}

libs/testserver/fake_workspace.go

@@ -638,6 +638,13 @@ func isGitCliFolder(repoPath string) bool {
 }
 
 func (s *FakeWorkspace) WorkspaceGetStatus(requestPath string, returnGitInfo bool) Response {
+	if !strings.HasPrefix(requestPath, "/") {
+		return Response{
+			StatusCode: 400,
+			Body:       map[string]string{"error_code": "INVALID_PARAMETER_VALUE", "message": fmt.Sprintf("Path (%s) doesn't start with '/'", requestPath)},
+		}
+	}
+
 	defer s.LockUnlock()()
 
 	// The real API collapses duplicate slashes, so look up the cleaned path.