Outputs were not being evaluated during import, because it was not added
to the walk filter.
Remove any unnecessary walk filters from all the Execute nodes.
Allow the evaluation of resource pending deleting only during a full
destroy. With this change we can ensure deposed instances are not
evaluated under normal circumstances, but can be references when needed.
This also allows us to remove the fixup transformer that added
connections so temporary values would evaluate in the correct order when
referencing destroy nodes.
In the majority of cases, we do not want to evaluate resources that are
pending deletion since configuration references only can refer to
resources that is intended to be managed by the configuration. An
exception to that rule is when Terraform is performing a full `destroy`
operation, and providers need to evaluate existing resources for their
configuration.
If a data source refers to a indexed managed resource, we need to
re-target that reference to the containing resource for planning. Since
data sources use the same mechanism as depends_on for managed resource
references, they can only refer to resources as a whole.
Treat any reference from a data source to a managed resource as a
dependency on the entire resource. While a resource's
attribute may be statically resolvable from the configuration, if the
user added a reference to that resource, it stands to reason that the
user intended there to be a dependency which we need to wait on.
This is an extension of an implicit behavior that existed previously in
Terraform, but was lost in the 0.13 release. That behavior was emergent
from the fact that the Refresh walk did not process the configuration
for managed resources, so any new resources in the config would be
evaluate as entirely unknown during Refresh, even if some attributes
were statically resolvable at that point.
This new implementation restores the old behavior, and extends it to
updates and replacements of the referenced resource.
Since we have to allow destroy nodes to be evaluated for providers
during a full destroy, this is adding a transformer to connect temporary
values to any destroy versions of their references when possible. The
ensures that the destroy happens before evaluation, even when there
isn't a full create-then-destroy set of instances.
The cases where the connection can't be made are when the temporary
value has a provider descendant, which means it must evaluate early in
the case of a full destroy. This means the value may contain incorrect
data when referencing resource that are create_before_destroy, or being
scaled-in via count or for_each. That will need to be addressed later by
reevaluating how we handle the full destroy case in terraform.
Outputs and locals cannot refer to destroy nodes. Since those nodes
types do not have different ordering for create and destroy operations,
connecting them directly to destroy nodes can cause cycles.
Since data source destruction is only state removal, and other resources
cannot depend on them creating any physical resources, the destroy
dependencies were not tracked in the state. It turns out that there is a
special case which requires this; running terraform destroy where the
provider depends on a data source. In that case the resources using that
provider need to record their indirect dependence on the data source, so
that they can be deleted before the data source is removed from the
state.
Our reference transformer analyses and our destroy transformer analyses
are built around static (not-yet-expanded) addresses so that they can
correctly handle mixtures of expanded and not-yet-expanded objects in the
same graph.
However, this characteristic also makes them unnecessarily conservative
in their handling of references between resources within different
instances of the same module: we know they can never interact with each
other in practice because the dependencies for all instances of a module
are the same and so one instance cannot possibly depend on another.
As a compromise then, here we introduce a new helper function that can
recognize when a proposed edge is between two resource instances that
belong to different instances of the same module, and thus allow us to
skip actually creating those edges even though our imprecise analyses
believe them to be needed.
As well as significantly reducing the number of edges in situations where
multi-instance resources appear inside multi-instance modules, this also
fixes some potential cycles in situations where a single plan includes
both destroying an instance of a module and creating a new instance of the
same module: the dependencies between the objects in the instance being
destroyed and the objects in the instance being created can, if allowed
to connect, cause Terraform to believe that the create and the destroy
both depend on one another even though there is no need for that to be
true in practice.
This involves a very specialized helper function to encode the situation
where this exception applies. This function has an ugly name to reflect
how specialized it is; it's not intended to be of any use outside of these
three situations in particular.
There aren't going to be any nodes specifically for module call
instances during plan, so we have to switch the reference subject to the
general module call.
During refresh, data sources need to know if their parent modules have
depends_on configured at all. Pass this info back through the search for
depends_on resources, and delay refresh when it's set.
Resources that are not yet created will not be in the graph during
refresh, and therefore cannot be attached to the data source nodes. In
this case we still need to indicate if there are depends_on entries
inherited from the module call, which we can do with the forceDependsOn
field.
create interfaces that nodes can implement to declare whether they
expand into instances of some sort, using the instances.Expander, and/or
whether use the instances.Expander to find instances.
included is a rough transformer implementation to remove these nodes
from the apply graph.
Rather than re-read the data source during every plan cycle, apply the
config to the prior state, and skip reading if there is no change.
Remove the TODOs, as we're going to accept that data-only changes will
still not be plan-able for the time being.
Fix the null data source test resource, as it had no computed fields at
all, even the id.
This transformer is what will provider the data sources with the
transitive dependencies needed to determine if they can read during plan
or must be deferred.
Adding a transformer to attach any transitive DependsOn references to
data sources during plan. Refactored the ReferenceMap from the
ReferenceTransformer so it can be reused for both.
GraphNodeAttachDependsOn give us a method for adding all transitive resource
dependencies found through depends_on references, so that data source
can determine if they can be read during plan. This will be done by
inspecting the changes of all dependency resources, and delaying read
until apply if any changes are planned.
NodeModuleRemoved is redundant now with the concept of
nodeCloseModule, so we can replace it within the graph. This does mean
that nodeCloseModule needs to know if it's evaluating an orphaned module
that can't be expanded, but the overhead to checking this isn't too
bad.
Now that nodeModuleClose is referenceable, and we can ensure it's always
in the graph at the correct time, we can eliminate the need to connect
each resource to every single node within a module it references, and
instead connect only to the nodeModuleClose, which acts as the module
root. Since module expansion can cause exponential growth in the number
of edges in graphs, this will help with performance problems when
transforming and reducing these graphs by eliminating the bulk of
redundant edges. This will also help with general debugging, making the
graphs easier to read.
There is not one more non-dependent type to look for when pruning unused
values. This fixes the oversight, but still leaves the ugly concrete
type checking which we need to remove.
During plan, anything dependent on a module can connect to the module
expansion node, because all instance nodes are created during
DynamicExpand. During apply the instance nodes are created from the
diff, so we need a root module to terminate the logical module subgraph.
Besides providing an anchor for the completion of a module, the
nodeCloseModule can also be used to cleanup the orphan resource and
module placeholders in the state.
Make the interface name reflect the new return type of the method.
Remove the confusingly named and unused ResourceAddress method from the
resource nodes as well.
Unexpanded nodes can't implement GraphNodeModuleInstance (nee
GraphNodeSubPath), because they aren't aware how they have been
expanded, and may be in multiple distinct paths.
Since that means the EvalContext won't be in the correct path during the
walk, we just have to ensure that we don't use `ctx.Path()` inside Eval.
Since references are always within the scope of a single module, and we
can connect all module instance outputs for proper ordering, the
existing transformer works directly with only module paths as opposed to
module instances.
TODO: TransformApplyReferences for more precise module instance
targeting?
GraphNodeModulePath is similar to GraphNodeSubPath, except that it
returns an addrs.Module rather than an addrs.ModuleInstance. This is
used by the ReferenceTransformer to connect references, when modules may
not yet be expanded.
Because references only exist within the scope of a module, we can
connect everything knowing only the module path. If the reference is to
an expanded module instance output, we can still properly order the
reference because we'll wait for the entire module to complete
evaluation.
* WIP: dynamic expand
* WIP: add variable and local support
* WIP: outputs
* WIP: Add referencer
* String representation, fixing tests it impacts
* Fixes TestContext2Apply_outputOrphanModule
* Fix TestContext2Apply_plannedDestroyInterpolatedCount
* Update DestroyOutputTransformer and associated types to reflect PlannableOutputs
* Remove comment about locals
* Remove module count enablement
* Removes allowing count for modules, and reverts the test,
while adding a Skip()'d test that works when you re-enable
the config
* update TargetDownstream signature to match master
* remove unnecessary method
Co-authored-by: James Bardin <j.bardin@gmail.com>
Start by removing the DestroyEdge type altogether. This is only used to
detect the natural edge between a resource's create and destroy nodes,
but that's not necessary for any transformations. The custom edge type
also interferes with normal graph manipulations, because you can't
delete an arbitrary edge without knowing the type, so deletion of the
edge based only on the endpoints is often done incorrectly. The dag
package itself does this incorrectly in TransitiveReduction, which
always assumes the BasicEdge type.
Now that inter-resource destroy dependencies are already connected in the
DestroyEdgeTransformer (from the stored deps in state), there's no need
to search out all dependant resources in the CBD transformation, as they
should all be connected. This makes the CBD transformation rule quite
simple: reverse any edges from create nodes.
Since a planned destroy can no longer indicate it is a full destroy,
unused values were being left in the apply graph for evaluation. If
these values contains interpolations that can fail, (for example, a
zipmap with mismatched list sizes), it will cause the apply to abort.
The PrunUnusedValuesTransformer was only previously run during destroy,
more out of conservatism than for any other particular reason. Adapt it
to always remove unused values from the graph, with the exception being
the root module outputs, which must be retained when we don't have a
clear indication that a full destroy is being executed.
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.
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.
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.
Due to how often the state and plan types are referenced throughout
Terraform, there isn't a great way to switch them out gradually. As a
consequence, this huge commit gets us from the old world to a _compilable_
new world, but still has a large number of known test failures due to
key functionality being stubbed out.
The stubs here are for anything that interacts with providers, since we
now need to do the follow-up work to similarly replace the old
terraform.ResourceProvider interface with its replacement in the new
"providers" package. That work, along with work to fix the remaining
failing tests, will follow in subsequent commits.
The aim here was to replace all references to terraform.State and its
downstream types with states.State, terraform.Plan with plans.Plan,
state.State with statemgr.State, and switch to the new implementations of
the state and plan file formats. However, due to the number of times those
types are used, this also ended up affecting numerous other parts of core
such as terraform.Hook, the backend.Backend interface, and most of the CLI
commands.
Just as with 5861dbf3fc49b19587a31816eb06f511ab861bb4 before, I apologize
in advance to the person who inevitably just found this huge commit while
spelunking through the commit history.
The "config" package is no longer used and will be removed as part
of the 0.12 release cleanup. Since configschema is part of the
"new world" of configuration modelling, it makes more sense for
it to live as a subdirectory of the newer "configs" package.
Because we currently rely on the ReferenceTransformer to introduce the
necessary edges between local/output values and resource destroy nodes, we
must include the destroy phase of any resource we depend on in the
references of these.
This works in conjunction with the changes in the prior commit to restore
correct handling of dependencies for local and output values during
destroy.
With the current design, several seemingly-separate parts of the code must
all coincidentally agree with one another for destroy edges to be created
properly, which makes this code very hard to maintain. In future we should
refactor this so that ReferenceTransformer doesn't create edges for
destroy nodes at all, and have _all_ destroy edges (including
create_before_destroy) be dealt with in the single DestroyEdgeTransformer,
where they can be maintained and unit tested together.
Prior to the introduction of our "addrs" package, we represented destroy
nodes as a special kind of address string ending in ".destroy" or
".destroy-cbd".
Using references to resolve these dependencies is a strange idea to begin
with, since these are not user-visible addresses, but rather than refactor
that now we instead have these weird pseudo-address types ResourcePhase
and ResourceInstancePhase that correspond go those weird address suffixes,
thus restoring the prior behavior.
In future we should rework this so that destroy node edges are not handled
as references at all, and instead handled as part of
DestroyEdgeTransformer where there's better context for implementing this
logic and it can be maintained and tested in a single place.
We previously added a special case for dealing with references to
instances in the plan graph where there are only resource nodes. However,
this was too general a fix and so it upset the handling of graphs where
instances _are_ present.
Now we'll do that fallback behavior only if there is no instance node in
the graph already, so the exact matching behavior will be used in graphs
where the instances are present.
The provider schema cache is keyed by provider configuration address
rather than provider type, so we need to do the same inheritance logic
to resolve providers needed because of reference as we do for providers
needed for direct use.
This allows resources that override "provider" or resources in child
modules that have their own provider configurations to be associated
with the provider config they will eventually get schema from, rather
than (as before) always the default configuration for the provider in
the root module.
Eventually it'd probably be better to switch to using a provider cache
that is keyed by provider _type_ rather than provider config, but since
it's currently fetched by visiting the individual provider graph nodes
we currently visit each provider configuration separately and fetch a
schema for each.
References can't be connected directly to the instances, because the
resources are expanded when ReferenceTransformer is run. Lookup
references by the resource type.
Due to how deeply the configuration types go into Terraform Core, there
isn't a great way to switch out to HCL2 gradually. As a consequence, this
huge commit gets us from the old state to a _compilable_ new state, but
does not yet attempt to fix any tests and has a number of known missing
parts and bugs. We will continue to iterate on this in forthcoming
commits, heading back towards passing tests and making Terraform
fully-functional again.
The three main goals here are:
- Use the configuration models from the "configs" package instead of the
older models in the "config" package, which is now deprecated and
preserved only to help us write our migration tool.
- Do expression inspection and evaluation using the functionality of the
new "lang" package, instead of the Interpolator type and related
functionality in the main "terraform" package.
- Represent addresses of various objects using types in the addrs package,
rather than hand-constructed strings. This is not critical to support
the above, but was a big help during the implementation of these other
points since it made it much more explicit what kind of address is
expected in each context.
Since our new packages are built to accommodate some future planned
features that are not yet implemented (e.g. the "for_each" argument on
resources, "count"/"for_each" on modules), and since there's still a fair
amount of functionality still using old-style APIs, there is a moderate
amount of shimming here to connect new assumptions with old, hopefully in
a way that makes it easier to find and eliminate these shims later.
I apologize in advance to the person who inevitably just found this huge
commit while spelunking through the commit history.
