Better wording on for_each block with unsuitable type value (#1485)

Signed-off-by: Zejun Chen <tibazq@gmail.com>
Signed-off-by: chenzj <tibazq@gmail.com>
This commit is contained in:
chenzj 2024-05-13 18:57:10 +08:00 committed by GitHub
parent 06df723ceb
commit cffe80d300
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 134 additions and 18 deletions

View File

@ -14,6 +14,7 @@ BUG FIXES:
* Added a check in the `tofu test` to validate that the names of test run blocks do not contain spaces. ([#1489](https://github.com/opentofu/opentofu/pull/1489))
* `tofu test` now supports accessing module outputs when the module has no resources. ([#1409](https://github.com/opentofu/opentofu/pull/1409))
* Fixed support for provider functions in tests ([#1603](https://github.com/opentofu/opentofu/pull/1603))
* Added a better error message on `for_each` block with sensitive value of unsuitable type. ([#1485](https://github.com/opentofu/opentofu/pull/1485))
## Previous Releases

View File

@ -83,9 +83,6 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU
})
}
if diags.HasErrors() {
return nullMap, diags
}
ty := forEachVal.Type()
var isAllowedType bool
@ -98,6 +95,23 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU
allowedTypesMessage = "map, or set of strings"
}
// Check if the type is allowed whether the value is marked or not
if forEachVal.IsKnown() && !isAllowedType {
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a %s, and you have provided a value of type %s.`, allowedTypesMessage, ty.FriendlyName()),
Subject: expr.Range().Ptr(),
Expression: expr,
EvalContext: hclCtx,
})
}
// Return it to avoid expose sensitive value by further check
if diags.HasErrors() {
return nullMap, diags
}
const errInvalidUnknownDetailMap = "The \"for_each\" map includes keys derived from resource attributes that cannot be determined until apply, and so OpenTofu 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 OpenTofu 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."
@ -134,18 +148,6 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU
}
// ensure that we have a map, and not a DynamicValue
return cty.UnknownVal(cty.Map(cty.DynamicPseudoType)), diags
case !(isAllowedType):
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a %s, and you have provided a value of type %s.`, allowedTypesMessage, ty.FriendlyName()),
Subject: expr.Range().Ptr(),
Expression: expr,
EvalContext: hclCtx,
})
return nullMap, diags
case markSafeLengthInt(forEachVal) == 0:
// If the map is empty ({}), return an empty map, because cty will
// return nil when representing {} AsValueMap. This also covers an empty

View File

@ -133,6 +133,12 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
"map includes keys derived from resource attributes that cannot be determined until apply",
true, false,
},
"unknown pseudo-type": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.DynamicPseudoType)),
"Invalid for_each argument",
"map includes keys derived from resource attributes that cannot be determined until apply",
true, false,
},
"marked map": {
hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{
"a": cty.BoolVal(true),
@ -213,11 +219,118 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
}
}
func TestEvaluateForEachExpression_multi_errors(t *testing.T) {
tests := map[string]struct {
Expr hcl.Expression
Wanted []struct {
Summary string
DetailSubstring string
CausedBySensitive bool
}
}{
"marked list": {
hcltest.MockExprLiteral(cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("a")}).Mark(marks.Sensitive)),
[]struct {
Summary string
DetailSubstring string
CausedBySensitive bool
}{
{
"Invalid for_each argument",
"Sensitive values, or values derived from sensitive values, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.",
true,
},
{
"Invalid for_each argument",
"must be a map, or set of strings, and you have provided a value of type list",
false,
},
},
},
"marked tuple": {
hcltest.MockExprLiteral(cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("a")}).Mark(marks.Sensitive)),
[]struct {
Summary string
DetailSubstring string
CausedBySensitive bool
}{
{
"Invalid for_each argument",
"Sensitive values, or values derived from sensitive values, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.",
true,
},
{
"Invalid for_each argument",
"must be a map, or set of strings, and you have provided a value of type tuple",
false,
},
},
},
"marked string": {
hcltest.MockExprLiteral(cty.StringVal("a").Mark(marks.Sensitive)),
[]struct {
Summary string
DetailSubstring string
CausedBySensitive bool
}{
{
"Invalid for_each argument",
"Sensitive values, or values derived from sensitive values, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.",
true,
},
{
"Invalid for_each argument",
"must be a map, or set of strings, and you have provided a value of type string",
false,
},
},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
_, diags := evaluateForEachExpression(test.Expr, ctx)
if len(diags) != len(test.Wanted) {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
}
for idx := range test.Wanted {
if got, want := diags[idx].Severity(), tfdiags.Error; got != want {
t.Errorf("wrong diagnostic severity %#v; want %#v", got, want)
}
if got, want := diags[idx].Description().Summary, test.Wanted[idx].Summary; got != want {
t.Errorf("wrong diagnostic summary\ngot: %s\nwant: %s", got, want)
}
if got, want := diags[idx].Description().Detail, test.Wanted[idx].DetailSubstring; !strings.Contains(got, want) {
t.Errorf("wrong diagnostic detail\ngot: %s\nwant substring: %s", got, want)
}
if fromExpr := diags[idx].FromExpr(); fromExpr != nil {
if fromExpr.Expression == nil {
t.Errorf("diagnostic does not refer to an expression")
}
if fromExpr.EvalContext == nil {
t.Errorf("diagnostic does not refer to an EvalContext")
}
} else {
t.Errorf("diagnostic does not support FromExpr\ngot: %s", spew.Sdump(diags[idx]))
}
if got, want := tfdiags.DiagnosticCausedBySensitive(diags[idx]), test.Wanted[idx].CausedBySensitive; got != want {
t.Errorf("wrong result from tfdiags.DiagnosticCausedBySensitive\ngot: %#v\nwant: %#v", got, want)
}
}
})
}
}
func TestEvaluateForEachExpressionKnown(t *testing.T) {
tests := map[string]hcl.Expression{
"unknown string set": hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
"unknown map": hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))),
"unknown tuple": hcltest.MockExprLiteral(cty.UnknownVal(cty.Tuple([]cty.Type{cty.String, cty.Number, cty.Bool}))),
"unknown string set": hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
"unknown map": hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))),
"unknown tuple": hcltest.MockExprLiteral(cty.UnknownVal(cty.Tuple([]cty.Type{cty.String, cty.Number, cty.Bool}))),
"unknown pseudo-type": hcltest.MockExprLiteral(cty.UnknownVal(cty.DynamicPseudoType)),
}
for name, expr := range tests {