Skip to content

Commit

Permalink
function/stdlib: ZipmapFunc precise mark handling
Browse files Browse the repository at this point in the history 
ZipmapFunc can now handle marks on an element-by-element basis, to the
extent that the cty type system is able to do so.

Since map keys can't be individually sensitive, any sensitive values in
the keys list will aggregate on the map as a whole, but the individual
value elements will have their marks preserved individually.

We previously didn't have unit tests for ZipmapFunc so this also includes
some additional tests not related to marks, in order to cover the other
interesting cases this function must handle.
  • Loading branch information
@apparentlymart
apparentlymart committed Apr 19, 2021
1 parent d879859 commit 35a1a3a
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 8 deletions.
38 changes: 30 additions & 8 deletions cty/function/stdlib/collection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1145,12 +1145,14 @@ var ValuesFunc = function.New(&function.Spec{
var ZipmapFunc = function.New(&function.Spec{
Params: []function.Parameter{
{
Name: "keys",
Type: cty.List(cty.String),
Name: "keys",
Type: cty.List(cty.String),
AllowMarked: true,
},
{
Name: "values",
Type: cty.DynamicPseudoType,
Name: "values",
Type: cty.DynamicPseudoType,
AllowMarked: true,
},
},
Type: func(args []cty.Value) (ret cty.Type, err error) {
Expand All @@ -1168,13 +1170,21 @@ var ZipmapFunc = function.New(&function.Spec{
return cty.DynamicPseudoType, nil
}

// NOTE: Marking of the keys list can't be represented in the
// result type, so the tuple type here will disclose the keys.
// This is unfortunate but is a common compromise with dynamic
// return types; the result from Impl will still reflect the marks
// from the keys list, so a mark-using caller should look out for
// that if it's important for their use-case.
keys, _ := keys.Unmark()
keysRaw := keys.AsValueSlice()
valueTypesRaw := valuesTy.TupleElementTypes()
if len(keysRaw) != len(valueTypesRaw) {
return cty.NilType, fmt.Errorf("number of keys (%d) does not match number of values (%d)", len(keysRaw), len(valueTypesRaw))
}
atys := make(map[string]cty.Type, len(valueTypesRaw))
for i, keyVal := range keysRaw {
keyVal, _ = keyVal.Unmark()
if keyVal.IsNull() {
return cty.NilType, fmt.Errorf("keys list has null value at index %d", i)
}
Expand All @@ -1190,11 +1200,17 @@ var ZipmapFunc = function.New(&function.Spec{
Impl: func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) {
keys := args[0]
values := args[1]
keys, keysMarks := keys.Unmark()
values, valuesMarks := values.Unmark()

// All of our return paths must pass through the merged marks from
// both the keys and the values, if any, using .WithMarks(retMarks)
retMarks := cty.NewValueMarks(keysMarks, valuesMarks)

if !keys.IsWhollyKnown() {
// Unknown map keys and object attributes are not supported, so
// our entire result must be unknown in this case.
return cty.UnknownVal(retType), nil
return cty.UnknownVal(retType).WithMarks(retMarks), nil
}

// both keys and values are guaranteed to be shallowly-known here,
Expand All @@ -1208,19 +1224,25 @@ var ZipmapFunc = function.New(&function.Spec{
i := 0
for it := keys.ElementIterator(); it.Next(); {
_, v := it.Element()
v, vMarks := v.Unmark()
val := values.Index(cty.NumberIntVal(int64(i)))
output[v.AsString()] = val

// We also need to accumulate the individual key marks on the
// returned map, because keys can't carry marks on their own.
retMarks = cty.NewValueMarks(retMarks, vMarks)

i++
}

switch {
case retType.IsMapType():
if len(output) == 0 {
return cty.MapValEmpty(retType.ElementType()), nil
return cty.MapValEmpty(retType.ElementType()).WithMarks(retMarks), nil
}
return cty.MapVal(output), nil
return cty.MapVal(output).WithMarks(retMarks), nil
case retType.IsObjectType():
return cty.ObjectVal(output), nil
return cty.ObjectVal(output).WithMarks(retMarks), nil
default:
// Should never happen because the type-check function should've
// caught any other case.
Expand Down
229 changes: 229 additions & 0 deletions cty/function/stdlib/collection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1098,3 +1098,232 @@ func TestValues(t *testing.T) {
})
}
}

func TestZipMap(t *testing.T) {
tests := []struct {
Keys cty.Value
Values cty.Value
Want cty.Value
Err string
}{
// Lists of values (map result)
{
cty.ListValEmpty(cty.String),
cty.ListValEmpty(cty.String),
cty.MapValEmpty(cty.String),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
cty.ListVal([]cty.Value{cty.StringVal("bloop")}),
cty.MapVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep"), cty.StringVal("beep")}),
cty.ListVal([]cty.Value{cty.StringVal("bloop"), cty.StringVal("boop")}),
cty.MapVal(map[string]cty.Value{
"beep": cty.StringVal("boop"),
"bleep": cty.StringVal("bloop"),
}),
``,
},
{
cty.UnknownVal(cty.List(cty.String)),
cty.UnknownVal(cty.List(cty.String)),
cty.UnknownVal(cty.Map(cty.String)),
``,
},
{
cty.UnknownVal(cty.List(cty.String)),
cty.ListValEmpty(cty.String),
cty.UnknownVal(cty.Map(cty.String)),
``,
},
{
cty.ListValEmpty(cty.String),
cty.UnknownVal(cty.List(cty.String)),
cty.UnknownVal(cty.Map(cty.String)),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
cty.ListVal([]cty.Value{cty.StringVal("bloop")}),
cty.MapVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("a"),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
cty.ListVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
cty.MapVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("b"),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
cty.ListVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
cty.MapVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("a").Mark("b"),
``,
},
{
// cty map keys don't have individual marks, so marks on elements
// in the keys list aggregate with the resulting map as a whole.
cty.ListVal([]cty.Value{cty.StringVal("bleep").Mark("a")}),
cty.ListVal([]cty.Value{cty.StringVal("bloop")}),
cty.MapVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("a"),
``,
},
{
// cty map _values_ can have individual marks, so individual
// elements in the values list should have their marks preserved.
cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
cty.ListVal([]cty.Value{cty.StringVal("bloop").Mark("a")}),
cty.MapVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop").Mark("a"),
}),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("boop")}),
cty.ListValEmpty(cty.String),
cty.NilVal,
`number of keys (1) does not match number of values (0)`,
},
{
cty.ListValEmpty(cty.String),
cty.ListVal([]cty.Value{cty.StringVal("boop")}),
cty.NilVal,
`number of keys (0) does not match number of values (1)`,
},

// Tuple of values (object result)
{
cty.ListValEmpty(cty.String),
cty.EmptyTupleVal,
cty.EmptyObjectVal,
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
cty.TupleVal([]cty.Value{cty.StringVal("bloop")}),
cty.ObjectVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep"), cty.StringVal("beep")}),
cty.TupleVal([]cty.Value{cty.StringVal("bloop"), cty.StringVal("boop")}),
cty.ObjectVal(map[string]cty.Value{
"beep": cty.StringVal("boop"),
"bleep": cty.StringVal("bloop"),
}),
``,
},
{
cty.UnknownVal(cty.List(cty.String)),
cty.UnknownVal(cty.EmptyTuple),
cty.DynamicVal,
``,
},
{
cty.UnknownVal(cty.List(cty.String)),
cty.EmptyTupleVal,
cty.DynamicVal,
``,
},
{
cty.ListValEmpty(cty.String),
cty.UnknownVal(cty.EmptyTuple),
cty.UnknownVal(cty.EmptyObject),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
cty.TupleVal([]cty.Value{cty.StringVal("bloop")}),
cty.ObjectVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("a"),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
cty.TupleVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
cty.ObjectVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("b"),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
cty.TupleVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
cty.ObjectVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("a").Mark("b"),
``,
},
{
// cty object attributes don't have individual marks, so marks on
// elements in the keys list aggregate with the resulting object as
// a whole.
cty.ListVal([]cty.Value{cty.StringVal("bleep").Mark("a")}),
cty.TupleVal([]cty.Value{cty.StringVal("bloop")}),
cty.ObjectVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop"),
}).Mark("a"),
``,
},
{
// cty attribute _values_ can have individual marks, so individual
// elements in the values list should have their marks preserved.
cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
cty.TupleVal([]cty.Value{cty.StringVal("bloop").Mark("a")}),
cty.ObjectVal(map[string]cty.Value{
"bleep": cty.StringVal("bloop").Mark("a"),
}),
``,
},
{
cty.ListVal([]cty.Value{cty.StringVal("boop")}),
cty.EmptyTupleVal,
cty.NilVal,
`number of keys (1) does not match number of values (0)`,
},
{
cty.ListValEmpty(cty.String),
cty.TupleVal([]cty.Value{cty.StringVal("boop")}),
cty.NilVal,
`number of keys (0) does not match number of values (1)`,
},
}

for _, test := range tests {
t.Run(fmt.Sprintf("ZipMap(%#v, %#v)", test.Keys, test.Values), func(t *testing.T) {
got, err := Zipmap(test.Keys, test.Values)
if test.Err != "" {
if err == nil {
t.Fatal("succeeded; want error")
}
if got, want := err.Error(), test.Err; got != want {
t.Fatalf("wrong error\ngot: %s\nwant: %s", got, want)
}
return
} else if err != nil {
t.Fatalf("unexpected error: %s", err)
}

if !got.RawEquals(test.Want) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want)
}
})
}
}
18 changes: 18 additions & 0 deletions cty/marks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,32 @@ type marker struct {
type ValueMarks map[interface{}]struct{}

// NewValueMarks constructs a new ValueMarks set with the given mark values.
//
// If any of the arguments are already ValueMarks values then they'll be merged
// into the result, rather than used directly as individual marks.
func NewValueMarks(marks ...interface{}) ValueMarks {
if len(marks) == 0 {
return nil
}
ret := make(ValueMarks, len(marks))
for _, v := range marks {
if vm, ok := v.(ValueMarks); ok {
// Constructing a new ValueMarks with an existing ValueMarks
// implements a merge operation. (This can cause our result to
// have a larger size than we expected, but that's okay.)
for v := range vm {
ret[v] = struct{}{}
}
continue
}
ret[v] = struct{}{}
}
if len(ret) == 0 {
// If we were merging ValueMarks values together and they were all
// empty then we'll avoid returning a zero-length map and return a
// nil instead, as is conventional.
return nil
}
return ret
}

Expand Down

0 comments on commit 35a1a3a

Please sign in to comment.