diff --git a/.nextchanges/bundles/immutable-breakglass.md b/.nextchanges/bundles/immutable-breakglass.md new file mode 100644 index 00000000000..586600d58c4 --- /dev/null +++ b/.nextchanges/bundles/immutable-breakglass.md @@ -0,0 +1 @@ +* Immutable-folder deploys now refuse to deploy onto a snapshot that was modified outside the bundle (break glass); `bundle deploy --force` moves the deployment to a fresh snapshot. ([#6774](https://github.com/databricks/cli/pull/6774)) diff --git a/acceptance/bundle/deploy/immutable-dev-mode/out.test.toml b/acceptance/bundle/deploy/immutable-dev-mode/out.test.toml deleted file mode 100644 index a927a5fbc06..00000000000 --- a/acceptance/bundle/deploy/immutable-dev-mode/out.test.toml +++ /dev/null @@ -1,2 +0,0 @@ -Cloud = false -EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deploy/immutable-dev-mode/test.toml b/acceptance/bundle/deploy/immutable-dev-mode/test.toml deleted file mode 100644 index 267c545f8e0..00000000000 --- a/acceptance/bundle/deploy/immutable-dev-mode/test.toml +++ /dev/null @@ -1,14 +0,0 @@ -Cloud = false # Temporary disable cloud tests until the API is fully available - -# immutable_folder only works with the direct engine. - -Ignore = [ - "databricks.yml", - ".databricks", -] - -# Normalize the content-addressed snapshot hash so it doesn't need to be -# hardcoded in output.txt and the test stays stable across file changes. -[[Repls]] -Old = '[0-9a-f]{64}' -New = '[SNAPSHOT_HASH]' diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/out.test.toml b/acceptance/bundle/deploy/immutable-no-artifacts/out.test.toml deleted file mode 100644 index f7fafbcc731..00000000000 --- a/acceptance/bundle/deploy/immutable-no-artifacts/out.test.toml +++ /dev/null @@ -1,3 +0,0 @@ -Cloud = false -EnvMatrix.DMS = ["", "true"] -EnvMatrix.READPLAN = ["", "1"] diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/test.toml b/acceptance/bundle/deploy/immutable-no-artifacts/test.toml deleted file mode 100644 index a2373c4f892..00000000000 --- a/acceptance/bundle/deploy/immutable-no-artifacts/test.toml +++ /dev/null @@ -1,23 +0,0 @@ -Cloud = false # Temporary disable cloud tests until the API is fully available -EnvMatrix.READPLAN = ["", "1"] - -# immutable_folder only works with the direct engine. - -Ignore = [ - "databricks.yml", - ".databricks", - ".venv", - "*.pyc", -] - -# Normalize the content-addressed snapshot hash so it doesn't need to be -# hardcoded in output.txt and the test stays stable across file changes. -[[Repls]] -Old = '[0-9a-f]{64}' -New = '[SNAPSHOT_HASH]' - -# When READPLAN=1, "bundle deploy" is called as "bundle deploy --plan plan.json". -# Normalize so both variants produce identical output. -[[Repls]] -Old = ' --plan plan.json' -New = '' diff --git a/acceptance/bundle/deploy/immutable-permissions-change/out.test.toml b/acceptance/bundle/deploy/immutable-permissions-change/out.test.toml deleted file mode 100644 index a927a5fbc06..00000000000 --- a/acceptance/bundle/deploy/immutable-permissions-change/out.test.toml +++ /dev/null @@ -1,2 +0,0 @@ -Cloud = false -EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deploy/immutable-permissions-change/test.toml b/acceptance/bundle/deploy/immutable-permissions-change/test.toml deleted file mode 100644 index 0439d8243bc..00000000000 --- a/acceptance/bundle/deploy/immutable-permissions-change/test.toml +++ /dev/null @@ -1,13 +0,0 @@ -Cloud = false # Temporary disable cloud tests until the API is fully available - -# immutable_folder only works with the direct engine. - -Ignore = [ - "databricks.yml", - ".databricks", -] - -[[Repls]] -# Replace snapshot hash with SNAPSHOT_HASH -Old = "[0-9a-f]{64}" -New = "[SNAPSHOT_HASH]" diff --git a/acceptance/bundle/deploy/immutable/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/basic/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/deploy/immutable/databricks.yml.tmpl rename to acceptance/bundle/deploy/immutable/basic/databricks.yml.tmpl diff --git a/acceptance/bundle/deploy/immutable/basic/out.test.toml b/acceptance/bundle/deploy/immutable/basic/out.test.toml new file mode 100644 index 00000000000..f68b4a89c36 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/basic/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable/output.txt b/acceptance/bundle/deploy/immutable/basic/output.txt similarity index 73% rename from acceptance/bundle/deploy/immutable/output.txt rename to acceptance/bundle/deploy/immutable/basic/output.txt index 56de6c28d57..0930c3d5e51 100644 --- a/acceptance/bundle/deploy/immutable/output.txt +++ b/acceptance/bundle/deploy/immutable/basic/output.txt @@ -15,7 +15,7 @@ Building python_artifact... [ { "notebook_task": { - "notebook_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/notebook" + "notebook_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/notebook" }, "task_key": "notebook_task" }, @@ -30,7 +30,7 @@ Building python_artifact... { "environment_key": "env", "spark_python_task": { - "python_file": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/main.py" + "python_file": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py" }, "task_key": "spark_python_task" } @@ -42,7 +42,7 @@ Building python_artifact... "environment_key": "env", "spec": { "dependencies": [ - "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/artifacts/.internal/immutable-0.0.1-py3-none-any.whl" + "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/artifacts/.internal/immutable-0.0.1-py3-none-any.whl" ], "environment_version": "4" } @@ -54,8 +54,8 @@ Building python_artifact... "action": "create", "new_state": { "value": { - "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", + "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]", + "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot", "bundle_id": "[SNAPSHOT_HASH]", "acl": [ { @@ -63,7 +63,12 @@ Building python_artifact... "permission_level": "CAN_READ" } ], - "zip_path": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH].zip" + "can_manage": [ + { + "service_principal_name": "[USERNAME]" + } + ], + "zip_path": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH]-[UUID].zip" } } } @@ -76,14 +81,14 @@ Files: 0 uploaded, 0 deleted Resources: 2 created, 0 changed, 0 deleted, 0 unchanged >>> jq .settings.tasks job.json -"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/main.py" +"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py" >>> jq .settings.tasks job.json -"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/notebook" +"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/notebook" >>> jq .settings.environments[0].spec.dependencies job.json [ - "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/artifacts/.internal/immutable-0.0.1-py3-none-any.whl" + "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/artifacts/.internal/immutable-0.0.1-py3-none-any.whl" ] >>> title Check snapshot plan after deploy @@ -97,8 +102,8 @@ Building python_artifact... "action": "recreate", "new_state": { "value": { - "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", + "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]", + "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot", "bundle_id": "[SNAPSHOT_HASH]", "acl": [ { @@ -106,12 +111,17 @@ Building python_artifact... "permission_level": "CAN_READ" } ], - "zip_path": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH].zip" + "can_manage": [ + { + "service_principal_name": "[USERNAME]" + } + ], + "zip_path": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH]-[UUID].zip" } }, "remote_state": { - "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]" + "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]", + "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot" }, "changes": { "acl": { @@ -138,25 +148,39 @@ Building python_artifact... "new": "[SNAPSHOT_HASH]", "remote": "" }, + "can_manage": { + "action": "skip", + "reason": "missing_in_remote", + "old": [ + { + "service_principal_name": "[USERNAME]" + } + ], + "new": [ + { + "service_principal_name": "[USERNAME]" + } + ] + }, "full_path": { "action": "recreate", "reason": "immutable", - "old": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "new": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "remote": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]" + "old": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot", + "new": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot", + "remote": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot" }, "relative_path": { "action": "recreate", "reason": "immutable", - "old": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "new": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "remote": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]" + "old": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]", + "new": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]", + "remote": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]" }, "zip_path": { "action": "recreate", "reason": "immutable", - "old": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH].zip", - "new": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH].zip", + "old": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH]-[UUID].zip", + "new": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH]-[UUID].zip", "remote": "" } } diff --git a/acceptance/bundle/deploy/immutable/pyproject.toml b/acceptance/bundle/deploy/immutable/basic/pyproject.toml similarity index 100% rename from acceptance/bundle/deploy/immutable/pyproject.toml rename to acceptance/bundle/deploy/immutable/basic/pyproject.toml diff --git a/acceptance/bundle/deploy/immutable/script b/acceptance/bundle/deploy/immutable/basic/script similarity index 100% rename from acceptance/bundle/deploy/immutable/script rename to acceptance/bundle/deploy/immutable/basic/script diff --git a/acceptance/bundle/deploy/immutable/src/immutable/__init__.py b/acceptance/bundle/deploy/immutable/basic/src/immutable/__init__.py similarity index 100% rename from acceptance/bundle/deploy/immutable/src/immutable/__init__.py rename to acceptance/bundle/deploy/immutable/basic/src/immutable/__init__.py diff --git a/acceptance/bundle/deploy/immutable/src/immutable/main.py b/acceptance/bundle/deploy/immutable/basic/src/immutable/main.py similarity index 100% rename from acceptance/bundle/deploy/immutable/src/immutable/main.py rename to acceptance/bundle/deploy/immutable/basic/src/immutable/main.py diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/src/main.py b/acceptance/bundle/deploy/immutable/basic/src/main.py similarity index 100% rename from acceptance/bundle/deploy/immutable-no-artifacts/src/main.py rename to acceptance/bundle/deploy/immutable/basic/src/main.py diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/src/notebook.py b/acceptance/bundle/deploy/immutable/basic/src/notebook.py similarity index 100% rename from acceptance/bundle/deploy/immutable-no-artifacts/src/notebook.py rename to acceptance/bundle/deploy/immutable/basic/src/notebook.py diff --git a/acceptance/bundle/deploy/immutable/basic/test.toml b/acceptance/bundle/deploy/immutable/basic/test.toml new file mode 100644 index 00000000000..dc5a1df1c80 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/basic/test.toml @@ -0,0 +1,7 @@ +Ignore = [ + "dist", + "build", + ".venv", + "*.pyc", + "src/*.egg-info", +] diff --git a/acceptance/bundle/deploy/immutable/breakglass/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/breakglass/databricks.yml.tmpl new file mode 100644 index 00000000000..671725dcf5c --- /dev/null +++ b/acceptance/bundle/deploy/immutable/breakglass/databricks.yml.tmpl @@ -0,0 +1,19 @@ +bundle: + name: test-bundle-immutable-breakglass-$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/breakglass/out.test.toml b/acceptance/bundle/deploy/immutable/breakglass/out.test.toml new file mode 100644 index 00000000000..f68b4a89c36 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/breakglass/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable/breakglass/output.txt b/acceptance/bundle/deploy/immutable/breakglass/output.txt new file mode 100644 index 00000000000..ba71879f6f0 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/breakglass/output.txt @@ -0,0 +1,42 @@ + +=== Deploy the immutable snapshot +>>> [CLI] bundle deploy +Created internal_immutable_snapshots.immutable +Created jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged +First snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py + +=== Break the glass out of band +>>> [CLI] api post /api/2.0/snapshots:breakGlass --json {"snapshot_content_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot"} +true + +=== Deploy without --force fails +>>> musterr [CLI] bundle deploy +Error: the last deployment was modified in the workspace. Changes might be overwritten by this deployment. To confirm it, use the --force flag + +Files: 0 uploaded, 0 deleted + +=== Deploy with --force moves to a new snapshot +>>> [CLI] bundle deploy --force +Recreated internal_immutable_snapshots.immutable +Updated jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 1 created, 1 changed, 1 deleted, 0 unchanged +Second snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py +OK: --force created a new snapshot path + +=== Redeploy reuses the new snapshot +>>> [CLI] bundle deploy +Files: 0 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 2 unchanged +Third snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py +OK: new snapshot reused on redeploy + +>>> [CLI] bundle destroy --auto-approve +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-breakglass-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/deploy/immutable/breakglass/script b/acceptance/bundle/deploy/immutable/breakglass/script new file mode 100644 index 00000000000..b504207ce10 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/breakglass/script @@ -0,0 +1,42 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +title "Deploy the immutable snapshot" +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 "Break the glass out of band" +# Breaking the glass grants write access to the snapshot content path (the folder holding files/ +# and artifacts/) and marks the snapshot modified (dirty) for good. +CONTENT_PATH="${SNAPSHOT_PATH_1%/files/src/main.py}" +trace $CLI api post /api/2.0/snapshots:breakGlass --json "{\"snapshot_content_path\": \"$CONTENT_PATH\"}" | jq .status.dirty + +title "Deploy without --force fails" +trace musterr $CLI bundle deploy + +title "Deploy with --force moves to a new snapshot" +trace $CLI bundle deploy --force +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" +if [ "$SNAPSHOT_PATH_1" = "$SNAPSHOT_PATH_2" ]; then + echo "ERROR: snapshot path did not change after --force" + exit 1 +fi +echo "OK: --force created a new snapshot path" + +title "Redeploy reuses the new snapshot" +trace $CLI bundle deploy +SNAPSHOT_PATH_3=$($CLI jobs get "$JOB_ID" | jq -r '.settings.tasks[0].spark_python_task.python_file') +echo "Third snapshot path: $SNAPSHOT_PATH_3" +if [ "$SNAPSHOT_PATH_2" != "$SNAPSHOT_PATH_3" ]; then + echo "ERROR: new snapshot path was not reused on redeploy" + exit 1 +fi +echo "OK: new snapshot reused on redeploy" diff --git a/acceptance/bundle/deploy/immutable-permissions-change/src/main.py b/acceptance/bundle/deploy/immutable/breakglass/src/main.py similarity index 100% rename from acceptance/bundle/deploy/immutable-permissions-change/src/main.py rename to acceptance/bundle/deploy/immutable/breakglass/src/main.py diff --git a/acceptance/bundle/deploy/immutable/breakglass/test.toml b/acceptance/bundle/deploy/immutable/breakglass/test.toml new file mode 100644 index 00000000000..07b549c7195 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/breakglass/test.toml @@ -0,0 +1,4 @@ +# The test breaks the glass with "api post /api/2.0/snapshots:breakGlass". On Windows the script +# runs under MSYS2, which rewrites leading-slash arguments to Windows paths before the CLI sees +# them. Setting this environment variable disables that conversion. +Env.MSYS_NO_PATHCONV = "1" diff --git a/acceptance/bundle/deploy/immutable/can-manage/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/can-manage/databricks.yml.tmpl new file mode 100644 index 00000000000..96e57c53fe5 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/can-manage/databricks.yml.tmpl @@ -0,0 +1,23 @@ +bundle: + name: test-bundle-immutable-can-manage-$UNIQUE_NAME + +experimental: + immutable_folder: true + +permissions: + - level: CAN_MANAGE + user_name: $TEST_USER_EMAIL + +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/can-manage/out.test.toml b/acceptance/bundle/deploy/immutable/can-manage/out.test.toml new file mode 100644 index 00000000000..3970ea98616 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/can-manage/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +CloudEnvs.gcp = false +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable/can-manage/output.txt b/acceptance/bundle/deploy/immutable/can-manage/output.txt new file mode 100644 index 00000000000..cdcca39708d --- /dev/null +++ b/acceptance/bundle/deploy/immutable/can-manage/output.txt @@ -0,0 +1,51 @@ + +=== Deploy with a CAN_MANAGE permission entry +>>> [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: + - service_principal_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 + +Created internal_immutable_snapshots.immutable +Created jobs.my_job +Created jobs.my_job.permissions +Files: 0 uploaded, 0 deleted +Resources: 3 created, 0 changed, 0 deleted, 0 unchanged + +=== Check can_manage_principals sent to the snapshot create API +>>> print_requests.py //api/2.0/snapshots +[ + { + "service_principal_name": "[USERNAME]" + }, + { + "user_name": "deco-test-user@databricks.com" + } +] + +>>> [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: + - service_principal_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 + +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-can-manage-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/deploy/immutable/can-manage/script b/acceptance/bundle/deploy/immutable/can-manage/script new file mode 100644 index 00000000000..1bf48588c08 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/can-manage/script @@ -0,0 +1,17 @@ +if [ -z "$CLOUD_ENV" ]; then + export TEST_USER_EMAIL="deco-test-user@databricks.com" +fi + +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve + rm -f out.requests.txt +} +trap cleanup EXIT + +title "Deploy with a CAN_MANAGE permission entry" +trace $CLI bundle deploy + +title "Check can_manage_principals sent to the snapshot create API" +trace print_requests.py //api/2.0/snapshots | jq '.body.multipart_form.can_manage_principals | fromjson' diff --git a/acceptance/bundle/deploy/immutable/src/main.py b/acceptance/bundle/deploy/immutable/can-manage/src/main.py similarity index 100% rename from acceptance/bundle/deploy/immutable/src/main.py rename to acceptance/bundle/deploy/immutable/can-manage/src/main.py diff --git a/acceptance/bundle/deploy/immutable/can-manage/test.toml b/acceptance/bundle/deploy/immutable/can-manage/test.toml new file mode 100644 index 00000000000..9b8d29984a9 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/can-manage/test.toml @@ -0,0 +1,6 @@ +# The snapshot create request carries can_manage_principals, which the test reads back from the +# recorded request. +RecordRequests = true + +# The current user on GCP is a user and not a SP, so the deploying identity is sent as user_name. +CloudEnvs.gcp = false diff --git a/acceptance/bundle/deploy/immutable/destroy-redeploy/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/destroy-redeploy/databricks.yml.tmpl new file mode 100644 index 00000000000..a1729107bdb --- /dev/null +++ b/acceptance/bundle/deploy/immutable/destroy-redeploy/databricks.yml.tmpl @@ -0,0 +1,19 @@ +bundle: + name: test-bundle-immutable-redeploy-$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/destroy-redeploy/out.test.toml b/acceptance/bundle/deploy/immutable/destroy-redeploy/out.test.toml new file mode 100644 index 00000000000..f68b4a89c36 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/destroy-redeploy/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable/destroy-redeploy/output.txt b/acceptance/bundle/deploy/immutable/destroy-redeploy/output.txt new file mode 100644 index 00000000000..cf05c9e7e33 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/destroy-redeploy/output.txt @@ -0,0 +1,58 @@ + +=== Deploy the immutable snapshot +>>> [CLI] bundle deploy +Created internal_immutable_snapshots.immutable +Created jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged +First snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py + +=== Destroy the bundle +>>> [CLI] bundle destroy --auto-approve +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-redeploy-[UNIQUE_NAME]/default + +Destroy: 1 deleted + +=== Deploy again moves to a new snapshot instead of the one left behind +>>> [CLI] bundle deploy +Created internal_immutable_snapshots.immutable +Created jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged +Second snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py +OK: deployment moved to a new snapshot + +=== Redeploy reuses the new snapshot +>>> [CLI] bundle deploy +Files: 0 uploaded, 0 deleted +Resources: 0 created, 0 changed, 0 deleted, 2 unchanged +Third snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py +OK: new snapshot reused on redeploy + +=== Destroy and deploy again moves to yet another snapshot +>>> [CLI] bundle destroy --auto-approve +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-redeploy-[UNIQUE_NAME]/default + +Destroy: 1 deleted + +>>> [CLI] bundle deploy +Created internal_immutable_snapshots.immutable +Created jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged +Fourth snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py +OK: deployment moved to a new snapshot + +>>> [CLI] bundle destroy --auto-approve +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-redeploy-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/deploy/immutable/destroy-redeploy/script b/acceptance/bundle/deploy/immutable/destroy-redeploy/script new file mode 100644 index 00000000000..43618ed3886 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/destroy-redeploy/script @@ -0,0 +1,51 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +snapshot_path() { + JOB_ID=$($CLI bundle summary -o json | jq -r '.resources.jobs.my_job.id') + $CLI jobs get "$JOB_ID" | jq -r '.settings.tasks[0].spark_python_task.python_file' +} + +title "Deploy the immutable snapshot" +trace $CLI bundle deploy +SNAPSHOT_PATH_1=$(snapshot_path) +echo "First snapshot path: $SNAPSHOT_PATH_1" + +title "Destroy the bundle" +# The snapshot is immutable and stays in the workspace; destroy only drops it from the state. +trace $CLI bundle destroy --auto-approve + +title "Deploy again moves to a new snapshot instead of the one left behind" +trace $CLI bundle deploy +SNAPSHOT_PATH_2=$(snapshot_path) +echo "Second snapshot path: $SNAPSHOT_PATH_2" +if [ "$SNAPSHOT_PATH_1" = "$SNAPSHOT_PATH_2" ]; then + echo "ERROR: deployment reused the snapshot left behind by destroy" + exit 1 +fi +echo "OK: deployment moved to a new snapshot" + +title "Redeploy reuses the new snapshot" +trace $CLI bundle deploy +SNAPSHOT_PATH_3=$(snapshot_path) +echo "Third snapshot path: $SNAPSHOT_PATH_3" +if [ "$SNAPSHOT_PATH_2" != "$SNAPSHOT_PATH_3" ]; then + echo "ERROR: new snapshot path was not reused on redeploy" + exit 1 +fi +echo "OK: new snapshot reused on redeploy" + +title "Destroy and deploy again moves to yet another snapshot" +trace $CLI bundle destroy --auto-approve +trace $CLI bundle deploy +SNAPSHOT_PATH_4=$(snapshot_path) +echo "Fourth snapshot path: $SNAPSHOT_PATH_4" +if [ "$SNAPSHOT_PATH_4" = "$SNAPSHOT_PATH_1" ] || [ "$SNAPSHOT_PATH_4" = "$SNAPSHOT_PATH_2" ]; then + echo "ERROR: deployment reused a snapshot left behind by destroy" + exit 1 +fi +echo "OK: deployment moved to a new snapshot" diff --git a/acceptance/bundle/deploy/immutable/destroy-redeploy/src/main.py b/acceptance/bundle/deploy/immutable/destroy-redeploy/src/main.py new file mode 100644 index 00000000000..6c285f7e2f5 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/destroy-redeploy/src/main.py @@ -0,0 +1 @@ +print("Hello from Spark Python Task!") diff --git a/acceptance/bundle/deploy/immutable-dev-mode/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/dev-mode/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/deploy/immutable-dev-mode/databricks.yml.tmpl rename to acceptance/bundle/deploy/immutable/dev-mode/databricks.yml.tmpl diff --git a/acceptance/bundle/deploy/immutable/dev-mode/out.test.toml b/acceptance/bundle/deploy/immutable/dev-mode/out.test.toml new file mode 100644 index 00000000000..f68b4a89c36 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/dev-mode/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable-dev-mode/output.txt b/acceptance/bundle/deploy/immutable/dev-mode/output.txt similarity index 95% rename from acceptance/bundle/deploy/immutable-dev-mode/output.txt rename to acceptance/bundle/deploy/immutable/dev-mode/output.txt index 9d38d6c271e..51b14e79830 100644 --- a/acceptance/bundle/deploy/immutable-dev-mode/output.txt +++ b/acceptance/bundle/deploy/immutable/dev-mode/output.txt @@ -26,7 +26,7 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged >>> [CLI] jobs get [MY_JOB_ID] { "name": "[dev [USERNAME]] my job", - "python_file": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/main.py" + "python_file": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py" } >>> [CLI] bundle destroy -t dev --auto-approve diff --git a/acceptance/bundle/deploy/immutable-dev-mode/script b/acceptance/bundle/deploy/immutable/dev-mode/script similarity index 100% rename from acceptance/bundle/deploy/immutable-dev-mode/script rename to acceptance/bundle/deploy/immutable/dev-mode/script diff --git a/acceptance/bundle/deploy/immutable-dev-mode/src/main.py b/acceptance/bundle/deploy/immutable/dev-mode/src/main.py similarity index 100% rename from acceptance/bundle/deploy/immutable-dev-mode/src/main.py rename to acceptance/bundle/deploy/immutable/dev-mode/src/main.py diff --git a/acceptance/bundle/deploy/immutable/legacy-state/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/legacy-state/databricks.yml.tmpl new file mode 100644 index 00000000000..202edfebf32 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/legacy-state/databricks.yml.tmpl @@ -0,0 +1,19 @@ +bundle: + name: test-bundle-immutable-legacy-$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/legacy-state/out.test.toml b/acceptance/bundle/deploy/immutable/legacy-state/out.test.toml new file mode 100644 index 00000000000..f68b4a89c36 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/legacy-state/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable/legacy-state/output.txt b/acceptance/bundle/deploy/immutable/legacy-state/output.txt new file mode 100644 index 00000000000..b9858f2b3c0 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/legacy-state/output.txt @@ -0,0 +1,34 @@ + +=== Deploy an immutable snapshot +>>> [CLI] bundle deploy +Created internal_immutable_snapshots.immutable +Created jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== Rewrite the state the way a CLI without this feature wrote it +=== Deploying on top of that state succeeds and moves to the current layout +>>> [CLI] bundle deploy +Recreated internal_immutable_snapshots.immutable +Updated jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 1 created, 1 changed, 1 deleted, 0 unchanged +Snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py +OK: deployed to the current snapshot layout + +=== The check still applies to a deployment carried over from the older layout +>>> [CLI] api post /api/2.0/snapshots:breakGlass --json {"snapshot_content_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot"} +true + +>>> musterr [CLI] bundle deploy +Error: the last deployment was modified in the workspace. Changes might be overwritten by this deployment. To confirm it, use the --force flag + +Files: 0 uploaded, 0 deleted + +>>> [CLI] bundle destroy --auto-approve +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-legacy-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/deploy/immutable/legacy-state/script b/acceptance/bundle/deploy/immutable/legacy-state/script new file mode 100644 index 00000000000..8f6fba02120 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/legacy-state/script @@ -0,0 +1,31 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +title "Deploy an immutable snapshot" +trace $CLI bundle deploy + +title "Rewrite the state the way a CLI without this feature wrote it" +# Back then full_path had no trailing "/snapshot" segment. +STATE=.databricks/bundle/default/resources.json +jq '.state["resources.internal_immutable_snapshots.immutable"].state.full_path |= rtrimstr("/snapshot")' "$STATE" > tmp.json +mv tmp.json "$STATE" + +title "Deploying on top of that state succeeds and moves to the current layout" +trace $CLI bundle deploy + +JOB_ID=$($CLI bundle summary -o json | jq -r '.resources.jobs.my_job.id') +SNAPSHOT_PATH=$($CLI jobs get "$JOB_ID" | jq -r '.settings.tasks[0].spark_python_task.python_file') +echo "Snapshot path: $SNAPSHOT_PATH" +case "$SNAPSHOT_PATH" in + */snapshot/files/src/main.py) echo "OK: deployed to the current snapshot layout" ;; + *) echo "ERROR: unexpected snapshot path"; exit 1 ;; +esac + +title "The check still applies to a deployment carried over from the older layout" +CONTENT_PATH="${SNAPSHOT_PATH%/files/src/main.py}" +trace $CLI api post /api/2.0/snapshots:breakGlass --json "{\"snapshot_content_path\": \"$CONTENT_PATH\"}" | jq .status.dirty +trace musterr $CLI bundle deploy diff --git a/acceptance/bundle/deploy/immutable/legacy-state/src/main.py b/acceptance/bundle/deploy/immutable/legacy-state/src/main.py new file mode 100644 index 00000000000..6c285f7e2f5 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/legacy-state/src/main.py @@ -0,0 +1 @@ +print("Hello from Spark Python Task!") diff --git a/acceptance/bundle/deploy/immutable/legacy-state/test.toml b/acceptance/bundle/deploy/immutable/legacy-state/test.toml new file mode 100644 index 00000000000..5d348f4d12f --- /dev/null +++ b/acceptance/bundle/deploy/immutable/legacy-state/test.toml @@ -0,0 +1,8 @@ +# The test rewrites the deployment state by hand, which the recording service refuses because it +# no longer matches the deployment it has recorded, so run without deployment history. +EnvMatrix.DMS = [""] + +# The test breaks the glass with "api post /api/2.0/snapshots:breakGlass". On Windows the script +# runs under MSYS2, which rewrites leading-slash arguments to Windows paths before the CLI sees +# them. Setting this environment variable disables that conversion. +Env.MSYS_NO_PATHCONV = "1" diff --git a/acceptance/bundle/deploy/immutable/missing-snapshot/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/missing-snapshot/databricks.yml.tmpl new file mode 100644 index 00000000000..e44ed8ec436 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/missing-snapshot/databricks.yml.tmpl @@ -0,0 +1,19 @@ +bundle: + name: test-bundle-immutable-missing-$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/missing-snapshot/out.test.toml b/acceptance/bundle/deploy/immutable/missing-snapshot/out.test.toml new file mode 100644 index 00000000000..f68b4a89c36 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/missing-snapshot/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable/missing-snapshot/output.txt b/acceptance/bundle/deploy/immutable/missing-snapshot/output.txt new file mode 100644 index 00000000000..c938ad1143f --- /dev/null +++ b/acceptance/bundle/deploy/immutable/missing-snapshot/output.txt @@ -0,0 +1,23 @@ + +=== Deploy an immutable snapshot +>>> [CLI] bundle deploy +Created internal_immutable_snapshots.immutable +Created jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +=== Point the state at a snapshot that is not there +=== Deploying still works instead of failing on the snapshot that is not there +>>> [CLI] bundle deploy +Recreated internal_immutable_snapshots.immutable +Updated jobs.my_job +Files: 0 uploaded, 0 deleted +Resources: 1 created, 1 changed, 1 deleted, 0 unchanged + +>>> [CLI] bundle destroy --auto-approve +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-missing-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/deploy/immutable/missing-snapshot/script b/acceptance/bundle/deploy/immutable/missing-snapshot/script new file mode 100644 index 00000000000..d1a54836020 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/missing-snapshot/script @@ -0,0 +1,20 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +title "Deploy an immutable snapshot" +trace $CLI bundle deploy + +title "Point the state at a snapshot that is not there" +# This is what a deployment made by a CLI without this feature looks like: its content sits one +# level above the content subfolder, so nothing is stored where the snapshot is looked up now. +# The same shape arises when the snapshot folder is deleted out of band. +STATE=.databricks/bundle/default/resources.json +jq '.state["resources.internal_immutable_snapshots.immutable"].state.relative_path += "-missing"' "$STATE" > tmp.json +mv tmp.json "$STATE" + +title "Deploying still works instead of failing on the snapshot that is not there" +trace $CLI bundle deploy diff --git a/acceptance/bundle/deploy/immutable/missing-snapshot/src/main.py b/acceptance/bundle/deploy/immutable/missing-snapshot/src/main.py new file mode 100644 index 00000000000..6c285f7e2f5 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/missing-snapshot/src/main.py @@ -0,0 +1 @@ +print("Hello from Spark Python Task!") diff --git a/acceptance/bundle/deploy/immutable/missing-snapshot/test.toml b/acceptance/bundle/deploy/immutable/missing-snapshot/test.toml new file mode 100644 index 00000000000..00611b6195e --- /dev/null +++ b/acceptance/bundle/deploy/immutable/missing-snapshot/test.toml @@ -0,0 +1,7 @@ +# The test rewrites the deployment state by hand, which the recording service refuses because it +# no longer matches the deployment it has recorded, so run without deployment history. +EnvMatrix.DMS = [""] + +Ignore = [ + "tmp.*", +] diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/.gitignore b/acceptance/bundle/deploy/immutable/no-artifacts/.gitignore similarity index 100% rename from acceptance/bundle/deploy/immutable-no-artifacts/.gitignore rename to acceptance/bundle/deploy/immutable/no-artifacts/.gitignore diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/no-artifacts/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/deploy/immutable-no-artifacts/databricks.yml.tmpl rename to acceptance/bundle/deploy/immutable/no-artifacts/databricks.yml.tmpl diff --git a/acceptance/bundle/deploy/immutable/no-artifacts/out.test.toml b/acceptance/bundle/deploy/immutable/no-artifacts/out.test.toml new file mode 100644 index 00000000000..de136de256d --- /dev/null +++ b/acceptance/bundle/deploy/immutable/no-artifacts/out.test.toml @@ -0,0 +1,3 @@ +Cloud = true +EnvMatrix.DMS = [""] +EnvMatrix.READPLAN = ["", "1"] diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/output.txt b/acceptance/bundle/deploy/immutable/no-artifacts/output.txt similarity index 76% rename from acceptance/bundle/deploy/immutable-no-artifacts/output.txt rename to acceptance/bundle/deploy/immutable/no-artifacts/output.txt index 2fd5d2c3ef1..96f778441ac 100644 --- a/acceptance/bundle/deploy/immutable-no-artifacts/output.txt +++ b/acceptance/bundle/deploy/immutable/no-artifacts/output.txt @@ -15,16 +15,16 @@ Validation OK! { "notebook_task": { "base_parameters": { - "path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/some_path" + "path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/some_path" }, - "notebook_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/notebook" + "notebook_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/notebook" }, "task_key": "notebook_task" }, { "environment_key": "env", "spark_python_task": { - "python_file": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/main.py" + "python_file": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py" }, "task_key": "spark_python_task" } @@ -35,8 +35,8 @@ Validation OK! "action": "create", "new_state": { "value": { - "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", + "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]", + "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot", "bundle_id": "[SNAPSHOT_HASH]", "acl": [ { @@ -44,7 +44,12 @@ Validation OK! "permission_level": "CAN_READ" } ], - "zip_path": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH].zip" + "can_manage": [ + { + "service_principal_name": "[USERNAME]" + } + ], + "zip_path": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH]-[UUID].zip" } } } @@ -56,13 +61,13 @@ Files: 0 uploaded, 0 deleted Resources: 2 created, 0 changed, 0 deleted, 0 unchanged >>> jq .settings.tasks job.json -"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/main.py" +"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py" >>> jq .settings.tasks job.json -"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/notebook" +"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/notebook" >>> jq .settings.tasks job.json -"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/some_path" +"/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/some_path" >>> title Check snapshot plan after deploy: @@ -73,8 +78,8 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged { "action": "skip", "remote_state": { - "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]", - "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]" + "relative_path": "[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]", + "full_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot" }, "changes": { "acl": { @@ -101,11 +106,25 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged "new": "[SNAPSHOT_HASH]", "remote": "" }, + "can_manage": { + "action": "skip", + "reason": "missing_in_remote", + "old": [ + { + "service_principal_name": "[USERNAME]" + } + ], + "new": [ + { + "service_principal_name": "[USERNAME]" + } + ] + }, "zip_path": { "action": "skip", "reason": "missing_in_remote", - "old": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH].zip", - "new": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH].zip", + "old": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH]-[UUID].zip", + "new": "[TEST_TMP_DIR]/.databricks/bundle/default/snapshots/[SNAPSHOT_HASH]-[UUID].zip", "remote": "" } } diff --git a/acceptance/bundle/deploy/immutable-no-artifacts/script b/acceptance/bundle/deploy/immutable/no-artifacts/script similarity index 100% rename from acceptance/bundle/deploy/immutable-no-artifacts/script rename to acceptance/bundle/deploy/immutable/no-artifacts/script diff --git a/acceptance/bundle/deploy/immutable/no-artifacts/src/main.py b/acceptance/bundle/deploy/immutable/no-artifacts/src/main.py new file mode 100644 index 00000000000..6c285f7e2f5 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/no-artifacts/src/main.py @@ -0,0 +1 @@ +print("Hello from Spark Python Task!") diff --git a/acceptance/bundle/deploy/immutable/src/notebook.py b/acceptance/bundle/deploy/immutable/no-artifacts/src/notebook.py similarity index 100% rename from acceptance/bundle/deploy/immutable/src/notebook.py rename to acceptance/bundle/deploy/immutable/no-artifacts/src/notebook.py diff --git a/acceptance/bundle/deploy/immutable/no-artifacts/test.toml b/acceptance/bundle/deploy/immutable/no-artifacts/test.toml new file mode 100644 index 00000000000..aff1c17e1c6 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/no-artifacts/test.toml @@ -0,0 +1,12 @@ +EnvMatrix.READPLAN = ["", "1"] + +Ignore = [ + ".venv", + "*.pyc", +] + +# When READPLAN=1, "bundle deploy" is called as "bundle deploy --plan plan.json". +# Normalize so both variants produce identical output. +[[Repls]] +Old = ' --plan plan.json' +New = '' diff --git a/acceptance/bundle/deploy/immutable/out.test.toml b/acceptance/bundle/deploy/immutable/out.test.toml deleted file mode 100644 index a927a5fbc06..00000000000 --- a/acceptance/bundle/deploy/immutable/out.test.toml +++ /dev/null @@ -1,2 +0,0 @@ -Cloud = false -EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/deploy/immutable-permissions-change/databricks.yml.tmpl b/acceptance/bundle/deploy/immutable/permissions-change/databricks.yml.tmpl similarity index 100% rename from acceptance/bundle/deploy/immutable-permissions-change/databricks.yml.tmpl rename to acceptance/bundle/deploy/immutable/permissions-change/databricks.yml.tmpl 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..f68b4a89c36 --- /dev/null +++ b/acceptance/bundle/deploy/immutable/permissions-change/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = [""] diff --git a/acceptance/bundle/deploy/immutable-permissions-change/output.txt b/acceptance/bundle/deploy/immutable/permissions-change/output.txt similarity index 90% rename from acceptance/bundle/deploy/immutable-permissions-change/output.txt rename to acceptance/bundle/deploy/immutable/permissions-change/output.txt index 7e45089b269..41d23e84549 100644 --- a/acceptance/bundle/deploy/immutable-permissions-change/output.txt +++ b/acceptance/bundle/deploy/immutable/permissions-change/output.txt @@ -5,7 +5,7 @@ Created internal_immutable_snapshots.immutable Created jobs.my_job Files: 0 uploaded, 0 deleted Resources: 2 created, 0 changed, 0 deleted, 0 unchanged -First snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/main.py +First snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py === Add permissions and redeploy >>> [CLI] bundle deploy @@ -15,18 +15,18 @@ If it is not included, CAN_MANAGE permissions are only applied if the present id Consider using a adding a top-level permissions section such as the following: permissions: - - user_name: [USERNAME] + - service_principal_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 +Created jobs.my_job.permissions Recreated internal_immutable_snapshots.immutable Updated jobs.my_job -Created jobs.my_job.permissions Files: 0 uploaded, 0 deleted Resources: 2 created, 1 changed, 1 deleted, 0 unchanged -Second snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/src/main.py +Second snapshot path: /Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/src/main.py === Verify snapshot path changed after permissions change OK: snapshot path changed as expected @@ -38,7 +38,7 @@ If it is not included, CAN_MANAGE permissions are only applied if the present id Consider using a adding a top-level permissions section such as the following: permissions: - - user_name: [USERNAME] + - service_principal_name: [USERNAME] level: CAN_MANAGE See https://docs.databricks.com/dev-tools/bundles/permissions.html to learn more about permission configuration. diff --git a/acceptance/bundle/deploy/immutable-permissions-change/script b/acceptance/bundle/deploy/immutable/permissions-change/script similarity index 100% rename from acceptance/bundle/deploy/immutable-permissions-change/script rename to acceptance/bundle/deploy/immutable/permissions-change/script 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/test.toml b/acceptance/bundle/deploy/immutable/test.toml index 81910694737..85e2650da1a 100644 --- a/acceptance/bundle/deploy/immutable/test.toml +++ b/acceptance/bundle/deploy/immutable/test.toml @@ -1,15 +1,15 @@ -Cloud = false # Temporary disable cloud tests until the API is fully available +Cloud = true -# immutable_folder only works with the direct engine. +# Skipping DMS tests for now because DMS do not support immutable resource yet +EnvMatrix.DMS = [""] + +# The deploying identity shows in the output keyed by its kind (can_manage principals, permission +# hints). Cloud tests deploy as a service principal, so do the same locally to get the same output. +IsServicePrincipal = true Ignore = [ - "dist", - "build", "databricks.yml", ".databricks", - ".venv", - "*.pyc", - "src/*.egg-info", ] [[Repls]] diff --git a/acceptance/bundle/dms/immutable-folder/output.txt b/acceptance/bundle/dms/immutable-folder/output.txt index a45aa8ba189..53ff59a9f4e 100644 --- a/acceptance/bundle/dms/immutable-folder/output.txt +++ b/acceptance/bundle/dms/immutable-folder/output.txt @@ -10,7 +10,7 @@ Resources: 2 created, 0 changed, 0 deleted, 0 unchanged { "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" + "file_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files" } } @@ -25,7 +25,7 @@ Resources: 1 created, 1 changed, 1 deleted, 0 unchanged { "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" + "file_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files" } } @@ -38,6 +38,6 @@ Resources: 0 created, 0 changed, 0 deleted, 2 unchanged { "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" + "file_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files" } } diff --git a/acceptance/bundle/refschema/out.fields.txt b/acceptance/bundle/refschema/out.fields.txt index 8eb16b169a5..5782cffec24 100644 --- a/acceptance/bundle/refschema/out.fields.txt +++ b/acceptance/bundle/refschema/out.fields.txt @@ -1088,6 +1088,11 @@ resources.internal_immutable_snapshots.*.acl[*].permission_level string INPUT ST resources.internal_immutable_snapshots.*.acl[*].service_principal_name string INPUT STATE resources.internal_immutable_snapshots.*.acl[*].user_name string INPUT STATE resources.internal_immutable_snapshots.*.bundle_id string INPUT STATE +resources.internal_immutable_snapshots.*.can_manage []snapshot.ManagePrincipal INPUT STATE +resources.internal_immutable_snapshots.*.can_manage[*] snapshot.ManagePrincipal INPUT STATE +resources.internal_immutable_snapshots.*.can_manage[*].group_name string INPUT STATE +resources.internal_immutable_snapshots.*.can_manage[*].service_principal_name string INPUT STATE +resources.internal_immutable_snapshots.*.can_manage[*].user_name string INPUT STATE resources.internal_immutable_snapshots.*.full_path string REMOTE STATE resources.internal_immutable_snapshots.*.relative_path string REMOTE STATE resources.internal_immutable_snapshots.*.remote_root string INPUT diff --git a/acceptance/bundle/resources/apps/immutable/output.txt b/acceptance/bundle/resources/apps/immutable/output.txt index c411677bca0..356636ef6f5 100644 --- a/acceptance/bundle/resources/apps/immutable/output.txt +++ b/acceptance/bundle/resources/apps/immutable/output.txt @@ -37,6 +37,6 @@ You can access the app at my-immutable-app-123.cloud.databricksapps.com "path": "/api/2.0/apps/my-immutable-app/deployments", "body": { "mode": "SNAPSHOT", - "source_code_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]/files/app" + "source_code_path": "/Workspace/Users/[UUID]/.snapshots/[SNAPSHOT_HASH]/[SNAPSHOT_HASH]-[UUID]/snapshot/files/app" } } diff --git a/bundle/config/resources/snapshot.go b/bundle/config/resources/snapshot.go index 95d98c89271..7aee13a1d08 100644 --- a/bundle/config/resources/snapshot.go +++ b/bundle/config/resources/snapshot.go @@ -47,12 +47,15 @@ type Snapshot struct { // ACL is the access control list applied to the uploaded snapshot, granting // CAN_READ to the deploying user and to every principal in bundle.permissions. ACL []snapshot.ACLEntry `json:"acl"` + // CanManage lists the principals allowed to break the glass on the snapshot, derived + // from top-level bundle.permissions entries with level CAN_MANAGE. + CanManage []snapshot.ManagePrincipal `json:"can_manage,omitempty"` // ZipPath is the local path of the bundle zip staged by the deploy pipeline. - // The file is named ".zip", so its base name is the content hash used - // as the last component of RelativePath. Storing the path (not the bytes) keeps - // the zip off the config, the state, and the plan file, and lets DoCreate stream - // it straight from disk at upload time. - // Example: "/.../.databricks/bundle/default/snapshots/e3b0c4...zip". + // The file is named "-.zip", and its base name is the last component + // of RelativePath. Storing the path (not the bytes) keeps the zip off the config, + // the state, and the plan file, and lets DoCreate stream it straight from disk at + // upload time. + // Example: "/.../.databricks/bundle/default/snapshots/e3b0c4...-0199e2c1-....zip". ZipPath string `json:"zip_path"` // RemoteRoot is the workspace directory that holds every snapshot for this user, // returned by the snapshot rootpath API. @@ -62,19 +65,20 @@ type Snapshot struct { Lifecycle Lifecycle `json:"-"` } -// RelativePath is the snapshot's location under RemoteRoot: "/". -// It is the resource ID; the last component is the zip content hash (the ZipPath base -// name), which makes the pre-computed path match the uploaded content. -// Example: "1a2b3c4d-.../e3b0c4...". +// RelativePath is the snapshot's location under RemoteRoot: "/". It is the +// resource ID. The name is the ZipPath base name, "-": every new snapshot gets a +// fresh one, because a path can be uploaded to only once (see snapshot.PlanUpload). +// Example: "1a2b3c4d-.../e3b0c4...-0199e2c1-...". func (s *Snapshot) RelativePath() string { - hash := strings.TrimSuffix(path.Base(s.ZipPath), ".zip") - return path.Join(s.BundleID, hash) + return path.Join(s.BundleID, strings.TrimSuffix(path.Base(s.ZipPath), ".zip")) } -// FullPath is the absolute workspace path of the snapshot: RemoteRoot + "/" + RelativePath. -// Example: "/Workspace/Users//.snapshots/1a2b3c4d-.../e3b0c4...". +// FullPath is the absolute workspace content path of the snapshot: +// RemoteRoot + "/" + RelativePath + "/" + ContentSubdir. The API stores the content under a +// fixed "snapshot" subfolder, and downstream resources / the inspect check reference it. +// Example: "/Workspace/Users//.snapshots/1a2b3c4d-.../e3b0c4.../snapshot". func (s *Snapshot) FullPath() string { - return path.Join(s.RemoteRoot, s.RelativePath()) + return path.Join(s.RemoteRoot, s.RelativePath(), snapshot.ContentSubdir) } func (s *Snapshot) Exists(ctx context.Context, w *databricks.WorkspaceClient, name string) (bool, error) { diff --git a/bundle/deploy/metadata/compute.go b/bundle/deploy/metadata/compute.go index 1fa4c3bf2d9..61b7029f35c 100644 --- a/bundle/deploy/metadata/compute.go +++ b/bundle/deploy/metadata/compute.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/metadata" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/log" @@ -127,6 +128,12 @@ func (m *compute) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics b.Metadata.Config.Presets.SourceLinkedDeployment = true } + // For immutable-folder deployments the content lives in a snapshot rather than a regular + // workspace directory. Record its path so downstream services (e.g. the jobs UI) can locate it. + if snap, ok := b.Config.Resources.Snapshots[resources.SnapshotResourceKey]; ok { + b.Metadata.Config.Workspace.SnapshotContentPath = snap.FullPath() + } + // Set the git folder path for deployments from the workspace if b.WorktreeRoot != nil && strings.HasPrefix(b.WorktreeRoot.Native(), "/Workspace/") { b.Metadata.Extra.GitFolderPath = b.WorktreeRoot.Native() diff --git a/bundle/deploy/metadata/compute_test.go b/bundle/deploy/metadata/compute_test.go index cb163914bdb..3d8ec0f9006 100644 --- a/bundle/deploy/metadata/compute_test.go +++ b/bundle/deploy/metadata/compute_test.go @@ -221,3 +221,24 @@ func TestComputeMetadataMutatorGitFolderPath(t *testing.T) { assert.Equal(t, gitFolderPath, b.Metadata.Extra.GitFolderPath) } + +func TestComputeMetadataMutatorSnapshotContentPath(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Snapshots: map[string]*resources.Snapshot{ + resources.SnapshotResourceKey: { + RemoteRoot: "/Workspace/Users/test@databricks.com/.snapshots", + BundleID: "bundle-id", + ZipPath: "/tmp/content-hash.zip", + }, + }, + }, + }, + } + + diags := bundle.Apply(t.Context(), b, Compute()) + require.NoError(t, diags.Error()) + + assert.Equal(t, "/Workspace/Users/test@databricks.com/.snapshots/bundle-id/content-hash/snapshot", b.Metadata.Config.Workspace.SnapshotContentPath) +} diff --git a/bundle/deploy/snapshot/upload.go b/bundle/deploy/snapshot/upload.go index 3b7d7d1fc1a..8ddc343203f 100644 --- a/bundle/deploy/snapshot/upload.go +++ b/bundle/deploy/snapshot/upload.go @@ -4,14 +4,23 @@ import ( "context" "crypto/sha256" "encoding/hex" + "encoding/json" + "errors" "fmt" "os" + "path" "path/filepath" + "reflect" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/iamutil" "github.com/databricks/cli/libs/snapshot" + "github.com/databricks/databricks-sdk-go/apierr" + "github.com/google/uuid" ) // fileLimitWarning is the file count above which immutable folder deployments may fail. @@ -19,9 +28,9 @@ const fileLimitWarning = 1000 type snapshotUpload struct { // clean is true for a fresh plan or deploy: the mutator discards zips staged by a - // previous run (so content-addressed ".zip" files don't accumulate) and - // stages the current one. It is false when applying a pre-existing plan - // (deploy --plan): the mutator reuses the zip staged when the plan was produced + // previous run (so they don't accumulate), decides the snapshot's path and stages the + // current zip. It is false when applying a pre-existing plan (deploy --plan): the mutator + // reuses the zip staged when the plan was produced, and with it the path the plan chose, // instead of rebuilding it. clean bool } @@ -32,11 +41,11 @@ type PlanUploadOptions struct { // PlanUpload returns a mutator that registers the immutable snapshot as an internal // 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 ".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 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. +// bundle zip, stages it under the bundle's local state directory as ".zip" (see +// snapshotName), 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 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} } @@ -63,10 +72,17 @@ func (m *snapshotUpload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn return nil } + snap := &resources.Snapshot{ + BundleID: BundleID(b), + ACL: BuildACL(b), + CanManage: BuildCanManage(b), + RemoteRoot: remoteRoot, + } + var zipPath string var diags diag.Diagnostics if m.clean { - zipPath, diags, err = stageZip(ctx, b) + zipPath, diags, err = stageZip(ctx, b, uploader, snap) } else { zipPath, err = stagedZipPath(ctx, b) } @@ -74,19 +90,23 @@ func (m *snapshotUpload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn 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), - } + snap.ZipPath = filepath.ToSlash(zipPath) + b.Config.Resources.Snapshots[resources.SnapshotResourceKey] = snap + return diags } // stageZip builds the bundle zip and stages it under the bundle's local state directory as -// ".zip", discarding zips staged by a previous run so content-addressed files don't +// ".zip" (see snapshotName), discarding zips staged by a previous run so they don't // accumulate. -func stageZip(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics, error) { +func stageZip(ctx context.Context, b *bundle.Bundle, uploader *snapshot.SnapshotClient, snap *resources.Snapshot) (string, diag.Diagnostics, error) { + // Check the previous snapshot before doing any work, so a deploy onto a snapshot that was + // modified outside the bundle fails before building a zip. + prev, err := checkPrevSnapshot(ctx, b, uploader, snap.RemoteRoot) + if err != nil { + return "", nil, err + } + zipContent, fileCount, err := BundleZip(ctx, b) if err != nil { return "", nil, fmt.Errorf("failed to build snapshot zip: %w", err) @@ -99,7 +119,11 @@ func stageZip(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics, if err != nil { return "", nil, err } - zipPath := filepath.Join(dir, snapshot.HashFromContent(zipContent)+".zip") + name, err := snapshotName(snap, prev, dir, snapshot.HashFromContent(zipContent)) + if err != nil { + return "", nil, err + } + zipPath := filepath.Join(dir, name+".zip") if err := os.WriteFile(zipPath, zipContent, 0o600); err != nil { return "", nil, fmt.Errorf("failed to write snapshot zip: %w", err) } @@ -114,6 +138,31 @@ func stageZip(ctx context.Context, b *bundle.Bundle) (string, diag.Diagnostics, return zipPath, diags, nil } +// snapshotName returns the name the zip is staged under, which is also the last component of +// the snapshot's path: "-". A snapshot cannot be deleted, so a path can be +// uploaded to only once and stays taken after "bundle destroy" or a deploy that moved on to other +// content. Every new snapshot therefore goes to a path of its own, made unique by a UUIDv7 +// (a timestamp followed by random bits). The previous snapshot's name is kept only when nothing +// about the snapshot would change, so the deploy leaves it in place instead of uploading it again. +func snapshotName(snap *resources.Snapshot, prev *dresources.SnapshotState, dir, hash string) (string, error) { + if prev != nil { + name := path.Base(prev.RelativePath) + reused := *snap + reused.ZipPath = filepath.ToSlash(filepath.Join(dir, name+".zip")) + // The name carries the hash of the content uploaded to it. The rest of the state has to + // match as well: any difference makes the engine recreate the snapshot, which needs a new path. + if strings.HasPrefix(name, hash+"-") && reflect.DeepEqual((&dresources.ResourceSnapshot{}).PrepareState(&reused), prev) { + return name, nil + } + } + + id, err := uuid.NewV7() + if err != nil { + return "", err + } + return hash + "-" + id.String(), nil +} + // 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) { @@ -155,3 +204,93 @@ func BuildACL(b *bundle.Bundle) []snapshot.ACLEntry { } return acl } + +// BuildCanManage constructs can_manage_principals for the snapshot upload: the principals that +// may break the glass on the snapshot. Those are the deploying identity plus everyone granted +// CAN_MANAGE in the top-level permissions section. The deploying identity is always included +// because the API rejects an empty list, and it is the one principal always available. The +// recommended permissions section names it with CAN_MANAGE as well, so skip it in the loop +// rather than sending it twice. +func BuildCanManage(b *bundle.Bundle) []snapshot.ManagePrincipal { + currentUser := b.Config.Workspace.CurrentUser.User + var canManage []snapshot.ManagePrincipal + if iamutil.IsServicePrincipal(currentUser) { + canManage = append(canManage, snapshot.ManagePrincipal{ + ServicePrincipalName: currentUser.UserName, + }) + } else { + canManage = append(canManage, snapshot.ManagePrincipal{ + UserName: currentUser.UserName, + }) + } + for _, p := range b.Config.Permissions { + if p.Level != "CAN_MANAGE" { + continue + } + canManage = append(canManage, snapshot.ManagePrincipal{ + UserName: p.UserName, + GroupName: p.GroupName, + ServicePrincipalName: p.ServicePrincipalName, + }) + } + return canManage +} + +// checkPrevSnapshot asks the backend whether the snapshot deployed last time was modified outside +// of the bundle, and returns its state when the deployment may keep using it: +// - the previous state while the snapshot is untouched; +// - nil when there is no previous snapshot, or it was modified and --force was given, which +// moves the deployment to a new snapshot; +// - an error when it was modified and --force was not given. +// +// The check has to happen here rather than in the resource's DoRead: it decides the snapshot's +// path, and resources referencing ${...full_path} resolve it from the desired state computed +// before any resource is read (see LookupReferencePreDeploy). Erroring unless --force also has no +// equivalent in the resource layer. CheckDashboardsModifiedRemotely is the same shape: read state, +// ask the API, refuse unless forced. +func checkPrevSnapshot(ctx context.Context, b *bundle.Bundle, uploader *snapshot.SnapshotClient, remoteRoot string) (*dresources.SnapshotState, error) { + // The deploy/plan pipeline opens the state DB for read before this runs, but callers that + // exercise PlanUpload in isolation (unit tests) may not. No open state means no previous + // snapshot to check, which is the first-deploy case. + if !b.DeploymentBundle.StateDB.IsOpen() { + return nil, nil + } + + entry, ok := b.DeploymentBundle.StateDB.GetResourceEntry(resources.SnapshotKey) + if !ok || len(entry.State) == 0 { + return nil, nil + } + + // Decode into the state type the engine persists, so the field names cannot drift apart. + var prev dresources.SnapshotState + if err := json.Unmarshal(entry.State, &prev); err != nil { + return nil, fmt.Errorf("reading previous snapshot state: %w", err) + } + if prev.RelativePath == "" { + return nil, nil + } + + // Compose the content path from relative_path rather than reading full_path: relative_path + // means the same thing in every CLI version, while full_path from a CLI that predates the + // content subfolder points one level too high. + contentPath := path.Join(remoteRoot, prev.RelativePath, snapshot.ContentSubdir) + + status, err := uploader.InspectSnapshot(ctx, contentPath) + // Nothing to protect when no snapshot is there: the deployment predates the content + // subfolder, or the snapshot was deleted out of band, or an earlier deploy never created it. + // Failing here would also be a dead end, since this runs before the --force check and so + // could not be forced past. + if errors.Is(err, apierr.ErrNotFound) { + return nil, nil + } + if err != nil { + return nil, err + } + if !status.Dirty { + return &prev, nil + } + if !b.Config.Bundle.Force { + return nil, errors.New("the last deployment was modified in the workspace. Changes might be overwritten by this deployment. To confirm it, use the --force flag") + } + return nil, nil +} diff --git a/bundle/deploy/snapshot/upload_warning_test.go b/bundle/deploy/snapshot/upload_warning_test.go index 85ff5a21b51..1d66b30d17b 100644 --- a/bundle/deploy/snapshot/upload_warning_test.go +++ b/bundle/deploy/snapshot/upload_warning_test.go @@ -6,14 +6,17 @@ import ( "math" "os" "path/filepath" + "strings" "testing" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/logdiag" + "github.com/databricks/cli/libs/snapshot" "github.com/databricks/cli/libs/testserver" "github.com/databricks/cli/libs/vfs" "github.com/databricks/databricks-sdk-go" @@ -90,6 +93,48 @@ func TestUploadNoWarningBelowFileLimit(t *testing.T) { assert.True(t, diags.HasError() == false && len(diags) == 0, "expected no diagnostics") } +// canManageBundle returns a bundle deployed by deployer@example.test with the given permissions. +func canManageBundle(permissions []resources.Permission) *bundle.Bundle { + return &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + CurrentUser: &config.User{ + User: &iam.User{UserName: "deployer@example.test"}, + }, + }, + Permissions: permissions, + }, + } +} + +func TestBuildCanManage(t *testing.T) { + b := canManageBundle([]resources.Permission{ + {Level: "CAN_MANAGE", UserName: "manager@example.test"}, + {Level: "CAN_VIEW", UserName: "viewer@example.test"}, + {Level: "CAN_MANAGE", GroupName: "admins"}, + {Level: "CAN_MANAGE", ServicePrincipalName: "sp-1"}, + }) + + // The deploying identity, then only the CAN_MANAGE principals. + assert.Equal(t, []snapshot.ManagePrincipal{ + {UserName: "deployer@example.test"}, + {UserName: "manager@example.test"}, + {GroupName: "admins"}, + {ServicePrincipalName: "sp-1"}, + }, BuildCanManage(b)) +} + +func TestBuildCanManageWithoutManagers(t *testing.T) { + b := canManageBundle([]resources.Permission{ + {Level: "CAN_VIEW", UserName: "viewer@example.test"}, + }) + + // The API rejects an empty list, so the deploying identity is always there. + assert.Equal(t, []snapshot.ManagePrincipal{ + {UserName: "deployer@example.test"}, + }, BuildCanManage(b)) +} + func TestUploadReusesStagedZipWhenNotClean(t *testing.T) { // clean=false is the deploy --plan path: the zip staged when the plan was produced is // registered on the config as is, without rebuilding or warning again, even for a @@ -121,3 +166,61 @@ func TestUploadNotCleanWithoutStagedZip(t *testing.T) { require.True(t, diags.HasError()) assert.Contains(t, diags[0].Summary, "run 'bundle plan' again") } + +func TestSnapshotName(t *testing.T) { + const hash = "e3b0c4" + const prevName = hash + "-0199e2c1-7b1a-7c2d-8e3f-4a5b6c7d8e9f" + snap := &resources.Snapshot{ + BundleID: "bundle-id", + ACL: []snapshot.ACLEntry{{UserName: "deployer@example.test", PermissionLevel: "CAN_READ"}}, + CanManage: []snapshot.ManagePrincipal{{UserName: "deployer@example.test"}}, + RemoteRoot: "/Workspace/Users/sp/.snapshots", + } + unchanged := &dresources.SnapshotState{ + RelativePath: "bundle-id/" + prevName, + FullPath: "/Workspace/Users/sp/.snapshots/bundle-id/" + prevName + "/snapshot", + BundleID: "bundle-id", + ACL: []snapshot.ACLEntry{{UserName: "deployer@example.test", PermissionLevel: "CAN_READ"}}, + CanManage: []snapshot.ManagePrincipal{{UserName: "deployer@example.test"}}, + ZipPath: "/stage/" + prevName + ".zip", + } + otherManagers := *unchanged + otherManagers.CanManage = []snapshot.ManagePrincipal{{GroupName: "admins"}} + + tests := []struct { + name string + prev *dresources.SnapshotState + dir string + hash string + reused bool + }{ + {name: "no previous snapshot", prev: nil, dir: "/stage", hash: hash, reused: false}, + {name: "unchanged", prev: unchanged, dir: "/stage", hash: hash, reused: true}, + {name: "content changed", prev: unchanged, dir: "/stage", hash: "a1b2c3", reused: false}, + {name: "staged elsewhere", prev: unchanged, dir: "/other", hash: hash, reused: false}, + {name: "managers changed", prev: &otherManagers, dir: "/stage", hash: hash, reused: false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + name, err := snapshotName(snap, tc.prev, tc.dir, tc.hash) + require.NoError(t, err) + if tc.reused { + assert.Equal(t, prevName, name) + return + } + assert.NotEqual(t, prevName, name) + assert.True(t, strings.HasPrefix(name, tc.hash+"-"), name) + }) + } +} + +func TestSnapshotNameIsUniquePerUpload(t *testing.T) { + snap := &resources.Snapshot{BundleID: "bundle-id"} + + first, err := snapshotName(snap, nil, "/stage", "e3b0c4") + require.NoError(t, err) + second, err := snapshotName(snap, nil, "/stage", "e3b0c4") + require.NoError(t, err) + + assert.NotEqual(t, first, second) +} diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 1b0da0602a2..140f3acf1e5 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -290,7 +290,7 @@ var testConfig map[string]any = map[string]any{ }, "internal_immutable_snapshots": &resources.Snapshot{ - RemoteRoot: "/Workspace/Users/" + testserver.TestUserSP.UserName + "/.snapshots", + RemoteRoot: "/Workspace/Users/" + testserver.SnapshotRootUUID + "/.snapshots", BundleID: "test-bundle-id", ACL: []snapshot.ACLEntry{{UserName: "user@example.com", PermissionLevel: "CAN_READ"}}, }, @@ -1054,7 +1054,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W require.NoError(t, os.WriteFile(zipPath, content, 0o600)) ss.ZipPath = filepath.ToSlash(zipPath) ss.RelativePath = ss.BundleID + "/" + hash - ss.FullPath = snap.RemoteRoot + "/" + ss.RelativePath + ss.FullPath = snap.RemoteRoot + "/" + ss.RelativePath + "/" + snapshot.ContentSubdir } ctx := t.Context() diff --git a/bundle/direct/dresources/snapshot.go b/bundle/direct/dresources/snapshot.go index 5c59e5110c8..02c562a0565 100644 --- a/bundle/direct/dresources/snapshot.go +++ b/bundle/direct/dresources/snapshot.go @@ -23,10 +23,12 @@ type SnapshotState struct { FullPath string `json:"full_path"` BundleID string `json:"bundle_id"` ACL []snapshot.ACLEntry `json:"acl"` + // CanManage lists the principals allowed to break the glass on the snapshot. + CanManage []snapshot.ManagePrincipal `json:"can_manage,omitempty"` // ZipPath locates the bundle zip staged locally by the deploy pipeline. It is // small enough to round-trip through the plan file, so deploying from a plan - // restores it directly (no re-injection needed). The file name is the content - // hash, which is also the last component of RelativePath. + // restores it directly (no re-injection needed). The file name is also the last + // component of RelativePath. ZipPath string `json:"zip_path"` } @@ -59,6 +61,7 @@ func (s *ResourceSnapshot) PrepareState(input *resources.Snapshot) *SnapshotStat FullPath: input.FullPath(), BundleID: input.BundleID, ACL: input.ACL, + CanManage: input.CanManage, ZipPath: input.ZipPath, } } @@ -69,6 +72,7 @@ func (s *ResourceSnapshot) RemapState(remote *SnapshotRemote) *SnapshotState { FullPath: remote.FullPath, BundleID: "", ACL: nil, + CanManage: nil, ZipPath: "", } } @@ -91,7 +95,7 @@ func (s *ResourceSnapshot) DoCreate(ctx context.Context, state *SnapshotState) ( } path := state.RelativePath - info, err := s.uploader.Upload(ctx, path, state.BundleID, state.ACL, content) + info, err := s.uploader.Upload(ctx, path, state.ACL, state.CanManage, content) if err != nil { return "", nil, err } diff --git a/bundle/direct/dresources/type_test.go b/bundle/direct/dresources/type_test.go index bc12c994621..9fc611d245a 100644 --- a/bundle/direct/dresources/type_test.go +++ b/bundle/direct/dresources/type_test.go @@ -56,6 +56,7 @@ var knownMissingInRemoteType = map[string][]string{ "internal_immutable_snapshots": { "bundle_id", "acl", + "can_manage", "zip_path", }, "job_runs": { diff --git a/bundle/metadata/metadata.go b/bundle/metadata/metadata.go index 1c61cb093f0..16d2ba00901 100644 --- a/bundle/metadata/metadata.go +++ b/bundle/metadata/metadata.go @@ -15,6 +15,10 @@ type Bundle struct { type Workspace struct { FilePath string `json:"file_path"` + + // SnapshotContentPath is the workspace path of the immutable snapshot, present only when + // experimental.immutable_folder is true. The jobs UI uses it to display the file origin. + SnapshotContentPath string `json:"snapshot_content_path,omitempty"` } type Resource struct { diff --git a/libs/snapshot/client.go b/libs/snapshot/client.go index 8539cf0bda5..1fd05b3e708 100644 --- a/libs/snapshot/client.go +++ b/libs/snapshot/client.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "mime/multipart" "net/http" @@ -15,6 +16,11 @@ import ( databricksclient "github.com/databricks/databricks-sdk-go/client" ) +// ContentSubdir is the fixed subfolder the snapshot API appends to a caller-supplied +// relative_path to form the immutable content path (e.g. "//snapshot"). +// The content path is what the create API echoes back and what InspectSnapshot inspects. +const ContentSubdir = "snapshot" + // SnapshotInfo holds the result of a successful snapshot upload. type SnapshotInfo struct { // Path is the immutable workspace path for the uploaded snapshot content. @@ -30,13 +36,28 @@ type ACLEntry struct { PermissionLevel string `json:"permission_level"` } -// SnapshotClient implements the /api/2.0/repos/snapshots endpoint. +// ManagePrincipal is one element of can_manage_principals: a principal allowed to break the +// glass on a snapshot, i.e. to modify otherwise-immutable content. +type ManagePrincipal struct { + UserName string `json:"user_name,omitempty"` + GroupName string `json:"group_name,omitempty"` + ServicePrincipalName string `json:"service_principal_name,omitempty"` +} + +// SnapshotStatus is the status of a snapshot's content path, as reported by InspectSnapshot. +// Dirty is true when the immutable content was modified out of band. +type SnapshotStatus struct { + Dirty bool +} + +// SnapshotClient implements the /api/2.0/snapshots endpoint. type SnapshotClient struct { workspaceClient *databricks.WorkspaceClient client *databricksclient.DatabricksClient } -// snapshotUploadResponse mirrors the /api/2.0/repos/snapshots response body. +// snapshotUploadResponse mirrors the /api/2.0/snapshots response body. Only the content +// path is used; the operation name, done flag and creation metadata are ignored. type snapshotUploadResponse struct { Snapshot struct { Path string `json:"path"` @@ -47,7 +68,26 @@ type snapshotRootPathResponse struct { Path string `json:"path"` } -// NewSnapshotClient creates a SnapshotClient backed by /api/2.0/repos/snapshots. +// inspectSnapshotPath reports whether a snapshot's content was modified out of band. +const inspectSnapshotPath = "/api/2.0/snapshots:inspect" + +// inspectSnapshotRequest carries the arguments of an inspect call. They travel in the query +// string: the endpoint is a GET, and the SDK builds a GET's query string from the request +// value's `url` tags. +type inspectSnapshotRequest struct { + SnapshotContentPath string `url:"snapshot_content_path"` +} + +// inspectSnapshotResponse mirrors the /api/2.0/snapshots:inspect response body. The +// permissions the status also carries are not used. +type inspectSnapshotResponse struct { + Status *struct { + SnapshotContentPath string `json:"snapshot_content_path"` + Dirty bool `json:"dirty"` + } `json:"status"` +} + +// NewSnapshotClient creates a SnapshotClient backed by /api/2.0/snapshots. func NewSnapshotClient(w *databricks.WorkspaceClient) (*SnapshotClient, error) { c, err := databricksclient.New(w.Config) if err != nil { @@ -56,19 +96,16 @@ func NewSnapshotClient(w *databricks.WorkspaceClient) (*SnapshotClient, error) { return &SnapshotClient{workspaceClient: w, client: c}, nil } -// Upload uploads zipContent as an immutable snapshot. The server derives the -// content-addressed path from the SHA-256 of the zip, and acl grants CAN_READ to -// each listed principal. path is reserved for forthcoming API support for a -// caller-specified upload location; it is not sent yet. -func (c *SnapshotClient) Upload(ctx context.Context, path, bundleID string, acl []ACLEntry, zipContent []byte) (*SnapshotInfo, error) { +// Upload uploads zipContent as an immutable snapshot at relativePath. The server stores the +// content under "//snapshot" and returns that content path. acl grants +// CAN_READ to each listed principal; canManage lists the principals allowed to break the +// glass on the snapshot and must hold at least one, which the API requires. +func (c *SnapshotClient) Upload(ctx context.Context, relativePath string, acl []ACLEntry, canManage []ManagePrincipal, zipContent []byte) (*SnapshotInfo, error) { var body bytes.Buffer mw := multipart.NewWriter(&body) - if err := mw.WriteField("snapshot_id", HashFromContent(zipContent)); err != nil { - return nil, fmt.Errorf("failed to write snapshot_id: %w", err) - } - if err := mw.WriteField("bundle_id", bundleID); err != nil { - return nil, fmt.Errorf("failed to write bundle_id: %w", err) + if err := mw.WriteField("relative_path", relativePath); err != nil { + return nil, fmt.Errorf("failed to write relative_path: %w", err) } aclJSON, err := json.Marshal(acl) @@ -79,6 +116,14 @@ func (c *SnapshotClient) Upload(ctx context.Context, path, bundleID string, acl return nil, fmt.Errorf("failed to write access_control_list: %w", err) } + canManageJSON, err := json.Marshal(canManage) + if err != nil { + return nil, fmt.Errorf("failed to marshal can_manage_principals: %w", err) + } + if err := mw.WriteField("can_manage_principals", string(canManageJSON)); err != nil { + return nil, fmt.Errorf("failed to write can_manage_principals: %w", err) + } + // Attach the zip with an explicit content-type so the server treats it as binary. fh := make(textproto.MIMEHeader) fh.Set("Content-Disposition", `form-data; name="file"; filename="snapshot.zip"`) @@ -103,7 +148,7 @@ func (c *SnapshotClient) Upload(ctx context.Context, path, bundleID string, acl headers["Content-Type"] = mw.FormDataContentType() var resp snapshotUploadResponse - err = c.client.Do(ctx, http.MethodPost, "/api/2.0/repos/snapshots", headers, nil, body.Bytes(), &resp) + err = c.client.Do(ctx, http.MethodPost, "/api/2.0/snapshots", headers, nil, body.Bytes(), &resp) if err != nil { return nil, fmt.Errorf("snapshot upload: %w", err) } @@ -111,12 +156,31 @@ func (c *SnapshotClient) Upload(ctx context.Context, path, bundleID string, acl return &SnapshotInfo{Path: resp.Snapshot.Path}, nil } +// InspectSnapshot reports the status of the snapshot content at contentPath, which tells the +// caller whether the immutable content was modified out of band. +func (c *SnapshotClient) InspectSnapshot(ctx context.Context, contentPath string) (*SnapshotStatus, error) { + req := inspectSnapshotRequest{SnapshotContentPath: contentPath} + + var resp inspectSnapshotResponse + err := c.client.Do(ctx, http.MethodGet, inspectSnapshotPath, auth.WorkspaceIDHeaders(c.client.Config), nil, req, &resp) + if err != nil { + return nil, fmt.Errorf("snapshot inspect: %w", err) + } + // A 200 without a status would otherwise read as "not modified", silently disabling the + // check, so treat it as an error instead. + if resp.Status == nil { + return nil, errors.New("snapshot inspect: response contained no status") + } + + return &SnapshotStatus{Dirty: resp.Status.Dirty}, nil +} + func (c *SnapshotClient) Get(ctx context.Context, snapshotRelativePath string) (*SnapshotInfo, error) { rootPath, err := c.GetSnapshotRootPath(ctx) if err != nil { return nil, fmt.Errorf("failed to get snapshot root path: %w", err) } - snapshotPath := path.Join(rootPath, snapshotRelativePath) + snapshotPath := path.Join(rootPath, snapshotRelativePath, ContentSubdir) resp, err := c.workspaceClient.Workspace.GetStatusByPath(ctx, snapshotPath) if err != nil { return nil, fmt.Errorf("snapshot get: %w", err) diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 78234c5ebfa..647bd4c38cd 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -181,6 +181,16 @@ type fakeDashboard struct { InputSerializedDashboard string `json:"-"` } +// snapshotRecord holds the create-request fields and the out-of-band modification flag for one +// immutable snapshot. Its JSON tags match what the accept test's info endpoint returns so the +// test script can call the endpoint and assert the fields with jq. +type snapshotRecord struct { + Dirty bool `json:"dirty"` + RelPath string `json:"relative_path"` + ACL []map[string]any `json:"access_control_list"` + CanManage []map[string]any `json:"can_manage_principals"` +} + // FakeWorkspace holds a state of a workspace for acceptance tests. type FakeWorkspace struct { mu sync.Mutex @@ -198,6 +208,10 @@ type FakeWorkspace struct { files map[string]FileEntry repoIdByPath map[string]int64 + // snapshots records immutable-snapshot content paths. Breaking the glass on a + // recorded path flips it dirty for good, as in the real API. + snapshots map[string]*snapshotRecord + Jobs map[int64]jobs.Job JobRuns map[int64]jobs.Run JobRunOutputs map[int64]jobs.RunOutput @@ -495,6 +509,7 @@ func NewFakeWorkspace(url, token string) *FakeWorkspace { }, files: make(map[string]FileEntry), repoIdByPath: make(map[string]int64), + snapshots: make(map[string]*snapshotRecord), Jobs: map[int64]jobs.Job{}, JobRuns: map[int64]jobs.Run{}, @@ -830,6 +845,48 @@ func (s *FakeWorkspace) FsDeleteFile(filePath string) Response { return Response{} } +// RecordSnapshot stores a create-request record for the snapshot at contentPath. +func (s *FakeWorkspace) RecordSnapshot(contentPath, relPath string, acl, canManage []map[string]any) { + defer s.LockUnlock()() + s.snapshots[contentPath] = &snapshotRecord{ + Dirty: false, + RelPath: relPath, + ACL: acl, + CanManage: canManage, + } +} + +// SnapshotDirty reports whether the immutable snapshot at contentPath was modified out of band, +// and whether a snapshot is recorded there at all. +func (s *FakeWorkspace) SnapshotDirty(contentPath string) (dirty, ok bool) { + defer s.LockUnlock()() + rec, ok := s.snapshots[contentPath] + if !ok { + return false, false + } + return rec.Dirty, true +} + +// SnapshotInfo returns the stored create-request record for the snapshot at contentPath. +// ok is false when no snapshot is recorded there. +func (s *FakeWorkspace) SnapshotInfo(contentPath string) (*snapshotRecord, bool) { + defer s.LockUnlock()() + rec, ok := s.snapshots[contentPath] + return rec, ok +} + +// BreakGlassSnapshot marks the snapshot at contentPath dirty. The real API flips the flag when +// the glass is broken, not when a file changes: until then the content is read-only to callers. +// ok is false when no snapshot is recorded there. +func (s *FakeWorkspace) BreakGlassSnapshot(contentPath string) (ok bool) { + defer s.LockUnlock()() + rec, ok := s.snapshots[contentPath] + if ok { + rec.Dirty = true + } + return ok +} + func (s *FakeWorkspace) WorkspaceMkdirs(request workspace.Mkdirs) { defer s.LockUnlock()() // The real mkdirs API creates all intermediate directories ("mkdir -p"), @@ -1099,7 +1156,6 @@ func (s *FakeWorkspace) importFile(filePath string, body []byte, overwrite, dete Data: body, } } - return Response{} } @@ -1156,7 +1212,6 @@ func (s *FakeWorkspace) WorkspaceImportNotebook(filePath string, body []byte, la }, Data: body, } - return Response{} } diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index e1a14b9800b..c8b54ec672b 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -25,6 +25,11 @@ import ( // TestWorkspaceID is the id of the fake workspace. const TestWorkspaceID = 900800700600 +// SnapshotRootUUID is the workspace user UUID the snapshot root is under. The real API uses a +// UUID that is not the deploying identity's user name (an email or a service principal's +// application ID), so snapshot paths never contain the user name. +const SnapshotRootUUID = "3f9d2c1b-7a6e-4b5d-8c4f-0e1a2b3c4d5e" + var TestMetastore = catalog.MetastoreAssignment{ DefaultCatalogName: "hive_metastore", MetastoreId: "120efa64-9b68-46ba-be38-f319458430d2", @@ -167,6 +172,29 @@ func ParseImportForm(req Request) (ImportForm, error) { } } +// snapshotStatus is the body the snapshot inspect and break glass APIs answer with. +func snapshotStatus(contentPath string, dirty bool) map[string]any { + return map[string]any{ + "status": map[string]any{ + "snapshot_content_path": contentPath, + "dirty": dirty, + "permissions": []string{"SNAPSHOT_PERMISSION_CAN_BREAK_GLASS"}, + }, + } +} + +// snapshotNotFound is how the snapshot inspect and break glass APIs answer for a path that +// holds no snapshot. +func snapshotNotFound(contentPath string) Response { + return Response{ + StatusCode: http.StatusNotFound, + Body: map[string]string{ + "error_code": "RESOURCE_DOES_NOT_EXIST", + "message": "Snapshot not found: " + contentPath, + }, + } +} + func AddDefaultHandlers(server *Server) { server.Handle("GET", "/api/2.0/policies/clusters/list", func(req Request) any { return req.Workspace.ClusterPoliciesList(req) @@ -907,11 +935,11 @@ func AddDefaultHandlers(server *Server) { server.Handle("GET", "/api/2.0/repos/snapshots/rootpath", func(req Request) any { return map[string]any{ - "path": "/Workspace/Users/" + TestUserSP.UserName + "/.snapshots/", + "path": "/Workspace/Users/" + SnapshotRootUUID + "/.snapshots/", } }) - server.Handle("POST", "/api/2.0/repos/snapshots", func(req Request) any { + server.Handle("POST", "/api/2.0/snapshots", func(req Request) any { contentType := req.Headers.Get("Content-Type") mediaType, params, err := mime.ParseMediaType(contentType) if err != nil || !strings.HasPrefix(mediaType, "multipart/") { @@ -919,7 +947,8 @@ func AddDefaultHandlers(server *Server) { } mr := multipart.NewReader(bytes.NewReader(req.Body), params["boundary"]) - var bundleID, snapshotID string + var relativePath string + var acl, canManage []map[string]any for { p, err := mr.NextPart() if errors.Is(err, io.EOF) { @@ -933,22 +962,84 @@ func AddDefaultHandlers(server *Server) { return Response{StatusCode: http.StatusInternalServerError} } switch p.FormName() { - case "bundle_id": - bundleID = string(data) - case "snapshot_id": - snapshotID = string(data) + case "relative_path": + relativePath = string(data) + case "access_control_list": + _ = json.Unmarshal(data, &acl) + case "can_manage_principals": + _ = json.Unmarshal(data, &canManage) } } - // The real API uses the workspace user UUID (not email) in the snapshot path, - // matching service-principal identities used in cloud acceptance tests. - snapshotPath := fmt.Sprintf("/Workspace/Users/%s/.snapshots/%s/%s", TestUserSP.UserName, bundleID, snapshotID) - req.Workspace.WorkspaceMkdirs(workspace.Mkdirs{Path: snapshotPath}) + // The server stores content under a fixed "snapshot" subfolder of the caller's + // relative_path. The real API uses the workspace user UUID (not email) in the path, + // see SnapshotRootUUID. + contentPath := fmt.Sprintf("/Workspace/Users/%s/.snapshots/%s/snapshot", SnapshotRootUUID, relativePath) + // Snapshots are immutable, so a relative_path can be created only once. The real API + // answers with a 400 that carries the code in the message and sets no error_code. + if _, exists := req.Workspace.SnapshotInfo(contentPath); exists { + return Response{ + StatusCode: http.StatusBadRequest, + Body: map[string]string{ + "message": "RESOURCE_ALREADY_EXISTS: relative_path already exists", + }, + } + } + req.Workspace.WorkspaceMkdirs(workspace.Mkdirs{Path: contentPath}) + req.Workspace.RecordSnapshot(contentPath, relativePath, acl, canManage) return map[string]any{ - "snapshot": map[string]any{ - "path": snapshotPath, - }, + "name": "workspaces/snapshotOperations/" + relativePath, + "done": true, + "snapshot": map[string]any{"path": contentPath}, + } + }) + + // Returns the create-request fields (ACL, can_manage_principals) stored for the snapshot + // at the given relative_path. Intended for acceptance test assertions. + server.Handle("GET", "/api/2.0/snapshots/info", func(req Request) any { + relPath := req.URL.Query().Get("relative_path") + // Build the content path the same way the create handler does. + contentPath := fmt.Sprintf("/Workspace/Users/%s/.snapshots/%s/snapshot", SnapshotRootUUID, relPath) + rec, ok := req.Workspace.SnapshotInfo(contentPath) + if !ok { + return Response{ + StatusCode: http.StatusNotFound, + Body: map[string]string{ + "error_code": "RESOURCE_DOES_NOT_EXIST", + "message": "Snapshot not found: " + relPath, + }, + } + } + return rec + }) + + // Reports whether a snapshot's content was modified out of band. + server.Handle("GET", "/api/2.0/snapshots:inspect", func(req Request) any { + contentPath := req.URL.Query().Get("snapshot_content_path") + dirty, ok := req.Workspace.SnapshotDirty(contentPath) + // A path that holds no snapshot is not inspectable, so the API reports NOT_FOUND + // rather than calling it clean. + if !ok { + return snapshotNotFound(contentPath) + } + return snapshotStatus(contentPath, dirty) + }) + + // Breaks the glass on a snapshot: grants write access to its content and marks it dirty. + server.Handle("POST", "/api/2.0/snapshots:breakGlass", func(req Request) any { + var request struct { + SnapshotContentPath string `json:"snapshot_content_path"` + } + if err := json.Unmarshal(req.Body, &request); err != nil { + return Response{ + Body: fmt.Sprintf("internal error: %s", err), + StatusCode: http.StatusInternalServerError, + } + } + if !req.Workspace.BreakGlassSnapshot(request.SnapshotContentPath) { + return snapshotNotFound(request.SnapshotContentPath) } + return snapshotStatus(request.SnapshotContentPath, true) }) // SQL Warehouses: