diff --git a/backend/remote-state/inmem/client.go b/backend/remote-state/inmem/client.go index 21f229bdacf2..9933deace5f1 100644 --- a/backend/remote-state/inmem/client.go +++ b/backend/remote-state/inmem/client.go @@ -45,3 +45,5 @@ func (c *RemoteClient) Lock(info *statemgr.LockInfo) (string, error) { func (c *RemoteClient) Unlock(id string) error { return locks.unlock(c.Name, id) } + +func (c *RemoteClient) EnableForcePush() {} diff --git a/states/remote/remote_test.go b/states/remote/remote_test.go index bbd73ae5f41d..b39188469a5b 100644 --- a/states/remote/remote_test.go +++ b/states/remote/remote_test.go @@ -105,11 +105,6 @@ func (c *mockClient) Delete() error { return nil } -// Implements remote.ClientForcePusher -func (c *mockClient) EnableForcePush() { - c.force = true -} - func (c *mockClient) appendLog(method string, content []byte) { // For easier test assertions, we actually log the result of decoding // the content JSON rather than the raw bytes. Callers are in principle @@ -126,3 +121,14 @@ func (c *mockClient) appendLog(method string, content []byte) { } c.log = append(c.log, mockClientRequest{method, contentVal}) } + +// mockForceClient is a client which simply extends the mockClient +// to work as a remote.ClientForcePusher. +type mockForceClient struct { + mockClient +} + +// Implements remote.ClientForcePusher +func (c *mockForceClient) EnableForcePush() { + c.force = true +} diff --git a/states/remote/state.go b/states/remote/state.go index bd9494f53ee4..d635ddeb9264 100644 --- a/states/remote/state.go +++ b/states/remote/state.go @@ -77,7 +77,10 @@ func (s *State) WriteStateForMigration(f *statefile.File, force bool) error { // in the backend. If force is specified we skip verifications and hand the // context off to the client to use when persitence operations actually take place. c, isForcePusher := s.Client.(ClientForcePusher) - if force && isForcePusher { + if force && !isForcePusher { + return fmt.Errorf("The configured backend does not support forced state push.") + } + if force { c.EnableForcePush() } else { checkFile := statefile.New(s.state, s.lineage, s.serial) diff --git a/states/remote/state_test.go b/states/remote/state_test.go index 413c6e75ed3d..453d7965a326 100644 --- a/states/remote/state_test.go +++ b/states/remote/state_test.go @@ -302,6 +302,168 @@ func TestWriteStateForMigration(t *testing.T) { }, } + testCases := []migrationTestCase{ + // Refreshing state before we run the test loop causes a GET + { + name: "refresh state", + stateFile: func(mgr *State) *statefile.File { + return mgr.StateForMigration() + }, + expectedRequest: mockClientRequest{ + Method: "Get", + Content: map[string]interface{}{ + "version": 4.0, + "lineage": "mock-lineage", + "serial": 3.0, + "terraform_version": "0.0.0", + "outputs": map[string]interface{}{"foo": map[string]interface{}{"type": string("string"), "value": string("bar")}}, + "resources": []interface{}{}, + }, + }, + }, + { + name: "cannot import lesser serial without force", + stateFile: func(mgr *State) *statefile.File { + return statefile.New(mgr.state, mgr.lineage, 1) + }, + expectedError: "cannot import state with serial 1 over newer state with serial 3", + }, + { + name: "cannot import differing lineage without force", + stateFile: func(mgr *State) *statefile.File { + return statefile.New(mgr.state, "different-lineage", mgr.serial) + }, + expectedError: `cannot import state with lineage "different-lineage" over unrelated state with lineage "mock-lineage"`, + }, + { + name: "cannot import lesser serial with force", + stateFile: func(mgr *State) *statefile.File { + return statefile.New(mgr.state, mgr.lineage, 1) + }, + expectedRequest: mockClientRequest{ + Method: "Force Put", + Content: map[string]interface{}{ + "version": 4.0, + "lineage": "mock-lineage", + "serial": 2.0, + "terraform_version": version.Version, + "outputs": map[string]interface{}{"foo": map[string]interface{}{"type": string("string"), "value": string("bar")}}, + "resources": []interface{}{}, + }, + }, + force: true, + expectedError: "The configured backend does not support forced state push.", + }, + { + name: "cannot import differing lineage with force", + stateFile: func(mgr *State) *statefile.File { + return statefile.New(mgr.state, "different-lineage", mgr.serial) + }, + expectedRequest: mockClientRequest{ + Method: "Force Put", + Content: map[string]interface{}{ + "version": 4.0, + "lineage": "different-lineage", + "serial": 3.0, + "terraform_version": version.Version, + "outputs": map[string]interface{}{"foo": map[string]interface{}{"type": string("string"), "value": string("bar")}}, + "resources": []interface{}{}, + }, + }, + force: true, + expectedError: "The configured backend does not support forced state push.", + }, + } + + // In normal use (during a Terraform operation) we always refresh and read + // before any writes would happen, so we'll mimic that here for realism. + // NB This causes a GET to be logged so the first item in the test cases + // must account for this + if err := mgr.RefreshState(); err != nil { + t.Fatalf("failed to RefreshState: %s", err) + } + + if err := mgr.WriteState(mgr.State()); err != nil { + t.Fatalf("failed to write initial state: %s", err) + } + + // Our client is a mockClient which has a log we + // use to check that operations generate expected requests + mockClient := mgr.Client.(*mockClient) + + if mockClient.force { + t.Fatalf("client should not default to force") + } + + // logIdx tracks the current index of the log separate from + // the loop iteration so we can check operations that don't + // cause any requests to be generated + logIdx := 0 + + for _, tc := range testCases { + // Always reset client to not be force pushing + mockClient.force = false + sf := tc.stateFile(mgr) + err := mgr.WriteStateForMigration(sf, tc.force) + shouldError := tc.expectedError != "" + + // If we are expecting and error check it and move on + if shouldError { + if err == nil { + t.Fatalf("test case %q should have failed with error %q", tc.name, tc.expectedError) + } else if err.Error() != tc.expectedError { + t.Fatalf("test case %q expected error %q but got %q", tc.name, tc.expectedError, err) + } + continue + } + + if err != nil { + t.Fatalf("test case %q failed: %v", tc.name, err) + } + + if tc.force && !mockClient.force { + t.Fatalf("test case %q should have enabled force push", tc.name) + } + + // At this point we should just do a normal write and persist + // as would happen from the CLI + mgr.WriteState(mgr.State()) + mgr.PersistState() + + if logIdx >= len(mockClient.log) { + t.Fatalf("request lock and index are out of sync on %q: idx=%d len=%d", tc.name, logIdx, len(mockClient.log)) + } + loggedRequest := mockClient.log[logIdx] + logIdx++ + if diff := cmp.Diff(tc.expectedRequest, loggedRequest); len(diff) > 0 { + t.Fatalf("incorrect client requests for %q:\n%s", tc.name, diff) + } + } + + logCnt := len(mockClient.log) + if logIdx != logCnt { + log.Fatalf("not all requests were read. Expected logIdx to be %d but got %d", logCnt, logIdx) + } +} + +func TestWriteStateForForcedMigration(t *testing.T) { + mgr := &State{ + Client: &mockForceClient{ + mockClient: mockClient{ + current: []byte(` + { + "version": 4, + "lineage": "mock-lineage", + "serial": 3, + "terraform_version":"0.0.0", + "outputs": {"foo": {"value":"bar", "type": "string"}}, + "resources": [] + } + `), + }, + }, + } + testCases := []migrationTestCase{ // Refreshing state before we run the test loop causes a GET { @@ -387,7 +549,7 @@ func TestWriteStateForMigration(t *testing.T) { // Our client is a mockClient which has a log we // use to check that operations generate expected requests - mockClient := mgr.Client.(*mockClient) + mockClient := mgr.Client.(*mockForceClient) if mockClient.force { t.Fatalf("client should not default to force")