diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index f7e683b5a..ef33f5073 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -101,7 +101,7 @@ func (f Filter) setMapping(node *yaml.RNode) error { return errors.Wrap(err, "trying to match 'name' field") } if nameNode == nil { - return fmt.Errorf("no 'name' field in node") + return fmt.Errorf("path config error; no 'name' field in node") } namespaceNode, err := node.Pipe(yaml.FieldMatcher{Name: "namespace"}) if err != nil { diff --git a/api/filters/nameref/nameref_test.go b/api/filters/nameref/nameref_test.go index ed38e4381..a41f016a2 100644 --- a/api/filters/nameref/nameref_test.go +++ b/api/filters/nameref/nameref_test.go @@ -142,31 +142,38 @@ apiVersion: apps/v1 kind: Deployment metadata: name: dep + namespace: someNs map: name: oldName - namespace: oldNs + namespace: someNs `, candidates: ` apiVersion: apps/v1 kind: Secret metadata: name: newName - namespace: oldNs + namespace: someNs --- apiVersion: apps/v1 kind: NotSecret metadata: name: newName2 +--- +apiVersion: apps/v1 +kind: Secret +metadata: + name: thirdName `, - originalNames: []string{"oldName", ""}, + originalNames: []string{"oldName", "oldName", "oldName"}, referrerFinal: ` apiVersion: apps/v1 kind: Deployment metadata: name: dep + namespace: someNs map: name: newName - namespace: oldNs + namespace: someNs `, filter: Filter{ NameFieldToUpdate: types.FieldSpec{Path: "map"}, @@ -236,10 +243,10 @@ map: candidates := resMapFactory.FromResourceSlice(candidatesRes) tc.filter.ReferralCandidates = candidates + result := filtertest_test.RunFilter(t, tc.referrerOriginal, tc.filter) if !assert.Equal(t, strings.TrimSpace(tc.referrerFinal), - strings.TrimSpace( - filtertest_test.RunFilter(t, tc.referrerOriginal, tc.filter))) { + strings.TrimSpace(result)) { t.FailNow() } }) diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index 1097fdb45..88a4315cd 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -528,7 +528,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { `: considering field 'rules/resourceNames' of object {"apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRole", "metadata": { "name": "cr"}, "rules": [{"resourceNames": {"foo": "bar"}, "resources": ["secrets"]}]} -: visit traversal on path: [resourceNames]: no 'name' field in node`, +: visit traversal on path: [resourceNames]: path config error; no 'name' field in node`, `updating name reference in 'rules/resourceNames' field of `+ `'rbac.authorization.k8s.io_v1_ClusterRole|~X|cr'`+ `: considering field 'rules/resourceNames' of object @@ -541,7 +541,7 @@ rules: foo: bar resources: - secrets -: visit traversal on path: [resourceNames]: no 'name' field in node`)}, +: visit traversal on path: [resourceNames]: path config error; no 'name' field in node`)}, } nrt := newNameReferenceTransformer(builtinconfig.MakeDefaultConfig().NameReference) diff --git a/api/krusty/nameprefixsuffixpatch_test.go b/api/krusty/nameprefixsuffixpatch_test.go index da83b92dc..46b8ef687 100644 --- a/api/krusty/nameprefixsuffixpatch_test.go +++ b/api/krusty/nameprefixsuffixpatch_test.go @@ -15,7 +15,6 @@ func TestNamePrefixSuffixPatch(t *testing.T) { th.WriteF("handlers/kustomization.yaml", ` nameSuffix: -suffix - resources: - deployment.yaml `) diff --git a/api/krusty/namereference_test.go b/api/krusty/namereference_test.go index c32e1cce2..d214bf9e7 100644 --- a/api/krusty/namereference_test.go +++ b/api/krusty/namereference_test.go @@ -7,6 +7,54 @@ import ( kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) +func TestIssue3489Simplified(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteK(".", ` +namespace: kube-system +resources: +- aa +- bb +`) + th.WriteK("aa", ` +resources: +- ../base +`) + th.WriteK("bb", ` +resources: +- ../base +nameSuffix: -private +`) + th.WriteK("base", ` +resources: +- deployment.yaml +- serviceAccount.yaml +`) + th.WriteF("base/deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: myDep +spec: + template: + spec: + serviceAccountName: mySvcAcct + containers: + - name: whatever + image: k8s.gcr.io/governmentCheese +`) + th.WriteF("base/serviceAccount.yaml", ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: mySvcAcct +`) + // TODO(3489): This shouldn't be an error. + err := th.RunWithErr(".", th.MakeDefaultOptions()) + if !strings.Contains(err.Error(), "found multiple possible referrals") { + t.Fatalf("unexpected error: %q", err) + } +} + func TestIssue3489(t *testing.T) { const assets = `{ "tenantId": "XXXXX-XXXXXX-XXXXX-XXXXXX-XXXXXX", diff --git a/api/krusty/namespaces_test.go b/api/krusty/namespaces_test.go index 0a3966e65..30c91f245 100644 --- a/api/krusty/namespaces_test.go +++ b/api/krusty/namespaces_test.go @@ -53,7 +53,7 @@ resources: // This validates fix for Issue #1044. This should not be an error anymore - // the secrets have the same name but are in different namespaces. // The ClusterRole (by def) is not in a namespace, - // an in this case applies to *any* Secret resource + // and in this case applies to *any* Secret resource // named "dummy" m := th.Run("/app", th.MakeDefaultOptions()) th.AssertActualEqualsExpected(m, ` @@ -91,7 +91,7 @@ rules: `) } -func TestNameReferenceDeployment(t *testing.T) { +func TestNameReferenceDeploymentIssue3489(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK("base", ` resources: diff --git a/api/resource/resource.go b/api/resource/resource.go index 8bed9953c..1d98ac9d8 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -5,6 +5,7 @@ package resource import ( "fmt" + "log" "reflect" "strings" @@ -293,7 +294,7 @@ func (r *Resource) GetOutermostNameSuffix() string { return nameSuffixes[len(nameSuffixes)-1] } -func sameEndingSubarray(a, b []string) bool { +func SameEndingSubarray(a, b []string) bool { compareLen := len(b) if len(a) < len(b) { compareLen = len(a) @@ -340,16 +341,8 @@ func (r *Resource) OutermostPrefixSuffixEquals(o ResCtx) bool { // PrefixesSuffixesEquals is conceptually doing the same task // as OutermostPrefixSuffix but performs a deeper comparison // of the suffix and prefix slices. -// -// Important note: The PrefixSuffixTransformer is stacking the -// prefix values in the reverse order of appearance in -// the transformed name. For this reason the sameEndingSubarray -// method is used (as opposed to the sameBeginningSubarray) -// to compare the prefix slice. In the same spirit, the -// GetOutermostNamePrefix is using the last element of the -// nameprefix slice and not the first. func (r *Resource) PrefixesSuffixesEquals(o ResCtx) bool { - return sameEndingSubarray(r.GetNamePrefixes(), o.GetNamePrefixes()) && sameEndingSubarray(r.GetNameSuffixes(), o.GetNameSuffixes()) + return SameEndingSubarray(r.GetNamePrefixes(), o.GetNamePrefixes()) && SameEndingSubarray(r.GetNameSuffixes(), o.GetNameSuffixes()) } // RemoveBuildAnnotations removes annotations created by the build process. @@ -433,6 +426,15 @@ func (r *Resource) AsYAML() ([]byte, error) { return yaml.JSONToYAML(json) } +// MustYaml returns YAML or panics. +func (r *Resource) MustYaml() string { + yml, err := r.AsYAML() + if err != nil { + log.Fatal(err) + } + return string(yml) +} + // SetOptions updates the generator options for the resource. func (r *Resource) SetOptions(o *types.GenArgs) { r.options = o diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index ccaa13a20..e56985072 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -1177,3 +1177,55 @@ spec: name: nginx `, imagename) } + +func TestSameEndingSubarray(t *testing.T) { + testCases := map[string]struct { + a []string + b []string + expected bool + }{ + "both nil": { + expected: true, + }, + "one nil": { + b: []string{}, + expected: true, + }, + "both empty": { + a: []string{}, + b: []string{}, + expected: true, + }, + "no1 - TODO(3489) this should be false": { + a: []string{"a"}, + b: []string{}, + expected: true, + }, + "no2": { + a: []string{"b", "a"}, + b: []string{"b"}, + expected: false, + }, + "yes1": { + a: []string{"a", "b"}, + b: []string{"b"}, + expected: true, + }, + "yes2": { + a: []string{"a", "b", "c"}, + b: []string{"b", "c"}, + expected: true, + }, + "yes3": { + a: []string{"a", "b", "c", "d", "e", "f"}, + b: []string{"f"}, + expected: true, + }, + } + for n := range testCases { + tc := testCases[n] + t.Run(n, func(t *testing.T) { + assert.Equal(t, tc.expected, SameEndingSubarray(tc.a, tc.b)) + }) + } +}