Merge pull request #24783 from hashicorp/alisdair/implied-provider-lookup-by-localname

configs: Fix for resources with implied providers

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 = existing.Type
-			} else {
-				r.Provider = addrs.ImpliedProviderForUnqualifiedType(r.ProviderConfigAddr().LocalName)
-			}
-			continue
+			r.Provider = m.ProviderForLocalConfig(r.ProviderConfigAddr())
+		} else {
+			r.Provider = m.ImpliedProviderForUnqualifiedType(r.Addr().ImpliedProvider())
 		}
-		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 = existing.Type
-			} else {
-				r.Provider = addrs.ImpliedProviderForUnqualifiedType(r.ProviderConfigAddr().LocalName)
-			}
-			continue
+			r.Provider = m.ProviderForLocalConfig(r.ProviderConfigAddr())
+		} else {
+			r.Provider = m.ImpliedProviderForUnqualifiedType(r.Addr().ImpliedProvider())
 		}
-		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" {}