Merge pull request #26421 from hashicorp/jbardin/ignore-changes-map

allow ignore_changes to reference any map key

terraform/eval_diff.go

@@ -608,38 +608,127 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl
 		ignoreChangesPath[i] = path
 	}
 
-	var diags tfdiags.Diagnostics
+	type ignoreChange struct {
+		// Path is the full path, minus any trailing map index
+		path cty.Path
+		// Value is the value we are to retain at the above path. If there is a
+		// key value, this must be a map and the desired value will be at the
+		// key index.
+		value cty.Value
+		// Key is the index key if the ignored path ends in a map index.
+		key cty.Value
+	}
+	var ignoredValues []ignoreChange
+
+	// Find the actual changes first and store them in the ignoreChange struct.
+	// If the change was to a map value, and the key doesn't exist in the
+	// config, it would never be visited in the transform walk.
+	for _, icPath := range ignoreChangesPath {
+		key := cty.NullVal(cty.String)
+		// check for a map index, since maps are the only structure where we
+		// could have invalid path steps.
+		last, ok := icPath[len(icPath)-1].(cty.IndexStep)
+		if ok {
+			if last.Key.Type() == cty.String {
+				icPath = icPath[:len(icPath)-1]
+				key = last.Key
+			}
+		}
+
+		// The structure should have been validated already, and we already
+		// trimmed the trailing map index. Any other intermediate index error
+		// means we wouldn't be able to apply the value below, so no need to
+		// record this.
+		p, err := icPath.Apply(prior)
+		if err != nil {
+			continue
+		}
+		c, err := icPath.Apply(config)
+		if err != nil {
+			continue
+		}
+
+		// If this is a map, it is checking the entire map value for equality
+		// rather than the individual key. This means that the change is stored
+		// here even if our ignored key doesn't change. That is OK since it
+		// won't cause any changes in the transformation, but allows us to skip
+		// breaking up the maps and checking for key existence here too.
+		eq := p.Equals(c)
+		if eq.IsKnown() && eq.False() {
+			// there a change to ignore at this path, store the prior value
+			ignoredValues = append(ignoredValues, ignoreChange{icPath, p, key})
+		}
+	}
+
+	if len(ignoredValues) == 0 {
+		return config, nil
+	}
+
 	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.
+		for _, ignored := range ignoredValues {
-		// We're looking for an exact match here because this walk will visit
+			if !path.Equals(ignored.path) {
-		// leaf values first and then their containers, and we want to do
-		// the "ignore" transform once we reach the point indicated, throwing
-		// away any deeper values we already produced at that point.
-		var ignoreTraversal hcl.Traversal
-		for i, candidate := range ignoreChangesPath {
-			if path.Equals(candidate) {
-				ignoreTraversal = ignoreChanges[i]
-			}
-		}
-		if ignoreTraversal == nil {
 				return v, nil
 			}
 
-		// If we're able to follow the same path through the prior value,
+			// no index, so we can return the entire value
-		// we'll take the value there instead, effectively undoing the
+			if ignored.key.IsNull() {
-		// change that was planned.
+				return ignored.value, nil
-		priorV, diags := hcl.ApplyPath(prior, path, nil)
+			}
-		if diags.HasErrors() {
+
-			// We just ignore the errors and move on here, since we assume it's
+			// we have an index key, so make sure we have a map
-			// just because the prior value was a slightly-different shape.
+			if !v.Type().IsMapType() {
-			// It could potentially also be that the traversal doesn't match
+				// we'll let other validation catch any type mismatch
-			// the schema, but we should've caught that during the validate
-			// walk if so.
 				return v, nil
 			}
-		return priorV, nil
+
+			// Now we know we are ignoring a specific index of this map, so get
+			// the config map and modify, add, or remove the desired key.
+			var configMap map[string]cty.Value
+			var priorMap map[string]cty.Value
+
+			if !v.IsNull() {
+				if !v.IsKnown() {
+					// if the entire map is not known, we can't ignore any
+					// specific keys yet.
+					continue
+				}
+				configMap = v.AsValueMap()
+			}
+			if configMap == nil {
+				configMap = map[string]cty.Value{}
+			}
+
+			// We also need to create a prior map, so we can check for
+			// existence while getting the value. Value.Index will always
+			// return null.
+			if !ignored.value.IsNull() {
+				priorMap = ignored.value.AsValueMap()
+			}
+			if priorMap == nil {
+				priorMap = map[string]cty.Value{}
+			}
+
+			key := ignored.key.AsString()
+			priorElem, keep := priorMap[key]
+
+			switch {
+			case !keep:
+				// this didn't exist in the old map value, so we're keeping the
+				// "absence" of the key by removing it from the config
+				delete(configMap, key)
+			default:
+				configMap[key] = priorElem
+			}
+
+			if len(configMap) == 0 {
+				return cty.MapValEmpty(v.Type().ElementType()), nil
+			}
+
+			return cty.MapVal(configMap), nil
+		}
+		return v, nil
 	})
-	return ret, diags
+	return ret, nil
 }
 
 // a group of key-*ResourceAttrDiff pairs from the same flatmapped container

terraform/eval_diff_test.go

@@ -68,6 +68,164 @@ func TestProcessIgnoreChangesIndividual(t *testing.T) {
 				"b": cty.StringVal("new b value"),
 			}),
 		},
