From ef19fb6203cbc5733fa7c19d4c3334f38a9accc6 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Mon, 16 Mar 2020 14:36:16 -0400 Subject: [PATCH] configs: attach provider fqn to Resource (#24382) * configs: attach provider fqn to Resource --- configs/config_test.go | 4 +- configs/module.go | 34 +++++++++++++- configs/module_merge.go | 11 ++++- configs/module_merge_test.go | 31 +++++++++++++ configs/module_test.go | 44 +++++++++++++++++++ configs/resource.go | 1 + .../nested-providers-fqns/child/main.tf | 10 +++++ .../nested-providers-fqns/main.tf | 9 ++++ .../override-resource-provider/a_override.tf | 3 ++ .../override-resource-provider/base.tf | 17 +++++++ 10 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 configs/testdata/valid-modules/override-resource-provider/a_override.tf create mode 100644 configs/testdata/valid-modules/override-resource-provider/base.tf diff --git a/configs/config_test.go b/configs/config_test.go index 5d0f1407b..ec85208cd 100644 --- a/configs/config_test.go +++ b/configs/config_test.go @@ -33,7 +33,7 @@ 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()) @@ -41,6 +41,8 @@ func TestConfigProviderTypes_nested(t *testing.T) { 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 index b6c50b75c..1accbf933 100644 --- 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 index ea1cecb99..8f8454482 100644 --- 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 index 4575339d3..d4cb3f5a7 100644 --- 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 index 18a2a18d6..51059c896 100644 --- 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 index ce7a41f87..cd3cc9689 100644 --- 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 index da32291d0..1973912d5 100644 --- 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 index 30f12137d..2a3bdb30a 100644 --- 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 new file mode 100644 index 000000000..9a7c3f9e3 --- /dev/null +++ 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 new file mode 100644 index 000000000..0ea12dde5 --- /dev/null +++ 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" {}