From 43157f5d35216dcfd6e47a4db0d52b071d776420 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Wed, 3 Feb 2021 14:30:25 -0800 Subject: [PATCH] cleaned up resource refactoring --- api/filters/nameref/nameref.go | 2 +- api/filters/nameref/nameref_test.go | 8 ++-- .../accumulator/resaccumulator_test.go | 6 +-- api/resmap/reswrangler.go | 6 +-- api/resmap/reswrangler_test.go | 18 ++++---- api/resource/factory.go | 2 +- api/resource/resource.go | 42 ++++++++++--------- api/resource/resource_test.go | 28 ++++++------- .../PrefixSuffixTransformer_test.go | 12 +++--- 9 files changed, 62 insertions(+), 62 deletions(-) diff --git a/api/filters/nameref/nameref.go b/api/filters/nameref/nameref.go index 1ab04eced..125dcebbc 100644 --- a/api/filters/nameref/nameref.go +++ b/api/filters/nameref/nameref.go @@ -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/filters/nameref/nameref_test.go b/api/filters/nameref/nameref_test.go index a41f016a2..8ddc2aa84 100644 --- a/api/filters/nameref/nameref_test.go +++ b/api/filters/nameref/nameref_test.go @@ -40,7 +40,7 @@ kind: NotSecret metadata: name: newName2 `, - originalNames: []string{"oldName", ""}, + originalNames: []string{"oldName", "newName2"}, referrerFinal: ` apiVersion: apps/v1 kind: Deployment @@ -79,7 +79,7 @@ kind: NotSecret metadata: name: newName2 `, - originalNames: []string{"oldName1", ""}, + originalNames: []string{"oldName1", "newName2"}, referrerFinal: ` apiVersion: apps/v1 kind: Deployment @@ -118,7 +118,7 @@ kind: NotSecret metadata: name: newName2 `, - originalNames: []string{"oldName", ""}, + originalNames: []string{"oldName", "newName2"}, referrerFinal: ` apiVersion: apps/v1 kind: Deployment @@ -204,7 +204,7 @@ kind: NotSecret metadata: name: newName2 `, - originalNames: []string{"oldName", ""}, + originalNames: []string{"oldName", "newName2"}, referrerFinal: ` apiVersion: apps/v1 kind: Deployment diff --git a/api/internal/accumulator/resaccumulator_test.go b/api/internal/accumulator/resaccumulator_test.go index b853f4728..e10972298 100644 --- a/api/internal/accumulator/resaccumulator_test.go +++ b/api/internal/accumulator/resaccumulator_test.go @@ -352,9 +352,9 @@ func TestResolveVarsWithNoambiguation(t *testing.T) { "metadata": map[string]interface{}{ "name": "sub-backendOne", "annotations": map[string]interface{}{ - "config.kubernetes.io/originalName": "backendOne", - "config.kubernetes.io/originalNs": "default", - "config.kubernetes.io/prefixes": "sub-", + "config.kubernetes.io/previousNames": "backendOne", + "config.kubernetes.io/previousNamespaces": "default", + "config.kubernetes.io/prefixes": "sub-", }, }}).ResMap() diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index 95b8369f5..efd37fc99 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -168,10 +168,8 @@ func (m *resWrangler) GetMatchingResourcesByAnyId( matches IdMatcher) []*resource.Resource { var result []*resource.Resource for _, r := range m.rList { - prevIds := r.PrevIds() - prevIds = append(prevIds, r.CurId()) - for _, prevId := range prevIds { - if matches(prevId) { + for _, id := range append(r.PrevIds(), r.CurId()) { + if matches(id) { result = append(result, r) break } diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index 26d36cff4..6b3d8c387 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -331,7 +331,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { } } -func TestGetMatchingResourcesByPreviousId(t *testing.T) { +func TestGetMatchingResourcesByAnyId(t *testing.T) { r1 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -339,8 +339,8 @@ func TestGetMatchingResourcesByPreviousId(t *testing.T) { "metadata": map[string]interface{}{ "name": "new-alice", "annotations": map[string]interface{}{ - "config.kubernetes.io/originalName": "alice", - "config.kubernetes.io/originalNs": "default", + "config.kubernetes.io/previousNames": "alice", + "config.kubernetes.io/previousNamespaces": "default", }, }, }) @@ -351,8 +351,8 @@ func TestGetMatchingResourcesByPreviousId(t *testing.T) { "metadata": map[string]interface{}{ "name": "new-bob", "annotations": map[string]interface{}{ - "config.kubernetes.io/originalName": "bob,bob2", - "config.kubernetes.io/originalNs": "default,default", + "config.kubernetes.io/previousNames": "bob,bob2", + "config.kubernetes.io/previousNamespaces": "default,default", }, }, }) @@ -364,8 +364,8 @@ func TestGetMatchingResourcesByPreviousId(t *testing.T) { "name": "new-bob", "namespace": "new-happy", "annotations": map[string]interface{}{ - "config.kubernetes.io/originalName": "bob", - "config.kubernetes.io/originalNs": "happy", + "config.kubernetes.io/previousNames": "bob", + "config.kubernetes.io/previousNamespaces": "happy", }, }, }) @@ -377,8 +377,8 @@ func TestGetMatchingResourcesByPreviousId(t *testing.T) { "name": "charlie", "namespace": "happy", "annotations": map[string]interface{}{ - "config.kubernetes.io/originalName": "charlie", - "config.kubernetes.io/originalNs": "default", + "config.kubernetes.io/previousNames": "charlie", + "config.kubernetes.io/previousNamespaces": "default", }, }, }) diff --git a/api/resource/factory.go b/api/resource/factory.go index 685759160..017b04b36 100644 --- a/api/resource/factory.go +++ b/api/resource/factory.go @@ -153,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.setPreviousNamespaceAndName("", names[i]) + res.setPreviousNamespaceAndName(resid.DefaultNamespace, names[i]) } return result, nil } diff --git a/api/resource/resource.go b/api/resource/resource.go index 9b132a21d..53a62dffd 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -4,6 +4,7 @@ package resource import ( + "errors" "fmt" "log" "reflect" @@ -32,17 +33,17 @@ type Resource struct { } const ( - buildAnnotationPreviousName = konfig.ConfigAnnoDomain + "/originalName" - buildAnnotationPrefixes = konfig.ConfigAnnoDomain + "/prefixes" - buildAnnotationSuffixes = konfig.ConfigAnnoDomain + "/suffixes" - buildAnnotationPreviousNamespace = konfig.ConfigAnnoDomain + "/originalNs" + buildAnnotationPreviousNames = konfig.ConfigAnnoDomain + "/previousNames" + buildAnnotationPrefixes = konfig.ConfigAnnoDomain + "/prefixes" + buildAnnotationSuffixes = konfig.ConfigAnnoDomain + "/suffixes" + buildAnnotationPreviousNamespaces = konfig.ConfigAnnoDomain + "/previousNamespaces" ) var buildAnnotations = []string{ - buildAnnotationPreviousName, + buildAnnotationPreviousNames, buildAnnotationPrefixes, buildAnnotationSuffixes, - buildAnnotationPreviousNamespace, + buildAnnotationPreviousNamespaces, } func (r *Resource) ResetPrimaryData(incoming *Resource) { @@ -338,13 +339,8 @@ func (r *Resource) RemoveBuildAnnotations() { } func (r *Resource) setPreviousNamespaceAndName(ns string, n string) *Resource { - // name - r.appendCsvAnnotation(buildAnnotationPreviousName, n) - // namespace - if ns == "" { - ns = resid.DefaultNamespace - } - r.appendCsvAnnotation(buildAnnotationPreviousNamespace, ns) + r.appendCsvAnnotation(buildAnnotationPreviousNames, n) + r.appendCsvAnnotation(buildAnnotationPreviousNamespaces, ns) return r } @@ -412,15 +408,20 @@ func (r *Resource) OrgId() resid.ResId { // 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. +// I.e. the oldest ID is first. // The returned array does not include the resource's current -// ID. -// If there are no previous IDs, this will return nil. +// 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) + // TODO: merge previous names and namespaces into one list of + // pairs on one annotation so there is no chance of error + names := r.getCsvAnnotation(buildAnnotationPreviousNames) + ns := r.getCsvAnnotation(buildAnnotationPreviousNamespaces) + if len(names) != len(ns) { + panic(errors.New( + "number of previous names not equal to " + + "number of previous namespaces")) + } for i := range names { ids = append(ids, resid.NewResIdWithNamespace( r.GetGvk(), names[i], ns[i])) @@ -430,7 +431,8 @@ func (r *Resource) PrevIds() []resid.ResId { // StorePreviousId stores the resource's current ID via build annotations. func (r *Resource) StorePreviousId() { - r.setPreviousNamespaceAndName(r.GetNamespace(), r.GetName()) + id := r.CurId() + r.setPreviousNamespaceAndName(id.EffectiveNamespace(), id.Name) } // CurId returns a ResId for the resource using the diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index 01c4f3d09..7329bf559 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -714,8 +714,8 @@ metadata: kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName - config.kubernetes.io/originalNs: default + config.kubernetes.io/previousNames: oldName + config.kubernetes.io/previousNamespaces: default name: newName `, }, @@ -725,8 +725,8 @@ metadata: kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName - config.kubernetes.io/originalNs: default + config.kubernetes.io/previousNames: oldName + config.kubernetes.io/previousNamespaces: default name: oldName2 `, newName: "newName", @@ -735,8 +735,8 @@ metadata: kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName,oldName2 - config.kubernetes.io/originalNs: default,default + config.kubernetes.io/previousNames: oldName,oldName2 + config.kubernetes.io/previousNamespaces: default,default name: newName `, }, @@ -746,8 +746,8 @@ metadata: kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName - config.kubernetes.io/originalNs: default + config.kubernetes.io/previousNames: oldName + config.kubernetes.io/previousNamespaces: default name: oldName2 namespace: oldNamespace `, @@ -757,8 +757,8 @@ metadata: kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName,oldName2 - config.kubernetes.io/originalNs: default,oldNamespace + config.kubernetes.io/previousNames: oldName,oldName2 + config.kubernetes.io/previousNamespaces: default,oldNamespace name: newName namespace: newNamespace `, @@ -806,8 +806,8 @@ metadata: kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName - config.kubernetes.io/originalNs: default + config.kubernetes.io/previousNames: oldName + config.kubernetes.io/previousNamespaces: default name: newName `, expected: []resid.ResId{ @@ -824,8 +824,8 @@ metadata: kind: Secret metadata: annotations: - config.kubernetes.io/originalName: oldName,oldName2 - config.kubernetes.io/originalNs: default,oldNamespace + config.kubernetes.io/previousNames: oldName,oldName2 + config.kubernetes.io/previousNamespaces: default,oldNamespace name: newName namespace: newNamespace `, diff --git a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go index 880c70796..192183991 100644 --- a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go +++ b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go @@ -63,9 +63,9 @@ apiVersion: v1 kind: Service metadata: annotations: - config.kubernetes.io/originalName: apple - config.kubernetes.io/originalNs: default config.kubernetes.io/prefixes: baked- + config.kubernetes.io/previousNames: apple + config.kubernetes.io/previousNamespaces: default config.kubernetes.io/suffixes: -pie name: baked-apple-pie spec: @@ -86,9 +86,9 @@ apiVersion: v1 kind: ConfigMap metadata: annotations: - config.kubernetes.io/originalName: cm - config.kubernetes.io/originalNs: default config.kubernetes.io/prefixes: baked- + config.kubernetes.io/previousNames: cm + config.kubernetes.io/previousNamespaces: default config.kubernetes.io/suffixes: -pie name: baked-cm-pie `) @@ -137,9 +137,9 @@ apiVersion: apps/v1 kind: Deployment metadata: annotations: - config.kubernetes.io/originalName: deployment - config.kubernetes.io/originalNs: default config.kubernetes.io/prefixes: test- + config.kubernetes.io/previousNames: deployment + config.kubernetes.io/previousNamespaces: default name: test-deployment spec: template: