Skip to content

Commit c68c9ee

Browse files
jpbetzjpbetz
committed
Accept filter instead of exclude mask for ignored fields
1 parent f034ec0 commit c68c9ee

File tree

5 files changed

+81
-51
lines changed

5 files changed

+81
-51
lines changed

Diff for: ‎fieldpath/set.go

+24
Original file line numberDiff line numberDiff line change
@@ -503,3 +503,27 @@ func (s *SetNodeMap) Leaves() *SetNodeMap {
503503
}
504504
return out
505505
}
506+
507+
type Filter interface {
508+
Filter(*Set) *Set
509+
}
510+
511+
func NewExcludeFilter(set *Set) Filter {
512+
return excludeFilter{set}
513+
}
514+
515+
func NewExcludeFilterMap(resetFields map[APIVersion]*Set) map[APIVersion]Filter {
516+
result := make(map[APIVersion]Filter)
517+
for k, v := range resetFields {
518+
result[k] = excludeFilter{v}
519+
}
520+
return result
521+
}
522+
523+
type excludeFilter struct {
524+
exeludeSet *Set
525+
}
526+
527+
func (t excludeFilter) Filter(set *Set) *Set {
528+
return set.RecursiveDifference(t.exeludeSet)
529+
}

Diff for: ‎internal/fixture/state.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -538,8 +538,8 @@ type TestCase struct {
538538
// ReportInputOnNoop if we don't want to compare the output and
539539
// always return it.
540540
ReturnInputOnNoop bool
541-
// IgnoredFields containing the set to ignore for every version
542-
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
541+
// IgnoreFilter filters out ignored fields from a fieldpath.Set.
542+
IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter
543543
}
544544

545545
// Test runs the test-case using the given parser and a dummy converter.
@@ -572,7 +572,7 @@ func (tc TestCase) PreprocessOperations(parser Parser) error {
572572
func (tc TestCase) BenchWithConverter(parser Parser, converter merge.Converter) error {
573573
updaterBuilder := merge.UpdaterBuilder{
574574
Converter: converter,
575-
IgnoredFields: tc.IgnoredFields,
575+
IgnoreFilter: tc.IgnoreFilter,
576576
ReturnInputOnNoop: tc.ReturnInputOnNoop,
577577
}
578578
state := State{
@@ -594,7 +594,7 @@ func (tc TestCase) BenchWithConverter(parser Parser, converter merge.Converter)
594594
func (tc TestCase) TestWithConverter(parser Parser, converter merge.Converter) error {
595595
updaterBuilder := merge.UpdaterBuilder{
596596
Converter: converter,
597-
IgnoredFields: tc.IgnoredFields,
597+
IgnoreFilter: tc.IgnoreFilter,
598598
ReturnInputOnNoop: tc.ReturnInputOnNoop,
599599
}
600600
state := State{

Diff for: ‎merge/ignore_test.go

+28-28
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ func TestIgnoredFields(t *testing.T) {
5050
false,
5151
),
5252
},
53-
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
54-
"v1": _NS(
53+
IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{
54+
"v1": fieldpath.NewExcludeFilter(_NS(
5555
_P("string"),
56-
),
56+
)),
5757
},
5858
},
5959
"update_does_not_own_deep_ignored": {
@@ -75,10 +75,10 @@ func TestIgnoredFields(t *testing.T) {
7575
false,
7676
),
7777
},
78-
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
79-
"v1": _NS(
78+
IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{
79+
"v1": fieldpath.NewExcludeFilter(_NS(
8080
_P("obj"),
81-
),
81+
)),
8282
},
8383
},
8484
"apply_does_not_own_ignored": {
@@ -106,10 +106,10 @@ func TestIgnoredFields(t *testing.T) {
106106
true,
107107
),
108108
},
109-
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
110-
"v1": _NS(
109+
IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{
110+
"v1": fieldpath.NewExcludeFilter(_NS(
111111
_P("string"),
112-
),
112+
)),
113113
},
114114
},
115115
"apply_does_not_own_deep_ignored": {
@@ -131,10 +131,10 @@ func TestIgnoredFields(t *testing.T) {
131131
true,
132132
),
133133
},
134-
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
135-
"v1": _NS(
134+
IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{
135+
"v1": fieldpath.NewExcludeFilter(_NS(
136136
_P("obj"),
137-
),
137+
)),
138138
},
139139
},
140140
}
@@ -148,7 +148,7 @@ func TestIgnoredFields(t *testing.T) {
148148
}
149149
}
150150

151-
func TestIgnoredFieldsUsesVersions(t *testing.T) {
151+
func TestFilteredFieldsUsesVersions(t *testing.T) {
152152
tests := map[string]TestCase{
153153
"does_use_ignored_fields_versions": {
154154
Ops: []Operation{
@@ -205,19 +205,19 @@ func TestIgnoredFieldsUsesVersions(t *testing.T) {
205205
false,
206206
),
207207
},
208-
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
209-
"v1": _NS(
208+
IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{
209+
"v1": fieldpath.NewExcludeFilter(_NS(
210210
_P("mapOfMapsRecursive", "c"),
211-
),
212-
"v2": _NS(
211+
)),
212+
"v2": fieldpath.NewExcludeFilter(_NS(
213213
_P("mapOfMapsRecursive", "cc"),
214-
),
215-
"v3": _NS(
214+
)),
215+
"v3": fieldpath.NewExcludeFilter(_NS(
216216
_P("mapOfMapsRecursive", "ccc"),
217-
),
218-
"v4": _NS(
217+
)),
218+
"v4": fieldpath.NewExcludeFilter(_NS(
219219
_P("mapOfMapsRecursive", "cccc"),
220-
),
220+
)),
221221
},
222222
},
223223
"update_does_not_steal_ignored": {
@@ -273,10 +273,10 @@ func TestIgnoredFieldsUsesVersions(t *testing.T) {
273273
false,
274274
),
275275
},
276-
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
277-
"v2": _NS(
276+
IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{
277+
"v2": fieldpath.NewExcludeFilter(_NS(
278278
_P("mapOfMapsRecursive", "c"),
279-
),
279+
)),
280280
},
281281
},
282282
"apply_does_not_steal_ignored": {
@@ -332,10 +332,10 @@ func TestIgnoredFieldsUsesVersions(t *testing.T) {
332332
false,
333333
),
334334
},
335-
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
336-
"v2": _NS(
335+
IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{
336+
"v2": fieldpath.NewExcludeFilter(_NS(
337337
_P("mapOfMapsRecursive", "c"),
338-
),
338+
)),
339339
},
340340
},
341341
}

