Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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:[DASHBOARD_ID][DASHBOARD_ID]",
"warehouse_id": "[TEST_DEFAULT_WAREHOUSE_ID]"
}
}
Expand Down
2 changes: 1 addition & 1 deletion acceptance/bundle/migrate/dashboards/out.new_state.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:9dc171d249749a592c717bb13f2d948d2f1fa5530b82ac308a3df0c48d347ef3",
"warehouse_id": "123456"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,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:9dc171d249749a592c717bb13f2d948d2f1fa5530b82ac308a3df0c48d347ef3",
"new": "sha256:9dc171d249749a592c717bb13f2d948d2f1fa5530b82ac308a3df0c48d347ef3",
"remote": "sha256:aae99d4284b3390b1b35974f7bd4c03603cacd6a5a4a838ca563f652257ab6a7"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,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:[ALPHANUMID]",
"new": "sha256:[ALPHANUMID]",
"remote": "sha256:[ALPHANUMID]"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
"serialized_dashboard": {
"action": "skip",
"reason": "etag_based",
"old": "{ }\n",
"new": "{ }\n",
"remote": "{}\n"
"old": "sha256:cf207804e[NUMID]becbb5703765c4978cd7a8466dbdd[NUMID]e571ad09",
"new": "sha256:cf207804e[NUMID]becbb5703765c4978cd7a8466dbdd[NUMID]e571ad09",
"remote": "sha256:bb511d1e2723214ac1c3710d[NUMID]d57e31b0145d33d427d9628b209be38"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,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:a196fded8b5e0ebe5af2c6bed934d90fe055644de7773bbd84c851b577ff4f19",
"new": "sha256:a196fded8b5e0ebe5af2c6bed934d90fe055644de7773bbd84c851b577ff4f19",
"remote": "sha256:df70601c54be8ac2a66ac4a01dbd5224f4fa20ca9e8d15a615beab794cfdca08"
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions acceptance/bundle/resources/genie_spaces/inline/out.plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
"serialized_space": {
"action": "skip",
"reason": "etag_based",
"old": "{\"config\":{\"sample_questions\":[{\"id\":\"sq-001\",\"question\":[\"What is the total revenue?\"]}]},\"data_sources\":{\"tables\":[{\"column_configs\":[{\"column_name\":\"amount\",\"get_example_values\":true}],\"identifier\":\"main.sales.orders\"}]},\"version\":1}",
"new": "{\"config\":{\"sample_questions\":[{\"id\":\"sq-001\",\"question\":[\"What is the total revenue?\"]}]},\"data_sources\":{\"tables\":[{\"column_configs\":[{\"column_name\":\"amount\",\"get_example_values\":true}],\"identifier\":\"main.sales.orders\"}]},\"version\":1}",
"remote": "{\"config\":{\"sample_questions\":[{\"id\":\"sq-001\",\"question\":[\"What is the total revenue?\"]}]},\"data_sources\":{\"tables\":[{\"column_configs\":[{\"column_name\":\"amount\",\"get_example_values\":true}],\"identifier\":\"main.sales.orders\"}]},\"version\":2}"
"old": "sha256:ea4b505f061a2b65db49891ea5cab6ac875350aa7a8afed4778860a50d8f4db5",
"new": "sha256:ea4b505f061a2b65db49891ea5cab6ac875350aa7a8afed4778860a50d8f4db5",
"remote": "sha256:8df899b73697f19d25ed51f3a97ddce65f97d61cd7fc3b402aef349eb2fe93e8"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ Plan: 0 to add, 0 to change, 0 to delete, 1 unchanged
=== serialized_space: local version 1, remote version 2, skipped (etag_based){
"action": "skip",
"reason": "etag_based",
"local_version": 1,
"remote_version": 2
"local_remote_differ": true
}

>>> [CLI] bundle destroy --auto-approve
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ trace $CLI bundle plan
# The mismatch the etag masks: the local config stays version 1 while the remote
# (and the etag guarding it) is version 2. The serialized_space change is
# recorded but skipped via the etag_based rule.
#
# serialized_space is stored in state as a content hash (it is never compared
# against the remote value), so the plan reports old/new/remote as hashes, not
# the full content. We assert that local and remote differ and that the change is
# skipped, rather than parsing version numbers out of the content.
title "serialized_space: local version 1, remote version 2, skipped (etag_based)"
$CLI bundle plan -o json \
| jq '.plan["resources.genie_spaces.foo"].changes.serialized_space
| {action, reason,
local_version: (.new | fromjson | .version),
remote_version: (.remote | fromjson | .version)}'
| {action, reason, local_remote_differ: (.new != .remote)}'
18 changes: 14 additions & 4 deletions bundle/direct/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ func (d *DeploymentUnit) Deploy(ctx context.Context, db *dstate.DeploymentState,
}
}

// saveState compacts the state (replacing large equality-only fields with content
// hashes, see Adapter.CompactState) before persisting it.
func (d *DeploymentUnit) saveState(db *dstate.DeploymentState, id string, newState any) error {
compacted, err := d.Adapter.CompactState(newState)
if err != nil {
return fmt.Errorf("compacting state: %w", err)
}
return db.SaveState(d.ResourceKey, id, compacted, d.DependsOn)
}

func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState, newState any) error {
var newID string
var remoteState any
Expand All @@ -75,7 +85,7 @@ func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState,
return err
}

