Merge pull request #2847 from Shell32-Natsu/prefix-suffix-name-reference

fix name reference with prefixsuffix

api/filters/nameref/nameref.go

@@ -86,6 +86,18 @@ func (f Filter) setScalar(node *yaml.RNode) error {
 	return nil
 }
 
+func filterReferralCandidates(
+	referrer *resource.Resource,
+	matches []*resource.Resource) []*resource.Resource {
+	var ret []*resource.Resource
+	for _, m := range matches {
+		if referrer.PrefixesSuffixesEquals(m) {
+			ret = append(ret, m)
+		}
+	}
+	return ret
+}
+
 // selectReferral picks the referral among a subset of candidates.
 // It returns the current name and namespace of the selected candidate.
 // Note that the content of the referricalCandidateSubset slice is most of the time
@@ -103,12 +115,19 @@ func selectReferral(
 		id := res.OrgId()
 		if id.IsSelected(&target) && res.GetOriginalName() == oldName {
 			matches := referralCandidates.GetMatchingResourcesByOriginalId(id.Equals)
-			// If there's more than one match, there's no way
+			// If there's more than one match,
-			// to know which one to pick, so emit error.
+			// filter the matches by prefix and suffix
 			if len(matches) > 1 {
+				filteredMatches := filterReferralCandidates(referrer, matches)
+				if len(filteredMatches) > 1 {
 					return "", "", fmt.Errorf(
 						"multiple matches for %s:\n  %v",
-					id, getIds(matches))
+						id, getIds(filteredMatches))
+				}
+				// Check is the match the resource we are working on
+				if len(filteredMatches) == 0 || res != filteredMatches[0] {
+					continue
+				}
 			}
 			// In the resource, note that it is referenced
 			// by the referrer.

api/filters/nameref/nameref_test.go

@@ -331,3 +331,434 @@ metadata:
 		})
 	}
 }
+
+func TestCandidatesWithDifferentPrefixSuffix(t *testing.T) {
+	testCases := map[string]struct {
+		input         string
+		candidates    string
+		expected      string
+		filter        Filter
+		originalNames []string
+		prefix        []string
+		suffix        []string
+		inputPrefix   string
+		inputSuffix   string
+		err           bool
+	}{
+		"prefix match": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"prefix1", "prefix2"},
+			suffix:        []string{"", "suffix2"},
+			inputPrefix:   "prefix1",
+			inputSuffix:   "",
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: newName
+`,
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: false,
+		},
+		"suffix match": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"", "prefix2"},
+			suffix:        []string{"suffix1", "suffix2"},
+			inputPrefix:   "",
+			inputSuffix:   "suffix1",
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: newName
+`,
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: false,
+		},
+		"prefix suffix both match": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"prefix1", "prefix2"},
+			suffix:        []string{"suffix1", "suffix2"},
+			inputPrefix:   "prefix1",
+			inputSuffix:   "suffix1",
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: newName
+`,
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: false,
+		},
+		"multiple match: both": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"prefix", "prefix"},
+			suffix:        []string{"suffix", "suffix"},
+			inputPrefix:   "prefix",
+			inputSuffix:   "suffix",
+			expected:      "",
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: true,
+		},
+		"multiple match: only prefix": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"prefix", "prefix"},
+			suffix:        []string{"", ""},
+			inputPrefix:   "prefix",
+			inputSuffix:   "",
+			expected:      "",
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: true,
+		},
+		"multiple match: only suffix": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"", ""},
+			suffix:        []string{"suffix", "suffix"},
+			inputPrefix:   "",
+			inputSuffix:   "suffix",
+			expected:      "",
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: true,
+		},
+		"no match: neither match": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"prefix1", "prefix2"},
+			suffix:        []string{"suffix1", "suffix2"},
+			inputPrefix:   "prefix",
+			inputSuffix:   "suffix",
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: false,
+		},
+		"no match: prefix doesn't match": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"prefix1", "prefix2"},
+			suffix:        []string{"suffix", "suffix"},
+			inputPrefix:   "prefix",
+			inputSuffix:   "suffix",
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: false,
+		},
+		"no match: suffix doesn't match": {
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			candidates: `
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName
+---
+apiVersion: apps/v1
+kind: Secret
+metadata:
+  name: newName2
+`,
+			originalNames: []string{"oldName", "oldName"},
+			prefix:        []string{"prefix", "prefix"},
+			suffix:        []string{"suffix1", "suffix2"},
+			inputPrefix:   "prefix",
+			inputSuffix:   "suffix",
+			expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: dep
+ref:
+  name: oldName
+`,
+			filter: Filter{
+				FieldSpec: types.FieldSpec{Path: "ref/name"},
+				Target: resid.Gvk{
+					Group:   "apps",
+					Version: "v1",
+					Kind:    "Secret",
+				},
+			},
+			err: false,
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			factory := resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl())
+			referrer, err := factory.FromBytes([]byte(tc.input))
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tc.inputPrefix != "" {
+				referrer.AddNamePrefix(tc.inputPrefix)
+			}
+			if tc.inputSuffix != "" {
+				referrer.AddNameSuffix(tc.inputSuffix)
+			}
+			tc.filter.Referrer = referrer
+
+			resMapFactory := resmap.NewFactory(factory, nil)
+			candidatesRes, err := factory.SliceFromBytesWithNames(
+				tc.originalNames, []byte(tc.candidates))
+			if err != nil {
+				t.Fatal(err)
+			}
+			for i := range candidatesRes {
+				if tc.prefix[i] != "" {
+					candidatesRes[i].AddNamePrefix(tc.prefix[i])
+				}
+				if tc.suffix[i] != "" {
+					candidatesRes[i].AddNameSuffix(tc.suffix[i])
+				}
+			}
+
+			candidates := resMapFactory.FromResourceSlice(candidatesRes)
+			tc.filter.ReferralCandidates = candidates
+
+			if !tc.err {
+				if !assert.Equal(t,
+					strings.TrimSpace(tc.expected),
+					strings.TrimSpace(
+						filtertest_test.RunFilter(t, tc.input, tc.filter))) {
+					t.FailNow()
+				}
+			} else {
+				_, err := filtertest_test.RunFilterE(t, tc.input, tc.filter)
+				if err == nil {
+					t.Fatalf("an error is expected")
+				}
+			}
+		})
+	}
+}

