Commit Graph

22539 Commits

Author SHA1 Message Date
Martin Atkins db9718faef core: Track changes to outputs in the plan along with the state
Our state models cannot store unknown values (since state only deals with
knowns) and so following the lead of recent similar changes for resource
instances we'll treat the planned changeset as a sort of overlay on the
state, preferring values stored there if present, and then write in basic
planned output changes to the plan when we evaluate them.

We're abusing the plan model a little here: its current design is intended
to lay the groundwork for a future release where output values have a
full lifecycle similar to resource instances where we can properly track
changes during the plan phase, but the rest of Terraform isn't yet ready
for that and so we'll just retain an approximation of the planned action
by only using Create and Destroy actions.

A future release should change this so that output changes can be tracked
accurately using an approach similar to that of resource instances.
2018-10-16 19:14:11 -07:00
Martin Atkins d53c3d5c1b plans: Retain output value changes for all outputs in memory
During the plan operation we need to retain _somewhere_ the planned
changes for all outputs so we can refer to them during expression
evaluation. For consistency with how we handle resource instance changes,
we'll keep them in the plan so we can properly retain unknown values,
which cannot be written to state.

As with output values in the state, only root output plans are retained
in a round-trip through the on-disk plan file format, but that's okay
because we can trivially re-calculate all of these during apply. We
include the _root_ outputs in the plan file only because they are
externally-visible side effects that ought to be included in any rendering
of the plan made from the plan file for user inspection.
2018-10-16 19:14:11 -07:00
Martin Atkins fe1e4d8e87 core: In all tests, prevent go-spew from using fmt.Stringer impls 2018-10-16 19:14:11 -07:00
Martin Atkins b0193253b9 core: Fix TestContext2Apply_resourceCountOneList
We've intentionally changed the behavior for "count = 1" so that it'll
assign an index to the created instance even though there's only one. The
un-indexed behavior now applies only if count isn't set _at all_, thus
avoiding weird behavior if a count is _dynamically_ set to 1 via an
expression but is assumed to be a list elsewhere in configuration.
2018-10-16 19:14:11 -07:00
Martin Atkins 80b20a1d9b core: Improve failure output for TestContext2Apply_resourceCountOneList 2018-10-16 19:14:11 -07:00
Martin Atkins e488ff126e core: InstanceDiff.ApplyToValue correctly handle creates
We previously tried to take a shortcut for an empty diff, just returning
the given value directly. This is incorrect in the weird case where we're
creating a new instance but it has no attributes (and thus an empty diff)
because in that case we'd return the given null value, turning the result
into a no-op or destroy change.

To fix this, we just always do the work to construct a new value, even
if we might end up doing all this just to reconstruct the same value we
started with in some cases.
2018-10-16 19:14:11 -07:00
Martin Atkins 27745af0ea core: EvalApply should pass configuration value to provider
This allows the provider to distinguish whether a particular value is set
in configuration or whether it's coming from prior state. It has no
particular purpose other than that.
2018-10-16 19:14:11 -07:00
Martin Atkins b83a563cf0 core: MockProvider.PlanResourceChange must lock before mutating 2018-10-16 19:14:11 -07:00
Martin Atkins ac81511ba6 terraform: test helpers for printing diagnostics in a useful way
We often want to bail out of a test if diagnostics are present, and it's
easiest to debug that when the diagnostics are printed in a compact but
complete manner that is non-trivial to produce.

Rather than duplicating that diagnostic formatting in every test, these
helpers allow us to succinctly print diagnostics and bail out when they
are present.
2018-10-16 19:14:11 -07:00
Martin Atkins efe631d9ec lang/funcs: in "sort", don't panic if given a null string
It is incorrect to use a null string, but that should be reported as an
error rather than a panic.
2018-10-16 19:14:11 -07:00
Martin Atkins 030784eb97 core: Include "type" attribute in the mock _instance type schema
This is a computed attribute populated by our testDiffFn which a number of
tests depend on for correct operation.
2018-10-16 19:14:11 -07:00
Martin Atkins ab1117bcef core: update test fixtures for new name of the "count boundary" vertex 2018-10-16 19:14:11 -07:00
Martin Atkins 54be8c02d4 core: Implement ValidateFn shim for MockProvider 2018-10-16 19:14:11 -07:00
Martin Atkins d1da65b6be core: Allow ProviderMock to fetch its own schema without deadlock 2018-10-16 19:14:11 -07:00
James Bardin 426a976c93 finish context refresh tests 2018-10-16 19:14:11 -07:00
Martin Atkins a8d62478c6 core: Better error message for faulty provider in EvalRefresh 2018-10-16 19:14:11 -07:00
Martin Atkins 297f1019d5 core: Fix logic error in TestContext2Apply_multiRef 2018-10-16 19:14:11 -07:00
Martin Atkins 9632e032c4 core: Fix TestContext2Apply_unstable
This expected count was a faulty port from the old logic. It expects 36
characters because this attribute value is a UUID string.
2018-10-16 19:14:11 -07:00
Martin Atkins a7680ad175 core: Shim to old p.ApplyFn interface in MockProvider
This is a pretty basic attempt to turn a pair of values into an old-school
diff. It probably won't work correctly for all tests, but hopefully works
well enough that we can just update the remaining tests in-place to use
the new API directly.
2018-10-16 19:14:11 -07:00
Martin Atkins 33151f5011 core: Move StateValueFromInstanceState shim from helper/schema
This one doesn't depend on any helper/schema specific bits and it'll also
be useful for the shims in our mock provider in core.
2018-10-16 19:14:11 -07:00
Martin Atkins 8003b3408f states: Fix incorrect ResourceInstanceObjectSrc.DeepCopy
Accidental shadowing of the top-level attrsFlat variable meant that the
flatmap portion of these objects was getting lost in the DeepCopy result.
2018-10-16 19:14:11 -07:00
James Bardin 6f429cc81b make state output match legacy output 2018-10-16 19:14:11 -07:00
James Bardin d50956bdfc updates to the context refresh tests
Add comparers for go-cmp to compare deep cty.Values.
Fix a number of Context2Refresh fixtures.
2018-10-16 19:14:11 -07:00
Martin Atkins 57ca9e3c0a vendor: update go-cty, and some other dependencies
The primary reason for this update is to get cty.PathSet.Equal, for more
convenient deep comparisons using "cmp" in tests.
2018-10-16 19:14:11 -07:00
Martin Atkins 04f076d780 addrs: Implement Equal for resource address types
This is primarily to get good default behavior for test helpers that do
deep comparison of values, but may also be convenient elsewhere.
2018-10-16 19:14:11 -07:00
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