diff --git a/internal/backend/backend.go b/internal/backend/backend.go index d8b28b943..9f1855eb2 100644 --- a/internal/backend/backend.go +++ b/internal/backend/backend.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/planfile" "github.com/hashicorp/terraform/internal/states" @@ -237,6 +238,16 @@ type Operation struct { // configuration from ConfigDir. ConfigLoader *configload.Loader + // DependencyLocks represents the locked dependencies associated with + // the configuration directory given in ConfigDir. + // + // Note that if field PlanFile is set then the plan file should contain + // its own dependency locks. The backend is responsible for correctly + // selecting between these two sets of locks depending on whether it + // will be using ConfigDir or PlanFile to get the configuration for + // this operation. + DependencyLocks *depsfile.Locks + // Hooks can be used to perform actions triggered by various events during // the operation's lifecycle. Hooks []terraform.Hook diff --git a/internal/backend/local/backend_apply_test.go b/internal/backend/local/backend_apply_test.go index 4ffc0fa0a..1f9514ad5 100644 --- a/internal/backend/local/backend_apply_test.go +++ b/internal/backend/local/backend_apply_test.go @@ -11,11 +11,13 @@ import ( "github.com/zclconf/go-cty/cty" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend" "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/configschema" + "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" @@ -319,12 +321,18 @@ func testOperationApply(t *testing.T, configDir string) (*backend.Operation, fun streams, done := terminal.StreamsForTesting(t) view := views.NewOperation(arguments.ViewHuman, false, views.NewView(streams)) + // Many of our tests use an overridden "test" provider that's just in-memory + // inside the test process, not a separate plugin on disk. + depLocks := depsfile.NewLocks() + depLocks.SetProviderOverridden(addrs.MustParseProviderSourceString("registry.terraform.io/hashicorp/test")) + return &backend.Operation{ - Type: backend.OperationTypeApply, - ConfigDir: configDir, - ConfigLoader: configLoader, - StateLocker: clistate.NewNoopLocker(), - View: view, + Type: backend.OperationTypeApply, + ConfigDir: configDir, + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + View: view, + DependencyLocks: depLocks, }, configCleanup, done } diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 5ce5929ee..209757e43 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "sort" + "strings" "github.com/hashicorp/terraform/internal/backend" "github.com/hashicorp/terraform/internal/configs" @@ -83,7 +84,7 @@ func (b *Local) localRun(op *backend.Operation) (*backend.LocalRun, *configload. stateMeta = &m } log.Printf("[TRACE] backend/local: populating backend.LocalRun from plan file") - ret, configSnap, ctxDiags = b.localRunForPlanFile(op.PlanFile, ret, &coreOpts, stateMeta) + ret, configSnap, ctxDiags = b.localRunForPlanFile(op, op.PlanFile, ret, &coreOpts, stateMeta) if ctxDiags.HasErrors() { diags = diags.Append(ctxDiags) return nil, nil, nil, diags @@ -138,6 +139,32 @@ func (b *Local) localRunDirect(op *backend.Operation, run *backend.LocalRun, cor } 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]backend.UnparsedVariableValue if op.AllowUnsetVariables { // Rather than prompting for input, we'll just stub out the required @@ -181,7 +208,7 @@ func (b *Local) localRunDirect(op *backend.Operation, run *backend.LocalRun, cor return run, configSnap, diags } -func (b *Local) localRunForPlanFile(pf *planfile.Reader, run *backend.LocalRun, coreOpts *terraform.ContextOpts, currentStateMeta *statemgr.SnapshotMeta) (*backend.LocalRun, *configload.Snapshot, tfdiags.Diagnostics) { +func (b *Local) localRunForPlanFile(op *backend.Operation, pf *planfile.Reader, run *backend.LocalRun, coreOpts *terraform.ContextOpts, currentStateMeta *statemgr.SnapshotMeta) (*backend.LocalRun, *configload.Snapshot, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics const errSummary = "Invalid plan file" @@ -206,6 +233,46 @@ func (b *Local) localRunForPlanFile(pf *planfile.Reader, run *backend.LocalRun, } 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(), + ), + )) + } + + // This check is an important complement to the check above: the locked + // dependencies in the configuration must match the configuration, and + // the locked dependencies in the plan must match the locked dependencies + // in the configuration, and so transitively we ensure that the locked + // dependencies in the plan match the configuration too. However, this + // additionally catches any inconsistency between the two sets of locks + // even if they both happen to be valid per the current configuration, + // which is one of several ways we try to catch the mistake of applying + // a saved plan file in a different place than where we created it. + depLocksFromPlan, moreDiags := pf.ReadDependencyLocks() + diags = diags.Append(moreDiags) + if depLocksFromPlan != nil && !op.DependencyLocks.Equal(depLocksFromPlan) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Inconsistent dependency lock file", + "The given plan file was created with a different set of external dependency selections than the current configuration. A saved plan can be applied only to the same configuration it was created from.\n\nCreate a new plan from the updated configuration.", + )) + } + // A plan file also contains a snapshot of the prior state the changes // are intended to apply to. priorStateFile, err := pf.ReadStateFile() diff --git a/internal/backend/local/backend_plan.go b/internal/backend/local/backend_plan.go index 655ece102..c721074b4 100644 --- a/internal/backend/local/backend_plan.go +++ b/internal/backend/local/backend_plan.go @@ -138,6 +138,7 @@ func (b *Local) opPlan( PreviousRunStateFile: prevStateFile, StateFile: plannedStateFile, Plan: plan, + DependencyLocks: op.DependencyLocks, }) if err != nil { diags = diags.Append(tfdiags.Sourceless( diff --git a/internal/backend/local/backend_plan_test.go b/internal/backend/local/backend_plan_test.go index 2a9f3f828..5121d45e7 100644 --- a/internal/backend/local/backend_plan_test.go +++ b/internal/backend/local/backend_plan_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/planfile" @@ -716,12 +717,18 @@ func testOperationPlan(t *testing.T, configDir string) (*backend.Operation, func streams, done := terminal.StreamsForTesting(t) view := views.NewOperation(arguments.ViewHuman, false, views.NewView(streams)) + // Many of our tests use an overridden "test" provider that's just in-memory + // inside the test process, not a separate plugin on disk. + depLocks := depsfile.NewLocks() + depLocks.SetProviderOverridden(addrs.MustParseProviderSourceString("registry.terraform.io/hashicorp/test")) + return &backend.Operation{ - Type: backend.OperationTypePlan, - ConfigDir: configDir, - ConfigLoader: configLoader, - StateLocker: clistate.NewNoopLocker(), - View: view, + Type: backend.OperationTypePlan, + ConfigDir: configDir, + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + View: view, + DependencyLocks: depLocks, }, configCleanup, done } diff --git a/internal/backend/local/backend_refresh_test.go b/internal/backend/local/backend_refresh_test.go index 0e502267c..886c18418 100644 --- a/internal/backend/local/backend_refresh_test.go +++ b/internal/backend/local/backend_refresh_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/states" @@ -260,12 +261,18 @@ func testOperationRefresh(t *testing.T, configDir string) (*backend.Operation, f streams, done := terminal.StreamsForTesting(t) view := views.NewOperation(arguments.ViewHuman, false, views.NewView(streams)) + // Many of our tests use an overridden "test" provider that's just in-memory + // inside the test process, not a separate plugin on disk. + depLocks := depsfile.NewLocks() + depLocks.SetProviderOverridden(addrs.MustParseProviderSourceString("registry.terraform.io/hashicorp/test")) + return &backend.Operation{ - Type: backend.OperationTypeRefresh, - ConfigDir: configDir, - ConfigLoader: configLoader, - StateLocker: clistate.NewNoopLocker(), - View: view, + Type: backend.OperationTypeRefresh, + ConfigDir: configDir, + ConfigLoader: configLoader, + StateLocker: clistate.NewNoopLocker(), + View: view, + DependencyLocks: depLocks, }, configCleanup, done } diff --git a/internal/backend/remote/backend.go b/internal/backend/remote/backend.go index 5e09e7207..e427befa0 100644 --- a/internal/backend/remote/backend.go +++ b/internal/backend/remote/backend.go @@ -710,6 +710,7 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend // Record that we're forced to run operations locally to allow the // command package UI to operate correctly b.forceLocal = true + log.Printf("[DEBUG] Remote backend is delegating %s to the local backend", op.Type) return b.local.Operation(ctx, op) } diff --git a/internal/backend/remote/backend_apply_test.go b/internal/backend/remote/backend_apply_test.go index 4bc2a909f..71c819b9e 100644 --- a/internal/backend/remote/backend_apply_test.go +++ b/internal/backend/remote/backend_apply_test.go @@ -17,6 +17,7 @@ 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/depsfile" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/planfile" @@ -43,13 +44,19 @@ func testOperationApplyWithTimeout(t *testing.T, configDir string, timeout time. stateLockerView := views.NewStateLocker(arguments.ViewHuman, view) operationView := views.NewOperation(arguments.ViewHuman, false, view) + // Many of our tests use an overridden "null" provider that's just in-memory + // inside the test process, not a separate plugin on disk. + depLocks := depsfile.NewLocks() + depLocks.SetProviderOverridden(addrs.MustParseProviderSourceString("registry.terraform.io/hashicorp/null")) + return &backend.Operation{ - ConfigDir: configDir, - ConfigLoader: configLoader, - PlanRefresh: true, - StateLocker: clistate.NewLocker(timeout, stateLockerView), - Type: backend.OperationTypeApply, - View: operationView, + ConfigDir: configDir, + ConfigLoader: configLoader, + PlanRefresh: true, + StateLocker: clistate.NewLocker(timeout, stateLockerView), + Type: backend.OperationTypeApply, + View: operationView, + DependencyLocks: depLocks, }, configCleanup, done } diff --git a/internal/backend/remote/backend_plan_test.go b/internal/backend/remote/backend_plan_test.go index 6d4ced7b8..147c68e9d 100644 --- a/internal/backend/remote/backend_plan_test.go +++ b/internal/backend/remote/backend_plan_test.go @@ -16,6 +16,7 @@ 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/depsfile" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/plans/planfile" @@ -41,13 +42,19 @@ func testOperationPlanWithTimeout(t *testing.T, configDir string, timeout time.D stateLockerView := views.NewStateLocker(arguments.ViewHuman, view) operationView := views.NewOperation(arguments.ViewHuman, false, view) + // Many of our tests use an overridden "null" provider that's just in-memory + // inside the test process, not a separate plugin on disk. + depLocks := depsfile.NewLocks() + depLocks.SetProviderOverridden(addrs.MustParseProviderSourceString("registry.terraform.io/hashicorp/null")) + return &backend.Operation{ - ConfigDir: configDir, - ConfigLoader: configLoader, - PlanRefresh: true, - StateLocker: clistate.NewLocker(timeout, stateLockerView), - Type: backend.OperationTypePlan, - View: operationView, + ConfigDir: configDir, + ConfigLoader: configLoader, + PlanRefresh: true, + StateLocker: clistate.NewLocker(timeout, stateLockerView), + Type: backend.OperationTypePlan, + View: operationView, + DependencyLocks: depLocks, }, configCleanup, done } diff --git a/internal/command/command_test.go b/internal/command/command_test.go index a0bc4710d..a9ba2a044 100644 --- a/internal/command/command_test.go +++ b/internal/command/command_test.go @@ -29,6 +29,7 @@ import ( "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/copy" + "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/initwd" legacy "github.com/hashicorp/terraform/internal/legacy/terraform" @@ -247,6 +248,7 @@ func testPlanFile(t *testing.T, configSnap *configload.Snapshot, state *states.S PreviousRunStateFile: prevStateFile, StateFile: stateFile, Plan: plan, + DependencyLocks: depsfile.NewLocks(), }) if err != nil { t.Fatalf("failed to create temporary plan file: %s", err) diff --git a/internal/command/import_test.go b/internal/command/import_test.go index 1469ea81d..091646ecc 100644 --- a/internal/command/import_test.go +++ b/internal/command/import_test.go @@ -332,7 +332,7 @@ func TestImport_initializationErrorShouldUnlock(t *testing.T) { // specifically, it should fail due to a missing provider msg := strings.ReplaceAll(ui.ErrorWriter.String(), "\n", " ") - if want := `unavailable provider "registry.terraform.io/hashicorp/unknown"`; !strings.Contains(msg, want) { + if want := `provider registry.terraform.io/hashicorp/unknown: required by this configuration but no version is selected`; !strings.Contains(msg, want) { t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg) } diff --git a/internal/command/meta_backend.go b/internal/command/meta_backend.go index f467579ba..5d2aa5ed3 100644 --- a/internal/command/meta_backend.go +++ b/internal/command/meta_backend.go @@ -355,13 +355,24 @@ func (m *Meta) Operation(b backend.Backend) *backend.Operation { stateLocker = clistate.NewLocker(m.stateLockTimeout, view) } + depLocks, diags := m.lockedDependencies() + if diags.HasErrors() { + // We can't actually report errors from here, but m.lockedDependencies + // should always have been called earlier to prepare the "ContextOpts" + // for the backend anyway, so we should never actually get here in + // a real situation. If we do get here then the backend will inevitably + // fail downstream somwhere if it tries to use the empty depLocks. + log.Printf("[WARN] Failed to load dependency locks while preparing backend operation (ignored): %s", diags.Err().Error()) + } + return &backend.Operation{ - PlanOutBackend: planOutBackend, - Targets: m.targets, - UIIn: m.UIInput(), - UIOut: m.Ui, - Workspace: workspace, - StateLocker: stateLocker, + PlanOutBackend: planOutBackend, + Targets: m.targets, + UIIn: m.UIInput(), + UIOut: m.Ui, + Workspace: workspace, + StateLocker: stateLocker, + DependencyLocks: depLocks, } } diff --git a/internal/command/meta_dependencies.go b/internal/command/meta_dependencies.go index efda3f103..1b0cb97f8 100644 --- a/internal/command/meta_dependencies.go +++ b/internal/command/meta_dependencies.go @@ -1,6 +1,7 @@ package command import ( + "log" "os" "github.com/hashicorp/terraform/internal/depsfile" @@ -48,10 +49,11 @@ func (m *Meta) lockedDependencies() (*depsfile.Locks, tfdiags.Diagnostics) { // promising to support two concurrent dependency installation processes. _, err := os.Stat(dependencyLockFilename) if os.IsNotExist(err) { - return depsfile.NewLocks(), nil + return m.annotateDependencyLocksWithOverrides(depsfile.NewLocks()), nil } - return depsfile.LoadLocksFromFile(dependencyLockFilename) + ret, diags := depsfile.LoadLocksFromFile(dependencyLockFilename) + return m.annotateDependencyLocksWithOverrides(ret), diags } // replaceLockedDependencies creates or overwrites the lock file in the @@ -60,3 +62,32 @@ func (m *Meta) lockedDependencies() (*depsfile.Locks, tfdiags.Diagnostics) { func (m *Meta) replaceLockedDependencies(new *depsfile.Locks) tfdiags.Diagnostics { return depsfile.SaveLocksToFile(new, dependencyLockFilename) } + +// annotateDependencyLocksWithOverrides modifies the given Locks object in-place +// to track as overridden any provider address that's subject to testing +// overrides, development overrides, or "unmanaged provider" status. +// +// This is just an implementation detail of the lockedDependencies method, +// not intended for use anywhere else. +func (m *Meta) annotateDependencyLocksWithOverrides(ret *depsfile.Locks) *depsfile.Locks { + if ret == nil { + return ret + } + + for addr := range m.ProviderDevOverrides { + log.Printf("[DEBUG] Provider %s is overridden by dev_overrides", addr) + ret.SetProviderOverridden(addr) + } + for addr := range m.UnmanagedProviders { + log.Printf("[DEBUG] Provider %s is overridden as an \"unmanaged provider\"", addr) + ret.SetProviderOverridden(addr) + } + if m.testingOverrides != nil { + for addr := range m.testingOverrides.Providers { + log.Printf("[DEBUG] Provider %s is overridden in Meta.testingOverrides", addr) + ret.SetProviderOverridden(addr) + } + } + + return ret +} diff --git a/internal/command/meta_providers.go b/internal/command/meta_providers.go index 84ccc89d8..ffe52ffde 100644 --- a/internal/command/meta_providers.go +++ b/internal/command/meta_providers.go @@ -282,6 +282,12 @@ func (m *Meta) providerFactories() (map[addrs.Provider]providers.Factory, error) factories[provider] = providerFactoryError(thisErr) } + if locks.ProviderIsOverridden(provider) { + // Overridden providers we'll handle with the other separate + // loops below, for dev overrides etc. + continue + } + version := lock.Version() cached := cacheDir.ProviderVersion(provider, version) if cached == nil { @@ -313,8 +319,6 @@ func (m *Meta) providerFactories() (map[addrs.Provider]providers.Factory, error) factories[provider] = providerFactory(cached) } for provider, localDir := range devOverrideProviders { - // It's likely that providers in this map will conflict with providers - // in providerLocks factories[provider] = devOverrideProviderFactory(provider, localDir) } for provider, reattach := range unmanagedProviders { diff --git a/internal/command/plan_test.go b/internal/command/plan_test.go index 68f051f55..84950d7e0 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, "Error: Missing required provider") { + if !(strings.Contains(got, "terraform init") && strings.Contains(got, "provider registry.terraform.io/hashicorp/test: required by this configuration but no version is selected")) { t.Fatal("wrong error message in output:", got) } } diff --git a/internal/configs/config.go b/internal/configs/config.go index cc3f06e79..f38d3cd85 100644 --- a/internal/configs/config.go +++ b/internal/configs/config.go @@ -2,11 +2,13 @@ package configs import ( "fmt" + "log" "sort" version "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/getproviders" ) @@ -194,6 +196,89 @@ func (c *Config) EntersNewPackage() bool { return moduleSourceAddrEntersNewPackage(c.SourceAddr) } +// VerifyDependencySelections checks whether the given locked dependencies +// are acceptable for all of the version constraints reported in the +// configuration tree represented by the reciever. +// +// This function will errors only if any of the locked dependencies are out of +// range for corresponding constraints in the configuration. If there are +// multiple inconsistencies then it will attempt to describe as many of them +// as possible, rather than stopping at the first problem. +// +// It's typically the responsibility of "terraform init" to change the locked +// dependencies to conform with the configuration, and so +// VerifyDependencySelections is intended for other commands to check whether +// it did so correctly and to catch if anything has changed in configuration +// since the last "terraform init" which requires re-initialization. However, +// it's up to the caller to decide how to advise users recover from these +// errors, because the advise can vary depending on what operation the user +// is attempting. +func (c *Config) VerifyDependencySelections(depLocks *depsfile.Locks) []error { + var errs []error + + reqs, diags := c.ProviderRequirements() + if diags.HasErrors() { + // It should be very unusual to get here, but unfortunately we can + // end up here in some edge cases where the config loader doesn't + // process version constraint strings in exactly the same way as + // the requirements resolver. (See the addProviderRequirements method + // for more information.) + errs = append(errs, fmt.Errorf("failed to determine the configuration's provider requirements: %s", diags.Error())) + } + + for providerAddr, constraints := range reqs { + if !depsfile.ProviderIsLockable(providerAddr) { + continue // disregard builtin providers, and such + } + if depLocks != nil && depLocks.ProviderIsOverridden(providerAddr) { + // The "overridden" case is for unusual special situations like + // dev overrides, so we'll explicitly note it in the logs just in + // case we see bug reports with these active and it helps us + // understand why we ended up using the "wrong" plugin. + log.Printf("[DEBUG] Config.VerifyDependencySelections: skipping %s because it's overridden by a special configuration setting", providerAddr) + continue + } + + var lock *depsfile.ProviderLock + if depLocks != nil { // Should always be true in main code, but unfortunately sometimes not true in old tests that don't fill out arguments completely + lock = depLocks.Provider(providerAddr) + } + if lock == nil { + log.Printf("[TRACE] Config.VerifyDependencySelections: provider %s has no lock file entry to satisfy %q", providerAddr, getproviders.VersionConstraintsString(constraints)) + errs = append(errs, fmt.Errorf("provider %s: required by this configuration but no version is selected", providerAddr)) + continue + } + + selectedVersion := lock.Version() + allowedVersions := getproviders.MeetingConstraints(constraints) + log.Printf("[TRACE] Config.VerifyDependencySelections: provider %s has %s to satisfy %q", providerAddr, selectedVersion.String(), getproviders.VersionConstraintsString(constraints)) + if !allowedVersions.Has(selectedVersion) { + // The most likely cause of this is that the author of a module + // has changed its constraints, but this could also happen in + // some other unusual situations, such as the user directly + // editing the lock file to record something invalid. We'll + // distinguish those cases here in order to avoid the more + // specific error message potentially being a red herring in + // the edge-cases. + currentConstraints := getproviders.VersionConstraintsString(constraints) + lockedConstraints := getproviders.VersionConstraintsString(lock.VersionConstraints()) + switch { + case currentConstraints != lockedConstraints: + errs = append(errs, fmt.Errorf("provider %s: locked version selection %s doesn't match the updated version constraints %q", providerAddr, selectedVersion.String(), currentConstraints)) + default: + errs = append(errs, fmt.Errorf("provider %s: version constraints %q don't match the locked version selection %s", providerAddr, currentConstraints, selectedVersion.String())) + } + } + } + + // Return multiple errors in an arbitrary-but-deterministic order. + sort.Slice(errs, func(i, j int) bool { + return errs[i].Error() < errs[j].Error() + }) + + return errs +} + // ProviderRequirements searches the full tree of modules under the receiver // for both explicit and implicit dependencies on providers. // diff --git a/internal/configs/config_test.go b/internal/configs/config_test.go index 4677f3416..21c400a85 100644 --- a/internal/configs/config_test.go +++ b/internal/configs/config_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/hcl/v2/hclsyntax" svchost "github.com/hashicorp/terraform-svchost" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/getproviders" ) @@ -262,6 +263,128 @@ func TestConfigProviderRequirementsByModule(t *testing.T) { } } +func TestVerifyDependencySelections(t *testing.T) { + cfg, diags := testNestedModuleConfigFromDir(t, "testdata/provider-reqs") + // TODO: Version Constraint Deprecation. + // Once we've removed the version argument from provider configuration + // blocks, this can go back to expected 0 diagnostics. + // assertNoDiagnostics(t, diags) + assertDiagnosticCount(t, diags, 1) + assertDiagnosticSummary(t, diags, "Version constraints inside provider configuration blocks are deprecated") + + tlsProvider := addrs.NewProvider( + addrs.DefaultProviderRegistryHost, + "hashicorp", "tls", + ) + happycloudProvider := addrs.NewProvider( + svchost.Hostname("tf.example.com"), + "awesomecorp", "happycloud", + ) + nullProvider := addrs.NewDefaultProvider("null") + randomProvider := addrs.NewDefaultProvider("random") + impliedProvider := addrs.NewDefaultProvider("implied") + configuredProvider := addrs.NewDefaultProvider("configured") + grandchildProvider := addrs.NewDefaultProvider("grandchild") + + tests := map[string]struct { + PrepareLocks func(*depsfile.Locks) + WantErrs []string + }{ + "empty locks": { + func(*depsfile.Locks) { + // Intentionally blank + }, + []string{ + `provider registry.terraform.io/hashicorp/configured: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/grandchild: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/implied: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/null: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/random: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/tls: required by this configuration but no version is selected`, + `provider tf.example.com/awesomecorp/happycloud: required by this configuration but no version is selected`, + }, + }, + "suitable locks": { + func(locks *depsfile.Locks) { + locks.SetProvider(configuredProvider, getproviders.MustParseVersion("1.4.0"), nil, nil) + locks.SetProvider(grandchildProvider, getproviders.MustParseVersion("0.1.0"), nil, nil) + locks.SetProvider(impliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil) + locks.SetProvider(nullProvider, getproviders.MustParseVersion("2.0.1"), nil, nil) + locks.SetProvider(randomProvider, getproviders.MustParseVersion("1.2.2"), nil, nil) + locks.SetProvider(tlsProvider, getproviders.MustParseVersion("3.0.1"), nil, nil) + locks.SetProvider(happycloudProvider, getproviders.MustParseVersion("0.0.1"), nil, nil) + }, + nil, + }, + "null provider constraints changed": { + func(locks *depsfile.Locks) { + locks.SetProvider(configuredProvider, getproviders.MustParseVersion("1.4.0"), nil, nil) + locks.SetProvider(grandchildProvider, getproviders.MustParseVersion("0.1.0"), nil, nil) + locks.SetProvider(impliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil) + locks.SetProvider(nullProvider, getproviders.MustParseVersion("3.0.0"), nil, nil) + locks.SetProvider(randomProvider, getproviders.MustParseVersion("1.2.2"), nil, nil) + locks.SetProvider(tlsProvider, getproviders.MustParseVersion("3.0.1"), nil, nil) + locks.SetProvider(happycloudProvider, getproviders.MustParseVersion("0.0.1"), nil, nil) + }, + []string{ + `provider registry.terraform.io/hashicorp/null: locked version selection 3.0.0 doesn't match the updated version constraints "~> 2.0.0, 2.0.1"`, + }, + }, + "null provider lock changed": { + func(locks *depsfile.Locks) { + // In this case, we set the lock file version constraints to + // match the configuration, and so our error message changes + // to not assume the configuration changed anymore. + locks.SetProvider(nullProvider, getproviders.MustParseVersion("3.0.0"), getproviders.MustParseVersionConstraints("~> 2.0.0, 2.0.1"), nil) + + locks.SetProvider(configuredProvider, getproviders.MustParseVersion("1.4.0"), nil, nil) + locks.SetProvider(grandchildProvider, getproviders.MustParseVersion("0.1.0"), nil, nil) + locks.SetProvider(impliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil) + locks.SetProvider(randomProvider, getproviders.MustParseVersion("1.2.2"), nil, nil) + locks.SetProvider(tlsProvider, getproviders.MustParseVersion("3.0.1"), nil, nil) + locks.SetProvider(happycloudProvider, getproviders.MustParseVersion("0.0.1"), nil, nil) + }, + []string{ + `provider registry.terraform.io/hashicorp/null: version constraints "~> 2.0.0, 2.0.1" don't match the locked version selection 3.0.0`, + }, + }, + "overridden provider": { + func(locks *depsfile.Locks) { + locks.SetProviderOverridden(happycloudProvider) + }, + []string{ + // We still catch all of the other ones, because only happycloud was overridden + `provider registry.terraform.io/hashicorp/configured: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/grandchild: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/implied: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/null: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/random: required by this configuration but no version is selected`, + `provider registry.terraform.io/hashicorp/tls: required by this configuration but no version is selected`, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + depLocks := depsfile.NewLocks() + test.PrepareLocks(depLocks) + gotErrs := cfg.VerifyDependencySelections(depLocks) + + var gotErrsStr []string + if gotErrs != nil { + gotErrsStr = make([]string, len(gotErrs)) + for i, err := range gotErrs { + gotErrsStr[i] = err.Error() + } + } + + if diff := cmp.Diff(test.WantErrs, gotErrsStr); diff != "" { + t.Errorf("wrong errors\n%s", diff) + } + }) + } +} + func TestConfigProviderForConfigAddr(t *testing.T) { cfg, diags := testModuleConfigFromDir("testdata/valid-modules/providers-fqns") assertNoDiagnostics(t, diags) diff --git a/internal/depsfile/locks.go b/internal/depsfile/locks.go index 751399070..0dec32a1d 100644 --- a/internal/depsfile/locks.go +++ b/internal/depsfile/locks.go @@ -18,6 +18,20 @@ import ( type Locks struct { providers map[addrs.Provider]*ProviderLock + // overriddenProviders is a subset of providers which we might be tracking + // in field providers but whose lock information we're disregarding for + // this particular run due to some feature that forces Terraform to not + // use a normally-installed plugin for it. For example, the "provider dev + // overrides" feature means that we'll be using an arbitrary directory on + // disk as the package, regardless of what might be selected in "providers". + // + // overriddenProviders is an in-memory-only annotation, never stored as + // part of a lock file and thus not persistent between Terraform runs. + // The CLI layer is generally the one responsible for populating this, + // by calling SetProviderOverridden in response to CLI Configuration + // settings, environment variables, or whatever similar sources. + overriddenProviders map[addrs.Provider]struct{} + // TODO: In future we'll also have module locks, but the design of that // still needs some more work and we're deferring that to get the // provider locking capability out sooner, because it's more common to @@ -84,6 +98,48 @@ func (l *Locks) SetProvider(addr addrs.Provider, version getproviders.Version, c return new } +// SetProviderOverridden records that this particular Terraform process will +// not pay attention to the recorded lock entry for the given provider, and +// will instead access that provider's functionality in some other special +// way that isn't sensitive to provider version selections or checksums. +// +// This is an in-memory-only annotation which lives only inside a particular +// Locks object, and is never persisted as part of a saved lock file on disk. +// It's valid to still use other methods of the reciever to access +// already-stored lock information and to update lock information for an +// overridden provider, but some callers may need to use ProviderIsOverridden +// to selectively disregard stored lock information for overridden providers, +// depending on what they intended to use the lock information for. +func (l *Locks) SetProviderOverridden(addr addrs.Provider) { + if l.overriddenProviders == nil { + l.overriddenProviders = make(map[addrs.Provider]struct{}) + } + l.overriddenProviders[addr] = struct{}{} +} + +// ProviderIsOverridden returns true only if the given provider address was +// previously registered as overridden by calling SetProviderOverridden. +func (l *Locks) ProviderIsOverridden(addr addrs.Provider) bool { + _, ret := l.overriddenProviders[addr] + return ret +} + +// SetSameOverriddenProviders updates the receiver to mark as overridden all +// of the same providers already marked as overridden in the other given locks. +// +// This allows propagating override information between different lock objects, +// as if calling SetProviderOverridden for each address already overridden +// in the other given locks. If the reciever already has overridden providers, +// SetSameOverriddenProviders will preserve them. +func (l *Locks) SetSameOverriddenProviders(other *Locks) { + if other == nil { + return + } + for addr := range other.overriddenProviders { + l.SetProviderOverridden(addr) + } +} + // NewProviderLock creates a new ProviderLock object that isn't associated // with any Locks object. //