From 914a82bfa405328351fd8cccd9a6fdbc53bace4d Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Tue, 27 Apr 2021 23:53:43 -0700 Subject: [PATCH 1/2] Support multiple implementations for IsSameResource --- kyaml/kio/filters/merge3.go | 90 ++++++++++++++++++++++++-------- kyaml/kio/filters/merge3_test.go | 3 +- 2 files changed, 69 insertions(+), 24 deletions(-) diff --git a/kyaml/kio/filters/merge3.go b/kyaml/kio/filters/merge3.go index 27dd0f766..30932800c 100644 --- a/kyaml/kio/filters/merge3.go +++ b/kyaml/kio/filters/merge3.go @@ -19,17 +19,24 @@ const ( mergeSourceDest = "dest" ) +// ResourceMatcher interface is used to match two resources based on IsSameResource implementation +// This is the way to group same logical resources in upstream, local and origin for merge +// The default way to group them is using GVKNN similar to how kubernetes server identifies resources +// Users of this library might have their own interpretation of grouping similar resources +// for e.g. if consumer adds a name-prefix to local resource, it should not be treated as new resource +// for updates etc. +// Hence, the callers of this library may pass different implementation for IsSameResource +type ResourceMatcher interface { + IsSameResource(node1, node2 *yaml.RNode) bool +} + // Merge3 performs a 3-way merge on the original, updated, and destination packages. type Merge3 struct { OriginalPath string UpdatedPath string DestPath string MatchFilesGlob []string - - // MergeOnPath will use the relative filepath as part of the merge key. - // This may be necessary if the directory contains multiple copies of - // the same resource, or resources patches. - MergeOnPath bool + Matcher ResourceMatcher } func (m Merge3) Merge() error { @@ -67,7 +74,11 @@ func (m Merge3) Merge() error { // Filter combines Resources with the same GVK + N + NS into tuples, and then merges them func (m Merge3) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { // index the nodes by their identity - tl := tuples{mergeOnPath: m.MergeOnPath} + matcher := m.Matcher + if matcher == nil { + matcher = &DefaultGVKNNMatcher{MergeOnPath: true} + } + tl := tuples{matcher: matcher} for i := range nodes { if err := tl.add(nodes[i]); err != nil { return nil, err @@ -82,7 +93,7 @@ func (m Merge3) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { case t.original == nil && t.updated == nil && t.dest != nil: // added locally -- keep dest output = append(output, t.dest) - case t.original == nil && t.updated != nil && t.dest == nil: + case t.updated != nil && t.dest == nil: // added in the update -- add update output = append(output, t.updated) case t.original != nil && t.updated == nil: @@ -109,13 +120,35 @@ func (m Merge3) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { type tuples struct { list []*tuple - // mergeOnPath if set to true will use the resource filepath - // as part of the merge key - mergeOnPath bool + // matcher matches the resources for merge + matcher ResourceMatcher } -// isSameResource returns true if meta1 and meta2 are for the same logic resource -func (ts *tuples) isSameResource(meta1, meta2 yaml.ResourceMeta) bool { +// DefaultGVKNNMatcher holds the default matching of resources implementation based on +// Group, Version, Kind, Name and Namespace of the resource +type DefaultGVKNNMatcher struct { + // MergeOnPath will use the relative filepath as part of the merge key. + // This may be necessary if the directory contains multiple copies of + // the same resource, or resources patches. + MergeOnPath bool +} + +// IsSameResource returns true if metadata of node1 and metadata of node2 belongs to same logical resource +func (dm *DefaultGVKNNMatcher) IsSameResource(node1, node2 *yaml.RNode) bool { + if node1 == nil || node2 == nil { + return false + } + + meta1, err := node1.GetMeta() + if err != nil { + return false + } + + meta2, err := node2.GetMeta() + if err != nil { + return false + } + if meta1.Name != meta2.Name { return false } @@ -128,7 +161,7 @@ func (ts *tuples) isSameResource(meta1, meta2 yaml.ResourceMeta) bool { if meta1.Kind != meta2.Kind { return false } - if ts.mergeOnPath { + if dm.MergeOnPath { // directories may contain multiple copies of a resource with the same // name, namespace, apiVersion and kind -- e.g. kustomize patches, or // multiple environments @@ -143,17 +176,13 @@ func (ts *tuples) isSameResource(meta1, meta2 yaml.ResourceMeta) bool { // add adds a node to the list, combining it with an existing matching Resource if found func (ts *tuples) add(node *yaml.RNode) error { - nodeMeta, err := node.GetMeta() - if err != nil { - return err - } for i := range ts.list { t := ts.list[i] - if ts.isSameResource(t.meta, nodeMeta) { + if ts.matcher.IsSameResource(addedNode(t), node) { return t.add(node) } } - t := &tuple{meta: nodeMeta} + t := &tuple{} if err := t.add(node); err != nil { return err } @@ -161,9 +190,19 @@ func (ts *tuples) add(node *yaml.RNode) error { return nil } +// addedNode returns one on the existing added nodes in the tuple +func addedNode(t *tuple) *yaml.RNode { + if t.updated != nil { + return t.updated + } + if t.original != nil { + return t.original + } + return t.dest +} + // tuple wraps an original, updated, and dest tuple for a given Resource type tuple struct { - meta yaml.ResourceMeta original *yaml.RNode updated *yaml.RNode dest *yaml.RNode @@ -178,17 +217,17 @@ func (t *tuple) add(node *yaml.RNode) error { switch meta.Annotations[mergeSourceAnnotation] { case mergeSourceDest: if t.dest != nil { - return fmt.Errorf("dest source already specified") + return duplicateError("local", meta.Annotations[kioutil.PathAnnotation]) } t.dest = node case mergeSourceOriginal: if t.original != nil { - return fmt.Errorf("original source already specified") + return duplicateError("original upstream", meta.Annotations[kioutil.PathAnnotation]) } t.original = node case mergeSourceUpdated: if t.updated != nil { - return fmt.Errorf("updated source already specified") + return duplicateError("updated upstream", meta.Annotations[kioutil.PathAnnotation]) } t.updated = node default: @@ -201,3 +240,8 @@ func (t *tuple) add(node *yaml.RNode) error { func (t *tuple) merge() (*yaml.RNode, error) { return merge3.Merge(t.dest, t.original, t.updated) } + +// duplicateError returns duplicate resources error +func duplicateError(source, filePath string) error { + return fmt.Errorf(`found duplicate %q resources in file %q, please refer to "update" documentation for the fix`, source, filePath) +} diff --git a/kyaml/kio/filters/merge3_test.go b/kyaml/kio/filters/merge3_test.go index 0703233d5..f587acd32 100644 --- a/kyaml/kio/filters/merge3_test.go +++ b/kyaml/kio/filters/merge3_test.go @@ -44,6 +44,7 @@ func TestMerge3_Merge(t *testing.T) { OriginalPath: filepath.Join(datadir, "dataset1"), UpdatedPath: filepath.Join(datadir, "dataset1-remoteupdates"), DestPath: filepath.Join(dir, "dataset1"), + Matcher: &filters.DefaultGVKNNMatcher{MergeOnPath: false}, }.Merge() if !assert.NoError(t, err) { t.FailNow() @@ -89,7 +90,6 @@ func TestMerge3_Merge_path(t *testing.T) { OriginalPath: filepath.Join(datadir, "dataset1"), UpdatedPath: filepath.Join(datadir, "dataset1-remoteupdates"), DestPath: filepath.Join(dir, "dataset1"), - MergeOnPath: true, }.Merge() if !assert.NoError(t, err) { t.FailNow() @@ -135,6 +135,7 @@ func TestMerge3_Merge_fail(t *testing.T) { OriginalPath: filepath.Join(datadir, "dataset1"), UpdatedPath: filepath.Join(datadir, "dataset1-remoteupdates"), DestPath: filepath.Join(dir, "dataset1"), + Matcher: &filters.DefaultGVKNNMatcher{MergeOnPath: false}, }.Merge() if !assert.Error(t, err) { t.FailNow() From 8a529ca399ea583d25cd94b0cba632489740e6ac Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Tue, 27 Apr 2021 23:53:54 -0700 Subject: [PATCH 2/2] Update merge3 with deafult GVKNN matcher --- cmd/config/internal/commands/merge3.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/config/internal/commands/merge3.go b/cmd/config/internal/commands/merge3.go index a1ac5b3c5..5c48efc4e 100644 --- a/cmd/config/internal/commands/merge3.go +++ b/cmd/config/internal/commands/merge3.go @@ -46,12 +46,13 @@ type Merge3Runner struct { path bool } -func (r *Merge3Runner) runE(c *cobra.Command, args []string) error { +func (r *Merge3Runner) runE(_ *cobra.Command, _ []string) error { + matcher := filters.DefaultGVKNNMatcher{MergeOnPath: r.path} err := filters.Merge3{ OriginalPath: r.ancestor, UpdatedPath: r.fromDir, DestPath: r.toDir, - MergeOnPath: r.path, + Matcher: &matcher, }.Merge() if err != nil { return err