configs: Fix for resources with implied providers

Previously, resources without explicit provider configuration (i.e. a
`provider =` attribute) would be assigned a default provider based upon
the resource type. For example, a resource `foo_bar` would be assigned
provider `hashicorp/foo`.

This behaviour did not work well with community or partner providers,
with sources configured in `terraform.required_providers` blocks. With
the following configuration:

    terraform {
      required_providers {
        foo = {
          source = "acme/foo"
        }
      }
    }

    resource foo_bar "a" { }

the resource would be configured with the `hashicorp/foo` provider.

This commit fixes this implied provider behaviour. First we look for a
provider with local name matching the resource type in the module's
required providers map. If one is found, this provider is assigned to
the resource. Otherwise, we still fall back to a default provider.

configs/module.go

@@ -287,14 +287,10 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
 
 		// set the provider FQN for the resource
 		if r.ProviderConfigRef != nil {
-			if existing, exists := m.ProviderRequirements.RequiredProviders[r.ProviderConfigAddr().LocalName]; exists {
+			r.Provider = m.ProviderForLocalConfig(r.ProviderConfigAddr())
-				r.Provider = existing.Type
+		} else {
-			} else {
+			r.Provider = m.ImpliedProviderForUnqualifiedType(r.Addr().ImpliedProvider())
-				r.Provider = addrs.ImpliedProviderForUnqualifiedType(r.ProviderConfigAddr().LocalName)
-			}
-			continue
 		}
-		r.Provider = addrs.ImpliedProviderForUnqualifiedType(r.Addr().ImpliedProvider())
 	}
 
 	for _, r := range file.DataResources {
@@ -312,14 +308,10 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
 
 		// set the provider FQN for the resource
 		if r.ProviderConfigRef != nil {
-			if existing, exists := m.ProviderRequirements.RequiredProviders[r.ProviderConfigAddr().LocalName]; exists {
+			r.Provider = m.ProviderForLocalConfig(r.ProviderConfigAddr())
-				r.Provider = existing.Type
+		} else {
-			} else {
+			r.Provider = m.ImpliedProviderForUnqualifiedType(r.Addr().ImpliedProvider())
-				r.Provider = addrs.ImpliedProviderForUnqualifiedType(r.ProviderConfigAddr().LocalName)
-			}
-			continue
 		}
-		r.Provider = addrs.ImpliedProviderForUnqualifiedType(r.Addr().ImpliedProvider())
 	}
 
 	return diags
@@ -505,10 +497,22 @@ func (m *Module) LocalNameForProvider(p addrs.Provider) string {
 	}
 }
 
-// ProviderForLocalConfig returns the provider FQN for a given LocalProviderConfig
+// ProviderForLocalConfig returns the provider FQN for a given
+// LocalProviderConfig, based on its local name.
 func (m *Module) ProviderForLocalConfig(pc addrs.LocalProviderConfig) addrs.Provider {
-	if provider, exists := m.ProviderRequirements.RequiredProviders[pc.LocalName]; exists {
+	return m.ImpliedProviderForUnqualifiedType(pc.LocalName)
+}
+
+// ImpliedProviderForUnqualifiedType returns the provider FQN for a given type,
+// first by looking up the type in the provider requirements map, and falling
+// back to an implied default provider.
+//
+// The intended behaviour is that configuring a provider with local name "foo"
+// in a required_providers block will result in resources with type "foo" using
+// that provider.
+func (m *Module) ImpliedProviderForUnqualifiedType(pType string) addrs.Provider {
+	if provider, exists := m.ProviderRequirements.RequiredProviders[pType]; exists {
 		return provider.Type
 	}
-	return addrs.ImpliedProviderForUnqualifiedType(pc.LocalName)
+	return addrs.ImpliedProviderForUnqualifiedType(pType)
 }

configs/module_test.go

@@ -203,3 +203,109 @@ func TestModule_required_provider_overrides(t *testing.T) {
 		)
 	}
 }
+
+// Resources without explicit provider configuration are assigned a provider
+// implied based on the resource type. For example, this resource:
+//
+//  resource foo_instance "test" { }
+//
+// is assigned a provider with type "foo".
+//
+// To find the correct provider, we first look in the module's provider
+// requirements map for a local name matching the resource type, and fall back
+// to a default provider if none is found. This applies to both managed and
+// data resources.
+func TestModule_implied_provider(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/implied-providers")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	// The three providers used in the config resources
+	foo := addrs.NewProvider("registry.acme.corp", "acme", "foo")
+	whatever := addrs.NewProvider(addrs.DefaultRegistryHost, "acme", "something")
+	bar := addrs.NewDefaultProvider("bar")
+
+	// Verify that the registry.acme.corp/acme/foo provider is defined in the
+	// module provider requirements with local name "foo"
+	req, exists := mod.ProviderRequirements.RequiredProviders["foo"]
+	if !exists {
+		t.Fatal("no provider requirements found for \"foo\"")
+	}
+	if req.Type != foo {
+		t.Errorf("wrong provider addr for \"foo\"\ngot:  %s\nwant: %s",
+			req.Type, foo,
+		)
+	}
+
+	// Verify that the acme/something provider is defined in the
+	// module provider requirements with local name "whatever"
+	req, exists = mod.ProviderRequirements.RequiredProviders["whatever"]
+	if !exists {
+		t.Fatal("no provider requirements found for \"foo\"")
+	}
+	if req.Type != whatever {
+		t.Errorf("wrong provider addr for \"whatever\"\ngot:  %s\nwant: %s",
+			req.Type, whatever,
+		)
+	}
+
+	// Check that resources are assigned the correct providers: foo_* resources
+	// should have the custom foo provider, bar_* resources the default bar
+	// provider.
+	tests := []struct {
+		Address  string
+		Provider addrs.Provider
+	}{
+		{"foo_resource.a", foo},
+		{"data.foo_resource.b", foo},
+		{"bar_resource.c", bar},
+		{"data.bar_resource.d", bar},
+		{"whatever_resource.e", whatever},
+		{"data.whatever_resource.f", whatever},
+	}
+	for _, test := range tests {
+		resources := mod.ManagedResources
+		if strings.HasPrefix(test.Address, "data.") {
+			resources = mod.DataResources
+		}
+		resource, exists := resources[test.Address]
+		if !exists {
+			t.Errorf("could not find resource %q in %#v", test.Address, resources)
+			continue
+		}
+		if got := resource.Provider; !got.Equals(test.Provider) {
+			t.Errorf("wrong provider addr for %q\ngot:  %s\nwant: %s",
+				test.Address, got, test.Provider,
+			)
+		}
+	}
+}
+
+func TestImpliedProviderForUnqualifiedType(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/implied-providers")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	foo := addrs.NewProvider("registry.acme.corp", "acme", "foo")
+	whatever := addrs.NewProvider(addrs.DefaultRegistryHost, "acme", "something")
+	bar := addrs.NewDefaultProvider("bar")
+	tf := addrs.NewBuiltInProvider("terraform")
+
+	tests := []struct {
+		Type     string
+		Provider addrs.Provider
+	}{
+		{"foo", foo},
+		{"whatever", whatever},
+		{"bar", bar},
+		{"terraform", tf},
+	}
+	for _, test := range tests {
+		got := mod.ImpliedProviderForUnqualifiedType(test.Type)
+		if !got.Equals(test.Provider) {
+			t.Errorf("wrong result for %q: got %#v, want %#v\n", test.Type, got, test.Provider)
+		}
+	}
+}

configs/testdata/valid-modules/implied-providers/providers.tf

@@ -0,0 +1,21 @@
+terraform {
+  required_providers {
+    // This is an expected "real world" example of a community provider, which
+    // has resources named "foo_*" and will likely be used in configurations
+    // with the local name of "foo".
+    foo = {
+      source = "registry.acme.corp/acme/foo"
+    }
+
+    // However, implied provider lookups are based on local name, not provider
+    // type, and this example clarifies that. Only resources with addresses
+    // starting "whatever_" will be assigned this provider implicitly.
+    //
+    // This is _not_ a recommended usage pattern. The best practice is for
+    // local name and type to be the same, and only use a different local name
+    // if there are provider type collisions.
+    whatever = {
+      source = "acme/something"
+    }
+  }
+}

configs/testdata/valid-modules/implied-providers/resources.tf

@@ -0,0 +1,12 @@
+// These resources map to the configured "foo" provider"
+resource foo_resource "a" {}
+data foo_resource "b" {}
+
+// These resources map to a default "hashicorp/bar" provider
+resource bar_resource "c" {}
+data bar_resource "d" {}
+
+// These resources map to the configured "whatever" provider, which has FQN
+// "acme/something".
+resource whatever_resource "e" {}
+data whatever_resource "f" {}