Our diagnostics model allows for optionally annotating an error or warning
with information about the expression and eval context it was generated
from, which the diagnostic renderer for the UI will then use to give the
user some additional hints about what values may have contributed to the
error.
We previously didn't have those annotations on the results of evaluating
for_each expressions though, because in that case we were using the helper
function to evaluate an expression in one shot and thus we didn't ever
have a reference to the EvalContext in order to include it in the
diagnostic values.
Now, at the expense of having to handle the evaluation at a slightly lower
level of abstraction, we'll annotate all of the for_each error messages
with source expression information. This is valuable because we see users
often confused as to how their complex for_each expressions ended up being
invalid, and hopefully giving some information about what the inputs were
will allow more users to self-solve.
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.
We'll need this again for getting the transitive depends_on references
from parent module calls. This is needed to inform us how to handle data
sources during refresh and plan.
simplification allows us to settle on a single interface,
graphNodeExpandsInstances for all types if instance expanders. The only
other specific class of resource we need to detect during pruning is the
nodeExpandApplyableResource node, which is already classified under the
GraphNodeResourceInstance interface.
ModulePath was incorrectly returning the parent module, because it did
not implement ReferenceOutside. With ReferenceOutside working correctly,
we can have ModulePath return the real path and remove the special case
for this during pruning.
Create a single transformer to remove all unused nodes from the apply
graph. This is similar to the combination of the resource pruning done
in the destroy edge transformer, and the unused values transformer. In
addition to resources, variables, locals, and outputs, we now need to
remove unused module expansion nodes as well. Since these can all be
interdependent, we need to process them as whole in a single
transformation.
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.
Connect references from depends_on in modules calls. This will "just
work" for a lot of cases, but data sources will be read too early in the
case where they require the dependencies to be created. While
data sources will be properly ordered behind the module head node, there
is nothing preventing them from being being evaluated during refresh.
That name tag was left in only to reduce the diff when during
implementation. Fix the naming now for these nodes so it is correct, and
prevent any possible name collision between types.
Stop evaluating count and for each if they aren't set in the config.
Remove "Resource" from the function names, as they are also now used
with modules.
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.
This is all that is required to make module reference ordering work
during apply, by adding and edge to the nodeCloseModule node, which will
be the last node evaluated in the module.
We cannot evaluate expansion during validation, since the values may not
be known at that time.
Inject a nodeValidateModule, using the "Concrete" pattern used for other
node types during graph building. This node will always evaluate to a
single module instance, so that we have a valid context within which to
evaluate all sub resources.
Make the expansion logic easier to follow, keeping the evaluation and
registration local to switch cases. We don't validate anything between
count or for_each (config loading should handle that), and we don't need
to keep relying on the count == -1 sentinel value.
Replace the graphNodeRoot for the main graph with a nodeCloseModule for
the root module. USe a new transformer as well, so as to not change any
behavior of DynamicExpand graphs.
Closing out the root module like we do with sub modules means we no
longer need the OrphanResourceTransformer, or the NodeDestroyResource.
The old resource destroy logic has mostly moved into the instance nodes,
and the remaining resource node was just for cleanup, which need to be
done again by the module since there isn't always a NodeDestroyResource
to be evaluated.
The more-correct state caused a few tests to fail, which need to be
cleaned up to match the state without empty resource husks.
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.
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.
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>
This is a minimal integration of instances.Expander used just for resource
count and for_each, for now just forcing modules to always be singletons
because the rest of Terraform Core isn't ready to deal with expanding
module calls yet.
This doesn't integrate super cleanly yet because we still have some
cleanup work to do in the design of the plan walk, to make it explicit
that the nodes in the plan graph represent static configuration objects
rather than expanded instances, including for modules. To make this work
in the meantime, there is some shimming between addrs.Module and
addrs.ModuleInstance to correct for the discontinuities that result from
the fact that Terraform currently assumes that modules are always
singletons.
