From 50afee2a30d88f5829dec86fcdb205454929ea2c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 12 Sep 2016 19:37:19 -0600 Subject: [PATCH 01/82] terraform: Diff.Empty should be true for nil Diff --- terraform/diff.go | 4 ++++ terraform/diff_test.go | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/terraform/diff.go b/terraform/diff.go index e0e097e80..86be4bb5a 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -72,6 +72,10 @@ func (d *Diff) RootModule() *ModuleDiff { // Empty returns true if the diff has no changes. func (d *Diff) Empty() bool { + if d == nil { + return true + } + for _, m := range d.Modules { if !m.Empty() { return false diff --git a/terraform/diff_test.go b/terraform/diff_test.go index 5234c6aaf..a9cb8b47b 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -7,7 +7,12 @@ import ( ) func TestDiffEmpty(t *testing.T) { - diff := new(Diff) + var diff *Diff + if !diff.Empty() { + t.Fatal("should be empty") + } + + diff = new(Diff) if !diff.Empty() { t.Fatal("should be empty") } From 361bc5a8df2928cbd95a355416c80a2f4ab8b415 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Sep 2016 10:44:44 -0700 Subject: [PATCH 02/82] terraform: parse internal resource addresses used in state/diff --- terraform/resource_address.go | 31 +++++++++++++++ terraform/resource_address_test.go | 60 ++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/terraform/resource_address.go b/terraform/resource_address.go index da22b2321..3d15c7f99 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -85,6 +85,37 @@ func (r *ResourceAddress) String() string { return strings.Join(result, ".") } +// parseResourceAddressInternal parses the somewhat bespoke resource +// identifier used in states and diffs, such as "instance.name.0". +func parseResourceAddressInternal(s string) (*ResourceAddress, error) { + // Split based on ".". Every resource address should have at least two + // elements (type and name). + parts := strings.Split(s, ".") + if len(parts) < 2 || len(parts) > 3 { + return nil, fmt.Errorf("Invalid internal resource address format: %s", s) + } + + // Build the parts of the resource address that are guaranteed to exist + addr := &ResourceAddress{ + Type: parts[0], + Name: parts[1], + Index: -1, + InstanceType: TypePrimary, + } + + // If we have more parts, then we have an index. Parse that. + if len(parts) > 2 { + idx, err := strconv.ParseInt(parts[2], 0, 0) + if err != nil { + return nil, fmt.Errorf("Error parsing resource address %q: %s", s, err) + } + + addr.Index = int(idx) + } + + return addr, nil +} + func ParseResourceAddress(s string) (*ResourceAddress, error) { matches, err := tokenizeResourceAddress(s) if err != nil { diff --git a/terraform/resource_address_test.go b/terraform/resource_address_test.go index 144d7a9ec..ac0720e4e 100644 --- a/terraform/resource_address_test.go +++ b/terraform/resource_address_test.go @@ -7,6 +7,66 @@ import ( "github.com/hashicorp/terraform/config" ) +func TestParseResourceAddressInternal(t *testing.T) { + cases := map[string]struct { + Input string + Expected *ResourceAddress + Output string + }{ + "basic resource": { + "aws_instance.foo", + &ResourceAddress{ + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: -1, + }, + "aws_instance.foo", + }, + + "basic resource with count": { + "aws_instance.foo.1", + &ResourceAddress{ + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: 1, + }, + "aws_instance.foo[1]", + }, + } + + for tn, tc := range cases { + t.Run(tc.Input, func(t *testing.T) { + out, err := parseResourceAddressInternal(tc.Input) + if err != nil { + t.Fatalf("%s: unexpected err: %#v", tn, err) + } + + if !reflect.DeepEqual(out, tc.Expected) { + t.Fatalf("bad: %q\n\nexpected:\n%#v\n\ngot:\n%#v", tn, tc.Expected, out) + } + + // Compare outputs if those exist + expected := tc.Input + if tc.Output != "" { + expected = tc.Output + } + if out.String() != expected { + t.Fatalf("bad: %q\n\nexpected: %s\n\ngot: %s", tn, expected, out) + } + + // Compare equality because the internal parse is used + // to compare equality to equal inputs. + if !out.Equals(tc.Expected) { + t.Fatalf("expected equality:\n\n%#v\n\n%#v", out, tc.Expected) + } + }) + } +} + func TestParseResourceAddress(t *testing.T) { cases := map[string]struct { Input string From 3f090df26ea56c26ac6122a285e51056fcbd2cd1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Sep 2016 10:56:37 -0700 Subject: [PATCH 03/82] terraform: start apply-based graph builder, basic diff transform --- terraform/graph_builder_apply.go | 44 +++++++++++++++++++++ terraform/graph_builder_apply_test.go | 57 +++++++++++++++++++++++++++ terraform/node_resource.go | 10 +++++ terraform/transform_diff.go | 53 +++++++++++++++++++++++++ 4 files changed, 164 insertions(+) create mode 100644 terraform/graph_builder_apply.go create mode 100644 terraform/graph_builder_apply_test.go create mode 100644 terraform/node_resource.go create mode 100644 terraform/transform_diff.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go new file mode 100644 index 000000000..f5f84d719 --- /dev/null +++ b/terraform/graph_builder_apply.go @@ -0,0 +1,44 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/config/module" +) + +// ApplyGraphBuilder implements GraphBuilder and is responsible for building +// a graph for applying a Terraform diff. +// +// Because the graph is built from the diff (vs. the config or state), +// this helps ensure that the apply-time graph doesn't modify any resources +// that aren't explicitly in the diff. There are other scenarios where the +// diff can be deviated, so this is just one layer of protection. +type ApplyGraphBuilder struct { + // Config is the root module for the graph to build. + Config *module.Tree + + // Diff is the diff to apply. + Diff *Diff + + // Providers is the list of providers supported. + Providers []string + + // Provisioners is the list of provisioners supported. + Provisioners []string +} + +// See GraphBuilder +func (b *ApplyGraphBuilder) Build(path []string) (*Graph, error) { + return (&BasicGraphBuilder{ + Steps: b.Steps(), + Validate: true, + }).Build(path) +} + +// See GraphBuilder +func (b *ApplyGraphBuilder) Steps() []GraphTransformer { + steps := []GraphTransformer{ + // Creates all the nodes represented in the diff. + &DiffTransformer{Diff: b.Diff}, + } + + return steps +} diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go new file mode 100644 index 000000000..45d4c95bc --- /dev/null +++ b/terraform/graph_builder_apply_test.go @@ -0,0 +1,57 @@ +package terraform + +import ( + "reflect" + "strings" + "testing" +) + +func TestApplyGraphBuilder_impl(t *testing.T) { + var _ GraphBuilder = new(ApplyGraphBuilder) +} + +func TestApplyGraphBuilder(t *testing.T) { + diff := &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: []string{"root"}, + Resources: map[string]*InstanceDiff{ + // Verify noop doesn't show up in graph + "aws_instance.noop": &InstanceDiff{}, + + "aws_instance.create": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + }, + }, + }, + } + + b := &ApplyGraphBuilder{ + Diff: diff, + } + + g, err := b.Build(RootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + + if !reflect.DeepEqual(g.Path, RootModulePath) { + t.Fatalf("bad: %#v", g.Path) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testApplyGraphBuilderStr) + if actual != expected { + t.Fatalf("bad: %s", actual) + } +} + +const testApplyGraphBuilderStr = ` +aws_instance.create +` diff --git a/terraform/node_resource.go b/terraform/node_resource.go new file mode 100644 index 000000000..86dd245f2 --- /dev/null +++ b/terraform/node_resource.go @@ -0,0 +1,10 @@ +package terraform + +// NodeResource is a graph node for referencing a resource. +type NodeResource struct { + Addr *ResourceAddress // Addr is the address for this resource +} + +func (n *NodeResource) Name() string { + return n.Addr.String() +} diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go new file mode 100644 index 000000000..e51834629 --- /dev/null +++ b/terraform/transform_diff.go @@ -0,0 +1,53 @@ +package terraform + +import ( + "fmt" +) + +// DiffTransformer is a GraphTransformer that adds the elements of +// the diff to the graph. +// +// This transform is used for example by the ApplyGraphBuilder to ensure +// that only resources that are being modified are represented in the graph. +type DiffTransformer struct { + Diff *Diff +} + +func (t *DiffTransformer) Transform(g *Graph) error { + // If the diff is nil or empty (nil is empty) then do nothing + if t.Diff.Empty() { + return nil + } + + // Go through all the modules in the diff. + for _, m := range t.Diff.Modules { + // TODO: If this is a destroy diff then add a module destroy node + + // Go through all the resources in this module. + for name, inst := range m.Resources { + // TODO: Destroy diff + + // If this diff has no attribute changes, then we have + // nothing to do and therefore won't add it to the graph. + if len(inst.Attributes) == 0 { + continue + } + + // We have changes! This is a create or update operation. + // First grab the address so we have a unique way to + // reference this resource. + addr, err := parseResourceAddressInternal(name) + if err != nil { + return fmt.Errorf( + "Error parsing internal name, this is a bug: %q", name) + } + + // Add the resource to the graph + g.Add(&NodeResource{ + Addr: addr, + }) + } + } + + return nil +} From 77b9177bd5e94618b87c13029a06f78ded1f9944 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Sep 2016 14:34:15 -0700 Subject: [PATCH 04/82] terraform: an incredible number of failing tests! --- terraform/context.go | 34 ++++++++++++++++++++++++++++++-- terraform/context_apply_test.go | 6 +++--- terraform/graph_builder_apply.go | 3 +++ terraform/resource.go | 4 ++++ 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index 3e2e8aa3e..707a8749f 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -353,8 +353,20 @@ func (c *Context) Apply() (*State, error) { // Copy our own state c.state = c.state.DeepCopy() - // Build the graph - graph, err := c.Graph(&ContextGraphOpts{Validate: true}) + // Build the graph. If it is a destroy operation, we use the + // standard graph builder. If it is an apply operation, we use + // our new graph builder. + var graph *Graph + var err error + if c.destroy { + graph, err = c.Graph(&ContextGraphOpts{Validate: true}) + } else { + graph, err = (&ApplyGraphBuilder{ + Diff: c.diff, + Providers: c.providersList(), + Provisioners: c.provisionersList(), + }).Build(RootModulePath) + } if err != nil { return nil, err } @@ -709,6 +721,24 @@ func (c *Context) walk( return walker, realErr } +func (c *Context) providersList() []string { + providers := make([]string, 0, len(c.providers)) + for k, _ := range c.providers { + providers = append(providers, k) + } + + return providers +} + +func (c *Context) provisionersList() []string { + provisioners := make([]string, 0, len(c.provisioners)) + for k, _ := range c.provisioners { + provisioners = append(provisioners, k) + } + + return provisioners +} + // parseVariableAsHCL parses the value of a single variable as would have been specified // on the command line via -var or in an environment variable named TF_VAR_x, where x is // the name of the variable. In order to get around the restriction of HCL requiring a diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index f11263f52..8e7ccce38 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1751,7 +1751,7 @@ func TestContext2Apply_multiVar(t *testing.T) { actual := state.RootModule().Outputs["output"] expected := "bar0,bar1,bar2" - if actual.Value != expected { + if actual == nil || actual.Value != expected { t.Fatalf("bad: \n%s", actual) } @@ -4483,8 +4483,8 @@ func TestContext2Apply_targetedModuleResource(t *testing.T) { } mod := state.ModuleByPath([]string{"root", "child"}) - if len(mod.Resources) != 1 { - t.Fatalf("expected 1 resource, got: %#v", mod.Resources) + if mod == nil || len(mod.Resources) != 1 { + t.Fatalf("expected 1 resource, got: %#v", mod) } checkStateString(t, state, ` diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index f5f84d719..f168a6a3d 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -38,6 +38,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { steps := []GraphTransformer{ // Creates all the nodes represented in the diff. &DiffTransformer{Diff: b.Diff}, + + // Single root + &RootTransformer{}, } return steps diff --git a/terraform/resource.go b/terraform/resource.go index 7f1ec3ca4..904d441af 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -72,6 +72,10 @@ type InstanceInfo struct { // HumanId is a unique Id that is human-friendly and useful for UI elements. func (i *InstanceInfo) HumanId() string { + if i == nil { + return "" + } + if len(i.ModulePath) <= 1 { return i.Id } From dc9b9eee4471fe0626fbce7f1bd8fd693b9ee926 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Sep 2016 17:11:34 -0700 Subject: [PATCH 05/82] terraform: connect providers in the apply graph --- terraform/graph.go | 61 +++++++++++++++++++++ terraform/graph_builder_apply.go | 4 ++ terraform/graph_builder_apply_test.go | 5 +- terraform/node_resource.go | 31 +++++++++-- terraform/resource_address.go | 12 +++++ terraform/transform_diff.go | 78 +++++++++++++++++++++++++-- 6 files changed, 182 insertions(+), 9 deletions(-) diff --git a/terraform/graph.go b/terraform/graph.go index e75d93663..f20ad4448 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -28,6 +28,11 @@ type Graph struct { // RootModuleName Path []string + // annotations are the annotations that are added to vertices. Annotations + // are arbitrary metadata taht is used for various logic. Annotations + // should have unique keys that are referenced via constants. + annotations map[dag.Vertex]map[string]interface{} + // dependableMap is a lookaside table for fast lookups for connecting // dependencies by their GraphNodeDependable value to avoid O(n^3)-like // situations and turn them into O(1) with respect to the number of new @@ -37,6 +42,29 @@ type Graph struct { once sync.Once } +// Annotations returns the annotations that are configured for the +// given vertex. The map is guaranteed to be non-nil but may be empty. +// +// The returned map may be modified to modify the annotations of the +// vertex. +func (g *Graph) Annotations(v dag.Vertex) map[string]interface{} { + g.once.Do(g.init) + + // If this vertex isn't in the graph, then just return an empty map + if !g.HasVertex(v) { + return map[string]interface{}{} + } + + // Get the map, if it doesn't exist yet then initialize it + m, ok := g.annotations[v] + if !ok { + m = make(map[string]interface{}) + g.annotations[v] = m + } + + return m +} + // Add is the same as dag.Graph.Add. func (g *Graph) Add(v dag.Vertex) dag.Vertex { g.once.Do(g.init) @@ -51,6 +79,14 @@ func (g *Graph) Add(v dag.Vertex) dag.Vertex { } } + // If this initializes annotations, then do that + if av, ok := v.(GraphNodeAnnotationInit); ok { + as := g.Annotations(v) + for k, v := range av.AnnotationInit() { + as[k] = v + } + } + return v } @@ -65,12 +101,17 @@ func (g *Graph) Remove(v dag.Vertex) dag.Vertex { } } + // Remove the annotations + delete(g.annotations, v) + // Call upwards to remove it from the actual graph return g.Graph.Remove(v) } // Replace is the same as dag.Graph.Replace func (g *Graph) Replace(o, n dag.Vertex) bool { + g.once.Do(g.init) + // Go through and update our lookaside to point to the new vertex for k, v := range g.dependableMap { if v == o { @@ -82,6 +123,12 @@ func (g *Graph) Replace(o, n dag.Vertex) bool { } } + // Move the annotation if it exists + if m, ok := g.annotations[o]; ok { + g.annotations[n] = m + delete(g.annotations, o) + } + return g.Graph.Replace(o, n) } @@ -153,6 +200,10 @@ func (g *Graph) Walk(walker GraphWalker) error { } func (g *Graph) init() { + if g.annotations == nil { + g.annotations = make(map[dag.Vertex]map[string]interface{}) + } + if g.dependableMap == nil { g.dependableMap = make(map[string]dag.Vertex) } @@ -237,6 +288,16 @@ func (g *Graph) walk(walker GraphWalker) error { return g.AcyclicGraph.Walk(walkFn) } +// GraphNodeAnnotationInit is an interface that allows a node to +// initialize it's annotations. +// +// AnnotationInit will be called _once_ when the node is added to a +// graph for the first time and is expected to return it's initial +// annotations. +type GraphNodeAnnotationInit interface { + AnnotationInit() map[string]interface{} +} + // GraphNodeDependable is an interface which says that a node can be // depended on (an edge can be placed between this node and another) according // to the well-known name returned by DependableName. diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index f168a6a3d..18dba7ab8 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -39,6 +39,10 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Creates all the nodes represented in the diff. &DiffTransformer{Diff: b.Diff}, + // Create all the providers + &MissingProviderTransformer{Providers: b.Providers}, + &ProviderTransformer{}, + // Single root &RootTransformer{}, } diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 45d4c95bc..2a9cde191 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -33,7 +33,8 @@ func TestApplyGraphBuilder(t *testing.T) { } b := &ApplyGraphBuilder{ - Diff: diff, + Diff: diff, + Providers: []string{"aws"}, } g, err := b.Build(RootModulePath) @@ -54,4 +55,6 @@ func TestApplyGraphBuilder(t *testing.T) { const testApplyGraphBuilderStr = ` aws_instance.create + provider.aws +provider.aws ` diff --git a/terraform/node_resource.go b/terraform/node_resource.go index 86dd245f2..7529418e7 100644 --- a/terraform/node_resource.go +++ b/terraform/node_resource.go @@ -1,10 +1,33 @@ package terraform -// NodeResource is a graph node for referencing a resource. -type NodeResource struct { - Addr *ResourceAddress // Addr is the address for this resource +import ( + "github.com/hashicorp/terraform/config" +) + +// NodeApplyableResource represents a resource that is "applyable": +// it is ready to be applied and is represented by a diff. +type NodeApplyableResource struct { + Addr *ResourceAddress // Addr is the address for this resource + Config *config.Resource // Config is the resource in the config + ResourceState *ResourceState // ResourceState is the ResourceState for this } -func (n *NodeResource) Name() string { +func (n *NodeApplyableResource) Name() string { return n.Addr.String() } + +// GraphNodeProviderConsumer +func (n *NodeApplyableResource) ProvidedBy() []string { + // If we have a config we prefer that above all else + if n.Config != nil { + return []string{resourceProvider(n.Config.Type, n.Config.Provider)} + } + + // If we have state, then we will use the provider from there + if n.ResourceState != nil { + return []string{n.ResourceState.Provider} + } + + // Use our type + return []string{resourceProvider(n.Addr.Type, "")} +} diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 3d15c7f99..e6bd61615 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -85,6 +85,17 @@ func (r *ResourceAddress) String() string { return strings.Join(result, ".") } +// parseResourceAddressConfig creates a resource address from a config.Resource +func parseResourceAddressConfig(r *config.Resource) (*ResourceAddress, error) { + return &ResourceAddress{ + Type: r.Type, + Name: r.Name, + Index: -1, + InstanceType: TypePrimary, + Mode: r.Mode, + }, nil +} + // parseResourceAddressInternal parses the somewhat bespoke resource // identifier used in states and diffs, such as "instance.name.0". func parseResourceAddressInternal(s string) (*ResourceAddress, error) { @@ -101,6 +112,7 @@ func parseResourceAddressInternal(s string) (*ResourceAddress, error) { Name: parts[1], Index: -1, InstanceType: TypePrimary, + Mode: config.ManagedResourceMode, } // If we have more parts, then we have an index. Parse that. diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index e51834629..c154fa7f8 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -2,6 +2,8 @@ package terraform import ( "fmt" + + "github.com/hashicorp/terraform/config/module" ) // DiffTransformer is a GraphTransformer that adds the elements of @@ -9,8 +11,16 @@ import ( // // This transform is used for example by the ApplyGraphBuilder to ensure // that only resources that are being modified are represented in the graph. +// +// Config and State is still required for the DiffTransformer for annotations +// since the Diff doesn't contain all the information required to build the +// complete graph (such as create-before-destroy information). The graph +// is built based on the diff first, though, ensuring that only resources +// that are being modified are present in the graph. type DiffTransformer struct { - Diff *Diff + Diff *Diff + Config *module.Tree + State *State } func (t *DiffTransformer) Transform(g *Graph) error { @@ -20,6 +30,7 @@ func (t *DiffTransformer) Transform(g *Graph) error { } // Go through all the modules in the diff. + var nodes []*NodeApplyableResource for _, m := range t.Diff.Modules { // TODO: If this is a destroy diff then add a module destroy node @@ -38,16 +49,75 @@ func (t *DiffTransformer) Transform(g *Graph) error { // reference this resource. addr, err := parseResourceAddressInternal(name) if err != nil { - return fmt.Errorf( - "Error parsing internal name, this is a bug: %q", name) + panic(fmt.Sprintf( + "Error parsing internal name, this is a bug: %q", name)) } + // Very important: add the module path for this resource to + // the address. Remove "root" from it. + addr.Path = m.Path[1:] + // Add the resource to the graph - g.Add(&NodeResource{ + nodes = append(nodes, &NodeApplyableResource{ Addr: addr, }) } } + // NOTE: Lots of room for performance optimizations below. For + // resource-heavy diffs this part alone is probably pretty slow. + + // Annotate all nodes with their config and state + for _, n := range nodes { + // Grab the configuration at this path. + if t := t.Config.Child(n.Addr.Path); t != nil { + for _, r := range t.Config().Resources { + // Get a resource address so we can compare + addr, err := parseResourceAddressConfig(r) + if err != nil { + panic(fmt.Sprintf( + "Error parsing config address, this is a bug: %#v", r)) + } + addr.Path = n.Addr.Path + + // If this is not the same resource, then continue + if !addr.Equals(n.Addr) { + continue + } + + // Same resource! Mark it and exit + n.Config = r + break + } + } + + // Grab the state at this path + if ms := t.State.ModuleByPath(normalizeModulePath(n.Addr.Path)); ms != nil { + for name, rs := range ms.Resources { + // Parse the name for comparison + addr, err := parseResourceAddressInternal(name) + if err != nil { + panic(fmt.Sprintf( + "Error parsing internal name, this is a bug: %q", name)) + } + addr.Path = n.Addr.Path + + // If this is not the same resource, then continue + if !addr.Equals(n.Addr) { + continue + } + + // Same resource! + n.ResourceState = rs + break + } + } + } + + // Add all the nodes to the graph + for _, n := range nodes { + g.Add(n) + } + return nil } From 5828a0a9acab592e85c88e34bb5fcf1c12d7ebae Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Sep 2016 17:52:09 -0700 Subject: [PATCH 06/82] terraform: minimal applies work! --- terraform/context.go | 2 + terraform/graph_builder_apply.go | 9 +- terraform/node_resource.go | 205 +++++++++++++++++++++++++++++++ terraform/resource_address.go | 18 +++ 4 files changed, 233 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index 707a8749f..1eaeada13 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -362,7 +362,9 @@ func (c *Context) Apply() (*State, error) { graph, err = c.Graph(&ContextGraphOpts{Validate: true}) } else { graph, err = (&ApplyGraphBuilder{ + Config: c.module, Diff: c.diff, + State: c.state, Providers: c.providersList(), Provisioners: c.provisionersList(), }).Build(RootModulePath) diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 18dba7ab8..d96c2d3f2 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -18,6 +18,9 @@ type ApplyGraphBuilder struct { // Diff is the diff to apply. Diff *Diff + // State is the current state + State *State + // Providers is the list of providers supported. Providers []string @@ -37,7 +40,11 @@ func (b *ApplyGraphBuilder) Build(path []string) (*Graph, error) { func (b *ApplyGraphBuilder) Steps() []GraphTransformer { steps := []GraphTransformer{ // Creates all the nodes represented in the diff. - &DiffTransformer{Diff: b.Diff}, + &DiffTransformer{ + Diff: b.Diff, + Config: b.Config, + State: b.State, + }, // Create all the providers &MissingProviderTransformer{Providers: b.Providers}, diff --git a/terraform/node_resource.go b/terraform/node_resource.go index 7529418e7..5a3af7696 100644 --- a/terraform/node_resource.go +++ b/terraform/node_resource.go @@ -1,6 +1,8 @@ package terraform import ( + "fmt" + "github.com/hashicorp/terraform/config" ) @@ -31,3 +33,206 @@ func (n *NodeApplyableResource) ProvidedBy() []string { // Use our type return []string{resourceProvider(n.Addr.Type, "")} } + +// GraphNodeEvalable +func (n *NodeApplyableResource) EvalTree() EvalNode { + // stateId is the ID to put into the state + stateId := n.Addr.stateId() + if n.Addr.Index > -1 { + stateId = fmt.Sprintf("%s.%d", stateId, n.Addr.Index) + } + + // Build the instance info. More of this will be populated during eval + info := &InstanceInfo{ + Id: stateId, + Type: n.Addr.Type, + } + + // Build the resource for eval + resource := &Resource{ + Name: n.Addr.Name, + Type: n.Addr.Type, + CountIndex: n.Addr.Index, + } + if resource.CountIndex < 0 { + resource.CountIndex = 0 + } + + // Determine the dependencies for the state. We use some older + // code for this that we've used for a long time. + var stateDeps []string + { + oldN := &graphNodeExpandedResource{Resource: n.Config} + stateDeps = oldN.StateDependencies() + } + + // Declare a bunch of variables that are used for state during + // evaluation. Most of this are written to by-address below. + var provider ResourceProvider + var diff *InstanceDiff + var state *InstanceState + var resourceConfig *ResourceConfig + var err error + var createNew bool + var createBeforeDestroyEnabled bool + + return &EvalSequence{ + Nodes: []EvalNode{ + // Build the instance info + &EvalInstanceInfo{ + Info: info, + }, + + // Get the saved diff for apply + &EvalReadDiff{ + Name: stateId, + Diff: &diff, + }, + + // We don't want to do any destroys + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if diff == nil { + return true, EvalEarlyExitError{} + } + + if diff.GetDestroy() && diff.GetAttributesLen() == 0 { + return true, EvalEarlyExitError{} + } + + diff.SetDestroy(false) + return true, nil + }, + Then: EvalNoop{}, + }, + + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + destroy := false + if diff != nil { + destroy = diff.GetDestroy() || diff.RequiresNew() + } + + createBeforeDestroyEnabled = + n.Config.Lifecycle.CreateBeforeDestroy && + destroy + + return createBeforeDestroyEnabled, nil + }, + Then: &EvalDeposeState{ + Name: stateId, + }, + }, + + &EvalInterpolate{ + Config: n.Config.RawConfig.Copy(), + Resource: resource, + Output: &resourceConfig, + }, + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + &EvalReadState{ + Name: stateId, + Output: &state, + }, + // Re-run validation to catch any errors we missed, e.g. type + // mismatches on computed values. + &EvalValidateResource{ + Provider: &provider, + Config: &resourceConfig, + ResourceName: n.Config.Name, + ResourceType: n.Config.Type, + ResourceMode: n.Config.Mode, + IgnoreWarnings: true, + }, + &EvalDiff{ + Info: info, + Config: &resourceConfig, + Resource: n.Config, + Provider: &provider, + Diff: &diff, + State: &state, + OutputDiff: &diff, + }, + + // Get the saved diff + &EvalReadDiff{ + Name: stateId, + Diff: &diff, + }, + + // Compare the diffs + &EvalCompareDiff{ + Info: info, + One: &diff, + Two: &diff, + }, + + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + &EvalReadState{ + Name: stateId, + Output: &state, + }, + &EvalApply{ + Info: info, + State: &state, + Diff: &diff, + Provider: &provider, + Output: &state, + Error: &err, + CreateNew: &createNew, + }, + &EvalWriteState{ + Name: stateId, + ResourceType: n.Config.Type, + Provider: n.Config.Provider, + Dependencies: stateDeps, + State: &state, + }, + &EvalApplyProvisioners{ + Info: info, + State: &state, + Resource: n.Config, + InterpResource: resource, + CreateNew: &createNew, + Error: &err, + }, + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + return createBeforeDestroyEnabled && err != nil, nil + }, + Then: &EvalUndeposeState{ + Name: stateId, + State: &state, + }, + Else: &EvalWriteState{ + Name: stateId, + ResourceType: n.Config.Type, + Provider: n.Config.Provider, + Dependencies: stateDeps, + State: &state, + }, + }, + + // We clear the diff out here so that future nodes + // don't see a diff that is already complete. There + // is no longer a diff! + &EvalWriteDiff{ + Name: stateId, + Diff: nil, + }, + + &EvalApplyPost{ + Info: info, + State: &state, + Error: &err, + }, + &EvalUpdateStateHook{}, + }, + } +} diff --git a/terraform/resource_address.go b/terraform/resource_address.go index e6bd61615..13c7170d5 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -85,6 +85,24 @@ func (r *ResourceAddress) String() string { return strings.Join(result, ".") } +// stateId returns the ID that this resource should be entered with +// in the state. This is also used for diffs. In the future, we'd like to +// move away from this string field so I don't export this. +// TODO: test +func (r *ResourceAddress) stateId() string { + result := fmt.Sprintf("%s.%s", r.Type, r.Name) + switch r.Mode { + case config.ManagedResourceMode: + // Done + case config.DataResourceMode: + result = fmt.Sprintf("data.%s", result) + default: + panic(fmt.Errorf("unknown resource mode: %s", r.Mode)) + } + + return result +} + // parseResourceAddressConfig creates a resource address from a config.Resource func parseResourceAddressConfig(r *config.Resource) (*ResourceAddress, error) { return &ResourceAddress{ From dcc3eb301149b2c6e433799e97dd26033989158f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 13 Sep 2016 17:55:01 -0700 Subject: [PATCH 07/82] terraform: test for ResourceAddress.stateId() --- terraform/resource_address.go | 1 - terraform/resource_address_test.go | 49 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 13c7170d5..643833ff3 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -88,7 +88,6 @@ func (r *ResourceAddress) String() string { // stateId returns the ID that this resource should be entered with // in the state. This is also used for diffs. In the future, we'd like to // move away from this string field so I don't export this. -// TODO: test func (r *ResourceAddress) stateId() string { result := fmt.Sprintf("%s.%s", r.Type, r.Name) switch r.Mode { diff --git a/terraform/resource_address_test.go b/terraform/resource_address_test.go index ac0720e4e..02d9e82e5 100644 --- a/terraform/resource_address_test.go +++ b/terraform/resource_address_test.go @@ -521,3 +521,52 @@ func TestResourceAddressEquals(t *testing.T) { } } } + +func TestResourceAddressStateId(t *testing.T) { + cases := map[string]struct { + Input *ResourceAddress + Expected string + }{ + "basic resource": { + &ResourceAddress{ + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: -1, + }, + "aws_instance.foo", + }, + + "basic resource ignores count": { + &ResourceAddress{ + Mode: config.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: 2, + }, + "aws_instance.foo", + }, + + "data resource": { + &ResourceAddress{ + Mode: config.DataResourceMode, + Type: "aws_instance", + Name: "foo", + InstanceType: TypePrimary, + Index: -1, + }, + "data.aws_instance.foo", + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + actual := tc.Input.stateId() + if actual != tc.Expected { + t.Fatalf("bad: %q\n\nexpected: %s\n\ngot: %s", tn, tc.Expected, actual) + } + }) + } +} From b0bae6dbfefb65883b1cc51a530ffd9ec6d33956 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 11:29:17 -0700 Subject: [PATCH 08/82] Adding my test helper while I work on this branch --- go.sh | 1 + 1 file changed, 1 insertion(+) create mode 100755 go.sh diff --git a/go.sh b/go.sh new file mode 100755 index 000000000..eb7a1697e --- /dev/null +++ b/go.sh @@ -0,0 +1 @@ +go test ./terraform | grep -E '(FAIL|panic)' | tee /dev/tty | wc -l From 9ea9e52185221fed995d57ba73d30e629efd05b3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 11:44:41 -0700 Subject: [PATCH 09/82] terraform: rename Config to Module, tests for diff transform --- terraform/context.go | 2 +- terraform/graph_builder_apply.go | 6 +- .../transform-diff-basic/main.tf | 1 + terraform/transform_diff.go | 6 +- terraform/transform_diff_test.go | 60 +++++++++++++++++++ 5 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 terraform/test-fixtures/transform-diff-basic/main.tf create mode 100644 terraform/transform_diff_test.go diff --git a/terraform/context.go b/terraform/context.go index 1eaeada13..a48e46863 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -362,7 +362,7 @@ func (c *Context) Apply() (*State, error) { graph, err = c.Graph(&ContextGraphOpts{Validate: true}) } else { graph, err = (&ApplyGraphBuilder{ - Config: c.module, + Module: c.module, Diff: c.diff, State: c.state, Providers: c.providersList(), diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index d96c2d3f2..8dde58519 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -12,8 +12,8 @@ import ( // that aren't explicitly in the diff. There are other scenarios where the // diff can be deviated, so this is just one layer of protection. type ApplyGraphBuilder struct { - // Config is the root module for the graph to build. - Config *module.Tree + // Module is the root module for the graph to build. + Module *module.Tree // Diff is the diff to apply. Diff *Diff @@ -42,7 +42,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Creates all the nodes represented in the diff. &DiffTransformer{ Diff: b.Diff, - Config: b.Config, + Module: b.Module, State: b.State, }, diff --git a/terraform/test-fixtures/transform-diff-basic/main.tf b/terraform/test-fixtures/transform-diff-basic/main.tf new file mode 100644 index 000000000..919f140bb --- /dev/null +++ b/terraform/test-fixtures/transform-diff-basic/main.tf @@ -0,0 +1 @@ +resource "aws_instance" "foo" {} diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index c154fa7f8..f0455f5e3 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -12,14 +12,14 @@ import ( // This transform is used for example by the ApplyGraphBuilder to ensure // that only resources that are being modified are represented in the graph. // -// Config and State is still required for the DiffTransformer for annotations +// Module and State is still required for the DiffTransformer for annotations // since the Diff doesn't contain all the information required to build the // complete graph (such as create-before-destroy information). The graph // is built based on the diff first, though, ensuring that only resources // that are being modified are present in the graph. type DiffTransformer struct { Diff *Diff - Config *module.Tree + Module *module.Tree State *State } @@ -70,7 +70,7 @@ func (t *DiffTransformer) Transform(g *Graph) error { // Annotate all nodes with their config and state for _, n := range nodes { // Grab the configuration at this path. - if t := t.Config.Child(n.Addr.Path); t != nil { + if t := t.Module.Child(n.Addr.Path); t != nil { for _, r := range t.Config().Resources { // Get a resource address so we can compare addr, err := parseResourceAddressConfig(r) diff --git a/terraform/transform_diff_test.go b/terraform/transform_diff_test.go new file mode 100644 index 000000000..c3e3176e6 --- /dev/null +++ b/terraform/transform_diff_test.go @@ -0,0 +1,60 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestDiffTransformer_nilDiff(t *testing.T) { + g := Graph{Path: RootModulePath} + tf := &DiffTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + if len(g.Vertices()) > 0 { + t.Fatal("graph should be empty") + } +} + +func TestDiffTransformer(t *testing.T) { + g := Graph{Path: RootModulePath} + tf := &DiffTransformer{ + Module: testModule(t, "transform-diff-basic"), + Diff: &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: []string{"root"}, + Resources: map[string]*InstanceDiff{ + "aws_instance.foo": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + }, + }, + }, + }, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformDiffBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } + + v := g.Vertices()[0].(*NodeApplyableResource) + if v.Config == nil { + t.Fatal("no config") + } +} + +const testTransformDiffBasicStr = ` +aws_instance.foo +` From e784e4a434e9d6c56c180b92bb7ef1c4495a76a3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 11:46:36 -0700 Subject: [PATCH 10/82] terraform: remove more nil panics (doesn't change test logic) --- terraform/context_apply_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 8e7ccce38..d3a14f64d 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -835,17 +835,21 @@ func TestContext2Apply_cancel(t *testing.T) { } // Start the Apply in a goroutine + var applyErr error stateCh := make(chan *State) go func() { state, err := ctx.Apply() if err != nil { - panic(err) + applyErr = err } stateCh <- state }() state := <-stateCh + if applyErr != nil { + t.Fatalf("err: %s", applyErr) + } mod := state.RootModule() if len(mod.Resources) != 1 { From ba751c4e3bf8b2f46745a3ecef42aa363f018669 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 11:47:40 -0700 Subject: [PATCH 11/82] terraform: comment to avoid panic --- terraform/node_resource.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/terraform/node_resource.go b/terraform/node_resource.go index 5a3af7696..e2761bd9d 100644 --- a/terraform/node_resource.go +++ b/terraform/node_resource.go @@ -194,14 +194,17 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { Dependencies: stateDeps, State: &state, }, - &EvalApplyProvisioners{ - Info: info, - State: &state, - Resource: n.Config, - InterpResource: resource, - CreateNew: &createNew, - Error: &err, - }, + /* + TODO: this has to work + &EvalApplyProvisioners{ + Info: info, + State: &state, + Resource: n.Config, + InterpResource: resource, + CreateNew: &createNew, + Error: &err, + }, + */ &EvalIf{ If: func(ctx EvalContext) (bool, error) { return createBeforeDestroyEnabled && err != nil, nil From 55ef966b88223a46ed51f37f11bd73b1a4791510 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 13:35:54 -0700 Subject: [PATCH 12/82] config/module: tree.Child on a nil tree works --- config/module/tree.go | 4 ++++ config/module/tree_test.go | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/config/module/tree.go b/config/module/tree.go index a0818bfa4..8a322650b 100644 --- a/config/module/tree.go +++ b/config/module/tree.go @@ -66,6 +66,10 @@ func (t *Tree) Config() *config.Config { // Child returns the child with the given path (by name). func (t *Tree) Child(path []string) *Tree { + if t == nil { + return nil + } + if len(path) == 0 { return t } diff --git a/config/module/tree_test.go b/config/module/tree_test.go index f455c2a02..d46d5ed27 100644 --- a/config/module/tree_test.go +++ b/config/module/tree_test.go @@ -12,6 +12,11 @@ import ( ) func TestTreeChild(t *testing.T) { + var nilTree *Tree + if nilTree.Child(nil) != nil { + t.Fatal("child should be nil") + } + storage := testStorage(t) tree := NewTree("", testConfig(t, "child")) if err := tree.Load(storage, GetModeGet); err != nil { From 87bff933ef3b3cb1fade9b052ee0b31dfbc9e60f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 14:43:14 -0700 Subject: [PATCH 13/82] terraform: ParentProviderTransform to connect parent providers --- terraform/context_apply_test.go | 4 +- terraform/graph.go | 2 +- terraform/graph_builder_apply.go | 1 + terraform/graph_builder_apply_test.go | 21 +++++++++ terraform/node_resource.go | 5 ++ terraform/transform_provider.go | 67 ++++++++++++++++++++++++++- 6 files changed, 96 insertions(+), 4 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index d3a14f64d..46fc8c43d 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1232,7 +1232,7 @@ aws_instance.foo: } } -func TestContext2Apply_module(t *testing.T) { +func TestContext2Apply_moduleBasic(t *testing.T) { m := testModule(t, "apply-module") p := testProvider("aws") p.ApplyFn = testApplyFn @@ -1256,7 +1256,7 @@ func TestContext2Apply_module(t *testing.T) { actual := strings.TrimSpace(state.String()) expected := strings.TrimSpace(testTerraformApplyModuleStr) if actual != expected { - t.Fatalf("bad: \n%s", actual) + t.Fatalf("bad, expected:\n%s\n\nactual:\n%s", expected, actual) } } diff --git a/terraform/graph.go b/terraform/graph.go index f20ad4448..97214f809 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -230,7 +230,7 @@ func (g *Graph) walk(walker GraphWalker) error { // with a GraphNodeSubPath impl. vertexCtx := ctx if pn, ok := v.(GraphNodeSubPath); ok && len(pn.Path()) > 0 { - vertexCtx = walker.EnterPath(pn.Path()) + vertexCtx = walker.EnterPath(normalizeModulePath(pn.Path())) defer walker.ExitPath(pn.Path()) } diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 8dde58519..a853ed33c 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -49,6 +49,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Create all the providers &MissingProviderTransformer{Providers: b.Providers}, &ProviderTransformer{}, + &ParentProviderTransformer{}, // Single root &RootTransformer{}, diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 2a9cde191..e62521d91 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -29,6 +29,20 @@ func TestApplyGraphBuilder(t *testing.T) { }, }, }, + + &ModuleDiff{ + Path: []string{"root", "child"}, + Resources: map[string]*InstanceDiff{ + "aws_instance.create": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + }, + }, }, } @@ -56,5 +70,12 @@ func TestApplyGraphBuilder(t *testing.T) { const testApplyGraphBuilderStr = ` aws_instance.create provider.aws +module.child.aws_instance.create + module.child.provider.aws +module.child.provider.aws + provider.aws provider.aws +root + aws_instance.create + module.child.aws_instance.create ` diff --git a/terraform/node_resource.go b/terraform/node_resource.go index e2761bd9d..6373ba167 100644 --- a/terraform/node_resource.go +++ b/terraform/node_resource.go @@ -18,6 +18,11 @@ func (n *NodeApplyableResource) Name() string { return n.Addr.String() } +// GraphNodeSubPath +func (n *NodeApplyableResource) Path() []string { + return n.Addr.Path +} + // GraphNodeProviderConsumer func (n *NodeApplyableResource) ProvidedBy() []string { // If we have a config we prefer that above all else diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 99ea0c3f4..46568d93d 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -217,7 +217,11 @@ func (t *MissingProviderTransformer) Transform(g *Graph) error { } // Add the missing provider node to the graph - raw := &graphNodeProvider{ProviderNameValue: p} + raw := &graphNodeProvider{ + ProviderNameValue: p, + PathValue: path, + } + var v dag.Vertex = raw if len(path) > 0 { var err error @@ -242,6 +246,66 @@ func (t *MissingProviderTransformer) Transform(g *Graph) error { return nil } +// ParentProviderTransformer connects provider nodes to their parents. +// +// This works by finding nodes that are both GraphNodeProviders and +// GraphNodeSubPath. It then connects the providers to their parent +// path. +type ParentProviderTransformer struct{} + +func (t *ParentProviderTransformer) Transform(g *Graph) error { + // Make a mapping of path to dag.Vertex, where path is: "path.name" + m := make(map[string]dag.Vertex) + + // Also create a map that maps a provider to its parent + parentMap := make(map[dag.Vertex]string) + for _, raw := range g.Vertices() { + // If it is the flat version, then make it the non-flat version. + // We eventually want to get rid of the flat version entirely so + // this is a stop-gap while it still exists. + var v dag.Vertex = raw + if f, ok := v.(*graphNodeProviderFlat); ok { + v = f.graphNodeProvider + } + + // Only care about providers + pn, ok := v.(GraphNodeProvider) + if !ok || pn.ProviderName() == "" { + continue + } + + // Also require a subpath, if there is no subpath then we + // just totally ignore it. The expectation of this transform is + // that it is used with a graph builder that is already flattened. + var path []string + if pn, ok := raw.(GraphNodeSubPath); ok { + path = pn.Path() + } + path = normalizeModulePath(path) + + // Build the key with path.name i.e. "child.subchild.aws" + key := fmt.Sprintf("%s.%s", strings.Join(path, "."), pn.ProviderName()) + m[key] = v + + // Determine the parent if we're non-root. This is length 1 since + // the 0 index should be "root" since we normalize above. + if len(path) > 1 { + path = path[:len(path)-1] + key := fmt.Sprintf("%s.%s", strings.Join(path, "."), pn.ProviderName()) + parentMap[raw] = key + } + } + + // Connect! + for v, key := range parentMap { + if parent, ok := m[key]; ok { + g.Connect(dag.BasicEdge(v, parent)) + } + } + + return nil +} + // PruneProviderTransformer is a GraphTransformer that prunes all the // providers that aren't needed from the graph. A provider is unneeded if // no resource or module is using that provider. @@ -448,6 +512,7 @@ func (n *graphNodeCloseProvider) DotNode(name string, opts *GraphDotOpts) *dot.N type graphNodeProvider struct { ProviderNameValue string + PathValue []string } func (n *graphNodeProvider) Name() string { From 11578f079278c2e9458094109381e1df0abc2c75 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 14:55:02 -0700 Subject: [PATCH 14/82] terraform: tests for ParentProviderTransformer --- terraform/transform_provider.go | 2 +- terraform/transform_provider_test.go | 99 ++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 46568d93d..80d51f526 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -285,7 +285,7 @@ func (t *ParentProviderTransformer) Transform(g *Graph) error { // Build the key with path.name i.e. "child.subchild.aws" key := fmt.Sprintf("%s.%s", strings.Join(path, "."), pn.ProviderName()) - m[key] = v + m[key] = raw // Determine the parent if we're non-root. This is length 1 since // the 0 index should be "root" since we normalize above. diff --git a/terraform/transform_provider_test.go b/terraform/transform_provider_test.go index 85a6266f9..22870fc77 100644 --- a/terraform/transform_provider_test.go +++ b/terraform/transform_provider_test.go @@ -230,6 +230,89 @@ func TestMissingProviderTransformer_moduleGrandchild(t *testing.T) { } } +func TestParentProviderTransformer(t *testing.T) { + g := Graph{Path: RootModulePath} + + // Introduce a cihld module + { + tf := &ImportStateTransformer{ + Targets: []*ImportTarget{ + &ImportTarget{ + Addr: "module.moo.foo_instance.qux", + ID: "bar", + }, + }, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + // Add the missing modules + { + tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + // Connect parents + { + tf := &ParentProviderTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformParentProviderStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestParentProviderTransformer_moduleGrandchild(t *testing.T) { + g := Graph{Path: RootModulePath} + + // We use the import state transformer since at the time of writing + // this test it is the first and only transformer that will introduce + // multiple module-path nodes at a single go. + { + tf := &ImportStateTransformer{ + Targets: []*ImportTarget{ + &ImportTarget{ + Addr: "module.a.module.b.foo_instance.qux", + ID: "bar", + }, + }, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + // Connect parents + { + tf := &ParentProviderTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformParentProviderModuleGrandchildStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestPruneProviderTransformer(t *testing.T) { mod := testModule(t, "transform-provider-prune") @@ -382,6 +465,22 @@ module.a.provider.foo provider.foo ` +const testTransformParentProviderStr = ` +module.moo.foo_instance.qux (import id: bar) +module.moo.provider.foo + provider.foo +provider.foo +` + +const testTransformParentProviderModuleGrandchildStr = ` +module.a.module.b.foo_instance.qux (import id: bar) +module.a.module.b.provider.foo + module.a.provider.foo +module.a.provider.foo + provider.foo +provider.foo +` + const testTransformProviderModuleChildStr = ` module.moo.foo_instance.qux (import id: bar) module.moo.provider.foo From b2ef4e9ac08b9ced31546a3dc9cddab4c50497b7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 20:59:02 -0700 Subject: [PATCH 15/82] terraform: add way to toggle the graphs to use for apply --- terraform/context.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index a48e46863..bde4b1c79 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -17,6 +17,11 @@ import ( // is called on Context. type InputMode byte +var ( + // NOTE: Internal only to toggle between the new and old apply graph + newApplyGraph = false +) + const ( // InputModeVar asks for all variables InputModeVar InputMode = 1 << iota @@ -358,7 +363,7 @@ func (c *Context) Apply() (*State, error) { // our new graph builder. var graph *Graph var err error - if c.destroy { + if c.destroy || !newApplyGraph { graph, err = c.Graph(&ContextGraphOpts{Validate: true}) } else { graph, err = (&ApplyGraphBuilder{ From 79a742c1aea9fafbc8f7dee2ba7beca5024c3f6a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 14 Sep 2016 23:58:16 -0700 Subject: [PATCH 16/82] terraform: new provider graph node for flattened world --- terraform/graph_builder_apply.go | 13 ++++- terraform/node_provider.go | 55 +++++++++++++++++++++ terraform/transform_attach_config.go | 74 ++++++++++++++++++++++++++++ terraform/transform_provider.go | 36 +++++++++----- 4 files changed, 165 insertions(+), 13 deletions(-) create mode 100644 terraform/node_provider.go create mode 100644 terraform/transform_attach_config.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index a853ed33c..eeda2a857 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -38,6 +38,14 @@ func (b *ApplyGraphBuilder) Build(path []string) (*Graph, error) { // See GraphBuilder func (b *ApplyGraphBuilder) Steps() []GraphTransformer { + // Custom factory for creating providers. + providerFactory := func(name string, path []string) GraphNodeProvider { + return &NodeApplyableProvider{ + NameValue: name, + PathValue: path, + } + } + steps := []GraphTransformer{ // Creates all the nodes represented in the diff. &DiffTransformer{ @@ -47,10 +55,13 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { }, // Create all the providers - &MissingProviderTransformer{Providers: b.Providers}, + &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, &ProviderTransformer{}, &ParentProviderTransformer{}, + // Attach the configurations + &AttachConfigTransformer{Module: b.Module}, + // Single root &RootTransformer{}, } diff --git a/terraform/node_provider.go b/terraform/node_provider.go new file mode 100644 index 000000000..476be64b1 --- /dev/null +++ b/terraform/node_provider.go @@ -0,0 +1,55 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodeApplyableProvider represents a provider during an apply. +// +// NOTE: There is a lot of logic here that will be shared with non-Apply. +// The plan is to abstract that eventually into an embedded abstract struct. +type NodeApplyableProvider struct { + NameValue string + PathValue []string + Config *config.ProviderConfig +} + +func (n *NodeApplyableProvider) Name() string { + result := fmt.Sprintf("provider.%s", n.NameValue) + if len(n.PathValue) > 1 { + result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) + } + + return result +} + +// GraphNodeSubPath +func (n *NodeApplyableProvider) Path() []string { + return n.PathValue +} + +// GraphNodeProvider +func (n *NodeApplyableProvider) ProviderName() string { + return n.NameValue +} + +// GraphNodeProvider +func (n *NodeApplyableProvider) ProviderConfig() *config.RawConfig { + if n.Config == nil { + return nil + } + + return n.Config.RawConfig +} + +// GraphNodeAttachProvider +func (n *NodeApplyableProvider) AttachProvider(c *config.ProviderConfig) { + n.Config = c +} + +// GraphNodeEvalable +func (n *NodeApplyableProvider) EvalTree() EvalNode { + return ProviderEvalTree(n.NameValue, nil) +} diff --git a/terraform/transform_attach_config.go b/terraform/transform_attach_config.go new file mode 100644 index 000000000..1802ac5d9 --- /dev/null +++ b/terraform/transform_attach_config.go @@ -0,0 +1,74 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" +) + +// GraphNodeAttachProvider is an interface that must be implemented by nodes +// that want provider configurations attached. +type GraphNodeAttachProvider interface { + // Must be implemented to determine the path for the configuration + GraphNodeSubPath + + // ProviderName with no module prefix. Example: "aws". + ProviderName() string + + // Sets the configuration + AttachProvider(*config.ProviderConfig) +} + +// AttachConfigTransformer goes through the graph and attaches configuration +// structures to nodes that implement the interfaces above. +// +// The attached configuration structures are directly from the configuration. +// If they're going to be modified, a copy should be made. +type AttachConfigTransformer struct { + Module *module.Tree // Module is the root module for the config +} + +func (t *AttachConfigTransformer) Transform(g *Graph) error { + if err := t.attachProviders(g); err != nil { + return err + } + + return nil +} + +func (t *AttachConfigTransformer) attachProviders(g *Graph) error { + // Go through and find GraphNodeAttachProvider + for _, v := range g.Vertices() { + // Only care about GraphNodeAttachProvider implementations + apn, ok := v.(GraphNodeAttachProvider) + if !ok { + continue + } + + // TODO: aliases? + + // Determine what we're looking for + path := normalizeModulePath(apn.Path()) + path = path[1:] + name := apn.ProviderName() + log.Printf("[TRACE] Attach provider request: %#v %s", path, name) + + // Get the configuration. + tree := t.Module.Child(path) + if tree == nil { + continue + } + + // Go through the provider configs to find the matching config + for _, p := range tree.Config().ProviderConfigs { + if p.Name == name { + log.Printf("[TRACE] Attaching provider config: %#v", p) + apn.AttachProvider(p) + break + } + } + } + + return nil +} diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 80d51f526..0a762245a 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -163,9 +163,19 @@ func (t *CloseProviderTransformer) Transform(g *Graph) error { type MissingProviderTransformer struct { // Providers is the list of providers we support. Providers []string + + // Factory, if set, overrides how the providers are made. + Factory func(name string, path []string) GraphNodeProvider } func (t *MissingProviderTransformer) Transform(g *Graph) error { + // Initialize factory + if t.Factory == nil { + t.Factory = func(name string, path []string) GraphNodeProvider { + return &graphNodeProvider{ProviderNameValue: name} + } + } + // Create a set of our supported providers supported := make(map[string]struct{}, len(t.Providers)) for _, v := range t.Providers { @@ -217,17 +227,14 @@ func (t *MissingProviderTransformer) Transform(g *Graph) error { } // Add the missing provider node to the graph - raw := &graphNodeProvider{ - ProviderNameValue: p, - PathValue: path, - } - - var v dag.Vertex = raw + v := t.Factory(p, path).(dag.Vertex) if len(path) > 0 { - var err error - v, err = raw.Flatten(path) - if err != nil { - return err + if fn, ok := v.(GraphNodeFlattenable); ok { + var err error + v, err = fn.Flatten(path) + if err != nil { + return err + } } // We'll need the parent provider as well, so let's @@ -347,7 +354,12 @@ func providerVertexMap(g *Graph) map[string]dag.Vertex { m := make(map[string]dag.Vertex) for _, v := range g.Vertices() { if pv, ok := v.(GraphNodeProvider); ok { - m[pv.ProviderName()] = v + key := pv.ProviderName() + if _, ok := v.(*NodeApplyableProvider); ok { + key = providerMapKey(pv.ProviderName(), v) + } + + m[key] = v } } @@ -512,7 +524,6 @@ func (n *graphNodeCloseProvider) DotNode(name string, opts *GraphDotOpts) *dot.N type graphNodeProvider struct { ProviderNameValue string - PathValue []string } func (n *graphNodeProvider) Name() string { @@ -529,6 +540,7 @@ func (n *graphNodeProvider) DependableName() []string { return []string{n.Name()} } +// GraphNodeProvider func (n *graphNodeProvider) ProviderName() string { return n.ProviderNameValue } From 39abec4970084a36860e73e906b11f8eec7c7df3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 00:00:12 -0700 Subject: [PATCH 17/82] terraform: NodeApplyableProvider evals with config --- terraform/context.go | 2 +- terraform/node_provider.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index bde4b1c79..49054ffff 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -19,7 +19,7 @@ type InputMode byte var ( // NOTE: Internal only to toggle between the new and old apply graph - newApplyGraph = false + newApplyGraph = true ) const ( diff --git a/terraform/node_provider.go b/terraform/node_provider.go index 476be64b1..43d45848e 100644 --- a/terraform/node_provider.go +++ b/terraform/node_provider.go @@ -51,5 +51,5 @@ func (n *NodeApplyableProvider) AttachProvider(c *config.ProviderConfig) { // GraphNodeEvalable func (n *NodeApplyableProvider) EvalTree() EvalNode { - return ProviderEvalTree(n.NameValue, nil) + return ProviderEvalTree(n.NameValue, n.ProviderConfig()) } From 4033e90474ff2ccd82a6d98dbfd205ff01b5bee6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 00:02:29 -0700 Subject: [PATCH 18/82] terraform: clarify commment --- terraform/transform_provider.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 0a762245a..5ae8b3fe3 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -355,6 +355,10 @@ func providerVertexMap(g *Graph) map[string]dag.Vertex { for _, v := range g.Vertices() { if pv, ok := v.(GraphNodeProvider); ok { key := pv.ProviderName() + + // This special case is because the new world view of providers + // is that they should return only their pure name (not the full + // module path with ProviderName). Working towards this future. if _, ok := v.(*NodeApplyableProvider); ok { key = providerMapKey(pv.ProviderName(), v) } From 0f0eecfee7f001ba87025f727fdd1d82dd727cae Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 09:32:37 -0700 Subject: [PATCH 19/82] terraform: add provisioner nodes to the apply graph --- terraform/graph_builder_apply.go | 4 ++++ terraform/graph_builder_apply_test.go | 8 ++++++-- terraform/node_resource.go | 17 +++++++++++++++++ .../graph-builder-apply-basic/child/main.tf | 3 +++ .../graph-builder-apply-basic/main.tf | 3 +++ 5 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 terraform/test-fixtures/graph-builder-apply-basic/child/main.tf create mode 100644 terraform/test-fixtures/graph-builder-apply-basic/main.tf diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index eeda2a857..d5be10162 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -59,6 +59,10 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &ProviderTransformer{}, &ParentProviderTransformer{}, + // Provisioner-related transformations + &MissingProvisionerTransformer{Provisioners: b.Provisioners}, + &ProvisionerTransformer{}, + // Attach the configurations &AttachConfigTransformer{Module: b.Module}, diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index e62521d91..78c5dd61c 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -47,8 +47,10 @@ func TestApplyGraphBuilder(t *testing.T) { } b := &ApplyGraphBuilder{ - Diff: diff, - Providers: []string{"aws"}, + Module: testModule(t, "graph-builder-apply-basic"), + Diff: diff, + Providers: []string{"aws"}, + Provisioners: []string{"exec"}, } g, err := b.Build(RootModulePath) @@ -72,9 +74,11 @@ aws_instance.create provider.aws module.child.aws_instance.create module.child.provider.aws + provisioner.exec module.child.provider.aws provider.aws provider.aws +provisioner.exec root aws_instance.create module.child.aws_instance.create diff --git a/terraform/node_resource.go b/terraform/node_resource.go index 6373ba167..9d05d60f0 100644 --- a/terraform/node_resource.go +++ b/terraform/node_resource.go @@ -39,6 +39,23 @@ func (n *NodeApplyableResource) ProvidedBy() []string { return []string{resourceProvider(n.Addr.Type, "")} } +// GraphNodeProvisionerConsumer +func (n *NodeApplyableResource) ProvisionedBy() []string { + // If we have no configuration, then we have no provisioners + if n.Config == nil { + return nil + } + + // Build the list of provisioners we need based on the configuration. + // It is okay to have duplicates here. + result := make([]string, len(n.Config.Provisioners)) + for i, p := range n.Config.Provisioners { + result[i] = p.Type + } + + return result +} + // GraphNodeEvalable func (n *NodeApplyableResource) EvalTree() EvalNode { // stateId is the ID to put into the state diff --git a/terraform/test-fixtures/graph-builder-apply-basic/child/main.tf b/terraform/test-fixtures/graph-builder-apply-basic/child/main.tf new file mode 100644 index 000000000..e486aaa2c --- /dev/null +++ b/terraform/test-fixtures/graph-builder-apply-basic/child/main.tf @@ -0,0 +1,3 @@ +resource "aws_instance" "create" { + provisioner "exec" {} +} diff --git a/terraform/test-fixtures/graph-builder-apply-basic/main.tf b/terraform/test-fixtures/graph-builder-apply-basic/main.tf new file mode 100644 index 000000000..0f6991c53 --- /dev/null +++ b/terraform/test-fixtures/graph-builder-apply-basic/main.tf @@ -0,0 +1,3 @@ +module "child" { + source = "./child" +} From 5220cba77cce33ced3d2b27072320653f56a799d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 09:33:08 -0700 Subject: [PATCH 20/82] terraform: enable provisioners to execute --- terraform/node_resource.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/terraform/node_resource.go b/terraform/node_resource.go index 9d05d60f0..27c6b30f3 100644 --- a/terraform/node_resource.go +++ b/terraform/node_resource.go @@ -216,17 +216,14 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { Dependencies: stateDeps, State: &state, }, - /* - TODO: this has to work - &EvalApplyProvisioners{ - Info: info, - State: &state, - Resource: n.Config, - InterpResource: resource, - CreateNew: &createNew, - Error: &err, - }, - */ + &EvalApplyProvisioners{ + Info: info, + State: &state, + Resource: n.Config, + InterpResource: resource, + CreateNew: &createNew, + Error: &err, + }, &EvalIf{ If: func(ctx EvalContext) (bool, error) { return createBeforeDestroyEnabled && err != nil, nil From 7dd48137304b70a1031c93c213b34c2dd1e3f40d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 11:38:49 -0700 Subject: [PATCH 21/82] terraform: rename test to be more easily targetable --- terraform/context_apply_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 46fc8c43d..377aae3da 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -3757,7 +3757,7 @@ func TestContext2Apply_idAttr(t *testing.T) { } } -func TestContext2Apply_output(t *testing.T) { +func TestContext2Apply_outputBasic(t *testing.T) { m := testModule(t, "apply-output") p := testProvider("aws") p.ApplyFn = testApplyFn From e9e8304e95da3a71541c25511b5b9bfda59d6124 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 11:39:11 -0700 Subject: [PATCH 22/82] terraform: new output transform that isn't used yet --- terraform/transform_output.go | 99 ++----------------- terraform/transform_output_orphan.go | 98 ++++++++++++++++++ ...est.go => transform_output_orphan_test.go} | 0 3 files changed, 108 insertions(+), 89 deletions(-) create mode 100644 terraform/transform_output_orphan.go rename terraform/{transform_output_test.go => transform_output_orphan_test.go} (100%) diff --git a/terraform/transform_output.go b/terraform/transform_output.go index d3e839ce1..0aed7b017 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -1,98 +1,19 @@ package terraform import ( - "fmt" - - "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/config/module" ) -// GraphNodeOutput is an interface that nodes that are outputs must -// implement. The OutputName returned is the name of the output key -// that they manage. -type GraphNodeOutput interface { - OutputName() string +// OutputTransformer is a GraphTransformer that adds all the outputs +// in the configuration to the graph. +// +// This is done for the apply graph builder even if dependent nodes +// aren't changing since there is no downside: the state will be available +// even if the dependent items aren't changing. +type OutputTransformer struct { + Module *module.Tree } -// AddOutputOrphanTransformer is a transformer that adds output orphans -// to the graph. Output orphans are outputs that are no longer in the -// configuration and therefore need to be removed from the state. -type AddOutputOrphanTransformer struct { - State *State -} - -func (t *AddOutputOrphanTransformer) Transform(g *Graph) error { - // Get the state for this module. If we have no state, we have no orphans - state := t.State.ModuleByPath(g.Path) - if state == nil { - return nil - } - - // Create the set of outputs we do have in the graph - found := make(map[string]struct{}) - for _, v := range g.Vertices() { - on, ok := v.(GraphNodeOutput) - if !ok { - continue - } - - found[on.OutputName()] = struct{}{} - } - - // Go over all the outputs. If we don't have a graph node for it, - // create it. It doesn't need to depend on anything, since its just - // setting it empty. - for k, _ := range state.Outputs { - if _, ok := found[k]; ok { - continue - } - - g.Add(&graphNodeOrphanOutput{OutputName: k}) - } - +func (t *OutputTransformer) Transform(g *Graph) error { return nil } - -type graphNodeOrphanOutput struct { - OutputName string -} - -func (n *graphNodeOrphanOutput) Name() string { - return fmt.Sprintf("output.%s (orphan)", n.OutputName) -} - -func (n *graphNodeOrphanOutput) EvalTree() EvalNode { - return &EvalOpFilter{ - Ops: []walkOperation{walkApply, walkDestroy, walkRefresh}, - Node: &EvalDeleteOutput{ - Name: n.OutputName, - }, - } -} - -// GraphNodeFlattenable impl. -func (n *graphNodeOrphanOutput) Flatten(p []string) (dag.Vertex, error) { - return &graphNodeOrphanOutputFlat{ - graphNodeOrphanOutput: n, - PathValue: p, - }, nil -} - -type graphNodeOrphanOutputFlat struct { - *graphNodeOrphanOutput - - PathValue []string -} - -func (n *graphNodeOrphanOutputFlat) Name() string { - return fmt.Sprintf( - "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeOrphanOutput.Name()) -} - -func (n *graphNodeOrphanOutputFlat) EvalTree() EvalNode { - return &EvalOpFilter{ - Ops: []walkOperation{walkApply, walkDestroy, walkRefresh}, - Node: &EvalDeleteOutput{ - Name: n.OutputName, - }, - } -} diff --git a/terraform/transform_output_orphan.go b/terraform/transform_output_orphan.go new file mode 100644 index 000000000..d3e839ce1 --- /dev/null +++ b/terraform/transform_output_orphan.go @@ -0,0 +1,98 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/dag" +) + +// GraphNodeOutput is an interface that nodes that are outputs must +// implement. The OutputName returned is the name of the output key +// that they manage. +type GraphNodeOutput interface { + OutputName() string +} + +// AddOutputOrphanTransformer is a transformer that adds output orphans +// to the graph. Output orphans are outputs that are no longer in the +// configuration and therefore need to be removed from the state. +type AddOutputOrphanTransformer struct { + State *State +} + +func (t *AddOutputOrphanTransformer) Transform(g *Graph) error { + // Get the state for this module. If we have no state, we have no orphans + state := t.State.ModuleByPath(g.Path) + if state == nil { + return nil + } + + // Create the set of outputs we do have in the graph + found := make(map[string]struct{}) + for _, v := range g.Vertices() { + on, ok := v.(GraphNodeOutput) + if !ok { + continue + } + + found[on.OutputName()] = struct{}{} + } + + // Go over all the outputs. If we don't have a graph node for it, + // create it. It doesn't need to depend on anything, since its just + // setting it empty. + for k, _ := range state.Outputs { + if _, ok := found[k]; ok { + continue + } + + g.Add(&graphNodeOrphanOutput{OutputName: k}) + } + + return nil +} + +type graphNodeOrphanOutput struct { + OutputName string +} + +func (n *graphNodeOrphanOutput) Name() string { + return fmt.Sprintf("output.%s (orphan)", n.OutputName) +} + +func (n *graphNodeOrphanOutput) EvalTree() EvalNode { + return &EvalOpFilter{ + Ops: []walkOperation{walkApply, walkDestroy, walkRefresh}, + Node: &EvalDeleteOutput{ + Name: n.OutputName, + }, + } +} + +// GraphNodeFlattenable impl. +func (n *graphNodeOrphanOutput) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeOrphanOutputFlat{ + graphNodeOrphanOutput: n, + PathValue: p, + }, nil +} + +type graphNodeOrphanOutputFlat struct { + *graphNodeOrphanOutput + + PathValue []string +} + +func (n *graphNodeOrphanOutputFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeOrphanOutput.Name()) +} + +func (n *graphNodeOrphanOutputFlat) EvalTree() EvalNode { + return &EvalOpFilter{ + Ops: []walkOperation{walkApply, walkDestroy, walkRefresh}, + Node: &EvalDeleteOutput{ + Name: n.OutputName, + }, + } +} diff --git a/terraform/transform_output_test.go b/terraform/transform_output_orphan_test.go similarity index 100% rename from terraform/transform_output_test.go rename to terraform/transform_output_orphan_test.go From 994f5ce773fccfd8e18e30a8a85b80687e6fc490 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 18:30:11 -0700 Subject: [PATCH 23/82] terraform: ReferenceTransform to connect references --- terraform/graph_builder_apply.go | 3 + terraform/graph_builder_apply_test.go | 14 ++- terraform/node_resource.go | 30 +++++ .../graph-builder-apply-basic/main.tf | 6 + terraform/transform_reference.go | 112 ++++++++++++++++++ 5 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 terraform/transform_reference.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index d5be10162..1deea69c5 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -63,6 +63,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &MissingProvisionerTransformer{Provisioners: b.Provisioners}, &ProvisionerTransformer{}, + // Connect references so ordering is correct + &ReferenceTransformer{}, + // Attach the configurations &AttachConfigTransformer{Module: b.Module}, diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 78c5dd61c..6c384a9f4 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -27,6 +27,15 @@ func TestApplyGraphBuilder(t *testing.T) { }, }, }, + + "aws_instance.other": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, }, }, @@ -72,6 +81,9 @@ func TestApplyGraphBuilder(t *testing.T) { const testApplyGraphBuilderStr = ` aws_instance.create provider.aws +aws_instance.other + aws_instance.create + provider.aws module.child.aws_instance.create module.child.provider.aws provisioner.exec @@ -80,6 +92,6 @@ module.child.provider.aws provider.aws provisioner.exec root - aws_instance.create + aws_instance.other module.child.aws_instance.create ` diff --git a/terraform/node_resource.go b/terraform/node_resource.go index 27c6b30f3..0503f0a7b 100644 --- a/terraform/node_resource.go +++ b/terraform/node_resource.go @@ -23,6 +23,36 @@ func (n *NodeApplyableResource) Path() []string { return n.Addr.Path } +// GraphNodeReferenceable +func (n *NodeApplyableResource) ReferenceableName() []string { + if n.Config == nil { + return nil + } + + return []string{n.Config.Id()} +} + +// GraphNodeReferencer +func (n *NodeApplyableResource) References() []string { + // Let's make this a little shorter so it is easier to reference + c := n.Config + if c == nil { + return nil + } + + // Grab all the references + var result []string + result = append(result, c.DependsOn...) + result = append(result, ReferencesFromConfig(c.RawCount)...) + result = append(result, ReferencesFromConfig(c.RawConfig)...) + for _, p := range c.Provisioners { + result = append(result, ReferencesFromConfig(p.ConnInfo)...) + result = append(result, ReferencesFromConfig(p.RawConfig)...) + } + + return result +} + // GraphNodeProviderConsumer func (n *NodeApplyableResource) ProvidedBy() []string { // If we have a config we prefer that above all else diff --git a/terraform/test-fixtures/graph-builder-apply-basic/main.tf b/terraform/test-fixtures/graph-builder-apply-basic/main.tf index 0f6991c53..d077a4ae5 100644 --- a/terraform/test-fixtures/graph-builder-apply-basic/main.tf +++ b/terraform/test-fixtures/graph-builder-apply-basic/main.tf @@ -1,3 +1,9 @@ module "child" { source = "./child" } + +resource "aws_instance" "create" {} + +resource "aws_instance" "other" { + foo = "${aws_instance.create.bar}" +} diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go new file mode 100644 index 000000000..d0c15c118 --- /dev/null +++ b/terraform/transform_reference.go @@ -0,0 +1,112 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" +) + +// GraphNodeReferenceable must be implemented by any node that represents +// a Terraform thing that can be referenced (resource, module, etc.). +type GraphNodeReferenceable interface { + // ReferenceableName is the name by which this can be referenced. + // This can be either just the type, or include the field. Example: + // "aws_instance.bar" or "aws_instance.bar.id". + ReferenceableName() []string +} + +// GraphNodeReferencer must be implemented by nodes that reference other +// Terraform items and therefore depend on them. +type GraphNodeReferencer interface { + // References are the list of things that this node references. This + // can include fields or just the type, just like GraphNodeReferenceable + // above. + References() []string +} + +// ReferenceTransformer is a GraphTransformer that connects all the +// nodes that reference each other in order to form the proper ordering. +type ReferenceTransformer struct{} + +func (t *ReferenceTransformer) Transform(g *Graph) error { + // Build the mapping of reference => vertex for efficient lookups. + refMap := make(map[string][]dag.Vertex) + for _, v := range g.Vertices() { + // We're only looking for referenceable nodes + rn, ok := v.(GraphNodeReferenceable) + if !ok { + continue + } + + // If this node represents a sub path then we prefix + var prefix string + if pn, ok := v.(GraphNodeSubPath); ok { + if path := normalizeModulePath(pn.Path()); len(path) > 1 { + prefix = modulePrefixStr(path[1:]) + "." + } + } + + // Go through and cache them + for _, n := range rn.ReferenceableName() { + n = prefix + n + refMap[n] = append(refMap[n], v) + } + } + + // Find the things that reference things and connect them + for _, v := range g.Vertices() { + rn, ok := v.(GraphNodeReferencer) + if !ok { + continue + } + + // If this node represents a sub path then we prefix + var prefix string + if pn, ok := v.(GraphNodeSubPath); ok { + if path := normalizeModulePath(pn.Path()); len(path) > 1 { + prefix = modulePrefixStr(path[1:]) + "." + } + } + + for _, n := range rn.References() { + n = prefix + n + if parents, ok := refMap[n]; ok { + for _, parent := range parents { + g.Connect(dag.BasicEdge(v, parent)) + } + } + } + } + + return nil +} + +// ReferencesFromConfig returns the references that a configuration has +// based on the interpolated variables in a configuration. +func ReferencesFromConfig(c *config.RawConfig) []string { + var result []string + for _, v := range c.Variables { + if r := ReferenceFromInterpolatedVar(v); r != "" { + result = append(result, r) + } + + } + + return result +} + +// ReferenceFromInterpolatedVar returns the reference from this variable, +// or an empty string if there is no reference. +func ReferenceFromInterpolatedVar(v config.InterpolatedVariable) string { + switch v := v.(type) { + case *config.ModuleVariable: + return fmt.Sprintf("module.%s.output.%s", v.Name, v.Field) + case *config.ResourceVariable: + return v.ResourceId() + case *config.UserVariable: + return fmt.Sprintf("var.%s", v.Name) + default: + return "" + } +} From 21888b12273c2484d94973c7b891458ef3caa666 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 18:31:13 -0700 Subject: [PATCH 24/82] terraform: test for referencetransform for modules --- terraform/graph_builder_apply_test.go | 14 +++++++++++++- .../graph-builder-apply-basic/child/main.tf | 4 ++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 6c384a9f4..ff6d71e28 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -50,6 +50,15 @@ func TestApplyGraphBuilder(t *testing.T) { }, }, }, + + "aws_instance.other": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, }, }, }, @@ -87,11 +96,14 @@ aws_instance.other module.child.aws_instance.create module.child.provider.aws provisioner.exec +module.child.aws_instance.other + module.child.aws_instance.create + module.child.provider.aws module.child.provider.aws provider.aws provider.aws provisioner.exec root aws_instance.other - module.child.aws_instance.create + module.child.aws_instance.other ` diff --git a/terraform/test-fixtures/graph-builder-apply-basic/child/main.tf b/terraform/test-fixtures/graph-builder-apply-basic/child/main.tf index e486aaa2c..b802817b8 100644 --- a/terraform/test-fixtures/graph-builder-apply-basic/child/main.tf +++ b/terraform/test-fixtures/graph-builder-apply-basic/child/main.tf @@ -1,3 +1,7 @@ resource "aws_instance" "create" { provisioner "exec" {} } + +resource "aws_instance" "other" { + value = "${aws_instance.create.id}" +} From ba512952674b497b19d367ead13cd68715b1a205 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 18:41:09 -0700 Subject: [PATCH 25/82] terraform: ReferenceTransform test --- terraform/transform_reference_test.go | 97 +++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 terraform/transform_reference_test.go diff --git a/terraform/transform_reference_test.go b/terraform/transform_reference_test.go new file mode 100644 index 000000000..f9c7715e7 --- /dev/null +++ b/terraform/transform_reference_test.go @@ -0,0 +1,97 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestReferenceTransformer_simple(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeRefParentTest{ + NameValue: "A", + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "B", + Refs: []string{"A"}, + }) + + tf := &ReferenceTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformRefBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestReferenceTransformer_path(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeRefParentTest{ + NameValue: "A", + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "B", + Refs: []string{"A"}, + }) + g.Add(&graphNodeRefParentTest{ + NameValue: "child.A", + PathValue: []string{"root", "child"}, + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "child.B", + PathValue: []string{"root", "child"}, + Refs: []string{"A"}, + }) + + tf := &ReferenceTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformRefPathStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +type graphNodeRefParentTest struct { + NameValue string + PathValue []string + Names []string +} + +func (n *graphNodeRefParentTest) Name() string { return n.NameValue } +func (n *graphNodeRefParentTest) ReferenceableName() []string { return n.Names } +func (n *graphNodeRefParentTest) Path() []string { return n.PathValue } + +type graphNodeRefChildTest struct { + NameValue string + PathValue []string + Refs []string +} + +func (n *graphNodeRefChildTest) Name() string { return n.NameValue } +func (n *graphNodeRefChildTest) References() []string { return n.Refs } +func (n *graphNodeRefChildTest) Path() []string { return n.PathValue } + +const testTransformRefBasicStr = ` +A +B + A +` + +const testTransformRefPathStr = ` +A +B + A +child.A +child.B + child.A +` From 0d7674b0791037918d5347b52c1789448c6c9776 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 15 Sep 2016 23:20:35 -0700 Subject: [PATCH 26/82] terraform: apply builder adds outputs to graphs --- terraform/graph_builder_apply.go | 3 ++ terraform/node_output.go | 57 ++++++++++++++++++++ terraform/transform_output.go | 55 ++++++++++++++++++++ terraform/transform_reference.go | 89 ++++++++++++++++++++++---------- 4 files changed, 176 insertions(+), 28 deletions(-) create mode 100644 terraform/node_output.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 1deea69c5..580dc7f31 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -63,6 +63,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &MissingProvisionerTransformer{Provisioners: b.Provisioners}, &ProvisionerTransformer{}, + // Add the outputs + &OutputTransformer{Module: b.Module}, + // Connect references so ordering is correct &ReferenceTransformer{}, diff --git a/terraform/node_output.go b/terraform/node_output.go new file mode 100644 index 000000000..517a795af --- /dev/null +++ b/terraform/node_output.go @@ -0,0 +1,57 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodeApplyableOutput represents an output that is "applyable": +// it is ready to be applied. +type NodeApplyableOutput struct { + PathValue []string + Config *config.Output // Config is the output in the config +} + +func (n *NodeApplyableOutput) Name() string { + result := fmt.Sprintf("output.%s", n.Config.Name) + if len(n.PathValue) > 1 { + result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) + } + + return result +} + +// GraphNodeSubPath +func (n *NodeApplyableOutput) Path() []string { + return n.PathValue +} + +// GraphNodeReferenceable +func (n *NodeApplyableOutput) ReferenceableName() []string { + return []string{n.Name()} +} + +// GraphNodeReferencer +func (n *NodeApplyableOutput) References() []string { + var result []string + result = append(result, ReferencesFromConfig(n.Config.RawConfig)...) + return result +} + +// GraphNodeEvalable +func (n *NodeApplyableOutput) EvalTree() EvalNode { + return &EvalOpFilter{ + Ops: []walkOperation{walkRefresh, walkPlan, walkApply, + walkDestroy, walkInput, walkValidate}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalWriteOutput{ + Name: n.Config.Name, + Sensitive: n.Config.Sensitive, + Value: n.Config.RawConfig, + }, + }, + }, + } +} diff --git a/terraform/transform_output.go b/terraform/transform_output.go index 0aed7b017..3e5f33986 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -1,6 +1,8 @@ package terraform import ( + "log" + "github.com/hashicorp/terraform/config/module" ) @@ -15,5 +17,58 @@ type OutputTransformer struct { } func (t *OutputTransformer) Transform(g *Graph) error { + return t.transform(g, t.Module) +} + +func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { + // If no config, no outputs + if m == nil { + return nil + } + + // Transform all the children. We must do this first because + // we can reference module outputs and they must show up in the + // reference map. + for _, c := range m.Children() { + if err := t.transform(g, c); err != nil { + return err + } + } + + // If we have no outputs, we're done! + os := m.Config().Outputs + if len(os) == 0 { + return nil + } + + // Build the reference map so we can determine if we're referencing things. + refMap := NewReferenceMap(g.Vertices()) + + // Add all outputs here + for _, o := range os { + // Build the node + node := &NodeApplyableOutput{ + PathValue: m.Path(), + Config: o, + } + + // If the node references something, then we check to make sure + // that the thing it references is in the graph. If it isn't, then + // we don't add it because we may not be able to compute the output. + // + // If the node references nothing, we always include it since there + // is no other clear time to compute it. + matches, missing := refMap.References(node) + if len(matches) == 0 || len(missing) > 0 { + log.Printf( + "[INFO] Not including %q in graph, matches: %v, missing: %s", + node, matches, missing) + continue + } + + // Add it! + g.Add(node) + } + return nil } diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index d0c15c118..0f061dda6 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -30,9 +30,67 @@ type GraphNodeReferencer interface { type ReferenceTransformer struct{} func (t *ReferenceTransformer) Transform(g *Graph) error { - // Build the mapping of reference => vertex for efficient lookups. + // Build a reference map so we can efficiently look up the references + vs := g.Vertices() + m := NewReferenceMap(vs) + + // Find the things that reference things and connect them + for _, v := range vs { + parents, _ := m.References(v) + for _, parent := range parents { + g.Connect(dag.BasicEdge(v, parent)) + } + } + + return nil +} + +// ReferenceMap is a structure that can be used to efficiently check +// for references on a graph. +type ReferenceMap struct { + // m is the mapping of referenceable name to list of verticies that + // implement that name. This is built on initialization. + m map[string][]dag.Vertex +} + +// References returns the list of vertices that this vertex +// references along with any missing references. +func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) { + rn, ok := v.(GraphNodeReferencer) + if !ok { + return nil, nil + } + + // If this node represents a sub path then we prefix + var prefix string + if pn, ok := v.(GraphNodeSubPath); ok { + if path := normalizeModulePath(pn.Path()); len(path) > 1 { + prefix = modulePrefixStr(path[1:]) + "." + } + } + + var matches []dag.Vertex + var missing []string + for _, n := range rn.References() { + n = prefix + n + parents, ok := m.m[n] + if !ok { + missing = append(missing, n) + continue + } + + matches = append(matches, parents...) + } + + return matches, missing +} + +// NewReferenceMap is used to create a new reference map for the +// given set of vertices. +func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { + // Build the lookup table refMap := make(map[string][]dag.Vertex) - for _, v := range g.Vertices() { + for _, v := range vs { // We're only looking for referenceable nodes rn, ok := v.(GraphNodeReferenceable) if !ok { @@ -54,32 +112,7 @@ func (t *ReferenceTransformer) Transform(g *Graph) error { } } - // Find the things that reference things and connect them - for _, v := range g.Vertices() { - rn, ok := v.(GraphNodeReferencer) - if !ok { - continue - } - - // If this node represents a sub path then we prefix - var prefix string - if pn, ok := v.(GraphNodeSubPath); ok { - if path := normalizeModulePath(pn.Path()); len(path) > 1 { - prefix = modulePrefixStr(path[1:]) + "." - } - } - - for _, n := range rn.References() { - n = prefix + n - if parents, ok := refMap[n]; ok { - for _, parent := range parents { - g.Connect(dag.BasicEdge(v, parent)) - } - } - } - } - - return nil + return &ReferenceMap{m: refMap} } // ReferencesFromConfig returns the references that a configuration has From 7d07f208930495b3675e5ee9952ece0f3423efca Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 00:05:57 -0700 Subject: [PATCH 27/82] terraform: fix references to module outputs --- terraform/node_output.go | 3 ++- terraform/transform_output.go | 7 ++++--- terraform/transform_reference.go | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/terraform/node_output.go b/terraform/node_output.go index 517a795af..08bb892d6 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -29,7 +29,8 @@ func (n *NodeApplyableOutput) Path() []string { // GraphNodeReferenceable func (n *NodeApplyableOutput) ReferenceableName() []string { - return []string{n.Name()} + name := fmt.Sprintf("output.%s", n.Config.Name) + return []string{name} } // GraphNodeReferencer diff --git a/terraform/transform_output.go b/terraform/transform_output.go index 3e5f33986..b163860ae 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -4,6 +4,7 @@ import ( "log" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" ) // OutputTransformer is a GraphTransformer that adds all the outputs @@ -48,7 +49,7 @@ func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { for _, o := range os { // Build the node node := &NodeApplyableOutput{ - PathValue: m.Path(), + PathValue: normalizeModulePath(m.Path()), Config: o, } @@ -59,10 +60,10 @@ func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { // If the node references nothing, we always include it since there // is no other clear time to compute it. matches, missing := refMap.References(node) - if len(matches) == 0 || len(missing) > 0 { + if len(missing) > 0 { log.Printf( "[INFO] Not including %q in graph, matches: %v, missing: %s", - node, matches, missing) + dag.VertexName(node), matches, missing) continue } diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 0f061dda6..c678091b0 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -101,7 +101,7 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { var prefix string if pn, ok := v.(GraphNodeSubPath); ok { if path := normalizeModulePath(pn.Path()); len(path) > 1 { - prefix = modulePrefixStr(path[1:]) + "." + prefix = modulePrefixStr(path) + "." } } From f2aa8806253f187fb3f2269c16712caca5a65f7b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 00:10:19 -0700 Subject: [PATCH 28/82] terraform: proper prefix for output connects --- terraform/transform_reference.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index c678091b0..43fdfc91d 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -65,7 +65,7 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) { var prefix string if pn, ok := v.(GraphNodeSubPath); ok { if path := normalizeModulePath(pn.Path()); len(path) > 1 { - prefix = modulePrefixStr(path[1:]) + "." + prefix = modulePrefixStr(path) + "." } } From 6376c4ca9b16ea1d62a234aeda0d76f96a4511f9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 00:14:06 -0700 Subject: [PATCH 29/82] terraform: update comment --- terraform/transform_output.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/terraform/transform_output.go b/terraform/transform_output.go index b163860ae..1462f8cea 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -47,7 +47,11 @@ func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { // Add all outputs here for _, o := range os { - // Build the node + // Build the node. + // + // NOTE: For now this is just an "applyable" output. As we build + // new graph builders for the other operations I suspect we'll + // find a way to parameterize this, require new transforms, etc. node := &NodeApplyableOutput{ PathValue: normalizeModulePath(m.Path()), Config: o, From 4dfdc52ba08f0c22c36d94f1563c213aeb214261 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 13:56:10 -0700 Subject: [PATCH 30/82] terraform: first stap at module variables, going to redo some things --- terraform/graph_builder_apply.go | 3 + terraform/node_variable.go | 93 +++++++++++++++++++++++++ terraform/transform_reference.go | 57 +++++++++++----- terraform/transform_variable.go | 114 +++++++++++++++++++++++++++++++ 4 files changed, 250 insertions(+), 17 deletions(-) create mode 100644 terraform/node_variable.go create mode 100644 terraform/transform_variable.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 580dc7f31..93212e210 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -63,6 +63,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &MissingProvisionerTransformer{Provisioners: b.Provisioners}, &ProvisionerTransformer{}, + // Add module variables + &VariableTransformer{Module: b.Module}, + // Add the outputs &OutputTransformer{Module: b.Module}, diff --git a/terraform/node_variable.go b/terraform/node_variable.go new file mode 100644 index 000000000..0f0f80e9e --- /dev/null +++ b/terraform/node_variable.go @@ -0,0 +1,93 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" +) + +// NodeApplyableVariable represents a variable during the apply step. +type NodeApplyableVariable struct { + PathValue []string + Config *config.Variable // Config is the var in the config + Value *config.RawConfig // Value is the value that is set + + Module *module.Tree // Antiquated, want to remove +} + +func (n *NodeApplyableVariable) Name() string { + result := fmt.Sprintf("var.%s", n.Config.Name) + if len(n.PathValue) > 1 { + result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) + } + + return result +} + +// GraphNodeSubPath +func (n *NodeApplyableVariable) Path() []string { + // We execute in the parent scope (above our own module) so that + // we can access the proper interpolations. + if len(n.PathValue) > 2 { + return n.PathValue[:len(n.PathValue)-1] + } + + return nil +} + +// GraphNodeReferenceGlobal +func (n *NodeApplyableVariable) ReferenceGlobal() bool { + // We have to create fully qualified references because we cross + // boundaries here: our ReferenceableName is in one path and our + // References are from another path. + return true +} + +// GraphNodeReferenceable +func (n *NodeApplyableVariable) ReferenceableName() []string { + return []string{n.Name()} +} + +// GraphNodeEvalable +func (n *NodeApplyableVariable) 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]interface{}) + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.Value, + Output: &config, + }, + + &EvalVariableBlock{ + Config: &config, + VariableValues: variables, + }, + + &EvalCoerceMapVariable{ + Variables: variables, + ModulePath: n.PathValue, + ModuleTree: n.Module, + }, + + &EvalTypeCheckVariable{ + Variables: variables, + ModulePath: n.PathValue, + ModuleTree: n.Module, + }, + + &EvalSetVariables{ + Module: &n.PathValue[len(n.PathValue)-1], + Variables: variables, + }, + }, + } +} diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 43fdfc91d..b85877fd4 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -25,6 +25,22 @@ type GraphNodeReferencer interface { References() []string } +// GraphNodeReferenceGlobal is an interface that can optionally be +// implemented. If ReferenceGlobal returns true, then the References() +// and ReferenceableName() must be _fully qualified_ with "module.foo.bar" +// etc. +// +// This allows a node to reference and be referenced by a specific name +// that may cross module boundaries. This can be very dangerous so use +// this wisely. +// +// The primary use case for this is module boundaries (variables coming in). +type GraphNodeReferenceGlobal interface { + // Set to true to signal that references and name are fully + // qualified. See the above docs for more information. + ReferenceGlobal() bool +} + // ReferenceTransformer is a GraphTransformer that connects all the // nodes that reference each other in order to form the proper ordering. type ReferenceTransformer struct{} @@ -61,16 +77,9 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) { return nil, nil } - // If this node represents a sub path then we prefix - var prefix string - if pn, ok := v.(GraphNodeSubPath); ok { - if path := normalizeModulePath(pn.Path()); len(path) > 1 { - prefix = modulePrefixStr(path) + "." - } - } - var matches []dag.Vertex var missing []string + prefix := m.prefix(v) for _, n := range rn.References() { n = prefix + n parents, ok := m.m[n] @@ -85,9 +94,29 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) { return matches, missing } +func (m *ReferenceMap) prefix(v dag.Vertex) string { + // If the node is stating it is already fully qualified then + // we don't have to create the prefix! + if gn, ok := v.(GraphNodeReferenceGlobal); ok && gn.ReferenceGlobal() { + return "" + } + + // Create the prefix based on the path + var prefix string + if pn, ok := v.(GraphNodeSubPath); ok { + if path := normalizeModulePath(pn.Path()); len(path) > 1 { + prefix = modulePrefixStr(path) + "." + } + } + + return prefix +} + // NewReferenceMap is used to create a new reference map for the // given set of vertices. func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { + var m ReferenceMap + // Build the lookup table refMap := make(map[string][]dag.Vertex) for _, v := range vs { @@ -97,22 +126,16 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { continue } - // If this node represents a sub path then we prefix - var prefix string - if pn, ok := v.(GraphNodeSubPath); ok { - if path := normalizeModulePath(pn.Path()); len(path) > 1 { - prefix = modulePrefixStr(path) + "." - } - } - // Go through and cache them + prefix := m.prefix(v) for _, n := range rn.ReferenceableName() { n = prefix + n refMap[n] = append(refMap[n], v) } } - return &ReferenceMap{m: refMap} + m.m = refMap + return &m } // ReferencesFromConfig returns the references that a configuration has diff --git a/terraform/transform_variable.go b/terraform/transform_variable.go new file mode 100644 index 000000000..9200f6cff --- /dev/null +++ b/terraform/transform_variable.go @@ -0,0 +1,114 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" +) + +// VariableTransformer is a GraphTransformer that adds all the variables +// in the configuration to the graph. +// +// This only adds variables that either have no dependencies (and therefore +// always succeed) or has dependencies that are 100% represented in the +// graph. +type VariableTransformer struct { + Module *module.Tree +} + +func (t *VariableTransformer) Transform(g *Graph) error { + return t.transform(g, nil, t.Module) +} + +func (t *VariableTransformer) transform(g *Graph, parent, m *module.Tree) error { + // If no config, no variables + if m == nil { + return nil + } + + // Transform all the children. + for _, c := range m.Children() { + if err := t.transform(g, m, c); err != nil { + return err + } + } + + // If we have no parent, then don't do anything. This is because + // we need to be able to get the set value from the module declaration. + if parent == nil { + return nil + } + + // If we have no vars, we're done! + vars := m.Config().Variables + if len(vars) == 0 { + return nil + } + + // Look for usage of this module + var mod *config.Module + for _, modUse := range parent.Config().Modules { + if modUse.Name == m.Name() { + mod = modUse + break + } + } + if mod == nil { + log.Printf("[INFO] Module %q not used, not adding variables", m.Name()) + return nil + } + + // Build the reference map so we can determine if we're referencing things. + refMap := NewReferenceMap(g.Vertices()) + + // Add all variables here + for _, v := range vars { + // Determine the value of the variable. If it isn't in the + // configuration then it was never set and that's not a problem. + var value *config.RawConfig + if raw, ok := mod.RawConfig.Raw[v.Name]; ok { + var err error + value, err = config.NewRawConfig(map[string]interface{}{ + v.Name: raw, + }) + if err != nil { + // This shouldn't happen because it is already in + // a RawConfig above meaning it worked once before. + panic(err) + } + } + + // Build the node. + // + // NOTE: For now this is just an "applyable" variable. As we build + // new graph builders for the other operations I suspect we'll + // find a way to parameterize this, require new transforms, etc. + node := &NodeApplyableVariable{ + PathValue: normalizeModulePath(m.Path()), + Config: v, + Value: value, + Module: t.Module, + } + + // If the node references something, then we check to make sure + // that the thing it references is in the graph. If it isn't, then + // we don't add it because we may not be able to compute the output. + // + // If the node references nothing, we always include it since there + // is no other clear time to compute it. + matches, missing := refMap.References(node) + if len(missing) > 0 { + log.Printf( + "[INFO] Not including %q in graph, matches: %v, missing: %s", + dag.VertexName(node), matches, missing) + continue + } + + // Add it! + g.Add(node) + } + + return nil +} From ad03a21040b006c04841d4c6463df86c129ac5a8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 14:00:21 -0700 Subject: [PATCH 31/82] terraform: rename to ModuleVariable --- terraform/graph_builder_apply.go | 2 +- .../{node_variable.go => node_module_variable.go} | 15 ++++++++------- terraform/transform_variable.go | 10 +++++----- 3 files changed, 14 insertions(+), 13 deletions(-) rename terraform/{node_variable.go => node_module_variable.go} (80%) diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 93212e210..bb626c89c 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -64,7 +64,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &ProvisionerTransformer{}, // Add module variables - &VariableTransformer{Module: b.Module}, + &ModuleVariableTransformer{Module: b.Module}, // Add the outputs &OutputTransformer{Module: b.Module}, diff --git a/terraform/node_variable.go b/terraform/node_module_variable.go similarity index 80% rename from terraform/node_variable.go rename to terraform/node_module_variable.go index 0f0f80e9e..c50dfa1d1 100644 --- a/terraform/node_variable.go +++ b/terraform/node_module_variable.go @@ -7,8 +7,9 @@ import ( "github.com/hashicorp/terraform/config/module" ) -// NodeApplyableVariable represents a variable during the apply step. -type NodeApplyableVariable struct { +// NodeApplyableModuleVariable represents a module variable input during +// the apply step. +type NodeApplyableModuleVariable struct { PathValue []string Config *config.Variable // Config is the var in the config Value *config.RawConfig // Value is the value that is set @@ -16,7 +17,7 @@ type NodeApplyableVariable struct { Module *module.Tree // Antiquated, want to remove } -func (n *NodeApplyableVariable) Name() string { +func (n *NodeApplyableModuleVariable) Name() string { result := fmt.Sprintf("var.%s", n.Config.Name) if len(n.PathValue) > 1 { result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) @@ -26,7 +27,7 @@ func (n *NodeApplyableVariable) Name() string { } // GraphNodeSubPath -func (n *NodeApplyableVariable) Path() []string { +func (n *NodeApplyableModuleVariable) Path() []string { // We execute in the parent scope (above our own module) so that // we can access the proper interpolations. if len(n.PathValue) > 2 { @@ -37,7 +38,7 @@ func (n *NodeApplyableVariable) Path() []string { } // GraphNodeReferenceGlobal -func (n *NodeApplyableVariable) ReferenceGlobal() bool { +func (n *NodeApplyableModuleVariable) ReferenceGlobal() bool { // We have to create fully qualified references because we cross // boundaries here: our ReferenceableName is in one path and our // References are from another path. @@ -45,12 +46,12 @@ func (n *NodeApplyableVariable) ReferenceGlobal() bool { } // GraphNodeReferenceable -func (n *NodeApplyableVariable) ReferenceableName() []string { +func (n *NodeApplyableModuleVariable) ReferenceableName() []string { return []string{n.Name()} } // GraphNodeEvalable -func (n *NodeApplyableVariable) EvalTree() EvalNode { +func (n *NodeApplyableModuleVariable) EvalTree() EvalNode { // If we have no value, do nothing if n.Value == nil { return &EvalNoop{} diff --git a/terraform/transform_variable.go b/terraform/transform_variable.go index 9200f6cff..13c80845e 100644 --- a/terraform/transform_variable.go +++ b/terraform/transform_variable.go @@ -8,21 +8,21 @@ import ( "github.com/hashicorp/terraform/dag" ) -// VariableTransformer is a GraphTransformer that adds all the variables +// ModuleVariableTransformer is a GraphTransformer that adds all the variables // in the configuration to the graph. // // This only adds variables that either have no dependencies (and therefore // always succeed) or has dependencies that are 100% represented in the // graph. -type VariableTransformer struct { +type ModuleVariableTransformer struct { Module *module.Tree } -func (t *VariableTransformer) Transform(g *Graph) error { +func (t *ModuleVariableTransformer) Transform(g *Graph) error { return t.transform(g, nil, t.Module) } -func (t *VariableTransformer) transform(g *Graph, parent, m *module.Tree) error { +func (t *ModuleVariableTransformer) transform(g *Graph, parent, m *module.Tree) error { // If no config, no variables if m == nil { return nil @@ -85,7 +85,7 @@ func (t *VariableTransformer) transform(g *Graph, parent, m *module.Tree) error // NOTE: For now this is just an "applyable" variable. As we build // new graph builders for the other operations I suspect we'll // find a way to parameterize this, require new transforms, etc. - node := &NodeApplyableVariable{ + node := &NodeApplyableModuleVariable{ PathValue: normalizeModulePath(m.Path()), Config: v, Value: value, From 3fb83f013ec348fe7bd77d3faca8c589752ac5ac Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 14:04:52 -0700 Subject: [PATCH 32/82] terraform: depend on parent items --- terraform/node_module_variable.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index c50dfa1d1..0fb3eb27a 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -50,6 +50,27 @@ func (n *NodeApplyableModuleVariable) ReferenceableName() []string { return []string{n.Name()} } +// GraphNodeReferencer +func (n *NodeApplyableModuleVariable) References() []string { + // If we have no value set, we depend on nothing + if n.Value == nil { + return nil + } + + // Can't depend on anything if we're in the root + path := n.Path() + if len(path) < 2 { + return nil + } + + // Otherwise, we depend on anything that is in our value, but + // specifically in the namespace of the parent path. + // Create the prefix based on the path + prefix := modulePrefixStr(path) + "." + result := ReferencesFromConfig(n.Value) + return modulePrefixList(result, prefix) +} + // GraphNodeEvalable func (n *NodeApplyableModuleVariable) EvalTree() EvalNode { // If we have no value, do nothing From 0d815872e1ff1b952c03a88ceacac07c92ea8c72 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 14:18:43 -0700 Subject: [PATCH 33/82] terraform: tests for module variable node --- terraform/node_module_variable.go | 11 +++-- terraform/node_module_variable_test.go | 66 ++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) create mode 100644 terraform/node_module_variable_test.go diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index 0fb3eb27a..9b7ddbf2f 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -34,7 +34,7 @@ func (n *NodeApplyableModuleVariable) Path() []string { return n.PathValue[:len(n.PathValue)-1] } - return nil + return rootModulePath } // GraphNodeReferenceGlobal @@ -58,15 +58,18 @@ func (n *NodeApplyableModuleVariable) References() []string { } // Can't depend on anything if we're in the root - path := n.Path() - if len(path) < 2 { + if len(n.PathValue) < 2 { return nil } // Otherwise, we depend on anything that is in our value, but // specifically in the namespace of the parent path. // Create the prefix based on the path - prefix := modulePrefixStr(path) + "." + var prefix string + if p := n.Path(); len(p) > 0 { + prefix = modulePrefixStr(p) + } + result := ReferencesFromConfig(n.Value) return modulePrefixList(result, prefix) } diff --git a/terraform/node_module_variable_test.go b/terraform/node_module_variable_test.go new file mode 100644 index 000000000..4fb6663b9 --- /dev/null +++ b/terraform/node_module_variable_test.go @@ -0,0 +1,66 @@ +package terraform + +import ( + "reflect" + "testing" + + "github.com/hashicorp/terraform/config" +) + +func TestNodeApplyableModuleVariablePath(t *testing.T) { + n := &NodeApplyableModuleVariable{ + PathValue: []string{"root", "child"}, + Config: &config.Variable{Name: "foo"}, + } + + expected := []string{"root"} + actual := n.Path() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("%#v != %#v", actual, expected) + } +} + +func TestNodeApplyableModuleVariableReferenceableName(t *testing.T) { + n := &NodeApplyableModuleVariable{ + PathValue: []string{"root", "child"}, + Config: &config.Variable{Name: "foo"}, + } + + expected := []string{"module.child.var.foo"} + actual := n.ReferenceableName() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("%#v != %#v", actual, expected) + } +} + +func TestNodeApplyableModuleVariableReference(t *testing.T) { + n := &NodeApplyableModuleVariable{ + PathValue: []string{"root", "child"}, + Config: &config.Variable{Name: "foo"}, + Value: config.TestRawConfig(t, map[string]interface{}{ + "foo": `${var.foo}`, + }), + } + + expected := []string{"var.foo"} + actual := n.References() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("%#v != %#v", actual, expected) + } +} + +func TestNodeApplyableModuleVariableReference_grandchild(t *testing.T) { + n := &NodeApplyableModuleVariable{ + PathValue: []string{"root", "child", "grandchild"}, + Config: &config.Variable{Name: "foo"}, + Value: config.TestRawConfig(t, map[string]interface{}{ + "foo": `${var.foo}`, + }), + } + + expected := []string{"module.child.var.foo"} + actual := n.References() + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("%#v != %#v", actual, expected) + } +} From 993c29f34aa14523ba2676058bd718d3d96a9aa8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 14:31:22 -0700 Subject: [PATCH 34/82] terraform: move ModuleVariableTransformer to its own file --- terraform/transform_module_test.go | 1 - .../{transform_variable.go => transform_module_variable.go} | 0 2 files changed, 1 deletion(-) delete mode 100644 terraform/transform_module_test.go rename terraform/{transform_variable.go => transform_module_variable.go} (100%) diff --git a/terraform/transform_module_test.go b/terraform/transform_module_test.go deleted file mode 100644 index cc3ee2f47..000000000 --- a/terraform/transform_module_test.go +++ /dev/null @@ -1 +0,0 @@ -package terraform diff --git a/terraform/transform_variable.go b/terraform/transform_module_variable.go similarity index 100% rename from terraform/transform_variable.go rename to terraform/transform_module_variable.go From 0463ad74a81a683fc232aff9346e6c232d22af81 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 14:36:35 -0700 Subject: [PATCH 35/82] terraform: RootVariableTransform --- terraform/graph_builder_apply.go | 3 ++ terraform/node_root_variable.go | 22 ++++++++++ ..._module.go => transform_module_destroy.go} | 0 terraform/transform_variable.go | 40 +++++++++++++++++++ 4 files changed, 65 insertions(+) create mode 100644 terraform/node_root_variable.go rename terraform/{transform_module.go => transform_module_destroy.go} (100%) create mode 100644 terraform/transform_variable.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index bb626c89c..8ff13343f 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -63,6 +63,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &MissingProvisionerTransformer{Provisioners: b.Provisioners}, &ProvisionerTransformer{}, + // Add root variables + &RootVariableTransformer{Module: b.Module}, + // Add module variables &ModuleVariableTransformer{Module: b.Module}, diff --git a/terraform/node_root_variable.go b/terraform/node_root_variable.go new file mode 100644 index 000000000..cb61a4e3a --- /dev/null +++ b/terraform/node_root_variable.go @@ -0,0 +1,22 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodeRootVariable represents a root variable input. +type NodeRootVariable struct { + Config *config.Variable +} + +func (n *NodeRootVariable) Name() string { + result := fmt.Sprintf("var.%s", n.Config.Name) + return result +} + +// GraphNodeReferenceable +func (n *NodeRootVariable) ReferenceableName() []string { + return []string{n.Name()} +} diff --git a/terraform/transform_module.go b/terraform/transform_module_destroy.go similarity index 100% rename from terraform/transform_module.go rename to terraform/transform_module_destroy.go diff --git a/terraform/transform_variable.go b/terraform/transform_variable.go new file mode 100644 index 000000000..b31e2c765 --- /dev/null +++ b/terraform/transform_variable.go @@ -0,0 +1,40 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/config/module" +) + +// RootVariableTransformer is a GraphTransformer that adds all the root +// variables to the graph. +// +// Root variables are currently no-ops but they must be added to the +// graph since downstream things that depend on them must be able to +// reach them. +type RootVariableTransformer struct { + Module *module.Tree +} + +func (t *RootVariableTransformer) Transform(g *Graph) error { + // If no config, no variables + if t.Module == nil { + return nil + } + + // If we have no vars, we're done! + vars := t.Module.Config().Variables + if len(vars) == 0 { + return nil + } + + // Add all variables here + for _, v := range vars { + node := &NodeRootVariable{ + Config: v, + } + + // Add it! + g.Add(node) + } + + return nil +} From 0e666aa57558f16c171689a54385ac01b923de3b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 14:48:40 -0700 Subject: [PATCH 36/82] terraform: get tests to not panic on failures --- terraform/context_apply_test.go | 6 ++++++ terraform/transform_output.go | 20 -------------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 377aae3da..4d4d7b7d9 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1759,6 +1759,8 @@ func TestContext2Apply_multiVar(t *testing.T) { t.Fatalf("bad: \n%s", actual) } + t.Logf("Initial state: %s", state.String()) + // Apply again, reduce the count to 1 { ctx := testContext2(t, &ContextOpts{ @@ -1782,6 +1784,10 @@ func TestContext2Apply_multiVar(t *testing.T) { } actual := state.RootModule().Outputs["output"] + if actual == nil { + t.Fatal("missing output") + } + expected := "bar0" if actual.Value != expected { t.Fatalf("bad: \n%s", actual) diff --git a/terraform/transform_output.go b/terraform/transform_output.go index 1462f8cea..b260f4caa 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -1,10 +1,7 @@ package terraform import ( - "log" - "github.com/hashicorp/terraform/config/module" - "github.com/hashicorp/terraform/dag" ) // OutputTransformer is a GraphTransformer that adds all the outputs @@ -42,9 +39,6 @@ func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { return nil } - // Build the reference map so we can determine if we're referencing things. - refMap := NewReferenceMap(g.Vertices()) - // Add all outputs here for _, o := range os { // Build the node. @@ -57,20 +51,6 @@ func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { Config: o, } - // If the node references something, then we check to make sure - // that the thing it references is in the graph. If it isn't, then - // we don't add it because we may not be able to compute the output. - // - // If the node references nothing, we always include it since there - // is no other clear time to compute it. - matches, missing := refMap.References(node) - if len(missing) > 0 { - log.Printf( - "[INFO] Not including %q in graph, matches: %v, missing: %s", - dag.VertexName(node), matches, missing) - continue - } - // Add it! g.Add(node) } From dfa02e4412b351c38981c71b5c2851b629221848 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 16:30:29 -0700 Subject: [PATCH 37/82] terraform: rename attach config to only attach provider config --- config/testing.go | 15 +++++++++++++++ terraform/context_apply_test.go | 2 +- terraform/graph_builder_apply.go | 4 +--- ...fig.go => transform_attach_config_provider.go} | 11 ++++++----- terraform/transform_diff.go | 4 ++++ 5 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 config/testing.go rename terraform/{transform_attach_config.go => transform_attach_config_provider.go} (82%) diff --git a/config/testing.go b/config/testing.go new file mode 100644 index 000000000..f7bfadd9e --- /dev/null +++ b/config/testing.go @@ -0,0 +1,15 @@ +package config + +import ( + "testing" +) + +// TestRawConfig is used to create a RawConfig for testing. +func TestRawConfig(t *testing.T, c map[string]interface{}) *RawConfig { + cfg, err := NewRawConfig(c) + if err != nil { + t.Fatalf("err: %s", err) + } + + return cfg +} diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 4d4d7b7d9..b6955b5c5 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -960,7 +960,7 @@ func TestContext2Apply_countDecrease(t *testing.T) { } } -func TestContext2Apply_countDecreaseToOne(t *testing.T) { +func TestContext2Apply_countDecreaseToOneX(t *testing.T) { m := testModule(t, "apply-count-dec-one") p := testProvider("aws") p.ApplyFn = testApplyFn diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 8ff13343f..f5e7e7b85 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -58,6 +58,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, &ProviderTransformer{}, &ParentProviderTransformer{}, + &AttachProviderConfigTransformer{Module: b.Module}, // Provisioner-related transformations &MissingProvisionerTransformer{Provisioners: b.Provisioners}, @@ -75,9 +76,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Connect references so ordering is correct &ReferenceTransformer{}, - // Attach the configurations - &AttachConfigTransformer{Module: b.Module}, - // Single root &RootTransformer{}, } diff --git a/terraform/transform_attach_config.go b/terraform/transform_attach_config_provider.go similarity index 82% rename from terraform/transform_attach_config.go rename to terraform/transform_attach_config_provider.go index 1802ac5d9..4b41a2d0f 100644 --- a/terraform/transform_attach_config.go +++ b/terraform/transform_attach_config_provider.go @@ -20,16 +20,17 @@ type GraphNodeAttachProvider interface { AttachProvider(*config.ProviderConfig) } -// AttachConfigTransformer goes through the graph and attaches configuration -// structures to nodes that implement the interfaces above. +// AttachProviderConfigTransformer goes through the graph and attaches +// provider configuration structures to nodes that implement the interfaces +// above. // // The attached configuration structures are directly from the configuration. // If they're going to be modified, a copy should be made. -type AttachConfigTransformer struct { +type AttachProviderConfigTransformer struct { Module *module.Tree // Module is the root module for the config } -func (t *AttachConfigTransformer) Transform(g *Graph) error { +func (t *AttachProviderConfigTransformer) Transform(g *Graph) error { if err := t.attachProviders(g); err != nil { return err } @@ -37,7 +38,7 @@ func (t *AttachConfigTransformer) Transform(g *Graph) error { return nil } -func (t *AttachConfigTransformer) attachProviders(g *Graph) error { +func (t *AttachProviderConfigTransformer) attachProviders(g *Graph) error { // Go through and find GraphNodeAttachProvider for _, v := range g.Vertices() { // Only care about GraphNodeAttachProvider implementations diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index f0455f5e3..8f91d5729 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "github.com/hashicorp/terraform/config/module" ) @@ -30,12 +31,15 @@ func (t *DiffTransformer) Transform(g *Graph) error { } // Go through all the modules in the diff. + log.Printf("[TRACE] DiffTransformer: starting") var nodes []*NodeApplyableResource for _, m := range t.Diff.Modules { + log.Printf("[TRACE] DiffTransformer: Module: %s", m) // TODO: If this is a destroy diff then add a module destroy node // Go through all the resources in this module. for name, inst := range m.Resources { + log.Printf("[TRACE] DiffTransformer: Resource %q: %#v", name, inst) // TODO: Destroy diff // If this diff has no attribute changes, then we have From 2e8a419fd83c4eae8a1deb096a66815ac80ea417 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 17:29:36 -0700 Subject: [PATCH 38/82] terraform: starting work on destroy --- ...ode_resource.go => node_resource_apply.go} | 0 terraform/node_resource_destroy.go | 51 +++++++++++++++++++ terraform/transform_diff.go | 5 +- 3 files changed, 55 insertions(+), 1 deletion(-) rename terraform/{node_resource.go => node_resource_apply.go} (100%) create mode 100644 terraform/node_resource_destroy.go diff --git a/terraform/node_resource.go b/terraform/node_resource_apply.go similarity index 100% rename from terraform/node_resource.go rename to terraform/node_resource_apply.go diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go new file mode 100644 index 000000000..5eaa80c27 --- /dev/null +++ b/terraform/node_resource_destroy.go @@ -0,0 +1,51 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodeDestroyResource represents a resource that is to be destroyed. +type NodeApplyableResource struct { + Addr *ResourceAddress // Addr is the address for this resource +} + +func (n *NodeApplyableResource) Name() string { + return n.Addr.String() +} + +// GraphNodeSubPath +func (n *NodeApplyableResource) Path() []string { + return n.Addr.Path +} + +// GraphNodeReferenceable +func (n *NodeApplyableResource) ReferenceableName() []string { + if n.Config == nil { + return nil + } + + return []string{n.Config.Id()} +} + +// GraphNodeProviderConsumer +func (n *NodeApplyableResource) ProvidedBy() []string { + // If we have a config we prefer that above all else + if n.Config != nil { + return []string{resourceProvider(n.Config.Type, n.Config.Provider)} + } + + // If we have state, then we will use the provider from there + if n.ResourceState != nil { + return []string{n.ResourceState.Provider} + } + + // Use our type + return []string{resourceProvider(n.Addr.Type, "")} +} + +// GraphNodeEvalable +func (n *NodeApplyableResource) EvalTree() EvalNode { + return nil +} diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index 8f91d5729..cae5d7528 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -40,7 +40,10 @@ func (t *DiffTransformer) Transform(g *Graph) error { // Go through all the resources in this module. for name, inst := range m.Resources { log.Printf("[TRACE] DiffTransformer: Resource %q: %#v", name, inst) - // TODO: Destroy diff + + // TODO: destroy + if inst.Destroy { + } // If this diff has no attribute changes, then we have // nothing to do and therefore won't add it to the graph. From 9e8cd48cdaad9f61cc7aaede99c31dd96115eaf9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 20:26:10 -0700 Subject: [PATCH 39/82] terraform: add destroy nodes, destroys kind of work --- terraform/graph_builder_apply.go | 3 + terraform/node_resource_apply.go | 10 ++ terraform/node_resource_destroy.go | 152 +++++++++++++++++++++++----- terraform/transform_attach_state.go | 68 +++++++++++++ terraform/transform_diff.go | 48 +++------ 5 files changed, 220 insertions(+), 61 deletions(-) create mode 100644 terraform/transform_attach_state.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index f5e7e7b85..fe415c49f 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -54,6 +54,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { State: b.State, }, + // Attach the state + &AttachStateTransformer{State: b.State}, + // Create all the providers &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, &ProviderTransformer{}, diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 0503f0a7b..c9c987474 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -86,6 +86,16 @@ func (n *NodeApplyableResource) ProvisionedBy() []string { return result } +// GraphNodeAttachResourceState +func (n *NodeApplyableResource) ResourceAddr() *ResourceAddress { + return n.Addr +} + +// GraphNodeAttachResourceState +func (n *NodeApplyableResource) AttachResourceState(s *ResourceState) { + n.ResourceState = s +} + // GraphNodeEvalable func (n *NodeApplyableResource) EvalTree() EvalNode { // stateId is the ID to put into the state diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 5eaa80c27..9955a7c4b 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -2,42 +2,27 @@ package terraform import ( "fmt" - - "github.com/hashicorp/terraform/config" ) // NodeDestroyResource represents a resource that is to be destroyed. -type NodeApplyableResource struct { - Addr *ResourceAddress // Addr is the address for this resource +type NodeDestroyResource struct { + Addr *ResourceAddress // Addr is the address for this resource + ResourceState *ResourceState // State is the resource state for this resource } -func (n *NodeApplyableResource) Name() string { +func (n *NodeDestroyResource) Name() string { return n.Addr.String() } // GraphNodeSubPath -func (n *NodeApplyableResource) Path() []string { +func (n *NodeDestroyResource) Path() []string { return n.Addr.Path } -// GraphNodeReferenceable -func (n *NodeApplyableResource) ReferenceableName() []string { - if n.Config == nil { - return nil - } - - return []string{n.Config.Id()} -} - // GraphNodeProviderConsumer -func (n *NodeApplyableResource) ProvidedBy() []string { - // If we have a config we prefer that above all else - if n.Config != nil { - return []string{resourceProvider(n.Config.Type, n.Config.Provider)} - } - +func (n *NodeDestroyResource) ProvidedBy() []string { // If we have state, then we will use the provider from there - if n.ResourceState != nil { + if n.ResourceState != nil && n.ResourceState.Provider != "" { return []string{n.ResourceState.Provider} } @@ -45,7 +30,124 @@ func (n *NodeApplyableResource) ProvidedBy() []string { return []string{resourceProvider(n.Addr.Type, "")} } -// GraphNodeEvalable -func (n *NodeApplyableResource) EvalTree() EvalNode { - return nil +// GraphNodeAttachResourceState +func (n *NodeDestroyResource) ResourceAddr() *ResourceAddress { + return n.Addr +} + +// GraphNodeAttachResourceState +func (n *NodeDestroyResource) AttachResourceState(s *ResourceState) { + n.ResourceState = s +} + +// GraphNodeEvalable +func (n *NodeDestroyResource) EvalTree() EvalNode { + // stateId is the ID to put into the state + stateId := n.Addr.stateId() + if n.Addr.Index > -1 { + stateId = fmt.Sprintf("%s.%d", stateId, n.Addr.Index) + } + + // Build the instance info. More of this will be populated during eval + info := &InstanceInfo{ + Id: stateId, + Type: n.Addr.Type, + } + + // Get our state + rs := n.ResourceState + if rs == nil { + rs = &ResourceState{} + } + rs.Provider = n.ProvidedBy()[0] + + var diffApply *InstanceDiff + var provider ResourceProvider + var state *InstanceState + var err error + return &EvalOpFilter{ + Ops: []walkOperation{walkApply, walkDestroy}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + // Get the saved diff for apply + &EvalReadDiff{ + Name: stateId, + Diff: &diffApply, + }, + + // Filter the diff so we only get the destroy + &EvalFilterDiff{ + Diff: &diffApply, + Output: &diffApply, + Destroy: true, + }, + + // If we're not destroying, then compare diffs + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if diffApply != nil && diffApply.GetDestroy() { + return true, nil + } + + return true, EvalEarlyExitError{} + }, + Then: EvalNoop{}, + }, + + // Load the instance info so we have the module path set + &EvalInstanceInfo{Info: info}, + + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + &EvalReadState{ + Name: stateId, + Output: &state, + }, + &EvalRequireState{ + State: &state, + }, + // Make sure we handle data sources properly. + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + /* TODO: data source + if n.Resource.Mode == config.DataResourceMode { + return true, nil + } + */ + + return false, nil + }, + + Then: &EvalReadDataApply{ + Info: info, + Diff: &diffApply, + Provider: &provider, + Output: &state, + }, + Else: &EvalApply{ + Info: info, + State: &state, + Diff: &diffApply, + Provider: &provider, + Output: &state, + Error: &err, + }, + }, + &EvalWriteState{ + Name: stateId, + ResourceType: n.Addr.Type, + Provider: rs.Provider, + Dependencies: rs.Dependencies, + State: &state, + }, + &EvalApplyPost{ + Info: info, + State: &state, + Error: &err, + }, + }, + }, + } } diff --git a/terraform/transform_attach_state.go b/terraform/transform_attach_state.go new file mode 100644 index 000000000..68a8f3b07 --- /dev/null +++ b/terraform/transform_attach_state.go @@ -0,0 +1,68 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/dag" +) + +// GraphNodeAttachResourceState is an interface that can be implemented +// to request that a ResourceState is attached to the node. +type GraphNodeAttachResourceState interface { + // The address to the resource for the state + ResourceAddr() *ResourceAddress + + // Sets the state + AttachResourceState(*ResourceState) +} + +// AttachStateTransformer goes through the graph and attaches +// state to nodes that implement the interfaces above. +type AttachStateTransformer struct { + State *State // State is the root state +} + +func (t *AttachStateTransformer) Transform(g *Graph) error { + // If no state, then nothing to do + if t.State == nil { + log.Printf("[DEBUG] Not attaching any state: state is nil") + return nil + } + + filter := &StateFilter{State: t.State} + for _, v := range g.Vertices() { + // Only care about nodes requesting we're adding state + an, ok := v.(GraphNodeAttachResourceState) + if !ok { + continue + } + addr := an.ResourceAddr() + + // Get the module state + results, err := filter.Filter(addr.String()) + if err != nil { + return err + } + + // Attach the first resource state we get + found := false + for _, result := range results { + if rs, ok := result.Value.(*ResourceState); ok { + log.Printf( + "[DEBUG] Attaching resource state to %q: %s", + dag.VertexName(v), rs) + an.AttachResourceState(rs) + found = true + break + } + } + + if !found { + log.Printf( + "[DEBUG] Resource state not foudn for %q: %s", + dag.VertexName(v), addr) + } + } + + return nil +} diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index cae5d7528..21fd0bbc6 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -41,16 +41,6 @@ func (t *DiffTransformer) Transform(g *Graph) error { for name, inst := range m.Resources { log.Printf("[TRACE] DiffTransformer: Resource %q: %#v", name, inst) - // TODO: destroy - if inst.Destroy { - } - - // If this diff has no attribute changes, then we have - // nothing to do and therefore won't add it to the graph. - if len(inst.Attributes) == 0 { - continue - } - // We have changes! This is a create or update operation. // First grab the address so we have a unique way to // reference this resource. @@ -64,10 +54,18 @@ func (t *DiffTransformer) Transform(g *Graph) error { // the address. Remove "root" from it. addr.Path = m.Path[1:] - // Add the resource to the graph - nodes = append(nodes, &NodeApplyableResource{ - Addr: addr, - }) + // If we're destroying, add the destroy node + if inst.Destroy { + g.Add(&NodeDestroyResource{Addr: addr}) + } + + // If we have changes, then add the applyable version + if len(inst.Attributes) > 0 { + // Add the resource to the graph + nodes = append(nodes, &NodeApplyableResource{ + Addr: addr, + }) + } } } @@ -97,28 +95,6 @@ func (t *DiffTransformer) Transform(g *Graph) error { break } } - - // Grab the state at this path - if ms := t.State.ModuleByPath(normalizeModulePath(n.Addr.Path)); ms != nil { - for name, rs := range ms.Resources { - // Parse the name for comparison - addr, err := parseResourceAddressInternal(name) - if err != nil { - panic(fmt.Sprintf( - "Error parsing internal name, this is a bug: %q", name)) - } - addr.Path = n.Addr.Path - - // If this is not the same resource, then continue - if !addr.Equals(n.Addr) { - continue - } - - // Same resource! - n.ResourceState = rs - break - } - } } // Add all the nodes to the graph From 80ef7f1acfd34d0a5ffe4caca0a4c609adfabb01 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 16 Sep 2016 20:36:10 -0700 Subject: [PATCH 40/82] terraform: properly compare bad diffs --- terraform/node_resource_apply.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index c9c987474..894b651f1 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -131,7 +131,7 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { // Declare a bunch of variables that are used for state during // evaluation. Most of this are written to by-address below. var provider ResourceProvider - var diff *InstanceDiff + var diff, diffApply *InstanceDiff var state *InstanceState var resourceConfig *ResourceConfig var err error @@ -148,21 +148,21 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { // Get the saved diff for apply &EvalReadDiff{ Name: stateId, - Diff: &diff, + Diff: &diffApply, }, // We don't want to do any destroys &EvalIf{ If: func(ctx EvalContext) (bool, error) { - if diff == nil { + if diffApply == nil { return true, EvalEarlyExitError{} } - if diff.GetDestroy() && diff.GetAttributesLen() == 0 { + if diffApply.GetDestroy() && diffApply.GetAttributesLen() == 0 { return true, EvalEarlyExitError{} } - diff.SetDestroy(false) + diffApply.SetDestroy(false) return true, nil }, Then: EvalNoop{}, @@ -171,8 +171,8 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { &EvalIf{ If: func(ctx EvalContext) (bool, error) { destroy := false - if diff != nil { - destroy = diff.GetDestroy() || diff.RequiresNew() + if diffApply != nil { + destroy = diffApply.GetDestroy() || diffApply.RequiresNew() } createBeforeDestroyEnabled = @@ -214,9 +214,9 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { Config: &resourceConfig, Resource: n.Config, Provider: &provider, - Diff: &diff, + Diff: &diffApply, State: &state, - OutputDiff: &diff, + OutputDiff: &diffApply, }, // Get the saved diff @@ -229,7 +229,7 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { &EvalCompareDiff{ Info: info, One: &diff, - Two: &diff, + Two: &diffApply, }, &EvalGetProvider{ @@ -243,7 +243,7 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { &EvalApply{ Info: info, State: &state, - Diff: &diff, + Diff: &diffApply, Provider: &provider, Output: &state, Error: &err, From cd04ccfa6223d609288f695828e08ab46063a0c4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 17 Sep 2016 17:36:15 -0700 Subject: [PATCH 41/82] terraform: update a test to be easier to target --- terraform/context_apply_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index b6955b5c5..0cee63051 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -3945,7 +3945,7 @@ func TestContext2Apply_outputMultiIndex(t *testing.T) { } } -func TestContext2Apply_taint(t *testing.T) { +func TestContext2Apply_taintX(t *testing.T) { m := testModule(t, "apply-taint") p := testProvider("aws") @@ -3993,8 +3993,10 @@ func TestContext2Apply_taint(t *testing.T) { State: s, }) - if _, err := ctx.Plan(); err != nil { + if p, err := ctx.Plan(); err != nil { t.Fatalf("err: %s", err) + } else { + t.Logf("plan: %s", p) } state, err := ctx.Apply() From 5018617049602d095cff399f3c5c7bb2f29144c9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 17 Sep 2016 17:45:44 -0700 Subject: [PATCH 42/82] terraform: change node name so that it shows up properly --- terraform/node_resource_destroy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 9955a7c4b..417e35b77 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -11,7 +11,7 @@ type NodeDestroyResource struct { } func (n *NodeDestroyResource) Name() string { - return n.Addr.String() + return n.Addr.String() + " (destroy)" } // GraphNodeSubPath From 38b9f7794d23037ee0dd1ee0f1d9a635583dfac3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 18 Sep 2016 21:31:03 -0700 Subject: [PATCH 43/82] terraform: reference transformer shouldn't make loop to self --- terraform/transform_reference.go | 12 ++++++++++++ terraform/transform_reference_test.go | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index b85877fd4..f848274aa 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -88,6 +88,18 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []string) { continue } + // Make sure this isn't a self reference, which isn't included + selfRef := false + for _, p := range parents { + if p == v { + selfRef = true + break + } + } + if selfRef { + continue + } + matches = append(matches, parents...) } diff --git a/terraform/transform_reference_test.go b/terraform/transform_reference_test.go index f9c7715e7..525add6bd 100644 --- a/terraform/transform_reference_test.go +++ b/terraform/transform_reference_test.go @@ -28,6 +28,29 @@ func TestReferenceTransformer_simple(t *testing.T) { } } +func TestReferenceTransformer_self(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeRefParentTest{ + NameValue: "A", + Names: []string{"A"}, + }) + g.Add(&graphNodeRefChildTest{ + NameValue: "B", + Refs: []string{"A", "B"}, + }) + + tf := &ReferenceTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformRefBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestReferenceTransformer_path(t *testing.T) { g := Graph{Path: RootModulePath} g.Add(&graphNodeRefParentTest{ From 2e8cb94a5e0dbb7633a7b68377e89dc7810e98c6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 18 Sep 2016 21:45:19 -0700 Subject: [PATCH 44/82] terraform: orphan outputs are deleted from the state --- terraform/graph_builder_apply.go | 3 ++ terraform/node_output_orphan.go | 32 ++++++++++++++ terraform/transform_orphan_output.go | 64 ++++++++++++++++++++++++++++ terraform/transform_output_orphan.go | 3 ++ 4 files changed, 102 insertions(+) create mode 100644 terraform/node_output_orphan.go create mode 100644 terraform/transform_orphan_output.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index fe415c49f..7a6e735ca 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -54,6 +54,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { State: b.State, }, + // Create orphan output nodes + &OrphanOutputTransformer{Module: b.Module, State: b.State}, + // Attach the state &AttachStateTransformer{State: b.State}, diff --git a/terraform/node_output_orphan.go b/terraform/node_output_orphan.go new file mode 100644 index 000000000..651638d7e --- /dev/null +++ b/terraform/node_output_orphan.go @@ -0,0 +1,32 @@ +package terraform + +import ( + "fmt" +) + +// NodeOutputOrphan represents an output that is an orphan. +type NodeOutputOrphan struct { + OutputName string + PathValue []string +} + +func (n *NodeOutputOrphan) Name() string { + result := fmt.Sprintf("output.%s (orphan)", n.OutputName) + if len(n.PathValue) > 1 { + result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) + } + + return result +} + +// GraphNodeSubPath +func (n *NodeOutputOrphan) Path() []string { + return n.PathValue +} + +// GraphNodeEvalable +func (n *NodeOutputOrphan) EvalTree() EvalNode { + return &EvalDeleteOutput{ + Name: n.OutputName, + } +} diff --git a/terraform/transform_orphan_output.go b/terraform/transform_orphan_output.go new file mode 100644 index 000000000..49568d5bc --- /dev/null +++ b/terraform/transform_orphan_output.go @@ -0,0 +1,64 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" +) + +// OrphanOutputTransformer finds the outputs that aren't present +// in the given config that are in the state and adds them to the graph +// for deletion. +type OrphanOutputTransformer struct { + Module *module.Tree // Root module + State *State // State is the root state +} + +func (t *OrphanOutputTransformer) Transform(g *Graph) error { + if t.State == nil { + log.Printf("[DEBUG] No state, no orphan outputs") + return nil + } + + return t.transform(g, t.Module) +} + +func (t *OrphanOutputTransformer) transform(g *Graph, m *module.Tree) error { + // Get our configuration, and recurse into children + var c *config.Config + if m != nil { + c = m.Config() + for _, child := range m.Children() { + if err := t.transform(g, child); err != nil { + return err + } + } + } + + // Get the state. If there is no state, then we have no orphans! + path := normalizeModulePath(m.Path()) + state := t.State.ModuleByPath(path) + if state == nil { + return nil + } + + // Make a map of the valid outputs + valid := make(map[string]struct{}) + for _, o := range c.Outputs { + valid[o.Name] = struct{}{} + } + + // Go through the outputs and find the ones that aren't in our config. + for n, _ := range state.Outputs { + // If it is in the valid map, then ignore + if _, ok := valid[n]; ok { + continue + } + + // Orphan! + g.Add(&NodeOutputOrphan{OutputName: n, PathValue: path}) + } + + return nil +} diff --git a/terraform/transform_output_orphan.go b/terraform/transform_output_orphan.go index d3e839ce1..ffaa0b7e2 100644 --- a/terraform/transform_output_orphan.go +++ b/terraform/transform_output_orphan.go @@ -16,6 +16,9 @@ type GraphNodeOutput interface { // AddOutputOrphanTransformer is a transformer that adds output orphans // to the graph. Output orphans are outputs that are no longer in the // configuration and therefore need to be removed from the state. +// +// NOTE: This is the _old_ way to add output orphans that is used with +// legacy graph builders. The new way is OrphanOutputTransformer. type AddOutputOrphanTransformer struct { State *State } From ebc7d209a7acbd7bf21bace51a2c651be97f49c1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 19 Sep 2016 09:28:24 -0700 Subject: [PATCH 45/82] terraform: new graph fixes ".0" and "" boundaries on counts --- terraform/eval_count_boundary.go | 78 +++++++++++++++++++++++++++ terraform/graph_builder_apply.go | 7 +++ terraform/node_count_boundary.go | 14 +++++ terraform/transform_count_boundary.go | 28 ++++++++++ 4 files changed, 127 insertions(+) create mode 100644 terraform/eval_count_boundary.go create mode 100644 terraform/node_count_boundary.go create mode 100644 terraform/transform_count_boundary.go diff --git a/terraform/eval_count_boundary.go b/terraform/eval_count_boundary.go new file mode 100644 index 000000000..91e2b904e --- /dev/null +++ b/terraform/eval_count_boundary.go @@ -0,0 +1,78 @@ +package terraform + +import ( + "log" +) + +// EvalCountFixZeroOneBoundaryGlobal is an EvalNode that fixes up the state +// when there is a resource count with zero/one boundary, i.e. fixing +// a resource named "aws_instance.foo" to "aws_instance.foo.0" and vice-versa. +// +// This works on the global state. +type EvalCountFixZeroOneBoundaryGlobal struct{} + +// TODO: test +func (n *EvalCountFixZeroOneBoundaryGlobal) Eval(ctx EvalContext) (interface{}, error) { + // Get the state and lock it since we'll potentially modify it + state, lock := ctx.State() + lock.Lock() + defer lock.Unlock() + + // Prune the state since we require a clean state to work + state.prune() + + // Go through each modules since the boundaries are restricted to a + // module scope. + for _, m := range state.Modules { + if err := n.fixModule(m); err != nil { + return nil, err + } + } + + return nil, nil +} + +func (n *EvalCountFixZeroOneBoundaryGlobal) fixModule(m *ModuleState) error { + // Counts keeps track of keys and their counts + counts := make(map[string]int) + for k, _ := range m.Resources { + // Parse the key + key, err := ParseResourceStateKey(k) + if err != nil { + return err + } + + // Set the index to -1 so that we can keep count + key.Index = -1 + + // Increment + counts[key.String()]++ + } + + // Go through the counts and do the fixup for each resource + for raw, count := range counts { + // Search and replace this resource + search := raw + replace := raw + ".0" + if count < 2 { + search, replace = replace, search + } + log.Printf("[TRACE] EvalCountFixZeroOneBoundaryGlobal: count %d, search %q, replace %q", count, search, replace) + + // Look for the resource state. If we don't have one, then it is okay. + rs, ok := m.Resources[search] + if !ok { + continue + } + + // If the replacement key exists, we just keep both + if _, ok := m.Resources[replace]; ok { + continue + } + + m.Resources[replace] = rs + delete(m.Resources, search) + } + + return nil +} diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 7a6e735ca..4889bda55 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -82,8 +82,15 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Connect references so ordering is correct &ReferenceTransformer{}, + // Add the node to fix the state count boundaries + &CountBoundaryTransformer{}, + // Single root &RootTransformer{}, + + // Perform the transitive reduction to make our graph a bit + // more sane if possible (it usually is possible). + &TransitiveReductionTransformer{}, } return steps diff --git a/terraform/node_count_boundary.go b/terraform/node_count_boundary.go new file mode 100644 index 000000000..bd32c79f3 --- /dev/null +++ b/terraform/node_count_boundary.go @@ -0,0 +1,14 @@ +package terraform + +// NodeCountBoundary fixes any "count boundarie" in the state: resources +// that are named "foo.0" when they should be named "foo" +type NodeCountBoundary struct{} + +func (n *NodeCountBoundary) Name() string { + return "meta.count-boundary (count boundary fixup)" +} + +// GraphNodeEvalable +func (n *NodeCountBoundary) EvalTree() EvalNode { + return &EvalCountFixZeroOneBoundaryGlobal{} +} diff --git a/terraform/transform_count_boundary.go b/terraform/transform_count_boundary.go new file mode 100644 index 000000000..83415f352 --- /dev/null +++ b/terraform/transform_count_boundary.go @@ -0,0 +1,28 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/dag" +) + +// CountBoundaryTransformer adds a node that depends on everything else +// so that it runs last in order to clean up the state for nodes that +// are on the "count boundary": "foo.0" when only one exists becomes "foo" +type CountBoundaryTransformer struct{} + +func (t *CountBoundaryTransformer) Transform(g *Graph) error { + node := &NodeCountBoundary{} + g.Add(node) + + // Depends on everything + for _, v := range g.Vertices() { + // Don't connect to ourselves + if v == node { + continue + } + + // Connect! + g.Connect(dag.BasicEdge(node, v)) + } + + return nil +} From 56b4521d8faccf6a3e67200e8d2ff8c0edfb7d7d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 19 Sep 2016 10:01:57 -0700 Subject: [PATCH 46/82] terraform: provider depends on config references --- terraform/node_provider.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/terraform/node_provider.go b/terraform/node_provider.go index 43d45848e..9b8c34b4d 100644 --- a/terraform/node_provider.go +++ b/terraform/node_provider.go @@ -30,6 +30,15 @@ func (n *NodeApplyableProvider) Path() []string { return n.PathValue } +// GraphNodeReferencer +func (n *NodeApplyableProvider) References() []string { + if n.Config == nil { + return nil + } + + return ReferencesFromConfig(n.Config.RawConfig) +} + // GraphNodeProvider func (n *NodeApplyableProvider) ProviderName() string { return n.NameValue From ceb5c53d5697a6c08b0ea29e7157f903e1c184aa Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 19 Sep 2016 10:04:09 -0700 Subject: [PATCH 47/82] terraform: destroy nodes should call post state update hook --- terraform/node_resource_destroy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 417e35b77..c4cb5fac5 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -147,6 +147,7 @@ func (n *NodeDestroyResource) EvalTree() EvalNode { State: &state, Error: &err, }, + &EvalUpdateStateHook{}, }, }, } From bd5d97f9f582f6b9a7f0df6291287fcccc285516 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Sep 2016 09:32:34 -0700 Subject: [PATCH 48/82] terraform: transform to attach resource configs --- terraform/context_apply_test.go | 61 ++++++++++++++- terraform/graph_builder_apply.go | 3 + terraform/node_resource_apply.go | 5 ++ terraform/transform_attach_config_resource.go | 74 +++++++++++++++++++ terraform/transform_destroy_edge.go | 64 ++++++++++++++++ terraform/transform_diff.go | 28 ------- 6 files changed, 206 insertions(+), 29 deletions(-) create mode 100644 terraform/transform_attach_config_resource.go create mode 100644 terraform/transform_destroy_edge.go diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 0cee63051..841530aee 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2789,7 +2789,7 @@ func TestContext2Apply_Provisioner_ConnInfo(t *testing.T) { } } -func TestContext2Apply_destroy(t *testing.T) { +func TestContext2Apply_destroyX(t *testing.T) { m := testModule(t, "apply-destroy") h := new(HookRecordApplyOrder) p := testProvider("aws") @@ -2849,6 +2849,65 @@ func TestContext2Apply_destroy(t *testing.T) { } } +func TestContext2Apply_destroyOrder(t *testing.T) { + m := testModule(t, "apply-destroy") + h := new(HookRecordApplyOrder) + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Hooks: []Hook{h}, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + // First plan and apply a create operation + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + // Next, plan and apply config-less to force a destroy with "apply" + h.Active = true + ctx = testContext2(t, &ContextOpts{ + State: state, + Module: module.NewEmptyTree(), + Hooks: []Hook{h}, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + state, err = ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + // Test that things were destroyed + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyDestroyStr) + if actual != expected { + t.Fatalf("bad: \n%s", actual) + } + + // Test that things were destroyed _in the right order_ + expected2 := []string{"aws_instance.bar", "aws_instance.foo"} + actual2 := h.IDs + if !reflect.DeepEqual(actual2, expected2) { + t.Fatalf("expected: %#v\n\ngot:%#v", expected2, actual2) + } +} + // https://github.com/hashicorp/terraform/issues/2767 func TestContext2Apply_destroyModulePrefix(t *testing.T) { m := testModule(t, "apply-destroy-module-resource-prefix") diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 4889bda55..d54a70948 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -54,6 +54,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { State: b.State, }, + // Attach the configuration to any resources + &AttachResourceConfigTransformer{Module: b.Module}, + // Create orphan output nodes &OrphanOutputTransformer{Module: b.Module, State: b.State}, diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 894b651f1..7b8d7b0ed 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -91,6 +91,11 @@ func (n *NodeApplyableResource) ResourceAddr() *ResourceAddress { return n.Addr } +// GraphNodeAttachResource +func (n *NodeApplyableResource) AttachResourceConfig(c *config.Resource) { + n.Config = c +} + // GraphNodeAttachResourceState func (n *NodeApplyableResource) AttachResourceState(s *ResourceState) { n.ResourceState = s diff --git a/terraform/transform_attach_config_resource.go b/terraform/transform_attach_config_resource.go new file mode 100644 index 000000000..449d1ac16 --- /dev/null +++ b/terraform/transform_attach_config_resource.go @@ -0,0 +1,74 @@ +package terraform + +import ( + "fmt" + "log" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/config/module" +) + +// GraphNodeAttachResourceConfig is an interface that must be implemented by nodes +// that want resource configurations attached. +type GraphNodeAttachResourceConfig interface { + // ResourceAddr is the address to the resource + ResourceAddr() *ResourceAddress + + // Sets the configuration + AttachResourceConfig(*config.Resource) +} + +// AttachResourceConfigTransformer goes through the graph and attaches +// resource configuration structures to nodes that implement the interfaces +// above. +// +// The attached configuration structures are directly from the configuration. +// If they're going to be modified, a copy should be made. +type AttachResourceConfigTransformer struct { + Module *module.Tree // Module is the root module for the config +} + +func (t *AttachResourceConfigTransformer) Transform(g *Graph) error { + // Go through and find GraphNodeAttachResource + for _, v := range g.Vertices() { + // Only care about GraphNodeAttachResource implementations + arn, ok := v.(GraphNodeAttachResourceConfig) + if !ok { + continue + } + + // Determine what we're looking for + addr := arn.ResourceAddr() + log.Printf("[TRACE] Attach resource request: %s", addr) + + // Get the configuration. + path := normalizeModulePath(addr.Path) + path = path[1:] + tree := t.Module.Child(path) + if tree == nil { + continue + } + + // Go through the resource configs to find the matching config + for _, r := range tree.Config().Resources { + // Get a resource address so we can compare + a, err := parseResourceAddressConfig(r) + if err != nil { + panic(fmt.Sprintf( + "Error parsing config address, this is a bug: %#v", r)) + } + a.Path = addr.Path + + // If this is not the same resource, then continue + if !a.Equals(addr) { + continue + } + + log.Printf("[TRACE] Attaching resource config: %#v", r) + arn.AttachResourceConfig(r) + break + } + } + + return nil +} diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go new file mode 100644 index 000000000..d1e9d194e --- /dev/null +++ b/terraform/transform_destroy_edge.go @@ -0,0 +1,64 @@ +package terraform + +// GraphNodeDestroyer must be implemented by nodes that destroy resources. +type GraphNodeDestroyer interface { + // ResourceAddr is the address of the resource that is being + // destroyed by this node. If this returns nil, then this node + // is not destroying anything. + DestroyAddr() *ResourceAddress +} + +// DestroyEdgeTransformer is a GraphTransformer that creates the proper +// references for destroy resources. Destroy resources are more complex +// in that they must be depend on the destruction of resources that +// in turn depend on the CREATION of the node being destroy. +// +// That is complicated. Visually: +// +// B_d -> A_d -> A -> B +// +// Notice that A destroy depends on B destroy, while B create depends on +// A create. They're inverted. This must be done for example because often +// dependent resources will block parent resources from deleting. Concrete +// example: VPC with subnets, the VPC can't be deleted while there are +// still subnets. +type DestroyEdgeTransformer struct{} + +func (t *DestroyEdgeTransformer) Transform(g *Graph) error { + // Build a map of what is being destroyed (by address string) to + // the list of destroyers. In general there will only be one destroyer + // but to make it more robust we support multiple. + destroyers := make(map[string][]GraphNodeDestroyer) + for _, v := range g.Vertices() { + dn, ok := v.(GraphNodeDestroyer) + if !ok { + continue + } + + addr := dn.DestroyAddr() + if addr == nil { + continue + } + + key := addr.String() + destroyers[key] = append(destroyers[key], dn) + } + + // If we aren't destroying anything, there will be no edges to make + // so just exit early and avoid future work. + if len(destroyers) == 0 { + return nil + } + + // Go through the all destroyers and find what they're destroying. + // Use this to find the dependencies, look up if any of them are being + // destroyed, and to make the proper edge. + for _, ds := range destroyers { + for _, d := range ds { + // TODO + println(d) + } + } + + return nil +} diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index 21fd0bbc6..81bebab9a 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -69,34 +69,6 @@ func (t *DiffTransformer) Transform(g *Graph) error { } } - // NOTE: Lots of room for performance optimizations below. For - // resource-heavy diffs this part alone is probably pretty slow. - - // Annotate all nodes with their config and state - for _, n := range nodes { - // Grab the configuration at this path. - if t := t.Module.Child(n.Addr.Path); t != nil { - for _, r := range t.Config().Resources { - // Get a resource address so we can compare - addr, err := parseResourceAddressConfig(r) - if err != nil { - panic(fmt.Sprintf( - "Error parsing config address, this is a bug: %#v", r)) - } - addr.Path = n.Addr.Path - - // If this is not the same resource, then continue - if !addr.Equals(n.Addr) { - continue - } - - // Same resource! Mark it and exit - n.Config = r - break - } - } - } - // Add all the nodes to the graph for _, n := range nodes { g.Add(n) From 7b2bd9309450e56db5c0b192b5cfafa82c7e3809 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Sep 2016 10:16:49 -0700 Subject: [PATCH 49/82] terraform: test the destroy edge transform --- .../transform-destroy-edge-basic/main.tf | 2 + terraform/transform_attach_config_resource.go | 4 +- terraform/transform_destroy_edge.go | 99 ++++++++++++++++++- terraform/transform_destroy_edge_test.go | 44 +++++++++ 4 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 terraform/test-fixtures/transform-destroy-edge-basic/main.tf create mode 100644 terraform/transform_destroy_edge_test.go diff --git a/terraform/test-fixtures/transform-destroy-edge-basic/main.tf b/terraform/test-fixtures/transform-destroy-edge-basic/main.tf new file mode 100644 index 000000000..a6a6a5ec4 --- /dev/null +++ b/terraform/test-fixtures/transform-destroy-edge-basic/main.tf @@ -0,0 +1,2 @@ +resource "test" "A" {} +resource "test" "B" { value = "${test.A.value}" } diff --git a/terraform/transform_attach_config_resource.go b/terraform/transform_attach_config_resource.go index 449d1ac16..83612fede 100644 --- a/terraform/transform_attach_config_resource.go +++ b/terraform/transform_attach_config_resource.go @@ -29,6 +29,8 @@ type AttachResourceConfigTransformer struct { } func (t *AttachResourceConfigTransformer) Transform(g *Graph) error { + log.Printf("[TRACE] AttachResourceConfigTransformer: Beginning...") + // Go through and find GraphNodeAttachResource for _, v := range g.Vertices() { // Only care about GraphNodeAttachResource implementations @@ -39,7 +41,7 @@ func (t *AttachResourceConfigTransformer) Transform(g *Graph) error { // Determine what we're looking for addr := arn.ResourceAddr() - log.Printf("[TRACE] Attach resource request: %s", addr) + log.Printf("[TRACE] AttachResourceConfigTransformer: Attach resource request: %s", addr) // Get the configuration. path := normalizeModulePath(addr.Path) diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index d1e9d194e..2b863bcc5 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -1,7 +1,16 @@ package terraform +import ( + "log" + + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" +) + // GraphNodeDestroyer must be implemented by nodes that destroy resources. type GraphNodeDestroyer interface { + dag.Vertex + // ResourceAddr is the address of the resource that is being // destroyed by this node. If this returns nil, then this node // is not destroying anything. @@ -22,9 +31,16 @@ type GraphNodeDestroyer interface { // dependent resources will block parent resources from deleting. Concrete // example: VPC with subnets, the VPC can't be deleted while there are // still subnets. -type DestroyEdgeTransformer struct{} +type DestroyEdgeTransformer struct { + // Module and State are only needed to look up dependencies in + // any way possible. Either can be nil if not availabile. + Module *module.Tree + State *State +} func (t *DestroyEdgeTransformer) Transform(g *Graph) error { + log.Printf("[TRACE] DestroyEdgeTransformer: Beginning destroy edge transformation...") + // Build a map of what is being destroyed (by address string) to // the list of destroyers. In general there will only be one destroyer // but to make it more robust we support multiple. @@ -41,6 +57,9 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { } key := addr.String() + log.Printf( + "[TRACE] DestroyEdgeTransformer: %s destroying %q", + dag.VertexName(dn), key) destroyers[key] = append(destroyers[key], dn) } @@ -50,13 +69,83 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { return nil } + // This is strange but is the easiest way to get the dependencies + // of a node that is being destroyed. We use another graph to make sure + // the resource is in the graph and ask for references. We have to do this + // because the node that is being destroyed may NOT be in the graph. + // + // Example: resource A is force new, then destroy A AND create A are + // in the graph. BUT if resource A is just pure destroy, then only + // destroy A is in the graph, and create A is not. + steps := []GraphTransformer{ + &AttachResourceConfigTransformer{Module: t.Module}, + &AttachStateTransformer{State: t.State}, + } + // Go through the all destroyers and find what they're destroying. // Use this to find the dependencies, look up if any of them are being // destroyed, and to make the proper edge. - for _, ds := range destroyers { - for _, d := range ds { - // TODO - println(d) + for d, dns := range destroyers { + // d is what is being destroyed. We parse the resource address + // which it came from it is a panic if this fails. + addr, err := ParseResourceAddress(d) + if err != nil { + panic(err) + } + + // This part is a little bit weird but is the best way to + // find the dependencies we need to: build a graph and use the + // attach config and state transformers then ask for references. + node := &NodeApplyableResource{Addr: addr} + { + var g Graph + g.Add(node) + for _, s := range steps { + if err := s.Transform(&g); err != nil { + return err + } + } + } + + // Get the references of the creation node. If it has none, + // then there are no edges to make here. + prefix := modulePrefixStr(normalizeModulePath(addr.Path)) + deps := modulePrefixList(node.References(), prefix) + log.Printf( + "[TRACE] DestroyEdgeTransformer: creation of %q depends on %#v", + d, deps) + if len(deps) == 0 { + continue + } + + // We have dependencies, check if any are being destroyed + // to build the list of things that we must depend on! + // + // In the example of the struct, if we have: + // + // B_d => A_d => A => B + // + // Then at this point in the algorithm we started with A_d, + // we built A (to get dependencies), and we found B. We're now looking + // to see if B_d exists. + var depDestroyers []dag.Vertex + for _, d := range deps { + if ds, ok := destroyers[d]; ok { + for _, d := range ds { + depDestroyers = append(depDestroyers, d.(dag.Vertex)) + log.Printf( + "[TRACE] DestroyEdgeTransformer: destruction of %q depends on %s", + addr.String(), dag.VertexName(d)) + } + } + } + + // Go through and make the connections. Use the variable + // names "a_d" and "b_d" to reference our example. + for _, a_d := range dns { + for _, b_d := range depDestroyers { + g.Connect(dag.BasicEdge(b_d, a_d)) + } } } diff --git a/terraform/transform_destroy_edge_test.go b/terraform/transform_destroy_edge_test.go new file mode 100644 index 000000000..49b2552be --- /dev/null +++ b/terraform/transform_destroy_edge_test.go @@ -0,0 +1,44 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestDestroyEdgeTransformer(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) + tf := &DestroyEdgeTransformer{ + Module: testModule(t, "transform-destroy-edge-basic"), + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformDestroyEdgeBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +type graphNodeDestroyerTest struct { + AddrString string +} + +func (n *graphNodeDestroyerTest) Name() string { return n.DestroyAddr().String() + " (destroy)" } +func (n *graphNodeDestroyerTest) DestroyAddr() *ResourceAddress { + addr, err := ParseResourceAddress(n.AddrString) + if err != nil { + panic(err) + } + + return addr +} + +const testTransformDestroyEdgeBasicStr = ` +test.A (destroy) + test.B (destroy) +test.B (destroy) +` From 08dade547519319befd4a169e0d80179ef3f4660 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Sep 2016 10:18:31 -0700 Subject: [PATCH 50/82] terraform: more destroy edge tests --- .../transform-destroy-edge-multi/main.tf | 3 +++ terraform/transform_destroy_edge_test.go | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 terraform/test-fixtures/transform-destroy-edge-multi/main.tf diff --git a/terraform/test-fixtures/transform-destroy-edge-multi/main.tf b/terraform/test-fixtures/transform-destroy-edge-multi/main.tf new file mode 100644 index 000000000..93e8211ca --- /dev/null +++ b/terraform/test-fixtures/transform-destroy-edge-multi/main.tf @@ -0,0 +1,3 @@ +resource "test" "A" {} +resource "test" "B" { value = "${test.A.value}" } +resource "test" "C" { value = "${test.B.value}" } diff --git a/terraform/transform_destroy_edge_test.go b/terraform/transform_destroy_edge_test.go index 49b2552be..5d124bfc8 100644 --- a/terraform/transform_destroy_edge_test.go +++ b/terraform/transform_destroy_edge_test.go @@ -23,6 +23,25 @@ func TestDestroyEdgeTransformer(t *testing.T) { } } +func TestDestroyEdgeTransformer_multi(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.C"}) + tf := &DestroyEdgeTransformer{ + Module: testModule(t, "transform-destroy-edge-multi"), + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformDestroyEdgeMultiStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + type graphNodeDestroyerTest struct { AddrString string } @@ -42,3 +61,11 @@ test.A (destroy) test.B (destroy) test.B (destroy) ` + +const testTransformDestroyEdgeMultiStr = ` +test.A (destroy) + test.B (destroy) +test.B (destroy) + test.C (destroy) +test.C (destroy) +` From 311d27108e85d18910581cb163e3d8eee8d66881 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Sep 2016 10:23:48 -0700 Subject: [PATCH 51/82] terraform: Enable DestroyEdgeTransformer --- terraform/graph_builder_apply.go | 9 ++++++--- terraform/node_resource_apply.go | 30 +++++++++++++++++------------- terraform/node_resource_destroy.go | 5 +++++ 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index d54a70948..c753cd97c 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -54,15 +54,18 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { State: b.State, }, - // Attach the configuration to any resources - &AttachResourceConfigTransformer{Module: b.Module}, - // Create orphan output nodes &OrphanOutputTransformer{Module: b.Module, State: b.State}, + // Attach the configuration to any resources + &AttachResourceConfigTransformer{Module: b.Module}, + // Attach the state &AttachStateTransformer{State: b.State}, + // Destruction ordering + &DestroyEdgeTransformer{Module: b.Module, State: b.State}, + // Create all the providers &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, &ProviderTransformer{}, diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 7b8d7b0ed..b3dc4bd53 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -34,23 +34,27 @@ func (n *NodeApplyableResource) ReferenceableName() []string { // GraphNodeReferencer func (n *NodeApplyableResource) References() []string { - // Let's make this a little shorter so it is easier to reference - c := n.Config - if c == nil { - return nil + // If we have a config, that is our source of truth + if c := n.Config; c != nil { + // Grab all the references + var result []string + result = append(result, c.DependsOn...) + result = append(result, ReferencesFromConfig(c.RawCount)...) + result = append(result, ReferencesFromConfig(c.RawConfig)...) + for _, p := range c.Provisioners { + result = append(result, ReferencesFromConfig(p.ConnInfo)...) + result = append(result, ReferencesFromConfig(p.RawConfig)...) + } + + return result } - // Grab all the references - var result []string - result = append(result, c.DependsOn...) - result = append(result, ReferencesFromConfig(c.RawCount)...) - result = append(result, ReferencesFromConfig(c.RawConfig)...) - for _, p := range c.Provisioners { - result = append(result, ReferencesFromConfig(p.ConnInfo)...) - result = append(result, ReferencesFromConfig(p.RawConfig)...) + // If we have state, that is our next source + if s := n.ResourceState; s != nil { + return s.Dependencies } - return result + return nil } // GraphNodeProviderConsumer diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index c4cb5fac5..e0d558528 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -30,6 +30,11 @@ func (n *NodeDestroyResource) ProvidedBy() []string { return []string{resourceProvider(n.Addr.Type, "")} } +// GraphNodeDestroyer +func (n *NodeDestroyResource) DestroyAddr() *ResourceAddress { + return n.Addr +} + // GraphNodeAttachResourceState func (n *NodeDestroyResource) ResourceAddr() *ResourceAddress { return n.Addr From 4988378ccb0b80ca097ecb52631710eb884170ee Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 20 Sep 2016 10:26:38 -0700 Subject: [PATCH 52/82] terraform: remove diff transformer test that no longer happens --- terraform/transform_diff_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/terraform/transform_diff_test.go b/terraform/transform_diff_test.go index c3e3176e6..1ee7fabde 100644 --- a/terraform/transform_diff_test.go +++ b/terraform/transform_diff_test.go @@ -48,11 +48,6 @@ func TestDiffTransformer(t *testing.T) { if actual != expected { t.Fatalf("bad:\n\n%s", actual) } - - v := g.Vertices()[0].(*NodeApplyableResource) - if v.Config == nil { - t.Fatal("no config") - } } const testTransformDiffBasicStr = ` From 7baf64f806a987fa403d75fa98f94fb2e2925daf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 10:55:07 -0700 Subject: [PATCH 53/82] terraform: starting CBD, destroy edge for the destroy relationship --- terraform/edge_destroy.go | 17 ++++++ terraform/node_resource_apply.go | 5 ++ terraform/transform_destroy_cbd.go | 67 ++++++++++++++++++++++++ terraform/transform_destroy_cbd_test.go | 40 ++++++++++++++ terraform/transform_destroy_edge.go | 34 ++++++++++++ terraform/transform_destroy_edge_test.go | 47 ++++++++++++++++- 6 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 terraform/edge_destroy.go create mode 100644 terraform/transform_destroy_cbd.go create mode 100644 terraform/transform_destroy_cbd_test.go diff --git a/terraform/edge_destroy.go b/terraform/edge_destroy.go new file mode 100644 index 000000000..bc9d638aa --- /dev/null +++ b/terraform/edge_destroy.go @@ -0,0 +1,17 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/dag" +) + +// DestroyEdge is an edge that represents a standard "destroy" relationship: +// Target depends on Source because Source is destroying. +type DestroyEdge struct { + S, T dag.Vertex +} + +func (e *DestroyEdge) Hashcode() interface{} { return fmt.Sprintf("%p-%p", e.S, e.T) } +func (e *DestroyEdge) Source() dag.Vertex { return e.S } +func (e *DestroyEdge) Target() dag.Vertex { return e.T } diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index b3dc4bd53..b5676bcbe 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -90,6 +90,11 @@ func (n *NodeApplyableResource) ProvisionedBy() []string { return result } +// GraphNodeCreator +func (n *NodeApplyableResource) CreateAddr() *ResourceAddress { + return n.Addr +} + // GraphNodeAttachResourceState func (n *NodeApplyableResource) ResourceAddr() *ResourceAddress { return n.Addr diff --git a/terraform/transform_destroy_cbd.go b/terraform/transform_destroy_cbd.go new file mode 100644 index 000000000..b398920bc --- /dev/null +++ b/terraform/transform_destroy_cbd.go @@ -0,0 +1,67 @@ +package terraform + +import ( + "log" + + "github.com/hashicorp/terraform/config/module" +) + +// GraphNodeDestroyerCBD must be implemented by nodes that might be +// create-before-destroy destroyers. +type GraphNodeDestroyerCBD interface { + GraphNodeDestroyer + + // CreateBeforeDestroy returns true if this node represents a node + // that is doing a CBD. + CreateBeforeDestroy() bool +} + +// CBDEdgeTransformer modifies the edges of CBD nodes that went through +// the DestroyEdgeTransformer to have the right dependencies. There are +// two real tasks here: +// +// 1. With CBD, the destroy edge is inverted: the destroy depends on +// the creation. +// +// 2. A_d must depend on resources that depend on A. This is to enable +// the destroy to only happen once nodes that depend on A successfully +// update to A. Example: adding a web server updates the load balancer +// before deleting the old web server. +// +type CBDEdgeTransformer struct { + // Module and State are only needed to look up dependencies in + // any way possible. Either can be nil if not availabile. + Module *module.Tree + State *State +} + +func (t *CBDEdgeTransformer) Transform(g *Graph) error { + log.Printf("[TRACE] CBDEdgeTransformer: Beginning CBD transformation...") + + // Go through and reverse any destroy edges + for _, v := range g.Vertices() { + dn, ok := v.(GraphNodeDestroyerCBD) + if !ok { + continue + } + + if !dn.CreateBeforeDestroy() { + continue + } + + // Find the destroy edge. There should only be one. + for _, e := range g.DownEdges(v).List() { + // Not a destroy edge, ignore it + de, ok := e.(*DestroyEdge) + if !ok { + continue + } + + // Found it! Invert. + g.RemoveEdge(de) + g.Connect(&DestroyEdge{S: de.Target(), T: de.Source()}) + } + } + + return nil +} diff --git a/terraform/transform_destroy_cbd_test.go b/terraform/transform_destroy_cbd_test.go new file mode 100644 index 000000000..7df75dc3d --- /dev/null +++ b/terraform/transform_destroy_cbd_test.go @@ -0,0 +1,40 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestCBDEdgeTransformer(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) + + { + tf := &DestroyEdgeTransformer{ + Module: testModule(t, "transform-destroy-edge-basic"), + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &CBDEdgeTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformCBDEdgeBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testTransformCBDEdgeBasicStr = ` +test.A (destroy) + test.B (destroy) +test.B (destroy) +` diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index 2b863bcc5..462258df0 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -17,6 +17,12 @@ type GraphNodeDestroyer interface { DestroyAddr() *ResourceAddress } +// GraphNodeCreator must be implemented by nodes that create OR update resources. +type GraphNodeCreator interface { + // ResourceAddr is the address of the resource being created or updated + CreateAddr() *ResourceAddress +} + // DestroyEdgeTransformer is a GraphTransformer that creates the proper // references for destroy resources. Destroy resources are more complex // in that they must be depend on the destruction of resources that @@ -69,6 +75,34 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { return nil } + // Go through and connect creators to destroyers. Going along with + // our example, this makes: A_d => A + for _, v := range g.Vertices() { + cn, ok := v.(GraphNodeCreator) + if !ok { + continue + } + + addr := cn.CreateAddr() + if addr == nil { + continue + } + + key := addr.String() + ds := destroyers[key] + if len(ds) == 0 { + continue + } + + for _, d := range ds { + // For illustrating our example + a_d := d.(dag.Vertex) + a := v + + g.Connect(&DestroyEdge{S: a, T: a_d}) + } + } + // This is strange but is the easiest way to get the dependencies // of a node that is being destroyed. We use another graph to make sure // the resource is in the graph and ask for references. We have to do this diff --git a/terraform/transform_destroy_edge_test.go b/terraform/transform_destroy_edge_test.go index 5d124bfc8..e2c1a8a37 100644 --- a/terraform/transform_destroy_edge_test.go +++ b/terraform/transform_destroy_edge_test.go @@ -23,6 +23,25 @@ func TestDestroyEdgeTransformer(t *testing.T) { } } +func TestDestroyEdgeTransformer_create(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) + g.Add(&graphNodeCreatorTest{AddrString: "test.A"}) + tf := &DestroyEdgeTransformer{ + Module: testModule(t, "transform-destroy-edge-basic"), + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformDestroyEdgeCreatorStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + func TestDestroyEdgeTransformer_multi(t *testing.T) { g := Graph{Path: RootModulePath} g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) @@ -42,11 +61,27 @@ func TestDestroyEdgeTransformer_multi(t *testing.T) { } } -type graphNodeDestroyerTest struct { +type graphNodeCreatorTest struct { AddrString string } -func (n *graphNodeDestroyerTest) Name() string { return n.DestroyAddr().String() + " (destroy)" } +func (n *graphNodeCreatorTest) Name() string { return n.CreateAddr().String() } +func (n *graphNodeCreatorTest) CreateAddr() *ResourceAddress { + addr, err := ParseResourceAddress(n.AddrString) + if err != nil { + panic(err) + } + + return addr +} + +type graphNodeDestroyerTest struct { + AddrString string + CBD bool +} + +func (n *graphNodeDestroyerTest) Name() string { return n.DestroyAddr().String() + " (destroy)" } +func (n *graphNodeDestroyerTest) CreateBeforeDestroy() bool { return n.CBD } func (n *graphNodeDestroyerTest) DestroyAddr() *ResourceAddress { addr, err := ParseResourceAddress(n.AddrString) if err != nil { @@ -62,6 +97,14 @@ test.A (destroy) test.B (destroy) ` +const testTransformDestroyEdgeCreatorStr = ` +test.A + test.A (destroy) +test.A (destroy) + test.B (destroy) +test.B (destroy) +` + const testTransformDestroyEdgeMultiStr = ` test.A (destroy) test.B (destroy) From b9b23e84830a64e9c35df9119b28294c33fb9026 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 11:03:04 -0700 Subject: [PATCH 54/82] terraform: improved logging --- terraform/transform_destroy_edge.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index 462258df0..6c156f360 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -99,6 +99,10 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { a_d := d.(dag.Vertex) a := v + log.Printf( + "[TRACE] DestroyEdgeTransformer: connecting creator/destroyer: %s, %s", + dag.VertexName(a), dag.VertexName(a_d)) + g.Connect(&DestroyEdge{S: a, T: a_d}) } } From 4e8e6cd66154fd385fead535c54cedf45ebdcc36 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 11:33:47 -0700 Subject: [PATCH 55/82] dag: add EdgesFrom, EdgesTo, needs tests --- dag/graph.go | 26 +++++++++++++++++++++++++ terraform/transform_destroy_cbd.go | 3 ++- terraform/transform_destroy_cbd_test.go | 5 ++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/dag/graph.go b/dag/graph.go index 012118057..75bc5dbe5 100644 --- a/dag/graph.go +++ b/dag/graph.go @@ -48,6 +48,32 @@ func (g *Graph) Edges() []Edge { return result } +// EdgesFrom returns the list of edges from the given source. +func (g *Graph) EdgesFrom(v Vertex) []Edge { + var result []Edge + from := hashcode(v) + for _, e := range g.Edges() { + if hashcode(e.Source()) == from { + result = append(result, e) + } + } + + return result +} + +// EdgesTo returns the list of edges to the given target. +func (g *Graph) EdgesTo(v Vertex) []Edge { + var result []Edge + search := hashcode(v) + for _, e := range g.Edges() { + if hashcode(e.Target()) == search { + result = append(result, e) + } + } + + return result +} + // HasVertex checks if the given Vertex is present in the graph. func (g *Graph) HasVertex(v Vertex) bool { return g.vertices.Include(v) diff --git a/terraform/transform_destroy_cbd.go b/terraform/transform_destroy_cbd.go index b398920bc..d2abb7dc8 100644 --- a/terraform/transform_destroy_cbd.go +++ b/terraform/transform_destroy_cbd.go @@ -50,7 +50,8 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error { } // Find the destroy edge. There should only be one. - for _, e := range g.DownEdges(v).List() { + for _, e := range g.EdgesTo(v) { + log.Printf("WHAT: %#v", e) // Not a destroy edge, ignore it de, ok := e.(*DestroyEdge) if !ok { diff --git a/terraform/transform_destroy_cbd_test.go b/terraform/transform_destroy_cbd_test.go index 7df75dc3d..73314b60b 100644 --- a/terraform/transform_destroy_cbd_test.go +++ b/terraform/transform_destroy_cbd_test.go @@ -7,7 +7,8 @@ import ( func TestCBDEdgeTransformer(t *testing.T) { g := Graph{Path: RootModulePath} - g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) + g.Add(&graphNodeCreatorTest{AddrString: "test.A"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.A", CBD: true}) g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) { @@ -34,7 +35,9 @@ func TestCBDEdgeTransformer(t *testing.T) { } const testTransformCBDEdgeBasicStr = ` +test.A test.A (destroy) + test.A test.B (destroy) test.B (destroy) ` From 6e632ec2bacc7c700c1987e342f4bd0a09c18d14 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 13:38:33 -0700 Subject: [PATCH 56/82] dag: test for EdgesFrom, EdgesTo --- dag/graph_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/dag/graph_test.go b/dag/graph_test.go index 7acd4a831..02c4debd5 100644 --- a/dag/graph_test.go +++ b/dag/graph_test.go @@ -124,6 +124,52 @@ func TestGraphHasEdge(t *testing.T) { } } +func TestGraphEdgesFrom(t *testing.T) { + var g Graph + g.Add(1) + g.Add(2) + g.Add(3) + g.Connect(BasicEdge(1, 3)) + g.Connect(BasicEdge(2, 3)) + + edges := g.EdgesFrom(1) + + var expected Set + expected.Add(BasicEdge(1, 3)) + + var s Set + for _, e := range edges { + s.Add(e) + } + + if s.Intersection(&expected).Len() != expected.Len() { + t.Fatalf("bad: %#v", edges) + } +} + +func TestGraphEdgesTo(t *testing.T) { + var g Graph + g.Add(1) + g.Add(2) + g.Add(3) + g.Connect(BasicEdge(1, 3)) + g.Connect(BasicEdge(1, 2)) + + edges := g.EdgesTo(3) + + var expected Set + expected.Add(BasicEdge(1, 3)) + + var s Set + for _, e := range edges { + s.Add(e) + } + + if s.Intersection(&expected).Len() != expected.Len() { + t.Fatalf("bad: %#v", edges) + } +} + type hashVertex struct { code interface{} } From 046faf247aed703291555add3479eecac612ec68 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 13:54:19 -0700 Subject: [PATCH 57/82] terraform: cleanup and failing test for CBD --- terraform/transform_destroy_cbd.go | 1 - terraform/transform_destroy_cbd_test.go | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/terraform/transform_destroy_cbd.go b/terraform/transform_destroy_cbd.go index d2abb7dc8..b409b2304 100644 --- a/terraform/transform_destroy_cbd.go +++ b/terraform/transform_destroy_cbd.go @@ -51,7 +51,6 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error { // Find the destroy edge. There should only be one. for _, e := range g.EdgesTo(v) { - log.Printf("WHAT: %#v", e) // Not a destroy edge, ignore it de, ok := e.(*DestroyEdge) if !ok { diff --git a/terraform/transform_destroy_cbd_test.go b/terraform/transform_destroy_cbd_test.go index 73314b60b..15a3ea436 100644 --- a/terraform/transform_destroy_cbd_test.go +++ b/terraform/transform_destroy_cbd_test.go @@ -8,8 +8,8 @@ import ( func TestCBDEdgeTransformer(t *testing.T) { g := Graph{Path: RootModulePath} g.Add(&graphNodeCreatorTest{AddrString: "test.A"}) + g.Add(&graphNodeCreatorTest{AddrString: "test.B"}) g.Add(&graphNodeDestroyerTest{AddrString: "test.A", CBD: true}) - g.Add(&graphNodeDestroyerTest{AddrString: "test.B"}) { tf := &DestroyEdgeTransformer{ @@ -38,6 +38,6 @@ const testTransformCBDEdgeBasicStr = ` test.A test.A (destroy) test.A - test.B (destroy) -test.B (destroy) + test.B +test.B ` From 6622ca001d4902c2d31451e355d8bd0f468b7f30 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 14:30:41 -0700 Subject: [PATCH 58/82] terraform: abstract resource nodes --- terraform/graph_builder_apply.go | 9 +++ terraform/node_resource_abstract.go | 115 ++++++++++++++++++++++++++++ terraform/node_resource_apply.go | 115 +++------------------------- terraform/transform_destroy_edge.go | 2 +- terraform/transform_diff.go | 15 +++- 5 files changed, 147 insertions(+), 109 deletions(-) create mode 100644 terraform/node_resource_abstract.go diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index c753cd97c..6929985c8 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -2,6 +2,7 @@ package terraform import ( "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" ) // ApplyGraphBuilder implements GraphBuilder and is responsible for building @@ -46,9 +47,17 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { } } + concreteResource := func(a *NodeAbstractResource) dag.Vertex { + return &NodeApplyableResource{ + NodeAbstractResource: a, + } + } + steps := []GraphTransformer{ // Creates all the nodes represented in the diff. &DiffTransformer{ + Concrete: concreteResource, + Diff: b.Diff, Module: b.Module, State: b.State, diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go new file mode 100644 index 000000000..7d7a21b5e --- /dev/null +++ b/terraform/node_resource_abstract.go @@ -0,0 +1,115 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" +) + +// ConcreteResourceNodeFunc is a callback type used to convert an +// abstract resource to a concrete one of some type. +type ConcreteResourceNodeFunc func(*NodeAbstractResource) dag.Vertex + +// NodeAbstractResource represents a resource that has no associated +// operations. It registers all the interfaces for a resource that common +// across multiple operation types. +type NodeAbstractResource struct { + Addr *ResourceAddress // Addr is the address for this resource + + // The fields below will be automatically set using the Attach + // interfaces if you're running those transforms, but also be explicitly + // set if you already have that information. + + Config *config.Resource // Config is the resource in the config + ResourceState *ResourceState // ResourceState is the ResourceState for this +} + +func (n *NodeAbstractResource) Name() string { + return n.Addr.String() +} + +// GraphNodeSubPath +func (n *NodeAbstractResource) Path() []string { + return n.Addr.Path +} + +// GraphNodeReferenceable +func (n *NodeAbstractResource) ReferenceableName() []string { + if n.Config == nil { + return nil + } + + return []string{n.Config.Id()} +} + +// GraphNodeReferencer +func (n *NodeAbstractResource) References() []string { + // If we have a config, that is our source of truth + if c := n.Config; c != nil { + // Grab all the references + var result []string + result = append(result, c.DependsOn...) + result = append(result, ReferencesFromConfig(c.RawCount)...) + result = append(result, ReferencesFromConfig(c.RawConfig)...) + for _, p := range c.Provisioners { + result = append(result, ReferencesFromConfig(p.ConnInfo)...) + result = append(result, ReferencesFromConfig(p.RawConfig)...) + } + + return result + } + + // If we have state, that is our next source + if s := n.ResourceState; s != nil { + return s.Dependencies + } + + return nil +} + +// GraphNodeProviderConsumer +func (n *NodeAbstractResource) ProvidedBy() []string { + // If we have a config we prefer that above all else + if n.Config != nil { + return []string{resourceProvider(n.Config.Type, n.Config.Provider)} + } + + // If we have state, then we will use the provider from there + if n.ResourceState != nil { + return []string{n.ResourceState.Provider} + } + + // Use our type + return []string{resourceProvider(n.Addr.Type, "")} +} + +// GraphNodeProvisionerConsumer +func (n *NodeAbstractResource) ProvisionedBy() []string { + // If we have no configuration, then we have no provisioners + if n.Config == nil { + return nil + } + + // Build the list of provisioners we need based on the configuration. + // It is okay to have duplicates here. + result := make([]string, len(n.Config.Provisioners)) + for i, p := range n.Config.Provisioners { + result[i] = p.Type + } + + return result +} + +// GraphNodeAttachResourceState +func (n *NodeAbstractResource) ResourceAddr() *ResourceAddress { + return n.Addr +} + +// GraphNodeAttachResourceState +func (n *NodeAbstractResource) AttachResourceState(s *ResourceState) { + n.ResourceState = s +} + +// GraphNodeAttachResourceConfig +func (n *NodeAbstractResource) AttachResourceConfig(c *config.Resource) { + n.Config = c +} diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index b5676bcbe..f663cd893 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -2,133 +2,40 @@ package terraform import ( "fmt" - - "github.com/hashicorp/terraform/config" ) // NodeApplyableResource represents a resource that is "applyable": // it is ready to be applied and is represented by a diff. type NodeApplyableResource struct { - Addr *ResourceAddress // Addr is the address for this resource - Config *config.Resource // Config is the resource in the config - ResourceState *ResourceState // ResourceState is the ResourceState for this -} - -func (n *NodeApplyableResource) Name() string { - return n.Addr.String() -} - -// GraphNodeSubPath -func (n *NodeApplyableResource) Path() []string { - return n.Addr.Path -} - -// GraphNodeReferenceable -func (n *NodeApplyableResource) ReferenceableName() []string { - if n.Config == nil { - return nil - } - - return []string{n.Config.Id()} -} - -// GraphNodeReferencer -func (n *NodeApplyableResource) References() []string { - // If we have a config, that is our source of truth - if c := n.Config; c != nil { - // Grab all the references - var result []string - result = append(result, c.DependsOn...) - result = append(result, ReferencesFromConfig(c.RawCount)...) - result = append(result, ReferencesFromConfig(c.RawConfig)...) - for _, p := range c.Provisioners { - result = append(result, ReferencesFromConfig(p.ConnInfo)...) - result = append(result, ReferencesFromConfig(p.RawConfig)...) - } - - return result - } - - // If we have state, that is our next source - if s := n.ResourceState; s != nil { - return s.Dependencies - } - - return nil -} - -// GraphNodeProviderConsumer -func (n *NodeApplyableResource) ProvidedBy() []string { - // If we have a config we prefer that above all else - if n.Config != nil { - return []string{resourceProvider(n.Config.Type, n.Config.Provider)} - } - - // If we have state, then we will use the provider from there - if n.ResourceState != nil { - return []string{n.ResourceState.Provider} - } - - // Use our type - return []string{resourceProvider(n.Addr.Type, "")} -} - -// GraphNodeProvisionerConsumer -func (n *NodeApplyableResource) ProvisionedBy() []string { - // If we have no configuration, then we have no provisioners - if n.Config == nil { - return nil - } - - // Build the list of provisioners we need based on the configuration. - // It is okay to have duplicates here. - result := make([]string, len(n.Config.Provisioners)) - for i, p := range n.Config.Provisioners { - result[i] = p.Type - } - - return result + *NodeAbstractResource } // GraphNodeCreator func (n *NodeApplyableResource) CreateAddr() *ResourceAddress { - return n.Addr -} - -// GraphNodeAttachResourceState -func (n *NodeApplyableResource) ResourceAddr() *ResourceAddress { - return n.Addr -} - -// GraphNodeAttachResource -func (n *NodeApplyableResource) AttachResourceConfig(c *config.Resource) { - n.Config = c -} - -// GraphNodeAttachResourceState -func (n *NodeApplyableResource) AttachResourceState(s *ResourceState) { - n.ResourceState = s + return n.NodeAbstractResource.Addr } // GraphNodeEvalable func (n *NodeApplyableResource) EvalTree() EvalNode { + addr := n.NodeAbstractResource.Addr + // stateId is the ID to put into the state - stateId := n.Addr.stateId() - if n.Addr.Index > -1 { - stateId = fmt.Sprintf("%s.%d", stateId, n.Addr.Index) + stateId := addr.stateId() + if addr.Index > -1 { + stateId = fmt.Sprintf("%s.%d", stateId, addr.Index) } // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ Id: stateId, - Type: n.Addr.Type, + Type: addr.Type, } // Build the resource for eval resource := &Resource{ - Name: n.Addr.Name, - Type: n.Addr.Type, - CountIndex: n.Addr.Index, + Name: addr.Name, + Type: addr.Type, + CountIndex: addr.Index, } if resource.CountIndex < 0 { resource.CountIndex = 0 diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index 6c156f360..a3a8f8d9f 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -134,7 +134,7 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { // This part is a little bit weird but is the best way to // find the dependencies we need to: build a graph and use the // attach config and state transformers then ask for references. - node := &NodeApplyableResource{Addr: addr} + node := &NodeAbstractResource{Addr: addr} { var g Graph g.Add(node) diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index 81bebab9a..d33140911 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -5,6 +5,7 @@ import ( "log" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" ) // DiffTransformer is a GraphTransformer that adds the elements of @@ -19,6 +20,8 @@ import ( // is built based on the diff first, though, ensuring that only resources // that are being modified are present in the graph. type DiffTransformer struct { + Concrete ConcreteResourceNodeFunc + Diff *Diff Module *module.Tree State *State @@ -32,7 +35,7 @@ func (t *DiffTransformer) Transform(g *Graph) error { // Go through all the modules in the diff. log.Printf("[TRACE] DiffTransformer: starting") - var nodes []*NodeApplyableResource + var nodes []dag.Vertex for _, m := range t.Diff.Modules { log.Printf("[TRACE] DiffTransformer: Module: %s", m) // TODO: If this is a destroy diff then add a module destroy node @@ -62,9 +65,13 @@ func (t *DiffTransformer) Transform(g *Graph) error { // If we have changes, then add the applyable version if len(inst.Attributes) > 0 { // Add the resource to the graph - nodes = append(nodes, &NodeApplyableResource{ - Addr: addr, - }) + abstract := &NodeAbstractResource{Addr: addr} + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) + } + + nodes = append(nodes, node) } } } From 3d4937b7840e157f5b0134f6cdd1bb1b9370de7b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 14:43:18 -0700 Subject: [PATCH 59/82] terraform: FlatConfigTransformer --- .../transform-flat-config-basic/child/main.tf | 1 + .../transform-flat-config-basic/main.tf | 6 ++ terraform/transform_config_flat.go | 72 +++++++++++++++++++ terraform/transform_config_flat_test.go | 40 +++++++++++ 4 files changed, 119 insertions(+) create mode 100644 terraform/test-fixtures/transform-flat-config-basic/child/main.tf create mode 100644 terraform/test-fixtures/transform-flat-config-basic/main.tf create mode 100644 terraform/transform_config_flat.go create mode 100644 terraform/transform_config_flat_test.go diff --git a/terraform/test-fixtures/transform-flat-config-basic/child/main.tf b/terraform/test-fixtures/transform-flat-config-basic/child/main.tf new file mode 100644 index 000000000..0c70b1b5d --- /dev/null +++ b/terraform/test-fixtures/transform-flat-config-basic/child/main.tf @@ -0,0 +1 @@ +resource "aws_instance" "baz" {} diff --git a/terraform/test-fixtures/transform-flat-config-basic/main.tf b/terraform/test-fixtures/transform-flat-config-basic/main.tf new file mode 100644 index 000000000..c588350d4 --- /dev/null +++ b/terraform/test-fixtures/transform-flat-config-basic/main.tf @@ -0,0 +1,6 @@ +resource "aws_instance" "foo" {} +resource "aws_instance" "bar" { value = "${aws_instance.foo.value}" } + +module "child" { + source = "./child" +} diff --git a/terraform/transform_config_flat.go b/terraform/transform_config_flat.go new file mode 100644 index 000000000..9d13130d6 --- /dev/null +++ b/terraform/transform_config_flat.go @@ -0,0 +1,72 @@ +package terraform + +import ( + "errors" + + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" +) + +// FlatConfigTransformer is a GraphTransformer that adds the configuration +// to the graph. The module used to configure this transformer must be +// the root module. +// +// In relation to ConfigTransformer: this is a newer generation config +// transformer. It puts the _entire_ config into the graph (there is no +// "flattening" step as before). +type FlatConfigTransformer struct { + Concrete ConcreteResourceNodeFunc // What to turn resources into + + Module *module.Tree +} + +func (t *FlatConfigTransformer) Transform(g *Graph) error { + // If no module, we do nothing + if t.Module == nil { + return nil + } + + // If the module is not loaded, that is an error + if !t.Module.Loaded() { + return errors.New("module must be loaded") + } + + return t.transform(g, t.Module) +} + +func (t *FlatConfigTransformer) transform(g *Graph, m *module.Tree) error { + // If no module, no problem + if m == nil { + return nil + } + + // Transform all the children. + for _, c := range m.Children() { + if err := t.transform(g, c); err != nil { + return err + } + } + + // Get the configuration for this module + config := m.Config() + + // Write all the resources out + for _, r := range config.Resources { + // Grab the address for this resource + addr, err := parseResourceAddressConfig(r) + if err != nil { + return err + } + addr.Path = m.Path() + + abstract := &NodeAbstractResource{Addr: addr} + var node dag.Vertex = abstract + if f := t.Concrete; f != nil { + node = f(abstract) + } + + g.Add(node) + } + + return nil +} diff --git a/terraform/transform_config_flat_test.go b/terraform/transform_config_flat_test.go new file mode 100644 index 000000000..aa393d5a9 --- /dev/null +++ b/terraform/transform_config_flat_test.go @@ -0,0 +1,40 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestFlatConfigTransformer_nilModule(t *testing.T) { + g := Graph{Path: RootModulePath} + tf := &FlatConfigTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + if len(g.Vertices()) > 0 { + t.Fatal("graph should be empty") + } +} + +func TestFlatConfigTransformer(t *testing.T) { + g := Graph{Path: RootModulePath} + tf := &FlatConfigTransformer{ + Module: testModule(t, "transform-flat-config-basic"), + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformFlatConfigBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testTransformFlatConfigBasicStr = ` +aws_instance.bar +aws_instance.foo +module.child.aws_instance.baz +` From 4aa84a2071a54e56b64facf74e00f7ac4c381cc2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 16:37:41 -0700 Subject: [PATCH 60/82] terraform: CBD makes the proper edge connections for dependent resources --- terraform/node_resource_abstract.go | 9 +- terraform/transform_config_flat.go | 12 ++- terraform/transform_destroy_cbd.go | 123 ++++++++++++++++++++++++ terraform/transform_destroy_cbd_test.go | 6 +- 4 files changed, 145 insertions(+), 5 deletions(-) diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 7d7a21b5e..bdde958bf 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -9,6 +9,13 @@ import ( // abstract resource to a concrete one of some type. type ConcreteResourceNodeFunc func(*NodeAbstractResource) dag.Vertex +// GraphNodeResource is implemented by any nodes that represent a resource. +// The type of operation cannot be assumed, only that this node represents +// the given resource. +type GraphNodeResource interface { + ResourceAddr() *ResourceAddress +} + // NodeAbstractResource represents a resource that has no associated // operations. It registers all the interfaces for a resource that common // across multiple operation types. @@ -99,7 +106,7 @@ func (n *NodeAbstractResource) ProvisionedBy() []string { return result } -// GraphNodeAttachResourceState +// GraphNodeResource, GraphNodeAttachResourceState func (n *NodeAbstractResource) ResourceAddr() *ResourceAddress { return n.Addr } diff --git a/terraform/transform_config_flat.go b/terraform/transform_config_flat.go index 9d13130d6..92f9888d6 100644 --- a/terraform/transform_config_flat.go +++ b/terraform/transform_config_flat.go @@ -11,7 +11,10 @@ import ( // to the graph. The module used to configure this transformer must be // the root module. // -// In relation to ConfigTransformer: this is a newer generation config +// This transform adds the nodes but doesn't connect any of the references. +// The ReferenceTransformer should be used for that. +// +// NOTE: In relation to ConfigTransformer: this is a newer generation config // transformer. It puts the _entire_ config into the graph (there is no // "flattening" step as before). type FlatConfigTransformer struct { @@ -59,7 +62,12 @@ func (t *FlatConfigTransformer) transform(g *Graph, m *module.Tree) error { } addr.Path = m.Path() - abstract := &NodeAbstractResource{Addr: addr} + // Build the abstract resource. We have the config already so + // we'll just pre-populate that. + abstract := &NodeAbstractResource{ + Addr: addr, + Config: r, + } var node dag.Vertex = abstract if f := t.Concrete; f != nil { node = f(abstract) diff --git a/terraform/transform_destroy_cbd.go b/terraform/transform_destroy_cbd.go index b409b2304..0dde08888 100644 --- a/terraform/transform_destroy_cbd.go +++ b/terraform/transform_destroy_cbd.go @@ -4,6 +4,7 @@ import ( "log" "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" ) // GraphNodeDestroyerCBD must be implemented by nodes that might be @@ -39,6 +40,7 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error { log.Printf("[TRACE] CBDEdgeTransformer: Beginning CBD transformation...") // Go through and reverse any destroy edges + destroyMap := make(map[string][]dag.Vertex) for _, v := range g.Vertices() { dn, ok := v.(GraphNodeDestroyerCBD) if !ok { @@ -57,11 +59,132 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error { continue } + log.Printf("[TRACE] CBDEdgeTransformer: inverting edge: %s => %s", + dag.VertexName(de.Source()), dag.VertexName(de.Target())) + // Found it! Invert. g.RemoveEdge(de) g.Connect(&DestroyEdge{S: de.Target(), T: de.Source()}) } + + // Add this to the list of nodes that we need to fix up + // the edges for (step 2 above in the docs). + key := dn.DestroyAddr().String() + destroyMap[key] = append(destroyMap[key], v) + } + + // If we have no CBD nodes, then our work here is done + if len(destroyMap) == 0 { + return nil + } + + // We have CBD nodes. We now have to move on to the much more difficult + // task of connecting dependencies of the creation side of the destroy + // to the destruction node. The easiest way to explain this is an example: + // + // Given a pre-destroy dependence of: A => B + // And A has CBD set. + // + // The resulting graph should be: A => B => A_d + // + // They key here is that B happens before A is destroyed. This is to + // facilitate the primary purpose for CBD: making sure that downstreams + // are properly updated to avoid downtime before the resource is destroyed. + // + // We can't trust that the resource being destroyed or anything that + // depends on it is actually in our current graph so we make a new + // graph in order to determine those dependencies and add them in. + log.Printf("[TRACE] CBDEdgeTransformer: building graph to find dependencies...") + depMap, err := t.depMap(destroyMap) + if err != nil { + return err + } + + // We now have the mapping of resource addresses to the destroy + // nodes they need to depend on. We now go through our own vertices to + // find any matching these addresses and make the connection. + for _, v := range g.Vertices() { + // We're looking for creators + rn, ok := v.(GraphNodeCreator) + if !ok { + continue + } + + // Get the address + addr := rn.CreateAddr() + key := addr.String() + + // If there is nothing this resource should depend on, ignore it + dns, ok := depMap[key] + if !ok { + continue + } + + // We have nodes! Make the connection + for _, dn := range dns { + log.Printf("[TRACE] CBDEdgeTransformer: destroy depends on dependence: %s => %s", + dag.VertexName(dn), dag.VertexName(v)) + g.Connect(dag.BasicEdge(dn, v)) + } } return nil } + +func (t *CBDEdgeTransformer) depMap( + destroyMap map[string][]dag.Vertex) (map[string][]dag.Vertex, error) { + // Build the graph of our config, this ensures that all resources + // are present in the graph. + g, err := (&BasicGraphBuilder{ + Steps: []GraphTransformer{ + &FlatConfigTransformer{Module: t.Module}, + &AttachResourceConfigTransformer{Module: t.Module}, + &AttachStateTransformer{State: t.State}, + &ReferenceTransformer{}, + }, + }).Build(nil) + if err != nil { + return nil, err + } + + // Using this graph, build the list of destroy nodes that each resource + // address should depend on. For example, when we find B, we map the + // address of B to A_d in the "depMap" variable below. + depMap := make(map[string][]dag.Vertex) + for _, v := range g.Vertices() { + // We're looking for resources. + rn, ok := v.(GraphNodeResource) + if !ok { + continue + } + + // Get the address + addr := rn.ResourceAddr() + key := addr.String() + + // Get the destroy nodes that are destroying this resource. + // If there aren't any, then we don't need to worry about + // any connections. + dns, ok := destroyMap[key] + if !ok { + continue + } + + // Get the nodes that depend on this on. In the example above: + // finding B in A => B. + for _, v := range g.UpEdges(v).List() { + // We're looking for resources. + rn, ok := v.(GraphNodeResource) + if !ok { + continue + } + + // Keep track of the destroy nodes that this address + // needs to depend on. + key := rn.ResourceAddr().String() + depMap[key] = append(depMap[key], dns...) + } + } + + return depMap, nil +} diff --git a/terraform/transform_destroy_cbd_test.go b/terraform/transform_destroy_cbd_test.go index 15a3ea436..dad1d27bb 100644 --- a/terraform/transform_destroy_cbd_test.go +++ b/terraform/transform_destroy_cbd_test.go @@ -11,9 +11,11 @@ func TestCBDEdgeTransformer(t *testing.T) { g.Add(&graphNodeCreatorTest{AddrString: "test.B"}) g.Add(&graphNodeDestroyerTest{AddrString: "test.A", CBD: true}) + module := testModule(t, "transform-destroy-edge-basic") + { tf := &DestroyEdgeTransformer{ - Module: testModule(t, "transform-destroy-edge-basic"), + Module: module, } if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) @@ -21,7 +23,7 @@ func TestCBDEdgeTransformer(t *testing.T) { } { - tf := &CBDEdgeTransformer{} + tf := &CBDEdgeTransformer{Module: module} if err := tf.Transform(&g); err != nil { t.Fatalf("err: %s", err) } From aaee4df36389059f878aaec4f5b1b6550e85af9c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 21 Sep 2016 18:48:21 -0700 Subject: [PATCH 61/82] terraform: working on enabling CBD, some cycles --- terraform/graph_builder_apply.go | 1 + terraform/node_resource_abstract.go | 2 +- terraform/node_resource_destroy.go | 35 ++++++++--------------------- terraform/transform_diff.go | 3 ++- 4 files changed, 13 insertions(+), 28 deletions(-) diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 6929985c8..598e33adb 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -74,6 +74,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Destruction ordering &DestroyEdgeTransformer{Module: b.Module, State: b.State}, + &CBDEdgeTransformer{Module: b.Module, State: b.State}, // Create all the providers &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index bdde958bf..cddccaef2 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -81,7 +81,7 @@ func (n *NodeAbstractResource) ProvidedBy() []string { } // If we have state, then we will use the provider from there - if n.ResourceState != nil { + if n.ResourceState != nil && n.ResourceState.Provider != "" { return []string{n.ResourceState.Provider} } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index e0d558528..af392bdc5 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -6,28 +6,11 @@ import ( // NodeDestroyResource represents a resource that is to be destroyed. type NodeDestroyResource struct { - Addr *ResourceAddress // Addr is the address for this resource - ResourceState *ResourceState // State is the resource state for this resource + *NodeAbstractResource } func (n *NodeDestroyResource) Name() string { - return n.Addr.String() + " (destroy)" -} - -// GraphNodeSubPath -func (n *NodeDestroyResource) Path() []string { - return n.Addr.Path -} - -// GraphNodeProviderConsumer -func (n *NodeDestroyResource) ProvidedBy() []string { - // If we have state, then we will use the provider from there - if n.ResourceState != nil && n.ResourceState.Provider != "" { - return []string{n.ResourceState.Provider} - } - - // Use our type - return []string{resourceProvider(n.Addr.Type, "")} + return n.NodeAbstractResource.Name() + " (destroy)" } // GraphNodeDestroyer @@ -35,14 +18,14 @@ func (n *NodeDestroyResource) DestroyAddr() *ResourceAddress { return n.Addr } -// GraphNodeAttachResourceState -func (n *NodeDestroyResource) ResourceAddr() *ResourceAddress { - return n.Addr -} +// GraphNodeDestroyerCBD +func (n *NodeDestroyResource) CreateBeforeDestroy() bool { + // If we have no config, we just assume no + if n.Config == nil { + return false + } -// GraphNodeAttachResourceState -func (n *NodeDestroyResource) AttachResourceState(s *ResourceState) { - n.ResourceState = s + return n.Config.Lifecycle.CreateBeforeDestroy } // GraphNodeEvalable diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index d33140911..68e905346 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -59,7 +59,8 @@ func (t *DiffTransformer) Transform(g *Graph) error { // If we're destroying, add the destroy node if inst.Destroy { - g.Add(&NodeDestroyResource{Addr: addr}) + abstract := &NodeAbstractResource{Addr: addr} + g.Add(&NodeDestroyResource{NodeAbstractResource: abstract}) } // If we have changes, then add the applyable version From 23665790f302f3c188b2c0e413bb82a9dfdd80d7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 22 Sep 2016 10:10:57 -0700 Subject: [PATCH 62/82] terraform: destroy resource should have no references --- terraform/node_resource_destroy.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index af392bdc5..9021d77e3 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -28,6 +28,16 @@ func (n *NodeDestroyResource) CreateBeforeDestroy() bool { return n.Config.Lifecycle.CreateBeforeDestroy } +// GraphNodeReferenceable, overriding NodeAbstractResource +func (n *NodeDestroyResource) ReferenceableName() []string { + return nil +} + +// GraphNodeReferencer, overriding NodeAbstractResource +func (n *NodeDestroyResource) References() []string { + return nil +} + // GraphNodeEvalable func (n *NodeDestroyResource) EvalTree() EvalNode { // stateId is the ID to put into the state From c1664d2eaae77cb3bb671e80a201925fb8c327a3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 22 Sep 2016 11:03:03 -0700 Subject: [PATCH 63/82] terraform: cbd works! --- terraform/context_apply_test.go | 10 +++++++-- terraform/graph.go | 9 ++++---- terraform/graph_builder_apply.go | 7 ++++++- terraform/graph_builder_apply_test.go | 20 +++++++++++------- terraform/node_resource_destroy.go | 30 ++++++++++++++++++++++++++- terraform/terraform_test.go | 3 +++ terraform/transform_diff.go | 2 +- 7 files changed, 65 insertions(+), 16 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 841530aee..b77d5b0e0 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2297,6 +2297,7 @@ func TestContext2Apply_multiDepose_createBeforeDestroy(t *testing.T) { checkStateString(t, state, ` aws_instance.web: (1 deposed) ID = bar + provider = aws Deposed ID 1 = foo `) @@ -2321,6 +2322,7 @@ aws_instance.web: (1 deposed) checkStateString(t, state, ` aws_instance.web: (2 deposed) ID = baz + provider = aws Deposed ID 1 = foo Deposed ID 2 = bar `) @@ -2348,6 +2350,7 @@ aws_instance.web: (2 deposed) checkStateString(t, state, ` aws_instance.web: (1 deposed) ID = qux + provider = aws Deposed ID 1 = bar `) @@ -2369,6 +2372,7 @@ aws_instance.web: (1 deposed) checkStateString(t, state, ` aws_instance.web: ID = quux + provider = aws `) } @@ -4746,8 +4750,10 @@ func TestContext2Apply_createBefore_depends(t *testing.T) { State: state, }) - if _, err := ctx.Plan(); err != nil { + if p, err := ctx.Plan(); err != nil { t.Fatalf("err: %s", err) + } else { + t.Logf("plan: %s", p) } h.Active = true @@ -4764,7 +4770,7 @@ func TestContext2Apply_createBefore_depends(t *testing.T) { actual := strings.TrimSpace(state.String()) expected := strings.TrimSpace(testTerraformApplyDependsCreateBeforeStr) if actual != expected { - t.Fatalf("bad: \n%s\n%s", actual, expected) + t.Fatalf("bad: \n%s\n\n%s", actual, expected) } // Test that things were managed _in the right order_ diff --git a/terraform/graph.go b/terraform/graph.go index 97214f809..d056bab1f 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -263,10 +263,11 @@ func (g *Graph) walk(walker GraphWalker) error { rerr = err return } - - // Walk the subgraph - if rerr = g.walk(walker); rerr != nil { - return + if g != nil { + // Walk the subgraph + if rerr = g.walk(walker); rerr != nil { + return + } } } diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 598e33adb..e53b7caaf 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -27,6 +27,9 @@ type ApplyGraphBuilder struct { // Provisioners is the list of provisioners supported. Provisioners []string + + // DisableReduce, if true, will not reduce the graph. Great for testing. + DisableReduce bool } // See GraphBuilder @@ -103,10 +106,12 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Single root &RootTransformer{}, + } + if !b.DisableReduce { // Perform the transitive reduction to make our graph a bit // more sane if possible (it usually is possible). - &TransitiveReductionTransformer{}, + steps = append(steps, &TransitiveReductionTransformer{}) } return steps diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index ff6d71e28..f224e53f1 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -65,10 +65,11 @@ func TestApplyGraphBuilder(t *testing.T) { } b := &ApplyGraphBuilder{ - Module: testModule(t, "graph-builder-apply-basic"), - Diff: diff, - Providers: []string{"aws"}, - Provisioners: []string{"exec"}, + Module: testModule(t, "graph-builder-apply-basic"), + Diff: diff, + Providers: []string{"aws"}, + Provisioners: []string{"exec"}, + DisableReduce: true, } g, err := b.Build(RootModulePath) @@ -93,6 +94,14 @@ aws_instance.create aws_instance.other aws_instance.create provider.aws +meta.count-boundary (count boundary fixup) + aws_instance.create + aws_instance.other + module.child.aws_instance.create + module.child.aws_instance.other + module.child.provider.aws + provider.aws + provisioner.exec module.child.aws_instance.create module.child.provider.aws provisioner.exec @@ -103,7 +112,4 @@ module.child.provider.aws provider.aws provider.aws provisioner.exec -root - aws_instance.other - module.child.aws_instance.other ` diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 9021d77e3..640f5d158 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -6,7 +6,7 @@ import ( // NodeDestroyResource represents a resource that is to be destroyed. type NodeDestroyResource struct { - *NodeAbstractResource + NodeAbstractResource } func (n *NodeDestroyResource) Name() string { @@ -38,6 +38,34 @@ func (n *NodeDestroyResource) References() []string { return nil } +// GraphNodeDynamicExpandable +func (n *NodeDestroyResource) DynamicExpand(ctx EvalContext) (*Graph, error) { + // If we have no config we do nothing + if n.Config == nil { + return nil, nil + } + + state, lock := ctx.State() + lock.RLock() + defer lock.RUnlock() + + // Start creating the steps + steps := make([]GraphTransformer, 0, 5) + + // We want deposed resources in the state to be destroyed + steps = append(steps, &DeposedTransformer{ + State: state, + View: n.Config.Id(), + }) + + // Always end with the root being added + steps = append(steps, &RootTransformer{}) + + // Build the graph + b := &BasicGraphBuilder{Steps: steps} + return b.Build(ctx.Path()) +} + // GraphNodeEvalable func (n *NodeDestroyResource) EvalTree() EvalNode { // stateId is the ID to put into the state diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index dd6185613..379bccd80 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -298,6 +298,7 @@ aws_instance.lb: aws_instance.web aws_instance.web: ID = foo + provider = aws require_new = ami-new type = aws_instance ` @@ -305,6 +306,7 @@ aws_instance.web: const testTerraformApplyCreateBeforeStr = ` aws_instance.bar: ID = foo + provider = aws require_new = xyz type = aws_instance ` @@ -591,6 +593,7 @@ aws_instance.bar: const testTerraformApplyErrorDestroyCreateBeforeDestroyStr = ` aws_instance.bar: (1 deposed) ID = foo + provider = aws Deposed ID 1 = bar ` diff --git a/terraform/transform_diff.go b/terraform/transform_diff.go index 68e905346..7943af685 100644 --- a/terraform/transform_diff.go +++ b/terraform/transform_diff.go @@ -59,7 +59,7 @@ func (t *DiffTransformer) Transform(g *Graph) error { // If we're destroying, add the destroy node if inst.Destroy { - abstract := &NodeAbstractResource{Addr: addr} + abstract := NodeAbstractResource{Addr: addr} g.Add(&NodeDestroyResource{NodeAbstractResource: abstract}) } From 924f7a49e08b4f264baf6ac47f92f65f6a624118 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 23 Sep 2016 09:19:09 -0700 Subject: [PATCH 64/82] terraform: module variable transform must do children later (tested) --- terraform/context_apply_test.go | 2 + .../transform-module-var-basic/child/main.tf | 3 + .../transform-module-var-basic/main.tf | 4 ++ .../child/child/main.tf | 3 + .../transform-module-var-nested/child/main.tf | 6 ++ .../transform-module-var-nested/main.tf | 4 ++ terraform/transform_module_variable.go | 26 ++++++-- terraform/transform_module_variable_test.go | 65 +++++++++++++++++++ 8 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 terraform/test-fixtures/transform-module-var-basic/child/main.tf create mode 100644 terraform/test-fixtures/transform-module-var-basic/main.tf create mode 100644 terraform/test-fixtures/transform-module-var-nested/child/child/main.tf create mode 100644 terraform/test-fixtures/transform-module-var-nested/child/main.tf create mode 100644 terraform/test-fixtures/transform-module-var-nested/main.tf create mode 100644 terraform/transform_module_variable_test.go diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index b77d5b0e0..8c2af8a70 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -3094,6 +3094,8 @@ func TestContext2Apply_destroyModuleWithAttrsReferencingResource(t *testing.T) { if err != nil { t.Fatalf("apply err: %s", err) } + + t.Logf("Step 1 state: %s", state) } h := new(HookRecordApplyOrder) diff --git a/terraform/test-fixtures/transform-module-var-basic/child/main.tf b/terraform/test-fixtures/transform-module-var-basic/child/main.tf new file mode 100644 index 000000000..0568aa053 --- /dev/null +++ b/terraform/test-fixtures/transform-module-var-basic/child/main.tf @@ -0,0 +1,3 @@ +variable "value" {} + +output "result" { value = "${var.value}" } diff --git a/terraform/test-fixtures/transform-module-var-basic/main.tf b/terraform/test-fixtures/transform-module-var-basic/main.tf new file mode 100644 index 000000000..0adb513f1 --- /dev/null +++ b/terraform/test-fixtures/transform-module-var-basic/main.tf @@ -0,0 +1,4 @@ +module "child" { + source = "./child" + value = "foo" +} diff --git a/terraform/test-fixtures/transform-module-var-nested/child/child/main.tf b/terraform/test-fixtures/transform-module-var-nested/child/child/main.tf new file mode 100644 index 000000000..0568aa053 --- /dev/null +++ b/terraform/test-fixtures/transform-module-var-nested/child/child/main.tf @@ -0,0 +1,3 @@ +variable "value" {} + +output "result" { value = "${var.value}" } diff --git a/terraform/test-fixtures/transform-module-var-nested/child/main.tf b/terraform/test-fixtures/transform-module-var-nested/child/main.tf new file mode 100644 index 000000000..b8c7f0bac --- /dev/null +++ b/terraform/test-fixtures/transform-module-var-nested/child/main.tf @@ -0,0 +1,6 @@ +variable "value" {} + +module "child" { + source = "./child" + value = "${var.value}" +} diff --git a/terraform/test-fixtures/transform-module-var-nested/main.tf b/terraform/test-fixtures/transform-module-var-nested/main.tf new file mode 100644 index 000000000..2c20f1979 --- /dev/null +++ b/terraform/test-fixtures/transform-module-var-nested/main.tf @@ -0,0 +1,4 @@ +module "child" { + source = "./child" + value = "foo" +} diff --git a/terraform/transform_module_variable.go b/terraform/transform_module_variable.go index 13c80845e..3a3a7de88 100644 --- a/terraform/transform_module_variable.go +++ b/terraform/transform_module_variable.go @@ -28,15 +28,32 @@ func (t *ModuleVariableTransformer) transform(g *Graph, parent, m *module.Tree) return nil } - // Transform all the children. + // If we have no parent, then don't do anything. This is because + // we need to be able to get the set value from the module declaration. + if err := t.transformSingle(g, parent, m); err != nil { + return nil + } + + // Transform all the children. This has to be _after_ the above + // since children can reference parent variables but parents can't + // access children. Example: + // + // module foo { value = "${var.foo}" } + // + // The "value" var in "foo" (a child) is accessing the "foo" bar + // in the parent (current module). However, there is no way for the + // current module to reference a variable in the child module. for _, c := range m.Children() { if err := t.transform(g, m, c); err != nil { return err } } - // If we have no parent, then don't do anything. This is because - // we need to be able to get the set value from the module declaration. + return nil +} + +func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, m *module.Tree) error { + // If we have no parent, we can't determine if the parent uses our variables if parent == nil { return nil } @@ -44,6 +61,7 @@ func (t *ModuleVariableTransformer) transform(g *Graph, parent, m *module.Tree) // If we have no vars, we're done! vars := m.Config().Variables if len(vars) == 0 { + log.Printf("[TRACE] Module %#v has no variables, skipping.", m.Path()) return nil } @@ -56,7 +74,7 @@ func (t *ModuleVariableTransformer) transform(g *Graph, parent, m *module.Tree) } } if mod == nil { - log.Printf("[INFO] Module %q not used, not adding variables", m.Name()) + log.Printf("[INFO] Module %#v not used, not adding variables", m.Path()) return nil } diff --git a/terraform/transform_module_variable_test.go b/terraform/transform_module_variable_test.go new file mode 100644 index 000000000..6842b325c --- /dev/null +++ b/terraform/transform_module_variable_test.go @@ -0,0 +1,65 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestModuleVariableTransformer(t *testing.T) { + g := Graph{Path: RootModulePath} + module := testModule(t, "transform-module-var-basic") + + { + tf := &RootVariableTransformer{Module: module} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &ModuleVariableTransformer{Module: module} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformModuleVarBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +func TestModuleVariableTransformer_nested(t *testing.T) { + g := Graph{Path: RootModulePath} + module := testModule(t, "transform-module-var-nested") + + { + tf := &RootVariableTransformer{Module: module} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &ModuleVariableTransformer{Module: module} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformModuleVarNestedStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testTransformModuleVarBasicStr = ` +module.child.var.value +` + +const testTransformModuleVarNestedStr = ` +module.child.module.child.var.value +module.child.var.value +` From 9ac4ee4b52e1935e4b9e8ca41bb23dcbe1ff50cf Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 23 Sep 2016 17:20:25 -0700 Subject: [PATCH 65/82] terraform: transform module variables does parent first --- terraform/transform_module_variable.go | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/terraform/transform_module_variable.go b/terraform/transform_module_variable.go index 3a3a7de88..1e035107c 100644 --- a/terraform/transform_module_variable.go +++ b/terraform/transform_module_variable.go @@ -28,21 +28,16 @@ func (t *ModuleVariableTransformer) transform(g *Graph, parent, m *module.Tree) return nil } - // If we have no parent, then don't do anything. This is because - // we need to be able to get the set value from the module declaration. - if err := t.transformSingle(g, parent, m); err != nil { - return nil + // If we have a parent, we can determine if a module variable is being + // used, so we transform this. + if parent != nil { + if err := t.transformSingle(g, parent, m); err != nil { + return err + } } - // Transform all the children. This has to be _after_ the above - // since children can reference parent variables but parents can't - // access children. Example: - // - // module foo { value = "${var.foo}" } - // - // The "value" var in "foo" (a child) is accessing the "foo" bar - // in the parent (current module). However, there is no way for the - // current module to reference a variable in the child module. + // Transform all the children. This must be done AFTER the transform + // above since child module variables can reference parent module variables. for _, c := range m.Children() { if err := t.transform(g, m, c); err != nil { return err @@ -53,11 +48,6 @@ func (t *ModuleVariableTransformer) transform(g *Graph, parent, m *module.Tree) } func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, m *module.Tree) error { - // If we have no parent, we can't determine if the parent uses our variables - if parent == nil { - return nil - } - // If we have no vars, we're done! vars := m.Config().Variables if len(vars) == 0 { From 7c2c9b82a321ad292e53fc08475f77d0ce7699fb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 12 Oct 2016 18:43:26 -0700 Subject: [PATCH 66/82] terraform: interpolation for multi-var checks both ".0" and "" suffix --- terraform/context_apply_test.go | 2 ++ terraform/node_output.go | 4 ++++ terraform/node_resource_destroy.go | 7 ++++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 8c2af8a70..698c5da5a 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1783,6 +1783,8 @@ func TestContext2Apply_multiVar(t *testing.T) { t.Fatalf("err: %s", err) } + t.Logf("End state: %s", state.String()) + actual := state.RootModule().Outputs["output"] if actual == nil { t.Fatal("missing output") diff --git a/terraform/node_output.go b/terraform/node_output.go index 08bb892d6..c10c6e4f8 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -37,6 +37,10 @@ func (n *NodeApplyableOutput) ReferenceableName() []string { func (n *NodeApplyableOutput) References() []string { var result []string result = append(result, ReferencesFromConfig(n.Config.RawConfig)...) + for _, v := range result { + result = append(result, v+".destroy") + } + return result } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 640f5d158..6ef7b7d5f 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -30,7 +30,12 @@ func (n *NodeDestroyResource) CreateBeforeDestroy() bool { // GraphNodeReferenceable, overriding NodeAbstractResource func (n *NodeDestroyResource) ReferenceableName() []string { - return nil + result := n.NodeAbstractResource.ReferenceableName() + for i, v := range result { + result[i] = v + ".destroy" + } + + return result } // GraphNodeReferencer, overriding NodeAbstractResource From eb9ecea86322d6927d26bfdf27f654fe2baeab3a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 13 Oct 2016 18:27:22 -0700 Subject: [PATCH 67/82] terraform: don't set Provider on destroy nodes This doesn't explicitly set `rs.Provider` on destroy nodes. To be honest, I'm not sure why this was done in the first place (git blame points to 6fda7bb5483a155b8ae1e1e4e4b7b7c4073bc1d9). Tests always passed without it, and by adding it it causes other tests to fail. I should've never changed those other tests. Removing it now to get tests passing, this also reverts the test changes made in 8213824962f085279810f04b60b95d1176a3a3f2. --- terraform/context_apply_test.go | 4 ---- terraform/node_resource_destroy.go | 1 - terraform/terraform_test.go | 3 --- 3 files changed, 8 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 698c5da5a..c1ca9e5f7 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2299,7 +2299,6 @@ func TestContext2Apply_multiDepose_createBeforeDestroy(t *testing.T) { checkStateString(t, state, ` aws_instance.web: (1 deposed) ID = bar - provider = aws Deposed ID 1 = foo `) @@ -2324,7 +2323,6 @@ aws_instance.web: (1 deposed) checkStateString(t, state, ` aws_instance.web: (2 deposed) ID = baz - provider = aws Deposed ID 1 = foo Deposed ID 2 = bar `) @@ -2352,7 +2350,6 @@ aws_instance.web: (2 deposed) checkStateString(t, state, ` aws_instance.web: (1 deposed) ID = qux - provider = aws Deposed ID 1 = bar `) @@ -2374,7 +2371,6 @@ aws_instance.web: (1 deposed) checkStateString(t, state, ` aws_instance.web: ID = quux - provider = aws `) } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 6ef7b7d5f..a55a2c96f 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -90,7 +90,6 @@ func (n *NodeDestroyResource) EvalTree() EvalNode { if rs == nil { rs = &ResourceState{} } - rs.Provider = n.ProvidedBy()[0] var diffApply *InstanceDiff var provider ResourceProvider diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 379bccd80..dd6185613 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -298,7 +298,6 @@ aws_instance.lb: aws_instance.web aws_instance.web: ID = foo - provider = aws require_new = ami-new type = aws_instance ` @@ -306,7 +305,6 @@ aws_instance.web: const testTerraformApplyCreateBeforeStr = ` aws_instance.bar: ID = foo - provider = aws require_new = xyz type = aws_instance ` @@ -593,7 +591,6 @@ aws_instance.bar: const testTerraformApplyErrorDestroyCreateBeforeDestroyStr = ` aws_instance.bar: (1 deposed) ID = foo - provider = aws Deposed ID 1 = bar ` From ec15783f24a5f16900301f6bf7e3e5bdc49580ef Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 15 Oct 2016 15:08:11 -0700 Subject: [PATCH 68/82] -Xnew-apply to enable the new apply graph --- .travis.yml | 1 + command/meta.go | 3 +++ terraform/context.go | 11 ++++++++--- terraform/terraform_test.go | 9 +++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2b1501a62..b0c3641ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,7 @@ install: - bash scripts/gogetcookie.sh script: - make test vet +- make test TEST=./terraform TESTARGS=-Xnew-apply branches: only: - master diff --git a/command/meta.go b/command/meta.go index f18b910d4..aaeb151ea 100644 --- a/command/meta.go +++ b/command/meta.go @@ -328,6 +328,9 @@ func (m *Meta) flagSet(n string) *flag.FlagSet { f.Var((*FlagKVFile)(&m.autoVariables), m.autoKey, "variable file") } + // Experimental features + f.BoolVar(&terraform.X_newApply, "Xnew-apply", false, "experiment: new apply") + // Create an io.Writer that writes to our Ui properly for errors. // This is kind of a hack, but it does the job. Basically: create // a pipe, use a scanner to break it into lines, and output each line diff --git a/terraform/context.go b/terraform/context.go index 49054ffff..b8788ad86 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -17,9 +17,14 @@ import ( // is called on Context. type InputMode byte +// Variables prefixed with X_ are experimental features. They can be enabled +// by setting them to true. This should be done before any API is called. +// These should be expected to be removed at some point in the future; each +// option should mention a schedule. var ( - // NOTE: Internal only to toggle between the new and old apply graph - newApplyGraph = true + // X_newApply will enable the new apply graph. This will be removed + // and be on by default in 0.8.0. + X_newApply = false ) const ( @@ -363,7 +368,7 @@ func (c *Context) Apply() (*State, error) { // our new graph builder. var graph *Graph var err error - if c.destroy || !newApplyGraph { + if c.destroy || !X_newApply { graph, err = c.Graph(&ContextGraphOpts{Validate: true}) } else { graph, err = (&ApplyGraphBuilder{ diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index dd6185613..d81e2f1cb 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -22,8 +22,17 @@ import ( const fixtureDir = "./test-fixtures" func TestMain(m *testing.M) { + // Experimental features + xNewApply := flag.Bool("Xnew-apply", false, "Experiment: new apply graph") + flag.Parse() + // Setup experimental features + X_newApply = *xNewApply + if X_newApply { + println("Xnew-apply enabled") + } + if testing.Verbose() { // if we're verbose, use the logging requested by TF_LOG logging.SetOutput() From e8516f259dfdb38ae7da6b45436c1705e7aab09a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 15 Oct 2016 15:19:43 -0700 Subject: [PATCH 69/82] command/apply: Xnew-apply --- command/apply.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/command/apply.go b/command/apply.go index e1200b91d..345b2f7ee 100644 --- a/command/apply.go +++ b/command/apply.go @@ -95,6 +95,26 @@ func (c *ApplyCommand) Run(args []string) int { } } + // Check for the new apply + if terraform.X_newApply { + desc := "Experimental new apply graph has been enabled. This may still\n" + + "have bugs, and should be used with care. If you'd like to continue,\n" + + "you must enter exactly 'yes' as a response." + v, err := c.UIInput().Input(&terraform.InputOpts{ + Id: "Xnew-apply", + Query: "Experimental feature enabled: new apply graph. Continue?", + Description: desc, + }) + if err != nil { + c.Ui.Error(fmt.Sprintf("Error asking for confirmation: %s", err)) + return 1 + } + if v != "yes" { + c.Ui.Output("Apply cancelled.") + return 1 + } + } + // Build the context based on the arguments given ctx, planned, err := c.Context(contextOpts{ Destroy: c.Destroy, From 5cd6898109343bb0d297027416f66f7afbeefb21 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 15 Oct 2016 15:25:41 -0700 Subject: [PATCH 70/82] config: fix a conflicting test name A public API TestNewRawConfig was added to easily create a raw config for testing, but this conflicted with the test. Just rename it. --- config/raw_config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/raw_config_test.go b/config/raw_config_test.go index e718ca994..5d4b881a4 100644 --- a/config/raw_config_test.go +++ b/config/raw_config_test.go @@ -27,7 +27,7 @@ func TestNewRawConfig(t *testing.T) { } } -func TestRawConfig(t *testing.T) { +func TestRawConfig_basic(t *testing.T) { raw := map[string]interface{}{ "foo": "${var.bar}", } From e59efa024b31044143601de975692e7c0510ff7e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 13:41:30 -0700 Subject: [PATCH 71/82] terraform: fix merge issues with master --- terraform/context.go | 32 ++++++-------------------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index b8788ad86..702685399 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -13,10 +13,6 @@ import ( "github.com/hashicorp/terraform/config/module" ) -// InputMode defines what sort of input will be asked for when Input -// is called on Context. -type InputMode byte - // Variables prefixed with X_ are experimental features. They can be enabled // by setting them to true. This should be done before any API is called. // These should be expected to be removed at some point in the future; each @@ -27,6 +23,10 @@ var ( X_newApply = false ) +// InputMode defines what sort of input will be asked for when Input +// is called on Context. +type InputMode byte + const ( // InputModeVar asks for all variables InputModeVar InputMode = 1 << iota @@ -372,11 +372,9 @@ func (c *Context) Apply() (*State, error) { graph, err = c.Graph(&ContextGraphOpts{Validate: true}) } else { graph, err = (&ApplyGraphBuilder{ - Module: c.module, Diff: c.diff, - State: c.state, - Providers: c.providersList(), - Provisioners: c.provisionersList(), + Providers: c.components.ResourceProviders(), + Provisioners: c.components.ResourceProvisioners(), }).Build(RootModulePath) } if err != nil { @@ -733,24 +731,6 @@ func (c *Context) walk( return walker, realErr } -func (c *Context) providersList() []string { - providers := make([]string, 0, len(c.providers)) - for k, _ := range c.providers { - providers = append(providers, k) - } - - return providers -} - -func (c *Context) provisionersList() []string { - provisioners := make([]string, 0, len(c.provisioners)) - for k, _ := range c.provisioners { - provisioners = append(provisioners, k) - } - - return provisioners -} - // parseVariableAsHCL parses the value of a single variable as would have been specified // on the command line via -var or in an environment variable named TF_VAR_x, where x is // the name of the variable. In order to get around the restriction of HCL requiring a From c9c1912b34924abb18c106e5041697e6fd4b4520 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 13:46:32 -0700 Subject: [PATCH 72/82] terraform: missing fields from ApplyGraphBuilder after master rebase --- terraform/context.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/terraform/context.go b/terraform/context.go index 702685399..eebae1c15 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -372,7 +372,9 @@ func (c *Context) Apply() (*State, error) { graph, err = c.Graph(&ContextGraphOpts{Validate: true}) } else { graph, err = (&ApplyGraphBuilder{ + Module: c.module, Diff: c.diff, + State: c.state, Providers: c.components.ResourceProviders(), Provisioners: c.components.ResourceProvisioners(), }).Build(RootModulePath) From d87bdc2d2b3890471d69909f38663cd21028ff9d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 13:51:11 -0700 Subject: [PATCH 73/82] terraform: update destroy resource with proper unique-ifier for shadow This adds a proper unique extra field so that the shadow graph can properly compare values. --- terraform/node_resource_destroy.go | 5 +++-- terraform/shadow_resource_provider.go | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index a55a2c96f..52c34d1c7 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -81,8 +81,9 @@ func (n *NodeDestroyResource) EvalTree() EvalNode { // Build the instance info. More of this will be populated during eval info := &InstanceInfo{ - Id: stateId, - Type: n.Addr.Type, + Id: stateId, + Type: n.Addr.Type, + uniqueExtra: "destroy", } // Get our state diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index 4d7643438..bb80be31d 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -518,16 +518,16 @@ func (p *shadowResourceProviderShadow) Apply( if !state.Equal(result.State) { p.ErrorLock.Lock() p.Error = multierror.Append(p.Error, fmt.Errorf( - "Apply: state had unequal states (real, then shadow):\n\n%#v\n\n%#v", - result.State, state)) + "Apply %q: state had unequal states (real, then shadow):\n\n%#v\n\n%#v", + key, result.State, state)) p.ErrorLock.Unlock() } if !diff.Equal(result.Diff) { p.ErrorLock.Lock() p.Error = multierror.Append(p.Error, fmt.Errorf( - "Apply: unequal diffs (real, then shadow):\n\n%#v\n\n%#v", - result.Diff, diff)) + "Apply %q: unequal diffs (real, then shadow):\n\n%#v\n\n%#v", + key, result.Diff, diff)) p.ErrorLock.Unlock() } From 5d598ad217b71df57d3212a508723e0e1c68306a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 14:10:46 -0700 Subject: [PATCH 74/82] terraform: if components is closed, initialize closed components This was happening if the shadow initializes a provider that is never used by the real side. We need to make sure it starts closed. --- terraform/shadow_components.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/terraform/shadow_components.go b/terraform/shadow_components.go index 141493df2..116cf84f9 100644 --- a/terraform/shadow_components.go +++ b/terraform/shadow_components.go @@ -208,6 +208,10 @@ func (f *shadowComponentFactoryShared) ResourceProvider( real, shadow := newShadowResourceProvider(p) entry.Real = real entry.Shadow = shadow + + if f.closed { + shadow.CloseShadow() + } } // Store the value @@ -246,6 +250,10 @@ func (f *shadowComponentFactoryShared) ResourceProvisioner( real, shadow := newShadowResourceProvisioner(p) entry.Real = real entry.Shadow = shadow + + if f.closed { + shadow.CloseShadow() + } } // Store the value From 7d36e991da319a725ee733ef1137ce419bd2ceee Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 16 Oct 2016 18:27:02 -0700 Subject: [PATCH 75/82] terraform: resource address internal can parse data resource addrs --- terraform/resource_address.go | 16 ++++++++++++-- terraform/resource_address_test.go | 35 +++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/terraform/resource_address.go b/terraform/resource_address.go index 643833ff3..06e943f9e 100644 --- a/terraform/resource_address.go +++ b/terraform/resource_address.go @@ -119,7 +119,19 @@ func parseResourceAddressInternal(s string) (*ResourceAddress, error) { // Split based on ".". Every resource address should have at least two // elements (type and name). parts := strings.Split(s, ".") - if len(parts) < 2 || len(parts) > 3 { + if len(parts) < 2 || len(parts) > 4 { + return nil, fmt.Errorf("Invalid internal resource address format: %s", s) + } + + // Data resource if we have at least 3 parts and the first one is data + mode := config.ManagedResourceMode + if len(parts) > 2 && parts[0] == "data" { + mode = config.DataResourceMode + parts = parts[1:] + } + + // If we're not a data resource and we have more than 3, then it is an error + if len(parts) > 3 && mode != config.DataResourceMode { return nil, fmt.Errorf("Invalid internal resource address format: %s", s) } @@ -129,7 +141,7 @@ func parseResourceAddressInternal(s string) (*ResourceAddress, error) { Name: parts[1], Index: -1, InstanceType: TypePrimary, - Mode: config.ManagedResourceMode, + Mode: mode, } // If we have more parts, then we have an index. Parse that. diff --git a/terraform/resource_address_test.go b/terraform/resource_address_test.go index 02d9e82e5..2604441fe 100644 --- a/terraform/resource_address_test.go +++ b/terraform/resource_address_test.go @@ -36,14 +36,47 @@ func TestParseResourceAddressInternal(t *testing.T) { }, "aws_instance.foo[1]", }, + + "data resource": { + "data.aws_ami.foo", + &ResourceAddress{ + Mode: config.DataResourceMode, + Type: "aws_ami", + Name: "foo", + InstanceType: TypePrimary, + Index: -1, + }, + "data.aws_ami.foo", + }, + + "data resource with count": { + "data.aws_ami.foo.1", + &ResourceAddress{ + Mode: config.DataResourceMode, + Type: "aws_ami", + Name: "foo", + InstanceType: TypePrimary, + Index: 1, + }, + "data.aws_ami.foo[1]", + }, + + "non-data resource with 4 elements": { + "aws_instance.foo.bar.1", + nil, + "", + }, } for tn, tc := range cases { t.Run(tc.Input, func(t *testing.T) { out, err := parseResourceAddressInternal(tc.Input) - if err != nil { + if (err != nil) != (tc.Expected == nil) { t.Fatalf("%s: unexpected err: %#v", tn, err) } + if err != nil { + return + } if !reflect.DeepEqual(out, tc.Expected) { t.Fatalf("bad: %q\n\nexpected:\n%#v\n\ngot:\n%#v", tn, tc.Expected, out) From 13b900747421aa3eb720cef5ce5fdaee62e4fc6e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 14:17:12 -0700 Subject: [PATCH 76/82] terraform: logic for shadowing the original graph This introduces failing tests. How many is unknown since shadow graph errors cause a panic. --- terraform/context.go | 88 +++++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 22 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index eebae1c15..d15b9f6c2 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -363,35 +363,79 @@ func (c *Context) Apply() (*State, error) { // Copy our own state c.state = c.state.DeepCopy() - // Build the graph. If it is a destroy operation, we use the - // standard graph builder. If it is an apply operation, we use - // our new graph builder. - var graph *Graph - var err error - if c.destroy || !X_newApply { - graph, err = c.Graph(&ContextGraphOpts{Validate: true}) - } else { - graph, err = (&ApplyGraphBuilder{ - Module: c.module, - Diff: c.diff, - State: c.state, - Providers: c.components.ResourceProviders(), - Provisioners: c.components.ResourceProvisioners(), - }).Build(RootModulePath) + // Build the original graph. This is before the new graph builders + // coming in 0.8. We do this for shadow graphing. + oldGraph, err := c.Graph(&ContextGraphOpts{Validate: true}) + if err != nil && X_newApply { + // If we had an error graphing but we're using the new graph, + // just set it to nil and let it go. There are some features that + // may work with the new graph that don't with the old. + oldGraph = nil + err = nil } if err != nil { return nil, err } - // Do the walk - var walker *ContextGraphWalker - if c.destroy { - walker, err = c.walk(graph, graph, walkDestroy) - } else { - //walker, err = c.walk(graph, nil, walkApply) - walker, err = c.walk(graph, graph, walkApply) + // Build the new graph. We do this no matter what so we can shadow it. + newGraph, err := (&ApplyGraphBuilder{ + Module: c.module, + Diff: c.diff, + State: c.state, + Providers: c.components.ResourceProviders(), + Provisioners: c.components.ResourceProvisioners(), + }).Build(RootModulePath) + if err != nil && !X_newApply { + // If we had an error graphing but we're not using this graph, just + // set it to nil and record it as a shadow error. + c.shadowErr = multierror.Append(c.shadowErr, fmt.Errorf( + "Error building new apply graph: %s", err)) + + newGraph = nil + err = nil + } + if err != nil { + return nil, err } + // Determine what is the real and what is the shadow. The logic here + // is straightforward though the if statements are not: + // + // * Destroy mode - always use original, shadow with nothing because + // we're only testing the new APPLY graph. + // * Apply with new apply - use new graph, shadow is new graph. We can't + // shadow with the old graph because the old graph does a lot more + // that it shouldn't. + // * Apply with old apply - use old graph, shadow with new graph. + // + real := oldGraph + shadow := newGraph + if c.destroy { + log.Printf("[WARN] terraform: real graph is original, shadow is nil") + shadow = nil + } else { + if X_newApply { + log.Printf("[WARN] terraform: real graph is Xnew-apply, shadow is Xnew-apply") + real = shadow + } else { + log.Printf("[WARN] terraform: real graph is original, shadow is Xnew-apply") + } + } + + // Determine the operation + operation := walkApply + if c.destroy { + operation = walkDestroy + } + + // This shouldn't happen, so assert it. This is before any state changes + // so it is safe to crash here. + if real == nil { + panic("nil real graph") + } + + // Walk the graph + walker, err := c.walk(real, shadow, operation) if len(walker.ValidationErrors) > 0 { err = multierror.Append(err, walker.ValidationErrors...) } From e4ef1fe5530b56482726f756268ca5312a3575f4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 14:54:00 -0700 Subject: [PATCH 77/82] terraform: disable providers in new apply graph This adds the proper logic for "disabling" providers to the new apply graph: interolating and storing the config for inheritance but not actually initializing and configuring the provider. This is important since parent modules will often contain incomplete provider configurations for the purpose of inheritance that would error if they were actually attempted to be configured (since they're incomplete). If the provider is not used, it should be "disabled". --- terraform/context_apply_test.go | 37 ++++ terraform/graph_builder.go | 2 +- terraform/graph_builder_apply.go | 1 + terraform/node_provider_abstract.go | 62 +++++++ terraform/node_provider_disabled.go | 38 ++++ .../child/main.tf | 5 + .../apply-provider-configure-disabled/main.tf | 7 + terraform/transform_provider.go | 161 ---------------- terraform/transform_provider_disable.go | 50 +++++ terraform/transform_provider_old.go | 172 ++++++++++++++++++ 10 files changed, 373 insertions(+), 162 deletions(-) create mode 100644 terraform/node_provider_abstract.go create mode 100644 terraform/node_provider_disabled.go create mode 100644 terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf create mode 100644 terraform/test-fixtures/apply-provider-configure-disabled/main.tf create mode 100644 terraform/transform_provider_disable.go create mode 100644 terraform/transform_provider_old.go diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index c1ca9e5f7..397334bbb 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -1915,6 +1915,43 @@ func TestContext2Apply_providerComputedVar(t *testing.T) { } } +func TestContext2Apply_providerConfigureDisabled(t *testing.T) { + m := testModule(t, "apply-provider-configure-disabled") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + called := false + p.ConfigureFn = func(c *ResourceConfig) error { + called = true + + if _, ok := c.Get("value"); !ok { + return fmt.Errorf("value is not found") + } + + return nil + } + + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + }) + + if _, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } + + if _, err := ctx.Apply(); err != nil { + t.Fatalf("err: %s", err) + } + + if !called { + t.Fatal("configure never called") + } +} + func TestContext2Apply_Provisioner_compute(t *testing.T) { m := testModule(t, "apply-provisioner-compute") p := testProvider("aws") diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index abc9aca37..1c0a2d595 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -115,7 +115,7 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { // Provider-related transformations &MissingProviderTransformer{Providers: b.Providers}, &ProviderTransformer{}, - &DisableProviderTransformer{}, + &DisableProviderTransformerOld{}, // Provisioner-related transformations &MissingProvisionerTransformer{Provisioners: b.Provisioners}, diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index e53b7caaf..83cc89374 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -82,6 +82,7 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Create all the providers &MissingProviderTransformer{Providers: b.Providers, Factory: providerFactory}, &ProviderTransformer{}, + &DisableProviderTransformer{}, &ParentProviderTransformer{}, &AttachProviderConfigTransformer{Module: b.Module}, diff --git a/terraform/node_provider_abstract.go b/terraform/node_provider_abstract.go new file mode 100644 index 000000000..5cb18caec --- /dev/null +++ b/terraform/node_provider_abstract.go @@ -0,0 +1,62 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" +) + +// NodeAbstractProvider represents a provider that has no associated operations. +// It registers all the common interfaces across operations for providers. +type NodeAbstractProvider struct { + NameValue string + PathValue []string + + // The fields below will be automatically set using the Attach + // interfaces if you're running those transforms, but also be explicitly + // set if you already have that information. + + Config *config.ProviderConfig +} + +func (n *NodeAbstractProvider) Name() string { + result := fmt.Sprintf("provider.%s", n.NameValue) + if len(n.PathValue) > 1 { + result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) + } + + return result +} + +// GraphNodeSubPath +func (n *NodeAbstractProvider) Path() []string { + return n.PathValue +} + +// GraphNodeReferencer +func (n *NodeAbstractProvider) References() []string { + if n.Config == nil { + return nil + } + + return ReferencesFromConfig(n.Config.RawConfig) +} + +// GraphNodeProvider +func (n *NodeAbstractProvider) ProviderName() string { + return n.NameValue +} + +// GraphNodeProvider +func (n *NodeAbstractProvider) ProviderConfig() *config.RawConfig { + if n.Config == nil { + return nil + } + + return n.Config.RawConfig +} + +// GraphNodeAttachProvider +func (n *NodeAbstractProvider) AttachProvider(c *config.ProviderConfig) { + n.Config = c +} diff --git a/terraform/node_provider_disabled.go b/terraform/node_provider_disabled.go new file mode 100644 index 000000000..25e7e620e --- /dev/null +++ b/terraform/node_provider_disabled.go @@ -0,0 +1,38 @@ +package terraform + +import ( + "fmt" +) + +// NodeDisabledProvider represents a provider that is disabled. A disabled +// provider does nothing. It exists to properly set inheritance information +// for child providers. +type NodeDisabledProvider struct { + *NodeAbstractProvider +} + +func (n *NodeDisabledProvider) Name() string { + return fmt.Sprintf("%s (disabled)", n.NodeAbstractProvider.Name()) +} + +// GraphNodeEvalable +func (n *NodeDisabledProvider) EvalTree() EvalNode { + var resourceConfig *ResourceConfig + return &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.ProviderConfig(), + Output: &resourceConfig, + }, + &EvalBuildProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + Output: &resourceConfig, + }, + &EvalSetProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + }, + }, + } +} diff --git a/terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf b/terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf new file mode 100644 index 000000000..c421bf743 --- /dev/null +++ b/terraform/test-fixtures/apply-provider-configure-disabled/child/main.tf @@ -0,0 +1,5 @@ +provider "aws" { + value = "foo" +} + +resource "aws_instance" "foo" {} diff --git a/terraform/test-fixtures/apply-provider-configure-disabled/main.tf b/terraform/test-fixtures/apply-provider-configure-disabled/main.tf new file mode 100644 index 000000000..dbfc52745 --- /dev/null +++ b/terraform/test-fixtures/apply-provider-configure-disabled/main.tf @@ -0,0 +1,7 @@ +provider "aws" { + foo = "bar" +} + +module "child" { + source = "./child" +} diff --git a/terraform/transform_provider.go b/terraform/transform_provider.go index 5ae8b3fe3..606b81dcd 100644 --- a/terraform/transform_provider.go +++ b/terraform/transform_provider.go @@ -33,55 +33,6 @@ type GraphNodeProviderConsumer interface { ProvidedBy() []string } -// DisableProviderTransformer "disables" any providers that are only -// depended on by modules. -type DisableProviderTransformer struct{} - -func (t *DisableProviderTransformer) Transform(g *Graph) error { - // Since we're comparing against edges, we need to make sure we connect - g.ConnectDependents() - - for _, v := range g.Vertices() { - // We only care about providers - pn, ok := v.(GraphNodeProvider) - if !ok || pn.ProviderName() == "" { - continue - } - - // Go through all the up-edges (things that depend on this - // provider) and if any is not a module, then ignore this node. - nonModule := false - for _, sourceRaw := range g.UpEdges(v).List() { - source := sourceRaw.(dag.Vertex) - cn, ok := source.(graphNodeConfig) - if !ok { - nonModule = true - break - } - - if cn.ConfigType() != GraphNodeConfigTypeModule { - nonModule = true - break - } - } - if nonModule { - // We found something that depends on this provider that - // isn't a module, so skip it. - continue - } - - // Disable the provider by replacing it with a "disabled" provider - disabled := &graphNodeDisabledProvider{GraphNodeProvider: pn} - if !g.Replace(v, disabled) { - panic(fmt.Sprintf( - "vertex disappeared from under us: %s", - dag.VertexName(v))) - } - } - - return nil -} - // ProviderTransformer is a GraphTransformer that maps resources to // providers within the graph. This will error if there are any resources // that don't map to proper resources. @@ -381,118 +332,6 @@ func closeProviderVertexMap(g *Graph) map[string]dag.Vertex { return m } -type graphNodeDisabledProvider struct { - GraphNodeProvider -} - -// GraphNodeEvalable impl. -func (n *graphNodeDisabledProvider) EvalTree() EvalNode { - var resourceConfig *ResourceConfig - - return &EvalOpFilter{ - Ops: []walkOperation{walkInput, walkValidate, walkRefresh, walkPlan, walkApply, walkDestroy}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalInterpolate{ - Config: n.ProviderConfig(), - Output: &resourceConfig, - }, - &EvalBuildProviderConfig{ - Provider: n.ProviderName(), - Config: &resourceConfig, - Output: &resourceConfig, - }, - &EvalSetProviderConfig{ - Provider: n.ProviderName(), - Config: &resourceConfig, - }, - }, - }, - } -} - -// GraphNodeFlattenable impl. -func (n *graphNodeDisabledProvider) Flatten(p []string) (dag.Vertex, error) { - return &graphNodeDisabledProviderFlat{ - graphNodeDisabledProvider: n, - PathValue: p, - }, nil -} - -func (n *graphNodeDisabledProvider) Name() string { - return fmt.Sprintf("%s (disabled)", dag.VertexName(n.GraphNodeProvider)) -} - -// GraphNodeDotter impl. -func (n *graphNodeDisabledProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node { - return dot.NewNode(name, map[string]string{ - "label": n.Name(), - "shape": "diamond", - }) -} - -// GraphNodeDotterOrigin impl. -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() -} - -// Same as graphNodeDisabledProvider, but for flattening -type graphNodeDisabledProviderFlat struct { - *graphNodeDisabledProvider - - PathValue []string -} - -func (n *graphNodeDisabledProviderFlat) Name() string { - return fmt.Sprintf( - "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeDisabledProvider.Name()) -} - -func (n *graphNodeDisabledProviderFlat) Path() []string { - return n.PathValue -} - -func (n *graphNodeDisabledProviderFlat) ProviderName() string { - return fmt.Sprintf( - "%s.%s", modulePrefixStr(n.PathValue), - n.graphNodeDisabledProvider.ProviderName()) -} - -// GraphNodeDependable impl. -func (n *graphNodeDisabledProviderFlat) DependableName() []string { - return modulePrefixList( - n.graphNodeDisabledProvider.DependableName(), - modulePrefixStr(n.PathValue)) -} - -func (n *graphNodeDisabledProviderFlat) 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]) - result = modulePrefixList( - n.graphNodeDisabledProvider.DependableName(), prefix) - } - - return result -} - type graphNodeCloseProvider struct { ProviderNameValue string } diff --git a/terraform/transform_provider_disable.go b/terraform/transform_provider_disable.go new file mode 100644 index 000000000..d9919f3a7 --- /dev/null +++ b/terraform/transform_provider_disable.go @@ -0,0 +1,50 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/dag" +) + +// DisableProviderTransformer "disables" any providers that are not actually +// used by anything. This avoids the provider being initialized and configured. +// This both saves resources but also avoids errors since configuration +// may imply initialization which may require auth. +type DisableProviderTransformer struct{} + +func (t *DisableProviderTransformer) Transform(g *Graph) error { + for _, v := range g.Vertices() { + // We only care about providers + pn, ok := v.(GraphNodeProvider) + if !ok || pn.ProviderName() == "" { + continue + } + + // If we have dependencies, then don't disable + if g.UpEdges(v).Len() > 0 { + continue + } + + // Get the path + var path []string + if pn, ok := v.(GraphNodeSubPath); ok { + path = pn.Path() + } + + // Disable the provider by replacing it with a "disabled" provider + disabled := &NodeDisabledProvider{ + NodeAbstractProvider: &NodeAbstractProvider{ + NameValue: pn.ProviderName(), + PathValue: path, + }, + } + + if !g.Replace(v, disabled) { + panic(fmt.Sprintf( + "vertex disappeared from under us: %s", + dag.VertexName(v))) + } + } + + return nil +} diff --git a/terraform/transform_provider_old.go b/terraform/transform_provider_old.go new file mode 100644 index 000000000..eb533f230 --- /dev/null +++ b/terraform/transform_provider_old.go @@ -0,0 +1,172 @@ +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" + "github.com/hashicorp/terraform/dot" +) + +// DisableProviderTransformer "disables" any providers that are only +// depended on by modules. +// +// NOTE: "old" = used by old graph builders, will be removed one day +type DisableProviderTransformerOld struct{} + +func (t *DisableProviderTransformerOld) Transform(g *Graph) error { + // Since we're comparing against edges, we need to make sure we connect + g.ConnectDependents() + + for _, v := range g.Vertices() { + // We only care about providers + pn, ok := v.(GraphNodeProvider) + if !ok || pn.ProviderName() == "" { + continue + } + + // Go through all the up-edges (things that depend on this + // provider) and if any is not a module, then ignore this node. + nonModule := false + for _, sourceRaw := range g.UpEdges(v).List() { + source := sourceRaw.(dag.Vertex) + cn, ok := source.(graphNodeConfig) + if !ok { + nonModule = true + break + } + + if cn.ConfigType() != GraphNodeConfigTypeModule { + nonModule = true + break + } + } + if nonModule { + // We found something that depends on this provider that + // isn't a module, so skip it. + continue + } + + // Disable the provider by replacing it with a "disabled" provider + disabled := &graphNodeDisabledProvider{GraphNodeProvider: pn} + if !g.Replace(v, disabled) { + panic(fmt.Sprintf( + "vertex disappeared from under us: %s", + dag.VertexName(v))) + } + } + + return nil +} + +type graphNodeDisabledProvider struct { + GraphNodeProvider +} + +// GraphNodeEvalable impl. +func (n *graphNodeDisabledProvider) EvalTree() EvalNode { + var resourceConfig *ResourceConfig + + return &EvalOpFilter{ + Ops: []walkOperation{walkInput, walkValidate, walkRefresh, walkPlan, walkApply, walkDestroy}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalInterpolate{ + Config: n.ProviderConfig(), + Output: &resourceConfig, + }, + &EvalBuildProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + Output: &resourceConfig, + }, + &EvalSetProviderConfig{ + Provider: n.ProviderName(), + Config: &resourceConfig, + }, + }, + }, + } +} + +// GraphNodeFlattenable impl. +func (n *graphNodeDisabledProvider) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeDisabledProviderFlat{ + graphNodeDisabledProvider: n, + PathValue: p, + }, nil +} + +func (n *graphNodeDisabledProvider) Name() string { + return fmt.Sprintf("%s (disabled)", dag.VertexName(n.GraphNodeProvider)) +} + +// GraphNodeDotter impl. +func (n *graphNodeDisabledProvider) DotNode(name string, opts *GraphDotOpts) *dot.Node { + return dot.NewNode(name, map[string]string{ + "label": n.Name(), + "shape": "diamond", + }) +} + +// GraphNodeDotterOrigin impl. +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() +} + +// Same as graphNodeDisabledProvider, but for flattening +type graphNodeDisabledProviderFlat struct { + *graphNodeDisabledProvider + + PathValue []string +} + +func (n *graphNodeDisabledProviderFlat) Name() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeDisabledProvider.Name()) +} + +func (n *graphNodeDisabledProviderFlat) Path() []string { + return n.PathValue +} + +func (n *graphNodeDisabledProviderFlat) ProviderName() string { + return fmt.Sprintf( + "%s.%s", modulePrefixStr(n.PathValue), + n.graphNodeDisabledProvider.ProviderName()) +} + +// GraphNodeDependable impl. +func (n *graphNodeDisabledProviderFlat) DependableName() []string { + return modulePrefixList( + n.graphNodeDisabledProvider.DependableName(), + modulePrefixStr(n.PathValue)) +} + +func (n *graphNodeDisabledProviderFlat) 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]) + result = modulePrefixList( + n.graphNodeDisabledProvider.DependableName(), prefix) + } + + return result +} From d27c8fbbbc5cbe620622e6d5ba880186c417af7f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 15:01:54 -0700 Subject: [PATCH 78/82] terraform: compared states from shadow graph must be pruned --- terraform/shadow_context.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/terraform/shadow_context.go b/terraform/shadow_context.go index 0e213d019..226fd396b 100644 --- a/terraform/shadow_context.go +++ b/terraform/shadow_context.go @@ -101,6 +101,10 @@ func newShadowContext(c *Context) (*Context, *Context, Shadow) { func shadowContextVerify(real, shadow *Context) error { var result error + // The states compared must be pruned so they're minimal/clean + real.state.prune() + shadow.state.prune() + // Compare the states if !real.state.Equal(shadow.state) { result = multierror.Append(result, fmt.Errorf( From fa25a3051b38307b278f996ef2a9611b48713326 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 15:05:38 -0700 Subject: [PATCH 79/82] terraform: orphan resources in old graph need unique ID --- terraform/context.go | 2 +- terraform/shadow_resource_provider.go | 2 +- terraform/transform_orphan.go | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/terraform/context.go b/terraform/context.go index d15b9f6c2..80f610919 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -435,7 +435,7 @@ func (c *Context) Apply() (*State, error) { } // Walk the graph - walker, err := c.walk(real, shadow, operation) + walker, err := c.walk(real, real, operation) if len(walker.ValidationErrors) > 0 { err = multierror.Append(err, walker.ValidationErrors...) } diff --git a/terraform/shadow_resource_provider.go b/terraform/shadow_resource_provider.go index bb80be31d..816d344a2 100644 --- a/terraform/shadow_resource_provider.go +++ b/terraform/shadow_resource_provider.go @@ -510,7 +510,7 @@ func (p *shadowResourceProviderShadow) Apply( p.ErrorLock.Lock() defer p.ErrorLock.Unlock() p.Error = multierror.Append(p.Error, fmt.Errorf( - "Unknown 'apply' shadow value: %#v", raw)) + "Unknown 'apply' shadow value for %q: %#v", key, raw)) return nil, nil } diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index f47f51681..27f032251 100644 --- a/terraform/transform_orphan.go +++ b/terraform/transform_orphan.go @@ -209,6 +209,8 @@ func (n *graphNodeOrphanResource) EvalTree() EvalNode { // Build instance info info := &InstanceInfo{Id: n.ResourceKey.String(), Type: n.ResourceKey.Type} + info.uniqueExtra = "destroy" + seq.Nodes = append(seq.Nodes, &EvalInstanceInfo{Info: info}) // Each resource mode has its own lifecycle From 51e90cd64127e01adaa17de20bb30f04fe1f2363 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 15:07:00 -0700 Subject: [PATCH 80/82] terraform: move references for disable provider transform to old --- terraform/graph_builder_import.go | 2 +- terraform/transform_provider_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/terraform/graph_builder_import.go b/terraform/graph_builder_import.go index 06763710c..6d87d487d 100644 --- a/terraform/graph_builder_import.go +++ b/terraform/graph_builder_import.go @@ -46,7 +46,7 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { // Provider-related transformations &MissingProviderTransformer{Providers: b.Providers}, &ProviderTransformer{}, - &DisableProviderTransformer{}, + &DisableProviderTransformerOld{}, &PruneProviderTransformer{}, // Single root diff --git a/terraform/transform_provider_test.go b/terraform/transform_provider_test.go index 22870fc77..bc2cff532 100644 --- a/terraform/transform_provider_test.go +++ b/terraform/transform_provider_test.go @@ -367,7 +367,7 @@ func TestDisableProviderTransformer(t *testing.T) { &ConfigTransformer{Module: mod}, &MissingProviderTransformer{Providers: []string{"aws"}}, &ProviderTransformer{}, - &DisableProviderTransformer{}, + &DisableProviderTransformerOld{}, &CloseProviderTransformer{}, &PruneProviderTransformer{}, } @@ -393,7 +393,7 @@ func TestDisableProviderTransformer_keep(t *testing.T) { &ConfigTransformer{Module: mod}, &MissingProviderTransformer{Providers: []string{"aws"}}, &ProviderTransformer{}, - &DisableProviderTransformer{}, + &DisableProviderTransformerOld{}, &CloseProviderTransformer{}, &PruneProviderTransformer{}, } From a89dcfd1b107a25e89b3e9c2f706f3ddbf7b8d46 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 15:09:01 -0700 Subject: [PATCH 81/82] terraform: re-enable shadow tests --- terraform/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/context.go b/terraform/context.go index 80f610919..d15b9f6c2 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -435,7 +435,7 @@ func (c *Context) Apply() (*State, error) { } // Walk the graph - walker, err := c.walk(real, real, operation) + walker, err := c.walk(real, shadow, operation) if len(walker.ValidationErrors) > 0 { err = multierror.Append(err, walker.ValidationErrors...) } From fee0351c660828a09eeccc1c68efc578b8e1676d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Oct 2016 15:21:09 -0700 Subject: [PATCH 82/82] config: RawConfig merge should only set unknown keys non-nil if non-empty --- config/raw_config.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/config/raw_config.go b/config/raw_config.go index 260e315bb..ce57945cc 100644 --- a/config/raw_config.go +++ b/config/raw_config.go @@ -191,17 +191,19 @@ func (r *RawConfig) Merge(other *RawConfig) *RawConfig { } // Build the unknown keys - unknownKeys := make(map[string]struct{}) - for _, k := range r.unknownKeys { - unknownKeys[k] = struct{}{} - } - for _, k := range other.unknownKeys { - unknownKeys[k] = struct{}{} - } + if len(r.unknownKeys) > 0 || len(other.unknownKeys) > 0 { + unknownKeys := make(map[string]struct{}) + for _, k := range r.unknownKeys { + unknownKeys[k] = struct{}{} + } + for _, k := range other.unknownKeys { + unknownKeys[k] = struct{}{} + } - result.unknownKeys = make([]string, 0, len(unknownKeys)) - for k, _ := range unknownKeys { - result.unknownKeys = append(result.unknownKeys, k) + result.unknownKeys = make([]string, 0, len(unknownKeys)) + for k, _ := range unknownKeys { + result.unknownKeys = append(result.unknownKeys, k) + } } return result