Skip to content

Commit 0462b84

Browse files
jbardinjbardin
committed
better determine when to plan optional+computed
We can check if an object in state must have at least partially come from configuration, by seeing if the prior value has any non-null attributes which are not computed in the schema. This is used when the configuration contains a null optional+computed value, and we want to know if we should plan to send the null value or the prior state. Simplify the proposedNewAttributes cases, and add another test for coverage.
1 parent e16b848 commit 0462b84

File tree

2 files changed

+225
-58
lines changed

2 files changed

+225
-58
lines changed

‎internal/plans/objchange/objchange.go‎

Lines changed: 101 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,14 @@ func proposedNewNestedBlock(schema *configschema.NestedBlock, prior, config cty.
275275
return newV
276276
}
277277

278+
func proposedNewObjectAttributes(attrs map[string]*configschema.Attribute, prior, config cty.Value) cty.Value {
279+
if config.IsNull() {
280+
return config
281+
}
282+
283+
return cty.ObjectVal(proposedNewAttributes(attrs, prior, config))
284+
}
285+
278286
func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, config cty.Value) map[string]cty.Value {
279287
newAttrs := make(map[string]cty.Value, len(attrs))
280288
for name, attr := range attrs {
@@ -297,6 +305,14 @@ func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, conf
297305
// configV will always be null in this case, by definition.
298306
// priorV may also be null, but that's okay.
299307
newV = priorV
308+
309+
// the exception to the above is that if the config is optional and
310+
// the _prior_ value contains non-computed values, we can infer
311+
// that the config must have been non-null previously.
312+
if optionalValueNotComputable(attr, priorV) {
313+
newV = configV
314+
}
315+
300316
case attr.NestedType != nil:
301317
// For non-computed NestedType attributes, we need to descend
302318
// into the individual nested attributes to build the final
@@ -316,7 +332,7 @@ func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, conf
316332

317333
func proposedNewNestedType(schema *configschema.Object, prior, config cty.Value) cty.Value {
318334
// if the config isn't known at all, then we must use that value
319-
if !config.IsNull() && !config.IsKnown() {
335+
if !config.IsKnown() {
320336
return config
321337
}
322338

@@ -332,7 +348,7 @@ func proposedNewNestedType(schema *configschema.Object, prior, config cty.Value)
332348
break
333349
}
334350

335-
newV = cty.ObjectVal(proposedNewAttributes(schema.Attributes, prior, config))
351+
newV = proposedNewObjectAttributes(schema.Attributes, prior, config)
336352

337353
case configschema.NestingList:
338354
// Nested blocks are correlated by index.
@@ -353,8 +369,8 @@ func proposedNewNestedType(schema *configschema.Object, prior, config cty.Value)
353369
}
354370
priorEV := prior.Index(idx)
355371

356-
newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
357-
newVals = append(newVals, cty.ObjectVal(newEV))
372+
newEV := proposedNewObjectAttributes(schema.Attributes, priorEV, configEV)
373+
newVals = append(newVals, newEV)
358374
}
359375
// Despite the name, a NestingList might also be a tuple, if
360376
// its nested schema contains dynamically-typed attributes.
@@ -366,58 +382,55 @@ func proposedNewNestedType(schema *configschema.Object, prior, config cty.Value)
366382
}
367383

