From 3c8a1635839db20fc1f097d1b1f69b50ec83ca7c Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 15 Jun 2023 09:29:42 -0400 Subject: [PATCH] set length is unknown with partially known elems If a set contains partially known values the length is unknown which causes assertPlannedObjectValid to fail valid plans. Revert to the old method if using LengthInt for the set lengths, which returns the maximum number of possible elements, with a guard for entirely unknown set values. --- internal/plans/objchange/plan_valid.go | 17 ++--- internal/plans/objchange/plan_valid_test.go | 70 +++++++++++++++++++-- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/internal/plans/objchange/plan_valid.go b/internal/plans/objchange/plan_valid.go index ad7c051783..c20241267b 100644 --- a/internal/plans/objchange/plan_valid.go +++ b/internal/plans/objchange/plan_valid.go @@ -342,6 +342,11 @@ func assertPlannedObjectValid(schema *configschema.Object, prior, config, planne errs = append(errs, path.NewErrorf("planned for existence but config wants absence")) return errs } + if !config.IsNull() && !planned.IsKnown() { + errs = append(errs, path.NewErrorf("planned unknown for configured value")) + return errs + } + if planned.IsNull() { // No further checks possible if the planned value is null return errs @@ -442,16 +447,14 @@ func assertPlannedObjectValid(schema *configschema.Object, prior, config, planne } case configschema.NestingSet: - plannedL := planned.Length() - configL := config.Length() - - // config wasn't known, then planned should be unknown too - if !plannedL.IsKnown() && !configL.IsKnown() { + if !planned.IsKnown() || !config.IsKnown() { + // if either is unknown we cannot check the lengths return errs } - lenEqual := plannedL.Equals(configL) - if !lenEqual.IsKnown() || lenEqual.False() { + plannedL := planned.LengthInt() + configL := config.LengthInt() + if plannedL != configL { errs = append(errs, path.NewErrorf("count in plan (%#v) disagrees with count in config (%#v)", plannedL, configL)) return errs } diff --git a/internal/plans/objchange/plan_valid_test.go b/internal/plans/objchange/plan_valid_test.go index 655c6e22f7..56fb4cfa81 100644 --- a/internal/plans/objchange/plan_valid_test.go +++ b/internal/plans/objchange/plan_valid_test.go @@ -1700,7 +1700,7 @@ func TestAssertPlanValid(t *testing.T) { &configschema.Block{ Attributes: map[string]*configschema.Attribute{ "set": { - Computed: true, + //Computed: true, Optional: true, NestedType: &configschema.Object{ Nesting: configschema.NestingSet, @@ -1796,9 +1796,9 @@ func TestAssertPlanValid(t *testing.T) { )), }), []string{ - `.set: count in plan (cty.UnknownVal(cty.Number).Refine().NotNull().NumberLowerBound(cty.NumberIntVal(0), true).NumberUpperBound(cty.NumberIntVal(9.223372036854775807e+18), true).NewValue()) disagrees with count in config (cty.NumberIntVal(1))`, - `.list: count in plan (cty.UnknownVal(cty.Number).Refine().NotNull().NumberLowerBound(cty.NumberIntVal(0), true).NumberUpperBound(cty.NumberIntVal(9.223372036854775807e+18), true).NewValue()) disagrees with count in config (cty.NumberIntVal(1))`, - `.map: count in plan (cty.UnknownVal(cty.Number).Refine().NotNull().NumberLowerBound(cty.NumberIntVal(0), true).NumberUpperBound(cty.NumberIntVal(9.223372036854775807e+18), true).NewValue()) disagrees with count in config (cty.NumberIntVal(1))`, + `.set: planned unknown for configured value`, + `.list: planned unknown for configured value`, + `.map: planned unknown for configured value`, }, }, @@ -1829,6 +1829,68 @@ func TestAssertPlanValid(t *testing.T) { }), nil, }, + + "nested set values can contain computed unknown": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "set": { + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSet, + Attributes: map[string]*configschema.Attribute{ + "input": { + Type: cty.String, + Optional: true, + }, + "computed": { + Type: cty.String, + Computed: true, + Optional: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "input": cty.StringVal("a"), + "computed": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "input": cty.StringVal("b"), + "computed": cty.NullVal(cty.String), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "input": cty.StringVal("a"), + "computed": cty.NullVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "input": cty.StringVal("b"), + "computed": cty.NullVal(cty.String), + }), + }), + }), + // Plan can mark the null computed values as unknown + cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "input": cty.StringVal("a"), + "computed": cty.UnknownVal(cty.String), + }), + cty.ObjectVal(map[string]cty.Value{ + "input": cty.StringVal("b"), + "computed": cty.UnknownVal(cty.String), + }), + }), + }), + []string{}, + }, } for name, test := range tests {