Commit Graph

168 Commits

Author SHA1 Message Date
James Bardin ef88c54604
Merge pull request #28687 from hashicorp/jbardin/sensitive-changes
Decode change values with marks
2021-05-13 12:44:03 -04:00
Martin Atkins 917309fb5a command/diff: Small additional context about deposed objects
In the very unusual situation where we end up planning to destroy a
deposed object alone, it's likely that we're exposing users to this idea
of "deposed" for the very first time.

This additional sentence will hopefully give some extra context for what
that means. We don't really have room here for a lengthy explanation about
how deposed objects come to exist but this will hopefully be enough of
a hook to help users connect this with an error they saw on a previous
run, or at least to have some additional keywords to search for if they
want to research further.
2021-05-13 09:05:06 -07:00
Martin Atkins 3c8a4e6e05 command+backend/local: -refresh-only and drift detection
This is a light revamp of our plan output to make use of Terraform core's
new ability to report both the previous run state and the refreshed state,
allowing us to explicitly report changes made outside of Terraform.

Because whether a plan has "changes" or not is no longer such a
straightforward matter, this now merges views.Operation.Plan with
views.Operation.PlanNoChanges to produce a single function that knows how
to report all of the various permutations. This was also an opportunity
to fill some holes in our previous logic which caused it to produce some
confusing messages, including a new tailored message for when
"terraform destroy" detects that nothing needs to be destroyed.

This also allows users to request the refresh-only planning mode using a
new -refresh-only command line option. In that case, Terraform _only_
performs drift detection, and so applying a refresh-only plan only
involves writing a new state snapshot, without changing any real
infrastructure objects.
2021-05-13 09:05:06 -07:00
Martin Atkins ce69c3903f command/views: Show refresh-detected changes as part of a plan
We've always had a mechanism to synchronize the Terraform state with
remote objects before creating a plan, but we previously kept the result
of that to ourselves, and so it would sometimes lead to Terraform
generating a planned action to undo some upstream drift, but Terraform
would give no context as to why that action was planned even though the
relevant part of the configuration hadn't changed.

Now we'll detect any differences between the previous run state and the
refreshed state and, if any managed resources now look different, show
an additional note about it as extra context for the planned changes that
follow.

