From 52c77ba33effd041a9ce7956eb6809f676ed8587 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Tue, 12 Jan 2021 12:59:28 -0500 Subject: [PATCH 1/2] Print addresses in state mv errors Using the addrTo after it has failed its check means /no address will be printed. Change this throughout, but particularly add a test for the origin issue for this. --- command/state_mv.go | 6 ++-- command/state_mv_test.go | 67 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/command/state_mv.go b/command/state_mv.go index 912f128c8..818811ddf 100644 --- a/command/state_mv.go +++ b/command/state_mv.go @@ -139,7 +139,7 @@ func (c *StateMvCommand) Run(args []string) int { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, msgInvalidTarget, - fmt.Sprintf("Cannot move %s to %s: the target must also be a module.", addrFrom, addrTo), + fmt.Sprintf("Cannot move %s to %s: the target must also be a module.", addrFrom, destAddr), )) c.showDiagnostics(diags) return 1 @@ -184,7 +184,7 @@ func (c *StateMvCommand) Run(args []string) int { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, msgInvalidTarget, - fmt.Sprintf("Cannot move %s to %s: the target must also be a whole resource.", addrFrom, addrTo), + fmt.Sprintf("Cannot move %s to %s: the target must also be a whole resource.", addrFrom, destAddr), )) c.showDiagnostics(diags) return 1 @@ -231,7 +231,7 @@ func (c *StateMvCommand) Run(args []string) int { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, msgInvalidTarget, - fmt.Sprintf("Cannot move %s to %s: the target must also be a resource instance.", addrFrom, addrTo), + fmt.Sprintf("Cannot move %s to %s: the target must also be a resource instance.", addrFrom, destAddr), )) c.showDiagnostics(diags) return 1 diff --git a/command/state_mv_test.go b/command/state_mv_test.go index 7a631c44f..558246b5c 100644 --- a/command/state_mv_test.go +++ b/command/state_mv_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/mitchellh/cli" "github.com/hashicorp/terraform/addrs" @@ -148,6 +149,7 @@ func TestStateMv(t *testing.T) { } func TestStateMv_resourceToInstance(t *testing.T) { + // A single resource (no count defined) state := states.BuildState(func(s *states.SyncState) { s.SetResourceInstanceCurrent( addrs.Resource{ @@ -236,6 +238,71 @@ test_instance.baz: testStateOutput(t, backups[0], testStateMvOutputOriginal) } +func TestStateMv_resourceToInstanceErr(t *testing.T) { + state := states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "foo", + }.Instance(addrs.IntKey(0)).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, + }, + ) + s.SetResourceProvider( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test_instance", + Name: "bar", + }.Absolute(addrs.RootModuleInstance), + addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("test"), + Module: addrs.RootModule, + }, + ) + }) + statePath := testStateFile(t, state) + + p := testProvider() + ui := new(cli.MockUi) + c := &StateMvCommand{ + StateMeta{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + Ui: ui, + }, + }, + } + + args := []string{ + "-state", statePath, + "test_instance.foo", + "test_instance.bar[0]", + } + + if code := c.Run(args); code == 0 { + t.Fatalf("expected error output, got:\n%s", ui.OutputWriter.String()) + } + + expectedErr := ` +Error: Invalid target address + +Cannot move test_instance.foo to test_instance.bar[0]: the target must also be +a whole resource. + +` + errOutput := ui.ErrorWriter.String() + if errOutput != expectedErr { + t.Errorf("Unexpected diff.\ngot:\n%s\nwant:\n%s\n", errOutput, expectedErr) + t.Errorf("%s", cmp.Diff(errOutput, expectedErr)) + } +} func TestStateMv_instanceToResource(t *testing.T) { state := states.BuildState(func(s *states.SyncState) { s.SetResourceInstanceCurrent( From 9371b60d71e69d03d9ec643bdfb88a499a3ed76e Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Tue, 12 Jan 2021 13:21:25 -0500 Subject: [PATCH 2/2] More verbose error for whole resource move failure When running state mv with a resource source, but the destination fails, provide a hint that the source is a resource (not an instance) in case the user means to address it this way --- command/state_mv.go | 2 +- command/state_mv_test.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/command/state_mv.go b/command/state_mv.go index 818811ddf..20282382a 100644 --- a/command/state_mv.go +++ b/command/state_mv.go @@ -184,7 +184,7 @@ func (c *StateMvCommand) Run(args []string) int { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, msgInvalidTarget, - fmt.Sprintf("Cannot move %s to %s: the target must also be a whole resource.", addrFrom, destAddr), + fmt.Sprintf("Cannot move %s to %s: the source is a whole resource (not a resource instance) so the target must also be a whole resource.", addrFrom, destAddr), )) c.showDiagnostics(diags) return 1 diff --git a/command/state_mv_test.go b/command/state_mv_test.go index 558246b5c..b9c21e5b7 100644 --- a/command/state_mv_test.go +++ b/command/state_mv_test.go @@ -293,8 +293,9 @@ func TestStateMv_resourceToInstanceErr(t *testing.T) { expectedErr := ` Error: Invalid target address -Cannot move test_instance.foo to test_instance.bar[0]: the target must also be -a whole resource. +Cannot move test_instance.foo to test_instance.bar[0]: the source is a whole +resource (not a resource instance) so the target must also be a whole +resource. ` errOutput := ui.ErrorWriter.String()