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/deploy/snapshot/upload.go

@@ -21,8 +21,8 @@ type snapshotUpload struct {
 	// clean is true for a fresh plan or deploy: the mutator discards zips staged by a
 	// previous run (so content-addressed "<hash>.zip" files don't accumulate) and
 	// stages the current one. It is false when applying a pre-existing plan
-	// (deploy --plan): the plan already records the resource and the zip staged when it
-	// was produced, so the mutator reuses that staged file instead of rebuilding it.
+	// (deploy --plan): the mutator reuses the zip staged when the plan was produced
+	// instead of rebuilding it.
 	clean bool
 }
 
@@ -34,8 +34,9 @@ type PlanUploadOptions struct {
 // resource. When Clean is true it discards zips staged by a previous run, builds the
 // bundle zip, stages it under the bundle's local state directory as "<hash>.zip", and
 // records the path on the resource; the staged file is uploaded when the resource is
-// created on apply. Pass Clean=false when applying a pre-existing plan: the plan
-// already carries the resource and its staged zip, so the mutator reuses them.
+// created on apply. Pass Clean=false when applying a pre-existing plan: the mutator reuses the
+// zip staged when the plan was produced. Either way the resource is registered in the config,
+// so its path is known to the rest of the deploy before the snapshot is uploaded.
 func PlanUpload(opts PlanUploadOptions) bundle.Mutator {
 	return &snapshotUpload{clean: opts.Clean}
 }
@@ -45,13 +46,6 @@ func (m *snapshotUpload) Name() string {
 }
 
 func (m *snapshotUpload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-	// Applying a pre-existing plan: the plan already records the snapshot resource and
-	// the path of the zip staged when the plan was produced (InitForApply restores that
-	// state and DoCreate reads the staged file). Reuse it instead of rebuilding.
-	if !m.clean {
-		return nil
-	}
-
 	uploader, err := snapshot.NewSnapshotClient(b.WorkspaceClient(ctx))
 	if err != nil {
 		return diag.FromErr(err)
@@ -69,41 +63,68 @@ func (m *snapshotUpload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
 		return nil
 	}
 
+	var zipPath string
+	var diags diag.Diagnostics
+	if m.clean {
+		zipPath, diags, err = stageZip(ctx, b)
+	} else {
+		zipPath, err = stagedZipPath(ctx, b)
+	}
+	if err != nil {
+		return diags.Extend(diag.FromErr(err))
+	}
+
+	b.Config.Resources.Snapshots[resources.SnapshotResourceKey] = &resources.Snapshot{
+		BundleID:   BundleID(b),
+		ACL:        BuildACL(b),
+		RemoteRoot: remoteRoot,
+		ZipPath:    filepath.ToSlash(zipPath),
+	}
+	return diags
+}
+
+// stageZip builds the bundle zip and stages it under the bundle's local state directory as
+// "<hash>.zip", discarding zips staged by a previous run so content-addressed files don't
+// accumulate.
+func stageZip(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics, error) {
 	zipContent, fileCount, err := BundleZip(ctx, b)
 	if err != nil {
-		return diag.FromErr(fmt.Errorf("failed to build snapshot zip: %w", err))
+		return "", nil, fmt.Errorf("failed to build snapshot zip: %w", err)
 	}
 
-	// Discard zips staged by a previous run so content-addressed "<hash>.zip" files
-	// don't accumulate.
 	if err := os.RemoveAll(b.GetLocalStateDir(ctx, "snapshots")); err != nil {
-		return diag.FromErr(fmt.Errorf("failed to clean snapshot dir: %w", err))
+		return "", nil, fmt.Errorf("failed to clean snapshot dir: %w", err)
 	}
 	dir, err := b.LocalStateDir(ctx, "snapshots")
 	if err != nil {
-		return diag.FromErr(err)
+		return "", nil, err
 	}
 	zipPath := filepath.Join(dir, snapshot.HashFromContent(zipContent)+".zip")
 	if err := os.WriteFile(zipPath, zipContent, 0o600); err != nil {
-		return diag.FromErr(fmt.Errorf("failed to write snapshot zip: %w", err))
-	}
-
-	b.Config.Resources.Snapshots[resources.SnapshotResourceKey] = &resources.Snapshot{
-		BundleID:   BundleID(b),
-		ACL:        BuildACL(b),
-		RemoteRoot: remoteRoot,
-		ZipPath:    filepath.ToSlash(zipPath),
+		return "", nil, fmt.Errorf("failed to write snapshot zip: %w", err)
 	}
 
 	var diags diag.Diagnostics
 	if fileCount > fileLimitWarning {
-		diags = append(diags, diag.Warningf(
+		diags = diag.Warningf(
 			"immutable folder deployment may not work correctly: bundle contains %d files (limit is %d)",
 			fileCount, fileLimitWarning,
-		)...)
+		)
 	}
+	return zipPath, diags, nil
+}
 
-	return diags
+// stagedZipPath returns the zip staged when the plan being applied was produced. Each fresh
+// plan clears the directory before staging, so it holds exactly that one zip.
+func stagedZipPath(ctx context.Context, b *bundle.Bundle) (string, error) {
+	zips, err := filepath.Glob(filepath.Join(b.GetLocalStateDir(ctx, "snapshots"), "*.zip"))
+	if err != nil {
+		return "", err
+	}
+	if len(zips) != 1 {
+		return "", fmt.Errorf("expected one staged snapshot zip, found %d; run 'bundle plan' again", len(zips))
+	}
+	return zips[0], nil
 }
 
 // BundleID returns a stable hash that identifies the bundle deployment.