aherman
/
terraform
mirror of https://github.com/hashicorp/terraform
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

stackeval: Reusable and testable for_each handling

Browse Source 
Previously we had some similar code duplicated into each of the three
kinds of object that support for_each: components, embedded stack calls,
and provider configurations.

Instead, we'll hoist that code out into a function that all three can
call. We can then actually test that function in isolation (independently
of the three callers) to get more confidence that future maintenance won't
inadvertently break the slightly-unusual contract of treating a nil map
differently from a non-nil empty one, which is unfortunately likely to
defy the expectations of a typical Go developer since those two values
are normally considered equivalent.
pull/34738/head
Martin Atkins 3 years ago
parent 1180096850
commit 5aaed3111e
5 changed files with 448 additions and 208 deletions
Show Stats Download Patch File Download Diff File

67
internal/stacks/stackruntime/internal/stackeval/component.go
Unescape View File

@ -170,70 +170,9 @@ func (c *Component) CheckInstances(ctx context.Context, phase EvalPhase) (map[ad
var diags tfdiags.Diagnostics
forEachVal := c.ForEachValue(ctx, phase)
var ret map[addrs.InstanceKey]*ComponentInstance
switch {
case forEachVal == cty.NilVal:
// No for_each expression at all, then. We have exactly one instance
// without an instance key and with no repetition data.
ret = map[addrs.InstanceKey]*ComponentInstance{
addrs.NoKey: newComponentInstance(c, addrs.NoKey, instances.RepetitionData{
// no repetition symbols available in this case
}),
}
case !forEachVal.IsKnown():
// The for_each expression is too invalid for us to be able to
// know which instances exist. A totally nil map (as opposed to a
// non-nil map of length zero) signals that situation.
ret = nil
default:
// Otherwise we should be able to assume the value is valid per the
// definition of [CheckForEachValue]. The following will panic if
// that other function doesn't satisfy its documented contract;
// if that happens, prefer to correct [CheckForEachValue] than to
// add additional complexity here.
// NOTE: We MUST return a non-nil map from every return path under
// this case, even if there are zero elements in it, because a nil map
// represents an _invalid_ for_each expression (handled above).
ty := forEachVal.Type()
switch {
case ty.IsObjectType() || ty.IsMapType():
elems := forEachVal.AsValueMap()
ret = make(map[addrs.InstanceKey]*ComponentInstance, len(elems))
for k, v := range elems {
ik := addrs.StringKey(k)
ret[ik] = newComponentInstance(c, ik, instances.RepetitionData{
EachKey: cty.StringVal(k),
EachValue: v,
})
}
case ty.IsSetType():
// ForEachValue should have already guaranteed us a set of strings,
// but we'll check again here just so we can panic more intellgibly
// if that function is buggy.
if ty.ElementType() != cty.String {
panic(fmt.Sprintf("ForEachValue returned invalid result %#v", forEachVal))
}
elems := forEachVal.AsValueSlice()
ret = make(map[addrs.InstanceKey]*ComponentInstance, len(elems))
for _, sv := range elems {
k := addrs.StringKey(sv.AsString())
ret[k] = newComponentInstance(c, k, instances.RepetitionData{
EachKey: sv,
EachValue: sv,
})
}
default:
panic(fmt.Sprintf("ForEachValue returned invalid result %#v", forEachVal))
}
}
ret := instancesMap(forEachVal, func(ik addrs.InstanceKey, rd instances.RepetitionData) *ComponentInstance {
return newComponentInstance(c, ik, rd)
})
addrs := make([]stackaddrs.AbsComponentInstance, 0, len(ret))
for _, ci := range ret {

133
internal/stacks/stackruntime/internal/stackeval/for_each.go
Unescape View File

@ -5,8 +5,11 @@ package stackeval
import (
"context"
"fmt"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
)
@ -50,17 +53,15 @@ func evaluateForEachExpr(ctx context.Context, expr hcl.Expression, phase EvalPha
return DerivedExprResult(result, cty.DynamicVal), diags
}
default:
if !ty.ElementType().Equals(cty.String) {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: invalidForEachSummary,
Detail: invalidForEachDetail,
Subject: result.Expression.Range().Ptr(),
Expression: result.Expression,
EvalContext: result.EvalContext,
})
return DerivedExprResult(result, cty.DynamicVal), diags
}
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: invalidForEachSummary,
Detail: invalidForEachDetail,
Subject: result.Expression.Range().Ptr(),
Expression: result.Expression,
EvalContext: result.EvalContext,
})
return DerivedExprResult(result, cty.DynamicVal), diags
}
if result.Value.IsNull() {
diags = diags.Append(&hcl.Diagnostic{
@ -74,9 +75,113 @@ func evaluateForEachExpr(ctx context.Context, expr hcl.Expression, phase EvalPha
return DerivedExprResult(result, cty.DynamicVal), diags
}
// Unknown and sensitive values are also typically disallowed, but
// known-ness and sensitivity get decided dynamically based on data flow
// and so we'll treat those as plan-time errors only.
// Sensitive values are also typically disallowed, but sensitivity gets
// decided dynamically based on data flow and so we'll treat those as
// plan-time errors, to be handled by the caller.
return result, diags
}
// instancesMap constructs a map of instances of some expandable object,
// based on its for_each value or on the absence of such a value.
//
// If maybeForEachVal is [cty.NilVal] then the result is always a
// single-element map with an `addrs.NoKey` instance.
//
// If maybeForEachVal is non-nil then it must be a non-error result from
// an earlier call to [evaluateForEachExpr] which analyzed the given for_each
// expression. If the value is unknown then the result will be nil. Otherwise,
// the result is guaranteed to be a non-nil map with the same number of elements
// as the given for_each collection/structure.
//
// If maybeForEach value is non-nil but not a valid value produced by
// [evaluateForEachExpr] then the behavior is unpredictable, including the
// possibility of a panic.
func instancesMap[T any](maybeForEachVal cty.Value, makeInst func(addrs.InstanceKey, instances.RepetitionData) T) map[addrs.InstanceKey]T {
switch {
case maybeForEachVal == cty.NilVal:
// No for_each expression at all, then. We have exactly one instance
// without an instance key and with no repetition data.
return noForEachInstancesMap(makeInst)
case !maybeForEachVal.IsKnown():
// The for_each expression is too invalid for us to be able to
// know which instances exist. A totally nil map (as opposed to a
// non-nil map of length zero) signals that situation.
return nil
default:
// Otherwise we should be able to assume the value is valid per the
// definition of [evaluateForEachExpr]. The following will panic if
// that other function doesn't satisfy its documented contract;
// if that happens, prefer to correct the either that function or
// its caller rather than adding further complexity here.
// NOTE: We MUST return a non-nil map from every return path under
// this case, even if there are zero elements in it, because a nil map
// represents an _invalid_ for_each expression (handled above).
// forEachInstancesMap guarantees to never return a nil map.
return forEachInstancesMap(maybeForEachVal, makeInst)
}
}
// forEachInstanceKeys takes a value previously returned by
// [evaluateForEachExpr] and produces a map where each element maps from an
// instance key to a corresponding object decided by the givenc callback
// function.
//
// The result is guaranteed to be a non-nil map, even if the given value
// produces zero instances, because some callers use a nil map to represent
// the situation where the for_each value is too invalid to construct any
// map at all.
//
// This function is only designed to deal with valid (non-error) results from
// [evaluateForEachExpr] and so might panic if given other values.
func forEachInstancesMap[T any](forEachVal cty.Value, makeInst func(addrs.InstanceKey, instances.RepetitionData) T) map[addrs.InstanceKey]T {
ty := forEachVal.Type()
switch {
case ty.IsObjectType() || ty.IsMapType():
elems := forEachVal.AsValueMap()
ret := make(map[addrs.InstanceKey]T, len(elems))
for k, v := range elems {
ik := addrs.StringKey(k)
ret[ik] = makeInst(ik, instances.RepetitionData{
EachKey: cty.StringVal(k),
EachValue: v,
})
}
return ret
case ty.IsSetType():
// evaluateForEachExpr should have already guaranteed us a set of
// strings, but we'll check again here just so we can panic more
// intellgibly if that function is buggy.
if ty.ElementType() != cty.String {
panic(fmt.Sprintf("invalid forEachVal %#v", forEachVal))
}
elems := forEachVal.AsValueSlice()
ret := make(map[addrs.InstanceKey]T, len(elems))
for _, sv := range elems {
k := addrs.StringKey(sv.AsString())
ret[k] = makeInst(k, instances.RepetitionData{
EachKey: sv,
EachValue: sv,
})
}
return ret
default:
panic(fmt.Sprintf("invalid forEachVal %#v", forEachVal))
}
}
func noForEachInstancesMap[T any](makeInst func(addrs.InstanceKey, instances.RepetitionData) T) map[addrs.InstanceKey]T {
return map[addrs.InstanceKey]T{
addrs.NoKey: makeInst(addrs.NoKey, instances.RepetitionData{
// no repetition symbols available in this case
}),
}
}

320
internal/stacks/stackruntime/internal/stackeval/for_each_test.go
Unescape View File

@ -0,0 +1,320 @@
package stackeval
import (
"context"
"fmt"
"testing"
"github.com/davecgh/go-spew/spew"
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hcltest"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/instances"
"github.com/hashicorp/terraform/internal/lang/marks"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty-debug/ctydebug"
"github.com/zclconf/go-cty/cty"
)
func TestEvaluateForEachExpr(t *testing.T) {
tests := map[string]struct {
Expr hcl.Expression
Want cty.Value
WantErr string
}{
// Objects
"empty object": {
Expr: hcltest.MockExprLiteral(cty.EmptyObjectVal),
Want: cty.EmptyObjectVal,
},
"non-empty object": {
Expr: hcltest.MockExprLiteral(cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("beep"),
"b": cty.StringVal("beep"),
})),
Want: cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("beep"),
"b": cty.StringVal("beep"),
}),
},
"unknown object": {
Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.EmptyObject)),
Want: cty.UnknownVal(cty.EmptyObject),
},
// Maps
"map of string": {
Expr: hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("beep"),
"b": cty.StringVal("boop"),
})),
Want: cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("beep"),
"b": cty.StringVal("boop"),
}),
},
"empty map of string": {
Expr: hcltest.MockExprLiteral(cty.MapValEmpty(cty.String)),
Want: cty.MapValEmpty(cty.String),
},
"unknown map of string": {
Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.String))),
Want: cty.UnknownVal(cty.Map(cty.String)),
},
"sensitive map of string": {
Expr: hcltest.MockExprLiteral(cty.MapValEmpty(cty.String).Mark(marks.Sensitive)),
Want: cty.MapValEmpty(cty.String).Mark(marks.Sensitive),
},
"map of object": {
Expr: hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{
"a": cty.EmptyObjectVal,
"b": cty.EmptyObjectVal,
})),
Want: cty.MapVal(map[string]cty.Value{
"a": cty.EmptyObjectVal,
"b": cty.EmptyObjectVal,
}),
},
"empty map of object": {
Expr: hcltest.MockExprLiteral(cty.MapValEmpty(cty.EmptyObject)),
Want: cty.MapValEmpty(cty.EmptyObject),
},
// Sets
"set of string": {
Expr: hcltest.MockExprLiteral(cty.SetVal([]cty.Value{
cty.StringVal("beep"),
cty.StringVal("boop"),
})),
Want: cty.SetVal([]cty.Value{
cty.StringVal("beep"),
cty.StringVal("boop"),
}),
},
"empty set of string": {
Expr: hcltest.MockExprLiteral(cty.SetValEmpty(cty.String)),
Want: cty.SetValEmpty(cty.String),
},
"unknown set of string": {
Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
Want: cty.UnknownVal(cty.Set(cty.String)),
},
"sensitive set of string": {
Expr: hcltest.MockExprLiteral(cty.SetValEmpty(cty.String).Mark(marks.Sensitive)),
Want: cty.SetValEmpty(cty.String).Mark(marks.Sensitive),
},
"empty set of object": {
Expr: hcltest.MockExprLiteral(cty.SetValEmpty(cty.EmptyObject)),
WantErr: `Invalid for_each value`,
},
// Nulls of any type are not allowed
"null object": {
Expr: hcltest.MockExprLiteral(cty.NullVal(cty.EmptyObject)),
WantErr: `Invalid for_each value`,
},
"null map": {
Expr: hcltest.MockExprLiteral(cty.NullVal(cty.Map(cty.String))),
WantErr: `Invalid for_each value`,
},
"null set": {
Expr: hcltest.MockExprLiteral(cty.NullVal(cty.Set(cty.String))),
WantErr: `Invalid for_each value`,
},
"null string": {
Expr: hcltest.MockExprLiteral(cty.NullVal(cty.String)),
WantErr: `Invalid for_each value`,
},
}
ctx := context.Background()
scope := newStaticExpressionScope()
for name, test := range tests {
t.Run(name, func(t *testing.T) {
gotResult, diags := evaluateForEachExpr(ctx, test.Expr, PlanPhase, scope)
got := gotResult.Value
if test.WantErr != "" {
if !diags.HasErrors() {
t.Fatalf("unexpected success; want error\ngot: %#v", got)
}
foundErr := false
for _, diag := range diags {
if diag.Severity() != tfdiags.Error {
continue
}
if diag.Description().Summary == test.WantErr {
foundErr = true
break
}
}
if !foundErr {
t.Errorf("missing expected error\nwant summary: %s\ngot: %s", test.WantErr, spew.Sdump(diags.ForRPC()))
}
return
}
if diags.HasErrors() {
t.Errorf("unexpected errors\n%s", spew.Sdump(diags.ForRPC()))
}
if !test.Want.RawEquals(got) {
t.Errorf("wrong result\ngot: %#v\nwant: %#v", got, test.Want)
}
})
}
}
func TestInstancesMap(t *testing.T) {
type InstanceObj struct {
Key addrs.InstanceKey
Rep instances.RepetitionData
}
makeObj := func(k addrs.InstanceKey, r instances.RepetitionData) InstanceObj {
return InstanceObj{
Key: k,
Rep: r,
}
}
tests := []struct {
Input cty.Value
Want map[addrs.InstanceKey]InstanceObj
// This function always either succeeds or panics, because it
// expects to be given already-validated input from another function.
// We're only testing the success cases here.
}{
// No for_each at all
{
cty.NilVal,
map[addrs.InstanceKey]InstanceObj{
addrs.NoKey: {
Key: addrs.NoKey,
Rep: instances.RepetitionData{
// No data available for the non-repeating case
},
},
},
},
// Unknowns
{
cty.UnknownVal(cty.EmptyObject),
nil, // a nil map means "unknown" for this function
},
{
cty.UnknownVal(cty.Map(cty.Bool)),
nil, // a nil map means "unknown" for this function
},
{
cty.UnknownVal(cty.Set(cty.String)),
nil, // a nil map means "unknown" for this function
},
// Empties
{
cty.EmptyObjectVal,
map[addrs.InstanceKey]InstanceObj{
// intentionally a non-nil empty map to assert that we know
// that there are zero instances, rather than that we don't
// know how many there are.
},
},
{
cty.MapValEmpty(cty.String),
map[addrs.InstanceKey]InstanceObj{
// intentionally a non-nil empty map to assert that we know
// that there are zero instances, rather than that we don't
// know how many there are.
},
},
{
cty.SetValEmpty(cty.String),
map[addrs.InstanceKey]InstanceObj{
// intentionally a non-nil empty map to assert that we know
// that there are zero instances, rather than that we don't
// know how many there are.
},
},
// Known and not empty
{
cty.ObjectVal(map[string]cty.Value{
"a": cty.StringVal("beep"),
"b": cty.StringVal("boop"),
}),
map[addrs.InstanceKey]InstanceObj{
addrs.StringKey("a"): {
Key: addrs.StringKey("a"),
Rep: instances.RepetitionData{
EachKey: cty.StringVal("a"),
EachValue: cty.StringVal("beep"),
},
},
addrs.StringKey("b"): {
Key: addrs.StringKey("b"),
Rep: instances.RepetitionData{
EachKey: cty.StringVal("b"),
EachValue: cty.StringVal("boop"),
},
},
},
},
{
cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("beep"),
"b": cty.StringVal("boop"),
}),
map[addrs.InstanceKey]InstanceObj{
addrs.StringKey("a"): {
Key: addrs.StringKey("a"),
Rep: instances.RepetitionData{
EachKey: cty.StringVal("a"),
EachValue: cty.StringVal("beep"),
},
},
addrs.StringKey("b"): {
Key: addrs.StringKey("b"),
Rep: instances.RepetitionData{
EachKey: cty.StringVal("b"),
EachValue: cty.StringVal("boop"),
},
},
},
},
{
cty.SetVal([]cty.Value{
cty.StringVal("beep"),
cty.StringVal("boop"),
}),
map[addrs.InstanceKey]InstanceObj{
addrs.StringKey("beep"): {
Key: addrs.StringKey("beep"),
Rep: instances.RepetitionData{
EachKey: cty.StringVal("beep"),
EachValue: cty.StringVal("beep"),
},
},
addrs.StringKey("boop"): {
Key: addrs.StringKey("boop"),
Rep: instances.RepetitionData{
EachKey: cty.StringVal("boop"),
EachValue: cty.StringVal("boop"),
},
},
},
},
}
for _, test := range tests {
t.Run(fmt.Sprintf("%s", test.Input), func(t *testing.T) {
got := instancesMap(test.Input, makeObj)
if diff := cmp.Diff(test.Want, got, ctydebug.CmpOptions); diff != "" {
t.Errorf("wrong result\ninput: %#v\n%s", test.Input, diff)
}
})
}
}

