From 12ad89b2d63c9623d8e49838d08aa30c600eedd9 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Fri, 7 Jul 2023 01:28:15 +0100 Subject: [PATCH 1/7] Add regression tests --- api/krusty/configmaps_test.go | 183 ++++++++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) diff --git a/api/krusty/configmaps_test.go b/api/krusty/configmaps_test.go index b161d88b35..97cb07eede 100644 --- a/api/krusty/configmaps_test.go +++ b/api/krusty/configmaps_test.go @@ -591,3 +591,186 @@ metadata: name: test-m8t7bmb6g2 `) } + +// Regression test for https://github.com/kubernetes-sigs/kustomize/issues/5047 +func TestPrefixSuffix(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("kustomization.yaml", ` +resources: +- a +- b +`) + + th.WriteF("a/kustomization.yaml", ` +resources: +- ../common + +namePrefix: a +`) + + th.WriteF("b/kustomization.yaml", ` +resources: +- ../common + +namePrefix: b +`) + + th.WriteF("common/kustomization.yaml", ` +resources: +- service + +configMapGenerator: +- name: "-example-configmap" +`) + + th.WriteF("common/service/deployment.yaml", ` +kind: Deployment +apiVersion: apps/v1 + +metadata: + name: "-" + +spec: + template: + spec: + containers: + - name: app + envFrom: + - configMapRef: + name: "-example-configmap" +`) + + th.WriteF("common/service/kustomization.yaml", ` +resources: +- deployment.yaml + +nameSuffix: api +`) + + + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: a-api +spec: + template: + spec: + containers: + - envFrom: + - configMapRef: + name: a-example-configmap-6ct58987ht + name: app +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: a-example-configmap-6ct58987ht +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: b-api +spec: + template: + spec: + containers: + - envFrom: + - configMapRef: + name: b-example-configmap-6ct58987ht + name: app +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: b-example-configmap-6ct58987ht +`) +} + +// Regression test for https://github.com/kubernetes-sigs/kustomize/issues/5047 +func TestPrefixSuffix2(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("kustomization.yaml", ` +resources: +- a +- b +`) + + th.WriteF("a/kustomization.yaml", ` +resources: +- ../common + +namePrefix: a +`) + + th.WriteF("b/kustomization.yaml", ` +resources: +- ../common + +namePrefix: b +`) + + th.WriteF("common/deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: "-example" +spec: + template: + spec: + containers: + - name: app + envFrom: + - configMapRef: + name: "-example-configmap" +`) + + th.WriteF("common/kustomization.yaml", ` +resources: +- deployment.yaml + +configMapGenerator: +- name: "-example-configmap" +`) + + + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: a-example +spec: + template: + spec: + containers: + - envFrom: + - configMapRef: + name: a-example-configmap-6ct58987ht + name: app +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: a-example-configmap-6ct58987ht +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: b-example +spec: + template: + spec: + containers: + - envFrom: + - configMapRef: + name: b-example-configmap-6ct58987ht + name: app +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: b-example-configmap-6ct58987ht +`) +} From 534ac517c95a0df8dbf8e605028dc56f93d3da78 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Fri, 7 Jul 2023 01:28:28 +0100 Subject: [PATCH 2/7] Update PrefixesSuffixesEquals function --- api/resource/resource.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/api/resource/resource.go b/api/resource/resource.go index ae1a98be0e..c8405a4af6 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -291,8 +291,11 @@ func (r *Resource) getCsvAnnotation(name string) []string { // as OutermostPrefixSuffix but performs a deeper comparison // of the suffix and prefix slices. func (r *Resource) PrefixesSuffixesEquals(o ResCtx) bool { - return utils.SameEndingSubSlice(r.GetNamePrefixes(), o.GetNamePrefixes()) && - utils.SameEndingSubSlice(r.GetNameSuffixes(), o.GetNameSuffixes()) + eitherPrefixEmpty := len(r.GetNamePrefixes()) == 0 || len(o.GetNamePrefixes()) == 0 + eitherSuffixEmpty := len(r.GetNameSuffixes()) == 0 || len(o.GetNameSuffixes()) == 0 + + return (eitherPrefixEmpty || utils.SameEndingSubSlice(r.GetNamePrefixes(), o.GetNamePrefixes())) && + (eitherSuffixEmpty || utils.SameEndingSubSlice(r.GetNameSuffixes(), o.GetNameSuffixes())) } // RemoveBuildAnnotations removes annotations created by the build process. From e052771eeaabdebabc8f298411950e2647560ba6 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Sat, 8 Jul 2023 23:25:18 +0100 Subject: [PATCH 3/7] Try empty prefix/suffix but fall back on duplicates --- api/filters/nameref/nameref.go | 9 ++++++--- api/resource/resource.go | 16 +++++++++++----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index 4815f10a29..3fc485a49d 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -284,9 +284,9 @@ func (f Filter) roleRefFilter() sieveFunc { return previousIdSelectedByGvk(roleRefGvk) } -func prefixSuffixEquals(other resource.ResCtx) sieveFunc { +func prefixSuffixEquals(other resource.ResCtx, allowEmpty bool) sieveFunc { return func(r *resource.Resource) bool { - return r.PrefixesSuffixesEquals(other) + return r.PrefixesSuffixesEquals(other, allowEmpty) } } @@ -325,7 +325,10 @@ func (f Filter) selectReferral( if len(candidates) == 1 { return candidates[0], nil } - candidates = doSieve(candidates, prefixSuffixEquals(f.Referrer)) + candidates = doSieve(candidates, prefixSuffixEquals(f.Referrer, true)) + if len(candidates) > 1 { + candidates = doSieve(candidates, prefixSuffixEquals(f.Referrer, false)) + } if len(candidates) == 1 { return candidates[0], nil } diff --git a/api/resource/resource.go b/api/resource/resource.go index c8405a4af6..a645071093 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -290,12 +290,18 @@ func (r *Resource) getCsvAnnotation(name string) []string { // PrefixesSuffixesEquals is conceptually doing the same task // as OutermostPrefixSuffix but performs a deeper comparison // of the suffix and prefix slices. -func (r *Resource) PrefixesSuffixesEquals(o ResCtx) bool { - eitherPrefixEmpty := len(r.GetNamePrefixes()) == 0 || len(o.GetNamePrefixes()) == 0 - eitherSuffixEmpty := len(r.GetNameSuffixes()) == 0 || len(o.GetNameSuffixes()) == 0 +func (r *Resource) PrefixesSuffixesEquals(o ResCtx, allowEmpty bool) bool { + if allowEmpty { + eitherPrefixEmpty := len(r.GetNamePrefixes()) == 0 || len(o.GetNamePrefixes()) == 0 + eitherSuffixEmpty := len(r.GetNameSuffixes()) == 0 || len(o.GetNameSuffixes()) == 0 - return (eitherPrefixEmpty || utils.SameEndingSubSlice(r.GetNamePrefixes(), o.GetNamePrefixes())) && - (eitherSuffixEmpty || utils.SameEndingSubSlice(r.GetNameSuffixes(), o.GetNameSuffixes())) + return (eitherPrefixEmpty || utils.SameEndingSubSlice(r.GetNamePrefixes(), o.GetNamePrefixes())) && + (eitherSuffixEmpty || utils.SameEndingSubSlice(r.GetNameSuffixes(), o.GetNameSuffixes())) + } else { + return utils.SameEndingSubSlice(r.GetNamePrefixes(), o.GetNamePrefixes()) && + utils.SameEndingSubSlice(r.GetNameSuffixes(), o.GetNameSuffixes()) + + } } // RemoveBuildAnnotations removes annotations created by the build process. From 8e03a67aad1b64c80ac1d6e4ab1843e695cdcb3a Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Wed, 6 Mar 2024 23:47:09 +0000 Subject: [PATCH 4/7] Run gofmt --- api/filters/nameref/nameref.go | 2 +- api/krusty/configmaps_test.go | 6 ++---- api/resource/resource.go | 4 +++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index 3fc485a49d..ff83420cb1 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -327,7 +327,7 @@ func (f Filter) selectReferral( } candidates = doSieve(candidates, prefixSuffixEquals(f.Referrer, true)) if len(candidates) > 1 { - candidates = doSieve(candidates, prefixSuffixEquals(f.Referrer, false)) + candidates = doSieve(candidates, prefixSuffixEquals(f.Referrer, false)) } if len(candidates) == 1 { return candidates[0], nil diff --git a/api/krusty/configmaps_test.go b/api/krusty/configmaps_test.go index 97cb07eede..e36767fa74 100644 --- a/api/krusty/configmaps_test.go +++ b/api/krusty/configmaps_test.go @@ -594,7 +594,7 @@ metadata: // Regression test for https://github.com/kubernetes-sigs/kustomize/issues/5047 func TestPrefixSuffix(t *testing.T) { - th := kusttest_test.MakeHarness(t) + th := kusttest_test.MakeHarness(t) th.WriteF("kustomization.yaml", ` resources: - a @@ -647,7 +647,6 @@ resources: nameSuffix: api `) - m := th.Run(".", th.MakeDefaultOptions()) th.AssertActualEqualsExpected(m, ` apiVersion: apps/v1 @@ -690,7 +689,7 @@ metadata: // Regression test for https://github.com/kubernetes-sigs/kustomize/issues/5047 func TestPrefixSuffix2(t *testing.T) { - th := kusttest_test.MakeHarness(t) + th := kusttest_test.MakeHarness(t) th.WriteF("kustomization.yaml", ` resources: - a @@ -734,7 +733,6 @@ configMapGenerator: - name: "-example-configmap" `) - m := th.Run(".", th.MakeDefaultOptions()) th.AssertActualEqualsExpected(m, ` apiVersion: apps/v1 diff --git a/api/resource/resource.go b/api/resource/resource.go index a645071093..9aa91a648d 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -157,7 +157,9 @@ func (r *Resource) DeepCopy() *Resource { // the resource. // TODO: move to RNode, use GetMeta to improve performance. // TODO: make a version of mergeStringMaps that is build-annotation aware -// to avoid repeatedly setting refby and genargs annotations +// +// to avoid repeatedly setting refby and genargs annotations +// // Must remove the kustomize bit at the end. func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) error { if err := r.SetLabels( From d6f988a528c3552540743ff2c926e920b0980ef8 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Thu, 7 Mar 2024 01:31:44 +0000 Subject: [PATCH 5/7] Remove newline --- api/resource/resource.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/resource/resource.go b/api/resource/resource.go index 9aa91a648d..6e46cb42b7 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -302,7 +302,6 @@ func (r *Resource) PrefixesSuffixesEquals(o ResCtx, allowEmpty bool) bool { } else { return utils.SameEndingSubSlice(r.GetNamePrefixes(), o.GetNamePrefixes()) && utils.SameEndingSubSlice(r.GetNameSuffixes(), o.GetNameSuffixes()) - } } From d146a81c723e8fcc8df73a6f1cceb71685d6da74 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Thu, 7 Mar 2024 01:35:07 +0000 Subject: [PATCH 6/7] Revert unnecessary gofmt change --- api/resource/resource.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/resource/resource.go b/api/resource/resource.go index 6e46cb42b7..77ef1964c6 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -157,9 +157,7 @@ func (r *Resource) DeepCopy() *Resource { // the resource. // TODO: move to RNode, use GetMeta to improve performance. // TODO: make a version of mergeStringMaps that is build-annotation aware -// -// to avoid repeatedly setting refby and genargs annotations -// +// to avoid repeatedly setting refby and genargs annotations // Must remove the kustomize bit at the end. func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) error { if err := r.SetLabels( From 218e3eac681be251318f5820c333a7ccccd1b1bb Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Thu, 7 Mar 2024 20:28:08 +0000 Subject: [PATCH 7/7] Add comment --- api/resource/resource.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/api/resource/resource.go b/api/resource/resource.go index 77ef1964c6..9884a672c5 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -287,9 +287,14 @@ func (r *Resource) getCsvAnnotation(name string) []string { return strings.Split(annotations[name], ",") } -// PrefixesSuffixesEquals is conceptually doing the same task -// as OutermostPrefixSuffix but performs a deeper comparison -// of the suffix and prefix slices. +// PrefixesSuffixesEquals is conceptually doing the same task as +// OutermostPrefixSuffix but performs a deeper comparison of the suffix and +// prefix slices. +// The allowEmpty flag determines whether an empty prefix/suffix +// should be considered a match on anything. +// This is used when filtering, starting with a coarser pass allowing empty +// matches, before requiring exact matches if there are multiple +// remaining candidates. func (r *Resource) PrefixesSuffixesEquals(o ResCtx, allowEmpty bool) bool { if allowEmpty { eitherPrefixEmpty := len(r.GetNamePrefixes()) == 0 || len(o.GetNamePrefixes()) == 0