fix bug in ignore_changes Transform

The cty.Transform for ignore_changes could return early when building a
map that had multiple ignored keys.

Refactor the function to try and separate the fast-path a little better,
and hopefully make it easier to follow.
This commit is contained in:
James Bardin 2020-12-03 16:03:49 -05:00
parent 2c2ed53ff3
commit aa5c8add2e
2 changed files with 93 additions and 36 deletions

View File

@ -664,47 +664,58 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl
} }
ret, _ := cty.Transform(config, func(path cty.Path, v cty.Value) (cty.Value, error) { ret, _ := cty.Transform(config, func(path cty.Path, v cty.Value) (cty.Value, error) {
// Easy path for when we are only matching the entire value. The only
// values we break up for inspection are maps.
if !v.Type().IsMapType() {
for _, ignored := range ignoredValues { for _, ignored := range ignoredValues {
if !path.Equals(ignored.path) { if path.Equals(ignored.path) {
return v, nil
}
// no index, so we can return the entire value
if ignored.key.IsNull() {
return ignored.value, nil return ignored.value, nil
} }
}
return v, nil
}
// We now know this must be a map, so we need to accumulate the values
// key-by-key.
// we have an index key, so make sure we have a map if !v.IsNull() && !v.IsKnown() {
if !v.Type().IsMapType() { // since v is not known, we cannot ignore individual keys
// we'll let other validation catch any type mismatch
return v, nil return v, nil
} }
// Now we know we are ignoring a specific index of this map, so get // The configMap is the current configuration value, which we will
// the config map and modify, add, or remove the desired key. // mutate based on the ignored paths and the prior map value.
var configMap map[string]cty.Value var configMap map[string]cty.Value
var priorMap map[string]cty.Value switch {
case v.IsNull() || v.LengthInt() == 0:
if !v.IsNull() { configMap = map[string]cty.Value{}
if !v.IsKnown() { default:
// if the entire map is not known, we can't ignore any
// specific keys yet.
continue
}
configMap = v.AsValueMap() configMap = v.AsValueMap()
} }
if configMap == nil {
configMap = map[string]cty.Value{} for _, ignored := range ignoredValues {
if !path.Equals(ignored.path) {
continue
} }
// We also need to create a prior map, so we can check for if ignored.key.IsNull() {
// existence while getting the value. Value.Index will always // The map address is confirmed to match at this point,
// return null. // so if there is no key, we want the entire map and can
if !ignored.value.IsNull() { // stop accumulating values.
priorMap = ignored.value.AsValueMap() return ignored.value, nil
} }
if priorMap == 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.
// We also need to create a prior map, so we can check for
// existence while getting the value, because Value.Index will
// return null for a key with a null value and for a non-existent
// key.
var priorMap map[string]cty.Value
switch {
case ignored.value.IsNull() || ignored.value.LengthInt() == 0:
priorMap = map[string]cty.Value{} priorMap = map[string]cty.Value{}
default:
priorMap = ignored.value.AsValueMap()
} }
key := ignored.key.AsString() key := ignored.key.AsString()
@ -718,14 +729,13 @@ func processIgnoreChangesIndividual(prior, config cty.Value, ignoreChanges []hcl
default: default:
configMap[key] = priorElem configMap[key] = priorElem
} }
}
if len(configMap) == 0 { if len(configMap) == 0 {
return cty.MapValEmpty(v.Type().ElementType()), nil return cty.MapValEmpty(v.Type().ElementType()), nil
} }
return cty.MapVal(configMap), nil return cty.MapVal(configMap), nil
}
return v, nil
}) })
return ret, nil return ret, nil
} }

View File

@ -136,6 +136,53 @@ func TestProcessIgnoreChangesIndividual(t *testing.T) {
"b": cty.StringVal("b value"), "b": cty.StringVal("b value"),
}), }),
}, },
"map_index_multiple_keys": {
cty.ObjectVal(map[string]cty.Value{
"a": cty.MapVal(map[string]cty.Value{
"a0": cty.StringVal("a0 value"),
"a1": cty.StringVal("a1 value"),
"a2": cty.StringVal("a2 value"),
"a3": cty.StringVal("a3 value"),
}),
"b": cty.StringVal("b value"),
}),
cty.ObjectVal(map[string]cty.Value{
"a": cty.NullVal(cty.Map(cty.String)),
"b": cty.StringVal("new b value"),
}),
[]string{`a["a1"]`, `a["a2"]`, `a["a3"]`, `b`},
cty.ObjectVal(map[string]cty.Value{
"a": cty.MapVal(map[string]cty.Value{
"a1": cty.StringVal("a1 value"),
"a2": cty.StringVal("a2 value"),
"a3": cty.StringVal("a3 value"),
}),
"b": cty.StringVal("b value"),
}),
},
"map_index_redundant": {
cty.ObjectVal(map[string]cty.Value{
"a": cty.MapVal(map[string]cty.Value{
"a0": cty.StringVal("a0 value"),
"a1": cty.StringVal("a1 value"),
"a2": cty.StringVal("a2 value"),
}),
"b": cty.StringVal("b value"),
}),
cty.ObjectVal(map[string]cty.Value{
"a": cty.NullVal(cty.Map(cty.String)),
"b": cty.StringVal("new b value"),
}),
[]string{`a["a1"]`, `a`, `b`},
cty.ObjectVal(map[string]cty.Value{
"a": cty.MapVal(map[string]cty.Value{
"a0": cty.StringVal("a0 value"),
"a1": cty.StringVal("a1 value"),
"a2": cty.StringVal("a2 value"),
}),
"b": cty.StringVal("b value"),
}),
},
"missing_map_index": { "missing_map_index": {
cty.ObjectVal(map[string]cty.Value{ cty.ObjectVal(map[string]cty.Value{
"a": cty.MapVal(map[string]cty.Value{ "a": cty.MapVal(map[string]cty.Value{