From 03e3d6d0c6cdbf118c50c6ec496eea65467ab3ea Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Tue, 18 Oct 2022 13:33:11 +0200 Subject: [PATCH] Allow maps to convert into objects when missing optional attributes are not compatible --- cty/convert/conversion_collection.go | 50 ++++++++++++++++++++++--- cty/convert/public_test.go | 55 +++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/cty/convert/conversion_collection.go b/cty/convert/conversion_collection.go index c21f4183..d1288580 100644 --- a/cty/convert/conversion_collection.go +++ b/cty/convert/conversion_collection.go @@ -448,13 +448,26 @@ func conversionMapToObject(mapType cty.Type, objType cty.Type, unsafe bool) conv elemConvs[name] = getConversion(mapEty, objectAty, unsafe) if elemConvs[name] == nil { - // If any of our element conversions are impossible, then the our - // whole conversion is impossible. + // This means that this conversion is impossible. Typically, we + // would give up at this point and declare the whole conversion + // impossible. But, if this attribute is optional then maybe we will + // be able to do this conversion anyway provided the actual concrete + // map doesn't have this value set. + + if objType.AttributeOptional(name) { + // This attribute is optional, so let's leave this conversion in + // as a nil, and we can error later if we actually have to + // convert this. + continue + } + + // Otherwise, give up. This conversion is impossible as we have a + // required attribute that doesn't match the map's inner type. return nil } } - // If we fall out here then a conversion is possible, using the + // If we fall out here then a conversion may be possible, using the // element conversions in elemConvs return func(val cty.Value, path cty.Path) (cty.Value, error) { elems := make(map[string]cty.Value, len(elemConvs)) @@ -474,12 +487,39 @@ func conversionMapToObject(mapType cty.Type, objType cty.Type, unsafe bool) conv Key: name, } - conv := elemConvs[name.AsString()] - if conv != nil { + // There are 3 cases here: + // 1. This attribute is not in elemConvs + // 2. This attribute is in elemConvs and is not nil + // 3. This attribute is in elemConvs and is nil. + + // In case 1, we do not enter any of the branches below. This case + // means the attribute type is the same between the map and the + // object, and we don't need to do any conversion. + + if conv, ok := elemConvs[name.AsString()]; conv != nil { + // This is case 2. The attribute type is different between the + // map and the object, and we know how to convert between them. + // So, we reset val to be the converted value and carry on. val, err = conv(val, elemPath) if err != nil { return cty.NilVal, err } + } else if ok { + // This is case 3 and it is an error. The attribute types are + // different between the map and the object, but we cannot + // convert between them. + // + // Now typically, this would be picked earlier on when we were + // building elemConvs. However, in the case of optional + // attributes there was a chance we could still convert the + // overall object even if this particular attribute was not + // convertable. This is because it could have not been set in + // the map, and we could skip over it here and set a null value. + // + // Since we reached this branch, we know that map did actually + // contain a non-convertable optional attribute. This means we + // error. + return cty.NilVal, path.NewErrorf("map element type is incompatible with attribute %q: %s", name.AsString(), MismatchMessage(val.Type(), objType.AttributeType(name.AsString()))) } elems[name.AsString()] = val diff --git a/cty/convert/public_test.go b/cty/convert/public_test.go index 3a3ee511..33158471 100644 --- a/cty/convert/public_test.go +++ b/cty/convert/public_test.go @@ -996,6 +996,10 @@ func TestConvert(t *testing.T) { { Value: cty.MapVal(map[string]cty.Value{ "a": cty.StringVal("boop"), + // It's okay to use a map of string to convert to this + // target type as long as the source map does not include + // any of the optional attributes that cannot be assigned + // from a string. }), Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{ "a": cty.String, @@ -1011,7 +1015,56 @@ func TestConvert(t *testing.T) { "d": cty.String, })), }), - WantError: false, + }, + { + Value: cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("boop"), + }), + Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{ + "a": cty.String, + "b": cty.String, + "c": cty.Object(map[string]cty.Type{ + "d": cty.DynamicPseudoType, + }), + }, []string{"b", "c"}), + Want: cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("boop"), + "b": cty.NullVal(cty.String), + "c": cty.NullVal(cty.Object(map[string]cty.Type{ + "d": cty.DynamicPseudoType, + })), + }), + }, + { + Value: cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("boop"), + }), + Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{ + "a": cty.String, + "b": cty.String, + "c": cty.DynamicPseudoType, + }, []string{"b", "c"}), + Want: cty.ObjectVal(map[string]cty.Value{ + "a": cty.StringVal("boop"), + "b": cty.NullVal(cty.String), + "c": cty.NullVal(cty.DynamicPseudoType), + }), + }, + { + Value: cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("boop"), + // This case is invalid, because an element of a map of + // string cannot be assigned to an object-typed attribute. + "c": cty.StringVal("foobar"), + }), + Type: cty.ObjectWithOptionalAttrs(map[string]cty.Type{ + "a": cty.String, + "b": cty.String, + "c": cty.Object(map[string]cty.Type{ + "d": cty.String, + }), + }, []string{"b", "c"}), + WantError: `map element type is incompatible with attribute "c": object required`, }, { Value: cty.ListVal([]cty.Value{