Check more constraints

This commit is contained in:
Fraser Waters 2021-11-17 15:49:17 +00:00
parent 23e9ec2c54
commit de3e95b7ab
2 changed files with 141 additions and 20 deletions

View file

@ -46,13 +46,31 @@ func NewGoalPlan(oldOutputs resource.PropertyMap, goal *resource.Goal) *GoalPlan
return nil
}
var adds resource.PropertyMap
var deletes []resource.PropertyKey
var updates resource.PropertyMap
if diff, hasDiff := oldOutputs.DiffIncludeUnknowns(goal.Properties); hasDiff {
adds = diff.Adds
updates = make(resource.PropertyMap)
for k := range diff.Updates {
updates[k] = diff.Updates[k].New
}
deletes = make([]resource.PropertyKey, len(diff.Deletes))
i := 0
for k := range diff.Deletes {
deletes[i] = k
i = i + 1
}
}
return &GoalPlan{
Type: goal.Type,
Name: goal.Name,
Custom: goal.Custom,
Adds: nil,
Deletes: nil,
Updates: nil,
Adds: adds,
Deletes: deletes,
Updates: updates,
Parent: goal.Parent,
Protect: goal.Protect,
Dependencies: goal.Dependencies,
@ -252,12 +270,14 @@ func (rp *ResourcePlan) checkGoal(
// Check that the property diffs meet the constraints set in the plan.
changes := []string{}
if diff, hasDiff := oldOutputs.DiffIncludeUnknowns(newInputs); hasDiff {
var diff *resource.ObjectDiff
var hasDiff bool
if diff, hasDiff = oldOutputs.DiffIncludeUnknowns(newInputs); hasDiff {
// Check that any adds are in the goal for adds
for k := range diff.Adds {
if expected, has := rp.Goal.Adds[k]; has {
actual := diff.Adds[k]
if !expected.IsComputed() && !expected.DeepEquals(actual) {
if !expected.DeepEqualsIncludeUnknowns(actual) {
// diff wants to add this with value X but constraint wants to add with value Y
changes = append(changes, "+"+string(k))
}
@ -283,19 +303,101 @@ func (rp *ResourcePlan) checkGoal(
}
}
// Check that any changes are in the goal for changes
// Check that any changes are in the goal for changes or adds
// "or adds" is because if our constraint says to add K=V and someone has already added K=W we don't consider it
// a constraint violation to update K to V. This is similar to how if we have a Create resource constraint we don't consider it
// a violation to just update it instead of creating it.
for k := range diff.Updates {
actual := diff.Updates[k].New
if expected, has := rp.Goal.Updates[k]; has {
actual := diff.Adds[k]
if !expected.IsComputed() && !expected.DeepEquals(actual) {
if !expected.DeepEqualsIncludeUnknowns(actual) {
// diff wants to change this with value X but constraint wants to change with value Y
changes = append(changes, "~"+string(k))
}
} else if expected, has := rp.Goal.Adds[k]; has {
if !expected.DeepEqualsIncludeUnknowns(actual) {
// diff wants to change this with value X but constraint wants to add with value Y
changes = append(changes, "~"+string(k))
}
} else {
// diff wants to update this, but not listed as an update in the constraints
changes = append(changes, "~"+string(k))
}
}
} else {
// No diff, just new up an empty ObjectDiff for checks below
diff = &resource.ObjectDiff{}
}
// Symmetric check, check that the constraints didn't expect things to happen that aren't in the new inputs
for k := range rp.Goal.Adds {
// We expected an add, make sure the value is in the new inputs. That means it's either an add, update, or a same, both are ok for an add constraint.
expected := rp.Goal.Adds[k]
// If this is in diff.Adds or diff.Updates we'll of already checked it
_, inAdds := diff.Adds[k]
_, inUpdates := diff.Updates[k]
if !inAdds && !inUpdates {
// It wasn't in the diff as an add or update so check we have a same
if actual, has := newInputs[k]; has {
if !expected.DeepEqualsIncludeUnknowns(actual) {
// diff wants to same this with value X but constraint wants to add with value Y
changes = append(changes, "~"+string(k))
}
} else {
// Not a same, update or an add but constraint wants to add it
changes = append(changes, "-"+string(k))
}
}
}
for k := range rp.Goal.Updates {
// We expected an update, make sure the value is in the new inputs as an update (not an add)
expected := rp.Goal.Updates[k]
// If this is in diff.Updates we'll of already checked it
_, inUpdates := diff.Updates[k]
if !inUpdates {
// Check if this was in adds, it's not ok to have an update constraint but actually do an add
_, inAdds := diff.Adds[k]
if inAdds {
// Constraint wants to update it, but diff wants to add it
changes = append(changes, "+"+string(k))
} else if actual, has := newInputs[k]; has {
// It wasn't in the diff as an add so check we have a same
if !expected.DeepEqualsIncludeUnknowns(actual) {
// diff wants to same this with value X but constraint wants to update with value Y
changes = append(changes, "~"+string(k))
}
} else {
// Not a same or an update but constraint wants to update it
changes = append(changes, "-"+string(k))
}
}
}
for i := range rp.Goal.Deletes {
// We expected a delete, make sure its not present
k := rp.Goal.Deletes[i]
// If this is in diff.Deletes we'll of already checked it
_, inDeletes := diff.Deletes[k]
if !inDeletes {
// See if this is an add, update, or same
if _, has := diff.Adds[k]; has {
// Constraint wants to delete this but diff wants to add it
changes = append(changes, "+"+string(k))
} else if _, has := diff.Updates[k]; has {
// Constraint wants to delete this but diff wants to update it
changes = append(changes, "~"+string(k))
} else if _, has := diff.Sames[k]; has {
// Constraint wants to delete this but diff wants to leave it same
changes = append(changes, "~"+string(k))
}
}
}
if len(changes) > 0 {

View file

@ -280,13 +280,12 @@ func (v PropertyValue) Diff(other PropertyValue, ignoreKeys ...IgnoreKeyFunc) *V
return v.diff(other, false, ignoreKeys)
}
// DeepEquals returns true if this property map is deeply equal to the other property map; and false otherwise.
func (props PropertyMap) DeepEquals(other PropertyMap) bool {
func (props PropertyMap) deepEquals(other PropertyMap, includeUnknowns bool) bool {
// If any in props either doesn't exist, or is of a different value, return false.
for _, k := range props.StableKeys() {
v := props[k]
if p, has := other[k]; has {
if !v.DeepEquals(p) {
if !v.deepEquals(p, includeUnknowns) {
return false
}
} else if v.HasValue() {
@ -304,13 +303,19 @@ func (props PropertyMap) DeepEquals(other PropertyMap) bool {
return true
}
// DeepEquals returns true if this property value is deeply equal to the other property value; and false otherwise.
func (v PropertyValue) DeepEquals(other PropertyValue) bool {
func (v PropertyValue) deepEquals(other PropertyValue, includeUnknowns bool) bool {
// Computed values are always equal.
if v.IsComputed() && other.IsComputed() {
return true
}
// If includeUnknowns is true then anything is equal to a computed
if includeUnknowns {
if v.IsComputed() || other.IsComputed() {
return true
}
}
// Arrays are equal if they are both of the same size and elements are deeply equal.
if v.IsArray() {
if !other.IsArray() {
@ -322,7 +327,7 @@ func (v PropertyValue) DeepEquals(other PropertyValue) bool {
return false
}
for i, elem := range va {
if !elem.DeepEquals(oa[i]) {
if !elem.deepEquals(oa[i], includeUnknowns) {
return false
}
}
@ -349,7 +354,7 @@ func (v PropertyValue) DeepEquals(other PropertyValue) bool {
}
vo := v.ObjectValue()
oa := other.ObjectValue()
return vo.DeepEquals(oa)
return vo.deepEquals(oa, includeUnknowns)
}
// Secret are equal if the value they wrap are equal.
@ -360,7 +365,7 @@ func (v PropertyValue) DeepEquals(other PropertyValue) bool {
vs := v.SecretValue()
os := other.SecretValue()
return vs.Element.DeepEquals(os.Element)
return vs.Element.deepEquals(os.Element, includeUnknowns)
}
// Resource references are equal if they refer to the same resource. The package version is ignored.
@ -379,7 +384,7 @@ func (v PropertyValue) DeepEquals(other PropertyValue) bool {
if vid.IsComputed() && oid.IsComputed() {
return true
}
return vid.DeepEquals(oid)
return vid.deepEquals(oid, includeUnknowns)
}
// Outputs are equal if each of their fields is deeply equal.
@ -407,14 +412,28 @@ func (v PropertyValue) DeepEquals(other PropertyValue) bool {
}
}
return vo.Element.DeepEquals(oo.Element)
return vo.Element.deepEquals(oo.Element, includeUnknowns)
}
// For all other cases, primitives are equal if their values are equal.
return v.V == other.V
}
func (props PropertyMap) DiffIncludeUnknowns(constraints PropertyMap) (*ObjectDiff, bool) {
diff := constraints.diff(props, true, nil)
// DeepEquals returns true if this property map is deeply equal to the other property map; and false otherwise.
func (props PropertyMap) DeepEquals(other PropertyMap) bool {
return props.deepEquals(other, false)
}
// DeepEquals returns true if this property value is deeply equal to the other property value; and false otherwise.
func (v PropertyValue) DeepEquals(other PropertyValue) bool {
return v.deepEquals(other, false)
}
func (props PropertyMap) DiffIncludeUnknowns(other PropertyMap) (*ObjectDiff, bool) {
diff := props.diff(other, true, nil)
return diff, diff != nil
}
func (v PropertyValue) DeepEqualsIncludeUnknowns(other PropertyValue) bool {
return v.deepEquals(other, true)
}