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..c177eb0b450 100644 --- a/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json +++ b/acceptance/bundle/resources/dashboards/detect-change/out.plan.direct.json @@ -25,7 +25,7 @@ "lifecycle_state": "ACTIVE", "parent_path": "/Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources", "path": "/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources/test-dashboard-[UNIQUE_NAME].lvdash.json", - "published": true, + "published": false, "serialized_dashboard": "{}\n", "update_time": "[TIMESTAMP]", "warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]" @@ -56,6 +56,12 @@ "reason": "spec:output_only", "remote": "/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/resources/test-dashboard-[UNIQUE_NAME].lvdash.json" }, + "published": { + "action": "update", + "old": true, + "new": true, + "remote": false + }, "serialized_dashboard": { "action": "skip", "reason": "etag_based", diff --git a/acceptance/bundle/resources/dashboards/publish-failure-stale-content/output.txt b/acceptance/bundle/resources/dashboards/publish-failure-stale-content/output.txt index 8b0f79427d2..a88b5057bc6 100644 --- a/acceptance/bundle/resources/dashboards/publish-failure-stale-content/output.txt +++ b/acceptance/bundle/resources/dashboards/publish-failure-stale-content/output.txt @@ -84,10 +84,14 @@ The remote modifications will be lost. json.plan.resources.dashboards.dashboard1.new_state.value.published = true; json.plan.resources.dashboards.dashboard1.remote_state.etag = "[ETAG_2]"; -json.plan.resources.dashboards.dashboard1.remote_state.published = true; +json.plan.resources.dashboards.dashboard1.remote_state.published = false; json.plan.resources.dashboards.dashboard1.changes.etag.action = "update"; json.plan.resources.dashboards.dashboard1.changes.etag.old = "[ETAG_1]"; json.plan.resources.dashboards.dashboard1.changes.etag.remote = "[ETAG_2]"; +json.plan.resources.dashboards.dashboard1.changes.published.action = "update"; +json.plan.resources.dashboards.dashboard1.changes.published.old = true; +json.plan.resources.dashboards.dashboard1.changes.published.new = true; +json.plan.resources.dashboards.dashboard1.changes.published.remote = false; json.plan.resources.dashboards.dashboard1.changes.serialized_dashboard.reason = "etag_based"; >>> [CLI] bundle deploy diff --git a/acceptance/bundle/resources/dashboards/republish-after-draft-update/dashboard.lvdash.json b/acceptance/bundle/resources/dashboards/republish-after-draft-update/dashboard.lvdash.json new file mode 100644 index 00000000000..397a9a1259c --- /dev/null +++ b/acceptance/bundle/resources/dashboards/republish-after-draft-update/dashboard.lvdash.json @@ -0,0 +1,34 @@ +{ + "pages": [ + { + "displayName": "New Page", + "layout": [ + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 0 + }, + "widget": { + "name": "82eb9107", + "textbox_spec": "# I'm a title" + } + }, + { + "position": { + "height": 2, + "width": 6, + "x": 0, + "y": 2 + }, + "widget": { + "name": "ffa6de4f", + "textbox_spec": "Text" + } + } + ], + "name": "fdd21a3c" + } + ] +} diff --git a/acceptance/bundle/resources/dashboards/republish-after-draft-update/databricks.yml.tmpl b/acceptance/bundle/resources/dashboards/republish-after-draft-update/databricks.yml.tmpl new file mode 100644 index 00000000000..0fae00d3fcc --- /dev/null +++ b/acceptance/bundle/resources/dashboards/republish-after-draft-update/databricks.yml.tmpl @@ -0,0 +1,12 @@ +bundle: + name: republish-after-draft-update + +workspace: + root_path: ~/.bundle/$UNIQUE_NAME + +resources: + dashboards: + dashboard1: + display_name: test-dashboard-$UNIQUE_NAME + file_path: ./dashboard.lvdash.json + warehouse_id: $TEST_DEFAULT_WAREHOUSE_ID diff --git a/acceptance/bundle/resources/dashboards/republish-after-draft-update/out.test.toml b/acceptance/bundle/resources/dashboards/republish-after-draft-update/out.test.toml new file mode 100644 index 00000000000..fd5c0ccb0ad --- /dev/null +++ b/acceptance/bundle/resources/dashboards/republish-after-draft-update/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false +RequiresWarehouse = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +EnvMatrix.Ignore = ["databricks.yml"] diff --git a/acceptance/bundle/resources/dashboards/republish-after-draft-update/output.txt b/acceptance/bundle/resources/dashboards/republish-after-draft-update/output.txt new file mode 100644 index 00000000000..3593cbf24ba --- /dev/null +++ b/acceptance/bundle/resources/dashboards/republish-after-draft-update/output.txt @@ -0,0 +1,41 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/[UNIQUE_NAME]/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +=== Plan right after deploy -- published is current, no change: +>>> [CLI] bundle plan -o json +null + +=== Plan after out-of-band draft update -- a republish is owed: +>>> [CLI] bundle plan -o json +Warning: dashboard "dashboard1" has been modified remotely + at resources.dashboards.dashboard1 + in databricks.yml:10:7 + +This dashboard has been modified remotely since the last bundle deployment. +These modifications are untracked and will be overwritten on deploy. + +Make sure that the local dashboard definition matches what you intend to deploy +before proceeding with the deployment. + +To overwrite the remote changes with your local version, use --force. +The remote modifications will be lost. + +{ + "action": "update", + "old": true, + "new": true, + "remote": false +} + +>>> [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/[UNIQUE_NAME] + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/dashboards/republish-after-draft-update/script b/acceptance/bundle/resources/dashboards/republish-after-draft-update/script new file mode 100644 index 00000000000..7e131a5de78 --- /dev/null +++ b/acceptance/bundle/resources/dashboards/republish-after-draft-update/script @@ -0,0 +1,28 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +cleanup() { + trace $CLI bundle destroy --auto-approve +} +trap cleanup EXIT + +# Deploy: the dashboard is created and published, so the published revision matches the +# draft (revision_create_time >= update_time). +trace $CLI bundle deploy +DASHBOARD_ID=$($CLI bundle summary --output json | jq -r '.resources.dashboards.dashboard1.id') +echo "$DASHBOARD_ID:DASHBOARD_ID" >> ACC_REPLS + +# A clean re-plan is a no-op: the published content matches the draft. +title "Plan right after deploy -- published is current, no change:" +trace $CLI bundle plan -o json | jq '.plan["resources.dashboards.dashboard1"].changes.published' + +# Make an out-of-band DRAFT update without republishing. This advances the draft's +# update_time past the published revision_create_time, so the published content is now +# stale. GetPublished still returns 200 (the previous revision), so the old +# "published = (GetPublished succeeds)" check reported published=true and MISSED this -- +# the regression this test guards. The timestamp comparison reports published=false, so +# the plan surfaces a pending republish (published: update, remote=false). +DASHBOARD_JSON="{\"serialized_dashboard\": \"{}\", \"warehouse_id\": \"$TEST_DEFAULT_WAREHOUSE_ID\"}" +$CLI lakeview update "$DASHBOARD_ID" --json "${DASHBOARD_JSON}" > /dev/null + +title "Plan after out-of-band draft update -- a republish is owed:" +trace $CLI bundle plan -o json | jq '.plan["resources.dashboards.dashboard1"].changes.published' diff --git a/acceptance/bundle/resources/dashboards/republish-after-draft-update/test.toml b/acceptance/bundle/resources/dashboards/republish-after-draft-update/test.toml new file mode 100644 index 00000000000..a40dd40986a --- /dev/null +++ b/acceptance/bundle/resources/dashboards/republish-after-draft-update/test.toml @@ -0,0 +1,27 @@ +# Regression test for stale-publish detection. The dashboard "published" state is a +# direct-engine concept derived from the publish/update timestamps, so run direct only. +Cloud = false +Local = true +RequiresWarehouse = true +# This test asserts plan output, not the recorded request sequence, so keep request +# recording off (inherited as true from the parent) to avoid flaky ordering. +RecordRequests = false + +[EnvMatrix] +DATABRICKS_BUNDLE_ENGINE = ["direct"] + +Ignore = [ + "databricks.yml", +] + +[Env] +# This test makes an out-of-band draft update and expects `bundle plan` to detect that a +# republish is owed. The inherited eventual-consistency simulation would make the plan's +# own read observe stale values and mask the change, so opt out of it here. +INJECT_STALE_ON_DIRECT = "0" +MSYS_NO_PATHCONV = "1" + +[[Repls]] +Old = '2\d\d\d-\d\d-\d\d(T| )\d\d:\d\d:\d\d(\.\d+(Z|\+\d\d:\d\d)?)?Z' +New = "[TIMESTAMP]" +Order = 9 diff --git a/bundle/configsync/defaults.go b/bundle/configsync/defaults.go index 4dedadedfc9..4dff5c143db 100644 --- a/bundle/configsync/defaults.go +++ b/bundle/configsync/defaults.go @@ -120,6 +120,9 @@ var serverSideDefaults = map[string]any{ // modified-remotely detection), so configsync cannot rely on the plan's Skip // action and must exclude the field explicitly. "resources.dashboards.*.etag": alwaysSkip, + // published is an internal state field (not part of the user config): DoRead + // derives it from the publish lifecycle, so it must never be synced into config. + "resources.dashboards.*.published": alwaysSkip, } // shouldSkipField checks if a field should be skipped in change detection. diff --git a/bundle/direct/dresources/dashboard.go b/bundle/direct/dresources/dashboard.go index bb3b3921a43..aaf1feea616 100644 --- a/bundle/direct/dresources/dashboard.go +++ b/bundle/direct/dresources/dashboard.go @@ -7,6 +7,7 @@ import ( "path" "slices" "strings" + "time" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/deployplan" @@ -27,6 +28,16 @@ type ResourceDashboard struct { type DashboardState struct { resources.DashboardConfig + // Published reports whether the current draft content is published. DoRead sets it + // to true only when GetPublished succeeds AND the published revision is at least as + // new as the draft (revision_create_time >= update_time). A draft update without a + // republish leaves the previously-published content in place, but the API keeps + // returning it from GetPublished, so a naive "does GetPublished succeed" cannot + // detect stale content; the timestamp comparison can. + // + // The desired value (PrepareState) is always true, so when the published content is + // stale, remote Published=false differs from desired and the planner runs DoUpdate, + // which republishes. Published bool `json:"published"` } @@ -141,8 +152,14 @@ func (r *ResourceDashboard) DoRead(ctx context.Context, id string) (*DashboardSt return nil, apierr.ErrNotFound } - // Determine if the dashboard is published - published := publishedErr == nil + // A dashboard is published (in the sense that matters for planning: the current + // draft content is what's published) only when GetPublished succeeds AND the + // published revision is at least as new as the draft. A draft update without a + // republish leaves stale content published, but GetPublished keeps returning it, + // so publishedErr == nil alone cannot detect staleness — the timestamp comparison + // can. If either timestamp is unparseable, fall back to publish-existence so we + // don't spuriously republish. See DashboardState.Published. + published := publishedErr == nil && publishedIsCurrent(dashboard, publishedDashboard) forceSendFields := utils.FilterFields[DashboardState](dashboard.ForceSendFields) // EmbedCredentials must always be included in ForceSendFields to ensure it's serialized @@ -181,6 +198,29 @@ func (r *ResourceDashboard) DoRead(ctx context.Context, id string) (*DashboardSt }, nil } +// publishedIsCurrent reports whether the published revision reflects the current +// draft, i.e. the dashboard was published after its last draft update. The published +// API exposes no etag, only revision_create_time, so we compare it against the draft's +// update_time. Both are RFC3339 timestamps set by the backend on a shared clock +// (verified on AWS/GCP/Azure/dogfood: a publish always leaves revision_create_time >= +// update_time, and a draft update pushes update_time ahead until the next publish). +// If either timestamp is missing or unparseable, return true so we do not force a +// spurious republish. +func publishedIsCurrent(dashboard *dashboards.Dashboard, published *dashboards.PublishedDashboard) bool { + if published == nil { + return false + } + updateTime, err := time.Parse(time.RFC3339, dashboard.UpdateTime) + if err != nil { + return true + } + revisionTime, err := time.Parse(time.RFC3339, published.RevisionCreateTime) + if err != nil { + return true + } + return !revisionTime.Before(updateTime) +} + func prepareDashboardRequest(config *DashboardState) (dashboards.Dashboard, error) { dashboard := dashboards.Dashboard{ DisplayName: config.DisplayName, diff --git a/libs/testserver/dashboards.go b/libs/testserver/dashboards.go index aa7f102849d..84b8527212a 100644 --- a/libs/testserver/dashboards.go +++ b/libs/testserver/dashboards.go @@ -8,7 +8,6 @@ import ( "path" "strconv" "strings" - "time" "github.com/databricks/databricks-sdk-go/service/dashboards" "github.com/databricks/databricks-sdk-go/service/workspace" @@ -142,7 +141,7 @@ func (s *FakeWorkspace) DashboardCreate(req Request) Response { dashboard.Path = dashboard.ParentPath + "/" + dashboard.DisplayName + ".lvdash.json" } - dashboard.CreateTime = strings.TrimSuffix(time.Now().UTC().Format(time.RFC3339), "Z") + dashboard.CreateTime = nextTimestamp() dashboard.UpdateTime = dashboard.CreateTime inputSerializedDashboard := dashboard.SerializedDashboard @@ -230,7 +229,7 @@ func (s *FakeWorkspace) DashboardUpdate(req Request) Response { updated.SerializedDashboard = transformSerializedDashboard(updateReq.SerializedDashboard, datasetCatalog, datasetSchema) } updated.WarehouseId = updateReq.WarehouseId - updated.UpdateTime = time.Now().UTC().Format(time.RFC3339) + updated.UpdateTime = nextTimestamp() // Write stages a stale value: like a real backend, the first read after an update // returns the pre-update value. Tests that read right after an update wait for the @@ -264,7 +263,7 @@ func (s *FakeWorkspace) DashboardPublish(req Request) Response { WarehouseId: dashboard.WarehouseId, DisplayName: dashboard.DisplayName, EmbedCredentials: publishReq.EmbedCredentials, - RevisionCreateTime: time.Now().UTC().Format(time.RFC3339), + RevisionCreateTime: nextTimestamp(), ForceSendFields: []string{"EmbedCredentials"}, } diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index 8d6e8ee0dd3..404742132af 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -128,6 +128,14 @@ func nowMilli() int64 { return lastNowMilli } +// nextTimestamp returns a strictly-increasing RFC3339 timestamp with nanosecond +// precision. The sub-second component keeps distinct events ordered even within the +// same wall-clock second, which the dashboard publish lifecycle relies on to compare +// a draft's update_time against a published revision's revision_create_time. +func nextTimestamp() string { + return time.Unix(0, nowNano()).UTC().Format(time.RFC3339Nano) +} + func nextUUID() string { var b [16]byte binary.BigEndian.PutUint64(b[0:8], uint64(nextID()))