From 168296b507892ba9292c9993a78e7b05bdf22c04 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 3 May 2021 17:43:55 -0400 Subject: [PATCH] validate that modules name providers passed to mod Passing a provider into a module requires that it be named within the module. This would previously pass validation, however core would fail to resolve the provider resulting in an unclear "provider not found" error. --- configs/provider_validation.go | 97 +++++++++++++++---- .../pass-inherited-provider/main.tf | 7 ++ .../pass-inherited-provider/mod/main.tf | 17 ++++ .../pass-inherited-provider/mod2/main.tf | 12 +++ .../pass-inherited-provider/warnings | 1 + .../unknown-root-provider/main.tf | 7 ++ .../unknown-root-provider/mod/main.tf | 10 ++ .../unknown-root-provider/warnings | 1 + 8 files changed, 132 insertions(+), 20 deletions(-) create mode 100644 configs/testdata/config-diagnostics/pass-inherited-provider/main.tf create mode 100644 configs/testdata/config-diagnostics/pass-inherited-provider/mod/main.tf create mode 100644 configs/testdata/config-diagnostics/pass-inherited-provider/mod2/main.tf create mode 100644 configs/testdata/config-diagnostics/pass-inherited-provider/warnings create mode 100644 configs/testdata/config-diagnostics/unknown-root-provider/main.tf create mode 100644 configs/testdata/config-diagnostics/unknown-root-provider/mod/main.tf create mode 100644 configs/testdata/config-diagnostics/unknown-root-provider/warnings diff --git a/configs/provider_validation.go b/configs/provider_validation.go index ec8bda2f3..425d1dffb 100644 --- a/configs/provider_validation.go +++ b/configs/provider_validation.go @@ -23,9 +23,11 @@ import ( // noProviderConfig argument is passed down the call stack, indicating that the // module call, or a parent module call, has used a feature that precludes // providers from being configured at all within the module. -func validateProviderConfigs(call *ModuleCall, cfg *Config, noProviderConfig bool) (diags hcl.Diagnostics) { +func validateProviderConfigs(parentCall *ModuleCall, cfg *Config, noProviderConfig bool) (diags hcl.Diagnostics) { + mod := cfg.Module + for name, child := range cfg.Children { - mc := cfg.Module.ModuleCalls[name] + mc := mod.ModuleCalls[name] // if the module call has any of count, for_each or depends_on, // providers are prohibited from being configured in this module, or @@ -34,11 +36,6 @@ func validateProviderConfigs(call *ModuleCall, cfg *Config, noProviderConfig boo diags = append(diags, validateProviderConfigs(mc, child, nope)...) } - // nothing else to do in the root module - if call == nil { - return diags - } - // the set of provider configuration names passed into the module, with the // source range of the provider assignment in the module call. passedIn := map[string]PassedProviderConfig{} @@ -59,13 +56,6 @@ func validateProviderConfigs(call *ModuleCall, cfg *Config, noProviderConfig boo // their provider types. localNames := map[string]addrs.AbsProviderConfig{} - for _, passed := range call.Providers { - name := providerName(passed.InChild.Name, passed.InChild.Alias) - passedIn[name] = passed - } - - mod := cfg.Module - for _, pc := range mod.ProviderConfigs { name := providerName(pc.Name, pc.Alias) // Validate the config against an empty schema to see if it's empty. @@ -95,6 +85,76 @@ func validateProviderConfigs(call *ModuleCall, cfg *Config, noProviderConfig boo } } + // collect providers passed from the parent + if parentCall != nil { + for _, passed := range parentCall.Providers { + name := providerName(passed.InChild.Name, passed.InChild.Alias) + passedIn[name] = passed + } + } + + parentModuleText := "the root module" + moduleText := "the root module" + if !cfg.Path.IsRoot() { + moduleText = cfg.Path.String() + if parent := cfg.Path.Parent(); !parent.IsRoot() { + // module address are prefixed with `module.` + parentModuleText = parent.String() + } + } + + // Verify that any module calls only refer to named providers, and that + // those providers will have a configuration at runtime. This way we can + // direct users where to add the missing configuration, because the runtime + // error is only "missing provider X". + for _, modCall := range mod.ModuleCalls { + for _, passed := range modCall.Providers { + // aliased providers are handled more strictly, and are never + // inherited, so they are validated within modules further down. + // Skip these checks to prevent redundant diagnostics. + if passed.InParent.Alias != "" { + continue + } + + name := passed.InParent.String() + _, confOK := configured[name] + _, localOK := localNames[name] + _, passedOK := passedIn[name] + + // This name was not declared somewhere within in the + // configuration. We ignore empty configs, because they will + // already produce a warning. + if !(confOK || localOK) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("Provider %s is undefined", name), + Detail: fmt.Sprintf("No provider named %s has been declared in %s.\n", name, moduleText) + + fmt.Sprintf("If you wish to refer to the %s provider within the module, add a provider configuration, or an entry in the required_providers block.", name), + Subject: &passed.InParent.NameRange, + }) + continue + } + + // Now we may have named this provider within the module, but + // there won't be a configuration available at runtime if the + // parent module did not pass one in. + if !cfg.Path.IsRoot() && !(confOK || passedOK) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("No configuration passed in for provider %s in %s", name, cfg.Path), + Detail: fmt.Sprintf("Provider %s is referenced within %s, but no configuration has been supplied.\n", name, moduleText) + + fmt.Sprintf("Add a provider named %s to the providers map for %s in %s.", name, cfg.Path, parentModuleText), + Subject: &passed.InParent.NameRange, + }) + } + } + } + + if cfg.Path.IsRoot() { + // nothing else to do in the root module + return diags + } + // there cannot be any configurations if no provider config is allowed if len(configured) > 0 && noProviderConfig { diags = append(diags, &hcl.Diagnostic{ @@ -129,8 +189,9 @@ func validateProviderConfigs(call *ModuleCall, cfg *Config, noProviderConfig boo diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: fmt.Sprintf("No configuration for provider %s", name), - Detail: fmt.Sprintf("Configuration required for %s.", providerAddr), - Subject: &call.DeclRange, + Detail: fmt.Sprintf("Configuration required for %s.\n", providerAddr) + + fmt.Sprintf("Add a provider named %s to the providers map for %s in %s.", name, cfg.Path, parentModuleText), + Subject: &parentCall.DeclRange, }) } @@ -240,10 +301,6 @@ func validateProviderConfigs(call *ModuleCall, cfg *Config, noProviderConfig boo }) } - if diags.HasErrors() { - return diags - } - return diags } diff --git a/configs/testdata/config-diagnostics/pass-inherited-provider/main.tf b/configs/testdata/config-diagnostics/pass-inherited-provider/main.tf new file mode 100644 index 000000000..76456399c --- /dev/null +++ b/configs/testdata/config-diagnostics/pass-inherited-provider/main.tf @@ -0,0 +1,7 @@ +provider "test" { + value = "ok" +} + +module "mod" { + source = "./mod" +} diff --git a/configs/testdata/config-diagnostics/pass-inherited-provider/mod/main.tf b/configs/testdata/config-diagnostics/pass-inherited-provider/mod/main.tf new file mode 100644 index 000000000..e0d142d55 --- /dev/null +++ b/configs/testdata/config-diagnostics/pass-inherited-provider/mod/main.tf @@ -0,0 +1,17 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + } + } +} + +module "mod2" { + source = "./mod2" + + // the test provider is named here, but a config must be supplied from the + // parent module. + providers = { + test.foo = test + } +} diff --git a/configs/testdata/config-diagnostics/pass-inherited-provider/mod2/main.tf b/configs/testdata/config-diagnostics/pass-inherited-provider/mod2/main.tf new file mode 100644 index 000000000..0b7361691 --- /dev/null +++ b/configs/testdata/config-diagnostics/pass-inherited-provider/mod2/main.tf @@ -0,0 +1,12 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + configuration_aliases = [test.foo] + } + } +} + +resource "test_resource" "foo" { + provider = test.foo +} diff --git a/configs/testdata/config-diagnostics/pass-inherited-provider/warnings b/configs/testdata/config-diagnostics/pass-inherited-provider/warnings new file mode 100644 index 000000000..fee53bc4f --- /dev/null +++ b/configs/testdata/config-diagnostics/pass-inherited-provider/warnings @@ -0,0 +1 @@ +pass-inherited-provider/mod/main.tf:15,16-20: No configuration passed in for provider test in module.mod; Provider test is referenced within module.mod, but no configuration has been supplied diff --git a/configs/testdata/config-diagnostics/unknown-root-provider/main.tf b/configs/testdata/config-diagnostics/unknown-root-provider/main.tf new file mode 100644 index 000000000..cf60f915a --- /dev/null +++ b/configs/testdata/config-diagnostics/unknown-root-provider/main.tf @@ -0,0 +1,7 @@ +module "mod" { + source = "./mod" + providers = { + // bar may be required by the module, but the name is not defined here + bar = bar + } +} diff --git a/configs/testdata/config-diagnostics/unknown-root-provider/mod/main.tf b/configs/testdata/config-diagnostics/unknown-root-provider/mod/main.tf new file mode 100644 index 000000000..ea8d0321d --- /dev/null +++ b/configs/testdata/config-diagnostics/unknown-root-provider/mod/main.tf @@ -0,0 +1,10 @@ +terraform { + required_providers { + bar = { + version = "~>1.0.0" + } + } +} + +resource "bar_resource" "x" { +} diff --git a/configs/testdata/config-diagnostics/unknown-root-provider/warnings b/configs/testdata/config-diagnostics/unknown-root-provider/warnings new file mode 100644 index 000000000..766670d93 --- /dev/null +++ b/configs/testdata/config-diagnostics/unknown-root-provider/warnings @@ -0,0 +1 @@ +unknown-root-provider/main.tf:5,11-14: Provider bar is undefined; No provider named bar has been declared in the root module