core: Graph walk loads plugin schemas opportunistically

Previously our graph walker expected to recieve a data structure
containing schemas for all of the provider and provisioner plugins used in
the configuration and state. That made sense back when
terraform.NewContext was responsible for loading all of the schemas before
taking any other action, but it no longer has that responsiblity.

Instead, we'll now make sure that the "contextPlugins" object reaches all
of the locations where we need schema -- many of which already had access
to that object anyway -- and then load the needed schemas just in time.

The contextPlugins object memoizes schema lookups, so we can safely call
it many times with the same provider address or provisioner type name and
know that it'll still only load each distinct plugin once per Context
object.

As of this commit, the Context.Schemas method is now a public interface
only and not used by logic in the "terraform" package at all. However,
that does leave us in a rather tenuous situation of relying on the fact
that all practical users of terraform.Context end up calling "Schemas" at
some point in order to verify that we have all of the expected versions
of plugins. That's a non-obvious implicit dependency, and so in subsequent
commits we'll gradually move all responsibility for verifying plugin
versions into the caller of terraform.NewContext, which'll heal a
long-standing architectural wart whereby the caller is responsible for
installing and locating the plugin executables but not for verifying that
what's installed is conforming to the current configuration and dependency
lock file.

internal/backend/local/backend_plan_test.go

