diff --git a/internal/backend/init/init.go b/internal/backend/init/init.go index e632a64474..02f7aa6529 100644 --- a/internal/backend/init/init.go +++ b/internal/backend/init/init.go @@ -54,6 +54,10 @@ func Init(services *disco.Disco) { backendsLock.Lock() defer backendsLock.Unlock() + // NOTE: Underscore-prefixed named are reserved for unit testing use via + // the RegisterTemp function. Do not add any underscore-prefixed names + // to the following table. + backends = map[string]backend.InitFn{ "local": func(enc encryption.StateEncryption) backend.Backend { return backendLocal.New(enc) }, "remote": func(enc encryption.StateEncryption) backend.Backend { return backendRemote.New(services, enc) }, @@ -100,6 +104,10 @@ func Backend(name string) backend.InitFn { // This method sets this backend globally and care should be taken to do // this only before OpenTofu is executing to prevent odd behavior of backends // changing mid-execution. +// +// NOTE: Underscore-prefixed named are reserved for unit testing use via +// the RegisterTemp function. Do not add any underscore-prefixed names +// using this function. func Set(name string, f backend.InitFn) { backendsLock.Lock() defer backendsLock.Unlock() diff --git a/internal/backend/init/testing.go b/internal/backend/init/testing.go new file mode 100644 index 0000000000..965492d586 --- /dev/null +++ b/internal/backend/init/testing.go @@ -0,0 +1,204 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package init + +import ( + "fmt" + "strings" + + "github.com/opentofu/opentofu/internal/backend" + "github.com/opentofu/opentofu/internal/configs/configschema" + "github.com/opentofu/opentofu/internal/states/statemgr" + "github.com/opentofu/opentofu/internal/tfdiags" + "github.com/zclconf/go-cty/cty" +) + +// RegisterTemp adds a new entry to the table of backends and returns a +// function that will deregister it once called. +// +// This is essentially a workaround for the fact that the OpenTofu CLI +// layer expects backends to always come from a centrally-maintained table +// and doesn't have any way to directly pass an anonymous backend +// implementation. +// +// The given name MUST start with an underscore, to ensure that it cannot +// collide with any "real" backend. If the given name does not start with +// an underscore then this function will panic. If we introduce plugin-based +// backends in future then we might consider reserving part of the plugin +// address namespace to represent temporary backends for test purposes only, +// which would then replace this special underscore prefix as the differentiator. +// +// This is intended for unit tests that use [MockBackend], or a derivative +// thereof. A typical usage pattern from a unit test would be: +// +// // ("backendInit" represents _this_ package) +// t.Cleanup(backendInit.RegisterTemp("_test", func (enc encryption.StateEncryption) Backend { +// return &backendInit.MockBackend{ +// // (and then whichever mock settings your test case needs) +// } +// })) +// +// Because this function modifies global state observable throughout the +// program, any test using this function MUST NOT use t.Parallel. If a +// single test needs to register multiple temporary backends for some reason +// then it must select a different name for each one. +func RegisterTemp(name string, f backend.InitFn) func() { + // FIXME: It would be better to add a map of backends to command.Meta's existing + // "testingOverrides" field, but at the time of writing direct calls to this + // package's func Backend are made from too many different places in the + // codebase to retrofit "testing overrides" without considerable risk to + // already-working code, so for now we compromise and just offer this helper + // function to hopefully help tests properly manage their temporary addition + // to the global backend table. + + if !strings.HasPrefix(name, "_") { + panic("temporary backend name must begin with underscore") + } + backendsLock.Lock() + defer backendsLock.Unlock() + + if _, exists := backends[name]; exists { + // If we get in here then it suggests one of the following mistakes in + // the calling code: + // - Using RegisterTemp in a test case that uses t.Parallel. + // - Forgetting to call the cleanup function in some other earlier test that + // happened to choose the same temporary name. + // - Registering more than one temporary backend in a single test without + // assigning each one a unique name. + panic(fmt.Sprintf("there is already a temporary backend named %q", name)) + } + + // The given init function is temporarily added to the global table, so that + // the CLI package can find it using the given (underscore-prefixed) name. + backends[name] = f + + return func() { + backendsLock.Lock() + delete(backends, name) + backendsLock.Unlock() + } +} + +// MockBackend is an implementation of [Backend] that largely just routes incoming +// calls to a set of closures provided by a caller. +// +// This is included for testing purposes only. Use [RegisterTemp] to +// temporarily add a MockBackend instance to the table of available backends +// from a unit test function. Do not include MockBackend instances in the +// initial static backend table. +// +// The mock automatically tracks the most recent call to each method for ease +// of writing assertions in simple cases. If you need more complex tracking such +// as a log of all calls then you can implement that inside your provided callback +// functions. +// +// This implementation intentionally covers only the basic [Backend] interface, +// and not any extension interfaces like [CLI] and [Enhanced]. Consider embedding +// this into another type if you need to mock extension interfaces too, since +// OpenTofu backend init uses type assertions to check for extension interfaces +// and so having this type implement them would prevent its use in testing +// situations that occur with non-extended backend implementations. +type MockBackend struct { + // If you add support for new methods here in future, please preserve the + // alphabetical order by function name and the other naming suffix conventions + // for each field. + + ConfigSchemaFn func() *configschema.Block + ConfigSchemaCalled bool + + ConfigureFn func(configObj cty.Value) tfdiags.Diagnostics + ConfigureCalled bool + ConfigureConfigObj cty.Value + + DeleteWorkspaceFn func(name string, force bool) error + DeleteWorkspaceCalled bool + DeleteWorkspaceName string + DeleteWorkspaceForce bool + + PrepareConfigFn func(configObj cty.Value) (cty.Value, tfdiags.Diagnostics) + PrepareConfigCalled bool + PrepareConfigConfigObj cty.Value + + StateMgrFn func(workspace string) (statemgr.Full, error) + StateMgrCalled bool + StateMgrWorkspace string + + WorkspacesFn func() ([]string, error) + WorkspacesCalled bool +} + +var _ backend.Backend = (*MockBackend)(nil) + +// ConfigSchema implements Backend. +func (m *MockBackend) ConfigSchema() *configschema.Block { + m.ConfigSchemaCalled = true + + if m.ConfigSchemaFn == nil { + // Default behavior: return an empty schema + return &configschema.Block{} + } + return m.ConfigSchemaFn() +} + +// Configure implements Backend. +func (m *MockBackend) Configure(configObj cty.Value) tfdiags.Diagnostics { + m.ConfigureCalled = true + m.ConfigureConfigObj = configObj + + if m.ConfigureFn == nil { + // Default behavior: do nothing at all, and report success + return nil + } + return m.ConfigureFn(configObj) +} + +// DeleteWorkspace implements Backend. +func (m *MockBackend) DeleteWorkspace(name string, force bool) error { + m.DeleteWorkspaceCalled = true + m.DeleteWorkspaceName = name + m.DeleteWorkspaceForce = force + + if m.DeleteWorkspaceFn == nil { + // Default behavior: do nothing at all, and report success + return nil + } + return m.DeleteWorkspaceFn(name, force) +} + +// PrepareConfig implements Backend. +func (m *MockBackend) PrepareConfig(configObj cty.Value) (cty.Value, tfdiags.Diagnostics) { + m.PrepareConfigCalled = true + m.PrepareConfigConfigObj = configObj + + if m.PrepareConfigFn == nil { + // Default behavior: just echo back the given config object and indicate success + return configObj, nil + } + return m.PrepareConfigFn(configObj) +} + +// StateMgr implements Backend. +func (m *MockBackend) StateMgr(workspace string) (statemgr.Full, error) { + m.StateMgrCalled = true + m.StateMgrWorkspace = workspace + + if m.StateMgrFn == nil { + // Default behavior: fail as if there is no workspace of the given name + return nil, fmt.Errorf("no workspace named %q", workspace) + } + return m.StateMgrFn(workspace) +} + +// Workspaces implements Backend. +func (m *MockBackend) Workspaces() ([]string, error) { + m.WorkspacesCalled = true + + if m.WorkspacesFn == nil { + // Default behavior: report no workspaces at all. + return nil, nil + } + return m.WorkspacesFn() +} diff --git a/internal/command/meta_backend.go b/internal/command/meta_backend.go index 22178bf2f9..f780414b7d 100644 --- a/internal/command/meta_backend.go +++ b/internal/command/meta_backend.go @@ -1361,7 +1361,12 @@ func (m *Meta) backendConfigNeedsMigration(c *configs.Backend, s *legacy.Backend } b := f(nil) // We don't need encryption here as it's only used for config/schema - schema := b.ConfigSchema() + // We use "NoneRequired" here because we're only evaluating the body written directly + // in the root module configuration, and we're intentionally not including any + // additional arguments passed on the command line (using -backend-config), so + // some of the required arguments might be satisfied from outside of the body we're + // evaluating here. + schema := b.ConfigSchema().NoneRequired() givenVal, diags := c.Decode(schema) if diags.HasErrors() { log.Printf("[TRACE] backendConfigNeedsMigration: failed to decode given config; migration codepath must handle problem: %s", diags.Error()) diff --git a/internal/command/meta_backend_test.go b/internal/command/meta_backend_test.go index dddee6b321..e26b5a6c55 100644 --- a/internal/command/meta_backend_test.go +++ b/internal/command/meta_backend_test.go @@ -22,6 +22,7 @@ import ( "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/backend" "github.com/opentofu/opentofu/internal/configs" + "github.com/opentofu/opentofu/internal/configs/configschema" "github.com/opentofu/opentofu/internal/copy" "github.com/opentofu/opentofu/internal/encryption" "github.com/opentofu/opentofu/internal/plans" @@ -682,6 +683,53 @@ func TestMetaBackend_configuredUnchangedWithStaticEvalVars(t *testing.T) { defer testChdir(t, testFixturePath("backend-unchanged-vars"))() + // We'll use a mock backend here because we need to control the schema to + // make sure that we always have a required field for the ConfigOverride + // argument to populate. This is covering the regression caused by the first + // fix to the original bug, discussed here: + // https://github.com/opentofu/opentofu/issues/2118 + t.Cleanup( + backendInit.RegisterTemp("_test_local", func(enc encryption.StateEncryption) backend.Backend { + return &backendInit.MockBackend{ + ConfigSchemaFn: func() *configschema.Block { + // The following is based on a subset of the normal "local" + // backend at the time of writing this test, but subsetted + // to only what we need and with all of the arguments + // marked as required (even though the real backend doesn't) + // so we can make sure that we handle required arguments + // properly. + return &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "path": { + Type: cty.String, + Required: true, + // We'll set this one in the root module, using early eval. + }, + "workspace_dir": { + Type: cty.String, + Required: true, + // We'll treat this one as if it were set with the -backend-config option to "tofu init" + }, + }, + } + }, + WorkspacesFn: func() ([]string, error) { + return []string{"default"}, nil + }, + StateMgrFn: func(workspace string) (statemgr.Full, error) { + // The migration-detection code actually fetches the state to + // decide if it's "empty" so it can avoid proposing to migrate + // an empty state, and so unfortunately we do need to have + // a relatively-realistic implementation of this. We'll + // just use the same filesystem-based implementation that + // the real local backend would use, but fixed to use our + // local-state.tfstate file from the test fixture. + return statemgr.NewFilesystem("local-state.tfstate", enc), nil + }, + } + }), + ) + // Setup the meta m := testMetaBackend(t, nil) // testMetaBackend normally sets migrateState on, because most of the tests diff --git a/internal/command/testdata/backend-unchanged-vars/.terraform/terraform.tfstate b/internal/command/testdata/backend-unchanged-vars/.terraform/terraform.tfstate index fbafddd728..697013de0a 100644 --- a/internal/command/testdata/backend-unchanged-vars/.terraform/terraform.tfstate +++ b/internal/command/testdata/backend-unchanged-vars/.terraform/terraform.tfstate @@ -3,12 +3,12 @@ "serial": 1, "lineage": "666f9301-7e65-4b19-ae23-71184bb19b03", "backend": { - "type": "local", + "type": "_test_local", "config": { "path": "local-state.tfstate", "workspace_dir": "doesnt-actually-matter-what-this-is" }, - "hash": 4282859327 + "hash": 117044127 }, "modules": [ { diff --git a/internal/command/testdata/backend-unchanged-vars/main.tf b/internal/command/testdata/backend-unchanged-vars/main.tf index 03f297754b..0ac5d29fa9 100644 --- a/internal/command/testdata/backend-unchanged-vars/main.tf +++ b/internal/command/testdata/backend-unchanged-vars/main.tf @@ -4,7 +4,7 @@ variable "state_filename" { } terraform { - backend "local" { + backend "_test_local" { path = var.state_filename } }