Skip to content

Commit

Permalink
core: Invalid for_each argument messaging improvements
Browse files Browse the repository at this point in the history
Our original messaging here was largely just lifted from the equivalent
message for unknown values in "count", and it didn't really include any
specific advice on how to update a configuration to make for_each valid,
instead focusing only on the workaround of using the -target planning
option.

It's tough to pack in a fully-actionable suggestion here since unknown
values in for_each keys tends to be a gnarly architectural problem rather
than a local quirk -- when data flows between modules it can sometimes be
unclear whether it'll end up being used in a context which allows unknown
values.

I did my best to summarize the advice we've been giving in community forum
though, in the hope that more people will be able to address this for
themselves without asking for help, until we're one day able to smooth
this out better with a mechanism such as "partial apply".
  • Loading branch information
apparentlymart committed Jan 10, 2022
1 parent 1e9075b commit 171e7ef
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
17 changes: 13 additions & 4 deletions internal/terraform/eval_for_each.go
Expand Up @@ -78,6 +78,9 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU
}
ty := forEachVal.Type()

const errInvalidUnknownDetailMap = "The \"for_each\" map includes keys derived from resource attributes that cannot be determined until apply, and so Terraform cannot determine the full set of keys that will identify the instances of this resource.\n\nWhen working with unknown values in for_each, it's better to define the map keys statically in your configuration and place apply-time results only in the map values.\n\nAlternatively, you could use the -target planning option to first apply only the resources that the for_each value depends on, and then apply a second time to fully converge."
const errInvalidUnknownDetailSet = "The \"for_each\" set includes values derived from resource attributes that cannot be determined until apply, and so Terraform cannot determine the full set of keys that will identify the instances of this resource.\n\nWhen working with unknown values in for_each, it's better to use a map value where the keys are defined statically in your configuration and where only the values contain apply-time results.\n\nAlternatively, you could use the -target planning option to first apply only the resources that the for_each value depends on, and then apply a second time to fully converge."

switch {
case forEachVal.IsNull():
diags = diags.Append(&hcl.Diagnostic{
Expand All @@ -91,10 +94,18 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU
return nullMap, diags
case !forEachVal.IsKnown():
if !allowUnknown {
var detailMsg string
switch {
case ty.IsSetType():
detailMsg = errInvalidUnknownDetailSet
default:
detailMsg = errInvalidUnknownDetailMap
}

diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: errInvalidForEachUnknownDetail,
Detail: detailMsg,
Subject: expr.Range().Ptr(),
Expression: expr,
EvalContext: hclCtx,
Expand Down Expand Up @@ -129,7 +140,7 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: errInvalidForEachUnknownDetail,
Detail: errInvalidUnknownDetailSet,
Subject: expr.Range().Ptr(),
Expression: expr,
EvalContext: hclCtx,
Expand Down Expand Up @@ -172,8 +183,6 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU
return forEachVal, nil
}

const errInvalidForEachUnknownDetail = `The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the for_each depends on.`

// markSafeLengthInt allows calling LengthInt on marked values safely
func markSafeLengthInt(val cty.Value) int {
v, _ := val.UnmarkDeep()
Expand Down
12 changes: 6 additions & 6 deletions internal/terraform/eval_for_each_test.go
Expand Up @@ -114,12 +114,12 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
"unknown string set": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
"Invalid for_each argument",
"depends on resource attributes that cannot be determined until apply",
"set includes values derived from resource attributes that cannot be determined until apply",
},
"unknown map": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))),
"Invalid for_each argument",
"depends on resource attributes that cannot be determined until apply",
"map includes keys derived from resource attributes that cannot be determined until apply",
},
"marked map": {
hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{
Expand All @@ -142,12 +142,12 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
"set containing unknown value": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)})),
"Invalid for_each argument",
"depends on resource attributes that cannot be determined until apply",
"set includes values derived from resource attributes that cannot be determined until apply",
},
"set containing dynamic unknown value": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.DynamicPseudoType)})),
"Invalid for_each argument",
"depends on resource attributes that cannot be determined until apply",
"set includes values derived from resource attributes that cannot be determined until apply",
},
"set containing marked values": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.StringVal("beep").Mark(marks.Sensitive), cty.StringVal("boop")})),
Expand All @@ -169,10 +169,10 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
t.Errorf("wrong diagnostic severity %#v; want %#v", got, want)
}
if got, want := diags[0].Description().Summary, test.Summary; got != want {
t.Errorf("wrong diagnostic summary %#v; want %#v", got, want)
t.Errorf("wrong diagnostic summary\ngot: %s\nwant: %s", got, want)
}
if got, want := diags[0].Description().Detail, test.DetailSubstring; !strings.Contains(got, want) {
t.Errorf("wrong diagnostic detail %#v; want %#v", got, want)
t.Errorf("wrong diagnostic detail\ngot: %s\nwant substring: %s", got, want)
}
if fromExpr := diags[0].FromExpr(); fromExpr != nil {
if fromExpr.Expression == nil {
Expand Down

0 comments on commit 171e7ef

Please sign in to comment.