Commit Graph

136 Commits

Author SHA1 Message Date
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
Pam Selle 5b549224ae Refactor to call ContainsMarked less and use len() instead 2020-09-24 16:42:03 -04:00
Pam Selle 3c9fad0b0e Move plan action check into the sensitivity warning method 2020-09-24 13:49:34 -04:00
Pam Selle 531728f6e9 Sensitive diffs for primitive types
When showing primitive type diffs, hide possibly
sensitive values
2020-09-24 13:27:15 -04:00
Pam Selle 20921dbfb8 Add warning about sensitivity change
This commit adds a warning before displaying
a sensitive diff, and always obfuscates the old value (even
if it was not previously marked as sensitive)
2020-09-24 12:57:40 -04:00
Pam Selle 0a02e7040f
Store sensitive attribute paths in state (#26338)
* 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>
2020-09-24 12:40:17 -04:00
Pam Selle 8d8389da74 Add diff test with a sensitive change
Adds a diff test for a changed value,
and modifies the diff file to cover variable
diffs on sensitive values
2020-09-10 16:45:31 -04:00
Pam Selle e4e16ccbd3 Rebase fix 2020-09-10 11:06:40 -04:00
Pam Selle 61c78fd3b9 Add case to compactValueStr not to expose sensitive vals in diagnostics 2020-09-10 11:04:17 -04:00
Pam Selle bc55b6a28b Use UnmarkDeepWithPaths and MarkWithPaths
Updates existing code to use the new Value
methods for unmarking/marking and removes
panics/workarounds in cty marshall methods
2020-09-10 11:04:17 -04:00
Pam Selle 896d277a69 If the path is empty, we should not be marking the path 2020-09-10 11:04:17 -04:00
Pam Selle 84d118e18f Track sensitivity through evaluation
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
2020-09-10 11:04:17 -04:00
Alisdair McDiarmid 09d8355f43 command: Add experimental concise diff renderer
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.
2020-09-10 10:35:55 -04:00
Alisdair McDiarmid ba9baac36d command: Add tests for format.OutputChanges
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.
2020-09-04 16:22:23 -04:00
Alisdair McDiarmid 3547f9e368 format: Don't wrap space-prefixed diag details
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.
2020-09-01 14:02:19 -04:00
FGtatsuro b6cf1c3673 Use valid heredoc begin symbol in diff output. 2020-08-03 00:33:35 +09:00
Alisdair McDiarmid c87c0a9d2a command/format: Fix empty overlap diagnostics
Diagnostics where the highlight range has an empty overlap with a line
would skip lines of the output. This is because if two ranges abut each
other, they can be considered to overlap, but that overlap is empty.
This results in an edge case in the diagnostic printer which causes the
line not to be printed.
2020-06-05 16:17:27 -04:00
Martin Atkins 31a4b44d2e backend/local: treat output changes as side-effects to be applied
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.
2020-05-29 07:36:40 -07:00
Kristin Laemmert 27a794062e Mildwonkey/command tests (#24535)
* command: refactor testBackendState to write states.State

testBackendState was using the older terraform.State format, which is no
longer sufficient for most tests since the state upgrader does not
encode provider FQNs automatically. Users will run `terraform
0.13upgrade` to update their state to include provider FQNs in
resources, but tests need to use the modern state format instead of
relying on the automatic upgrade.

* plan tests passing
* graph tests passing
* json packages test update
* command test updates
* update show test fixtures
* state show tests passing
2020-04-06 09:24:23 -07:00
Alisdair McDiarmid a23c3e3c81 command/format: Fix multi-line diagnostic output
Previously, if a diagnostic context spanned multiple lines, any lines
which did not overlap with the highlight range would be displayed as
blank. This commit fixes the bug.

The problem was caused by the unconditional use of `PartitionAround` to
split the line into before/highlighted/after ranges. When two ranges
don't overlap, this method returns empty ranges, which results in a
blank line. Instead, we first check if the ranges do overlap, and if not
we print the entire line from the context.
2020-03-26 15:53:52 -04:00
James Bardin a8b9547e0d fixup states.Resource change throughout packages 2020-03-16 16:50:48 -04:00
James Bardin e13eecbc5b finish provider ModuleInstance replacement 2020-03-11 14:19:52 -04:00
Kristin Laemmert 47a16b0937
addrs: embed Provider in AbsProviderConfig instead of Type
a large refactor to addrs.AbsProviderConfig, embedding the addrs.Provider instead of a Type string. I've added and updated tests, added some Legacy functions to support older state formats and shims, and added a normalization step when reading v4 (current) state files (not the added tests under states/statefile/roundtrip which work with both current and legacy-style AbsProviderConfig strings).

The remaining 'fixme' and 'todo' comments are mostly going to be addressed in a subsequent PR and involve looking up a given local provider config's FQN. This is fine for now as we are only working with default assumption.
2020-02-13 15:32:58 -05:00
Kristin Laemmert 927999a820
command/state show: use configured provider (#24027)
The `state show`  command was not checking if a given resource had a
configured provider, and instead was only using the default provider
config. This PR checks for a configured provider, using the default
provider if one is not set.

Fixes #22010
2020-02-04 12:07:59 -05:00
Kristin Laemmert 80ab551867
terraform: use addrs.Provider as map keys for provider schemas (#24002)
This is a stepping-stone PR for the provider source project. In this PR
"legcay-stype" FQNs are created from the provider name string. Future
work involves encoding the FQN directly in the AbsProviderConfig and
removing the calls to addrs.NewLegacyProvider().
2020-02-03 08:18:04 -05:00
Martin Atkins 8b511524d6
Initial steps towards AbsProviderConfig/LocalProviderConfig separation (#23978)
* Introduce "Local" terminology for non-absolute provider config addresses

In a future change AbsProviderConfig and LocalProviderConfig are going to
become two entirely distinct types, rather than Abs embedding Local as
written here. This naming change is in preparation for that subsequent
work, which will also include introducing a new "ProviderConfig" type
that is an interface that AbsProviderConfig and LocalProviderConfig both
implement.

This is intended to be largely just a naming change to get started, so
we can deal with all of the messy renaming. However, this did also require
a slight change in modeling where the Resource.DefaultProviderConfig
method has become Resource.DefaultProvider returning a Provider address
directly, because this method doesn't have enough information to construct
a true and accurate LocalProviderConfig -- it would need to refer to the
configuration to know what this module is calling the provider it has
selected.

In order to leave a trail to follow for subsequent work, all of the
changes here are intended to ensure that remaining work will become
obvious via compile-time errors when all of the following changes happen:
- The concept of "legacy" provider addresses is removed from the addrs
  package, including removing addrs.NewLegacyProvider and
  addrs.Provider.LegacyString.
- addrs.AbsProviderConfig stops having addrs.LocalProviderConfig embedded
  in it and has an addrs.Provider and a string alias directly instead.
- The provider-schema-handling parts of Terraform core are updated to
  work with addrs.Provider to identify providers, rather than legacy
  strings.

In particular, there are still several codepaths here making legacy
provider address assumptions (in order to limit the scope of this change)
but I've made sure each one is doing something that relies on at least
one of the above changes not having been made yet.

* addrs: ProviderConfig interface

In a (very) few special situations in the main "terraform" package we need
to make runtime decisions about whether a provider config is absolute
or local.

We currently do that by exploiting the fact that AbsProviderConfig has
LocalProviderConfig nested inside of it and so in the local case we can
just ignore the wrapping AbsProviderConfig and use the embedded value.

In a future change we'll be moving away from that embedding and making
these two types distinct in order to represent that mapping between them
requires consulting a lookup table in the configuration, and so here we
introduce a new interface type ProviderConfig that can represent either
AbsProviderConfig or LocalProviderConfig decided dynamically at runtime.

This also includes the Config.ResolveAbsProviderAddr method that will
eventually be responsible for that local-to-absolute translation, so
that callers with access to the configuration can normalize to an
addrs.AbsProviderConfig given a non-nil addrs.ProviderConfig. That's
currently unused because existing callers are still relying on the
simplistic structural transform, but we'll switch them over in a later
commit.

* rename LocalType to LocalName

Co-authored-by: Kristin Laemmert <mildwonkey@users.noreply.github.com>
2020-01-31 08:23:07 -05:00
Kristin Laemmert 6541775ce4
addrs: roll back change to Type field in ProviderConfig (#23937) 2020-01-28 08:13:30 -05:00
Kristin Laemmert 92f427779d
command/show: fix issue with show and aliased provider (#23848)
The formatter in `command/format/state.go`, when formatting a resource
with an aliased provider, was looking for a schema with the alias (ie,
test.foo), but the schemas are only listed by provider type (test).
Update the state formatter to lookup schemas by provider type only.

Some of the show tests (and a couple others) were not properly cleaning
up the created tmpdirs, so I fixed those. Also, the show tests are using
a statefile named `state.tfstate`, but were not passing that path to the
show command, so we were getting some false positives (a `show` command
that returns `no state` exits 0).

Fixes #21462
2020-01-13 15:10:00 -05:00
Martin Atkins c06675c616 command: New -compact-warnings option
When warnings appear in isolation (not accompanied by an error) it's
reasonable to want to defer resolving them for a while because they are
not actually blocking immediate work.

However, our warning messages tend to be long by default in order to
include all of the necessary context to understand the implications of
the warning, and that can make them overwhelming when combined with other
output.

As a compromise, this adds a new CLI option -compact-warnings which is
supported for all the main operation commands and which uses a more
compact format to print out warnings as long as they aren't also
accompanied by errors.

The default remains unchanged except that the threshold for consolidating
warning messages is reduced to one so that we'll now only show one of
each distinct warning summary.

Full warning messages are always shown if there's at least one error
included in the diagnostic set too, because in that case the warning
message could contain additional context to help understand the error.
2019-12-10 11:53:14 -08:00
Pam Selle d8c31a1efa
Merge pull request #23581 from hashicorp/pselle/show-panic-23377
Fix panic on show plan
2019-12-06 12:08:16 -05:00
Kristin Laemmert e3416124cc
addrs: replace "Type string" with "Type Provider" in ProviderConfig
* huge change to weave new addrs.Provider into addrs.ProviderConfig
* terraform: do not include an empty string in the returned Providers /
Provisioners
- Fixed a minor bug where results included an extra empty string
2019-12-06 08:00:18 -05:00
Pam Selle 2b8e876bdb Don't inspect an empty set, return false 2019-12-05 16:00:19 -05:00
tmshn fcc1a76d5e make plan-diff format a bit more dry 2019-12-03 19:02:59 +09:00