Merge pull request #27824 from hashicorp/jbardin/proxy-providers

Provider transformer cleanup

terraform/transform_provider.go

@@ -31,8 +31,6 @@ func TransformProviders(providers []string, concrete ConcreteProviderNodeFunc, c
 		},
 		// Remove unused providers and proxies
 		&PruneProviderTransformer{},
-		// Connect provider to their parent provider nodes
-		&ParentProviderTransformer{},
 	)
 }
 
@@ -227,7 +225,7 @@ func (t *ProviderTransformer) Transform(g *Graph) error {
 				break
 			}
 
-			// see if this in  an inherited provider
+			// see if this is a proxy provider pointing to another concrete config
 			if p, ok := target.(*graphNodeProxyProvider); ok {
 				g.Remove(p)
 				target = p.Target()
@@ -357,42 +355,6 @@ func (t *MissingProviderTransformer) Transform(g *Graph) error {
 	return err
 }
 
-// ParentProviderTransformer connects provider nodes to their parents.
-//
-// This works by finding nodes that are both GraphNodeProviders and
-// GraphNodeModuleInstance. It then connects the providers to their parent
-// path. The parent provider is always at the root level.
-type ParentProviderTransformer struct{}
-
-func (t *ParentProviderTransformer) Transform(g *Graph) error {
-	pm := providerVertexMap(g)
-	for _, v := range g.Vertices() {
-		// Only care about providers
-		pn, ok := v.(GraphNodeProvider)
-		if !ok {
-			continue
-		}
-
-		// Also require non-empty path, since otherwise we're in the root
-		// module and so cannot have a parent.
-		if len(pn.ModulePath()) <= 1 {
-			continue
-		}
-
-		// this provider may be disabled, but we can only get it's name from
-		// the ProviderName string
-		addr := pn.ProviderAddr()
-		parentAddr, ok := addr.Inherited()
-		if ok {
-			parent := pm[parentAddr.String()]
-			if parent != nil {
-				g.Connect(dag.BasicEdge(v, parent))
-			}
-		}
-	}
-	return nil
-}
-
 // PruneProviderTransformer removes 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
@@ -605,43 +567,6 @@ func (t *ProviderConfigTransformer) transformSingle(g *Graph, c *configs.Config)
 		t.proxiable[key] = !diags.HasErrors()
 	}
 
-	if mod.ProviderRequirements != nil {
-		// Add implied provider configs from the required_providers
-		// Since we're still treating empty configs as proxies, we can just add
-		// these as empty configs too. We'll ensure that these are given a
-		// configuration during validation to prevent them from becoming
-		// fully-fledged config instances.
-		for _, p := range mod.ProviderRequirements.RequiredProviders {
-			for _, aliasAddr := range p.Aliases {
-				addr := addrs.AbsProviderConfig{
-					Provider: mod.ProviderForLocalConfig(aliasAddr),
-					Module:   path,
-					Alias:    aliasAddr.Alias,
-				}
-
-				key := addr.String()
-				if _, ok := t.providers[key]; ok {
-					continue
-				}
-
-				abstract := &NodeAbstractProvider{
-					Addr: addr,
-				}
-				var v dag.Vertex
-				if t.Concrete != nil {
-					v = t.Concrete(abstract)
-				} else {
-					v = abstract
-				}
-
-				// Add it to the graph
-				g.Add(v)
-				t.providers[key] = v.(GraphNodeProvider)
-				t.proxiable[key] = true
-			}
-		}
-	}
-
 	// Now replace the provider nodes with proxy nodes if a provider was being
 	// passed in, and create implicit proxies if there was no config. Any extra
 	// proxies will be removed in the prune step.
