configs: Fix nested provider requirements bug

In a recent PR, we changed the provider requirements code to permit
per-module requirements gathering, to enhance the provider command
output. This had an incorrect implementation of recursive requirements
gathering for the normal case, which resulted in only depth-1 modules
being inspected.

This commit fixes the broken recursion and adds a grandchild module to
the unit tests as test coverage. This also demanded fixing the
testNestedModuleConfigFromDir helper function to cope with nested
modules in test configs.

configs/config.go

@@ -185,12 +185,7 @@ func (c *Config) DescendentForInstance(path addrs.ModuleInstance) *Config {
 // may be incomplete.
 func (c *Config) ProviderRequirements() (getproviders.Requirements, hcl.Diagnostics) {
 	reqs := make(getproviders.Requirements)
-	diags := c.addProviderRequirements(reqs)
+	diags := c.addProviderRequirements(reqs, true)
-
-	for _, childConfig := range c.Children {
-		moreDiags := childConfig.addProviderRequirements(reqs)
-		diags = append(diags, moreDiags...)
-	}
 
 	return reqs, diags
 }
@@ -203,7 +198,7 @@ func (c *Config) ProviderRequirements() (getproviders.Requirements, hcl.Diagnost
 // may be incomplete.
 func (c *Config) ProviderRequirementsByModule() (*ModuleRequirements, hcl.Diagnostics) {
 	reqs := make(getproviders.Requirements)
-	diags := c.addProviderRequirements(reqs)
+	diags := c.addProviderRequirements(reqs, false)
 
 	children := make(map[string]*ModuleRequirements)
 	for name, child := range c.Children {
@@ -225,9 +220,9 @@ func (c *Config) ProviderRequirementsByModule() (*ModuleRequirements, hcl.Diagno
 
 // addProviderRequirements is the main part of the ProviderRequirements
 // implementation, gradually mutating a shared requirements object to
-// eventually return. This function only adds requirements for the top-level
+// eventually return. If the recurse argument is true, the requirements will
-// module.
+// include all descendant modules; otherwise, only the specified module.
-func (c *Config) addProviderRequirements(reqs getproviders.Requirements) hcl.Diagnostics {
+func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse bool) hcl.Diagnostics {
 	var diags hcl.Diagnostics
 
 	// First we'll deal with the requirements directly in _our_ module...
@@ -309,6 +304,13 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements) hcl.Dia
 		}
 	}
 
+	if recurse {
+		for _, childConfig := range c.Children {
+			moreDiags := childConfig.addProviderRequirements(reqs, true)
+			diags = append(diags, moreDiags...)
+		}
+	}
+
 	return diags
 }
 

configs/config_test.go

@@ -130,6 +130,7 @@ func TestConfigProviderRequirements(t *testing.T) {
 	impliedProvider := addrs.NewDefaultProvider("implied")
 	terraformProvider := addrs.NewBuiltInProvider("terraform")
 	configuredProvider := addrs.NewDefaultProvider("configured")
+	grandchildProvider := addrs.NewDefaultProvider("grandchild")
 
 	got, diags := cfg.ProviderRequirements()
 	assertNoDiagnostics(t, diags)
@@ -142,6 +143,7 @@ func TestConfigProviderRequirements(t *testing.T) {
 		impliedProvider:    nil,
 		happycloudProvider: nil,
 		terraformProvider:  nil,
+		grandchildProvider: nil,
 	}
 
 	if diff := cmp.Diff(want, got); diff != "" {
@@ -166,6 +168,7 @@ func TestConfigProviderRequirementsByModule(t *testing.T) {
 	impliedProvider := addrs.NewDefaultProvider("implied")
 	terraformProvider := addrs.NewBuiltInProvider("terraform")
 	configuredProvider := addrs.NewDefaultProvider("configured")
+	grandchildProvider := addrs.NewDefaultProvider("grandchild")
 
 	got, diags := cfg.ProviderRequirementsByModule()
 	assertNoDiagnostics(t, diags)
@@ -191,9 +194,19 @@ func TestConfigProviderRequirementsByModule(t *testing.T) {
 					nullProvider:       getproviders.MustParseVersionConstraints("= 2.0.1"),
 					happycloudProvider: nil,
 				},
+				Children: map[string]*ModuleRequirements{
+					"nested": {
+						Name:       "nested",
+						SourceAddr: "./grandchild",
+						SourceDir:  "testdata/provider-reqs/child/grandchild",
+						Requirements: getproviders.Requirements{
+							grandchildProvider: nil,
+						},
 						Children: map[string]*ModuleRequirements{},
 					},
 				},
+			},
+		},
 	}
 
 	ignore := cmpopts.IgnoreUnexported(version.Constraint{}, cty.Value{}, hclsyntax.Body{})
@@ -227,6 +240,6 @@ func TestConfigAddProviderRequirements(t *testing.T) {
 	reqs := getproviders.Requirements{
 		addrs.NewDefaultProvider("null"): nil,
 	}
-	diags = cfg.addProviderRequirements(reqs)
+	diags = cfg.addProviderRequirements(reqs, true)
 	assertNoDiagnostics(t, diags)
 }

configs/parser_test.go

@@ -92,10 +92,18 @@ func testNestedModuleConfigFromDir(t *testing.T, path string) (*Config, hcl.Diag
 	cfg, diags := BuildConfig(mod, ModuleWalkerFunc(
 		func(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) {
 			// For the sake of this test we're going to just treat our
-			// SourceAddr as a path relative to our fixture directory.
+			// SourceAddr as a path relative to the calling module.
 			// A "real" implementation of ModuleWalker should accept the
 			// various different source address syntaxes Terraform supports.
-			sourcePath := filepath.Join(path, req.SourceAddr)
+
+			// Build a full path by walking up the module tree, prepending each
+			// source address path until we hit the root
+			paths := []string{req.SourceAddr}
+			for config := req.Parent; config != nil && config.Parent != nil; config = config.Parent {
+				paths = append([]string{config.SourceAddr}, paths...)
+			}
+			paths = append([]string{path}, paths...)
+			sourcePath := filepath.Join(paths...)
 
 			mod, diags := parser.LoadConfigDir(sourcePath)
 			version, _ := version.NewVersion(fmt.Sprintf("1.0.%d", versionI))

configs/testdata/provider-reqs/child/grandchild/provider-reqs-grandchild.tf

@@ -0,0 +1,4 @@
+# There is no provider in required_providers called "grandchild", so this
+# implicitly declares a dependency on "hashicorp/grandchild".
+resource "grandchild_foo" "bar" {
+}

configs/testdata/provider-reqs/child/provider-reqs-child.tf

@@ -9,3 +9,7 @@ terraform {
     }
   }
 }
+
+module "nested" {
+  source = "./grandchild"
+}