diff --git a/internal/stacks/stackruntime/internal/stackeval/component.go b/internal/stacks/stackruntime/internal/stackeval/component.go index e5c2dd933677..26f549da24bb 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component.go +++ b/internal/stacks/stackruntime/internal/stackeval/component.go @@ -116,17 +116,6 @@ func (c *Component) CheckForEachValue(ctx context.Context, phase EvalPhase) (cty return cty.DynamicVal, diags } - if !result.Value.IsKnown() { - // FIXME: We should somehow allow this and emit a - // "deferred change" representing all of the as-yet-unknown - // instances of this call and everything beneath it. - diags = diags.Append(result.Diagnostic( - tfdiags.Error, - "Invalid for_each value", - "The for_each value must not be derived from values that will be determined only during the apply phase.", - )) - } - return result.Value, diags default: @@ -174,7 +163,7 @@ func (c *Component) CheckInstances(ctx context.Context, phase EvalPhase) (map[ad ret := instancesMap(forEachVal, func(ik addrs.InstanceKey, rd instances.RepetitionData) *ComponentInstance { return newComponentInstance(c, ik, rd) - }) + }, true) addrs := make([]stackaddrs.AbsComponentInstance, 0, len(ret)) for _, ci := range ret { @@ -209,6 +198,12 @@ func (c *Component) ResultValue(ctx context.Context, phase EvalPhase) cty.Value return cty.DynamicVal } + if insts[addrs.WildcardKey] != nil { + // If the wildcard key is used the instance originates from an unknown + // for_each value, which means the result is unknown. + return cty.DynamicVal + } + // We expect that the instances all have string keys, which will // become the keys of a map that we're returning. elems := make(map[string]cty.Value, len(insts)) diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 6d1d3ef8e530..6b33800f80b3 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -576,6 +576,11 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla } } + // The instance is also upstream deferred if the for_each value for this instance is unknown. + if c.key == addrs.WildcardKey { + upstreamDeferred = true // TODO: Test this + } + // NOTE: This ComponentInstance type only deals with component // instances currently declared in the configuration. See // [ComponentInstanceRemoved] for the model of a component instance diff --git a/internal/stacks/stackruntime/internal/stackeval/component_test.go b/internal/stacks/stackruntime/internal/stackeval/component_test.go index 0f5cd2769b65..612fe48cad49 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_test.go @@ -190,14 +190,12 @@ func TestComponentCheckInstances(t *testing.T) { } // When the for_each expression is invalid, CheckInstances should - // return nil to represent that we don't know enough to predict - // how many instances there are. This is a different result than - // when we know there are zero instances, which would be a non-nil - // empty map. + // return a single instance with dynamic values in the repetition data. + // We don't distinguish between invalid and unknown for_each values. gotInsts, diags := component.CheckInstances(ctx, InspectPhase) assertNoDiags(t, diags) - if gotInsts != nil { - t.Errorf("wrong instances; want nil\n%#v", gotInsts) + if got, want := len(gotInsts), 1; got != want { + t.Fatalf("wrong number of instances %d; want %d\n%#v", got, want, gotInsts) } }) subtestInPromisingTask(t, "unknown", func(ctx context.Context, t *testing.T) { @@ -208,30 +206,33 @@ func TestComponentCheckInstances(t *testing.T) { }, }) - // For now it's invalid to use an unknown value in for_each. - // Later we're expecting to make this succeed but announce that - // planning everything beneath this component must be deferred to a - // future plan after everything else has been applied first. component := getComponent(ctx, main) gotVal, diags := component.CheckForEachValue(ctx, InspectPhase) - assertMatchingDiag(t, diags, func(diag tfdiags.Diagnostic) bool { - return (diag.Severity() == tfdiags.Error && - diag.Description().Detail == "The for_each value must not be derived from values that will be determined only during the apply phase.") - }) + assertNoDiags(t, diags) + wantVal := cty.UnknownVal(cty.Map(cty.EmptyObject)) if !wantVal.RawEquals(gotVal) { t.Errorf("wrong result\ngot: %#v\nwant: %#v", gotVal, wantVal) } - // When the for_each expression is invalid, CheckInstances should - // return nil to represent that we don't know enough to predict - // how many instances there are. This is a different result than - // when we know there are zero instances, which would be a non-nil - // empty map. + // When the for_each expression is unknown, CheckInstances should + // return a single instance with dynamic values in the repetition data. gotInsts, diags := component.CheckInstances(ctx, InspectPhase) assertNoDiags(t, diags) - if gotInsts != nil { - t.Errorf("wrong instances; want nil\n%#v", gotInsts) + if got, want := len(gotInsts), 1; got != want { + t.Fatalf("wrong number of instances %d; want %d\n%#v", got, want, gotInsts) + } + + if gotInsts[addrs.WildcardKey] == nil { + t.Fatalf("missing expected addrs.WildcardKey instance\n%#v", gotInsts) + } + + if gotInsts[addrs.WildcardKey].repetition.EachKey.IsKnown() { + t.Errorf("EachKey should be unknown, but is known") + } + + if gotInsts[addrs.WildcardKey].repetition.EachValue.IsKnown() { + t.Errorf("EachValue should be unknown, but is known") } }) }) diff --git a/internal/stacks/stackruntime/internal/stackeval/for_each.go b/internal/stacks/stackruntime/internal/stackeval/for_each.go index 5c717e3eab7a..ffb4985f521a 100644 --- a/internal/stacks/stackruntime/internal/stackeval/for_each.go +++ b/internal/stacks/stackruntime/internal/stackeval/for_each.go @@ -145,7 +145,7 @@ func evaluateForEachExpr(ctx context.Context, expr hcl.Expression, phase EvalPha // If maybeForEach value is non-nil but not a valid value produced by // [evaluateForEachExpr] then the behavior is unpredictable, including the // possibility of a panic. -func instancesMap[T any](maybeForEachVal cty.Value, makeInst func(addrs.InstanceKey, instances.RepetitionData) T) map[addrs.InstanceKey]T { +func instancesMap[T any](maybeForEachVal cty.Value, makeInst func(addrs.InstanceKey, instances.RepetitionData) T, supportsUnknownForEach bool) map[addrs.InstanceKey]T { switch { case maybeForEachVal == cty.NilVal: @@ -154,10 +154,12 @@ func instancesMap[T any](maybeForEachVal cty.Value, makeInst func(addrs.Instance return noForEachInstancesMap(makeInst) case !maybeForEachVal.IsKnown(): - // The for_each expression is too invalid for us to be able to - // know which instances exist. A totally nil map (as opposed to a - // non-nil map of length zero) signals that situation. - return nil + // This is temporary to gradually rollout support for unknown for_each values + if supportsUnknownForEach { + return unknownForEachInstancesMap(makeInst) + } else { + return nil + } default: // Otherwise we should be able to assume the value is valid per the @@ -239,6 +241,17 @@ func noForEachInstancesMap[T any](makeInst func(addrs.InstanceKey, instances.Rep } } +func unknownForEachInstancesMap[T any](makeInst func(addrs.InstanceKey, instances.RepetitionData) T) map[addrs.InstanceKey]T { + return map[addrs.InstanceKey]T{ + addrs.WildcardKey: makeInst(addrs.WildcardKey, instances.RepetitionData{ + // As we don't know the for_each value, we can only provide dynamic values + // for the repetition symbols. + EachKey: cty.DynamicVal, + EachValue: cty.DynamicVal, + }), + } +} + // markSafeLengthInt allows calling LengthInt on marked values safely func markSafeLengthInt(val cty.Value) int { v, _ := val.UnmarkDeep() diff --git a/internal/stacks/stackruntime/internal/stackeval/for_each_test.go b/internal/stacks/stackruntime/internal/stackeval/for_each_test.go index 018d6849fca6..34029bf85c27 100644 --- a/internal/stacks/stackruntime/internal/stackeval/for_each_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/for_each_test.go @@ -178,10 +178,17 @@ func TestEvaluateForEachExpr(t *testing.T) { } func TestInstancesMap(t *testing.T) { + type InstanceObj struct { Key addrs.InstanceKey Rep instances.RepetitionData } + // This is a temporary nusiance while we gradually rollout support for + // unknown for_each values. + type Expectation struct { + UnknownForEachSupported map[addrs.InstanceKey]InstanceObj + UnknownForEachUnsupported map[addrs.InstanceKey]InstanceObj + } makeObj := func(k addrs.InstanceKey, r instances.RepetitionData) InstanceObj { return InstanceObj{ Key: k, @@ -191,7 +198,7 @@ func TestInstancesMap(t *testing.T) { tests := []struct { Input cty.Value - Want map[addrs.InstanceKey]InstanceObj + Want Expectation // This function always either succeeds or panics, because it // expects to be given already-validated input from another function. @@ -200,11 +207,21 @@ func TestInstancesMap(t *testing.T) { // No for_each at all { cty.NilVal, - map[addrs.InstanceKey]InstanceObj{ - addrs.NoKey: { - Key: addrs.NoKey, - Rep: instances.RepetitionData{ - // No data available for the non-repeating case + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + addrs.NoKey: { + Key: addrs.NoKey, + Rep: instances.RepetitionData{ + // No data available for the non-repeating case + }, + }, + }, + UnknownForEachUnsupported: map[addrs.InstanceKey]InstanceObj{ + addrs.NoKey: { + Key: addrs.NoKey, + Rep: instances.RepetitionData{ + // No data available for the non-repeating case + }, }, }, }, @@ -213,40 +230,94 @@ func TestInstancesMap(t *testing.T) { // Unknowns { cty.UnknownVal(cty.EmptyObject), - nil, // a nil map means "unknown" for this function + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + addrs.WildcardKey: { + Key: addrs.WildcardKey, + Rep: instances.RepetitionData{ + EachKey: cty.DynamicVal, + EachValue: cty.DynamicVal, + }, + }, + }, + UnknownForEachUnsupported: nil, // a nil map means "unknown" for this function + }, }, { cty.UnknownVal(cty.Map(cty.Bool)), - nil, // a nil map means "unknown" for this function + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + addrs.WildcardKey: { + Key: addrs.WildcardKey, + Rep: instances.RepetitionData{ + EachKey: cty.DynamicVal, + EachValue: cty.DynamicVal, + }, + }, + }, + UnknownForEachUnsupported: nil, // a nil map means "unknown" for this function + }, }, { cty.UnknownVal(cty.Set(cty.String)), - nil, // a nil map means "unknown" for this function + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + addrs.WildcardKey: { + Key: addrs.WildcardKey, + Rep: instances.RepetitionData{ + EachKey: cty.DynamicVal, + EachValue: cty.DynamicVal, + }, + }, + }, + UnknownForEachUnsupported: nil, // a nil map means "unknown" for this function + }, }, // Empties { cty.EmptyObjectVal, - map[addrs.InstanceKey]InstanceObj{ - // intentionally a non-nil empty map to assert that we know - // that there are zero instances, rather than that we don't - // know how many there are. + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + // intentionally a non-nil empty map to assert that we know + // that there are zero instances, rather than that we don't + // know how many there are. + }, + UnknownForEachUnsupported: map[addrs.InstanceKey]InstanceObj{ + // intentionally a non-nil empty map to assert that we know + // that there are zero instances, rather than that we don't + // know how many there are. + }, }, }, { cty.MapValEmpty(cty.String), - map[addrs.InstanceKey]InstanceObj{ - // intentionally a non-nil empty map to assert that we know - // that there are zero instances, rather than that we don't - // know how many there are. + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + // intentionally a non-nil empty map to assert that we know + // that there are zero instances, rather than that we don't + // know how many there are. + }, + UnknownForEachUnsupported: map[addrs.InstanceKey]InstanceObj{ + // intentionally a non-nil empty map to assert that we know + // that there are zero instances, rather than that we don't + // know how many there are. + }, }, }, { cty.SetValEmpty(cty.String), - map[addrs.InstanceKey]InstanceObj{ - // intentionally a non-nil empty map to assert that we know - // that there are zero instances, rather than that we don't - // know how many there are. + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + // intentionally a non-nil empty map to assert that we know + // that there are zero instances, rather than that we don't + // know how many there are. + }, + UnknownForEachUnsupported: map[addrs.InstanceKey]InstanceObj{ + // intentionally a non-nil empty map to assert that we know + // that there are zero instances, rather than that we don't + // know how many there are. + }, }, }, @@ -256,19 +327,37 @@ func TestInstancesMap(t *testing.T) { "a": cty.StringVal("beep"), "b": cty.StringVal("boop"), }), - map[addrs.InstanceKey]InstanceObj{ - addrs.StringKey("a"): { - Key: addrs.StringKey("a"), - Rep: instances.RepetitionData{ - EachKey: cty.StringVal("a"), - EachValue: cty.StringVal("beep"), + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + addrs.StringKey("a"): { + Key: addrs.StringKey("a"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("a"), + EachValue: cty.StringVal("beep"), + }, + }, + addrs.StringKey("b"): { + Key: addrs.StringKey("b"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("b"), + EachValue: cty.StringVal("boop"), + }, }, }, - addrs.StringKey("b"): { - Key: addrs.StringKey("b"), - Rep: instances.RepetitionData{ - EachKey: cty.StringVal("b"), - EachValue: cty.StringVal("boop"), + UnknownForEachUnsupported: map[addrs.InstanceKey]InstanceObj{ + addrs.StringKey("a"): { + Key: addrs.StringKey("a"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("a"), + EachValue: cty.StringVal("beep"), + }, + }, + addrs.StringKey("b"): { + Key: addrs.StringKey("b"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("b"), + EachValue: cty.StringVal("boop"), + }, }, }, }, @@ -278,19 +367,37 @@ func TestInstancesMap(t *testing.T) { "a": cty.StringVal("beep"), "b": cty.StringVal("boop"), }), - map[addrs.InstanceKey]InstanceObj{ - addrs.StringKey("a"): { - Key: addrs.StringKey("a"), - Rep: instances.RepetitionData{ - EachKey: cty.StringVal("a"), - EachValue: cty.StringVal("beep"), + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + addrs.StringKey("a"): { + Key: addrs.StringKey("a"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("a"), + EachValue: cty.StringVal("beep"), + }, + }, + addrs.StringKey("b"): { + Key: addrs.StringKey("b"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("b"), + EachValue: cty.StringVal("boop"), + }, }, }, - addrs.StringKey("b"): { - Key: addrs.StringKey("b"), - Rep: instances.RepetitionData{ - EachKey: cty.StringVal("b"), - EachValue: cty.StringVal("boop"), + UnknownForEachUnsupported: map[addrs.InstanceKey]InstanceObj{ + addrs.StringKey("a"): { + Key: addrs.StringKey("a"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("a"), + EachValue: cty.StringVal("beep"), + }, + }, + addrs.StringKey("b"): { + Key: addrs.StringKey("b"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("b"), + EachValue: cty.StringVal("boop"), + }, }, }, }, @@ -300,19 +407,37 @@ func TestInstancesMap(t *testing.T) { cty.StringVal("beep"), cty.StringVal("boop"), }), - map[addrs.InstanceKey]InstanceObj{ - addrs.StringKey("beep"): { - Key: addrs.StringKey("beep"), - Rep: instances.RepetitionData{ - EachKey: cty.StringVal("beep"), - EachValue: cty.StringVal("beep"), + Expectation{ + UnknownForEachSupported: map[addrs.InstanceKey]InstanceObj{ + addrs.StringKey("beep"): { + Key: addrs.StringKey("beep"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("beep"), + EachValue: cty.StringVal("beep"), + }, + }, + addrs.StringKey("boop"): { + Key: addrs.StringKey("boop"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("boop"), + EachValue: cty.StringVal("boop"), + }, }, }, - addrs.StringKey("boop"): { - Key: addrs.StringKey("boop"), - Rep: instances.RepetitionData{ - EachKey: cty.StringVal("boop"), - EachValue: cty.StringVal("boop"), + UnknownForEachUnsupported: map[addrs.InstanceKey]InstanceObj{ + addrs.StringKey("beep"): { + Key: addrs.StringKey("beep"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("beep"), + EachValue: cty.StringVal("beep"), + }, + }, + addrs.StringKey("boop"): { + Key: addrs.StringKey("boop"), + Rep: instances.RepetitionData{ + EachKey: cty.StringVal("boop"), + EachValue: cty.StringVal("boop"), + }, }, }, }, @@ -321,11 +446,20 @@ func TestInstancesMap(t *testing.T) { for _, test := range tests { t.Run(fmt.Sprintf("%s", test.Input), func(t *testing.T) { - got := instancesMap(test.Input, makeObj) + t.Run("unknown for_each supported", func(t *testing.T) { + got := instancesMap(test.Input, makeObj, true) - if diff := cmp.Diff(test.Want, got, ctydebug.CmpOptions); diff != "" { - t.Errorf("wrong result\ninput: %#v\n%s", test.Input, diff) - } + if diff := cmp.Diff(test.Want.UnknownForEachSupported, got, ctydebug.CmpOptions); diff != "" { + t.Errorf("wrong result\ninput: %#v\n%s", test.Input, diff) + } + }) + t.Run("unknown for_each unsupported", func(t *testing.T) { + got := instancesMap(test.Input, makeObj, false) + + if diff := cmp.Diff(test.Want.UnknownForEachUnsupported, got, ctydebug.CmpOptions); diff != "" { + t.Errorf("wrong result\ninput: %#v\n%s", test.Input, diff) + } + }) }) } } diff --git a/internal/stacks/stackruntime/internal/stackeval/provider.go b/internal/stacks/stackruntime/internal/stackeval/provider.go index b3d83c244f5f..c5c9990829f0 100644 --- a/internal/stacks/stackruntime/internal/stackeval/provider.go +++ b/internal/stacks/stackruntime/internal/stackeval/provider.go @@ -181,7 +181,7 @@ func (p *Provider) CheckInstances(ctx context.Context, phase EvalPhase) (map[add forEachVal := p.ForEachValue(ctx, phase) return instancesMap(forEachVal, func(ik addrs.InstanceKey, rd instances.RepetitionData) *ProviderInstance { return newProviderInstance(p, ik, rd) - }), diags + }, false), diags }, ) } diff --git a/internal/stacks/stackruntime/internal/stackeval/stack_call.go b/internal/stacks/stackruntime/internal/stackeval/stack_call.go index 9a384b05b526..49f36ff151f0 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack_call.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack_call.go @@ -167,7 +167,7 @@ func (c *StackCall) CheckInstances(ctx context.Context, phase EvalPhase) (map[ad return instancesMap(forEachVal, func(ik addrs.InstanceKey, rd instances.RepetitionData) *StackCallInstance { return newStackCallInstance(c, ik, rd) - }), diags + }, false), diags }, ) }