From c68c9ee5bc5f9b73b5785e38a6a28a6c5b654572 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 28 Oct 2024 20:02:41 -0400 Subject: [PATCH 1/7] Accept filter instead of exclude mask for ignored fields --- fieldpath/set.go | 24 +++++++++++++++++ internal/fixture/state.go | 8 +++--- merge/ignore_test.go | 56 +++++++++++++++++++-------------------- merge/update.go | 34 +++++++++++------------- typed/compare.go | 10 +++++++ 5 files changed, 81 insertions(+), 51 deletions(-) diff --git a/fieldpath/set.go b/fieldpath/set.go index 6d182768..eb5d184f 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -503,3 +503,27 @@ func (s *SetNodeMap) Leaves() *SetNodeMap { } return out } + +type Filter interface { + Filter(*Set) *Set +} + +func NewExcludeFilter(set *Set) Filter { + return excludeFilter{set} +} + +func NewExcludeFilterMap(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 { + exeludeSet *Set +} + +func (t excludeFilter) Filter(set *Set) *Set { + return set.RecursiveDifference(t.exeludeSet) +} 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..35d6b805 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.NewExcludeFilter(_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.NewExcludeFilter(_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.NewExcludeFilter(_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.NewExcludeFilter(_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.NewExcludeFilter(_NS( _P("mapOfMapsRecursive", "c"), - ), - "v2": _NS( + )), + "v2": fieldpath.NewExcludeFilter(_NS( _P("mapOfMapsRecursive", "cc"), - ), - "v3": _NS( + )), + "v3": fieldpath.NewExcludeFilter(_NS( _P("mapOfMapsRecursive", "ccc"), - ), - "v4": _NS( + )), + "v4": fieldpath.NewExcludeFilter(_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.NewExcludeFilter(_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.NewExcludeFilter(_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 From 083ce27d38181e56449fcd4b75f331cefdfb3919 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 29 Oct 2024 20:24:52 -0400 Subject: [PATCH 2/7] Add field path pattern matching --- fieldpath/set.go | 164 ++++++++++++++++++++++++++++++++++++++++-- fieldpath/set_test.go | 93 ++++++++++++++++++++++++ 2 files changed, 253 insertions(+), 4 deletions(-) diff --git a/fieldpath/set.go b/fieldpath/set.go index eb5d184f..94235895 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -17,6 +17,7 @@ limitations under the License. package fieldpath import ( + "fmt" "sort" "strings" @@ -136,6 +137,111 @@ func (s *Set) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.TypeRef) } } +// MustPrefixPattern is the same as PrefixPattern except it panics if parts can't be +// turned into a SetPattern. +func MustPrefixPattern(parts ...interface{}) *SetPattern { + result, err := PrefixPattern(parts...) + if err != nil { + panic(err) + } + return result +} + +// PrefixPattern creates a SetPattern that matches all field paths prefixed by the given list of path parts. +// The parts may be PathPatterns, PathElements, strings (for field names) or ints (for array indices). +// `MatchAnyPathElement()` may be used to "wildcard" match any PathElement at that position in the field path. +func PrefixPattern(parts ...interface{}) (*SetPattern, error) { + current := MatchAnySet() // match all field patch suffixes + for i := len(parts) - 1; i >= 0; i-- { + part := parts[i] + var pattern PathPattern + switch t := part.(type) { + case PathPattern: + pattern = t + case PathElement: + pattern = PathPattern{PathElement: t} + case string: + pattern = PathPattern{PathElement: PathElement{FieldName: &t}} + case int: + pattern = PathPattern{PathElement: PathElement{Index: &t}} + default: + return nil, fmt.Errorf("unexpected type %T", t) + } + current = &SetPattern{ + Members: []*MemberSetPattern{{ + Path: pattern, + Child: current, + }}, + } + } + return current, nil +} + +// MatchAnyPathElement returns a PathPattern that matches any path element. +func MatchAnyPathElement() PathPattern { + return PathPattern{Wildcard: true} +} + +// MatchAnySet returns a SetPattern that matches any set. +func MatchAnySet() *SetPattern { + return &SetPattern{Wildcard: true} +} + +// SetPattern defines a pattern that matches fields in a Set. +// SetPattern is structured much like a Set but with wildcard support. +type SetPattern struct { + // Wildcard indicates that all members and children are matched. + // If set, the Members and Children fields are ignored. + Wildcard bool + // Members provides patterns to match the Members of a Set. + // If any PatchPattern is a wildcard, then all members of a Set are matched. + // Otherwise, if any PathPattern is Equal to a member of a Set, that member is matched. + Members []*MemberSetPattern +} + +// MemberSetPattern defines a pattern that matches the Members of a Set. +// MemberSetPattern is structured much like the elements of a SetNodeMap, but with wildcard support. +type MemberSetPattern struct { + // Path provides a pattern to match Members of a Set. + // If Path is a wildcard, all Members of a Set are matched. + // Otherwise, the Member of a Set with a path that is Equal to this Path is matched. + Path PathPattern + + // Child provides a pattern to use for Member of a Set that were matched by this MemberSetPattern's Path. + Child *SetPattern +} + +// PathPattern defined a match pattern for a PathElement. +type PathPattern struct { + // Wildcard indicates that all PathElements are matched by this pattern. + // If set, PathElement is ignored. + Wildcard bool + + // PathElement matches another PathElement if it is Equal to this PathElement. + PathElement +} + +// FilterByPattern returns a Set with only fields that match the pattern. +func (s *Set) FilterByPattern(pattern *SetPattern) *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.FilterByPattern(pattern), + } +} + // Size returns the number of members of the set. func (s *Set) Size() int { return s.Members.Size() + s.Children.Size() @@ -476,6 +582,33 @@ func (s *SetNodeMap) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.Ty } } +// FilterByPattern returns a set that is filtered by the pattern. +func (s *SetNodeMap) FilterByPattern(pattern *SetPattern) *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.FilterByPattern(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 { @@ -504,14 +637,23 @@ func (s *SetNodeMap) Leaves() *SetNodeMap { return out } +// Filter defines an interface for filtering Set. +// NewExcludeFilter can be used to create a filter that removes fields at the +// excluded field paths. +// NewPatternFilter can be used to create a filter that removes all fields except +// the fields that match a field path pattern. PrefixPattern and MustPrefixPattern +// can help create field path patterns. type Filter interface { + // Filter returns a filtered copy of the set. Filter(*Set) *Set } -func NewExcludeFilter(set *Set) Filter { - return excludeFilter{set} +// NewExcludeFilter returns a filter that removes field paths in the exclude set. +func NewExcludeFilter(exclude *Set) Filter { + return excludeFilter{exclude} } +// NewExcludeFilterMap converts a map of APIVersion to exclude set to a map of APIVersion to exclude filters. func NewExcludeFilterMap(resetFields map[APIVersion]*Set) map[APIVersion]Filter { result := make(map[APIVersion]Filter) for k, v := range resetFields { @@ -521,9 +663,23 @@ func NewExcludeFilterMap(resetFields map[APIVersion]*Set) map[APIVersion]Filter } type excludeFilter struct { - exeludeSet *Set + excludeSet *Set } func (t excludeFilter) Filter(set *Set) *Set { - return set.RecursiveDifference(t.exeludeSet) + return set.RecursiveDifference(t.excludeSet) +} + +// NewPatternFilter returns a filter that only includes field paths that match the pattern. +// PrefixPattern and MustPrefixPattern can help create basic SetPatterns. +func NewPatternFilter(pattern *SetPattern) Filter { + return patternFilter{pattern} +} + +type patternFilter struct { + pattern *SetPattern +} + +func (pf patternFilter) Filter(set *Set) *Set { + return set.FilterByPattern(pf.pattern) } diff --git a/fieldpath/set_test.go b/fieldpath/set_test.go index cbb645e5..b3e9aa65 100644 --- a/fieldpath/set_test.go +++ b/fieldpath/set_test.go @@ -755,3 +755,96 @@ func TestSetNodeMapIterate(t *testing.T) { } } } + +func TestFilterByPattern(t *testing.T) { + testCases := []struct { + name string + input *Set + expect *Set + }{ + { + name: "exact match", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + 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, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "request"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "request"), + MakePathOrDie("spec", "containers", 1, "resources"), + MakePathOrDie("spec", "containers", 1, "resources", "limits"), + MakePathOrDie("spec", "containers", 1, "resources", "limits", "cpu"), + ), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + 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, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 0, "request"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "name"), + MakePathOrDie("spec", "containers", 0, "resources", "claims", 1, "request"), + MakePathOrDie("spec", "containers", 1, "resources"), + MakePathOrDie("spec", "containers", 1, "resources", "limits"), + MakePathOrDie("spec", "containers", 1, "resources", "limits", "cpu"), + ), + }, + { + name: "filter status and metadata", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("status"), + MakePathOrDie("metadata"), + ), + expect: NewSet( + MakePathOrDie("spec"), + ), + }, + { + name: "filter non-container spec fields", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "volumes"), + MakePathOrDie("spec", "hostNetwork"), + ), + expect: NewSet( + MakePathOrDie("spec"), + ), + }, + { + name: "filter non-resource container fields", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + MakePathOrDie("spec", "containers", 0, "image"), + MakePathOrDie("spec", "containers", 0, "workingDir"), + MakePathOrDie("spec", "containers", 0, "resources"), + ), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "containers"), + MakePathOrDie("spec", "containers", 0, "resources"), + ), + }, + } + + for _, tc := range testCases { + filter := NewPatternFilter(MustPrefixPattern("spec", "containers", MatchAnyPathElement(), "resources")) + t.Run(tc.name, func(t *testing.T) { + filtered := filter.Filter(tc.input) + if !filtered.Equals(tc.expect) { + t.Errorf("Expected:\n%v\n\nbut got:\n%v", tc.expect, filtered) + } + }) + } +} From 1206de614bb9ae95ce82d1da804c6de3de255ddb Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 30 Oct 2024 10:23:09 -0400 Subject: [PATCH 3/7] Clean up comments --- fieldpath/set.go | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/fieldpath/set.go b/fieldpath/set.go index 94235895..f92af880 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -147,9 +147,9 @@ func MustPrefixPattern(parts ...interface{}) *SetPattern { return result } -// PrefixPattern creates a SetPattern that matches all field paths prefixed by the given list of path parts. -// The parts may be PathPatterns, PathElements, strings (for field names) or ints (for array indices). -// `MatchAnyPathElement()` may be used to "wildcard" match any PathElement at that position in the field path. +// PrefixPattern creates a SetPattern that matches all field paths prefixed by the given list of pattern path parts. +// The pattern parts may be PathPatterns, PathElements, strings (for field names) or ints (for array indices). +// `MatchAnyPathElement()` may be used as a pattern path part to wildcard match a field path part. func PrefixPattern(parts ...interface{}) (*SetPattern, error) { current := MatchAnySet() // match all field patch suffixes for i := len(parts) - 1; i >= 0; i-- { @@ -190,24 +190,25 @@ func MatchAnySet() *SetPattern { // SetPattern defines a pattern that matches fields in a Set. // SetPattern is structured much like a Set but with wildcard support. type SetPattern struct { - // Wildcard indicates that all members and children are matched. - // If set, the Members and Children fields are ignored. + // 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. - // If any PatchPattern is a wildcard, then all members of a Set are matched. - // Otherwise, if any PathPattern is Equal to a member of a Set, that member is matched. + // Members provides patterns to match the members of a Set. Members []*MemberSetPattern } -// MemberSetPattern defines a pattern that matches the Members of a Set. -// MemberSetPattern is structured much like the elements of a SetNodeMap, but with wildcard support. +// MemberSetPattern defines a pattern that matches the members of a Set. +// MemberSetPattern is structured much like the elements of a SetNodeMap, but +// with wildcard support. type MemberSetPattern struct { - // Path provides a pattern to match Members of a Set. - // If Path is a wildcard, all Members of a Set are matched. - // Otherwise, the Member of a Set with a path that is Equal to this Path is matched. + // Path provides a pattern 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 pattern. Path PathPattern - // Child provides a pattern to use for Member of a Set that were matched by this MemberSetPattern's Path. + // Child provides a pattern to use for the children of matched members of a Set. Child *SetPattern } @@ -217,11 +218,12 @@ type PathPattern struct { // If set, PathElement is ignored. Wildcard bool - // PathElement matches another PathElement if it is Equal to this PathElement. + // PathElement indicates that a PathElement is matched if it is Equal + // to this PathElement. PathElement } -// FilterByPattern returns a Set with only fields that match the pattern. +// FilterByPattern returns a Set with only the field paths that match the pattern. func (s *Set) FilterByPattern(pattern *SetPattern) *Set { if pattern.Wildcard { return s @@ -582,7 +584,7 @@ func (s *SetNodeMap) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.Ty } } -// FilterByPattern returns a set that is filtered by the pattern. +// FilterByPattern returns a SetNodeMap with only the field paths that match the pattern. func (s *SetNodeMap) FilterByPattern(pattern *SetPattern) *SetNodeMap { if pattern.Wildcard { return s @@ -637,12 +639,12 @@ func (s *SetNodeMap) Leaves() *SetNodeMap { return out } -// Filter defines an interface for filtering Set. -// NewExcludeFilter can be used to create a filter that removes fields at the +// Filter defines an interface for filtering a set. +// NewExcludeFilter can be used to create a filter that removes // excluded field paths. // NewPatternFilter can be used to create a filter that removes all fields except // the fields that match a field path pattern. PrefixPattern and MustPrefixPattern -// can help create field path patterns. +// can be used to define field path patterns. type Filter interface { // Filter returns a filtered copy of the set. Filter(*Set) *Set From 1311e4d03f19bf01ef0679c2e2940763840eeaec Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 30 Oct 2024 13:55:11 -0400 Subject: [PATCH 4/7] Add support for more field path pattern types, clarify comments --- fieldpath/set.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/fieldpath/set.go b/fieldpath/set.go index f92af880..a16fc23a 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -18,6 +18,7 @@ package fieldpath import ( "fmt" + "sigs.k8s.io/structured-merge-diff/v4/value" "sort" "strings" @@ -148,8 +149,14 @@ func MustPrefixPattern(parts ...interface{}) *SetPattern { } // PrefixPattern creates a SetPattern that matches all field paths prefixed by the given list of pattern path parts. -// The pattern parts may be PathPatterns, PathElements, strings (for field names) or ints (for array indices). -// `MatchAnyPathElement()` may be used as a pattern path part to wildcard match a field path part. +// The pattern parts may any of: +// +// - PathPattern - for wildcards, `MatchAnyPathElement()` can be used as well. +// - PathElement - for any path element +// - value.FieldList - for associative list keys +// - value.Value - for scalar list elements +// - string - For field names +// - int - for array indices func PrefixPattern(parts ...interface{}) (*SetPattern, error) { current := MatchAnySet() // match all field patch suffixes for i := len(parts) - 1; i >= 0; i-- { @@ -160,6 +167,13 @@ func PrefixPattern(parts ...interface{}) (*SetPattern, error) { pattern = t case PathElement: pattern = PathPattern{PathElement: t} + case *value.FieldList: + if len(*t) == 0 { + return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)") + } + pattern = PathPattern{PathElement: PathElement{Key: t}} + case value.Value: + pattern = PathPattern{PathElement: PathElement{Value: &t}} case string: pattern = PathPattern{PathElement: PathElement{FieldName: &t}} case int: @@ -639,7 +653,7 @@ func (s *SetNodeMap) Leaves() *SetNodeMap { return out } -// Filter defines an interface for filtering a set. +// Filter defines an interface for excluding field paths from a set. // NewExcludeFilter can be used to create a filter that removes // excluded field paths. // NewPatternFilter can be used to create a filter that removes all fields except From f395ded435c48b071762bb3af322759a5fd956b3 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 30 Oct 2024 22:11:22 -0400 Subject: [PATCH 5/7] Apply feedback --- fieldpath/set.go | 157 ++++++++++++++++++++++++-------------- fieldpath/set_test.go | 172 ++++++++++++++++++++++++++++++++++++++++-- merge/ignore_test.go | 20 ++--- 3 files changed, 277 insertions(+), 72 deletions(-) diff --git a/fieldpath/set.go b/fieldpath/set.go index a16fc23a..c35b2062 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -138,51 +138,51 @@ func (s *Set) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.TypeRef) } } -// MustPrefixPattern is the same as PrefixPattern except it panics if parts can't be -// turned into a SetPattern. -func MustPrefixPattern(parts ...interface{}) *SetPattern { - result, err := PrefixPattern(parts...) +// 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 } -// PrefixPattern creates a SetPattern that matches all field paths prefixed by the given list of pattern path parts. +// PrefixMatcher creates a SetMatcher that matches all field paths prefixed by the given list of pattern path parts. // The pattern parts may any of: // -// - PathPattern - for wildcards, `MatchAnyPathElement()` can be used as well. +// - PathElementMatcher - for wildcards, `MatchAnyPathElement()` can be used as well. // - PathElement - for any path element // - value.FieldList - for associative list keys // - value.Value - for scalar list elements // - string - For field names // - int - for array indices -func PrefixPattern(parts ...interface{}) (*SetPattern, error) { - current := MatchAnySet() // match all field patch suffixes +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 PathPattern + var pattern PathElementMatcher switch t := part.(type) { - case PathPattern: + case PathElementMatcher: pattern = t case PathElement: - pattern = PathPattern{PathElement: t} + pattern = PathElementMatcher{PathElement: t} case *value.FieldList: if len(*t) == 0 { return nil, fmt.Errorf("associative list key type path elements must have at least one key (got zero)") } - pattern = PathPattern{PathElement: PathElement{Key: t}} + pattern = PathElementMatcher{PathElement: PathElement{Key: t}} case value.Value: - pattern = PathPattern{PathElement: PathElement{Value: &t}} + pattern = PathElementMatcher{PathElement: PathElement{Value: &t}} case string: - pattern = PathPattern{PathElement: PathElement{FieldName: &t}} + pattern = PathElementMatcher{PathElement: PathElement{FieldName: &t}} case int: - pattern = PathPattern{PathElement: PathElement{Index: &t}} + pattern = PathElementMatcher{PathElement: PathElement{Index: &t}} default: return nil, fmt.Errorf("unexpected type %T", t) } - current = &SetPattern{ - Members: []*MemberSetPattern{{ + current = &SetMatcher{ + Members: []*SetMemberMatcher{{ Path: pattern, Child: current, }}, @@ -191,43 +191,80 @@ func PrefixPattern(parts ...interface{}) (*SetPattern, error) { return current, nil } -// MatchAnyPathElement returns a PathPattern that matches any path element. -func MatchAnyPathElement() PathPattern { - return PathPattern{Wildcard: true} +// MatchAnyPathElement returns a PathElementMatcher that matches any path element. +func MatchAnyPathElement() PathElementMatcher { + return PathElementMatcher{Wildcard: true} } -// MatchAnySet returns a SetPattern that matches any set. -func MatchAnySet() *SetPattern { - return &SetPattern{Wildcard: true} +// MatchAnySet returns a SetMatcher that matches any set. +func MatchAnySet() *SetMatcher { + return &SetMatcher{Wildcard: true} } -// SetPattern defines a pattern that matches fields in a Set. -// SetPattern is structured much like a Set but with wildcard support. -type SetPattern struct { +// SetMatcher defines a pattern 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. - Members []*MemberSetPattern + Members []*SetMemberMatcher +} + +// Merge merges two SetMatchers into a single SetMatcher that matches the field paths of both SetMatchers. +// During the merge, Members of s2 with the same PathElementMatcher as a member of s are merged into the member of s. +// All other members of s2 are appended to the resulting member list in their original relative order. +// When members are merged, the child SetMatchers are merged by calling this function recursively. +func (s *SetMatcher) Merge(s2 *SetMatcher) *SetMatcher { + if s.Wildcard || s2.Wildcard { + return &SetMatcher{Wildcard: true} + } + + // TODO: Optimize. This is O(n^2). In practice, usually a single member is being inserted at a time, + // but that's still O(n). Sorting would help a ton. + var unionedMembers []*SetMemberMatcher + for _, m := range s.Members { + for _, m2 := range s2.Members { + if m.Path.PathElement.Equals(m2.Path.PathElement) && m.Path.Wildcard == m2.Path.Wildcard { + unionedMembers = append(unionedMembers, &SetMemberMatcher{ + Path: m.Path, + Child: m.Child.Merge(m2.Child), + }) + } else { + unionedMembers = append(unionedMembers, m) + } + } + } + for _, m2 := range s2.Members { + for _, existing := range unionedMembers { + if !m2.Path.PathElement.Equals(existing.Path.PathElement) { + unionedMembers = append(unionedMembers, m2) + } + } + } + + return &SetMatcher{ + Members: unionedMembers, + } } -// MemberSetPattern defines a pattern that matches the members of a Set. -// MemberSetPattern is structured much like the elements of a SetNodeMap, but +// SetMemberMatcher defines a pattern that matches the members of a Set. +// SetMemberMatcher is structured much like the elements of a SetNodeMap, but // with wildcard support. -type MemberSetPattern struct { +type SetMemberMatcher struct { // Path provides a pattern 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 pattern. - Path PathPattern + Path PathElementMatcher // Child provides a pattern to use for the children of matched members of a Set. - Child *SetPattern + Child *SetMatcher } -// PathPattern defined a match pattern for a PathElement. -type PathPattern struct { +// PathElementMatcher defined a match pattern for a PathElement. +type PathElementMatcher struct { // Wildcard indicates that all PathElements are matched by this pattern. // If set, PathElement is ignored. Wildcard bool @@ -237,8 +274,8 @@ type PathPattern struct { PathElement } -// FilterByPattern returns a Set with only the field paths that match the pattern. -func (s *Set) FilterByPattern(pattern *SetPattern) *Set { +// FilterIncludeMatches returns a Set with only the field paths that match. +func (s *Set) FilterIncludeMatches(pattern *SetMatcher) *Set { if pattern.Wildcard { return s } @@ -254,7 +291,7 @@ func (s *Set) FilterByPattern(pattern *SetPattern) *Set { } return &Set{ Members: members, - Children: *s.Children.FilterByPattern(pattern), + Children: *s.Children.FilterIncludeMatches(pattern), } } @@ -598,8 +635,8 @@ func (s *SetNodeMap) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.Ty } } -// FilterByPattern returns a SetNodeMap with only the field paths that match the pattern. -func (s *SetNodeMap) FilterByPattern(pattern *SetPattern) *SetNodeMap { +// FilterIncludeMatches returns a SetNodeMap with only the field paths that match the pattern. +func (s *SetNodeMap) FilterIncludeMatches(pattern *SetMatcher) *SetNodeMap { if pattern.Wildcard { return s } @@ -608,7 +645,7 @@ func (s *SetNodeMap) FilterByPattern(pattern *SetPattern) *SetNodeMap { for _, member := range s.members { for _, c := range pattern.Members { if c.Path.Wildcard || c.Path.PathElement.Equals(member.pathElement) { - childSet := member.set.FilterByPattern(c.Child) + childSet := member.set.FilterIncludeMatches(c.Child) if childSet.Size() > 0 { out = append(out, setNode{ pathElement: member.pathElement, @@ -654,23 +691,23 @@ func (s *SetNodeMap) Leaves() *SetNodeMap { } // Filter defines an interface for excluding field paths from a set. -// NewExcludeFilter can be used to create a filter that removes -// excluded field paths. -// NewPatternFilter can be used to create a filter that removes all fields except -// the fields that match a field path pattern. PrefixPattern and MustPrefixPattern +// 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 pattern. 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 } -// NewExcludeFilter returns a filter that removes field paths in the exclude set. -func NewExcludeFilter(exclude *Set) Filter { +// NewExcludeSetFilter returns a filter that removes field paths in the exclude set. +func NewExcludeSetFilter(exclude *Set) Filter { return excludeFilter{exclude} } -// NewExcludeFilterMap converts a map of APIVersion to exclude set to a map of APIVersion to exclude filters. -func NewExcludeFilterMap(resetFields map[APIVersion]*Set) map[APIVersion]Filter { +// 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} @@ -686,16 +723,24 @@ func (t excludeFilter) Filter(set *Set) *Set { return set.RecursiveDifference(t.excludeSet) } -// NewPatternFilter returns a filter that only includes field paths that match the pattern. -// PrefixPattern and MustPrefixPattern can help create basic SetPatterns. -func NewPatternFilter(pattern *SetPattern) Filter { - return patternFilter{pattern} +// NewIncludeMatcherFilter returns a filter that only includes field paths that match the pattern. +// PrefixMatcher and MakePrefixMatcherOrDie can help create basic SetPatterns. +func NewIncludeMatcherFilter(patterns ...*SetMatcher) Filter { + if len(patterns) == 0 { + return includeMatcherFilter{&SetMatcher{Wildcard: true}} + } + pattern := patterns[0] + for i := 1; i < len(patterns); i++ { + pattern = pattern.Merge(patterns[i]) + } + + return includeMatcherFilter{pattern} } -type patternFilter struct { - pattern *SetPattern +type includeMatcherFilter struct { + pattern *SetMatcher } -func (pf patternFilter) Filter(set *Set) *Set { - return set.FilterByPattern(pf.pattern) +func (pf includeMatcherFilter) Filter(set *Set) *Set { + return set.FilterIncludeMatches(pf.pattern) } diff --git a/fieldpath/set_test.go b/fieldpath/set_test.go index b3e9aa65..8122dd28 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" @@ -761,9 +762,10 @@ func TestFilterByPattern(t *testing.T) { name string input *Set expect *Set + filter Filter }{ { - name: "exact match", + name: "container resize fields: exact match", input: NewSet( MakePathOrDie("spec"), MakePathOrDie("spec", "containers"), @@ -781,6 +783,7 @@ func TestFilterByPattern(t *testing.T) { 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"), @@ -800,29 +803,31 @@ func TestFilterByPattern(t *testing.T) { ), }, { - name: "filter status and metadata", + 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: "filter non-container spec fields", + 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: "filter non-resource container fields", + name: "container resize fields: filter non-resource container fields", input: NewSet( MakePathOrDie("spec"), MakePathOrDie("spec", "containers"), @@ -830,18 +835,173 @@ func TestFilterByPattern(t *testing.T) { 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, "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 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", 0, "value"), + MakePathOrDie("spec", "list", 1, "value"), + ), + filter: NewIncludeMatcherFilter(MakePrefixMatcherOrDie("spec", "list", 1, "value")), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list", 1, "value"), + ), + }, + { + name: "multiple index matchers", + input: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list", 0, "value"), + MakePathOrDie("spec", "list", 1, "value"), + ), + filter: NewIncludeMatcherFilter( + MakePrefixMatcherOrDie("spec", "list", 0, "value"), + MakePrefixMatcherOrDie("spec", "list", 1, "value"), + ), + expect: NewSet( + MakePathOrDie("spec"), + MakePathOrDie("spec", "list", 0, "value"), + MakePathOrDie("spec", "list", 1, "value"), + ), + }, + { + name: "multiple field matchers", + input: NewSet( + MakePathOrDie("spec", "f1", "f11"), + MakePathOrDie("spec", "f2", "f21"), + MakePathOrDie("spec", "f3", "f31"), + ), + filter: NewIncludeMatcherFilter( + MakePrefixMatcherOrDie("spec", "f1"), + MakePrefixMatcherOrDie("spec", "f3"), + ), + expect: NewSet( + MakePathOrDie("spec", "f1", "f11"), + MakePathOrDie("spec", "f3", "f31"), + ), + }, + { + name: "wildcard takes precedence", + input: NewSet( + MakePathOrDie("spec", "list", 0, "f1"), + MakePathOrDie("spec", "list", 0, "f2"), + MakePathOrDie("spec", "list", 1, "f1"), + MakePathOrDie("spec", "list", 1, "f2"), + MakePathOrDie("spec", "list", 2, "f1"), + MakePathOrDie("spec", "list", 2, "f2"), + ), + filter: NewIncludeMatcherFilter( + MakePrefixMatcherOrDie("spec", "list", MatchAnyPathElement(), "f1"), // wildcard matches first and runs + MakePrefixMatcherOrDie("spec", "list", 1, "f2"), // already matched by wildcard so this is ignored + ), + expect: NewSet( + MakePathOrDie("spec", "list", 0, "f1"), + MakePathOrDie("spec", "list", 1, "f1"), + MakePathOrDie("spec", "list", 2, "f1"), + ), + }, } for _, tc := range testCases { - filter := NewPatternFilter(MustPrefixPattern("spec", "containers", MatchAnyPathElement(), "resources")) t.Run(tc.name, func(t *testing.T) { - filtered := filter.Filter(tc.input) + 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/merge/ignore_test.go b/merge/ignore_test.go index 35d6b805..5a8b4200 100644 --- a/merge/ignore_test.go +++ b/merge/ignore_test.go @@ -51,7 +51,7 @@ func TestIgnoredFields(t *testing.T) { ), }, IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ - "v1": fieldpath.NewExcludeFilter(_NS( + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("string"), )), }, @@ -76,7 +76,7 @@ func TestIgnoredFields(t *testing.T) { ), }, IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ - "v1": fieldpath.NewExcludeFilter(_NS( + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("obj"), )), }, @@ -107,7 +107,7 @@ func TestIgnoredFields(t *testing.T) { ), }, IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ - "v1": fieldpath.NewExcludeFilter(_NS( + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("string"), )), }, @@ -132,7 +132,7 @@ func TestIgnoredFields(t *testing.T) { ), }, IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ - "v1": fieldpath.NewExcludeFilter(_NS( + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("obj"), )), }, @@ -206,16 +206,16 @@ func TestFilteredFieldsUsesVersions(t *testing.T) { ), }, IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ - "v1": fieldpath.NewExcludeFilter(_NS( + "v1": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "c"), )), - "v2": fieldpath.NewExcludeFilter(_NS( + "v2": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "cc"), )), - "v3": fieldpath.NewExcludeFilter(_NS( + "v3": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "ccc"), )), - "v4": fieldpath.NewExcludeFilter(_NS( + "v4": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "cccc"), )), }, @@ -274,7 +274,7 @@ func TestFilteredFieldsUsesVersions(t *testing.T) { ), }, IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ - "v2": fieldpath.NewExcludeFilter(_NS( + "v2": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "c"), )), }, @@ -333,7 +333,7 @@ func TestFilteredFieldsUsesVersions(t *testing.T) { ), }, IgnoreFilter: map[fieldpath.APIVersion]fieldpath.Filter{ - "v2": fieldpath.NewExcludeFilter(_NS( + "v2": fieldpath.NewExcludeSetFilter(_NS( _P("mapOfMapsRecursive", "c"), )), }, From a8ac1f52b61e8506de6d4914931e7eafe8993b1d Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 31 Oct 2024 10:06:19 -0400 Subject: [PATCH 6/7] Optimize merge, make wildcards always take precedence --- fieldpath/set.go | 158 +++++++++++++++++++++++++----------------- fieldpath/set_test.go | 4 +- 2 files changed, 96 insertions(+), 66 deletions(-) diff --git a/fieldpath/set.go b/fieldpath/set.go index c35b2062..e07d0f3a 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -148,8 +148,8 @@ func MakePrefixMatcherOrDie(parts ...interface{}) *SetMatcher { return result } -// PrefixMatcher creates a SetMatcher that matches all field paths prefixed by the given list of pattern path parts. -// The pattern parts may any of: +// 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 @@ -182,7 +182,7 @@ func PrefixMatcher(parts ...interface{}) (*SetMatcher, error) { return nil, fmt.Errorf("unexpected type %T", t) } current = &SetMatcher{ - Members: []*SetMemberMatcher{{ + members: []*SetMemberMatcher{{ Path: pattern, Child: current, }}, @@ -198,74 +198,81 @@ func MatchAnyPathElement() PathElementMatcher { // MatchAnySet returns a SetMatcher that matches any set. func MatchAnySet() *SetMatcher { - return &SetMatcher{Wildcard: true} + return &SetMatcher{wildcard: true} } -// SetMatcher defines a pattern that matches fields in a Set. +// 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. - Members []*SetMemberMatcher + // 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 two SetMatchers into a single SetMatcher that matches the field paths of both SetMatchers. -// During the merge, Members of s2 with the same PathElementMatcher as a member of s are merged into the member of s. -// All other members of s2 are appended to the resulting member list in their original relative order. -// When members are merged, the child SetMatchers are merged by calling this function recursively. +// 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 &SetMatcher{Wildcard: true} - } - - // TODO: Optimize. This is O(n^2). In practice, usually a single member is being inserted at a time, - // but that's still O(n). Sorting would help a ton. - var unionedMembers []*SetMemberMatcher - for _, m := range s.Members { - for _, m2 := range s2.Members { - if m.Path.PathElement.Equals(m2.Path.PathElement) && m.Path.Wildcard == m2.Path.Wildcard { - unionedMembers = append(unionedMembers, &SetMemberMatcher{ - Path: m.Path, - Child: m.Child.Merge(m2.Child), - }) - } else { - unionedMembers = append(unionedMembers, m) - } - } - } - for _, m2 := range s2.Members { - for _, existing := range unionedMembers { - if !m2.Path.PathElement.Equals(existing.Path.PathElement) { - unionedMembers = append(unionedMembers, m2) + 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 &SetMatcher{ - Members: unionedMembers, - } + return NewSetMatcher(false, merged...) // sort happens here } -// SetMemberMatcher defines a pattern that matches the members of a Set. +// 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 pattern to match members of a Set. + // 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 pattern. + // against the Child matcher. Path PathElementMatcher - // Child provides a pattern to use for the children of matched members of a Set. + // Child provides a matcher to use for the children of matched members of a Set. Child *SetMatcher } -// PathElementMatcher defined a match pattern for a PathElement. +// PathElementMatcher defined a match matcher for a PathElement. type PathElementMatcher struct { - // Wildcard indicates that all PathElements are matched by this pattern. + // Wildcard indicates that all PathElements are matched by this matcher. // If set, PathElement is ignored. Wildcard bool @@ -274,15 +281,37 @@ type PathElementMatcher struct { 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 { + if pattern.wildcard { return s } members := PathElementSet{} for _, m := range s.Members.members { - for _, pm := range pattern.Members { + for _, pm := range pattern.members { if pm.Path.Wildcard || pm.Path.PathElement.Equals(m) { members.Insert(m) break @@ -635,15 +664,15 @@ func (s *SetNodeMap) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.Ty } } -// FilterIncludeMatches returns a SetNodeMap with only the field paths that match the pattern. +// FilterIncludeMatches returns a SetNodeMap with only the field paths that match the matcher. func (s *SetNodeMap) FilterIncludeMatches(pattern *SetMatcher) *SetNodeMap { - if pattern.Wildcard { + if pattern.wildcard { return s } var out sortedSetNode for _, member := range s.members { - for _, c := range pattern.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 { @@ -694,7 +723,7 @@ func (s *SetNodeMap) Leaves() *SetNodeMap { // 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 pattern. PrefixMatcher and MakePrefixMatcherOrDie +// 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. @@ -723,24 +752,25 @@ func (t excludeFilter) Filter(set *Set) *Set { return set.RecursiveDifference(t.excludeSet) } -// NewIncludeMatcherFilter returns a filter that only includes field paths that match the pattern. -// PrefixMatcher and MakePrefixMatcherOrDie can help create basic SetPatterns. -func NewIncludeMatcherFilter(patterns ...*SetMatcher) Filter { - if len(patterns) == 0 { - return includeMatcherFilter{&SetMatcher{Wildcard: true}} +// 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}} } - pattern := patterns[0] - for i := 1; i < len(patterns); i++ { - pattern = pattern.Merge(patterns[i]) + matcher := matchers[0] + for i := 1; i < len(matchers); i++ { + matcher = matcher.Merge(matchers[i]) } - return includeMatcherFilter{pattern} + return includeMatcherFilter{matcher} } type includeMatcherFilter struct { - pattern *SetMatcher + matcher *SetMatcher } func (pf includeMatcherFilter) Filter(set *Set) *Set { - return set.FilterIncludeMatches(pf.pattern) + return set.FilterIncludeMatches(pf.matcher) } diff --git a/fieldpath/set_test.go b/fieldpath/set_test.go index 8122dd28..ef6aff13 100644 --- a/fieldpath/set_test.go +++ b/fieldpath/set_test.go @@ -988,8 +988,8 @@ func TestFilterByPattern(t *testing.T) { MakePathOrDie("spec", "list", 2, "f2"), ), filter: NewIncludeMatcherFilter( - MakePrefixMatcherOrDie("spec", "list", MatchAnyPathElement(), "f1"), // wildcard matches first and runs - MakePrefixMatcherOrDie("spec", "list", 1, "f2"), // already matched by wildcard so this is ignored + MakePrefixMatcherOrDie("spec", "list", MatchAnyPathElement(), "f1"), // takes precedence + MakePrefixMatcherOrDie("spec", "list", 1, "f2"), // ignored ), expect: NewSet( MakePathOrDie("spec", "list", 0, "f1"), From bda634ef20dbc5f02fd6acb73919d70bf091b642 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 31 Oct 2024 13:40:44 -0400 Subject: [PATCH 7/7] Apply feedback --- fieldpath/set.go | 10 ++++-- fieldpath/set_test.go | 78 ++++++++++++++++++++++--------------------- 2 files changed, 48 insertions(+), 40 deletions(-) diff --git a/fieldpath/set.go b/fieldpath/set.go index e07d0f3a..77ae2511 100644 --- a/fieldpath/set.go +++ b/fieldpath/set.go @@ -153,7 +153,7 @@ func MakePrefixMatcherOrDie(parts ...interface{}) *SetMatcher { // // - PathElementMatcher - for wildcards, `MatchAnyPathElement()` can be used as well. // - PathElement - for any path element -// - value.FieldList - for associative list keys +// - value.FieldList - for listMap keys // - value.Value - for scalar list elements // - string - For field names // - int - for array indices @@ -164,19 +164,25 @@ func PrefixMatcher(parts ...interface{}) (*SetMatcher, error) { 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) @@ -270,7 +276,7 @@ type SetMemberMatcher struct { Child *SetMatcher } -// PathElementMatcher defined a match matcher for a PathElement. +// 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. diff --git a/fieldpath/set_test.go b/fieldpath/set_test.go index ef6aff13..7efb374e 100644 --- a/fieldpath/set_test.go +++ b/fieldpath/set_test.go @@ -769,16 +769,20 @@ func TestFilterByPattern(t *testing.T) { 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"), @@ -787,16 +791,20 @@ func TestFilterByPattern(t *testing.T) { 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"), @@ -831,6 +839,7 @@ func TestFilterByPattern(t *testing.T) { 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"), @@ -839,6 +848,7 @@ func TestFilterByPattern(t *testing.T) { expect: NewSet( MakePathOrDie("spec"), MakePathOrDie("spec", "containers"), + MakePathOrDie("spec", "containers", 0), MakePathOrDie("spec", "containers", 0, "resources"), ), }, @@ -880,44 +890,6 @@ func TestFilterByPattern(t *testing.T) { }, "field"), ), }, - { - 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( @@ -935,12 +907,17 @@ func TestFilterByPattern(t *testing.T) { 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"), ), }, @@ -948,8 +925,13 @@ func TestFilterByPattern(t *testing.T) { 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"), @@ -957,15 +939,22 @@ func TestFilterByPattern(t *testing.T) { ), 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( @@ -973,17 +962,25 @@ func TestFilterByPattern(t *testing.T) { 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"), ), @@ -992,8 +989,13 @@ func TestFilterByPattern(t *testing.T) { 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"), ), },