When the InstanceState.Meta fields are marshaled, numeric values may
change types. The timeout system currently inserts integer values, which
will be unmarshal as float64s.
To ensure that a state which has round-tripped through json is equal to
itself, compare the json representation of the Meta values.
Added a new test that ensures that pre/post-diff hooks are not called
when EvalDiff is run with Stub set, tested through a full refresh run.
This helps test the expected behaviour of EvalDiff itself, versus the
end result of the diff being counted in a plan, which is what the
TestLocal_planScaleOutNoDupeCount test in backend/local checks.
Rather than overloading InstanceDiff with a "Stub" attribute that is
going to be largely meaningless, we are just going to skip
pre/post-diff hooks altogether. This is under the notion that we will
eventually not need to "stub" a diff for scale-out, stateless nodes on
refresh at all, so diff behaviour won't be necessary at that point, so
we should not assume that hooks will run at this stage anyway.
Also as part of this removed the CountHook test that is now failing
because CountHook is out of scope of the new behaviour.
This should make things a bit more clear as to what we are doing in the
EvalTree scale-out test - ensuring that we get the correct eval sequence
for a node with no state through EvalTree.
During plan and apply, because the provider constraints need to be built
from a plan, they are not checked until the terraform.Context is
created. Since the context is always requested by the backend during the
Operation, the backend needs to be responsible for generating contextual
error messages for the user.
Instead of formatting the ResolveProviders errors during NewContext,
return a special error type, ResourceProviderError to signal that
init will be required. The backend can then extract and format the
errors.
This is a specialized thin wrapper around parseResourceAddressInternal
that can be used to obtain a ResourceAddress from the keys in
ModuleDiff.Resources.
This is not something we'd ideally expose, but since the internal address
format is already exposed in the ModuleDiff object this ends up being
necessary to process the ModuleDiff from other packages, e.g. for
display in the UI.
Lexicographic sorting by the string form produces the wrong result because
[9] sorts after [10], so this custom comparison function takes that into
account and compares each portion separately to get a more intuitive
result.
This transformer is no longer needed, as we are not transforming
scale-out resource nodes into plannable nodes anymore, but rather just
taking a different eval sequence for resource refresh nodes with no
state.
This test ensures that the right EvalSequence gets set for a refresh
node with no state. This will ultimately assert that nodes on scale out
will not go down the regular refresh path, which would result in an
error due to the nil state - instead, we stub this node so that we get a
diff on it that can be used to effect computed/unknown values on
interpolations that may depend on this node.
Since the transformer that changed stateless nodes in refresh to
NodePlannableResourceInstance is not being used anymore, this test
needed to be adjusted to ensure that the right output was expected.
Changed the language of this field to indicate that this diff is not a
"real" diff, in that it should not be acted on, versus a "quiet" mode,
which would indicate just simply to act silently.
This fixes a bug with the new refresh graph behaviour where a resource
was being counted twice in the UI on part of being scaled out:
* We are no longer transforming refresh nodes without state to
plannable resources (the transformer will be removed shortly)
* A Quiet flag has been added to EvalDiff and InstanceDiff - this
allows for the flagging of a diff that should not be treated as real
diff for purposes of planning
* When there is no state for a refresh node now, a new path is taken
that is similar to plan, but flags Quiet, and does nothing with the
diff afterwards.
Tests pending - light testing has confirmed this should fix the double
count issue, but we should have some tests to actually confirm the bug.
Previously the behavior for -target when given a module address was to
target only resources directly within that module, ignoring any resources
defined in child modules.
This behavior turned out to be counter-intuitive, since users expected
the -target address to be interpreted hierarchically.
We'll now use the new "Contains" function for addresses, which provides
a hierarchical "containment" concept that is more consistent with user
expectations. In particular, it allows module.foo to match
module.foo.module.bar.aws_instance.baz, where before that would not have
been true.
Since Contains isn't commutative (unlike Equals) this requires some
special handling for targeting specific indices. When given an argument
like -target=aws_instance.foo[0], the initial graph construction (for
both plan and refresh) is for the resource nodes from configuration, which
have not yet been expanded to separate indexed instances. Thus we need
to do the first pass of TargetsTransformer in mode where indices are
ignored, with the work then completed by the DynamicExpand method which
re-applies the TargetsTransformer in index-sensitive mode.
This is a breaking change for anyone depending on the previous behavior
of -target, since it will now select more resources than before. There is
no way provided to obtain the previous behavior. Eventually we may support
negative targeting, which could then combine with positive targets to
regain the previous behavior as an explicit choice.
This is similar in purpose to Equals but it takes a hierarchical approach
where modules contain their child modules, resources are contained by
their modules, and indexed resource instances are contained by their
resource names.
Unlike "Equals", Contains is intended to be transitive, so if A contains B
and B contains C, then C necessarily contains A. It is also directional:
if A contains B then B does not also contain A unless A and B are
identical. This results in more intuitive behavior for use-cases where
the goal is to select a portion of the address space for an operation.
As part of our terminology shift, the interpolation variable for the name
of the current workspace changes to terraform.workspace. The old name
continues to be supported for compatibility.
We can't generate a deprecation warning from here so for now we'll just
silently accept terraform.env as an alias, but not mention it at all in
the error message in the hope that its use phases out over time before we
actually remove it.
The information stored in a plan is tightly coupled to the Terraform core
and provider plugins that were used to create it, since we have no
mechanism to "upgrade" a plan to reflect schema changes and so mismatching
versions are likely to lead to the "diffs didn't match during apply"
error.
To allow us to catch this early and return an error message that _doesn't_
say it's a bug in Terraform, we'll remember the Terraform version and
plugin binaries that created a particular plan and then require that
those match when loading the plan in order to apply it.
The planFormatVersion is increased here so that plan files produced by
earlier Terraform versions _without_ this information won't be accepted
by this new version, and also that older versions won't try to process
plans created by newer versions.
When set, this information gets passed on to the provider resolver as
part of the requirements information, causing us to reject any plugins
that do not match during initialization.
Previously one of the errors had a built-in context message and the other
did not, making it hard for callers to present a user-friendly message
in both cases.
Now we generate an error message of the same form in both cases, with one
case providing additional information. Ideally the main case would be
able to give more specific guidance too, but that's hard to achieve with
the current regexp-based parsing implementation.
This is a useful building block for filtering configuration based on a
resource address. It is similar in principle to state filtering, but for
specific resource configuration blocks.
This allows growing the scope of a resource address to include all of the
resources in the same module as the targeted resource. This is useful to
give context in error messages.
The resource address documentation defines a resource address as being in
two parts: the module path and the resource spec. The resource spec can
be omitted, which represents addressing _all_ resources in a module.
In some cases (such as import) it doesn't make sense to address an entire
module, so this helper makes it easy for validation code to check for
this to reject insufficiently-specific resource addresses.
We're going to use config to determine provider dependencies, so we need
to always provide a config when instantiating a context or we'll end up
loading no providers at all.
We previously had a test for running "terraform import -config=''" to
disable the config entirely, but this test is now removed because it makes
no sense. The actual functionality its testing still remains for now,
but it will be removed in a subsequent commit when we start requiring that
a resource to be imported must already exist in configuration.
Rather than providing an already-resolved map of plugins to core, we now
provide a "provider resolver" which knows how to resolve a set of provider
dependencies, to be determined later, and produce that map.
This requires the context to be instantiated in a different way, so this
very noisy diff is a mostly-mechanical update of all of the existing
places where contexts get created for testing, using some adapted versions
of the pre-existing utilities for passing in mock providers.
Previously the set of providers was fixed early on in the command package
processing. In order to be version-aware we need to defer this work until
later, so this interface exists so we can hold on to the possibly-many
versions of plugins we have available and then later, once we've finished
determining the provider dependencies, select the appropriate version of
each provider to produce the final set of providers to use.
This commit establishes the use of this new mechanism, and thus populates
the provider factory map with only the providers that result from the
dependency resolution process.
This disables support for internal provider plugins, though the
mechanisms for building and launching these are still here vestigially,
to be cleaned up in a subsequent commit.
This also adds a new awkward quirk to the "terraform import" workflow
where one can't import a resource from a provider that isn't already
mentioned (implicitly or explicitly) in config. We will do some UX work
in subsequent commits to make this behavior better.
This breaks many tests due to the change in interface, but to keep this
particular diff reasonably easy to read the test fixes are split into
a separate commit.
ResourceProviderResolver is an extra level of indirection before we
get to a map[string]ResourceProviderFactory, which accepts a map of
version constraints and uses it to choose from potentially-many available
versions of each provider to produce a single ResourceProviderFactory
for each one requested.
As of this commit the ResourceProviderResolver interface is not used. In
a future commit the ContextOpts.Providers map will be replaced with a
resolver instance, with the creation of the factory delayed until the
version constraints have been resolved.
Previously the Type of a ResourceState was generally ignored, but we're
now starting to use it to figure out which providers are needed to
support the resources in state so our tests need to set it accurately
in order to get the expected result.
This new private function takes a configuration tree and a state structure
and finds all of the explicit and implied provider dependencies
represented, returning them as a moduledeps.Module tree structure.
It annotates each dependency with a "reason", which is intended to be
useful to a user trying to figure out where a particular dependency is
coming from, though we don't yet have any UI to view this.
Nothing calls this yet, but a subsequent commit will use the result of
this to produce a constraint-conforming map of provider factories during
context initialization.
Previously the logic for inferring a provider type from a resource name
was buried a utility function in the 'terraform' package. Instead here we
lift it up into the 'config' package where we can make broader use of it
and where it's easier to discover.
* core: Add 'UserAgentString' helper function to generate a standard UserAgent string. Example generation: 'Terraform 0.9.7-dev (go1.8.1)'
* provider/openstack: Add Terraform version to UserAgent string
Prior to Terraform 0.7, lists in Terraform were just a shallow abstraction
on top of strings with a magic delimiter between items. Wrapping a single
string in brackets in the configuration was Terraform's prompt that it
needed to split the string on that delimiter during interpolation.
In 0.7, when first-class lists were added, this convention was preserved
by flattening lists-of-lists by one level when they were encountered in
configuration. However, there was an oversight in that change where it
did not correctly handle the case where the inner list was unknown.
In #14135 we removed some code that was flattening partially-unknown lists
into fully-unknown (untyped) values. This inadvertently exposed the missed
case from the previous paragraph, causing issues for list-wrapped splat
expressions with unknown members. While this worked fine for resources,
due to some fixup done inside helper/schema, this did not work for other
interpolation contexts such as module blocks.
Various attempts to fix this up and restore the flattening behavior
selectively were unsuccessful, due to a proliferation of assumptions all
over the core code that would be too risky to change just to fix this bug.
This change, then, takes the different approach of removing the
requirement that splats be presented inside list brackets. This
requirement didn't make much sense anymore anyway, since no other
list-returning expression had this constraint and so the rest of Terraform
was already successfully dealing with both cases.
This leaves us with two different scenarios:
- For resource arguments, existing normalization code in helper/schema
does its own flattening that preserves compatibility with the common
practice of using bracketed splats. This change proves this with a test
within the "test" provider that exercises the whole Terraform core and
helper/schema stack that assigns bracketed splats to list and set
attributes.
- For arguments in other blocks, such as in module callsites, the
interpolator's own flattening behavior applies to known lists,
preserving compatibility with configurations from before
partially-computed splats were possible, but those wishing to use
partially-computed splats are required to drop the surrounding brackets.
This is less concerning because this scenario was introduced only in
0.9.5, so the scope for breakage is limited to those who adopted this
new feature quickly after upgrading.
As of this commit, the recommendation is to stop using brackets around
splats but the old form continues to be supported for backward
compatibility. In a future _major_ version of Terraform we will probably
phase out this legacy form to improve consistency, but for now both
forms are acceptable at the expense of some (pre-existing) weird behavior
when _actual_ lists-of-lists are used.
This addresses #14521 by officially adopting the suggested workaround of
dropping the brackets around the splat. However, it doesn't yet allow
passing of a partially-unknown list between modules: that still violates
assumptions in Terraform's core, so for the moment partially-unknown lists
work only within a _single_ interpolation expression, and cannot be
passed around between expressions. Until more holistic work is done to
improve Terraform's type handling, passing a partially-unknown splat
through to a module will result in a fully-unknown list emerging on
the other side, just as was the case before #14135; this change just
addresses the fact that this was failing with an error in 0.9.5.
Instead of using a hardcoded version prerelease string, which makes release automation difficult, set the version prerelease string from an environment variable via the go linker tool during compile time.
The environment variable `TF_RELEASE` should only be set via the `make bin` target, and thus leaves the version prerelease string unset. Otherwise, when running a local compile of terraform via the `make dev` makefile target, the version prerelease string is set to `"dev"`, as usual.
This also requires some changes to both the circonus and postgresql providers, as they directly used the `VersionPrerelease` constant. We now simply call the `VersionString()` function, which returns the proper interpolated version string with the prerelease string populated correctly.
`TF_RELEASE` is unset:
```sh
$ make dev
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/22 10:38:19 Generated command/internal_plugin_list.go
==> Removing old directory...
==> Building...
Number of parallel builds: 3
--> linux/amd64: github.com/hashicorp/terraform
==> Results:
total 209M
-rwxr-xr-x 1 jake jake 209M May 22 10:39 terraform
$ terraform version
Terraform v0.9.6-dev (fd472e4a86500606b03c314f70d11f2bc4bc84e5+CHANGES)
```
`TF_RELEASE` is set (mimicking the `make bin` target):
```sh
$ TF_RELEASE=1 make dev
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/22 10:40:39 Generated command/internal_plugin_list.go
==> Removing old directory...
==> Building...
Number of parallel builds: 3
--> linux/amd64: github.com/hashicorp/terraform
==> Results:
total 121M
-rwxr-xr-x 1 jake jake 121M May 22 10:42 terraform
$ terraform version
Terraform v0.9.6
```
For child modules, a ModuleState isn't allocated until the first time a
module instance is inserted into the state under the module's path.
Normally interpolations of resource attributes are delayed until at least
one resource has been created due to the nature of the dependency graph,
but if the interpolation value is a multi-var (splat) then it is possible
that the referenced resource has count=0 and thus created _no_ resource
states when it was visited.
Previously we would crash when trying to access the resource map for the
nil module in order to count how many instances are present. Since we know
there can't be any instances present in a nil module, we now preempt
this crash by returning zero early.
This edge-case does not apply to the root module because its ModuleState
is allocated as part of initializing the main State instance.
This fixes#14438.
Currently, the refresh graph uses the resources from state as a base,
with data sources then layered on. Config is not consulted for resources
and hence new resources that are added with count (or any new resource
from config, for that matter) do not get added to the graph during
refresh.
This is leading to issues with scale in and scale out when the same
value for count is used in both resources, and data sources that may
depend on that resource (and possibly vice versa). While the resources
exist in config and can be used, the fact that ConfigTransformer for
resources is missing means that they don't get added into the graph,
leading to "index out of range" errors and what not.
Further to that, if we add these new resources to the graph for scale
out, considerations need to be taken for scale in as well, which are not
being caught 100% by the current implementation of
NodeRefreshableDataResource. Scale-in resources should be treated as
orphans, which according to the instance-form NodeRefreshableResource
node, should be NodeDestroyableDataResource nodes, but this this logic
is currently not rolled into NodeRefreshableDataResource. This causes
issues on scale-in in the form of race-ish "index out of range" errors
again.
This commit updates the refresh graph so that StateTransformer is no
longer used as the base of the graph. Instead, we add resources from the
state and config in a hybrid fashion:
* First off, resource nodes are added from config, but only if
resources currently exist in state. NodeRefreshableManagedResource
is a new expandable resource node that will expand count and add
orphans from state. Any count-expanded node that has config but no
state is also transformed into a plannable resource, via a new
ResourceRefreshPlannableTransformer.
* The NodeRefreshableDataResource node type will now add count orphans
as NodeDestroyableDataResource nodes. This achieves the same effect
as if the data sources were added by StateTransformer, but ensures
there are no races in the dependency chain, with the added benefit of
directing these nodes straight to the proper
NodeDestroyableDataResource node.
* Finally, config orphans (nodes that don't exist in config anymore
period) are then added, to complete the graph.
This should ensure as much as possible that there is a refresh graph
that best represents both the current state and config with updated
variables and counts.
The previous behavior of targets was that targeting a particular node
would implicitly target everything it depends on. This makes sense when
the dependencies in question are between resources, since we need to
make sure all of a resource's dependencies are in place before we can
create or update it.
However, it had the undesirable side-effect that targeting a resource
would _exclude_ any outputs referring to it, since the dependency edge
goes from output to resource. This then causes the output to be "stale",
which is problematic when outputs are being consumed by downstream
configs using terraform_remote_state.
GraphNodeTargetDownstream allows nodes to opt-in to a new behavior where
they can be targeted by _inverted_ dependency edges. That is, it allows
outputs to be considered targeted if anything they directly depend on
is targeted.
This is different than the implied targeting behavior in the other
direction because transitive dependencies are not considered unless the
intermediate nodes themselves have TargetDownstream. This means that
an output1→output2→resource chain can implicitly target both outputs, but
an output→resource1→resource2 chain _won't_ target the output if only
resource2 is targeted.
This behavior creates a scenario where an output can be visited before
all of its dependencies are ready, since it may have a mixture of both
targeted and untargeted dependencies. This is fine for outputs because
they silently ignore any errors encountered during interpolation anyway,
but other hypothetical future implementers of this interface may need to
be more careful.
This fixes#14186.
This was actually redundant anyway since HIL itself applied a similar
rule where any partially-unknown list would be automatically flattened
to a single unknown value.
However, now we're changing HIL to explicitly permit partially-unknown
lists so that we can allow the index operator [...] to succeed when
applied to one of the elements that _is_ known.
This, in conjunction with hashicorp/hil#51 and hashicorp/hil#52,
fixes#3449.
stringer has changed the boilerplate it generates in a recent version.
We'd previously updated to the new format but accientally rolled back
to the old while merging a long-running feature branch.
This restores us back to the new format again.
Moving the transformer wholesale looks like it broke some tests, with
some actually doing legit work in normalizing singular resources from a
foo.0 notation to just foo.
Adjusted the TestPlanGraphBuilder to account for the extra
meta.count-boundary nodes in the graph output now, as well as added
another context test that tests this case. It appears the issue happens
during validate, as this is where the state can be altered to a broken
state if things are not properly transformed in the plan graph.
This fixes interpolation issues on grandchild data sources that have
multiple instances (ie: counts). For example, baz depends on bar, which
depends on foo.
In this instance, after an initial TF run is done and state is saved,
the next refresh/plan is not properly transformed, and instead of the
graph/state coming through as data.x.bar.0, it comes through as
data.x.bar. This breaks interpolations that rely on splat operators -
ie: data.x.bar.*.out.
* Revert #11245, #11321, #11498 and #11757
These PR’s are all related to issue #11170 for which I would like to propose a different solution then the one currently implemented.
* A different approach to solve #11170
This approach has (IMHO) a few advantages with regards to the solution currently implemented. I will elaborate on this in the PR.
The documentation for Refresh indicates that it will always return a
valid state, but that wasn't true in the case of a graph builder error.
While this same concept wasn't documented for Apply, it was still
assumed in the terraform apply code.
Since the helper testing framework relies on the absence of a state to
determine if it can call Destroy, the Context can't can't start
returning a state in all cases. Document this, and use the State method
to fetch the correct state value after Apply.
Add a nil check to the WriteState function, so that writing a nil state
is a noop.
Make sure to init before sorting the state, to make sure we're not
attempting to sort nil values. This isn't technically needed with the
current code, but it's just safer in general.
Make sure duplicate depends_on entries are pruned from existing states
on read.
Make sure new state built from configs with multiple references to the
same resource only add it once to the Dependencies.
duplicate entries could end up in "depends_on" in the state, which could
possible lead to erroneous state comparisons. Remove them when walking
the graph, and remove existing duplicates when pruning the state.
Previously this function was depending on the mapstructure behavior of
failing with an error when trying to decode a map into a list or
vice-versa, but mapstructure's WeakDecode behavior changed so that it
will go to greater lengths to coerce the given value to fit into the
target type, causing us to mis-handle certain ambigous cases.
Here we exert a bit more control over what's going on by using 'reflect'
to first check whether we have a slice or map value and only then try
to decode into one with mapstructure. This allows us to still rely on
mapstructure's ability to decode nested structures but ensure that lists
and maps never get implicitly converted to each other.
Since the validation of connection blocks is delegated to the communicator
selected by "type", we were not previously doing any validation of the
attribute names in these blocks until running provisioners during apply.
Proper validation here requires us to already have the instance state,
since the final connection info is a merge of values provided in config
with values assigned automatically by the resource. However, we can do
some basic name validation to catch typos during the validation pass, even
though semantic validation and checking for missing attributes will still
wait until the provisioner is instantiated.
This fixes#6582 as much as we reasonably can.
This previously lacked tests altogether. This new test verifies the
"happy path", ensuring that both literal and computed values pass through
correctly into the VariableValues map.
This crash resulted because the type switch checked for either of two
types but the type assertion within it assumed only one of them.
A straightforward (if inelegant) fix is to simply duplicate the relevant
case block and change the type assertion, thus allowing the types to match
up in all cases.
This fixes#13297.
During the input walk we stash the values resulting from user input
(if any) in the eval context for use when later walks need to resolve
the provider config.
However, this repository of input results is only able to represent
literal values, since it does not retain the record of which of the keys
have values that are "computed".
Previously we were blindly stashing all of the results, failing to
consider that some of them might be computed. That resulted in the
UnknownValue placeholder being misinterpreted as a literal value when
the data is used later, which ultimately resulted in it clobbering the
actual expression evaluation result and thus causing the provider to
fail to configure itself.
Now we are careful to only retain in this repository the keys whose values
are known statically during the input phase. This eventually gets merged
with the dynamic evaluation results on subsequent walks, with the dynamic
keys left untouched due to their absence from the stored input map.
This fixes#11264.
This method mirrors that of config.Backend, so we can compare the
configration of a backend read from a config vs that of a backend read
from a state. This will prevent init from reinitializing when using
`-backend-config` options that match the existing state.
golang/tools commit 23ca8a263 changed the format of the leading comment
to comply with some new standards discussed here:
https://golang.org/issue/13560
This is the result of running generate with the latest version of
stringer. Everyone working on Terraform will need to update stringer
after this is merged, to avoid reverting this:
go get -u golang.org/x/tools/cmd/stringer
It appears there are no tests for this as far as I can find.
We change V1 states (very old) to assume a nil path is a root path.
Staet.Validate() later will catch any duplicate paths.
When transforming a diff from DestroyCreate to a simple Update,
ignore_changes can cause keys from flatmapped objects to be filtered
form the diff. We need to filter each flatmapped container as a whole to
ensure that unchanged keys aren't lost in the update.
ignore_changes is causing changes in other flatmapped sets to be
filtered out incorrectly.
This required fixing the testDiffFn to create diffs which include the
old value, breaking one other test.
