From c44c589f0ab64b8dd4ba964f9c671a9c9bf82a88 Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Fri, 29 Oct 2021 14:53:18 -0500 Subject: [PATCH 1/2] Error if backup or backup-out options are used without the state option on non-local backends for the state mv command --- internal/command/state_mv.go | 38 +++ internal/command/state_mv_test.go | 259 ++++++++++++++++++ .../testdata/init-backend-http/main.tf | 4 + 3 files changed, 301 insertions(+) create mode 100644 internal/command/testdata/init-backend-http/main.tf diff --git a/internal/command/state_mv.go b/internal/command/state_mv.go index 2a0cd5d48..a35cfd6aa 100644 --- a/internal/command/state_mv.go +++ b/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 { diff --git a/internal/command/state_mv_test.go b/internal/command/state_mv_test.go index 1106879f4..2778de61b 100644 --- a/internal/command/state_mv_test.go +++ b/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 diff --git a/internal/command/testdata/init-backend-http/main.tf b/internal/command/testdata/init-backend-http/main.tf new file mode 100644 index 000000000..4ca44e9b5 --- /dev/null +++ b/internal/command/testdata/init-backend-http/main.tf @@ -0,0 +1,4 @@ +terraform { + backend "http" { + } +} From a62cff939edfeeb84188e6a23b86a2a3eba68e4e Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Fri, 5 Nov 2021 15:26:22 -0500 Subject: [PATCH 2/2] Update documentation for state mv command to clarify use of backup and backup-out options --- website/docs/cli/commands/state/mv.html.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/website/docs/cli/commands/state/mv.html.md b/website/docs/cli/commands/state/mv.html.md index 7670ae7a4..2cf2f8cd6 100644 --- a/website/docs/cli/commands/state/mv.html.md +++ b/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 -[`-state`, `-state-out`, and `-backup`](/docs/language/settings/backends/local.html#command-line-arguments). +`terraform state mv` also accepts the legacy options +[`-state`, `-state-out`, `-backup`, and `-backup-out`](/docs/language/settings/backends/local.html#command-line-arguments). ## Example: Rename a Resource