direct: store serialized_dashboard/serialized_space in state as content hashes#5609
Draft
shreyas-goenka wants to merge 1 commit into
Draft
Conversation
…nt hashes The direct deploy engine persists the full planned config per resource in resources.json. For dashboards and genie spaces, that includes the inlined serialized_dashboard / serialized_space contents, which routinely run into the hundreds of KB to several MB. These fields are never read back from state: drift is detected via etag (they are ignore_remote_changes), and a deploy always sends the contents from the plan's new_state, never from saved state. Add an optional CompactState adapter method that replaces such equality-only fields with a "sha256:<hex>" content hash. The framework applies it both before persisting state and to every value entering the diff, so stored and compared values share one form and unchanged content still produces an equal-hash skip. The plan's new_state (sent to the API) and the raw remote_state snapshot keep full content. No state version bump: legacy full-content state is hashed on read for the comparison and rewritten compactly on the next save. Co-authored-by: Isaac
Contributor
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
The direct deploy engine persists the full planned config per resource in
resources.json. For dashboards and genie spaces that includes the inlinedserialized_dashboard/serialized_spacecontents, which routinely run from hundreds of KB to several MB (and roughly double once JSON-escaped into state). That content is never read back from state:etag(both fields areignore_remote_changes,etag_based), so the remote serialized value is never meaningfully compared;new_state, never from saved state;So the only thing the saved value is used for is a local "did the content change since last deploy?" equality check — which a hash serves exactly.
This PR adds an optional
CompactState(state *T) (*T, error)adapter method (same idiom asRemapState/OverrideChangeDesc) that replaces such equality-only fields with asha256:<hex>placeholder. The framework applies it both before persisting state and to every value entering the diff (saved state, the local config copy, and the remapped remote), so stored and compared values share one canonical form: unchanged content still yields an equal-hashskip, changed content yields a different hash, exactly as before.dashboards.serialized_dashboardandgenie_spaces.serialized_spaceimplement it. The plan'snew_state(sent to the API on apply) and the raw top-levelremote_statesnapshot keep their full content.Compatibility
No state version bump. Legacy full-content state is hashed on read for the comparison and rewritten compactly on the next save (lazy migration). An older CLI reading new state sees a hash, plans one redundant update, and rewrites full content — safe. Bumping the version would instead hard-fail older CLIs, a worse failure mode for mixed-version CI/teams.
User-visible effect
bundle planreports these fields assha256:...in thechangessection rather than embedding the (potentially multi-MB) serialized blob.resources.jsonshrinks correspondingly, as does the per-deploy state upload.Field selection
A field qualifies only if it is
ignore_remote_changes, is never read back from state, and is large enough to matter. Surveying all direct-engine resource types, only these two fields qualify today; the mechanism is declarative-by-method so a future file-inlined blob can opt in by implementingCompactState. A unit test guards the core invariant (the field must beignore_remote_changes).Tests
hashStateValue(determinism, idempotence, nil/empty),CompactStatefor both resources, and theignore_remote_changesinvariant guards.simple/detect-change/unpublish-out-of-band, genieinline, bind, migrate). Thegenie_spaces/version_migrationscript previously parsed the schema version out of the plan's serialized content; it now assertslocal_remote_differ+ theetag_basedskip, which is the behavior it was really demonstrating.This pull request and its description were written by Isaac.