apply ignore_changes directly to config

In order to ensure all the starting values agree, and since
ignore_changes is only meant to apply to the configuration, we need to
process the ignore_changes values on the config itself rather than the
proposed value.

This ensures the proposed new value and the config value seen by
providers are coordinated, and still allows us to use the rules laid out
by objchange.AssertPlanValid to compare the result to the configuration.

terraform/context_plan_test.go

@@ -4626,18 +4626,14 @@ func TestContext2Plan_ignoreChanges(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if res.Action != plans.Update {
-		t.Fatalf("resource %s should be updated, got %s", ric.Addr, res.Action)
-	}
 
 	if ric.Addr.String() != "aws_instance.foo" {
 		t.Fatalf("unexpected resource: %s", ric.Addr)
 	}
 
 	checkVals(t, objectVal(t, schema, map[string]cty.Value{
-		"id":   cty.StringVal("bar"),
-		"ami":  cty.StringVal("ami-abcd1234"),
-		"type": cty.StringVal("aws_instance"),
+		"id":  cty.StringVal("bar"),
+		"ami": cty.StringVal("ami-abcd1234"),
 	}), ric.After)
 }
 

terraform/eval_diff.go

@@ -159,17 +159,6 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, diags.Err()
 	}
 
-	// Create an unmarked version of our config val, defaulting
-	// to the configVal so we don't do the work of unmarking unless
-	// necessary
-	unmarkedConfigVal := configVal
-	var unmarkedPaths []cty.PathValueMarks
-	if configVal.ContainsMarked() {
-		// store the marked values so we can re-mark them later after
-		// we've sent things over the wire.
-		unmarkedConfigVal, unmarkedPaths = configVal.UnmarkDeepWithPaths()
-	}
-
 	metaConfigVal := cty.NullVal(cty.DynamicPseudoType)
 	if n.ProviderMetas != nil {
 		if m, ok := n.ProviderMetas[n.ProviderAddr.Provider]; ok && m != nil {
@@ -213,6 +202,29 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		priorVal = cty.NullVal(schema.ImpliedType())
 	}
 
+	// ignore_changes is meant to only apply to the configuration, so it must
+	// be applied before we generate a plan. This ensures the config used for
+	// the proposed value, the proposed value itself, and the config presented
+	// to the provider in the PlanResourceChange request all agree on the
+	// starting values.
+	var ignoreChangeDiags tfdiags.Diagnostics
+	configVal, ignoreChangeDiags = n.processIgnoreChanges(priorVal, configVal)
+	diags = diags.Append(ignoreChangeDiags)
+	if ignoreChangeDiags.HasErrors() {
+		return nil, diags.Err()
+	}
+
+	// Create an unmarked version of our config val, defaulting
+	// to the configVal so we don't do the work of unmarking unless
+	// necessary
+	unmarkedConfigVal := configVal
+	var unmarkedPaths []cty.PathValueMarks
+	if configVal.ContainsMarked() {
+		// store the marked values so we can re-mark them later after
+		// we've sent things over the wire.
+		unmarkedConfigVal, unmarkedPaths = configVal.UnmarkDeepWithPaths()
+	}
+
 	unmarkedPriorVal := priorVal
 	if priorVal.ContainsMarked() {
 		// store the marked values so we can re-mark them later after
@@ -246,17 +258,6 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, validateResp.Diagnostics.InConfigBody(config.Config).Err()
 	}
 
-	// The provider gets an opportunity to customize the proposed 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(unmarkedPriorVal, proposedNewVal)
-	diags = diags.Append(ignoreChangeDiags)
-	if ignoreChangeDiags.HasErrors() {
-		return nil, diags.Err()
-	}
-
 	resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{
 		TypeName:         n.Addr.Resource.Type,
 		Config:           unmarkedConfigVal,
@@ -553,32 +554,32 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 	return nil, nil
 }
 
-func (n *EvalDiff) processIgnoreChanges(prior, proposed cty.Value) (cty.Value, tfdiags.Diagnostics) {
+func (n *EvalDiff) processIgnoreChanges(prior, config cty.Value) (cty.Value, tfdiags.Diagnostics) {
 	// ignore_changes only applies when an object already exists, since we
 	// can't ignore changes to a thing we've not created yet.
 	if prior.IsNull() {
-		return proposed, nil
+		return config, nil
 	}
 
 	ignoreChanges := n.Config.Managed.IgnoreChanges
 	ignoreAll := n.Config.Managed.IgnoreAllChanges
 
 	if len(ignoreChanges) == 0 && !ignoreAll {
-		return proposed, nil
+		return config, nil
 	}
 	if ignoreAll {
 		return prior, nil
 	}
-	if prior.IsNull() || proposed.IsNull() {
+	if prior.IsNull() || config.IsNull() {
 		// Ignore changes doesn't apply when we're creating for the first time.
 		// Proposed should never be null here, but if it is then we'll just let it be.
-		return proposed, nil
+		return config, nil
 	}
 
-	return processIgnoreChangesIndividual(prior, proposed, ignoreChanges)
+	return processIgnoreChangesIndividual(prior, config, ignoreChanges)
 }
 
-func processIgnoreChangesIndividual(prior, proposed cty.Value, ignoreChanges []hcl.Traversal) (cty.Value, tfdiags.Diagnostics) {
+func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl.Traversal) (cty.Value, tfdiags.Diagnostics) {
 	// When we walk below we will be using cty.Path values for comparison, so
 	// we'll convert our traversals here so we can compare more easily.
 	ignoreChangesPath := make([]cty.Path, len(ignoreChanges))
@@ -606,7 +607,7 @@ func processIgnoreChangesIndividual(prior, proposed cty.Value, ignoreChanges []h
 	}
 
 	var diags tfdiags.Diagnostics
-	ret, _ := cty.Transform(proposed, func(path cty.Path, v cty.Value) (cty.Value, error) {
+	ret, _ := cty.Transform(config, func(path cty.Path, v cty.Value) (cty.Value, error) {
 		// First we must see if this is a path that's being ignored at all.
 		// We're looking for an exact match here because this walk will visit
 		// leaf values first and then their containers, and we want to do