terraform: large refactor to use Provider from configs.Resource (#24396)

* terraform: large refactor to use Provider from configs.Resource

configs.Resource.ImpliedProvider() now returns a string; it is the
callers' responsibility to turn that into an addrs.Provider if needed.

GraphNodeProviderConsumer ProvidedBy() no longer returns nil (reverting
to earlier, pre-provider-fqn behavior): it will return either the
provider set in config, provider set in state, or the default provider.

addrs/resource.go

@@ -50,28 +50,15 @@ func (r Resource) Absolute(module ModuleInstance) AbsResource {
 	}
 }
 
-// DefaultProvider returns the address of the provider whose default
-// configuration shouldbe used for the resource identified by the reciever if
-// it does not have a provider configuration address explicitly set in
-// configuration.
-//
-// This method is not able to verify that such a configuration exists, nor
-// represent the behavior of automatically inheriting certain provider
-// configurations from parent modules. It just does a static analysis of the
-// receiving address and returns an address to start from, relative to the
-// same module that contains the resource.
-func (r Resource) DefaultProvider() Provider {
+// ImpliedProvider returns the implied provider type name, for e.g. the "aws" in
+// "aws_instance"
+func (r Resource) ImpliedProvider() string {
 	typeName := r.Type
 	if under := strings.Index(typeName, "_"); under != -1 {
 		typeName = typeName[:under]
 	}
 
-	// TODO: For now we're returning a _legacy_ provider address here
-	// because the rest of Terraform isn't yet prepared to deal with
-	// non-legacy ones. Once we phase out legacy addresses this should
-	// switch to being a _default_ provider address, i.e. one in the
-	// releases.hashicorp.com/hashicorp/... namespace.
-	return NewLegacyProvider(typeName)
+	return typeName
 }
 
 // ResourceInstance is an address for a specific instance of a resource.

configs/configupgrade/analysis.go

@@ -177,7 +177,9 @@ func (u *Upgrader) analyze(ms ModuleSources) (*analysis, error) {
 
 				var fqn addrs.Provider
 				if providerKey == "" {
-					fqn = rAddr.DefaultProvider()
+					// we are assuming a default, legacy provider for 012
+					// configurations
+					fqn = addrs.NewLegacyProvider(rAddr.ImpliedProvider())
 				} else {
 					// ProviderDependencies only need to know the provider FQN
 					// strip any alias from the providerKey

configs/module.go

@@ -293,8 +293,9 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
 			r.Provider = provider
 			continue
 		}
-		// FIXME: r.Addr().DefaultProvider() will be refactored to return a string
-		r.Provider = r.Addr().DefaultProvider()
+		// FIXME: this will replaced with NewDefaultProvider when provider
+		// source is fully implemented.
+		r.Provider = addrs.NewLegacyProvider(r.Addr().ImpliedProvider())
 	}
 
 	for _, r := range file.DataResources {
@@ -322,8 +323,9 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
 			r.Provider = provider
 			continue
 		}
-		// FIXME: r.Addr().DefaultProvider() will be refactored to return a string
-		r.Provider = r.Addr().DefaultProvider()
+		// FIXME: this will replaced with NewDefaultProvider when provider
+		// source is fully implemented.
+		r.Provider = addrs.NewLegacyProvider(r.Addr().ImpliedProvider())
 	}
 
 	return diags

configs/module_test.go

@@ -57,6 +57,14 @@ func TestNewModule_resource_providers(t *testing.T) {
 		)
 	}
 
+	// a data source
+	if !cfg.Module.DataResources["data.test_resource.explicit"].Provider.Equals(wantFoo) {
+		t.Fatalf("wrong provider for \"module.child.test_instance.explicit\"\ngot:  %s\nwant: %s",
+			cfg.Module.ManagedResources["test_instance.explicit"].Provider,
+			wantBar,
+		)
+	}
+
 	// child module
 	cm := cfg.Children["child"].Module
 	if !cm.ManagedResources["test_instance.explicit"].Provider.Equals(wantBar) {
@@ -84,5 +92,4 @@ func TestProviderForLocalConfig(t *testing.T) {
 	if !got.Equals(want) {
 		t.Fatalf("wrong result! got %#v, want %#v\n", got, want)
 	}
-
 }

configs/resource.go

@@ -61,22 +61,13 @@ func (r *Resource) Addr() addrs.Resource {
 	}
 }
 
