From 4ef9684188a79c04405dea52efb8f9be003bcaf0 Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Thu, 26 Feb 2026 17:33:01 +0100 Subject: [PATCH] Rework most of the configuration loading We previously used a loader -> BuildConfig flow to load configuration. This commit changes most (but not all yet) flows to use the new graph-based approach. Instead of simply recursively loading the modules, we now need to take a stepped approach: 1. Load the root module 2. Collect the variables and their values 3. Build the configuration with the graph-based approach Because this approach relies on different parts from different packages, it can't easliy be done within the `configload` package. So, now we do most of in the backend or command. --- internal/backend/backendrun/operation.go | 10 -- internal/backend/backendrun/unparsed_value.go | 57 +++++++ internal/backend/local/backend_local.go | 149 +++++++++++------- internal/backend/remote/backend_common.go | 7 +- internal/backend/remote/backend_context.go | 25 ++- internal/checks/state_test.go | 4 +- internal/cloud/backend_common.go | 4 +- internal/cloud/backend_context.go | 27 +++- internal/cloud/test_test.go | 4 +- internal/command/apply.go | 4 +- internal/command/command_test.go | 25 ++- internal/command/graph_test.go | 2 +- internal/command/init2_test.go | 101 ++++++++++++ internal/command/meta.go | 6 - internal/command/meta_config.go | 45 +++++- internal/command/test_test.go | 4 +- .../.terraform/modules/child/empty.tf | 0 .../.terraform/modules/modules.json | 4 +- .../add-version-constraint.tf | 0 .../main.tf} | 2 +- .../local-source-with-version/main.tf} | 2 +- internal/command/validate_test.go | 10 +- internal/configs/config_build.go | 144 ++++++++++++++++- internal/configs/configload/loader.go | 5 + internal/configs/configload/loader_load.go | 14 +- .../configs/configload/loader_load_test.go | 32 +--- .../configs/configload/loader_snapshot.go | 20 +-- .../configload/loader_snapshot_test.go | 145 ----------------- internal/configs/import_test.go | 14 +- .../invalid-files/version-variable.tf | 6 - internal/lang/globalref/analyzer_test.go | 4 +- .../moduletest/graph/eval_context_test.go | 4 +- internal/providercache/installer.go | 9 +- internal/refactoring/move_validate_test.go | 8 +- internal/terraform/terraform_test.go | 84 +++++++++- 35 files changed, 641 insertions(+), 340 deletions(-) create mode 100644 internal/command/init2_test.go rename internal/{configs/configload/testdata => command/testdata/dynamic-module-sources}/add-version-constraint/.terraform/modules/child/empty.tf (100%) rename internal/{configs/configload/testdata => command/testdata/dynamic-module-sources}/add-version-constraint/.terraform/modules/modules.json (61%) rename internal/{configs/configload/testdata => command/testdata/dynamic-module-sources}/add-version-constraint/add-version-constraint.tf (100%) rename internal/{configs/testdata/error-files/module-local-source-with-version.tf => command/testdata/dynamic-module-sources/invalid-registry-source-with-module/main.tf} (57%) rename internal/{configs/testdata/error-files/module-invalid-registry-source-with-module.tf => command/testdata/dynamic-module-sources/local-source-with-version/main.tf} (50%) delete mode 100644 internal/configs/configload/loader_snapshot_test.go delete mode 100644 internal/configs/testdata/invalid-files/version-variable.tf diff --git a/internal/backend/backendrun/operation.go b/internal/backend/backendrun/operation.go index 071f7c7583..4168f7dbc1 100644 --- a/internal/backend/backendrun/operation.go +++ b/internal/backend/backendrun/operation.go @@ -14,7 +14,6 @@ import ( "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" - "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/plans" @@ -175,15 +174,6 @@ func (o *Operation) HasConfig() bool { return o.ConfigLoader.IsConfigDir(o.ConfigDir) } -// Config loads the configuration that the operation applies to, using the -// ConfigDir and ConfigLoader fields within the receiving operation. -func (o *Operation) Config() (*configs.Config, tfdiags.Diagnostics) { - var diags tfdiags.Diagnostics - config, hclDiags := o.ConfigLoader.LoadConfig(o.ConfigDir) - diags = diags.Append(hclDiags) - return config, diags -} - // ReportResult is a helper for the common chore of setting the status of // a running operation and showing any diagnostics produced during that // operation. diff --git a/internal/backend/backendrun/unparsed_value.go b/internal/backend/backendrun/unparsed_value.go index 95d8b3f8d6..bf83dc933e 100644 --- a/internal/backend/backendrun/unparsed_value.go +++ b/internal/backend/backendrun/unparsed_value.go @@ -200,3 +200,60 @@ func ParseVariableValues(vv map[string]arguments.UnparsedVariableValue, decls ma return ret, diags } + +func ParseConstVariableValues(vv map[string]arguments.UnparsedVariableValue, decls map[string]*configs.Variable) (terraform.InputValues, tfdiags.Diagnostics) { + ret, diags := ParseDeclaredVariableValues(vv, decls) + undeclared, diagsUndeclared := ParseUndeclaredVariableValues(vv, decls) + + diags = diags.Append(diagsUndeclared) + + // By this point we should've gathered all of the required root module + // variables from one of the many possible sources. We'll now populate + // any we haven't gathered as unset placeholders which Terraform Core + // can then react to. + for name, vc := range decls { + if isDefinedAny(name, ret, undeclared) { + continue + } + + // This check is redundant with a check made in Terraform Core when + // processing undeclared variables, but allows us to generate a more + // specific error message which mentions -var and -var-file command + // line options, whereas the one in Terraform Core is more general + // due to supporting both root and child module variables. + if vc.Const && vc.Required() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "No value for required variable", + Detail: fmt.Sprintf("The root module input variable %q is not set, and has no default value. Use a -var or -var-file command line argument to provide a value for this variable.", name), + Subject: vc.DeclRange.Ptr(), + }) + } + + if vc.Required() { + // We'll include a placeholder value anyway, just so that our + // result is complete for any calling code that wants to cautiously + // analyze it for diagnostic purposes. Since our diagnostics now + // includes an error, normal processing will ignore this result. + ret[name] = &terraform.InputValue{ + Value: cty.DynamicVal, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRangeFromHCL(vc.DeclRange), + } + } else { + // We're still required to put an entry for this variable + // in the mapping to be explicit to Terraform Core that we + // visited it, but its value will be cty.NilVal to represent + // that it wasn't set at all at this layer, and so Terraform Core + // should substitute a default if available, or generate an error + // if not. + ret[name] = &terraform.InputValue{ + Value: cty.NilVal, + SourceType: terraform.ValueFromConfig, + SourceRange: tfdiags.SourceRangeFromHCL(vc.DeclRange), + } + } + } + + return ret, diags +} diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index a179352aed..768b94355a 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -146,39 +146,11 @@ func (b *Local) localRun(op *backendrun.Operation) (*backendrun.LocalRun, *confi func (b *Local) localRunDirect(op *backendrun.Operation, run *backendrun.LocalRun, coreOpts *terraform.ContextOpts, s statemgr.Full) (*backendrun.LocalRun, *configload.Snapshot, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - // Load the configuration using the caller-provided configuration loader. - config, configSnap, configDiags := op.ConfigLoader.LoadConfigWithSnapshot(op.ConfigDir) + rootMod, configDiags := op.ConfigLoader.LoadRootModule(op.ConfigDir) diags = diags.Append(configDiags) if configDiags.HasErrors() { return nil, nil, diags } - run.Config = config - - if errs := config.VerifyDependencySelections(op.DependencyLocks); len(errs) > 0 { - var buf strings.Builder - for _, err := range errs { - fmt.Fprintf(&buf, "\n - %s", err.Error()) - } - var suggestion string - switch { - case op.DependencyLocks == nil: - // If we get here then it suggests that there's a caller that we - // didn't yet update to populate DependencyLocks, which is a bug. - suggestion = "This run has no dependency lock information provided at all, which is a bug in Terraform; please report it!" - case op.DependencyLocks.Empty(): - suggestion = "To make the initial dependency selections that will initialize the dependency lock file, run:\n terraform init" - default: - suggestion = "To update the locked dependency selections to match a changed configuration, run:\n terraform init -upgrade" - } - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Inconsistent dependency lock file", - fmt.Sprintf( - "The following dependency selections recorded in the lock file are inconsistent with the current configuration:%s\n\n%s", - buf.String(), suggestion, - ), - )) - } var rawVariables map[string]arguments.UnparsedVariableValue if op.AllowUnsetVariables { @@ -186,16 +158,16 @@ func (b *Local) localRunDirect(op *backendrun.Operation, run *backendrun.LocalRu // but unset variables with unknown values to represent that they are // placeholders for values the user would need to provide for other // operations. - rawVariables = b.stubUnsetRequiredVariables(op.Variables, config.Module.Variables) + rawVariables = b.stubUnsetRequiredVariables(op.Variables, rootMod.Variables) } else { // If interactive input is enabled, we might gather some more variable // values through interactive prompts. // TODO: Need to route the operation context through into here, so that // the interactive prompts can be sensitive to its timeouts/etc. - rawVariables = b.interactiveCollectVariables(context.TODO(), op.Variables, config.Module.Variables, op.UIIn) + rawVariables = b.interactiveCollectVariables(context.TODO(), op.Variables, rootMod.Variables, op.UIIn) } - variables, varDiags := backendrun.ParseVariableValues(rawVariables, config.Module.Variables) + variables, varDiags := backendrun.ParseVariableValues(rawVariables, rootMod.Variables) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, nil, diags @@ -224,6 +196,52 @@ func (b *Local) localRunDirect(op *backendrun.Operation, run *backendrun.LocalRu return nil, nil, diags } run.Core = tfCtx + + walkerSnapshot, configSnap := op.ConfigLoader.ModuleWalkerSnapshot() + config, buildDiags := terraform.BuildConfigWithGraph( + rootMod, + walkerSnapshot, + variables, + configs.MockDataLoaderFunc(op.ConfigLoader.LoadExternalMockData), + ) + diags = diags.Append(buildDiags) + if buildDiags.HasErrors() { + return nil, nil, diags + } + run.Config = config + + snapDiags := op.ConfigLoader.AddRootModuleToSnapshot(configSnap, op.ConfigDir) + diags = diags.Append(snapDiags) + if snapDiags.HasErrors() { + return nil, nil, diags + } + + if errs := config.VerifyDependencySelections(op.DependencyLocks); len(errs) > 0 { + var buf strings.Builder + for _, err := range errs { + fmt.Fprintf(&buf, "\n - %s", err.Error()) + } + var suggestion string + switch { + case op.DependencyLocks == nil: + // If we get here then it suggests that there's a caller that we + // didn't yet update to populate DependencyLocks, which is a bug. + suggestion = "This run has no dependency lock information provided at all, which is a bug in Terraform; please report it!" + case op.DependencyLocks.Empty(): + suggestion = "To make the initial dependency selections that will initialize the dependency lock file, run:\n terraform init" + default: + suggestion = "To update the locked dependency selections to match a changed configuration, run:\n terraform init -upgrade" + } + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Inconsistent dependency lock file", + fmt.Sprintf( + "The following dependency selections recorded in the lock file are inconsistent with the current configuration:%s\n\n%s", + buf.String(), suggestion, + ), + )) + } + return run, configSnap, diags } @@ -235,6 +253,7 @@ func (b *Local) localRunForPlanFile(op *backendrun.Operation, pf *planfile.Reade // A plan file has a snapshot of configuration embedded inside it, which // is used instead of whatever configuration might be already present // in the filesystem. + //TODO why not use pf.ReadConfig? snap, err := pf.ReadConfigSnapshot() if err != nil { diags = diags.Append(tfdiags.Sourceless( @@ -246,32 +265,16 @@ func (b *Local) localRunForPlanFile(op *backendrun.Operation, pf *planfile.Reade } loader := configload.NewLoaderFromSnapshot(snap) loader.AllowLanguageExperiments(op.ConfigLoader.AllowsLanguageExperiments()) - config, configDiags := loader.LoadConfig(snap.Modules[""].Dir) - diags = diags.Append(configDiags) - if configDiags.HasErrors() { - return nil, snap, diags + rootMod, rootDiags := loader.LoadRootModule(snap.Modules[""].Dir) + diags = diags.Append(rootDiags) + if rootDiags.HasErrors() { + return nil, nil, diags } - run.Config = config - // NOTE: We're intentionally comparing the current locks with the - // configuration snapshot, rather than the lock snapshot in the plan file, - // because it's the current locks which dictate our plugin selections - // in coreOpts below. However, we'll also separately check that the - // plan file has identical locked plugins below, and thus we're effectively - // checking consistency with both here. - if errs := config.VerifyDependencySelections(op.DependencyLocks); len(errs) > 0 { - var buf strings.Builder - for _, err := range errs { - fmt.Fprintf(&buf, "\n - %s", err.Error()) - } - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Inconsistent dependency lock file", - fmt.Sprintf( - "The following dependency selections recorded in the lock file are inconsistent with the configuration in the saved plan:%s\n\nA saved plan can be applied only to the same configuration it was created from. Create a new plan from the updated configuration.", - buf.String(), - ), - )) + variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables) + diags = diags.Append(varDiags) + if diags.HasErrors() { + return nil, nil, diags } // This check is an important complement to the check above: the locked @@ -359,6 +362,40 @@ func (b *Local) localRunForPlanFile(op *backendrun.Operation, pf *planfile.Reade return nil, nil, diags } run.Core = tfCtx + + config, buildDiags := terraform.BuildConfigWithGraph( + rootMod, + loader.ModuleWalker(), + variables, + configs.MockDataLoaderFunc(loader.LoadExternalMockData), + ) + diags = diags.Append(buildDiags) + if buildDiags.HasErrors() { + return nil, nil, diags + } + run.Config = config + + // NOTE: We're intentionally comparing the current locks with the + // configuration snapshot, rather than the lock snapshot in the plan file, + // because it's the current locks which dictate our plugin selections + // in coreOpts below. However, we'll also separately check that the + // plan file has identical locked plugins below, and thus we're effectively + // checking consistency with both here. + if errs := config.VerifyDependencySelections(op.DependencyLocks); len(errs) > 0 { + var buf strings.Builder + for _, err := range errs { + fmt.Fprintf(&buf, "\n - %s", err.Error()) + } + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Inconsistent dependency lock file", + fmt.Sprintf( + "The following dependency selections recorded in the lock file are inconsistent with the configuration in the saved plan:%s\n\nA saved plan can be applied only to the same configuration it was created from. Create a new plan from the updated configuration.", + buf.String(), + ), + )) + } + return run, snap, diags } diff --git a/internal/backend/remote/backend_common.go b/internal/backend/remote/backend_common.go index 85d42ae879..137c01aa41 100644 --- a/internal/backend/remote/backend_common.go +++ b/internal/backend/remote/backend_common.go @@ -249,11 +249,8 @@ func (b *Remote) waitForRun(stopCtx, cancelCtx context.Context, op *backendrun.O // remote system's responsibility to do final validation of the input. func (b *Remote) hasExplicitVariableValues(op *backendrun.Operation) bool { // Load the configuration using the caller-provided configuration loader. - config, _, configDiags := op.ConfigLoader.LoadConfigWithSnapshot(op.ConfigDir) + config, configDiags := op.ConfigLoader.LoadRootModule(op.ConfigDir) if configDiags.HasErrors() { - // If we can't load the configuration then we'll assume no explicit - // variable values just to let the remote operation start and let - // the remote system return the same set of configuration errors. return false } @@ -262,7 +259,7 @@ func (b *Remote) hasExplicitVariableValues(op *backendrun.Operation) bool { // goal here is just to make a best effort count of how many variable // values are coming from -var or -var-file CLI arguments so that we can // hint the user that those are not supported for remote operations. - variables, _ := backendrun.ParseVariableValues(op.Variables, config.Module.Variables) + variables, _ := backendrun.ParseVariableValues(op.Variables, config.Variables) // Check for explicitly-defined (-var and -var-file) variables, which the // remote backend does not support. All other source types are okay, diff --git a/internal/backend/remote/backend_context.go b/internal/backend/remote/backend_context.go index bf7ec7190d..db72d709ed 100644 --- a/internal/backend/remote/backend_context.go +++ b/internal/backend/remote/backend_context.go @@ -81,19 +81,18 @@ func (b *Remote) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, state ret.InputState = stateMgr.State() log.Printf("[TRACE] backend/remote: loading configuration for the current working directory") - config, configDiags := op.ConfigLoader.LoadConfig(op.ConfigDir) + rootMod, configDiags := op.ConfigLoader.LoadRootModule(op.ConfigDir) diags = diags.Append(configDiags) if configDiags.HasErrors() { return nil, nil, diags } - ret.Config = config if op.AllowUnsetVariables { // If we're not going to use the variables in an operation we'll be // more lax about them, stubbing out any unset ones as unknown. // This gives us enough information to produce a consistent context, // but not enough information to run a real operation (plan, apply, etc) - ret.PlanOpts.SetVariables = stubAllVariables(op.Variables, config.Module.Variables) + ret.PlanOpts.SetVariables = stubAllVariables(op.Variables, rootMod.Variables) } else { // The underlying API expects us to use the opaque workspace id to request // variables, so we'll need to look that up using our organization name @@ -136,7 +135,7 @@ func (b *Remote) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, state } if op.Variables != nil { - variables, varDiags := backendrun.ParseVariableValues(op.Variables, config.Module.Variables) + variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, nil, diags @@ -148,6 +147,24 @@ func (b *Remote) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, state tfCtx, ctxDiags := terraform.NewContext(&opts) diags = diags.Append(ctxDiags) ret.Core = tfCtx + if diags.HasErrors() { + return nil, nil, diags + } + + log.Printf("[TRACE] backend/remote: building configuration for the current working directory") + + config, buildDiags := terraform.BuildConfigWithGraph( + rootMod, + op.ConfigLoader.ModuleWalker(), + ret.PlanOpts.SetVariables, + configs.MockDataLoaderFunc(op.ConfigLoader.LoadExternalMockData), + ) + diags = diags.Append(buildDiags) + if diags.HasErrors() { + return nil, nil, diags + } + + ret.Config = config log.Printf("[TRACE] backend/remote: finished building terraform.Context") diff --git a/internal/checks/state_test.go b/internal/checks/state_test.go index 2e5a30a30d..276f6dc62c 100644 --- a/internal/checks/state_test.go +++ b/internal/checks/state_test.go @@ -18,7 +18,7 @@ func TestChecksHappyPath(t *testing.T) { const fixtureDir = "testdata/happypath" loader, close := configload.NewLoaderForTests(t) defer close() - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, nil) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, nil, nil) _, instDiags := inst.InstallModules(context.Background(), fixtureDir, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) @@ -29,7 +29,7 @@ func TestChecksHappyPath(t *testing.T) { ///////////////////////////////////////////////////////////////////////// - cfg, hclDiags := loader.LoadConfig(fixtureDir) + cfg, hclDiags := loader.LoadStaticConfig(fixtureDir) if hclDiags.HasErrors() { t.Fatalf("invalid configuration: %s", hclDiags.Error()) } diff --git a/internal/cloud/backend_common.go b/internal/cloud/backend_common.go index 360da2a08b..b4f98ef2d0 100644 --- a/internal/cloud/backend_common.go +++ b/internal/cloud/backend_common.go @@ -654,12 +654,12 @@ in order to capture the filesystem context the remote workspace expects: } func (b *Cloud) parseRunVariables(op *backendrun.Operation) ([]*tfe.RunVariable, error) { - config, _, configDiags := op.ConfigLoader.LoadConfigWithSnapshot(op.ConfigDir) + config, configDiags := op.ConfigLoader.LoadRootModule(op.ConfigDir) if configDiags.HasErrors() { return nil, fmt.Errorf("error loading config with snapshot: %w", configDiags.Errs()[0]) } - variables, varDiags := ParseCloudRunVariables(op.Variables, config.Module.Variables) + variables, varDiags := ParseCloudRunVariables(op.Variables, config.Variables) if varDiags.HasErrors() { return nil, varDiags.Err() diff --git a/internal/cloud/backend_context.go b/internal/cloud/backend_context.go index 36f9ab1e3a..e3fc587a33 100644 --- a/internal/cloud/backend_context.go +++ b/internal/cloud/backend_context.go @@ -79,20 +79,19 @@ func (b *Cloud) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, statem log.Printf("[TRACE] cloud: retrieving remote state snapshot for workspace %q", remoteWorkspaceName) ret.InputState = stateMgr.State() - log.Printf("[TRACE] cloud: loading configuration for the current working directory") - config, configDiags := op.ConfigLoader.LoadConfig(op.ConfigDir) + log.Printf("[TRACE] cloud: loading root module for the current working directory") + rootMod, configDiags := op.ConfigLoader.LoadRootModule(op.ConfigDir) diags = diags.Append(configDiags) if configDiags.HasErrors() { return nil, nil, diags } - ret.Config = config if op.AllowUnsetVariables { // If we're not going to use the variables in an operation we'll be // more lax about them, stubbing out any unset ones as unknown. // This gives us enough information to produce a consistent context, // but not enough information to run a real operation (plan, apply, etc) - ret.PlanOpts.SetVariables = stubAllVariables(op.Variables, config.Module.Variables) + ret.PlanOpts.SetVariables = stubAllVariables(op.Variables, rootMod.Variables) } else { // The underlying API expects us to use the opaque workspace id to request // variables, so we'll need to look that up using our organization name @@ -136,7 +135,7 @@ func (b *Cloud) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, statem } if op.Variables != nil { - variables, varDiags := backendrun.ParseVariableValues(op.Variables, config.Module.Variables) + variables, varDiags := backendrun.ParseVariableValues(op.Variables, rootMod.Variables) diags = diags.Append(varDiags) if diags.HasErrors() { return nil, nil, diags @@ -148,6 +147,24 @@ func (b *Cloud) LocalRun(op *backendrun.Operation) (*backendrun.LocalRun, statem tfCtx, ctxDiags := terraform.NewContext(&opts) diags = diags.Append(ctxDiags) ret.Core = tfCtx + if diags.HasErrors() { + return nil, nil, diags + } + + log.Printf("[TRACE] cloud: building configuration for the current working directory") + + config, buildDiags := terraform.BuildConfigWithGraph( + rootMod, + op.ConfigLoader.ModuleWalker(), + ret.PlanOpts.SetVariables, + configs.MockDataLoaderFunc(op.ConfigLoader.LoadExternalMockData), + ) + diags = diags.Append(buildDiags) + if diags.HasErrors() { + return nil, nil, diags + } + + ret.Config = config log.Printf("[TRACE] cloud: finished building terraform.Context") diff --git a/internal/cloud/test_test.go b/internal/cloud/test_test.go index d3a97372bc..ca1971ff7e 100644 --- a/internal/cloud/test_test.go +++ b/internal/cloud/test_test.go @@ -267,7 +267,7 @@ func TestTest_Verbose(t *testing.T) { loader, close := configload.NewLoaderForTests(t) defer close() - config, configDiags := loader.LoadConfigWithTests(directory, "tests") + config, configDiags := loader.LoadStaticConfigWithTests(directory, "tests") if configDiags.HasErrors() { t.Fatalf("failed to load config: %v", configDiags.Error()) } @@ -664,7 +664,7 @@ func TestTest_ForceCancel(t *testing.T) { loader, close := configload.NewLoaderForTests(t) defer close() - config, configDiags := loader.LoadConfigWithTests("testdata/test-force-cancel", "tests") + config, configDiags := loader.LoadStaticConfigWithTests("testdata/test-force-cancel", "tests") if configDiags.HasErrors() { t.Fatalf("failed to load config: %v", configDiags.Error()) } diff --git a/internal/command/apply.go b/internal/command/apply.go index 1fa43cf950..29cfb7698e 100644 --- a/internal/command/apply.go +++ b/internal/command/apply.go @@ -359,7 +359,7 @@ Options: Defaults to 10. -replace=resource Terraform will plan to replace this resource instance - instead of doing an update or no-op action. + instead of doing an update or no-op action. -state=path Path to read and save state (unless state-out is specified). Defaults to "terraform.tfstate". @@ -372,7 +372,7 @@ Options: Legacy option for the local backend only. See the local backend's documentation for more information. - + -var 'foo=bar' Set a value for one of the input variables in the root module of the configuration. Use this option more than once to set more than one variable. diff --git a/internal/command/command_test.go b/internal/command/command_test.go index 6be429ba62..8eecaf1ef2 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -50,6 +50,7 @@ import ( "github.com/hashicorp/terraform/internal/states/statefile" "github.com/hashicorp/terraform/internal/states/statemgr" "github.com/hashicorp/terraform/internal/terminal" + "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/version" ) @@ -158,15 +159,31 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config // Test modules usually do not refer to remote sources, and for local // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) } - config, snap, diags := loader.LoadConfigWithSnapshot(dir) - if diags.HasErrors() { - t.Fatal(diags.Error()) + rootMod, configDiags := loader.LoadRootModule(dir) + if configDiags.HasErrors() { + t.Fatal(configDiags.Error()) + } + + walkerSnapshot, snap := loader.ModuleWalkerSnapshot() + config, buildDiags := terraform.BuildConfigWithGraph( + rootMod, + walkerSnapshot, + nil, + configs.MockDataLoaderFunc(loader.LoadExternalMockData), + ) + if buildDiags.HasErrors() { + t.Fatal(buildDiags.Err()) + } + + snapDiags := loader.AddRootModuleToSnapshot(snap, dir) + if snapDiags.HasErrors() { + t.Fatal(snapDiags.Error()) } return config, snap diff --git a/internal/command/graph_test.go b/internal/command/graph_test.go index 97bdc78621..9490ed130c 100644 --- a/internal/command/graph_test.go +++ b/internal/command/graph_test.go @@ -224,7 +224,7 @@ func TestGraph_resourcesOnly(t *testing.T) { if err != nil { t.Fatal(err) } - inst := initwd.NewModuleInstaller(".terraform/modules", loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(".terraform/modules", loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), ".", "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) diff --git a/internal/command/init2_test.go b/internal/command/init2_test.go new file mode 100644 index 0000000000..9dcfcf50c4 --- /dev/null +++ b/internal/command/init2_test.go @@ -0,0 +1,101 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package command + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/hashicorp/cli" +) + +func TestInit2_versionConstraintAdded(t *testing.T) { + // This test is for what happens when there is a version constraint added + // to a module that previously didn't have one. + td := t.TempDir() + testCopyDir(t, testFixturePath(filepath.Join("dynamic-module-sources", "add-version-constraint")), td) + t.Chdir(td) + + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } + + args := []string{"-get=false"} + code := c.Run(args) + testOutput := done(t) + if code != 1 { + t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, testOutput.Stderr(), testOutput.Stdout()) + } + got := testOutput.All() + + 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 TestInit2_invalidRegistrySourceWithModule(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath(filepath.Join("dynamic-module-sources", "invalid-registry-source-with-module")), td) + t.Chdir(td) + + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } + + args := []string{} + code := c.Run(args) + testOutput := done(t) + if code != 1 { + t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, testOutput.Stderr(), testOutput.Stdout()) + } + got := testOutput.All() + + want := "Invalid registry module source address" + if !strings.Contains(got, want) { + t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want) + } +} + +func TestInit2_localSourceWithVersion(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath(filepath.Join("dynamic-module-sources", "local-source-with-version")), td) + t.Chdir(td) + + ui := new(cli.MockUi) + view, done := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + View: view, + }, + } + + args := []string{} + code := c.Run(args) + testOutput := done(t) + if code != 1 { + t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, testOutput.Stderr(), testOutput.Stdout()) + } + got := testOutput.All() + + want := "Invalid registry module source address" + if !strings.Contains(got, want) { + t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want) + } +} diff --git a/internal/command/meta.go b/internal/command/meta.go index cebd76f7f8..bbdf4ed09f 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -825,12 +825,6 @@ func (m *Meta) applyStateArguments(args *arguments.State) { func (m *Meta) checkRequiredVersion() tfdiags.Diagnostics { var diags tfdiags.Diagnostics - loader, err := m.initConfigLoader() - if err != nil { - diags = diags.Append(err) - return diags - } - pwd, err := os.Getwd() if err != nil { diags = diags.Append(fmt.Errorf("Error getting pwd: %s", err)) diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index c99898d74f..15b56cc2ee 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -17,6 +17,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "github.com/hashicorp/terraform/internal/backend/backendrun" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/configs/configschema" @@ -48,8 +49,28 @@ func (m *Meta) loadConfig(rootDir string) (*configs.Config, tfdiags.Diagnostics) return nil, diags } - config, hclDiags := loader.LoadConfig(rootDir) + rootMod, hclDiags := loader.LoadRootModule(rootDir) diags = diags.Append(hclDiags) + if rootMod == nil || diags.HasErrors() { + cfg := &configs.Config{ + Module: rootMod, + } + cfg.Root = cfg // Root module is self-referential. + return cfg, diags + } + betterVars, parseDiags := backendrun.ParseVariableValues(m.VariableValues, rootMod.Variables) + diags = diags.Append(parseDiags) + if parseDiags.HasErrors() { + return nil, diags + } + config, buildDiags := terraform.BuildConfigWithGraph( + rootMod, + loader.ModuleWalker(), + betterVars, + configs.MockDataLoaderFunc(loader.LoadExternalMockData), + ) + diags = diags.Append(buildDiags) + return config, diags } @@ -65,8 +86,28 @@ func (m *Meta) loadConfigWithTests(rootDir, testDir string) (*configs.Config, tf return nil, diags } - config, hclDiags := loader.LoadConfigWithTests(rootDir, testDir) + rootMod, hclDiags := loader.LoadRootModuleWithTests(rootDir, testDir) diags = diags.Append(hclDiags) + if rootMod == nil || diags.HasErrors() { + cfg := &configs.Config{ + Module: rootMod, + } + cfg.Root = cfg // Root module is self-referential. + return cfg, diags + } + betterVars, parseDiags := backendrun.ParseConstVariableValues(m.VariableValues, rootMod.Variables) + diags = diags.Append(parseDiags) + if parseDiags.HasErrors() { + return nil, diags + } + config, buildDiags := terraform.BuildConfigWithGraph( + rootMod, + loader.ModuleWalker(), + betterVars, + configs.MockDataLoaderFunc(loader.LoadExternalMockData), + ) + diags = diags.Append(buildDiags) + return config, diags } diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 92adc85738..01d3f548de 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -5707,7 +5707,7 @@ func testModuleInline(t *testing.T, sources map[string]string) (*configs.Config, // Test modules usually do not refer to remote sources, and for local // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), cfgPath, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) @@ -5719,7 +5719,7 @@ func testModuleInline(t *testing.T, sources map[string]string) (*configs.Config, t.Fatalf("failed to refresh modules after installation: %s", err) } - config, diags := loader.LoadConfigWithTests(cfgPath, "tests") + config, diags := loader.LoadStaticConfigWithTests(cfgPath, "tests") if diags.HasErrors() { t.Fatal(diags.Error()) } diff --git a/internal/configs/configload/testdata/add-version-constraint/.terraform/modules/child/empty.tf b/internal/command/testdata/dynamic-module-sources/add-version-constraint/.terraform/modules/child/empty.tf similarity index 100% rename from internal/configs/configload/testdata/add-version-constraint/.terraform/modules/child/empty.tf rename to internal/command/testdata/dynamic-module-sources/add-version-constraint/.terraform/modules/child/empty.tf diff --git a/internal/configs/configload/testdata/add-version-constraint/.terraform/modules/modules.json b/internal/command/testdata/dynamic-module-sources/add-version-constraint/.terraform/modules/modules.json similarity index 61% rename from internal/configs/configload/testdata/add-version-constraint/.terraform/modules/modules.json rename to internal/command/testdata/dynamic-module-sources/add-version-constraint/.terraform/modules/modules.json index c02f40016b..a55de19395 100644 --- a/internal/configs/configload/testdata/add-version-constraint/.terraform/modules/modules.json +++ b/internal/command/testdata/dynamic-module-sources/add-version-constraint/.terraform/modules/modules.json @@ -3,12 +3,12 @@ { "Key": "", "Source": "", - "Dir": "testdata/add-version-constraint" + "Dir": "" }, { "Key": "child", "Source": "hashicorp/module-installer-acctest/aws", - "Dir": "testdata/add-version-constraint/.terraform/modules/child" + "Dir": ".terraform/modules/child" } ] } diff --git a/internal/configs/configload/testdata/add-version-constraint/add-version-constraint.tf b/internal/command/testdata/dynamic-module-sources/add-version-constraint/add-version-constraint.tf similarity index 100% rename from internal/configs/configload/testdata/add-version-constraint/add-version-constraint.tf rename to internal/command/testdata/dynamic-module-sources/add-version-constraint/add-version-constraint.tf diff --git a/internal/configs/testdata/error-files/module-local-source-with-version.tf b/internal/command/testdata/dynamic-module-sources/invalid-registry-source-with-module/main.tf similarity index 57% rename from internal/configs/testdata/error-files/module-local-source-with-version.tf rename to internal/command/testdata/dynamic-module-sources/invalid-registry-source-with-module/main.tf index f570d65fe9..99036006e8 100644 --- a/internal/configs/testdata/error-files/module-local-source-with-version.tf +++ b/internal/command/testdata/dynamic-module-sources/invalid-registry-source-with-module/main.tf @@ -1,5 +1,5 @@ module "test" { - source = "../boop" # ERROR: Invalid registry module source address + source = "---.com/HashiCorp/Consul/aws" version = "1.0.0" # Makes Terraform assume "source" is a module address } diff --git a/internal/configs/testdata/error-files/module-invalid-registry-source-with-module.tf b/internal/command/testdata/dynamic-module-sources/local-source-with-version/main.tf similarity index 50% rename from internal/configs/testdata/error-files/module-invalid-registry-source-with-module.tf rename to internal/command/testdata/dynamic-module-sources/local-source-with-version/main.tf index 0029be8f4a..6ff0bcd606 100644 --- a/internal/configs/testdata/error-files/module-invalid-registry-source-with-module.tf +++ b/internal/command/testdata/dynamic-module-sources/local-source-with-version/main.tf @@ -1,5 +1,5 @@ module "test" { - source = "---.com/HashiCorp/Consul/aws" # ERROR: Invalid registry module source address + source = "../boop" version = "1.0.0" # Makes Terraform assume "source" is a module address } diff --git a/internal/command/validate_test.go b/internal/command/validate_test.go index 9270066ee9..3e4ef667e9 100644 --- a/internal/command/validate_test.go +++ b/internal/command/validate_test.go @@ -5,7 +5,7 @@ package command import ( "encoding/json" - "io/ioutil" + "io" "os" "path" "strings" @@ -190,10 +190,6 @@ func TestModuleWithIncorrectNameShouldFail(t *testing.T) { if !strings.Contains(output.Stderr(), wantError) { t.Fatalf("Missing error string %q\n\n'%s'", wantError, output.Stderr()) } - wantError = `Error: Variables not allowed` - if !strings.Contains(output.Stderr(), wantError) { - t.Fatalf("Missing error string %q\n\n'%s'", wantError, output.Stderr()) - } } func TestWronglyUsedInterpolationShouldFail(t *testing.T) { @@ -406,14 +402,14 @@ func TestValidate_json(t *testing.T) { for _, tc := range tests { t.Run(tc.path, func(t *testing.T) { - var want, got map[string]interface{} + var want, got map[string]any wantFile, err := os.Open(path.Join(testFixturePath(tc.path), "output.json")) if err != nil { t.Fatalf("failed to open output file: %s", err) } defer wantFile.Close() - wantBytes, err := ioutil.ReadAll(wantFile) + wantBytes, err := io.ReadAll(wantFile) if err != nil { t.Fatalf("failed to read output file: %s", err) } diff --git a/internal/configs/config_build.go b/internal/configs/config_build.go index 28e522d226..84abf6df3b 100644 --- a/internal/configs/config_build.go +++ b/internal/configs/config_build.go @@ -12,8 +12,10 @@ import ( version "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/getmodules/moduleaddrs" ) // BuildConfig constructs a Config from a root module by loading all of its @@ -32,6 +34,20 @@ func BuildConfig(root *Module, walker ModuleWalker, loader MockDataLoader) (*Con } cfg.Root = cfg // Root module is self-referential. cfg.Children, diags = buildChildModules(cfg, walker) + diags = append(diags, FinalizeConfig(cfg, walker, loader)...) + + return cfg, diags +} + +// FinalizeConfig performs the post-load validation and setup steps that are +// shared by different configuration loaders. +// +// Callers must ensure cfg.Root is set correctly before calling this function. +func FinalizeConfig(cfg *Config, walker ModuleWalker, loader MockDataLoader) hcl.Diagnostics { + var diags hcl.Diagnostics + if cfg == nil { + return diags + } diags = append(diags, buildTestModules(cfg, walker)...) // Skip provider resolution if there are any errors, since the provider @@ -42,7 +58,7 @@ func BuildConfig(root *Module, walker ModuleWalker, loader MockDataLoader) (*Con providers := cfg.resolveProviderTypes() cfg.resolveProviderTypesForTests(providers) - if cfg.Module.StateStore != nil { + if cfg.Module != nil && cfg.Module.StateStore != nil { stateProviderDiags := cfg.resolveStateStoreProviderType() diags = append(diags, stateProviderDiags...) } @@ -54,7 +70,7 @@ func BuildConfig(root *Module, walker ModuleWalker, loader MockDataLoader) (*Con // Final step, let's side load any external mock data into our test files. diags = append(diags, installMockDataFiles(cfg, loader)...) - return cfg, diags + return diags } func installMockDataFiles(root *Config, loader MockDataLoader) hcl.Diagnostics { @@ -148,6 +164,108 @@ func buildTestModules(root *Config, walker ModuleWalker) hcl.Diagnostics { return diags } +// legacySourceHelper is used to decode module sources from the old-style +// string-only "source". It assumes that the expression does not contain any +// references and can be decoded without an evaluation context. +// In the long term, we want to get rid of this helper method. +func legacySourceHelper(expr hcl.Expression, haveVersionArg bool) (addrs.ModuleSource, hcl.Diagnostics) { + var diags hcl.Diagnostics + var sourceAddrRaw string + var addr addrs.ModuleSource + + valDiags := gohcl.DecodeExpression(expr, nil, &sourceAddrRaw) + diags = append(diags, valDiags...) + if !valDiags.HasErrors() { + var err error + if haveVersionArg { + addr, err = moduleaddrs.ParseModuleSourceRegistry(sourceAddrRaw) + } else { + addr, err = moduleaddrs.ParseModuleSource(sourceAddrRaw) + } + if err != nil { + // NOTE: We leave addr as nil for any situation where the + // source attribute is invalid, so any code which tries to carefully + // use the partial result of a failed config decode must be + // resilient to that. + addr = nil + + // NOTE: In practice it's actually very unlikely to end up here, + // because our source address parser can turn just about any string + // into some sort of remote package address, and so for most errors + // we'll detect them only during module installation. There are + // still a _few_ purely-syntax errors we can catch at parsing time, + // though, mostly related to remote package sub-paths and local + // paths. + switch err := err.(type) { + case *moduleaddrs.MaybeRelativePathErr: + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: fmt.Sprintf( + "Terraform failed to determine your intended installation method for remote module package %q.\n\nIf you intended this as a path relative to the current module, use \"./%s\" instead. The \"./\" prefix indicates that the address is a relative filesystem path.", + err.Addr, err.Addr, + ), + Subject: expr.Range().Ptr(), + }) + default: + if haveVersionArg { + // In this case we'll include some extra context that + // we assumed a registry source address due to the + // version argument. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid registry module source address", + Detail: fmt.Sprintf("Failed to parse module registry address: %s.\n\nTerraform assumed that you intended a module registry source address because you also set the argument \"version\", which applies only to registry modules.", err), + Subject: expr.Range().Ptr(), + }) + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: fmt.Sprintf("Failed to parse module source address: %s.", err), + Subject: expr.Range().Ptr(), + }) + } + } + } + } + + return addr, diags +} + +// legacyVersionHelper is used to decode version constraints from the old-style +// string-only "version". It assumes that the expression does not contain any +// references and can be decoded without an evaluation context. +// In the long term, we want to get rid of this helper method. +func legacyVersionHelper(expr hcl.Expression) (VersionConstraint, hcl.Diagnostics) { + var diags hcl.Diagnostics + var versionRaw string + + ret := VersionConstraint{ + DeclRange: expr.Range(), + } + + valDiags := gohcl.DecodeExpression(expr, nil, &versionRaw) + diags = append(diags, valDiags...) + if !valDiags.HasErrors() { + constraints, err := version.NewConstraint(versionRaw) + if err != nil { + // NewConstraint doesn't return user-friendly errors, so we'll just + // ignore the provided error and produce our own generic one. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + Detail: "This string does not use correct version constraint syntax.", // Not very actionable :( + Subject: expr.Range().Ptr(), + }) + return ret, diags + } + ret.Required = constraints + } + + return ret, diags +} + func buildChildModules(parent *Config, walker ModuleWalker) (map[string]*Config, hcl.Diagnostics) { var diags hcl.Diagnostics ret := map[string]*Config{} @@ -161,12 +279,28 @@ func buildChildModules(parent *Config, walker ModuleWalker) (map[string]*Config, path := slices.Clone(parent.Path) path = append(path, call.Name) + sourceAddr, sourceDiags := legacySourceHelper(call.SourceExpr, call.VersionExpr != nil) + diags = append(diags, sourceDiags...) + if sourceDiags.HasErrors() { + continue + } + + var versionConstraint VersionConstraint + if call.VersionExpr != nil { + var versionDiags hcl.Diagnostics + versionConstraint, versionDiags = legacyVersionHelper(call.VersionExpr) + diags = append(diags, versionDiags...) + if versionDiags.HasErrors() { + continue + } + } + req := ModuleRequest{ Name: call.Name, Path: path, - SourceAddr: call.SourceAddr, - SourceAddrRange: call.SourceAddrRange, - VersionConstraint: call.Version, + SourceAddr: sourceAddr, + SourceAddrRange: call.SourceExpr.Range(), + VersionConstraint: versionConstraint, Parent: parent, CallRange: call.DeclRange, } diff --git a/internal/configs/configload/loader.go b/internal/configs/configload/loader.go index 11be47b02f..06becad468 100644 --- a/internal/configs/configload/loader.go +++ b/internal/configs/configload/loader.go @@ -187,3 +187,8 @@ func (l *Loader) AllowLanguageExperiments(allowed bool) { func (l *Loader) AllowsLanguageExperiments() bool { return l.parser.AllowsLanguageExperiments() } + +// ModuleWalker returns a walker suitable for loading already-installed modules. +func (l *Loader) ModuleWalker() configs.ModuleWalker { + return configs.ModuleWalkerFunc(l.moduleWalkerLoad) +} diff --git a/internal/configs/configload/loader_load.go b/internal/configs/configload/loader_load.go index 236575dcd2..5fbe017915 100644 --- a/internal/configs/configload/loader_load.go +++ b/internal/configs/configload/loader_load.go @@ -22,16 +22,26 @@ import ( // // LoadConfig performs the basic syntax and uniqueness validations that are // required to process the individual modules -func (l *Loader) LoadConfig(rootDir string) (*configs.Config, hcl.Diagnostics) { +func (l *Loader) LoadStaticConfig(rootDir string) (*configs.Config, hcl.Diagnostics) { return l.loadConfig(l.parser.LoadConfigDir(rootDir, l.parserOpts...)) } // LoadConfigWithTests matches LoadConfig, except the configs.Config contains // any relevant .tftest.hcl files. -func (l *Loader) LoadConfigWithTests(rootDir string, testDir string) (*configs.Config, hcl.Diagnostics) { +func (l *Loader) LoadStaticConfigWithTests(rootDir string, testDir string) (*configs.Config, hcl.Diagnostics) { return l.loadConfig(l.parser.LoadConfigDir(rootDir, append(l.parserOpts, configs.MatchTestFiles(testDir))...)) } +// LoadRootModule reads the root module using the loader's parser options. +func (l *Loader) LoadRootModule(rootDir string) (*configs.Module, hcl.Diagnostics) { + return l.parser.LoadConfigDir(rootDir, l.parserOpts...) +} + +// LoadRootModuleWithTests reads the root module and includes test files from the given directory. +func (l *Loader) LoadRootModuleWithTests(rootDir string, testDir string) (*configs.Module, hcl.Diagnostics) { + return l.parser.LoadConfigDir(rootDir, append(l.parserOpts, configs.MatchTestFiles(testDir))...) +} + func (l *Loader) loadConfig(rootMod *configs.Module, diags hcl.Diagnostics) (*configs.Config, hcl.Diagnostics) { if rootMod == nil || diags.HasErrors() { // Ensure we return any parsed modules here so that required_version diff --git a/internal/configs/configload/loader_load_test.go b/internal/configs/configload/loader_load_test.go index 22fadb6249..b81814960e 100644 --- a/internal/configs/configload/loader_load_test.go +++ b/internal/configs/configload/loader_load_test.go @@ -25,7 +25,7 @@ func TestLoaderLoadConfig_okay(t *testing.T) { t.Fatalf("unexpected error from NewLoader: %s", err) } - cfg, diags := loader.LoadConfig(fixtureDir) + cfg, diags := loader.LoadStaticConfig(fixtureDir) assertNoDiagnostics(t, diags) if cfg == nil { t.Fatalf("config is nil; want non-nil") @@ -62,28 +62,6 @@ func TestLoaderLoadConfig_okay(t *testing.T) { }) } -func TestLoaderLoadConfig_addVersion(t *testing.T) { - // This test is for what happens when there is a version constraint added - // to a module that previously didn't have one. - fixtureDir := filepath.Clean("testdata/add-version-constraint") - 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 version requirements have changed" - if !strings.Contains(got, want) { - t.Fatalf("wrong error\ngot:\n%s\n\nwant: containing %q", got, want) - } -} - func TestLoaderLoadConfig_loadDiags(t *testing.T) { // building a config which didn't load correctly may cause configs to panic fixtureDir := filepath.Clean("testdata/invalid-names") @@ -94,7 +72,7 @@ func TestLoaderLoadConfig_loadDiags(t *testing.T) { t.Fatalf("unexpected error from NewLoader: %s", err) } - cfg, diags := loader.LoadConfig(fixtureDir) + cfg, diags := loader.LoadStaticConfig(fixtureDir) if !diags.HasErrors() { t.Fatal("success; want error") } @@ -118,7 +96,7 @@ func TestLoaderLoadConfig_loadDiagsFromSubmodules(t *testing.T) { t.Fatalf("unexpected error from NewLoader: %s", err) } - cfg, diags := loader.LoadConfig(fixtureDir) + cfg, diags := loader.LoadStaticConfig(fixtureDir) if !diags.HasErrors() { t.Fatalf("loading succeeded; want an error") } @@ -168,7 +146,7 @@ func TestLoaderLoadConfig_childProviderGrandchildCount(t *testing.T) { t.Fatalf("unexpected error from NewLoader: %s", err) } - cfg, diags := loader.LoadConfig(fixtureDir) + cfg, diags := loader.LoadStaticConfig(fixtureDir) assertNoDiagnostics(t, diags) if cfg == nil { t.Fatalf("config is nil; want non-nil") @@ -198,7 +176,7 @@ func TestLoaderLoadConfig_childProviderGrandchildCount(t *testing.T) { t.Fatalf("unexpected error from NewLoader: %s", err) } - _, diags := loader.LoadConfig(fixtureDir) + _, diags := loader.LoadStaticConfig(fixtureDir) if !diags.HasErrors() { t.Fatalf("loading succeeded; want an error") } diff --git a/internal/configs/configload/loader_snapshot.go b/internal/configs/configload/loader_snapshot.go index 5388e8bb1b..ba982527c1 100644 --- a/internal/configs/configload/loader_snapshot.go +++ b/internal/configs/configload/loader_snapshot.go @@ -20,26 +20,16 @@ import ( "github.com/hashicorp/terraform/internal/modsdir" ) -// LoadConfigWithSnapshot is a variant of LoadConfig that also simultaneously -// creates an in-memory snapshot of the configuration files used, which can -// be later used to create a loader that may read only from this snapshot. -func (l *Loader) LoadConfigWithSnapshot(rootDir string) (*configs.Config, *Snapshot, hcl.Diagnostics) { - rootMod, diags := l.parser.LoadConfigDir(rootDir, l.parserOpts...) - if rootMod == nil { - return nil, nil, diags - } - +func (l *Loader) ModuleWalkerSnapshot() (configs.ModuleWalker, *Snapshot) { snap := &Snapshot{ Modules: map[string]*SnapshotModule{}, } - walker := l.makeModuleWalkerSnapshot(snap) - cfg, cDiags := configs.BuildConfig(rootMod, walker, configs.MockDataLoaderFunc(l.LoadExternalMockData)) - diags = append(diags, cDiags...) - addDiags := l.addModuleToSnapshot(snap, "", rootDir, "", nil) - diags = append(diags, addDiags...) + return l.makeModuleWalkerSnapshot(snap), snap +} - return cfg, snap, diags +func (l *Loader) AddRootModuleToSnapshot(snap *Snapshot, rootDir string) hcl.Diagnostics { + return l.addModuleToSnapshot(snap, "", rootDir, "", nil) } // NewLoaderFromSnapshot creates a Loader that reads files only from the diff --git a/internal/configs/configload/loader_snapshot_test.go b/internal/configs/configload/loader_snapshot_test.go deleted file mode 100644 index 00411ecc18..0000000000 --- a/internal/configs/configload/loader_snapshot_test.go +++ /dev/null @@ -1,145 +0,0 @@ -// Copyright IBM Corp. 2014, 2026 -// SPDX-License-Identifier: BUSL-1.1 - -package configload - -import ( - "os" - "path/filepath" - "reflect" - "testing" - - "github.com/davecgh/go-spew/spew" - "github.com/go-test/deep" -) - -func TestLoadConfigWithSnapshot(t *testing.T) { - fixtureDir := filepath.Clean("testdata/already-installed") - loader, err := NewLoader(&Config{ - ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"), - }) - if err != nil { - t.Fatalf("unexpected error from NewLoader: %s", err) - } - - _, got, diags := loader.LoadConfigWithSnapshot(fixtureDir) - assertNoDiagnostics(t, diags) - if got == nil { - t.Fatalf("snapshot is nil; want non-nil") - } - - t.Log(spew.Sdump(got)) - - { - gotModuleDirs := map[string]string{} - for k, m := range got.Modules { - gotModuleDirs[k] = m.Dir - } - wantModuleDirs := map[string]string{ - "": "testdata/already-installed", - "child_a": "testdata/already-installed/.terraform/modules/child_a", - "child_a.child_c": "testdata/already-installed/.terraform/modules/child_a/child_c", - "child_b": "testdata/already-installed/.terraform/modules/child_b", - "child_b.child_d": "testdata/already-installed/.terraform/modules/child_b.child_d", - } - - problems := deep.Equal(wantModuleDirs, gotModuleDirs) - for _, problem := range problems { - t.Error(problem) - } - if len(problems) > 0 { - return - } - } - - gotRoot := got.Modules[""] - wantRoot := &SnapshotModule{ - Dir: "testdata/already-installed", - Files: map[string][]byte{ - "root.tf": []byte(` -module "child_a" { - source = "example.com/foo/bar_a/baz" - version = ">= 1.0.0" -} - -module "child_b" { - source = "example.com/foo/bar_b/baz" - version = ">= 1.0.0" -} -`), - }, - } - if !reflect.DeepEqual(gotRoot, wantRoot) { - t.Errorf("wrong root module snapshot\ngot: %swant: %s", spew.Sdump(gotRoot), spew.Sdump(wantRoot)) - } - -} - -func TestLoadConfigWithSnapshot_invalidSource(t *testing.T) { - fixtureDir := filepath.Clean("testdata/already-installed-now-invalid") - - old, _ := os.Getwd() - os.Chdir(fixtureDir) - defer os.Chdir(old) - - loader, err := NewLoader(&Config{ - ModulesDir: ".terraform/modules", - }) - if err != nil { - t.Fatalf("unexpected error from NewLoader: %s", err) - } - - _, _, diags := loader.LoadConfigWithSnapshot(".") - if !diags.HasErrors() { - t.Error("LoadConfigWithSnapshot succeeded; want errors") - } -} - -func TestSnapshotRoundtrip(t *testing.T) { - fixtureDir := filepath.Clean("testdata/already-installed") - loader, err := NewLoader(&Config{ - ModulesDir: filepath.Join(fixtureDir, ".terraform/modules"), - }) - if err != nil { - t.Fatalf("unexpected error from NewLoader: %s", err) - } - - _, snap, diags := loader.LoadConfigWithSnapshot(fixtureDir) - assertNoDiagnostics(t, diags) - if snap == nil { - t.Fatalf("snapshot is nil; want non-nil") - } - - snapLoader := NewLoaderFromSnapshot(snap) - if loader == nil { - t.Fatalf("loader is nil; want non-nil") - } - - config, diags := snapLoader.LoadConfig(fixtureDir) - assertNoDiagnostics(t, diags) - if config == nil { - t.Fatalf("config is nil; want non-nil") - } - if config.Module == nil { - t.Fatalf("config has no root module") - } - if got, want := config.Module.SourceDir, "testdata/already-installed"; got != want { - t.Errorf("wrong root module sourcedir %q; want %q", got, want) - } - if got, want := len(config.Module.ModuleCalls), 2; got != want { - t.Errorf("wrong number of module calls in root module %d; want %d", got, want) - } - childA := config.Children["child_a"] - if childA == nil { - t.Fatalf("child_a config is nil; want non-nil") - } - if childA.Module == nil { - t.Fatalf("child_a config has no module") - } - if got, want := childA.Module.SourceDir, "testdata/already-installed/.terraform/modules/child_a"; got != want { - t.Errorf("wrong child_a sourcedir %q; want %q", got, want) - } - if got, want := len(childA.Module.ModuleCalls), 1; got != want { - t.Errorf("wrong number of module calls in child_a %d; want %d", got, want) - } -} diff --git a/internal/configs/import_test.go b/internal/configs/import_test.go index 659d8f51c4..1d1c383958 100644 --- a/internal/configs/import_test.go +++ b/internal/configs/import_test.go @@ -16,13 +16,6 @@ import ( ) func TestParseConfigResourceFromExpression(t *testing.T) { - mustExpr := func(expr hcl.Expression, diags hcl.Diagnostics) hcl.Expression { - if diags != nil { - panic(diags.Error()) - } - return expr - } - tests := []struct { expr hcl.Expression expect addrs.ConfigResource @@ -280,3 +273,10 @@ func mustAbsResourceInstanceAddr(str string) addrs.AbsResourceInstance { } return addr } + +func mustExpr(expr hcl.Expression, diags hcl.Diagnostics) hcl.Expression { + if diags != nil { + panic(diags.Error()) + } + return expr +} diff --git a/internal/configs/testdata/invalid-files/version-variable.tf b/internal/configs/testdata/invalid-files/version-variable.tf deleted file mode 100644 index 7c871053de..0000000000 --- a/internal/configs/testdata/invalid-files/version-variable.tf +++ /dev/null @@ -1,6 +0,0 @@ -variable "module_version" { default = "v1.0" } - -module "foo" { - source = "./ff" - version = var.module_version -} diff --git a/internal/lang/globalref/analyzer_test.go b/internal/lang/globalref/analyzer_test.go index 7f50e6aca4..416cccce48 100644 --- a/internal/lang/globalref/analyzer_test.go +++ b/internal/lang/globalref/analyzer_test.go @@ -24,7 +24,7 @@ func testAnalyzer(t *testing.T, fixtureName string) *Analyzer { loader, cleanup := configload.NewLoaderForTests(t) defer cleanup() - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), configDir, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatalf("unexpected module installation errors: %s", instDiags.Err().Error()) @@ -33,7 +33,7 @@ func testAnalyzer(t *testing.T, fixtureName string) *Analyzer { t.Fatalf("failed to refresh modules after install: %s", err) } - cfg, loadDiags := loader.LoadConfig(configDir) + cfg, loadDiags := loader.LoadStaticConfig(configDir) if loadDiags.HasErrors() { t.Fatalf("unexpected configuration errors: %s", loadDiags.Error()) } diff --git a/internal/moduletest/graph/eval_context_test.go b/internal/moduletest/graph/eval_context_test.go index eec1a08cdc..4b033f771a 100644 --- a/internal/moduletest/graph/eval_context_test.go +++ b/internal/moduletest/graph/eval_context_test.go @@ -835,7 +835,7 @@ func testModuleInline(t *testing.T, sources map[string]string) *configs.Config { // Test modules usually do not refer to remote sources, and for local // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), cfgPath, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) @@ -847,7 +847,7 @@ func testModuleInline(t *testing.T, sources map[string]string) *configs.Config { t.Fatalf("failed to refresh modules after installation: %s", err) } - config, diags := loader.LoadConfigWithTests(cfgPath, "tests") + config, diags := loader.LoadStaticConfigWithTests(cfgPath, "tests") if diags.HasErrors() { t.Fatal(diags.Error()) } diff --git a/internal/providercache/installer.go b/internal/providercache/installer.go index d54652b6d9..d375e90ca5 100644 --- a/internal/providercache/installer.go +++ b/internal/providercache/installer.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "log" + "slices" "sort" "strings" @@ -205,13 +206,7 @@ func (i *Installer) EnsureProviderVersions(ctx context.Context, locks *depsfile. if provider.IsBuiltIn() { // Built in providers do not require installation but we'll still // verify that the requested provider name is valid. - valid := false - for _, name := range i.builtInProviderTypes { - if name == provider.Type { - valid = true - break - } - } + valid := slices.Contains(i.builtInProviderTypes, provider.Type) var err error if valid { if len(versionConstraints) == 0 { diff --git a/internal/refactoring/move_validate_test.go b/internal/refactoring/move_validate_test.go index 9e44da0e58..bc9b66c2db 100644 --- a/internal/refactoring/move_validate_test.go +++ b/internal/refactoring/move_validate_test.go @@ -10,6 +10,8 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/hcl/v2/hcltest" + "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" "github.com/hashicorp/terraform/internal/addrs" @@ -519,7 +521,7 @@ func loadRefactoringFixture(t *testing.T, dir string) (*configs.Config, instance loader, cleanup := configload.NewLoaderForTests(t) defer cleanup() - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) @@ -531,7 +533,7 @@ func loadRefactoringFixture(t *testing.T, dir string) (*configs.Config, instance t.Fatalf("failed to refresh modules after installation: %s", err) } - rootCfg, diags := loader.LoadConfig(dir) + rootCfg, diags := loader.LoadStaticConfig(dir) if diags.HasErrors() { t.Fatalf("failed to load root module: %s", diags.Error()) } @@ -565,7 +567,7 @@ func staticPopulateExpanderModule(t *testing.T, rootCfg *configs.Config, moduleA // module to be something that counts as a separate package, // so we can test rules relating to crossing package boundaries // even though we really just loaded the module from a local path. - call.SourceAddr = fakeExternalModuleSource + call.SourceExpr = hcltest.MockExprLiteral(cty.StringVal(fakeExternalModuleSource.String())) } // In order to get a valid, useful set of instances here we're going diff --git a/internal/terraform/terraform_test.go b/internal/terraform/terraform_test.go index 53071437f3..309f4d044e 100644 --- a/internal/terraform/terraform_test.go +++ b/internal/terraform/terraform_test.go @@ -26,6 +26,7 @@ import ( "github.com/hashicorp/terraform/internal/provisioners" "github.com/hashicorp/terraform/internal/registry" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/tfdiags" _ "github.com/hashicorp/terraform/internal/logging" ) @@ -67,7 +68,7 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config // Test modules usually do not refer to remote sources, and for local // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), dir, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) @@ -79,14 +80,44 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config t.Fatalf("failed to refresh modules after installation: %s", err) } - config, snap, diags := loader.LoadConfigWithSnapshot(dir) + config, snap, diags := testLoadWithSnapshot(dir, loader, nil) if diags.HasErrors() { - t.Fatal(diags.Error()) + t.Fatal(diags.Err()) } return config, snap } +func testLoadWithSnapshot(dir string, loader *configload.Loader, vars InputValues) (*configs.Config, *configload.Snapshot, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + + rootMod, configDiags := loader.LoadRootModule(dir) + if configDiags.HasErrors() { + diags = diags.Append(configDiags) + return nil, nil, diags + } + + walkerSnapshot, snap := loader.ModuleWalkerSnapshot() + config, buildDiags := BuildConfigWithGraph( + rootMod, + walkerSnapshot, + vars, + configs.MockDataLoaderFunc(loader.LoadExternalMockData), + ) + if buildDiags.HasErrors() { + diags = diags.Append(buildDiags) + return nil, nil, diags + } + + snapDiags := loader.AddRootModuleToSnapshot(snap, dir) + if snapDiags.HasErrors() { + diags = diags.Append(snapDiags) + return nil, nil, diags + } + + return config, snap, nil +} + // testModuleInline takes a map of path -> config strings and yields a config // structure with those files loaded from disk func testModuleInline(t testing.TB, sources map[string]string, parserOpts ...configs.Option) *configs.Config { @@ -127,7 +158,7 @@ func testModuleInline(t testing.TB, sources map[string]string, parserOpts ...con // Test modules usually do not refer to remote sources, and for local // sources only this ultimately just records all of the module paths // in a JSON file so that we can load them below. - inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil)) + inst := initwd.NewModuleInstaller(loader.ModulesDir(), loader, registry.NewClient(nil, nil), nil) _, instDiags := inst.InstallModules(context.Background(), cfgPath, "tests", true, false, initwd.ModuleInstallHooksImpl{}) if instDiags.HasErrors() { t.Fatal(instDiags.Err()) @@ -139,7 +170,7 @@ func testModuleInline(t testing.TB, sources map[string]string, parserOpts ...con t.Fatalf("failed to refresh modules after installation: %s", err) } - config, diags := loader.LoadConfigWithTests(cfgPath, "tests") + config, diags := loader.LoadStaticConfigWithTests(cfgPath, "tests") if diags.HasErrors() { t.Fatal(diags.Error()) } @@ -147,6 +178,49 @@ func testModuleInline(t testing.TB, sources map[string]string, parserOpts ...con return config } +func testRootModuleInline(t testing.TB, sources map[string]string) *configs.Module { + t.Helper() + + cfgPath, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + + for path, configStr := range sources { + dir := filepath.Dir(path) + if dir != "." { + err := os.MkdirAll(filepath.Join(cfgPath, dir), os.FileMode(0777)) + if err != nil { + t.Fatalf("Error creating subdir: %s", err) + } + } + // Write the configuration + cfgF, err := os.Create(filepath.Join(cfgPath, path)) + if err != nil { + t.Fatalf("Error creating temporary file for config: %s", err) + } + + _, err = io.Copy(cfgF, strings.NewReader(configStr)) + cfgF.Close() + if err != nil { + t.Fatalf("Error creating temporary file for config: %s", err) + } + } + + loader, cleanup := configload.NewLoaderForTests(t) + defer cleanup() + + // We need to be able to exercise experimental features in our integration tests. + loader.AllowLanguageExperiments(true) + + mod, diags := loader.Parser().LoadConfigDir(cfgPath) + if diags.HasErrors() { + t.Fatal(diags.Error()) + } + + return mod +} + // testSetResourceInstanceCurrent is a helper function for tests that sets a Current, // Ready resource instance for the given module. func testSetResourceInstanceCurrent(module *states.Module, resource, attrsJson, provider string) {