-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(terraform): evaluateStep
to correctly set EvalContext
for multiple instances of blocks
#8555
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
Changes from all commits
be674f7
b43cac8
6921256
04605e2
0103bd8
ce933c9
6fe1164
332c4ad
8f8e652
7271d20
271dd05
fbaa5b9
a16013e
91ded49
31084ec
bac0ac4
f96d147
a6d4c97
d2fff87
3ac5bfe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package parser | ||
|
||
import "github.com/zclconf/go-cty/cty" | ||
|
||
// insertTupleElement inserts a value into a tuple at the specified index. | ||
// If the idx is outside the bounds of the list, it grows the tuple to | ||
// the new size, and fills in `cty.NilVal` for the missing elements. | ||
// | ||
// This function will not panic. If the list value is not a list, it will | ||
// be replaced with an empty list. | ||
func insertTupleElement(list cty.Value, idx int, val cty.Value) cty.Value { | ||
if list.IsNull() || !list.Type().IsTupleType() { | ||
// better than a panic | ||
list = cty.EmptyTupleVal | ||
} | ||
|
||
if idx < 0 { | ||
// Nothing to do? | ||
return list | ||
} | ||
|
||
// Create a new list of the correct length, copying in the old list | ||
// values for matching indices. | ||
newList := make([]cty.Value, max(idx+1, list.LengthInt())) | ||
for it := list.ElementIterator(); it.Next(); { | ||
key, elem := it.Element() | ||
elemIdx, _ := key.AsBigFloat().Int64() | ||
newList[elemIdx] = elem | ||
} | ||
// Insert the new value. | ||
newList[idx] = val | ||
|
||
return cty.TupleVal(newList) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
package parser | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
"github.com/zclconf/go-cty/cty" | ||
) | ||
|
||
func Test_insertTupleElement(t *testing.T) { | ||
t.Parallel() | ||
|
||
tests := []struct { | ||
name string | ||
start cty.Value | ||
index int | ||
value cty.Value | ||
want cty.Value | ||
}{ | ||
{ | ||
name: "empty", | ||
start: cty.Value{}, | ||
index: 0, | ||
value: cty.NilVal, | ||
want: cty.TupleVal([]cty.Value{cty.NilVal}), | ||
}, | ||
{ | ||
name: "empty to length", | ||
start: cty.Value{}, | ||
index: 2, | ||
value: cty.NilVal, | ||
want: cty.TupleVal([]cty.Value{cty.NilVal, cty.NilVal, cty.NilVal}), | ||
}, | ||
{ | ||
name: "insert to empty", | ||
start: cty.EmptyTupleVal, | ||
index: 1, | ||
value: cty.NumberIntVal(5), | ||
want: cty.TupleVal([]cty.Value{cty.NilVal, cty.NumberIntVal(5)}), | ||
}, | ||
{ | ||
name: "insert to existing", | ||
start: cty.TupleVal([]cty.Value{cty.NumberIntVal(1), cty.NumberIntVal(2), cty.NumberIntVal(3)}), | ||
index: 1, | ||
value: cty.NumberIntVal(5), | ||
want: cty.TupleVal([]cty.Value{cty.NumberIntVal(1), cty.NumberIntVal(5), cty.NumberIntVal(3)}), | ||
}, | ||
{ | ||
name: "insert to existing, extends", | ||
start: cty.TupleVal([]cty.Value{cty.NumberIntVal(1), cty.NumberIntVal(2), cty.NumberIntVal(3)}), | ||
index: 4, | ||
value: cty.NumberIntVal(5), | ||
want: cty.TupleVal([]cty.Value{ | ||
cty.NumberIntVal(1), cty.NumberIntVal(2), | ||
cty.NumberIntVal(3), cty.NilVal, | ||
cty.NumberIntVal(5), | ||
}), | ||
}, | ||
{ | ||
name: "mixed list", | ||
start: cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.NumberIntVal(2), cty.NumberIntVal(3)}), | ||
index: 1, | ||
value: cty.BoolVal(true), | ||
want: cty.TupleVal([]cty.Value{ | ||
cty.StringVal("a"), cty.BoolVal(true), cty.NumberIntVal(3), | ||
}), | ||
}, | ||
{ | ||
name: "replace end", | ||
start: cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.NumberIntVal(2), cty.NumberIntVal(3)}), | ||
index: 2, | ||
value: cty.StringVal("end"), | ||
want: cty.TupleVal([]cty.Value{ | ||
cty.StringVal("a"), cty.NumberIntVal(2), cty.StringVal("end"), | ||
}), | ||
}, | ||
|
||
// Some bad arguments | ||
{ | ||
name: "negative index", | ||
start: cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.NumberIntVal(2), cty.NumberIntVal(3)}), | ||
index: -1, | ||
value: cty.BoolVal(true), | ||
want: cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.NumberIntVal(2), cty.NumberIntVal(3)}), | ||
}, | ||
{ | ||
name: "non-list", | ||
start: cty.BoolVal(true), | ||
index: 1, | ||
value: cty.BoolVal(true), | ||
want: cty.TupleVal([]cty.Value{cty.NilVal, cty.BoolVal(true)}), | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
tt := tt | ||
t.Run(tt.name, func(t *testing.T) { | ||
t.Parallel() | ||
|
||
require.Equal(t, tt.want, insertTupleElement(tt.start, tt.index, tt.value)) | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |
"context" | ||
"errors" | ||
"io/fs" | ||
"maps" | ||
"reflect" | ||
"slices" | ||
|
||
|
@@ -524,7 +525,6 @@ func (e *evaluator) getValuesByBlockType(blockType string) cty.Value { | |
values := make(map[string]cty.Value) | ||
|
||
for _, b := range blocksOfType { | ||
|
||
switch b.Type() { | ||
case "variable": // variables are special in that their value comes from the "default" attribute | ||
val, err := e.evaluateVariable(b) | ||
|
@@ -539,9 +539,7 @@ func (e *evaluator) getValuesByBlockType(blockType string) cty.Value { | |
} | ||
values[b.Label()] = val | ||
case "locals", "moved", "import": | ||
for key, val := range b.Values().AsValueMap() { | ||
values[key] = val | ||
} | ||
maps.Copy(values, b.Values().AsValueMap()) | ||
case "provider", "module", "check": | ||
if b.Label() == "" { | ||
continue | ||
|
@@ -552,19 +550,27 @@ func (e *evaluator) getValuesByBlockType(blockType string) cty.Value { | |
continue | ||
} | ||
|
||
blockMap, ok := values[b.Labels()[0]] | ||
// Data blocks should all be loaded into the top level 'values' | ||
// object. The hierarchy of the map is: | ||
// values = map[<type>]map[<name>] = | ||
// Block -> Block's attributes as a cty.Object | ||
// Tuple(Block) -> Instances of the block | ||
// Object(Block) -> Field values are instances of the block | ||
ref := b.Reference() | ||
typeValues, ok := values[ref.TypeLabel()] | ||
if !ok { | ||
values[b.Labels()[0]] = cty.ObjectVal(make(map[string]cty.Value)) | ||
blockMap = values[b.Labels()[0]] | ||
typeValues = cty.ObjectVal(make(map[string]cty.Value)) | ||
values[ref.TypeLabel()] = typeValues | ||
} | ||
|
||
valueMap := blockMap.AsValueMap() | ||
valueMap := typeValues.AsValueMap() | ||
if valueMap == nil { | ||
valueMap = make(map[string]cty.Value) | ||
} | ||
valueMap[ref.NameLabel()] = blockInstanceValues(b, valueMap) | ||
|
||
valueMap[b.Labels()[1]] = b.Values() | ||
values[b.Labels()[0]] = cty.ObjectVal(valueMap) | ||
// Update the map of all blocks with the same type. | ||
values[ref.TypeLabel()] = cty.ObjectVal(valueMap) | ||
} | ||
} | ||
|
||
|
@@ -575,23 +581,57 @@ func (e *evaluator) getResources() map[string]cty.Value { | |
values := make(map[string]map[string]cty.Value) | ||
|
||
for _, b := range e.blocks { | ||
if b.Type() != "resource" { | ||
continue | ||
} | ||
|
||
if len(b.Labels()) < 2 { | ||
if b.Type() != "resource" || len(b.Labels()) < 2 { | ||
continue | ||
} | ||
|
||
val, exists := values[b.Labels()[0]] | ||
ref := b.Reference() | ||
typeValues, exists := values[ref.TypeLabel()] | ||
if !exists { | ||
val = make(map[string]cty.Value) | ||
values[b.Labels()[0]] = val | ||
typeValues = make(map[string]cty.Value) | ||
values[ref.TypeLabel()] = typeValues | ||
} | ||
val[b.Labels()[1]] = b.Values() | ||
typeValues[ref.NameLabel()] = blockInstanceValues(b, typeValues) | ||
} | ||
|
||
return lo.MapValues(values, func(v map[string]cty.Value, _ string) cty.Value { | ||
return cty.ObjectVal(v) | ||
}) | ||
} | ||
|
||
// blockInstanceValues returns a cty.Value containing the values of the block instances. | ||
// If the count argument is used, a tuple is returned where the index corresponds to the argument index. | ||
// If the for_each argument is used, an object is returned where the key corresponds to the argument key. | ||
// In other cases, the values of the block itself are returned. | ||
func blockInstanceValues(b *terraform.Block, typeValues map[string]cty.Value) cty.Value { | ||
ref := b.Reference() | ||
key := ref.RawKey() | ||
|
||
switch { | ||
case key.Type().Equals(cty.Number) && b.GetAttribute("count") != nil: | ||
idx, _ := key.AsBigFloat().Int64() | ||
return insertTupleElement(typeValues[ref.NameLabel()], int(idx), b.Values()) | ||
Comment on lines
+612
to
+613
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Element may be larger than the tuple capacity when the context does not yet contain all instances of the block, and then we need to grow it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We check that it is a number. Also the key cannot be unknown, it is either Nil or a known value. |
||
case isForEachKey(key) && b.GetAttribute("for_each") != nil: | ||
keyStr := ref.Key() | ||
|
||
instancesVal, exists := typeValues[ref.NameLabel()] | ||
if !exists || !instancesVal.CanIterateElements() { | ||
instancesVal = cty.EmptyObjectVal | ||
} | ||
|
||
instances := instancesVal.AsValueMap() | ||
if instances == nil { | ||
instances = make(map[string]cty.Value) | ||
} | ||
|
||
instances[keyStr] = b.Values() | ||
return cty.ObjectVal(instances) | ||
|
||
default: | ||
return b.Values() | ||
} | ||
} | ||
Emyrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
func isForEachKey(key cty.Value) bool { | ||
return key.Type().Equals(cty.Number) || key.Type().Equals(cty.String) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capturing loop variable is no longer required https://go.dev/blog/loopvar-preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Emyrk can we remove this then?