* command/format: check for sensitive NestedTypes
Eventually, the diff formatter will need to be updated to properly
handle NestedTypes, but for now we can let the existing function deal
with them as regular cty.Object-type attributes.
To avoid printing sensitive nested attributes, we will treat any
attribute with at least one sensitive nested attribute as an entirely
sensitive attribute.
* bugfix for Object ImpliedType()
ImpliedType() was returning too early when the given object had optional
attributes, therefore skipping the incredibly important step of
accounting for the nesting mode when returning said type.
* command/format: concise diff is no longer an experiment
Since state formatting goes through the "diff" printer, I have
repurposed the concise flag as a verbose flag, used only when printing
state. It's silly but it works!
* remove helper/experiment
With this experiment concluded, we no longer need helper/experiment. The
shadow experiment had not been touched in many years, so I removed all
references, and removed the package entirely. Any new experiments are
expected to be configuration experiments handled by our (other)
experiments package.
* check for the verbose flag consistently, in case we end up using it in plans in the future
When an attribute's sensitivity changes, but its value remains the same,
we consider this an update operation for the plan. This commit updates
the diff renderer to match this, detecting and displaying the change in
sensitivity.
Previously, the renderer would detect no changes to the value of the
attribute, and consider it a no-op action. This resulted in suppression
of the attribute when the plan is in concise mode.
This is achieved with a new helper function, ctyEqualValueAndMarks. We
call this function whenever we want to check that two values are equal
in order to determine whether the action is update or no-op.
If an entire block is marked sensitive (possibly because it is of type
NestedSet) and results in replacement of the resource, we should render
the standard "forces replacement" text after the opening line of the
block.
If a value rendered for the diff is sensitive and results in replacement
of the resource, we should render the standard "forces replacement" text
after the "(sensitive)" value display.
When a value in a nested block is marked as sensitive,
it will result in the behavior of every member of
that block being sensitive. As such, show a
specialized diff that reduces noise of (sensitive)
for many attributes that we won't show. Also update
the warning to differentiate between showing a warning
for an attribute or warning for blocks.
Mark sensitivity on a value. However, when the value is encoded to send to the
provider to produce a changeset we must remove the marks, so unmark the value
and remark it with the saved path afterwards
When rendering a diff between current state and projected state, we only
show resources and outputs which have changes. However, we show a full
structural diff for these values, which includes all attributes and
blocks for a changed resource or output. The result can be a very long
diff, which makes it difficult to verify what the changed fields are.
This commit adds an experimental concise diff renderer, which suppresses
most unchanged fields, only displaying the most relevant changes and
some identifying context. This means:
- Always show all identifying attributes, initially defined as `id`,
`name`, and `tags`, even if unchanged;
- Only show changed, added, or removed primitive values: `string`,
`number`, or `bool`;
- Only show added or removed elements in unordered collections and
structural types: `map`, `set`, and `object`;
- Show added or removed elements with any surrounding unchanged elements
for sequence types: `list` and `tuple`;
- Only show added or removed nested blocks, or blocks with changed
attributes.
If any attributes, collection elements, or blocks are hidden, a count
is kept and displayed at the end of the parent scope. This ensures that
it is clear that the diff is only displaying a subset of the resource.
The experiment is currently enabled by default, but can be disabled by
setting the TF_X_CONCISE_DIFF environment variable to 0.
This is a baby-step towards an intended future where all Terraform actions
which have side-effects in either remote objects or the Terraform state
can go through the plan+apply workflow.
This initial change is focused only on allowing plan+apply for changes to
root module output values, so that these can be written into a new state
snapshot (for consumption by terraform_remote_state elsewhere) without
having to go outside of the primary workflow by running
"terraform refresh".
This is also better than "terraform refresh" because it gives an
opportunity to review the proposed changes before applying them, as we're
accustomed to with resource changes.
The downside here is that Terraform Core was not designed to produce
accurate changesets for root module outputs. Although we added a place for
it in the plan model in Terraform 0.12, Terraform Core currently produces
inaccurate changesets there which don't properly track the prior values.
We're planning to rework Terraform Core's evaluation approach in a
forthcoming release so it would itself be able to distinguish between the
prior state and the planned new state to produce an accurate changeset,
but this commit introduces a temporary stop-gap solution of implementing
the logic up in the local backend code, where we can freeze a snapshot of
the prior state before we take any other actions and then use that to
produce an accurate output changeset to decide whether the plan has
externally-visible side-effects and render any changes to output values.
This temporary approach should be replaced by a more appropriately-placed
solution in Terraform Core in a release, which should then allow further
behaviors in similar vein, such as user-visible drift detection for
resource instances.
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.
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.
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.
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.
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.
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.
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.
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.
Kristin Laemmert