Diff for: ‎merge/update.go

+15-19
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ package merge
1515

1616
import (
1717
"fmt"
18-
1918
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
2019
"sigs.k8s.io/structured-merge-diff/v4/typed"
2120
"sigs.k8s.io/structured-merge-diff/v4/value"
@@ -31,8 +30,8 @@ type Converter interface {
3130
// UpdateBuilder allows you to create a new Updater by exposing all of
3231
// the options and setting them once.
3332
type UpdaterBuilder struct {
34-
Converter Converter
35-
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
33+
Converter Converter
34+
IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter
3635

3736
// Stop comparing the new object with old object after applying.
3837
// This was initially used to avoid spurious etcd update, but
@@ -46,7 +45,7 @@ type UpdaterBuilder struct {
4645
func (u *UpdaterBuilder) BuildUpdater() *Updater {
4746
return &Updater{
4847
Converter: u.Converter,
49-
IgnoredFields: u.IgnoredFields,
48+
IgnoreFilter: u.IgnoreFilter,
5049
returnInputOnNoop: u.ReturnInputOnNoop,
5150
}
5251
}
@@ -58,7 +57,7 @@ type Updater struct {
5857
Converter Converter
5958

6059
// Deprecated: This will eventually become private.
61-
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
60+
IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter
6261

6362
returnInputOnNoop bool
6463
}
@@ -72,7 +71,7 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
7271
}
7372

7473
versions := map[fieldpath.APIVersion]*typed.Comparison{
75-
version: compare.ExcludeFields(s.IgnoredFields[version]),
74+
version: compare.FilterFields(s.IgnoreFilter[version]),
7675
}
7776

7877
for manager, managerSet := range managers {
@@ -102,7 +101,7 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
102101
if err != nil {
103102
return nil, nil, fmt.Errorf("failed to compare objects: %v", err)
104103
}
105-
versions[managerSet.APIVersion()] = compare.ExcludeFields(s.IgnoredFields[managerSet.APIVersion()])
104+
versions[managerSet.APIVersion()] = compare.FilterFields(s.IgnoreFilter[managerSet.APIVersion()])
106105
}
107106

108107
conflictSet := managerSet.Set().Intersection(compare.Modified.Union(compare.Added))
@@ -154,13 +153,14 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
154153
if _, ok := managers[manager]; !ok {
155154
managers[manager] = fieldpath.NewVersionedSet(fieldpath.NewSet(), version, false)
156155
}
157-
158-
ignored := s.IgnoredFields[version]
159-
if ignored == nil {
160-
ignored = fieldpath.NewSet()
156+
set := managers[manager].Set().Difference(compare.Removed).Union(compare.Modified).Union(compare.Added)
157+
ignoreFilter := s.IgnoreFilter[version]
158+
if ignoreFilter != nil {
159+
set = ignoreFilter.Filter(set)
161160
}
161+
162162
managers[manager] = fieldpath.NewVersionedSet(
163-
managers[manager].Set().Difference(compare.Removed).Union(compare.Modified).Union(compare.Added).RecursiveDifference(ignored),
163+
set,
164164
version,
165165
false,
166166
)
@@ -189,13 +189,9 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel
189189
return nil, fieldpath.ManagedFields{}, fmt.Errorf("failed to get field set: %v", err)
190190
}
191191

192-
ignored := s.IgnoredFields[version]
193-
if ignored != nil {
194-
set = set.RecursiveDifference(ignored)
195-
// TODO: is this correct. If we don't remove from lastSet pruning might remove the fields?
196-
if lastSet != nil {
197-
lastSet.Set().RecursiveDifference(ignored)
198-
}
192+
ignoreFilter := s.IgnoreFilter[version]
193+
if ignoreFilter != nil {
194+
set = ignoreFilter.Filter(set)
199195
}
200196
managers[manager] = fieldpath.NewVersionedSet(set, version, true)
201197
newObject, err = s.prune(newObject, managers, manager, lastSet)

Diff for: ‎typed/compare.go

+10
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,16 @@ func (c *Comparison) ExcludeFields(fields *fieldpath.Set) *Comparison {
7272
return c
7373
}
7474

75+
func (c *Comparison) FilterFields(filter fieldpath.Filter) *Comparison {
76+
if filter == nil {
77+
return c
78+
}
79+
c.Removed = filter.Filter(c.Removed)
80+
c.Modified = filter.Filter(c.Modified)
81+
c.Added = filter.Filter(c.Added)
82+
return c
83+
}
84+
7585
type compareWalker struct {
7686
lhs value.Value
7787
rhs value.Value

0 commit comments

Comments
 (0)