Commit Graph

42 Commits

Author SHA1 Message Date
tmshn fcc1a76d5e make plan-diff format a bit more dry 2019-12-03 19:02:59 +09:00
Simon Brady 7a9fa93724 command/plan: Fix panic in plan output with string containing null and whitespace (#23102)
* command/plan: Fix panic in plan output with string containing null and whitespace
* command/format: add test for null string with whitespace
2019-11-15 10:25:49 -05:00
Thayne McCombs a895a42f85 command/format: fix missing elements at the end of lists in diffs 2019-11-08 16:05:23 -08:00
Martin Atkins 7db2825646 command/format: multi-line rendering for unchanged strings
We have a special treatment for multi-line strings that are being updated
in-place where we show them across multiple lines in the plan output, but
we didn't use that same treatment for rendering multi-line strings in
isolation such as when they are being added for the first time.

Here we detect when we're rendering a multi-line string in a no-change
situation and render it using the diff renderer instead, using the same
value for old and new and thus producing a multi-line result without any
diff markers at all.

This improves consistency between the change and no-change cases, and
makes multi-line strings (such as YAML in block mode) readable in all
cases.
2019-11-07 15:25:40 -08:00
Martin Atkins d0cbbb6a00 command/format: Remove defunct "Plan" type and associated symbols
This "Plan" type, along with the other types it directly or indirectly
embeds and the associated functions, are adaptations of the
flatmap-oriented plan renderer logic from Terraform 0.11 and prior.

The current diff rendering logic is in diff.go, and so the contents of the
plan.go file are defunct apart from the DiffActionSymbol function that
both implementations share. Therefore here we move DiffActionSymbol into
diff.go and then remove plan.go entirely, in the interests of dead code
removal.
2019-11-06 06:53:32 -08:00
James Bardin f79a768a4e command/format: take noop changes from lcs
When rendering the diff, the NoOp changes should come from the LCS
sequence, rather than the new sequence. The two indexes will not align
in many cases, adding the wrong new object or indexing out of bounds.
2019-04-27 11:28:02 -04:00
Martin Atkins 88e76fa9ef configs/configschema: Introduce the NestingGroup mode for blocks
In study of existing providers we've found a pattern we werent previously
accounting for of using a nested block type to represent a group of
arguments that relate to a particular feature that is always enabled but
where it improves configuration readability to group all of its settings
together in a nested block.

The existing NestingSingle was not a good fit for this because it is
designed under the assumption that the presence or absence of the block
has some significance in enabling or disabling the relevant feature, and
so for these always-active cases we'd generate a misleading plan where
the settings for the feature appear totally absent, rather than showing
the default values that will be selected.

NestingGroup is, therefore, a slight variation of NestingSingle where
presence vs. absence of the block is not distinguishable (it's never null)
and instead its contents are treated as unset when the block is absent.
This then in turn causes any default values associated with the nested
arguments to be honored and displayed in the plan whenever the block is
not explicitly configured.

The current SDK cannot activate this mode, but that's okay because its
"legacy type system" opt-out flag allows it to force a block to be
processed in this way anyway. We're adding this now so that we can
introduce the feature in a future SDK without causing a breaking change
to the protocol, since the set of possible block nesting modes is not
extensible.
2019-04-10 14:53:52 -07:00
Frederic 0f1f504c22 command/format: indicate in diff when adding an attribute forces replacement 2019-03-29 14:52:13 -07:00
Martin Atkins e918fa83ec command/format: Don't panic when item removed from list of objects
Due to these tests happening in the wrong order, removing an object from
the end of a sequence of objects would previously cause a bounds-check
panic.

Rather than a more severe rework of the logic here, for now we'll just
introduce an extra precondition to prevent the panic. The code that
follows already handles the case where there _is_ no new object (i.e. the
"old" object is being deleted) as long as we're able to pass through this
type-checking logic.

The new "JSON list of objects - removing item" test covers this problem
by rendering a diff for an object being removed from the end of a list
of objects within a JSON value.
2019-03-19 15:46:40 -07:00
James Bardin 0569e39788 don't try to treat "null" as json in diff output
Trying to decode and write "null" as json will panic, since it decodes
to nil.
2019-03-14 17:20:42 -04:00
Martin Atkins 2d41c1009b command/format: Diffs for NestingMap block types
Our initial prototype of new-style diff rendering excluded this because
the old SDK has no support for this construct. However, we want to be able
to introduce this construct in the new SDK without breaking compatibility
with existing versions of Terraform Core, so we need to implement it now
so it's ready to be used once the SDK implements it.

The key associated with each block allows us to properly correlate the
items to recognize the difference between an in-place update of an
existing block and the addition/deletion of a block.
2019-03-11 08:18:26 -07:00
Martin Atkins dd1fa322a7 command/format: Support list/map blocks with dynamic-typed attrs
Our null-to-empty normalization was previously assuming these would always
be collection types, but that isn't true when a block contains something
dynamic since we must then use tuple or object types instead to properly
represent all of the individual element types.
2019-03-11 08:18:26 -07:00
James Bardin b8e53255b4 Revert "remove NormalizeObjectFromLegacySDK from diff"
This reverts commit 5c40d6610c.
2019-03-08 17:32:41 -05:00
James Bardin a95d97f066
Merge pull request #20595 from hashicorp/jbardin/normalize-objects
normalize all objects read from the provider
2019-03-06 17:13:23 -05:00
James Bardin 5c40d6610c remove NormalizeObjectFromLegacySDK from diff
The NormalizeObjectFromLegacySDK calls have been moved into the
provider shims, so all objects generated by the provider should conform
now.
2019-03-06 16:21:32 -05:00
Kristin Laemmert b9d8e96e0c
command/plan: plan output should indicate if a resource is being (#20580)
replaced because the instance was tainted.
2019-03-05 16:18:55 -08:00
Martin Atkins f193b11073 command/format: Normalize before/after values before rendering
We are now allowing the legacy SDK to opt out of the safety checks we try
to do after plan and apply, and so in such cases the before/after values
in planned changes may be inconsistent with our usual rules.

To avoid adding lots of extra complexity to the diff renderer to deal with
these situations, instead we'll normalize the handling of nested blocks
prior to using these values.

In the long run it'd be better to do this normalization at the source,
immediately after we receive an object from a provider using the opt-out,
but we're doing this at the outermost layer for now to avoid risking
unintended impacts on other Terraform Core components when we're just
about to enter the beta phase of the v0.12.0 release cycle.
2019-02-27 16:53:29 -08:00
Radek Simko 953eae7e4b
command/format: Fix rendering of different types 2019-01-23 13:13:48 +00:00
Radek Simko f3d1565d6f
command/format: Fix tuple diff formatting 2019-01-23 11:17:55 +00:00
Radek Simko 3d0a25c65d
command/format: Fix nested (JSON) object formatting 2019-01-22 16:26:28 +00:00
Radek Simko c5ba7469be
command/format: Fix rendering of unknown elements in set/map/list 2019-01-21 15:05:59 +00:00
Radek Simko 13896d72c5
command/format: Render empty object as {} 2019-01-15 14:34:49 +00:00
Radek Simko 5f14b7a7f2
command/format: Render empty primitive list/set as [] 2019-01-15 14:34:49 +00:00
Radek Simko 09d19ca9d9
command/format: Render empty JSON object as {} 2019-01-15 14:34:48 +00:00
Radek Simko 0dd2d56f18
command/format: Render empty maps as {} 2019-01-15 14:34:46 +00:00
Radek Simko d96f4fa77b
command/format: Ignore removal of empty strings 2019-01-13 22:56:04 +00:00
Radek Simko bc4b7cad68
command/format: Render null in dark gray (#19616) 2019-01-11 19:27:09 +00:00
Radek Simko 0b981fa641
command/format: Fix rendering of force-new updates 2018-12-12 14:19:17 +00:00
Radek Simko fe117e9f02
command/format: Fix rendering of nested blocks during update 2018-12-11 18:07:21 +00:00
James Bardin e93d69f18b more nil/known checks before val.LengthInt 2018-10-19 16:51:15 -04:00
James Bardin e08a388d3c check IsKnown on values that may panic 2018-10-18 19:21:32 -04:00
Martin Atkins 3b8d46418f command/format: print correctly nested blocks with labels 2018-10-16 19:14:11 -07:00
Martin Atkins a43b7df282 core: Handle forced-create_before_destroy during the plan walk
Previously we used a single plan action "Replace" to represent both the
destroy-before-create and the create-before-destroy variants of replacing.
However, this forces the apply graph builder to jump through a lot of
hoops to figure out which nodes need it forced on and rebuild parts of
the graph to represent that.

If we instead decide between these two cases at plan time, the actual
determination of it is more straightforward because each resource is
represented by only one node in the plan graph, and then we can ensure
we put the right nodes in the graph during DiffTransformer and thus avoid
the logic for dealing with deposed instances being spread across various
different transformers and node types.

As a nice side-effect, this also allows us to show the difference between
destroy-then-create and create-then-destroy in the rendered diff in the
CLI, although this change doesn't fully implement that yet.
2018-10-16 19:14:11 -07:00
Martin Atkins a37d5268eb command/format: Render unified diff for list and tuple value changes 2018-10-16 19:14:11 -07:00
Martin Atkins 30a46ed8ed command/format: Further refinement on the heuristic JSON-based diff
We'll now show an "update" symbol prior to the argument to this synthetic
jsonencode(...) call, for consistency with how we show nested values in
other cases and to attach a verb to any "# forces replacement".

We'll also show a special form in the case where the value seems to differ
only in whitespace, so users can understand what's going on in that
hopefully-rare situation, particularly if those whitespace-only changes
end up forcing us to replace a remote object.
2018-10-16 19:14:11 -07:00
Martin Atkins 783205948c command/format: JSON value formatting heuristic not for primitive values
Since our own syntax for primitive values is similar to that of JSON, and
since we permit automatic conversions from number and bool to string, we
must do this special JSON value diff formatting only if the value is a
JSON array or object to avoid confusing results.
2018-10-16 19:14:11 -07:00
Martin Atkins 4406b06ef8 command/format: Don't include commas after elements in map diffs
Although commas are allowed in this context, it's not idiomatic to use
them.
2018-10-16 19:14:11 -07:00
Martin Atkins ce157c7f23 command/format: show deep diffs for old/new JSON strings
Because so far we've not supported dynamically-typed complex data
structures, several providers have used strings containing JSON to stand
in for these.

In order to get a readable diff in those cases, we'll recognize situations
where old and new are both JSON and present a diff of the effective value
of the JSON, using a faux call to the jsonencode(...) function to indicate
when we've done so.

This is a bit of a "cute" heuristic, but is important at least for now
until we can migrate away from that practice of passing large JSON strings
to providers and use dynamically-typed attributes instead.
2018-10-16 19:14:11 -07:00
Martin Atkins 9706a00b3a command/format: correct alignment for multi-line values in map diff 2018-10-16 19:14:11 -07:00
Martin Atkins 04d8c17be8 command/format: Nice rendering for changes to map values 2018-10-16 19:14:11 -07:00
Martin Atkins a2f4b8b058 command/format: Show resource diff with header "comment"
This extra comment line gives us a place to show the full resource address
(since the block header line only includes type and name) and also allows
us to explain in long form the meaning of the change icon on the following
line.
2018-10-16 19:14:11 -07:00
Martin Atkins 239a54ad6f command: initial structural diff rendering
This is a light adaptation of our earlier prototype of structural diff
rendering, as a starting point for what we'll actually ship. This is not
consistent with the latest mocks, so will need some additional work before
it is ready, but integrating this allows us to at least see the plan
contents while fixing up remaining issues elsewhere.
2018-10-16 19:14:11 -07:00