Commit Graph

22314 Commits

Author SHA1 Message Date
Martin Atkins 8048e9a585 plans/objchange: Don't panic if old or new values are null 2018-10-16 19:14:11 -07:00
Martin Atkins 972b28745b core: Avoid concurrent map access in TestContext2Apply_multiVar 2018-10-16 19:14:11 -07:00
Martin Atkins 7f1954e70c core: Don't panic if EvalWriteDiff gets a change in a non-root module 2018-10-16 19:14:11 -07:00
Martin Atkins a7f948ab90 core: Fix TestContext2Apply_unstable
We now handle impure functions by having them return an unknown value
during plan, since we can't predict what the value will be during apply.
This test was assuming the old behavior.
2018-10-16 19:14:11 -07:00
Martin Atkins ee49be73be core: Don't panic if ApplyResourceChange returns invalid new value
The provider is allowed to return a partial result if it also includes
error diagnostics. Real providers still return at least a null value in
that case due to the RPC format, but test mocks are often more sloppy.
2018-10-16 19:14:11 -07:00
Martin Atkins 84102170b3 core: MockProvider shim handling of DiffFn
This should allow many of our existing tests using DiffFn to continue
working with little or no further modification.
2018-10-16 19:14:11 -07:00
Martin Atkins 76d11f44cc core: Move some of the helper/schema shims so provider mock can use them
The old names are now wrappers around these new functions.
2018-10-16 19:14:11 -07:00
Martin Atkins 9c4aed52b3 core: Don't panic in refresh tests
Since the refresh walk creates a partial plan to account for objects that
are yet to be created, we need to provide at least a basic mock of the
PlanProviderChange provider method.

For now we're using the old-style "DiffFn" shim interface since that's
already available for use in other tests.
2018-10-16 19:14:11 -07:00
Martin Atkins 70c555cfd3 core: EvalDiff to panic earlier if it gets back nil value from provider
It's not possible for a normal RPC-based provider to get into this
situation because a nil value can't go over the wire, but it's easy to
cause this by not correctly configuring a provider mock during tests.

By panicking early here we produce a more helpful error message and stack
trace than we'd otherwise produce if we let this nil value escape out
into the rest of Terraform.
2018-10-16 19:14:11 -07:00
Martin Atkins 8cc8bacce3 config/hcl2shim: Treat DynamicVal like any other unknown value in flatmap 2018-10-16 19:14:11 -07:00
Martin Atkins 52c28183b5 core: MockProvider not to panic if import mock function returns no attrs 2018-10-16 19:14:11 -07:00
Martin Atkins ebd3aba0be core: Fix various compile-time errors in tests
Significant changes to the provider interface left a lot of the
tests in a non-buildable state. This set of changes gets the
tests building again but does not attempt to make them run to
completion or pass.

After this commit, it is possible to build a test program for
the ./terraform package but it will panic during its run. That
will be addressed in subsequent commits.
2018-10-16 19:14:11 -07:00
Martin Atkins 6fd82ef97e core: Split Replace changes into separate Delete/Create changes
Since we do our deletes using a separate graph node from all of the other
actions, and a "Replace" change implies both a delete _and_ a create, we
need to pretend at apply time that a single replace change was actually
two separate changes.

This will also early-exit eval if a destroy node finds a non-Delete change
or if an apply node finds a Delete change. These should not happen in
practice because we leave these nodes out of the graph when they are not
needed for the given action, but we do this here for robustness so as not
to have an invisible dependency between the graph builder and the eval
phase.
2018-10-16 19:14:11 -07:00
Martin Atkins 1cc9d00da6 core: don't panic if NewResourceConfigShimmed gets a null
When we're working on a create or destroy change it's expected for one of
the values to be null. Here we mimick the pre-0.12 behavior of producing
just an empty map in that case, which the helper/schema code (now the only
caller of this shim) then ignores completely.
2018-10-16 19:14:11 -07:00
Martin Atkins 6365b9ec7f core: EvalCheckPlannedChange to check change action
Previously we were checking only the before and after values, and not
verifying that the change action is unchanged between plan and apply.
2018-10-16 19:14:11 -07:00
Martin Atkins b428746cfd command: UiHook to report when it gets incorrect values
For PreApply hook purposes we only actually use the Delete, Create, and
Update actions, because other actions are handled in different ways than
a direct call to ApplyResourceChange.

