diff --git a/ext/tryfunc/tryfunc.go b/ext/tryfunc/tryfunc.go index 47789858..fa601da4 100644 --- a/ext/tryfunc/tryfunc.go +++ b/ext/tryfunc/tryfunc.go @@ -72,18 +72,35 @@ func try(args []cty.Value) (cty.Value, error) { var diags hcl.Diagnostics for _, arg := range args { closure := customdecode.ExpressionClosureFromVal(arg) - if dependsOnUnknowns(closure.Expression, closure.EvalContext) { - // We can't safely decide if this expression will succeed yet, - // and so our entire result must be unknown until we have - // more information. - return cty.DynamicVal, nil - } v, moreDiags := closure.Value() diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { - continue // try the next one, if there is one to try + // If there's an error we know it will always fail and can + // continue. A more refined value will not remove an error from + // the expression. + continue } + + if !v.IsWhollyKnown() { + // If there are any unknowns in the value at all, we cannot be + // certain that the final value will be consistent or have the same + // type, so wee need to be conservative and return a dynamic value. + + // There are two different classes of failure that can happen when + // an expression transitions from unknown to known; an operation on + // a dynamic value becomes invalid for the type once the type is + // known, or an index expression on a collection fails once the + // collection value is known. These changes from a + // valid-partially-unknown expression to an invalid-known + // expression can produce inconsistent results by changing which + // "try" argument is returned, which may be a collection with + // different previously known values, or a different type entirely + // ("try" does not require consistent argument types) + return cty.DynamicVal, nil + } + return v, nil // ignore any accumulated diagnostics if one succeeds } @@ -111,43 +128,17 @@ func try(args []cty.Value) (cty.Value, error) { func can(arg cty.Value) (cty.Value, error) { closure := customdecode.ExpressionClosureFromVal(arg) - if dependsOnUnknowns(closure.Expression, closure.EvalContext) { - // Can't decide yet, then. - return cty.UnknownVal(cty.Bool), nil - } - - _, diags := closure.Value() + v, diags := closure.Value() if diags.HasErrors() { return cty.False, nil } - return cty.True, nil -} -// dependsOnUnknowns returns true if any of the variables that the given -// expression might access are unknown values or contain unknown values. -// -// This is a conservative result that prefers to return true if there's any -// chance that the expression might derive from an unknown value during its -// evaluation; it is likely to produce false-positives for more complex -// expressions involving deep data structures. -func dependsOnUnknowns(expr hcl.Expression, ctx *hcl.EvalContext) bool { - for _, traversal := range expr.Variables() { - val, diags := traversal.TraverseAbs(ctx) - if diags.HasErrors() { - // If the traversal returned a definitive error then it must - // not traverse through any unknowns. - continue - } - if !val.IsWhollyKnown() { - // The value will be unknown if either it refers directly to - // an unknown value or if the traversal moves through an unknown - // collection. We're using IsWhollyKnown, so this also catches - // situations where the traversal refers to a compound data - // structure that contains any unknown values. That's important, - // because during evaluation the expression might evaluate more - // deeply into this structure and encounter the unknowns. - return true - } + if !v.IsWhollyKnown() { + // If the value is not wholly known, we still cannot be certain that + // the expression was valid. There may be yet index expressions which + // will fail once values are completely known. + return cty.UnknownVal(cty.Bool), nil } - return false + + return cty.True, nil } diff --git a/ext/tryfunc/tryfunc_test.go b/ext/tryfunc/tryfunc_test.go index 1e75a163..7bd7af6d 100644 --- a/ext/tryfunc/tryfunc_test.go +++ b/ext/tryfunc/tryfunc_test.go @@ -107,6 +107,36 @@ func TestTryFunc(t *testing.T) { cty.StringVal("list").Mark("secret"), ``, }, + "nested known expression from unknown": { + // this expression contains an unknown, but will always return in + // "bar" + `try({u: false ? unknown : "bar"}, other)`, + map[string]cty.Value{ + "unknown": cty.UnknownVal(cty.String), + "other": cty.MapVal(map[string]cty.Value{ + "v": cty.StringVal("oops"), + }), + }, + cty.ObjectVal(map[string]cty.Value{ + "u": cty.StringVal("bar"), + }), + ``, + }, + "nested index op on unknown": { + // unknown and other have identical types, but we must return a + // dynamic value since v could change within the final result value + // after the first argument becomes known. + `try({u: unknown["foo"], v: "orig"}, other)`, + map[string]cty.Value{ + "unknown": cty.UnknownVal(cty.Map(cty.String)), + "other": cty.MapVal(map[string]cty.Value{ + "u": cty.StringVal("oops"), + "v": cty.StringVal("oops"), + }), + }, + cty.DynamicVal, + ``, + }, "three arguments, all fail": { `try(this, that, this_thing_in_particular)`, nil,