complete passing providers through modules

Here we complete the passing of providers between modules via the
module/providers configuration, add another test and update broken test
outputs.

The DisbableProviderTransformer is being removed, since it was really
only for provider configuration inheritance. Since configuration is no
longer inherited, there's no need to keep around unused providers. The
actually shouldn't be any unused providers going into the graph any
longer, but put off verifying that condition for later.  Replace it's
usage with the PruneProviderTransformer, and use that to also remove the
unneeded proxy provider nodes.

terraform/test-fixtures/transform-provider-grandchild-inherit/child/grandchild/main.tf

@@ -0,0 +1,7 @@
+provider "aws" {
+  alias = "baz"
+}
+
+resource "aws_instance" "baz" {
+  provider = "aws.baz"
+}

terraform/test-fixtures/transform-provider-grandchild-inherit/child/main.tf

@@ -0,0 +1,10 @@
+provider "aws" {
+  alias = "bar"
+}
+
+module "grandchild" {
+  source = "./grandchild"
+  providers = {
+    "aws.baz" = "aws.bar"
+  }
+}

terraform/test-fixtures/transform-provider-grandchild-inherit/main.tf

@@ -0,0 +1,11 @@
+provider "aws" {
+  alias = "foo"
+  value = "config"
+}
+
+module "child" {
+  source = "child"
+  providers = {
+    "aws.bar" = "aws.foo"
+  }
+}

terraform/test-fixtures/transform-provider-inherit/child/main.tf

@@ -0,0 +1,7 @@
+provider "aws" {
+    alias = "bar"
+}
+
+resource "aws_instance" "thing" {
+    provider = "aws.bar"
+}

terraform/test-fixtures/transform-provider-inherit/main.tf

@@ -0,0 +1,11 @@
+provider "aws" {
+  alias = "foo"
+  value = "config"
+}
+
+module "child" {
+  source = "child"
+  providers = {
+    "aws.bar" = "aws.foo"
+  }
+}

terraform/transform_provider.go

@@ -27,8 +27,8 @@ func TransformProviders(providers []string, concrete ConcreteProviderNodeFunc, m
 		},
 		// Connect the providers
 		&ProviderTransformer{},
-		// Disable unused providers
+		// Remove unused providers and proxies
-		&DisableProviderTransformer{},
+		&PruneProviderTransformer{},
 		// Connect provider to their parent provider nodes
 		&ParentProviderTransformer{},
 	)
