Merge pull request #28606 from hashicorp/jbardin/providers-in-modules

Validate passing default providers through modules

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
 }
 

configs/testdata/config-diagnostics/pass-inherited-provider/main.tf

@@ -0,0 +1,7 @@
+provider "test" {
+  value = "ok"
+}
+
+module "mod" {
+  source = "./mod"
+}

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
+  }
+}

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
+}

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

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
+  }
+}

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" {
+}

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

terraform/context_validate_test.go

@@ -2093,3 +2093,71 @@ resource "test_instance" "c" {
 		t.Fatal(diags.ErrWithWarnings())
 	}
 }
+
+func TestContext2Validate_passInheritedProvider(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+terraform {
+  required_providers {
+	test = {
+	  source = "hashicorp/test"
+	}
+  }
+}
+
+module "first" {
+  source = "./first"
+  providers = {
+    test = test
+  }
+}
+`,
+
+		// This module does not define a config for the test provider, but we
+		// should be able to pass whatever the implied config is to a child
+		// module.
+		"first/main.tf": `
+terraform {
+  required_providers {
+    test = {
+	  source = "hashicorp/test"
+    }
+  }
+}
+
+module "second" {
+  source = "./second"
+  providers = {
+	test.alias = test
+  }
+}`,
+
+		"first/second/main.tf": `
+terraform {
+  required_providers {
+    test = {
+	  source = "hashicorp/test"
+      configuration_aliases = [test.alias]
+    }
+  }
+}
+
+resource "test_object" "t" {
+  provider = test.alias
+}
+`,
+	})
+
+	p := simpleMockProvider()
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	diags := ctx.Validate()
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+}

terraform/transform_provider.go

@@ -534,6 +534,37 @@ func (t *ProviderConfigTransformer) transformSingle(g *Graph, c *configs.Config)
 	mod := c.Module
 	path := c.Path
 
+	// If this is the root module, we can add nodes for required providers that
+	// have no configuration, equivalent to having an empty configuration
+	// block. This will ensure that a provider node exists for modules to
+	// access when passing around configuration and inheritance.
+	if path.IsRoot() && c.Module.ProviderRequirements != nil {
+		for name, p := range c.Module.ProviderRequirements.RequiredProviders {
+			if _, configured := mod.ProviderConfigs[name]; configured {
+				continue
+			}
+
+			addr := addrs.AbsProviderConfig{
+				Provider: p.Type,
+				Module:   path,
+			}
+
+			abstract := &NodeAbstractProvider{
+				Addr: addr,
+			}
+
+			var v dag.Vertex
+			if t.Concrete != nil {
+				v = t.Concrete(abstract)
+			} else {
+				v = abstract
+			}
+
+			g.Add(v)
+			t.providers[addr.String()] = v.(GraphNodeProvider)
+		}
+	}
+
 	// add all providers from the configuration
 	for _, p := range mod.ProviderConfigs {
 		fqn := mod.ProviderForLocalConfig(p.Addr())
@@ -558,13 +589,17 @@ func (t *ProviderConfigTransformer) transformSingle(g *Graph, c *configs.Config)
 		key := addr.String()
 		t.providers[key] = v.(GraphNodeProvider)
 
-		// A provider configuration is "proxyable" if its configuration is
-		// entirely empty. This means it's standing in for a provider
-		// configuration that must be passed in from the parent module.
-		// We decide this by evaluating the config with an empty schema;
-		// if this succeeds, then we know there's nothing in the body.
-		_, diags := p.Config.Content(&hcl.BodySchema{})
-		t.proxiable[key] = !diags.HasErrors()
+		// While deprecated, we still accept empty configuration blocks within
+		// modules as being a possible proxy for passed configuration.
+		if !path.IsRoot() {
+			// A provider configuration is "proxyable" if its configuration is
+			// entirely empty. This means it's standing in for a provider
+			// configuration that must be passed in from the parent module.
+			// We decide this by evaluating the config with an empty schema;
+			// if this succeeds, then we know there's nothing in the body.
+			_, diags := p.Config.Content(&hcl.BodySchema{})
+			t.proxiable[key] = !diags.HasErrors()
+		}
 	}
 
 	// Now replace the provider nodes with proxy nodes if a provider was being
@@ -577,7 +612,7 @@ func (t *ProviderConfigTransformer) addProxyProviders(g *Graph, c *configs.Confi
 	path := c.Path
 
 	// can't add proxies at the root
-	if len(path) == 0 {
+	if path.IsRoot() {
 		return nil
 	}
 
@@ -648,6 +683,7 @@ func (t *ProviderConfigTransformer) addProxyProviders(g *Graph, c *configs.Confi
 		g.Add(proxy)
 		t.providers[fullName] = proxy
 	}
+
 	return nil
 }