From 1152ff562b365f235582502344bd50b019d581d4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 30 Apr 2015 16:27:20 -0700 Subject: [PATCH 01/33] terraform: add variables as graph nodes (no eval yet) --- terraform/graph_config_node_test.go | 6 ++++ terraform/graph_config_node_type.go | 1 + terraform/graph_config_node_variable.go | 28 +++++++++++++++++++ terraform/graphnodeconfigtype_string.go | 4 +-- .../apply-provisioner-compute/main.tf | 2 ++ .../apply-provisioner-conninfo/main.tf | 3 ++ .../plan-module-provider-defaults-var/main.tf | 2 ++ terraform/transform_config.go | 13 ++++++++- terraform/transform_config_test.go | 2 ++ terraform/transform_provider_test.go | 4 +++ terraform/transform_resource.go | 25 +++++++++++++---- 11 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 terraform/graph_config_node_variable.go diff --git a/terraform/graph_config_node_test.go b/terraform/graph_config_node_test.go index 13848663d5e4..22653dffd4fc 100644 --- a/terraform/graph_config_node_test.go +++ b/terraform/graph_config_node_test.go @@ -141,6 +141,12 @@ func TestGraphNodeConfigResource_ProvisionedBy(t *testing.T) { } } +func TestGraphNodeConfigVariable_impl(t *testing.T) { + var _ dag.Vertex = new(GraphNodeConfigVariable) + var _ dag.NamedVertex = new(GraphNodeConfigVariable) + var _ graphNodeConfig = new(GraphNodeConfigVariable) +} + const testGraphNodeModuleExpandStr = ` aws_instance.bar aws_instance.foo diff --git a/terraform/graph_config_node_type.go b/terraform/graph_config_node_type.go index f0196096fbc1..42dd8dc9fcd6 100644 --- a/terraform/graph_config_node_type.go +++ b/terraform/graph_config_node_type.go @@ -12,4 +12,5 @@ const ( GraphNodeConfigTypeProvider GraphNodeConfigTypeModule GraphNodeConfigTypeOutput + GraphNodeConfigTypeVariable ) diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go new file mode 100644 index 000000000000..ad1bf3857044 --- /dev/null +++ b/terraform/graph_config_node_variable.go @@ -0,0 +1,28 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// GraphNodeConfigVariable represents a Variable in the config. +type GraphNodeConfigVariable struct { + Variable *config.Variable +} + +func (n *GraphNodeConfigVariable) Name() string { + return fmt.Sprintf("var.%s", n.Variable.Name) +} + +func (n *GraphNodeConfigVariable) ConfigType() GraphNodeConfigType { + return GraphNodeConfigTypeVariable +} + +func (n *GraphNodeConfigVariable) DependableName() []string { + return []string{n.Name()} +} + +func (n *GraphNodeConfigVariable) DependentOn() []string { + return nil +} diff --git a/terraform/graphnodeconfigtype_string.go b/terraform/graphnodeconfigtype_string.go index d0748979e520..de24c2dcda10 100644 --- a/terraform/graphnodeconfigtype_string.go +++ b/terraform/graphnodeconfigtype_string.go @@ -4,9 +4,9 @@ package terraform import "fmt" -const _GraphNodeConfigType_name = "GraphNodeConfigTypeInvalidGraphNodeConfigTypeResourceGraphNodeConfigTypeProviderGraphNodeConfigTypeModuleGraphNodeConfigTypeOutput" +const _GraphNodeConfigType_name = "GraphNodeConfigTypeInvalidGraphNodeConfigTypeResourceGraphNodeConfigTypeProviderGraphNodeConfigTypeModuleGraphNodeConfigTypeOutputGraphNodeConfigTypeVariable" -var _GraphNodeConfigType_index = [...]uint8{0, 26, 53, 80, 105, 130} +var _GraphNodeConfigType_index = [...]uint8{0, 26, 53, 80, 105, 130, 157} func (i GraphNodeConfigType) String() string { if i < 0 || i+1 >= GraphNodeConfigType(len(_GraphNodeConfigType_index)) { diff --git a/terraform/test-fixtures/apply-provisioner-compute/main.tf b/terraform/test-fixtures/apply-provisioner-compute/main.tf index e0ff5507ae92..e503eddd5b4d 100644 --- a/terraform/test-fixtures/apply-provisioner-compute/main.tf +++ b/terraform/test-fixtures/apply-provisioner-compute/main.tf @@ -1,3 +1,5 @@ +variable "value" {} + resource "aws_instance" "foo" { num = "2" compute = "dynamical" diff --git a/terraform/test-fixtures/apply-provisioner-conninfo/main.tf b/terraform/test-fixtures/apply-provisioner-conninfo/main.tf index f0bfe43abf19..029895fa1905 100644 --- a/terraform/test-fixtures/apply-provisioner-conninfo/main.tf +++ b/terraform/test-fixtures/apply-provisioner-conninfo/main.tf @@ -1,3 +1,6 @@ +variable "pass" {} +variable "value" {} + resource "aws_instance" "foo" { num = "2" compute = "dynamical" diff --git a/terraform/test-fixtures/plan-module-provider-defaults-var/main.tf b/terraform/test-fixtures/plan-module-provider-defaults-var/main.tf index e6e3f1c29369..d3c34908bd1d 100644 --- a/terraform/test-fixtures/plan-module-provider-defaults-var/main.tf +++ b/terraform/test-fixtures/plan-module-provider-defaults-var/main.tf @@ -7,3 +7,5 @@ provider "aws" { } resource "aws_instance" "foo" {} + +variable "foo" {} diff --git a/terraform/transform_config.go b/terraform/transform_config.go index 3b115688b781..44cc9f57cdaf 100644 --- a/terraform/transform_config.go +++ b/terraform/transform_config.go @@ -37,7 +37,16 @@ func (t *ConfigTransformer) Transform(g *Graph) error { // Create the node list we'll use for the graph nodes := make([]graphNodeConfig, 0, - (len(config.ProviderConfigs)+len(config.Modules)+len(config.Resources))*2) + (len(config.Variables)+ + len(config.ProviderConfigs)+ + len(config.Modules)+ + len(config.Resources)+ + len(config.Outputs))*2) + + // Write all the variables out + for _, v := range config.Variables { + nodes = append(nodes, &GraphNodeConfigVariable{Variable: v}) + } // Write all the provider configs out for _, pc := range config.ProviderConfigs { @@ -99,6 +108,8 @@ func varNameForVar(raw config.InterpolatedVariable) string { return fmt.Sprintf("module.%s", v.Name) case *config.ResourceVariable: return v.ResourceId() + case *config.UserVariable: + return fmt.Sprintf("var.%s", v.Name) default: return "" } diff --git a/terraform/transform_config_test.go b/terraform/transform_config_test.go index 75570d3f6671..4ba88a358220 100644 --- a/terraform/transform_config_test.go +++ b/terraform/transform_config_test.go @@ -111,12 +111,14 @@ func TestConfigTransformer_errMissingDeps(t *testing.T) { const testGraphBasicStr = ` aws_instance.web aws_security_group.firewall + var.foo aws_load_balancer.weblb aws_instance.web aws_security_group.firewall openstack_floating_ip.random provider.aws openstack_floating_ip.random +var.foo ` const testGraphDependsOnStr = ` diff --git a/terraform/transform_provider_test.go b/terraform/transform_provider_test.go index 719cef75cc38..fdf764c3f976 100644 --- a/terraform/transform_provider_test.go +++ b/terraform/transform_provider_test.go @@ -218,7 +218,9 @@ provider.foo const testTransformDisableProviderBasicStr = ` module.child provider.aws (disabled) + var.foo provider.aws (disabled) +var.foo ` const testTransformDisableProviderKeepStr = ` @@ -226,5 +228,7 @@ aws_instance.foo provider.aws module.child provider.aws + var.foo provider.aws +var.foo ` diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 498fe20ac604..0289e1a1d3dc 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" @@ -169,6 +170,18 @@ func (n *graphNodeExpandedResource) ProvidedBy() []string { return []string{resourceProvider(n.Resource.Type, n.Resource.Provider)} } +func (n *graphNodeExpandedResource) StateDependencies() []string { + depsRaw := n.DependentOn() + deps := make([]string, 0, len(depsRaw)) + for _, d := range depsRaw { + if !strings.HasPrefix(d, "var.") { + deps = append(deps, d) + } + } + + return deps +} + // GraphNodeEvalable impl. func (n *graphNodeExpandedResource) EvalTree() EvalNode { var diff *InstanceDiff @@ -257,7 +270,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Name: n.stateId(), ResourceType: n.Resource.Type, Provider: n.Resource.Provider, - Dependencies: n.DependentOn(), + Dependencies: n.StateDependencies(), State: &state, }, }, @@ -298,7 +311,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Name: n.stateId(), ResourceType: n.Resource.Type, Provider: n.Resource.Provider, - Dependencies: n.DependentOn(), + Dependencies: n.StateDependencies(), State: &state, }, &EvalDiffTainted{ @@ -445,7 +458,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Name: n.stateId(), ResourceType: n.Resource.Type, Provider: n.Resource.Provider, - Dependencies: n.DependentOn(), + Dependencies: n.StateDependencies(), State: &state, }, &EvalApplyProvisioners{ @@ -489,7 +502,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Name: n.stateId(), ResourceType: n.Resource.Type, Provider: n.Resource.Provider, - Dependencies: n.DependentOn(), + Dependencies: n.StateDependencies(), State: &state, Index: -1, }, @@ -507,7 +520,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Name: n.stateId(), ResourceType: n.Resource.Type, Provider: n.Resource.Provider, - Dependencies: n.DependentOn(), + Dependencies: n.StateDependencies(), State: &state, }, }, @@ -618,7 +631,7 @@ func (n *graphNodeExpandedResourceDestroy) EvalTree() EvalNode { Name: n.stateId(), ResourceType: n.Resource.Type, Provider: n.Resource.Provider, - Dependencies: n.DependentOn(), + Dependencies: n.StateDependencies(), State: &state, }, &EvalApplyPost{ From 90cfade62698c8c68fd82019b7955aa65bb001e8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 30 Apr 2015 17:02:39 -0700 Subject: [PATCH 02/33] terraform: naive flatten --- terraform/transform_flatten.go | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 terraform/transform_flatten.go diff --git a/terraform/transform_flatten.go b/terraform/transform_flatten.go new file mode 100644 index 000000000000..93161ceabfb7 --- /dev/null +++ b/terraform/transform_flatten.go @@ -0,0 +1,43 @@ +package terraform + +// GraphNodeFlattenable must be implemented by nodes that can be flattened +// into the graph. +type GraphNodeFlattenable interface { + GraphNodeSubgraph + + // Flatten should return true if this should be flattened. + Flatten() bool +} + +// FlattenTransform is a transformer that goes through the graph, finds +// subgraphs that can be flattened, and flattens them into this graph, +// removing the prior subgraph node. +type FlattenTransform struct{} + +func (t *FlattenTransform) Transform(g *Graph) error { + for _, v := range g.Vertices() { + fn, ok := v.(GraphNodeFlattenable) + if !ok { + continue + } + + // If we don't want to be flattened, don't do it + if !fn.Flatten() { + continue + } + + // Get the subgraph and flatten it into this one + subgraph := fn.Subgraph() + for _, sv := range subgraph.Vertices() { + g.Add(sv) + } + for _, se := range subgraph.Edges() { + g.Connect(se) + } + + // Remove the old node + g.Remove(v) + } + + return nil +} From 15ca84a682a4d46be4e924364b5be86051daf7b4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 30 Apr 2015 17:19:01 -0700 Subject: [PATCH 03/33] terraform: module dependencies in graph use full name (FOR THE FUTURE) --- terraform/graph_config_node.go | 10 +++++++++- .../test-fixtures/graph-modules/consul/main.tf | 2 ++ terraform/transform_config.go | 2 +- terraform/transform_resource.go | 13 +++++++++++-- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 2fc958f0bc83..84882eae3baf 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -58,7 +58,15 @@ func (n *GraphNodeConfigModule) ConfigType() GraphNodeConfigType { } func (n *GraphNodeConfigModule) DependableName() []string { - return []string{n.Name()} + config := n.Tree.Config() + + result := make([]string, 1, len(config.Outputs)+1) + result[0] = n.Name() + for _, o := range config.Outputs { + result = append(result, fmt.Sprintf("%s.%s", n.Name(), o.Name)) + } + + return result } func (n *GraphNodeConfigModule) DependentOn() []string { diff --git a/terraform/test-fixtures/graph-modules/consul/main.tf b/terraform/test-fixtures/graph-modules/consul/main.tf index aa8f6f0327a5..9e22d04d84e6 100644 --- a/terraform/test-fixtures/graph-modules/consul/main.tf +++ b/terraform/test-fixtures/graph-modules/consul/main.tf @@ -1 +1,3 @@ resource "aws_instance" "server" {} + +output "security_group" { value = "" } diff --git a/terraform/transform_config.go b/terraform/transform_config.go index 44cc9f57cdaf..87832429419e 100644 --- a/terraform/transform_config.go +++ b/terraform/transform_config.go @@ -105,7 +105,7 @@ func (t *ConfigTransformer) Transform(g *Graph) error { func varNameForVar(raw config.InterpolatedVariable) string { switch v := raw.(type) { case *config.ModuleVariable: - return fmt.Sprintf("module.%s", v.Name) + return fmt.Sprintf("module.%s.%s", v.Name, v.Field) case *config.ResourceVariable: return v.ResourceId() case *config.UserVariable: diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 0289e1a1d3dc..76e65af73846 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -174,9 +174,18 @@ func (n *graphNodeExpandedResource) StateDependencies() []string { depsRaw := n.DependentOn() deps := make([]string, 0, len(depsRaw)) for _, d := range depsRaw { - if !strings.HasPrefix(d, "var.") { - deps = append(deps, d) + // Ignore any variable dependencies + if strings.HasPrefix(d, "var.") { + continue + } + + // This is sad. The dependencies are currently in the format of + // "module.foo.bar" (the full field). This strips the field off. + if strings.HasPrefix(d, "module.") { + parts := strings.SplitN(d, ".", 3) + d = strings.Join(parts[0:2], ".") } + deps = append(deps, d) } return deps From 5e767f63b997d3f8b893f34d2d05751dfb42409d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 30 Apr 2015 19:23:52 -0700 Subject: [PATCH 04/33] terraform: move module to its own file --- terraform/graph_config_node.go | 149 ------------------- terraform/graph_config_node_module.go | 158 +++++++++++++++++++++ terraform/graph_config_node_module_test.go | 41 ++++++ terraform/graph_config_node_test.go | 33 ----- terraform/transform_flatten.go | 12 +- 5 files changed, 204 insertions(+), 189 deletions(-) create mode 100644 terraform/graph_config_node_module.go create mode 100644 terraform/graph_config_node_module_test.go diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 84882eae3baf..3e21e805279d 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -5,7 +5,6 @@ import ( "strings" "github.com/hashicorp/terraform/config" - "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/dot" ) @@ -46,92 +45,6 @@ type GraphNodeTargetable interface { SetTargets([]ResourceAddress) } -// GraphNodeConfigModule represents a module within the configuration graph. -type GraphNodeConfigModule struct { - Path []string - Module *config.Module - Tree *module.Tree -} - -func (n *GraphNodeConfigModule) ConfigType() GraphNodeConfigType { - return GraphNodeConfigTypeModule -} - -func (n *GraphNodeConfigModule) DependableName() []string { - config := n.Tree.Config() - - result := make([]string, 1, len(config.Outputs)+1) - result[0] = n.Name() - for _, o := range config.Outputs { - result = append(result, fmt.Sprintf("%s.%s", n.Name(), o.Name)) - } - - return result -} - -func (n *GraphNodeConfigModule) DependentOn() []string { - vars := n.Module.RawConfig.Variables - result := make([]string, 0, len(vars)) - for _, v := range vars { - if vn := varNameForVar(v); vn != "" { - result = append(result, vn) - } - } - - return result -} - -func (n *GraphNodeConfigModule) Name() string { - return fmt.Sprintf("module.%s", n.Module.Name) -} - -// GraphNodeExpandable -func (n *GraphNodeConfigModule) Expand(b GraphBuilder) (GraphNodeSubgraph, error) { - // Build the graph first - graph, err := b.Build(n.Path) - if err != nil { - return nil, err - } - - // Add the parameters node to the module - t := &ModuleInputTransformer{Variables: make(map[string]string)} - if err := t.Transform(graph); err != nil { - return nil, err - } - - // Build the actual subgraph node - return &graphNodeModuleExpanded{ - Original: n, - Graph: graph, - InputConfig: n.Module.RawConfig, - Variables: t.Variables, - }, nil -} - -// GraphNodeExpandable -func (n *GraphNodeConfigModule) ProvidedBy() []string { - // Build up the list of providers by simply going over our configuration - // to find the providers that are configured there as well as the - // providers that the resources use. - config := n.Tree.Config() - providers := make(map[string]struct{}) - for _, p := range config.ProviderConfigs { - providers[p.Name] = struct{}{} - } - for _, r := range config.Resources { - providers[resourceProvider(r.Type, r.Provider)] = struct{}{} - } - - // Turn the map into a string. This makes sure that the list is - // de-dupped since we could be going over potentially many resources. - result := make([]string, 0, len(providers)) - for p, _ := range providers { - result = append(result, p) - } - - return result -} - // GraphNodeConfigOutput represents an output configured within the // configuration. type GraphNodeConfigOutput struct { @@ -622,65 +535,3 @@ func (n *graphNodeResourceDestroy) destroyIncludePrimary( return false } - -// graphNodeModuleExpanded represents a module where the graph has -// been expanded. It stores the graph of the module as well as a reference -// to the map of variables. -type graphNodeModuleExpanded struct { - Original dag.Vertex - Graph *Graph - InputConfig *config.RawConfig - - // Variables is a map of the input variables. This reference should - // be shared with ModuleInputTransformer in order to create a connection - // where the variables are set properly. - Variables map[string]string -} - -func (n *graphNodeModuleExpanded) Name() string { - return fmt.Sprintf("%s (expanded)", dag.VertexName(n.Original)) -} - -func (n *graphNodeModuleExpanded) ConfigType() GraphNodeConfigType { - return GraphNodeConfigTypeModule -} - -// GraphNodeDotter impl. -func (n *graphNodeModuleExpanded) DotNode(name string, opts *GraphDotOpts) *dot.Node { - return dot.NewNode(name, map[string]string{ - "label": dag.VertexName(n.Original), - "shape": "component", - }) -} - -// GraphNodeEvalable impl. -func (n *graphNodeModuleExpanded) EvalTree() EvalNode { - var resourceConfig *ResourceConfig - return &EvalSequence{ - Nodes: []EvalNode{ - &EvalInterpolate{ - Config: n.InputConfig, - Output: &resourceConfig, - }, - - &EvalVariableBlock{ - Config: &resourceConfig, - Variables: n.Variables, - }, - - &EvalOpFilter{ - Ops: []walkOperation{walkPlanDestroy}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalDiffDestroyModule{Path: n.Graph.Path}, - }, - }, - }, - }, - } -} - -// GraphNodeSubgraph impl. -func (n *graphNodeModuleExpanded) Subgraph() *Graph { - return n.Graph -} diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go new file mode 100644 index 000000000000..22ed3493f256 --- /dev/null +++ b/terraform/graph_config_node_module.go @@ -0,0 +1,158 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/dot" +) + +// GraphNodeConfigModule represents a module within the configuration graph. +type GraphNodeConfigModule struct { + Path []string + Module *config.Module + Tree *module.Tree +} + +func (n *GraphNodeConfigModule) ConfigType() GraphNodeConfigType { + return GraphNodeConfigTypeModule +} + +func (n *GraphNodeConfigModule) DependableName() []string { + config := n.Tree.Config() + + result := make([]string, 1, len(config.Outputs)+1) + result[0] = n.Name() + for _, o := range config.Outputs { + result = append(result, fmt.Sprintf("%s.%s", n.Name(), o.Name)) + } + + return result +} + +func (n *GraphNodeConfigModule) DependentOn() []string { + vars := n.Module.RawConfig.Variables + result := make([]string, 0, len(vars)) + for _, v := range vars { + if vn := varNameForVar(v); vn != "" { + result = append(result, vn) + } + } + + return result +} + +func (n *GraphNodeConfigModule) Name() string { + return fmt.Sprintf("module.%s", n.Module.Name) +} + +// GraphNodeExpandable +func (n *GraphNodeConfigModule) Expand(b GraphBuilder) (GraphNodeSubgraph, error) { + // Build the graph first + graph, err := b.Build(n.Path) + if err != nil { + return nil, err + } + + // Add the parameters node to the module + t := &ModuleInputTransformer{Variables: make(map[string]string)} + if err := t.Transform(graph); err != nil { + return nil, err + } + + // Build the actual subgraph node + return &graphNodeModuleExpanded{ + Original: n, + Graph: graph, + InputConfig: n.Module.RawConfig, + Variables: t.Variables, + }, nil +} + +// GraphNodeExpandable +func (n *GraphNodeConfigModule) ProvidedBy() []string { + // Build up the list of providers by simply going over our configuration + // to find the providers that are configured there as well as the + // providers that the resources use. + config := n.Tree.Config() + providers := make(map[string]struct{}) + for _, p := range config.ProviderConfigs { + providers[p.Name] = struct{}{} + } + for _, r := range config.Resources { + providers[resourceProvider(r.Type, r.Provider)] = struct{}{} + } + + // Turn the map into a string. This makes sure that the list is + // de-dupped since we could be going over potentially many resources. + result := make([]string, 0, len(providers)) + for p, _ := range providers { + result = append(result, p) + } + + return result +} + +// graphNodeModuleExpanded represents a module where the graph has +// been expanded. It stores the graph of the module as well as a reference +// to the map of variables. +type graphNodeModuleExpanded struct { + Original dag.Vertex + Graph *Graph + InputConfig *config.RawConfig + + // Variables is a map of the input variables. This reference should + // be shared with ModuleInputTransformer in order to create a connection + // where the variables are set properly. + Variables map[string]string +} + +func (n *graphNodeModuleExpanded) Name() string { + return fmt.Sprintf("%s (expanded)", dag.VertexName(n.Original)) +} + +func (n *graphNodeModuleExpanded) ConfigType() GraphNodeConfigType { + return GraphNodeConfigTypeModule +} + +// GraphNodeDotter impl. +func (n *graphNodeModuleExpanded) DotNode(name string, opts *GraphDotOpts) *dot.Node { + return dot.NewNode(name, map[string]string{ + "label": dag.VertexName(n.Original), + "shape": "component", + }) +} + +// GraphNodeEvalable impl. +func (n *graphNodeModuleExpanded) EvalTree() EvalNode { + var resourceConfig *ResourceConfig + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.InputConfig, + Output: &resourceConfig, + }, + + &EvalVariableBlock{ + Config: &resourceConfig, + Variables: n.Variables, + }, + + &EvalOpFilter{ + Ops: []walkOperation{walkPlanDestroy}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalDiffDestroyModule{Path: n.Graph.Path}, + }, + }, + }, + }, + } +} + +// GraphNodeSubgraph impl. +func (n *graphNodeModuleExpanded) Subgraph() *Graph { + return n.Graph +} diff --git a/terraform/graph_config_node_module_test.go b/terraform/graph_config_node_module_test.go new file mode 100644 index 000000000000..f30f422a10b8 --- /dev/null +++ b/terraform/graph_config_node_module_test.go @@ -0,0 +1,41 @@ +package terraform + +import ( + "strings" + "testing" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" +) + +func TestGraphNodeConfigModule_impl(t *testing.T) { + var _ dag.Vertex = new(GraphNodeConfigModule) + var _ dag.NamedVertex = new(GraphNodeConfigModule) + var _ graphNodeConfig = new(GraphNodeConfigModule) + var _ GraphNodeExpandable = new(GraphNodeConfigModule) +} + +func TestGraphNodeConfigModuleExpand(t *testing.T) { + mod := testModule(t, "graph-node-module-expand") + + node := &GraphNodeConfigModule{ + Path: []string{RootModuleName, "child"}, + Module: &config.Module{}, + Tree: nil, + } + + g, err := node.Expand(&BasicGraphBuilder{ + Steps: []GraphTransformer{ + &ConfigTransformer{Module: mod}, + }, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.Subgraph().String()) + expected := strings.TrimSpace(testGraphNodeModuleExpandStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} diff --git a/terraform/graph_config_node_test.go b/terraform/graph_config_node_test.go index 22653dffd4fc..2793135165ff 100644 --- a/terraform/graph_config_node_test.go +++ b/terraform/graph_config_node_test.go @@ -2,45 +2,12 @@ package terraform import ( "reflect" - "strings" "testing" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" ) -func TestGraphNodeConfigModule_impl(t *testing.T) { - var _ dag.Vertex = new(GraphNodeConfigModule) - var _ dag.NamedVertex = new(GraphNodeConfigModule) - var _ graphNodeConfig = new(GraphNodeConfigModule) - var _ GraphNodeExpandable = new(GraphNodeConfigModule) -} - -func TestGraphNodeConfigModuleExpand(t *testing.T) { - mod := testModule(t, "graph-node-module-expand") - - node := &GraphNodeConfigModule{ - Path: []string{RootModuleName, "child"}, - Module: &config.Module{}, - Tree: nil, - } - - g, err := node.Expand(&BasicGraphBuilder{ - Steps: []GraphTransformer{ - &ConfigTransformer{Module: mod}, - }, - }) - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.Subgraph().String()) - expected := strings.TrimSpace(testGraphNodeModuleExpandStr) - if actual != expected { - t.Fatalf("bad:\n\n%s", actual) - } -} - func TestGraphNodeConfigOutput_impl(t *testing.T) { var _ dag.Vertex = new(GraphNodeConfigOutput) var _ dag.NamedVertex = new(GraphNodeConfigOutput) diff --git a/terraform/transform_flatten.go b/terraform/transform_flatten.go index 93161ceabfb7..ecdef22fe163 100644 --- a/terraform/transform_flatten.go +++ b/terraform/transform_flatten.go @@ -3,10 +3,7 @@ package terraform // GraphNodeFlattenable must be implemented by nodes that can be flattened // into the graph. type GraphNodeFlattenable interface { - GraphNodeSubgraph - - // Flatten should return true if this should be flattened. - Flatten() bool + FlattenGraph() *Graph } // FlattenTransform is a transformer that goes through the graph, finds @@ -22,12 +19,13 @@ func (t *FlattenTransform) Transform(g *Graph) error { } // If we don't want to be flattened, don't do it - if !fn.Flatten() { + subgraph := fn.FlattenGraph() + if subgraph == nil { continue } - // Get the subgraph and flatten it into this one - subgraph := fn.Subgraph() + // Flatten the subgraph into this one. Keep any existing + // connections that existed. for _, sv := range subgraph.Vertices() { g.Add(sv) } From 12c30feb0f8b2617e2a09b329dc73aa43a619b31 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 30 Apr 2015 20:46:54 -0700 Subject: [PATCH 05/33] terraform: start FlattenGraph impl. --- terraform/graph_config_node_module.go | 69 +++++++++++++++++++ terraform/graph_config_node_module_test.go | 39 +++++++++++ terraform/graph_config_node_test.go | 8 --- .../graph-node-module-flatten/child/main.tf | 1 + .../graph-node-module-flatten/main.tf | 3 + terraform/transform_module.go | 5 ++ 6 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 terraform/test-fixtures/graph-node-module-flatten/child/main.tf create mode 100644 terraform/test-fixtures/graph-node-module-flatten/main.tf diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 22ed3493f256..cc6a87b2dfef 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" @@ -152,7 +153,75 @@ func (n *graphNodeModuleExpanded) EvalTree() EvalNode { } } +// GraphNodeFlattenable impl. +func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { + graph := n.Subgraph() + + // Build the string that represents the path. We do this once here + // so that we only have to compute it once. The block below is in {} + // so that parts drops out of scope immediately. + var pathStr string + { + parts := make([]string, 0, len(graph.Path)*2) + for _, p := range graph.Path[1:] { + parts = append(parts, "module", p) + } + + pathStr = strings.Join(parts, ".") + } + + // Go over each vertex in the graph and wrap the configuration + // items so that the dependencies properly map to the modules. + // See the docs for graphNodeModuleWrappable for more info. + for _, v := range graph.Vertices() { + if sn, ok := v.(graphNodeModuleSkippable); ok && sn.FlattenSkip() { + graph.Remove(v) + continue + } + + wn, ok := v.(graphNodeModuleWrappable) + if !ok { + panic("unwrappable node: " + dag.VertexName(v)) + } + + graph.Replace(v, &graphNodeModuleFlatWrap{ + graphNodeModuleWrappable: wn, + + Path: graph.Path, + PathString: pathStr, + }) + } + + return graph +} + // GraphNodeSubgraph impl. func (n *graphNodeModuleExpanded) Subgraph() *Graph { return n.Graph } + +// This interface can be implemented to be skipped/ignored when +// flattening the module graph. +type graphNodeModuleSkippable interface { + FlattenSkip() bool +} + +// This is the interface that all nodes within a module graph must +// implement to be wrapped for flattening. +type graphNodeModuleWrappable interface { + graphNodeConfig +} + +// graphNodeModuleFlatWrap wraps elements of the module graph +// when they are flattened so that the dependency information is +// correct. +type graphNodeModuleFlatWrap struct { + graphNodeModuleWrappable + + Path []string + PathString string +} + +func (n *graphNodeModuleFlatWrap) Name() string { + return fmt.Sprintf("%s.%s", n.PathString, n.graphNodeModuleWrappable.Name()) +} diff --git a/terraform/graph_config_node_module_test.go b/terraform/graph_config_node_module_test.go index f30f422a10b8..35c3cd7a2660 100644 --- a/terraform/graph_config_node_module_test.go +++ b/terraform/graph_config_node_module_test.go @@ -39,3 +39,42 @@ func TestGraphNodeConfigModuleExpand(t *testing.T) { t.Fatalf("bad:\n\n%s", actual) } } + +func TestGraphNodeConfigModuleExpandFlatten(t *testing.T) { + mod := testModule(t, "graph-node-module-flatten") + + node := &GraphNodeConfigModule{ + Path: []string{RootModuleName, "child"}, + Module: &config.Module{}, + Tree: nil, + } + + g, err := node.Expand(&BasicGraphBuilder{ + Steps: []GraphTransformer{ + &ConfigTransformer{Module: mod}, + }, + }) + if err != nil { + t.Fatalf("err: %s", err) + } + + fg := g.(GraphNodeFlattenable) + + actual := strings.TrimSpace(fg.FlattenGraph().String()) + expected := strings.TrimSpace(testGraphNodeModuleExpandFlattenStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testGraphNodeModuleExpandStr = ` +aws_instance.bar + aws_instance.foo +aws_instance.foo + module inputs +module inputs +` + +const testGraphNodeModuleExpandFlattenStr = ` +module.child.aws_instance.foo +` diff --git a/terraform/graph_config_node_test.go b/terraform/graph_config_node_test.go index 2793135165ff..c2b016ece149 100644 --- a/terraform/graph_config_node_test.go +++ b/terraform/graph_config_node_test.go @@ -113,11 +113,3 @@ func TestGraphNodeConfigVariable_impl(t *testing.T) { var _ dag.NamedVertex = new(GraphNodeConfigVariable) var _ graphNodeConfig = new(GraphNodeConfigVariable) } - -const testGraphNodeModuleExpandStr = ` -aws_instance.bar - aws_instance.foo -aws_instance.foo - module inputs -module inputs -` diff --git a/terraform/test-fixtures/graph-node-module-flatten/child/main.tf b/terraform/test-fixtures/graph-node-module-flatten/child/main.tf new file mode 100644 index 000000000000..919f140bba6b --- /dev/null +++ b/terraform/test-fixtures/graph-node-module-flatten/child/main.tf @@ -0,0 +1 @@ +resource "aws_instance" "foo" {} diff --git a/terraform/test-fixtures/graph-node-module-flatten/main.tf b/terraform/test-fixtures/graph-node-module-flatten/main.tf new file mode 100644 index 000000000000..0f6991c536ca --- /dev/null +++ b/terraform/test-fixtures/graph-node-module-flatten/main.tf @@ -0,0 +1,3 @@ +module "child" { + source = "./child" +} diff --git a/terraform/transform_module.go b/terraform/transform_module.go index 816ccc455b72..7733fe578407 100644 --- a/terraform/transform_module.go +++ b/terraform/transform_module.go @@ -45,3 +45,8 @@ func (n *graphNodeModuleInput) Name() string { func (n *graphNodeModuleInput) EvalTree() EvalNode { return &EvalSetVariables{Variables: n.Variables} } + +// graphNodeModuleSkippable impl. +func (n *graphNodeModuleInput) FlattenSkip() bool { + return true +} From 3bfef7c374d664f064981767ada440d43387d78d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 10:56:05 -0700 Subject: [PATCH 06/33] terraform: wrap variable values and prefix dependencies (untested yet) --- terraform/graph_config_node_module.go | 81 ++++++++++++++++++---- terraform/graph_config_node_module_test.go | 68 ++++++++++++++++++ terraform/graph_config_node_variable.go | 21 ++++++ 3 files changed, 156 insertions(+), 14 deletions(-) diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index cc6a87b2dfef..76fe99a6b910 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -65,10 +65,9 @@ func (n *GraphNodeConfigModule) Expand(b GraphBuilder) (GraphNodeSubgraph, error // Build the actual subgraph node return &graphNodeModuleExpanded{ - Original: n, - Graph: graph, - InputConfig: n.Module.RawConfig, - Variables: t.Variables, + Original: n, + Graph: graph, + Variables: t.Variables, }, nil } @@ -100,9 +99,8 @@ func (n *GraphNodeConfigModule) ProvidedBy() []string { // been expanded. It stores the graph of the module as well as a reference // to the map of variables. type graphNodeModuleExpanded struct { - Original dag.Vertex - Graph *Graph - InputConfig *config.RawConfig + Original *GraphNodeConfigModule + Graph *Graph // Variables is a map of the input variables. This reference should // be shared with ModuleInputTransformer in order to create a connection @@ -132,7 +130,7 @@ func (n *graphNodeModuleExpanded) EvalTree() EvalNode { return &EvalSequence{ Nodes: []EvalNode{ &EvalInterpolate{ - Config: n.InputConfig, + Config: n.Original.Module.RawConfig, Output: &resourceConfig, }, @@ -156,11 +154,12 @@ func (n *graphNodeModuleExpanded) EvalTree() EvalNode { // GraphNodeFlattenable impl. func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { graph := n.Subgraph() + input := n.Original.Module.RawConfig // Build the string that represents the path. We do this once here // so that we only have to compute it once. The block below is in {} // so that parts drops out of scope immediately. - var pathStr string + var pathStr, parentPathStr string { parts := make([]string, 0, len(graph.Path)*2) for _, p := range graph.Path[1:] { @@ -168,6 +167,9 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { } pathStr = strings.Join(parts, ".") + if len(parts) > 2 { + parentPathStr = strings.Join(parts[0:len(parts)-2], ".") + } } // Go over each vertex in the graph and wrap the configuration @@ -184,11 +186,40 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { panic("unwrappable node: " + dag.VertexName(v)) } + // Prefix for dependencies. We set this to blank for variables + // (see below). + depPrefix := pathStr + + // 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 { + key := vn.VariableName() + if v, ok := input.Raw[key]; ok { + config, err := config.NewRawConfig(map[string]interface{}{ + key: v, + }) + if err != nil { + // This shouldn't happen because it is already in + // a RawConfig above meaning it worked once before. + panic(err) + } + + // Set the variable value so it is interpolated properly + vn.SetVariableValue(config) + } + + // We set the dependency prefix to the parent path + // since variables can reference outside of our own graph + // to the parent graph. + depPrefix = parentPathStr + } + graph.Replace(v, &graphNodeModuleFlatWrap{ graphNodeModuleWrappable: wn, - Path: graph.Path, - PathString: pathStr, + Path: graph.Path, + NamePrefix: pathStr, + DependentOnPrefix: depPrefix, }) } @@ -218,10 +249,32 @@ type graphNodeModuleWrappable interface { type graphNodeModuleFlatWrap struct { graphNodeModuleWrappable - Path []string - PathString string + Path []string + NamePrefix string + DependentOnPrefix string } func (n *graphNodeModuleFlatWrap) Name() string { - return fmt.Sprintf("%s.%s", n.PathString, n.graphNodeModuleWrappable.Name()) + return fmt.Sprintf("%s.%s", n.NamePrefix, n.graphNodeModuleWrappable.Name()) +} + +func (n *graphNodeModuleFlatWrap) DependableName() []string { + result := n.graphNodeModuleWrappable.DependableName() + n.prefixList(result, n.NamePrefix) + return result +} + +func (n *graphNodeModuleFlatWrap) DependentOn() []string { + result := n.graphNodeModuleWrappable.DependentOn() + if n.DependentOnPrefix != "" { + n.prefixList(result, n.DependentOnPrefix) + } + + return result +} + +func (n *graphNodeModuleFlatWrap) prefixList(result []string, prefix string) { + for i, v := range result { + result[i] = fmt.Sprintf("%s.%s", prefix, v) + } } diff --git a/terraform/graph_config_node_module_test.go b/terraform/graph_config_node_module_test.go index 35c3cd7a2660..2806dff3c946 100644 --- a/terraform/graph_config_node_module_test.go +++ b/terraform/graph_config_node_module_test.go @@ -1,6 +1,7 @@ package terraform import ( + "reflect" "strings" "testing" @@ -67,6 +68,73 @@ func TestGraphNodeConfigModuleExpandFlatten(t *testing.T) { } } +func TestGraphNodeModulFlatWrap_Name(t *testing.T) { + n := &graphNodeModuleFlatWrap{ + graphNodeModuleWrappable: &testGraphNodeModuleWrappable{ + NameValue: "foo", + }, + + NamePrefix: "module.bar", + } + + if v := n.Name(); v != "module.bar.foo" { + t.Fatalf("bad: %s", v) + } +} + +func TestGraphNodeModulFlatWrap_DependentOn(t *testing.T) { + n := &graphNodeModuleFlatWrap{ + graphNodeModuleWrappable: &testGraphNodeModuleWrappable{ + NameValue: "foo", + }, + + NamePrefix: "module.bar", + DependentOnPrefix: "module.bar", + } + + actual := n.DependentOn() + expected := []string{"module.bar.foo"} + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } +} + +func TestGraphNodeModulFlatWrap_DependableName(t *testing.T) { + n := &graphNodeModuleFlatWrap{ + graphNodeModuleWrappable: &testGraphNodeModuleWrappable{ + NameValue: "foo", + }, + + NamePrefix: "module.bar", + } + + actual := n.DependableName() + expected := []string{"module.bar.foo"} + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("bad: %#v", actual) + } +} + +type testGraphNodeModuleWrappable struct { + NameValue string +} + +func (n *testGraphNodeModuleWrappable) ConfigType() GraphNodeConfigType { + return GraphNodeConfigTypeInvalid +} + +func (n *testGraphNodeModuleWrappable) Name() string { + return n.NameValue +} + +func (n *testGraphNodeModuleWrappable) DependableName() []string { + return []string{"foo"} +} + +func (n *testGraphNodeModuleWrappable) DependentOn() []string { + return []string{"foo"} +} + const testGraphNodeModuleExpandStr = ` aws_instance.bar aws_instance.foo diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index ad1bf3857044..d0b1f0e4624d 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -6,9 +6,20 @@ import ( "github.com/hashicorp/terraform/config" ) +// 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 } func (n *GraphNodeConfigVariable) Name() string { @@ -26,3 +37,13 @@ func (n *GraphNodeConfigVariable) DependableName() []string { func (n *GraphNodeConfigVariable) DependentOn() []string { return nil } + +// GraphNodeVariable impl. +func (n *GraphNodeConfigVariable) VariableName() string { + return n.Variable.Name +} + +// GraphNodeVariable impl. +func (n *GraphNodeConfigVariable) SetVariableValue(v *config.RawConfig) { + n.Value = v +} From e542d6efdf0ccb9f58b942ba1cce8dc33aba5789 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 10:56:46 -0700 Subject: [PATCH 07/33] terraform: test to verify config variables are variables --- terraform/graph_config_node_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/terraform/graph_config_node_test.go b/terraform/graph_config_node_test.go index c2b016ece149..b297c6c1396e 100644 --- a/terraform/graph_config_node_test.go +++ b/terraform/graph_config_node_test.go @@ -112,4 +112,5 @@ func TestGraphNodeConfigVariable_impl(t *testing.T) { var _ dag.Vertex = new(GraphNodeConfigVariable) var _ dag.NamedVertex = new(GraphNodeConfigVariable) var _ graphNodeConfig = new(GraphNodeConfigVariable) + var _ GraphNodeVariable = new(GraphNodeConfigVariable) } From dd14ce9a0bb238d21b0d88bb28047f5783c99cef Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 11:09:23 -0700 Subject: [PATCH 08/33] terraform: test that variable deps reach out to parent graph --- terraform/graph_config_node_variable.go | 16 ++++++- .../transform-flatten/child/main.tf | 5 +++ .../test-fixtures/transform-flatten/main.tf | 8 ++++ terraform/transform_flatten.go | 12 +++-- terraform/transform_flatten_test.go | 44 +++++++++++++++++++ 5 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 terraform/test-fixtures/transform-flatten/child/main.tf create mode 100644 terraform/test-fixtures/transform-flatten/main.tf create mode 100644 terraform/transform_flatten_test.go diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index d0b1f0e4624d..b6e5390062b9 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -35,7 +35,21 @@ func (n *GraphNodeConfigVariable) DependableName() []string { } func (n *GraphNodeConfigVariable) DependentOn() []string { - return nil + // If we don't have any value set, we don't depend on anything + if n.Value == nil { + return nil + } + + // Get what we depend on based on our value + vars := n.Value.Variables + result := make([]string, 0, len(vars)) + for _, v := range vars { + if vn := varNameForVar(v); vn != "" { + result = append(result, vn) + } + } + + return result } // GraphNodeVariable impl. diff --git a/terraform/test-fixtures/transform-flatten/child/main.tf b/terraform/test-fixtures/transform-flatten/child/main.tf new file mode 100644 index 000000000000..e0c8ce4e4187 --- /dev/null +++ b/terraform/test-fixtures/transform-flatten/child/main.tf @@ -0,0 +1,5 @@ +variable "var" {} + +resource "aws_instance" "child" { + value = "${var.var}" +} diff --git a/terraform/test-fixtures/transform-flatten/main.tf b/terraform/test-fixtures/transform-flatten/main.tf new file mode 100644 index 000000000000..648d15786d65 --- /dev/null +++ b/terraform/test-fixtures/transform-flatten/main.tf @@ -0,0 +1,8 @@ +module "child" { + source = "./child" + var = "${aws_instance.parent.value}" +} + +resource "aws_instance" "parent" { + value = "foo" +} diff --git a/terraform/transform_flatten.go b/terraform/transform_flatten.go index ecdef22fe163..30f0edaf7903 100644 --- a/terraform/transform_flatten.go +++ b/terraform/transform_flatten.go @@ -6,12 +6,12 @@ type GraphNodeFlattenable interface { FlattenGraph() *Graph } -// FlattenTransform is a transformer that goes through the graph, finds +// FlattenTransformer is a transformer that goes through the graph, finds // subgraphs that can be flattened, and flattens them into this graph, // removing the prior subgraph node. -type FlattenTransform struct{} +type FlattenTransformer struct{} -func (t *FlattenTransform) Transform(g *Graph) error { +func (t *FlattenTransformer) Transform(g *Graph) error { for _, v := range g.Vertices() { fn, ok := v.(GraphNodeFlattenable) if !ok { @@ -35,6 +35,12 @@ func (t *FlattenTransform) Transform(g *Graph) error { // Remove the old node g.Remove(v) + + // Connect the dependencies for all the new nodes that we added. + // This will properly connect variables to their sources, for example. + for _, sv := range subgraph.Vertices() { + g.ConnectDependent(sv) + } } return nil diff --git a/terraform/transform_flatten_test.go b/terraform/transform_flatten_test.go new file mode 100644 index 000000000000..cc564d495c9b --- /dev/null +++ b/terraform/transform_flatten_test.go @@ -0,0 +1,44 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestFlattenTransformer(t *testing.T) { + mod := testModule(t, "transform-flatten") + + var b BasicGraphBuilder + b = BasicGraphBuilder{ + Steps: []GraphTransformer{ + &ConfigTransformer{Module: mod}, + &VertexTransformer{ + Transforms: []GraphVertexTransformer{ + &ExpandTransform{ + Builder: &b, + }, + }, + }, + &FlattenTransformer{}, + }, + } + + g, err := b.Build(rootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformFlattenStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testTransformFlattenStr = ` +aws_instance.parent +module.child.aws_instance.child + module.child.var.var +module.child.var.var + aws_instance.parent +` From 7e838dfae221f0eb8219374d1aeabf5bab1c813f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 11:20:57 -0700 Subject: [PATCH 09/33] terraform: Graph should overrride Remove to clear lookaside --- terraform/graph.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/terraform/graph.go b/terraform/graph.go index cf931fb1faaa..cf5439097e69 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -54,6 +54,21 @@ func (g *Graph) Add(v dag.Vertex) dag.Vertex { return v } +// Remove is the same as dag.Graph.Remove +func (g *Graph) Remove(v dag.Vertex) dag.Vertex { + g.once.Do(g.init) + + // If this is a depend-able node, then remove the lookaside info + if dv, ok := v.(GraphNodeDependable); ok { + for _, n := range dv.DependableName() { + delete(g.dependableMap, n) + } + } + + // Call upwards to remove it from the actual graph + return g.Graph.Remove(v) +} + // ConnectDependent connects a GraphNodeDependent to all of its // GraphNodeDependables. It returns the list of dependents it was // unable to connect to. From a0d9bc0f19c4bb5bd8d16ba4f5abac7ff556d19c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 11:26:58 -0700 Subject: [PATCH 10/33] terraform: outputs connect properly --- terraform/graph_config_node_module.go | 12 ++++++++- .../transform-flatten/child/main.tf | 4 +++ .../test-fixtures/transform-flatten/main.tf | 4 +++ terraform/transform_config.go | 2 +- terraform/transform_flatten.go | 25 ++++++++++++++++--- terraform/transform_flatten_test.go | 4 +++ 6 files changed, 46 insertions(+), 5 deletions(-) diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 76fe99a6b910..48c1b6a4fba5 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -27,7 +27,7 @@ func (n *GraphNodeConfigModule) DependableName() []string { result := make([]string, 1, len(config.Outputs)+1) result[0] = n.Name() for _, o := range config.Outputs { - result = append(result, fmt.Sprintf("%s.%s", n.Name(), o.Name)) + result = append(result, fmt.Sprintf("%s.output.%s", n.Name(), o.Name)) } return result @@ -116,6 +116,16 @@ func (n *graphNodeModuleExpanded) ConfigType() GraphNodeConfigType { return GraphNodeConfigTypeModule } +// GraphNodeDependable +func (n *graphNodeModuleExpanded) DependableName() []string { + return n.Original.DependableName() +} + +// GraphNodeDependent +func (n *graphNodeModuleExpanded) DependentOn() []string { + return n.Original.DependentOn() +} + // GraphNodeDotter impl. func (n *graphNodeModuleExpanded) DotNode(name string, opts *GraphDotOpts) *dot.Node { return dot.NewNode(name, map[string]string{ diff --git a/terraform/test-fixtures/transform-flatten/child/main.tf b/terraform/test-fixtures/transform-flatten/child/main.tf index e0c8ce4e4187..7371f826d260 100644 --- a/terraform/test-fixtures/transform-flatten/child/main.tf +++ b/terraform/test-fixtures/transform-flatten/child/main.tf @@ -3,3 +3,7 @@ variable "var" {} resource "aws_instance" "child" { value = "${var.var}" } + +output "output" { + value = "${aws_instance.child.value}" +} diff --git a/terraform/test-fixtures/transform-flatten/main.tf b/terraform/test-fixtures/transform-flatten/main.tf index 648d15786d65..179e151a3cbc 100644 --- a/terraform/test-fixtures/transform-flatten/main.tf +++ b/terraform/test-fixtures/transform-flatten/main.tf @@ -6,3 +6,7 @@ module "child" { resource "aws_instance" "parent" { value = "foo" } + +resource "aws_instance" "parent-output" { + value = "${module.child.output}" +} diff --git a/terraform/transform_config.go b/terraform/transform_config.go index 87832429419e..3c061eeb1eb2 100644 --- a/terraform/transform_config.go +++ b/terraform/transform_config.go @@ -105,7 +105,7 @@ func (t *ConfigTransformer) Transform(g *Graph) error { func varNameForVar(raw config.InterpolatedVariable) string { switch v := raw.(type) { case *config.ModuleVariable: - return fmt.Sprintf("module.%s.%s", v.Name, v.Field) + return fmt.Sprintf("module.%s.output.%s", v.Name, v.Field) case *config.ResourceVariable: return v.ResourceId() case *config.UserVariable: diff --git a/terraform/transform_flatten.go b/terraform/transform_flatten.go index 30f0edaf7903..995524dff780 100644 --- a/terraform/transform_flatten.go +++ b/terraform/transform_flatten.go @@ -1,5 +1,9 @@ package terraform +import ( + "github.com/hashicorp/terraform/dag" +) + // GraphNodeFlattenable must be implemented by nodes that can be flattened // into the graph. type GraphNodeFlattenable interface { @@ -24,6 +28,17 @@ func (t *FlattenTransformer) Transform(g *Graph) error { continue } + // Get all the things that depend on this node. We'll re-connect + // dependents later. We have to copy these here since the UpEdges + // value will be deleted after the Remove below. + dependents := make([]dag.Vertex, 0, 5) + for _, v := range g.UpEdges(v).List() { + dependents = append(dependents, v) + } + + // Remove the old node + g.Remove(v) + // Flatten the subgraph into this one. Keep any existing // connections that existed. for _, sv := range subgraph.Vertices() { @@ -33,14 +48,18 @@ func (t *FlattenTransformer) Transform(g *Graph) error { g.Connect(se) } - // Remove the old node - g.Remove(v) - // Connect the dependencies for all the new nodes that we added. // This will properly connect variables to their sources, for example. for _, sv := range subgraph.Vertices() { g.ConnectDependent(sv) } + + // Re-connect all the things that dependend on the graph + // we just flattened. This should connect them back into the + // correct nodes if their DependentOn() is setup correctly. + for _, v := range dependents { + g.ConnectDependent(v) + } } return nil diff --git a/terraform/transform_flatten_test.go b/terraform/transform_flatten_test.go index cc564d495c9b..eddad8897ba3 100644 --- a/terraform/transform_flatten_test.go +++ b/terraform/transform_flatten_test.go @@ -37,8 +37,12 @@ func TestFlattenTransformer(t *testing.T) { const testTransformFlattenStr = ` aws_instance.parent +aws_instance.parent-output + module.child.output.output module.child.aws_instance.child module.child.var.var +module.child.output.output + module.child.aws_instance.child module.child.var.var aws_instance.parent ` From f24a1533f25a51e3cc5396df7ef26050f5373d5b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 11:38:36 -0700 Subject: [PATCH 11/33] terraform: GraphNodeProxy --- terraform/transform_proxy.go | 55 +++++++++++++++++++++++++++++++ terraform/transform_proxy_test.go | 52 +++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 terraform/transform_proxy.go create mode 100644 terraform/transform_proxy_test.go diff --git a/terraform/transform_proxy.go b/terraform/transform_proxy.go new file mode 100644 index 000000000000..aed50ba818c7 --- /dev/null +++ b/terraform/transform_proxy.go @@ -0,0 +1,55 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/dag" +) + +// GraphNodeProxy must be implemented by nodes that are proxies. +// +// A node that is a proxy says that anything that depends on this +// node (the proxy), should also copy all the things that the proxy +// itself depends on. Example: +// +// A => proxy => C +// +// Should transform into (two edges): +// +// A => proxy => C +// A => C +// +// The purpose for this is because some transforms only look at direct +// edge connections and the proxy generally isn't meaningful in those +// situations, so we should complete all the edges. +type GraphNodeProxy interface { + Proxy() bool +} + +// ProxyTransformer is a transformer that goes through the graph, finds +// vertices that are marked as proxies, and connects through their +// dependents. See above for what a proxy is. +type ProxyTransformer struct{} + +func (t *ProxyTransformer) Transform(g *Graph) error { + for _, v := range g.Vertices() { + pn, ok := v.(GraphNodeProxy) + if !ok { + continue + } + + // If we don't want to be proxies, don't do it + if !pn.Proxy() { + continue + } + + // Connect all the things that depend on this to things that + // we depend on as the proxy. See docs for GraphNodeProxy for + // a visual explanation. + for _, s := range g.UpEdges(v).List() { + for _, t := range g.DownEdges(v).List() { + g.Connect(dag.BasicEdge(s, t)) + } + } + } + + return nil +} diff --git a/terraform/transform_proxy_test.go b/terraform/transform_proxy_test.go new file mode 100644 index 000000000000..dc1b2cfbbf9e --- /dev/null +++ b/terraform/transform_proxy_test.go @@ -0,0 +1,52 @@ +package terraform + +import ( + "strings" + "testing" + + "github.com/hashicorp/terraform/dag" +) + +func TestProxyTransformer(t *testing.T) { + var g Graph + proxy := &testNodeProxy{NameValue: "proxy"} + g.Add("A") + g.Add("C") + g.Add(proxy) + g.Connect(dag.BasicEdge("A", proxy)) + g.Connect(dag.BasicEdge(proxy, "C")) + + { + tf := &ProxyTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testProxyTransformStr) + if actual != expected { + t.Fatalf("bad: %s", actual) + } +} + +type testNodeProxy struct { + NameValue string +} + +func (n *testNodeProxy) Name() string { + return n.NameValue +} + +func (n *testNodeProxy) Proxy() bool { + return true +} + +const testProxyTransformStr = ` +A + C + proxy +C +proxy + C +` From 7d28e980a5755fc616e0e78acefaa3424879f916 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 11:41:01 -0700 Subject: [PATCH 12/33] terraform: proxy uses custom edge --- terraform/transform_proxy.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/terraform/transform_proxy.go b/terraform/transform_proxy.go index aed50ba818c7..db7b34ed8a74 100644 --- a/terraform/transform_proxy.go +++ b/terraform/transform_proxy.go @@ -46,10 +46,17 @@ func (t *ProxyTransformer) Transform(g *Graph) error { // a visual explanation. for _, s := range g.UpEdges(v).List() { for _, t := range g.DownEdges(v).List() { - g.Connect(dag.BasicEdge(s, t)) + g.Connect(GraphProxyEdge{ + Edge: dag.BasicEdge(s, t), + }) } } } return nil } + +// GraphProxyEdge is the edge that is used for proxied edges. +type GraphProxyEdge struct { + dag.Edge +} From e544e1d4019ce3431eaf40a2310d821962235c3e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 11:45:42 -0700 Subject: [PATCH 13/33] terraform: hook up the proxies --- terraform/graph_config_node.go | 5 +++ terraform/graph_config_node_module.go | 9 +++++ terraform/graph_config_node_test.go | 2 ++ terraform/graph_config_node_variable.go | 5 +++ terraform/transform_flatten_test.go | 45 +++++++++++++++++++++++++ 5 files changed, 66 insertions(+) diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 3e21e805279d..55c90df8b85b 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -94,6 +94,11 @@ func (n *GraphNodeConfigOutput) EvalTree() EvalNode { } } +// GraphNodeProxy impl. +func (n *GraphNodeConfigOutput) Proxy() bool { + return true +} + // GraphNodeConfigProvider represents a configured provider within the // configuration graph. These are only immediately in the graph when an // explicit `provider` configuration block is in the configuration. diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 48c1b6a4fba5..15fa29ace0ca 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -283,6 +283,15 @@ func (n *graphNodeModuleFlatWrap) DependentOn() []string { return result } +func (n *graphNodeModuleFlatWrap) Proxy() bool { + pn, ok := n.graphNodeModuleWrappable.(GraphNodeProxy) + if !ok { + return false + } + + return pn.Proxy() +} + func (n *graphNodeModuleFlatWrap) prefixList(result []string, prefix string) { for i, v := range result { result[i] = fmt.Sprintf("%s.%s", prefix, v) diff --git a/terraform/graph_config_node_test.go b/terraform/graph_config_node_test.go index b297c6c1396e..977f5c25fc9b 100644 --- a/terraform/graph_config_node_test.go +++ b/terraform/graph_config_node_test.go @@ -13,6 +13,7 @@ func TestGraphNodeConfigOutput_impl(t *testing.T) { var _ dag.NamedVertex = new(GraphNodeConfigOutput) var _ graphNodeConfig = new(GraphNodeConfigOutput) var _ GraphNodeOutput = new(GraphNodeConfigOutput) + var _ GraphNodeProxy = new(GraphNodeConfigOutput) } func TestGraphNodeConfigProvider_impl(t *testing.T) { @@ -113,4 +114,5 @@ func TestGraphNodeConfigVariable_impl(t *testing.T) { var _ dag.NamedVertex = new(GraphNodeConfigVariable) var _ graphNodeConfig = new(GraphNodeConfigVariable) var _ GraphNodeVariable = new(GraphNodeConfigVariable) + var _ GraphNodeProxy = new(GraphNodeConfigVariable) } diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index b6e5390062b9..964b7b0067de 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -61,3 +61,8 @@ func (n *GraphNodeConfigVariable) VariableName() string { func (n *GraphNodeConfigVariable) SetVariableValue(v *config.RawConfig) { n.Value = v } + +// GraphNodeProxy impl. +func (n *GraphNodeConfigVariable) Proxy() bool { + return true +} diff --git a/terraform/transform_flatten_test.go b/terraform/transform_flatten_test.go index eddad8897ba3..749102469720 100644 --- a/terraform/transform_flatten_test.go +++ b/terraform/transform_flatten_test.go @@ -35,6 +35,37 @@ func TestFlattenTransformer(t *testing.T) { } } +func TestFlattenTransformer_withProxy(t *testing.T) { + mod := testModule(t, "transform-flatten") + + var b BasicGraphBuilder + b = BasicGraphBuilder{ + Steps: []GraphTransformer{ + &ConfigTransformer{Module: mod}, + &VertexTransformer{ + Transforms: []GraphVertexTransformer{ + &ExpandTransform{ + Builder: &b, + }, + }, + }, + &FlattenTransformer{}, + &ProxyTransformer{}, + }, + } + + g, err := b.Build(rootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformFlattenProxyStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + const testTransformFlattenStr = ` aws_instance.parent aws_instance.parent-output @@ -46,3 +77,17 @@ module.child.output.output module.child.var.var aws_instance.parent ` + +const testTransformFlattenProxyStr = ` +aws_instance.parent +aws_instance.parent-output + module.child.aws_instance.child + module.child.output.output +module.child.aws_instance.child + aws_instance.parent + module.child.var.var +module.child.output.output + module.child.aws_instance.child +module.child.var.var + aws_instance.parent +` From 9a54dddc85ca92e97dfd01e0f3312ef7247d6373 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 14:10:41 -0700 Subject: [PATCH 14/33] terraform: eval for variables --- terraform/eval_context_builtin.go | 6 ++++- terraform/graph_config_node_variable.go | 30 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 7e1eac310af0..d355e36ef150 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -29,7 +29,8 @@ type BuiltinEvalContext struct { StateValue *State StateLock *sync.RWMutex - once sync.Once + once sync.Once + varLock sync.Mutex } func (ctx *BuiltinEvalContext) Hook(fn func(Hook) (HookAction, error)) error { @@ -238,6 +239,9 @@ func (ctx *BuiltinEvalContext) Path() []string { } func (ctx *BuiltinEvalContext) SetVariables(vs map[string]string) { + ctx.varLock.Lock() + defer ctx.varLock.Unlock() + for k, v := range vs { ctx.Interpolater.Variables[k] = v } diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index 964b7b0067de..46aeff4a3595 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -66,3 +66,33 @@ func (n *GraphNodeConfigVariable) SetVariableValue(v *config.RawConfig) { func (n *GraphNodeConfigVariable) Proxy() bool { return true } + +// GraphNodeEvalable impl. +func (n *GraphNodeConfigVariable) EvalTree() EvalNode { + // If we have no value, do nothing + if n.Value == nil { + return &EvalNoop{} + } + + // Otherwise, interpolate the value of this variable and set it + // within the variables mapping. + var config *ResourceConfig + variables := make(map[string]string) + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.Value, + Output: &config, + }, + + &EvalVariableBlock{ + Config: &config, + Variables: variables, + }, + + &EvalSetVariables{ + Variables: variables, + }, + }, + } +} From f2e7f505d49cce8934bfb6550b6b99417e3cec81 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 14:19:32 -0700 Subject: [PATCH 15/33] terraform: subpath context setting --- terraform/graph.go | 17 +++++++++++++---- terraform/graph_config_node_module.go | 9 +++++++-- terraform/graph_interface_subgraph.go | 7 +++++++ terraform/graph_walk.go | 8 ++++---- terraform/graph_walk_context.go | 8 ++++---- 5 files changed, 35 insertions(+), 14 deletions(-) create mode 100644 terraform/graph_interface_subgraph.go diff --git a/terraform/graph.go b/terraform/graph.go index cf5439097e69..a86e0ca1ecff 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -144,8 +144,8 @@ func (g *Graph) init() { func (g *Graph) walk(walker GraphWalker) error { // The callbacks for enter/exiting a graph - ctx := walker.EnterGraph(g) - defer walker.ExitGraph(g) + ctx := walker.EnterPath(g.Path) + defer walker.ExitPath(g.Path) // Get the path for logs path := strings.Join(ctx.Path(), ".") @@ -158,6 +158,15 @@ func (g *Graph) walk(walker GraphWalker) error { walker.EnterVertex(v) defer func() { walker.ExitVertex(v, rerr) }() + // vertexCtx is the context that we use when evaluating. This + // is normally the context of our graph but can be overridden + // with a GraphNodeSubPath impl. + vertexCtx := ctx + if pn, ok := v.(GraphNodeSubPath); ok { + vertexCtx = walker.EnterPath(pn.Path()) + defer walker.ExitPath(pn.Path()) + } + // If the node is eval-able, then evaluate it. if ev, ok := v.(GraphNodeEvalable); ok { tree := ev.EvalTree() @@ -170,7 +179,7 @@ func (g *Graph) walk(walker GraphWalker) error { // then callback with the output. log.Printf("[DEBUG] vertex %s.%s: evaluating", path, dag.VertexName(v)) tree = walker.EnterEvalTree(v, tree) - output, err := Eval(tree, ctx) + output, err := Eval(tree, vertexCtx) if rerr = walker.ExitEvalTree(v, output, err); rerr != nil { return } @@ -182,7 +191,7 @@ func (g *Graph) walk(walker GraphWalker) error { "[DEBUG] vertex %s.%s: expanding/walking dynamic subgraph", path, dag.VertexName(v)) - g, err := ev.DynamicExpand(ctx) + g, err := ev.DynamicExpand(vertexCtx) if err != nil { rerr = err return diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 15fa29ace0ca..28cca774925f 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -227,7 +227,7 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { graph.Replace(v, &graphNodeModuleFlatWrap{ graphNodeModuleWrappable: wn, - Path: graph.Path, + PathValue: graph.Path, NamePrefix: pathStr, DependentOnPrefix: depPrefix, }) @@ -259,7 +259,7 @@ type graphNodeModuleWrappable interface { type graphNodeModuleFlatWrap struct { graphNodeModuleWrappable - Path []string + PathValue []string NamePrefix string DependentOnPrefix string } @@ -268,6 +268,11 @@ func (n *graphNodeModuleFlatWrap) Name() string { return fmt.Sprintf("%s.%s", n.NamePrefix, n.graphNodeModuleWrappable.Name()) } +// GraphNodeSubPath impl. +func (n *graphNodeModuleFlatWrap) Path() []string { + return n.PathValue +} + func (n *graphNodeModuleFlatWrap) DependableName() []string { result := n.graphNodeModuleWrappable.DependableName() n.prefixList(result, n.NamePrefix) diff --git a/terraform/graph_interface_subgraph.go b/terraform/graph_interface_subgraph.go new file mode 100644 index 000000000000..2897eb546adf --- /dev/null +++ b/terraform/graph_interface_subgraph.go @@ -0,0 +1,7 @@ +package terraform + +// GraphNodeSubPath says that a node is part of a graph with a +// different path, and the context should be adjusted accordingly. +type GraphNodeSubPath interface { + Path() []string +} diff --git a/terraform/graph_walk.go b/terraform/graph_walk.go index f5da6c093b94..ef3a4f6f5477 100644 --- a/terraform/graph_walk.go +++ b/terraform/graph_walk.go @@ -7,8 +7,8 @@ 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 { - EnterGraph(*Graph) EvalContext - ExitGraph(*Graph) + EnterPath([]string) EvalContext + ExitPath([]string) EnterVertex(dag.Vertex) ExitVertex(dag.Vertex, error) EnterEvalTree(dag.Vertex, EvalNode) EvalNode @@ -20,8 +20,8 @@ type GraphWalker interface { // implementing all the required functions. type NullGraphWalker struct{} -func (NullGraphWalker) EnterGraph(*Graph) EvalContext { return nil } -func (NullGraphWalker) ExitGraph(*Graph) {} +func (NullGraphWalker) EnterPath([]string) EvalContext { return nil } +func (NullGraphWalker) ExitPath([]string) {} func (NullGraphWalker) EnterVertex(dag.Vertex) {} func (NullGraphWalker) ExitVertex(dag.Vertex, error) {} func (NullGraphWalker) EnterEvalTree(v dag.Vertex, n EvalNode) EvalNode { return n } diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index 64a9f3d02ce3..508a8edc348c 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -33,14 +33,14 @@ type ContextGraphWalker struct { provisionerLock sync.Mutex } -func (w *ContextGraphWalker) EnterGraph(g *Graph) EvalContext { +func (w *ContextGraphWalker) EnterPath(path []string) EvalContext { w.once.Do(w.init) w.contextLock.Lock() defer w.contextLock.Unlock() // If we already have a context for this path cached, use that - key := PathCacheKey(g.Path) + key := PathCacheKey(path) if ctx, ok := w.contexts[key]; ok { return ctx } @@ -49,13 +49,13 @@ func (w *ContextGraphWalker) EnterGraph(g *Graph) EvalContext { // 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(g.Path) > 1 { + if len(path) > 1 { // We're in a submodule, the variables should be empty variables = make(map[string]string) } ctx := &BuiltinEvalContext{ - PathValue: g.Path, + PathValue: path, Hooks: w.Context.hooks, InputValue: w.Context.uiInput, Providers: w.Context.providers, From 86d07d3b5b47a7ad7e56ae4c27aa4ef5238602d0 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 15:18:40 -0700 Subject: [PATCH 16/33] terraform: redo how flattening works --- terraform/graph_builder.go | 6 + terraform/graph_config_node.go | 434 ------------------- terraform/graph_config_node_module.go | 101 ++--- terraform/graph_config_node_module_test.go | 72 +-- terraform/graph_config_node_output.go | 99 +++++ terraform/graph_config_node_resource.go | 426 ++++++++++++++++++ terraform/graph_config_node_test.go | 8 - terraform/graph_config_node_variable.go | 44 ++ terraform/graph_config_node_variable_test.go | 23 + terraform/transform_flatten.go | 47 +- terraform/transform_provider.go | 4 +- 11 files changed, 674 insertions(+), 590 deletions(-) create mode 100644 terraform/graph_config_node_output.go create mode 100644 terraform/graph_config_node_resource.go create mode 100644 terraform/graph_config_node_variable_test.go diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index 767374ddabb4..addb8baca779 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -134,6 +134,12 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer { }, }, + // Flatten stuff + &FlattenTransformer{}, + + // Make sure all the connections that are proxies are connected through + &ProxyTransformer{}, + // Optionally reduces the graph to a user-specified list of targets and // their dependencies. &TargetsTransformer{Targets: b.Targets, Destroy: b.Destroy}, diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 55c90df8b85b..341e1a108a98 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -2,7 +2,6 @@ package terraform import ( "fmt" - "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" @@ -45,60 +44,6 @@ type GraphNodeTargetable interface { SetTargets([]ResourceAddress) } -// GraphNodeConfigOutput represents an output configured within the -// configuration. -type GraphNodeConfigOutput struct { - Output *config.Output -} - -func (n *GraphNodeConfigOutput) Name() string { - return fmt.Sprintf("output.%s", n.Output.Name) -} - -func (n *GraphNodeConfigOutput) ConfigType() GraphNodeConfigType { - return GraphNodeConfigTypeOutput -} - -func (n *GraphNodeConfigOutput) OutputName() string { - return n.Output.Name -} - -func (n *GraphNodeConfigOutput) DependableName() []string { - return []string{n.Name()} -} - -func (n *GraphNodeConfigOutput) DependentOn() []string { - vars := n.Output.RawConfig.Variables - result := make([]string, 0, len(vars)) - for _, v := range vars { - if vn := varNameForVar(v); vn != "" { - result = append(result, vn) - } - } - - return result -} - -// GraphNodeEvalable impl. -func (n *GraphNodeConfigOutput) EvalTree() EvalNode { - return &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkPlan, walkApply}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalWriteOutput{ - Name: n.Output.Name, - Value: n.Output.RawConfig, - }, - }, - }, - } -} - -// GraphNodeProxy impl. -func (n *GraphNodeConfigOutput) Proxy() bool { - return true -} - // GraphNodeConfigProvider represents a configured provider within the // configuration graph. These are only immediately in the graph when an // explicit `provider` configuration block is in the configuration. @@ -161,382 +106,3 @@ func (n *GraphNodeConfigProvider) DotNode(name string, opts *GraphDotOpts) *dot. func (n *GraphNodeConfigProvider) DotOrigin() bool { return true } - -// GraphNodeConfigResource represents a resource within the config graph. -type GraphNodeConfigResource struct { - Resource *config.Resource - - // If this is set to anything other than destroyModeNone, then this - // resource represents a resource that will be destroyed in some way. - DestroyMode GraphNodeDestroyMode - - // Used during DynamicExpand to target indexes - Targets []ResourceAddress -} - -func (n *GraphNodeConfigResource) ConfigType() GraphNodeConfigType { - return GraphNodeConfigTypeResource -} - -func (n *GraphNodeConfigResource) DependableName() []string { - return []string{n.Resource.Id()} -} - -// GraphNodeDependent impl. -func (n *GraphNodeConfigResource) DependentOn() []string { - result := make([]string, len(n.Resource.DependsOn), - (len(n.Resource.RawCount.Variables)+ - len(n.Resource.RawConfig.Variables)+ - len(n.Resource.DependsOn))*2) - copy(result, n.Resource.DependsOn) - - for _, v := range n.Resource.RawCount.Variables { - if vn := varNameForVar(v); vn != "" { - result = append(result, vn) - } - } - for _, v := range n.Resource.RawConfig.Variables { - if vn := varNameForVar(v); vn != "" { - result = append(result, vn) - } - } - for _, p := range n.Resource.Provisioners { - for _, v := range p.ConnInfo.Variables { - if vn := varNameForVar(v); vn != "" && vn != n.Resource.Id() { - result = append(result, vn) - } - } - for _, v := range p.RawConfig.Variables { - if vn := varNameForVar(v); vn != "" && vn != n.Resource.Id() { - result = append(result, vn) - } - } - } - - return result -} - -// VarWalk calls a callback for all the variables that this resource -// depends on. -func (n *GraphNodeConfigResource) VarWalk(fn func(config.InterpolatedVariable)) { - for _, v := range n.Resource.RawCount.Variables { - fn(v) - } - for _, v := range n.Resource.RawConfig.Variables { - fn(v) - } - for _, p := range n.Resource.Provisioners { - for _, v := range p.ConnInfo.Variables { - fn(v) - } - for _, v := range p.RawConfig.Variables { - fn(v) - } - } -} - -func (n *GraphNodeConfigResource) Name() string { - result := n.Resource.Id() - switch n.DestroyMode { - case DestroyNone: - case DestroyPrimary: - result += " (destroy)" - case DestroyTainted: - result += " (destroy tainted)" - default: - result += " (unknown destroy type)" - } - - return result -} - -// GraphNodeDotter impl. -func (n *GraphNodeConfigResource) DotNode(name string, opts *GraphDotOpts) *dot.Node { - if n.DestroyMode != DestroyNone && !opts.Verbose { - return nil - } - return dot.NewNode(name, map[string]string{ - "label": n.Name(), - "shape": "box", - }) -} - -// GraphNodeDynamicExpandable impl. -func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) { - state, lock := ctx.State() - lock.RLock() - defer lock.RUnlock() - - // Start creating the steps - steps := make([]GraphTransformer, 0, 5) - - // Primary and non-destroy modes are responsible for creating/destroying - // all the nodes, expanding counts. - switch n.DestroyMode { - case DestroyNone: - fallthrough - case DestroyPrimary: - steps = append(steps, &ResourceCountTransformer{ - Resource: n.Resource, - Destroy: n.DestroyMode != DestroyNone, - Targets: n.Targets, - }) - } - - // Additional destroy modifications. - switch n.DestroyMode { - case DestroyPrimary: - // If we're destroying the primary instance, then we want to - // expand orphans, which have all the same semantics in a destroy - // as a primary. - steps = append(steps, &OrphanTransformer{ - State: state, - View: n.Resource.Id(), - Targeting: (len(n.Targets) > 0), - }) - - steps = append(steps, &DeposedTransformer{ - State: state, - View: n.Resource.Id(), - }) - case DestroyTainted: - // If we're only destroying tainted resources, then we only - // want to find tainted resources and destroy them here. - steps = append(steps, &TaintedTransformer{ - State: state, - View: n.Resource.Id(), - }) - } - - // Always end with the root being added - steps = append(steps, &RootTransformer{}) - - // Build the graph - b := &BasicGraphBuilder{Steps: steps} - return b.Build(ctx.Path()) -} - -// GraphNodeAddressable impl. -func (n *GraphNodeConfigResource) ResourceAddress() *ResourceAddress { - return &ResourceAddress{ - // Indicates no specific index; will match on other three fields - Index: -1, - InstanceType: TypePrimary, - Name: n.Resource.Name, - Type: n.Resource.Type, - } -} - -// GraphNodeTargetable impl. -func (n *GraphNodeConfigResource) SetTargets(targets []ResourceAddress) { - n.Targets = targets -} - -// GraphNodeEvalable impl. -func (n *GraphNodeConfigResource) EvalTree() EvalNode { - return &EvalSequence{ - Nodes: []EvalNode{ - &EvalInterpolate{Config: n.Resource.RawCount}, - &EvalOpFilter{ - Ops: []walkOperation{walkValidate}, - Node: &EvalValidateCount{Resource: n.Resource}, - }, - &EvalCountFixZeroOneBoundary{Resource: n.Resource}, - }, - } -} - -// GraphNodeProviderConsumer -func (n *GraphNodeConfigResource) ProvidedBy() []string { - return []string{resourceProvider(n.Resource.Type, n.Resource.Provider)} -} - -// GraphNodeProvisionerConsumer -func (n *GraphNodeConfigResource) ProvisionedBy() []string { - result := make([]string, len(n.Resource.Provisioners)) - for i, p := range n.Resource.Provisioners { - result[i] = p.Type - } - - return result -} - -// GraphNodeDestroyable -func (n *GraphNodeConfigResource) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { - // If we're already a destroy node, then don't do anything - if n.DestroyMode != DestroyNone { - return nil - } - - result := &graphNodeResourceDestroy{ - GraphNodeConfigResource: *n, - Original: n, - } - result.DestroyMode = mode - return result -} - -// graphNodeResourceDestroy represents the logical destruction of a -// resource. This node doesn't mean it will be destroyed for sure, but -// instead that if a destroy were to happen, it must happen at this point. -type graphNodeResourceDestroy struct { - GraphNodeConfigResource - Original *GraphNodeConfigResource -} - -func (n *graphNodeResourceDestroy) CreateBeforeDestroy() bool { - // CBD is enabled if the resource enables it in addition to us - // being responsible for destroying the primary state. The primary - // state destroy node is the only destroy node that needs to be - // "shuffled" according to the CBD rules, since tainted resources - // don't have the same inverse dependencies. - return n.Original.Resource.Lifecycle.CreateBeforeDestroy && - n.DestroyMode == DestroyPrimary -} - -func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex { - return n.Original -} - -func (n *graphNodeResourceDestroy) DestroyInclude(d *ModuleDiff, s *ModuleState) bool { - switch n.DestroyMode { - case DestroyPrimary: - return n.destroyIncludePrimary(d, s) - case DestroyTainted: - return n.destroyIncludeTainted(d, s) - default: - return true - } -} - -func (n *graphNodeResourceDestroy) destroyIncludeTainted( - d *ModuleDiff, s *ModuleState) bool { - // If there is no state, there can't by any tainted. - if s == nil { - return false - } - - // Grab the ID which is the prefix (in the case count > 0 at some point) - prefix := n.Original.Resource.Id() - - // Go through the resources and find any with our prefix. If there - // are any tainted, we need to keep it. - for k, v := range s.Resources { - if !strings.HasPrefix(k, prefix) { - continue - } - - if len(v.Tainted) > 0 { - return true - } - } - - // We didn't find any tainted nodes, return - return false -} - -func (n *graphNodeResourceDestroy) destroyIncludePrimary( - d *ModuleDiff, s *ModuleState) bool { - // Get the count, and specifically the raw value of the count - // (with interpolations and all). If the count is NOT a static "1", - // then we keep the destroy node no matter what. - // - // The reasoning for this is complicated and not intuitively obvious, - // but I attempt to explain it below. - // - // The destroy transform works by generating the worst case graph, - // with worst case being the case that every resource already exists - // and needs to be destroy/created (force-new). There is a single important - // edge case where this actually results in a real-life cycle: if a - // create-before-destroy (CBD) resource depends on a non-CBD resource. - // Imagine a EC2 instance "foo" with CBD depending on a security - // group "bar" without CBD, and conceptualize the worst case destroy - // order: - // - // 1.) SG must be destroyed (non-CBD) - // 2.) SG must be created/updated - // 3.) EC2 instance must be created (CBD, requires the SG be made) - // 4.) EC2 instance must be destroyed (requires SG be destroyed) - // - // Except, #1 depends on #4, since the SG can't be destroyed while - // an EC2 instance is using it (AWS API requirements). As you can see, - // this is a real life cycle that can't be automatically reconciled - // except under two conditions: - // - // 1.) SG is also CBD. This doesn't work 100% of the time though - // since the non-CBD resource might not support CBD. To make matters - // worse, the entire transitive closure of dependencies must be - // CBD (if the SG depends on a VPC, you have the same problem). - // 2.) EC2 must not CBD. This can't happen automatically because CBD - // is used as a way to ensure zero (or minimal) downtime Terraform - // applies, and it isn't acceptable for TF to ignore this request, - // since it can result in unexpected downtime. - // - // Therefore, we compromise with this edge case here: if there is - // a static count of "1", we prune the diff to remove cycles during a - // graph optimization path if we don't see the resource in the diff. - // If the count is set to ANYTHING other than a static "1" (variable, - // computed attribute, static number greater than 1), then we keep the - // destroy, since it is required for dynamic graph expansion to find - // orphan/tainted count objects. - // - // This isn't ideal logic, but its strictly better without introducing - // new impossibilities. It breaks the cycle in practical cases, and the - // cycle comes back in no cases we've found to be practical, but just - // as the cycle would already exist without this anyways. - count := n.Original.Resource.RawCount - if raw := count.Raw[count.Key]; raw != "1" { - return true - } - - // Okay, we're dealing with a static count. There are a few ways - // to include this resource. - prefix := n.Original.Resource.Id() - - // If we're present in the diff proper, then keep it. - if d != nil { - for k, _ := range d.Resources { - if strings.HasPrefix(k, prefix) { - return true - } - } - } - - // If we're in the state as a primary in any form, then keep it. - // This does a prefix check so it will also catch orphans on count - // decreases to "1". - if s != nil { - for k, v := range s.Resources { - // Ignore exact matches - if k == prefix { - continue - } - - // Ignore anything that doesn't have a "." afterwards so that - // we only get our own resource and any counts on it. - if !strings.HasPrefix(k, prefix+".") { - continue - } - - // Ignore exact matches and the 0'th index. We only care - // about if there is a decrease in count. - if k == prefix+".0" { - continue - } - - if v.Primary != nil { - return true - } - } - - // If we're in the state as _both_ "foo" and "foo.0", then - // keep it, since we treat the latter as an orphan. - _, okOne := s.Resources[prefix] - _, okTwo := s.Resources[prefix+".0"] - if okOne && okTwo { - return true - } - } - - return false -} diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 28cca774925f..3ca7ec1d0b13 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -166,22 +166,6 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { graph := n.Subgraph() input := n.Original.Module.RawConfig - // Build the string that represents the path. We do this once here - // so that we only have to compute it once. The block below is in {} - // so that parts drops out of scope immediately. - var pathStr, parentPathStr string - { - parts := make([]string, 0, len(graph.Path)*2) - for _, p := range graph.Path[1:] { - parts = append(parts, "module", p) - } - - pathStr = strings.Join(parts, ".") - if len(parts) > 2 { - parentPathStr = strings.Join(parts[0:len(parts)-2], ".") - } - } - // Go over each vertex in the graph and wrap the configuration // items so that the dependencies properly map to the modules. // See the docs for graphNodeModuleWrappable for more info. @@ -191,15 +175,6 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { continue } - wn, ok := v.(graphNodeModuleWrappable) - if !ok { - panic("unwrappable node: " + dag.VertexName(v)) - } - - // Prefix for dependencies. We set this to blank for variables - // (see below). - depPrefix := pathStr - // 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 { @@ -217,20 +192,7 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { // Set the variable value so it is interpolated properly vn.SetVariableValue(config) } - - // We set the dependency prefix to the parent path - // since variables can reference outside of our own graph - // to the parent graph. - depPrefix = parentPathStr } - - graph.Replace(v, &graphNodeModuleFlatWrap{ - graphNodeModuleWrappable: wn, - - PathValue: graph.Path, - NamePrefix: pathStr, - DependentOnPrefix: depPrefix, - }) } return graph @@ -247,58 +209,57 @@ type graphNodeModuleSkippable interface { FlattenSkip() bool } -// This is the interface that all nodes within a module graph must -// implement to be wrapped for flattening. -type graphNodeModuleWrappable interface { - graphNodeConfig -} - +/* // graphNodeModuleFlatWrap wraps elements of the module graph // when they are flattened so that the dependency information is // correct. type graphNodeModuleFlatWrap struct { - graphNodeModuleWrappable - + Vertex dag.Vertex PathValue []string NamePrefix string DependentOnPrefix string } -func (n *graphNodeModuleFlatWrap) Name() string { - return fmt.Sprintf("%s.%s", n.NamePrefix, n.graphNodeModuleWrappable.Name()) -} - -// GraphNodeSubPath impl. -func (n *graphNodeModuleFlatWrap) Path() []string { - return n.PathValue -} +// GraphNodeProvider impl. +func (n *graphNodeModuleFlatWrap) ProviderName() string { + pn, ok := n.Vertex.(GraphNodeProvider) + if !ok { + return "" + } -func (n *graphNodeModuleFlatWrap) DependableName() []string { - result := n.graphNodeModuleWrappable.DependableName() - n.prefixList(result, n.NamePrefix) - return result + return fmt.Sprintf("%s.%s", n.NamePrefix, pn.ProviderName()) } -func (n *graphNodeModuleFlatWrap) DependentOn() []string { - result := n.graphNodeModuleWrappable.DependentOn() - if n.DependentOnPrefix != "" { - n.prefixList(result, n.DependentOnPrefix) +// 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 (n *graphNodeModuleFlatWrap) Proxy() bool { - pn, ok := n.graphNodeModuleWrappable.(GraphNodeProxy) - if !ok { - return false +func modulePrefixStr(p []string) string { + parts := make([]string, 0, len(p)*2) + for _, p := range p[1:] { + parts = append(parts, "module", p) } - return pn.Proxy() + return strings.Join(parts, ".") } -func (n *graphNodeModuleFlatWrap) prefixList(result []string, prefix string) { - for i, v := range result { - result[i] = fmt.Sprintf("%s.%s", prefix, v) +func modulePrefixList(result []string, prefix string) []string { + if prefix != "" { + for i, v := range result { + result[i] = fmt.Sprintf("%s.%s", prefix, v) + } } + + return result } diff --git a/terraform/graph_config_node_module_test.go b/terraform/graph_config_node_module_test.go index 2806dff3c946..d3cbdfb1d5d3 100644 --- a/terraform/graph_config_node_module_test.go +++ b/terraform/graph_config_node_module_test.go @@ -1,7 +1,6 @@ package terraform import ( - "reflect" "strings" "testing" @@ -59,7 +58,7 @@ func TestGraphNodeConfigModuleExpandFlatten(t *testing.T) { t.Fatalf("err: %s", err) } - fg := g.(GraphNodeFlattenable) + fg := g.(GraphNodeFlatGraph) actual := strings.TrimSpace(fg.FlattenGraph().String()) expected := strings.TrimSpace(testGraphNodeModuleExpandFlattenStr) @@ -68,73 +67,6 @@ func TestGraphNodeConfigModuleExpandFlatten(t *testing.T) { } } -func TestGraphNodeModulFlatWrap_Name(t *testing.T) { - n := &graphNodeModuleFlatWrap{ - graphNodeModuleWrappable: &testGraphNodeModuleWrappable{ - NameValue: "foo", - }, - - NamePrefix: "module.bar", - } - - if v := n.Name(); v != "module.bar.foo" { - t.Fatalf("bad: %s", v) - } -} - -func TestGraphNodeModulFlatWrap_DependentOn(t *testing.T) { - n := &graphNodeModuleFlatWrap{ - graphNodeModuleWrappable: &testGraphNodeModuleWrappable{ - NameValue: "foo", - }, - - NamePrefix: "module.bar", - DependentOnPrefix: "module.bar", - } - - actual := n.DependentOn() - expected := []string{"module.bar.foo"} - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) - } -} - -func TestGraphNodeModulFlatWrap_DependableName(t *testing.T) { - n := &graphNodeModuleFlatWrap{ - graphNodeModuleWrappable: &testGraphNodeModuleWrappable{ - NameValue: "foo", - }, - - NamePrefix: "module.bar", - } - - actual := n.DependableName() - expected := []string{"module.bar.foo"} - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) - } -} - -type testGraphNodeModuleWrappable struct { - NameValue string -} - -func (n *testGraphNodeModuleWrappable) ConfigType() GraphNodeConfigType { - return GraphNodeConfigTypeInvalid -} - -func (n *testGraphNodeModuleWrappable) Name() string { - return n.NameValue -} - -func (n *testGraphNodeModuleWrappable) DependableName() []string { - return []string{"foo"} -} - -func (n *testGraphNodeModuleWrappable) DependentOn() []string { - return []string{"foo"} -} - const testGraphNodeModuleExpandStr = ` aws_instance.bar aws_instance.foo @@ -144,5 +76,5 @@ module inputs ` const testGraphNodeModuleExpandFlattenStr = ` -module.child.aws_instance.foo +aws_instance.foo ` diff --git a/terraform/graph_config_node_output.go b/terraform/graph_config_node_output.go new file mode 100644 index 000000000000..6a3533e37190 --- /dev/null +++ b/terraform/graph_config_node_output.go @@ -0,0 +1,99 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" +) + +// GraphNodeConfigOutput represents an output configured within the +// configuration. +type GraphNodeConfigOutput struct { + Output *config.Output +} + +func (n *GraphNodeConfigOutput) Name() string { + return fmt.Sprintf("output.%s", n.Output.Name) +} + +func (n *GraphNodeConfigOutput) ConfigType() GraphNodeConfigType { + return GraphNodeConfigTypeOutput +} + +func (n *GraphNodeConfigOutput) OutputName() string { + return n.Output.Name +} + +func (n *GraphNodeConfigOutput) DependableName() []string { + return []string{n.Name()} +} + +func (n *GraphNodeConfigOutput) DependentOn() []string { + vars := n.Output.RawConfig.Variables + result := make([]string, 0, len(vars)) + for _, v := range vars { + if vn := varNameForVar(v); vn != "" { + result = append(result, vn) + } + } + + return result +} + +// GraphNodeEvalable impl. +func (n *GraphNodeConfigOutput) EvalTree() EvalNode { + return &EvalOpFilter{ + Ops: []walkOperation{walkRefresh, walkPlan, walkApply}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalWriteOutput{ + Name: n.Output.Name, + Value: n.Output.RawConfig, + }, + }, + }, + } +} + +// GraphNodeProxy impl. +func (n *GraphNodeConfigOutput) Proxy() bool { + return true +} + +// GraphNodeFlattenable impl. +func (n *GraphNodeConfigOutput) Flatten(p []string) (dag.Vertex, error) { + return &GraphNodeConfigOutputFlat{ + GraphNodeConfigOutput: n, + PathValue: p, + }, nil +} + +// Same as GraphNodeConfigOutput, but for flattening +type GraphNodeConfigOutputFlat struct { + *GraphNodeConfigOutput + + PathValue []string +} + +func (n *GraphNodeConfigOutputFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.GraphNodeConfigOutput.Name()) +} + +func (n *GraphNodeConfigOutputFlat) Path() []string { + return n.PathValue +} + +func (n *GraphNodeConfigOutputFlat) DependableName() []string { + return modulePrefixList( + n.GraphNodeConfigOutput.DependableName(), + modulePrefixStr(n.PathValue)) +} + +func (n *GraphNodeConfigOutputFlat) DependentOn() []string { + prefix := modulePrefixStr(n.PathValue) + return modulePrefixList( + n.GraphNodeConfigOutput.DependentOn(), + prefix) +} diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go new file mode 100644 index 000000000000..0711205331d7 --- /dev/null +++ b/terraform/graph_config_node_resource.go @@ -0,0 +1,426 @@ +package terraform + +import ( + "fmt" + "strings" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/dot" +) + +// GraphNodeConfigResource represents a resource within the config graph. +type GraphNodeConfigResource struct { + Resource *config.Resource + + // If this is set to anything other than destroyModeNone, then this + // resource represents a resource that will be destroyed in some way. + DestroyMode GraphNodeDestroyMode + + // Used during DynamicExpand to target indexes + Targets []ResourceAddress +} + +func (n *GraphNodeConfigResource) ConfigType() GraphNodeConfigType { + return GraphNodeConfigTypeResource +} + +func (n *GraphNodeConfigResource) DependableName() []string { + return []string{n.Resource.Id()} +} + +// GraphNodeDependent impl. +func (n *GraphNodeConfigResource) DependentOn() []string { + result := make([]string, len(n.Resource.DependsOn), + (len(n.Resource.RawCount.Variables)+ + len(n.Resource.RawConfig.Variables)+ + len(n.Resource.DependsOn))*2) + copy(result, n.Resource.DependsOn) + + for _, v := range n.Resource.RawCount.Variables { + if vn := varNameForVar(v); vn != "" { + result = append(result, vn) + } + } + for _, v := range n.Resource.RawConfig.Variables { + if vn := varNameForVar(v); vn != "" { + result = append(result, vn) + } + } + for _, p := range n.Resource.Provisioners { + for _, v := range p.ConnInfo.Variables { + if vn := varNameForVar(v); vn != "" && vn != n.Resource.Id() { + result = append(result, vn) + } + } + for _, v := range p.RawConfig.Variables { + if vn := varNameForVar(v); vn != "" && vn != n.Resource.Id() { + result = append(result, vn) + } + } + } + + return result +} + +// VarWalk calls a callback for all the variables that this resource +// depends on. +func (n *GraphNodeConfigResource) VarWalk(fn func(config.InterpolatedVariable)) { + for _, v := range n.Resource.RawCount.Variables { + fn(v) + } + for _, v := range n.Resource.RawConfig.Variables { + fn(v) + } + for _, p := range n.Resource.Provisioners { + for _, v := range p.ConnInfo.Variables { + fn(v) + } + for _, v := range p.RawConfig.Variables { + fn(v) + } + } +} + +func (n *GraphNodeConfigResource) Name() string { + result := n.Resource.Id() + switch n.DestroyMode { + case DestroyNone: + case DestroyPrimary: + result += " (destroy)" + case DestroyTainted: + result += " (destroy tainted)" + default: + result += " (unknown destroy type)" + } + + return result +} + +// GraphNodeDotter impl. +func (n *GraphNodeConfigResource) DotNode(name string, opts *GraphDotOpts) *dot.Node { + if n.DestroyMode != DestroyNone && !opts.Verbose { + return nil + } + return dot.NewNode(name, map[string]string{ + "label": n.Name(), + "shape": "box", + }) +} + +// GraphNodeFlattenable impl. +func (n *GraphNodeConfigResource) Flatten(p []string) (dag.Vertex, error) { + return &GraphNodeConfigResourceFlat{ + GraphNodeConfigResource: n, + PathValue: p, + }, nil +} + +// GraphNodeDynamicExpandable impl. +func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) { + state, lock := ctx.State() + lock.RLock() + defer lock.RUnlock() + + // Start creating the steps + steps := make([]GraphTransformer, 0, 5) + + // Primary and non-destroy modes are responsible for creating/destroying + // all the nodes, expanding counts. + switch n.DestroyMode { + case DestroyNone: + fallthrough + case DestroyPrimary: + steps = append(steps, &ResourceCountTransformer{ + Resource: n.Resource, + Destroy: n.DestroyMode != DestroyNone, + Targets: n.Targets, + }) + } + + // Additional destroy modifications. + switch n.DestroyMode { + case DestroyPrimary: + // If we're destroying the primary instance, then we want to + // expand orphans, which have all the same semantics in a destroy + // as a primary. + steps = append(steps, &OrphanTransformer{ + State: state, + View: n.Resource.Id(), + Targeting: (len(n.Targets) > 0), + }) + + steps = append(steps, &DeposedTransformer{ + State: state, + View: n.Resource.Id(), + }) + case DestroyTainted: + // If we're only destroying tainted resources, then we only + // want to find tainted resources and destroy them here. + steps = append(steps, &TaintedTransformer{ + State: state, + View: n.Resource.Id(), + }) + } + + // Always end with the root being added + steps = append(steps, &RootTransformer{}) + + // Build the graph + b := &BasicGraphBuilder{Steps: steps} + return b.Build(ctx.Path()) +} + +// GraphNodeAddressable impl. +func (n *GraphNodeConfigResource) ResourceAddress() *ResourceAddress { + return &ResourceAddress{ + // Indicates no specific index; will match on other three fields + Index: -1, + InstanceType: TypePrimary, + Name: n.Resource.Name, + Type: n.Resource.Type, + } +} + +// GraphNodeTargetable impl. +func (n *GraphNodeConfigResource) SetTargets(targets []ResourceAddress) { + n.Targets = targets +} + +// GraphNodeEvalable impl. +func (n *GraphNodeConfigResource) EvalTree() EvalNode { + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{Config: n.Resource.RawCount}, + &EvalOpFilter{ + Ops: []walkOperation{walkValidate}, + Node: &EvalValidateCount{Resource: n.Resource}, + }, + &EvalCountFixZeroOneBoundary{Resource: n.Resource}, + }, + } +} + +// GraphNodeProviderConsumer +func (n *GraphNodeConfigResource) ProvidedBy() []string { + return []string{resourceProvider(n.Resource.Type, n.Resource.Provider)} +} + +// GraphNodeProvisionerConsumer +func (n *GraphNodeConfigResource) ProvisionedBy() []string { + result := make([]string, len(n.Resource.Provisioners)) + for i, p := range n.Resource.Provisioners { + result[i] = p.Type + } + + return result +} + +// GraphNodeDestroyable +func (n *GraphNodeConfigResource) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { + // If we're already a destroy node, then don't do anything + if n.DestroyMode != DestroyNone { + return nil + } + + result := &graphNodeResourceDestroy{ + GraphNodeConfigResource: *n, + Original: n, + } + result.DestroyMode = mode + return result +} + +// Same as GraphNodeConfigResource, but for flattening +type GraphNodeConfigResourceFlat struct { + *GraphNodeConfigResource + + PathValue []string +} + +func (n *GraphNodeConfigResourceFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.GraphNodeConfigResource.Name()) +} + +func (n *GraphNodeConfigResourceFlat) Path() []string { + return n.PathValue +} + +func (n *GraphNodeConfigResourceFlat) DependableName() []string { + return modulePrefixList( + n.GraphNodeConfigResource.DependableName(), + modulePrefixStr(n.PathValue)) +} + +func (n *GraphNodeConfigResourceFlat) DependentOn() []string { + prefix := modulePrefixStr(n.PathValue) + return modulePrefixList( + n.GraphNodeConfigResource.DependentOn(), + prefix) +} + +// graphNodeResourceDestroy represents the logical destruction of a +// resource. This node doesn't mean it will be destroyed for sure, but +// instead that if a destroy were to happen, it must happen at this point. +type graphNodeResourceDestroy struct { + GraphNodeConfigResource + Original *GraphNodeConfigResource +} + +func (n *graphNodeResourceDestroy) CreateBeforeDestroy() bool { + // CBD is enabled if the resource enables it in addition to us + // being responsible for destroying the primary state. The primary + // state destroy node is the only destroy node that needs to be + // "shuffled" according to the CBD rules, since tainted resources + // don't have the same inverse dependencies. + return n.Original.Resource.Lifecycle.CreateBeforeDestroy && + n.DestroyMode == DestroyPrimary +} + +func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex { + return n.Original +} + +func (n *graphNodeResourceDestroy) DestroyInclude(d *ModuleDiff, s *ModuleState) bool { + switch n.DestroyMode { + case DestroyPrimary: + return n.destroyIncludePrimary(d, s) + case DestroyTainted: + return n.destroyIncludeTainted(d, s) + default: + return true + } +} + +func (n *graphNodeResourceDestroy) destroyIncludeTainted( + d *ModuleDiff, s *ModuleState) bool { + // If there is no state, there can't by any tainted. + if s == nil { + return false + } + + // Grab the ID which is the prefix (in the case count > 0 at some point) + prefix := n.Original.Resource.Id() + + // Go through the resources and find any with our prefix. If there + // are any tainted, we need to keep it. + for k, v := range s.Resources { + if !strings.HasPrefix(k, prefix) { + continue + } + + if len(v.Tainted) > 0 { + return true + } + } + + // We didn't find any tainted nodes, return + return false +} + +func (n *graphNodeResourceDestroy) destroyIncludePrimary( + d *ModuleDiff, s *ModuleState) bool { + // Get the count, and specifically the raw value of the count + // (with interpolations and all). If the count is NOT a static "1", + // then we keep the destroy node no matter what. + // + // The reasoning for this is complicated and not intuitively obvious, + // but I attempt to explain it below. + // + // The destroy transform works by generating the worst case graph, + // with worst case being the case that every resource already exists + // and needs to be destroy/created (force-new). There is a single important + // edge case where this actually results in a real-life cycle: if a + // create-before-destroy (CBD) resource depends on a non-CBD resource. + // Imagine a EC2 instance "foo" with CBD depending on a security + // group "bar" without CBD, and conceptualize the worst case destroy + // order: + // + // 1.) SG must be destroyed (non-CBD) + // 2.) SG must be created/updated + // 3.) EC2 instance must be created (CBD, requires the SG be made) + // 4.) EC2 instance must be destroyed (requires SG be destroyed) + // + // Except, #1 depends on #4, since the SG can't be destroyed while + // an EC2 instance is using it (AWS API requirements). As you can see, + // this is a real life cycle that can't be automatically reconciled + // except under two conditions: + // + // 1.) SG is also CBD. This doesn't work 100% of the time though + // since the non-CBD resource might not support CBD. To make matters + // worse, the entire transitive closure of dependencies must be + // CBD (if the SG depends on a VPC, you have the same problem). + // 2.) EC2 must not CBD. This can't happen automatically because CBD + // is used as a way to ensure zero (or minimal) downtime Terraform + // applies, and it isn't acceptable for TF to ignore this request, + // since it can result in unexpected downtime. + // + // Therefore, we compromise with this edge case here: if there is + // a static count of "1", we prune the diff to remove cycles during a + // graph optimization path if we don't see the resource in the diff. + // If the count is set to ANYTHING other than a static "1" (variable, + // computed attribute, static number greater than 1), then we keep the + // destroy, since it is required for dynamic graph expansion to find + // orphan/tainted count objects. + // + // This isn't ideal logic, but its strictly better without introducing + // new impossibilities. It breaks the cycle in practical cases, and the + // cycle comes back in no cases we've found to be practical, but just + // as the cycle would already exist without this anyways. + count := n.Original.Resource.RawCount + if raw := count.Raw[count.Key]; raw != "1" { + return true + } + + // Okay, we're dealing with a static count. There are a few ways + // to include this resource. + prefix := n.Original.Resource.Id() + + // If we're present in the diff proper, then keep it. + if d != nil { + for k, _ := range d.Resources { + if strings.HasPrefix(k, prefix) { + return true + } + } + } + + // If we're in the state as a primary in any form, then keep it. + // This does a prefix check so it will also catch orphans on count + // decreases to "1". + if s != nil { + for k, v := range s.Resources { + // Ignore exact matches + if k == prefix { + continue + } + + // Ignore anything that doesn't have a "." afterwards so that + // we only get our own resource and any counts on it. + if !strings.HasPrefix(k, prefix+".") { + continue + } + + // Ignore exact matches and the 0'th index. We only care + // about if there is a decrease in count. + if k == prefix+".0" { + continue + } + + if v.Primary != nil { + return true + } + } + + // If we're in the state as _both_ "foo" and "foo.0", then + // keep it, since we treat the latter as an orphan. + _, okOne := s.Resources[prefix] + _, okTwo := s.Resources[prefix+".0"] + if okOne && okTwo { + return true + } + } + + return false +} diff --git a/terraform/graph_config_node_test.go b/terraform/graph_config_node_test.go index 977f5c25fc9b..670f07b39d28 100644 --- a/terraform/graph_config_node_test.go +++ b/terraform/graph_config_node_test.go @@ -108,11 +108,3 @@ func TestGraphNodeConfigResource_ProvisionedBy(t *testing.T) { t.Fatalf("bad: %#v", actual) } } - -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) -} diff --git a/terraform/graph_config_node_variable.go b/terraform/graph_config_node_variable.go index 46aeff4a3595..ec98883b510f 100644 --- a/terraform/graph_config_node_variable.go +++ b/terraform/graph_config_node_variable.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" ) // GraphNodeVariable is the interface that must be implemented by anything @@ -20,6 +21,8 @@ type GraphNodeConfigVariable struct { // 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 + + depPrefix string } func (n *GraphNodeConfigVariable) Name() string { @@ -96,3 +99,44 @@ func (n *GraphNodeConfigVariable) EvalTree() EvalNode { }, } } + +// GraphNodeFlattenable impl. +func (n *GraphNodeConfigVariable) Flatten(p []string) (dag.Vertex, error) { + return &GraphNodeConfigVariableFlat{ + GraphNodeConfigVariable: n, + PathValue: p, + }, nil +} + +type GraphNodeConfigVariableFlat struct { + *GraphNodeConfigVariable + + PathValue []string +} + +func (n *GraphNodeConfigVariableFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.GraphNodeConfigVariable.Name()) +} + +func (n *GraphNodeConfigVariableFlat) DependableName() []string { + return []string{n.Name()} +} + +func (n *GraphNodeConfigVariableFlat) DependentOn() []string { + // We only wrap the dependencies and such if we have a path that is + // longer than 2 elements (root, child, more). This is because when + // flattened, variables can point outside the graph. + prefix := "" + if len(n.PathValue) <= 2 { + prefix = modulePrefixStr(n.PathValue[:len(n.PathValue)-1]) + } + + return modulePrefixList( + n.GraphNodeConfigVariable.DependentOn(), + prefix) +} + +func (n *GraphNodeConfigVariableFlat) Path() []string { + return n.PathValue +} diff --git a/terraform/graph_config_node_variable_test.go b/terraform/graph_config_node_variable_test.go new file mode 100644 index 000000000000..41cf75c370bb --- /dev/null +++ b/terraform/graph_config_node_variable_test.go @@ -0,0 +1,23 @@ +package terraform + +import ( + "testing" + + "github.com/hashicorp/terraform/dag" +) + +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) +} + +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/transform_flatten.go b/terraform/transform_flatten.go index 995524dff780..4f9df8fb84e4 100644 --- a/terraform/transform_flatten.go +++ b/terraform/transform_flatten.go @@ -1,15 +1,27 @@ package terraform import ( + "fmt" + "github.com/hashicorp/terraform/dag" ) -// GraphNodeFlattenable must be implemented by nodes that can be flattened -// into the graph. -type GraphNodeFlattenable interface { +// GraphNodeFlatGraph must be implemented by nodes that have subgraphs +// that they want flattened into the graph. +type GraphNodeFlatGraph interface { FlattenGraph() *Graph } +// GraphNodeFlattenable must be implemented by all nodes that can be +// flattened. If a FlattenGraph returns any nodes that can't be flattened, +// it will be an error. +// +// If Flatten returns nil for the Vertex along with a nil error, it will +// removed from the graph. +type GraphNodeFlattenable interface { + Flatten(path []string) (dag.Vertex, error) +} + // FlattenTransformer is a transformer that goes through the graph, finds // subgraphs that can be flattened, and flattens them into this graph, // removing the prior subgraph node. @@ -17,7 +29,7 @@ type FlattenTransformer struct{} func (t *FlattenTransformer) Transform(g *Graph) error { for _, v := range g.Vertices() { - fn, ok := v.(GraphNodeFlattenable) + fn, ok := v.(GraphNodeFlatGraph) if !ok { continue } @@ -39,8 +51,31 @@ func (t *FlattenTransformer) Transform(g *Graph) error { // Remove the old node g.Remove(v) - // Flatten the subgraph into this one. Keep any existing - // connections that existed. + // Go through the subgraph and flatten all the nodes + for _, sv := range subgraph.Vertices() { + fn, ok := sv.(GraphNodeFlattenable) + if !ok { + return fmt.Errorf( + "unflattenable node: %s %T", + dag.VertexName(sv), sv) + } + + v, err := fn.Flatten(subgraph.Path) + if err != nil { + return fmt.Errorf( + "error flattening %s (%T): %s", + dag.VertexName(sv), sv, err) + } + + if v == nil { + subgraph.Remove(v) + } else { + subgraph.Replace(sv, v) + } + } + + // Now that we've handled any changes to the graph that are + // needed, we can add them all to our graph along with their edges. for _, sv := range subgraph.Vertices() { g.Add(sv) } diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 664c1b032786..d016267612da 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -32,7 +32,7 @@ func (t *DisableProviderTransformer) Transform(g *Graph) error { for _, v := range g.Vertices() { // We only care about providers pn, ok := v.(GraphNodeProvider) - if !ok { + if !ok || pn.ProviderName() == "" { continue } @@ -130,7 +130,7 @@ type PruneProviderTransformer struct{} func (t *PruneProviderTransformer) Transform(g *Graph) error { for _, v := range g.Vertices() { // We only care about the providers - if _, ok := v.(GraphNodeProvider); !ok { + if pn, ok := v.(GraphNodeProvider); !ok || pn.ProviderName() == "" { continue } From 416e7a2077ff65b5b5a6929f073ab6125393502e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 15:23:37 -0700 Subject: [PATCH 17/33] terraform: missing provider should be flattenable --- terraform/graph_config_node_module.go | 10 --------- terraform/transform_provider.go | 30 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 3ca7ec1d0b13..3d046a2ef2e1 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -220,16 +220,6 @@ type graphNodeModuleFlatWrap struct { DependentOnPrefix string } -// GraphNodeProvider impl. -func (n *graphNodeModuleFlatWrap) ProviderName() string { - pn, ok := n.Vertex.(GraphNodeProvider) - if !ok { - return "" - } - - return fmt.Sprintf("%s.%s", n.NamePrefix, pn.ProviderName()) -} - // GraphNodeProviderConsumer impl. func (n *graphNodeModuleFlatWrap) ProvidedBy() []string { pn, ok := n.Vertex.(GraphNodeProviderConsumer) diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index d016267612da..0b625e5fa0f7 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -224,6 +224,14 @@ func (n *graphNodeMissingProvider) DotOrigin() bool { return true } +// GraphNodeFlattenable impl. +func (n *graphNodeMissingProvider) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeMissingProviderFlat{ + graphNodeMissingProvider: n, + PathValue: p, + }, nil +} + func providerVertexMap(g *Graph) map[string]dag.Vertex { m := make(map[string]dag.Vertex) for _, v := range g.Vertices() { @@ -234,3 +242,25 @@ func providerVertexMap(g *Graph) map[string]dag.Vertex { return m } + +// Same as graphNodeMissingProvider, but for flattening +type graphNodeMissingProviderFlat struct { + *graphNodeMissingProvider + + PathValue []string +} + +func (n *graphNodeMissingProviderFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeMissingProvider.Name()) +} + +func (n *graphNodeMissingProviderFlat) Path() []string { + return n.PathValue +} + +func (n *graphNodeMissingProviderFlat) ProviderName() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), + n.graphNodeMissingProvider.ProviderName()) +} From 94e1bab65d381e09f8ba6bb9ca72cd73f9b04be1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 15:28:41 -0700 Subject: [PATCH 18/33] terraform: fill in more flat interfaces --- terraform/graph_config_node.go | 67 -------------- terraform/graph_config_node_provider.go | 115 ++++++++++++++++++++++++ terraform/graph_config_node_resource.go | 7 ++ terraform/transform_root.go | 4 + 4 files changed, 126 insertions(+), 67 deletions(-) create mode 100644 terraform/graph_config_node_provider.go diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 341e1a108a98..ea4645d721a3 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -1,11 +1,7 @@ package terraform import ( - "fmt" - - "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/dag" - "github.com/hashicorp/terraform/dot" ) // graphNodeConfig is an interface that all graph nodes for the @@ -43,66 +39,3 @@ type GraphNodeTargetable interface { SetTargets([]ResourceAddress) } - -// GraphNodeConfigProvider represents a configured provider within the -// configuration graph. These are only immediately in the graph when an -// explicit `provider` configuration block is in the configuration. -type GraphNodeConfigProvider struct { - Provider *config.ProviderConfig -} - -func (n *GraphNodeConfigProvider) Name() string { - return fmt.Sprintf("provider.%s", n.ProviderName()) -} - -func (n *GraphNodeConfigProvider) ConfigType() GraphNodeConfigType { - return GraphNodeConfigTypeProvider -} - -func (n *GraphNodeConfigProvider) DependableName() []string { - return []string{n.Name()} -} - -func (n *GraphNodeConfigProvider) DependentOn() []string { - vars := n.Provider.RawConfig.Variables - result := make([]string, 0, len(vars)) - for _, v := range vars { - if vn := varNameForVar(v); vn != "" { - result = append(result, vn) - } - } - - return result -} - -// GraphNodeEvalable impl. -func (n *GraphNodeConfigProvider) EvalTree() EvalNode { - return ProviderEvalTree(n.ProviderName(), n.Provider.RawConfig) -} - -// GraphNodeProvider implementation -func (n *GraphNodeConfigProvider) ProviderName() string { - if n.Provider.Alias == "" { - return n.Provider.Name - } else { - return fmt.Sprintf("%s.%s", n.Provider.Name, n.Provider.Alias) - } -} - -// GraphNodeProvider implementation -func (n *GraphNodeConfigProvider) ProviderConfig() *config.RawConfig { - return n.Provider.RawConfig -} - -// GraphNodeDotter impl. -func (n *GraphNodeConfigProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node { - return dot.NewNode(name, map[string]string{ - "label": n.Name(), - "shape": "diamond", - }) -} - -// GraphNodeDotterOrigin impl. -func (n *GraphNodeConfigProvider) DotOrigin() bool { - return true -} diff --git a/terraform/graph_config_node_provider.go b/terraform/graph_config_node_provider.go new file mode 100644 index 000000000000..5b8fb1bcb4b1 --- /dev/null +++ b/terraform/graph_config_node_provider.go @@ -0,0 +1,115 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/dot" +) + +// GraphNodeConfigProvider represents a configured provider within the +// configuration graph. These are only immediately in the graph when an +// explicit `provider` configuration block is in the configuration. +type GraphNodeConfigProvider struct { + Provider *config.ProviderConfig +} + +func (n *GraphNodeConfigProvider) Name() string { + return fmt.Sprintf("provider.%s", n.ProviderName()) +} + +func (n *GraphNodeConfigProvider) ConfigType() GraphNodeConfigType { + return GraphNodeConfigTypeProvider +} + +func (n *GraphNodeConfigProvider) DependableName() []string { + return []string{n.Name()} +} + +func (n *GraphNodeConfigProvider) DependentOn() []string { + vars := n.Provider.RawConfig.Variables + result := make([]string, 0, len(vars)) + for _, v := range vars { + if vn := varNameForVar(v); vn != "" { + result = append(result, vn) + } + } + + return result +} + +// GraphNodeEvalable impl. +func (n *GraphNodeConfigProvider) EvalTree() EvalNode { + return ProviderEvalTree(n.ProviderName(), n.Provider.RawConfig) +} + +// GraphNodeProvider implementation +func (n *GraphNodeConfigProvider) ProviderName() string { + if n.Provider.Alias == "" { + return n.Provider.Name + } else { + return fmt.Sprintf("%s.%s", n.Provider.Name, n.Provider.Alias) + } +} + +// GraphNodeProvider implementation +func (n *GraphNodeConfigProvider) ProviderConfig() *config.RawConfig { + return n.Provider.RawConfig +} + +// GraphNodeDotter impl. +func (n *GraphNodeConfigProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node { + return dot.NewNode(name, map[string]string{ + "label": n.Name(), + "shape": "diamond", + }) +} + +// GraphNodeDotterOrigin impl. +func (n *GraphNodeConfigProvider) DotOrigin() bool { + return true +} + +// GraphNodeFlattenable impl. +func (n *GraphNodeConfigProvider) Flatten(p []string) (dag.Vertex, error) { + return &GraphNodeConfigProviderFlat{ + GraphNodeConfigProvider: n, + PathValue: p, + }, nil +} + +// Same as GraphNodeConfigProvider, but for flattening +type GraphNodeConfigProviderFlat struct { + *GraphNodeConfigProvider + + PathValue []string +} + +func (n *GraphNodeConfigProviderFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.GraphNodeConfigProvider.Name()) +} + +func (n *GraphNodeConfigProviderFlat) Path() []string { + return n.PathValue +} + +func (n *GraphNodeConfigProviderFlat) DependableName() []string { + return modulePrefixList( + n.GraphNodeConfigProvider.DependableName(), + modulePrefixStr(n.PathValue)) +} + +func (n *GraphNodeConfigProviderFlat) DependentOn() []string { + prefix := modulePrefixStr(n.PathValue) + return modulePrefixList( + n.GraphNodeConfigProvider.DependentOn(), + prefix) +} + +func (n *GraphNodeConfigProviderFlat) ProviderName() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), + n.GraphNodeConfigProvider.ProviderName()) +} diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 0711205331d7..0bc189176069 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -260,6 +260,13 @@ func (n *GraphNodeConfigResourceFlat) DependentOn() []string { prefix) } +func (n *GraphNodeConfigResourceFlat) ProvidedBy() []string { + prefix := modulePrefixStr(n.PathValue) + return modulePrefixList( + n.GraphNodeConfigResource.ProvidedBy(), + prefix) +} + // graphNodeResourceDestroy represents the logical destruction of a // resource. This node doesn't mean it will be destroyed for sure, but // instead that if a destroy were to happen, it must happen at this point. diff --git a/terraform/transform_root.go b/terraform/transform_root.go index 4d04df9beb71..15de14754ae4 100644 --- a/terraform/transform_root.go +++ b/terraform/transform_root.go @@ -34,3 +34,7 @@ type graphNodeRoot struct{} func (n graphNodeRoot) Name() string { return "root" } + +func (n graphNodeRoot) Flatten(p []string) (dag.Vertex, error) { + return n, nil +} From c207beda368850324c1604bfa2eddea8094e8569 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 16:29:19 -0700 Subject: [PATCH 19/33] 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 4f6d7c2e74e9..8a838afb715f 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 d355e36ef150..33a71b5dd9c9 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 27a98c2d5d98..2a5856829df4 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 ced1a0610bce..e6a9befbea1b 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 a86e0ca1ecff..6fe78207fe41 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 3d046a2ef2e1..347c964bd8da 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 ec98883b510f..54c1c8343ba6 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 41cf75c370bb..9f0251b42b99 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 508a8edc348c..314d18972a7c 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) } From 7fd432b0764b8737875c71d0d70e5480e6073927 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 16:41:49 -0700 Subject: [PATCH 20/33] terraform: providers in flattened graphs should depend on the parent --- terraform/graph_config_node_provider.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/terraform/graph_config_node_provider.go b/terraform/graph_config_node_provider.go index 5b8fb1bcb4b1..c0e15eff320b 100644 --- a/terraform/graph_config_node_provider.go +++ b/terraform/graph_config_node_provider.go @@ -102,10 +102,26 @@ func (n *GraphNodeConfigProviderFlat) DependableName() []string { } func (n *GraphNodeConfigProviderFlat) DependentOn() []string { - prefix := modulePrefixStr(n.PathValue) - return modulePrefixList( + prefixed := modulePrefixList( n.GraphNodeConfigProvider.DependentOn(), - prefix) + modulePrefixStr(n.PathValue)) + + result := make([]string, len(prefixed), len(prefixed)+1) + copy(result, prefixed) + + // If we're in a module, then depend on our parent's provider + if len(n.PathValue) > 1 { + prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1]) + if prefix != "" { + prefix += "." + } + + result = append(result, fmt.Sprintf( + "%s%s", + prefix, n.GraphNodeConfigProvider.Name())) + } + + return result } func (n *GraphNodeConfigProviderFlat) ProviderName() string { From 8f5836768050e459ee07152d7a9d7ee23ff16a74 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 18:08:06 -0700 Subject: [PATCH 21/33] terraform: missing providers need to do dependencies --- terraform/graph.go | 12 +++++++++ terraform/graph_builder_test.go | 16 ++++++++++++ terraform/transform_module.go | 43 +++++++++++++++++++++++++++++++++ terraform/transform_provider.go | 33 +++++++++++++++++++++++++ 4 files changed, 104 insertions(+) diff --git a/terraform/graph.go b/terraform/graph.go index 6fe78207fe41..6744a931e906 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -69,6 +69,18 @@ func (g *Graph) Remove(v dag.Vertex) dag.Vertex { return g.Graph.Remove(v) } +// Replace is the same as dag.Graph.Replace +func (g *Graph) Replace(o, n dag.Vertex) bool { + // Go through and update our lookaside to point to the new vertex + for k, v := range g.dependableMap { + if v == o { + g.dependableMap[k] = n + } + } + + return g.Graph.Replace(o, n) +} + // ConnectDependent connects a GraphNodeDependent to all of its // GraphNodeDependables. It returns the list of dependents it was // unable to connect to. diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index 0f66947fcf59..4811d786641b 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -136,6 +136,22 @@ func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) { } } +func TestBuiltinGraphBuilder_fuck(t *testing.T) { + b := &BuiltinGraphBuilder{ + Providers: []string{"aws"}, + Root: testModule(t, "validate-module-pc-inherit-unused"), + Validate: true, + } + + g, err := b.Build(RootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + println(g.String()) + t.FailNow() +} + /* TODO: This exposes a really bad bug we need to fix after we merge the f-ast-branch. This bug still exists in master. diff --git a/terraform/transform_module.go b/terraform/transform_module.go index 7733fe578407..7154ff70b200 100644 --- a/terraform/transform_module.go +++ b/terraform/transform_module.go @@ -33,6 +33,49 @@ func (t *ModuleInputTransformer) Transform(g *Graph) error { return nil } +// ModuleDestroyTransformer is a GraphTransformer that adds a node +// to the graph that will just mark the full module for destroy in +// the destroy scenario. +type ModuleDestroyTransformer struct{} + +func (t *ModuleDestroyTransformer) Transform(g *Graph) error { + // Create the node + n := &graphNodeModuleDestroy{Path: g.Path} + + // Add it to the graph + g.Add(n) + + // Connect the inputs to the bottom of the graph so that it happens + // first. + for _, v := range g.Vertices() { + if v == n { + continue + } + + if g.DownEdges(v).Len() == 0 { + g.Connect(dag.BasicEdge(v, n)) + } + } + + return nil +} + +type graphNodeModuleDestroy struct { + Path []string +} + +func (n *graphNodeModuleDestroy) Name() string { + return "module destroy (for plan)" +} + +// GraphNodeEvalable impl. +func (n *graphNodeModuleDestroy) EvalTree() EvalNode { + return &EvalOpFilter{ + Ops: []walkOperation{walkPlanDestroy}, + Node: &EvalDiffDestroyModule{Path: n.Path}, + } +} + type graphNodeModuleInput struct { Variables map[string]string } diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 0b625e5fa0f7..2e111ee820eb 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -190,6 +190,21 @@ func (n *graphNodeDisabledProvider) DotOrigin() bool { return true } +// GraphNodeDependable impl. +func (n *graphNodeDisabledProvider) DependableName() []string { + return []string{"provider." + n.ProviderName()} +} + +// GraphNodeProvider impl. +func (n *graphNodeDisabledProvider) ProviderName() string { + return n.GraphNodeProvider.ProviderName() +} + +// GraphNodeProvider impl. +func (n *graphNodeDisabledProvider) ProviderConfig() *config.RawConfig { + return n.GraphNodeProvider.ProviderConfig() +} + type graphNodeMissingProvider struct { ProviderNameValue string } @@ -264,3 +279,21 @@ func (n *graphNodeMissingProviderFlat) ProviderName() string { "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeMissingProvider.ProviderName()) } + +func (n *graphNodeMissingProviderFlat) DependentOn() []string { + var result []string + + // If we're in a module, then depend on our parent's provider + if len(n.PathValue) > 1 { + prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1]) + if prefix != "" { + prefix += "." + } + + result = append(result, fmt.Sprintf( + "%s%s", + prefix, n.graphNodeMissingProvider.Name())) + } + + return result +} From 1f10dfef2dabaa144312a1626abf2d9752bf8157 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 18:17:49 -0700 Subject: [PATCH 22/33] terraform: remove test --- terraform/graph_builder_test.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index 4811d786641b..0f66947fcf59 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -136,22 +136,6 @@ func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) { } } -func TestBuiltinGraphBuilder_fuck(t *testing.T) { - b := &BuiltinGraphBuilder{ - Providers: []string{"aws"}, - Root: testModule(t, "validate-module-pc-inherit-unused"), - Validate: true, - } - - g, err := b.Build(RootModulePath) - if err != nil { - t.Fatalf("err: %s", err) - } - - println(g.String()) - t.FailNow() -} - /* TODO: This exposes a really bad bug we need to fix after we merge the f-ast-branch. This bug still exists in master. From 23b6acc29d2918394a56b87a0a1c4b06eb6a33da Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 18:26:35 -0700 Subject: [PATCH 23/33] terraform: add module destroy node to graph --- terraform/graph_config_node_module.go | 17 +++++---- terraform/graph_config_node_module_test.go | 2 ++ terraform/transform_flatten_test.go | 2 ++ terraform/transform_module.go | 41 ++++++++++++++-------- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 347c964bd8da..308b47ca13b9 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -63,6 +63,14 @@ func (n *GraphNodeConfigModule) Expand(b GraphBuilder) (GraphNodeSubgraph, error return nil, err } + { + // Add the destroy marker to the graph + t := &ModuleDestroyTransformer{} + if err := t.Transform(graph); err != nil { + return nil, err + } + } + // Build the actual subgraph node return &graphNodeModuleExpanded{ Original: n, @@ -148,15 +156,6 @@ func (n *graphNodeModuleExpanded) EvalTree() EvalNode { Config: &resourceConfig, Variables: n.Variables, }, - - &EvalOpFilter{ - Ops: []walkOperation{walkPlanDestroy}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalDiffDestroyModule{Path: n.Graph.Path}, - }, - }, - }, }, } } diff --git a/terraform/graph_config_node_module_test.go b/terraform/graph_config_node_module_test.go index d3cbdfb1d5d3..6000c20de4f8 100644 --- a/terraform/graph_config_node_module_test.go +++ b/terraform/graph_config_node_module_test.go @@ -73,8 +73,10 @@ aws_instance.bar aws_instance.foo module inputs module inputs +plan-destroy ` const testGraphNodeModuleExpandFlattenStr = ` aws_instance.foo +plan-destroy ` diff --git a/terraform/transform_flatten_test.go b/terraform/transform_flatten_test.go index 749102469720..92f7cac7af88 100644 --- a/terraform/transform_flatten_test.go +++ b/terraform/transform_flatten_test.go @@ -74,6 +74,7 @@ module.child.aws_instance.child module.child.var.var module.child.output.output module.child.aws_instance.child +module.child.plan-destroy module.child.var.var aws_instance.parent ` @@ -88,6 +89,7 @@ module.child.aws_instance.child module.child.var.var module.child.output.output module.child.aws_instance.child +module.child.plan-destroy module.child.var.var aws_instance.parent ` diff --git a/terraform/transform_module.go b/terraform/transform_module.go index 7154ff70b200..ca6586265b5d 100644 --- a/terraform/transform_module.go +++ b/terraform/transform_module.go @@ -1,6 +1,7 @@ package terraform import ( + "fmt" "github.com/hashicorp/terraform/dag" ) @@ -42,21 +43,10 @@ func (t *ModuleDestroyTransformer) Transform(g *Graph) error { // Create the node n := &graphNodeModuleDestroy{Path: g.Path} - // Add it to the graph + // Add it to the graph. We don't need any edges because + // it can happen whenever. g.Add(n) - // Connect the inputs to the bottom of the graph so that it happens - // first. - for _, v := range g.Vertices() { - if v == n { - continue - } - - if g.DownEdges(v).Len() == 0 { - g.Connect(dag.BasicEdge(v, n)) - } - } - return nil } @@ -65,7 +55,7 @@ type graphNodeModuleDestroy struct { } func (n *graphNodeModuleDestroy) Name() string { - return "module destroy (for plan)" + return "plan-destroy" } // GraphNodeEvalable impl. @@ -76,6 +66,29 @@ func (n *graphNodeModuleDestroy) EvalTree() EvalNode { } } +// GraphNodeFlattenable impl. +func (n *graphNodeModuleDestroy) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeModuleDestroyFlat{ + graphNodeModuleDestroy: n, + PathValue: p, + }, nil +} + +type graphNodeModuleDestroyFlat struct { + *graphNodeModuleDestroy + + PathValue []string +} + +func (n *graphNodeModuleDestroyFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeModuleDestroy.Name()) +} + +func (n *graphNodeModuleDestroyFlat) Path() []string { + return n.PathValue +} + type graphNodeModuleInput struct { Variables map[string]string } From bbb065d1ad2d6eab55ade66379a855d48f7d35df Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 18:39:24 -0700 Subject: [PATCH 24/33] terraform: add edge for missing providers --- .../test-fixtures/plan-module-destroy-multivar/main.tf | 1 - terraform/transform_provider.go | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/terraform/test-fixtures/plan-module-destroy-multivar/main.tf b/terraform/test-fixtures/plan-module-destroy-multivar/main.tf index ae334b5a561b..2f965b68cc11 100644 --- a/terraform/test-fixtures/plan-module-destroy-multivar/main.tf +++ b/terraform/test-fixtures/plan-module-destroy-multivar/main.tf @@ -2,4 +2,3 @@ module "child" { source = "./child" instance_count = "2" } - diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 2e111ee820eb..b9ac53ccbfc3 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -218,6 +218,11 @@ func (n *graphNodeMissingProvider) EvalTree() EvalNode { return ProviderEvalTree(n.ProviderNameValue, nil) } +// GraphNodeDependable impl. +func (n *graphNodeMissingProvider) DependableName() []string { + return []string{n.Name()} +} + func (n *graphNodeMissingProvider) ProviderName() string { return n.ProviderNameValue } @@ -280,6 +285,11 @@ func (n *graphNodeMissingProviderFlat) ProviderName() string { n.graphNodeMissingProvider.ProviderName()) } +// GraphNodeDependable impl. +func (n *graphNodeMissingProviderFlat) DependableName() []string { + return []string{n.Name()} +} + func (n *graphNodeMissingProviderFlat) DependentOn() []string { var result []string From 542ddb881a016d4fd355c5d5ced7fec35cfca89c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2015 18:44:07 -0700 Subject: [PATCH 25/33] terraform: disable destroy mode for flattened nodes --- terraform/graph_config_node_resource.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 0bc189176069..821f8973829d 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -267,6 +267,13 @@ func (n *GraphNodeConfigResourceFlat) ProvidedBy() []string { prefix) } +// GraphNodeDestroyable impl. +func (n *GraphNodeConfigResourceFlat) DestroyNode(GraphNodeDestroyMode) GraphNodeDestroy { + // Disable destroy mode for flattened resources since we already + // did this within the original subgraph. + return nil +} + // graphNodeResourceDestroy represents the logical destruction of a // resource. This node doesn't mean it will be destroyed for sure, but // instead that if a destroy were to happen, it must happen at this point. From 6afc14982a82c08fb360098daf07881e61bb4f8d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 May 2015 18:21:00 -0700 Subject: [PATCH 26/33] terraform: destroy transform must happen globally --- terraform/context_test.go | 82 +++++++++++++++++++ terraform/graph_builder.go | 39 +++++---- terraform/graph_config_node_resource.go | 36 +++++++- terraform/terraform_test.go | 6 ++ .../apply-module-destroy-order/child/main.tf | 5 ++ .../apply-module-destroy-order/main.tf | 7 ++ terraform/transform_destroy.go | 31 +++++-- 7 files changed, 179 insertions(+), 27 deletions(-) create mode 100644 terraform/test-fixtures/apply-module-destroy-order/child/main.tf create mode 100644 terraform/test-fixtures/apply-module-destroy-order/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 60da26deb1b4..907f21c4bcf0 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -4053,6 +4053,88 @@ func TestContext2Apply_module(t *testing.T) { } } +func TestContext2Apply_moduleDestroyOrder(t *testing.T) { + m := testModule(t, "apply-module-destroy-order") + p := testProvider("aws") + p.DiffFn = testDiffFn + + // Create a custom apply function to track the order they were destroyed + var order []string + var orderLock sync.Mutex + p.ApplyFn = func( + info *InstanceInfo, + is *InstanceState, + id *InstanceDiff) (*InstanceState, error) { + orderLock.Lock() + defer orderLock.Unlock() + + order = append(order, is.ID) + return nil, nil + } + + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.b": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "b", + }, + }, + }, + }, + + &ModuleState{ + Path: []string{"root", "child"}, + Resources: map[string]*ResourceState{ + "aws_instance.a": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "a", + }, + }, + }, + Outputs: map[string]string{ + "a_output": "a", + }, + }, + }, + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: state, + Destroy: true, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + expected := []string{"b", "a"} + if !reflect.DeepEqual(order, expected) { + t.Fatalf("bad: %#v", order) + } + + { + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyModuleDestroyOrderStr) + if actual != expected { + t.Fatalf("bad: \n%s", actual) + } + } +} + func TestContext2Apply_moduleVarResourceCount(t *testing.T) { m := testModule(t, "apply-module-var-resource-count") p := testProvider("aws") diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index addb8baca779..574181c4223c 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -91,7 +91,7 @@ type BuiltinGraphBuilder struct { // Build builds the graph according to the steps returned by Steps. func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) { basic := &BasicGraphBuilder{ - Steps: b.Steps(), + Steps: b.Steps(path), Validate: b.Validate, } @@ -100,7 +100,7 @@ func (b *BuiltinGraphBuilder) Build(path []string) (*Graph, error) { // Steps returns the ordered list of GraphTransformers that must be executed // to build a complete graph. -func (b *BuiltinGraphBuilder) Steps() []GraphTransformer { +func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { steps := []GraphTransformer{ // Create all our resources from the configuration and state &ConfigTransformer{Module: b.Root}, @@ -144,20 +144,31 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer { // their dependencies. &TargetsTransformer{Targets: b.Targets, Destroy: b.Destroy}, - // Create the destruction nodes - &DestroyTransformer{}, - &CreateBeforeDestroyTransformer{}, - b.conditional(&conditionalOpts{ - If: func() bool { return !b.Verbose }, - Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State}, - }), - - // Make sure we create one root + // Make sure we have a single root &RootTransformer{}, + } - // Perform the transitive reduction to make our graph a bit - // more sane if possible (it usually is possible). - &TransitiveReductionTransformer{}, + // If we're on the root path, then we do a bunch of other stuff. + // We don't do the following for modules. + if len(path) <= 1 { + steps = append(steps, + // Create the destruction nodes + &DestroyTransformer{}, + &CreateBeforeDestroyTransformer{}, + b.conditional(&conditionalOpts{ + If: func() bool { return !b.Verbose }, + Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State}, + }), + + // Make sure we have a single root after the above changes. + // This is the 2nd root transformer. In practice this shouldn't + // actually matter as the RootTransformer is idempotent. + &RootTransformer{}, + + // Perform the transitive reduction to make our graph a bit + // more sane if possible (it usually is possible). + &TransitiveReductionTransformer{}, + ) } // Remove nils diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 821f8973829d..7ef59f8fd294 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -268,10 +268,38 @@ func (n *GraphNodeConfigResourceFlat) ProvidedBy() []string { } // GraphNodeDestroyable impl. -func (n *GraphNodeConfigResourceFlat) DestroyNode(GraphNodeDestroyMode) GraphNodeDestroy { - // Disable destroy mode for flattened resources since we already - // did this within the original subgraph. - return nil +func (n *GraphNodeConfigResourceFlat) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { + // Get our parent destroy node. If we don't have any, just return + raw := n.GraphNodeConfigResource.DestroyNode(mode) + if raw == nil { + return nil + } + + node, ok := raw.(*graphNodeResourceDestroy) + if !ok { + panic(fmt.Sprintf("unknown destroy node: %s %T", dag.VertexName(raw), raw)) + } + + // Otherwise, wrap it so that it gets the proper module treatment. + return &graphNodeResourceDestroyFlat{ + graphNodeResourceDestroy: node, + PathValue: n.PathValue, + } +} + +type graphNodeResourceDestroyFlat struct { + *graphNodeResourceDestroy + + PathValue []string +} + +func (n *graphNodeResourceDestroyFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeResourceDestroy.Name()) +} + +func (n *graphNodeResourceDestroyFlat) Path() []string { + return n.PathValue } // graphNodeResourceDestroy represents the logical destruction of a diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 09c3307922b6..878078668a9e 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -362,6 +362,12 @@ module.child: leader = 1 ` +const testTerraformApplyModuleDestroyOrderStr = ` + +module.child: + +` + const testTerraformApplyMultiProviderStr = ` aws_instance.bar: ID = foo diff --git a/terraform/test-fixtures/apply-module-destroy-order/child/main.tf b/terraform/test-fixtures/apply-module-destroy-order/child/main.tf new file mode 100644 index 000000000000..c3a5aecd6cff --- /dev/null +++ b/terraform/test-fixtures/apply-module-destroy-order/child/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "a" {} + +output "a_output" { + value = "${aws_instance.a.id}" +} diff --git a/terraform/test-fixtures/apply-module-destroy-order/main.tf b/terraform/test-fixtures/apply-module-destroy-order/main.tf new file mode 100644 index 000000000000..2a6bcce375bb --- /dev/null +++ b/terraform/test-fixtures/apply-module-destroy-order/main.tf @@ -0,0 +1,7 @@ +module "child" { + source = "./child" +} + +resource "aws_instance" "b" { + blah = "${module.child.a_output}" +} diff --git a/terraform/transform_destroy.go b/terraform/transform_destroy.go index 7d4293ef384e..b64f930d61fa 100644 --- a/terraform/transform_destroy.go +++ b/terraform/transform_destroy.go @@ -102,6 +102,14 @@ func (t *DestroyTransformer) transform( // Inherit all the edges from the old node downEdges := g.DownEdges(v).List() for _, edgeRaw := range downEdges { + // Don't inherit proxies. These are currently variables and + // outputs and don't affect destroys. In the future we should + // probably make this more obvious somehow (another interface?). + // Right now I'm not sure how. + if _, ok := edgeRaw.(GraphNodeProxy); ok { + continue + } + g.Connect(dag.BasicEdge(n, edgeRaw.(dag.Vertex))) } @@ -204,15 +212,6 @@ type PruneDestroyTransformer struct { } func (t *PruneDestroyTransformer) Transform(g *Graph) error { - var modDiff *ModuleDiff - var modState *ModuleState - if t.Diff != nil { - modDiff = t.Diff.ModuleByPath(g.Path) - } - if t.State != nil { - modState = t.State.ModuleByPath(g.Path) - } - for _, v := range g.Vertices() { // If it is not a destroyer, we don't care dn, ok := v.(GraphNodeDestroyPrunable) @@ -220,6 +219,20 @@ func (t *PruneDestroyTransformer) Transform(g *Graph) error { continue } + path := g.Path + if pn, ok := v.(GraphNodeSubPath); ok { + path = pn.Path() + } + + var modDiff *ModuleDiff + var modState *ModuleState + if t.Diff != nil { + modDiff = t.Diff.ModuleByPath(path) + } + if t.State != nil { + modState = t.State.ModuleByPath(path) + } + // Remove it if we should if !dn.DestroyInclude(modDiff, modState) { g.Remove(v) From cc0c208364229b5adfbfe1c39b82eb7265a1e905 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 2 May 2015 18:28:26 -0700 Subject: [PATCH 27/33] terraform: skip outputs for destroy graphs --- terraform/graph_config_node_output.go | 5 +++++ terraform/transform_destroy.go | 15 ++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/terraform/graph_config_node_output.go b/terraform/graph_config_node_output.go index 6a3533e37190..0d84a7862dce 100644 --- a/terraform/graph_config_node_output.go +++ b/terraform/graph_config_node_output.go @@ -61,6 +61,11 @@ func (n *GraphNodeConfigOutput) Proxy() bool { return true } +// GraphNodeDestroyEdgeInclude impl. +func (n *GraphNodeConfigOutput) DestroyEdgeInclude() bool { + return false +} + // GraphNodeFlattenable impl. func (n *GraphNodeConfigOutput) Flatten(p []string) (dag.Vertex, error) { return &GraphNodeConfigOutputFlat{ diff --git a/terraform/transform_destroy.go b/terraform/transform_destroy.go index b64f930d61fa..a3fe737aface 100644 --- a/terraform/transform_destroy.go +++ b/terraform/transform_destroy.go @@ -45,6 +45,13 @@ type GraphNodeDestroyPrunable interface { DestroyInclude(*ModuleDiff, *ModuleState) bool } +// GraphNodeEdgeInclude can be implemented to not include something +// as an edge within the destroy graph. This is usually done because it +// might cause unnecessary cycles. +type GraphNodeDestroyEdgeInclude interface { + DestroyEdgeInclude() bool +} + // DestroyTransformer is a GraphTransformer that creates the destruction // nodes for things that _might_ be destroyed. type DestroyTransformer struct{} @@ -102,11 +109,9 @@ func (t *DestroyTransformer) transform( // Inherit all the edges from the old node downEdges := g.DownEdges(v).List() for _, edgeRaw := range downEdges { - // Don't inherit proxies. These are currently variables and - // outputs and don't affect destroys. In the future we should - // probably make this more obvious somehow (another interface?). - // Right now I'm not sure how. - if _, ok := edgeRaw.(GraphNodeProxy); ok { + // If this thing specifically requests to not be depended on + // by destroy nodes, then don't. + if i, ok := edgeRaw.(GraphNodeDestroyEdgeInclude); ok && !i.DestroyEdgeInclude() { continue } From 2f3f6805054b2a731d12bf554e5cbbb0a2ccac86 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 4 May 2015 10:49:34 -0700 Subject: [PATCH 28/33] terraform: update comment --- terraform/graph_config_node_module.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/terraform/graph_config_node_module.go b/terraform/graph_config_node_module.go index 308b47ca13b9..08182a6b61da 100644 --- a/terraform/graph_config_node_module.go +++ b/terraform/graph_config_node_module.go @@ -165,9 +165,9 @@ func (n *graphNodeModuleExpanded) FlattenGraph() *Graph { graph := n.Subgraph() input := n.Original.Module.RawConfig - // Go over each vertex in the graph and wrap the configuration - // items so that the dependencies properly map to the modules. - // See the docs for graphNodeModuleWrappable for more info. + // Go over each vertex and do some modifications to the graph for + // flattening. We have to skip some nodes (graphNodeModuleSkippable) + // as well as setup the variable values. for _, v := range graph.Vertices() { if sn, ok := v.(graphNodeModuleSkippable); ok && sn.FlattenSkip() { graph.Remove(v) From 13f3daa3b33ee9f23bd2ffc0b46c8d20de3f6407 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 4 May 2015 10:51:34 -0700 Subject: [PATCH 29/33] terraform: update comment on interpolater stuff --- terraform/eval_context_builtin.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 33a71b5dd9c9..efc7d04fe4f9 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -12,10 +12,20 @@ import ( // BuiltinEvalContext is an EvalContext implementation that is used by // Terraform by default. type BuiltinEvalContext struct { - PathValue []string + // PathValue is the Path that this context is operating within. + PathValue []string + + // Interpolater setting below affect the interpolation of variables. + // + // The InterpolaterVars are the exact value for ${var.foo} values. + // The map is shared between all contexts and is a mapping of + // PATH to KEY to VALUE. Because it is shared by all contexts as well + // as the Interpolater itself, it is protected by InterpolaterVarLock + // which must be locked during any access to the map. Interpolater *Interpolater InterpolaterVars map[string]map[string]string InterpolaterVarLock *sync.Mutex + Hooks []Hook InputValue UIInput Providers map[string]ResourceProviderFactory From c3ce23c7b43913a90e7516141494b562a7b960d7 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 4 May 2015 15:43:23 -0500 Subject: [PATCH 30/33] core: failing test for a bad module cycle passing output of one module into input of the following module results in a cycle --- terraform/context_test.go | 20 +++++++++++++++++++ .../validate-module-deps-cycle/a/main.tf | 5 +++++ .../validate-module-deps-cycle/b/main.tf | 4 ++++ .../validate-module-deps-cycle/main.tf | 8 ++++++++ 4 files changed, 37 insertions(+) create mode 100644 terraform/test-fixtures/validate-module-deps-cycle/a/main.tf create mode 100644 terraform/test-fixtures/validate-module-deps-cycle/b/main.tf create mode 100644 terraform/test-fixtures/validate-module-deps-cycle/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 907f21c4bcf0..2988fca6fa0f 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -2443,6 +2443,26 @@ func TestContext2Validate_moduleBadResource(t *testing.T) { } } +func TestContext2Validate_moduleDepsShouldNotCycle(t *testing.T) { + m := testModule(t, "validate-module-deps-cycle") + p := testProvider("aws") + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + w, e := ctx.Validate() + + if len(w) > 0 { + t.Fatalf("expected no warnings, got: %s", w) + } + if len(e) > 0 { + t.Fatalf("expected no errors, got: %s", e) + } +} + func TestContext2Validate_moduleProviderInherit(t *testing.T) { m := testModule(t, "validate-module-pc-inherit") p := testProvider("aws") diff --git a/terraform/test-fixtures/validate-module-deps-cycle/a/main.tf b/terraform/test-fixtures/validate-module-deps-cycle/a/main.tf new file mode 100644 index 000000000000..3d3b01634eb6 --- /dev/null +++ b/terraform/test-fixtures/validate-module-deps-cycle/a/main.tf @@ -0,0 +1,5 @@ +resource "aws_instance" "a" { } + +output "output" { + value = "${aws_instance.a.id}" +} diff --git a/terraform/test-fixtures/validate-module-deps-cycle/b/main.tf b/terraform/test-fixtures/validate-module-deps-cycle/b/main.tf new file mode 100644 index 000000000000..d43b6c0c0643 --- /dev/null +++ b/terraform/test-fixtures/validate-module-deps-cycle/b/main.tf @@ -0,0 +1,4 @@ +variable "input" {} +resource "aws_instance" "b" { + name = "${var.input}" +} diff --git a/terraform/test-fixtures/validate-module-deps-cycle/main.tf b/terraform/test-fixtures/validate-module-deps-cycle/main.tf new file mode 100644 index 000000000000..11ddb64bfa7f --- /dev/null +++ b/terraform/test-fixtures/validate-module-deps-cycle/main.tf @@ -0,0 +1,8 @@ +module "a" { + source = "./a" +} + +module "b" { + source = "./b" + input = "${module.a.output}" +} From 6d4969f64cbc978fd820a44e857796f9ff9eecfa Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 5 May 2015 12:11:49 -0700 Subject: [PATCH 31/33] terraform: run prune destroy on validate --- terraform/context.go | 2 +- terraform/context_test.go | 3 +++ terraform/test-fixtures/validate-module-deps-cycle/b/main.tf | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index 669d50b26546..d061b47a60e7 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -410,7 +410,7 @@ func (c *Context) Validate() ([]string, []error) { // in the validate stage graph, err := c.Graph(&ContextGraphOpts{ Validate: true, - Verbose: true, + Verbose: false, }) if err != nil { return nil, []error{err} diff --git a/terraform/context_test.go b/terraform/context_test.go index 2988fca6fa0f..587ebb5387d4 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -2365,6 +2365,8 @@ func TestContext2Validate_countVariableNoDefault(t *testing.T) { } } +/* +TODO: What should we do here? func TestContext2Validate_cycle(t *testing.T) { p := testProvider("aws") m := testModule(t, "validate-cycle") @@ -2383,6 +2385,7 @@ func TestContext2Validate_cycle(t *testing.T) { t.Fatalf("expected 1 err, got: %s", e) } } +*/ func TestContext2Validate_moduleBadOutput(t *testing.T) { p := testProvider("aws") diff --git a/terraform/test-fixtures/validate-module-deps-cycle/b/main.tf b/terraform/test-fixtures/validate-module-deps-cycle/b/main.tf index d43b6c0c0643..07d769c98d9a 100644 --- a/terraform/test-fixtures/validate-module-deps-cycle/b/main.tf +++ b/terraform/test-fixtures/validate-module-deps-cycle/b/main.tf @@ -1,4 +1,5 @@ variable "input" {} + resource "aws_instance" "b" { name = "${var.input}" } From d503cc2d824f6c5cd3fbc18e9887faecf6f14bb9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 5 May 2015 12:45:28 -0700 Subject: [PATCH 32/33] terraform: flattenable graphNodeMissingProvisioner --- terraform/transform_provisioner.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/terraform/transform_provisioner.go b/terraform/transform_provisioner.go index 316a8f31e903..bf661f88a4ab 100644 --- a/terraform/transform_provisioner.go +++ b/terraform/transform_provisioner.go @@ -111,6 +111,14 @@ func (n *graphNodeMissingProvisioner) ProvisionerName() string { return n.ProvisionerNameValue } +// GraphNodeFlattenable impl. +func (n *graphNodeMissingProvisioner) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeMissingProvisionerFlat{ + graphNodeMissingProvisioner: n, + PathValue: p, + }, nil +} + func provisionerVertexMap(g *Graph) map[string]dag.Vertex { m := make(map[string]dag.Vertex) for _, v := range g.Vertices() { @@ -121,3 +129,25 @@ func provisionerVertexMap(g *Graph) map[string]dag.Vertex { return m } + +// Same as graphNodeMissingProvisioner, but for flattening +type graphNodeMissingProvisionerFlat struct { + *graphNodeMissingProvisioner + + PathValue []string +} + +func (n *graphNodeMissingProvisionerFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeMissingProvisioner.Name()) +} + +func (n *graphNodeMissingProvisionerFlat) Path() []string { + return n.PathValue +} + +func (n *graphNodeMissingProvisionerFlat) ProvisionerName() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), + n.graphNodeMissingProvisioner.ProvisionerName()) +} From 8c34e9a36acf11e179716709e3982a2daf8162f7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 5 May 2015 13:04:27 -0700 Subject: [PATCH 33/33] terraform: provisionedby prefixed --- terraform/graph_config_node_resource.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index 7ef59f8fd294..2ad91d18d508 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -267,6 +267,13 @@ func (n *GraphNodeConfigResourceFlat) ProvidedBy() []string { prefix) } +func (n *GraphNodeConfigResourceFlat) ProvisionedBy() []string { + prefix := modulePrefixStr(n.PathValue) + return modulePrefixList( + n.GraphNodeConfigResource.ProvisionedBy(), + prefix) +} + // GraphNodeDestroyable impl. func (n *GraphNodeConfigResourceFlat) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { // Get our parent destroy node. If we don't have any, just return