368384
case configschema.NestingMap:
385+
configVLen := 0
386+
if config.IsKnown() && !config.IsNull() {
387+
configVLen = config.LengthInt()
388+
}
389+
390+
if configVLen == 0 {
391+
break
392+
}
393+
394+
newVals := make(map[string]cty.Value, configVLen)
395+
369396
// Despite the name, a NestingMap may produce either a map or
370397
// object value, depending on whether the nested schema contains
371398
// dynamically-typed attributes.
372399
if config.Type().IsObjectType() {
373400
// Nested blocks are correlated by key.
374-
configVLen := 0
375-
if config.IsKnown() && !config.IsNull() {
376-
configVLen = config.LengthInt()
377-
}
378-
if configVLen > 0 {
379-
newVals := make(map[string]cty.Value, configVLen)
380-
atys := config.Type().AttributeTypes()
381-
for name := range atys {
382-
configEV := config.GetAttr(name)
383-
if !prior.IsKnown() || prior.IsNull() || !prior.Type().HasAttribute(name) {
384-
// If there is no corresponding prior element then
385-
// we just take the config value as-is.
386-
newVals[name] = configEV
387-
continue
388-
}
389-
priorEV := prior.GetAttr(name)
390-
newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
391-
newVals[name] = cty.ObjectVal(newEV)
401+
atys := config.Type().AttributeTypes()
402+
for name := range atys {
403+
configEV := config.GetAttr(name)
404+
if !prior.IsKnown() || prior.IsNull() || !prior.Type().HasAttribute(name) {
405+
// If there is no corresponding prior element then
406+
// we just take the config value as-is.
407+
newVals[name] = configEV
408+
continue
392409
}
393-
// Although we call the nesting mode "map", we actually use
394-
// object values so that elements might have different types
395-
// in case of dynamically-typed attributes.
396-
newV = cty.ObjectVal(newVals)
410+
priorEV := prior.GetAttr(name)
411+
newEV := proposedNewObjectAttributes(schema.Attributes, priorEV, configEV)
412+
newVals[name] = newEV
397413
}
414+
// Although we call the nesting mode "map", we actually use
415+
// object values so that elements might have different types
416+
// in case of dynamically-typed attributes.
417+
newV = cty.ObjectVal(newVals)
398418
} else {
399-
configVLen := 0
400-
if config.IsKnown() && !config.IsNull() {
401-
configVLen = config.LengthInt()
402-
}
403-
if configVLen > 0 {
404-
newVals := make(map[string]cty.Value, configVLen)
405-
for it := config.ElementIterator(); it.Next(); {
406-
idx, configEV := it.Element()
407-
k := idx.AsString()
408-
if prior.IsKnown() && (prior.IsNull() || !prior.HasIndex(idx).True()) {
409-
// If there is no corresponding prior element then
410-
// we just take the config value as-is.
411-
newVals[k] = configEV
412-
continue
413-
}
414-
priorEV := prior.Index(idx)
415-
416-
newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
417-
newVals[k] = cty.ObjectVal(newEV)
419+
for it := config.ElementIterator(); it.Next(); {
420+
idx, configEV := it.Element()
421+
k := idx.AsString()
422+
if prior.IsKnown() && (prior.IsNull() || !prior.HasIndex(idx).True()) {
423+
// If there is no corresponding prior element then
424+
// we just take the config value as-is.
425+
newVals[k] = configEV
426+
continue
418427
}
419-
newV = cty.MapVal(newVals)
428+
priorEV := prior.Index(idx)
429+
430+
newEV := proposedNewObjectAttributes(schema.Attributes, priorEV, configEV)
431+
newVals[k] = newEV
420432
}
433+
newV = cty.MapVal(newVals)
421434
}
422435

423436
case configschema.NestingSet:
@@ -453,8 +466,8 @@ func proposedNewNestedType(schema *configschema.Object, prior, config cty.Value)
453466
if priorEV == cty.NilVal {
454467
newVals = append(newVals, configEV)
455468
} else {
456-
newEV := proposedNewAttributes(schema.Attributes, priorEV, configEV)
457-
newVals = append(newVals, cty.ObjectVal(newEV))
469+
newEV := proposedNewObjectAttributes(schema.Attributes, priorEV, configEV)
470+
newVals = append(newVals, newEV)
458471
}
459472
}
460473
newV = cty.SetVal(newVals)
@@ -518,3 +531,43 @@ func setElementComputedAsNull(schema attrPath, elem cty.Value) cty.Value {
518531

519532
return elem
520533
}
534+
535+
// optionalValueNotComputable is used to check if an object in state must
536+
// have at least partially come from configuration. If the prior value has any
537+
// non-null attributes which are not computed in the schema, then we know there
538+
// was previously a configuration value which set those.
539+
//
540+
// This is used when the configuration contains a null optional+computed value,
541+
// and we want to know if we should plan to send the null value or the prior
542+
// state.
543+
func optionalValueNotComputable(schema *configschema.Attribute, val cty.Value) bool {
544+
if !schema.Optional {
545+
return false
546+
}
547+
548+
// We must have a NestedType for complex nested attributes in order
549+
// to find nested computed values in the first place.
550+
if schema.NestedType == nil {
551+
return false
552+
}
553+
554+
foundNonComputedAttr := false
555+
cty.Walk(val, func(path cty.Path, v cty.Value) (bool, error) {
556+
if v.IsNull() {
557+
return true, nil
558+
}
559+
560+
attr := schema.NestedType.AttributeByPath(path)
561+
if attr == nil {
562+
return true, nil
563+
}
564+
565+
if !attr.Computed {
566+
foundNonComputedAttr = true
567+
return false, nil
568+
}
569+
return true, nil
570+
})
571+
572+
return foundNonComputedAttr
573+
}

‎internal/plans/objchange/objchange_test.go‎

Lines changed: 124 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -460,10 +460,10 @@ func TestProposedNew(t *testing.T) {
460460
})),
461461
}),
462462
cty.ObjectVal(map[string]cty.Value{
463-
"bloop": cty.ObjectVal(map[string]cty.Value{
464-
"blop": cty.StringVal("glub"),
465-
"bleep": cty.NullVal(cty.String),
466-
}),
463+
"bloop": cty.NullVal(cty.Object(map[string]cty.Type{
464+
"blop": cty.String,
465+
"bleep": cty.String,
466+
})),
467467
}),
468468
},
469469

