diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index 1da3396fd..dff9fe03a 100644 --- a/api/filters/nameref/nameref.go +++ b/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 - // to know which one to pick, so emit error. + // If there's more than one match, + // filter the matches by prefix and suffix if len(matches) > 1 { - return "", "", fmt.Errorf( - "multiple matches for %s:\n %v", - id, getIds(matches)) + filteredMatches := filterReferralCandidates(referrer, matches) + if len(filteredMatches) > 1 { + return "", "", fmt.Errorf( + "multiple matches for %s:\n %v", + 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. diff --git a/api/filters/nameref/nameref_test.go b/api/filters/nameref/nameref_test.go index 951ff73eb..290878995 100644 --- a/api/filters/nameref/nameref_test.go +++ b/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") + } + } + }) + } +} diff --git a/api/krusty/nameprefixsuffixpatch_test.go b/api/krusty/nameprefixsuffixpatch_test.go index 4b71ea59f..c42b103aa 100644 --- a/api/krusty/nameprefixsuffixpatch_test.go +++ b/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 `) } diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index 68ba98c66..ed95572a9 100644 --- a/api/resmap/reswrangler.go +++ b/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) || - isRoleBindingNamespace(&subjectNamespaces, r.GetNamespace())) && r.InSameKustomizeCtx(rctxm) { + if !isInputIdNamespaceable || !resId.IsNamespaceableKind() || resId.IsNsEquals(inputId) || + isRoleBindingNamespace(&subjectNamespaces, r.GetNamespace()) { result.append(r) } } diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index b16a60f76..35d8050b8 100644 --- a/api/resmap/reswrangler_test.go +++ b/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{}{ diff --git a/api/resource/resource.go b/api/resource/resource.go index 61a2aee8c..15bc2897a 100644 --- a/api/resource/resource.go +++ b/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 }