Skip to content

Commit

Permalink
prevent errors from NoOp deposed changes
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jbardin committed Sep 29, 2022
1 parent 8c98e1f commit ec56580
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 1 deletion.
66 changes: 66 additions & 0 deletions internal/terraform/context_plan2_test.go
Expand Up @@ -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)
}
5 changes: 4 additions & 1 deletion internal/terraform/transform_diff.go
Expand Up @@ -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:
Expand Down

0 comments on commit ec56580

Please sign in to comment.