diff --git a/internal/stacks/stackruntime/internal/stackeval/component.go b/internal/stacks/stackruntime/internal/stackeval/component.go index e5c2dd933677..7906ba71e898 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: @@ -170,11 +159,16 @@ func (c *Component) CheckInstances(ctx context.Context, phase EvalPhase) (map[ad ctx, c.instances.For(phase), c.main, func(ctx context.Context) (map[addrs.InstanceKey]*ComponentInstance, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics - forEachVal := c.ForEachValue(ctx, phase) + forEachVal, forEachValueDiags := c.CheckForEachValue(ctx, phase) + + diags = diags.Append(forEachValueDiags) + if diags.HasErrors() { + return nil, diags + } 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 { @@ -257,6 +251,12 @@ func (c *Component) PlanIsComplete(ctx context.Context) bool { return false } + 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 false + } + for _, inst := range insts { plan := inst.ModuleTreePlan(ctx) if plan == nil { @@ -266,6 +266,7 @@ func (c *Component) PlanIsComplete(ctx context.Context) bool { // get returned by a different return path. return false } + if !plan.Complete { return false } @@ -283,9 +284,7 @@ func (c *Component) ExprReferenceValue(ctx context.Context, phase EvalPhase) cty func (c *Component) checkValid(ctx context.Context, phase EvalPhase) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - _, moreDiags := c.CheckForEachValue(ctx, phase) - diags = diags.Append(moreDiags) - _, moreDiags = c.CheckInstances(ctx, phase) + _, moreDiags := c.CheckInstances(ctx, phase) diags = diags.Append(moreDiags) return diags diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 6d1d3ef8e530..b4eba207ba56 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -558,24 +558,29 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla // If any of our upstream components have incomplete plans then // we need to force treating everything in this component as // deferred so we can preserve the correct dependency ordering. - upstreamDeferred := false + deferred := false for _, depAddr := range c.call.RequiredComponents(ctx).Elems() { depStack := c.main.Stack(ctx, depAddr.Stack, PlanPhase) if depStack == nil { - upstreamDeferred = true // to be conservative + deferred = true // to be conservative break } depComponent := depStack.Component(ctx, depAddr.Item) if depComponent == nil { - upstreamDeferred = true // to be conservative + deferred = true // to be conservative break } if !depComponent.PlanIsComplete(ctx) { - upstreamDeferred = true + deferred = true break } } + // The instance is also upstream deferred if the for_each value for this instance is unknown. + if c.key == addrs.WildcardKey { + deferred = true + } + // NOTE: This ComponentInstance type only deals with component // instances currently declared in the configuration. See // [ComponentInstanceRemoved] for the model of a component instance @@ -586,7 +591,7 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla SetVariables: inputValues, ExternalProviders: providerClients, DeferralAllowed: stackPlanOpts.DeferralAllowed, - ExternalDependencyDeferred: upstreamDeferred, + ExternalDependencyDeferred: deferred, // This is set by some tests but should not be used in main code. // (nil means to use the real time when tfCtx.Plan was called.) diff --git a/internal/stacks/stackruntime/internal/stackeval/component_test.go b/internal/stacks/stackruntime/internal/stackeval/component_test.go index 0f5cd2769b65..754dfc5d9de0 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_test.go @@ -190,15 +190,17 @@ 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 nil and diagnostics. gotInsts, diags := component.CheckInstances(ctx, InspectPhase) - assertNoDiags(t, diags) + if gotInsts != nil { - t.Errorf("wrong instances; want nil\n%#v", gotInsts) + t.Fatalf("unexpected instances\ngot: %#v\nwant: nil", gotInsts) } + + assertMatchingDiag(t, diags, func(diag tfdiags.Diagnostic) bool { + return (diag.Severity() == tfdiags.Error && + diag.Description().Detail == "The for_each expression must produce either a map of any type or a set of strings. The keys of the map or the set elements will serve as unique identifiers for multiple instances of this component.") + }) }) subtestInPromisingTask(t, "unknown", func(ctx context.Context, t *testing.T) { main := testEvaluator(t, testEvaluatorOpts{ @@ -208,30 +210,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") } }) }) @@ -388,16 +393,13 @@ func TestComponentResultValue(t *testing.T) { component := getComponent(ctx, t, main) got := component.ResultValue(ctx, InspectPhase) // When the for_each expression is unknown, the result value - // is unknown too so we can use it as a placeholder for partial - // downstream checking. - want := cty.DynamicVal - // FIXME: the cmp transformer ctydebug.CmpOptions seems to find - // this particular pair of values troubling, causing it to get - // into an infinite recursion. For now we'll just use RawEquals, - // at the expense of a less helpful failure message. This seems - // to be a bug in upstream ctydebug. - if !want.RawEquals(got) { - t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, want) + // is an instance map with the wildcard key and an empty object + want := cty.ObjectVal(map[string]cty.Value{ + "*": cty.EmptyObjectVal, + }) + + if diff := cmp.Diff(want, got, ctydebug.CmpOptions); diff != "" { + t.Fatalf("wrong result\n%s", diff) } }) }) diff --git a/internal/stacks/stackruntime/internal/stackeval/for_each.go b/internal/stacks/stackruntime/internal/stackeval/for_each.go index 5c717e3eab7a..db410c5f31ce 100644 --- a/internal/stacks/stackruntime/internal/stackeval/for_each.go +++ b/internal/stacks/stackruntime/internal/stackeval/for_each.go @@ -69,22 +69,18 @@ func evaluateForEachExpr(ctx context.Context, expr hcl.Expression, phase EvalPha case ty.IsObjectType() || ty.IsMapType(): // okay - case !result.Value.IsKnown(): - // we can't validate further without knowing the value - return result, diags - case ty.IsSetType(): - if markSafeLengthInt(result.Value) == 0 { - // we are okay with an empty set - return result, diags - } - // since we can't use a set values that are unknown, we treat the // entire set as unknown if !result.Value.IsWhollyKnown() { return result, diags } + if markSafeLengthInt(result.Value) == 0 { + // we are okay with an empty set + return result, diags + } + if !ty.ElementType().Equals(cty.String) { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, @@ -111,6 +107,12 @@ func evaluateForEachExpr(ctx context.Context, expr hcl.Expression, phase EvalPha } } + case !result.Value.IsWhollyKnown() && ty.HasDynamicTypes(): + // If the value is unknown and has dynamic types, we can't + // determine if it's a valid for_each value, so we'll just + // return the unknown value. + return result, diags + default: diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, @@ -145,19 +147,20 @@ 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, allowsUnknown bool) map[addrs.InstanceKey]T { switch { - case maybeForEachVal == cty.NilVal: // No for_each expression at all, then. We have exactly one instance // without an instance key and with no repetition data. 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 allowsUnknown { + return unknownForEachInstancesMap(maybeForEachVal.Type(), makeInst) + } else { + return nil + } default: // Otherwise we should be able to assume the value is valid per the @@ -239,6 +242,12 @@ func noForEachInstancesMap[T any](makeInst func(addrs.InstanceKey, instances.Rep } } +func unknownForEachInstancesMap[T any](ty cty.Type, makeInst func(addrs.InstanceKey, instances.RepetitionData) T) map[addrs.InstanceKey]T { + return map[addrs.InstanceKey]T{ + addrs.WildcardKey: makeInst(addrs.WildcardKey, instances.UnknownForEachRepetitionData(ty)), + } +} + // 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..d3d93c3cf831 100644 --- a/internal/stacks/stackruntime/internal/stackeval/for_each_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/for_each_test.go @@ -5,7 +5,6 @@ package stackeval import ( "context" - "fmt" "testing" "github.com/davecgh/go-spew/spew" @@ -41,10 +40,6 @@ func TestEvaluateForEachExpr(t *testing.T) { "b": cty.StringVal("beep"), }), }, - "unknown object": { - Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.EmptyObject)), - Want: cty.UnknownVal(cty.EmptyObject), - }, // Maps "map of string": { @@ -137,6 +132,24 @@ func TestEvaluateForEachExpr(t *testing.T) { Expr: hcltest.MockExprLiteral(cty.NullVal(cty.String)), WantErr: `Invalid for_each value`, }, + + // Unknown sets, maps, objects, and dynamic types are allowed + "unknown set": { + Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))), + Want: cty.UnknownVal(cty.Set(cty.String)), + }, + "unknown map": { + Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.String))), + Want: cty.UnknownVal(cty.Map(cty.String)), + }, + "unknown object": { + Expr: hcltest.MockExprLiteral(cty.UnknownVal(cty.EmptyObject)), + Want: cty.UnknownVal(cty.EmptyObject), + }, + "unknown dynamic type": { + Expr: hcltest.MockExprLiteral(cty.DynamicVal), + Want: cty.DynamicVal, + }, } ctx := context.Background() @@ -178,10 +191,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, @@ -190,8 +210,9 @@ func TestInstancesMap(t *testing.T) { } tests := []struct { + Name string 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. @@ -199,12 +220,23 @@ func TestInstancesMap(t *testing.T) { }{ // No for_each at all { + "nil", 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 + }, }, }, }, @@ -212,107 +244,224 @@ func TestInstancesMap(t *testing.T) { // Unknowns { + "unknown empty object", 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.UnknownVal(cty.String), + EachValue: cty.DynamicVal, + }, + }, + }, + UnknownForEachUnsupported: nil, // a nil map means "unknown" for this function + }, }, { + "unknown bool map", 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.UnknownVal(cty.String), + EachValue: cty.UnknownVal(cty.Bool), + }, + }, + }, + UnknownForEachUnsupported: nil, // a nil map means "unknown" for this function + }, }, { + "unknown set of strings", 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.UnknownVal(cty.String), + EachValue: cty.UnknownVal(cty.String), + }, + }, + }, + UnknownForEachUnsupported: nil, // a nil map means "unknown" for this function + }, }, // Empties { + "empty object", 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. + }, }, }, { + "empty string map", 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. + }, }, }, { + "empty string set", 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. + }, }, }, // Known and not empty { + "object", cty.ObjectVal(map[string]cty.Value{ "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"), + }, }, }, }, }, { + "map", cty.MapVal(map[string]cty.Value{ "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"), + }, }, }, }, }, { + "set", cty.SetVal([]cty.Value{ 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"), + }, }, }, }, @@ -320,12 +469,21 @@ 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(test.Name, func(t *testing.T) { + 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 }, ) } diff --git a/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go b/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go index 7d6b9188e8c6..92ed14b189e4 100644 --- a/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go +++ b/internal/stacks/stackruntime/internal/stackeval/walk_dynamic.go @@ -70,10 +70,8 @@ func walkDynamicObjectsInStack[Output any]( // We also need to visit and check all of the other declarations in // the current stack. - for _, component := range stack.Components(ctx) { component := component // separate symbol per loop iteration - visit(ctx, walk, component) // We need to perform the instance expansion in an overall async task diff --git a/internal/stacks/stackruntime/plan_test.go b/internal/stacks/stackruntime/plan_test.go index f7179a49b106..ad5bf7c87b54 100644 --- a/internal/stacks/stackruntime/plan_test.go +++ b/internal/stacks/stackruntime/plan_test.go @@ -1574,6 +1574,191 @@ func TestPlanWithCheckableObjects(t *testing.T) { } } +func TestPlanWithDeferredComponentForEach(t *testing.T) { + ctx := context.Background() + cfg := loadMainBundleConfigForTest(t, path.Join("with-single-input", "deferred-component-for-each")) + + fakePlanTimestamp, err := time.Parse(time.RFC3339, "1991-08-25T20:57:08Z") + if err != nil { + t.Fatal(err) + } + + changesCh := make(chan stackplan.PlannedChange, 8) + diagsCh := make(chan tfdiags.Diagnostic, 2) + req := PlanRequest{ + Config: cfg, + ProviderFactories: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("testing"): func() (providers.Interface, error) { + return stacks_testing_provider.NewProvider(), nil + }, + }, + ForcePlanTimestamp: &fakePlanTimestamp, + InputValues: map[stackaddrs.InputVariable]ExternalInputValue{ + {Name: "components"}: { + Value: cty.UnknownVal(cty.Set(cty.String)), + DefRange: tfdiags.SourceRange{}, + }, + }, + } + resp := PlanResponse{ + PlannedChanges: changesCh, + Diagnostics: diagsCh, + } + go Plan(ctx, &req, &resp) + gotChanges, diags := collectPlanOutput(changesCh, diagsCh) + + reportDiagnosticsForTest(t, diags) + if len(diags) != 0 { + t.FailNow() // We reported the diags above/ + } + + sort.SliceStable(gotChanges, func(i, j int) bool { + return plannedChangeSortKey(gotChanges[i]) < plannedChangeSortKey(gotChanges[j]) + }) + + wantChanges := []stackplan.PlannedChange{ + &stackplan.PlannedChangeApplyable{ + Applyable: true, + }, + &stackplan.PlannedChangeComponentInstance{ + Addr: stackaddrs.Absolute( + stackaddrs.RootStackInstance, + stackaddrs.ComponentInstance{ + Component: stackaddrs.Component{Name: "self"}, + Key: addrs.WildcardKey, + }, + ), + PlanApplyable: true, + PlanComplete: true, + Action: plans.Create, + PlannedInputValues: map[string]plans.DynamicValue{ + "id": mustPlanDynamicValueDynamicType(cty.NullVal(cty.String)), + "input": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)), + }, + PlannedOutputValues: map[string]cty.Value{}, + PlannedCheckResults: &states.CheckResults{}, + PlanTimestamp: fakePlanTimestamp, + PlannedInputValueMarks: map[string][]cty.PathValueMarks{ + "id": nil, + "input": nil, + }, + }, + &stackplan.PlannedChangeResourceInstancePlanned{ + ResourceInstanceObjectAddr: stackaddrs.AbsResourceInstanceObject{ + Component: stackaddrs.Absolute( + stackaddrs.RootStackInstance, + stackaddrs.ComponentInstance{ + Component: stackaddrs.Component{Name: "self"}, + Key: addrs.WildcardKey, + }, + ), + Item: addrs.AbsResourceInstanceObject{ + ResourceInstance: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "testing_resource", + Name: "data", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + }, + }, + ProviderConfigAddr: addrs.AbsProviderConfig{ + Module: addrs.RootModule, + Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), + }, + ChangeSrc: &plans.ResourceInstanceChangeSrc{ + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "testing_resource", + Name: "data", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + PrevRunAddr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "testing_resource", + Name: "data", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ProviderAddr: addrs.AbsProviderConfig{ + Module: addrs.RootModule, + Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), + }, + ChangeSrc: plans.ChangeSrc{ + Action: plans.Create, + Before: mustPlanDynamicValue(cty.NullVal(cty.DynamicPseudoType)), + After: mustPlanDynamicValueSchema(cty.ObjectVal(map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "value": cty.UnknownVal(cty.String), + }), stacks_testing_provider.TestingResourceSchema), + AfterSensitivePaths: nil, + }, + }, + Schema: stacks_testing_provider.TestingResourceSchema, + }, + &stackplan.PlannedChangeHeader{ + TerraformVersion: version.SemVer, + }, + &stackplan.PlannedChangeRootInputValue{ + Addr: stackaddrs.InputVariable{Name: "components"}, + Value: cty.UnknownVal(cty.Set(cty.String)), + }, + } + + if diff := cmp.Diff(wantChanges, gotChanges, ctydebug.CmpOptions, cmpCollectionsSet); diff != "" { + t.Errorf("wrong changes\n%s", diff) + } +} + +func TestPlanWithDeferredComponentForEachOfInvalidType(t *testing.T) { + ctx := context.Background() + cfg := loadMainBundleConfigForTest(t, "deferred-component-for-each-from-component-of-invalid-type") + + fakePlanTimestamp, err := time.Parse(time.RFC3339, "1991-08-25T20:57:08Z") + if err != nil { + t.Fatal(err) + } + + changesCh := make(chan stackplan.PlannedChange, 8) + diagsCh := make(chan tfdiags.Diagnostic, 2) + req := PlanRequest{ + Config: cfg, + ProviderFactories: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("testing"): func() (providers.Interface, error) { + return stacks_testing_provider.NewProvider(), nil + }, + }, + + ForcePlanTimestamp: &fakePlanTimestamp, + + InputValues: map[stackaddrs.InputVariable]ExternalInputValue{ + {Name: "components"}: { + Value: cty.UnknownVal(cty.Set(cty.String)), + DefRange: tfdiags.SourceRange{}, + }, + }, + } + resp := PlanResponse{ + PlannedChanges: changesCh, + Diagnostics: diagsCh, + } + go Plan(ctx, &req, &resp) + _, diags := collectPlanOutput(changesCh, diagsCh) + + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %d: %s", len(diags), diags) + } + + if diags[0].Severity() != tfdiags.Error { + t.Errorf("expected error diagnostic, got %q", diags[0].Severity()) + } + + expectedSummary := "Invalid for_each value" + if diags[0].Description().Summary != expectedSummary { + t.Errorf("expected diagnostic with summary %q, got %q", expectedSummary, diags[0].Description().Summary) + } + + expectedDetail := "The for_each expression must produce either a map of any type or a set of strings. The keys of the map or the set elements will serve as unique identifiers for multiple instances of this component." + if diags[0].Description().Detail != expectedDetail { + t.Errorf("expected diagnostic with detail %q, got %q", expectedDetail, diags[0].Description().Detail) + } +} + // collectPlanOutput consumes the two output channels emitting results from // a call to [Plan], and collects all of the data written to them before // returning once changesCh has been closed by the sender to indicate that diff --git "a/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/\020deferred-component-for-each-from-component-of-invalid-type.tfstack.hcl" "b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/\020deferred-component-for-each-from-component-of-invalid-type.tfstack.hcl" new file mode 100644 index 000000000000..4ace97c0115c --- /dev/null +++ "b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/\020deferred-component-for-each-from-component-of-invalid-type.tfstack.hcl" @@ -0,0 +1,37 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + + +provider "testing" "default" {} + + +component "parent" { + source = "./parent" + + providers = { + testing = provider.testing.default + } + + inputs = { + input = "parent" + } +} + + +component "self" { + source = "./self" + + providers = { + testing = provider.testing.default + } + + inputs = { + input = each.value + } + + for_each = component.parent.letters_in_id // This is a list and no set or map +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/parent/parent.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/parent/parent.tf new file mode 100644 index 000000000000..006a3818fec6 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/parent/parent.tf @@ -0,0 +1,27 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +variable "id" { + type = string + default = null + nullable = true # We'll generate an ID if none provided. +} + +variable "input" { + type = string +} + +resource "testing_resource" "data" { + id = var.id + value = var.input +} + +output "letters_in_id" { + value = split("", testing_resource.data.id) +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/self/self.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/self/self.tf new file mode 100644 index 000000000000..4da49727a5b0 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/self/self.tf @@ -0,0 +1,23 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +variable "id" { + type = string + default = null + nullable = true # We'll generate an ID if none provided. +} + +variable "input" { + type = string +} + +resource "testing_resource" "data" { + id = var.id + value = var.input +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/deferred-component-for-each/deferred-component-for-each.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/deferred-component-for-each/deferred-component-for-each.tfstack.hcl new file mode 100644 index 000000000000..be31797b6f6f --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-input/deferred-component-for-each/deferred-component-for-each.tfstack.hcl @@ -0,0 +1,26 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +variable "components" { + type = set(string) +} + +provider "testing" "default" {} + +component "self" { + source = "../" + + providers = { + testing = provider.testing.default + } + + inputs = { + input = each.value + } + + for_each = var.components +}