evalchecks: Suggest -exclude as a workaround for unknown count/for_each

Previously we made a very generic suggestion to use -target to split a
change into two parts as a workaround for the fact that count and for_each
must be known during planning. That works, but we didn't have enough
information available to tell the operator exactly what to target and so
anyone who is not an expert on the configuration they're working with tends
to get stuck unable to figure out exactly what they need to do.

The new -exclude option gives us an opportunity to do better here: we tend
to know for which object we're currently evaluating count or for_each, and
so we can mention that object directly in the error message when if we
recommend to use -exclude instead of -target.

Not all objects that support count/for_each will necessarily be directly
targetable, so we can still potentially recommend -target when we're
dealing with one of those objects. For example, as of this commit that
is true for for_each in a provider block, because there is not currently
any syntax for specifying a provider configuration as an addrs.Targetable.
Perhaps we'll introduce such a thing in the future, but that's outside the
scope of this change that's primarily focused on improving the messaging
for resource and module count/for_each.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This commit is contained in:
Martin Atkins 2024-11-07 13:48:10 -08:00
parent be5b14625d
commit f1358f9fe8
14 changed files with 361 additions and 41 deletions

View File

@ -179,7 +179,7 @@ func (p *Provider) decodeStaticFields(eval *StaticEvaluator) hcl.Diagnostics {
return evalContext, diags.Append(evalDiags)
}
forVal, evalDiags := evalchecks.EvaluateForEachExpression(p.ForEach, forEachRefsFunc)
forVal, evalDiags := evalchecks.EvaluateForEachExpression(p.ForEach, forEachRefsFunc, nil)
diags = append(diags, evalDiags.ToHCL()...)
if evalDiags.HasErrors() {
return diags

View File

@ -6,6 +6,7 @@
package evalchecks
import (
"github.com/apparentlymart/go-shquot/shquot"
"github.com/opentofu/opentofu/internal/tfdiags"
)
@ -45,3 +46,46 @@ var _ tfdiags.DiagnosticExtraBecauseSensitive = DiagnosticCausedBySensitive(true
func (e DiagnosticCausedBySensitive) DiagnosticCausedBySensitive() bool {
return bool(e)
}
// commandLineArgumentsSuggestion returns a representation of the given command line
// arguments that includes suitable quoting or escaping to make it more likely to work
// unaltered if copy-pasted into the command line on the current host system.
//
// We don't try to determine exactly what shell someone is using, so this won't be
// 100% correct in all cases but will at least deal with the most common hazards of
// quoting strings that contain spaces, and escaping some metacharacters.
//
// The "goos" argument should be the value of [runtime.GOOS] when calling this from
// real code, but can be fixed to a particular value when writing unit tests to ensure
// that they behave the same regardless of what platform is being used for development.
func commandLineArgumentsSuggestion(args []string, goos string) string {
// For now we assume that there are only two possibilities: Windows or "POSIX-ish".
// We use the "shquot" library for this, since we already have it as
// a dependency for other purposes anyway.
var quot shquot.QS
switch goos {
case "windows":
// "WindowsArgvSplit" produces something that's compatible with
// the de-facto standard argument parsing implemented by Microsoft's
// Visual C++ runtime library, which the Go runtime mimics and
// is therefore the most likely to succeed for running OpenTofu.
//
// Unfortunately running normal programs through PowerShell adds
// some additional quoting/escaping hazards which we don't attend
// to here because doing so tends to make the result invalid
// for use outside of PowerShell.
quot = shquot.WindowsArgvSplit
default:
// We'll assume that all other platforms we support are compatible
// with the POSIX shell escaping rules.
quot = shquot.POSIXShellSplit
}
// We're only interested in the "arguments" part of the result, but
// the shquot library wants us to provide an argv[0] anyway so we'll
// hard-code that as "tofu" but then ignore it altogether in the
// result.
cmdLine := append([]string{"tofu"}, args...)
_, ret := quot(cmdLine)
return ret
}

View File

@ -6,8 +6,10 @@ package evalchecks
import (
"fmt"
"runtime"
"github.com/hashicorp/hcl/v2"
"github.com/opentofu/opentofu/internal/addrs"
"github.com/opentofu/opentofu/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/gocty"
@ -23,7 +25,11 @@ type EvaluateFunc func(expr hcl.Expression) (cty.Value, tfdiags.Diagnostics)
// EvaluateCountExpression differs from EvaluateCountExpressionValue by
// returning an error if the count value is not known, and converting the
// cty.Value to an integer.
func EvaluateCountExpression(expr hcl.Expression, ctx EvaluateFunc) (int, tfdiags.Diagnostics) {
//
// If excludableAddr is non-nil then the unknown value error will include
// an additional idea to exclude that address using the -exclude
// planning option to converge over multiple plan/apply rounds.
func EvaluateCountExpression(expr hcl.Expression, ctx EvaluateFunc, excludableAddr addrs.Targetable) (int, tfdiags.Diagnostics) {
countVal, diags := EvaluateCountExpressionValue(expr, ctx)
if !countVal.IsKnown() {
// Currently this is a rather bad outcome from a UX standpoint, since we have
@ -32,10 +38,12 @@ func EvaluateCountExpression(expr hcl.Expression, ctx EvaluateFunc) (int, tfdiag
// FIXME: In future, implement a built-in mechanism for deferring changes that
// can't yet be predicted, and use it to guide the user through several
// plan/apply steps until the desired configuration is eventually reached.
suggestion := countCommandLineExcludeSuggestion(excludableAddr)
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid count argument",
Detail: `The "count" value depends on resource attributes that cannot be determined until apply, so OpenTofu cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the count depends on.`,
Detail: "The \"count\" value depends on resource attributes that cannot be determined until apply, so OpenTofu cannot predict how many instances will be created.\n\n" + suggestion,
Subject: expr.Range().Ptr(),
// TODO: Also populate Expression and EvalContext in here, but
@ -111,3 +119,34 @@ func EvaluateCountExpressionValue(expr hcl.Expression, ctx EvaluateFunc) (cty.Va
return countVal, diags
}
// Returns some English-language text describing a workaround using the -exclude
// planning option to converge over two plan/apply rounds when count has an
// unknown value.
//
// This is intended only for when a count value is too unknown for
// planning to proceed, in [EvaluateCountExpression].
//
// If excludableAddr is non-nil then the message will refer to it directly, giving
// a full copy-pastable command line argument. Otherwise, the message is a generic
// one without any specific address indicated.
func countCommandLineExcludeSuggestion(excludableAddr addrs.Targetable) string {
// We use an extra indirection here so that we can write tests that make
// the same assertions on all development platforms.
return countCommandLineExcludeSuggestionImpl(excludableAddr, runtime.GOOS)
}
func countCommandLineExcludeSuggestionImpl(excludableAddr addrs.Targetable, goos string) string {
if excludableAddr == nil {
// We use -target for this case because we can't be sure that the
// object we're complaining about even has its own addrs.Targetable
// address, and so the user might need to target only what it depends
// on instead.
return `To work around this, use the -target option to first apply only the resources that the count depends on, and then apply normally to converge.`
}
return fmt.Sprintf(
"To work around this, use the planning option -exclude=%s to first apply without this object, and then apply normally to converge.",
commandLineArgumentsSuggestion([]string{excludableAddr.String()}, goos),
)
}

View File

@ -5,6 +5,7 @@
package evalchecks
import (
"fmt"
"reflect"
"strings"
"testing"
@ -12,6 +13,7 @@ import (
"github.com/davecgh/go-spew/spew"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hcltest"
"github.com/opentofu/opentofu/internal/addrs"
"github.com/opentofu/opentofu/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
)
@ -36,7 +38,7 @@ func TestEvaluateCountExpression_valid(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
actual, diags := EvaluateCountExpression(hcltest.MockExprLiteral(test.val), mockEvaluateFunc(test.val))
actual, diags := EvaluateCountExpression(hcltest.MockExprLiteral(test.val), mockEvaluateFunc(test.val), nil)
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
@ -55,50 +57,68 @@ func TestEvaluateCountExpression_valid(t *testing.T) {
func TestEvaluateCountExpression_errors(t *testing.T) {
tests := map[string]struct {
val cty.Value
excludableAddr addrs.Targetable
Summary, DetailSubstring string
CausedByUnknown bool
}{
"null": {
cty.NullVal(cty.Number),
nil,
"Invalid count argument",
`The given "count" argument value is null. An integer is required.`,
false,
},
"negative": {
cty.NumberIntVal(-1),
nil,
"Invalid count argument",
`The given "count" argument value is unsuitable: must be greater than or equal to zero.`,
false,
},
"string": {
cty.StringVal("i am definitely a number"),
nil,
"Invalid count argument",
"The given \"count\" argument value is unsuitable: number value is required.",
false,
},
"list": {
cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("a")}),
nil,
"Invalid count argument",
"The given \"count\" argument value is unsuitable: number value is required.",
false,
},
"tuple": {
cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")}),
nil,
"Invalid count argument",
"The given \"count\" argument value is unsuitable: number value is required.",
false,
},
"unknown": {
cty.UnknownVal(cty.Number),
nil,
"Invalid count argument",
`he "count" value depends on resource attributes that cannot be determined until apply, so OpenTofu cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the count depends on.`,
"The \"count\" value depends on resource attributes that cannot be determined until apply, so OpenTofu cannot predict how many instances will be created.\n\nTo work around this, use the -target option to first apply only the resources that the count depends on, and then apply normally to converge.",
true,
},
"unknown with excludable address": {
cty.UnknownVal(cty.Number),
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Name: "foo",
Type: "bar",
}.Absolute(addrs.RootModuleInstance.Child("a", addrs.NoKey)),
"Invalid count argument",
"The \"count\" value depends on resource attributes that cannot be determined until apply, so OpenTofu cannot predict how many instances will be created.\n\nTo work around this, use the planning option -exclude=module.a.bar.foo to first apply without this object, and then apply normally to converge.",
true,
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
_, diags := EvaluateCountExpression(hcltest.MockExprLiteral(test.val), mockEvaluateFunc(test.val))
_, diags := EvaluateCountExpression(hcltest.MockExprLiteral(test.val), mockEvaluateFunc(test.val), test.excludableAddr)
if len(diags) != 1 {
t.Fatalf("got %d diagnostics; want 1", diags)
@ -119,3 +139,77 @@ func TestEvaluateCountExpression_errors(t *testing.T) {
})
}
}
func TestCountCommandLineExcludeSuggestion(t *testing.T) {
noKeyResourceAddr := addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "happycloud_virtual_machine",
Name: "boop",
}.Absolute(addrs.RootModuleInstance)
stringKeyResourceAddr := addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "happycloud_virtual_machine",
Name: "boop",
}.Absolute(addrs.RootModuleInstance.Child("foo", addrs.StringKey("bleep bloop")))
tests := []struct {
excludableAddr addrs.Targetable
goos string
want string
}{
{
nil,
"linux",
`To work around this, use the -target option to first apply only the resources that the count depends on, and then apply normally to converge.`,
},
{
nil,
"windows",
`To work around this, use the -target option to first apply only the resources that the count depends on, and then apply normally to converge.`,
},
{
nil,
"darwin",
`To work around this, use the -target option to first apply only the resources that the count depends on, and then apply normally to converge.`,
},
{
noKeyResourceAddr,
"linux",
`To work around this, use the planning option -exclude=happycloud_virtual_machine.boop to first apply without this object, and then apply normally to converge.`,
},
{
noKeyResourceAddr,
"windows",
`To work around this, use the planning option -exclude=happycloud_virtual_machine.boop to first apply without this object, and then apply normally to converge.`,
},
{
noKeyResourceAddr,
"darwin",
`To work around this, use the planning option -exclude=happycloud_virtual_machine.boop to first apply without this object, and then apply normally to converge.`,
},
{
stringKeyResourceAddr,
"linux",
`To work around this, use the planning option -exclude='module.foo["bleep bloop"].happycloud_virtual_machine.boop' to first apply without this object, and then apply normally to converge.`,
},
{
stringKeyResourceAddr,
"windows",
`To work around this, use the planning option -exclude="module.foo[\"bleep bloop\"].happycloud_virtual_machine.boop" to first apply without this object, and then apply normally to converge.`,
},
{
stringKeyResourceAddr,
"darwin",
`To work around this, use the planning option -exclude='module.foo["bleep bloop"].happycloud_virtual_machine.boop' to first apply without this object, and then apply normally to converge.`,
},
}
for _, test := range tests {
t.Run(fmt.Sprintf("%v on %s", test.excludableAddr, test.goos), func(t *testing.T) {
got := countCommandLineExcludeSuggestionImpl(test.excludableAddr, test.goos)
if got != test.want {
t.Errorf("wrong suggestion\ngot: %s\nwant: %s", got, test.want)
}
})
}
}

View File

@ -6,6 +6,7 @@ package evalchecks
import (
"fmt"
"runtime"
"github.com/hashicorp/hcl/v2"
"github.com/opentofu/opentofu/internal/addrs"
@ -16,8 +17,8 @@ import (
)
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."
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."
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\n"
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\n"
)
type ContextFunc func(refs []*addrs.Reference) (*hcl.EvalContext, tfdiags.Diagnostics)
@ -30,10 +31,14 @@ type ContextFunc func(refs []*addrs.Reference) (*hcl.EvalContext, tfdiags.Diagno
// EvaluateForEachExpression differs from EvaluateForEachExpressionValue by
// returning an error if the count value is not known, and converting the
// cty.Value to a map[string]cty.Value for compatibility with other calls.
func EvaluateForEachExpression(expr hcl.Expression, ctx ContextFunc) (map[string]cty.Value, tfdiags.Diagnostics) {
//
// If excludableAddr is non-nil then the unknown value error will include
// an additional idea to exclude that address using the -exclude
// planning option to converge over multiple plan/apply rounds.
func EvaluateForEachExpression(expr hcl.Expression, ctx ContextFunc, excludableAddr addrs.Targetable) (map[string]cty.Value, tfdiags.Diagnostics) {
const unknownsNotAllowed = false
const tupleNotAllowed = false
forEachVal, diags := EvaluateForEachExpressionValue(expr, ctx, unknownsNotAllowed, tupleNotAllowed)
forEachVal, diags := EvaluateForEachExpressionValue(expr, ctx, unknownsNotAllowed, tupleNotAllowed, excludableAddr)
// forEachVal might be unknown, but if it is then there should already
// be an error about it in diags, which we'll return below.
@ -49,7 +54,11 @@ func EvaluateForEachExpression(expr hcl.Expression, ctx ContextFunc) (map[string
// except that it returns a cty.Value map or set which can be unknown.
// The 'allowTuple' argument is used to support evaluating for_each from tuple
// values, and is currently supported when using for_each in import blocks.
func EvaluateForEachExpressionValue(expr hcl.Expression, ctx ContextFunc, allowUnknown bool, allowTuple bool) (cty.Value, tfdiags.Diagnostics) {
//
// If excludableAddr is non-nil then any unknown-value-related error will
// include an additional idea to exclude that address using the -exclude
// planning option to converge over multiple plan/apply rounds.
func EvaluateForEachExpressionValue(expr hcl.Expression, ctx ContextFunc, allowUnknown bool, allowTuple bool, excludableAddr addrs.Targetable) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
nullMap := cty.NullVal(cty.Map(cty.DynamicPseudoType))
@ -112,7 +121,7 @@ func EvaluateForEachExpressionValue(expr hcl.Expression, ctx ContextFunc, allowU
return nullMap, diags
}
forEachVal, diags = performForEachValueChecks(expr, hclCtx, allowUnknown, forEachVal, allowedTypesMessage)
forEachVal, diags = performForEachValueChecks(expr, hclCtx, allowUnknown, forEachVal, allowedTypesMessage, excludableAddr)
if diags.HasErrors() {
return forEachVal, diags
}
@ -121,7 +130,7 @@ func EvaluateForEachExpressionValue(expr hcl.Expression, ctx ContextFunc, allowU
}
// performForEachValueChecks ensures the for_each argument is valid
func performForEachValueChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, allowUnknown bool, forEachVal cty.Value, allowedTypesMessage string) (cty.Value, tfdiags.Diagnostics) {
func performForEachValueChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, allowUnknown bool, forEachVal cty.Value, allowedTypesMessage string, excludableAddr addrs.Targetable) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
nullMap := cty.NullVal(cty.Map(cty.DynamicPseudoType))
ty := forEachVal.Type()
@ -146,6 +155,7 @@ func performForEachValueChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, all
default:
detailMsg = errInvalidUnknownDetailMap
}
detailMsg += forEachCommandLineExcludeSuggestion(excludableAddr)
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
@ -167,7 +177,7 @@ func performForEachValueChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, all
}
if ty.IsSetType() {
setVal, setTypeDiags := performSetTypeChecks(expr, hclCtx, allowUnknown, forEachVal)
setVal, setTypeDiags := performSetTypeChecks(expr, hclCtx, allowUnknown, forEachVal, excludableAddr)
diags = diags.Append(setTypeDiags)
if diags.HasErrors() {
return setVal, diags
@ -178,7 +188,7 @@ func performForEachValueChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, all
}
// performSetTypeChecks does checks when we have a Set type, as sets have some gotchas
func performSetTypeChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, allowUnknown bool, forEachVal cty.Value) (cty.Value, tfdiags.Diagnostics) {
func performSetTypeChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, allowUnknown bool, forEachVal cty.Value, excludableAddr addrs.Targetable) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
ty := forEachVal.Type()
@ -189,7 +199,7 @@ func performSetTypeChecks(expr hcl.Expression, hclCtx *hcl.EvalContext, allowUnk
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid for_each argument",
Detail: errInvalidUnknownDetailSet,
Detail: errInvalidUnknownDetailSet + forEachCommandLineExcludeSuggestion(excludableAddr),
Subject: expr.Range().Ptr(),
Expression: expr,
EvalContext: hclCtx,
@ -236,3 +246,36 @@ func markSafeLengthInt(val cty.Value) int {
v, _ := val.UnmarkDeep()
return v.LengthInt()
}
// Returns some English-language text describing a workaround using the -exclude
// planning option to converge over two plan/apply rounds when for_each has an
// unknown value.
//
// This is intended only for when a for_each value is too unknown for
// planning to proceed, in [EvaluateForEachExpression] or [EvaluateForEachExpressionValue].
// The message always begins with "Alternatively, " because it's intended to be
// appended to one of either [errInvalidUnknownDetailMap] or [errInvalidUnknownDetailSet].
//
// If excludableAddr is non-nil then the message will refer to it directly, giving
// a full copy-pastable command line argument. Otherwise, the message is a generic
// one without any specific address indicated.
func forEachCommandLineExcludeSuggestion(excludableAddr addrs.Targetable) string {
// We use an extra indirection here so that we can write tests that make
// the same assertions on all development platforms.
return forEachCommandLineExcludeSuggestionImpl(excludableAddr, runtime.GOOS)
}
func forEachCommandLineExcludeSuggestionImpl(excludableAddr addrs.Targetable, goos string) string {
if excludableAddr == nil {
// We use -target for this case because we can't be sure that the
// object we're complaining about even has its own addrs.Targetable
// address, and so the user might need to target only what it depends
// on instead.
return `Alternatively, you could use the -target option to first apply only the resources that for_each depends on, and then apply normally to converge.`
}
return fmt.Sprintf(
`Alternatively, you could use the planning option -exclude=%s to first apply without this object, and then apply normally to converge.`,
commandLineArgumentsSuggestion([]string{excludableAddr.String()}, goos),
)
}

View File

@ -5,6 +5,7 @@
package evalchecks
import (
"fmt"
"reflect"
"strings"
"testing"
@ -81,7 +82,7 @@ func TestEvaluateForEachExpression(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
forEachMap, diags := EvaluateForEachExpression(test.Expr, mockRefsFunc())
forEachMap, diags := EvaluateForEachExpression(test.Expr, mockRefsFunc(), nil)
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
@ -100,47 +101,55 @@ func TestEvaluateForEachExpression(t *testing.T) {
func TestEvaluateForEachExpression_errors(t *testing.T) {
tests := map[string]struct {
Expr hcl.Expression
ExcludableAddr addrs.Targetable
Summary, DetailSubstring string
CausedByUnknown, CausedBySensitive bool
}{
"null set": {
hcltest.MockExprLiteral(cty.NullVal(cty.Set(cty.String))),
nil,
"Invalid for_each argument",
`the given "for_each" argument value is null`,
false, false,
},
"string": {
hcltest.MockExprLiteral(cty.StringVal("i am definitely a set")),
nil,
"Invalid for_each argument",
"must be a map, or set of strings, and you have provided a value of type string",
false, false,
},
"list": {
hcltest.MockExprLiteral(cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("a")})),
nil,
"Invalid for_each argument",
"must be a map, or set of strings, and you have provided a value of type list",
false, false,
},
"tuple": {
hcltest.MockExprLiteral(cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")})),
nil,
"Invalid for_each argument",
"must be a map, or set of strings, and you have provided a value of type tuple",
false, false,
},
"unknown string set": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
nil,
"Invalid for_each argument",
"set includes values derived from resource attributes that cannot be determined until apply",
true, false,
},
"unknown map": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))),
nil,
"Invalid for_each argument",
"map includes keys derived from resource attributes that cannot be determined until apply",
true, false,
},
"unknown pseudo-type": {
hcltest.MockExprLiteral(cty.UnknownVal(cty.DynamicPseudoType)),
nil,
"Invalid for_each argument",
"map includes keys derived from resource attributes that cannot be determined until apply",
true, false,
@ -150,36 +159,53 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
"a": cty.BoolVal(true),
"b": cty.BoolVal(false),
}).Mark(marks.Sensitive)),
nil,
"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.",
false, true,
},
"set containing booleans": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.BoolVal(true)})),
nil,
"Invalid for_each set argument",
"supports sets of strings, but you have provided a set containing type bool",
false, false,
},
"set containing null": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.NullVal(cty.String)})),
nil,
"Invalid for_each set argument",
"must not contain null values",
false, false,
},
"set containing unknown value": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)})),
nil,
"Invalid for_each argument",
"set includes values derived from resource attributes that cannot be determined until apply",
"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 option to first apply only the resources that for_each depends on, and then apply normally to converge.",
true, false,
},
"set containing unknown value in excludable object": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)})),
addrs.Resource{
Mode: addrs.ManagedResourceMode,
Name: "foo",
Type: "bar",
}.Absolute(addrs.RootModuleInstance.Child("a", addrs.NoKey)),
"Invalid for_each argument",
"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 planning option -exclude=module.a.bar.foo to first apply without this object, and then apply normally to converge.",
true, false,
},
"set containing dynamic unknown value": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.DynamicPseudoType)})),
nil,
"Invalid for_each argument",
"set includes values derived from resource attributes that cannot be determined until apply",
true, false,
},
"set containing marked values": {
hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.StringVal("beep").Mark(marks.Sensitive), cty.StringVal("boop")})),
nil,
"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.",
false, true,
@ -188,7 +214,7 @@ func TestEvaluateForEachExpression_errors(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
_, diags := EvaluateForEachExpression(test.Expr, mockRefsFunc())
_, diags := EvaluateForEachExpression(test.Expr, mockRefsFunc(), test.ExcludableAddr)
if len(diags) != 1 {
t.Fatalf("got %d diagnostics; want 1", diags)
@ -293,7 +319,7 @@ func TestEvaluateForEachExpression_multi_errors(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
_, diags := EvaluateForEachExpression(test.Expr, mockRefsFunc())
_, diags := EvaluateForEachExpression(test.Expr, mockRefsFunc(), nil)
if len(diags) != len(test.Wanted) {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
}
@ -336,7 +362,7 @@ func TestEvaluateForEachExpressionKnown(t *testing.T) {
for name, expr := range tests {
t.Run(name, func(t *testing.T) {
forEachVal, diags := EvaluateForEachExpressionValue(expr, mockRefsFunc(), true, true)
forEachVal, diags := EvaluateForEachExpressionValue(expr, mockRefsFunc(), true, true, nil)
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
@ -382,7 +408,7 @@ func TestEvaluateForEachExpressionValueTuple(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
_, diags := EvaluateForEachExpressionValue(test.Expr, mockRefsFunc(), true, test.AllowTuple)
_, diags := EvaluateForEachExpressionValue(test.Expr, mockRefsFunc(), true, test.AllowTuple, nil)
if test.ExpectedError == "" {
if len(diags) != 0 {
@ -396,3 +422,77 @@ func TestEvaluateForEachExpressionValueTuple(t *testing.T) {
})
}
}
func TestForEachCommandLineExcludeSuggestion(t *testing.T) {
noKeyResourceAddr := addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "happycloud_virtual_machine",
Name: "boop",
}.Absolute(addrs.RootModuleInstance)
stringKeyResourceAddr := addrs.Resource{
Mode: addrs.ManagedResourceMode,
Type: "happycloud_virtual_machine",
Name: "boop",
}.Absolute(addrs.RootModuleInstance.Child("foo", addrs.StringKey("bleep bloop")))
tests := []struct {
excludableAddr addrs.Targetable
goos string
want string
}{
{
nil,
"linux",
`Alternatively, you could use the -target option to first apply only the resources that for_each depends on, and then apply normally to converge.`,
},
{
nil,
"windows",
`Alternatively, you could use the -target option to first apply only the resources that for_each depends on, and then apply normally to converge.`,
},
{
nil,
"darwin",
`Alternatively, you could use the -target option to first apply only the resources that for_each depends on, and then apply normally to converge.`,
},
{
noKeyResourceAddr,
"linux",
`Alternatively, you could use the planning option -exclude=happycloud_virtual_machine.boop to first apply without this object, and then apply normally to converge.`,
},
{
noKeyResourceAddr,
"windows",
`Alternatively, you could use the planning option -exclude=happycloud_virtual_machine.boop to first apply without this object, and then apply normally to converge.`,
},
{
noKeyResourceAddr,
"darwin",
`Alternatively, you could use the planning option -exclude=happycloud_virtual_machine.boop to first apply without this object, and then apply normally to converge.`,
},
{
stringKeyResourceAddr,
"linux",
`Alternatively, you could use the planning option -exclude='module.foo["bleep bloop"].happycloud_virtual_machine.boop' to first apply without this object, and then apply normally to converge.`,
},
{
stringKeyResourceAddr,
"windows",
`Alternatively, you could use the planning option -exclude="module.foo[\"bleep bloop\"].happycloud_virtual_machine.boop" to first apply without this object, and then apply normally to converge.`,
},
{
stringKeyResourceAddr,
"darwin",
`Alternatively, you could use the planning option -exclude='module.foo["bleep bloop"].happycloud_virtual_machine.boop' to first apply without this object, and then apply normally to converge.`,
},
}
for _, test := range tests {
t.Run(fmt.Sprintf("%v on %s", test.excludableAddr, test.goos), func(t *testing.T) {
got := forEachCommandLineExcludeSuggestionImpl(test.excludableAddr, test.goos)
if got != test.want {
t.Errorf("wrong suggestion\ngot: %s\nwant: %s", got, test.want)
}
})
}
}

View File

@ -127,7 +127,7 @@ func (ri *ImportResolver) ExpandAndResolveImport(importTarget *ImportTarget, ctx
const tupleAllowed = true
// The import target has a for_each attribute, so we need to expand it
forEachVal, evalDiags := evaluateForEachExpressionValue(importTarget.Config.ForEach, rootCtx, unknownsNotAllowed, tupleAllowed)
forEachVal, evalDiags := evaluateForEachExpressionValue(importTarget.Config.ForEach, rootCtx, unknownsNotAllowed, tupleAllowed, nil)
diags = diags.Append(evalDiags)
if diags.HasErrors() {
return diags

View File

@ -34,7 +34,7 @@ func TestEvaluateCountExpression(t *testing.T) {
t.Run(name, func(t *testing.T) {
ctx := &MockEvalContext{}
ctx.installSimpleEval()
countVal, diags := evaluateCountExpression(test.Expr, ctx)
countVal, diags := evaluateCountExpression(test.Expr, ctx, nil)
if len(diags) != 0 {
t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))

View File

@ -32,16 +32,16 @@ func evalContextEvaluate(ctx EvalContext) evalchecks.EvaluateFunc {
}
}
func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext) (map[string]cty.Value, tfdiags.Diagnostics) {
return evalchecks.EvaluateForEachExpression(expr, evalContextScope(ctx))
func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext, excludeableAddr addrs.Targetable) (map[string]cty.Value, tfdiags.Diagnostics) {
return evalchecks.EvaluateForEachExpression(expr, evalContextScope(ctx), excludeableAddr)
}
func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowUnknown bool, allowTuple bool) (cty.Value, tfdiags.Diagnostics) {
return evalchecks.EvaluateForEachExpressionValue(expr, evalContextScope(ctx), allowUnknown, allowTuple)
func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowUnknown bool, allowTuple bool, excludeableAddr addrs.Targetable) (cty.Value, tfdiags.Diagnostics) {
return evalchecks.EvaluateForEachExpressionValue(expr, evalContextScope(ctx), allowUnknown, allowTuple, excludeableAddr)
}
func evaluateCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) {
return evalchecks.EvaluateCountExpression(expr, evalContextEvaluate(ctx))
func evaluateCountExpression(expr hcl.Expression, ctx EvalContext, excludeableAddr addrs.Targetable) (int, tfdiags.Diagnostics) {
return evalchecks.EvaluateCountExpression(expr, evalContextEvaluate(ctx), excludeableAddr)
}
func evaluateCountExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {

View File

@ -123,7 +123,7 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) (diags tfd
ctx = ctx.WithPath(module)
switch {
case n.ModuleCall.Count != nil:
count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx)
count, ctDiags := evaluateCountExpression(n.ModuleCall.Count, ctx, module)
diags = diags.Append(ctDiags)
if diags.HasErrors() {
return diags
@ -131,7 +131,7 @@ func (n *nodeExpandModule) Execute(ctx EvalContext, op walkOperation) (diags tfd
expander.SetModuleCount(module, call, count)
case n.ModuleCall.ForEach != nil:
forEach, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx)
forEach, feDiags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx, module)
diags = diags.Append(feDiags)
if diags.HasErrors() {
return diags
@ -268,7 +268,7 @@ func (n *nodeValidateModule) Execute(ctx EvalContext, op walkOperation) (diags t
case n.ModuleCall.ForEach != nil:
const unknownsAllowed = true
const tupleNotAllowed = false
_, forEachDiags := evaluateForEachExpressionValue(n.ModuleCall.ForEach, ctx, unknownsAllowed, tupleNotAllowed)
_, forEachDiags := evaluateForEachExpressionValue(n.ModuleCall.ForEach, ctx, unknownsAllowed, tupleNotAllowed, module)
diags = diags.Append(forEachDiags)
}

View File

@ -485,7 +485,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab
switch {
case n.Config != nil && n.Config.Count != nil:
count, countDiags := evaluateCountExpression(n.Config.Count, ctx)
count, countDiags := evaluateCountExpression(n.Config.Count, ctx, addr)
diags = diags.Append(countDiags)
if countDiags.HasErrors() {
return diags
@ -495,7 +495,7 @@ func (n *NodeAbstractResource) writeResourceState(ctx EvalContext, addr addrs.Ab
expander.SetResourceCount(addr.Module, n.Addr.Resource, count)
case n.Config != nil && n.Config.ForEach != nil:
forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, addr)
diags = diags.Append(forEachDiags)
if forEachDiags.HasErrors() {
return diags

View File

@ -839,7 +839,7 @@ func (n *NodeAbstractResourceInstance) plan(
}
// Evaluate the configuration
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx, n.Addr)
keyData = EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)
@ -1741,7 +1741,7 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule
objTy := schema.ImpliedType()
priorVal := cty.NullVal(objTy)
forEach, _ := evaluateForEachExpression(config.ForEach, ctx)
forEach, _ := evaluateForEachExpression(config.ForEach, ctx, n.Addr)
keyData = EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)
checkDiags := evalCheckRules(
@ -2020,7 +2020,7 @@ func (n *NodeAbstractResourceInstance) applyDataSource(ctx EvalContext, planned
return nil, keyData, diags
}
forEach, _ := evaluateForEachExpression(config.ForEach, ctx)
forEach, _ := evaluateForEachExpression(config.ForEach, ctx, n.Addr)
keyData = EvalDataForInstanceKey(n.Addr.Resource.Key, forEach)
checkDiags := evalCheckRules(
@ -2324,7 +2324,7 @@ func (n *NodeAbstractResourceInstance) applyProvisioners(ctx EvalContext, state
func (n *NodeAbstractResourceInstance) evalProvisionerConfig(ctx EvalContext, body hcl.Body, self cty.Value, schema *configschema.Block) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, n.Addr)
diags = diags.Append(forEachDiags)
keyData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)

View File

@ -402,7 +402,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
// values, which could result in a post-condition check relying on that
// value being inaccurate. Unless we decide to store the value of the
// for-each expression in state, this is unavoidable.
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx, n.ResourceAddr())
repeatData := EvalDataForInstanceKey(n.ResourceInstanceAddr().Resource.Key, forEach)
checkDiags := evalCheckRules(

View File

@ -568,7 +568,7 @@ func validateForEach(ctx EvalContext, expr hcl.Expression) (diags tfdiags.Diagno
const unknownsAllowed = true
const tupleNotAllowed = false
val, forEachDiags := evaluateForEachExpressionValue(expr, ctx, unknownsAllowed, tupleNotAllowed)
val, forEachDiags := evaluateForEachExpressionValue(expr, ctx, unknownsAllowed, tupleNotAllowed, nil)
// If the value isn't known then that's the best we can do for now, but
// we'll check more thoroughly during the plan walk
if !val.IsKnown() {