terraform: set variables in the proper location

terraform/eval_context.go

@@ -58,10 +58,10 @@ type EvalContext interface {
 	// that is currently being acted upon.
 	Interpolate(*config.RawConfig, *Resource) (*ResourceConfig, error)
 
-	// SetVariables sets the variables for interpolation. These variables
+	// SetVariables sets the variables for the module within
-	// should not have a "var." prefix. For example: "var.foo" should be
+	// this context with the name n. This function call is additive:
-	// "foo" as the key.
+	// the second parameter is merged with any previous call.
-	SetVariables(map[string]string)
+	SetVariables(string, map[string]string)
 
 	// Diff returns the global diff as well as the lock that should
 	// be used to modify that diff.

terraform/eval_context_builtin.go

@@ -14,6 +14,8 @@ import (
 type BuiltinEvalContext struct {
 	PathValue           []string
 	Interpolater        *Interpolater
+	InterpolaterVars    map[string]map[string]string
+	InterpolaterVarLock *sync.Mutex
 	Hooks               []Hook
 	InputValue          UIInput
 	Providers           map[string]ResourceProviderFactory
@@ -30,7 +32,6 @@ type BuiltinEvalContext struct {
 	StateLock           *sync.RWMutex
 
 	once sync.Once
-	varLock sync.Mutex
 }
 
 func (ctx *BuiltinEvalContext) Hook(fn func(Hook) (HookAction, error)) error {
@@ -238,12 +239,23 @@ func (ctx *BuiltinEvalContext) Path() []string {
 	return ctx.PathValue
 }
 
-func (ctx *BuiltinEvalContext) SetVariables(vs map[string]string) {
+func (ctx *BuiltinEvalContext) SetVariables(n string, vs map[string]string) {
-	ctx.varLock.Lock()
+	ctx.InterpolaterVarLock.Lock()
-	defer ctx.varLock.Unlock()
+	defer ctx.InterpolaterVarLock.Unlock()
+
+	path := make([]string, len(ctx.Path())+1)
+	copy(path, ctx.Path())
+	path[len(path)-1] = n
+	key := PathCacheKey(path)
+
+	vars := ctx.InterpolaterVars[key]
+	if vars == nil {
+		vars = make(map[string]string)
+		ctx.InterpolaterVars[key] = vars
+	}
 
 	for k, v := range vs {
-		ctx.Interpolater.Variables[k] = v
+		vars[k] = v
 	}
 }
 

terraform/eval_context_mock.go

@@ -65,6 +65,7 @@ type MockEvalContext struct {
 	PathPath   []string
 
 	SetVariablesCalled    bool
+	SetVariablesModule    string
 	SetVariablesVariables map[string]string
 
 	DiffCalled bool
@@ -162,8 +163,9 @@ func (c *MockEvalContext) Path() []string {
 	return c.PathPath
 }
 
-func (c *MockEvalContext) SetVariables(vs map[string]string) {
+func (c *MockEvalContext) SetVariables(n string, vs map[string]string) {
 	c.SetVariablesCalled = true
+	c.SetVariablesModule = n
 	c.SetVariablesVariables = vs
 }
 

terraform/eval_variable.go

@@ -11,12 +11,13 @@ import (
 // EvalSetVariables is an EvalNode implementation that sets the variables
 // explicitly for interpolation later.
 type EvalSetVariables struct {
+	Module    *string
 	Variables map[string]string
 }
 
 // TODO: test
 func (n *EvalSetVariables) Eval(ctx EvalContext) (interface{}, error) {
-	ctx.SetVariables(n.Variables)
+	ctx.SetVariables(*n.Module, n.Variables)
 	return nil, nil
 }
 

terraform/graph.go

@@ -162,7 +162,7 @@ func (g *Graph) walk(walker GraphWalker) error {
 		// is normally the context of our graph but can be overridden
 		// with a GraphNodeSubPath impl.
 		vertexCtx := ctx
-		if pn, ok := v.(GraphNodeSubPath); ok {
+		if pn, ok := v.(GraphNodeSubPath); ok && len(pn.Path()) > 0 {
 			vertexCtx = walker.EnterPath(pn.Path())
 			defer walker.ExitPath(pn.Path())
 		}

terraform/graph_config_node_module.go

@@ -177,7 +177,7 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph {
 
 		// If this is a variable, then look it up in the raw configuration.
 		// If it exists in the raw configuration, set the value of it.
-		if vn, ok := v.(GraphNodeVariable); ok && input != nil {
+		if vn, ok := v.(*GraphNodeConfigVariable); ok && input != nil {
 			key := vn.VariableName()
 			if v, ok := input.Raw[key]; ok {
 				config, err := config.NewRawConfig(map[string]interface{}{
@@ -189,8 +189,10 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph {
 					panic(err)
 				}
 
-				// Set the variable value so it is interpolated properly
+				// Set the variable value so it is interpolated properly.
-				vn.SetVariableValue(config)
+				// Also set the module so we set the value on it properly.
+				vn.Module = graph.Path[len(graph.Path)-1]
+				vn.Value = config
 			}
 		}
 	}
@@ -209,32 +211,6 @@ type graphNodeModuleSkippable interface {
 	FlattenSkip() bool
 }
 
-/*
-// graphNodeModuleFlatWrap wraps elements of the module graph
-// when they are flattened so that the dependency information is
-// correct.
-type graphNodeModuleFlatWrap struct {
-	Vertex            dag.Vertex
-	PathValue         []string
-	NamePrefix        string
-	DependentOnPrefix string
-}
-
-// GraphNodeProviderConsumer impl.
-func (n *graphNodeModuleFlatWrap) ProvidedBy() []string {
-	pn, ok := n.Vertex.(GraphNodeProviderConsumer)
-	if !ok {
-		return nil
-	}
-
-	result := make([]string, 0, 2)
-	for _, v := range pn.ProvidedBy() {
-		result = append(result, fmt.Sprintf("%s.%s", n.NamePrefix, v))
-	}
-	return result
-}
-*/
-
 func modulePrefixStr(p []string) string {
 	parts := make([]string, 0, len(p)*2)
 	for _, p := range p[1:] {

terraform/graph_config_node_variable.go

@@ -7,19 +7,15 @@ import (
 	"github.com/hashicorp/terraform/dag"
 )
 
-// GraphNodeVariable is the interface that must be implemented by anything
-// that is a variable.
-type GraphNodeVariable interface {
-	VariableName() string
-	SetVariableValue(*config.RawConfig)
-}
-
 // GraphNodeConfigVariable represents a Variable in the config.
 type GraphNodeConfigVariable struct {
 	Variable *config.Variable
 
 	// Value, if non-nil, will be used to set the value of the variable
 	// during evaluation. If this is nil, evaluation will do nothing.
+	//
+	// Module is the name of the module to set the variables on.
+	Module string
 	Value  *config.RawConfig
 
 	depPrefix string
@@ -55,16 +51,10 @@ func (n *GraphNodeConfigVariable) DependentOn() []string {
 	return result
 }
 
-// GraphNodeVariable impl.
 func (n *GraphNodeConfigVariable) VariableName() string {
 	return n.Variable.Name
 }
 
-// GraphNodeVariable impl.
-func (n *GraphNodeConfigVariable) SetVariableValue(v *config.RawConfig) {
-	n.Value = v
-}
-
 // GraphNodeProxy impl.
 func (n *GraphNodeConfigVariable) Proxy() bool {
 	return true
@@ -94,6 +84,7 @@ func (n *GraphNodeConfigVariable) EvalTree() EvalNode {
 			},
 
 			&EvalSetVariables{
+				Module:    &n.Module,
 				Variables: variables,
 			},
 		},
@@ -128,7 +119,7 @@ func (n *GraphNodeConfigVariableFlat) DependentOn() []string {
 	// longer than 2 elements (root, child, more). This is because when
 	// flattened, variables can point outside the graph.
 	prefix := ""
-	if len(n.PathValue) <= 2 {
+	if len(n.PathValue) > 2 {
 		prefix = modulePrefixStr(n.PathValue[:len(n.PathValue)-1])
 	}
 
@@ -138,5 +129,9 @@ func (n *GraphNodeConfigVariableFlat) DependentOn() []string {
 }
 
 func (n *GraphNodeConfigVariableFlat) Path() []string {
-	return n.PathValue
+	if len(n.PathValue) > 2 {
+		return n.PathValue[:len(n.PathValue)-1]
+	}
+
+	return nil
 }

terraform/graph_config_node_variable_test.go

@@ -10,7 +10,6 @@ func TestGraphNodeConfigVariable_impl(t *testing.T) {
 	var _ dag.Vertex = new(GraphNodeConfigVariable)
 	var _ dag.NamedVertex = new(GraphNodeConfigVariable)
 	var _ graphNodeConfig = new(GraphNodeConfigVariable)
-	var _ GraphNodeVariable = new(GraphNodeConfigVariable)
 	var _ GraphNodeProxy = new(GraphNodeConfigVariable)
 }
 
@@ -18,6 +17,5 @@ func TestGraphNodeConfigVariableFlat_impl(t *testing.T) {
 	var _ dag.Vertex = new(GraphNodeConfigVariableFlat)
 	var _ dag.NamedVertex = new(GraphNodeConfigVariableFlat)
 	var _ graphNodeConfig = new(GraphNodeConfigVariableFlat)
-	var _ GraphNodeVariable = new(GraphNodeConfigVariableFlat)
 	var _ GraphNodeProxy = new(GraphNodeConfigVariableFlat)
 }

terraform/graph_walk_context.go

@@ -26,6 +26,8 @@ type ContextGraphWalker struct {
 	once                sync.Once
 	contexts            map[string]*BuiltinEvalContext
 	contextLock         sync.Mutex
+	interpolaterVars    map[string]map[string]string
+	interpolaterVarLock sync.Mutex
 	providerCache       map[string]ResourceProvider
 	providerConfigCache map[string]*ResourceConfig
 	providerLock        sync.Mutex
@@ -45,14 +47,21 @@ func (w *ContextGraphWalker) EnterPath(path []string) EvalContext {
 		return ctx
 	}
 
-	// Variables should be our context variables, but these only apply
+	// Setup the variables for this interpolater
-	// to the root module. As we enter subgraphs, we don't want to set
+	variables := make(map[string]string)
-	// variables, which is set by the SetVariables EvalContext function.
+	if len(path) <= 1 {
-	variables := w.Context.variables
+		for k, v := range w.Context.variables {
-	if len(path) > 1 {
+			variables[k] = v
-		// We're in a submodule, the variables should be empty
-		variables = make(map[string]string)
 		}
+	}
+	w.interpolaterVarLock.Lock()
+	if m, ok := w.interpolaterVars[key]; ok {
+		for k, v := range m {
+			variables[k] = v
+		}
+	}
+	w.interpolaterVars[key] = variables
+	w.interpolaterVarLock.Unlock()
 
 	ctx := &BuiltinEvalContext{
 		PathValue:           path,
@@ -77,6 +86,8 @@ func (w *ContextGraphWalker) EnterPath(path []string) EvalContext {
 			StateLock: &w.Context.stateLock,
 			Variables: variables,
 		},
+		InterpolaterVars:    w.interpolaterVars,
+		InterpolaterVarLock: &w.interpolaterVarLock,
 	}
 
 	w.contexts[key] = ctx
@@ -131,4 +142,5 @@ func (w *ContextGraphWalker) init() {
 	w.providerCache = make(map[string]ResourceProvider, 5)
 	w.providerConfigCache = make(map[string]*ResourceConfig, 5)
 	w.provisionerCache = make(map[string]ResourceProvisioner, 5)
+	w.interpolaterVars = make(map[string]map[string]string, 5)
 }