remove provisioner graph nodes

Provisioners are always run via a single instance, and their
configuration and references are taken care of via their parent
resource, so there is no need to maintain the provisioner graph nodes.

The only action that will still need to be completed is calling the
Close method for external plugins. This was not consistently done with
the current handling of provisioners anyway, and we can now add that to
a single point with the new CloseProvisioners context method.

terraform/graph_builder_apply.go

@@ -108,10 +108,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
-		// Provisioner-related transformations
-		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
-		&ProvisionerTransformer{},
-
 		// add providers
 		TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config),
 
@@ -162,7 +158,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 
 		// Close opened plugin connections
 		&CloseProviderTransformer{},
-		&CloseProvisionerTransformer{},
 
 		// close the root module
 		&CloseRootModuleTransformer{},

terraform/graph_builder_apply_test.go

@@ -423,70 +423,6 @@ func TestApplyGraphBuilder_moduleDestroy(t *testing.T) {
 	)
 }
 
-func TestApplyGraphBuilder_provisioner(t *testing.T) {
-	changes := &plans.Changes{
-		Resources: []*plans.ResourceInstanceChangeSrc{
-			{
-				Addr: mustResourceInstanceAddr("test_object.foo"),
-				ChangeSrc: plans.ChangeSrc{
-					Action: plans.Create,
-				},
-			},
-		},
-	}
-
-	b := &ApplyGraphBuilder{
-		Config:     testModule(t, "graph-builder-apply-provisioner"),
-		Changes:    changes,
-		Components: simpleMockComponentFactory(),
-		Schemas:    simpleTestSchemas(),
-	}
-
-	g, err := b.Build(addrs.RootModuleInstance)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	testGraphContains(t, g, "provisioner.test")
-	testGraphHappensBefore(
-		t, g,
-		"provisioner.test",
-		"test_object.foo",
-	)
-}
-
-func TestApplyGraphBuilder_provisionerDestroy(t *testing.T) {
-	changes := &plans.Changes{
-		Resources: []*plans.ResourceInstanceChangeSrc{
-			{
-				Addr: mustResourceInstanceAddr("test_object.foo"),
-				ChangeSrc: plans.ChangeSrc{
-					Action: plans.Delete,
-				},
-			},
-		},
-	}
-
-	b := &ApplyGraphBuilder{
-		Config:     testModule(t, "graph-builder-apply-provisioner"),
-		Changes:    changes,
-		Components: simpleMockComponentFactory(),
-		Schemas:    simpleTestSchemas(),
-	}
-
-	g, err := b.Build(addrs.RootModuleInstance)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	testGraphContains(t, g, "provisioner.test")
-	testGraphHappensBefore(
-		t, g,
-		"provisioner.test",
-		"test_object.foo (destroy)",
-	)
-}
-
 func TestApplyGraphBuilder_targetModule(t *testing.T) {
 	changes := &plans.Changes{
 		Resources: []*plans.ResourceInstanceChangeSrc{
@@ -784,7 +720,6 @@ module.child.test_object.create
 module.child.test_object.create (expand)
   module.child (expand)
   provider["registry.terraform.io/hashicorp/test"]
-  provisioner.test
 module.child.test_object.other
   module.child.test_object.create
   module.child.test_object.other (expand)
@@ -795,13 +730,9 @@ provider["registry.terraform.io/hashicorp/test"]
 provider["registry.terraform.io/hashicorp/test"] (close)
   module.child.test_object.other
   test_object.other
-provisioner.test
-provisioner.test (close)
-  module.child.test_object.create
 root
   meta.count-boundary (EachMode fixup)
   provider["registry.terraform.io/hashicorp/test"] (close)
-  provisioner.test (close)
 test_object.create
   test_object.create (expand)
 test_object.create (expand)

terraform/graph_builder_plan.go

@@ -115,10 +115,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
-		// Provisioner-related transformations
-		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
-		&ProvisionerTransformer{},
-
 		// add providers
 		TransformProviders(b.Components.ResourceProviders(), b.ConcreteProvider, b.Config),
 

terraform/node_provisioner.go

@@ -1,45 +0,0 @@
-package terraform
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/terraform/addrs"
-	"github.com/hashicorp/terraform/tfdiags"
-)
-
-// NodeProvisioner represents a provider that has no associated operations.
-// It registers all the common interfaces across operations for providers.
-type NodeProvisioner struct {
-	NameValue string
-	PathValue addrs.ModuleInstance
-}
-
-var (
-	_ GraphNodeModuleInstance = (*NodeProvisioner)(nil)
-	_ GraphNodeProvisioner    = (*NodeProvisioner)(nil)
-	_ GraphNodeExecutable     = (*NodeProvisioner)(nil)
-)
-
-func (n *NodeProvisioner) Name() string {
-	result := fmt.Sprintf("provisioner.%s", n.NameValue)
-	if len(n.PathValue) > 0 {
-		result = fmt.Sprintf("%s.%s", n.PathValue.String(), result)
-	}
-
-	return result
-}
-
-// GraphNodeModuleInstance
-func (n *NodeProvisioner) Path() addrs.ModuleInstance {
-	return n.PathValue
-}
-
-// GraphNodeProvisioner
-func (n *NodeProvisioner) ProvisionerName() string {
-	return n.NameValue
-}
-
-// GraphNodeExecutable impl.
-func (n *NodeProvisioner) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
-	return diags.Append(ctx.InitProvisioner(n.NameValue))
-}

terraform/node_resource_abstract_instance.go

@@ -1638,7 +1638,12 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
 		log.Printf("[TRACE] applyProvisioners: provisioning %s with %q", n.Addr, prov.Type)
 
 		// Get the provisioner
-		provisioner := ctx.Provisioner(prov.Type)
+		provisioner, err := ctx.Provisioner(prov.Type)
+		if err != nil {
+			diags = diags.Append(err)
+			return diags.Err()
+		}
+
 		schema := ctx.ProvisionerSchema(prov.Type)
 
 		config, configDiags := evalScope(ctx, prov.Config, self, schema)

terraform/node_resource_validate.go

@@ -67,7 +67,12 @@ func (n *NodeValidatableResource) Execute(ctx EvalContext, op walkOperation) (di
 func (n *NodeValidatableResource) validateProvisioner(ctx EvalContext, p *configs.Provisioner, hasCount, hasForEach bool) tfdiags.Diagnostics {
 	var diags tfdiags.Diagnostics
 
-	provisioner := ctx.Provisioner(p.Type)
+	provisioner, err := ctx.Provisioner(p.Type)
+	if err != nil {
+		diags = diags.Append(err)
+		return diags
+	}
+
 	if provisioner == nil {
 		return diags.Append(fmt.Errorf("provisioner %s not initialized", p.Type))
 	}

terraform/transform_provisioner.go

@@ -1,182 +1,8 @@
 package terraform
 
-import (
-	"fmt"
-	"log"
-
-	"github.com/hashicorp/go-multierror"
-	"github.com/hashicorp/terraform/dag"
-	"github.com/hashicorp/terraform/tfdiags"
-)
-
-// GraphNodeProvisioner is an interface that nodes that can be a provisioner
-// must implement. The ProvisionerName returned is the name of the provisioner
-// they satisfy.
-type GraphNodeProvisioner interface {
-	ProvisionerName() string
-}
-
-// GraphNodeCloseProvisioner is an interface that nodes that can be a close
-// provisioner must implement. The CloseProvisionerName returned is the name
-// of the provisioner they satisfy.
-type GraphNodeCloseProvisioner interface {
-	CloseProvisionerName() string
-}
-
 // GraphNodeProvisionerConsumer is an interface that nodes that require
 // a provisioner must implement. ProvisionedBy must return the names of the
 // provisioners to use.
 type GraphNodeProvisionerConsumer interface {
 	ProvisionedBy() []string
 }
-
-// ProvisionerTransformer is a GraphTransformer that maps resources to
-// provisioners within the graph. This will error if there are any resources
-// that don't map to proper resources.
-type ProvisionerTransformer struct{}
-
-func (t *ProvisionerTransformer) Transform(g *Graph) error {
-	// Go through the other nodes and match them to provisioners they need
-	var err error
-	m := provisionerVertexMap(g)
-	for _, v := range g.Vertices() {
-		if pv, ok := v.(GraphNodeProvisionerConsumer); ok {
-			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), p))
-					continue
-				}
-
-				log.Printf("[TRACE] ProvisionerTransformer: %s is provisioned by %s (%q)", dag.VertexName(v), p, dag.VertexName(m[p]))
-				g.Connect(dag.BasicEdge(v, m[p]))
-			}
-		}
-	}
-
-	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, we skip it.
-				// Validation later will catch this as an error.
-				continue
-			}
-
-			// Build the vertex
-			var newV dag.Vertex = &NodeProvisioner{
-				NameValue: p,
-			}
-
-			// Add the missing provisioner node to the graph
-			m[p] = g.Add(newV)
-			log.Printf("[TRACE] MissingProviderTransformer: added implicit provisioner %s, first implied by %s", p, dag.VertexName(v))
-		}
-	}
-
-	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
-// resources in the graph are evaluated.
-type CloseProvisionerTransformer struct{}
-
-func (t *CloseProvisionerTransformer) Transform(g *Graph) error {
-	m := closeProvisionerVertexMap(g)
-	for _, v := range g.Vertices() {
-		if pv, ok := v.(GraphNodeProvisionerConsumer); ok {
-			for _, p := range pv.ProvisionedBy() {
-				source := m[p]
-
-				if source == nil {
-					// Create a new graphNodeCloseProvisioner and add it to the graph
-					source = &graphNodeCloseProvisioner{ProvisionerNameValue: p}
-					g.Add(source)
-
-					// Make sure we also add the new graphNodeCloseProvisioner to the map
-					// so we don't create and add any duplicate graphNodeCloseProvisioners.
-					m[p] = source
-				}
-
-				g.Connect(dag.BasicEdge(source, v))
-			}
-		}
-	}
-
-	return nil
-}
-
-func provisionerVertexMap(g *Graph) map[string]dag.Vertex {
-	m := make(map[string]dag.Vertex)
-	for _, v := range g.Vertices() {
-		if pv, ok := v.(GraphNodeProvisioner); ok {
-			m[pv.ProvisionerName()] = v
-		}
-	}
-
-	return m
-}
-
-func closeProvisionerVertexMap(g *Graph) map[string]dag.Vertex {
-	m := make(map[string]dag.Vertex)
-	for _, v := range g.Vertices() {
-		if pv, ok := v.(GraphNodeCloseProvisioner); ok {
-			m[pv.CloseProvisionerName()] = v
-		}
-	}
-
-	return m
-}
-
-type graphNodeCloseProvisioner struct {
-	ProvisionerNameValue string
-}
-
-var _ GraphNodeExecutable = (*graphNodeCloseProvisioner)(nil)
-
-func (n *graphNodeCloseProvisioner) Name() string {
-	return fmt.Sprintf("provisioner.%s (close)", n.ProvisionerNameValue)
-}
-
-// GraphNodeExecutable impl.
-func (n *graphNodeCloseProvisioner) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) {
-	return diags.Append(ctx.CloseProvisioner(n.ProvisionerNameValue))
-}
-
-func (n *graphNodeCloseProvisioner) CloseProvisionerName() string {
-	return n.ProvisionerNameValue
-}

