62 Commits (bf396b5f1b89536725a3be35d7500a17cb1045b3)

Author	SHA1	Message	Date
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.	5 years ago
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.	5 years ago
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.	5 years ago
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.	5 years ago
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.	5 years ago
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	5 years ago
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.	5 years ago
Alisdair McDiarmid	d05e3b40bf	Merge pull request #26492 from hashicorp/alisdair/sensitive-value-force-replacement ... command: Fix missing force new for sensitive vars and blocks	5 years ago
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.	5 years ago
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.	5 years ago
Alisdair McDiarmid	eb873f5021	Merge pull request #25725 from FGtatsuro/diff_heredoc_hyphen ... Use valid heredoc begin symbol in diff output.	5 years ago
Kristin Laemmert	3933cbd491	remove LegacyProvider (#26433)	5 years ago
Pam Selle	111aadd0f0	Extend tests further	5 years ago
Pam Selle	2ec95f1abc	Make an after val unknown to exercise the known check	5 years ago
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.	5 years ago
Pam Selle	6617c2729c	Test additions for nested blocks ... Add some coverage for in-place on nested block Add nested block to deletion test	5 years ago
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	5 years ago
Pam Selle	1780351636	If the whole map is marked, have the same sensitivity behavior as a single value	5 years ago
Pam Selle	0520f143a2	Add diff coverage for maps ... Considers wholly marked maps as well as map keys	5 years ago
Pam Selle	3dde9efc75	Support list diffs with sensitivity ... Adds support for specialized diffs with lists	5 years ago
Pam Selle	531728f6e9	Sensitive diffs for primitive types ... When showing primitive type diffs, hide possibly sensitive values	5 years ago
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)	5 years ago
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>	5 years ago
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	6 years ago
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.	6 years ago
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.	6 years ago
FGtatsuro	b6cf1c3673	Use valid heredoc begin symbol in diff output.	6 years ago
James Bardin	e13eecbc5b	finish provider ModuleInstance replacement	6 years ago
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.	6 years ago
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>	6 years ago
Kristin Laemmert	6541775ce4	addrs: roll back change to Type field in ProviderConfig (#23937)	6 years ago
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	6 years ago
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	6 years ago
Thayne McCombs	a895a42f85	command/format: fix missing elements at the end of lists in diffs	6 years ago
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.	6 years ago
Martin Atkins	6adcc7ab73	vendor: go get github.com/zclconf/go-cty@master ... cty now guarantees that sets of primitive values will iterate in a reasonable order. Previously it was the caller's responsibility to deal with that, but we invariably neglected to do so, causing inconsistent ordering. Since cty prioritizes consistent behavior over performance, it now imposes its own sort on set elements as part of iterating over them so that calling applications don't have to worry so much about it. This change also causes cty to consistently push unknown and null values in sets to the end of iteration, where before that was undefined. This means that our diff output will now consistently list additions before removals when showing sets, rather than the ordering being undefined as before. The ordering of known, non-null, non-primitive values is still not contractually fixed but remains consistent for a particular version of cty.	7 years ago
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.	7 years ago
Frederic	0f1f504c22	command/format: indicate in diff when adding an attribute forces replacement	7 years ago
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.	7 years ago
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.	7 years ago
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.	7 years ago
Martin Atkins	69772b11b1	command/format: test for diff rendering with dynamic-typed subattrs ... We use cty a little differently when a nested list block contains a dynamically-typed attribute: it appears as a tuple value instead of a list value so that we can retain the individual types of each element. Here we introduce a test for that case, but doing so required also making the runTestCases function handle types in a stricter way so that it will produce planned values that match how Terraform Core would do it, including the necessary late-bound type information for the dynamically-typed attribute.	7 years ago
Kristin Laemmert	b9d8e96e0c	command/plan: plan output should indicate if a resource is being (#20580) ... replaced because the instance was tainted.	7 years ago
Radek Simko	b492c3662c	Merge pull request #20089 from hashicorp/t-cmd-fmt-sensitive-update ... command/format: Add test to cover update of sensitive field	7 years ago
Radek Simko	f04d0b48bc	command/format: Add test to cover update of sensitive field	7 years ago
Radek Simko	953eae7e4b	command/format: Fix rendering of different types	7 years ago
Radek Simko	0dff8fe5e0	Add failing test case for tuple	7 years ago
Radek Simko	3d0a25c65d	command/format: Fix nested (JSON) object formatting	7 years ago
Radek Simko	98cc99e632	command/format: Add crashing test with unknown element in map	7 years ago
Radek Simko	73225c7aeb	command/format: Add crashing test with unknown element in list	7 years ago