NoEach and Each list both have this check, but it was missing in
EachMap. Refactor the EachList check to remove a level of indentation,
and make the check consistently near the start of the block.
The CreateBeforeDestroy transformer correctly handles the edge referred
to in the comment, and going forward it will probably be easier to use
the knowledge of this edge for CBD anyway.
A number of tests had no, or incomplete state for the transformations
they wanted to test. Add states state with the correct dependencies for
these tests.
An earlier change to eliminate the large amount of duplicate edges being
added by the original CreateBeforeDestroy dependency mapper mistakingly
prevented adding edges when there are multiple CBD dependencies.
This updates the algorithm to use a map to collect all possible edges
and de-deplucating them before processing.
EvalRefreshDependencies is used to update resource dependencies when
they don't exist, allow broken or old states to be updated. While
appending any newly found dependencies is tempting to have the largest
set available, changes to the config could conflict with the prior
dependencies causing cycles.
Since a create node cannot both depend on its destroy node AND be
CreateBeforeDestroy, the same goes for its dependencies. While we do
connect resources with dependency destroy nodes so that updates are
ordered correctly, this ordering does not make sense in the
CreateBeforeDestroy case.
If resource node is CreateBeforeDestroy, we need to remove any direct
dependencies from it to destroy nodes to prevent cycles. Since we don't
know for certain if a crate node is going to be CreateBeforeDestroy at
the time the edge is added in the graph, we add it unconditionally and
prune it out later on. The pruning happens during the CBD transformer
when the CBD destroy node reverses it's own destroy edge. The reason
this works for detecting the original edge, is that dependencies of CBD
resources are forced to be CBD themselves. This does have a false
positive where the case of the original node is NOT CBD, but this can be
taken care of later when we gather enough information in the graph to
prevent the connection in the first place.
The DestroyEdgeTransformer cannot determine ordering from the graph when
the destroyers are from orphaned resources, because there are no
references to resolve. The new stored Dependencies provides what we need
to connect the instances in this case.
We also add the StateDependencies method directly in the
GraphNodeResourceInstance interface, since all instances already
implement this, and we don't need another optional interface to check.
The old code in DestroyEdgeTransformer may no longer be needed in the
long run, but that can be determined separately, since too many of the
tests start with an incomplete state and rely on the Dependencies being
determined from the configuration alone.
Refresh should load any new dependencies found because of configuration
or state changes, but retain any dependencies already in the state.
Orphaned resources would not be in config, but we do not want to lose
the destroy ordering for the later apply.
Make use of the new Dependencies field in the instance state.
The inter-instance dependencies will be determined from the complete
reference graph, so that absolute addresses can be stored, rather than
just references within a module. The Dependencies are added to the node
in the same manner as state, i.e. via an "attacher" interface and
transformer. This is because dependencies are calculated from the graph
itself, and not from the config.
We need to be able to reference all possible dependencies for ordering
when the configuration is no longer present, which means that absolute
addresses must be used. Since this is only to recreate the proper
ordering for instance destruction, only resources addresses need to be
listed rather than individual instance addresses.
If a resource is only destroying instances, there is no reason to
prepare the state and we can remove the Resource (prepare state) nodes.
They normally have pose no issue, but if the instances are being
destroyed along with their dependencies, the resource node may fail to
evaluate due to the missing dependencies (since destroy happens in the
reverse order).
These failures were previously blocked by there being a cycle when the
destroy nodes were directly attached to the resource nodes.
Destroy nodes do not need to be connected to the resource (prepare
state) node when adding them to the graph. Destroy nodes already have a
complete state in the graph (which is being destroyed), any references
will be added in the ReferenceTransformer, and the proper
connection to the create node will be added in the
DestroyEdgeTransformer.
Under normal circumstances this makes no difference, as create and
destroy nodes always have an dependency, so having the prepare state
handled before both only linearizes the operation slightly in the
normal destroy-then-create scenario.
However if there is a dependency on a resource being replaced in another
module, there will be a dependency between the destroy nodes in each
module (to complete the destroy ordering), while the resource node will
depend on the variable->output->resource chain. If both the destroy and
create nodes depend on the resource node, there will be a cycle.
The CBDEdgeTransformer tests worked on fake data structures, with a
synthetic graph, and configs that didn't match. Update them to generate
a more complete graph, with real node implementations, from real
configs.
The output graph is filtered down to instances, and the results still
functionally match the original expected test results, with some minor
additions due to using the real implementation.
When looking for dependencies to fix when handling
create_before_destroy, we need to look past more than one edge, as
dependencies may appear transitively through outputs and variables. Use
Descendants rather than UpEdges.
We have the full graph to use for the CBD transformation, so there's no
longer any need to create a temporary graph, which may differ from the
original.
During the 0.12 work we intended to move all of the variable value
collection logic into the UI layer (command package and backend packages)
and present them all together as a unified data structure to Terraform
Core. However, we didn't quite succeed because the interactive prompts
for unset required variables were still being handled _after_ calling
into Terraform Core.
Here we complete that earlier work by moving the interactive prompts for
variables out into the UI layer too, thus allowing us to handle final
validation of the variables all together in one place and do so in the UI
layer where we have the most context still available about where all of
these values are coming from.
This allows us to fix a problem where previously disabling input with
-input=false on the command line could cause Terraform Core to receive an
incomplete set of variable values, and fail with a bad error message.
As a consequence of this refactoring, the scope of terraform.Context.Input
is now reduced to only gathering provider configuration arguments. Ideally
that too would move into the UI layer somehow in a future commit, but
that's a problem for another day.
We previously deferred this to Validate, but all of our operations require
a valid set of variables and so checking this up front makes it more
obvious when a call into Terraform Core from the CLI layer isn't
populating variables correctly, instead of having it fail downstream
somewhere.
It's the caller's responsibility to ensure that it has collected values
for all of the variables in some way before calling NewContext, because
in the main case driven by the CLI there are many different places that
variable values can be collected from and so handling the main user-facing
validation in the CLI allows us to return better error messages that take
into account which way a variable is (or is not) being set.
Previously we were using the experimental HCL 2 repository, but now we'll
shift over to the v2 import path within the main HCL repository as part of
actually releasing HCL 2.0 as stable.
This is a mechanical search/replace to the new import paths. It also
switches to the v2.0.0 release of HCL, which includes some new code that
Terraform didn't previously have but should not change any behavior that
matters for Terraform's purposes.
For the moment the experimental HCL2 repository is still an indirect
dependency via terraform-config-inspect, so it remains in our go.sum and
vendor directories for the moment. Because terraform-config-inspect uses
a much smaller subset of the HCL2 functionality, this does still manage
to prune the vendor directory a little. A subsequent release of
terraform-config-inspect should allow us to completely remove that old
repository in a future commit.
The old logic for `depends_on` was to short-circuit evaluation of the
data source, but that prevented a plan and state from being recorded.
Use the (currently unused) ForcePlanRead to ensure that the plan is
recorded when the config contains `depends_on`.
This does not fix the fact that depends on does not work with data
sources, and will still produce a perpetual diff. This is only to fix
evaluation errors when an indexed data source is evaluated during
refresh.
* command/import: properly use `-provider` supplied on the command line
The import command now attaches the provider configuration in the resource
instance, if set. That config is attached to the NodeAbstractResource
during the import graph building. This prevents errors when the implied
provider is not actually in the configuration at all, which may happen
when a configuration is using the `-beta` version of a provider (and
only that `-beta` version).
* command/import: fix variable reassignment and update docs
Fixes#22564
Now that the most common cause of unknowns (invalid resource indexes) is
caught earlier, we can validate that the final apply config is wholly
known before attempting to apply it. This ensures that we're applying
the configuration we intend, and not silently dropping values.
Always return the entire resource object from
evaluationStateData.GetResource, rather than parsing the references for
individual instances. This allows for later evaluation of resource
indexes so we can return errors when they don't exist, and prevent
errors when short-circuiting around invalid indexes in conditionals.
In order to allow lazy evaluation of resource indexes, we can't index
resources immediately via GetResourceInstance. Change the evaluation to
always return whole Resources via GetResource, and index individual
instances during expression evaluation.
This will allow us to always check for invalid index errors rather than
returning an unknown value and ignoring it during apply.
The documentation for the -target option warns that it's intended for
exceptional circumstances only and not for routine use, but that's not a
very prominent location for that warning and so some users miss it.
Here we make the warning more prominent by including it directly in the
Terraform output when -target is in use. We first warn during planning
that the plan might be incomplete, and then warn again after apply
concludes and direct the user to run "terraform plan" to make sure that
there are no further changes outstanding. The latter message is intended
to reinforce that -target should only be a one-off operation and that you
should always run without it soon after to ensure that the workspace is
left in a consistent, converged state.