err = db.SaveState(d.ResourceKey, newID, newState, d.DependsOn)
err = d.saveState(db, newID, newState)
if err != nil {
return fmt.Errorf("saving state after creating id=%s: %w", newID, err)
}
Expand Down Expand Up @@ -146,7 +156,7 @@ func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState,
return err
}

err = db.SaveState(d.ResourceKey, id, newState, d.DependsOn)
err = d.saveState(db, id, newState)
if err != nil {
return fmt.Errorf("saving state id=%s: %w", id, err)
}
Expand Down Expand Up @@ -190,7 +200,7 @@ func (d *DeploymentUnit) UpdateWithID(ctx context.Context, db *dstate.Deployment
return err
}

err = db.SaveState(d.ResourceKey, newID, newState, d.DependsOn)
err = d.saveState(db, newID, newState)
if err != nil {
return fmt.Errorf("saving state id=%s: %w", oldID, err)
}
Expand Down Expand Up @@ -252,7 +262,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.saveState(db, id, newState)
if err != nil {
return fmt.Errorf("saving state id=%s: %w", id, err)
}
Expand Down
11 changes: 10 additions & 1 deletion bundle/direct/bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,22 @@ func (b *DeploymentBundle) Bind(ctx context.Context, client *databricks.Workspac
}
}

adapter, err := b.getAdapterForKey(resourceKey)
if err != nil {
return nil, err
}
compacted, err := adapter.CompactState(sv.Value)
if err != nil {
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
Expand Down
2 changes: 1 addition & 1 deletion bundle/direct/bundle_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (b *DeploymentBundle) Apply(ctx context.Context, client *databricks.Workspa
logdiag.LogError(ctx, fmt.Errorf("state entry not found for %q", resourceKey))
return false
}
err = b.StateDB.SaveState(resourceKey, id, sv.Value, entry.DependsOn)
err = d.saveState(&b.StateDB, id, sv.Value)
} else {
// TODO: redo calcDiff to downgrade planned action if possible (?)
err = d.Deploy(ctx, &b.StateDB, sv.Value, action, entry)
Expand Down
27 changes: 25 additions & 2 deletions bundle/direct/bundle_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
return false
}

