Merge pull request #29892 from hashicorp/lafentres/warn-about-backups-on-non-local-backends

Command state mv: Error when backup or backup-out options are used without the state option on non-local backends

internal/command/state_mv.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/backend"
 	"github.com/hashicorp/terraform/internal/command/arguments"
 	"github.com/hashicorp/terraform/internal/command/clistate"
 	"github.com/hashicorp/terraform/internal/command/views"
@@ -42,6 +43,43 @@ func (c *StateMvCommand) Run(args []string) int {
 		return cli.RunResultHelp
 	}
 
+	// If backup or backup-out options are set
+	// and the state option is not set, make sure
+	// the backend is local
+	backupOptionSetWithoutStateOption := c.backupPath != "-" && c.statePath == ""
+	backupOutOptionSetWithoutStateOption := backupPathOut != "-" && c.statePath == ""
+
+	var setLegacyLocalBackendOptions []string
+	if backupOptionSetWithoutStateOption {
+		setLegacyLocalBackendOptions = append(setLegacyLocalBackendOptions, "-backup")
+	}
+	if backupOutOptionSetWithoutStateOption {
+		setLegacyLocalBackendOptions = append(setLegacyLocalBackendOptions, "-backup-out")
+	}
+
+	if len(setLegacyLocalBackendOptions) > 0 {
+		currentBackend, diags := c.backendFromConfig(&BackendOpts{})
+		if diags.HasErrors() {
+			c.showDiagnostics(diags)
+			return 1
+		}
+
+		// If currentBackend is nil and diags didn't have errors,
+		// this means we have an implicit local backend
+		_, isLocalBackend := currentBackend.(backend.Local)
+		if currentBackend != nil && !isLocalBackend {
+			diags = diags.Append(
+				tfdiags.Sourceless(
+					tfdiags.Error,
+					fmt.Sprintf("Invalid command line options: %s", strings.Join(setLegacyLocalBackendOptions[:], ", ")),
+					"Command line options -backup and -backup-out are legacy options that operate on a local state file only. You must specify a local state file with the -state option or switch to the local backend.",
+				),
+			)
+			c.showDiagnostics(diags)
+			return 1
+		}
+	}
+
 	// Read the from state
 	stateFromMgr, err := c.State()
 	if err != nil {

internal/command/state_mv_test.go

@@ -150,6 +150,257 @@ func TestStateMv(t *testing.T) {
 
 }
 
+func TestStateMv_backupAndBackupOutOptionsWithNonLocalBackend(t *testing.T) {
+	state := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "test_instance",
+				Name: "foo",
+			}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+			&states.ResourceInstanceObjectSrc{
+				AttrsJSON: []byte(`{"id":"bar","foo":"value","bar":"value"}`),
+				Status:    states.ObjectReady,
+			},
+			addrs.AbsProviderConfig{
+				Provider: addrs.NewDefaultProvider("test"),
+				Module:   addrs.RootModule,
+			},
+		)
+	})
+
+	t.Run("backup option specified", func(t *testing.T) {
+		td := tempDir(t)
+		testCopyDir(t, testFixturePath("init-backend-http"), td)
+		defer os.RemoveAll(td)
+		defer testChdir(t, td)()
+
+		backupPath := filepath.Join(td, "backup")
+
+		// Set up our backend state using mock state
+		dataState, srv := testBackendState(t, state, 200)
+		defer srv.Close()
+		testStateFileRemote(t, dataState)
+
+		p := testProvider()
+		ui := new(cli.MockUi)
+		view, _ := testView(t)
+		c := &StateMvCommand{
+			StateMeta{
+				Meta: Meta{
+					testingOverrides: metaOverridesForProvider(p),
+					Ui:               ui,
+					View:             view,
+				},
+			},
+		}
+
+		args := []string{
+			"-backup", backupPath,
+			"test_instance.foo",
+			"test_instance.bar",
+		}
+		if code := c.Run(args); code == 0 {
+			t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String())
+		}
+
+		gotErr := ui.ErrorWriter.String()
+		wantErr := `
+Error: Invalid command line options: -backup
+
+Command line options -backup and -backup-out are legacy options that operate
+on a local state file only. You must specify a local state file with the
+-state option or switch to the local backend.
+
+`
+		if gotErr != wantErr {
+			t.Fatalf("expected error\ngot:%s\n\nwant:%s", gotErr, wantErr)
+		}
+	})
+
+	t.Run("backup-out option specified", func(t *testing.T) {
+		td := tempDir(t)
+		testCopyDir(t, testFixturePath("init-backend-http"), td)
+		defer os.RemoveAll(td)
+		defer testChdir(t, td)()
+
+		backupOutPath := filepath.Join(td, "backup-out")
+
+		// Set up our backend state using mock state
+		dataState, srv := testBackendState(t, state, 200)
+		defer srv.Close()
+		testStateFileRemote(t, dataState)
+
+		p := testProvider()
+		ui := new(cli.MockUi)
+		view, _ := testView(t)
+		c := &StateMvCommand{
+			StateMeta{
+				Meta: Meta{
+					testingOverrides: metaOverridesForProvider(p),
+					Ui:               ui,
+					View:             view,
+				},
+			},
+		}
+
+		args := []string{
+			"-backup-out", backupOutPath,
+			"test_instance.foo",
+			"test_instance.bar",
+		}
+		if code := c.Run(args); code == 0 {
+			t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String())
+		}
+
+		gotErr := ui.ErrorWriter.String()
+		wantErr := `
+Error: Invalid command line options: -backup-out
+
+Command line options -backup and -backup-out are legacy options that operate
+on a local state file only. You must specify a local state file with the
+-state option or switch to the local backend.
+
+`
+		if gotErr != wantErr {
+			t.Fatalf("expected error\ngot:%s\n\nwant:%s", gotErr, wantErr)
+		}
+	})
+
+	t.Run("backup and backup-out options specified", func(t *testing.T) {
+		td := tempDir(t)
+		testCopyDir(t, testFixturePath("init-backend-http"), td)
+		defer os.RemoveAll(td)
+		defer testChdir(t, td)()
+
+		backupPath := filepath.Join(td, "backup")
+		backupOutPath := filepath.Join(td, "backup-out")
+
+		// Set up our backend state using mock state
+		dataState, srv := testBackendState(t, state, 200)
+		defer srv.Close()
+		testStateFileRemote(t, dataState)
+
+		p := testProvider()
+		ui := new(cli.MockUi)
+		view, _ := testView(t)
+		c := &StateMvCommand{
+			StateMeta{
+				Meta: Meta{
+					testingOverrides: metaOverridesForProvider(p),
+					Ui:               ui,
+					View:             view,
+				},
+			},
+		}
+
+		args := []string{
+			"-backup", backupPath,
+			"-backup-out", backupOutPath,
+			"test_instance.foo",
+			"test_instance.bar",
+		}
+		if code := c.Run(args); code == 0 {
+			t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String())
+		}
+
+		gotErr := ui.ErrorWriter.String()
+		wantErr := `
+Error: Invalid command line options: -backup, -backup-out
+
+Command line options -backup and -backup-out are legacy options that operate
+on a local state file only. You must specify a local state file with the
+-state option or switch to the local backend.
+
+`
+		if gotErr != wantErr {
+			t.Fatalf("expected error\ngot:%s\n\nwant:%s", gotErr, wantErr)
+		}
+	})
+
+	t.Run("backup option specified with state option", func(t *testing.T) {
+		td := tempDir(t)
+		testCopyDir(t, testFixturePath("init-backend-http"), td)
+		defer os.RemoveAll(td)
+		defer testChdir(t, td)()
+
+		statePath := testStateFile(t, state)
+		backupPath := filepath.Join(td, "backup")
+
+		// Set up our backend state using mock state
+		dataState, srv := testBackendState(t, state, 200)
+		defer srv.Close()
+		testStateFileRemote(t, dataState)
+
+		p := testProvider()
+		ui := new(cli.MockUi)
+		view, _ := testView(t)
+		c := &StateMvCommand{
+			StateMeta{
+				Meta: Meta{
+					testingOverrides: metaOverridesForProvider(p),
+					Ui:               ui,
+					View:             view,
+				},
+			},
+		}
+
+		args := []string{
+			"-state", statePath,
+			"-backup", backupPath,
+			"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, testStateMvBackupAndBackupOutOptionsWithNonLocalBackendOutput)
+	})
+
+	t.Run("backup-out option specified with state option", func(t *testing.T) {
+		td := tempDir(t)
+		testCopyDir(t, testFixturePath("init-backend-http"), td)
+		defer os.RemoveAll(td)
+		defer testChdir(t, td)()
+
+		statePath := testStateFile(t, state)
+		backupOutPath := filepath.Join(td, "backup-out")
+
+		// Set up our backend state using mock state
+		dataState, srv := testBackendState(t, state, 200)
+		defer srv.Close()
+		testStateFileRemote(t, dataState)
+
+		p := testProvider()
+		ui := new(cli.MockUi)
+		view, _ := testView(t)
+		c := &StateMvCommand{
+			StateMeta{
+				Meta: Meta{
+					testingOverrides: metaOverridesForProvider(p),
+					Ui:               ui,
+					View:             view,
+				},
+			},
+		}
+
+		args := []string{
+			"-state", statePath,
+			"-backup-out", backupOutPath,
+			"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, testStateMvBackupAndBackupOutOptionsWithNonLocalBackendOutput)
+	})
+}
+
 func TestStateMv_resourceToInstance(t *testing.T) {
 	// A single resource (no count defined)
 	state := states.BuildState(func(s *states.SyncState) {
@@ -1463,6 +1714,14 @@ test_instance.baz:
   foo = value
 `
 
+const testStateMvBackupAndBackupOutOptionsWithNonLocalBackendOutput = `
+test_instance.bar:
+  ID = bar
+  provider = provider["registry.terraform.io/hashicorp/test"]
+  bar = value
+  foo = value
+`
+
 const testStateMvCount_stateOut = `
 test_instance.bar.0:
   ID = foo

internal/command/testdata/init-backend-http/main.tf

@@ -0,0 +1,4 @@
+terraform {
+  backend "http" {
+  }
+}

website/docs/cli/commands/state/mv.html.md

@@ -73,10 +73,18 @@ only, `terraform state mv`
 also accepts the option
 [`-ignore-remote-version`](/docs/language/settings/backends/remote.html#command-line-arguments).
 
+The legacy options [`-backup` and `-backup-out`](/docs/language/settings/backends/local.html#command-line-arguments)
+operate on a local state file only. Configurations using
+[the `remote` backend](/docs/language/settings/backends/remote.html) 
+must specify a local state file with the [`-state`](/docs/language/settings/backends/local.html#command-line-arguments)
+option in order to use the [`-backup` and `-backup-out`](/docs/language/settings/backends/local.html#command-line-arguments)
+options.
+
+
 For configurations using
 [the `local` state mv](/docs/language/settings/backends/local.html) only,
-`terraform taint` also accepts the legacy options
+`terraform state mv` also accepts the legacy options
-[`-state`, `-state-out`, and `-backup`](/docs/language/settings/backends/local.html#command-line-arguments).
+[`-state`, `-state-out`, `-backup`, and `-backup-out`](/docs/language/settings/backends/local.html#command-line-arguments).
 
 ## Example: Rename a Resource