Merge pull request #15388 from hashicorp/jbardin/state-mv-backend

terraform state commands ignore the -state flag

command/state_meta.go

@@ -18,6 +18,16 @@ type StateMeta struct{}
 // backups to be timestamped rather than just the original state path plus a
 // backup path.
 func (c *StateMeta) State(m *Meta) (state.State, error) {
+	var realState state.State
+	backupPath := m.backupPath
+	stateOutPath := m.statePath
+
+	// use the specified state
+	if m.statePath != "" {
+		realState = &state.LocalState{
+			Path: m.statePath,
+		}
+	} else {
 		// Load the backend
 		b, err := m.Backend(nil)
 		if err != nil {
@@ -38,26 +48,33 @@ func (c *StateMeta) State(m *Meta) (state.State, error) {
 			panic(err)
 		}
 		localB := localRaw.(*backendlocal.Local)
-	_, stateOutPath, _ := localB.StatePaths(env)
+		_, stateOutPath, _ = localB.StatePaths(env)
 		if err != nil {
 			return nil, err
 		}
 
+		realState = s
+	}
+
+	// We always backup state commands, so set the back if none was specified
+	// (the default is "-", but some tests bypass the flag parsing).
+	if backupPath == "-" || backupPath == "" {
 		// Determine the backup path. stateOutPath is set to the resulting
 		// file where state is written (cached in the case of remote state)
-	backupPath := fmt.Sprintf(
+		backupPath = fmt.Sprintf(
 			"%s.%d%s",
 			stateOutPath,
 			time.Now().UTC().Unix(),
 			DefaultBackupExtension)
+	}
 
 	// Wrap it for backups
-	s = &state.BackupState{
+	realState = &state.BackupState{
-		Real: s,
+		Real: realState,
 		Path: backupPath,
 	}
 
-	return s, nil
+	return realState, nil
 }
 
 // filterInstance filters a single instance out of filter results.

command/state_mv.go

@@ -203,8 +203,7 @@ Options:
   -backup=PATH        Path where Terraform should write the backup for the original
                       state. This can't be disabled. If not set, Terraform
                       will write it to the same path as the statefile with
-                      a backup extension. This backup will be made in addition
+                      a backup extension.
-                      to the timestamped backup.
 
   -backup-out=PATH    Path where Terraform should write the backup for the destination
                       state. This can't be disabled. If not set, Terraform

command/state_mv_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/hashicorp/terraform/helper/copy"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/cli"
 )
@@ -72,6 +73,86 @@ func TestStateMv(t *testing.T) {
 	testStateOutput(t, backups[0], testStateMvOutputOriginal)
 }
 
+// don't modify backend state is we supply a -state flag
+func TestStateMv_explicitWithBackend(t *testing.T) {
+	td := tempDir(t)
+	copy.CopyDir(testFixturePath("init-backend"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	backupPath := filepath.Join(td, "backup")
+
+	state := &terraform.State{
+		Modules: []*terraform.ModuleState{
+			&terraform.ModuleState{
+				Path: []string{"root"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_instance.foo": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.baz": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	statePath := testStateFile(t, state)
+
+	// init our backend
+	ui := new(cli.MockUi)
+	ic := &InitCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(testProvider()),
+			Ui:               ui,
+		},
+	}
+
+	args := []string{}
+	if code := ic.Run(args); code != 0 {
+		t.Fatalf("bad: \n%s", ui.ErrorWriter.String())
+	}
+
+	// only modify statePath
+	p := testProvider()
+	ui = new(cli.MockUi)
+	c := &StateMvCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			Ui:               ui,
+		},
+	}
+
+	args = []string{
+		"-backup", backupPath,
+		"-state", statePath,
+		"test_instance.foo",
+		"test_instance.bar",
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// Test it is correct
+	testStateOutput(t, statePath, testStateMvOutput)
+}
+
 func TestStateMv_backupExplicit(t *testing.T) {
 	td := tempDir(t)
 	defer os.RemoveAll(td)
@@ -132,12 +213,7 @@ func TestStateMv_backupExplicit(t *testing.T) {
 	// Test it is correct
 	testStateOutput(t, statePath, testStateMvOutput)
 
-	// Test we have backups
+	// Test backup
-	backups := testStateBackups(t, filepath.Dir(statePath))
-	if len(backups) != 1 {
-		t.Fatalf("bad: %#v", backups)
-	}
-	testStateOutput(t, backups[0], testStateMvOutputOriginal)
 	testStateOutput(t, backupPath, testStateMvOutputOriginal)
 }
 

command/state_rm.go

@@ -78,8 +78,7 @@ Options:
   -backup=PATH        Path where Terraform should write the backup
                       state. This can't be disabled. If not set, Terraform
                       will write it to the same path as the statefile with
-                      a backup extension. This backup will be made in addition
+                      a backup extension.
-                      to the timestamped backup.
 
   -state=statefile    Path to a Terraform state file to use to look
                       up Terraform-managed resources. By default it will

command/state_rm_test.go

@@ -130,12 +130,7 @@ func TestStateRm_backupExplicit(t *testing.T) {
 	// Test it is correct
 	testStateOutput(t, statePath, testStateRmOutput)
 
-	// Test we have backups
+	// Test backup
-	backups := testStateBackups(t, filepath.Dir(statePath))
-	if len(backups) != 1 {
-		t.Fatalf("bad: %#v", backups)
-	}
-	testStateOutput(t, backups[0], testStateRmOutputOriginal)
 	testStateOutput(t, backupPath, testStateRmOutputOriginal)
 }