Skip to content

Commit 82b7e0f

Browse files
committed
prototype showing that granular->atomic changes can be fixed up by a pre-apply traversal
1 parent bc70e0d commit 82b7e0f

File tree

3 files changed

+373
-1
lines changed

3 files changed

+373
-1
lines changed

merge/update.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
122122
// this is a CREATE call).
123123
func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string) (*typed.TypedValue, fieldpath.ManagedFields, error) {
124124
var err error
125+
managers, err = s.fixupManagedFields(liveObject, managers)
126+
if err != nil {
127+
return nil, fieldpath.ManagedFields{}, err
128+
}
125129
if s.enableUnions {
126130
newObject, err = liveObject.NormalizeUnions(newObject)
127131
if err != nil {
@@ -152,13 +156,48 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
152156
return newObject, managers, nil
153157
}
154158

159+
// fixupManagedFields attempts to fix up the managed fields to be compatible with the live state of
160+
// the schema.
161+
// This is needed when an "unversioned" change are made to a schema that impact the managedFields.
162+
// By "unversionsed" we mean that a specific version of a schema was directly modified (introduction
163+
// of new versions does not require fixup).
164+
//
165+
// Supported schema changes:
166+
// - Changing a type to "atomic": All field manager that managed any granular fields become managers
167+
// of the atomic to ensure a conflict if any of the fields they previously managed are changed.
168+
// Granular managed fields of the struct are removed.
169+
//
170+
// TODO: Changes to add support for:
171+
// - Change a type from "atomic" to "granular", "set" or "map". All field manager that managed the
172+
// atomic become managers of all child fields to ensure a conflict if any child field is changed.
173+
//
174+
// Potential future optimization: Write a SHA-256 hash to managedFields that can be compared
175+
// with the current hash of the live schemas to determine if any fixup is needed.
176+
func (s *Updater) fixupManagedFields(liveObject *typed.TypedValue, managers fieldpath.ManagedFields) (fieldpath.ManagedFields, error) {
177+
// 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?
178+
result := fieldpath.ManagedFields{}
179+
for manager, versionedSet := range managers {
180+
tv, err := s.Converter.Convert(liveObject, versionedSet.APIVersion())
181+
if err != nil {
182+
return nil, err
183+
}
184+
fieldSet := versionedSet.Set()
185+
fixedFieldSet, err := typed.FixupFieldManagers(fieldSet, tv)
186+
result[manager] = fieldpath.NewVersionedSet(fixedFieldSet, versionedSet.APIVersion(), versionedSet.Applied())
187+
}
188+
return result, nil
189+
}
190+
155191
// Apply should be called when Apply is run, given the current object as
156192
// well as the configuration that is applied. This will merge the object
157193
// and return it. If the object hasn't changed, nil is returned (the
158194
// managers can still have changed though).
159195
func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) {
160-
managers = shallowCopyManagers(managers)
161196
var err error
197+
managers, err = s.fixupManagedFields(liveObject, managers)
198+
if err != nil {
199+
return nil, fieldpath.ManagedFields{}, err
200+
}
162201
if s.enableUnions {
163202
configObject, err = configObject.NormalizeUnionsApply(configObject)
164203
if err != nil {

typed/field_manager_fixup.go

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package typed
18+
19+
import (
20+
"fmt"
21+
"sync"
22+
23+
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
24+
"sigs.k8s.io/structured-merge-diff/v4/schema"
25+
)
26+
27+
var fmPool = sync.Pool{
28+
New: func() interface{} { return &fixupObjectWalker{} },
29+
}
30+
31+
func (v *fixupObjectWalker) finished() {
32+
v.fieldSet = nil
33+
v.schema = nil
34+
v.typeRef = schema.TypeRef{}
35+
fmPool.Put(v)
36+
}
37+
38+
type fixupObjectWalker struct {
39+
fieldSet *fieldpath.Set // the field set to fixup
40+
schema *schema.Schema
41+
typeRef schema.TypeRef
42+
path fieldpath.Path
43+
44+
out *fieldpath.Set // the resulting fixed up field set
45+
46+
// Allocate only as many walkers as needed for the depth by storing them here.
47+
spareWalkers *[]*fixupObjectWalker
48+
}
49+
50+
func (v *fixupObjectWalker) prepareDescent(pe fieldpath.PathElement, tr schema.TypeRef) *fixupObjectWalker {
51+
if v.spareWalkers == nil {
52+
// first descent.
53+
v.spareWalkers = &[]*fixupObjectWalker{}
54+
}
55+
var v2 *fixupObjectWalker
56+
if n := len(*v.spareWalkers); n > 0 {
57+
v2, *v.spareWalkers = (*v.spareWalkers)[n-1], (*v.spareWalkers)[:n-1]
58+
} else {
59+
v2 = &fixupObjectWalker{}
60+
}
61+
*v2 = *v
62+
v2.typeRef = tr
63+
v2.path = append(v.path.Copy(), pe) // TODO: avoid copy?
64+
return v2
65+
}
66+
67+
func (v *fixupObjectWalker) finishDescent(v2 *fixupObjectWalker) {
68+
v.fieldSet = nil
69+
v.schema = nil
70+
v.path = nil
71+
72+
v.typeRef = schema.TypeRef{}
73+
74+
// if the descent caused a realloc, ensure that we reuse the buffer
75+
// for the next sibling.
76+
*v.spareWalkers = append(*v.spareWalkers, v2)
77+
}
78+
79+
func FixupFieldManagers(fieldSet *fieldpath.Set, tv *TypedValue) (*fieldpath.Set, error) {
80+
v := fmPool.Get().(*fixupObjectWalker)
81+
v.fieldSet = fieldSet
82+
v.schema = tv.schema
83+
v.typeRef = tv.typeRef
84+
85+
defer v.finished()
86+
errs := v.fixupFieldManagers()
87+
if len(errs) > 0 {
88+
return nil, fmt.Errorf("fixup errors: %s", errs.Error())
89+
}
90+
return v.out, nil
91+
}
92+
93+
func (v *fixupObjectWalker) fixupFieldManagers() (errs ValidationErrors) {
94+
a, ok := v.schema.Resolve(v.typeRef)
95+
if !ok {
96+
errs = append(errs, errorf("could not resolve %v", v.typeRef)...)
97+
return
98+
}
99+
return handleAtom(a, v.typeRef, v)
100+
}
101+
102+
func (v *fixupObjectWalker) doScalar(t *schema.Scalar) (errs ValidationErrors) {
103+
v.out = v.fieldSet
104+
return errs
105+
}
106+
107+
func (v *fixupObjectWalker) visitListItems(t *schema.List, element *fieldpath.Set) (errs ValidationErrors) {
108+
out := fieldpath.NewSet()
109+
element.Children.Iterate(func(pe fieldpath.PathElement) {
110+
v2 := v.prepareDescent(pe, t.ElementType)
111+
v2.fieldSet = element.Children.Descend(pe)
112+
errs = append(errs, v2.fixupFieldManagers()...)
113+
out = out.Union(v2.out) // TODO(jpbetz): If nothing has changed, return null (and optimize the code paths for this)
114+
v.finishDescent(v2)
115+
})
116+
element.Members.Iterate(func(pe fieldpath.PathElement) {
117+
path := append(v.path.Copy(), pe) // TODO: avoid copy?
118+
out.Insert(path)
119+
})
120+
v.out = out
121+
return nil
122+
}
123+
124+
func (v *fixupObjectWalker) doList(t *schema.List) (errs ValidationErrors) {
125+
// fix up atomic lists by:
126+
// - including the list in the managed fields if any of the child field elements are present
127+
// - removing all child field elements from the managed fields
128+
if t.ElementRelationship == schema.Atomic {
129+
if v.fieldSet.Size() > 0 {
130+
v.out = fieldpath.NewSet(v.path)
131+
}
132+
return errs
133+
}
134+
135+
errs = v.visitListItems(t, v.fieldSet)
136+
return errs
137+
}
138+
139+
func (v *fixupObjectWalker) visitMapItems(t *schema.Map, element *fieldpath.Set) (errs ValidationErrors) {
140+
out := fieldpath.NewSet()
141+
element.Children.Iterate(func(pe fieldpath.PathElement) {
142+
tr := t.ElementType
143+
if pe.FieldName != nil {
144+
if sf, ok := t.FindField(*pe.FieldName); ok {
145+
tr = sf.Type
146+
}
147+
}
148+
if (tr == schema.TypeRef{}) {
149+
errs = append(errs, errorf("field not declared in schema").WithPrefix(pe.String())...)
150+
return
151+
}
152+
v2 := v.prepareDescent(pe, tr)
153+
v2.fieldSet = element.Children.Descend(pe)
154+
errs = append(errs, v2.fixupFieldManagers()...)
155+
out = out.Union(v2.out) // TODO(jpbetz): If nothing has changed, return null (and optimize the code paths for this)
156+
v.finishDescent(v2)
157+
})
158+
element.Members.Iterate(func(pe fieldpath.PathElement) {
159+
path := append(v.path.Copy(), pe) // TODO: avoid copy?
160+
out.Insert(path)
161+
})
162+
v.out = out
163+
return nil
164+
}
165+
166+
func (v *fixupObjectWalker) doMap(t *schema.Map) (errs ValidationErrors) {
167+
// fix up atomic maps by:
168+
// - including the map in the managed fields if any of the child field elements are present
169+
// - removing all child field elements from the managed fields
170+
if t.ElementRelationship == schema.Atomic {
171+
if v.fieldSet.Size() > 0 {
172+
v.out = fieldpath.NewSet(v.path)
173+
}
174+
return errs
175+
}
176+
177+
errs = v.visitMapItems(t, v.fieldSet)
178+
return errs
179+
}

typed/field_manager_fixup_test.go

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/*
2+
Copyright 2018 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package typed_test
18+
19+
import (
20+
"testing"
21+
22+
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
23+
"sigs.k8s.io/structured-merge-diff/v4/typed"
24+
)
25+
26+
type fixupTestCase struct {
27+
name string
28+
rootTypeName string
29+
oldSchema typed.YAMLObject
30+
newSchema typed.YAMLObject
31+
liveObject typed.YAMLObject
32+
oldFields *fieldpath.Set
33+
fixedFields *fieldpath.Set
34+
}
35+
36+
var fixupCases = []fixupTestCase{{
37+
name: "change-struct-to-atomic",
38+
rootTypeName: "v1",
39+
oldSchema: `types:
40+
- name: v1
41+
map:
42+
fields:
43+
- name: struct
44+
type:
45+
namedType: struct
46+
- name: struct
47+
map:
48+
fields:
49+
- name: numeric
50+
type:
51+
scalar: numeric
52+
- name: string
53+
type:
54+
scalar: string
55+
`,
56+
newSchema: `types:
57+
- name: v1
58+
map:
59+
fields:
60+
- name: struct
61+
type:
62+
namedType: struct
63+
- name: struct
64+
map:
65+
fields:
66+
- name: numeric
67+
type:
68+
scalar: numeric
69+
- name: string
70+
type:
71+
scalar: string
72+
elementRelationship: atomic
73+
`,
74+
liveObject: `
75+
struct:
76+
numeric: 1
77+
string: "two"
78+
`,
79+
oldFields: _NS(
80+
_P("struct", "numeric"),
81+
),
82+
fixedFields: _NS(
83+
_P("struct"),
84+
),
85+
}, {
86+
name: "change-list-to-atomic",
87+
rootTypeName: "v1",
88+
oldSchema: `types:
89+
- name: v1
90+
map:
91+
fields:
92+
- name: list
93+
type:
94+
namedType: list
95+
- name: list
96+
list:
97+
elementType:
98+
scalar: string
99+
`,
100+
newSchema: `types:
101+
- name: v1
102+
map:
103+
fields:
104+
- name: list
105+
type:
106+
namedType: list
107+
- name: list
108+
list:
109+
elementType:
110+
scalar: string
111+
elementRelationship: atomic
112+
`,
113+
liveObject: `
114+
list:
115+
- one
116+
- two
117+
`,
118+
oldFields: _NS(
119+
_P("list", _V("one")),
120+
),
121+
fixedFields: _NS(
122+
_P("list"),
123+
),
124+
}}
125+
126+
func TestFixupFieldManagers(t *testing.T) {
127+
for _, tt := range fixupCases {
128+
tt := tt
129+
t.Run(tt.name, func(t *testing.T) {
130+
t.Parallel()
131+
tt.testFixupCase(t)
132+
})
133+
}
134+
}
135+
136+
func (tt fixupTestCase) testFixupCase(t *testing.T) {
137+
parser, err := typed.NewParser(tt.newSchema)
138+
if err != nil {
139+
t.Fatalf("failed to create schema: %v", err)
140+
}
141+
pt := parser.Type(tt.rootTypeName)
142+
liveObject, err := pt.FromYAML(tt.liveObject)
143+
if err != nil {
144+
t.Fatalf("failed to parse/validate yaml: %v\n%v", err, tt.liveObject)
145+
}
146+
147+
fixed, err := typed.FixupFieldManagers(tt.oldFields, liveObject)
148+
if err != nil {
149+
t.Fatalf("fixup errors: %v", err)
150+
}
151+
if !fixed.Equals(tt.fixedFields) {
152+
t.Errorf("expected fieldset %s but got %s", tt.fixedFields.String(), fixed.String())
153+
}
154+
}

0 commit comments

Comments
 (0)