From 5222cc7fd8abfffee318dad9570a2a231ffc6a38 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 24 Jan 2024 11:16:01 -0800 Subject: [PATCH 1/5] terraform: Evaluator.StaticValidateReferences 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. --- internal/terraform/evaluate.go | 7 +++++ internal/terraform/evaluate_valid.go | 37 ++++++++++++----------- internal/terraform/evaluate_valid_test.go | 6 +--- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index f7bd5e9c81ed..8157b1d1ab9e 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -146,6 +146,13 @@ var EvalDataForNoInstanceKey = InstanceKeyEvalData{} // evaluationStateData must implement lang.Data var _ lang.Data = (*evaluationStateData)(nil) +// StaticValidateReferences calls [Evaluator.StaticValidateReferences] on +// the evaluator embedded in this data object, using this data object's +// static module path. +func (d *evaluationStateData) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { + return d.Evaluator.StaticValidateReferences(refs, d.ModulePath.Module(), self, source) +} + func (d *evaluationStateData) GetCountAttr(addr addrs.CountAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics switch addr.Name { diff --git a/internal/terraform/evaluate_valid.go b/internal/terraform/evaluate_valid.go index 1b0dc22a4c2a..50b122f9eb50 100644 --- a/internal/terraform/evaluate_valid.go +++ b/internal/terraform/evaluate_valid.go @@ -31,24 +31,27 @@ import ( // // The result may include warning diagnostics if, for example, deprecated // features are referenced. -func (d *evaluationStateData) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { +func (e *Evaluator) StaticValidateReferences(refs []*addrs.Reference, modAddr addrs.Module, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { var diags tfdiags.Diagnostics for _, ref := range refs { - moreDiags := d.staticValidateReference(ref, self, source) + moreDiags := e.StaticValidateReference(ref, modAddr, self, source) diags = diags.Append(moreDiags) } return diags } -func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { - modCfg := d.Evaluator.Config.DescendentForInstance(d.ModulePath) +func (e *Evaluator) StaticValidateReference(ref *addrs.Reference, modAddr addrs.Module, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { + modCfg := e.Config.Descendent(modAddr) if modCfg == nil { // This is a bug in the caller rather than a problem with the // reference, but rather than crashing out here in an unhelpful way // we'll just ignore it and trust a different layer to catch it. return nil } + return e.staticValidateReference(ref, modCfg, self, source) +} +func (e *Evaluator) staticValidateReference(ref *addrs.Reference, modCfg *configs.Config, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { if ref.Subject == addrs.Self { // The "self" address is a special alias for the address given as // our self parameter here, if present. @@ -80,20 +83,20 @@ func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self // staticValidateMultiResourceReference respectively. case addrs.Resource: var diags tfdiags.Diagnostics - diags = diags.Append(d.staticValidateSingleResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) - diags = diags.Append(d.staticValidateResourceReference(modCfg, addr, source, ref.Remaining, ref.SourceRange)) + diags = diags.Append(staticValidateSingleResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) + diags = diags.Append(staticValidateResourceReference(modCfg, addr, source, e.Plugins, ref.Remaining, ref.SourceRange)) return diags case addrs.ResourceInstance: var diags tfdiags.Diagnostics - diags = diags.Append(d.staticValidateMultiResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) - diags = diags.Append(d.staticValidateResourceReference(modCfg, addr.ContainingResource(), source, ref.Remaining, ref.SourceRange)) + diags = diags.Append(staticValidateMultiResourceReference(modCfg, addr, ref.Remaining, ref.SourceRange)) + diags = diags.Append(staticValidateResourceReference(modCfg, addr.ContainingResource(), source, e.Plugins, ref.Remaining, ref.SourceRange)) return diags // We also handle all module call references the same way, disregarding index. case addrs.ModuleCall: - return d.staticValidateModuleCallReference(modCfg, addr, ref.Remaining, ref.SourceRange) + return staticValidateModuleCallReference(modCfg, addr, ref.Remaining, ref.SourceRange) case addrs.ModuleCallInstance: - return d.staticValidateModuleCallReference(modCfg, addr.Call, ref.Remaining, ref.SourceRange) + return staticValidateModuleCallReference(modCfg, addr.Call, ref.Remaining, ref.SourceRange) case addrs.ModuleCallInstanceOutput: // This one is a funny one because we will take the output name referenced // and use it to fake up a "remaining" that would make sense for the @@ -109,7 +112,7 @@ func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self // but is close enough for our purposes. SrcRange: ref.SourceRange.ToHCL(), } - return d.staticValidateModuleCallReference(modCfg, addr.Call.Call, remain, ref.SourceRange) + return staticValidateModuleCallReference(modCfg, addr.Call.Call, remain, ref.SourceRange) default: // Anything else we'll just permit through without any static validation @@ -118,7 +121,7 @@ func (d *evaluationStateData) staticValidateReference(ref *addrs.Reference, self } } -func (d *evaluationStateData) staticValidateSingleResourceReference(modCfg *configs.Config, addr addrs.Resource, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { +func staticValidateSingleResourceReference(modCfg *configs.Config, addr addrs.Resource, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { // If we have at least one step in "remain" and this resource has // "count" set then we know for sure this in invalid because we have // something like: @@ -163,7 +166,7 @@ func (d *evaluationStateData) staticValidateSingleResourceReference(modCfg *conf return diags } -func (d *evaluationStateData) staticValidateMultiResourceReference(modCfg *configs.Config, addr addrs.ResourceInstance, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { +func staticValidateMultiResourceReference(modCfg *configs.Config, addr addrs.ResourceInstance, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { var diags tfdiags.Diagnostics cfg := modCfg.Module.ResourceByAddr(addr.ContainingResource()) @@ -175,7 +178,7 @@ func (d *evaluationStateData) staticValidateMultiResourceReference(modCfg *confi if addr.Key == addrs.NoKey { // This is a different path into staticValidateSingleResourceReference - return d.staticValidateSingleResourceReference(modCfg, addr.ContainingResource(), remain, rng) + return staticValidateSingleResourceReference(modCfg, addr.ContainingResource(), remain, rng) } else { if cfg.Count == nil && cfg.ForEach == nil { diags = diags.Append(&hcl.Diagnostic{ @@ -190,7 +193,7 @@ func (d *evaluationStateData) staticValidateMultiResourceReference(modCfg *confi return diags } -func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource, source addrs.Referenceable, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { +func staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource, source addrs.Referenceable, plugins *contextPlugins, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { var diags tfdiags.Diagnostics var modeAdjective string @@ -236,7 +239,7 @@ func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Co } providerFqn := modCfg.Module.ProviderForLocalConfig(cfg.ProviderConfigAddr()) - schema, _, err := d.Evaluator.Plugins.ResourceTypeSchema(providerFqn, addr.Mode, addr.Type) + schema, _, err := plugins.ResourceTypeSchema(providerFqn, addr.Mode, addr.Type) if err != nil { // Prior validation should've taken care of a schema lookup error, // so we should never get here but we'll handle it here anyway for @@ -286,7 +289,7 @@ func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Co return diags } -func (d *evaluationStateData) staticValidateModuleCallReference(modCfg *configs.Config, addr addrs.ModuleCall, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { +func staticValidateModuleCallReference(modCfg *configs.Config, addr addrs.ModuleCall, remain hcl.Traversal, rng tfdiags.SourceRange) tfdiags.Diagnostics { var diags tfdiags.Diagnostics // For now, our focus here is just in testing that the referenced module diff --git a/internal/terraform/evaluate_valid_test.go b/internal/terraform/evaluate_valid_test.go index c65e9aca03b9..778a5e674b44 100644 --- a/internal/terraform/evaluate_valid_test.go +++ b/internal/terraform/evaluate_valid_test.go @@ -135,11 +135,7 @@ For example, to correlate with indices of a referring resource, use: t.Fatal(diags.Err()) } - data := &evaluationStateData{ - Evaluator: evaluator, - } - - diags = data.StaticValidateReferences(refs, nil, test.Src) + diags = evaluator.StaticValidateReferences(refs, addrs.RootModule, nil, test.Src) if diags.HasErrors() { if test.WantErr == "" { t.Fatalf("Unexpected diagnostics: %s", diags.Err()) From d262d77f90729bc0cec7ffa938e1bae431563de9 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 24 Jan 2024 15:05:32 -0800 Subject: [PATCH 2/5] terraform: a lang.Data impl for unexpanded modules 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. --- internal/terraform/evaluate_placeholder.go | 275 +++++++++++++++++++++ 1 file changed, 275 insertions(+) create mode 100644 internal/terraform/evaluate_placeholder.go diff --git a/internal/terraform/evaluate_placeholder.go b/internal/terraform/evaluate_placeholder.go new file mode 100644 index 000000000000..70359bf6dccb --- /dev/null +++ b/internal/terraform/evaluate_placeholder.go @@ -0,0 +1,275 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "fmt" + + "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/lang" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// evaluationPlaceholderData is an implementation of lang.Data that deals +// with resolving references inside module prefixes whose full expansion +// isn't known yet, and thus returns placeholder values that represent +// only what we know to be true for all possible final module instances +// that could exist for the prefix. +type evaluationPlaceholderData struct { + Evaluator *Evaluator + + // ModulePath is the partially-expanded path through the dynamic module + // tree to a set of possible module instances that share a common known + // prefix. + ModulePath addrs.PartialExpandedModule + + // CountAvailable is true if this data object is representing an evaluation + // scope where the "count" symbol would be available. + CountAvailable bool + + // EachAvailable is true if this data object is representing an evaluation + // scope where the "each" symbol would be available. + EachAvailable bool + + // Operation records the type of walk the evaluationStateData is being used + // for. + Operation walkOperation +} + +// TODO: Historically we were inconsistent about whether static validation +// logic is implemented in Evaluator.StaticValidateReference or inline in +// methods of evaluationStateData, because the dedicated static validator +// came later. +// +// Some validation rules (and their associated error messages) have therefore +// ended up being duplicated between evaluationPlaceholderData and +// evaluationStateData. We've accepted that for now to avoid creating a bunch +// of churn in pre-existing code while adding support for partial expansion +// placeholders, but one day it would be nice to refactor this a little so +// that the division between these three units is a little clearer and so +// that all of the error checks are implemented in only one place each. + +var _ lang.Data = (*evaluationPlaceholderData)(nil) + +// GetCheckBlock implements lang.Data. +func (d *evaluationPlaceholderData) GetCheckBlock(addr addrs.Check, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + // check blocks don't produce any useful data and can only be referred + // to within an expect_failures attribute in the test language. + var diags tfdiags.Diagnostics + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to \"check\" in invalid context", + Detail: "The \"check\" object can only be used from an \"expect_failures\" attribute within a Terraform testing \"run\" block.", + Subject: rng.ToHCL().Ptr(), + }) + return cty.NilVal, diags + +} + +// GetCountAttr implements lang.Data. +func (d *evaluationPlaceholderData) GetCountAttr(addr addrs.CountAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + switch addr.Name { + + case "index": + if !d.CountAvailable { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Reference to "count" in non-counted context`, + Detail: `The "count" object can only be used in "module", "resource", and "data" blocks, and only when the "count" argument is set.`, + Subject: rng.ToHCL().Ptr(), + }) + } + // When we're under a partially-expanded prefix, the leaf instance + // keys are never known because otherwise we'd be under a fully-known + // prefix by definition. We do know it's always >= 0 and not null, + // though. + return cty.UnknownVal(cty.Number).Refine(). + NumberRangeLowerBound(cty.Zero, true). + NotNull(). + NewValue(), diags + + default: + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid "count" attribute`, + Detail: fmt.Sprintf(`The "count" object does not have an attribute named %q. The only supported attribute is count.index, which is the index of each instance of a resource block that has the "count" argument set.`, addr.Name), + Subject: rng.ToHCL().Ptr(), + }) + return cty.DynamicVal, diags + } +} + +// GetForEachAttr implements lang.Data. +func (d *evaluationPlaceholderData) GetForEachAttr(addr addrs.ForEachAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + // When we're under a partially-expanded prefix, the leaf instance + // keys are never known because otherwise we'd be under a fully-known + // prefix by definition. Therefore all return paths here produce unknown + // values. + + if !d.EachAvailable { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Reference to "each" in context without for_each`, + Detail: `The "each" object can be used only in "module" or "resource" blocks, and only when the "for_each" argument is set.`, + Subject: rng.ToHCL().Ptr(), + }) + return cty.UnknownVal(cty.DynamicPseudoType), diags + } + + switch addr.Name { + + case "key": + // each.key is always a string and is never null + return cty.UnknownVal(cty.String).RefineNotNull(), diags + case "value": + return cty.DynamicVal, diags + default: + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid "each" attribute`, + Detail: fmt.Sprintf(`The "each" object does not have an attribute named %q. The supported attributes are each.key and each.value, the current key and value pair of the "for_each" attribute set.`, addr.Name), + Subject: rng.ToHCL().Ptr(), + }) + return cty.DynamicVal, diags + } +} + +// GetInputVariable implements lang.Data. +func (d *evaluationPlaceholderData) GetInputVariable(addr addrs.InputVariable, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + namedVals := d.Evaluator.NamedValues + absAddr := addrs.ObjectInPartialExpandedModule(d.ModulePath, addr) + return namedVals.GetInputVariablePlaceholder(absAddr), nil +} + +// GetLocalValue implements lang.Data. +func (d *evaluationPlaceholderData) GetLocalValue(addr addrs.LocalValue, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + namedVals := d.Evaluator.NamedValues + absAddr := addrs.ObjectInPartialExpandedModule(d.ModulePath, addr) + return namedVals.GetLocalValuePlaceholder(absAddr), nil +} + +// GetModule implements lang.Data. +func (d *evaluationPlaceholderData) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + // We'll reuse the evaluator's "static evaluate" logic to check that the + // module call being referred to is even declared in the configuration, + // since it returns a good-quality error message for that case that + // we don't want to have to duplicate here. + diags := d.Evaluator.StaticValidateReference(&addrs.Reference{ + Subject: addr, + SourceRange: rng, + }, d.ModulePath.Module(), nil, nil) + if diags.HasErrors() { + return cty.DynamicVal, diags + } + + callerCfg := d.Evaluator.Config.Descendent(d.ModulePath.Module()) + if callerCfg == nil { + // Strange! The above StaticValidateReference should've failed if + // the module we're in isn't even declared. But we'll just tolerate + // it and return a very general placeholder. + return cty.DynamicVal, diags + } + callCfg := callerCfg.Module.ModuleCalls[addr.Name] + if callCfg == nil { + // Again strange, for the same reason as just above. + return cty.DynamicVal, diags + } + + // Any module call under an unexpanded prefix has an unknown set of instance + // keys itself by definition, unless that call isn't using count or for_each + // at all and thus we know it has exactly one "no-key" instance. + // + // If we don't know the instance keys then we cannot predict anything about + // the result, because module calls with repetition appear as either + // object or tuple types and we cannot predict those types here. + if callCfg.Count != nil || callCfg.ForEach != nil { + return cty.DynamicVal, diags + } + + // If we get down here then we know we have a single-instance module, and + // so we can return a more specific placeholder object that has all of + // the child module's declared output values represented, which could + // then potentially allow detecting a downstream error referring to + // an output value that doesn't actually exist. + calledCfg := d.Evaluator.Config.Descendent(d.ModulePath.Module().Child(addr.Name)) + if calledCfg == nil { + // This suggests that the config wasn't constructed correctly, since + // there should always be a child config node for any module call, + // but that's a "package configs" problem and so we'll just tolerate + // it here for robustness. + return cty.DynamicVal, diags + } + + attrs := make(map[string]cty.Value, len(calledCfg.Module.Outputs)) + for name := range calledCfg.Module.Outputs { + // Module output values are dynamically-typed, so we cannot + // predict anything about their results until finalized. + attrs[name] = cty.DynamicVal + } + return cty.ObjectVal(attrs), diags +} + +// GetOutput implements lang.Data. +func (d *evaluationPlaceholderData) GetOutput(addr addrs.OutputValue, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + namedVals := d.Evaluator.NamedValues + absAddr := addrs.ObjectInPartialExpandedModule(d.ModulePath, addr) + return namedVals.GetOutputValuePlaceholder(absAddr), nil + +} + +// GetPathAttr implements lang.Data. +func (d *evaluationPlaceholderData) GetPathAttr(addrs.PathAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + // TODO: It would be helpful to perform the same logic here as we do + // in the full-evaluation case, since the paths we'd return here cannot + // vary based on dynamic data, but we'll need to factor out the logic + // into a common location we can call from both places first. For now, + // we'll just leave these all as unknown value placeholders. + // + // What we _do_ know is that all valid attributes of "path" are strings + // that are definitely not null, so we can at least catch situations + // where someone tries to use them in a place where a string is + // unacceptable. + return cty.UnknownVal(cty.String).RefineNotNull(), nil +} + +// GetResource implements lang.Data. +func (d *evaluationPlaceholderData) GetResource(addrs.Resource, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + // TODO: Once we've implemented the evaluation of placeholders for + // deferred resources during the graph walk, we should return such + // placeholders here where possible. + // + // However, for resources that use count or for_each we'd not be able + // to predict anything more than cty.DynamicVal here anyway, since + // we don't know the instance keys, and so that improvement would only + // really help references to single-instance resources. + return cty.DynamicVal, nil +} + +// GetRunBlock implements lang.Data. +func (d *evaluationPlaceholderData) GetRunBlock(addrs.Run, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + // We should not get here because any scope that has an [evaluationPlaceholderData] + // as its Data should have a reference parser that doesn't accept addrs.Run + // addresses. + panic("GetRunBlock called on non-test evaluation dataset") +} + +// GetTerraformAttr implements lang.Data. +func (d *evaluationPlaceholderData) GetTerraformAttr(addrs.TerraformAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { + // TODO: It would be helpful to perform the same validation checks that + // occur in evaluationStateData.GetTerraformAttr, so authors can catch + // invalid usage of the "terraform" object even when under an unexpanded + // module prefix. + return cty.DynamicVal, nil +} + +// StaticValidateReferences implements lang.Data. +func (d *evaluationPlaceholderData) StaticValidateReferences(refs []*addrs.Reference, self addrs.Referenceable, source addrs.Referenceable) tfdiags.Diagnostics { + return d.Evaluator.StaticValidateReferences(refs, d.ModulePath.Module(), self, source) +} From 027b67d565b6272a87850412d7b7d71373b556ac Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 24 Jan 2024 16:40:06 -0800 Subject: [PATCH 3/5] terraform: EvalContextBuiltin supports partial-expanded module instances 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. --- internal/terraform/eval_context.go | 4 + internal/terraform/eval_context_builtin.go | 104 +++++++++++++-------- internal/terraform/eval_context_mock.go | 8 ++ internal/terraform/eval_context_scope.go | 61 ++++++++++++ 4 files changed, 138 insertions(+), 39 deletions(-) create mode 100644 internal/terraform/eval_context_scope.go diff --git a/internal/terraform/eval_context.go b/internal/terraform/eval_context.go index 5ab46f79a4d2..47dbbf60d880 100644 --- a/internal/terraform/eval_context.go +++ b/internal/terraform/eval_context.go @@ -193,4 +193,8 @@ type EvalContext interface { // WithPath returns a copy of the context with the internal path set to the // path argument. WithPath(path addrs.ModuleInstance) EvalContext + + // WithPartialExpandedPath returns a copy of the context with the internal + // path set to the path argument. + WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext } diff --git a/internal/terraform/eval_context_builtin.go b/internal/terraform/eval_context_builtin.go index 08d59384a425..ac696a514e86 100644 --- a/internal/terraform/eval_context_builtin.go +++ b/internal/terraform/eval_context_builtin.go @@ -34,19 +34,17 @@ import ( // BuiltinEvalContext is an EvalContext implementation that is used by // Terraform by default. type BuiltinEvalContext struct { + // scope is the scope (module instance or set of possible module instances) + // that this context is operating within. + // + // Note: this can be evalContextGlobal (i.e. nil) when visiting a graph + // node that doesn't belong to a particular module, in which case any + // method using it will panic. + scope evalContextScope + // StopContext is the context used to track whether we're complete StopContext context.Context - // PathValue is the Path that this context is operating within. - PathValue addrs.ModuleInstance - - // pathSet indicates that this context was explicitly created for a - // specific path, and can be safely used for evaluation. This lets us - // differentiate between PathValue being unset, and the zero value which is - // equivalent to RootModuleInstance. Path and Evaluation methods will - // panic if this is not set. - pathSet bool - // Evaluator is used for evaluating expressions within the scope of this // eval context. Evaluator *Evaluator @@ -90,8 +88,17 @@ var _ EvalContext = (*BuiltinEvalContext)(nil) func (ctx *BuiltinEvalContext) WithPath(path addrs.ModuleInstance) EvalContext { newCtx := *ctx - newCtx.pathSet = true - newCtx.PathValue = path + newCtx.scope = evalContextModuleInstance{ + Addr: path, + } + return &newCtx +} + +func (ctx *BuiltinEvalContext) WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext { + newCtx := *ctx + newCtx.scope = evalContextPartialExpandedModule{ + Addr: path, + } return &newCtx } @@ -436,28 +443,45 @@ func (ctx *BuiltinEvalContext) EvaluateReplaceTriggeredBy(expr hcl.Expression, r } func (ctx *BuiltinEvalContext) EvaluationScope(self addrs.Referenceable, source addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope { - if !ctx.pathSet { - panic("context path not set") - } - data := &evaluationStateData{ - Evaluator: ctx.Evaluator, - ModulePath: ctx.PathValue, - InstanceKeyData: keyData, - Operation: ctx.Evaluator.Operation, + switch scope := ctx.scope.(type) { + case evalContextModuleInstance: + data := &evaluationStateData{ + Evaluator: ctx.Evaluator, + ModulePath: scope.Addr, + InstanceKeyData: keyData, + Operation: ctx.Evaluator.Operation, + } + evalScope := ctx.Evaluator.Scope(data, self, source, ctx.evaluationExternalFunctions()) + + // ctx.PathValue is the path of the module that contains whatever + // expression the caller will be trying to evaluate, so this will + // activate only the experiments from that particular module, to + // be consistent with how experiment checking in the "configs" + // package itself works. The nil check here is for robustness in + // incompletely-mocked testing situations; mc should never be nil in + // real situations. + if mc := ctx.Evaluator.Config.DescendentForInstance(scope.Addr); mc != nil { + evalScope.SetActiveExperiments(mc.Module.ActiveExperiments) + } + return evalScope + case evalContextPartialExpandedModule: + data := &evaluationPlaceholderData{ + Evaluator: ctx.Evaluator, + ModulePath: scope.Addr, + CountAvailable: keyData.CountIndex != cty.NilVal, + EachAvailable: keyData.EachKey != cty.NilVal, + Operation: ctx.Evaluator.Operation, + } + evalScope := ctx.Evaluator.Scope(data, self, source, ctx.evaluationExternalFunctions()) + if mc := ctx.Evaluator.Config.Descendent(scope.Addr.Module()); mc != nil { + evalScope.SetActiveExperiments(mc.Module.ActiveExperiments) + } + return evalScope + default: + // This method is valid only for module-scoped EvalContext objects. + panic("no evaluation scope available: not in module context") } - scope := ctx.Evaluator.Scope(data, self, source, ctx.evaluationExternalFunctions()) - // ctx.PathValue is the path of the module that contains whatever - // expression the caller will be trying to evaluate, so this will - // activate only the experiments from that particular module, to - // be consistent with how experiment checking in the "configs" - // package itself works. The nil check here is for robustness in - // incompletely-mocked testing situations; mc should never be nil in - // real situations. - if mc := ctx.Evaluator.Config.DescendentForInstance(ctx.PathValue); mc != nil { - scope.SetActiveExperiments(mc.Module.ActiveExperiments) - } - return scope } // evaluationExternalFunctions is a helper for method EvaluationScope which @@ -531,10 +555,10 @@ func (ctx *BuiltinEvalContext) functionProvider(addr addrs.Provider) (providers. } func (ctx *BuiltinEvalContext) Path() addrs.ModuleInstance { - if !ctx.pathSet { - panic("context path not set") + if scope, ok := ctx.scope.(evalContextModuleInstance); ok { + return scope.Addr } - return ctx.PathValue + panic("not evaluating in the scope of a fully-expanded module") } func (ctx *BuiltinEvalContext) LanguageExperimentActive(experiment experiments.Experiment) bool { @@ -543,12 +567,14 @@ func (ctx *BuiltinEvalContext) LanguageExperimentActive(experiment experiments.E // if the context isn't fully populated. return false } - if !ctx.pathSet { - // An EvalContext that isn't associated with a module path cannot - // have active experiments. + scope := ctx.scope + if scope == evalContextGlobal { + // If we're not associated with a specific module then there can't + // be any language experiments in play, because experiment activation + // is module-scoped. return false } - cfg := ctx.Evaluator.Config.DescendentForInstance(ctx.Path()) + cfg := ctx.Evaluator.Config.Descendent(scope.evalContextScopeModule()) if cfg == nil { return false } diff --git a/internal/terraform/eval_context_mock.go b/internal/terraform/eval_context_mock.go index 89f31b5dd9b9..d96b89fc7360 100644 --- a/internal/terraform/eval_context_mock.go +++ b/internal/terraform/eval_context_mock.go @@ -332,6 +332,14 @@ func (c *MockEvalContext) WithPath(path addrs.ModuleInstance) EvalContext { return &newC } +func (c *MockEvalContext) WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext { + // This is not yet implemented as a mock, because we've not yet had any + // need for it. If we end up needing to test this behavior in isolation + // somewhere then we'll need to figure out how to fit it in here without + // upsetting too many existing tests that rely on the PathPath field. + panic("WithPartialExpandedPath not implemented for MockEvalContext") +} + func (c *MockEvalContext) Path() addrs.ModuleInstance { c.PathCalled = true return c.PathPath diff --git a/internal/terraform/eval_context_scope.go b/internal/terraform/eval_context_scope.go new file mode 100644 index 000000000000..084835db7c2c --- /dev/null +++ b/internal/terraform/eval_context_scope.go @@ -0,0 +1,61 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "github.com/hashicorp/terraform/internal/addrs" +) + +// evalContextScope represents the scope that an [EvalContext] (or rather, +// an [EvalContextBuiltin] is associated with. +// +// This is a closed interface representing a sum type, with three possible +// variants: +// +// - a nil value of this type represents a "global" evaluation context used +// for graph nodes that aren't considered to belong to any specific module +// instance. Some [EvalContext] methods are not appropriate for such a +// context, and so will panic on a global evaluation context. +// - [evalContextModuleInstance] is for an evaluation context used for +// graph nodes that implement [GraphNodeModuleInstance], meaning that +// they belong to a fully-expanded single module instance. +// - [evalContextPartialExpandedModule] is for an evaluation context used for +// graph nodes that implement [GraphNodeUnexpandedModule], meaning that +// they belong to an unbounded set of possible module instances sharing +// a common known prefix, in situations where a module call has an unknown +// value for its count or for_each argument. +type evalContextScope interface { + // evalContextScopeModule returns the static module address of whatever + // fully- or partially-expanded module instance address this scope is + // associated with. + // + // A "global" evaluation context is a nil [evalContextScope], and so + // this method will panic for that scope. + evalContextScopeModule() addrs.Module +} + +// evalContextGlobal is the nil [evalContextScope] used to represent an +// [EvalContext] that isn't associated with any module at all. +var evalContextGlobal evalContextScope + +// evalContextModuleInstance is an [evalContextScope] associated with a +// fully-expanded single module instance. +type evalContextModuleInstance struct { + Addr addrs.ModuleInstance +} + +func (s evalContextModuleInstance) evalContextScopeModule() addrs.Module { + return s.Addr.Module() +} + +// evalContextPartialExpandedModule is an [evalContextScope] associated with +// an unbounded set of possible module instances that share a common known +// address prefix. +type evalContextPartialExpandedModule struct { + Addr addrs.PartialExpandedModule +} + +func (s evalContextPartialExpandedModule) evalContextScopeModule() addrs.Module { + return s.Addr.Module() +} From 8bdb8d39ffd65adcdffdf3dc28592a01618737e9 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 24 Jan 2024 16:47:32 -0800 Subject: [PATCH 4/5] terraform: GraphWalker interface supports partial-expanded module addrs 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. --- internal/terraform/graph_walk.go | 12 +++++++++--- internal/terraform/graph_walk_context.go | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/internal/terraform/graph_walk.go b/internal/terraform/graph_walk.go index 54f5c674d24e..014ad1b36625 100644 --- a/internal/terraform/graph_walk.go +++ b/internal/terraform/graph_walk.go @@ -14,6 +14,8 @@ type GraphWalker interface { EvalContext() EvalContext EnterPath(addrs.ModuleInstance) EvalContext ExitPath(addrs.ModuleInstance) + EnterPartialExpandedPath(addrs.PartialExpandedModule) EvalContext + ExitPartialExpandedPath(addrs.PartialExpandedModule) Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics } @@ -22,7 +24,11 @@ type GraphWalker interface { // implementing all the required functions. type NullGraphWalker struct{} -func (NullGraphWalker) EvalContext() EvalContext { return new(MockEvalContext) } -func (NullGraphWalker) EnterPath(addrs.ModuleInstance) EvalContext { return new(MockEvalContext) } -func (NullGraphWalker) ExitPath(addrs.ModuleInstance) {} +func (NullGraphWalker) EvalContext() EvalContext { return new(MockEvalContext) } +func (NullGraphWalker) EnterPath(addrs.ModuleInstance) EvalContext { return new(MockEvalContext) } +func (NullGraphWalker) ExitPath(addrs.ModuleInstance) {} +func (NullGraphWalker) EnterPartialExpandedPath(addrs.PartialExpandedModule) EvalContext { + return new(MockEvalContext) +} +func (NullGraphWalker) ExitPartialExpandedPath(addrs.PartialExpandedModule) {} func (NullGraphWalker) Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics { return nil } diff --git a/internal/terraform/graph_walk_context.go b/internal/terraform/graph_walk_context.go index d55ad4d99d7f..69150610149a 100644 --- a/internal/terraform/graph_walk_context.go +++ b/internal/terraform/graph_walk_context.go @@ -63,6 +63,8 @@ type ContextGraphWalker struct { provisionerLock sync.Mutex } +var _ GraphWalker = (*ContextGraphWalker)(nil) + func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext { w.contextLock.Lock() defer w.contextLock.Unlock() @@ -78,6 +80,21 @@ func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext { return ctx } +func (w *ContextGraphWalker) EnterPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext { + w.contextLock.Lock() + defer w.contextLock.Unlock() + + // If we already have a context for this path cached, use that + key := path.String() + if ctx, ok := w.contexts[key]; ok { + return ctx + } + + ctx := w.EvalContext().WithPartialExpandedPath(path) + w.contexts[key] = ctx.(*BuiltinEvalContext) + return ctx +} + func (w *ContextGraphWalker) EvalContext() EvalContext { w.once.Do(w.init) From 9d4f44da9b52be3625068eaeaca2104f1c26809a Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Wed, 24 Jan 2024 18:17:27 -0800 Subject: [PATCH 5/5] terraform: Graph nodes can belong to partial-expanded module prefixes 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. --- internal/terraform/graph.go | 31 +++++++++++++------ .../terraform/graph_interface_subgraph.go | 18 +++++++++++ internal/terraform/node_local.go | 4 +++ internal/terraform/node_module_variable.go | 4 +++ internal/terraform/node_output.go | 4 +++ 5 files changed, 52 insertions(+), 9 deletions(-) diff --git a/internal/terraform/graph.go b/internal/terraform/graph.go index 559c44927718..e139f4533418 100644 --- a/internal/terraform/graph.go +++ b/internal/terraform/graph.go @@ -67,21 +67,34 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics { } }() - // vertexCtx is the context that we use when evaluating. This - // is normally the context of our graph but can be overridden - // with a GraphNodeModuleInstance impl. - vertexCtx := ctx - if pn, ok := v.(GraphNodeModuleInstance); ok { - vertexCtx = walker.EnterPath(pn.Path()) - defer walker.ExitPath(pn.Path()) - } - if g.checkAndApplyOverrides(ctx.Overrides(), v) { // We can skip whole vertices if they are in a module that has been // overridden. + log.Printf("[TRACE] vertex %q: overridden by a test double, so skipping", dag.VertexName(v)) return } + // vertexCtx is the context that we use when evaluating. This + // is normally the global context but can be overridden + // with a GraphNodeModuleInstance or GraphNodePartialExpandedModule + // impl. (The two interfaces are intentionally mutually-exclusive by + // both having the same method name but with different signatures, + // since a node can't belong to two different contexts at once.) + vertexCtx := ctx + if pn, ok := v.(GraphNodeModuleInstance); ok { + moduleAddr := pn.Path() // An addrs.ModuleInstance + log.Printf("[TRACE] vertex %q: belongs to %s", dag.VertexName(v), moduleAddr) + vertexCtx = walker.EnterPath(moduleAddr) + defer walker.ExitPath(pn.Path()) + } else if pn, ok := v.(GraphNodePartialExpandedModule); ok { + moduleAddr := pn.Path() // An addrs.PartialExpandedModule + log.Printf("[TRACE] vertex %q: belongs to all of %s", dag.VertexName(v), moduleAddr) + vertexCtx = walker.EnterPartialExpandedPath(pn.Path()) + defer walker.ExitPartialExpandedPath(pn.Path()) + } else { + log.Printf("[TRACE] vertex %q: does not belong to any module instance", dag.VertexName(v)) + } + // If the node is exec-able, then execute it. if ev, ok := v.(GraphNodeExecutable); ok { diags = diags.Append(walker.Execute(vertexCtx, ev)) diff --git a/internal/terraform/graph_interface_subgraph.go b/internal/terraform/graph_interface_subgraph.go index 2e013aeca0fa..f70fdeb84bd8 100644 --- a/internal/terraform/graph_interface_subgraph.go +++ b/internal/terraform/graph_interface_subgraph.go @@ -18,3 +18,21 @@ type GraphNodeModuleInstance interface { type GraphNodeModulePath interface { ModulePath() addrs.Module } + +// GraphNodePartialExpandedModule says that a node represents an unbounded +// set of objects within an unbounded set of module instances that happen +// to share a known address prefix. +// +// Nodes of this type typically produce placeholder data to support partial +// evaluation despite the full analysis of a module being deferred to a future +// plan when more information will be available. They might also perform +// checks and raise errors when something can be proven to be definitely +// invalid regardless of what the final set of module instances turns out to +// be. +// +// Node types implementing this interface cannot also implement +// [GraphNodeModuleInstance], because it is not possible to evaluate a +// node in two different contexts at once. +type GraphNodePartialExpandedModule interface { + Path() addrs.PartialExpandedModule +} diff --git a/internal/terraform/node_local.go b/internal/terraform/node_local.go index defd5fc10a21..067878e534a6 100644 --- a/internal/terraform/node_local.go +++ b/internal/terraform/node_local.go @@ -201,4 +201,8 @@ type nodeLocalInPartialModule struct { Config *configs.Local } +func (n *nodeLocalInPartialModule) Path() addrs.PartialExpandedModule { + return n.Addr.Module +} + // TODO: Implement nodeLocalUnexpandedPlaceholder.Execute diff --git a/internal/terraform/node_module_variable.go b/internal/terraform/node_module_variable.go index 93c23a562a3b..11a053e77224 100644 --- a/internal/terraform/node_module_variable.go +++ b/internal/terraform/node_module_variable.go @@ -323,4 +323,8 @@ type nodeModuleVariableInPartialModule struct { DestroyApply bool } +func (n *nodeModuleVariableInPartialModule) Path() addrs.PartialExpandedModule { + return n.Addr.Module +} + // TODO: Implement nodeModuleVariableInPartialModule.Execute diff --git a/internal/terraform/node_output.go b/internal/terraform/node_output.go index bfa53d1fa6cb..3be1fceeee9e 100644 --- a/internal/terraform/node_output.go +++ b/internal/terraform/node_output.go @@ -464,6 +464,10 @@ type nodeOutputInPartialModule struct { RefreshOnly bool } +func (n *nodeOutputInPartialModule) Path() addrs.PartialExpandedModule { + return n.Addr.Module +} + // TODO: Implement nodeOutputInPartialModule.Execute // NodeDestroyableOutput represents an output that is "destroyable":