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..d0b0216ccf1 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/dashboard.tmpl @@ -0,0 +1 @@ +{"pages":[{"name":"main","displayName":"$DASH_TITLE","layout":[{"widget":{"name":"counter1"}}]}]} 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..e054033f0ad --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.create.serialized.json @@ -0,0 +1,3 @@ +[ + "{\"pages\":[{\"name\":\"main\",\"displayName\":\"Sales Overview\",\"layout\":[{\"widget\":{\"name\":\"counter1\"}}]}]}\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..43e9be8ba00 --- /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\",\"layout\":[{\"widget\":{\"name\":\"counter1\"}}]}]}\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..af7e1c48f18 --- /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\",\"layout\":[{\"widget\":{\"name\":\"counter1\"}}],\"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..ef60e06b05c --- /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\",\"layout\":[{\"widget\":{\"name\":\"counter1\"}}]}]}\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\",\"layout\":[{\"widget\":{\"name\":\"counter1\"}}],\"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..10516eb9e66 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/out.update.serialized.json @@ -0,0 +1,3 @@ +[ + "{\"pages\":[{\"name\":\"main\",\"displayName\":\"Sales Overview v2\",\"layout\":[{\"widget\":{\"name\":\"counter1\"}}]}]}\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..6b239a11106 --- /dev/null +++ b/acceptance/bundle/resources/dashboard-state-sha/test.toml @@ -0,0 +1,27 @@ +# 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). +# dashboard.tmpl must stay large enough that hashing shrinks the state: content that encodes to +# no more than a placeholder (87 bytes) is persisted raw, which would silently stop this test from +# exercising hashing at all. +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 c177eb0b450..b7b6753f895 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json +++ b/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json @@ -65,8 +65,8 @@ "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", + "old": "sha256_hashed_in_state:[HASH][0]", + "new": "sha256_hashed_in_state:[HASH][0]", "remote": "{}\n" }, "update_time": { 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..21753c21f0e 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 @@ -68,7 +68,7 @@ "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\"}]}" + "remote": "sha256_hashed_in_state:[HASH]" }, "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 cbb0a2d45ff..dea45cdfa41 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) } @@ -163,7 +163,7 @@ func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState, return fmt.Errorf("deleting state id=%s: %w", id, err) } } else { - 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) } @@ -208,7 +208,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) } @@ -291,7 +291,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) } @@ -299,6 +299,17 @@ 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. Fields already +// smaller than a hash placeholder are persisted as is. +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 5ee4c8469a9..413730bffd7 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 29e234c85ac..cb366f2c348 100644 --- a/bundle/direct/dresources/README.md +++ b/bundle/direct/dresources/README.md @@ -53,6 +53,20 @@ The planner only consults it for nodes without a state entry: once state exists 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 applies the same transform to **every** value entering the diff: saved state, the local config, and the remapped remote. Correctness rests on `CompactState` being a deterministic pure function of the value — equal values always compact to the same thing — so the comparisons (`Old==New` for a local change, `Remote==New` for remote drift) stay meaningful: two equal values compact to two equal hashes, two different values to two different hashes. 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 and is never read back from state by any code path (e.g. not consumed raw by `OverrideChangeDesc` or by state export). + +Hashing is skipped when it would not pay for itself: a value whose JSON encoding is no longer than the 87-byte placeholder is persisted as is. The verdict depends only on the value, so the state being saved and all three sides of the diff agree on it and stay comparable — including when the same field is small for one resource and large for another, or grows past the threshold between deploys. + +**`hashed_in_state` is orthogonal to `ignore_remote_changes`.** Because the remote side goes through the same transform, 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. + +No state version bump is needed for reading forward: legacy full-content state is compacted on read for comparison and rewritten compactly on the next save. A downgrade is the reverse case — an older CLI that predates `hashed_in_state` reads the stored hash as if it were the contents; since `serialized_dashboard` is not `ignore_local_changes`, the `Old(hash) != New(contents)` mismatch is read as a local change and republishes the dashboard once. That is non-destructive and self-correcting (the old CLI rewrites full contents; a later upgrade re-hashes on read), so no version gate is imposed against it. + ## 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..d7b8baf4f8b 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,54 @@ func TestDashboardState_JSONSerialization_PublishedField(t *testing.T) { assert.Contains(t, string(data), `"published":true`) } + +func TestDashboardCompactState(t *testing.T) { + requireLargeEnoughToHash(t, largeDashboard) + + state := &DashboardState{ + DashboardConfig: resources.DashboardConfig{ + DisplayName: "test-dashboard", + Etag: "etag-123", + SerializedDashboard: largeDashboard, + }, + } + + 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, largeDashboard, 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 4099c536281..face3407374 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..02cb4fbaafe --- /dev/null +++ b/bundle/direct/dresources/state_compaction.go @@ -0,0 +1,116 @@ +package dresources + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "reflect" + "strings" + + "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:" + +// stateHashPlaceholderLen is the exact size of a placeholder: the prefix followed by a +// SHA-256 digest in hex (sha256.Size bytes, two characters each). +const stateHashPlaceholderLen = len(stateHashPrefix) + sha256.Size*2 + +// 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, a value that is already a +// placeholder, and a value too small to be worth hashing are returned unchanged. +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 + } + + // json.Marshal sorts map keys, so the digest is stable across runs for equal values. + data, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("marshalling value for hashing: %w", err) + } + + // Compare against the JSON encoding, since that is what the state file stores + if len(data) <= stateHashPlaceholderLen { + return v, nil + } + + sum := sha256.Sum256(data) + + return stateHashPrefix + hex.EncodeToString(sum[:]), 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. A field whose contents are already no larger than a placeholder is left as +// is (see hashStateValue). 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..5f171f72fe4 --- /dev/null +++ b/bundle/direct/dresources/state_compaction_test.go @@ -0,0 +1,216 @@ +package dresources + +import ( + "encoding/json" + "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" +) + +const ( + // largeDashboard is a serialized_dashboard whose JSON encoding exceeds + // stateHashPlaceholderLen, so it is actually compacted. + largeDashboard = `{"pages":[{"name":"p1","displayName":"Page One","layout":[{"widget":{"name":"w1"}}]}]}` + + // smallDashboard is a serialized_dashboard whose JSON encoding fits within + // stateHashPlaceholderLen, so hashing it would grow the state and it is persisted raw. + smallDashboard = `{"pages":[{"name":"p1"}]}` +) + +// requireLargeEnoughToHash asserts a fixture is on the hashed side of the size threshold. +// Whether a value is hashed depends on the length of its JSON encoding, so shrinking a +// fixture would silently leave it raw and make the tests below assert the opposite of what +// they were written for. Fail with an explicit message instead. +func requireLargeEnoughToHash(t *testing.T, content string) { + t.Helper() + encoded, err := json.Marshal(content) + require.NoError(t, err) + require.Greater(t, len(encoded), stateHashPlaceholderLen, + "fixture must encode to more than %d bytes to be hashed; enlarge it", stateHashPlaceholderLen) +} + +// requireTooSmallToHash is the inverse of requireLargeEnoughToHash: it asserts a fixture +// stays on the raw side of the threshold, so enlarging it cannot silently turn a +// persisted-raw test into a hashing test that passes for the wrong reason. +func requireTooSmallToHash(t *testing.T, content string) { + t.Helper() + encoded, err := json.Marshal(content) + require.NoError(t, err) + require.LessOrEqual(t, len(encoded), stateHashPlaceholderLen, + "fixture must encode to at most %d bytes to be persisted raw; shrink it", stateHashPlaceholderLen) +} + +// 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) { + requireLargeEnoughToHash(t, largeDashboard) + + legacy := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: largeDashboard}} + config := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: largeDashboard}} + + 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. +func TestHashStateValue(t *testing.T) { + requireLargeEnoughToHash(t, largeDashboard) + + stringHash, err := hashStateValue(largeDashboard) + require.NoError(t, err) + require.IsType(t, "", stringHash) + assert.True(t, strings.HasPrefix(stringHash.(string), stateHashPrefix)) + + again, err := hashStateValue(largeDashboard) + require.NoError(t, err) + assert.Equal(t, stringHash, again) + + other, err := hashStateValue(strings.Replace(largeDashboard, "p1", "p2", 1)) + 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) { + requireLargeEnoughToHash(t, largeDashboard) + + hashed, err := hashStateValue(largeDashboard) + require.NoError(t, err) + + again, err := hashStateValue(hashed) + require.NoError(t, err) + assert.Equal(t, hashed, again) +} + +// TestHashStateValueSkipsSmallValues verifies a value whose JSON encoding would not +// shrink is left raw, so state never grows to hold a placeholder longer than the content +// it replaces. The boundary cases pin the comparison to the JSON encoding (which adds the +// surrounding quotes for a string) rather than the raw string length. +func TestHashStateValueSkipsSmallValues(t *testing.T) { + // A string encodes with two surrounding quotes, so this is exactly at the limit. + atLimit := strings.Repeat("x", stateHashPlaceholderLen-2) + out, err := hashStateValue(atLimit) + require.NoError(t, err) + assert.Equal(t, atLimit, out) + + overLimit := atLimit + "x" + out, err = hashStateValue(overLimit) + require.NoError(t, err) + assert.True(t, strings.HasPrefix(out.(string), stateHashPrefix)) + assert.Len(t, out, stateHashPlaceholderLen) +} + +// TestCompactStateSkipsSmallField verifies the size check reaches CompactState, so a +// small field stays raw in the state it saves and on every side of the diff it compacts. +// Persisting it raw is what keeps the state smaller than a hash placeholder would. +func TestCompactStateSkipsSmallField(t *testing.T) { + requireTooSmallToHash(t, smallDashboard) + + state := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: smallDashboard}} + + cfg := GetResourceConfig("dashboards") + out, err := CompactState(cfg, state) + require.NoError(t, err) + assert.Equal(t, smallDashboard, out.(*DashboardState).SerializedDashboard) + + // The persisted form holds the content itself, and holding it costs less than the + // placeholder that replacing it would have written. + persisted, err := json.Marshal(out.(*DashboardState).SerializedDashboard) + require.NoError(t, err) + assert.LessOrEqual(t, len(persisted), stateHashPlaceholderLen) + + // Compacting the already-raw state again leaves it alone, so repeated saves and every + // side of the diff keep comparing content against content. + out2, err := CompactState(cfg, out) + require.NoError(t, err) + assert.Equal(t, smallDashboard, out2.(*DashboardState).SerializedDashboard) +} + +// TestCompactStateHashesLargeField is the counterpart of TestCompactStateSkipsSmallField: +// once the content outgrows a placeholder, compaction replaces it and the state shrinks. +func TestCompactStateHashesLargeField(t *testing.T) { + requireLargeEnoughToHash(t, largeDashboard) + + state := &DashboardState{DashboardConfig: resources.DashboardConfig{SerializedDashboard: largeDashboard}} + + out, err := CompactState(GetResourceConfig("dashboards"), state) + require.NoError(t, err) + compacted := out.(*DashboardState).SerializedDashboard + require.IsType(t, "", compacted) + assert.True(t, strings.HasPrefix(compacted.(string), stateHashPrefix)) + assert.Len(t, compacted, stateHashPlaceholderLen) + + // The whole point of hashing: the persisted form is smaller than the raw content. + persisted, err := json.Marshal(compacted) + require.NoError(t, err) + raw, err := json.Marshal(largeDashboard) + require.NoError(t, err) + assert.Less(t, len(persisted), len(raw)) +} + +// 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/fingerprint_test.go b/libs/cache/fingerprint_test.go index 8a85f7944ce..6fb5df37fff 100644 --- a/libs/cache/fingerprint_test.go +++ b/libs/cache/fingerprint_test.go @@ -32,3 +32,23 @@ func TestFingerprintStability(t *testing.T) { assert.Equal(t, hash1ToCompare, hash1) assert.NotEqual(t, hash1, hash2) } + +// TestFingerprintMapKeyOrder tests that maps hash independently of key insertion order, +// which is what makes a map usable as a cache key (json.Marshal sorts map keys). +func TestFingerprintMapKeyOrder(t *testing.T) { + hash1, err := fingerprintToHash(map[string]int{"a": 1, "b": 2}) + require.NoError(t, err) + + hash2, err := fingerprintToHash(map[string]int{"b": 2, "a": 1}) + require.NoError(t, err) + + assert.Equal(t, hash1, hash2) +} + +// TestFingerprintUnmarshallable tests that a value json.Marshal rejects is reported as an +// error rather than silently hashing to a constant. +func TestFingerprintUnmarshallable(t *testing.T) { + hash, err := fingerprintToHash(func() {}) + assert.Error(t, err) + assert.Empty(t, hash) +}