From 2e5a8c0f6e3617c09cccbea48e164f287c704cde Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Wed, 4 Sep 2019 16:51:22 -0400 Subject: [PATCH] Update when ignore_changes are evaluated, to impact customizediff --- builtin/providers/test/resource_test.go | 49 +++++++++++++++++++++++++ terraform/eval_diff.go | 20 +++++----- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/builtin/providers/test/resource_test.go b/builtin/providers/test/resource_test.go index 5aedba214..510b7816c 100644 --- a/builtin/providers/test/resource_test.go +++ b/builtin/providers/test/resource_test.go @@ -432,6 +432,55 @@ resource "test_resource" "foo" { }) } +func TestResource_ignoreChangesCustomizeDiff(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource" "foo" { + required = "yep" + required_map = { + key = "value" + } + optional = "a" + lifecycle { + ignore_changes = [optional] + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource.foo", "planned_computed", "a", + ), + ), + }, + // On this step, `optional` changes, but `planned_computed` + // should remain as "a" because we have set `ignore_changes` + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource" "foo" { + required = "yep" + required_map = { + key = "value" + } + optional = "b" + lifecycle { + ignore_changes = [optional] + } +} + `), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "test_resource.foo", "planned_computed", "a", + ), + ), + }, + }, + }) +} + // Reproduces plan-time panic when the wrong type is interpolated in a list of // maps. // TODO: this should return a type error, rather than silently setting an empty diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 695b5fe0e..6853f895c 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -189,7 +189,16 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } // The provider gets an opportunity to customize the proposed new value, - // which in turn produces the _planned_ new value. + // which in turn produces the _planned_ new value. But before + // we send back this information, we need to process ignore_changes + // so that CustomizeDiff will not act on them + var ignoreChangeDiags tfdiags.Diagnostics + proposedNewVal, ignoreChangeDiags = n.processIgnoreChanges(priorVal, proposedNewVal) + diags = diags.Append(ignoreChangeDiags) + if ignoreChangeDiags.HasErrors() { + return nil, diags.Err() + } + resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{ TypeName: n.Addr.Resource.Type, Config: configVal, @@ -258,15 +267,6 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } } - { - var moreDiags tfdiags.Diagnostics - plannedNewVal, moreDiags = n.processIgnoreChanges(priorVal, plannedNewVal) - diags = diags.Append(moreDiags) - if moreDiags.HasErrors() { - return nil, diags.Err() - } - } - // The provider produces a list of paths to attributes whose changes mean // that we must replace rather than update an existing remote object. // However, we only need to do that if the identified attributes _have_