Merge pull request #2527 from hashicorp/b-close-provider-target

core: fix provider/close provider race when targeting

terraform/context_test.go

@@ -4423,6 +4423,37 @@ func TestContext2Apply_moduleProviderAlias(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_moduleProviderAliasTargets(t *testing.T) {
+	m := testModule(t, "apply-module-provider-alias")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Targets: []string{"no.thing"},
+	})
+
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(`
+<no state>
+	`)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
 func TestContext2Apply_moduleVarResourceCount(t *testing.T) {
 	m := testModule(t, "apply-module-var-resource-count")
 	p := testProvider("aws")

terraform/graph_builder.go

@@ -116,13 +116,11 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer {
 		// Provider-related transformations
 		&MissingProviderTransformer{Providers: b.Providers},
 		&ProviderTransformer{},
-		&CloseProviderTransformer{},
 		&DisableProviderTransformer{},
 
 		// Provisioner-related transformations
 		&MissingProvisionerTransformer{Provisioners: b.Provisioners},
 		&ProvisionerTransformer{},
-		&CloseProvisionerTransformer{},
 
 		// Run our vertex-level transforms
 		&VertexTransformer{
@@ -166,6 +164,10 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer {
 				Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State},
 			}),
 
+			// Insert nodes to close opened plugin connections
+			&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.

terraform/resource_provider_mock.go

@@ -10,6 +10,8 @@ type MockResourceProvider struct {
 	// Anything you want, in case you need to store extra data with the mock.
 	Meta interface{}
 
+	CloseCalled                  bool
+	CloseError                   error
 	InputCalled                  bool
 	InputInput                   UIInput
 	InputConfig                  *ResourceConfig
@@ -55,6 +57,11 @@ type MockResourceProvider struct {
 	ValidateResourceReturnErrors []error
 }
 
+func (p *MockResourceProvider) Close() error {
+	p.CloseCalled = true
+	return p.CloseError
+}
+
 func (p *MockResourceProvider) Input(
 	input UIInput, c *ResourceConfig) (*ResourceConfig, error) {
 	p.InputCalled = true

terraform/resource_provider_mock_test.go

@@ -6,4 +6,5 @@ import (
 
 func TestMockResourceProvider_impl(t *testing.T) {
 	var _ ResourceProvider = new(MockResourceProvider)
+	var _ ResourceProviderCloser = new(MockResourceProvider)
 }

terraform/transform_provider.go

@@ -113,28 +113,41 @@ func (t *ProviderTransformer) Transform(g *Graph) error {
 type CloseProviderTransformer struct{}
 
 func (t *CloseProviderTransformer) Transform(g *Graph) error {
-	m := closeProviderVertexMap(g)
+	pm := providerVertexMap(g)
+	cpm := closeProviderVertexMap(g)
+	var err error
 	for _, v := range g.Vertices() {
 		if pv, ok := v.(GraphNodeProviderConsumer); ok {
 			for _, p := range pv.ProvidedBy() {
-				source := m[p]
+				source := cpm[p]
 
 				if source == nil {
 					// Create a new graphNodeCloseProvider and add it to the graph
 					source = &graphNodeCloseProvider{ProviderNameValue: p}
 					g.Add(source)
 
+					// Close node needs to depend on provider
+					provider, ok := pm[p]
+					if !ok {
+						err = multierror.Append(err, fmt.Errorf(
+							"%s: provider %s couldn't be found",
+							dag.VertexName(v), p))
+						continue
+					}
+					g.Connect(dag.BasicEdge(source, provider))
+
 					// Make sure we also add the new graphNodeCloseProvider to the map
 					// so we don't create and add any duplicate graphNodeCloseProviders.
-					m[p] = source
+					cpm[p] = source
 				}
 
+				// Close node depends on all nodes provided by the provider
 				g.Connect(dag.BasicEdge(source, v))
 			}
 		}
 	}
 
-	return nil
+	return err
 }
 
 // MissingProviderTransformer is a GraphTransformer that adds nodes
@@ -381,59 +394,6 @@ func (n *graphNodeCloseProvider) DotNode(name string, opts *GraphDotOpts) *dot.N
 	})
 }
 
-// GraphNodeFlattenable impl.
-func (n *graphNodeCloseProvider) Flatten(p []string) (dag.Vertex, error) {
-	return &graphNodeCloseProviderFlat{
-		graphNodeCloseProvider: n,
-		PathValue:              p,
-	}, nil
-}
-
-// Same as graphNodeCloseProvider, but for flattening
-type graphNodeCloseProviderFlat struct {
-	*graphNodeCloseProvider
-
-	PathValue []string
-}
-
-func (n *graphNodeCloseProviderFlat) Name() string {
-	return fmt.Sprintf(
-		"%s.%s", modulePrefixStr(n.PathValue), n.graphNodeCloseProvider.Name())
-}
-
-func (n *graphNodeCloseProviderFlat) Path() []string {
-	return n.PathValue
-}
-
-func (n *graphNodeCloseProviderFlat) ProviderName() string {
-	return fmt.Sprintf(
-		"%s.%s", modulePrefixStr(n.PathValue),
-		n.graphNodeCloseProvider.CloseProviderName())
-}
-
-// GraphNodeDependable impl.
-func (n *graphNodeCloseProviderFlat) DependableName() []string {
-	return []string{n.Name()}
-}
-
-func (n *graphNodeCloseProviderFlat) DependentOn() []string {
-	var result []string
-
-	// If we're in a module, then depend on our parent's provider
-	if len(n.PathValue) > 1 {
-		prefix := modulePrefixStr(n.PathValue[:len(n.PathValue)-1])
-		if prefix != "" {
-			prefix += "."
-		}
-
-		result = append(result, fmt.Sprintf(
-			"%s%s",
-			prefix, n.graphNodeCloseProvider.Name()))
-	}
-
-	return result
-}
-
 type graphNodeMissingProvider struct {
 	ProviderNameValue string
 }