69
internal/stacks/stackruntime/internal/stackeval/provider.go
Unescape View File

@ -176,73 +176,10 @@ func (p *Provider) CheckInstances(ctx context.Context, phase EvalPhase) (map[add
ctx, p.instances.For(phase), p.main,
func(ctx context.Context) (map[addrs.InstanceKey]*ProviderInstance, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
forEachVal := p.ForEachValue(ctx, phase)
switch {
case forEachVal == cty.NilVal:
// No for_each expression at all, then. We have exactly one instance
// without an instance key and with no repetition data.
return map[addrs.InstanceKey]*ProviderInstance{
addrs.NoKey: newProviderInstance(p, addrs.NoKey, instances.RepetitionData{
// no repetition symbols available in this case
}),
}, diags
case !forEachVal.IsKnown():
// The for_each expression is too invalid for us to be able to
// know which instances exist. A totally nil map (as opposed to a
// non-nil map of length zero) signals that situation.
return nil, diags
default:
// Otherwise we should be able to assume the value is valid per the
// definition of [CheckForEachValue]. The following will panic if
// that other function doesn't satisfy its documented contract;
// if that happens, prefer to correct [CheckForEachValue] than to
// add additional complexity here.
// NOTE: We MUST return a non-nil map from every return path under
// this case, even if there are zero elements in it, because a nil map
// represents an _invalid_ for_each expression (handled above).
ty := forEachVal.Type()
switch {
case ty.IsObjectType() || ty.IsMapType():
elems := forEachVal.AsValueMap()
ret := make(map[addrs.InstanceKey]*ProviderInstance, len(elems))
for k, v := range elems {
ik := addrs.StringKey(k)
ret[ik] = newProviderInstance(p, ik, instances.RepetitionData{
EachKey: cty.StringVal(k),
EachValue: v,
})
}
return ret, diags
case ty.IsSetType():
// ForEachValue should have already guaranteed us a set of strings,
// but we'll check again here just so we can panic more intellgibly
// if that function is buggy.
if ty.ElementType() != cty.String {
panic(fmt.Sprintf("ForEachValue returned invalid result %#v", forEachVal))
}
elems := forEachVal.AsValueSlice()
ret := make(map[addrs.InstanceKey]*ProviderInstance, len(elems))
for _, sv := range elems {
k := addrs.StringKey(sv.AsString())
ret[k] = newProviderInstance(p, k, instances.RepetitionData{
EachKey: sv,
EachValue: sv,
})
}
return ret, diags
default:
panic(fmt.Sprintf("ForEachValue returned invalid result %#v", forEachVal))
}
}
return instancesMap(forEachVal, func(ik addrs.InstanceKey, rd instances.RepetitionData) *ProviderInstance {
return newProviderInstance(p, ik, rd)
}), diags
},
)
}

