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.
During a full destroy when outputs are removed, the
NodeDestroyableOutput was preventing it's sibling output from being
destroyed. Prune the output node if it only has its destroy node as a
dependent.
The destroy output test is simply run a second time with no state, which
would cause the output interpolation to fail if it remained in the
graph.
Since outputs and local nodes are always evaluated, if the reference a
resource form the configuration that isn't in the state, the
interpolation could fail.
Prune any local or output values that have no references in the graph.
Now that outputs are always evaluated, we still need a way to remove
them from state when they are destroyed.
Previously, outputs were removed during destroy from the same
"Applyable" node type that evaluates them. Now that we need to possibly
both evaluate and remove output during an apply, we add a new node -
NodeDestroyableOutput.
This new node is added to the graph by the DestroyOutputTransformer,
which make the new destroy node depend on all descendants of the output
node. This ensures that the output remains in the state as long as
everything which may interpolate the output still exists.
Always evaluate outputs during destroy, just like we did for locals.
This breaks existing tests, which we will handle separately.
Don't reverse output/local node evaluation order during destroy, as they
are both being evaluated.
Destroy-time provisioners require us to re-evaluate during destroy.
Rather than destroying local values, which doesn't do much since they
aren't persisted to state, we always evaluate them regardless of the
type of apply. Since the destroy-time local node is no longer a
"destroy" operation, the order of evaluation need to be reversed. Take
the existing DestroyValueReferenceTransformer and change it to reverse
the outgoing edges, rather than in incoming edges. This makes it so that
any dependencies of a local or output node are destroyed after
evaluation.
Having locals evaluated during destroy failed one other test, but that
was the odd case where we need `id` to exist as an attribute as well as
a field.
There was a bug where all references would be discarded in the case when
a self-reference was encountered. Since a module references all
descendants by it's own path, it returns a self-reference by definition.
This turned out to be a big messy commit, since the way providers are
referenced is tightly coupled throughout the code. That starts to unify
how providers are referenced, using the format output node Name method.
Add a new field to the internal resource data types called
ResolvedProvider. This is set by a new setter method SetProvider when a
resource is connected to a provider during graph creation. This allows
us to later lookup the provider instance a resource is connected to,
without requiring it to have the same module path.
The InitProvider context method now takes 2 arguments, one if the
provider type and the second is the full name of the provider. While the
provider type could still be parsed from the full name, this makes it
more explicit and, and changes to the name format won't effect this
code.
DestroyValueReferenceTransformer is used during destroy to reverse the
edges for output and local values. Because destruction is going to
remove these from the state, nodes that depend on their value need to be
visited first.
A local value is similar to an output in that it exists only within state
and just always evaluates its value as best it can with the current state.
Therefore it has a single graph node type for all walks, which will
deal with that evaluation operation.
