From c207beda368850324c1604bfa2eddea8094e8569 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 16:29:19 -0700 Subject: [PATCH] terraform: set variables in the proper location --- terraform/eval_context.go | 8 ++--- terraform/eval_context_builtin.go | 24 ++++++++++---- terraform/eval_context_mock.go | 4 ++- terraform/eval_variable.go | 3 +- terraform/graph.go | 2 +- terraform/graph_config_node_module.go | 34 +++----------------- terraform/graph_config_node_variable.go | 27 +++++++--------- terraform/graph_config_node_variable_test.go | 2 -- terraform/graph_walk_context.go | 26 +++++++++++---- 9 files changed, 63 insertions(+), 67 deletions(-) diff --git a/terraform/eval_context.go b/terraform/eval_context.go index 4f6d7c2e7..8a838afb7 100644 --- a/terraform/eval_context.go +++ b/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 - // should not have a "var." prefix. For example: "var.foo" should be - // "foo" as the key. - SetVariables(map[string]string) + // SetVariables sets the variables for the module within + // this context with the name n. This function call is additive: + // the second parameter is merged with any previous call. + SetVariables(string, map[string]string) // Diff returns the global diff as well as the lock that should // be used to modify that diff. diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index d355e36ef..33a71b5dd 100644 --- a/terraform/eval_context_builtin.go +++ b/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 @@ -29,8 +31,7 @@ type BuiltinEvalContext struct { StateValue *State StateLock *sync.RWMutex - once sync.Once - varLock sync.Mutex + once sync.Once } 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) { - ctx.varLock.Lock() - defer ctx.varLock.Unlock() +func (ctx *BuiltinEvalContext) SetVariables(n string, vs map[string]string) { + ctx.InterpolaterVarLock.Lock() + 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 } } diff --git a/terraform/eval_context_mock.go b/terraform/eval_context_mock.go index 27a98c2d5..2a5856829 100644 --- a/terraform/eval_context_mock.go +++ b/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 } diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go index ced1a0610..e6a9befbe 100644 --- a/terraform/eval_variable.go +++ b/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 } diff --git a/terraform/graph.go b/terraform/graph.go index a86e0ca1e..6fe78207f 100644 --- a/terraform/graph.go +++ b/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()) } diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 3d046a2ef..347c964bd 100644 --- a/terraform/graph_config_node_module.go +++ b/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 - vn.SetVariableValue(config) + // Set the variable value so it is interpolated properly. + // 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:] { diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index ec98883b5..54c1c8343 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -7,20 +7,16 @@ 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. - Value *config.RawConfig + // + // 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 } diff --git a/terraform/graph_config_node_variable_test.go b/terraform/graph_config_node_variable_test.go index 41cf75c37..9f0251b42 100644 --- a/terraform/graph_config_node_variable_test.go +++ b/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) } diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 508a8edc3..314d18972 100644 --- a/terraform/graph_walk_context.go +++ b/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 - // to the root module. As we enter subgraphs, we don't want to set - // variables, which is set by the SetVariables EvalContext function. - variables := w.Context.variables - if len(path) > 1 { - // We're in a submodule, the variables should be empty - variables = make(map[string]string) + // Setup the variables for this interpolater + variables := make(map[string]string) + if len(path) <= 1 { + for k, v := range w.Context.variables { + variables[k] = v + } } + 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) }