From 8ab7af8c5f9199aa1d4e3b70a8f90b51bdb90d63 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 31 Mar 2023 14:01:15 -0400 Subject: [PATCH] validate unknown nested attribute collections It is not valid for a provider to return an unknown value for a configured nested collection, but we need to check for unknowns before comparing the number of values in the collection. --- internal/plans/objchange/plan_valid.go | 44 ++++++-- internal/plans/objchange/plan_valid_test.go | 109 ++++++++++++++++++++ 2 files changed, 145 insertions(+), 8 deletions(-) diff --git a/internal/plans/objchange/plan_valid.go b/internal/plans/objchange/plan_valid.go index 305bc10b18..c772c32fc7 100644 --- a/internal/plans/objchange/plan_valid.go +++ b/internal/plans/objchange/plan_valid.go @@ -355,10 +355,17 @@ func assertPlannedObjectValid(schema *configschema.Object, prior, config, planne // both support a similar-enough API that we can treat them the // same for our purposes here. - plannedL := planned.LengthInt() - configL := config.LengthInt() - if plannedL != configL { - errs = append(errs, path.NewErrorf("count in plan (%d) disagrees with count in config (%d)", plannedL, configL)) + plannedL := planned.Length() + configL := config.Length() + + // config wasn't known, then planned should be unknown too + if !plannedL.IsKnown() && !configL.IsKnown() { + return errs + } + + lenEqual := plannedL.Equals(configL) + if !lenEqual.IsKnown() || lenEqual.False() { + errs = append(errs, path.NewErrorf("count in plan (%#v) disagrees with count in config (%#v)", plannedL, configL)) return errs } for it := planned.ElementIterator(); it.Next(); { @@ -385,6 +392,20 @@ func assertPlannedObjectValid(schema *configschema.Object, prior, config, planne configVals := map[string]cty.Value{} priorVals := map[string]cty.Value{} + plannedL := planned.Length() + configL := config.Length() + + // config wasn't known, then planned should be unknown too + if !plannedL.IsKnown() && !configL.IsKnown() { + return errs + } + + lenEqual := plannedL.Equals(configL) + if !lenEqual.IsKnown() || lenEqual.False() { + errs = append(errs, path.NewErrorf("count in plan (%#v) disagrees with count in config (%#v)", plannedL, configL)) + return errs + } + if !planned.IsNull() { plannedVals = planned.AsValueMap() } @@ -418,10 +439,17 @@ func assertPlannedObjectValid(schema *configschema.Object, prior, config, planne } case configschema.NestingSet: - plannedL := planned.LengthInt() - configL := config.LengthInt() - if plannedL != configL { - errs = append(errs, path.NewErrorf("count in plan (%d) disagrees with count in config (%d)", plannedL, configL)) + plannedL := planned.Length() + configL := config.Length() + + // config wasn't known, then planned should be unknown too + if !plannedL.IsKnown() && !configL.IsKnown() { + return errs + } + + lenEqual := plannedL.Equals(configL) + if !lenEqual.IsKnown() || lenEqual.False() { + errs = append(errs, path.NewErrorf("count in plan (%#v) disagrees with count in config (%#v)", plannedL, configL)) return errs } // Because set elements have no identifier with which to correlate diff --git a/internal/plans/objchange/plan_valid_test.go b/internal/plans/objchange/plan_valid_test.go index 00a1602ac2..ab95e0c391 100644 --- a/internal/plans/objchange/plan_valid_test.go +++ b/internal/plans/objchange/plan_valid_test.go @@ -1689,6 +1689,115 @@ func TestAssertPlanValid(t *testing.T) { }), nil, }, + + // When validating collections we start by comparing length, which + // requires guarding for any unknown values incorrectly returned by the + // provider. + "nested collection attrs planned unknown": { + &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "set": { + Computed: true, + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSet, + Attributes: map[string]*configschema.Attribute{ + "name": { + Type: cty.String, + Computed: true, + Optional: true, + }, + }, + }, + }, + "list": { + Computed: true, + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingList, + Attributes: map[string]*configschema.Attribute{ + "name": { + Type: cty.String, + Computed: true, + Optional: true, + }, + }, + }, + }, + "map": { + Computed: true, + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingMap, + Attributes: map[string]*configschema.Attribute{ + "name": { + Type: cty.String, + Computed: true, + Optional: true, + }, + }, + }, + }, + }, + }, + cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("from_config"), + }), + }), + "list": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("from_config"), + }), + }), + "map": cty.MapVal(map[string]cty.Value{ + "key": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("from_config"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("from_config"), + }), + }), + "list": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("from_config"), + }), + }), + "map": cty.MapVal(map[string]cty.Value{ + "key": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("from_config"), + }), + }), + }), + // provider cannot override the config + cty.ObjectVal(map[string]cty.Value{ + "set": cty.UnknownVal(cty.Set( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + )), + "list": cty.UnknownVal(cty.Set( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + )), + "map": cty.UnknownVal(cty.Map( + cty.Object(map[string]cty.Type{ + "name": cty.String, + }), + )), + }), + []string{ + `.set: count in plan (cty.UnknownVal(cty.Number)) disagrees with count in config (cty.NumberIntVal(1))`, + `.list: count in plan (cty.UnknownVal(cty.Number)) disagrees with count in config (cty.NumberIntVal(1))`, + `.map: count in plan (cty.UnknownVal(cty.Number)) disagrees with count in config (cty.NumberIntVal(1))`, + }, + }, } for name, test := range tests {