-// ProviderConfigAddr returns the address for the provider configuration
-// that should be used for this resource. This function implements the
-// default behavior of extracting the type from the resource type name if
-// an explicit "provider" argument was not provided.
+// ProviderConfigAddr returns the address for the provider configuration that
+// should be used for this resource. This function returns a default provider
+// config addr if an explicit "provider" argument was not provided.
 func (r *Resource) ProviderConfigAddr() addrs.LocalProviderConfig {
 	if r.ProviderConfigRef == nil {
-		// TODO: This will become incorrect once we move away from legacy
-		// provider addresses, and we'll need to refactor here so that
-		// this lookup is on the Module type rather than the Resource
-		// type and can thus look at the local-to-FQN mapping table
-		// to find a suitable local name to use here.
-		fqn := r.Addr().DefaultProvider()
 		return addrs.LocalProviderConfig{
-			// This will panic once non-legacy addresses are in play.
-			// See the TODO comment above ^^
-			LocalName: fqn.LegacyString(),
+			LocalName: r.Provider.Type,
 		}
 	}
 

configs/testdata/valid-modules/nested-providers-fqns/main.tf

@@ -16,6 +16,10 @@ resource "test_instance" "explicit" {
   provider = foo-test
 }
 
+data "test_resource" "explicit" {
+  provider = foo-test
+}
+
 resource "test_instance" "implicit" {
   // since the provider type name "test" does not match an entry in
   // required_providers, the default provider "test" should be used

states/statefile/version3_upgrade.go

@@ -143,7 +143,7 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) {
 					} else {
 						providerAddr = addrs.AbsProviderConfig{
 							Module:   moduleAddr.Module(),
-							Provider: resAddr.DefaultProvider(),
+							Provider: addrs.NewLegacyProvider(resAddr.ImpliedProvider()),
 						}
 					}
 				}

states/statemgr/testing.go

@@ -151,7 +151,7 @@ func TestFullInitialState() *states.State {
 		Name: "foo",
 	}
 	providerAddr := addrs.AbsProviderConfig{
-		Provider: rAddr.DefaultProvider(),
+		Provider: addrs.NewLegacyProvider(rAddr.ImpliedProvider()),
 		Module:   addrs.RootModule,
 	}
 	childMod.SetResourceMeta(rAddr, states.EachList, providerAddr)

terraform/eval_state_upgrade.go

@@ -27,7 +27,7 @@ func UpgradeResourceState(addr addrs.AbsResourceInstance, provider providers.Int
 	stateIsFlatmap := len(src.AttrsJSON) == 0
 
 	// TODO: This should eventually use a proper FQN.
-	providerType := addr.Resource.Resource.DefaultProvider().LegacyString()
+	providerType := addr.Resource.Resource.ImpliedProvider()
 	if src.SchemaVersion > currentVersion {
 		log.Printf("[TRACE] UpgradeResourceState: can't downgrade state for %s from version %d to %d", addr, src.SchemaVersion, currentVersion)
 		var diags tfdiags.Diagnostics

terraform/node_resource_abstract.go

@@ -312,13 +312,20 @@ func (n *NodeAbstractResource) ProvidedBy() (addrs.ProviderConfig, bool) {
 		}, false
 	}
 
-	// No provider configuration found
-	return nil, false
+	// No provider configuration found; return a default address
+	return addrs.AbsProviderConfig{
+		Provider: n.Provider(),
+		Module:   n.ModulePath(),
+	}, false
 }
 
 // GraphNodeProviderConsumer
-func (n *NodeAbstractResource) ImpliedProvider() addrs.Provider {
-	return n.Addr.Resource.DefaultProvider()
+func (n *NodeAbstractResource) Provider() addrs.Provider {
+	if n.Config != nil {
+		return n.Config.Provider
+	}
+	// FIXME: this will be a default provider
+	return addrs.NewLegacyProvider(n.Addr.Resource.ImpliedProvider())
 }
 
 // GraphNodeProviderConsumer
@@ -340,13 +347,20 @@ func (n *NodeAbstractResourceInstance) ProvidedBy() (addrs.ProviderConfig, bool)
 		return n.ResourceState.ProviderConfig, true
 	}
 
