diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index 7963bcbf4f7a..e043e5cd111d 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -136,9 +136,6 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { // Make sure all the connections that are proxies are connected through &ProxyTransformer{}, - - // Make sure we have a single root - &RootTransformer{}, } // If we're on the root path, then we do a bunch of other stuff. @@ -149,11 +146,9 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { // their dependencies. &TargetsTransformer{Targets: b.Targets, Destroy: b.Destroy}, - // Prune the providers and provisioners. This must happen - // only once because flattened modules might depend on empty - // providers. + // Prune the providers. This must happen only once because flattened + // modules might depend on empty providers. &PruneProviderTransformer{}, - &PruneProvisionerTransformer{}, // Create the destruction nodes &DestroyTransformer{FullDestroy: b.Destroy}, @@ -170,17 +165,15 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer { &CloseProviderTransformer{}, &CloseProvisionerTransformer{}, - // Make sure we have a single root after the above changes. - // This is the 2nd root transformer. In practice this shouldn't - // actually matter as the RootTransformer is idempotent. - &RootTransformer{}, - // Perform the transitive reduction to make our graph a bit // more sane if possible (it usually is possible). &TransitiveReductionTransformer{}, ) } + // Make sure we have a single root + steps = append(steps, &RootTransformer{}) + // Remove nils for i, s := range steps { if s == nil { diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index dbc8ae61e158..13b3a906f0c8 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -265,6 +265,7 @@ provider.aws (close) const testBuiltinGraphBuilderMultiLevelStr = ` module.foo.module.bar.output.value module.foo.module.bar.var.bar + module.foo.var.foo module.foo.module.bar.plan-destroy module.foo.module.bar.var.bar module.foo.var.foo @@ -273,7 +274,9 @@ module.foo.var.foo root module.foo.module.bar.output.value module.foo.module.bar.plan-destroy + module.foo.module.bar.var.bar module.foo.plan-destroy + module.foo.var.foo ` const testBuiltinGraphBuilderOrphanDepsStr = ` diff --git a/terraform/transform_provisioner.go b/terraform/transform_provisioner.go index a99fe48b0bff..2d86275dc84f 100644 --- a/terraform/transform_provisioner.go +++ b/terraform/transform_provisioner.go @@ -39,16 +39,15 @@ func (t *ProvisionerTransformer) Transform(g *Graph) error { m := provisionerVertexMap(g) for _, v := range g.Vertices() { if pv, ok := v.(GraphNodeProvisionerConsumer); ok { - for _, provisionerName := range pv.ProvisionedBy() { - target := m[provisionerName] - if target == nil { + for _, p := range pv.ProvisionedBy() { + if m[p] == nil { err = multierror.Append(err, fmt.Errorf( "%s: provisioner %s couldn't be found", - dag.VertexName(v), provisionerName)) + dag.VertexName(v), p)) continue } - g.Connect(dag.BasicEdge(v, target)) + g.Connect(dag.BasicEdge(v, m[p])) } } } @@ -56,6 +55,51 @@ func (t *ProvisionerTransformer) Transform(g *Graph) error { return err } +// MissingProvisionerTransformer is a GraphTransformer that adds nodes +// for missing provisioners into the graph. +type MissingProvisionerTransformer struct { + // Provisioners is the list of provisioners we support. + Provisioners []string +} + +func (t *MissingProvisionerTransformer) Transform(g *Graph) error { + // Create a set of our supported provisioners + supported := make(map[string]struct{}, len(t.Provisioners)) + for _, v := range t.Provisioners { + supported[v] = struct{}{} + } + + // Get the map of provisioners we already have in our graph + m := provisionerVertexMap(g) + + // Go through all the provisioner consumers and make sure we add + // that provisioner if it is missing. + for _, v := range g.Vertices() { + pv, ok := v.(GraphNodeProvisionerConsumer) + if !ok { + continue + } + + for _, p := range pv.ProvisionedBy() { + if _, ok := m[p]; ok { + // This provisioner already exists as a configure node + continue + } + + if _, ok := supported[p]; !ok { + // If we don't support the provisioner type, skip it. + // Validation later will catch this as an error. + continue + } + + // Add the missing provisioner node to the graph + m[p] = g.Add(&graphNodeProvisioner{ProvisionerNameValue: p}) + } + } + + return nil +} + // CloseProvisionerTransformer is a GraphTransformer that adds nodes to the // graph that will close open provisioner connections that aren't needed // anymore. A provisioner connection is not needed anymore once all depended @@ -87,51 +131,6 @@ func (t *CloseProvisionerTransformer) Transform(g *Graph) error { return nil } -// MissingProvisionerTransformer is a GraphTransformer that adds nodes -// for missing provisioners into the graph. Specifically, it creates provisioner -// configuration nodes for all the provisioners that we support. These are -// pruned later during an optimization pass. -type MissingProvisionerTransformer struct { - // Provisioners is the list of provisioners we support. - Provisioners []string -} - -func (t *MissingProvisionerTransformer) Transform(g *Graph) error { - m := provisionerVertexMap(g) - for _, p := range t.Provisioners { - if _, ok := m[p]; ok { - // This provisioner already exists as a configured node - continue - } - - // Add our own missing provisioner node to the graph - g.Add(&graphNodeMissingProvisioner{ProvisionerNameValue: p}) - } - - return nil -} - -// PruneProvisionerTransformer is a GraphTransformer that prunes all the -// provisioners that aren't needed from the graph. A provisioner is unneeded if -// no resource or module is using that provisioner. -type PruneProvisionerTransformer struct{} - -func (t *PruneProvisionerTransformer) Transform(g *Graph) error { - for _, v := range g.Vertices() { - // We only care about the provisioners - if _, ok := v.(GraphNodeProvisioner); !ok { - continue - } - - // Does anything depend on this? If not, then prune it. - if s := g.UpEdges(v); s.Len() == 0 { - g.Remove(v) - } - } - - return nil -} - func provisionerVertexMap(g *Graph) map[string]dag.Vertex { m := make(map[string]dag.Vertex) for _, v := range g.Vertices() { @@ -171,49 +170,49 @@ func (n *graphNodeCloseProvisioner) CloseProvisionerName() string { return n.ProvisionerNameValue } -type graphNodeMissingProvisioner struct { +type graphNodeProvisioner struct { ProvisionerNameValue string } -func (n *graphNodeMissingProvisioner) Name() string { +func (n *graphNodeProvisioner) Name() string { return fmt.Sprintf("provisioner.%s", n.ProvisionerNameValue) } // GraphNodeEvalable impl. -func (n *graphNodeMissingProvisioner) EvalTree() EvalNode { +func (n *graphNodeProvisioner) EvalTree() EvalNode { return &EvalInitProvisioner{Name: n.ProvisionerNameValue} } -func (n *graphNodeMissingProvisioner) ProvisionerName() string { +func (n *graphNodeProvisioner) ProvisionerName() string { return n.ProvisionerNameValue } // GraphNodeFlattenable impl. -func (n *graphNodeMissingProvisioner) Flatten(p []string) (dag.Vertex, error) { - return &graphNodeMissingProvisionerFlat{ - graphNodeMissingProvisioner: n, - PathValue: p, +func (n *graphNodeProvisioner) Flatten(p []string) (dag.Vertex, error) { + return &graphNodeProvisionerFlat{ + graphNodeProvisioner: n, + PathValue: p, }, nil } // Same as graphNodeMissingProvisioner, but for flattening -type graphNodeMissingProvisionerFlat struct { - *graphNodeMissingProvisioner +type graphNodeProvisionerFlat struct { + *graphNodeProvisioner PathValue []string } -func (n *graphNodeMissingProvisionerFlat) Name() string { +func (n *graphNodeProvisionerFlat) Name() string { return fmt.Sprintf( - "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeMissingProvisioner.Name()) + "%s.%s", modulePrefixStr(n.PathValue), n.graphNodeProvisioner.Name()) } -func (n *graphNodeMissingProvisionerFlat) Path() []string { +func (n *graphNodeProvisionerFlat) Path() []string { return n.PathValue } -func (n *graphNodeMissingProvisionerFlat) ProvisionerName() string { +func (n *graphNodeProvisionerFlat) ProvisionerName() string { return fmt.Sprintf( "%s.%s", modulePrefixStr(n.PathValue), - n.graphNodeMissingProvisioner.ProvisionerName()) + n.graphNodeProvisioner.ProvisionerName()) } diff --git a/terraform/transform_provisioner_test.go b/terraform/transform_provisioner_test.go index ea5faff77091..38874bec829c 100644 --- a/terraform/transform_provisioner_test.go +++ b/terraform/transform_provisioner_test.go @@ -19,14 +19,14 @@ func TestMissingProvisionerTransformer(t *testing.T) { } { - transform := &MissingProvisionerTransformer{Provisioners: []string{"foo"}} + transform := &MissingProvisionerTransformer{Provisioners: []string{"shell"}} if err := transform.Transform(&g); err != nil { t.Fatalf("err: %s", err) } } { - transform := &CloseProvisionerTransformer{} + transform := &ProvisionerTransformer{} if err := transform.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -39,8 +39,8 @@ func TestMissingProvisionerTransformer(t *testing.T) { } } -func TestPruneProvisionerTransformer(t *testing.T) { - mod := testModule(t, "transform-provisioner-prune") +func TestCloseProvisionerTransformer(t *testing.T) { + mod := testModule(t, "transform-provisioner-basic") g := Graph{Path: RootModulePath} { @@ -51,8 +51,7 @@ func TestPruneProvisionerTransformer(t *testing.T) { } { - transform := &MissingProvisionerTransformer{ - Provisioners: []string{"foo", "bar"}} + transform := &MissingProvisionerTransformer{Provisioners: []string{"shell"}} if err := transform.Transform(&g); err != nil { t.Fatalf("err: %s", err) } @@ -72,28 +71,20 @@ func TestPruneProvisionerTransformer(t *testing.T) { } } - { - transform := &PruneProvisionerTransformer{} - if err := transform.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformPruneProvisionerBasicStr) + expected := strings.TrimSpace(testTransformCloseProvisionerBasicStr) if actual != expected { t.Fatalf("bad:\n\n%s", actual) } } - -func TestGraphNodeMissingProvisioner_impl(t *testing.T) { - var _ dag.Vertex = new(graphNodeMissingProvisioner) - var _ dag.NamedVertex = new(graphNodeMissingProvisioner) - var _ GraphNodeProvisioner = new(graphNodeMissingProvisioner) +func TestGraphNodeProvisioner_impl(t *testing.T) { + var _ dag.Vertex = new(graphNodeProvisioner) + var _ dag.NamedVertex = new(graphNodeProvisioner) + var _ GraphNodeProvisioner = new(graphNodeProvisioner) } -func TestGraphNodeMissingProvisioner_ProvisionerName(t *testing.T) { - n := &graphNodeMissingProvisioner{ProvisionerNameValue: "foo"} +func TestGraphNodeProvisioner_ProvisionerName(t *testing.T) { + n := &graphNodeProvisioner{ProvisionerNameValue: "foo"} if v := n.ProvisionerName(); v != "foo" { t.Fatalf("bad: %#v", v) } @@ -101,15 +92,14 @@ func TestGraphNodeMissingProvisioner_ProvisionerName(t *testing.T) { const testTransformMissingProvisionerBasicStr = ` aws_instance.web -provisioner.foo -provisioner.shell (close) - aws_instance.web + provisioner.shell +provisioner.shell ` -const testTransformPruneProvisionerBasicStr = ` +const testTransformCloseProvisionerBasicStr = ` aws_instance.web - provisioner.foo -provisioner.foo -provisioner.foo (close) + provisioner.shell +provisioner.shell +provisioner.shell (close) aws_instance.web `