Disallow provider configuration in expanding modules (#24892)

Validate providers in expanding modules. Expanding modules cannot have provider configurations with non-empty configs, which includes having a version configured. If an empty or alias-only block is passed, the provider must be passed through the providers argument on the module call

configs/configload/loader_load.go

@@ -17,7 +17,7 @@ import (
 // in spite of the errors.
 //
 // LoadConfig performs the basic syntax and uniqueness validations that are
-// required to process the individual modules, and also detects
+// required to process the individual modules
 func (l *Loader) LoadConfig(rootDir string) (*configs.Config, hcl.Diagnostics) {
 	rootMod, diags := l.parser.LoadConfigDir(rootDir)
 	if rootMod == nil {
@@ -31,8 +31,7 @@ func (l *Loader) LoadConfig(rootDir string) (*configs.Config, hcl.Diagnostics) {
 }
 
 // moduleWalkerLoad is a configs.ModuleWalkerFunc for loading modules that
-// are presumed to have already been installed. A different function
-// (moduleWalkerInstall) is used for installation.
+// are presumed to have already been installed.
 func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module, *version.Version, hcl.Diagnostics) {
 	// Since we're just loading here, we expect that all referenced modules
 	// will be already installed and described in our manifest. However, we
@@ -101,5 +100,55 @@ func (l *Loader) moduleWalkerLoad(req *configs.ModuleRequest) (*configs.Module,
 		}
 	}
 
+	// The providers associated with expanding modules must be present in the proxy/passed providers
+	// block. Guarding here for accessing the module call just in case.
+	if mc, exists := req.Parent.Module.ModuleCalls[req.Name]; exists {
+		if mc.Count != nil || mc.ForEach != nil {
+			for key, pc := range mod.ProviderConfigs {
+				// Use these to track if a provider is configured (not allowed),
+				// or if we've found its matching proxy
+				var isConfigured bool
+				var foundMatchingProxy bool
+
+				// Validate the config against an empty schema to see if it's empty.
+				_, pcConfigDiags := pc.Config.Content(&hcl.BodySchema{})
+				if pcConfigDiags.HasErrors() || pc.Version.Required != nil {
+					isConfigured = true
+				}
+
+				// If it is empty or only has an alias,
+				// does this provider exist in our proxy configs?
+				for _, r := range mc.Providers {
+					// Must match on name and Alias
+					if pc.Name == r.InChild.Name && pc.Alias == r.InChild.Alias {
+						foundMatchingProxy = true
+						break
+					}
+				}
+				if isConfigured || !foundMatchingProxy {
+					if mc.Count != nil {
+						diags = append(diags, &hcl.Diagnostic{
+							Severity: hcl.DiagError,
+							Summary:  "Module does not support count",
+							Detail:   fmt.Sprintf(moduleProviderError, mc.Name, "count", key, pc.NameRange),
+							Subject:  mc.Count.Range().Ptr(),
+						})
+					}
+					if mc.ForEach != nil {
+						diags = append(diags, &hcl.Diagnostic{
+							Severity: hcl.DiagError,
+							Summary:  "Module does not support for_each",
+							Detail:   fmt.Sprintf(moduleProviderError, mc.Name, "for_each", key, pc.NameRange),
+							Subject:  mc.ForEach.Range().Ptr(),
+						})
+					}
+				}
+			}
+		}
+	}
 	return mod, record.Version, diags
 }
+
+var moduleProviderError = `Module "%s" cannot be used with %s because it contains a nested provider configuration for "%s", at %s.
+
+This module can be made compatible with %[2]s by changing it to receive all of its provider configurations from the calling module, by using the "providers" argument in the calling module block.`

configs/configload/loader_load_test.go

@@ -1,6 +1,7 @@
 package configload
 
 import (
+	"fmt"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -75,8 +76,49 @@ func TestLoaderLoadConfig_addVersion(t *testing.T) {
 		t.Fatalf("success; want error")
 	}
 	got := diags.Error()
-	want := "Module requirements have changed"
-	if strings.Contains(got, want) {
+	want := "Module version requirements have changed"
+	if !strings.Contains(got, want) {
 		t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want)
 	}
 }
+
+func TestLoaderLoadConfig_moduleExpand(t *testing.T) {
+	// We do not allow providers to be configured in expanding modules
+	// In addition, if a provider is present but an empty block, it is allowed,
+	// but IFF a provider is passed through the module call
+	paths := []string{"provider-configured", "no-provider-passed"}
+	for _, p := range paths {
+		fixtureDir := filepath.Clean(fmt.Sprintf("testdata/expand-modules/%s", p))
+		loader, err := NewLoader(&Config{
+			ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
+		})
+		if err != nil {
+			t.Fatalf("unexpected error from NewLoader: %s", err)
+		}
+
+		_, diags := loader.LoadConfig(fixtureDir)
+		if !diags.HasErrors() {
+			t.Fatalf("success; want error")
+		}
+		got := diags.Error()
+		want := "Module does not support count"
+		if !strings.Contains(got, want) {
+			t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want)
+		}
+	}
+}
+
+func TestLoaderLoadConfig_moduleExpandValid(t *testing.T) {
+	// This tests for when valid configs are passing a provider through as a proxy,
+	// either with or without an alias present.
+	fixtureDir := filepath.Clean("testdata/expand-modules/valid")
+	loader, err := NewLoader(&Config{
+		ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"),
+	})
+	if err != nil {
+		t.Fatalf("unexpected error from NewLoader: %s", err)
+	}
+
+	_, diags := loader.LoadConfig(fixtureDir)
+	assertNoDiagnostics(t, diags)
+}

configs/configload/testdata/expand-modules/no-provider-passed/.terraform/modules/modules.json

@@ -0,0 +1,14 @@
+{
+    "Modules": [
+        {
+            "Key": "",
+            "Source": "",
+            "Dir": "testdata/expand-modules/no-provider-passed"
+        },
+        {
+            "Key": "child",
+            "Source": "./child",
+            "Dir": "testdata/expand-modules/no-provider-passed/child"
+        }
+    ]
+}

configs/configload/testdata/expand-modules/no-provider-passed/child/main.tf

@@ -0,0 +1,7 @@
+provider "aws" {
+}
+
+output "my_output" {
+  value = "my output"
+}
+

configs/configload/testdata/expand-modules/no-provider-passed/root.tf

@@ -0,0 +1,9 @@
+provider "aws" {
+  alias  = "usw2"
+  region = "us-west-2"
+}
+module "child" {
+  count = 1
+  source = "./child"
+  # To make this test fail, add a valid providers {} block passing "aws" to the child
+}

configs/configload/testdata/expand-modules/provider-configured/.terraform/modules/modules.json

@@ -0,0 +1,14 @@
+{
+    "Modules": [
+        {
+            "Key": "",
+            "Source": "",
+            "Dir": "testdata/expand-modules/provider-configured"
+        },
+        {
+            "Key": "child",
+            "Source": "./child",
+            "Dir": "testdata/expand-modules/provider-configured/child"
+        }
+    ]
+}

configs/configload/testdata/expand-modules/provider-configured/child/main.tf

@@ -0,0 +1,8 @@
+provider "aws" {
+  version = "1.0"
+}
+
+output "my_output" {
+  value = "my output"
+}
+

configs/configload/testdata/expand-modules/provider-configured/root.tf

@@ -0,0 +1,11 @@
+provider "aws" {
+  region = "us-west-2"
+}
+
+module "child" {
+  count = 1
+  source = "./child"
+  providers = {
+    aws = aws.w2
+  }
+}

configs/configload/testdata/expand-modules/valid/.terraform/modules/modules.json

@@ -0,0 +1,19 @@
+{
+    "Modules": [
+        {
+            "Key": "",
+            "Source": "",
+            "Dir": "testdata/expand-modules/valid"
+        },
+        {
+            "Key": "child",
+            "Source": "./child",
+            "Dir": "testdata/expand-modules/valid/child"
+        },
+        {
+            "Key": "child_with_alias",
+            "Source": "./child-with-alias",
+            "Dir": "testdata/expand-modules/valid/child-with-alias"
+        }
+    ]
+}

configs/configload/testdata/expand-modules/valid/child-with-alias/main.tf

@@ -0,0 +1,8 @@
+provider "aws" {
+  alias = "east"
+}
+
+output "my_output" {
+  value = "my output"
+}
+

configs/configload/testdata/expand-modules/valid/child/main.tf

@@ -0,0 +1,7 @@
+provider "aws" {
+}
+
+output "my_output" {
+  value = "my output"
+}
+

configs/configload/testdata/expand-modules/valid/root.tf

@@ -0,0 +1,20 @@
+provider "aws" {
+  region = "us-east-1"
+  alias = "east"
+}
+
+module "child" {
+  count = 1
+  source = "./child"
+  providers = {
+    aws = aws.east
+  }
+}
+
+module "child_with_alias" {
+  for_each = toset(["a", "b"])
+  source = "./child-with-alias"
+  providers = {
+    aws.east = aws.east
+  }
+}