@@ -708,15 +633,13 @@ func (t *ProviderConfigTransformer) addProxyProviders(g *Graph, c *configs.Confi
 
 		concreteProvider := t.providers[fullName]
 
-		// replace the concrete node with the provider passed in
+		// replace the concrete node with the provider passed in only if it is
-		if concreteProvider != nil && t.proxiable[fullName] {
+		// proxyable
+		if concreteProvider != nil {
+			if t.proxiable[fullName] {
 				g.Replace(concreteProvider, proxy)
 				t.providers[fullName] = proxy
-			continue
 			}
-
-		// aliased configurations can't be implicitly passed in
-		if fullAddr.Alias != "" {
 			continue
 		}
 

terraform/transform_provider_test.go

@@ -6,36 +6,39 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform/addrs"
+	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/dag"
 )
 
+func testProviderTransformerGraph(t *testing.T, cfg *configs.Config) *Graph {
+	t.Helper()
+
+	g := &Graph{Path: addrs.RootModuleInstance}
+	ct := &ConfigTransformer{Config: cfg}
+	if err := ct.Transform(g); err != nil {
+		t.Fatal(err)
+	}
+	arct := &AttachResourceConfigTransformer{Config: cfg}
+	if err := arct.Transform(g); err != nil {
+		t.Fatal(err)
+	}
+
+	return g
+}
+
 func TestProviderTransformer(t *testing.T) {
 	mod := testModule(t, "transform-provider-basic")
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
-	{
-		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 := &MissingProviderTransformer{Providers: []string{"aws"}}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	transform := &ProviderTransformer{}
-	if err := transform.Transform(&g); err != nil {
+	if err := transform.Transform(g); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 
@@ -49,23 +52,9 @@ func TestProviderTransformer(t *testing.T) {
 func TestProviderTransformer_ImportModuleChild(t *testing.T) {
 	mod := testModule(t, "import-module")
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
 
 	{
-		{
-			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)
-			}
-		}
-
 		tf := &ImportStateTransformer{
 			Config: mod,
 			Targets: []*ImportTarget{
@@ -83,7 +72,7 @@ func TestProviderTransformer_ImportModuleChild(t *testing.T) {
 			},
 		}
 
-		if err := tf.Transform(&g); err != nil {
+		if err := tf.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 		t.Logf("graph after ImportStateTransformer:\n%s", g.String())
@@ -91,7 +80,7 @@ func TestProviderTransformer_ImportModuleChild(t *testing.T) {
 
 	{
 		tf := &MissingProviderTransformer{Providers: []string{"foo", "bar"}}
-		if err := tf.Transform(&g); err != nil {
+		if err := tf.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 		t.Logf("graph after MissingProviderTransformer:\n%s", g.String())
@@ -99,7 +88,7 @@ func TestProviderTransformer_ImportModuleChild(t *testing.T) {
 
 	{
 		tf := &ProviderTransformer{}
-		if err := tf.Transform(&g); err != nil {
+		if err := tf.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 		t.Logf("graph after ProviderTransformer:\n%s", g.String())
@@ -117,30 +106,16 @@ func TestProviderTransformer_fqns(t *testing.T) {
 	for _, mod := range []string{"fqns", "fqns-module"} {
 		mod := testModule(t, fmt.Sprintf("transform-provider-%s", mod))
 
-		g := Graph{Path: addrs.RootModuleInstance}
+		g := testProviderTransformerGraph(t, mod)
-		{
-			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 := &MissingProviderTransformer{Providers: []string{"aws"}, Config: mod}
-			if err := transform.Transform(&g); err != nil {
+			if err := transform.Transform(g); err != nil {
 				t.Fatalf("err: %s", err)
 			}
 		}
 
 		transform := &ProviderTransformer{Config: mod}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 
@@ -154,39 +129,25 @@ func TestProviderTransformer_fqns(t *testing.T) {
 
 func TestCloseProviderTransformer(t *testing.T) {
 	mod := testModule(t, "transform-provider-basic")
-
+	g := testProviderTransformerGraph(t, mod)
-	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 := &MissingProviderTransformer{Providers: []string{"aws"}}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	{
 		transform := &ProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	{
 		transform := &CloseProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
@@ -201,9 +162,8 @@ func TestCloseProviderTransformer(t *testing.T) {
 func TestCloseProviderTransformer_withTargets(t *testing.T) {
 	mod := testModule(t, "transform-provider-basic")
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
 	transforms := []GraphTransformer{
-		&ConfigTransformer{Config: mod},
 		&MissingProviderTransformer{Providers: []string{"aws"}},
 		&ProviderTransformer{},
 		&CloseProviderTransformer{},
@@ -217,7 +177,7 @@ func TestCloseProviderTransformer_withTargets(t *testing.T) {
 	}
 
 	for _, tr := range transforms {
-		if err := tr.Transform(&g); err != nil {
+		if err := tr.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
@@ -232,38 +192,24 @@ func TestCloseProviderTransformer_withTargets(t *testing.T) {
 func TestMissingProviderTransformer(t *testing.T) {
 	mod := testModule(t, "transform-provider-missing")
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
-	{
-		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 := &MissingProviderTransformer{Providers: []string{"aws", "foo", "bar"}}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	{
 		transform := &ProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	{
 		transform := &CloseProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
@@ -280,30 +226,16 @@ func TestMissingProviderTransformer_grandchildMissing(t *testing.T) {
 
 	concrete := func(a *NodeAbstractProvider) dag.Vertex { return a }
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
-	{
-		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 := TransformProviders([]string{"aws", "foo", "bar"}, concrete, mod)
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 	{
 		transform := &TransitiveReductionTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
@@ -318,45 +250,31 @@ func TestMissingProviderTransformer_grandchildMissing(t *testing.T) {
 func TestPruneProviderTransformer(t *testing.T) {
 	mod := testModule(t, "transform-provider-prune")
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
-	{
-		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 := &MissingProviderTransformer{Providers: []string{"foo"}}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	{
 		transform := &ProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	{
 		transform := &CloseProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
 
 	{
 		transform := &PruneProviderTransformer{}
-		if err := transform.Transform(&g); err != nil {
+		if err := transform.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
@@ -373,23 +291,10 @@ func TestProviderConfigTransformer_parentProviders(t *testing.T) {
 	mod := testModule(t, "transform-provider-inherit")
 	concrete := func(a *NodeAbstractProvider) dag.Vertex { return a }
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
-	{
-		tf := &ConfigTransformer{Config: mod}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-	{
-		tf := &AttachResourceConfigTransformer{Config: 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 {
+		if err := tf.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
@@ -406,23 +311,10 @@ func TestProviderConfigTransformer_grandparentProviders(t *testing.T) {
 	mod := testModule(t, "transform-provider-grandchild-inherit")
 	concrete := func(a *NodeAbstractProvider) dag.Vertex { return a }
 
-	g := Graph{Path: addrs.RootModuleInstance}
+	g := testProviderTransformerGraph(t, mod)
-	{
-		tf := &ConfigTransformer{Config: mod}
-		if err := tf.Transform(&g); err != nil {
-			t.Fatalf("err: %s", err)
-		}
-	}
-	{
-		tf := &AttachResourceConfigTransformer{Config: 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 {
+		if err := tf.Transform(g); err != nil {
 			t.Fatalf("err: %s", err)
 		}
 	}
@@ -434,6 +326,126 @@ func TestProviderConfigTransformer_grandparentProviders(t *testing.T) {
 	}
 }
 
+func TestProviderConfigTransformer_inheritOldSkool(t *testing.T) {
+	mod := testModuleInline(t, map[string]string{
+		"main.tf": `
+provider "test" {
+  test_string = "config"
+}
+
+module "moda" {
+  source = "./moda"
+}
+`,
+
+		"moda/main.tf": `
+resource "test_object" "a" {
+}
+`,
+	})
+	concrete := func(a *NodeAbstractProvider) dag.Vertex { return a }
+
+	g := testProviderTransformerGraph(t, mod)
+	{
+		tf := TransformProviders([]string{"registry.terraform.io/hashicorp/test"}, concrete, mod)
+		if err := tf.Transform(g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	expected := `module.moda.test_object.a
+  provider["registry.terraform.io/hashicorp/test"]
+provider["registry.terraform.io/hashicorp/test"]`
+
+	actual := strings.TrimSpace(g.String())
+	if actual != expected {
+		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)
+	}
+}
+
+// Verify that configurations which are not recommended yet supported still work
+func TestProviderConfigTransformer_nestedModuleProviders(t *testing.T) {
+	mod := testModuleInline(t, map[string]string{
+		"main.tf": `
+terraform {
+  required_providers {
+    test = {
+      source = "registry.terraform.io/hashicorp/test"
+	}
+  }
+}
+
+provider "test" {
+  alias = "z"
+  test_string = "config"
+}
+
+module "moda" {
+  source = "./moda"
+  providers = {
+    test.x = test.z
+  }
+}
+`,
+
+		"moda/main.tf": `
+terraform {
+  required_providers {
+    test = {
+      source = "registry.terraform.io/hashicorp/test"
+      configuration_aliases = [ test.x ]
+	}
+  }
+}
+
+provider "test" {
+  test_string = "config"
+}
+
+// this should connect to this module's provider
+resource "test_object" "a" {
+}
+
+resource "test_object" "x" {
+  provider = test.x
+}
+
+module "modb" {
+  source = "./modb"
+}
+`,
+
+		"moda/modb/main.tf": `
+# this should end up with the provider from the parent module
+resource "test_object" "a" {
+}
+`,
+	})
+	concrete := func(a *NodeAbstractProvider) dag.Vertex { return a }
+
+	g := testProviderTransformerGraph(t, mod)
+	{
+		tf := TransformProviders([]string{"registry.terraform.io/hashicorp/test"}, concrete, mod)
+		if err := tf.Transform(g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	expected := `module.moda.module.modb.test_object.a
+  module.moda.provider["registry.terraform.io/hashicorp/test"]
+module.moda.provider["registry.terraform.io/hashicorp/test"]
+module.moda.test_object.a
+  module.moda.provider["registry.terraform.io/hashicorp/test"]
+module.moda.test_object.x
+  provider["registry.terraform.io/hashicorp/test"].z
+provider["registry.terraform.io/hashicorp/test"].z`
+
+	actual := strings.TrimSpace(g.String())
+	if actual != expected {
+		t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual)
+	}
+}
+
 const testTransformProviderBasicStr = `
 aws_instance.web
   provider["registry.terraform.io/hashicorp/aws"]