Commit Graph

94 Commits

Author SHA1 Message Date
Pam Selle fcae49611c
Merge pull request #26555 from hashicorp/pselle/sensitive-var-value-compat
Avoid disclosing values in errors on marked vals
2020-10-13 10:51:25 -04:00
James Bardin 5eca0788c6 rely solely on the plan changes for outputs
Now that outputs changes are tracked in full, we can remove the
comparisons with the prior state and use the planned changes directly.
2020-10-12 18:59:14 -04:00
Pam Selle da4ddd0160 Avoid disclosing values in errors on marked vals
AssertObjectCompatible is a special case that will
expose Go string values of values unless otherwise
stopped. This adds that check.
2020-10-12 15:53:34 -04:00
James Bardin c48af3f18b
Merge pull request #26470 from hashicorp/jbardin/inverse-destroy-references
Allow special-case evaluation of instances pending deletion.
2020-10-05 16:20:22 -04:00
James Bardin 0c72c6f144 s/FullDestroy/IsFullDdestroy/ 2020-10-05 10:50:25 -04:00
Kristin Laemmert 3933cbd491
remove LegacyProvider (#26433) 2020-10-05 08:33:49 -04:00
Pam Selle f35b530837 Update compatibility checks for blocks to not use marks
Remove marks for object compatibility tests to allow apply
to continue. Adds a block to the test provider to use
in testing, and extends the sensitivity apply test to include a block
2020-10-02 13:11:55 -04:00
James Bardin fa8f8df7b6 add ChangesSync.FullDestroy
In order to handle various edge cases during a full destroy, add
FullDestroy to the synchronized changes so we can attempt to deduce if
the plan was created from `terraform destroy`.

It's possible that the plan was created by removing all resourced from
the configuration, but in that case the end result is the same. Any of
the edge cases with provider or destroy provisioner configurations would
not apply, since there would not be any configuration references to
resolve.
2020-10-01 17:08:25 -04:00
Pam Selle 0b3c21a3eb Support lists of deeply marked values 2020-09-25 13:33:44 -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
James Bardin 8cef62e455 add state to plans.Plan
Since the refreshed state is now an artifact of the plan process, it
makes sense to add it to the Plan type, rather than adding an additional
return value to the Context.Plan method.
2020-09-17 09:54:59 -04:00
Pam Selle 20ee878d0e Updates and improvements to comments 2020-09-11 11:15:44 -04:00
Pam Selle 5b0b1a13a5 Update object compatible check to unmark
The hack approach appears consistent,
as we can remove marks before calling the
value validation
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 6c129a921b Unmark/remark in apply process to allow apply 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
James Bardin 2b4101fdff Unknown set blocks with dynamic may have 0 elems
The couldHaveUnknownBlockPlaceholder helper was added to detect when a
set block has a placeholder for an unknown number of values. This worked
fine when the number increased from 1, but we were still attempting to
validate the unknown placeholder against the empty set when the final
count turned out to be 0.

Since we can't differentiate the unknown dynamic placeholder value from
an actual set value, we have to skip that object's validation
altogether.
2020-07-23 15:47:34 -04:00
Chris Stephens 2dd64a7816
plans: Update error message for apply validation (#21312)
* Update error message for apply validation

Add a hint that the validation failure has occurred at the root of the resource
schema to the error message. This is because the root resource has an empty
path when being validated and the path is being relied upon to provide context
into the error message.
2020-06-05 15:08:10 -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
James Bardin e690fa1363
Merge pull request #24904 from hashicorp/jbardin/plan-data-sources
Evaluate data sources in plan when necessary
2020-05-20 10:00:32 -04:00
James Bardin 8e3728af54 rename methods for ConfigResource changes 2020-05-13 13:58:11 -04:00
Kazuki Higashiguchi 7e46b6b9e7 fix typo ResourceInstancChange to ResourceInstanceChange 2020-05-13 17:14:58 +09:00
James Bardin 18ca98a064 GetConfigResourceChanges from plans
In order to find any changes related to a particular configuration
address, we need a new method to get changes to all possible instances.
2020-05-08 12:25:56 -04:00
James Bardin 323d9fb69f plans fix 2020-04-13 16:21:09 -04:00
James Bardin 2490e6c84b provide a method to get all modules changes
Since modules need to be evaluated as whole objects, yet the outputs are
all handled individually, we need a method to collect and return all
output changes for a module from the plan, including all known
module instances.
2020-04-12 11:29:21 -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
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 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
Radek Simko 7860f55e4f
Version tools per Go convention under tools.go 2019-10-17 22:23:39 +02:00
Radek Simko 3d94baecf6
Regenerate protobuf files under latest versions
Used protobuf 3.9.1 with protoc-gen-go 1.3.2
2019-09-05 14:36:15 +02:00
James Bardin 7a183a0e90 don't assert set block length with unknowns
If a planned NestingList block value looks like it may represent a
dynamic block, we don't check the length since it may be unknown. This
check was missing in the NestingSet case, but it applies for the same
reason.

<
2019-07-12 16:48:49 -04:00
James Bardin bfa5e7f811 actual value may be unknown in nested list
When checking AssertObjectCompatible, we need to allow for a possible
unkown nested list block, just as we did for a set in 0b2cc62.
2019-06-25 16:43:57 -04:00
Radek Simko 8a6d1d62b6
stringer: Regenerate files with latest version 2019-05-13 15:34:27 +01:00
Martin Atkins 332010fd56 plans/objchange: Fix handling of dynamic block placeholders
If a dynamic block (in the HCL dynamic block extension sense) has an
unknown value for its for_each argument, it gets expanded to a single
placeholder block with all of its attributes set to a unknown values.

We can use this as part of a heuristic to relax our object compatibility
checks for situations where the plan included an object that appears to
be (but isn't necessarily) such a placeholder, allowing for the fact that
the one placeholder block could be replaced with zero or more real blocks
once the for_each value is known.

Previously our heuristic was too strict: it would match only if the only
block present was a dynamic placeholder. In practice, users may mix
dynamic blocks with static blocks of the same type, so we need to be more
liberal to avoid generating incorrect incompatibility errors in such
cases.
2019-05-02 14:08:40 -07:00
Martin Atkins 95e5ef13a7 vendor: go get github.com/zclconf/go-cty@master
This includes a more comprehensive implementation of Value.GoString, along
with various other changes that don't affect Terraform.
2019-04-25 14:22:57 -07:00
Martin Atkins 88e76fa9ef configs/configschema: Introduce the NestingGroup mode for blocks
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.
2019-04-10 14:53:52 -07:00
Martin Atkins 87fe6cbecd plans/objchange: Don't panic when prior state contains nested map blocks
We were using the wrong cty operation to access map members, causing a
panic whenever a prior value was present for a resource type with a nested
block backed by a map value.
2019-03-18 09:16:50 -07:00
Martin Atkins c5aa5c68bc plans/objchange: Don't panic when dynamic-typed attrs are present
When dynamically-typed attributes are in the schema, we use different
conventions for representing nested blocks containing them (using tuples
and objects instead of lists and maps).

The normalization code here doesn't deal with those because the legacy
SDK never generates them, but we must still pass them through properly or
else other SDKs will be blocked from using dynamic attributes.

Previously this function would panic in that situation. Now it will just
pass through nested blocks containing dynamic attribute values entirely
as-is, with no normalization whatsoever. That's okay, because the scope
of this function is only to normalize inconsistencies that the legacy
SDK is known to produce, and the legacy SDK never produces dynamic-typed
attributes.
2019-03-11 08:18:26 -07:00
James Bardin e50be82da4 don't add empty blocks in ProposedNewObject
If the config contained a null block, don't convert it to a block with
empty values for the proposed new value.
2019-03-02 11:21:59 -05:00
Martin Atkins c280c27d87 plans/objchange: func NormalizeObjectFromLegacySDK
Now that we have an opt-out to let the legacy SDK return values that are
inconsistent with the new conventions for representing configuration,
various parts of Terraform must now be prepared to deal with
inconsistencies.

This function normalizes the most egregious inconsistencies relating to
the representation of nested blocks, freeing any recipient of its result
from worrying about these inconsistencies itself.
2019-02-27 16:53:29 -08:00
Martin Atkins 0b2cc6298b plans/objchange: Fix panic in AssertObjectCompatible with set blocks
Our usual "ground rules" for mapping configschema to cty call for the
collection values representing nested block types to always be known and
non-null, using an empty collection to represent the absense of any blocks
of that type so that users can always safely use length(...) etc on them
without worrying about them sometimes being null.

However, due to some different behaviors in the legacy SDK we've allowed
it an exception to this rule which means that we can see unknown and null
collections in these positions in object values returned from provider
operations like PlanResourceChange and ApplyResourceChange when the legacy
SDK opt-out is activated.

As a consequence of this, we need to be mindful in our safety check
functions, like AssertObjectCompatible here, of tolerating these non-ideal
situations to allow the safety checks to complete. We run these checks
even when the provider requests an opt-out, because we want to note any
inconsistencies as WARNING level log lines to aid in debugging.
2019-02-14 10:04:51 -08:00
Martin Atkins e831182c8d plans/objchange: Hide sensitive attribute values in error messages
Since these error messages get printed in Terraform's output and we
encourage users to share them as part of bug reports, we should avoid
including sensitive information in them to reduce the risk of accidental
exposure.
2019-02-11 17:26:50 -08:00
Martin Atkins fec6e0328d plans/objchange: AssertPlanValid function
Completing our set of provider result safety-check functions,
AssertPlanValid checks a result from a provider's PlanResourceChange to
make sure it doesn't propose a change that is not valid within the user
model of Terraform.

Specifically, it forbids the provider from planning a value that
contradicts what the user gave in configuration, which is important to
ensure that making a reference to an attribute elsewhere in the
configuration will produce exactly the given result, as users reasonably
expect.

Providers _are_ allowed (and, in fact, required) to make changes to
any Computed attribute values declared in the schema in order to fill in
the default values that the provider has generated. Later checks during
the apply phase will ensure that the provider remains true to these
planned values, to ensure that Terraform can keep its promise of doing
exactly what was planned or presenting an error explaining why not.
2019-02-08 19:47:02 -08:00
Martin Atkins 312d798a89 core: Restore our EvalReadData behavior
In an earlier commit we changed objchange.ProposedNewObject so that the
task of populating unknown values for attributes not known during apply
is the responsibility of the provider's PlanResourceChange method, rather
than being handled automatically.

However, we were also using objchange.ProposedNewObject to construct the
placeholder new object for a deferred data resource read, and so we
inadvertently broke that deferral behavior. Here we restore the old
behavior by introducing a new function objchange.PlannedDataResourceObject
which is a specialized version of objchange.ProposedNewObject that
includes the forced behavior of populating unknown values, because the
provider gets no opportunity to customize a deferred read.

TestContext2Plan_createBeforeDestroy_depends_datasource required some
updates here because its implementation of PlanResourceChange was not
handling the insertion of the unknown value for attribute "computed".
The other changes here are just in an attempt to make the flow of this
test more obvious, by clarifying that it is simulating a -refresh=false
run, which effectively forces a deferred read since we skip the eager
read that would normally happen in the refresh step.
2019-02-07 18:33:14 -08:00
Martin Atkins c794bf5bcc plans/objchange: Don't presume unknown for values unset in config
Previously we would construct a proposed new state with unknown values in
place of any not-set-in-config computed attributes, trying to save the
provider a little work in specifying that itself.

Unfortunately that turns out to be problematic because it conflates two
concerns: attributes can be explicitly set in configuration to an unknown
value, in which case the final result of that unknown overrides any
default value the provider might normally populate.

In other words, this allows the provider to recognize in the proposed new
state the difference between an Optional+Computed attribute being set to
unknown in the config vs not being set in the config at all.

The provider now has the responsibility to replace these proposed null
values with unknown values during PlanResourceChange if it expects to
select a value during the apply step. It may also populate a known value
if the final result can be predicted at plan time, as is the case for
constant defaults specified in the provider code.

This change comes from a realization that from core's perspective the
helper/schema ideas of zero values, explicit default values, and
customizediff tweaks are all just examples of "defaults", and by allowing
the provider to see during plan whether these attributes are being
explicitly set in configuration and thus decide whether the default will
be provided immediately during plan or deferred until apply.
2019-02-07 14:01:39 -08:00
Martin Atkins 7216049fdb plans/objchange: Improve precision of AssertObjectCompatible with sets
Previously we were just asserting that the number of elements didn't grow
between planned and actual. We still can't precisely correlate elements in
sets with unknown values, but here we adapt some logic we added earlier
to config/hcl2shim to ensure that we can find a plausible correlation for
each element in each set to at least one element in the other set, and
thus catch more cases where set elements might vanish or appear between
plan and apply, for improved safety.

This will still generate false negatives in some cases where unknown
values are present due to having to assume correlation is intended
wherever it is possible, but we'll catch situations where the actual value
is obviously contrary to what was planned.
2019-02-04 18:19:46 -08:00
James Bardin f3fe6184a0 test for destroy plan round trip 2018-12-20 15:11:08 -05:00
James Bardin 1b8617cef0 don't attempt to decode empty changes values
An empty DynamicValue can't be decoded but indicates no state, so just
return a NullVal.
2018-12-20 13:06:53 -05:00