However, if there's a bug in core that causes it to pass a different
action, it's better for us to mark it as being explicitly unknown in the
UI rather than simply defaulting to "Modifying...", which can thus obscure
the problem and make for a confusing result.
2018-10-16 19:14:11 -07:00
James Bardin a87470cc15 resource ids must always have a value
The "id" field is assumed to always exist, and must have a valid value.
Set "id" to unknown when planning new resource changes to indicate that
it will be computed.
2018-10-16 19:14:11 -07:00
Martin Atkins e6c958048c build: Document in a machine-readable way that we are now using Go 1.11 2018-10-16 19:14:11 -07:00
Martin Atkins 0341c78320 vendor: update cty to v0.0.0-20180831220647-752f6a689f5e
This includes a fix to UnknownAsNull to make it not crash when given a
value that is _already_ null.
2018-10-16 19:14:11 -07:00
Martin Atkins 9c9a918790 core: Record correct provider address in orphan destroy plan 2018-10-16 19:14:11 -07:00
Martin Atkins 9af67806fc core: Prune placeholder objects from state after refresh
Prior to our refactoring here, we were relying on a lucky coincidence for
correct behavior of the plan walk following a refresh in the same run:

- The refresh phase created placeholder objects in the state to represent
  any resource instance pending creation, to allow the interpolator to
  read attributes from them when evaluating "provider" and "data" blocks.
  In effect, the refresh walk is creating a partial plan that only covers
  creation actions, but was immediately discarding the actual diff entries
  and storing only the planned new state.

- It happened that objects pending creation showed up in state with an
  empty ID value, since that only gets assigned by the provider during
  apply.

- The Refresh function concluded by calling terraform.State.Prune, which
  deletes from the state any objects that have an empty ID value, which
  therefore prevented these temporary objects from surviving into the
  plan phase.

After refactoring, we no longer have this special ID field on instance
object state, and we instead rely on the Status field for tracking such
things. We also no longer have an explicit "prune" step on state, since
the state mutation methods themselves keep the structure pruned.

To address this, here we introduce a new instance object status "planned",
which is equivalent to having an empty ID value in the old world. We also
introduce a new method on states.SyncState that deletes from the state
any planned objects, which therefore replaces that portion of the old
State.prune operation just for this refresh use-case.

Finally, we are now expecting the expression evaluator to pull pending
objects from the planned changeset rather than from the state directly,
and so for correct results these placeholder resource creation changes
must also be reported in a throwaway changeset during the refresh walk.

The addition of states.ObjectPlanned also permits a previously-missing
safety check in the expression evaluator to prevent us from relying on the
incomplete value stored in state for a pending object, in the event that
some bug prevents the real pending object from being written into the
planned changeset.
2018-10-16 19:14:11 -07:00
Martin Atkins 40f00d8db5 core: StateReferences method should return []addrs.Referenceable
We no longer use strings to represent addresses, so this method was a
leftover outlier from previous refactoring efforts.

At this time the result is not actually being used due to the state type
refactoring, which is a bug we'll address in a subsequent commit.
2018-10-16 19:14:11 -07:00
Martin Atkins 59175e466e core: Extra TRACE logging in the eval_state.go eval nodes.
Tracking when each instance's state is read, written, or removed is often
very useful in debugging strange interactions.
2018-10-16 19:14:11 -07:00
Martin Atkins a37d5268eb command/format: Render unified diff for list and tuple value changes 2018-10-16 19:14:11 -07:00
Martin Atkins 30a46ed8ed command/format: Further refinement on the heuristic JSON-based diff
We'll now show an "update" symbol prior to the argument to this synthetic
jsonencode(...) call, for consistency with how we show nested values in
other cases and to attach a verb to any "# forces replacement".

We'll also show a special form in the case where the value seems to differ
only in whitespace, so users can understand what's going on in that
hopefully-rare situation, particularly if those whitespace-only changes
end up forcing us to replace a remote object.
2018-10-16 19:14:11 -07:00
Martin Atkins 783205948c command/format: JSON value formatting heuristic not for primitive values
Since our own syntax for primitive values is similar to that of JSON, and
since we permit automatic conversions from number and bool to string, we
must do this special JSON value diff formatting only if the value is a
JSON array or object to avoid confusing results.
2018-10-16 19:14:11 -07:00
Martin Atkins 4406b06ef8 command/format: Don't include commas after elements in map diffs
Although commas are allowed in this context, it's not idiomatic to use
them.
2018-10-16 19:14:11 -07:00
Martin Atkins ce157c7f23 command/format: show deep diffs for old/new JSON strings
Because so far we've not supported dynamically-typed complex data
structures, several providers have used strings containing JSON to stand
in for these.

In order to get a readable diff in those cases, we'll recognize situations
where old and new are both JSON and present a diff of the effective value
of the JSON, using a faux call to the jsonencode(...) function to indicate
when we've done so.

