diff --git a/internal/terraform/context_plan.go b/internal/terraform/context_plan.go index a48676ffa..084a8ab8a 100644 --- a/internal/terraform/context_plan.go +++ b/internal/terraform/context_plan.go @@ -1,6 +1,7 @@ package terraform import ( + "bytes" "fmt" "log" "sort" @@ -417,6 +418,14 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, r diags = diags.Append(walker.NonFatalDiagnostics) diags = diags.Append(walkDiags) diags = diags.Append(c.postPlanValidateMoves(config, moveStmts, walker.InstanceExpander.AllInstances())) + if len(moveResults.Blocked) > 0 && !diags.HasErrors() { + // If we had blocked moves and we're not going to be returning errors + // then we'll report the blockers as a warning. We do this only in the + // absense of errors because invalid move statements might well be + // the root cause of the blockers, and so better to give an actionable + // error message than a less-actionable warning. + diags = diags.Append(blockedMovesWarningDiag(moveResults)) + } prevRunState = walker.PrevRunState.Close() priorState := walker.RefreshState.Close() @@ -633,3 +642,24 @@ func (c *Context) PlanGraphForUI(config *configs.Config, prevRunState *states.St diags = diags.Append(moreDiags) return graph, diags } + +func blockedMovesWarningDiag(results refactoring.MoveResults) tfdiags.Diagnostic { + if len(results.Blocked) < 1 { + // Caller should check first + panic("request to render blocked moves warning without any blocked moves") + } + + var itemsBuf bytes.Buffer + for _, blocked := range results.Blocked { + fmt.Fprintf(&itemsBuf, "\n - %s could not move to %s", blocked.Actual, blocked.Wanted) + } + + return tfdiags.Sourceless( + tfdiags.Warning, + "Unresolved resource instance address changes", + fmt.Sprintf( + "Terraform tried to adjust resource instance addresses in the prior state based on change information recorded in the configuration, but some adjustments did not succeed due to existing objects already at the intended addresses:%s\n\nTerraform has planned to destroy these objects. If Terraform's proposed changes aren't appropriate, you must first resolve the conflicts using the \"terraform state\" subcommands and then create a new plan.", + itemsBuf.String(), + ), + ) +} diff --git a/internal/terraform/context_plan2_test.go b/internal/terraform/context_plan2_test.go index 1167f234c..962142416 100644 --- a/internal/terraform/context_plan2_test.go +++ b/internal/terraform/context_plan2_test.go @@ -791,6 +791,102 @@ func TestContext2Plan_movedResourceBasic(t *testing.T) { }) } +func TestContext2Plan_movedResourceCollision(t *testing.T) { + addrNoKey := mustResourceInstanceAddr("test_object.a") + addrZeroKey := mustResourceInstanceAddr("test_object.a[0]") + m := testModuleInline(t, map[string]string{ + "main.tf": ` + resource "test_object" "a" { + # No "count" set, so test_object.a[0] will want + # to implicitly move to test_object.a, but will get + # blocked by the existing object at that address. + } + `, + }) + + state := states.BuildState(func(s *states.SyncState) { + s.SetResourceInstanceCurrent(addrNoKey, &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`)) + s.SetResourceInstanceCurrent(addrZeroKey, &states.ResourceInstanceObjectSrc{ + AttrsJSON: []byte(`{}`), + Status: states.ObjectReady, + }, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`)) + }) + + p := simpleMockProvider() + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + plan, diags := ctx.Plan(m, state, &PlanOpts{ + Mode: plans.NormalMode, + }) + if diags.HasErrors() { + t.Fatalf("unexpected errors\n%s", diags.Err().Error()) + } + + // We should have a warning, though! We'll lightly abuse the "for RPC" + // feature of diagnostics to get some more-readily-comparable diagnostic + // values. + gotDiags := diags.ForRPC() + wantDiags := tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Warning, + "Unresolved resource instance address changes", + `Terraform tried to adjust resource instance addresses in the prior state based on change information recorded in the configuration, but some adjustments did not succeed due to existing objects already at the intended addresses: + - test_object.a[0] could not move to test_object.a + +Terraform has planned to destroy these objects. If Terraform's proposed changes aren't appropriate, you must first resolve the conflicts using the "terraform state" subcommands and then create a new plan.`, + ), + }.ForRPC() + if diff := cmp.Diff(wantDiags, gotDiags); diff != "" { + t.Errorf("wrong diagnostics\n%s", diff) + } + + t.Run(addrNoKey.String(), func(t *testing.T) { + instPlan := plan.Changes.ResourceInstance(addrNoKey) + if instPlan == nil { + t.Fatalf("no plan for %s at all", addrNoKey) + } + + if got, want := instPlan.Addr, addrNoKey; !got.Equal(want) { + t.Errorf("wrong current address\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.PrevRunAddr, addrNoKey; !got.Equal(want) { + t.Errorf("wrong previous run address\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.Action, plans.NoOp; got != want { + t.Errorf("wrong planned action\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want { + t.Errorf("wrong action reason\ngot: %s\nwant: %s", got, want) + } + }) + t.Run(addrZeroKey.String(), func(t *testing.T) { + instPlan := plan.Changes.ResourceInstance(addrZeroKey) + if instPlan == nil { + t.Fatalf("no plan for %s at all", addrZeroKey) + } + + if got, want := instPlan.Addr, addrZeroKey; !got.Equal(want) { + t.Errorf("wrong current address\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.PrevRunAddr, addrZeroKey; !got.Equal(want) { + t.Errorf("wrong previous run address\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.Action, plans.Delete; got != want { + t.Errorf("wrong planned action\ngot: %s\nwant: %s", got, want) + } + if got, want := instPlan.ActionReason, plans.ResourceInstanceDeleteBecauseWrongRepetition; got != want { + t.Errorf("wrong action reason\ngot: %s\nwant: %s", got, want) + } + }) +} + func TestContext2Plan_movedResourceUntargeted(t *testing.T) { addrA := mustResourceInstanceAddr("test_object.a") addrB := mustResourceInstanceAddr("test_object.b")