configs: attach provider fqn to Resource (hashicorp#24382)

* configs: attach provider fqn to Resource
aliscott · Mar 16, 2020 · ef19fb6 · ef19fb6
1 parent 42f7bef
commit ef19fb6
Show file tree

Hide file tree

Showing 10 changed files with 160 additions and 4 deletions.
diff --git a/configs/config_test.go b/configs/config_test.go
@@ -33,14 +33,16 @@ func TestConfigProviderTypes_nested(t *testing.T) {
 		t.Fatalf("wrong result!\ngot: %#v\nwant: nil\n", got)
 	}
 
-	// config with two provider sources
+	// config with two provider sources, and one implicit (default) provider
 	cfg, diags := testNestedModuleConfigFromDir(t, "testdata/valid-modules/nested-providers-fqns")
 	if diags.HasErrors() {
 		t.Fatal(diags.Error())
 	}
 
 	got = cfg.ProviderTypes()
 	want := []addrs.Provider{
+		// FIXME: this will be updated to NewDefaultProvider as we remove `Legacy*`
+		addrs.NewLegacyProvider("test"),
 		addrs.NewProvider(addrs.DefaultRegistryHost, "bar", "test"),
 		addrs.NewProvider(addrs.DefaultRegistryHost, "foo", "test"),
 	}

diff --git a/configs/module.go b/configs/module.go
@@ -280,6 +280,21 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
 			continue
 		}
 		m.ManagedResources[key] = r
+
+		// set the provider FQN for the resource
+		var provider addrs.Provider
+		if r.ProviderConfigRef != nil {
+			if existing, exists := m.ProviderRequirements[r.ProviderConfigAddr().LocalName]; exists {
+				provider = existing.Type
+			} else {
+				// FIXME: This will be a NewDefaultProvider
+				provider = addrs.NewLegacyProvider(r.ProviderConfigAddr().LocalName)
+			}
+			r.Provider = provider
+			continue
+		}
+		// FIXME: r.Addr().DefaultProvider() will be refactored to return a string
+		r.Provider = r.Addr().DefaultProvider()
 	}
 
 	for _, r := range file.DataResources {
@@ -294,6 +309,21 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
 			continue
 		}
 		m.DataResources[key] = r
+
+		// set the provider FQN for the resource
+		var provider addrs.Provider
+		if r.ProviderConfigRef != nil {
+			if existing, exists := m.ProviderRequirements[r.ProviderConfigAddr().LocalName]; exists {
+				provider = existing.Type
+			} else {
+				// FIXME: This will be a NewDefaultProvider
+				provider = addrs.NewLegacyProvider(r.ProviderConfigAddr().LocalName)
+			}
+			r.Provider = provider
+			continue
+		}
+		// FIXME: r.Addr().DefaultProvider() will be refactored to return a string
+		r.Provider = r.Addr().DefaultProvider()
 	}
 
 	return diags
@@ -436,7 +466,7 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics {
 			})
 			continue
 		}
-		mergeDiags := existing.merge(r)
+		mergeDiags := existing.merge(r, m.ProviderRequirements)
 		diags = append(diags, mergeDiags...)
 	}
 
@@ -452,7 +482,7 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics {
 			})
 			continue
 		}
-		mergeDiags := existing.merge(r)
+		mergeDiags := existing.merge(r, m.ProviderRequirements)
 		diags = append(diags, mergeDiags...)
 	}
 

diff --git a/configs/module_merge.go b/configs/module_merge.go
@@ -197,7 +197,7 @@ func (mc *ModuleCall) merge(omc *ModuleCall) hcl.Diagnostics {
 	return diags
 }
 
