diff --git a/internal/backend/local/backend_plan_test.go b/internal/backend/local/backend_plan_test.go index 5866048f9..73bd78df4 100644 --- a/internal/backend/local/backend_plan_test.go +++ b/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) } } diff --git a/internal/command/plan_test.go b/internal/command/plan_test.go index 5638a9abf..880f2e971 100644 --- a/internal/command/plan_test.go +++ b/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) } } diff --git a/internal/terraform/context_apply.go b/internal/terraform/context_apply.go index ff32074ec..e94449b72 100644 --- a/internal/terraform/context_apply.go +++ b/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, diff --git a/internal/terraform/context_eval.go b/internal/terraform/context_eval.go index 8201db4d6..efc24767c 100644 --- a/internal/terraform/context_eval.go +++ b/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, } diff --git a/internal/terraform/context_import.go b/internal/terraform/context_import.go index f4bc6d809..af17cbd62 100644 --- a/internal/terraform/context_import.go +++ b/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, }) diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index 208ee771c..248673254 100644 --- a/internal/terraform/context_plan.go +++ b/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) - diags = diags.Append(moreDiags) - if diags.HasErrors() { - return nil, diags - } - - graph, _, moreDiags := c.planGraph(config, prevRunState, opts, schemas, false) + graph, _, moreDiags := c.planGraph(config, prevRunState, opts, false) diags = diags.Append(moreDiags) return graph, diags } diff --git a/internal/terraform/context_test.go b/internal/terraform/context_test.go index 47fd32f4b..02addb4db 100644 --- a/internal/terraform/context_test.go +++ b/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 { diff --git a/internal/terraform/context_validate.go b/internal/terraform/context_validate.go index f5aa8540c..b079477ba 100644 --- a/internal/terraform/context_validate.go +++ b/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) diff --git a/internal/terraform/context_validate_test.go b/internal/terraform/context_validate_test.go index 0c5a2a3a5..4f628945e 100644 --- a/internal/terraform/context_validate_test.go +++ b/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, - Schemas: schemas, + Config: fixture.Config, }) if diags.HasErrors() { t.Fatal(diags.Err()) diff --git a/internal/terraform/context_walk.go b/internal/terraform/context_walk.go index b44a5910c..e041f80b2 100644 --- a/internal/terraform/context_walk.go +++ b/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(), diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index efcc9c1f4..47889a9a4 100644 --- a/internal/terraform/evaluate.go +++ b/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 - // evaluate expressions. This must be constructed by the caller to - // include schemas for all of the providers, resource types, data sources - // and provisioners used by the given configuration and state. + // Plugins is the library of available plugin components (providers and + // provisioners) that we have available to help us evaluate expressions + // that interact with plugin-provided objects. // - // This must not be mutated during evaluation. - Schemas *Schemas + // From this we only access the schemas of the plugins, and don't otherwise + // 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, _ := schemas.ResourceTypeConfig(providerAddr.Provider, addr.Mode, addr.Type) + schema, _, err := d.Evaluator.Plugins.ResourceTypeSchema(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 } diff --git a/internal/terraform/evaluate_test.go b/internal/terraform/evaluate_test.go index f8a46d4fc..ffd067f3f 100644 --- a/internal/terraform/evaluate_test.go +++ b/internal/terraform/evaluate_test.go @@ -193,79 +193,77 @@ func TestEvaluatorGetResource(t *testing.T) { }, }, State: stateSync, - Schemas: &Schemas{ - Providers: map[addrs.Provider]*ProviderSchema{ - addrs.NewDefaultProvider("test"): { - Provider: &configschema.Block{}, - ResourceTypes: map[string]*configschema.Block{ - "test_resource": { - Attributes: map[string]*configschema.Attribute{ - "id": { - Type: cty.String, - Computed: true, - }, - "value": { - Type: cty.String, - Computed: true, - Sensitive: true, - }, + Plugins: schemaOnlyProvidersForTesting(map[addrs.Provider]*ProviderSchema{ + addrs.NewDefaultProvider("test"): { + Provider: &configschema.Block{}, + ResourceTypes: map[string]*configschema.Block{ + "test_resource": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: 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}, - }, + "value": { + Type: cty.String, + Computed: 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": { - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "foo": {Type: cty.String, Optional: true, Sensitive: true}, - }, + Nesting: configschema.NestingList, + }, + "nesting_map": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "foo": {Type: cty.String, Optional: true, Sensitive: true}, }, - Nesting: configschema.NestingMap, }, - "nesting_set": { - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "baz": {Type: cty.String, Optional: true, Sensitive: true}, - }, + Nesting: configschema.NestingMap, + }, + "nesting_set": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "baz": {Type: cty.String, Optional: true, Sensitive: true}, }, - Nesting: configschema.NestingSet, }, - "nesting_single": { - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "boop": {Type: cty.String, Optional: true, Sensitive: true}, - }, + Nesting: configschema.NestingSet, + }, + "nesting_single": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "boop": {Type: cty.String, Optional: true, Sensitive: true}, }, - Nesting: configschema.NestingSingle, }, - "nesting_nesting": { - Block: configschema.Block{ - 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.NestingSingle, + }, + "nesting_nesting": { + Block: configschema.Block{ + 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: 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{ diff --git a/internal/terraform/evaluate_valid.go b/internal/terraform/evaluate_valid.go index 68943a842..232f6913d 100644 --- a/internal/terraform/evaluate_valid.go +++ b/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 diff --git a/internal/terraform/evaluate_valid_test.go b/internal/terraform/evaluate_valid_test.go index ff8ca4397..cfdfdea1f 100644 --- a/internal/terraform/evaluate_valid_test.go +++ b/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{ - Providers: map[addrs.Provider]*ProviderSchema{ - addrs.NewDefaultProvider("aws"): { - ResourceTypes: map[string]*configschema.Block{ - "aws_instance": {}, - }, - }, - addrs.MustParseProviderSourceString("foobar/beep"): { - ResourceTypes: map[string]*configschema.Block{ - // intentional mismatch between resource type prefix and provider type - "boop_instance": {}, - }, + Plugins: schemaOnlyProvidersForTesting(map[addrs.Provider]*ProviderSchema{ + addrs.NewDefaultProvider("aws"): { + ResourceTypes: map[string]*configschema.Block{ + "aws_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 { diff --git a/internal/terraform/graph_walk_context.go b/internal/terraform/graph_walk_context.go index 6fd790e92..39a97032c 100644 --- a/internal/terraform/graph_walk_context.go +++ b/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, }