Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

terraform: Further generalization of the EvalContext scope handling #34614

Merged
merged 1 commit into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/terraform/context_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,6 @@ func evalScopeFromGraphWalk(walker *ContextGraphWalker, moduleAddr addrs.ModuleI
// just to get hold of an EvalContext for it. ContextGraphWalker
// caches its contexts, so we should get hold of the context that was
// previously used for evaluation here, unless we skipped walking.
evalCtx := walker.EnterPath(moduleAddr)
evalCtx := walker.enterScope(evalContextModuleInstance{Addr: moduleAddr})
return evalCtx.EvaluationScope(nil, nil, EvalDataForNoInstanceKey)
}
14 changes: 8 additions & 6 deletions internal/terraform/eval_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,13 @@ type EvalContext interface {
// this execution.
Overrides() *mocking.Overrides

// WithPath returns a copy of the context with the internal path set to the
// path argument.
WithPath(path addrs.ModuleInstance) EvalContext
// withScope derives a new EvalContext that has all of the same global
// context, but a new evaluation scope.
withScope(scope evalContextScope) EvalContext
}

// WithPartialExpandedPath returns a copy of the context with the internal
// path set to the path argument.
WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext
func evalContextForModuleInstance(baseCtx EvalContext, addr addrs.ModuleInstance) EvalContext {
return baseCtx.withScope(evalContextModuleInstance{
Addr: addr,
})
}
14 changes: 2 additions & 12 deletions internal/terraform/eval_context_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,9 @@ type BuiltinEvalContext struct {
// BuiltinEvalContext implements EvalContext
var _ EvalContext = (*BuiltinEvalContext)(nil)

func (ctx *BuiltinEvalContext) WithPath(path addrs.ModuleInstance) EvalContext {
func (ctx *BuiltinEvalContext) withScope(scope evalContextScope) EvalContext {
newCtx := *ctx
newCtx.scope = evalContextModuleInstance{
Addr: path,
}
return &newCtx
}

func (ctx *BuiltinEvalContext) WithPartialExpandedPath(path addrs.PartialExpandedModule) EvalContext {
newCtx := *ctx
newCtx.scope = evalContextPartialExpandedModule{
Addr: path,
}
newCtx.scope = scope
return &newCtx
}

Expand Down
6 changes: 3 additions & 3 deletions internal/terraform/eval_context_builtin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ func TestBuiltinEvalContextProviderInput(t *testing.T) {
cache := make(map[string]map[string]cty.Value)

ctx1 := testBuiltinEvalContext(t)
ctx1 = ctx1.WithPath(addrs.RootModuleInstance).(*BuiltinEvalContext)
ctx1 = ctx1.withScope(evalContextModuleInstance{Addr: addrs.RootModuleInstance}).(*BuiltinEvalContext)
ctx1.ProviderInputConfig = cache
ctx1.ProviderLock = &lock

ctx2 := testBuiltinEvalContext(t)
ctx2 = ctx2.WithPath(addrs.RootModuleInstance.Child("child", addrs.NoKey)).(*BuiltinEvalContext)
ctx2 = ctx2.withScope(evalContextModuleInstance{Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey)}).(*BuiltinEvalContext)
ctx2.ProviderInputConfig = cache
ctx2.ProviderLock = &lock

Expand Down Expand Up @@ -61,7 +61,7 @@ func TestBuildingEvalContextInitProvider(t *testing.T) {
testP := &MockProvider{}

ctx := testBuiltinEvalContext(t)
ctx = ctx.WithPath(addrs.RootModuleInstance).(*BuiltinEvalContext)
ctx = ctx.withScope(evalContextModuleInstance{Addr: addrs.RootModuleInstance}).(*BuiltinEvalContext)
ctx.ProviderLock = &lock
ctx.ProviderCache = make(map[string]providers.Interface)
ctx.Plugins = newContextPlugins(map[addrs.Provider]providers.Factory{
Expand Down
19 changes: 7 additions & 12 deletions internal/terraform/eval_context_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ type MockEvalContext struct {
EvaluationScopeScope *lang.Scope

PathCalled bool
PathPath addrs.ModuleInstance
Scope evalContextScope

LanguageExperimentsActive experiments.Set

Expand Down Expand Up @@ -326,23 +326,18 @@ func (c *MockEvalContext) EvaluationScope(self addrs.Referenceable, source addrs
return c.EvaluationScopeScope
}

func (c *MockEvalContext) WithPath(path addrs.ModuleInstance) EvalContext {
func (c *MockEvalContext) withScope(scope evalContextScope) EvalContext {
newC := *c
newC.PathPath = path
newC.Scope = scope
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
// This intentionally panics if scope isn't a module instance; callers
// should use this only for an eval context that's working in a
// fully-expanded module instance.
return c.Scope.(evalContextModuleInstance).Addr
}

func (c *MockEvalContext) LanguageExperimentActive(experiment experiments.Experiment) bool {
Expand Down
32 changes: 32 additions & 0 deletions internal/terraform/eval_context_scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package terraform

import (
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/collections"
)

// evalContextScope represents the scope that an [EvalContext] (or rather,
Expand All @@ -26,13 +27,17 @@ import (
// a common known prefix, in situations where a module call has an unknown
// value for its count or for_each argument.
type evalContextScope interface {
collections.UniqueKeyer[evalContextScope]

// 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

String() string
}

// evalContextGlobal is the nil [evalContextScope] used to represent an
Expand All @@ -49,6 +54,16 @@ func (s evalContextModuleInstance) evalContextScopeModule() addrs.Module {
return s.Addr.Module()
}

func (s evalContextModuleInstance) String() string {
return s.Addr.String()
}

func (s evalContextModuleInstance) UniqueKey() collections.UniqueKey[evalContextScope] {
return evalContextScopeUniqueKey{
k: s.Addr.UniqueKey(),
}
}

// evalContextPartialExpandedModule is an [evalContextScope] associated with
// an unbounded set of possible module instances that share a common known
// address prefix.
Expand All @@ -59,3 +74,20 @@ type evalContextPartialExpandedModule struct {
func (s evalContextPartialExpandedModule) evalContextScopeModule() addrs.Module {
return s.Addr.Module()
}

func (s evalContextPartialExpandedModule) String() string {
return s.Addr.String()
}

func (s evalContextPartialExpandedModule) UniqueKey() collections.UniqueKey[evalContextScope] {
return evalContextScopeUniqueKey{
k: s.Addr.UniqueKey(),
}
}

type evalContextScopeUniqueKey struct {
k addrs.UniqueKey
}

// IsUniqueKey implements collections.UniqueKey.
func (evalContextScopeUniqueKey) IsUniqueKey(evalContextScope) {}
2 changes: 1 addition & 1 deletion internal/terraform/eval_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func evaluateImportIdExpression(expr hcl.Expression, ctx EvalContext, keyData in

// import blocks only exist in the root module, and must be evaluated in
// that context.
ctx = ctx.WithPath(addrs.RootModuleInstance)
ctx = evalContextForModuleInstance(ctx, addrs.RootModuleInstance)

if expr == nil {
return "", diags.Append(&hcl.Diagnostic{
Expand Down
30 changes: 21 additions & 9 deletions internal/terraform/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,33 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics {

// 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.)
// with either a GraphNodeModuleInstance, GraphNodePartialExpandedModule,
// or graphNodeEvalContextScope implementation. (These interfaces are
// all intentionally mutually-exclusive by having the same method
// name but different signatures, since a node can only belong to
// one context at a time.)
vertexCtx := ctx
if pn, ok := v.(GraphNodeModuleInstance); ok {
if pn, ok := v.(graphNodeEvalContextScope); ok {
scope := pn.Path()
log.Printf("[TRACE] vertex %q: belongs to %s", dag.VertexName(v), scope)
vertexCtx = walker.enterScope(scope)
defer walker.exitScope(scope)
} else 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())
scope := evalContextModuleInstance{
Addr: moduleAddr,
}
vertexCtx = walker.enterScope(scope)
defer walker.exitScope(scope)
} 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())
scope := evalContextPartialExpandedModule{
Addr: moduleAddr,
}
vertexCtx = walker.enterScope(scope)
defer walker.exitScope(scope)
} else {
log.Printf("[TRACE] vertex %q: does not belong to any module instance", dag.VertexName(v))
}
Expand Down
14 changes: 14 additions & 0 deletions internal/terraform/graph_interface_subgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,17 @@ type GraphNodeModulePath interface {
type GraphNodePartialExpandedModule interface {
Path() addrs.PartialExpandedModule
}

// graphNodeEvalContextScope is essentially a combination of
// [GraphNodeModuleInstance] and [GraphNodePartialExpandedModule] for when
// the decision between the two must be made dynamically.
//
// When a graph node implements this interface, the [EvalContext] passed
// to its DynamicExpand and/or Execute method will be associated with whatever
// scope is returned by method Path.
type graphNodeEvalContextScope interface {
// Path must return a _non-nil_ evalContextScope value, which therefore
// describes either a fully-expanded module instance address or a
// partial-expanded module address.
Path() evalContextScope
}
17 changes: 5 additions & 12 deletions internal/terraform/graph_walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,15 @@
package terraform

import (
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/tfdiags"
)

// GraphWalker is an interface that can be implemented that when used
// with Graph.Walk will invoke the given callbacks under certain events.
type GraphWalker interface {
EvalContext() EvalContext
EnterPath(addrs.ModuleInstance) EvalContext
ExitPath(addrs.ModuleInstance)
EnterPartialExpandedPath(addrs.PartialExpandedModule) EvalContext
ExitPartialExpandedPath(addrs.PartialExpandedModule)
enterScope(evalContextScope) EvalContext
exitScope(evalContextScope)
Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics
}

Expand All @@ -24,11 +21,7 @@ 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) EnterPartialExpandedPath(addrs.PartialExpandedModule) EvalContext {
return new(MockEvalContext)
}
func (NullGraphWalker) ExitPartialExpandedPath(addrs.PartialExpandedModule) {}
func (NullGraphWalker) EvalContext() EvalContext { return new(MockEvalContext) }
func (NullGraphWalker) enterScope(evalContextScope) EvalContext { return new(MockEvalContext) }
func (NullGraphWalker) exitScope(evalContextScope) {}
func (NullGraphWalker) Execute(EvalContext, GraphNodeExecutable) tfdiags.Diagnostics { return nil }
33 changes: 12 additions & 21 deletions internal/terraform/graph_walk_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/checks"
"github.com/hashicorp/terraform/internal/collections"
"github.com/hashicorp/terraform/internal/configs"
"github.com/hashicorp/terraform/internal/configs/configschema"
"github.com/hashicorp/terraform/internal/instances"
Expand Down Expand Up @@ -51,7 +52,7 @@ type ContextGraphWalker struct {
NonFatalDiagnostics tfdiags.Diagnostics

once sync.Once
contexts map[string]*BuiltinEvalContext
contexts collections.Map[evalContextScope, *BuiltinEvalContext]
contextLock sync.Mutex
providerCache map[string]providers.Interface
providerFuncCache map[string]providers.Interface
Expand All @@ -65,33 +66,23 @@ type ContextGraphWalker struct {

var _ GraphWalker = (*ContextGraphWalker)(nil)

func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) 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
// enterScope provides an EvalContext associated with the given scope.
func (w *ContextGraphWalker) enterScope(scope evalContextScope) EvalContext {
if scope == nil {
// Just want a global EvalContext then, presumably.
return w.EvalContext()
}

ctx := w.EvalContext().WithPath(path)
w.contexts[key] = ctx.(*BuiltinEvalContext)
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 {
// We might already have a context for this scope.
if ctx, ok := w.contexts.GetOk(scope); ok {
return ctx
}

ctx := w.EvalContext().WithPartialExpandedPath(path)
w.contexts[key] = ctx.(*BuiltinEvalContext)
ctx := w.EvalContext().withScope(scope).(*BuiltinEvalContext)
w.contexts.Put(scope, ctx)
return ctx
}

Expand Down Expand Up @@ -142,7 +133,7 @@ func (w *ContextGraphWalker) EvalContext() EvalContext {
}

func (w *ContextGraphWalker) init() {
w.contexts = make(map[string]*BuiltinEvalContext)
w.contexts = collections.NewMap[evalContextScope, *BuiltinEvalContext]()
w.providerCache = make(map[string]providers.Interface)
w.providerFuncCache = make(map[string]providers.Interface)
w.providerSchemas = make(map[string]providers.ProviderSchema)
Expand Down
Loading
Loading