From e4d2193ed66c906f62883085033f7e41a8af4eed Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 21 Feb 2017 21:10:03 -0800 Subject: [PATCH] command/state: mv and rm -backup works Fixes #12154 The "-backup" flag before for "state *" CLI had some REALLY bizarre behavior: it would change the _destination_ state and actually not create any additional backup at all (the original state was unchanged and the normal timestamped backup still are written). Really weird. This PR makes the -backup flag work as you'd expect with one caveat: we'll _still_ create the timestamped backup file. The timestamped backup file helps make sure that you always get a backup history when using these commands. We don't want to make it easy for you to overwrite a state with the `-backup` flag. --- command/state_meta.go | 3 -- command/state_mv.go | 7 ++-- command/state_mv_test.go | 70 ++++++++++++++++++++++++++++++++++++++++ command/state_rm.go | 6 ++-- command/state_rm_test.go | 69 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 9 deletions(-) diff --git a/command/state_meta.go b/command/state_meta.go index 5a136d4d8..9155d0f95 100644 --- a/command/state_meta.go +++ b/command/state_meta.go @@ -17,9 +17,6 @@ type StateMeta struct{} // in the way that backups are done. This configures backups to be timestamped // rather than just the original state path plus a backup path. func (c *StateMeta) State(m *Meta) (state.State, error) { - // Disable backups since we wrap it manually below - m.backupPath = "-" - // Load the backend b, err := m.Backend(nil) if err != nil { diff --git a/command/state_mv.go b/command/state_mv.go index 37ce53bea..ddcc33517 100644 --- a/command/state_mv.go +++ b/command/state_mv.go @@ -20,9 +20,9 @@ func (c *StateMvCommand) Run(args []string) int { // We create two metas to track the two states var meta1, meta2 Meta cmdFlags := c.Meta.flagSet("state mv") - cmdFlags.StringVar(&meta1.stateOutPath, "backup", "", "backup") + cmdFlags.StringVar(&meta1.backupPath, "backup", "-", "backup") cmdFlags.StringVar(&meta1.statePath, "state", DefaultStateFilename, "path") - cmdFlags.StringVar(&meta2.stateOutPath, "backup-out", "", "backup") + cmdFlags.StringVar(&meta2.backupPath, "backup-out", "-", "backup") cmdFlags.StringVar(&meta2.statePath, "state-out", "", "path") if err := cmdFlags.Parse(args); err != nil { return cli.RunResultHelp @@ -193,7 +193,8 @@ 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. + a backup extension. This backup will be made in addition + 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 diff --git a/command/state_mv_test.go b/command/state_mv_test.go index 4a81e79b2..9033db4e0 100644 --- a/command/state_mv_test.go +++ b/command/state_mv_test.go @@ -1,6 +1,7 @@ package command import ( + "os" "path/filepath" "testing" @@ -71,6 +72,75 @@ func TestStateMv(t *testing.T) { testStateOutput(t, backups[0], testStateMvOutputOriginal) } +func TestStateMv_backupExplicit(t *testing.T) { + td := tempDir(t) + defer os.RemoveAll(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) + + p := testProvider() + ui := new(cli.MockUi) + c := &StateMvCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(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) + + // Test we have backups + backups := testStateBackups(t, filepath.Dir(statePath)) + if len(backups) != 1 { + t.Fatalf("bad: %#v", backups) + } + testStateOutput(t, backups[0], testStateMvOutputOriginal) + testStateOutput(t, backupPath, testStateMvOutputOriginal) +} + func TestStateMv_stateOutNew(t *testing.T) { state := &terraform.State{ Modules: []*terraform.ModuleState{ diff --git a/command/state_rm.go b/command/state_rm.go index 273fd46aa..d2173622d 100644 --- a/command/state_rm.go +++ b/command/state_rm.go @@ -16,9 +16,8 @@ type StateRmCommand struct { func (c *StateRmCommand) Run(args []string) int { args = c.Meta.process(args, true) - var backupPath string cmdFlags := c.Meta.flagSet("state show") - cmdFlags.StringVar(&backupPath, "backup", "", "backup") + cmdFlags.StringVar(&c.Meta.backupPath, "backup", "-", "backup") cmdFlags.StringVar(&c.Meta.statePath, "state", DefaultStateFilename, "path") if err := cmdFlags.Parse(args); err != nil { return cli.RunResultHelp @@ -75,7 +74,8 @@ 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. + a backup extension. This backup will be made in addition + to the timestamped backup. -state=statefile Path to a Terraform state file to use to look up Terraform-managed resources. By default it will diff --git a/command/state_rm_test.go b/command/state_rm_test.go index 036fed159..bb3734502 100644 --- a/command/state_rm_test.go +++ b/command/state_rm_test.go @@ -1,6 +1,7 @@ package command import ( + "os" "path/filepath" "testing" @@ -70,6 +71,74 @@ func TestStateRm(t *testing.T) { testStateOutput(t, backups[0], testStateRmOutputOriginal) } +func TestStateRm_backupExplicit(t *testing.T) { + td := tempDir(t) + defer os.RemoveAll(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.bar": &terraform.ResourceState{ + Type: "test_instance", + Primary: &terraform.InstanceState{ + ID: "foo", + Attributes: map[string]string{ + "foo": "value", + "bar": "value", + }, + }, + }, + }, + }, + }, + } + + statePath := testStateFile(t, state) + + p := testProvider() + ui := new(cli.MockUi) + c := &StateRmCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + args := []string{ + "-backup", backupPath, + "-state", statePath, + "test_instance.foo", + } + 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, testStateRmOutput) + + // Test we have backups + backups := testStateBackups(t, filepath.Dir(statePath)) + if len(backups) != 1 { + t.Fatalf("bad: %#v", backups) + } + testStateOutput(t, backups[0], testStateRmOutputOriginal) + testStateOutput(t, backupPath, testStateRmOutputOriginal) +} + func TestStateRm_noState(t *testing.T) { tmp, cwd := testCwd(t) defer testFixCwd(t, tmp, cwd)