From f71854a0c81a33fd296e4e504064251f59ddf32a Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Mon, 1 Feb 2021 14:50:20 -0800 Subject: [PATCH] Refactored resource to store all previous names and namespaces --- api/builtins/HashTransformer.go | 2 +- api/builtins/NamespaceTransformer.go | 2 +- api/builtins/PatchTransformer.go | 2 +- api/builtins/PrefixSuffixTransformer.go | 2 +- api/builtins/ReplicaCountTransformer.go | 4 +- api/filters/nameref/nameref.go | 4 +- api/internal/accumulator/resaccumulator.go | 2 +- .../accumulator/resaccumulator_test.go | 16 +- api/internal/plugins/utils/utils.go | 3 - api/resmap/merginator.go | 2 +- api/resmap/resmap.go | 16 +- api/resmap/reswrangler.go | 51 +-- api/resmap/reswrangler_test.go | 128 ++++++ api/resource/factory.go | 12 +- api/resource/resource.go | 143 +++---- api/resource/resource_test.go | 403 ++++++------------ api/testutils/resmaptest/rmbuilder.go | 11 +- .../hashtransformer/HashTransformer.go | 2 +- .../NamespaceTransformer.go | 2 +- .../patchtransformer/PatchTransformer.go | 2 +- .../PrefixSuffixTransformer.go | 2 +- .../PrefixSuffixTransformer_test.go | 3 + .../ReplicaCountTransformer.go | 4 +- 23 files changed, 376 insertions(+), 442 deletions(-) diff --git a/api/builtins/HashTransformer.go b/api/builtins/HashTransformer.go index bf308e51d..c7ce6f7e8 100644 --- a/api/builtins/HashTransformer.go +++ b/api/builtins/HashTransformer.go @@ -28,7 +28,7 @@ func (p *HashTransformerPlugin) Transform(m resmap.ResMap) error { if err != nil { return err } - res.SetOriginalName(res.GetName(), false) + res.StorePreviousId() res.SetName(fmt.Sprintf("%s-%s", res.GetName(), h)) } } diff --git a/api/builtins/NamespaceTransformer.go b/api/builtins/NamespaceTransformer.go index b2a6896e1..f97800f84 100644 --- a/api/builtins/NamespaceTransformer.go +++ b/api/builtins/NamespaceTransformer.go @@ -34,7 +34,7 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } - r.SetOriginalNs(r.GetNamespace(), false) + r.StorePreviousId() err := r.ApplyFilter(namespace.Filter{ Namespace: p.Namespace, FsSlice: p.FieldSpecs, diff --git a/api/builtins/PatchTransformer.go b/api/builtins/PatchTransformer.go index b0c8d4b04..0696f3e1a 100644 --- a/api/builtins/PatchTransformer.go +++ b/api/builtins/PatchTransformer.go @@ -104,7 +104,7 @@ func (p *PatchTransformerPlugin) transformJson6902(m resmap.ResMap, patch jsonpa return err } for _, res := range resources { - res.SetOriginalName(res.GetName(), false) + res.StorePreviousId() err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.Patch, }) diff --git a/api/builtins/PrefixSuffixTransformer.go b/api/builtins/PrefixSuffixTransformer.go index 858ebc14c..20265d592 100644 --- a/api/builtins/PrefixSuffixTransformer.go +++ b/api/builtins/PrefixSuffixTransformer.go @@ -70,7 +70,7 @@ func (p *PrefixSuffixTransformerPlugin) Transform(m resmap.ResMap) error { r.AddNamePrefix(p.Prefix) r.AddNameSuffix(p.Suffix) if p.Prefix != "" || p.Suffix != "" { - r.SetOriginalName(r.GetName(), false) + r.StorePreviousId() } } err := r.ApplyFilter(prefixsuffix.Filter{ diff --git a/api/builtins/ReplicaCountTransformer.go b/api/builtins/ReplicaCountTransformer.go index 47d21417d..03fbf6f1e 100644 --- a/api/builtins/ReplicaCountTransformer.go +++ b/api/builtins/ReplicaCountTransformer.go @@ -31,9 +31,7 @@ func (p *ReplicaCountTransformerPlugin) Transform(m resmap.ResMap) error { found := false for _, fs := range p.FieldSpecs { matcher := p.createMatcher(fs) - matchOriginal := m.GetMatchingResourcesByOriginalId(matcher) - resList := append( - matchOriginal, m.GetMatchingResourcesByCurrentId(matcher)...) + resList := m.GetMatchingResourcesByAnyId(matcher) if len(resList) > 0 { found = true for _, r := range resList { diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index 329c60420..1ab04eced 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -241,7 +241,7 @@ func acceptAll(r *resource.Resource) bool { func originalNameMatches(name string) sieveFunc { return func(r *resource.Resource) bool { - return r.GetOriginalName() == name + return r.OrgId().Name == name } } @@ -304,7 +304,7 @@ func (f Filter) sameCurrentNamespaceAsReferrer() sieveFunc { // selectReferral picks the best referral from a list of candidates. func (f Filter) selectReferral( - // The name referral that may need to be updated. +// The name referral that may need to be updated. oldName string, candidates []*resource.Resource) (*resource.Resource, error) { candidates = doSieve(candidates, originalNameMatches(oldName)) diff --git a/api/internal/accumulator/resaccumulator.go b/api/internal/accumulator/resaccumulator.go index f9c0f0764..b49db9b12 100644 --- a/api/internal/accumulator/resaccumulator.go +++ b/api/internal/accumulator/resaccumulator.go @@ -77,7 +77,7 @@ func (ra *ResAccumulator) MergeVars(incoming []types.Var) error { // wildcard search on the namespace hence we still use GvknEquals idMatcher = targetId.Equals } - matched := ra.resMap.GetMatchingResourcesByOriginalId(idMatcher) + matched := ra.resMap.GetMatchingResourcesByAnyId(idMatcher) if len(matched) > 1 { return fmt.Errorf( "found %d resId matches for var %s "+ diff --git a/api/internal/accumulator/resaccumulator_test.go b/api/internal/accumulator/resaccumulator_test.go index 8ac53a331..b853f4728 100644 --- a/api/internal/accumulator/resaccumulator_test.go +++ b/api/internal/accumulator/resaccumulator_test.go @@ -344,20 +344,20 @@ func TestResolveVarsWithNoambiguation(t *testing.T) { }, }, }}). + // Make it seem like this resource + // went through a prefix transformer. Add(map[string]interface{}{ "apiVersion": "v1", "kind": "Service", "metadata": map[string]interface{}{ - "name": "backendOne", + "name": "sub-backendOne", + "annotations": map[string]interface{}{ + "config.kubernetes.io/originalName": "backendOne", + "config.kubernetes.io/originalNs": "default", + "config.kubernetes.io/prefixes": "sub-", + }, }}).ResMap() - // Make it seem like this resource - // went through a prefix transformer. - r := m.GetByIndex(1) - r.AddNamePrefix("sub-") - r.SetName("sub-backendOne") - r.SetOriginalName("backendOne", true) - err = ra2.AppendAll(m) if err != nil { t.Fatalf("unexpected err: %v", err) diff --git a/api/internal/plugins/utils/utils.go b/api/internal/plugins/utils/utils.go index 46706627b..9e54983fd 100644 --- a/api/internal/plugins/utils/utils.go +++ b/api/internal/plugins/utils/utils.go @@ -135,9 +135,6 @@ func GetResMapWithIDAnnotation(rm resmap.ResMap) (resmap.ResMap, error) { return nil, err } annotations := r.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } annotations[idAnnotation] = string(idString) r.SetAnnotations(annotations) } diff --git a/api/resmap/merginator.go b/api/resmap/merginator.go index 0ca5b347c..12979063b 100644 --- a/api/resmap/merginator.go +++ b/api/resmap/merginator.go @@ -47,7 +47,7 @@ func (m *merginator) ConflatePatches(in []*resource.Resource) (ResMap, error) { func (m *merginator) appendIfNoMatch(index int) (*resource.Resource, error) { candidate := m.incoming[index] - matchedResources := m.result.GetMatchingResourcesByOriginalId( + matchedResources := m.result.GetMatchingResourcesByAnyId( candidate.OrgId().Equals) if len(matchedResources) == 0 { m.result.Append(candidate) diff --git a/api/resmap/resmap.go b/api/resmap/resmap.go index 4b977907b..f5d99abb6 100644 --- a/api/resmap/resmap.go +++ b/api/resmap/resmap.go @@ -142,24 +142,18 @@ type ResMap interface { // who's CurId is matched by the argument. GetMatchingResourcesByCurrentId(matches IdMatcher) []*resource.Resource - // GetMatchingResourcesByOriginalId returns the resources - // who's OriginalId is matched by the argument. - GetMatchingResourcesByOriginalId(matches IdMatcher) []*resource.Resource + // GetMatchingResourcesByAnyId returns the resources + // who's current or previous IDs is matched by the argument. + GetMatchingResourcesByAnyId(matches IdMatcher) []*resource.Resource // GetByCurrentId is shorthand for calling // GetMatchingResourcesByCurrentId with a matcher requiring // an exact match, returning an error on multiple or no matches. GetByCurrentId(resid.ResId) (*resource.Resource, error) - // GetByOriginalId is shorthand for calling - // GetMatchingResourcesByOriginalId with a matcher requiring + // GetByPreviousId is shorthand for calling + // GetMatchingResourcesByAnyId with a matcher requiring // an exact match, returning an error on multiple or no matches. - GetByOriginalId(resid.ResId) (*resource.Resource, error) - - // GetById is a helper function which first - // attempts GetByOriginalId, then GetByCurrentId, - // returning an error if both fail to find a single - // match. GetById(resid.ResId) (*resource.Resource, error) // GroupedByCurrentNamespace returns a map of namespace diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index 79ee617c9..95b8369f5 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -155,8 +155,7 @@ func (m *resWrangler) GetIndexOfCurrentId(id resid.ResId) (int, error) { type IdFromResource func(r *resource.Resource) resid.ResId -func GetOriginalId(r *resource.Resource) resid.ResId { return r.OrgId() } -func GetCurrentId(r *resource.Resource) resid.ResId { return r.CurId() } +func GetCurrentId(r *resource.Resource) resid.ResId { return r.CurId() } // GetMatchingResourcesByCurrentId implements ResMap. func (m *resWrangler) GetMatchingResourcesByCurrentId( @@ -164,10 +163,21 @@ func (m *resWrangler) GetMatchingResourcesByCurrentId( return m.filteredById(matches, GetCurrentId) } -// GetMatchingResourcesByOriginalId implements ResMap. -func (m *resWrangler) GetMatchingResourcesByOriginalId( +// GetMatchingResourcesByAnyId implements ResMap. +func (m *resWrangler) GetMatchingResourcesByAnyId( matches IdMatcher) []*resource.Resource { - return m.filteredById(matches, GetOriginalId) + var result []*resource.Resource + for _, r := range m.rList { + prevIds := r.PrevIds() + prevIds = append(prevIds, r.CurId()) + for _, prevId := range prevIds { + if matches(prevId) { + result = append(result, r) + break + } + } + } + return result } func (m *resWrangler) filteredById( @@ -187,26 +197,16 @@ func (m *resWrangler) GetByCurrentId( return demandOneMatch(m.GetMatchingResourcesByCurrentId, id, "Current") } -// GetByOriginalId implements ResMap. -func (m *resWrangler) GetByOriginalId( - id resid.ResId) (*resource.Resource, error) { - return demandOneMatch(m.GetMatchingResourcesByOriginalId, id, "Original") -} - // GetById implements ResMap. func (m *resWrangler) GetById( id resid.ResId) (*resource.Resource, error) { - match, err1 := m.GetByOriginalId(id) - if err1 == nil { - return match, nil + r, err := demandOneMatch(m.GetMatchingResourcesByAnyId, id, "Id") + if err != nil { + return nil, fmt.Errorf( + "%s; failed to find unique target for patch %s", + err.Error(), id.GvknString()) } - match, err2 := m.GetByCurrentId(id) - if err2 == nil { - return match, nil - } - return nil, fmt.Errorf( - "%s; %s; failed to find unique target for patch %s", - err1.Error(), err2.Error(), id.GvknString()) + return r, nil } type resFinder func(IdMatcher) []*resource.Resource @@ -465,10 +465,7 @@ func (m *resWrangler) AbsorbAll(other ResMap) error { func (m *resWrangler) appendReplaceOrMerge(res *resource.Resource) error { id := res.CurId() - matches := m.GetMatchingResourcesByOriginalId(id.Equals) - if len(matches) == 0 { - matches = m.GetMatchingResourcesByCurrentId(id.Equals) - } + matches := m.GetMatchingResourcesByAnyId(id.Equals) switch len(matches) { case 0: switch res.Behavior() { @@ -593,10 +590,8 @@ func (m *resWrangler) ApplySmPatch( continue } patchCopy := patch.DeepCopy() - patchCopy.SetName(res.GetName()) - patchCopy.SetNamespace(res.GetNamespace()) + patchCopy.CopyMergeMetaDataFieldsFrom(patch) patchCopy.SetGvk(res.GetGvk()) - patchCopy.SetOriginalName(res.GetOriginalName(), true) err := res.ApplySmPatch(patchCopy) if err != nil { // Check for an error string from UnmarshalJSON that's indicative diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index 57f618cbb..26d36cff4 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -331,6 +331,134 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { } } +func TestGetMatchingResourcesByPreviousId(t *testing.T) { + r1 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "new-alice", + "annotations": map[string]interface{}{ + "config.kubernetes.io/originalName": "alice", + "config.kubernetes.io/originalNs": "default", + }, + }, + }) + r2 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "new-bob", + "annotations": map[string]interface{}{ + "config.kubernetes.io/originalName": "bob,bob2", + "config.kubernetes.io/originalNs": "default,default", + }, + }, + }) + r3 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "new-bob", + "namespace": "new-happy", + "annotations": map[string]interface{}{ + "config.kubernetes.io/originalName": "bob", + "config.kubernetes.io/originalNs": "happy", + }, + }, + }) + r4 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "charlie", + "namespace": "happy", + "annotations": map[string]interface{}{ + "config.kubernetes.io/originalName": "charlie", + "config.kubernetes.io/originalNs": "default", + }, + }, + }) + r5 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "charlie", + "namespace": "happy", + }, + }) + + m := resmaptest_test.NewRmBuilder(t, rf). + AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).ResMap() + + // nolint:goconst + tests := []struct { + name string + matcher IdMatcher + count int + }{ + { + "match everything", + func(resid.ResId) bool { return true }, + 5, + }, + { + "match nothing", + func(resid.ResId) bool { return false }, + 0, + }, + { + "name is alice", + func(x resid.ResId) bool { return x.Name == "alice" }, + 1, + }, + { + "name is charlie", + func(x resid.ResId) bool { return x.Name == "charlie" }, + 2, + }, + { + "name is bob", + func(x resid.ResId) bool { return x.Name == "bob" }, + 2, + }, + { + "happy namespace", + func(x resid.ResId) bool { + return x.Namespace == "happy" + }, + 3, + }, + { + "happy deployment", + func(x resid.ResId) bool { + return x.Namespace == "happy" && + x.Gvk.Kind == "Deployment" + }, + 1, + }, + { + "happy ConfigMap", + func(x resid.ResId) bool { + return x.Namespace == "happy" && + x.Gvk.Kind == "ConfigMap" + }, + 2, + }, + } + for _, tst := range tests { + result := m.GetMatchingResourcesByAnyId(tst.matcher) + if len(result) != tst.count { + t.Fatalf("test '%s'; actual: %d, expected: %d", + tst.name, len(result), tst.count) + } + } +} + func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { r1 := rf.FromMap( map[string]interface{}{ diff --git a/api/resource/factory.go b/api/resource/factory.go index 8e60f3e44..685759160 100644 --- a/api/resource/factory.go +++ b/api/resource/factory.go @@ -11,6 +11,7 @@ import ( "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/kusterr" + "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/types" ) @@ -35,17 +36,12 @@ func (rf *Factory) FromMap(m map[string]interface{}) *Resource { // FromMapWithName returns a new instance with the given "original" name. func (rf *Factory) FromMapWithName(n string, m map[string]interface{}) *Resource { - return rf.makeOne(rf.kf.FromMap(m), nil).SetOriginalName(n, true) -} - -// FromMapWithNamespace returns a new instance with the given "original" namespace. -func (rf *Factory) FromMapWithNamespace(n string, m map[string]interface{}) *Resource { - return rf.makeOne(rf.kf.FromMap(m), nil).SetOriginalNs(n, true) + return rf.FromMapWithNamespaceAndName(resid.DefaultNamespace, n, m) } // FromMapWithNamespaceAndName returns a new instance with the given "original" namespace. func (rf *Factory) FromMapWithNamespaceAndName(ns string, n string, m map[string]interface{}) *Resource { - return rf.makeOne(rf.kf.FromMap(m), nil).SetOriginalNs(ns, true).SetOriginalName(n, true) + return rf.makeOne(rf.kf.FromMap(m), nil).setPreviousNamespaceAndName(ns, n) } // FromMapAndOption returns a new instance of Resource with given options. @@ -157,7 +153,7 @@ func (rf *Factory) SliceFromBytesWithNames(names []string, in []byte) ([]*Resour return nil, fmt.Errorf("number of names doesn't match number of resources") } for i, res := range result { - res.SetOriginalName(names[i], true) + res.setPreviousNamespaceAndName("", names[i]) } return result, nil } diff --git a/api/resource/resource.go b/api/resource/resource.go index b2dd8abb4..56c0694bf 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -32,18 +32,29 @@ type Resource struct { } const ( - buildAnnotationOriginalName = konfig.ConfigAnnoDomain + "/originalName" + buildAnnotationPreviousName = konfig.ConfigAnnoDomain + "/originalName" buildAnnotationPrefixes = konfig.ConfigAnnoDomain + "/prefixes" buildAnnotationSuffixes = konfig.ConfigAnnoDomain + "/suffixes" - buildAnnotationOriginalNamespace = konfig.ConfigAnnoDomain + "/originalNs" + buildAnnotationPreviousNamespace = konfig.ConfigAnnoDomain + "/originalNs" ) +var buildAnnotations = []string{ + buildAnnotationPreviousName, + buildAnnotationPrefixes, + buildAnnotationSuffixes, + buildAnnotationPreviousNamespace, +} + func (r *Resource) ResetPrimaryData(incoming *Resource) { r.kunStr = incoming.Copy() } func (r *Resource) GetAnnotations() map[string]string { - return r.kunStr.GetAnnotations() + annotations := r.kunStr.GetAnnotations() + if annotations == nil { + return make(map[string]string) + } + return annotations } func (r *Resource) Copy() ifc.Kunstructured { @@ -146,8 +157,6 @@ func (r *Resource) UnmarshalJSON(s []byte) error { type ResCtx interface { AddNamePrefix(p string) AddNameSuffix(s string) - GetOutermostNamePrefix() string - GetOutermostNameSuffix() string GetNamePrefixes() []string GetNameSuffixes() []string } @@ -252,22 +261,19 @@ func copyStringSlice(s []string) []string { // Implements ResCtx AddNamePrefix func (r *Resource) AddNamePrefix(p string) { - r.addAdditiveAnnotation(buildAnnotationPrefixes, p) + r.appendCsvAnnotation(buildAnnotationPrefixes, p) } // Implements ResCtx AddNameSuffix func (r *Resource) AddNameSuffix(s string) { - r.addAdditiveAnnotation(buildAnnotationSuffixes, s) + r.appendCsvAnnotation(buildAnnotationSuffixes, s) } -func (r *Resource) addAdditiveAnnotation(name, value string) { +func (r *Resource) appendCsvAnnotation(name, value string) { if value == "" { return } annotations := r.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } if existing, ok := annotations[name]; ok { annotations[name] = existing + "," + value } else { @@ -312,26 +318,20 @@ func SameEndingSubarray(shortest, longest []string) bool { // Implements ResCtx GetNamePrefixes func (r *Resource) GetNamePrefixes() []string { - annotations := r.GetAnnotations() - if _, ok := annotations[buildAnnotationPrefixes]; !ok { - return nil - } - return strings.Split(annotations[buildAnnotationPrefixes], ",") + return r.getCsvAnnotation(buildAnnotationPrefixes) } // Implements ResCtx GetNameSuffixes func (r *Resource) GetNameSuffixes() []string { - annotations := r.GetAnnotations() - if _, ok := annotations[buildAnnotationSuffixes]; !ok { - return nil - } - return strings.Split(annotations[buildAnnotationSuffixes], ",") + return r.getCsvAnnotation(buildAnnotationSuffixes) } -// OutermostPrefixSuffixEquals returns true if both resources -// outer suffix and prefix matches. -func (r *Resource) OutermostPrefixSuffixEquals(o ResCtx) bool { - return (r.GetOutermostNamePrefix() == o.GetOutermostNamePrefix()) && (r.GetOutermostNameSuffix() == o.GetOutermostNameSuffix()) +func (r *Resource) getCsvAnnotation(name string) []string { + annotations := r.GetAnnotations() + if _, ok := annotations[name]; !ok { + return nil + } + return strings.Split(annotations[name], ",") } // PrefixesSuffixesEquals is conceptually doing the same task @@ -349,57 +349,20 @@ func (r *Resource) RemoveBuildAnnotations() { if len(annotations) == 0 { return } - delete(annotations, buildAnnotationOriginalName) - delete(annotations, buildAnnotationPrefixes) - delete(annotations, buildAnnotationSuffixes) - delete(annotations, buildAnnotationOriginalNamespace) - r.SetAnnotations(annotations) -} - -func (r *Resource) GetOriginalName() string { - annotations := r.GetAnnotations() - if name, ok := annotations[buildAnnotationOriginalName]; ok { - return name - } - return r.kunStr.GetName() -} - -func (r *Resource) SetOriginalName(n string, overwrite bool) *Resource { - annotations := r.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - if _, ok := annotations[buildAnnotationOriginalName]; !ok || overwrite { - annotations[buildAnnotationOriginalName] = n - } - r.kunStr.SetAnnotations(annotations) - return r -} - -func (r *Resource) GetOriginalNs() string { - annotations := r.GetAnnotations() - if ns, ok := annotations[buildAnnotationOriginalNamespace]; ok { - return ns - } - ns := r.GetNamespace() - if ns == "default" { - return "" - } - return ns -} - -func (r *Resource) SetOriginalNs(n string, overwrite bool) *Resource { - if n == "" { - n = "default" - } - annotations := r.GetAnnotations() - if annotations == nil { - annotations = make(map[string]string) - } - if _, ok := annotations[buildAnnotationOriginalNamespace]; !ok || overwrite { - annotations[buildAnnotationOriginalNamespace] = n + for _, a := range buildAnnotations { + delete(annotations, a) } r.SetAnnotations(annotations) +} + +func (r *Resource) setPreviousNamespaceAndName(ns string, n string) *Resource { + // name + r.appendCsvAnnotation(buildAnnotationPreviousName, n) + // namespace + if ns == "" { + ns = resid.DefaultNamespace + } + r.appendCsvAnnotation(buildAnnotationPreviousNamespace, ns) return r } @@ -456,10 +419,36 @@ func (r *Resource) GetNamespace() string { // OrgId returns the original, immutable ResId for the resource. // This doesn't have to be unique in a ResMap. -// TODO: compute this once and save it in the resource. func (r *Resource) OrgId() resid.ResId { - return resid.NewResIdWithNamespace( - r.GetGvk(), r.GetOriginalName(), r.GetOriginalNs()) + ids := r.PrevIds() + if len(ids) > 0 { + return ids[0] + } + return r.CurId() +} + +// PrevIds returns a list of ResIds that includes every +// previous ResId the resource has had through all of its +// GVKN transformations, in the order that it had that ID. +// The returned array does not include the resource's current +// ID. +// If there are no previous IDs, this will return nil. +func (r *Resource) PrevIds() []resid.ResId { + var ids []resid.ResId + + // names and ns should always be the same length + names := r.getCsvAnnotation(buildAnnotationPreviousName) + ns := r.getCsvAnnotation(buildAnnotationPreviousNamespace) + for i := range names { + ids = append(ids, resid.NewResIdWithNamespace( + r.GetGvk(), names[i], ns[i])) + } + return ids +} + +// StorePreviousId stores the resource's current ID via build annotations. +func (r *Resource) StorePreviousId() { + r.setPreviousNamespaceAndName(r.GetNamespace(), r.GetName()) } // CurId returns a ResId for the resource using the diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index fc5460406..01c4f3d09 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -695,322 +695,165 @@ spec: } } -func TestSetOriginalNameAndNs(t *testing.T) { - input := `apiVersion: apps/v1 +func TestResource_StorePreviousId(t *testing.T) { + tests := map[string]struct { + input string + newName string + newNs string + expected string + }{ + "default namespace, first previous name": { + input: `apiVersion: apps/v1 kind: Secret metadata: - name: newName` - - factory := provider.NewDefaultDepProvider().GetResourceFactory() - resources, err := factory.SliceFromBytes([]byte(input)) - if err != nil { - t.Fatal(err) - } - - res := resources[0] - res.SetOriginalName("oldName", false) - res.SetOriginalNs("default", false) - - expected := `apiVersion: apps/v1 + name: oldName +`, + newName: "newName", + newNs: "", + expected: `apiVersion: apps/v1 kind: Secret metadata: annotations: config.kubernetes.io/originalName: oldName config.kubernetes.io/originalNs: default name: newName -` - bytes, err := res.AsYAML() - if err != nil { - t.Fatal(err) +`, + }, + + "default namespace, second previous name": { + input: `apiVersion: apps/v1 +kind: Secret +metadata: + annotations: + config.kubernetes.io/originalName: oldName + config.kubernetes.io/originalNs: default + name: oldName2 +`, + newName: "newName", + newNs: "", + expected: `apiVersion: apps/v1 +kind: Secret +metadata: + annotations: + config.kubernetes.io/originalName: oldName,oldName2 + config.kubernetes.io/originalNs: default,default + name: newName +`, + }, + + "non-default namespace": { + input: `apiVersion: apps/v1 +kind: Secret +metadata: + annotations: + config.kubernetes.io/originalName: oldName + config.kubernetes.io/originalNs: default + name: oldName2 + namespace: oldNamespace +`, + newName: "newName", + newNs: "newNamespace", + expected: `apiVersion: apps/v1 +kind: Secret +metadata: + annotations: + config.kubernetes.io/originalName: oldName,oldName2 + config.kubernetes.io/originalNs: default,oldNamespace + name: newName + namespace: newNamespace +`, + }, + } + factory := provider.NewDefaultDepProvider().GetResourceFactory() + for i := range tests { + test := tests[i] + t.Run(i, func(t *testing.T) { + resources, err := factory.SliceFromBytes([]byte(test.input)) + if !assert.NoError(t, err) || len(resources) == 0 { + t.FailNow() + } + r := resources[0] + r.StorePreviousId() + r.SetName(test.newName) + if test.newNs != "" { + r.SetNamespace(test.newNs) + } + bytes, err := r.AsYAML() + if !assert.NoError(t, err) { + t.FailNow() + } + assert.Equal(t, test.expected, string(bytes)) + }) } - assert.Equal(t, expected, string(bytes)) } -func TestGetOriginalName(t *testing.T) { - tests := []struct { +func TestResource_PrevIds(t *testing.T) { + tests := map[string]struct { input string - expected string + expected []resid.ResId }{ - { - // no name annotation, return the name + "no previous IDs": { input: `apiVersion: apps/v1 kind: Secret metadata: - name: mySecret`, - expected: "mySecret", + name: name +`, + expected: nil, }, - { - // return name from name annotation + "one previous ID": { input: `apiVersion: apps/v1 kind: Secret metadata: annotations: config.kubernetes.io/originalName: oldName - name: newName`, - expected: "oldName", - }, - } - - for _, test := range tests { - factory := provider.NewDefaultDepProvider().GetResourceFactory() - resources, err := factory.SliceFromBytes([]byte(test.input)) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.expected, resources[0].GetOriginalName()) - } -} - -func TestSetOriginalName(t *testing.T) { - tests := []struct { - input string - originalName string - overwrite bool - expected string - }{ - { - // no original name set, overwrite is false - input: `apiVersion: apps/v1 -kind: Secret -metadata: - name: newName`, - originalName: "oldName", - overwrite: false, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalName: oldName + config.kubernetes.io/originalNs: default name: newName `, + expected: []resid.ResId{ + { + Gvk: resid.Gvk{Group: "apps", Version: "v1", Kind: "Secret"}, + Name: "oldName", + Namespace: resid.DefaultNamespace, + }, + }, }, - { - // no original name set, overwrite is true - input: `apiVersion: apps/v1 -kind: Secret -metadata: - name: newName`, - originalName: "oldName", - overwrite: true, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalName: oldName - name: newName -`, - }, - - { - // original name is set, overwrite is false, resource shouldn't change + "two ids": { input: `apiVersion: apps/v1 kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName - name: newName`, - originalName: "newOriginalName", - overwrite: false, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalName: oldName - name: newName -`, - }, - - { - // original name is set, overwrite is true, resource should change - input: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalName: oldName - name: newName`, - originalName: "newOriginalName", - overwrite: true, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalName: newOriginalName - name: newName -`, - }, - } - - for _, test := range tests { - factory := provider.NewDefaultDepProvider().GetResourceFactory() - resources, err := factory.SliceFromBytes([]byte(test.input)) - if err != nil { - t.Fatal(err) - } - - res := resources[0] - res.SetOriginalName(test.originalName, test.overwrite) - - bytes, err := res.AsYAML() - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.expected, string(bytes)) - } -} - -func TestGetOriginalNs(t *testing.T) { - tests := []struct { - input string - expected string - }{ - { - // no namespace, return default - input: `apiVersion: apps/v1 -kind: Secret -metadata: - name: mySecret`, - expected: "", - }, - - { - // return old namespace - input: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalNs: oldNamespace - name: mySecret - namespace: myNamespace`, - expected: "oldNamespace", - }, - - { - // return namespace - input: `apiVersion: apps/v1 -kind: Secret -metadata: - name: mySecret - namespace: myNamespace`, - expected: "myNamespace", - }, - } - - for _, test := range tests { - factory := provider.NewDefaultDepProvider().GetResourceFactory() - resources, err := factory.SliceFromBytes([]byte(test.input)) - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.expected, resources[0].GetOriginalNs()) - } -} - -func TestSetOriginalNs(t *testing.T) { - tests := []struct { - input string - originalNs string - overwrite bool - expected string - }{ - { - // no original namespace set, overwrite is false - input: `apiVersion: apps/v1 -kind: Secret -metadata: - name: newName - namespace: newNamespace`, - originalNs: "oldNamespace", - overwrite: false, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalNs: oldNamespace - name: newName - namespace: newNamespace -`, - }, - - { - // no original name set, overwrite is true - input: `apiVersion: apps/v1 -kind: Secret -metadata: - name: newName - namespace: newNamespace`, - - originalNs: "oldNamespace", - overwrite: true, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalNs: oldNamespace - name: newName - namespace: newNamespace -`, - }, - - { - // original name is set, overwrite is false, resource shouldn't change - input: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalNs: oldNamespace - name: newName - namespace: newNamespace`, - originalNs: "newOriginalNamespace", - overwrite: false, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalNs: oldNamespace - name: newName - namespace: newNamespace -`, - }, - - { - // original name is set, overwrite is true, resource should change - input: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalNs: oldNamespace - name: newName - namespace: newNamespace`, - originalNs: "newOriginalNamespace", - overwrite: true, - expected: `apiVersion: apps/v1 -kind: Secret -metadata: - annotations: - config.kubernetes.io/originalNs: newOriginalNamespace + config.kubernetes.io/originalName: oldName,oldName2 + config.kubernetes.io/originalNs: default,oldNamespace name: newName namespace: newNamespace `, + expected: []resid.ResId{ + { + Gvk: resid.Gvk{Group: "apps", Version: "v1", Kind: "Secret"}, + Name: "oldName", + Namespace: resid.DefaultNamespace, + }, + { + Gvk: resid.Gvk{Group: "apps", Version: "v1", Kind: "Secret"}, + Name: "oldName2", + Namespace: "oldNamespace", + }, + }, }, } - - for _, test := range tests { - factory := provider.NewDefaultDepProvider().GetResourceFactory() - resources, err := factory.SliceFromBytes([]byte(test.input)) - if err != nil { - t.Fatal(err) - } - - res := resources[0] - res.SetOriginalNs(test.originalNs, test.overwrite) - - bytes, err := res.AsYAML() - if err != nil { - t.Fatal(err) - } - assert.Equal(t, test.expected, string(bytes)) + factory := provider.NewDefaultDepProvider().GetResourceFactory() + for i := range tests { + test := tests[i] + t.Run(i, func(t *testing.T) { + resources, err := factory.SliceFromBytes([]byte(test.input)) + if !assert.NoError(t, err) || len(resources) == 0 { + t.FailNow() + } + r := resources[0] + assert.Equal(t, test.expected, r.PrevIds()) + }) } } diff --git a/api/testutils/resmaptest/rmbuilder.go b/api/testutils/resmaptest/rmbuilder.go index c856c81b4..7acfd90e9 100644 --- a/api/testutils/resmaptest/rmbuilder.go +++ b/api/testutils/resmaptest/rmbuilder.go @@ -7,6 +7,7 @@ import ( "testing" "sigs.k8s.io/kustomize/api/provider" + "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" ) @@ -48,15 +49,7 @@ func (rm *rmBuilder) AddR(r *resource.Resource) *rmBuilder { } func (rm *rmBuilder) AddWithName(n string, m map[string]interface{}) *rmBuilder { - err := rm.m.Append(rm.rf.FromMapWithName(n, m)) - if err != nil { - rm.t.Fatalf("test setup failure: %v", err) - } - return rm -} - -func (rm *rmBuilder) AddWithNs(ns string, m map[string]interface{}) *rmBuilder { - err := rm.m.Append(rm.rf.FromMapWithNamespace(ns, m)) + err := rm.m.Append(rm.rf.FromMapWithNamespaceAndName(resid.DefaultNamespace, n, m)) if err != nil { rm.t.Fatalf("test setup failure: %v", err) } diff --git a/plugin/builtin/hashtransformer/HashTransformer.go b/plugin/builtin/hashtransformer/HashTransformer.go index c1a7dccdd..1b86f541a 100644 --- a/plugin/builtin/hashtransformer/HashTransformer.go +++ b/plugin/builtin/hashtransformer/HashTransformer.go @@ -32,7 +32,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { if err != nil { return err } - res.SetOriginalName(res.GetName(), false) + res.StorePreviousId() res.SetName(fmt.Sprintf("%s-%s", res.GetName(), h)) } } diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index d6c7024f1..38d1e4d79 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer.go @@ -38,7 +38,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } - r.SetOriginalNs(r.GetNamespace(), false) + r.StorePreviousId() err := r.ApplyFilter(namespace.Filter{ Namespace: p.Namespace, FsSlice: p.FieldSpecs, diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index 8030ef0e6..1da970b70 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -108,7 +108,7 @@ func (p *plugin) transformJson6902(m resmap.ResMap, patch jsonpatch.Patch) error return err } for _, res := range resources { - res.SetOriginalName(res.GetName(), false) + res.StorePreviousId() err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.Patch, }) diff --git a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go index 9c755c1b3..169f4e96b 100644 --- a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go +++ b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go @@ -73,7 +73,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { r.AddNamePrefix(p.Prefix) r.AddNameSuffix(p.Suffix) if p.Prefix != "" || p.Suffix != "" { - r.SetOriginalName(r.GetName(), false) + r.StorePreviousId() } } err := r.ApplyFilter(prefixsuffix.Filter{ diff --git a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go index 9e3efcb34..880c70796 100644 --- a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go +++ b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go @@ -64,6 +64,7 @@ kind: Service metadata: annotations: config.kubernetes.io/originalName: apple + config.kubernetes.io/originalNs: default config.kubernetes.io/prefixes: baked- config.kubernetes.io/suffixes: -pie name: baked-apple-pie @@ -86,6 +87,7 @@ kind: ConfigMap metadata: annotations: config.kubernetes.io/originalName: cm + config.kubernetes.io/originalNs: default config.kubernetes.io/prefixes: baked- config.kubernetes.io/suffixes: -pie name: baked-cm-pie @@ -136,6 +138,7 @@ kind: Deployment metadata: annotations: config.kubernetes.io/originalName: deployment + config.kubernetes.io/originalNs: default config.kubernetes.io/prefixes: test- name: test-deployment spec: diff --git a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go index 7a479812f..b3d0e1078 100644 --- a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go +++ b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go @@ -35,9 +35,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { found := false for _, fs := range p.FieldSpecs { matcher := p.createMatcher(fs) - matchOriginal := m.GetMatchingResourcesByOriginalId(matcher) - resList := append( - matchOriginal, m.GetMatchingResourcesByCurrentId(matcher)...) + resList := m.GetMatchingResourcesByAnyId(matcher) if len(resList) > 0 { found = true for _, r := range resList {