@@ -752,6 +752,120 @@ func TestProposedNew(t *testing.T) {
752752
}),
753753
}),
754754
},
755+
756+
"prior optional computed nested map elem to null": {
757+
&configschema.Block{
758+
Attributes: map[string]*configschema.Attribute{
759+
"bloop": {
760+
NestedType: &configschema.Object{
761+
Nesting: configschema.NestingMap,
762+
Attributes: map[string]*configschema.Attribute{
763+
"blop": {
764+
Type: cty.String,
765+
Optional: true,
766+
},
767+
"bleep": {
768+
Type: cty.String,
769+
Optional: true,
770+
Computed: true,
771+
},
772+
},
773+
},
774+
Optional: true,
775+
},
776+
},
777+
},
778+
cty.ObjectVal(map[string]cty.Value{
779+
"bloop": cty.MapVal(map[string]cty.Value{
780+
"a": cty.ObjectVal(map[string]cty.Value{
781+
"blop": cty.StringVal("glub"),
782+
"bleep": cty.StringVal("computed"),
783+
}),
784+
"b": cty.ObjectVal(map[string]cty.Value{
785+
"blop": cty.StringVal("blub"),
786+
"bleep": cty.StringVal("computed"),
787+
}),
788+
}),
789+
}),
790+
cty.ObjectVal(map[string]cty.Value{
791+
"bloop": cty.MapVal(map[string]cty.Value{
792+
"a": cty.NullVal(cty.Object(map[string]cty.Type{
793+
"blop": cty.String,
794+
"bleep": cty.String,
795+
})),
796+
"c": cty.ObjectVal(map[string]cty.Value{
797+
"blop": cty.StringVal("blub"),
798+
"bleep": cty.NullVal(cty.String),
799+
}),
800+
}),
801+
}),
802+
cty.ObjectVal(map[string]cty.Value{
803+
"bloop": cty.MapVal(map[string]cty.Value{
804+
"a": cty.NullVal(cty.Object(map[string]cty.Type{
805+
"blop": cty.String,
806+
"bleep": cty.String,
807+
})),
808+
"c": cty.ObjectVal(map[string]cty.Value{
809+
"blop": cty.StringVal("blub"),
810+
"bleep": cty.NullVal(cty.String),
811+
}),
812+
}),
813+
}),
814+
},
815+
816+
"prior optional computed nested map to null": {
817+
&configschema.Block{
818+
Attributes: map[string]*configschema.Attribute{
819+
"bloop": {
820+
NestedType: &configschema.Object{
821+
Nesting: configschema.NestingMap,
822+
Attributes: map[string]*configschema.Attribute{
823+
"blop": {
824+
Type: cty.String,
825+
Optional: true,
826+
},
827+
"bleep": {
828+
Type: cty.String,
829+
Optional: true,
830+
Computed: true,
831+
},
832+
},
833+
},
834+
Optional: true,
835+
Computed: true,
836+
},
837+
},
838+
},
839+
cty.ObjectVal(map[string]cty.Value{
840+
"bloop": cty.MapVal(map[string]cty.Value{
841+
"a": cty.ObjectVal(map[string]cty.Value{
842+
"blop": cty.StringVal("glub"),
843+
"bleep": cty.StringVal("computed"),
844+
}),
845+
"b": cty.ObjectVal(map[string]cty.Value{
846+
"blop": cty.StringVal("blub"),
847+
"bleep": cty.StringVal("computed"),
848+
}),
849+
}),
850+
}),
851+
cty.ObjectVal(map[string]cty.Value{
852+
"bloop": cty.NullVal(cty.Map(
853+
cty.Object(map[string]cty.Type{
854+
"blop": cty.String,
855+
"bleep": cty.String,
856+
}),
857+
)),
858+
}),
859+
cty.ObjectVal(map[string]cty.Value{
860+
"bloop": cty.NullVal(cty.Map(
861+
cty.Object(map[string]cty.Type{
862+
"blop": cty.String,
863+
"bleep": cty.String,
864+
}),
865+
)),
866+
}),
867+
},
868+
755869
"prior nested map with dynamic": {
756870
&configschema.Block{
757871
BlockTypes: map[string]*configschema.NestedBlock{
@@ -2036,14 +2150,14 @@ func TestProposedNew(t *testing.T) {
20362150
)),
20372151
}),
20382152
cty.ObjectVal(map[string]cty.Value{
2039-
"list_obj": cty.ListVal([]cty.Value{
2040-
cty.ObjectVal(map[string]cty.Value{
2041-
"obj": cty.ObjectVal(map[string]cty.Value{
2042-
"optional": cty.StringVal("prior"),
2043-
"computed": cty.StringVal("prior computed"),
2153+
"list_obj": cty.NullVal(cty.List(
2154+
cty.Object(map[string]cty.Type{
2155+
"obj": cty.Object(map[string]cty.Type{
2156+
"optional": cty.String,
2157+
"computed": cty.String,
20442158
}),
20452159
}),
2046-
}),
2160+
)),
20472161
}),
20482162
},
20492163

0 commit comments

Comments
 (0)