prototype showing that granular->atomic changes can be fixed up by a pre-apply traversal

Author: jpbetz

merge/update.go

@@ -122,6 +122,10 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
 // this is a CREATE call).
 func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string) (*typed.TypedValue, fieldpath.ManagedFields, error) {
 	var err error
+	managers, err = s.fixupManagedFields(liveObject, managers)
+	if err != nil {
+		return nil, fieldpath.ManagedFields{}, err
+	}
 	if s.enableUnions {
 		newObject, err = liveObject.NormalizeUnions(newObject)
 		if err != nil {
@@ -152,13 +156,48 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
 	return newObject, managers, nil
 }
 
+// fixupManagedFields attempts to fix up the managed fields to be compatible with the live state of
+// the schema.
+// This is needed when an "unversioned" change are made to a schema that impact the managedFields.
+// By "unversionsed" we mean that a specific version of a schema was directly modified (introduction
+// of new versions does not require fixup).
+//
+// Supported schema changes:
+// - Changing a type to "atomic": All field manager that managed any granular fields become managers
+//   of the atomic to ensure a conflict if any of the fields they previously managed are changed.
+//   Granular managed fields of the struct are removed.
+//
+// TODO: Changes to add support for:
+// - Change a type from "atomic" to "granular", "set" or "map". All field manager that managed the
+//   atomic become managers of all child fields to ensure a conflict if any child field is changed.
+//
+// Potential future optimization: Write a SHA-256 hash to managedFields that can be compared
+// with the current hash of the live schemas to determine if any fixup is needed.
+func (s *Updater) fixupManagedFields(liveObject *typed.TypedValue, managers fieldpath.ManagedFields) (fieldpath.ManagedFields, error) {
+	// TODO(jpbetz): I will use Converter to get schemas at different versions when prototyping, but might need to swap that out with something else for production? Is mixed version apply is quite rare?
+	result := fieldpath.ManagedFields{}
+	for manager, versionedSet := range managers {
+		tv, err := s.Converter.Convert(liveObject, versionedSet.APIVersion())
+		if err != nil {
+			return nil, err
+		}
+		fieldSet := versionedSet.Set()
+		fixedFieldSet, err := typed.FixupFieldManagers(fieldSet, tv)
+		result[manager] = fieldpath.NewVersionedSet(fixedFieldSet, versionedSet.APIVersion(), versionedSet.Applied())
+	}
+	return result, nil
+}
+
 // Apply should be called when Apply is run, given the current object as
 // well as the configuration that is applied. This will merge the object
 // and return it. If the object hasn't changed, nil is returned (the
 // managers can still have changed though).
 func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) {
-	managers = shallowCopyManagers(managers)
 	var err error
+	managers, err = s.fixupManagedFields(liveObject, managers)
+	if err != nil {
+		return nil, fieldpath.ManagedFields{}, err
+	}
 	if s.enableUnions {
 		configObject, err = configObject.NormalizeUnionsApply(configObject)
 		if err != nil {

typed/field_manager_fixup.go

@@ -0,0 +1,191 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package typed
+
+import (
+	"fmt"
+	"sync"
+
+	"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
+	"sigs.k8s.io/structured-merge-diff/v4/schema"
+)
+
+var fmPool = sync.Pool{
+	New: func() interface{} { return &fixupObjectWalker{} },
+}
+
+func (v *fixupObjectWalker) finished() {
+	v.fieldSet = nil
+	v.schema = nil
+	v.typeRef = schema.TypeRef{}
+	fmPool.Put(v)
+}
+
+type fixupObjectWalker struct {
+	fieldSet *fieldpath.Set // the field set to fixup
+	schema   *schema.Schema // the schema, must be the same APIVersion as the fieldSet
+	typeRef  schema.TypeRef // current type
+	path     fieldpath.Path // current path
+
+	out *fieldpath.Set // the resulting fixed up field set, TODO if out is nil, interpret it to mean that there is no change
+
+	// Allocate only as many walkers as needed for the depth by storing them here.
+	spareWalkers *[]*fixupObjectWalker
+}
+
+func (v *fixupObjectWalker) prepareDescent(pe fieldpath.PathElement, tr schema.TypeRef) *fixupObjectWalker {
+	if v.spareWalkers == nil {
+		// first descent.
+		v.spareWalkers = &[]*fixupObjectWalker{}
+	}
+	var v2 *fixupObjectWalker
+	if n := len(*v.spareWalkers); n > 0 {
+		v2, *v.spareWalkers = (*v.spareWalkers)[n-1], (*v.spareWalkers)[:n-1]
+	} else {
+		v2 = &fixupObjectWalker{}
+	}
+	*v2 = *v
+	v2.typeRef = tr
+	v2.path = append(v.path.Copy(), pe) // TODO: avoid copy?
+	return v2
+}
+
+func (v *fixupObjectWalker) finishDescent(v2 *fixupObjectWalker) {
+	v.fieldSet = nil
+	v.schema = nil
+	v.path = nil
+
+	v.typeRef = schema.TypeRef{}
+
+	// if the descent caused a realloc, ensure that we reuse the buffer
+	// for the next sibling.
+	*v.spareWalkers = append(*v.spareWalkers, v2)
+}
+
+// FixupFieldManagers attempts to fix up the managed fields to be compatible with the live state of
+// the schema.
+// This is needed when an "unversioned" change are made to a schema that impact the managedFields.
+// By "unversionsed" we mean that a specific version of a schema was directly modified (introduction
+// of new versions does not require fixup).
+//
+// Supported schema changes:
+// - Changing a type to "atomic": All field manager that managed any granular fields become managers
+//   of the atomic to ensure a conflict if any of the fields they previously managed are changed.
+//   Granular managed fields of the struct are removed.
+func FixupFieldManagers(fieldSet *fieldpath.Set, tv *TypedValue) (*fieldpath.Set, error) {
+	v := fmPool.Get().(*fixupObjectWalker)
+	v.fieldSet = fieldSet
+	v.schema = tv.schema
+	v.typeRef = tv.typeRef
+
+	defer v.finished()
+	errs := v.fixupFieldManagers()
+	if len(errs) > 0 {
+		return nil, fmt.Errorf("fixup errors: %s", errs.Error())
+	}
+	return v.out, nil
+}
+
+func (v *fixupObjectWalker) fixupFieldManagers() (errs ValidationErrors) {
+	a, ok := v.schema.Resolve(v.typeRef)
+	if !ok {
+		errs = append(errs, errorf("could not resolve %v", v.typeRef)...)
+		return
+	}
+	return handleAtom(a, v.typeRef, v)
+}
+
+func (v *fixupObjectWalker) doScalar(t *schema.Scalar) (errs ValidationErrors) {
+	v.out = v.fieldSet
+	return errs
+}
+
+func (v *fixupObjectWalker) visitListItems(t *schema.List, element *fieldpath.Set) (errs ValidationErrors) {
+	out := fieldpath.NewSet()
+	element.Children.Iterate(func(pe fieldpath.PathElement) {
+		v2 := v.prepareDescent(pe, t.ElementType)
+		v2.fieldSet = element.Children.Descend(pe)
+		errs = append(errs, v2.fixupFieldManagers()...)
+		out = out.Union(v2.out) // TODO(jpbetz): If nothing has changed, return null (and optimize the code paths for this)
+		v.finishDescent(v2)
+	})
+	element.Members.Iterate(func(pe fieldpath.PathElement) {
+		path := append(v.path.Copy(), pe) // TODO: avoid copy?
+		out.Insert(path)
+	})
+	v.out = out
+	return nil
+}
+
+func (v *fixupObjectWalker) doList(t *schema.List) (errs ValidationErrors) {
+	// fix up lists changed to 'atomic' by:
+	// - including the list in the managed fields if any of the child field elements are present
+	// - removing all child field elements from the managed fields
+	if t.ElementRelationship == schema.Atomic { // TODO: is this right for both Separable->Atomic and Associative->Atomic
+		if v.fieldSet.Size() > 0 {
+			v.out = fieldpath.NewSet(v.path)
+		}
+		return errs
+	}
+	// TODO: Support Atomic->Separable, Atomic->Associative
+
+	errs = v.visitListItems(t, v.fieldSet)
+	return errs
+}
+
+func (v *fixupObjectWalker) visitMapItems(t *schema.Map, element *fieldpath.Set) (errs ValidationErrors) {
+	out := fieldpath.NewSet()
+	element.Children.Iterate(func(pe fieldpath.PathElement) {
+		tr := t.ElementType
+		if pe.FieldName != nil {
+			if sf, ok := t.FindField(*pe.FieldName); ok {
+				tr = sf.Type
+			}
+		}
+		if (tr == schema.TypeRef{}) {
+			errs = append(errs, errorf("field not declared in schema").WithPrefix(pe.String())...)
+			return
+		}
+		v2 := v.prepareDescent(pe, tr)
+		v2.fieldSet = element.Children.Descend(pe)
+		errs = append(errs, v2.fixupFieldManagers()...)
+		out = out.Union(v2.out) // TODO(jpbetz): If nothing has changed, return null (and optimize the code paths for this)
+		v.finishDescent(v2)
+	})
+	element.Members.Iterate(func(pe fieldpath.PathElement) {
+		path := append(v.path.Copy(), pe) // TODO: avoid copy?
+		out.Insert(path)
+	})
+	v.out = out
+	return nil
+}
+
+func (v *fixupObjectWalker) doMap(t *schema.Map) (errs ValidationErrors) {
+	// fix up maps/structs changed to 'atomic' by:
+	// - including the map in the managed fields if any of the child field elements are present
+	// - removing all child field elements from the managed fields
+	if t.ElementRelationship == schema.Atomic {
+		if v.fieldSet.Size() > 0 {
+			v.out = fieldpath.NewSet(v.path)
+		}
+		return errs
+	}
+	// TODO: Support Atomic->Separable
+
+	errs = v.visitMapItems(t, v.fieldSet)
+	return errs
+}