Skip to content

Commit

Permalink
Merge pull request #32563 from hashicorp/jbardin/optional-computed-co…
Browse files Browse the repository at this point in the history 
…mparison-next

a new method of `ProposedNew` set comparison
  • Loading branch information
@jbardin
jbardin committed Jan 25, 2023
2 parents 799f4a7 + 60d6e52 commit b6906f3
Show file tree
Hide file tree
Showing 2 changed files with 234 additions and 87 deletions.
191 changes: 106 additions & 85 deletions internal/plans/objchange/objchange.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package objchange

import (
"errors"
"fmt"

"github.com/zclconf/go-cty/cty"
Expand Down Expand Up @@ -268,53 +269,54 @@ func proposedNewNestingMap(schema nestedSchema, prior, config cty.Value) cty.Val
}

func proposedNewNestingSet(schema nestedSchema, prior, config cty.Value) cty.Value {
newV := config

if !config.Type().IsSetType() {
panic("configschema.NestingSet value is not a set as expected")
}

// Nested set elements are correlated by comparing the element values after
// eliminating all of the computed attributes. In practice, this means that
// any config change produces an entirely new nested object, and we only
// propagate prior computed values if the non-computed attribute values are
// identical.
var cmpVals [][2]cty.Value
if prior.IsKnown() && !prior.IsNull() {
cmpVals = setElementCompareValues(schema, prior)
newV := config
if !config.IsKnown() || config.IsNull() || config.LengthInt() == 0 {
return newV
}

configVLen := 0
if config.IsKnown() && !config.IsNull() {
configVLen = config.LengthInt()
var priorVals []cty.Value
if prior.IsKnown() && !prior.IsNull() {
priorVals = prior.AsValueSlice()
}
if configVLen > 0 {
used := make([]bool, len(cmpVals)) // track used elements in case multiple have the same compare value
newVals := make([]cty.Value, 0, configVLen)
for it := config.ElementIterator(); it.Next(); {
_, configEV := it.Element()
var priorEV cty.Value
for i, cmp := range cmpVals {
if used[i] {
continue
}

if cmp[1].RawEquals(configEV) {
priorEV = cmp[0]
used[i] = true // we can't use this value on a future iteration
break
}

var newVals []cty.Value
// track which prior elements have been used
used := make([]bool, len(priorVals))

for _, configEV := range config.AsValueSlice() {
var priorEV cty.Value
for i, priorCmp := range priorVals {
if used[i] {
continue
}
if priorEV == cty.NilVal {
priorEV = cty.NullVal(config.Type().ElementType())

// It is possible that multiple prior elements could be valid
// matches for a configuration value, in which case we will end up
// picking the first match encountered (but it will always be
// consistent due to cty's iteration order). Because configured set
// elements must also be entirely unique in order to be included in
// the set, these matches either will not matter because they only
// differ by computed values, or could not have come from a valid
// config with all unique set elements.
if validPriorFromConfig(schema, priorCmp, configEV) {
priorEV = priorCmp
used[i] = true
break
}
}

newVals = append(newVals, proposedNewBlockOrObject(schema, priorEV, configEV))
if priorEV == cty.NilVal {
priorEV = cty.NullVal(config.Type().ElementType())
}
newV = cty.SetVal(newVals)

newVals = append(newVals, proposedNewBlockOrObject(schema, priorEV, configEV))
}

return newV
return cty.SetVal(newVals)
}

func proposedNewObjectAttributes(schema *configschema.Object, prior, config cty.Value) cty.Value {
Expand Down Expand Up @@ -372,63 +374,12 @@ func proposedNewAttributes(attrs map[string]*configschema.Attribute, prior, conf
return newAttrs
}

// setElementCompareValues takes a known, non-null value of a cty.Set type and
// returns a table -- constructed of two-element arrays -- that maps original
// set element values to corresponding values that have all of the computed
// values removed, making them suitable for comparison with values obtained
// from configuration. The element type of the set must conform to the implied
// type of the given schema, or this function will panic.
//
// In the resulting slice, the zeroth element of each array is the original
// value and the one-indexed element is the corresponding "compare value".
//
// This is intended to help correlate prior elements with configured elements
// in proposedNewBlock. The result is a heuristic rather than an exact science,
// since e.g. two separate elements may reduce to the same value through this
// process. The caller must therefore be ready to deal with duplicates.
func setElementCompareValues(schema nestedSchema, set cty.Value) [][2]cty.Value {
ret := make([][2]cty.Value, 0, set.LengthInt())
for it := set.ElementIterator(); it.Next(); {
_, ev := it.Element()
ret = append(ret, [2]cty.Value{ev, setElementComputedAsNull(schema, ev)})
}
return ret
}

// nestedSchema is used as a generic container for either a
// *configschema.Object, or *configschema.Block.
type nestedSchema interface {
AttributeByPath(cty.Path) *configschema.Attribute
}

func setElementComputedAsNull(schema nestedSchema, elem cty.Value) cty.Value {
elem, _ = cty.Transform(elem, func(path cty.Path, v cty.Value) (cty.Value, error) {
if v.IsNull() || !v.IsKnown() {
return v, nil
}

attr := schema.AttributeByPath(path)
if attr == nil {
// we must be at a nested block rather than an attribute, continue
return v, nil
}

if attr.Computed {
// If this could have been computed, we always return null in order
// to compare config and state values.
//
// The only way to determine if an optional+computed value is
// optional or computed is see if it is set in the config, however
// for set elements we don't yet know which element value
// corresponds to the configuration.
return cty.NullVal(v.Type()), nil
}
return v, nil
})

return elem
}

// optionalValueNotComputable is used to check if an object in state must
// have at least partially come from configuration. If the prior value has any
// non-null attributes which are not computed in the schema, then we know there
Expand Down Expand Up @@ -468,3 +419,73 @@ func optionalValueNotComputable(schema *configschema.Attribute, val cty.Value) b

return foundNonComputedAttr
}

// validPriorFromConfig returns true if the prior object could have been
// derived from the configuration. We do this by walking the prior value to
// determine if it is a valid superset of the config, and only computable
// values have been added. This function is only used to correlated
// configuration with possible valid prior values within sets.
func validPriorFromConfig(schema nestedSchema, prior, config cty.Value) bool {
if config.RawEquals(prior) {
return true
}

// error value to halt the walk
stop := errors.New("stop")

valid := true
cty.Walk(prior, func(path cty.Path, priorV cty.Value) (bool, error) {
configV, err := path.Apply(config)
if err != nil {
// most likely dynamic objects with different types
valid = false
return false, stop
}

// we don't need to know the schema if both are equal
if configV.RawEquals(priorV) {
// we know they are equal, so no need to descend further
return false, nil
}

// We can't descend into nested sets to correlate configuration, so the
// overall values must be equal.
if configV.Type().IsSetType() {
valid = false
return false, stop
}

attr := schema.AttributeByPath(path)
if attr == nil {
// Not at a schema attribute, so we can continue until we find leaf
// attributes.
return true, nil
}

// If we have nested object attributes we'll be descending into those
// to compare the individual values and determine why this level is not
// equal
if attr.NestedType != nil {
return true, nil
}

// This is a leaf attribute, so it must be computed in order to differ
// from config.
if !attr.Computed {
valid = false
return false, stop
}

// And if it is computed, the config must be null to allow a change.
if !configV.IsNull() {
valid = false
return false, stop
}

// We sill stop here. The cty value could be far larger, but this was
// the last level of prescribed schema.
return false, nil
})

return valid
}
130 changes: 128 additions & 2 deletions internal/plans/objchange/objchange_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2537,8 +2537,6 @@ func TestProposedNew(t *testing.T) {
}),
},

// If there are no configured attributes which cannot be computed, the
// values cannot be correlated and will always produce a change.
"set attr with all optional computed": {
&configschema.Block{
Attributes: map[string]*configschema.Attribute{
Expand Down Expand Up @@ -2574,6 +2572,10 @@ func TestProposedNew(t *testing.T) {
}),
}),
}),
// Each of these values can be correlated by the existence of the
// optional config attribute. Because "one" and "two" are set in
// the config, they must exist in the state regardless of
// optional&computed.
cty.ObjectVal(map[string]cty.Value{
"multi": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
Expand All @@ -2590,11 +2592,135 @@ func TestProposedNew(t *testing.T) {
"multi": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("one"),
"oc": cty.StringVal("OK"),
}),
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("two"),
"oc": cty.StringVal("OK"),
}),
}),
}),
},