67
internal/stacks/stackruntime/internal/stackeval/stack_call.go
Unescape View File

@ -163,70 +163,9 @@ func (c *StackCall) CheckInstances(ctx context.Context, phase EvalPhase) (map[ad
var diags tfdiags.Diagnostics
forEachVal := c.ForEachValue(ctx, phase)
switch {
case forEachVal == cty.NilVal:
// No for_each expression at all, then. We have exactly one instance
// without an instance key and with no repetition data.
return map[addrs.InstanceKey]*StackCallInstance{
addrs.NoKey: newStackCallInstance(c, addrs.NoKey, instances.RepetitionData{
// no repetition symbols available in this case
}),
}, diags
case !forEachVal.IsKnown():
// The for_each expression is too invalid for us to be able to
// know which instances exist. A totally nil map (as opposed to a
// non-nil map of length zero) signals that situation.
return nil, diags
default:
// Otherwise we should be able to assume the value is valid per the
// definition of [CheckForEachValue]. The following will panic if
// that other function doesn't satisfy its documented contract;
// if that happens, prefer to correct [CheckForEachValue] than to
// add additional complexity here.
// NOTE: We MUST return a non-nil map from every return path under
// this case, even if there are zero elements in it, because a nil map
// represents an _invalid_ for_each expression (handled above).
ty := forEachVal.Type()
switch {
case ty.IsObjectType() || ty.IsMapType():
elems := forEachVal.AsValueMap()
ret := make(map[addrs.InstanceKey]*StackCallInstance, len(elems))
for k, v := range elems {
ik := addrs.StringKey(k)
ret[ik] = newStackCallInstance(c, ik, instances.RepetitionData{
EachKey: cty.StringVal(k),
EachValue: v,
})
}
return ret, diags
case ty.IsSetType():
// ForEachValue should have already guaranteed us a set of strings,
// but we'll check again here just so we can panic more intellgibly
// if that function is buggy.
if ty.ElementType() != cty.String {
panic(fmt.Sprintf("ForEachValue returned invalid result %#v", forEachVal))
}
elems := forEachVal.AsValueSlice()
ret := make(map[addrs.InstanceKey]*StackCallInstance, len(elems))
for _, sv := range elems {
k := addrs.StringKey(sv.AsString())
ret[k] = newStackCallInstance(c, k, instances.RepetitionData{
EachKey: sv,
EachValue: sv,
})
}
return ret, diags
default:
panic(fmt.Sprintf("ForEachValue returned invalid result %#v", forEachVal))
}
}
return instancesMap(forEachVal, func(ik addrs.InstanceKey, rd instances.RepetitionData) *StackCallInstance {
return newStackCallInstance(c, ik, rd)
}), diags
},
)
}

Write Preview
Loading…
Cancel
Save