// Compact every value that enters the diff so they share one comparison form.
// For saved state this also hashes legacy full-content entries on the fly; the
// on-disk entry is rewritten compactly on the next save (lazy migration).
savedState, err = adapter.CompactState(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.
Expand All @@ -208,7 +217,15 @@ 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 := adapter.CompactState(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
Expand Down Expand Up @@ -241,7 +258,13 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks
return false
}

remoteDiff, err = structdiff.GetStructDiff(remoteStateComparable, sv.Value, adapter.KeyedSlices())
remoteStateComparable, err = adapter.CompactState(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
Expand Down
11 changes: 11 additions & 0 deletions bundle/direct/dresources/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ 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.

## CompactState: storing large fields as content hashes

Implement the optional `CompactState(state *T) (*T, error)` method when a field holds large content that is only ever compared for equality and never read back from state. It returns a copy of the state with such fields replaced by a content hash (use `hashStateValue`), and the framework applies it both before persisting state and to every value entering the diff, so stored and compared values share one form.

A field qualifies only if **all** of the following hold:
- it is declared `ignore_remote_changes` (so it is never meaningfully compared against the remote value — typically `etag_based` drift detection),
- it is not read back from state by any code path (e.g. not consumed raw by `OverrideChangeDesc` or by state export), and
- it can be large (a small field gains nothing — the hash placeholder is ~70 bytes).

`dashboards.serialized_dashboard` and `genie_spaces.serialized_space` use this: both inline a file's contents into state, both detect drift via `etag`, and both always send the full contents to the API from the plan's `new_state`. As a result the plan reports such fields as hashes (`sha256:...`) rather than full content. No state version bump is needed: legacy full-content state is hashed on read for comparison and rewritten compactly on the next save. Add a test asserting the field is `ignore_remote_changes` to guard the invariant.

## OverrideChangeDesc

Use `OverrideChangeDesc` only as a last resort when `resources.yml` settings cannot express the needed logic. Skipping an action with `change.Action = deployplan.Skip` in `OverrideChangeDesc` creates a silent no-op: the plan shows no change even if the user's config differs from remote. Document the skip reason clearly in both the comment and `change.Reason`.
Expand Down
37 changes: 37 additions & 0 deletions bundle/direct/dresources/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ type IResource interface {
// [Optional] OverrideChangeDesc can implement custom logic to update a given ChangeDesc; it is run last after built-in classifiers and field triggers.
OverrideChangeDesc(ctx context.Context, path *structpath.PathNode, changedesc *ChangeDesc, remoteState any) error

// [Optional] CompactState returns a copy of state with large, equality-only fields
// (e.g. dashboards' serialized_dashboard) replaced by content hashes before the state
// is persisted and diffed. Must be idempotent. Only valid for fields that are
// ignore_remote_changes and never read back from state.
// Example: func (r *ResourceDashboard) CompactState(state *DashboardState) (*DashboardState, error)
CompactState(state any) (any, error)

// DoCreate creates a new resource from the newState. Returns id of the resource and optionally remote state.
// If remote state is available as part of the operation, return it; otherwise return nil.
// Example: func (r *ResourceVolume) DoCreate(ctx context.Context, newState *catalog.CreateVolumeRequestContent) (string, *catalog.VolumeInfo, error)
Expand Down Expand Up @@ -102,6 +109,7 @@ type Adapter struct {
waitAfterUpdate *calladapt.BoundCaller
waitAfterDelete *calladapt.BoundCaller
overrideChangeDesc *calladapt.BoundCaller
compactState *calladapt.BoundCaller
doResize *calladapt.BoundCaller

resourceConfig *ResourceLifecycleConfig
Expand Down Expand Up @@ -135,6 +143,7 @@ func NewAdapter(typedNil any, resourceType string, client *databricks.WorkspaceC
waitAfterUpdate: nil,
waitAfterDelete: nil,
overrideChangeDesc: nil,
compactState: nil,
resourceConfig: GetResourceConfig(resourceType),
generatedResourceConfig: GetGeneratedResourceConfig(resourceType),
keyedSlices: nil,
Expand Down Expand Up @@ -226,6 +235,11 @@ func (a *Adapter) initMethods(resource any) error {
return err
}

a.compactState, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "CompactState")
if err != nil {
return err
}

a.doResize, err = calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), "DoResize")
if err != nil {
return err
Expand Down Expand Up @@ -339,6 +353,13 @@ func (a *Adapter) validate() error {
validations = append(validations, "WaitAfterUpdate remoteState return", a.waitAfterUpdate.OutTypes[0], remoteType)
}

if a.compactState != nil {
validations = append(validations,
"CompactState input", a.compactState.InTypes[0], stateType,
"CompactState return", a.compactState.OutTypes[0], stateType,
)
}

err = validateTypes(validations...)
if err != nil {
return err
Expand Down Expand Up @@ -556,6 +577,22 @@ func (a *Adapter) KeyedSlices() map[string]any {
return a.keyedSlices
}

// CompactState returns a copy of state with large, equality-only fields replaced by
// content hashes for compact storage, or state unchanged if the resource does not
// implement CompactState. It is applied both before persisting state and to every
// value entering the state diff, so stored and compared values share one form.
func (a *Adapter) CompactState(state any) (any, error) {
if a.compactState == nil {
return state, nil
}

outs, err := a.compactState.Call(state)
if err != nil {
return nil, err
}
return outs[0], nil
}

// prepareCallRequired prepares a call and ensures the method is found.
func prepareCallRequired(resource any, methodName string) (*calladapt.BoundCaller, error) {
caller, err := calladapt.PrepareCall(resource, reflect.TypeFor[IResource](), methodName)
Expand Down
16 changes: 16 additions & 0 deletions bundle/direct/dresources/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,22 @@ func (r *ResourceDashboard) RemapState(state *DashboardState) *DashboardState {
}
}

// CompactState replaces the inlined serialized_dashboard contents with a content
// hash before the state is persisted. The full contents are never needed back from
// state: remote drift is detected via etag (serialized_dashboard is ignore_remote_changes,
// see resources.yml), and a deploy always sends the contents from the plan's new_state,
// never from saved state. Hashing keeps resources.json small for dashboards whose
// serialized form can be several megabytes.
func (r *ResourceDashboard) CompactState(state *DashboardState) (*DashboardState, error) {
hashed, err := hashStateValue(state.SerializedDashboard)
if err != nil {
return nil, err
}
compacted := *state
compacted.SerializedDashboard = hashed
return &compacted, nil
}

func (r *ResourceDashboard) DoRead(ctx context.Context, id string) (*DashboardState, error) {
var dashboard *dashboards.Dashboard
var publishedDashboard *dashboards.PublishedDashboard
Expand Down
Loading
Loading