diff --git a/fieldpath/set.go b/fieldpath/set.go index 6d182768..77ae2511 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -17,6 +17,8 @@ limitations under the License. package fieldpath import ( + "fmt" + "sigs.k8s.io/structured-merge-diff/v4/value" "sort" "strings" @@ -136,6 +138,198 @@ func (s *Set) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.TypeRef) } } +// MakePrefixMatcherOrDie is the same as PrefixMatcher except it panics if parts can't be +// turned into a SetMatcher. +func MakePrefixMatcherOrDie(parts ...interface{}) *SetMatcher { + result, err := PrefixMatcher(parts...) + if err != nil { + panic(err) + } + return result +} + +// PrefixMatcher creates a SetMatcher that matches all field paths prefixed by the given list of matcher path parts. +// The matcher parts may any of: +// +// - PathElementMatcher - for wildcards, `MatchAnyPathElement()` can be used as well. +// - PathElement - for any path element +// - value.FieldList - for listMap keys +// - value.Value - for scalar list elements +// - string - For field names +// - int - for array indices +func PrefixMatcher(parts ...interface{}) (*SetMatcher, error) { + current := MatchAnySet() // match all field path suffixes + for i := len(parts) - 1; i >= 0; i-- { + part := parts[i] + var pattern PathElementMatcher + switch t := part.(type) { + case PathElementMatcher: + // any path matcher, including wildcard + pattern = t + case PathElement: + // any path element + pattern = PathElementMatcher{PathElement: t} + case *value.FieldList: + // a listMap key + if len(*t) == 0 { + return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)") + } + pattern = PathElementMatcher{PathElement: PathElement{Key: t}} + case value.Value: + // a scalar or set-type list element + pattern = PathElementMatcher{PathElement: PathElement{Value: &t}} + case string: + // a plain field name + pattern = PathElementMatcher{PathElement: PathElement{FieldName: &t}} + case int: + // a plain list index + pattern = PathElementMatcher{PathElement: PathElement{Index: &t}} + default: + return nil, fmt.Errorf("unexpected type %T", t) + } + current = &SetMatcher{ + members: []*SetMemberMatcher{{ + Path: pattern, + Child: current, + }}, + } + } + return current, nil +} + +// MatchAnyPathElement returns a PathElementMatcher that matches any path element. +func MatchAnyPathElement() PathElementMatcher { + return PathElementMatcher{Wildcard: true} +} + +// MatchAnySet returns a SetMatcher that matches any set. +func MatchAnySet() *SetMatcher { + return &SetMatcher{wildcard: true} +} + +// NewSetMatcher returns a new SetMatcher. +// Wildcard members take precedent over non-wildcard members; +// all non-wildcard members are ignored if there is a wildcard members. +func NewSetMatcher(wildcard bool, members ...*SetMemberMatcher) *SetMatcher { + sort.Sort(sortedMemberMatcher(members)) + return &SetMatcher{wildcard: wildcard, members: members} +} + +// SetMatcher defines a matcher that matches fields in a Set. +// SetMatcher is structured much like a Set but with wildcard support. +type SetMatcher struct { + // wildcard indicates that all members and children are included in the match. + // If set, the members field is ignored. + wildcard bool + // members provides patterns to match the members of a Set. + // Wildcard members are sorted before non-wildcards and take precedent over + // non-wildcard members. + members sortedMemberMatcher +} + +type sortedMemberMatcher []*SetMemberMatcher + +func (s sortedMemberMatcher) Len() int { return len(s) } +func (s sortedMemberMatcher) Less(i, j int) bool { return s[i].Path.Less(s[j].Path) } +func (s sortedMemberMatcher) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s sortedMemberMatcher) Find(p PathElementMatcher) (location int, ok bool) { + return sort.Find(len(s), func(i int) int { + return s[i].Path.Compare(p) + }) +} + +// Merge merges s and s2 and returns a SetMatcher that matches all field paths matched by either s or s2. +// During the merge, members of s and s2 with the same PathElementMatcher merged into a single member +// with the children of each merged by calling this function recursively. +func (s *SetMatcher) Merge(s2 *SetMatcher) *SetMatcher { + if s.wildcard || s2.wildcard { + return NewSetMatcher(true) + } + merged := make(sortedMemberMatcher, len(s.members), len(s.members)+len(s2.members)) + copy(merged, s.members) + for _, m := range s2.members { + if i, ok := s.members.Find(m.Path); ok { + // since merged is a shallow copy, do not modify elements in place + merged[i] = &SetMemberMatcher{ + Path: merged[i].Path, + Child: merged[i].Child.Merge(m.Child), + } + } else { + merged = append(merged, m) + } + } + return NewSetMatcher(false, merged...) // sort happens here +} + +// SetMemberMatcher defines a matcher that matches the members of a Set. +// SetMemberMatcher is structured much like the elements of a SetNodeMap, but +// with wildcard support. +type SetMemberMatcher struct { + // Path provides a matcher to match members of a Set. + // If Path is a wildcard, all members of a Set are included in the match. + // Otherwise, if any Path is Equal to a member of a Set, that member is + // included in the match and the children of that member are matched + // against the Child matcher. + Path PathElementMatcher + + // Child provides a matcher to use for the children of matched members of a Set. + Child *SetMatcher +} + +// PathElementMatcher defined a path matcher for a PathElement. +type PathElementMatcher struct { + // Wildcard indicates that all PathElements are matched by this matcher. + // If set, PathElement is ignored. + Wildcard bool + + // PathElement indicates that a PathElement is matched if it is Equal + // to this PathElement. + PathElement +} + +func (p PathElementMatcher) Equals(p2 PathElementMatcher) bool { + return p.Wildcard != p2.Wildcard && p.PathElement.Equals(p2.PathElement) +} + +func (p PathElementMatcher) Less(p2 PathElementMatcher) bool { + if p.Wildcard && !p2.Wildcard { + return true + } else if p2.Wildcard { + return false + } + return p.PathElement.Less(p2.PathElement) +} + +func (p PathElementMatcher) Compare(p2 PathElementMatcher) int { + if p.Wildcard && !p2.Wildcard { + return -1 + } else if p2.Wildcard { + return 1 + } + return p.PathElement.Compare(p2.PathElement) +} + +// FilterIncludeMatches returns a Set with only the field paths that match. +func (s *Set) FilterIncludeMatches(pattern *SetMatcher) *Set { + if pattern.wildcard { + return s + } + + members := PathElementSet{} + for _, m := range s.Members.members { + for _, pm := range pattern.members { + if pm.Path.Wildcard || pm.Path.PathElement.Equals(m) { + members.Insert(m) + break + } + } + } + return &Set{ + Members: members, + Children: *s.Children.FilterIncludeMatches(pattern), + } +} + // Size returns the number of members of the set. func (s *Set) Size() int { return s.Members.Size() + s.Children.Size() @@ -476,6 +670,33 @@ func (s *SetNodeMap) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.Ty } } +// FilterIncludeMatches returns a SetNodeMap with only the field paths that match the matcher. +func (s *SetNodeMap) FilterIncludeMatches(pattern *SetMatcher) *SetNodeMap { + if pattern.wildcard { + return s + } + + var out sortedSetNode + for _, member := range s.members { + for _, c := range pattern.members { + if c.Path.Wildcard || c.Path.PathElement.Equals(member.pathElement) { + childSet := member.set.FilterIncludeMatches(c.Child) + if childSet.Size() > 0 { + out = append(out, setNode{ + pathElement: member.pathElement, + set: childSet, + }) + } + break + } + } + } + + return &SetNodeMap{ + members: out, + } +} + // Iterate calls f for each PathElement in the set. func (s *SetNodeMap) Iterate(f func(PathElement)) { for _, n := range s.members { @@ -503,3 +724,59 @@ func (s *SetNodeMap) Leaves() *SetNodeMap { } return out } + +// Filter defines an interface for excluding field paths from a set. +// NewExcludeSetFilter can be used to create a filter that removes +// specific field paths and all of their children. +// NewIncludeMatcherFilter can be used to create a filter that removes all fields except +// the fields that match a field path matcher. PrefixMatcher and MakePrefixMatcherOrDie +// can be used to define field path patterns. +type Filter interface { + // Filter returns a filtered copy of the set. + Filter(*Set) *Set +} + +// NewExcludeSetFilter returns a filter that removes field paths in the exclude set. +func NewExcludeSetFilter(exclude *Set) Filter { + return excludeFilter{exclude} +} + +// NewExcludeFilterSetMap converts a map of APIVersion to exclude set to a map of APIVersion to exclude filters. +func NewExcludeFilterSetMap(resetFields map[APIVersion]*Set) map[APIVersion]Filter { + result := make(map[APIVersion]Filter) + for k, v := range resetFields { + result[k] = excludeFilter{v} + } + return result +} + +type excludeFilter struct { + excludeSet *Set +} + +func (t excludeFilter) Filter(set *Set) *Set { + return set.RecursiveDifference(t.excludeSet) +} + +// NewIncludeMatcherFilter returns a filter that only includes field paths that match. +// If no matchers are provided, the filter includes all field paths. +// PrefixMatcher and MakePrefixMatcherOrDie can help create basic matcher. +func NewIncludeMatcherFilter(matchers ...*SetMatcher) Filter { + if len(matchers) == 0 { + return includeMatcherFilter{&SetMatcher{wildcard: true}} + } + matcher := matchers[0] + for i := 1; i < len(matchers); i++ { + matcher = matcher.Merge(matchers[i]) + } + + return includeMatcherFilter{matcher} +} + +type includeMatcherFilter struct { + matcher *SetMatcher +} + +func (pf includeMatcherFilter) Filter(set *Set) *Set { + return set.FilterIncludeMatches(pf.matcher) +} diff --git a/fieldpath/set_test.go b/fieldpath/set_test.go index cbb645e5..7efb374e 100644 --- a/fieldpath/set_test.go +++ b/fieldpath/set_test.go @@ -20,6 +20,7 @@ import ( "bytes" "fmt" "math/rand" + "sigs.k8s.io/structured-merge-diff/v4/value" "testing" "sigs.k8s.io/structured-merge-diff/v4/schema" @@ -755,3 +756,257 @@ func TestSetNodeMapIterate(t *testing.T) { } } } + +func TestFilterByPattern(t *testing.T) { + testCases := []struct { + name string + input *Set + expect *Set + filter Filter + }{ + { + name: "container resize fields: exact match", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + MakePathOrDie("spec", "containers", 0), + MakePathOrDie("spec", "containers", 0, "resources"), + MakePathOrDie("spec", "containers", 0, "resources", "limits"), + MakePathOrDie("spec", "containers", 0, "resources", "limits", "cpu"), + MakePathOrDie("spec", "containers", 0, "resources", "requests"), + MakePathOrDie("spec", "containers", 0, "resources", "requests", "cpu"), + MakePathOrDie("spec", "containers", 0, "resources", "claims"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "request"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "request"), + MakePathOrDie("spec", "containers", 1), + MakePathOrDie("spec", "containers", 1, "resources"), + MakePathOrDie("spec", "containers", 1, "resources", "limits"), + MakePathOrDie("spec", "containers", 1, "resources", "limits", "cpu"), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "containers", MatchAnyPathElement(), "resources")), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + MakePathOrDie("spec", "containers", 0), + MakePathOrDie("spec", "containers", 0, "resources"), + MakePathOrDie("spec", "containers", 0, "resources", "limits"), + MakePathOrDie("spec", "containers", 0, "resources", "limits", "cpu"), + MakePathOrDie("spec", "containers", 0, "resources", "requests"), + MakePathOrDie("spec", "containers", 0, "resources", "requests", "cpu"), + MakePathOrDie("spec", "containers", 0, "resources", "claims"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "request"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "request"), + MakePathOrDie("spec", "containers", 1), + MakePathOrDie("spec", "containers", 1, "resources"), + MakePathOrDie("spec", "containers", 1, "resources", "limits"), + MakePathOrDie("spec", "containers", 1, "resources", "limits", "cpu"), + ), + }, + { + name: "container resize fields: filter status and metadata", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("status"), + MakePathOrDie("metadata"), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "containers", MatchAnyPathElement(), "resources")), + expect: NewSet( + MakePathOrDie("spec"), + ), + }, + { + name: "container resize fields: filter non-container spec fields", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "volumes"), + MakePathOrDie("spec", "hostNetwork"), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "containers", MatchAnyPathElement(), "resources")), + expect: NewSet( + MakePathOrDie("spec"), + ), + }, + { + name: "container resize fields: filter non-resource container fields", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + MakePathOrDie("spec", "containers", 0), + MakePathOrDie("spec", "containers", 0, "image"), + MakePathOrDie("spec", "containers", 0, "workingDir"), + MakePathOrDie("spec", "containers", 0, "resources"), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "containers", MatchAnyPathElement(), "resources")), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + MakePathOrDie("spec", "containers", 0), + MakePathOrDie("spec", "containers", 0, "resources"), + ), + }, + { + name: "filter listMap key", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "listMap", + &value.FieldList{ + {Name: "key1", Value: value.NewValueInterface("value1")}, + {Name: "key2", Value: value.NewValueInterface("value2")}, + }), + MakePathOrDie("spec", "listMap", + &value.FieldList{ + {Name: "key1", Value: value.NewValueInterface("value1")}, + {Name: "key2", Value: value.NewValueInterface("value2")}, + }, "field"), + MakePathOrDie("spec", "listMap", + &value.FieldList{ + {Name: "key1", Value: value.NewValueInterface("valueX")}, + {Name: "key2", Value: value.NewValueInterface("valueY")}, + }, "field"), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "listMap", &value.FieldList{ + {Name: "key1", Value: value.NewValueInterface("value1")}, + {Name: "key2", Value: value.NewValueInterface("value2")}, + })), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "listMap", + &value.FieldList{ + {Name: "key1", Value: value.NewValueInterface("value1")}, + {Name: "key2", Value: value.NewValueInterface("value2")}, + }), + MakePathOrDie("spec", "listMap", + &value.FieldList{ + {Name: "key1", Value: value.NewValueInterface("value1")}, + {Name: "key2", Value: value.NewValueInterface("value2")}, + }, "field"), + ), + }, + { + name: "filter value", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "set", value.NewValueInterface("v1")), + MakePathOrDie("spec", "set", value.NewValueInterface("v2")), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "set", value.NewValueInterface("v1"))), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "set", value.NewValueInterface("v1")), + ), + }, + { + name: "filter by index", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list"), + MakePathOrDie("spec", "list", 0), + MakePathOrDie("spec", "list", 0, "value"), + MakePathOrDie("spec", "list", 1), + MakePathOrDie("spec", "list", 1, "value"), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "list", 1, "value")), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list"), + MakePathOrDie("spec", "list", 1), + MakePathOrDie("spec", "list", 1, "value"), + ), + }, + { + name: "multiple index matchers", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list"), + MakePathOrDie("spec", "list", 0), + MakePathOrDie("spec", "list", 0, "value"), + MakePathOrDie("spec", "list", 1), + MakePathOrDie("spec", "list", 1, "value"), + MakePathOrDie("spec", "list", 2), + MakePathOrDie("spec", "list", 2, "value"), + ), + filter: NewIncludeMatcherFilter( + MakePrefixMatcherOrDie("spec", "list", 0, "value"), + MakePrefixMatcherOrDie("spec", "list", 1, "value"), + ), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list"), + MakePathOrDie("spec", "list", 0), + MakePathOrDie("spec", "list", 0, "value"), + MakePathOrDie("spec", "list", 1), + MakePathOrDie("spec", "list", 1, "value"), + ), + }, + { + name: "multiple field matchers", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "f1"), + MakePathOrDie("spec", "f1", "f11"), + MakePathOrDie("spec", "f2"), + MakePathOrDie("spec", "f2", "f21"), + MakePathOrDie("spec", "f3"), + MakePathOrDie("spec", "f3", "f31"), + ), + filter: NewIncludeMatcherFilter( + MakePrefixMatcherOrDie("spec", "f1"), + MakePrefixMatcherOrDie("spec", "f3"), + ), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "f1"), + MakePathOrDie("spec", "f1", "f11"), + MakePathOrDie("spec", "f3"), + MakePathOrDie("spec", "f3", "f31"), + ), + }, + { + name: "wildcard takes precedence", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list"), + MakePathOrDie("spec", "list", 0), + MakePathOrDie("spec", "list", 0, "f1"), + MakePathOrDie("spec", "list", 0, "f2"), + MakePathOrDie("spec", "list", 1), + MakePathOrDie("spec", "list", 1, "f1"), + MakePathOrDie("spec", "list", 1, "f2"), + MakePathOrDie("spec", "list", 2), + MakePathOrDie("spec", "list", 2, "f1"), + MakePathOrDie("spec", "list", 2, "f2"), + ), + filter: NewIncludeMatcherFilter( + MakePrefixMatcherOrDie("spec", "list", MatchAnyPathElement(), "f1"), // takes precedence + MakePrefixMatcherOrDie("spec", "list", 1, "f2"), // ignored + ), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list"), + MakePathOrDie("spec", "list", 0), + MakePathOrDie("spec", "list", 0, "f1"), + MakePathOrDie("spec", "list", 1), + MakePathOrDie("spec", "list", 1, "f1"), + MakePathOrDie("spec", "list", 2), + MakePathOrDie("spec", "list", 2, "f1"), + ), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + filtered := tc.filter.Filter(tc.input) + if !filtered.Equals(tc.expect) { + t.Errorf("Expected:\n%v\n\nbut got:\n%v", tc.expect, filtered) + } + }) + } +} diff --git a/internal/fixture/state.go b/internal/fixture/state.go index 5b5d4829..039125b2 100644 --- a/internal/fixture/state.go +++ b/internal/fixture/state.go @@ -538,8 +538,8 @@ type TestCase struct { // ReportInputOnNoop if we don't want to compare the output and // always return it. ReturnInputOnNoop bool - // IgnoredFields containing the set to ignore for every version - IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set + // IgnoreFilter filters out ignored fields from a fieldpath.Set. + IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter } // Test runs the test-case using the given parser and a dummy converter. @@ -572,7 +572,7 @@ func (tc TestCase) PreprocessOperations(parser Parser) error { func (tc TestCase) BenchWithConverter(parser Parser, converter merge.Converter) error { updaterBuilder := merge.UpdaterBuilder{ Converter: converter, - IgnoredFields: tc.IgnoredFields, + IgnoreFilter: tc.IgnoreFilter, ReturnInputOnNoop: tc.ReturnInputOnNoop, } state := State{ @@ -594,7 +594,7 @@ func (tc TestCase) BenchWithConverter(parser Parser, converter merge.Converter) func (tc TestCase) TestWithConverter(parser Parser, converter merge.Converter) error { updaterBuilder := merge.UpdaterBuilder{ Converter: converter, - IgnoredFields: tc.IgnoredFields, + IgnoreFilter: tc.IgnoreFilter, ReturnInputOnNoop: tc.ReturnInputOnNoop, } state := State{ diff --git a/merge/ignore_test.go b/merge/ignore_test.go index 4cf7a918..5a8b4200 100644 --- a/merge/ignore_test.go +++ b/merge/ignore_test.go @@ -50,10 +50,10 @@ func TestIgnoredFields(t *testing.T) { false, ), }, - IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{ - "v1": _NS( + IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("string"), - ), + )), }, }, "update_does_not_own_deep_ignored": { @@ -75,10 +75,10 @@ func TestIgnoredFields(t *testing.T) { false, ), }, - IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{ - "v1": _NS( + IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("obj"), - ), + )), }, }, "apply_does_not_own_ignored": { @@ -106,10 +106,10 @@ func TestIgnoredFields(t *testing.T) { true, ), }, - IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{ - "v1": _NS( + IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("string"), - ), + )), }, }, "apply_does_not_own_deep_ignored": { @@ -131,10 +131,10 @@ func TestIgnoredFields(t *testing.T) { true, ), }, - IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{ - "v1": _NS( + IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("obj"), - ), + )), }, }, } @@ -148,7 +148,7 @@ func TestIgnoredFields(t *testing.T) { } } -func TestIgnoredFieldsUsesVersions(t *testing.T) { +func TestFilteredFieldsUsesVersions(t *testing.T) { tests := map[string]TestCase{ "does_use_ignored_fields_versions": { Ops: []Operation{ @@ -205,19 +205,19 @@ func TestIgnoredFieldsUsesVersions(t *testing.T) { false, ), }, - IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{ - "v1": _NS( + IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "c"), - ), - "v2": _NS( + )), + "v2": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "cc"), - ), - "v3": _NS( + )), + "v3": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "ccc"), - ), - "v4": _NS( + )), + "v4": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "cccc"), - ), + )), }, }, "update_does_not_steal_ignored": { @@ -273,10 +273,10 @@ func TestIgnoredFieldsUsesVersions(t *testing.T) { false, ), }, - IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{ - "v2": _NS( + IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ + "v2": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "c"), - ), + )), }, }, "apply_does_not_steal_ignored": { @@ -332,10 +332,10 @@ func TestIgnoredFieldsUsesVersions(t *testing.T) { false, ), }, - IgnoredFields: map[fieldpath.APIVersion]*fieldpath.Set{ - "v2": _NS( + IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ + "v2": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "c"), - ), + )), }, }, } diff --git a/merge/update.go b/merge/update.go index d5a977d6..34ab2d6f 100644 --- a/merge/update.go +++ b/merge/update.go @@ -15,7 +15,6 @@ package merge import ( "fmt" - "sigs.k8s.io/structured-merge-diff/v4/fieldpath" "sigs.k8s.io/structured-merge-diff/v4/typed" "sigs.k8s.io/structured-merge-diff/v4/value" @@ -31,8 +30,8 @@ type Converter interface { // UpdateBuilder allows you to create a new Updater by exposing all of // the options and setting them once. type UpdaterBuilder struct { - Converter Converter - IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set + Converter Converter + IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter // Stop comparing the new object with old object after applying. // This was initially used to avoid spurious etcd update, but @@ -46,7 +45,7 @@ type UpdaterBuilder struct { func (u *UpdaterBuilder) BuildUpdater() *Updater { return &Updater{ Converter: u.Converter, - IgnoredFields: u.IgnoredFields, + IgnoreFilter: u.IgnoreFilter, returnInputOnNoop: u.ReturnInputOnNoop, } } @@ -58,7 +57,7 @@ type Updater struct { Converter Converter // Deprecated: This will eventually become private. - IgnoredFields map[fieldpath.APIVersion]*fieldpath.Set + IgnoreFilter map[fieldpath.APIVersion]fieldpath.Filter returnInputOnNoop bool } @@ -72,7 +71,7 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa } versions := map[fieldpath.APIVersion]*typed.Comparison{ - version: compare.ExcludeFields(s.IgnoredFields[version]), + version: compare.FilterFields(s.IgnoreFilter[version]), } for manager, managerSet := range managers { @@ -102,7 +101,7 @@ func (s *Updater) update(oldObject, newObject *typed.TypedValue, version fieldpa if err != nil { return nil, nil, fmt.Errorf("failed to compare objects: %v", err) } - versions[managerSet.APIVersion()] = compare.ExcludeFields(s.IgnoredFields[managerSet.APIVersion()]) + versions[managerSet.APIVersion()] = compare.FilterFields(s.IgnoreFilter[managerSet.APIVersion()]) } conflictSet := managerSet.Set().Intersection(compare.Modified.Union(compare.Added)) @@ -154,13 +153,14 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp if _, ok := managers[manager]; !ok { managers[manager] = fieldpath.NewVersionedSet(fieldpath.NewSet(), version, false) } - - ignored := s.IgnoredFields[version] - if ignored == nil { - ignored = fieldpath.NewSet() + set := managers[manager].Set().Difference(compare.Removed).Union(compare.Modified).Union(compare.Added) + ignoreFilter := s.IgnoreFilter[version] + if ignoreFilter != nil { + set = ignoreFilter.Filter(set) } + managers[manager] = fieldpath.NewVersionedSet( - managers[manager].Set().Difference(compare.Removed).Union(compare.Modified).Union(compare.Added).RecursiveDifference(ignored), + set, version, false, ) @@ -189,13 +189,9 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel return nil, fieldpath.ManagedFields{}, fmt.Errorf("failed to get field set: %v", err) } - ignored := s.IgnoredFields[version] - if ignored != nil { - set = set.RecursiveDifference(ignored) - // TODO: is this correct. If we don't remove from lastSet pruning might remove the fields? - if lastSet != nil { - lastSet.Set().RecursiveDifference(ignored) - } + ignoreFilter := s.IgnoreFilter[version] + if ignoreFilter != nil { + set = ignoreFilter.Filter(set) } managers[manager] = fieldpath.NewVersionedSet(set, version, true) newObject, err = s.prune(newObject, managers, manager, lastSet) diff --git a/typed/compare.go b/typed/compare.go index ed483cbb..5fffa5e2 100644 --- a/typed/compare.go +++ b/typed/compare.go @@ -72,6 +72,16 @@ func (c *Comparison) ExcludeFields(fields *fieldpath.Set) *Comparison { return c } +func (c *Comparison) FilterFields(filter fieldpath.Filter) *Comparison { + if filter == nil { + return c + } + c.Removed = filter.Filter(c.Removed) + c.Modified = filter.Filter(c.Modified) + c.Added = filter.Filter(c.Added) + return c +} + type compareWalker struct { lhs value.Value rhs value.Value