From 0b025d74e50cf15c117f492c85fa1334d94955c7 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 17 Mar 2020 14:02:46 -0400 Subject: [PATCH] add EvalContext.WithPath As the Graph is walked, the current way to set the context path was to have the walker return a context from EnterPath. This required that every node know it's absolute path, which can no longer be the case during plan when modules have not been expanded. This introduces a new method called WithPath, which returns a copy of the context with the internal path updated to reflect the method argument. Any use of the EvalContext that requires knowing the path will now panic if it wasn't explicitly set to ensure that evaluations always occur in the correct path. Add EvalContext to the GraphWalker interface. EvalContext returns an EvalContext that has not yet set a path. This will allow us to enforce that all context operations requiring a module instance path will require that a path be explicitly set rather than evaluating within the wrong path. --- terraform/eval.go | 16 ++++++++++------ terraform/eval_context.go | 4 ++++ terraform/eval_context_builtin.go | 24 ++++++++++++++++++++++++ terraform/eval_context_mock.go | 6 ++++++ terraform/graph.go | 5 ++--- terraform/graph_walk.go | 2 ++ terraform/graph_walk_context.go | 12 ++++++++---- 7 files changed, 56 insertions(+), 13 deletions(-) diff --git a/terraform/eval.go b/terraform/eval.go index 48ed3533a..51e89d8b5 100644 --- a/terraform/eval.go +++ b/terraform/eval.go @@ -46,12 +46,16 @@ func Eval(n EvalNode, ctx EvalContext) (interface{}, error) { // signal something normal such as EvalEarlyExitError. func EvalRaw(n EvalNode, ctx EvalContext) (interface{}, error) { path := "unknown" - if ctx != nil { - path = ctx.Path().String() - } - if path == "" { - path = "" - } + + // FIXME: restore the path here somehow or log this in another manner + // We cannot call Path here, since the context may not yet have the path + // set. + //if ctx != nil { + // path = ctx.Path().String() + //} + //if path == "" { + // path = "" + //} log.Printf("[TRACE] %s: eval: %T", path, n) output, err := n.Eval(ctx) diff --git a/terraform/eval_context.go b/terraform/eval_context.go index a682b3d6c..241662f7b 100644 --- a/terraform/eval_context.go +++ b/terraform/eval_context.go @@ -161,4 +161,8 @@ type EvalContext interface { // The InstanceExpander is a global object that is shared across all of the // EvalContext objects for a given configuration. InstanceExpander() *instances.Expander + + // WithPath returns a copy of the context with the internal path set to the + // path argument. + WithPath(path addrs.ModuleInstance) EvalContext } diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 746b00fe1..b5e400be4 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -32,6 +32,13 @@ type BuiltinEvalContext struct { // 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 @@ -70,6 +77,13 @@ type BuiltinEvalContext struct { // BuiltinEvalContext implements EvalContext var _ EvalContext = (*BuiltinEvalContext)(nil) +func (ctx *BuiltinEvalContext) WithPath(path addrs.ModuleInstance) EvalContext { + ctx.pathSet = true + newCtx := *ctx + newCtx.PathValue = path + return &newCtx +} + func (ctx *BuiltinEvalContext) Stopped() <-chan struct{} { // This can happen during tests. During tests, we just block forever. if ctx.StopContext == nil { @@ -291,6 +305,9 @@ func (ctx *BuiltinEvalContext) EvaluateExpr(expr hcl.Expression, wantType cty.Ty } func (ctx *BuiltinEvalContext) EvaluationScope(self addrs.Referenceable, keyData InstanceKeyEvalData) *lang.Scope { + if !ctx.pathSet { + panic("context path not set") + } data := &evaluationStateData{ Evaluator: ctx.Evaluator, ModulePath: ctx.PathValue, @@ -301,6 +318,9 @@ func (ctx *BuiltinEvalContext) EvaluationScope(self addrs.Referenceable, keyData } func (ctx *BuiltinEvalContext) Path() addrs.ModuleInstance { + if !ctx.pathSet { + panic("context path not set") + } return ctx.PathValue } @@ -308,6 +328,10 @@ func (ctx *BuiltinEvalContext) SetModuleCallArguments(n addrs.ModuleCallInstance ctx.VariableValuesLock.Lock() defer ctx.VariableValuesLock.Unlock() + if !ctx.pathSet { + panic("context path not set") + } + childPath := n.ModuleInstance(ctx.PathValue) key := childPath.String() diff --git a/terraform/eval_context_mock.go b/terraform/eval_context_mock.go index 210a40d80..15bf80569 100644 --- a/terraform/eval_context_mock.go +++ b/terraform/eval_context_mock.go @@ -305,6 +305,12 @@ func (c *MockEvalContext) EvaluationScope(self addrs.Referenceable, keyData Inst return c.EvaluationScopeScope } +func (c *MockEvalContext) WithPath(path addrs.ModuleInstance) EvalContext { + newC := *c + newC.PathPath = path + return &newC +} + func (c *MockEvalContext) Path() addrs.ModuleInstance { c.PathCalled = true return c.PathPath diff --git a/terraform/graph.go b/terraform/graph.go index 373819f7e..4c9f2f0c2 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -35,8 +35,7 @@ func (g *Graph) Walk(walker GraphWalker) tfdiags.Diagnostics { func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics { // The callbacks for enter/exiting a graph - ctx := walker.EnterPath(g.Path) - defer walker.ExitPath(g.Path) + ctx := walker.EvalContext() // Walk the graph. var walkFn dag.WalkFunc @@ -54,7 +53,7 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics { // is normally the context of our graph but can be overridden // with a GraphNodeModuleInstance impl. vertexCtx := ctx - if pn, ok := v.(GraphNodeModuleInstance); ok && len(pn.Path()) > 0 { + if pn, ok := v.(GraphNodeModuleInstance); ok { vertexCtx = walker.EnterPath(pn.Path()) defer walker.ExitPath(pn.Path()) } diff --git a/terraform/graph_walk.go b/terraform/graph_walk.go index e980e0c6d..706b7e0ab 100644 --- a/terraform/graph_walk.go +++ b/terraform/graph_walk.go @@ -9,6 +9,7 @@ import ( // 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) EnterVertex(dag.Vertex) @@ -22,6 +23,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) EnterVertex(dag.Vertex) {} diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index d53ebe97a..5025c98b6 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -51,8 +51,6 @@ type ContextGraphWalker struct { } func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext { - w.once.Do(w.init) - w.contextLock.Lock() defer w.contextLock.Unlock() @@ -62,6 +60,14 @@ func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext { return ctx } + ctx := w.EvalContext().WithPath(path) + w.contexts[key] = ctx.(*BuiltinEvalContext) + return ctx +} + +func (w *ContextGraphWalker) EvalContext() EvalContext { + w.once.Do(w.init) + // Our evaluator shares some locks with the main context and the walker // so that we can safely run multiple evaluations at once across // different modules. @@ -78,7 +84,6 @@ func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext { ctx := &BuiltinEvalContext{ StopContext: w.StopContext, - PathValue: path, Hooks: w.Context.hooks, InputValue: w.Context.uiInput, InstanceExpanderValue: w.InstanceExpander, @@ -96,7 +101,6 @@ func (w *ContextGraphWalker) EnterPath(path addrs.ModuleInstance) EvalContext { VariableValuesLock: &w.variableValuesLock, } - w.contexts[key] = ctx return ctx }