terraform/transform_provisioner_test.go

@@ -1,203 +0,0 @@
-package terraform
-
-import (
-	"strings"
-	"testing"
-
-	"github.com/hashicorp/terraform/addrs"
-	"github.com/hashicorp/terraform/dag"
-	"github.com/hashicorp/terraform/states"
-)
-
-func TestMissingProvisionerTransformer(t *testing.T) {
-	mod := testModule(t, "transform-provisioner-basic")
-
-	g := Graph{Path: addrs.RootModuleInstance}
-	{
-		tf := &ConfigTransformer{Config: mod}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &AttachResourceConfigTransformer{Config: mod}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &MissingProvisionerTransformer{Provisioners: []string{"shell"}}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &ProvisionerTransformer{}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformMissingProvisionerBasicStr)
-	if actual != expected {
-		t.Fatalf("bad:\n\n%s", actual)
-	}
-}
-
-func TestMissingProvisionerTransformer_module(t *testing.T) {
-	mod := testModule(t, "transform-provisioner-module")
-
-	g := Graph{Path: addrs.RootModuleInstance}
-	{
-		concreteResource := func(a *NodeAbstractResourceInstance) dag.Vertex {
-			return a
-		}
-
-		state := states.BuildState(func(s *states.SyncState) {
-			s.SetResourceInstanceCurrent(
-				addrs.Resource{
-					Mode: addrs.ManagedResourceMode,
-					Type: "aws_instance",
-					Name: "foo",
-				}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
-				&states.ResourceInstanceObjectSrc{
-					AttrsFlat: map[string]string{
-						"id": "foo",
-					},
-					Status: states.ObjectReady,
-				},
-				addrs.AbsProviderConfig{
-					Provider: addrs.NewDefaultProvider("aws"),
-					Module:   addrs.RootModule,
-				},
-			)
-			s.SetResourceInstanceCurrent(
-				addrs.Resource{
-					Mode: addrs.ManagedResourceMode,
-					Type: "aws_instance",
-					Name: "foo",
-				}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance.Child("child", addrs.NoKey)),
-				&states.ResourceInstanceObjectSrc{
-					AttrsFlat: map[string]string{
-						"id": "foo",
-					},
-					Status: states.ObjectReady,
-				},
-				addrs.AbsProviderConfig{
-					Provider: addrs.NewDefaultProvider("aws"),
-					Module:   addrs.RootModule,
-				},
-			)
-		})
-
-		tf := &StateTransformer{
-			ConcreteCurrent: concreteResource,
-			State:           state,
-		}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		t.Logf("graph after StateTransformer:\n%s", g.StringWithNodeTypes())
-	}
-
-	{
-		transform := &AttachResourceConfigTransformer{Config: mod}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &MissingProvisionerTransformer{Provisioners: []string{"shell"}}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		t.Logf("graph after MissingProvisionerTransformer:\n%s", g.StringWithNodeTypes())
-	}
-
-	{
-		transform := &ProvisionerTransformer{}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		t.Logf("graph after ProvisionerTransformer:\n%s", g.StringWithNodeTypes())
-	}
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformMissingProvisionerModuleStr)
-	if actual != expected {
-		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
-	}
-}
-
-func TestCloseProvisionerTransformer(t *testing.T) {
-	mod := testModule(t, "transform-provisioner-basic")
-
-	g := Graph{Path: addrs.RootModuleInstance}
-	{
-		tf := &ConfigTransformer{Config: mod}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &AttachResourceConfigTransformer{Config: mod}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &MissingProvisionerTransformer{Provisioners: []string{"shell"}}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &ProvisionerTransformer{}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	{
-		transform := &CloseProvisionerTransformer{}
-		if err := transform.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testTransformCloseProvisionerBasicStr)
-	if actual != expected {
-		t.Fatalf("bad:\n\n%s", actual)
-	}
-}
-
-const testTransformMissingProvisionerBasicStr = `
-aws_instance.web
-  provisioner.shell
-provisioner.shell
-`
-
-const testTransformMissingProvisionerModuleStr = `
-aws_instance.foo
-  provisioner.shell
-module.child.aws_instance.foo
-  provisioner.shell
-provisioner.shell
-`
-
-const testTransformCloseProvisionerBasicStr = `
-aws_instance.web
-  provisioner.shell
-provisioner.shell
-provisioner.shell (close)
-  aws_instance.web
-`