From ec565808e032d5410dbf9982d91bac5259fdc27b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 29 Sep 2022 10:44:30 -0400 Subject: [PATCH] prevent errors from NoOp deposed changes If a previously deposed object is deleted outside of Terraform, the next plan will result in a NoOp change for the deposed object. If that NoOp change was flagged as an update to run any associated condition checks, it would fail the following test to ensure a deposed change is only ever participating in a delete operation. Since there are no conditional checks to run on an object which does not exist, we can exclude this type of NoOp object. --- internal/terraform/context_plan2_test.go | 66 ++++++++++++++++++++++++ internal/terraform/transform_diff.go | 5 +- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/internal/terraform/context_plan2_test.go b/internal/terraform/context_plan2_test.go index d96da31cb13d..3c9ddaf2cf4d 100644 --- a/internal/terraform/context_plan2_test.go +++ b/internal/terraform/context_plan2_test.go @@ -3682,3 +3682,69 @@ output "out" { assertNoErrors(t, diags) } + +// A deposed instances which no longer exists during ReadResource creates NoOp +// change, which should not effect the plan. +func TestContext2Plan_deposedNoLongerExists(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +resource "test_object" "b" { + count = 1 + test_string = "updated" + lifecycle { + create_before_destroy = true + } +} +`, + }) + + p := simpleMockProvider() + p.ReadResourceFn = func(req providers.ReadResourceRequest) (resp providers.ReadResourceResponse) { + s := req.PriorState.GetAttr("test_string").AsString() + if s == "current" { + resp.NewState = req.PriorState + return resp + } + // pretend the non-current instance has been deleted already + resp.NewState = cty.NullVal(req.PriorState.Type()) + return resp + } + + // Here we introduce a cycle via state which only shows up in the apply + // graph where the actual destroy instances are connected in the graph. + // This could happen for example when a user has an existing state with + // stored dependencies, and changes the config in such a way that + // contradicts the stored dependencies. + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + root.SetResourceInstanceDeposed( + mustResourceInstanceAddr("test_object.a[0]").Resource, + states.DeposedKey("deposed"), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectTainted, + AttrsJSON: []byte(`{"test_string":"old"}`), + Dependencies: []addrs.ConfigResource{}, + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + root.SetResourceInstanceCurrent( + mustResourceInstanceAddr("test_object.a[0]").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectTainted, + AttrsJSON: []byte(`{"test_string":"current"}`), + Dependencies: []addrs.ConfigResource{}, + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`), + ) + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + _, diags := ctx.Plan(m, state, &PlanOpts{ + Mode: plans.NormalMode, + }) + assertNoErrors(t, diags) +} diff --git a/internal/terraform/transform_diff.go b/internal/terraform/transform_diff.go index 894db4c51901..f5d926b8591a 100644 --- a/internal/terraform/transform_diff.go +++ b/internal/terraform/transform_diff.go @@ -69,7 +69,10 @@ func (t *DiffTransformer) Transform(g *Graph) error { // run any condition checks associated with the object, to // make sure that they still hold when considering the // results of other changes. - update = true + // This is true except in the rare case where a deposed object + // no longer exists, which results in a NoOp change that does not + // count as an update. + update = dk == states.NotDeposed case plans.Delete: delete = true case plans.DeleteThenCreate, plans.CreateThenDelete: