Skip to content
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.

Sign up for GitHub

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

Jump to bottom

core: Invalid for_each argument messaging improvements #30327

Merged
merged 1 commit into from Jan 10, 2022
Merged

core: Invalid for_each argument messaging improvements #30327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 13 additions & 4 deletions internal/terraform/eval_for_each.go
View file
Open in desktop
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
View file
Open in desktop
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