stackeval: Return an error if a module contains a provider config

When using stacks the provider configurations belong in the stack
configuration rather than inline in the individual modules.

Shared modules with inline provider configurations has been a deprecated
legacy practice for many years now, but traditional Terraform continued
to support it for backward-compatibility with older modules despite the
significant downsides of doing so. Stacks now finally removes that
capability, since it isn't straightforward to continue supporting it once
we've made the stacks runtime be responsible for instantiating and
configuring providers.

internal/stacks/stackruntime/internal/stackeval/component_config.go

@@ -151,11 +151,70 @@ func (c *ComponentConfig) CheckModuleTree(ctx context.Context) (*configs.Config,
 				return nil, diags
 			}
 
+			// We also have a small selection of additional static validation
+			// rules that apply only to modules used within stack components.
+			diags = diags.Append(c.validateModuleTreeForStacks(configRoot))
+
 			return configRoot, diags
 		},
 	)
 }
 
+// validateModuleTreeForStacks imposes some additional validation constraints
+// on a module tree after it's been loaded by the main configuration packages.
+//
+// These rules deal with a small number of exceptions where the modules language
+// as used by stacks is a subset of the modules language from traditional
+// Terraform. Not all such exceptions are handled in this way because
+// some of them cannot be handled statically, but this is a reasonable place
+// to handle the simpler concerns and allows us to return error messages that
+// talk specifically about stacks, which would be harder to achieve if these
+// exceptions were made at a different layer.
+func (c *ComponentConfig) validateModuleTreeForStacks(startNode *configs.Config) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+	diags = diags.Append(c.validateModuleForStacks(startNode.Path, startNode.Module))
+	for _, childNode := range startNode.Children {
+		diags = diags.Append(c.validateModuleTreeForStacks(childNode))
+	}
+	return diags
+}
+
+func (c *ComponentConfig) validateModuleForStacks(moduleAddr addrs.Module, module *configs.Module) tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+
+	// Inline provider configurations are not allowed when running under stacks,
+	// because provider configurations live in the stack configuration and
+	// then get passed in to the modules as special arguments.
+	for _, pc := range module.ProviderConfigs {
+		// We use some slightly different language for the topmost module
+		// that's being directly called from the stack configuration, because
+		// we can give some direct advice for how to correct the problem there,
+		// whereas for a nested module we assume that it's a third-party module
+		// written for much older versions of Terraform before we deprecated
+		// inline provider configurations and thus the solution is most likely
+		// to be selecting a different module that is Stacks-compatible, because
+		// removing a legacy inline provider configuration from a shared module
+		// would be a breaking change to that module.
+		if moduleAddr.IsRoot() {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Inline provider configuration not allowed",
+				Detail:   "A module used as a stack component must have all of its provider configurations passed from the stack configuration, using the \"providers\" argument within the component configuration block.",
+				Subject:  pc.DeclRange.Ptr(),
+			})
+		} else {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Inline provider configuration not allowed",
+				Detail:   "This module is not compatible with Terraform Stacks, because it declares an inline provider configuration.\n\nTo be used with stacks, this module must instead accept provider configurations from its caller.",
+				Subject:  pc.DeclRange.Ptr(),
+			})
+		}
+	}
+
+	return diags
+}
+
 // InputsType returns an object type that the object representing the caller's
 // values for this component's input variables must conform to.
 func (c *ComponentConfig) InputsType(ctx context.Context) (cty.Type, *typeexpr.Defaults) {

internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/terraform-sources.json

@@ -24,6 +24,10 @@
       {
         "source": "https://testing.invalid/planning.tar.gz",
         "local": "planning"
+      },
+      {
+        "source": "https://testing.invalid/validating.tar.gz",
+        "local": "validating"
       }
     ]
   }

internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-a/modules-with-provider-configs-a.tf

@@ -0,0 +1,20 @@
+terraform {
+  required_providers {
+    test = {
+      source = "terraform.io/builtin/test"
+    }
+  }
+}
+
+provider "test" {
+  arg = "foo"
+}
+
+module "b" {
+  # FIXME: The following is an absolute remote address only because at the
+  # time of writing this test the stacks runtime's module loader can't deal
+  # with relative paths in this location.
+  source = "https://testing.invalid/validating.tar.gz//modules_with_provider_configs/module-b"
+  # Once that's been fixed, this should instead be:
+  # source = "../module-b"
+}

internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-b/modules-with-provider-configs-b.tf

@@ -0,0 +1,11 @@
+terraform {
+  required_providers {
+    test = {
+      source = "terraform.io/builtin/test"
+    }
+  }
+}
+
+provider "test" {
+  arg = "foo"
+}

internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/modules-with-provider-configs.tfstack.hcl

@@ -0,0 +1,3 @@
+component "a" {
+  source = "./module-a"
+}

internal/stacks/stackruntime/internal/stackeval/validating_test.go

@@ -0,0 +1,86 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package stackeval
+
+import (
+	"context"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/hcl/v2"
+
+	"github.com/hashicorp/terraform/internal/tfdiags"
+)
+
+func TestValidate_modulesWithProviderConfigs(t *testing.T) {
+	// This test checks that we're correctly prohibiting inline provider
+	// configurations in Terraform modules used as stack components, which
+	// is forbidden because the stacks language is responsible for provider
+	// configurations.
+	//
+	// The underlying modules runtime isn't configured with any ability to
+	// instantiate provider plugins itself, so failing to prohibit this
+	// at the stacks language layer would just cause a lower-quality and
+	// more confusing error message to be emited by the modules runtime.
+
+	cfg := testStackConfig(t, "validating", "modules_with_provider_configs")
+	main := NewForValidating(cfg, ValidateOpts{})
+
+	inPromisingTask(t, func(ctx context.Context, t *testing.T) {
+		diags := main.ValidateAll(ctx)
+		if !diags.HasErrors() {
+			t.Fatalf("succeeded; want errors")
+		}
+		diags.Sort()
+
+		// We'll use the ForRPC method just as a convenient way to discard
+		// the specific diagnostic object types, so that we can compare
+		// the objects without worrying about exactly which diagnostic
+		// implementation each is using.
+		gotDiags := diags.ForRPC()
+
+		var wantDiags tfdiags.Diagnostics
+		// TEMP: Because we're currently essentially just tricking the module
+		// loader into reading from a source bundle without actually knowing
+		// that's what its doing, these diagnostics show local filesystem
+		// paths instead of source addresses. Once we fix that in future,
+		// the following wanted diagnostics should switch to refer to
+		// source addresses starting with:
+		//   https://testing.invalid/validating.tar.gz//modules_with_provider_configs/
+		// ...which is the fake source address form used by the testStackConfig
+		// helper we used above.
+
+		// Configurations in the root module get a different detail message
+		// than those in descendent modules, because for descendents we don't
+		// assume that the author is empowered to make the module
+		// stacks-compatible, while for the root it's more likely to be
+		// directly intended for stacks use, at least for now while things are
+		// relatively early. (We could revisit this tradeoff later.)
+		wantDiags = wantDiags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Inline provider configuration not allowed",
+			Detail:   `A module used as a stack component must have all of its provider configurations passed from the stack configuration, using the "providers" argument within the component configuration block.`,
+			Subject: &hcl.Range{
+				Filename: "testdata/sourcebundle/validating/modules_with_provider_configs/module-a/modules-with-provider-configs-a.tf",
+				Start:    hcl.Pos{Line: 9, Column: 1, Byte: 104},
+				End:      hcl.Pos{Line: 9, Column: 16, Byte: 119},
+			},
+		})
+		wantDiags = wantDiags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Inline provider configuration not allowed",
+			Detail:   "This module is not compatible with Terraform Stacks, because it declares an inline provider configuration.\n\nTo be used with stacks, this module must instead accept provider configurations from its caller.",
+			Subject: &hcl.Range{
+				Filename: "testdata/sourcebundle/validating/modules_with_provider_configs/module-b/modules-with-provider-configs-b.tf",
+				Start:    hcl.Pos{Line: 9, Column: 1, Byte: 104},
+				End:      hcl.Pos{Line: 9, Column: 16, Byte: 119},
+			},
+		})
+		wantDiags = wantDiags.ForRPC()
+
+		if diff := cmp.Diff(wantDiags, gotDiags); diff != "" {
+			t.Errorf("wrong diagnostics\n%s", diff)
+		}
+	})
+}