-	// No provider configuration found
-	return nil, false
+	// No provider configuration found; return a default address
+	return addrs.AbsProviderConfig{
+		Provider: n.Provider(),
+		Module:   n.ModulePath(),
+	}, false
 }
 
 // GraphNodeProviderConsumer
-func (n *NodeAbstractResourceInstance) ImpliedProvider() addrs.Provider {
-	return n.Addr.Resource.DefaultProvider()
+func (n *NodeAbstractResourceInstance) Provider() addrs.Provider {
+	if n.Config != nil {
+		return n.Config.Provider
+	}
+	// FIXME: this will be a default provider
+	return addrs.NewLegacyProvider(n.Addr.Resource.ImpliedProvider())
 }
 
 // GraphNodeProvisionerConsumer

terraform/transform_attach_schema.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"log"
 
-	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/configs/configschema"
 	"github.com/hashicorp/terraform/dag"
@@ -61,29 +60,7 @@ func (t *AttachSchemaTransformer) Transform(g *Graph) error {
 			addr := tv.ResourceAddr()
 			mode := addr.Resource.Mode
 			typeName := addr.Resource.Type
-			providerAddr, _ := tv.ProvidedBy()
-
-			var providerFqn addrs.Provider
-			switch p := providerAddr.(type) {
-			case addrs.LocalProviderConfig:
-				if t.Config == nil {
-					providerFqn = addrs.NewLegacyProvider(p.LocalName)
-				} else {
-					modConfig := t.Config.Descendent(tv.ModulePath())
-					if modConfig == nil {
-						providerFqn = addrs.NewLegacyProvider(p.LocalName)
-					} else {
-						providerFqn = modConfig.Module.ProviderForLocalConfig(addrs.LocalProviderConfig{LocalName: p.LocalName})
-					}
-				}
-			case addrs.AbsProviderConfig:
-				providerFqn = p.Provider
-			case nil:
-				providerFqn = tv.ImpliedProvider()
-			default:
-				// This should never happen; the case statements are meant to be exhaustive
-				panic(fmt.Sprintf("%s: provider for %s couldn't be determined", dag.VertexName(v), addr))
-			}
+			providerFqn := tv.Provider()
 
 			schema, version := t.Schemas.ResourceTypeConfig(providerFqn, mode, typeName)
 			if schema == nil {

terraform/transform_import_state.go

@@ -21,7 +21,7 @@ func (t *ImportStateTransformer) Transform(g *Graph) error {
 		// may not specify implied provider addresses.
 		providerAddr := target.ProviderAddr
 		if providerAddr.Provider.Type == "" {
-			defaultFQN := target.Addr.Resource.Resource.DefaultProvider()
+			defaultFQN := addrs.NewLegacyProvider(target.Addr.Resource.Resource.ImpliedProvider())
 			providerAddr = addrs.AbsProviderConfig{
 				Provider: defaultFQN,
 				Module:   target.Addr.Module.Module(),
@@ -69,7 +69,7 @@ func (n *graphNodeImportState) ProvidedBy() (addrs.ProviderConfig, bool) {
 }
 
 // GraphNodeProviderConsumer
-func (n *graphNodeImportState) ImpliedProvider() addrs.Provider {
+func (n *graphNodeImportState) Provider() addrs.Provider {
 	// We assume that n.ProviderAddr has been properly populated here.
 	// It's the responsibility of the code creating a graphNodeImportState
 	// to populate this, possibly by calling DefaultProviderConfig() on the

terraform/transform_provider.go

@@ -68,24 +68,23 @@ type GraphNodeProviderConsumer interface {
 	// refers to, if available. The following value types may be returned:
 	//
 	// * addrs.LocalProviderConfig: the provider was set in the resource config
-	// * addrs.AbsProviderConfig: the provider configuration was taken from the
-	//   instance state.
-	// * nil: provider was not set in config or state. It is the caller's
-	//   responsibility to determine the implied default provider (see ImpliedProvider())
+	// * addrs.AbsProviderConfig + exact true: the provider configuration was
+	//   taken from the instance state.
+	// * addrs.AbsProviderConfig + exact false: no config or state; the returned
+	//   value is a default provider configuration address for the resource's
+	//   Provider
 	ProvidedBy() (addr addrs.ProviderConfig, exact bool)
 
-	// ImpliedProvider returns the provider FQN implied by the resource type
-	// name (for eg the "null" in "null_resource"). This should be used when
-	// ProvidedBy() returns nil.
-	ImpliedProvider() (addrs addrs.Provider)
+	// Provider() returns the Provider FQN for the node.
+	Provider() (provider addrs.Provider)
 
 	// Set the resolved provider address for this resource.
 	SetProvider(addrs.AbsProviderConfig)
 }
 
-// ProviderTransformer is a GraphTransformer that maps resources to
-// providers within the graph. This will error if there are any resources
-// that don't map to proper resources.
+// ProviderTransformer is a GraphTransformer that maps resources to providers
+// within the graph. This will error if there are any resources that don't map
+// to proper resources.
 type ProviderTransformer struct {
 	Config *configs.Config
 }
@@ -139,29 +138,17 @@ func (t *ProviderTransformer) Transform(g *Graph) error {
 			case addrs.LocalProviderConfig:
 				// ProvidedBy() return a LocalProviderConfig when the resource
 				// contains a `provider` attribute
+				absPc.Provider = pv.Provider()
 				modPath := pv.ModulePath()
 				if t.Config == nil {
-					absPc.Provider = addrs.NewLegacyProvider(p.LocalName)
 					absPc.Module = modPath
 					absPc.Alias = p.Alias
 					break
 				}
 
-				modConfig := t.Config.Descendent(modPath)
-				if modConfig == nil {
-					absPc.Provider = addrs.NewLegacyProvider(p.LocalName)
-				} else {
-					absPc.Provider = modConfig.Module.ProviderForLocalConfig(p)
-				}
 				absPc.Module = modPath
 				absPc.Alias = p.Alias
 
-			case nil:
-				// No provider found in config or state; fall back to implied default provider.
-				absPc.Provider = pv.ImpliedProvider()
-				absPc.Module = pv.ModulePath()
-				log.Printf("[TRACE] ProviderTransformer: %s is provided by %s or inherited equivalent", dag.VertexName(v), absPc)
-
 			default:
 				// This should never happen; the case statements are meant to be exhaustive
 				panic(fmt.Sprintf("%s: provider for %s couldn't be determined", dag.VertexName(v), absPc))
@@ -351,36 +338,8 @@ func (t *MissingProviderTransformer) Transform(g *Graph) error {
 		}
 
 		// For our work here we actually care only about the provider type and
-		// we plan to place all default providers in the root module, and so
-		// it's safe for us to rely on ProvidedBy here rather than waiting for
-		// the later proper resolution of provider inheritance done by
-		// ProviderTransformer.
-		providerAddr, _ := pv.ProvidedBy()
-		var providerFqn addrs.Provider
-		switch p := providerAddr.(type) {
-		case addrs.LocalProviderConfig:
-			if p.Alias != "" {
-				// We do not create default aliased configurations.
-				log.Println("[TRACE] MissingProviderTransformer: skipping implication of aliased config", p)
-				continue
-			}
-			modConfig := t.Config.Descendent(pv.ModulePath())
-			if modConfig == nil {
-				providerFqn = addrs.NewLegacyProvider(p.LocalName)
-			} else {
-				providerFqn = modConfig.Module.ProviderForLocalConfig(p)
-			}
-
-		case addrs.AbsProviderConfig:
-			providerFqn = p.Provider
-
-		case nil:
-			providerFqn = pv.ImpliedProvider()
-
-		default:
-			// This should never happen, the case statements are exhaustive
-			panic(fmt.Sprintf("%s: provider for %s couldn't be determined", dag.VertexName(v), p))
-		}
+		// we plan to place all default providers in the root module.
+		providerFqn := pv.Provider()
 
 		// We're going to create an implicit _default_ configuration for the
 		// referenced provider type in the _root_ module, ignoring all other