From 72c853e728e8283e080849220644157001781f1b Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 17 Jul 2026 11:11:50 +0200 Subject: [PATCH] Create a new immutable snapshot when bundle permissions change --- .../databricks.yml.tmpl | 19 +++++++ .../out.test.toml | 3 ++ .../immutable-permissions-change/output.txt | 52 +++++++++++++++++++ .../immutable-permissions-change/script | 35 +++++++++++++ .../immutable-permissions-change/src/main.py | 1 + .../immutable-permissions-change/test.toml | 15 ++++++ bundle/deploy/snapshot/path.go | 42 +++++++++++++++ bundle/deploy/snapshot/path_test.go | 39 +++++++++++++- 8 files changed, 204 insertions(+), 2 deletions(-) create mode 100644 acceptance/bundle/deploy/immutable-permissions-change/databricks.yml.tmpl create mode 100644 acceptance/bundle/deploy/immutable-permissions-change/out.test.toml create mode 100644 acceptance/bundle/deploy/immutable-permissions-change/output.txt create mode 100644 acceptance/bundle/deploy/immutable-permissions-change/script create mode 100644 acceptance/bundle/deploy/immutable-permissions-change/src/main.py create mode 100644 acceptance/bundle/deploy/immutable-permissions-change/test.toml diff --git a/acceptance/bundle/deploy/immutable-permissions-change/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable-permissions-change/databricks.yml.tmpl new file mode 100644 index 00000000000..e23e6f3af5c --- /dev/null +++ b/acceptance/bundle/deploy/immutable-permissions-change/databricks.yml.tmpl @@ -0,0 +1,19 @@ +bundle: + name: test-bundle-immutable-perms-$UNIQUE_NAME + +experimental: + immutable_folder: true + +resources: + jobs: + my_job: + name: my job + tasks: + - task_key: spark_python_task + spark_python_task: + python_file: ./src/main.py + environment_key: env + environments: + - environment_key: env + spec: + environment_version: "4" diff --git a/acceptance/bundle/deploy/immutable-permissions-change/out.test.toml b/acceptance/bundle/deploy/immutable-permissions-change/out.test.toml new file mode 100644 index 00000000000..e90b6d5d1ba --- /dev/null +++ b/acceptance/bundle/deploy/immutable-permissions-change/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/deploy/immutable-permissions-change/output.txt b/acceptance/bundle/deploy/immutable-permissions-change/output.txt new file mode 100644 index 00000000000..68b79efc831 --- /dev/null +++ b/acceptance/bundle/deploy/immutable-permissions-change/output.txt @@ -0,0 +1,52 @@ + +=== Deploy without permissions +>>> [CLI] bundle deploy +Uploading immutable bundle snapshot... +Deploying resources... +Updating deployment state... +Deployment complete! +First snapshot path: /Workspace/Users/[UUID]/.snapshots/[UUID]/[SNAPSHOT_HASH]/files/src/main.py + +=== Add permissions and redeploy +>>> [CLI] bundle deploy +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:22:3 + +Uploading immutable bundle snapshot... +Deploying resources... +Updating deployment state... +Deployment complete! +Second snapshot path: /Workspace/Users/[UUID]/.snapshots/[UUID]/[SNAPSHOT_HASH]/files/src/main.py + +=== Verify snapshot path changed after permissions change +OK: snapshot path changed as expected + +>>> [CLI] bundle destroy --auto-approve +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:22:3 + +The following resources will be deleted: + delete resources.jobs.my_job + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-immutable-perms-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/deploy/immutable-permissions-change/script b/acceptance/bundle/deploy/immutable-permissions-change/script new file mode 100644 index 00000000000..458bfdd2c85 --- /dev/null +++ b/acceptance/bundle/deploy/immutable-permissions-change/script @@ -0,0 +1,35 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +title "Deploy without permissions" +trace $CLI bundle deploy + +JOB_ID=$($CLI bundle summary -o json | jq -r '.resources.jobs.my_job.id') +SNAPSHOT_PATH_1=$($CLI jobs get "$JOB_ID" | jq -r '.settings.tasks[0].spark_python_task.python_file') +echo "First snapshot path: $SNAPSHOT_PATH_1" + +title "Add permissions and redeploy" +cat >> databricks.yml <<'EOF' + +permissions: + - level: CAN_VIEW + group_name: admins +EOF + +trace $CLI bundle deploy + +SNAPSHOT_PATH_2=$($CLI jobs get "$JOB_ID" | jq -r '.settings.tasks[0].spark_python_task.python_file') +echo "Second snapshot path: $SNAPSHOT_PATH_2" + +title "Verify snapshot path changed after permissions change" +echo "" +if [ "$SNAPSHOT_PATH_1" = "$SNAPSHOT_PATH_2" ]; then + echo "ERROR: snapshot path did not change after permissions were added" + exit 1 +else + echo "OK: snapshot path changed as expected" +fi diff --git a/acceptance/bundle/deploy/immutable-permissions-change/src/main.py b/acceptance/bundle/deploy/immutable-permissions-change/src/main.py new file mode 100644 index 00000000000..6c285f7e2f5 --- /dev/null +++ b/acceptance/bundle/deploy/immutable-permissions-change/src/main.py @@ -0,0 +1 @@ +print("Hello from Spark Python Task!") diff --git a/acceptance/bundle/deploy/immutable-permissions-change/test.toml b/acceptance/bundle/deploy/immutable-permissions-change/test.toml new file mode 100644 index 00000000000..3139210d0dc --- /dev/null +++ b/acceptance/bundle/deploy/immutable-permissions-change/test.toml @@ -0,0 +1,15 @@ +Local = true +Cloud = false # Temporary disable cloud tests until the API is fully available + +# immutable_folder only works with the direct engine. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + "databricks.yml", + ".databricks", +] + +[[Repls]] +# Replace snapshot hash with SNAPSHOT_HASH +Old = "[0-9a-f]{64}" +New = "[SNAPSHOT_HASH]" diff --git a/bundle/deploy/snapshot/path.go b/bundle/deploy/snapshot/path.go index 5aecc0148df..62a4b9f91ab 100644 --- a/bundle/deploy/snapshot/path.go +++ b/bundle/deploy/snapshot/path.go @@ -6,6 +6,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" "io" "os" @@ -24,11 +25,25 @@ import ( // the zip was built or the file's mtime. var zipEpoch = time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC) +// metadataFileName is the name of the metadata file embedded in every snapshot zip. +// It captures a hash of the ACL so that a change to top-level permissions always produces +// a different snapshot ID — necessary because immutable snapshots cannot be re-permissioned +// after creation. The hash avoids embedding principal names in the stored artifact. +const metadataFileName = ".databricks/snapshot-metadata.json" + +// snapshotMetadata is the structure written to metadataFileName inside the zip. +type snapshotMetadata struct { + // PermissionsHash is the SHA-256 hex digest of the JSON-serialized ACL. + PermissionsHash string `json:"permissions_hash"` +} + // BundleZip builds the zip that is uploaded to the snapshot API. // It contains: // - all files from the bundle sync root under the "files/" prefix, // selected with the same git-aware + include/exclude logic as files.Upload // - all built artifact files under the "artifacts/.internal/" prefix +// - a metadata file at metadataFileName that embeds the ACL so that any +// change to top-level permissions forces a new snapshot ID // // The snapshot ID is always IDFromContent(BundleZip(b)), ensuring the // pre-calculated path and the uploaded path are derived from the same content. @@ -44,6 +59,9 @@ func BundleZip(ctx context.Context, b *bundle.Bundle) ([]byte, int, error) { if err := addArtifactsToZip(zw, b); err != nil { return nil, 0, err } + if err := addMetadataToZip(zw, BuildACL(b)); err != nil { + return nil, 0, err + } if err := zw.Close(); err != nil { return nil, 0, err @@ -51,6 +69,30 @@ func BundleZip(ctx context.Context, b *bundle.Bundle) ([]byte, int, error) { return buf.Bytes(), fileCount, nil } +// addMetadataToZip writes the snapshot metadata file into the zip so that +// any change to the ACL changes the snapshot hash and forces a new snapshot. +func addMetadataToZip(zw *zip.Writer, acl []ACLEntry) error { + aclJSON, err := json.Marshal(acl) + if err != nil { + return fmt.Errorf("marshal ACL for permissions hash: %w", err) + } + data, err := json.Marshal(snapshotMetadata{PermissionsHash: IDFromContent(aclJSON)}) + if err != nil { + return fmt.Errorf("marshal snapshot metadata: %w", err) + } + h := &zip.FileHeader{ + Name: metadataFileName, + Method: zip.Deflate, + Modified: zipEpoch, + } + w, err := zw.CreateHeader(h) + if err != nil { + return fmt.Errorf("zip entry for %s: %w", metadataFileName, err) + } + _, err = w.Write(data) + return err +} + // IDFromContent returns the SHA-256 hex digest of content. func IDFromContent(content []byte) string { h := sha256.Sum256(content) diff --git a/bundle/deploy/snapshot/path_test.go b/bundle/deploy/snapshot/path_test.go index dd88a30e9ef..5f567d9f377 100644 --- a/bundle/deploy/snapshot/path_test.go +++ b/bundle/deploy/snapshot/path_test.go @@ -10,8 +10,10 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/deploy/snapshot" "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/service/iam" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -25,7 +27,7 @@ func makeBundleWithFiles(t *testing.T, files map[string]string) *bundle.Bundle { require.NoError(t, os.WriteFile(p, []byte(content), 0o644)) } root := vfs.MustNew(dir) - return &bundle.Bundle{ + b := &bundle.Bundle{ BundleRootPath: dir, SyncRoot: root, // WorktreeRoot = SyncRoot is the fallback used by LoadGitDetails when @@ -36,9 +38,12 @@ func makeBundleWithFiles(t *testing.T, files map[string]string) *bundle.Bundle { // The SyncDefaultPath mutator sets this to ["."] during initialize; // set it here since these tests bypass the mutator pipeline. Empty // sync paths select no files. - Sync: config.Sync{Paths: []string{"."}}, + Sync: config.Sync{Paths: []string{"."}}, + Workspace: config.Workspace{CurrentUser: &config.User{}}, }, } + b.Config.Workspace.CurrentUser.User = &iam.User{UserName: "tester@example.com"} + return b } func TestBundleZipIsDeterministic(t *testing.T) { @@ -134,3 +139,33 @@ func TestBundleZipDoNotStripNotebookExtensions(t *testing.T) { assert.True(t, slices.Contains(names, "files/src/my_notebook.ipynb"), "notebook should keep its extension") assert.True(t, slices.Contains(names, "files/src/script.py"), "regular Python file should keep its extension") } + +func TestBundleZipIncludesMetadataFile(t *testing.T) { + b := makeBundleWithFiles(t, map[string]string{"task.py": "x = 1"}) + + zipContent, _, err := snapshot.BundleZip(t.Context(), b) + require.NoError(t, err) + + names := zipEntryNames(t, zipContent) + assert.True(t, slices.Contains(names, ".databricks/snapshot-metadata.json"), "zip must contain the snapshot metadata file") +} + +func TestBundleZipChangesWithPermissions(t *testing.T) { + files := map[string]string{"task.py": "x = 1"} + + bNoPerms := makeBundleWithFiles(t, files) + + bWithPerms := makeBundleWithFiles(t, files) + bWithPerms.Config.Permissions = []resources.Permission{ + {Level: "CAN_VIEW", GroupName: "data-team"}, + } + + zipNoPerms, _, err := snapshot.BundleZip(t.Context(), bNoPerms) + require.NoError(t, err) + zipWithPerms, _, err := snapshot.BundleZip(t.Context(), bWithPerms) + require.NoError(t, err) + + assert.NotEqual(t, zipNoPerms, zipWithPerms, "adding top-level permissions must produce a different snapshot zip") + assert.NotEqual(t, snapshot.IDFromContent(zipNoPerms), snapshot.IDFromContent(zipWithPerms), + "snapshot IDs must differ when top-level permissions change") +}