-func (r *Resource) merge(or *Resource) hcl.Diagnostics {
+func (r *Resource) merge(or *Resource, prs map[string]ProviderRequirements) hcl.Diagnostics {
 	var diags hcl.Diagnostics
 
 	if r.Mode != or.Mode {
@@ -212,9 +212,18 @@ func (r *Resource) merge(or *Resource) hcl.Diagnostics {
 	if or.ForEach != nil {
 		r.ForEach = or.ForEach
 	}
+
 	if or.ProviderConfigRef != nil {
 		r.ProviderConfigRef = or.ProviderConfigRef
+		if existing, exists := prs[or.ProviderConfigRef.Name]; exists {
+			r.Provider = existing.Type
+		} else {
+			r.Provider = addrs.NewLegacyProvider(r.ProviderConfigRef.Name)
+		}
 	}
+
+	// Provider FQN is set by Terraform during Merge
+
 	if r.Mode == addrs.ManagedResourceMode {
 		// or.Managed is always non-nil for managed resource mode
 

diff --git a/configs/module_merge_test.go b/configs/module_merge_test.go
@@ -202,6 +202,37 @@ func TestModuleOverrideDynamic(t *testing.T) {
 	})
 }
 
+func TestModuleOverrideResourceFQNs(t *testing.T) {
+	mod, diags := testModuleFromDir("testdata/valid-modules/override-resource-provider")
+	assertNoDiagnostics(t, diags)
+
+	got := mod.ManagedResources["test_instance.explicit"]
+	wantProvider := addrs.NewProvider(addrs.DefaultRegistryHost, "bar", "test")
+	wantProviderCfg := &ProviderConfigRef{
+		Name: "bar-test",
+		NameRange: hcl.Range{
+			Filename: "testdata/valid-modules/override-resource-provider/a_override.tf",
+			Start:    hcl.Pos{Line: 2, Column: 14, Byte: 51},
+			End:      hcl.Pos{Line: 2, Column: 22, Byte: 59},
+		},
+	}
+
+	if !got.Provider.Equals(wantProvider) {
+		t.Fatalf("wrong provider %s, want %s", got.Provider, wantProvider)
+	}
+	assertResultDeepEqual(t, got.ProviderConfigRef, wantProviderCfg)
+
+	// now verify that a resource with no provider config falls back to default
+	got = mod.ManagedResources["test_instance.default"]
+	wantProvider = addrs.NewLegacyProvider("test")
+	if !got.Provider.Equals(wantProvider) {
+		t.Fatalf("wrong provider %s, want %s", got.Provider, wantProvider)
+	}
+	if got.ProviderConfigRef != nil {
+		t.Fatalf("wrong result: found provider config ref %s, expected nil", got.ProviderConfigRef)
+	}
+}
+
 func TestMergeProviderVersionConstraints(t *testing.T) {
 	v1, _ := version.NewConstraint("1.0.0")
 	vc1 := VersionConstraint{

diff --git a/configs/module_test.go b/configs/module_test.go
@@ -29,6 +29,50 @@ func TestNewModule_provider_local_name(t *testing.T) {
 	}
 }
 
+// This test validates the provider FQNs set in each Resource
+func TestNewModule_resource_providers(t *testing.T) {
+	cfg, diags := testNestedModuleConfigFromDir(t, "testdata/valid-modules/nested-providers-fqns")
+	if diags.HasErrors() {
+		t.Fatal(diags.Error())
+	}
+
+	// both the root and child module have two resources, one which should use
+	// the default implied provider and one explicitly using a provider set in
+	// required_providers
+	wantImplicit := addrs.NewLegacyProvider("test")
+	wantFoo := addrs.NewProvider(addrs.DefaultRegistryHost, "foo", "test")
+	wantBar := addrs.NewProvider(addrs.DefaultRegistryHost, "bar", "test")
+
+	// root module
+	if !cfg.Module.ManagedResources["test_instance.explicit"].Provider.Equals(wantFoo) {
+		t.Fatalf("wrong provider for \"test_instance.explicit\"\ngot:  %s\nwant: %s",
+			cfg.Module.ManagedResources["test_instance.explicit"].Provider,
+			wantFoo,
+		)
+	}
+	if !cfg.Module.ManagedResources["test_instance.implicit"].Provider.Equals(wantImplicit) {
+		t.Fatalf("wrong provider for \"test_instance.implicit\"\ngot:  %s\nwant: %s",
+			cfg.Module.ManagedResources["test_instance.implicit"].Provider,
+			wantImplicit,
+		)
+	}
+
+	// child module
+	cm := cfg.Children["child"].Module
+	if !cm.ManagedResources["test_instance.explicit"].Provider.Equals(wantBar) {
+		t.Fatalf("wrong provider for \"module.child.test_instance.explicit\"\ngot:  %s\nwant: %s",
+			cfg.Module.ManagedResources["test_instance.explicit"].Provider,
+			wantBar,
+		)
+	}
+	if !cm.ManagedResources["test_instance.implicit"].Provider.Equals(wantImplicit) {
+		t.Fatalf("wrong provider for \"module.child.test_instance.implicit\"\ngot:  %s\nwant: %s",
+			cfg.Module.ManagedResources["test_instance.implicit"].Provider,
+			wantImplicit,
+		)
+	}
+}
+
 func TestProviderForLocalConfig(t *testing.T) {
 	mod, diags := testModuleFromDir("testdata/providers-explicit-fqn")
 	if diags.HasErrors() {

diff --git a/configs/resource.go b/configs/resource.go
@@ -20,6 +20,7 @@ type Resource struct {
 	ForEach hcl.Expression
 
 	ProviderConfigRef *ProviderConfigRef
+	Provider          addrs.Provider
 
 	DependsOn []hcl.Traversal
 

diff --git a/configs/testdata/valid-modules/nested-providers-fqns/child/main.tf b/configs/testdata/valid-modules/nested-providers-fqns/child/main.tf
@@ -7,3 +7,13 @@ terraform {
 }
 
 provider "bar-test" {}
+
+resource "test_instance" "explicit" {
+  // explicitly setting provider bar-test
+  provider = bar-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
+}
diff --git a/configs/testdata/valid-modules/nested-providers-fqns/main.tf b/configs/testdata/valid-modules/nested-providers-fqns/main.tf
@@ -11,3 +11,12 @@ provider "foo-test" {}
 module "child" {
   source = "./child"
 }
+
+resource "test_instance" "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
+}
diff --git a/configs/testdata/valid-modules/override-resource-provider/a_override.tf b/configs/testdata/valid-modules/override-resource-provider/a_override.tf
@@ -0,0 +1,3 @@
+resource "test_instance" "explicit" {
+  provider = bar-test
+}
diff --git a/configs/testdata/valid-modules/override-resource-provider/base.tf b/configs/testdata/valid-modules/override-resource-provider/base.tf
@@ -0,0 +1,17 @@
+terraform {
+  required_providers {
+    foo-test = {
+      source = "foo/test"
+    }
+    bar-test = {
+      source = "bar/test"
+    }
+  }
+}
+
+resource "test_instance" "explicit" {
+  provider = foo-test
+}
+
+// the provider for this resource should default to "hashicorp/test"
+resource "test_instance" "default" {}