This appears as an optional extra block of information before the normal
plan output. It'll appear the same way in all of the contexts where we
render plans, including "terraform show" for saved plans.
2021-05-13 09:05:06 -07:00
James Bardin adf6ff7068 changes are now decoded with marks 2021-05-12 09:30:12 -04:00
Alisdair McDiarmid 53fab10b26
Merge pull request #28598 from hashicorp/alisdair/fix-diagnostic-snippet-crash
cli: Fix diagnostic snippet crash
2021-05-04 13:01:58 -04:00
Kristin Laemmert 3679de0630
command/format: fix repetitive "unchanged attribute hidden" message (#28589)
writeNestedAttrDiff and writeAttrDiff were both printing the "unchanged attribute" message.  This removes one of the redundant prints.

Fixing this led me (in a very roundabout way) to realize that NestedType attributes were printing a sum total of unchanged attributes, including those in entirely unchanged elements, while *not* printing the total of unchanged elements. I added the necessary logic to count and print the number of unchanged elements for maps and lists.
2021-05-04 10:23:50 -04:00
Alisdair McDiarmid 25f99857cf cli: Fix crash with invalid JSON diagnostics
If a JSON diagnostic value has a highlight end offset which is before
the highlight start offset, this would previously panic. This commit
adds a normalization step to prevent the crash.
2021-05-04 08:35:42 -04:00
Martin Atkins 1d3e34e35e command: New -replace=... planning option
This allows a similar effect to pre-tainting an object but does the action
within the context of a normal plan and apply, avoiding the need for an
intermediate state where the old object still exists but is marked as
tainted.

The core functionality for this was already present, so this commit is
just the UI-level changes to make that option available for use and to
explain how it contributed to the resulting plan in Terraform's output.
2021-05-03 15:43:23 -07:00
Alisdair McDiarmid c95e9ada6b cli: Show forces replacement for sensitive attrs
When rendering a plan diff, sensitive resource attributes would
previously omit the "forces replacement" comment, which can lead to
confusion when the only reason for a resource being replaced is a
sensitive attribute.
2021-05-03 06:48:10 -04:00
Martin Atkins b802237e03 plans: Track an optional extra "reason" for some planned actions
Previously we were repeating some logic in the UI layer in order to
recover relevant additional context about a change to report to a user.
In order to help keep things consistent, and to have a clearer path for
adding more such things in the future, here we capture this user-facing
idea of an "action reason" within the plan model, and then use that
directly in order to decide how to describe the change to the user.

For the moment the "tainted" situation is the only one that gets a special
message, matching what we had before, but we can expand on this in future
in order to give better feedback about the other replace situations too.

This also preemptively includes the "replacing by request" reason, which
is currently not reachable but will be used in the near future as part of
implementing the -replace=... plan command line option to allow forcing
a particular object to be replaced.

So far we don't have any special reasons for anything other than replacing,
which makes sense because replacing is the only one that is in a sense
a special case of another action (Update), but this could expand to
other kinds of reasons in the future, such as explaining which of the
few different reasons a data source read might be deferred until the
apply step.
2021-04-29 17:50:46 -07:00
James Bardin d04999863c "with" formatting 2021-04-06 15:50:30 -04:00
James Bardin 406ac97965 add the address field to the view diagnostics 2021-04-06 15:15:52 -04:00
Kristin Laemmert fac60ab124
Add formatting for attributes with nested types (#28069)
* format/diff: extract attributes-writing logic to a function

This is a stepping-stone commit (for easier reviewability, and to prove that tests did not change) as part of writing a NestedType-specific diff printer.

* command/format: add support for formatting attributes with NestedTypes

This commit adds custom formatting for NestedType attributes. THe logic was mostly copied from the block diff printer, with minor tweaks here and there. I used the (excellent) existing test coverage and added a NestedType attribute to every test.

Since the (nested-block specific) test schemas were nearly identical, I added a function that returns the schema with the requested NestingMode.
2021-03-15 09:31:23 -04:00
Alisdair McDiarmid d9d88b9243 cli: Add comprehensive JSON diagnostic structure
This commit adds a comprehensive JSON format for diagnostics, which
ensures that all current diagnostic output can be semantically
represented in a machine-readable format. The diagnostic formatter
interface remains unchanged, but it first transforms its input via the
JSON format to ensure that there is only one code path for creating the
diagnostic data.

The JSON diagnostic renderer extracts the non-presentational logic from
the format package, and returns a structure which can either be
marshaled into JSON or rendered as text. The resulting text diagnostic
output is unchanged for all cases covered by unit tests and my own
manual testing.

Included in this commit are a number of golden reference files for the
marshaled JSON output of a diagnostic. This format should change rarely
if at all, and these are in place to ensure that any changes to the
format are intentional and considered.
2021-03-12 13:12:37 -05:00
James Bardin 3da5d2bdf6
Merge pull request #27692 from alexwlchan/abolish-the-heirarchy
Correct the spelling of heirarchy/hierarchy throughout
2021-02-18 16:34:28 -05:00
Kristin Laemmert f6505870cc
Mildwonkey/providers interface renaming (#27805)
* 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.
2021-02-18 10:13:43 -05:00
Kristin Laemmert 2b4e389839
Mildwonkey/nested type format (#27793)
* 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.
2021-02-18 08:48:52 -05:00
Alisdair McDiarmid 943df48491 cli: Optimize for large multi-line string outputs 2021-02-11 10:42:06 -05:00
Alex Chan 4ec80fa18d Correct the spelling of heirarchy/hierarchy throughout 2021-02-05 15:07:04 +00:00
Katy Moe 51c687c2db command: no visual warning hierarchy in -no-color
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.
2021-01-27 09:26:53 -08:00
Katy Moe bc51932e08 refactor: extract appendSourceSnippets 2021-01-27 09:26:53 -08:00
Martin Atkins e865faf318 command: Better visual hierarchy for diagnostics
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.
2021-01-14 09:50:22 -08:00
Martin Atkins 1f1af87dea command/format: Don't try to fill the last terminal column
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.
2021-01-13 15:37:04 -08:00
Martin Atkins 0c84a56700 command/format: Use box drawing chars for rules in diagnostics
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.
2021-01-13 15:37:04 -08:00
Martin Atkins e6a516d87e backend/local: Use terminal properties to tweak the plan output
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.
2021-01-13 15:37:04 -08:00
James Bardin 3bc7d77230 update MockProvider usage 2021-01-12 17:47:55 -05:00
James Bardin dcf0dba6f4
Merge pull request #27081 from hashicorp/jbardin/staticcheck
Fixes to pass static analysis
2020-12-02 15:43:10 -05:00
Kristin Laemmert 3fa063b8dc
command/format: concise diff is now the default (#27079)
* 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
2020-12-02 15:42:41 -05:00
James Bardin 41d4dd82d6 format staticcheck 2020-12-02 13:59:19 -05:00
Martin Atkins d24fdce5b8 command/format: Include unknown values in diagnostics
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.
2020-10-29 09:07:48 -07:00
Pam Selle f61a342311 Hide sensitive outputs in terraform show 2020-10-28 15:46:09 -04:00
Alisdair McDiarmid c798dc98db command: Show diffs when only sensitivity changes
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.
2020-10-13 13:55:16 -04:00
Alisdair McDiarmid d05e3b40bf
Merge pull request #26492 from hashicorp/alisdair/sensitive-value-force-replacement
command: Fix missing force new for sensitive vars and blocks
2020-10-07 11:10:36 -04:00
Alisdair McDiarmid 62e6f56a50 command: Fix missing force new for sensitive blocks
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.
2020-10-07 10:50:54 -04:00
Alisdair McDiarmid 79a3e33c4d command: Fix missing force new for sensitive vars
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.
2020-10-06 13:05:30 -04:00
Alisdair McDiarmid eb873f5021
Merge pull request #25725 from FGtatsuro/diff_heredoc_hyphen
Use valid heredoc begin symbol in diff output.
2020-10-06 12:52:23 -04:00
Kristin Laemmert 3933cbd491
remove LegacyProvider (#26433) 2020-10-05 08:33:49 -04:00
Pam Selle 111aadd0f0 Extend tests further 2020-10-02 15:01:17 -04:00
Pam Selle 55c96da27e Move nested block printing to own method for readability 2020-10-02 14:56:50 -04:00
Pam Selle 2ec95f1abc Make an after val unknown to exercise the known check 2020-10-02 13:11:55 -04:00
Pam Selle 73b1d8b0d1 Add special diff for nested blocks
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.
2020-10-02 13:11:55 -04:00
Pam Selle 6617c2729c Test additions for nested blocks
Add some coverage for in-place on nested block

Add nested block to deletion test
2020-10-02 13:04:54 -04:00
Pam Selle 3e7be13dff Update ordering for marking/unmarking and asserting plan valid
Update when we unmark objects so we can assert the plan is valid,
and process UnknownAsNull on the unmarked value
2020-10-02 13:03:11 -04:00
Pam Selle 52b6f7f53e Move common IsMarked checks above switch statement 2020-10-01 14:34:44 -04:00
Pam Selle 1780351636 If the whole map is marked, have the same sensitivity behavior as a single value 2020-10-01 14:30:33 -04:00
Pam Selle 0520f143a2 Add diff coverage for maps
Considers wholly marked maps as well
as map keys
2020-10-01 14:18:40 -04:00
Pam Selle 40ea3f4cb8
Merge pull request #26373 from hashicorp/pselle/sensitive-vals-list
Support list diffs with sensitivity
2020-09-25 13:46:37 -04:00
Pam Selle 634e83ab63 Change sensitivity warning to be yellow only on 'Warning' 2020-09-25 10:22:56 -04:00
Pam Selle 3dde9efc75 Support list diffs with sensitivity
Adds support for specialized diffs with lists
2020-09-25 10:18:33 -04:00