From 671aace8ec069a17e6c4ebaecdea93448c570ed9 Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 26 Oct 2017 11:23:26 -0700
Subject: [PATCH] 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.
---
 backend/local/backend.go      | 23 ++--------------------
 backend/local/backend_test.go | 37 -----------------------------------
 2 files changed, 2 insertions(+), 58 deletions(-)

diff --git a/backend/local/backend.go b/backend/local/backend.go
index 3f6f61847..6b119c297 100644
--- 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
index 2b70df2fc..c22b92cd6 100644
--- 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")