@@ -108,8 +108,8 @@ func (t *ProviderTransformer) Transform(g *Graph) error {
 			// see if this in  an inherited provider
 			if p, ok := target.(*graphNodeProxyProvider); ok {
 				g.Remove(p)
-				target = p.Target
+				target = p.Target()
-				key = p.Target.Name()
+				key = target.(GraphNodeProvider).Name()
 			}
 
 			log.Printf("[DEBUG] resource %s using provider %s", dag.VertexName(pv), key)
@@ -253,22 +253,29 @@ func (t *ParentProviderTransformer) Transform(g *Graph) error {
 	return nil
 }
 
-// PruneProviderTransformer is a GraphTransformer that prunes all the
+// PruneProviderTransformer removes any providers that are not actually used by
-// providers that aren't needed from the graph. A provider is unneeded if
+// anything, and provider proxies. This avoids the provider being initialized
-// no resource or module is using that provider.
+// and configured.  This both saves resources but also avoids errors since
+// configuration may imply initialization which may require auth.
 type PruneProviderTransformer struct{}
 
 func (t *PruneProviderTransformer) Transform(g *Graph) error {
 	for _, v := range g.Vertices() {
-		// We only care about the providers
+		// We only care about providers
-		if pn, ok := v.(GraphNodeProvider); !ok || pn.ProviderName() == "" {
+		pn, ok := v.(GraphNodeProvider)
+		if !ok || pn.ProviderName() == "" {
 			continue
 		}
-		// Does anything depend on this? If not, then prune it.
+
-		if s := g.UpEdges(v); s.Len() == 0 {
+		// ProxyProviders will have up edges, but we're now done with them in the graph
-			if nv, ok := v.(dag.NamedVertex); ok {
+		if _, ok := v.(*graphNodeProxyProvider); ok {
-				log.Printf("[DEBUG] Pruning provider with no dependencies: %s", nv.Name())
+			log.Printf("[DEBUG] pruning proxy provider %s", dag.VertexName(v))
-			}
+			g.Remove(v)
+		}
+
+		// Remove providers with no dependencies.
+		if g.UpEdges(v).Len() == 0 {
+			log.Printf("[DEBUG] pruning unused provider %s", dag.VertexName(v))
 			g.Remove(v)
 		}
 	}
@@ -360,17 +367,27 @@ func (n *graphNodeCloseProvider) RemoveIfNotTargeted() bool {
 // configurations, and are removed after all the resources have been connected
 // to their providers.
 type graphNodeProxyProvider struct {
-	NameValue string
+	nameValue string
-	Path      []string
+	path      []string
-	Target    GraphNodeProvider
+	target    GraphNodeProvider
 }
 
 func (n *graphNodeProxyProvider) ProviderName() string {
-	return n.Target.ProviderName()
+	return n.Target().ProviderName()
 }
 
 func (n *graphNodeProxyProvider) Name() string {
-	return ResolveProviderName(n.NameValue, n.Path)
+	return ResolveProviderName(n.nameValue, n.path)
+}
+
+// find the concrete provider instance
+func (n *graphNodeProxyProvider) Target() GraphNodeProvider {
+	switch t := n.target.(type) {
+	case *graphNodeProxyProvider:
+		return t.Target()
+	default:
+		return n.target
+	}
 }
 
 // ProviderConfigTransformer adds all provider nodes from the configuration and
@@ -511,14 +528,14 @@ func (t *ProviderConfigTransformer) addProxyProvider(g *Graph, m *module.Tree, p
 	}
 
 	v := &graphNodeProxyProvider{
-		NameValue: name,
+		nameValue: name,
-		Path:      path,
+		path:      path,
-		Target:    parentProvider,
+		target:    parentProvider,
 	}
 
 	// Add it to the graph
 	g.Add(v)
-	t.providers[v.NameValue] = v
+	t.providers[ResolveProviderName(name, path)] = v
 	return true
 }
 

terraform/transform_provider_disable.go

@@ -1,56 +0,0 @@
-package terraform
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/terraform/dag"
-)
-
-// DisableProviderTransformer "disables" any providers that are not actually
-// used by anything, and provider proxies. 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
-		}
-
-		// remove the proxy nodes now that we're done with them
-		if pn, ok := v.(*graphNodeProxyProvider); ok {
-			g.Remove(pn)
-			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
-}

terraform/transform_provider_test.go

@@ -478,6 +478,39 @@ func TestProviderConfigTransformer_parentProviders(t *testing.T) {
 	}
 }
 
+// the child module resource is attached to the configured grand-parent provider
+func TestProviderConfigTransformer_grandparentProviders(t *testing.T) {
+	mod := testModule(t, "transform-provider-grandchild-inherit")
+	concrete := func(a *NodeAbstractProvider) dag.Vertex { return a }
+
+	g := Graph{Path: RootModulePath}
+	{
+		tf := &ConfigTransformer{Module: mod}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+	{
+		tf := &AttachResourceConfigTransformer{Module: mod}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		tf := TransformProviders([]string{"aws"}, concrete, mod)
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformModuleProviderGrandparentStr)
+	if actual != expected {
+		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)
+	}
+}
+
 const testTransformProviderBasicStr = `
 aws_instance.web
   provider.aws
@@ -514,9 +547,7 @@ module.sub.module.subsub.bar_instance.two
 module.sub.module.subsub.foo_instance.one
   module.sub.provider.foo
 module.sub.provider.foo
-  provider.foo (disabled)
 provider.bar
-provider.foo (disabled)
 `
 
 const testTransformMissingProviderModuleChildStr = `
@@ -584,3 +615,9 @@ module.child.aws_instance.thing
   provider.aws.foo
 provider.aws.foo
 `
+
+const testTransformModuleProviderGrandparentStr = `
+module.child.module.grandchild.aws_instance.baz
+  provider.aws.foo
+provider.aws.foo
+`