Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow maps to convert into objects when missing optional attributes are not compatible #139

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 47 additions & 5 deletions cty/convert/conversion_collection.go
Expand Up @@ -448,13 +448,28 @@ 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.
//
// We only do this in "unsafe" mode, because we cannot guarantee
// that the returned conversion will actually succeed once applied.
if objType.AttributeOptional(name) && unsafe {
// 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))
Expand All @@ -474,12 +489,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
Expand Down
73 changes: 73 additions & 0 deletions cty/convert/public_test.go
Expand Up @@ -993,6 +993,79 @@ func TestConvert(t *testing.T) {
cty.NullVal(cty.DynamicPseudoType),
}),
},
{
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,
"b": cty.String,
"c": cty.Object(map[string]cty.Type{
"d": cty.String,
}),
}, []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.String,
})),
}),
},
{
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{
cty.ObjectVal(map[string]cty.Value{
Expand Down