Skip to content

Commit adaddb2

Browse files
jpbetzjpbetz
committed
Add back IgnoredFields
1 parent ccf7a06 commit adaddb2

File tree

3 files changed

+177
-6
lines changed

3 files changed

+177
-6
lines changed

‎internal/fixture/state.go

+6
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,10 @@ type TestCase struct {
540540
ReturnInputOnNoop bool
541541
// IgnoreFilter filters out ignored fields from a fieldpath.Set.
542542
IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter
543+
544+
// IgnoredFields containing the set to ignore for every version.
545+
// IgnoredFields may not be set if IgnoreFilter is set.
546+
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
543547
}
544548

545549
// Test runs the test-case using the given parser and a dummy converter.
@@ -573,6 +577,7 @@ func (tc TestCase) BenchWithConverter(parser Parser, converter merge.Converter)
573577
updaterBuilder := merge.UpdaterBuilder{
574578
Converter: converter,
575579
IgnoreFilter: tc.IgnoreFilter,
580+
IgnoredFields: tc.IgnoredFields,
576581
ReturnInputOnNoop: tc.ReturnInputOnNoop,
577582
}
578583
state := State{
@@ -595,6 +600,7 @@ func (tc TestCase) TestWithConverter(parser Parser, converter merge.Converter) e
595600
updaterBuilder := merge.UpdaterBuilder{
596601
Converter: converter,
597602
IgnoreFilter: tc.IgnoreFilter,
603+
IgnoredFields: tc.IgnoredFields,
598604
ReturnInputOnNoop: tc.ReturnInputOnNoop,
599605
}
600606
state := State{

‎merge/ignore_test.go

+126-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
. "sigs.k8s.io/structured-merge-diff/v4/internal/fixture"
2424
)
2525

26-
func TestIgnoredFields(t *testing.T) {
26+
func TestIgnoreFilter(t *testing.T) {
2727
tests := map[string]TestCase{
2828
"update_does_not_own_ignored": {
2929
APIVersion: "v1",
@@ -148,6 +148,131 @@ func TestIgnoredFields(t *testing.T) {
148148
}
149149
}
150150

151+
func TestIgnoredFields(t *testing.T) {
152+
tests := map[string]TestCase{
153+
"update_does_not_own_ignored": {
154+
APIVersion: "v1",
155+
Ops: []Operation{
156+
Update{
157+
Manager: "default",
158+
APIVersion: "v1",
159+
Object: `
160+
numeric: 1
161+
string: "some string"
162+
`,
163+
},
164+
},
165+
Object: `
166+
numeric: 1
167+
string: "some string"
168+
`,
169+
Managed: fieldpath.ManagedFields{
170+
"default": fieldpath.NewVersionedSet(
171+
_NS(
172+
_P("numeric"),
173+
),
174+
"v1",
175+
false,
176+
),
177+
},
178+
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
179+
"v1": _NS(
180+
_P("string"),
181+
),
182+
},
183+
},
184+
"update_does_not_own_deep_ignored": {
185+
APIVersion: "v1",
186+
Ops: []Operation{
187+
Update{
188+
Manager: "default",
189+
APIVersion: "v1",
190+
Object: `{"numeric": 1, "obj": {"string": "foo", "numeric": 2}}`,
191+
},
192+
},
193+
Object: `{"numeric": 1, "obj": {"string": "foo", "numeric": 2}}`,
194+
Managed: fieldpath.ManagedFields{
195+
"default": fieldpath.NewVersionedSet(
196+
_NS(
197+
_P("numeric"),
198+
),
199+
"v1",
200+
false,
201+
),
202+
},
203+
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
204+
"v1": _NS(
205+
_P("obj"),
206+
),
207+
},
208+
},
209+
"apply_does_not_own_ignored": {
210+
APIVersion: "v1",
211+
Ops: []Operation{
212+
Apply{
213+
Manager: "default",
214+
APIVersion: "v1",
215+
Object: `
216+
numeric: 1
217+
string: "some string"
218+
`,
219+
},
220+
},
221+
Object: `
222+
numeric: 1
223+
string: "some string"
224+
`,
225+
Managed: fieldpath.ManagedFields{
226+
"default": fieldpath.NewVersionedSet(
227+
_NS(
228+
_P("numeric"),
229+
),
230+
"v1",
231+
true,
232+
),
233+
},
234+
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
235+
"v1": _NS(
236+
_P("string"),
237+
),
238+
},
239+
},
240+
"apply_does_not_own_deep_ignored": {
241+
APIVersion: "v1",
242+
Ops: []Operation{
243+
Apply{
244+
Manager: "default",
245+
APIVersion: "v1",
246+
Object: `{"numeric": 1, "obj": {"string": "foo", "numeric": 2}}`,
247+
},
248+
},
249+
Object: `{"numeric": 1, "obj": {"string": "foo", "numeric": 2}}`,
250+
Managed: fieldpath.ManagedFields{
251+
"default": fieldpath.NewVersionedSet(
252+
_NS(
253+
_P("numeric"),
254+
),
255+
"v1",
256+
true,
257+
),
258+
},
259+
IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{
260+
"v1": _NS(
261+
_P("obj"),
262+
),
263+
},
264+
},
265+
}
266+
267+
for name, test := range tests {
268+
t.Run(name, func(t *testing.T) {
269+
if err := test.Test(DeducedParser); err != nil {
270+
t.Fatal("Should fail:", err)
271+
}
272+
})
273+
}
274+
}
275+
151276
func TestFilteredFieldsUsesVersions(t *testing.T) {
152277
tests := map[string]TestCase{
153278
"does_use_ignored_fields_versions": {

‎merge/update.go

+45-5
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ type UpdaterBuilder struct {
3333
Converter Converter
3434
IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter
3535

36+
// IgnoredFields provides a set of fields to ignore for each
37+
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
38+
3639
// Stop comparing the new object with old object after applying.
3740
// This was initially used to avoid spurious etcd update, but
3841
// since that's vastly inefficient, we've come-up with a better
@@ -46,6 +49,7 @@ func (u *UpdaterBuilder) BuildUpdater() *Updater {
4649
return &Updater{
4750
Converter: u.Converter,
4851
IgnoreFilter: u.IgnoreFilter,
52+
IgnoredFields: u.IgnoredFields,
4953
returnInputOnNoop: u.ReturnInputOnNoop,
5054
}
5155
}
@@ -56,6 +60,9 @@ type Updater struct {
5660
// Deprecated: This will eventually become private.
5761
Converter Converter
5862

63+
// Deprecated: This will eventually become private.
64+
IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set
65+
5966
// Deprecated: This will eventually become private.
6067
IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter
6168

@@ -70,8 +77,19 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
7077
return nil, nil, fmt.Errorf("failed to compare objects: %v", err)
7178
}
7279

73-
versions := map[fieldpath.APIVersion]*typed.Comparison{
74-
version: compare.FilterFields(s.IgnoreFilter[version]),
80+
var versions map[fieldpath.APIVersion]*typed.Comparison
81+
82+
if s.IgnoredFields != nil && s.IgnoreFilter != nil {
83+
return nil, nil, fmt.Errorf("IgnoreFilter and IgnoreFilter may not both be set")
84+
}
85+
if s.IgnoredFields != nil {
86+
versions = map[fieldpath.APIVersion]*typed.Comparison{
87+
version: compare.ExcludeFields(s.IgnoredFields[version]),
88+
}
89+
} else {
90+
versions = map[fieldpath.APIVersion]*typed.Comparison{
91+
version: compare.FilterFields(s.IgnoreFilter[version]),
92+
}
7593
}
7694

7795
for manager, managerSet := range managers {
@@ -101,7 +119,12 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa
101119
if err != nil {
102120
return nil, nil, fmt.Errorf("failed to compare objects: %v", err)
103121
}
104-
versions[managerSet.APIVersion()] = compare.FilterFields(s.IgnoreFilter[managerSet.APIVersion()])
122+
123+
if s.IgnoredFields != nil {
124+
versions[managerSet.APIVersion()] = compare.ExcludeFields(s.IgnoredFields[managerSet.APIVersion()])
125+
} else {
126+
versions[managerSet.APIVersion()] = compare.FilterFields(s.IgnoreFilter[managerSet.APIVersion()])
127+
}
105128
}
106129

107130
conflictSet := managerSet.Set().Intersection(compare.Modified.Union(compare.Added))
@@ -154,7 +177,16 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp
154177
managers[manager] = fieldpath.NewVersionedSet(fieldpath.NewSet(), version, false)
155178
}
156179
set := managers[manager].Set().Difference(compare.Removed).Union(compare.Modified).Union(compare.Added)
157-
ignoreFilter := s.IgnoreFilter[version]
180+
181+
if s.IgnoredFields != nil && s.IgnoreFilter != nil {
182+
return nil, nil, fmt.Errorf("IgnoreFilter and IgnoreFilter may not both be set")
183+
}
184+
var ignoreFilter fieldpath.Filter
185+
if s.IgnoredFields != nil {
186+
ignoreFilter = fieldpath.NewExcludeSetFilter(s.IgnoredFields[version])
187+
} else {
188+
ignoreFilter = s.IgnoreFilter[version]
189+
}
158190
if ignoreFilter != nil {
159191
set = ignoreFilter.Filter(set)
160192
}
@@ -189,7 +221,15 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel
189221
return nil, fieldpath.ManagedFields{}, fmt.Errorf("failed to get field set: %v", err)
190222
}
191223

192-
ignoreFilter := s.IgnoreFilter[version]
224+
if s.IgnoredFields != nil && s.IgnoreFilter != nil {
225+
return nil, nil, fmt.Errorf("IgnoreFilter and IgnoreFilter may not both be set")
226+
}
227+
var ignoreFilter fieldpath.Filter
228+
if s.IgnoredFields != nil {
229+
ignoreFilter = fieldpath.NewExcludeSetFilter(s.IgnoredFields[version])
230+
} else {
231+
ignoreFilter = s.IgnoreFilter[version]
232+
}
193233
if ignoreFilter != nil {
194234
set = ignoreFilter.Filter(set)
195235
}

0 commit comments

Comments
 (0)