"set block with all optional computed and nested object types": {
&configschema.Block{
BlockTypes: map[string]*configschema.NestedBlock{
"multi": {
Nesting: configschema.NestingSet,
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"opt": {
Type: cty.String,
Optional: true,
Computed: true,
},
"oc": {
Type: cty.String,
Optional: true,
Computed: true,
},
"attr": {
Optional: true,
NestedType: &configschema.Object{
Nesting: configschema.NestingSet,
Attributes: map[string]*configschema.Attribute{
"opt": {
Type: cty.String,
Optional: true,
Computed: true,
},
"oc": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
},
},
},
},
},
},
cty.ObjectVal(map[string]cty.Value{
"multi": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("one"),
"oc": cty.StringVal("OK"),
"attr": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("one"),
"oc": cty.StringVal("OK"),
})}),
}),
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("two"),
"oc": cty.StringVal("OK"),
"attr": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("two"),
"oc": cty.StringVal("OK"),
})}),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"multi": cty.SetVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("one"),
"oc": cty.NullVal(cty.String),
"attr": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("one"),
"oc": cty.StringVal("OK"),
})}),
}),
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("two"),
"oc": cty.StringVal("OK"),
"attr": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("two"),
"oc": cty.NullVal(cty.String),
})}),
}),
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("three"),
"oc": cty.NullVal(cty.String),
"attr": cty.NullVal(cty.Set(cty.Object(map[string]cty.Type{
"opt": cty.String,
"oc": cty.String,
}))),
}),
}),
}),
cty.ObjectVal(map[string]cty.Value{
"multi": cty.SetVal([]cty.Value{
// We can correlate this with prior from the outer object
// attributes, and the equal nested set.
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("one"),
"oc": cty.StringVal("OK"),
"attr": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("one"),
"oc": cty.StringVal("OK"),
})}),
}),
// This value is overridden by config, because we can't
// correlate optional+computed config values within nested
// sets.
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("two"),
"oc": cty.StringVal("OK"),
"attr": cty.SetVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("two"),
"oc": cty.NullVal(cty.String),
})}),
}),
// This value was taken only from config
cty.ObjectVal(map[string]cty.Value{
"opt": cty.StringVal("three"),
"oc": cty.NullVal(cty.String),
"attr": cty.NullVal(cty.Set(cty.Object(map[string]cty.Type{
"opt": cty.String,
"oc": cty.String,
}))),
}),
}),
}),
Expand Down

0 comments on commit b6906f3

Please sign in to comment.