* providers.Interface: huge renamification
This commit renames a handful of functions in the providers.Interface to
match changes made in protocol v6. The following commit implements this
change across the rest of the codebase; I put this in a separate commit
for ease of reviewing and will squash these together when merging.
One noteworthy detail: protocol v6 removes the config from the
ValidateProviderConfigResponse, since it's never been used. I chose to
leave that in place in the interface until we deprecate support for
protocol v5 entirely.
Note that none of these changes impact current providers using protocol
v5; the protocol is unchanged. Only the translation layer between the
proto and terraform have changed.
* 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.
Commit e865faf adds visual indentation for diagnostic messages using various
vertical line characters. The present commit disables this behaviour when
running with colourised output disabled.
While the contents of stderr are not intended to be part of the Terraform API,
this is currently how the hashicorp/terraform-exec library detects certain
error types in order to present them as well-known Go errors to the user. Such
detection is complicated when vertical lines are added to the CLI output at
unpredictable points.
I expect this change will also be helpful for screen reader users.
I frequently see people attempting to ask questions about Terraform's
error and warning messages but either only copying part of the message or
accidentally copying a surrounding paragraph that isn't part of the
message.
While I'm sure some of these are just "careless" mistakes, I've also
noticed that this has sometimes overlapped with someone asking a question
whose answer is written directly in the part of the message they didn't
include when copying, and so I have a theory that our current output
doesn't create a good enough visual hierarchy for sighted users to
understand where the diagnostic messages start and end when we show them
in close proximity to other content, or to other diagnostic messages.
As a result, some folks fail to notice the relevant message that might've
answered their question.
I tried a few different experiments for different approaches here, such
as adding more horizontal rules to the output and coloring the detail
text differently, but the approach that felt like the nicest compromise
to me was what's implemented here, which is to add a vertical line
along the left edge of each diagnostic message, colored to match with the
typical color we use for each diagnostic severity. This means that the
diagnostics end up slightly indented from what's around them, and the
vertical line seems to help subtly signal how we intended the content
to be grouped together.
In some terminal emulators, writing a character into the last column on a
row causes the terminal to immediately wrap to the beginning of the next
line, even if the very next character in the stream is a hard newline.
That can then lead to errant blank lines in the final output which make
it harder to navigate the visual hierarchy.
As a compromise to avoid this, we'll format our horizontal rules and
paragraphs to one column less than the terminal width. That does mean that
our horizontal rules won't _quite_ cover the whole terminal width, but
it seems like a good compromise in order to get consistent behavior across
a wider variety of terminal implementations.
We were previously using some ASCII art to create some visual divisions
between parts of the diagnostic output. Now that we are requiring a UTF-8
terminal we can print out box drawing characters instead.
We now require the output to accept UTF-8 and we can determine how wide
the terminal (if any) is, so here we begin to make use of that for the
"terraform plan" command.
The horizontal rule is now made of box drawing characters instead of
hyphens and fills the whole terminal width.
The paragraphs of text in the output are now also wrapped to fill the
terminal width, instead of the hard-wrapping we did before.
This is just a start down the road of making better use of the terminal
capabilities. Lots of other commands could benefit from updates like these
too.
* 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
Previously when printing the relevant variables involved in a failed
expression evaluation we would just skip over unknown values entirely.
There are some errors, though, which are _caused by_ a value being
unknown, in which case it's helpful to show which of the inputs to that
expression were known vs. unknown so that the user can limit their further
investigation only to the unknown ones.
While here I also added a special case for sensitive values that overrides
all other display, because we don't know what about a value is sensitive
and so better to give nothing away at the expense of a slightly less
helpful error message.
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.
* Add creation test and simplify in-place test
* Add deletion test
* Start adding marking from state
Start storing paths that should be marked
when pulled out of state. Implements deep
copy for attr paths. This commit also includes some
comment noise from investigations, and fixing the diff test
* Fix apply stripping marks
* Expand diff tests
* Basic apply test
* Update comments on equality checks to clarify current understanding
* Add JSON serialization for sensitive paths
We need to serialize a slice of cty.Path values to be used to re-mark
the sensitive values of a resource instance when loading the state file.
Paths consist of a list of steps, each of which may be either getting an
attribute value by name, or indexing into a collection by string or
number.
To serialize these without building a complex parser for a compact
string form, we render a nested array of small objects, like so:
[
[
{ type: "get_attr", value: "foo" },
{ type: "index", value: { "type": "number", "value": 2 } }
]
]
The above example is equivalent to a path `foo[2]`.
* Format diffs with map types
Comparisons need unmarked values to operate on,
so create unmarked values for those operations. Additionally,
change diff to cover map types
* Remove debugging printing
* Fix bug with marking non-sensitive values
When pulling a sensitive value from state,
we were previously using those marks to remark
the planned new value, but that new value
might *not* be sensitive, so let's not do that
* Fix apply test
Apply was not passing the second state
through to the third pass at apply
* Consistency in checking for length of paths vs inspecting into value
* In apply, don't mark with before paths
* AttrPaths test coverage for DeepCopy
* Revert format changes
Reverts format changes in format/diff for this
branch so those changes can be discussed on a separate PR
* Refactor name of AttrPaths to AttrSensitivePaths
* Rename AttributePaths/attributePaths for naming consistency
Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>
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.
Most of the functionality for rendering output changes is covered by the
tests for ResourceChanges, as they both share the same diff renderer.
This commit adds a few tests to cover some of the output specific code.
Diagnostic detail lines sometimes contain lines which include commands
suggested for the user to execute. By convention, these start with
leading whitespace to indicate that they are not prose.
This commit changes the diagnostic formatter to wrap each line of the
detail separately, and skips word wrapping for lines prefixed with
space. This prevents ugly and confusing wrapping of long command lines.
James Bardin