Skip to content

state/remote: Generate error when force push is requested but not supported #26042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backend/remote-state/inmem/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
16 changes: 11 additions & 5 deletions states/remote/remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
5 changes: 4 additions & 1 deletion states/remote/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
164 changes: 163 additions & 1 deletion states/remote/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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")
Expand Down