@@ -136,7 +136,7 @@ func TestLocal_plan_context_error(t *testing.T) {
 	// the backend should be unlocked after a run
 	assertBackendStateUnlocked(t, b)
 
-	if got, want := done(t).Stderr(), "Error: Failed to load plugin schemas"; !strings.Contains(got, want) {
+	if got, want := done(t).Stderr(), "failed to read schema for test_instance.foo in registry.terraform.io/hashicorp/test"; !strings.Contains(got, want) {
 		t.Fatalf("unexpected error output:\n%s\nwant: %s", got, want)
 	}
 }

internal/command/plan_test.go

@@ -1051,7 +1051,7 @@ func TestPlan_init_required(t *testing.T) {
 		t.Fatalf("expected error, got success")
 	}
 	got := output.Stderr()
-	if !strings.Contains(got, `Please run "terraform init".`) {
+	if !strings.Contains(got, `failed to read schema for test_instance.foo in registry.terraform.io/hashicorp/test`) {
 		t.Fatal("wrong error message in output:", got)
 	}
 }

internal/terraform/context_apply.go

@@ -24,12 +24,6 @@ func (c *Context) Apply(plan *plans.Plan, config *configs.Config) (*states.State
 	defer c.acquireRun("apply")()
 	var diags tfdiags.Diagnostics
 
-	schemas, moreDiags := c.Schemas(config, plan.PriorState)
-	diags = diags.Append(moreDiags)
-	if moreDiags.HasErrors() {
-		return nil, diags
-	}
-
 	log.Printf("[DEBUG] Building and walking apply graph for %s plan", plan.UIMode)
 
 	graph, operation, moreDiags := c.applyGraph(plan, config, true)
@@ -58,7 +52,6 @@ func (c *Context) Apply(plan *plans.Plan, config *configs.Config) (*states.State
 	workingState := plan.PriorState.DeepCopy()
 	walker, walkDiags := c.walk(graph, operation, &graphWalkOpts{
 		Config:             config,
-		Schemas:            schemas,
 		InputState:         workingState,
 		Changes:            plan.Changes,
 		RootVariableValues: variables,

internal/terraform/context_eval.go

@@ -40,12 +40,6 @@ func (c *Context) Eval(config *configs.Config, state *states.State, moduleAddr a
 	var diags tfdiags.Diagnostics
 	defer c.acquireRun("eval")()
 
-	schemas, moreDiags := c.Schemas(config, state)
-	diags = diags.Append(moreDiags)
-	if moreDiags.HasErrors() {
-		return nil, diags
-	}
-
 	// Start with a copy of state so that we don't affect the instance that
 	// the caller is holding.
 	state = state.DeepCopy()
@@ -78,7 +72,6 @@ func (c *Context) Eval(config *configs.Config, state *states.State, moduleAddr a
 	walkOpts := &graphWalkOpts{
 		InputState:         state,
 		Config:             config,
-		Schemas:            schemas,
 		RootVariableValues: variables,
 	}
 

internal/terraform/context_import.go

@@ -48,12 +48,6 @@ func (c *Context) Import(config *configs.Config, prevRunState *states.State, opt
 	// Hold a lock since we can modify our own state here
 	defer c.acquireRun("import")()
 
-	schemas, moreDiags := c.Schemas(config, prevRunState)
-	diags = diags.Append(moreDiags)
-	if moreDiags.HasErrors() {
-		return nil, diags
-	}
-
 	// Don't modify our caller's state
 	state := prevRunState.DeepCopy()
 
@@ -78,7 +72,6 @@ func (c *Context) Import(config *configs.Config, prevRunState *states.State, opt
 	// Walk it
 	walker, walkDiags := c.walk(graph, walkImport, &graphWalkOpts{
 		Config:             config,
-		Schemas:            schemas,
 		InputState:         state,
 		RootVariableValues: variables,
 	})

internal/terraform/context_plan.go

@@ -324,16 +324,10 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, r
 	var diags tfdiags.Diagnostics
 	log.Printf("[DEBUG] Building and walking plan graph for %s", opts.Mode)
 
-	schemas, moreDiags := c.Schemas(config, prevRunState)
-	diags = diags.Append(moreDiags)
-	if diags.HasErrors() {
-		return nil, diags
-	}
-
 	prevRunState = prevRunState.DeepCopy() // don't modify the caller's object when we process the moves
 	moveStmts, moveResults := c.prePlanFindAndApplyMoves(config, prevRunState, opts.Targets)
 
-	graph, walkOp, moreDiags := c.planGraph(config, prevRunState, opts, schemas, true)
+	graph, walkOp, moreDiags := c.planGraph(config, prevRunState, opts, true)
 	diags = diags.Append(moreDiags)
 	if diags.HasErrors() {
 		return nil, diags
@@ -344,7 +338,6 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, r
 	changes := plans.NewChanges()
 	walker, walkDiags := c.walk(graph, walkOp, &graphWalkOpts{
 		Config:             config,
-		Schemas:            schemas,
 		InputState:         prevRunState,
 		Changes:            changes,
 		MoveResults:        moveResults,
@@ -365,7 +358,7 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, r
 	return plan, diags
 }
 
-func (c *Context) planGraph(config *configs.Config, prevRunState *states.State, opts *PlanOpts, schemas *Schemas, validate bool) (*Graph, walkOperation, tfdiags.Diagnostics) {
+func (c *Context) planGraph(config *configs.Config, prevRunState *states.State, opts *PlanOpts, validate bool) (*Graph, walkOperation, tfdiags.Diagnostics) {
 	switch mode := opts.Mode; mode {
 	case plans.NormalMode:
 		graph, diags := (&PlanGraphBuilder{
@@ -421,13 +414,7 @@ func (c *Context) PlanGraphForUI(config *configs.Config, prevRunState *states.St
 
 	opts := &PlanOpts{Mode: mode}
 
-	schemas, moreDiags := c.Schemas(config, prevRunState)
+	graph, _, moreDiags := c.planGraph(config, prevRunState, opts, false)
-	diags = diags.Append(moreDiags)
-	if diags.HasErrors() {
-		return nil, diags
-	}
-
-	graph, _, moreDiags := c.planGraph(config, prevRunState, opts, schemas, false)
 	diags = diags.Append(moreDiags)
 	return graph, diags
 }

internal/terraform/context_test.go

@@ -123,6 +123,10 @@ func TestNewContextRequiredVersion(t *testing.T) {
 }
 
 func TestNewContext_lockedDependencies(t *testing.T) {
+	// TODO: Remove this test altogether once we've factored out the version
+	// and checksum verification to be exclusively the caller's responsibility.
+	t.Skip("only one step away from locked dependencies being the caller's responsibility")
+
 	configBeepGreaterThanOne := `
 terraform {
   required_providers {

internal/terraform/context_validate.go

@@ -35,12 +35,6 @@ func (c *Context) Validate(config *configs.Config) tfdiags.Diagnostics {
 		return diags
 	}
 
-	schemas, moreDiags := c.Schemas(config, nil)
-	diags = diags.Append(moreDiags)
-	if moreDiags.HasErrors() {
-		return diags
-	}
-
 	log.Printf("[DEBUG] Building and walking validate graph")
 
 	graph, moreDiags := ValidateGraphBuilder(&PlanGraphBuilder{
@@ -74,7 +68,6 @@ func (c *Context) Validate(config *configs.Config) tfdiags.Diagnostics {
 
 	walker, walkDiags := c.walk(graph, walkValidate, &graphWalkOpts{
 		Config:             config,
-		Schemas:            schemas,
 		RootVariableValues: varValues,
 	})
 	diags = diags.Append(walker.NonFatalDiagnostics)

internal/terraform/context_validate_test.go

@@ -1193,10 +1193,6 @@ func TestContext2Validate_PlanGraphBuilder(t *testing.T) {
 	opts := fixture.ContextOpts()
 	c := testContext2(t, opts)
 
-	state := states.NewState()
-	schemas, diags := c.Schemas(fixture.Config, state)
-	assertNoDiagnostics(t, diags)
-
 	graph, diags := ValidateGraphBuilder(&PlanGraphBuilder{
 		Config:  fixture.Config,
 		State:   states.NewState(),
@@ -1207,8 +1203,7 @@ func TestContext2Validate_PlanGraphBuilder(t *testing.T) {
 	}
 	defer c.acquireRun("validate-test")()
 	walker, diags := c.walk(graph, walkValidate, &graphWalkOpts{
-		Config:  fixture.Config,
+		Config: fixture.Config,
-		Schemas: schemas,
 	})
 	if diags.HasErrors() {
 		t.Fatal(diags.Err())

internal/terraform/context_walk.go

@@ -23,7 +23,6 @@ type graphWalkOpts struct {
 	InputState *states.State
 	Changes    *plans.Changes
 	Config     *configs.Config
-	Schemas    *Schemas
 
 	RootVariableValues InputValues
 	MoveResults        map[addrs.UniqueKey]refactoring.MoveResult
@@ -95,12 +94,6 @@ func (c *Context) graphWalker(operation walkOperation, opts *graphWalkOpts) *Con
 		changes = plans.NewChanges()
 	}
 
-	if opts.Schemas == nil {
-		// Should never happen: caller must always set this one.
-		// (We catch this here, rather than later, to get a more intelligible
-		// stack trace when it _does_ panic.)
-		panic("Context.graphWalker call without Schemas")
-	}
 	if opts.Config == nil {
 		panic("Context.graphWalker call without Config")
 	}
@@ -109,7 +102,6 @@ func (c *Context) graphWalker(operation walkOperation, opts *graphWalkOpts) *Con
 		Context:            c,
 		State:              state,
 		Config:             opts.Config,
-		Schemas:            opts.Schemas,
 		RefreshState:       refreshState,
 		PrevRunState:       prevRunState,
 		Changes:            changes.SyncWrapper(),

internal/terraform/evaluate.go

@@ -46,13 +46,13 @@ type Evaluator struct {
 	VariableValues     map[string]map[string]cty.Value
 	VariableValuesLock *sync.Mutex
 
-	// Schemas is a repository of all of the schemas we should need to
+	// Plugins is the library of available plugin components (providers and
-	// evaluate expressions. This must be constructed by the caller to
+	// provisioners) that we have available to help us evaluate expressions
-	// include schemas for all of the providers, resource types, data sources
+	// that interact with plugin-provided objects.
-	// and provisioners used by the given configuration and state.
 	//
-	// This must not be mutated during evaluation.
+	// From this we only access the schemas of the plugins, and don't otherwise
-	Schemas *Schemas
+	// interact with plugin instances.
+	Plugins *contextPlugins
 
 	// State is the current state, embedded in a wrapper that ensures that
 	// it can be safely accessed and modified concurrently.
@@ -892,8 +892,13 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
 }
 
 func (d *evaluationStateData) getResourceSchema(addr addrs.Resource, providerAddr addrs.AbsProviderConfig) *configschema.Block {
-	schemas := d.Evaluator.Schemas
+	schema, _, err := d.Evaluator.Plugins.ResourceTypeSchema(providerAddr.Provider, addr.Mode, addr.Type)
-	schema, _ := schemas.ResourceTypeConfig(providerAddr.Provider, addr.Mode, addr.Type)
+	if err != nil {
+		// We have plently other codepaths that will detect and report
+		// schema lookup errors before we'd reach this point, so we'll just
+		// treat a failure here the same as having no schema.
+		return nil
+	}
 	return schema
 }
 

internal/terraform/evaluate_test.go

@@ -193,79 +193,77 @@ func TestEvaluatorGetResource(t *testing.T) {
 			},
 		},
 		State: stateSync,
-		Schemas: &Schemas{
+		Plugins: schemaOnlyProvidersForTesting(map[addrs.Provider]*ProviderSchema{
-			Providers: map[addrs.Provider]*ProviderSchema{
+			addrs.NewDefaultProvider("test"): {
-				addrs.NewDefaultProvider("test"): {
+				Provider: &configschema.Block{},
-					Provider: &configschema.Block{},
+				ResourceTypes: map[string]*configschema.Block{
-					ResourceTypes: map[string]*configschema.Block{
+					"test_resource": {
-						"test_resource": {
+						Attributes: map[string]*configschema.Attribute{
-							Attributes: map[string]*configschema.Attribute{
+							"id": {
-								"id": {
+								Type:     cty.String,
-									Type:     cty.String,
+								Computed: true,
-									Computed: true,
-								},
-								"value": {
-									Type:      cty.String,
-									Computed:  true,
-									Sensitive: true,
-								},
 							},
-							BlockTypes: map[string]*configschema.NestedBlock{
+							"value": {
-								"nesting_list": {
+								Type:      cty.String,
-									Block: configschema.Block{
+								Computed:  true,
-										Attributes: map[string]*configschema.Attribute{
+								Sensitive: true,
-											"value":           {Type: cty.String, Optional: true},
+							},
-											"sensitive_value": {Type: cty.String, Optional: true, Sensitive: true},
+						},
-										},
+						BlockTypes: map[string]*configschema.NestedBlock{
+							"nesting_list": {
+								Block: configschema.Block{
+									Attributes: map[string]*configschema.Attribute{
+										"value":           {Type: cty.String, Optional: true},
+										"sensitive_value": {Type: cty.String, Optional: true, Sensitive: true},
 									},
-									Nesting: configschema.NestingList,
 								},
-								"nesting_map": {
+								Nesting: configschema.NestingList,
-									Block: configschema.Block{
+							},
-										Attributes: map[string]*configschema.Attribute{
+							"nesting_map": {
-											"foo": {Type: cty.String, Optional: true, Sensitive: true},
+								Block: configschema.Block{
-										},
+									Attributes: map[string]*configschema.Attribute{
+										"foo": {Type: cty.String, Optional: true, Sensitive: true},
 									},
-									Nesting: configschema.NestingMap,
 								},
-								"nesting_set": {
+								Nesting: configschema.NestingMap,
-									Block: configschema.Block{
+							},
-										Attributes: map[string]*configschema.Attribute{
+							"nesting_set": {
-											"baz": {Type: cty.String, Optional: true, Sensitive: true},
+								Block: configschema.Block{
-										},
+									Attributes: map[string]*configschema.Attribute{
+										"baz": {Type: cty.String, Optional: true, Sensitive: true},
 									},
-									Nesting: configschema.NestingSet,
 								},
-								"nesting_single": {
+								Nesting: configschema.NestingSet,
-									Block: configschema.Block{
+							},
-										Attributes: map[string]*configschema.Attribute{
+							"nesting_single": {
-											"boop": {Type: cty.String, Optional: true, Sensitive: true},
+								Block: configschema.Block{
-										},
+									Attributes: map[string]*configschema.Attribute{
+										"boop": {Type: cty.String, Optional: true, Sensitive: true},
 									},
-									Nesting: configschema.NestingSingle,
 								},
-								"nesting_nesting": {
+								Nesting: configschema.NestingSingle,
-									Block: configschema.Block{
+							},
-										BlockTypes: map[string]*configschema.NestedBlock{
+							"nesting_nesting": {
-											"nesting_list": {
+								Block: configschema.Block{
-												Block: configschema.Block{
+									BlockTypes: map[string]*configschema.NestedBlock{
-													Attributes: map[string]*configschema.Attribute{
+										"nesting_list": {
-														"value":           {Type: cty.String, Optional: true},
+											Block: configschema.Block{
-														"sensitive_value": {Type: cty.String, Optional: true, Sensitive: true},
+												Attributes: map[string]*configschema.Attribute{
-													},
+													"value":           {Type: cty.String, Optional: true},
+													"sensitive_value": {Type: cty.String, Optional: true, Sensitive: true},
 												},
-												Nesting: configschema.NestingList,
 											},
+											Nesting: configschema.NestingList,
 										},
 									},
-									Nesting: configschema.NestingSingle,
 								},
+								Nesting: configschema.NestingSingle,
 							},
 						},
 					},
 				},
 			},
-		},
+		}),
 	}
 
 	data := &evaluationStateData{
@@ -430,7 +428,7 @@ func TestEvaluatorGetResource_changes(t *testing.T) {
 			},
 		},
 		State:   stateSync,
-		Schemas: schemas,
+		Plugins: schemaOnlyProvidersForTesting(schemas.Providers),
 	}
 
 	data := &evaluationStateData{

internal/terraform/evaluate_valid.go

@@ -224,7 +224,18 @@ func (d *evaluationStateData) staticValidateResourceReference(modCfg *configs.Co
 	}
 
 	providerFqn := modCfg.Module.ProviderForLocalConfig(cfg.ProviderConfigAddr())
-	schema, _ := d.Evaluator.Schemas.ResourceTypeConfig(providerFqn, addr.Mode, addr.Type)
+	schema, _, err := d.Evaluator.Plugins.ResourceTypeSchema(providerFqn, addr.Mode, addr.Type)
+	if err != nil {
+		// Prior validation should've taken care of a schema lookup error,
+		// so we should never get here but we'll handle it here anyway for
+		// robustness.
+		diags = diags.Append(&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  `Failed provider schema lookup`,
+			Detail:   fmt.Sprintf(`Couldn't load schema for %s resource type %q in %s: %s.`, modeAdjective, addr.Type, providerFqn.String(), err),
+			Subject:  rng.ToHCL().Ptr(),
+		})
+	}
 
 	if schema == nil {
 		// Prior validation should've taken care of a resource block with an

internal/terraform/evaluate_valid_test.go

@@ -69,21 +69,19 @@ For example, to correlate with indices of a referring resource, use:
 	cfg := testModule(t, "static-validate-refs")
 	evaluator := &Evaluator{
 		Config: cfg,
-		Schemas: &Schemas{
+		Plugins: schemaOnlyProvidersForTesting(map[addrs.Provider]*ProviderSchema{
-			Providers: map[addrs.Provider]*ProviderSchema{
+			addrs.NewDefaultProvider("aws"): {
-				addrs.NewDefaultProvider("aws"): {
+				ResourceTypes: map[string]*configschema.Block{
-					ResourceTypes: map[string]*configschema.Block{
+					"aws_instance": {},
-						"aws_instance": {},
-					},
-				},
-				addrs.MustParseProviderSourceString("foobar/beep"): {
-					ResourceTypes: map[string]*configschema.Block{
-						// intentional mismatch between resource type prefix and provider type
-						"boop_instance": {},
-					},
 				},
 			},
-		},
+			addrs.MustParseProviderSourceString("foobar/beep"): {
+				ResourceTypes: map[string]*configschema.Block{
+					// intentional mismatch between resource type prefix and provider type
+					"boop_instance": {},
+				},
+			},
+		}),
 	}
 
 	for _, test := range tests {

internal/terraform/graph_walk_context.go

@@ -34,7 +34,6 @@ type ContextGraphWalker struct {
 	Operation          walkOperation
 	StopContext        context.Context
 	RootVariableValues InputValues
-	Schemas            *Schemas
 	Config             *configs.Config
 
 	// This is an output. Do not set this, nor read it while a graph walk
@@ -81,7 +80,7 @@ func (w *ContextGraphWalker) EvalContext() EvalContext {
 		Operation:          w.Operation,
 		State:              w.State,
 		Changes:            w.Changes,
-		Schemas:            w.Schemas,
+		Plugins:            w.Context.plugins,
 		VariableValues:     w.variableValues,
 		VariableValuesLock: &w.variableValuesLock,
 	}