diff --git a/acceptance/bundle/deployment/bind/dashboard/recreation/out.state_after_bind.direct.json b/acceptance/bundle/deployment/bind/dashboard/recreation/out.state_after_bind.direct.json index eb3a71d91ff..c52412ab5fb 100644 --- a/acceptance/bundle/deployment/bind/dashboard/recreation/out.state_after_bind.direct.json +++ b/acceptance/bundle/deployment/bind/dashboard/recreation/out.state_after_bind.direct.json @@ -12,7 +12,7 @@ "etag": [ETAG], "parent_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/resources", "published": true, - "serialized_dashboard": "{\"pages\":[{\"displayName\":\"Page One\",\"name\":\"02724bf2\"}]}", + "serialized_dashboard": "sha256_hashed_in_state:[HASH]", "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" } } diff --git a/acceptance/bundle/migrate/dashboards/out.new_state.json b/acceptance/bundle/migrate/dashboards/out.new_state.json index ed623b6b487..33d4c0791b5 100644 --- a/acceptance/bundle/migrate/dashboards/out.new_state.json +++ b/acceptance/bundle/migrate/dashboards/out.new_state.json @@ -12,7 +12,7 @@ "etag": "[NUMID]", "parent_path": "/Workspace/Users/[USERNAME]", "published": true, - "serialized_dashboard": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}\n", + "serialized_dashboard": "sha256_hashed_in_state:[HASH]", "warehouse_id": "123456" } } diff --git a/acceptance/bundle/migrate/dashboards/out.plan_after_migrate.json b/acceptance/bundle/migrate/dashboards/out.plan_after_migrate.json index cbdc8b8203b..8fb86c2484f 100644 --- a/acceptance/bundle/migrate/dashboards/out.plan_after_migrate.json +++ b/acceptance/bundle/migrate/dashboards/out.plan_after_migrate.json @@ -50,9 +50,9 @@ "serialized_dashboard": { "action": "skip", "reason": "etag_based", - "old": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}\n", - "new": "{\"pages\":[{\"name\":\"02724bf2\",\"displayName\":\"Dashboard test bundle-deploy-dashboard\"}]}\n", - "remote": "{\"pages\":[{\"displayName\":\"Dashboard test bundle-deploy-dashboard\",\"name\":\"02724bf2\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}\n" + "old": "sha256_hashed_in_state:[HASH][0]", + "new": "sha256_hashed_in_state:[HASH][0]", + "remote": "sha256_hashed_in_state:[HASH][1]" }, "update_time": { "action": "skip", diff --git a/acceptance/bundle/resources/dashboard-state-sha/dashboard.tmpl b/acceptance/bundle/resources/dashboard-state-sha/dashboard.tmpl new file mode 100644 index 00000000000..bf5c695c6d2 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/dashboard.tmpl @@ -0,0 +1 @@ +{"pages":[{"name":"main","displayName":"$DASH_TITLE"}]} diff --git a/acceptance/bundle/resources/dashboard-state-sha/databricks.yml.tmpl b/acceptance/bundle/resources/dashboard-state-sha/databricks.yml.tmpl new file mode 100644 index 00000000000..a41576273c0 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/databricks.yml.tmpl @@ -0,0 +1,11 @@ +bundle: + name: dashboard-state-sha-$UNIQUE_NAME + +resources: + dashboards: + dashboard1: + display_name: $DASHBOARD_DISPLAY_NAME + warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID + embed_credentials: true + file_path: "dashboard.lvdash.json" + parent_path: /Users/$CURRENT_USER_NAME diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.create.serialized.json b/acceptance/bundle/resources/dashboard-state-sha/out.create.serialized.json new file mode 100644 index 00000000000..c21b142583a --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.create.serialized.json @@ -0,0 +1,3 @@ +[ + "{\"pages\":[{\"name\":\"main\",\"displayName\":\"Sales Overview\"}]}\n" +] diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.plan_create.direct.json b/acceptance/bundle/resources/dashboard-state-sha/out.plan_create.direct.json new file mode 100644 index 00000000000..b9d477135b6 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.plan_create.direct.json @@ -0,0 +1,19 @@ +{ + "plan_version": 2, + "cli_version": "[CLI_VERSION]", + "plan": { + "resources.dashboards.dashboard1": { + "action": "create", + "new_state": { + "value": { + "display_name": "dashboard-state-sha [UUID]", + "embed_credentials": true, + "parent_path": "/Workspace/Users/[USERNAME]", + "published": true, + "serialized_dashboard": "{\"pages\":[{\"name\":\"main\",\"displayName\":\"Sales Overview\"}]}\n", + "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" + } + } + } + } +} diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.plan_skip.direct.json b/acceptance/bundle/resources/dashboard-state-sha/out.plan_skip.direct.json new file mode 100644 index 00000000000..595950529ab --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.plan_skip.direct.json @@ -0,0 +1,65 @@ +{ + "plan_version": 2, + "cli_version": "[CLI_VERSION]", + "lineage": "[UUID]", + "serial": 1, + "plan": { + "resources.dashboards.dashboard1": { + "action": "skip", + "remote_state": { + "create_time": "[TIMESTAMP]", + "dashboard_id": "[DASHBOARD_ID]", + "display_name": "dashboard-state-sha [UUID]", + "embed_credentials": true, + "etag": [ETAG], + "lifecycle_state": "ACTIVE", + "parent_path": "/Workspace/Users/[USERNAME]", + "path": "/Users/[USERNAME]/dashboard-state-sha [UUID].lvdash.json", + "published": true, + "serialized_dashboard": "{\"pages\":[{\"displayName\":\"Sales Overview\",\"name\":\"main\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}\n", + "update_time": "[TIMESTAMP]", + "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" + }, + "changes": { + "create_time": { + "action": "skip", + "reason": "spec:output_only", + "remote": "[TIMESTAMP]" + }, + "dashboard_id": { + "action": "skip", + "reason": "spec:output_only", + "remote": "[DASHBOARD_ID]" + }, + "etag": { + "action": "skip", + "reason": "custom", + "old": [ETAG], + "remote": [ETAG] + }, + "lifecycle_state": { + "action": "skip", + "reason": "spec:output_only", + "remote": "ACTIVE" + }, + "path": { + "action": "skip", + "reason": "spec:output_only", + "remote": "/Users/[USERNAME]/dashboard-state-sha [UUID].lvdash.json" + }, + "serialized_dashboard": { + "action": "skip", + "reason": "etag_based", + "old": "sha256_hashed_in_state:[HASH][0]", + "new": "sha256_hashed_in_state:[HASH][0]", + "remote": "sha256_hashed_in_state:[HASH][1]" + }, + "update_time": { + "action": "skip", + "reason": "spec:output_only", + "remote": "[TIMESTAMP]" + } + } + } + } +} diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.plan_update.direct.json b/acceptance/bundle/resources/dashboard-state-sha/out.plan_update.direct.json new file mode 100644 index 00000000000..f6b9a4efa37 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.plan_update.direct.json @@ -0,0 +1,74 @@ +{ + "plan_version": 2, + "cli_version": "[CLI_VERSION]", + "lineage": "[UUID]", + "serial": 1, + "plan": { + "resources.dashboards.dashboard1": { + "action": "update", + "new_state": { + "value": { + "display_name": "dashboard-state-sha [UUID]", + "embed_credentials": true, + "parent_path": "/Workspace/Users/[USERNAME]", + "published": true, + "serialized_dashboard": "{\"pages\":[{\"name\":\"main\",\"displayName\":\"Sales Overview v2\"}]}\n", + "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" + } + }, + "remote_state": { + "create_time": "[TIMESTAMP]", + "dashboard_id": "[DASHBOARD_ID]", + "display_name": "dashboard-state-sha [UUID]", + "embed_credentials": true, + "etag": [ETAG], + "lifecycle_state": "ACTIVE", + "parent_path": "/Workspace/Users/[USERNAME]", + "path": "/Users/[USERNAME]/dashboard-state-sha [UUID].lvdash.json", + "published": true, + "serialized_dashboard": "{\"pages\":[{\"displayName\":\"Sales Overview\",\"name\":\"main\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}\n", + "update_time": "[TIMESTAMP]", + "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" + }, + "changes": { + "create_time": { + "action": "skip", + "reason": "spec:output_only", + "remote": "[TIMESTAMP]" + }, + "dashboard_id": { + "action": "skip", + "reason": "spec:output_only", + "remote": "[DASHBOARD_ID]" + }, + "etag": { + "action": "skip", + "reason": "custom", + "old": [ETAG], + "remote": [ETAG] + }, + "lifecycle_state": { + "action": "skip", + "reason": "spec:output_only", + "remote": "ACTIVE" + }, + "path": { + "action": "skip", + "reason": "spec:output_only", + "remote": "/Users/[USERNAME]/dashboard-state-sha [UUID].lvdash.json" + }, + "serialized_dashboard": { + "action": "update", + "old": "sha256_hashed_in_state:[HASH][0]", + "new": "sha256_hashed_in_state:[HASH][1]", + "remote": "sha256_hashed_in_state:[HASH][2]" + }, + "update_time": { + "action": "skip", + "reason": "spec:output_only", + "remote": "[TIMESTAMP]" + } + } + } + } +} diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.state.direct.txt b/acceptance/bundle/resources/dashboard-state-sha/out.state.direct.txt new file mode 100644 index 00000000000..f2a1bea9c55 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.state.direct.txt @@ -0,0 +1 @@ +json.state.resources.dashboards.dashboard1.state.serialized_dashboard = "sha256_hashed_in_state:[HASH]"; diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.state_after_update.direct.txt b/acceptance/bundle/resources/dashboard-state-sha/out.state_after_update.direct.txt new file mode 100644 index 00000000000..f2a1bea9c55 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.state_after_update.direct.txt @@ -0,0 +1 @@ +json.state.resources.dashboards.dashboard1.state.serialized_dashboard = "sha256_hashed_in_state:[HASH]"; diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.test.toml b/acceptance/bundle/resources/dashboard-state-sha/out.test.toml new file mode 100644 index 00000000000..71970b719d4 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.test.toml @@ -0,0 +1,4 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +EnvMatrix.READPLAN = ["", "1"] diff --git a/acceptance/bundle/resources/dashboard-state-sha/out.update.serialized.json b/acceptance/bundle/resources/dashboard-state-sha/out.update.serialized.json new file mode 100644 index 00000000000..5c8339e26b5 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.update.serialized.json @@ -0,0 +1,3 @@ +[ + "{\"pages\":[{\"name\":\"main\",\"displayName\":\"Sales Overview v2\"}]}\n" +] diff --git a/acceptance/bundle/resources/dashboard-state-sha/output.txt b/acceptance/bundle/resources/dashboard-state-sha/output.txt new file mode 100644 index 00000000000..bd79441ffe3 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/output.txt @@ -0,0 +1,26 @@ + +=== Create: plan keeps full content in new_state but reports the diff as a hash +>>> [CLI] bundle plan -o json +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dashboard-state-sha-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +=== Re-plan with no local change is a no-op (server normalization is ignored, etag_based) +>>> [CLI] bundle plan -o json + +=== Edit serialized_dashboard: the hash changes, so the local diff detects an update +>>> [CLI] bundle plan -o json +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/dashboard-state-sha-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.dashboards.dashboard1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/dashboard-state-sha-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/dashboard-state-sha/script b/acceptance/bundle/resources/dashboard-state-sha/script new file mode 100644 index 00000000000..70b49b04399 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/script @@ -0,0 +1,60 @@ +DASHBOARD_DISPLAY_NAME="dashboard-state-sha $(uuid)" +if [ -z "$CLOUD_ENV" ]; then + export TEST_DEFAULT_WAREHOUSE_ID="warehouse-1234" + echo "warehouse-1234:TEST_DEFAULT_WAREHOUSE_ID" >> ACC_REPLS +fi +export DASHBOARD_DISPLAY_NAME +envsubst < databricks.yml.tmpl > databricks.yml + +render_dashboard() { + # Strip CR so the rendered content is LF on every platform. dashboard.tmpl is not pinned to + # eol=lf, so on Windows it renders with a CRLF trailing newline; since serialized_dashboard is + # hashed into state, a CRLF here would hash differently than the LF content in a saved plan and + # make the no-op re-plan (READPLAN=1) spuriously report an update. + DASH_TITLE="$1" envsubst < dashboard.tmpl | tr -d '\r' > dashboard.lvdash.json +} + +cleanup() { + trace $CLI bundle destroy --auto-approve + rm -f out.requests.txt +} +trap cleanup EXIT +rm -f out.requests.txt + +title "Create: plan keeps full content in new_state but reports the diff as a hash" +render_dashboard "Sales Overview" +trace $CLI bundle plan -o json > out.plan_create.$DATABRICKS_BUNDLE_ENGINE.json + +# Deploy. With READPLAN=1 this applies the SAVED plan file instead of re-planning. The plan +# and the persisted state keep only the hash, but the create API call must still send the +# FULL serialized_dashboard. out.create.serialized.json is identical for READPLAN="" and +# READPLAN=1, which proves the saved plan applies the real content rather than the hash. +# Not traced: the deploy command line differs by READPLAN (--plan vs none). +$CLI bundle deploy $(readplanarg out.plan_create.direct.json) + +DASHBOARD_ID=$($CLI bundle summary --output json | jq -r '.resources.dashboards.dashboard1.id') +echo "$DASHBOARD_ID:DASHBOARD_ID" >> ACC_REPLS + +jq -s '[.[] | select(.method == "POST" and (.path | endswith("/api/2.0/lakeview/dashboards")) and .body.serialized_dashboard != null) | .body.serialized_dashboard]' out.requests.txt > out.create.serialized.json +rm -f out.requests.txt + +# Persisted state holds ONLY the content hash, never the dashboard JSON. +print_state.py | gron.py | grep serialized_dashboard > out.state.$DATABRICKS_BUNDLE_ENGINE.txt + +title "Re-plan with no local change is a no-op (server normalization is ignored, etag_based)" +trace $CLI bundle plan -o json > out.plan_skip.$DATABRICKS_BUNDLE_ENGINE.json + +title "Edit serialized_dashboard: the hash changes, so the local diff detects an update" +render_dashboard "Sales Overview v2" +trace $CLI bundle plan -o json > out.plan_update.$DATABRICKS_BUNDLE_ENGINE.json + +# Apply the update (in-memory or from the saved plan). The update API call must carry the +# FULL new content; out.update.serialized.json is identical across READPLAN, proving the +# saved plan applies the real content for updates too. +$CLI bundle deploy $(readplanarg out.plan_update.direct.json) + +jq -s '[.[] | select(.method == "PATCH" and (.path | contains("/api/2.0/lakeview/dashboards")) and .body.serialized_dashboard != null) | .body.serialized_dashboard]' out.requests.txt > out.update.serialized.json +rm -f out.requests.txt + +# State holds the NEW content hash after the update. +print_state.py | gron.py | grep serialized_dashboard > out.state_after_update.$DATABRICKS_BUNDLE_ENGINE.txt diff --git a/acceptance/bundle/resources/dashboard-state-sha/test.toml b/acceptance/bundle/resources/dashboard-state-sha/test.toml new file mode 100644 index 00000000000..3b31f0cae74 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/test.toml @@ -0,0 +1,24 @@ +# Direct-engine hashed-state behaviour for dashboards. Kept outside dashboards/ so it can run +# direct-only: terraform does not hash state, and the saved-plan path (deploy --plan, i.e. +# READPLAN=1) is direct-only. RecordRequests is inherited from resources/test.toml. +# Local-only: this exercises direct-engine state format (serialized_dashboard stored as a +# content hash), which the in-memory testserver covers fully and deterministically. On real +# cloud the read-after-deploy plan/state reads hit dashboard eventual consistency, and unlike +# the tests under dashboards/ this dir doesn't inherit their retry/INJECT_STALE machinery. +Local = true +Cloud = false + +# dashboard.lvdash.json is rendered from dashboard.tmpl in the script (re-rendered for the edit). +Ignore = ["dashboard.lvdash.json"] + +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +EnvMatrix.READPLAN = ["", "1"] + +# Etag can be both negative and positive. +[[Repls]] +Old = "\"[-0-9]{8,}\"" +New = "[ETAG]" + +[[Repls]] +Old = "\"[0-9]{8,}\"" +New = "[ETAG]" diff --git a/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json b/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json index b00e44d4fe8..bff656a3a32 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json +++ b/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json @@ -59,9 +59,9 @@ "serialized_dashboard": { "action": "skip", "reason": "etag_based", - "old": "{\n \"pages\": [\n {\n \"displayName\": \"New Page\",\n \"layout\": [\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 0\n },\n \"widget\": {\n \"name\": \"82eb9107\",\n \"textbox_spec\": \"# I'm a title\"\n }\n },\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 2\n },\n \"widget\": {\n \"name\": \"ffa6de4f\",\n \"textbox_spec\": \"Text\"\n }\n }\n ],\n \"name\": \"fdd21a3c\"\n }\n ]\n}\n", - "new": "{\n \"pages\": [\n {\n \"displayName\": \"New Page\",\n \"layout\": [\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 0\n },\n \"widget\": {\n \"name\": \"82eb9107\",\n \"textbox_spec\": \"# I'm a title\"\n }\n },\n {\n \"position\": {\n \"height\": 2,\n \"width\": 6,\n \"x\": 0,\n \"y\": 2\n },\n \"widget\": {\n \"name\": \"ffa6de4f\",\n \"textbox_spec\": \"Text\"\n }\n }\n ],\n \"name\": \"fdd21a3c\"\n }\n ]\n}\n", - "remote": "{}\n" + "old": "sha256_hashed_in_state:[HASH][0]", + "new": "sha256_hashed_in_state:[HASH][0]", + "remote": "sha256_hashed_in_state:[HASH][1]" }, "update_time": { "action": "skip", diff --git a/acceptance/bundle/resources/dashboards/simple/out.plan.direct.json b/acceptance/bundle/resources/dashboards/simple/out.plan.direct.json index 5e6ad9eb0a6..32fe8c1b420 100644 --- a/acceptance/bundle/resources/dashboards/simple/out.plan.direct.json +++ b/acceptance/bundle/resources/dashboards/simple/out.plan.direct.json @@ -50,9 +50,9 @@ "serialized_dashboard": { "action": "skip", "reason": "etag_based", - "old": "{ }\n", - "new": "{ }\n", - "remote": "{}\n" + "old": "sha256_hashed_in_state:[HASH][0]", + "new": "sha256_hashed_in_state:[HASH][0]", + "remote": "sha256_hashed_in_state:[HASH][1]" }, "update_time": { "action": "skip", diff --git a/acceptance/bundle/resources/dashboards/unpublish-out-of-band/out.plan.direct.json b/acceptance/bundle/resources/dashboards/unpublish-out-of-band/out.plan.direct.json index 852cc492879..0197870475c 100644 --- a/acceptance/bundle/resources/dashboards/unpublish-out-of-band/out.plan.direct.json +++ b/acceptance/bundle/resources/dashboards/unpublish-out-of-band/out.plan.direct.json @@ -66,9 +66,9 @@ "serialized_dashboard": { "action": "skip", "reason": "etag_based", - "old": "{\"pages\":[{\"displayName\":\"Test Dashboard\",\"name\":\"test-page\"}]}", - "new": "{\"pages\":[{\"displayName\":\"Test Dashboard\",\"name\":\"test-page\"}]}", - "remote": "{\"pages\":[{\"displayName\":\"Test Dashboard\",\"name\":\"test-page\",\"pageType\":\"PAGE_TYPE_CANVAS\"}]}" + "old": "sha256_hashed_in_state:[HASH][0]", + "new": "sha256_hashed_in_state:[HASH][0]", + "remote": "sha256_hashed_in_state:[HASH][1]" }, "update_time": { "action": "skip", diff --git a/acceptance/bundle/test.toml b/acceptance/bundle/test.toml index fca8259a3b5..56862f10cca 100644 --- a/acceptance/bundle/test.toml +++ b/acceptance/bundle/test.toml @@ -22,3 +22,13 @@ New = 'os/[OS]' [[Repls]] Old = ' cicd/github' New = '' + +# Mask the serialized_dashboard content hash before the generic numeric/id rules can +# nibble digit-runs out of the hex (Order below their default). Distinct numbers the +# hashes, so old==new stays one token and a differing remote gets another +# (e.g. [HASH][0] vs [HASH][1]). +[[Repls]] +Old = "sha256_hashed_in_state:[0-9a-f]{64}" +New = "sha256_hashed_in_state:[HASH]" +Order = -1 +Distinct = true diff --git a/bundle/direct/apply.go b/bundle/direct/apply.go index b3c46036c53..45949b0b7e4 100644 --- a/bundle/direct/apply.go +++ b/bundle/direct/apply.go @@ -75,7 +75,7 @@ func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState, return err } - err = db.SaveState(d.ResourceKey, newID, newState, d.DependsOn) + err = d.compactAndSaveState(db, newID, newState) if err != nil { return fmt.Errorf("saving state after creating id=%s: %w", newID, err) } @@ -149,7 +149,7 @@ func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState, return err } - err = db.SaveState(d.ResourceKey, id, newState, d.DependsOn) + err = d.compactAndSaveState(db, id, newState) if err != nil { return fmt.Errorf("saving state id=%s: %w", id, err) } @@ -193,7 +193,7 @@ func (d *DeploymentUnit) UpdateWithID(ctx context.Context, db *dstate.Deployment return err } - err = db.SaveState(d.ResourceKey, newID, newState, d.DependsOn) + err = d.compactAndSaveState(db, newID, newState) if err != nil { return fmt.Errorf("saving state id=%s: %w", oldID, err) } @@ -276,7 +276,7 @@ func (d *DeploymentUnit) Resize(ctx context.Context, db *dstate.DeploymentState, return fmt.Errorf("resizing id=%s: %w", id, err) } - err = db.SaveState(d.ResourceKey, id, newState, d.DependsOn) + err = d.compactAndSaveState(db, id, newState) if err != nil { return fmt.Errorf("saving state id=%s: %w", id, err) } @@ -284,6 +284,16 @@ func (d *DeploymentUnit) Resize(ctx context.Context, db *dstate.DeploymentState, return nil } +// compactAndSaveState compacts the state (replacing fields declared in hashed_in_state +// with content hashes, see dresources.CompactState) before persisting it. +func (d *DeploymentUnit) compactAndSaveState(db *dstate.DeploymentState, id string, newState any) error { + compacted, err := dresources.CompactState(d.Adapter.ResourceConfig(), newState) + if err != nil { + return fmt.Errorf("compacting state: %w", err) + } + return db.SaveState(d.ResourceKey, id, compacted, d.DependsOn) +} + func parseState(destType reflect.Type, raw json.RawMessage) (any, error) { destPtr := reflect.New(destType).Interface() err := json.Unmarshal(raw, destPtr) diff --git a/bundle/direct/bind.go b/bundle/direct/bind.go index 9760ce95666..dcd9bf28742 100644 --- a/bundle/direct/bind.go +++ b/bundle/direct/bind.go @@ -8,6 +8,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/bundle/direct/dstate" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/structs/structaccess" @@ -145,13 +146,28 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac } } + // Compact hashed_in_state fields (e.g. a dashboard's serialized_dashboard) so + // the state we persist here matches what a later deploy writes. Otherwise the + // next plan would compare this raw value against the hashed config side and + // report a spurious change on the resource we just bound. + adapter, err := b.getAdapterForKey(resourceKey) + if err != nil { + os.Remove(tmpStatePath) + return nil, err + } + compacted, err := dresources.CompactState(adapter.ResourceConfig(), sv.Value) + if err != nil { + os.Remove(tmpStatePath) + return nil, fmt.Errorf("compacting state: %w", err) + } + err = b.StateDB.Open(ctx, tmpStatePath, dstate.WithRecovery(true), dstate.WithWrite(true)) if err != nil { os.Remove(tmpStatePath) return nil, err } - err = b.StateDB.SaveState(resourceKey, resourceID, sv.Value, dependsOn) + err = b.StateDB.SaveState(resourceKey, resourceID, compacted, dependsOn) if err != nil { os.Remove(tmpStatePath) return nil, err diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 669c187aac5..363dd50c4fe 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -225,6 +225,18 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks return false } + // Compact the saved state so hashed_in_state fields are hashes, matching the + // local and remote sides below. The state read from disk may still hold the full + // contents (written by an older CLI, or the field was only just added to + // hashed_in_state); hashing it on read lines the sides up so an unchanged resource + // shows no change. This only affects the in-memory copy used for the diff; the + // on-disk entry keeps its full contents until the resource is next saved. + savedState, err = dresources.CompactState(adapter.ResourceConfig(), savedState) + if err != nil { + logdiag.LogError(ctx, fmt.Errorf("%s: compacting saved state: %w", errorPrefix, err)) + return false + } + // Note, currently we're diffing static structs, not dynamic value. // This means for fields that contain references like ${resources.group.foo.id} we do one of the following: // for strings: comparing unresolved string like "${resoures.group.foo.id}" with actual object id. As long as IDs do not have ${...} format we're good. @@ -236,7 +248,14 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks logdiag.LogError(ctx, fmt.Errorf("%s: internal error: no state cache entry found for %q", errorPrefix, resourceKey)) return false } - localDiff, err := structdiff.GetStructDiff(savedState, sv.Value, adapter.KeyedSlices()) + // Compact a copy for comparison only; sv.Value keeps the full contents, which + // the deploy sends to the API. + localState, err := dresources.CompactState(adapter.ResourceConfig(), sv.Value) + if err != nil { + logdiag.LogError(ctx, fmt.Errorf("%s: compacting local state: %w", errorPrefix, err)) + return false + } + localDiff, err := structdiff.GetStructDiff(savedState, localState, adapter.KeyedSlices()) if err != nil { logdiag.LogError(ctx, fmt.Errorf("%s: diffing local state: %w", errorPrefix, err)) return false @@ -269,7 +288,21 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks return false } - remoteDiff, err = structdiff.GetStructDiff(remoteStateComparable, sv.Value, adapter.KeyedSlices()) + // Compact the remapped remote on the same fields, so a hashed_in_state field + // is a hash on all three sides of the diff (saved, local, remote). Once the + // saved value is a hash, every comparison must be hash-vs-hash to be meaningful, + // including remote drift. This keeps hashed_in_state orthogonal to + // ignore_remote_changes: remote drift is still detected as hash != hash, so a + // field can be hashed without being ignored. serialized_dashboard is also + // ignore_remote_changes, but for the independent reason that the server + // normalizes it (see resources.yml). + remoteStateComparable, err = dresources.CompactState(adapter.ResourceConfig(), remoteStateComparable) + if err != nil { + logdiag.LogError(ctx, fmt.Errorf("%s: compacting remote state id=%q: %w", errorPrefix, dbentry.ID, err)) + return false + } + + remoteDiff, err = structdiff.GetStructDiff(remoteStateComparable, localState, adapter.KeyedSlices()) if err != nil { logdiag.LogError(ctx, fmt.Errorf("%s: diffing remote state: %w", errorPrefix, err)) return false diff --git a/bundle/direct/dresources/README.md b/bundle/direct/dresources/README.md index 472631f0e38..dcafe409603 100644 --- a/bundle/direct/dresources/README.md +++ b/bundle/direct/dresources/README.md @@ -47,6 +47,16 @@ If the API may return a slice's elements in a different order between calls (e.g The state struct is serialized to JSON and persisted between deploys. Backward incompatible changes will result in a drift, which depending on field behaviour might result in recreate. See dstate/migrate.go on how to handle state migration. +## hashed_in_state: storing large fields as content hashes + +Declare a field under `hashed_in_state` in `resources.yml` when it holds large content that is only ever compared for equality and never read back from state. The engine then persists only a `sha256_hashed_in_state:` content hash for that field (via `CompactState`), and — crucially — hashes it on **every** value entering the diff: saved state, the local config, and the remapped remote. Once the saved value is a hash, all three sides must be hashes or the comparisons (`Old==New` for a local change, `Remote==New` for remote drift) would be hash-vs-content nonsense. The full contents stay only in the plan's `new_state` and are sent to the API on every deploy, so the deploy is unaffected. + +A field qualifies if it is large (a small field gains nothing — the hash placeholder is ~70 bytes) and is never read back from state by any code path (e.g. not consumed raw by `OverrideChangeDesc` or by state export). + +**`hashed_in_state` is orthogonal to `ignore_remote_changes`.** Because the remote side is hashed too, remote drift on a hashed field is detected as `hash != hash` — so a field can be `hashed_in_state` *without* being `ignore_remote_changes` (as long as the server echoes the value back unchanged). The two are declared independently. + +`dashboards.serialized_dashboard` happens to need both, for unrelated reasons: it is `hashed_in_state` because the inlined dashboard JSON is large, and it is *separately* `ignore_remote_changes` because the **server normalizes** it (adds `pageType`, reorders keys) so its remote hash never equals the config hash — drift is detected via `etag` instead. The plan therefore reports the field as a hash on all three sides. No state version bump is needed: legacy full-content state is hashed on read for comparison and rewritten compactly on the next save. + ## RemapState is a dumb copy; DoRead owns all remapping `RemapState` converts `RemoteType` to `StateType` only because `StateType` is typically a diff --git a/bundle/direct/dresources/config.go b/bundle/direct/dresources/config.go index 91175594a6a..4b1133fb699 100644 --- a/bundle/direct/dresources/config.go +++ b/bundle/direct/dresources/config.go @@ -79,6 +79,12 @@ type ResourceLifecycleConfig struct { // BackendDefaults: fields where the backend may set defaults. // When old and new are nil but remote is set, and the remote value matches allowed values (if specified), the change is skipped. BackendDefaults []BackendDefaultRule `yaml:"backend_defaults,omitempty"` + + // HashedInState: field paths persisted to state as a content hash + // ("sha256_hashed_in_state:") instead of the raw value. This is only valid + // for large, equality-only fields that are never read back from state + // (e.g. serialized dashboards). + HashedInState []string `yaml:"hashed_in_state,omitempty"` } // Config is the root configuration structure for resource lifecycle behavior. @@ -100,6 +106,7 @@ var empty = ResourceLifecycleConfig{ UpdatableIDFields: nil, NormalizeSlash: nil, BackendDefaults: nil, + HashedInState: nil, } func mustParseConfig(data []byte) func() *Config { diff --git a/bundle/direct/dresources/dashboard_test.go b/bundle/direct/dresources/dashboard_test.go index 177e1236221..49895cea916 100644 --- a/bundle/direct/dresources/dashboard_test.go +++ b/bundle/direct/dresources/dashboard_test.go @@ -2,9 +2,12 @@ package dresources import ( "encoding/json" + "slices" + "strings" "testing" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structs/structpath" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -23,3 +26,52 @@ func TestDashboardState_JSONSerialization_PublishedField(t *testing.T) { assert.Contains(t, string(data), `"published":true`) } + +func TestDashboardCompactState(t *testing.T) { + state := &DashboardState{ + DashboardConfig: resources.DashboardConfig{ + DisplayName: "test-dashboard", + Etag: "etag-123", + SerializedDashboard: `{"pages":[{"name":"p1"}]}`, + }, + } + + out, err := CompactState(GetResourceConfig("dashboards"), state) + require.NoError(t, err) + compacted := out.(*DashboardState) + + // serialized_dashboard is replaced by a content hash; other fields are preserved. + require.IsType(t, "", compacted.SerializedDashboard) + assert.True(t, strings.HasPrefix(compacted.SerializedDashboard.(string), stateHashPrefix)) + assert.Equal(t, "test-dashboard", compacted.DisplayName) + assert.Equal(t, "etag-123", compacted.Etag) + + // The original state is not mutated. + assert.Equal(t, `{"pages":[{"name":"p1"}]}`, state.SerializedDashboard) + + // Compacting is idempotent. + out2, err := CompactState(GetResourceConfig("dashboards"), compacted) + require.NoError(t, err) + assert.Equal(t, compacted.SerializedDashboard, out2.(*DashboardState).SerializedDashboard) +} + +// TestDashboardSerializedDashboardStateRules documents that serialized_dashboard carries +// two independent declarations: hashed_in_state (persist only its hash, since the blob is +// large) and ignore_remote_changes (the server normalizes the content, so its remote hash +// never matches the config hash — drift is detected via etag instead). The two are +// orthogonal in general; serialized_dashboard just happens to need both. +func TestDashboardSerializedDashboardStateRules(t *testing.T) { + cfg := GetResourceConfig("dashboards") + path := structpath.NewStringKey(nil, "serialized_dashboard") + + ignoresRemote := false + for _, rule := range cfg.IgnoreRemoteChanges { + if path.HasPatternPrefix(rule.Field) { + ignoresRemote = true + break + } + } + + assert.True(t, slices.Contains(cfg.HashedInState, "serialized_dashboard"), "serialized_dashboard must be declared hashed_in_state") + assert.True(t, ignoresRemote, "serialized_dashboard must be ignore_remote_changes (server normalizes the content)") +} diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index 6061db0fb04..189bf85935e 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -472,6 +472,11 @@ resources: - field: dataset_schema reason: input_only + # "serialized_dashboard" holds inlined dashboard JSON that is never read back + # from state, so we persist only its content hash. + hashed_in_state: + - serialized_dashboard + genie_spaces: ignore_remote_changes: # serialized_space locally (structured YAML) and remotely (JSON string) will differ diff --git a/bundle/direct/dresources/state_compaction.go b/bundle/direct/dresources/state_compaction.go new file mode 100644 index 00000000000..d91df4be0f6 --- /dev/null +++ b/bundle/direct/dresources/state_compaction.go @@ -0,0 +1,101 @@ +package dresources + +import ( + "errors" + "fmt" + "reflect" + "strings" + + "github.com/databricks/cli/libs/hash" + "github.com/databricks/cli/libs/structs/structaccess" + "github.com/databricks/cli/libs/structs/structpath" +) + +// stateHashPrefix marks a state value that holds a content hash instead of the +// raw value. Since this is part of the on-disk state format, changing it is not +// backwards compatible. +const stateHashPrefix = "sha256_hashed_in_state:" + +// hashStateValue returns a content-hash placeholder ("sha256_hashed_in_state:") over the JSON +// encoding of v. It is used to store large, equality-only fields (e.g. a dashboard's +// serialized_dashboard) compactly in state instead of their full contents. +// +// It is idempotent and stable: nil, an empty string, and a value that is already a +// placeholder are returned unchanged, so re-compacting an already-compact state and +// comparing a fresh config against stored state both behave predictably. +func hashStateValue(v any) (any, error) { + if s, ok := v.(string); ok { + if s == "" || strings.HasPrefix(s, stateHashPrefix) { + return v, nil + } + } + + if v == nil { + return v, nil + } + + hashed, err := hash.OfJSON(v) + if err != nil { + return nil, err + } + + return stateHashPrefix + hashed, nil +} + +// CompactState returns a copy of state with every field declared in cfg.HashedInState +// replaced by a content hash, so the state persists only the hash and not the full +// contents. It is applied both before persisting state and to every value entering the +// state diff, so stored and compared values share one form. The caller's value is never +// mutated (it is reused for the deploy API call, which needs the full contents). +// +// Returns state unchanged when no fields are declared or state is not a non-nil pointer. +func CompactState(cfg *ResourceLifecycleConfig, state any) (any, error) { + if cfg == nil || len(cfg.HashedInState) == 0 { + return state, nil + } + + rv := reflect.ValueOf(state) + if rv.Kind() != reflect.Pointer || rv.IsNil() { + return state, nil + } + + // Shallow copy so the caller's value (reused for the deploy) is untouched. This is + // safe only because every hashed_in_state field is a top-level scalar (e.g. + // serialized_dashboard): SetByString overwrites it on the copy directly. A field + // reached through a shared pointer/slice/map would need a deep copy here. + out := reflect.New(rv.Type().Elem()) + out.Elem().Set(rv.Elem()) + compacted := out.Interface() + + for _, field := range cfg.HashedInState { + // The shallow copy above only isolates top-level fields: SetByString on a + // depth-1 path reassigns the field on the copy, but a nested path (e.g. + // "foo.bar") is reached through a pointer/slice/map still shared with the + // caller's value, so hashing it would mutate the value reused for the deploy + // API call. Reject nested paths loudly instead of corrupting state silently. + path, err := structpath.ParsePath(field) + if err != nil { + return nil, fmt.Errorf("compacting state field %q: %w", field, err) + } + if path.Len() != 1 { + return nil, fmt.Errorf("hashed_in_state field %q must be a top-level field", field) + } + + current, err := structaccess.GetByString(compacted, field) + if err != nil { + if _, ok := errors.AsType[*structaccess.NotFoundError](err); ok { + continue + } + return nil, fmt.Errorf("compacting state field %q: %w", field, err) + } + hashed, err := hashStateValue(current) + if err != nil { + return nil, fmt.Errorf("compacting state field %q: %w", field, err) + } + if err := structaccess.SetByString(compacted, field, hashed); err != nil { + return nil, fmt.Errorf("compacting state field %q: %w", field, err) + } + } + + return compacted, nil +} diff --git a/bundle/direct/dresources/state_compaction_test.go b/bundle/direct/dresources/state_compaction_test.go new file mode 100644 index 00000000000..ce3be353967 --- /dev/null +++ b/bundle/direct/dresources/state_compaction_test.go @@ -0,0 +1,111 @@ +package dresources + +import ( + "strings" + "testing" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structs/structpath" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHashedInStateFieldsAreTopLevel guards the shallow-copy assumption in CompactState: +// every hashed_in_state path declared in resources.yml must be a top-level field. A nested +// path would be mutated through memory shared with the deploy value (see CompactState), so +// this fails CI the moment such a declaration is added instead of corrupting state at runtime. +func TestHashedInStateFieldsAreTopLevel(t *testing.T) { + for name, rc := range MustLoadConfig().Resources { + for _, field := range rc.HashedInState { + path, err := structpath.ParsePath(field) + require.NoError(t, err, "%s: hashed_in_state field %q", name, field) + assert.Equal(t, 1, path.Len(), "%s: hashed_in_state field %q must be a top-level field", name, field) + } + } +} + +// TestCompactStateRejectsNestedField verifies CompactState errors on a nested +// hashed_in_state path rather than mutating memory shared with the deploy value. +func TestCompactStateRejectsNestedField(t *testing.T) { + cfg := &ResourceLifecycleConfig{HashedInState: []string{"foo.bar"}} + state := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: `{"a":1}`}} + + _, err := CompactState(cfg, state) + require.ErrorContains(t, err, "must be a top-level field") +} + +// TestCompactStateNoDeclaredFields verifies CompactState is a no-op for a resource +// type with no hashed_in_state declaration and for a nil config, returning the same +// value untouched. +func TestCompactStateNoDeclaredFields(t *testing.T) { + state := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: `{"a":1}`}} + + out, err := CompactState(GetResourceConfig("jobs"), state) + require.NoError(t, err) + assert.Same(t, state, out.(*DashboardState)) + + out, err = CompactState(nil, state) + require.NoError(t, err) + assert.Same(t, state, out.(*DashboardState)) +} + +// TestCompactStateMigratesLegacyFullContent verifies that a legacy state holding the +// full serialized_dashboard and a config holding identical content compact to the +// same hash, so a diff computed after hashing-on-read shows no spurious change and +// the next save rewrites the state compactly. +func TestCompactStateMigratesLegacyFullContent(t *testing.T) { + content := `{"pages":[{"name":"p1"}]}` + legacy := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: content}} + config := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: content}} + + cfg := GetResourceConfig("dashboards") + compactedLegacy, err := CompactState(cfg, legacy) + require.NoError(t, err) + compactedConfig, err := CompactState(cfg, config) + require.NoError(t, err) + + legacyHash := compactedLegacy.(*DashboardState).SerializedDashboard + assert.Equal(t, compactedConfig.(*DashboardState).SerializedDashboard, legacyHash) + assert.True(t, strings.HasPrefix(legacyHash.(string), stateHashPrefix)) +} + +// TestHashStateValue verifies hashStateValue adds the state hash prefix and produces +// a stable placeholder: the same content always hashes to the same value and +// different content differs. The hash body itself is covered by libs/hash. +func TestHashStateValue(t *testing.T) { + stringHash, err := hashStateValue("hello") + require.NoError(t, err) + require.IsType(t, "", stringHash) + assert.True(t, strings.HasPrefix(stringHash.(string), stateHashPrefix)) + + again, err := hashStateValue("hello") + require.NoError(t, err) + assert.Equal(t, stringHash, again) + + other, err := hashStateValue("world") + require.NoError(t, err) + assert.NotEqual(t, stringHash, other) +} + +// TestHashStateValueIdempotent verifies re-hashing an existing placeholder returns it +// unchanged, so re-compacting an already-compact state does not double-hash. +func TestHashStateValueIdempotent(t *testing.T) { + hashed, err := hashStateValue("some content") + require.NoError(t, err) + + again, err := hashStateValue(hashed) + require.NoError(t, err) + assert.Equal(t, hashed, again) +} + +// TestHashStateValueEmptyAndNil verifies empty and nil values pass through unchanged, +// since there is nothing to hash. +func TestHashStateValueEmptyAndNil(t *testing.T) { + empty, err := hashStateValue("") + require.NoError(t, err) + assert.Empty(t, empty) + + null, err := hashStateValue(nil) + require.NoError(t, err) + assert.Nil(t, null) +} diff --git a/bundle/migrate/build_state.go b/bundle/migrate/build_state.go index c08cf01c31f..01c3b039d6e 100644 --- a/bundle/migrate/build_state.go +++ b/bundle/migrate/build_state.go @@ -233,7 +233,16 @@ func BuildStateFromTF( } } - if err := stateDB.SaveState(node, id, sv.Value, dependsOn); err != nil { + // Compact hashed_in_state fields (e.g. a dashboard's serialized_dashboard) so the + // migrated state matches what a native deploy writes; otherwise the first plan + // after migrating from Terraform would compare this raw value against the hashed + // config side and report a spurious change. + compacted, err := dresources.CompactState(adapter.ResourceConfig(), sv.Value) + if err != nil { + return warningsSeen, fmt.Errorf("%s: compacting state: %w", node, err) + } + + if err := stateDB.SaveState(node, id, compacted, dependsOn); err != nil { return warningsSeen, fmt.Errorf("%s: SaveState: %w", node, err) } } diff --git a/libs/cache/file_cache.go b/libs/cache/file_cache.go index 163cbf3bacc..d86b5fcfcb9 100644 --- a/libs/cache/file_cache.go +++ b/libs/cache/file_cache.go @@ -12,6 +12,7 @@ import ( "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/hash" "github.com/databricks/cli/libs/log" ) @@ -155,7 +156,7 @@ func (fc *fileCache) putJSON(ctx context.Context, fingerprint any, data []byte) if !fc.cacheEnabled { return } - cacheKey, err := fingerprintToHash(fingerprint) + cacheKey, err := hash.OfJSON(fingerprint) if err != nil { log.Debugf(ctx, "[Local Cache] failed to generate cache key for put: %v", err) return @@ -169,7 +170,7 @@ func (fc *fileCache) getJSON(ctx context.Context, fingerprint any) ([]byte, bool if !fc.cacheEnabled { return nil, false } - cacheKey, err := fingerprintToHash(fingerprint) + cacheKey, err := hash.OfJSON(fingerprint) if err != nil { log.Debugf(ctx, "[Local Cache] failed to generate cache key: %v", err) return nil, false @@ -191,7 +192,7 @@ func (fc *fileCache) addTelemetryMetric(key string) { // but always computes and never returns cached values. func (fc *fileCache) getOrComputeJSON(ctx context.Context, fingerprint any, compute func(ctx context.Context) ([]byte, error)) ([]byte, error) { // Convert fingerprint to deterministic hash - this is our cache key - cacheKey, err := fingerprintToHash(fingerprint) + cacheKey, err := hash.OfJSON(fingerprint) if err != nil { // Fail open: if we can't generate cache key, just compute directly log.Debugf(ctx, "[Local Cache] failed to generate cache key, computing without cache: %v", err) diff --git a/libs/cache/file_cache_test.go b/libs/cache/file_cache_test.go index b20d03ce2c0..0ce9c571ebc 100644 --- a/libs/cache/file_cache_test.go +++ b/libs/cache/file_cache_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/hash" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -291,7 +292,7 @@ func TestFileCacheInvalidJSON(t *testing.T) { } // Manually write invalid JSON to the cache file - cacheKey, err := fingerprintToHash(fingerprint) + cacheKey, err := hash.OfJSON(fingerprint) require.NoError(t, err) cachePath := fc.getCachePath(cacheKey) err = os.WriteFile(cachePath, []byte("invalid json {{{"), 0o600) @@ -327,7 +328,7 @@ func TestFileCacheCorruptedData(t *testing.T) { } // Write valid JSON but wrong type (string instead of int) - cacheKey, err := fingerprintToHash(fingerprint) + cacheKey, err := hash.OfJSON(fingerprint) require.NoError(t, err) cachePath := fc.getCachePath(cacheKey) err = os.WriteFile(cachePath, []byte(`"not-an-integer"`), 0o600) diff --git a/libs/cache/fingerprint.go b/libs/cache/fingerprint.go deleted file mode 100644 index 8f866405122..00000000000 --- a/libs/cache/fingerprint.go +++ /dev/null @@ -1,22 +0,0 @@ -package cache - -import ( - "crypto/sha256" - "encoding/hex" - "encoding/json" - "fmt" -) - -// fingerprintToHash converts any struct to a deterministic string representation for use as a cache key. -func fingerprintToHash(fingerprint any) (string, error) { - // Marshal map - json.Marshal sorts map keys alphabetically - data, err := json.Marshal(fingerprint) - if err != nil { - return "", fmt.Errorf("failed to marshal normalized fingerprint: %w", err) - } - - // Hash for consistent, reasonably-sized key. - // hash[:] converts the [32]byte array returned by Sum256 to a []byte slice. - hash := sha256.Sum256(data) - return hex.EncodeToString(hash[:]), nil -} diff --git a/libs/cache/fingerprint_test.go b/libs/cache/fingerprint_test.go deleted file mode 100644 index 8a85f7944ce..00000000000 --- a/libs/cache/fingerprint_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package cache - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// TestFingerprintStability tests that the fingerprintToHash function returns the same hash for the same input. -func TestFingerprintStability(t *testing.T) { - fingerprint1 := struct { - Key string `json:"key"` - }{ - Key: "test-key", - } - - fingerprint2 := struct { - Key string `json:"key"` - }{ - Key: "test-key2", - } - - hash1, err := fingerprintToHash(fingerprint1) - require.NoError(t, err) - require.Equal(t, "1b329dc07a9fa87da7480f6b10cc917a40a4f460ac82aea3d09df477764f3101", hash1) - hash2, err := fingerprintToHash(fingerprint2) - require.NoError(t, err) - hash1ToCompare, err := fingerprintToHash(fingerprint1) - require.NoError(t, err) - - assert.Equal(t, hash1ToCompare, hash1) - assert.NotEqual(t, hash1, hash2) -} diff --git a/libs/hash/hash.go b/libs/hash/hash.go new file mode 100644 index 00000000000..ebee3519d38 --- /dev/null +++ b/libs/hash/hash.go @@ -0,0 +1,21 @@ +// Package hash provides deterministic content hashing helpers. +package hash + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" +) + +// OfJSON returns a deterministic sha256 hex digest of v's JSON encoding. +// json.Marshal sorts map keys, so the digest is stable across runs for equal values. +func OfJSON(v any) (string, error) { + data, err := json.Marshal(v) + if err != nil { + return "", fmt.Errorf("failed to marshal value for hashing: %w", err) + } + + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]), nil +} diff --git a/libs/hash/hash_test.go b/libs/hash/hash_test.go new file mode 100644 index 00000000000..7ddb04aa0d5 --- /dev/null +++ b/libs/hash/hash_test.go @@ -0,0 +1,88 @@ +package hash + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestOfJSONStable tests that the hash of the same value is the same across runs. +func TestOfJSONDeterministic(t *testing.T) { + v := struct { + Key string `json:"key"` + }{ + Key: "test-key", + } + + hash, err := OfJSON(v) + require.NoError(t, err) + + hashAgain, err := OfJSON(v) + require.NoError(t, err) + + assert.Equal(t, hash, hashAgain) +} + +// TestOfJSONReordering tests that the hash of a map is the same regardless of the order of the keys. +func TestOfJSONReordering(t *testing.T) { + v1 := map[string]int{"a": 1, "b": 2} + v2 := map[string]int{"b": 2, "a": 1} + + hash1, err := OfJSON(v1) + require.NoError(t, err) + + hash2, err := OfJSON(v2) + require.NoError(t, err) + + assert.Equal(t, hash1, hash2) +} + +// TestOfJSONDifferentValues tests that the hash of different values is different. +func TestOfJSONDifferentValues(t *testing.T) { + v1 := struct { + Key string `json:"key"` + }{ + Key: "test-key", + } + + v2 := struct { + Key string `json:"key"` + }{ + Key: "test-key2", + } + + hash1, err := OfJSON(v1) + require.NoError(t, err) + + hash2, err := OfJSON(v2) + require.NoError(t, err) + + assert.NotEqual(t, hash1, hash2) +} + +// TestOfJSONKnownValues tests that the hash of a known value is the expected value. +func TestOfJSONKnownValues(t *testing.T) { + v := struct { + Key string `json:"key"` + }{ + Key: "test-key", + } + + expectedHash := "1b329dc07a9fa87da7480f6b10cc917a40a4f460ac82aea3d09df477764f3101" + + hash, err := OfJSON(v) + require.NoError(t, err) + assert.Equal(t, expectedHash, hash) +} + +// TestOfJSONUnsupportedTypes tests that the hash of an unsupported type is an error. +func TestOfJSONUnsupportedTypes(t *testing.T) { + v := func() any { + return nil + } + + hash, err := OfJSON(v) + require.Empty(t, hash) + assert.Error(t, err) +}