Skip to content

Commit

Permalink
jsonformat: render forget-only attr changes as no-op (#34403)
Browse files Browse the repository at this point in the history
* jsonplan: document forget actions

* jsonformat: format forget changes as no-op

Previous to this commit, forget-only actions (i.e. "forget", not "create then forget") would be rendered using the forget action symbol for the top-level resource, and the delete action symbol for each resource attribute, with a new value of "null". This attribute rendering is identical to that for resource deletion, which might suggest to some users that Terraform plans to delete the resource, not just remove it from state.
This commit tweaks the renderer so forget-only changes render as no-ops but with the forget action symbol and resource change comment.
  • Loading branch information
kmoe authored Dec 12, 2023
1 parent 2e91b71 commit 8f3aa0e
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 15 deletions.
11 changes: 7 additions & 4 deletions internal/command/jsonformat/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,19 +572,22 @@ func TestResourceChange_primitiveTypes(t *testing.T) {
Action: plans.Forget,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-123"),
}),
After: cty.NullVal(cty.EmptyObject),
Schema: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Computed: true},
"id": {Type: cty.String, Computed: true},
"ami": {Type: cty.String, Optional: true},
},
},
RequiredReplace: cty.NewPathSet(),
ExpectedOutput: ` # test_instance.example will no longer be managed by Terraform, but will not be destroyed
# (destroy = false is set in the configuration)
. resource "test_instance" "example" {
- id = "i-02ae66f368e8518a9" -> null
id = "i-02ae66f368e8518a9"
# (1 unchanged attribute hidden)
}`,
},
"forget (deposed)": {
Expand All @@ -605,7 +608,7 @@ func TestResourceChange_primitiveTypes(t *testing.T) {
# (left over from a partially-failed replacement of this instance)
# (destroy = false is set in the configuration)
. resource "test_instance" "example" {
- id = "i-02ae66f368e8518a9" -> null
id = "i-02ae66f368e8518a9"
}`,
},
"create-then-forget": {
Expand Down
11 changes: 10 additions & 1 deletion internal/command/jsonformat/structured/change.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ type Change struct {
// FromJsonChange unmarshals the raw []byte values in the jsonplan.Change
// structs into generic interface{} types that can be reasoned about.
func FromJsonChange(change jsonplan.Change, relevantAttributes attribute_path.Matcher) Change {
return Change{
ret := Change{
Before: unmarshalGeneric(change.Before),
After: unmarshalGeneric(change.After),
Unknown: unmarshalGeneric(change.AfterUnknown),
Expand All @@ -103,6 +103,15 @@ func FromJsonChange(change jsonplan.Change, relevantAttributes attribute_path.Ma
ReplacePaths: attribute_path.Parse(change.ReplacePaths, false),
RelevantAttributes: relevantAttributes,
}

// A forget-only action (i.e. ["forget"], not ["create", "forget"])
// should be represented as a no-op, so it does not look like we are
// proposing to delete the resource.
if len(change.Actions) == 1 && change.Actions[0] == "forget" {
ret = ret.AsNoOp()
}

return ret
}

// FromJsonResource unmarshals the raw values in the jsonstate.Resource structs
Expand Down
22 changes: 12 additions & 10 deletions internal/command/jsonplan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,23 @@ type Change struct {
// ["create"]
// ["read"]
// ["update"]
// ["delete", "create"]
// ["create", "delete"]
// ["delete", "create"] (replace)
// ["create", "delete"] (replace)
// ["delete"]
// ["forget"]
// The two "replace" actions are represented in this way to allow callers to
// e.g. just scan the list for "delete" to recognize all three situations
// where the object will be deleted, allowing for any new deletion
// combinations that might be added in future.
// ["create", "forget"] (replace)
// The three "replace" actions are represented in this way to allow callers
// to, e.g., just scan the list for "delete" to recognize all three
// situations where the object will be deleted, allowing for any new
// deletion combinations that might be added in future.
Actions []string `json:"actions,omitempty"`

// Before and After are representations of the object value both before and
// after the action. For ["create"] and ["delete"] actions, either "before"
// or "after" is unset (respectively). For ["no-op"], the before and after
// values are identical. The "after" value will be incomplete if there are
// values within it that won't be known until after apply.
// after the action. For ["create"] and ["delete"]/["forget"] actions,
// either "before" or "after" is unset (respectively). For ["no-op"], the
// before and after values are identical. The "after" value will be
// incomplete if there are values within it that won't be known until after
// apply.
Before json.RawMessage `json:"before,omitempty"`
After json.RawMessage `json:"after,omitempty"`

Expand Down

0 comments on commit 8f3aa0e

Please sign in to comment.