feat(cli): show reason and property diff when a resource is replaced#425
Merged
Conversation
When formae plans to replace a resource because a createOnly (immutable)
property changed, the plan preview now shows *which* property(ies)
triggered the replacement, with old→new values, rendered with the same
diff tree used for in-place updates:
~ replace resource my-queue
of type FakeAWS::SQS::Queue
because these immutable properties changed:
- FifoQueue: false → true
The triggering patch ops — previously discarded by filterCreateOnlyFields
after the needsReplacement decision — are now preserved on the delete
half of the replace pair as ResourceUpdate.ReplacementPatchDocument
(operation-level metadata, NOT a field on Resource). The renderer
group-coalescer merges it onto the display update.
Scope is the plan-preview path only. Live apply output is unchanged.
The plugin boundary, Resource model, and datastore resource persistence
are untouched.
JeroenSoeters
requested changes
Apr 22, 2026
| OldStackName string `json:"OldStackName,omitempty"` | ||
| Operation string `json:"Operation"` | ||
| PatchDocument json.RawMessage `json:"PatchDocument,omitempty"` | ||
| ReplacementPatchDocument json.RawMessage `json:"ReplacementPatchDocument,omitempty"` |
Collaborator
There was a problem hiding this comment.
Can't we use the existing PatchDocument field?
Address review feedback on PR #425: the second patch field's name — ReplacementPatchDocument — read as a near-duplicate of PatchDocument and obscured what it actually carries. Rename to CreateOnlyPatch, which mirrors PatchDocument and describes the content (createOnly-field ops that force a replacement). GeneratePatch now returns (mutableOps, createOnlyOps, err); the caller checks len(createOnlyOps) > 0 to decide on a replace, which drops the redundant needsReplacement bool. Also drops the now-unused containsCreateOnlyFields helper and fixes the gofmt issue on ResourceUpdate that was tripping golangci-lint.
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.
Summary
When
formae applyplans a replacement (destroy + recreate) because acreateOnlyproperty changed, the plan preview now shows which immutable property(ies) triggered the replacement and their old→new values — rendered with the same diff tree used for in-place updates.Before:
After:
Approach
The triggering patch ops were previously computed and then discarded by
filterCreateOnlyFieldsafterneedsReplacementwas set. They're now preserved separately and plumbed through to the CLI:patch.GeneratePatchnow returns the strippedcreateOnlyops as a secondjson.RawMessage.ResourceUpdate(internal) andapimodel.ResourceUpdate(API) gainReplacementPatchDocumentas operation-level metadata.OperationReplacebranch that calls the existingFormatPatchDocument.Scope / Non-scope
formatSimulatedResourceUpdate). Live apply progress (formatResourceUpdate) is unchanged — it's deliberately terse.Resourcemodel untouched. The field lives onResourceUpdate, not onpkg/model/resource.go, because the replacement reason is operation metadata, not resource state. This keeps datastore resource persistence and secret-redaction paths untouched.resource_update_generator.go(target-replace / dependency-driven replace) passnil— they're not property-triggered, so there's no diff to show. The renderer's empty-check guard keeps their output identical to today.Test Plan
patch_document_test.go—TestGeneratePatchasserts the returned replacement ops contain exactly the createOnly pathsresource_update_generator_patch_test.go— extended the existing VPC-CIDR replace test to assert the delete half carriesReplacementPatchDocument, create half and implicit-delete do notrenderer_test.go— newTestRenderSimulation_Replacement_ShowsReasonrenders a replace and asserts the "because these immutable properties changed:" header + property name appearTestFormatHumanReadableStatus_Replacementcontinues to pass (no replacement patch → bare header, as today)tests/e2e/go/replace_test.go(AWS path) — simulate before apply, assertReplacementPatchDocumenton the delete half referencesRoleName(the createOnly field in the fixture). Runs against real AWS; not executed locally.patch,resource_update,resource_persister,metastructure,cli/renderer.