api/krusty/nameprefixsuffixpatch_test.go

@@ -104,10 +104,10 @@ spec:
         - valueFrom:
             configMapKeyRef:
               key: MYSQL_DATABASE
-              name: mysql
+              name: mysql-9792mdchtg
         envFrom:
         - configMapRef:
-            name: mysql
+            name: mysql-9792mdchtg
         name: handler
 `)
 }

api/resmap/reswrangler.go

@@ -382,15 +382,14 @@ func (m *resWrangler) SubsetThatCouldBeReferencedByResource(
 	result := newOne()
 	inputId := inputRes.CurId()
 	isInputIdNamespaceable := inputId.IsNamespaceableKind()
-	rctxm := inputRes.PrefixesSuffixesEquals
 	subjectNamespaces := getNamespacesForRoleBinding(inputRes)
 	for _, r := range m.Resources() {
 		// Need to match more accuratly both at the time of selection and transformation.
 		// OutmostPrefixSuffixEquals is not accurate enough since it is only using
 		// the outer most suffix and the last prefix. Use PrefixedSuffixesEquals instead.
 		resId := r.CurId()
-		if (!isInputIdNamespaceable || !resId.IsNamespaceableKind() || resId.IsNsEquals(inputId) ||
+		if !isInputIdNamespaceable || !resId.IsNamespaceableKind() || resId.IsNsEquals(inputId) ||
-			isRoleBindingNamespace(&subjectNamespaces, r.GetNamespace())) && r.InSameKustomizeCtx(rctxm) {
+			isRoleBindingNamespace(&subjectNamespaces, r.GetNamespace()) {
 			result.append(r)
 		}
 	}

api/resmap/reswrangler_test.go

@@ -437,173 +437,6 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) {
 	}
 }
 
-func addPfxSfx(r *resource.Resource, prefixes []string, suffixes []string) {
-	for _, pfx := range prefixes {
-		r.AddNamePrefix(pfx)
-	}
-
-	for _, sfx := range suffixes {
-		r.AddNameSuffix(sfx)
-	}
-}
-
-func TestSubsetThatCouldBeReferencedByResourceMultiLevel(t *testing.T) {
-	// Simulates ConfigMap and Deployment defined at level 1
-	// No prefix nor suffix added at that level
-	cm1 := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "ConfigMap",
-			"metadata": map[string]interface{}{
-				"name": "level1",
-			},
-		})
-	addPfxSfx(cm1, []string{""}, []string{""})
-	dep1 := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "Deployment",
-			"metadata": map[string]interface{}{
-				"name": "level1",
-			},
-		})
-	addPfxSfx(dep1, []string{""}, []string{""})
-
-	// Simulates ConfigMap and Deployment defined at level 1
-	// and prefix added in level 2 of kustomization
-	cm2p := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "ConfigMap",
-			"metadata": map[string]interface{}{
-				"name": "level2p",
-			},
-		})
-	addPfxSfx(cm2p, []string{"", "level2p-"}, []string{"", ""})
-
-	dep2p := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "Deployment",
-			"metadata": map[string]interface{}{
-				"name": "level2p",
-			},
-		})
-	addPfxSfx(dep2p, []string{"", "level2p-"}, []string{"", ""})
-
-	// Simulates ConfigMap and Deployment defined at level 1
-	// and suffix added in level 2 of kustomization
-	cm2s := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "ConfigMap",
-			"metadata": map[string]interface{}{
-				"name": "level2s",
-			},
-		})
-	addPfxSfx(cm2s, []string{"", ""}, []string{"", "-level2s"})
-
-	dep2s := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "Deployment",
-			"metadata": map[string]interface{}{
-				"name": "level2s",
-			},
-		})
-	addPfxSfx(dep2s, []string{"", ""}, []string{"", "-level2s"})
-
-	// Simulates ConfigMap and Deployment defined at level 1,
-	// prefix added in levels 2 and 3 of kustomization.
-	cm3e := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "ConfigMap",
-			"metadata": map[string]interface{}{
-				"name": "level3e",
-			},
-		})
-	addPfxSfx(cm3e, []string{"", "level2p-", "level3e-"}, []string{"", "", ""})
-
-	dep3e := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "Deployment",
-			"metadata": map[string]interface{}{
-				"name": "level3e",
-			},
-		})
-	addPfxSfx(dep3e, []string{"", "level2p-", "level3e-"}, []string{"", "", ""})
-
-	// Simulates Deployment defined at level 1, ConfigMap defined at level 2,
-	// prefix added in levels 2 and 3 of kustomization.
-	// This reproduce issue 1440.
-	cm3i := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "ConfigMap",
-			"metadata": map[string]interface{}{
-				"name": "level3i",
-			},
-		})
-	addPfxSfx(cm3i, []string{"level2p-", "level3i-"}, []string{"", ""})
-
-	dep3i := rf.FromMap(
-		map[string]interface{}{
-			"apiVersion": "v1",
-			"kind":       "Deployment",
-			"metadata": map[string]interface{}{
-				"name": "level3i",
-			},
-		})
-	addPfxSfx(dep3i, []string{"", "level2p-", "level3i-"}, []string{"", "", ""})
-
-	tests := map[string]struct {
-		filter   *resource.Resource
-		expected ResMap
-	}{
-		"level1": {
-			filter: dep1,
-			expected: resmaptest_test.NewRmBuilder(t, rf).
-				AddR(cm1).AddR(dep1).ResMap(),
-		},
-		"level2p": {
-			filter: dep2p,
-			expected: resmaptest_test.NewRmBuilder(t, rf).
-				AddR(cm2p).AddR(dep2p).ResMap(),
-		},
-		"level2s": {
-			filter: dep2s,
-			expected: resmaptest_test.NewRmBuilder(t, rf).
-				AddR(cm2s).AddR(dep2s).ResMap(),
-		},
-		"level3p": {
-			filter: dep3e,
-			expected: resmaptest_test.NewRmBuilder(t, rf).
-				AddR(cm3e).AddR(dep3e).ResMap(),
-		},
-		"level3i": {
-			filter: dep3i,
-			expected: resmaptest_test.NewRmBuilder(t, rf).
-				AddR(cm3i).AddR(dep3i).ResMap(),
-		},
-	}
-	m := resmaptest_test.NewRmBuilder(t, rf).
-		AddR(cm1).AddR(dep1).AddR(cm2s).AddR(dep2s).AddR(cm2p).AddR(dep2p).AddR(cm3e).AddR(dep3e).AddR(cm3i).AddR(dep3i).ResMap()
-	for name, test := range tests {
-		test := test
-		t.Run(name, func(t *testing.T) {
-			got := m.SubsetThatCouldBeReferencedByResource(test.filter)
-			err := test.expected.ErrorIfNotEqualLists(got)
-			if err != nil {
-				test.expected.Debug("expected")
-				got.Debug("actual")
-				t.Fatalf("Expected match")
-			}
-		})
-	}
-}
-
 func TestDeepCopy(t *testing.T) {
 	rm1 := resmaptest_test.NewRmBuilder(t, rf).Add(
 		map[string]interface{}{

api/resource/resource.go

@@ -277,12 +277,6 @@ func (r *Resource) PrefixesSuffixesEquals(o ResCtx) bool {
 	return sameEndingSubarray(r.GetNamePrefixes(), o.GetNamePrefixes()) && sameEndingSubarray(r.GetNameSuffixes(), o.GetNameSuffixes())
 }
 
-// This is used to compute if a referrer could potentially be impacted
-// by the change of name of a referral.
-func (r *Resource) InSameKustomizeCtx(rctxm ResCtxMatcher) bool {
-	return rctxm(r)
-}
-
 func (r *Resource) GetOriginalName() string {
 	return r.originalName
 }