+		"list_index": {
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.ListVal([]cty.Value{
+					cty.StringVal("a0 value"),
+					cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.ListVal([]cty.Value{
+					cty.StringVal("new a0 value"),
+					cty.StringVal("new a1 value"),
+				}),
+				"b": cty.StringVal("new b value"),
+			}),
+			[]string{"a[1]"},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.ListVal([]cty.Value{
+					cty.StringVal("new a0 value"),
+					cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("new b value"),
+			}),
+		},
+		"map_index": {
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("a0 value"),
+					"a1": cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("new a0 value"),
+					"a1": cty.StringVal("new a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+			[]string{`a["a1"]`},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("new a0 value"),
+					"a1": cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+		},
+		"map_index_no_config": {
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("a0 value"),
+					"a1": cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.NullVal(cty.Map(cty.String)),
+				"b": cty.StringVal("b value"),
+			}),
+			[]string{`a["a1"]`},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a1": cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+		},
+		"missing_map_index": {
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("a0 value"),
+					"a1": cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapValEmpty(cty.String),
+				"b": cty.StringVal("b value"),
+			}),
+			[]string{`a["a1"]`},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a1": cty.StringVal("a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+		},
+		"missing_map_index_empty": {
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapValEmpty(cty.String),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a": cty.StringVal("a0 value"),
+				}),
+			}),
+			[]string{`a["a"]`},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapValEmpty(cty.String),
+			}),
+		},
+		"missing_map_index_to_object": {
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a": cty.ObjectVal(map[string]cty.Value{
+						"a": cty.StringVal("aa0"),
+						"b": cty.StringVal("ab0"),
+					}),
+					"b": cty.ObjectVal(map[string]cty.Value{
+						"a": cty.StringVal("ba0"),
+						"b": cty.StringVal("bb0"),
+					}),
+				}),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapValEmpty(
+					cty.Object(map[string]cty.Type{
+						"a": cty.String,
+						"b": cty.String,
+					}),
+				),
+			}),
+			// we expect the config to be used here, as the ignore changes was
+			// `a["a"].b`, but the change was larger than that removing
+			// `a["a"]` entirely.
+			[]string{`a["a"].b`},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapValEmpty(
+					cty.Object(map[string]cty.Type{
+						"a": cty.String,
+						"b": cty.String,
+					}),
+				),
+			}),
+		},
+		"missing_prior_map_index": {
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("a0 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("a0 value"),
+					"a1": cty.StringVal("new a1 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+			[]string{`a["a1"]`},
+			cty.ObjectVal(map[string]cty.Value{
+				"a": cty.MapVal(map[string]cty.Value{
+					"a0": cty.StringVal("a0 value"),
+				}),
+				"b": cty.StringVal("b value"),
+			}),
+		},
 		"object attribute": {
 			cty.ObjectVal(map[string]cty.Value{
 				"a": cty.ObjectVal(map[string]cty.Value{

website/docs/configuration/resources.html.md

@@ -638,16 +638,22 @@ meta-arguments are supported:
   any difference in the current settings of a real infrastructure object
   and plans to update the remote object to match configuration.
 
-    In some rare cases, settings of a remote object are modified by processes
+    The `ignore_chages` feature is intended to be used when a resource is
-    outside of Terraform, which Terraform would then attempt to "fix" on the
+    created with references to data that may change in the future, but should
-    next run. In order to make Terraform share management responsibilities
+    not effect said resource after its creation. In some rare cases, settings
-    of a single object with a separate process, the `ignore_changes`
+    of a remote object are modified by processes outside of Terraform, which
-    meta-argument specifies resource attributes that Terraform should ignore
+    Terraform would then attempt to "fix" on the next run. In order to make
-    when planning updates to the associated remote object.
+    Terraform share management responsibilities of a single object with a
+    separate process, the `ignore_changes` meta-argument specifies resource
+    attributes that Terraform should ignore when planning updates to the
+    associated remote object.
 
     The arguments corresponding to the given attribute names are considered
     when planning a _create_ operation, but are ignored when planning an
-    _update_.
+    _update_. The arugments are the relative address of the attributes in the
+    resource. Map and list elements can be referenced using index notation,
+    like `tags["Name"]` and `list[0]` respectively.
+
 
     ```hcl
     resource "aws_instance" "example" {
@@ -663,35 +669,6 @@ meta-arguments are supported:
     }
     ```
 
-    You can also ignore specific map elements by writing references like
-    `tags["Name"]` in the `ignore_changes` list, though with an important
-    caveat: the ignoring applies only to in-place updates to an existing
-    key. Adding or removing a key is treated by Terraform as a change to the
-    containing map itself rather than to the individual key, and so if you
-    wish to ignore changes to a particular tag made by an external system
-    you must ensure that the Terraform configuration creates a placeholder
-    element for that tag name so that the external system changes will be
-    understood as an in-place edit of that key:
-
-    ```hcl
-    resource "aws_instance" "example" {
-      # ...
-
-      tags = {
-        # Initial value for Name is overridden by our automatic scheduled
-        # re-tagging process; changes to this are ignored by ignore_changes
-        # below.
-        Name = "placeholder"
-      }
-
-      lifecycle {
-        ignore_changes = [
-          tags["Name"],
-        ]
-      }
-    }
-    ```
-
     Instead of a list, the special keyword `all` may be used to instruct
     Terraform to ignore _all_ attributes, which means that Terraform can
     create and destroy the remote object but will never propose updates to it.