core: Plumbing for graph nodes that belong to partially-expanded module prefixes #34571
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We previously had the StaticValidateReferences method implemented on the evaluationStateData type directly, but really the only information it needs directly from that object is the static module path, with everything else just coming from the wrapped Evaluator. That means that we can instead implement this as a method of Evaluator that is thinly wrapped by the prior evaluationStateData method. The guts of that method are functions that don't really depend on either of the two objects as a whole, so thus further turns them into just plain unexported functions that take the data they need as arguments. This commit doesn't achieve anything other than some light refactoring, but it will become more important in a subsequent commit that will add an additional implementation of lang.Data for evaluating references in expressions beneath an incompletely-expanded module. Both of those implementations can safely share the same implementation of StaticValidateReference though, since that method doesn't consider any dynamic expansion by definition anyway.
apparentlymart
added
enhancement
core
unknown-values
apparentlymart
changed the title
When we're evaluating expressions inside a module that's beneath a partially-expanded prefix, we need to use placeholder values that describe only what we know to be true for all possible instances within that prefix, which requires quite a different reference evaluation strategy than we'd use in a fully-expanded module instance. evaluationPlaceholderData will therefore substitute for evaluationStateData in such cases. It is associated with an unbounded set of possible module instances sharing a known module instance prefix, and uses placeholder data generated by special graph nodes representing unexpanded objects, instead of finalized data coming from the main state or plan. As of this commit, the new type is not used at all. In subsequent commits we'll teach terraform.EvalContextBuiltin to support working under unexpanded module prefixes, and then this new lang.Data type will be part of the implementation of its expression-evaluation methods in that case.
Previously an EvalContextBuiltin could be associated either with no module at all or with a fully-qualified module instance, depending on whether each graph node implements the interface required to announce association with a particular module instance. For supporting partial-expanded module prefixes we now need a third mode where the context is associated with an addrs.PartialExpandedModule address, which conceptually represents an unbounded set of possible module instances that share a common known address prefix. The new type evalContextScope serves as a sum type which represents these three different situations. A nil value of that interface type represents no scope at all, whereas the two implementations of that type represent the two different possible scope types. Both scope types can produce an addrs.Module that they represent, allowing some degree of shared code between the two cases as long as no full instance addresses are required. The most important part of this change is that method EvaluationScope now uses a different implementation of lang.Data when the context scope is a partial-expanded module address. That implementation only returns placeholders that approximate what we expect all instances of the affected modules to have in common, in the hope of still catching some downstream errors even though full evaluation is being deferred to a future run. We currently have no means for a graph node to announce itself as belonging to a partially-expanded module address, and so in practice this new mode is not yet reachable. Future commits will teach the graph walker to be able to recognize and handle graph nodes that need this new treatment, making their DynamicExpand/Execute methods use this new scope type.
Nothing calls this yet, because we don't yet have an interface for a graph node to declare that it belongs to a partially-evaluated module prefix address. That will follow in later commits.
The new interface GraphNodePartialExpandedModule is a mutually-exclusive alternative to GraphNodeModuleInstance for graph nodes that represent the unbounded set of as-yet-unknown instances sharing a common known module prefix. For nodes that implement this interface, their Execute and DynamicExpand methods will receive an EvalContext that is configured to perform expression evaluation using safe placeholder values instead of using the real state and plan, since (once implemented in future commits) the graph nodes of this sort will be used for situations where we're deferring part of the plan until we have more information to use while planning.
apparentlymart
force-pushed
the
f-deferred-actions-placeholder-eval
branch
from
75e1fde to
9d4f44d
Compare
alisdair
approved these changes
Jan 29, 2024
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
This was referenced Feb 2, 2024
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a further iteration towards #30937, and follows on from yesterday's #34561.
This particular PR deals with plumbing in the evaluation nuts-and-bolts for graph nodes that are representing unbounded sets of as-yet-unknown instances of objects that are declared beneath a module call that has an insufficiently-known
countorfor_eachargument.This has a few different parts:
A new implementation of
lang.Datathat's designed to return placeholder values that should represent what all instances of a given object have in common, while using unknown values for the parts where they differ.In this initial round it's quite meagre and returns
cty.DynamicValin some situations where it could technically be more precise than that, but we can improve the precision later to give earlier feedback about problems that would definitely cause a failure on a subsequent plan.EvalContext(or rather the concreteEvalContextBuiltinimplementation of it) now has a third mode for evaluating in a partially-unknown module prefix, in addition to its existing modes for not being in a module at all or being in a fully-expanded module instance.I modeled this new tristate situation by replacing the previous
PathValueandpathSetfields with a single fieldscopeof an interface type, where that interface type is acting as a closed sum type covering the three different variants. The small amount of logic inEvalContextBuiltinthat varies based on the module path then uses type assertions and type switches to distinguish between the three cases.The "context graph walker" now knows of a new extension interface for graph nodes which announces that a node belongs to a partial-expanded module prefix. For graph nodes that implement that interface, their
ExecuteandEvalContextmethods get anEvalContextin the partially-unknown module prefix mode.I intentionally chose the same method name for the new interface as for the existing interface used for
ModuleInstanceaffinity, but with a different signature, to statically represent that these two interfaces are mutually-exclusive: a graph node cannot be evaluated in two different contexts at once.For good measure I also retrofitted the three stubby node types added previously to represent partial-expanded named values with implementations of the new interface, so once they have
Executeimplemented they'll get a suitably-configuredEvalContext. However, the actualExecuteimplementations are not included here, and will follow in future commits.Reviewers might prefer to take this on a commit-by-commit basis; the series of commits follows roughly the sequence of changes I described above.
Therefore this PR is just another nothingburger as far as functionality is concerned -- no externally-visible behavior should be any different than it was before -- but this is an internal milestone where we now have all of the shared plumbing implemented, so future PRs can implement the language-feature-specific evaluation logic in terms of this. Again, there are no new integration tests here because there isn't yet any real new behavior to test, but those too will follow along with the language-feature-specific implementations in later PRs.