backend/local: disable local backup of remote state

Previously we forced all remote state backends to be wrapped in a BackupState wrapper that generates a local "terraform.tfstate.backup" file before updating the remote state. This backup mechanism was motivated by allowing users to recover a previous state if user error caused an undesirable change such as loss of the record of one or more resources. However, it also has the downside of flushing a possibly-sensitive state to local disk in a location where users may not realize its purpose and accidentally check it into version control. Those using remote state would generally prefer that state never be flushed to local disk at all. The use-case of recovering older states can be dealt with for remote backends by selecting a backend that has preservation of older versions as a first-class feature, such as S3 versioning or Terraform Enterprise's first-class historical state versioning mechanism. There remains still one case where state can be flushed to local disk: if a write to the remote backend fails during "terraform apply" then we will still create the "errored.tfstate" file to allow the user to recover. This seems like a reasonable compromise because this is done only in an _exceptional_ case, and the console output makes it very clear that this file has been created. Fixes #15339.
hashicorp · Oct 28, 2017 · 671aace · 671aace
1 parent 9df2389
commit 671aace
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 58 deletions.
diff --git a/backend/local/backend.go b/backend/local/backend.go
@@ -180,28 180,9 @@ func (b *Local) DeleteState(name string) error {
 func (b *Local) State(name string) (state.State, error) {
  statePath, stateOutPath, backupPath := b.StatePaths(name)
 
- // If we have a backend handling state, defer to that.
  // If we have a backend handling state, delegate to that.
  if b.Backend != nil {
- s, err := b.Backend.State(name)
- if err != nil {
- return nil, err
- }
-
- // make sure we always have a backup state, unless it disabled
- if backupPath == "" {
- return s, nil
- }
-
- // see if the delegated backend returned a BackupState of its own
- if s, ok := s.(*state.BackupState); ok {
- return s, nil
- }
-
- s = &state.BackupState{
- Real: s,
- Path: backupPath,
- }
- return s, nil
  return b.Backend.State(name)
  }
 
  if s, ok := b.states[name]; ok {

diff --git a/backend/local/backend_test.go b/backend/local/backend_test.go
@@ -229,43 229,6 @@ func TestLocal_multiStateBackend(t *testing.T) {
  }
 }
 
-// verify that a remote state backend is always wrapped in a BackupState
-func TestLocal_remoteStateBackup(t *testing.T) {
- // assign a separate backend to mock a remote state backend
- b := &Local{
- Backend: &testDelegateBackend{},
- }
-
- s, err := b.State("default")
- if err != nil {
- t.Fatal(err)
- }
-
- bs, ok := s.(*state.BackupState)
- if !ok {
- t.Fatal("remote state is not backed up")
- }
-
- if bs.Path != DefaultStateFilename DefaultBackupExtension {
- t.Fatal("bad backup location:", bs.Path)
- }
-
- // do the same with a named state, which should use the local env directories
- s, err = b.State("test")
- if err != nil {
- t.Fatal(err)
- }
-
- bs, ok = s.(*state.BackupState)
- if !ok {
- t.Fatal("remote state is not backed up")
- }
-
- if bs.Path != filepath.Join(DefaultWorkspaceDir, "test", DefaultStateFilename DefaultBackupExtension) {
- t.Fatal("bad backup location:", bs.Path)
- }
-}
-
 // change into a tmp dir and return a deferable func to change back and cleanup
 func testTmpDir(t *testing.T) func() {
  tmp, err := ioutil.TempDir("", "tf")