Add tests in support of Issue 3489.

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 {

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()
 			}
 		})

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)

api/krusty/nameprefixsuffixpatch_test.go

@@ -15,7 +15,6 @@ func TestNamePrefixSuffixPatch(t *testing.T) {
 
 	th.WriteF("handlers/kustomization.yaml", `
 nameSuffix: -suffix
-
 resources:
 - deployment.yaml
 `)

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",

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:

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

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))
+		})
+	}
+}