This is a bit of a "cute" heuristic, but is important at least for now
until we can migrate away from that practice of passing large JSON strings
to providers and use dynamically-typed attributes instead.
2018-10-16 19:14:11 -07:00
Martin Atkins 9706a00b3a command/format: correct alignment for multi-line values in map diff 2018-10-16 19:14:11 -07:00
Martin Atkins 20318ca193 backend/local: Sort planned resource changes before rendering them
As in the old plan renderer, they are ordered by the natural ordering of
the resource addresses.
2018-10-16 19:14:11 -07:00
Martin Atkins 04d8c17be8 command/format: Nice rendering for changes to map values 2018-10-16 19:14:11 -07:00
Martin Atkins a2f4b8b058 command/format: Show resource diff with header "comment"
This extra comment line gives us a place to show the full resource address
(since the block header line only includes type and name) and also allows
us to explain in long form the meaning of the change icon on the following
line.
2018-10-16 19:14:11 -07:00
Martin Atkins 239a54ad6f command: initial structural diff rendering
This is a light adaptation of our earlier prototype of structural diff
rendering, as a starting point for what we'll actually ship. This is not
consistent with the latest mocks, so will need some additional work before
it is ready, but integrating this allows us to at least see the plan
contents while fixing up remaining issues elsewhere.
2018-10-16 19:14:11 -07:00
Martin Atkins 1aa9ac14cc plans/objchange: LongestCommonSubsequence
This algorithm is the usual first step when generating diffs. This package
is a bit of a strange home for it, but since it works with changes to
cty.Value this feels more natural than any other place it could be.
2018-10-16 19:14:11 -07:00
Martin Atkins 1ced176fc6 plans: Track RequiredReplace as a cty.PathSet
We were previously tracking this as a []cty.Path, but having it turned
into a pathset on creation makes downstream use of it more convenient and
ensures that it'll obey expected invariants like not containing the same
path twice.
2018-10-16 19:14:11 -07:00
Martin Atkins d54b52fa01 core: Fix error reporting for malformed provider response values 2018-10-16 19:14:11 -07:00
Martin Atkins 630b0d147d providers/terraform: Always produce correctly-typed result
We need to produce values for all of the attributes implied by our schema,
even if some of them are null.
2018-10-16 19:14:11 -07:00
Martin Atkins 235f582ae5 core: Re-implement EvalReadDataApply against new provider interface 2018-10-16 19:14:11 -07:00
Martin Atkins d96284f2e2 tfdiags: FormatErrorPrefixed
When presenting an error that may be a PathError, the error's path is
usually relative to some other value. If the caller is able to express
that value (or, more often, a reference to it) in HCL syntax then this
method will produce a complete expression in the error message,
concatenating any path information from the error to the end of the given
prefix string.
2018-10-16 19:14:11 -07:00
Martin Atkins 182e783885 core: EvalReadDataDiff to output direct config value alongside planned
We're now writing the "planned new value" to OutputValue, but the data
resource nodes during refresh need to see the verbatim config value in
order to decide whether read must be deferred to the apply phase, so we'll
optionally export that here too.
2018-10-16 19:14:11 -07:00
Martin Atkins 20adb9d9b7 core: Evaluate resource references from plan where possible
Our state representation is not able to preserve unknown values, so it's
not suitable for retaining the transient incomplete values we produce
during planning.

Instead, we'll discard the unknown values when writing to state and have
the expression evaluator prefer an object from the plan where possible.
We still use the shape of the transient state to inform things like the
resource's "each mode", so the plan only masks the object values
themselves.
2018-10-16 19:14:11 -07:00
Martin Atkins 1afdb055ca core: Re-implement ReadDataDiff around our new approach
This is no longer a call into the provider, since all of the data diff
logic is standard for all data sources anyway. Instead, we just compute
the planned new value and construct a planned change from that as-is.

Previously the provider could, in principle, customize the read diff. In
practice there is no real reason to do that and the existing SDK didn't
pass that possibility through to provider code, so we can safely change
this without impacting provider compatibility.
2018-10-16 19:14:11 -07:00
Martin Atkins 3f8a973846 plans/objchange: when prior is null, computed attributes are unknown 2018-10-16 19:14:11 -07:00
Martin Atkins 7249a73f79 core: Don't panic if "required replace" path doesn't resolve to a value 2018-10-16 19:14:11 -07:00
Martin Atkins 6acf335e60 core: Fix evaluation of collections of resource instances 2018-10-16 19:14:11 -07:00
Martin Atkins 97da905c6e helper/plugin: ReadResource to deal with missing remote object 2018-10-16 19:14:11 -07:00
Martin Atkins 1d672623eb core: Remove changes from the plan after they are applied 2018-10-16 19:14:11 -07:00
Martin Atkins a7342de274 core: Properly handle no-op changes in plan
Previously we just left these out of the plan altogether, but in the new
plan types we intentionally include change information for every resource
instance, even if no changes are actually planned, to allow alternative
plan file viewers to show what isn't changing as well as what is.
2018-10-16 19:14:11 -07:00
Martin Atkins 2f0e5d93c8 plans: ChangesSync.GetResourceInstanceChange must copy the change
This is promised in its doc comment, but wasn't actually done in practice.
2018-10-16 19:14:11 -07:00
Martin Atkins c27f900d92 helper/plugin: Don't panic while preparing response in ApplyResourceChange 2018-10-16 19:14:11 -07:00