stacks: validate provider configurations during static analysis (#34730)

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

@@ -423,10 +423,6 @@ func (c *ComponentConfig) checkValid(ctx context.Context, phase EvalPhase) tfdia
 			return diags, nil
 		}
 
-		// TODO: Manually validate the provider configs. Probably shouldn't
-		// actually do this here though. We can validate all the provider
-		// configs in the stack configuration in one go at a higher level.
-
 		tfCtx, err := terraform.NewContext(&terraform.ContextOpts{
 			PreloadedProviderSchemas: providerSchemas,
 			Provisioners:             c.main.availableProvisioners(),

internal/stacks/stackruntime/internal/stackeval/provider_config.go

@@ -19,6 +19,7 @@ import (
 	"github.com/hashicorp/terraform/internal/stacks/stackaddrs"
 	"github.com/hashicorp/terraform/internal/stacks/stackconfig"
 	"github.com/hashicorp/terraform/internal/stacks/stackconfig/stackconfigtypes"
+	"github.com/hashicorp/terraform/internal/stacks/stackplan"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
@@ -73,12 +74,12 @@ func (p *ProviderConfig) ProviderArgsDecoderSpec(ctx context.Context) (hcldec.Sp
 // provider instances declared by this provider configuration, or
 // an unknown value (possibly [cty.DynamicVal]) if the configuration is too
 // invalid to produce any answer at all.
-func (p *ProviderConfig) ProviderArgs(ctx context.Context) cty.Value {
-	v, _ := p.CheckProviderArgs(ctx)
+func (p *ProviderConfig) ProviderArgs(ctx context.Context, phase EvalPhase) cty.Value {
+	v, _ := p.CheckProviderArgs(ctx, phase)
 	return v
 }
 
-func (p *ProviderConfig) CheckProviderArgs(ctx context.Context) (cty.Value, tfdiags.Diagnostics) {
+func (p *ProviderConfig) CheckProviderArgs(ctx context.Context, phase EvalPhase) (cty.Value, tfdiags.Diagnostics) {
 	return doOnceWithDiags(
 		ctx, &p.providerArgs, p.main,
 		func(ctx context.Context) (cty.Value, tfdiags.Diagnostics) {
@@ -115,7 +116,15 @@ func (p *ProviderConfig) CheckProviderArgs(ctx context.Context) (cty.Value, tfdi
 			}
 			defer client.Close()
 
-			configVal, moreDiags := EvalBody(ctx, decl.Config, spec, ValidatePhase, p)
+			body := decl.Config
+			if body == nil {
+				// A provider with no configuration is valid (just means no
+				// attributes or blocks), but we need to pass an empty body to
+				// the evaluator to avoid a panic.
+				body = hcl.EmptyBody()
+			}
+
+			configVal, moreDiags := EvalBody(ctx, body, spec, phase, p)
 			diags = diags.Append(moreDiags)
 			if moreDiags.HasErrors() {
 				return cty.UnknownVal(hcldec.ImpliedType(spec)), diags
@@ -196,17 +205,19 @@ func providerInstanceRefType(sourceAddr addrs.Provider) cty.Type {
 	return providerInstanceRefTypes[sourceAddr]
 }
 
+func (p *ProviderConfig) checkValid(ctx context.Context, phase EvalPhase) tfdiags.Diagnostics {
+	_, diags := p.CheckProviderArgs(ctx, phase)
+	return diags
+}
+
 // Validate implements Validatable.
 func (p *ProviderConfig) Validate(ctx context.Context) tfdiags.Diagnostics {
-	var diags tfdiags.Diagnostics
-
-	// TODO: Actually validate the configuration against the schema.
-	// Currently we're doing that only during the plan phase, but
-	// it would be better to catch statically-detectable problems
-	// earlier and only once per provider block, rather than repeatedly
-	// for each instance of a provider.
+	return p.checkValid(ctx, ValidatePhase)
+}
 
-	return diags
+// PlanChanges implements Plannable.
+func (p *ProviderConfig) PlanChanges(ctx context.Context) ([]stackplan.PlannedChange, tfdiags.Diagnostics) {
+	return nil, p.checkValid(ctx, PlanPhase)
 }
 
 // tracingName implements Validatable.

internal/stacks/stackruntime/internal/stackeval/provider_config_test.go

@@ -20,7 +20,80 @@ import (
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
-func TestProviderConfigCheckProviderArgs(t *testing.T) {
+func TestProviderConfig_CheckProviderArgs_EmptyConfig(t *testing.T) {
+	cfg := testStackConfig(t, "provider", "single_instance")
+	providerTypeAddr := addrs.NewBuiltInProvider("foo")
+	newMockProvider := func(t *testing.T) (*testing_provider.MockProvider, providers.Factory) {
+		t.Helper()
+		mockProvider := &testing_provider.MockProvider{
+			GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
+				Provider: providers.Schema{},
+			},
+			ValidateProviderConfigFn: func(vpcr providers.ValidateProviderConfigRequest) providers.ValidateProviderConfigResponse {
+				if vpcr.Config.ContainsMarked() {
+					panic("config has marks")
+				}
+				var diags tfdiags.Diagnostics
+				if vpcr.Config.Type().HasAttribute("test") {
+					if vpcr.Config.GetAttr("test").RawEquals(cty.StringVal("invalid")) {
+						diags = diags.Append(fmt.Errorf("invalid value checked by provider itself"))
+					}
+				}
+				return providers.ValidateProviderConfigResponse{
+					PreparedConfig: vpcr.Config,
+					Diagnostics:    diags,
+				}
+			},
+		}
+		providerFactory := providers.FactoryFixed(mockProvider)
+		return mockProvider, providerFactory
+	}
+	getProviderConfig := func(ctx context.Context, t *testing.T, main *Main) *ProviderConfig {
+		t.Helper()
+		mainStack := main.MainStack(ctx)
+		provider := mainStack.Provider(ctx, stackaddrs.ProviderConfig{
+			Provider: providerTypeAddr,
+			Name:     "bar",
+		})
+		if provider == nil {
+			t.Fatal("no provider.foo.bar is available")
+		}
+		return provider.Config(ctx)
+	}
+
+	subtestInPromisingTask(t, "valid", func(ctx context.Context, t *testing.T) {
+		mockProvider, providerFactory := newMockProvider(t)
+		main := testEvaluator(t, testEvaluatorOpts{
+			Config: cfg,
+			ProviderFactories: ProviderFactories{
+				providerTypeAddr: providerFactory,
+			},
+		})
+		config := getProviderConfig(ctx, t, main)
+
+		want := cty.EmptyObjectVal
+		got, diags := config.CheckProviderArgs(ctx, ValidatePhase)
+		assertNoDiags(t, diags)
+
+		if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" {
+			t.Errorf("wrong result\n%s", diff)
+		}
+
+		if !mockProvider.ValidateProviderConfigCalled {
+			t.Error("ValidateProviderConfig was not called; should've been")
+		} else {
+			got := mockProvider.ValidateProviderConfigRequest
+			want := providers.ValidateProviderConfigRequest{
+				Config: cty.EmptyObjectVal,
+			}
+			if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" {
+				t.Errorf("wrong request\n%s", diff)
+			}
+		}
+	})
+}
+
+func TestProviderConfig_CheckProviderArgs(t *testing.T) {
 	cfg := testStackConfig(t, "provider", "single_instance_configured")
 	providerTypeAddr := addrs.NewBuiltInProvider("foo")
 	newMockProvider := func(t *testing.T) (*testing_provider.MockProvider, providers.Factory) {
@@ -86,7 +159,7 @@ func TestProviderConfigCheckProviderArgs(t *testing.T) {
 		want := cty.ObjectVal(map[string]cty.Value{
 			"test": cty.StringVal("yep"),
 		})
-		got, diags := config.CheckProviderArgs(ctx)
+		got, diags := config.CheckProviderArgs(ctx, ValidatePhase)
 		assertNoDiags(t, diags)
 
 		if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" {
@@ -123,7 +196,7 @@ func TestProviderConfigCheckProviderArgs(t *testing.T) {
 		want := cty.ObjectVal(map[string]cty.Value{
 			"test": cty.StringVal("yep").Mark("nope"),
 		})
-		got, diags := config.CheckProviderArgs(ctx)
+		got, diags := config.CheckProviderArgs(ctx, ValidatePhase)
 		assertNoDiags(t, diags)
 
 		if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" {

internal/stacks/stackruntime/internal/stackeval/stack_config.go

@@ -180,8 +180,8 @@ func (s *StackConfig) OutputValue(ctx context.Context, addr stackaddrs.OutputVal
 	return ret
 }
 
-// InputVariables returns a map of the objects representing all of the
-// input variables declared inside this stack configuration.
+// OutputValues returns a map of the objects representing all of the
+// output values declared inside this stack configuration.
 func (s *StackConfig) OutputValues(ctx context.Context) map[stackaddrs.OutputValue]*OutputValueConfig {
 	if len(s.config.Stack.OutputValues) == 0 {
 		return nil
@@ -194,6 +194,42 @@ func (s *StackConfig) OutputValues(ctx context.Context) map[stackaddrs.OutputVal
 	return ret
 }
 
+// ResultType returns the type of the result object that will be produced
+// by this stack configuration, based on the output values declared within
+// it.
+func (s *StackConfig) ResultType(ctx context.Context) cty.Type {
+	os := s.OutputValues(ctx)
+	atys := make(map[string]cty.Type, len(os))
+	for addr, o := range os {
+		atys[addr.Name] = o.ValueTypeConstraint(ctx)
+	}
+	return cty.Object(atys)
+}
+
+// Providers returns a map of the objects representing all of the provider
+// configurations declared inside this stack configuration.
+func (s *StackConfig) Providers(ctx context.Context) map[stackaddrs.ProviderConfig]*ProviderConfig {
+	if len(s.config.Stack.ProviderConfigs) == 0 {
+		return nil
+	}
+	ret := make(map[stackaddrs.ProviderConfig]*ProviderConfig, len(s.config.Stack.ProviderConfigs))
+	for configAddr := range s.config.Stack.ProviderConfigs {
+		provider, ok := s.config.Stack.RequiredProviders.ProviderForLocalName(configAddr.LocalName)
+		if !ok {
+			// Then we are missing a provider declaration, this will be caught
+			// elsewhere so we'll just skip it here.
+			continue
+		}
+
+		addr := stackaddrs.ProviderConfig{
+			Provider: provider,
+			Name:     configAddr.Alias,
+		}
+		ret[addr] = s.Provider(ctx, addr)
+	}
+	return ret
+}
+
 // Provider returns a [ProviderConfig] representing the provider configuration
 // block within the stack configuration that matches the given address,
 // or nil if there is no such declaration.
@@ -270,15 +306,6 @@ func (s *StackConfig) ProviderLocalName(ctx context.Context, addr addrs.Provider
 	return s.config.Stack.RequiredProviders.LocalNameForProvider(addr)
 }
 
-func (s *StackConfig) ResultType(ctx context.Context) cty.Type {
-	os := s.OutputValues(ctx)
-	atys := make(map[string]cty.Type, len(os))
-	for addr, o := range os {
-		atys[addr.Name] = o.ValueTypeConstraint(ctx)
-	}
-	return cty.Object(atys)
-}
-
 // StackCall returns a [StackCallConfig] representing the "stack" block
 // matching the given address declared within this stack config, or nil if
 // there is no such declaration.

internal/stacks/stackruntime/internal/stackeval/walk_static.go

@@ -53,6 +53,10 @@ func walkStaticObjectsInStackConfig[Output any](
 
 	// TODO: All of the other static object types
 
+	for _, obj := range stackConfig.Providers(ctx) {
+		visit(ctx, walk, obj)
+	}
+
 	for _, obj := range stackConfig.Components(ctx) {
 		visit(ctx, walk, obj)
 	}

internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/invalid-provider-config/invalid-provider-config.tfstack.hcl

@@ -0,0 +1,29 @@
+required_providers {
+  testing = {
+    source  = "hashicorp/testing"
+    version = "0.1.0"
+  }
+}
+
+provider "testing" "default" {
+  config {
+    // The `imaginary` attribute is not valid for the `testing` provider.
+    imaginary = "imaginary"
+  }
+}
+
+variable "input" {
+  type = string
+}
+
+component "self" {
+  source = "../"
+
+  providers = {
+    testing = provider.testing.default
+  }
+
+  inputs = {
+    input = var.input
+  }
+}

internal/stacks/stackruntime/validate_test.go

@@ -116,12 +116,28 @@ var (
 				return diags
 			},
 		},
-		filepath.Join("with-single-input", "invalid-provider"): {
+		filepath.Join("with-single-input", "invalid-provider-type"): {
 			// TODO: Enable this test case, when we have a good error message
 			//  for provider type mismatches. Currently, we return the same
 			//  error as for missing provider, which is not ideal.
 			skip: true,
 		},
+		filepath.Join("with-single-input", "invalid-provider-config"): {
+			diags: func() tfdiags.Diagnostics {
+				var diags tfdiags.Diagnostics
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Unsupported argument",
+					Detail:   "An argument named \"imaginary\" is not expected here.",
+					Subject: &hcl.Range{
+						Filename: mainBundleSourceAddrStr("with-single-input/invalid-provider-config/invalid-provider-config.tfstack.hcl"),
+						Start:    hcl.Pos{Line: 11, Column: 5, Byte: 218},
+						End:      hcl.Pos{Line: 11, Column: 14, Byte: 227},
+					},
+				})
+				return diags
+			},
+		},
 		filepath.Join("with-single-input", "undeclared-variable"): {
 			diags: func() tfdiags.Diagnostics {
 				var diags tfdiags.Diagnostics