From bd4580d73ab007b5efeb94460317e255a4cce44a Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Mon, 11 Jan 2021 13:06:21 -0800 Subject: [PATCH] Manage name changes (prefix/suffix) via YAML annotations rather than via in-memory-only fields. --- api/builtins/HashTransformer.go | 1 + api/builtins/NamespaceTransformer.go | 1 + api/builtins/PatchTransformer.go | 1 + api/builtins/PrefixSuffixTransformer.go | 4 + .../namereferencetransformer_test.go | 4 + .../accumulator/resaccumulator_test.go | 3 +- .../plugins/execplugin/execplugin_test.go | 1 + api/internal/target/kusttarget_test.go | 2 + api/krusty/kustomizer.go | 4 + api/krusty/transformerplugin_test.go | 2 +- api/resmap/factory_test.go | 1 - api/resmap/resmap.go | 2 + api/resmap/reswrangler.go | 11 + api/resmap/reswrangler_test.go | 3 +- api/resource/factory.go | 10 +- api/resource/resource.go | 158 +++++++-- api/resource/resource_test.go | 319 ++++++++++++++++++ api/testutils/kusttest/harness.go | 8 + api/testutils/kusttest/harnessenhanced.go | 3 +- .../hashtransformer/HashTransformer.go | 1 + .../hashtransformer/HashTransformer_test.go | 2 +- .../ImageTagTransformer_test.go | 16 +- .../labeltransformer/LabelTransformer_test.go | 2 +- .../LegacyOrderTransformer_test.go | 2 +- .../NamespaceTransformer.go | 1 + .../PatchStrategicMergeTransformer_test.go | 8 +- .../patchtransformer/PatchTransformer.go | 1 + .../patchtransformer/PatchTransformer_test.go | 2 +- .../PrefixSuffixTransformer.go | 5 +- .../PrefixSuffixTransformer_test.go | 12 + .../ReplicaCountTransformer_test.go | 4 +- .../v1/dateprefixer/DatePrefixer_test.go | 2 +- .../v1/sedtransformer/SedTransformer_test.go | 2 +- .../v1/stringprefixer/StringPrefixer_test.go | 2 +- 34 files changed, 539 insertions(+), 61 deletions(-) diff --git a/api/builtins/HashTransformer.go b/api/builtins/HashTransformer.go index 135c6e18e..bf308e51d 100644 --- a/api/builtins/HashTransformer.go +++ b/api/builtins/HashTransformer.go @@ -28,6 +28,7 @@ func (p *HashTransformerPlugin) Transform(m resmap.ResMap) error { if err != nil { return err } + res.SetOriginalName(res.GetName(), false) res.SetName(fmt.Sprintf("%s-%s", res.GetName(), h)) } } diff --git a/api/builtins/NamespaceTransformer.go b/api/builtins/NamespaceTransformer.go index 9d996d118..b2a6896e1 100644 --- a/api/builtins/NamespaceTransformer.go +++ b/api/builtins/NamespaceTransformer.go @@ -34,6 +34,7 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } + r.SetOriginalNs(r.GetNamespace(), false) err := r.ApplyFilter(namespace.Filter{ Namespace: p.Namespace, FsSlice: p.FieldSpecs, diff --git a/api/builtins/PatchTransformer.go b/api/builtins/PatchTransformer.go index a485c44c6..b0c8d4b04 100644 --- a/api/builtins/PatchTransformer.go +++ b/api/builtins/PatchTransformer.go @@ -104,6 +104,7 @@ func (p *PatchTransformerPlugin) transformJson6902(m resmap.ResMap, patch jsonpa return err } for _, res := range resources { + res.SetOriginalName(res.GetName(), false) err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.Patch, }) diff --git a/api/builtins/PrefixSuffixTransformer.go b/api/builtins/PrefixSuffixTransformer.go index f43a03fda..858ebc14c 100644 --- a/api/builtins/PrefixSuffixTransformer.go +++ b/api/builtins/PrefixSuffixTransformer.go @@ -66,8 +66,12 @@ func (p *PrefixSuffixTransformerPlugin) Transform(m resmap.ResMap) error { // this will add a prefix and a suffix // to the resource even if those are // empty + r.AddNamePrefix(p.Prefix) r.AddNameSuffix(p.Suffix) + if p.Prefix != "" || p.Suffix != "" { + r.SetOriginalName(r.GetName(), false) + } } err := r.ApplyFilter(prefixsuffix.Filter{ Prefix: p.Prefix, diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index 90620efd9..e162f193b 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -721,6 +721,7 @@ func TestNameReferenceNamespace(t *testing.T) { t.Fatalf("unexpected error: %v", err) } + m.RemoveIdAnnotations() if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } @@ -882,6 +883,7 @@ func TestNameReferenceClusterWide(t *testing.T) { t.Fatalf("unexpected error: %v", err) } + m.RemoveIdAnnotations() if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } @@ -1008,6 +1010,7 @@ func TestNameReferenceNamespaceTransformation(t *testing.T) { t.Fatalf("unexpected error: %v", err) } + m.RemoveIdAnnotations() if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } @@ -1044,6 +1047,7 @@ func TestNameReferenceCandidateSelection(t *testing.T) { t.Fatalf("unexpected error: %v", err) } + m.RemoveIdAnnotations() if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } diff --git a/api/internal/accumulator/resaccumulator_test.go b/api/internal/accumulator/resaccumulator_test.go index 94dde492a..8ac53a331 100644 --- a/api/internal/accumulator/resaccumulator_test.go +++ b/api/internal/accumulator/resaccumulator_test.go @@ -355,7 +355,8 @@ func TestResolveVarsWithNoambiguation(t *testing.T) { // went through a prefix transformer. r := m.GetByIndex(1) r.AddNamePrefix("sub-") - r.SetName("sub-backendOne") // original name remains "backendOne" + r.SetName("sub-backendOne") + r.SetOriginalName("backendOne", true) err = ra2.AppendAll(m) if err != nil { diff --git a/api/internal/plugins/execplugin/execplugin_test.go b/api/internal/plugins/execplugin/execplugin_test.go index dd71c6edc..16822cd73 100644 --- a/api/internal/plugins/execplugin/execplugin_test.go +++ b/api/internal/plugins/execplugin/execplugin_test.go @@ -45,6 +45,7 @@ s/$BAR/bar baz/g "argsFromFile": "sed-input.txt", }) + pluginConfig.RemoveIdAnnotations() p := NewExecPlugin( pLdr.AbsolutePluginPath( konfig.DisabledPluginConfig(), diff --git a/api/internal/target/kusttarget_test.go b/api/internal/target/kusttarget_test.go index 1e011437f..fbb19ebaf 100644 --- a/api/internal/target/kusttarget_test.go +++ b/api/internal/target/kusttarget_test.go @@ -244,6 +244,7 @@ metadata: t.Fatalf("unexpected error %v", err) } } + assert.NoError(t, expected.RemoveIdAnnotations()) expYaml, err := expected.AsYaml() assert.NoError(t, err) @@ -251,6 +252,7 @@ metadata: assert.NoError(t, kt.Load()) actual, err := kt.MakeCustomizedResMap() assert.NoError(t, err) + assert.NoError(t, actual.RemoveIdAnnotations()) actYaml, err := actual.AsYaml() assert.NoError(t, err) assert.Equal(t, expYaml, actYaml) diff --git a/api/krusty/kustomizer.go b/api/krusty/kustomizer.go index b3b7ec322..1d354c68e 100644 --- a/api/krusty/kustomizer.go +++ b/api/krusty/kustomizer.go @@ -93,5 +93,9 @@ func (b *Kustomizer) Run(path string) (resmap.ResMap, error) { } t.Transform(m) } + err = m.RemoveIdAnnotations() + if err != nil { + return nil, err + } return m, nil } diff --git a/api/krusty/transformerplugin_test.go b/api/krusty/transformerplugin_test.go index 049e53915..01d4163bb 100644 --- a/api/krusty/transformerplugin_test.go +++ b/api/krusty/transformerplugin_test.go @@ -80,7 +80,7 @@ data: `) m := th.Run("/app", th.MakeOptionsPluginsEnabled()) - th.AssertActualEqualsExpected(m, ` + th.AssertActualEqualsExpectedNoIdAnnotations(m, ` apiVersion: v1 data: foo: foo diff --git a/api/resmap/factory_test.go b/api/resmap/factory_test.go index 4f1eb2827..0d0880545 100644 --- a/api/resmap/factory_test.go +++ b/api/resmap/factory_test.go @@ -389,7 +389,6 @@ spec: rm, err := rmF.ConflatePatches([]*resource.Resource{r1, r2}) assert.NoError(t, err) - yml, err = rm.AsYaml() assert.NoError(t, err) diff --git a/api/resmap/resmap.go b/api/resmap/resmap.go index 3d3559ea1..efdef0763 100644 --- a/api/resmap/resmap.go +++ b/api/resmap/resmap.go @@ -245,4 +245,6 @@ type ResMap interface { // selected set of resources. ApplySmPatch( selectedSet *resource.IdSet, patch *resource.Resource) error + + RemoveIdAnnotations() error } diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index 2b2bde6b7..2369245f2 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -589,6 +589,7 @@ func (m *resWrangler) ApplySmPatch( patchCopy.SetName(res.GetName()) patchCopy.SetNamespace(res.GetNamespace()) 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 @@ -619,3 +620,13 @@ func (m *resWrangler) ApplySmPatch( m.AppendAll(newRm) return nil } + +func (m *resWrangler) RemoveIdAnnotations() error { + for _, r := range m.Resources() { + err := r.RemoveIdAnnotations() + if err != nil { + return err + } + } + return nil +} diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index bc8a20c96..8668a683f 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -745,7 +745,6 @@ rules: if err != nil { t.Fatalf("unexpected error: %v", err) } - rnodes, err := rm.ToRNodeSlice() if err != nil { t.Fatalf("unexpected error: %v", err) @@ -999,6 +998,7 @@ spec: return } assert.False(t, tc.errorExpected) + assert.NoError(t, m.RemoveIdAnnotations()) yml, err := m.AsYaml() assert.NoError(t, err) assert.Equal(t, strings.Join(tc.expected, "---\n"), string(yml)) @@ -1111,6 +1111,7 @@ $patch: delete assert.NoError(t, err, name) assert.NoError(t, m.ApplySmPatch(idSet, p), name) assert.Equal(t, tc.finalMapSize, m.Size(), name) + assert.NoError(t, m.RemoveIdAnnotations()) yml, err := m.AsYaml() assert.NoError(t, err, name) assert.Equal(t, tc.expected, string(yml), name) diff --git a/api/resource/factory.go b/api/resource/factory.go index 7b551356e..8e60f3e44 100644 --- a/api/resource/factory.go +++ b/api/resource/factory.go @@ -35,17 +35,17 @@ 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) + 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) + return rf.makeOne(rf.kf.FromMap(m), nil).SetOriginalNs(n, true) } // 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).setOriginalName(n) + return rf.makeOne(rf.kf.FromMap(m), nil).SetOriginalNs(ns, true).SetOriginalName(n, true) } // FromMapAndOption returns a new instance of Resource with given options. @@ -72,7 +72,7 @@ func (rf *Factory) makeOne( kunStr: u, options: o, } - return r.setOriginalName(r.kunStr.GetName()).setOriginalNs(r.GetNamespace()) + return r } // SliceFromPatches returns a slice of resources given a patch path @@ -157,7 +157,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.originalName = names[i] + res.SetOriginalName(names[i], true) } return result, nil } diff --git a/api/resource/resource.go b/api/resource/resource.go index f9a85d128..e569496e2 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -23,16 +23,19 @@ import ( // paired with metadata used by kustomize. // For more history, see sigs.k8s.io/kustomize/api/ifc.Unstructured type Resource struct { - kunStr ifc.Kunstructured - originalName string - originalNs string - options *types.GenArgs - refBy []resid.ResId - refVarNames []string - namePrefixes []string - nameSuffixes []string + kunStr ifc.Kunstructured + options *types.GenArgs + refBy []resid.ResId + refVarNames []string } +const ( + nameAnnotation = "config.kubernetes.io/originalName" + prefixAnnotation = "config.kubernetes.io/prefixes" + suffixAnnotation = "config.kubernetes.io/suffixes" + namespaceAnnotation = "config.kubernetes.io/originalNs" +) + func (r *Resource) ResetPrimaryData(incoming *Resource) { r.kunStr = incoming.Copy() } @@ -172,13 +175,9 @@ func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) { } func (r *Resource) copyOtherFields(other *Resource) { - r.originalName = other.originalName - r.originalNs = other.originalNs r.options = other.options r.refBy = other.copyRefBy() r.refVarNames = copyStringSlice(other.refVarNames) - r.namePrefixes = copyStringSlice(other.namePrefixes) - r.nameSuffixes = copyStringSlice(other.nameSuffixes) } func (r *Resource) MergeDataMapFrom(o *Resource) { @@ -245,28 +244,48 @@ func copyStringSlice(s []string) []string { // Implements ResCtx AddNamePrefix func (r *Resource) AddNamePrefix(p string) { - r.namePrefixes = append(r.namePrefixes, p) + annotations := r.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + if _, ok := annotations[prefixAnnotation]; !ok { + annotations[prefixAnnotation] = p + } else { + annotations[prefixAnnotation] = annotations[prefixAnnotation] + "," + p + } + r.SetAnnotations(annotations) } // Implements ResCtx AddNameSuffix func (r *Resource) AddNameSuffix(s string) { - r.nameSuffixes = append(r.nameSuffixes, s) + annotations := r.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + if _, ok := annotations[suffixAnnotation]; !ok { + annotations[suffixAnnotation] = s + } else { + annotations[suffixAnnotation] = annotations[suffixAnnotation] + "," + s + } + r.SetAnnotations(annotations) } // Implements ResCtx GetOutermostNamePrefix func (r *Resource) GetOutermostNamePrefix() string { - if len(r.namePrefixes) == 0 { + namePrefixes := r.GetNamePrefixes() + if len(namePrefixes) == 0 { return "" } - return r.namePrefixes[len(r.namePrefixes)-1] + return namePrefixes[len(namePrefixes)-1] } // Implements ResCtx GetOutermostNameSuffix func (r *Resource) GetOutermostNameSuffix() string { - if len(r.nameSuffixes) == 0 { + nameSuffixes := r.GetNameSuffixes() + if len(nameSuffixes) == 0 { return "" } - return r.nameSuffixes[len(r.nameSuffixes)-1] + return nameSuffixes[len(nameSuffixes)-1] } func sameEndingSubarray(a, b []string) bool { @@ -291,12 +310,20 @@ func sameEndingSubarray(a, b []string) bool { // Implements ResCtx GetNamePrefixes func (r *Resource) GetNamePrefixes() []string { - return r.namePrefixes + annotations := r.GetAnnotations() + if _, ok := annotations[prefixAnnotation]; !ok { + return nil + } + return strings.Split(annotations[prefixAnnotation], ",") } // Implements ResCtx GetNameSuffixes func (r *Resource) GetNameSuffixes() []string { - return r.nameSuffixes + annotations := r.GetAnnotations() + if _, ok := annotations[suffixAnnotation]; !ok { + return nil + } + return strings.Split(annotations[suffixAnnotation], ",") } // OutermostPrefixSuffixEquals returns true if both resources @@ -320,23 +347,96 @@ func (r *Resource) PrefixesSuffixesEquals(o ResCtx) bool { return sameEndingSubarray(r.GetNamePrefixes(), o.GetNamePrefixes()) && sameEndingSubarray(r.GetNameSuffixes(), o.GetNameSuffixes()) } -func (r *Resource) GetOriginalName() string { - return r.originalName +func (r *Resource) RemoveIdAnnotations() error { + annotations := r.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + delete(annotations, nameAnnotation) + delete(annotations, prefixAnnotation) + delete(annotations, suffixAnnotation) + delete(annotations, namespaceAnnotation) + if len(annotations) == 0 { + json, err := r.MarshalJSON() + if err != nil { + return err + } + + yml, err := yaml.JSONToYAML(json) + if err != nil { + return err + } + + rnode, err := kyaml.Parse(string(yml)) + if err != nil { + return err + } + + err = rnode.SetAnnotations(nil) + if err != nil { + return err + } + + b, err := rnode.MarshalJSON() + if err != nil { + return err + } + + err = r.UnmarshalJSON(b) + if err != nil { + return err + } + return nil + } + + r.SetAnnotations(annotations) + return nil } -// Making this public would be bad. -func (r *Resource) setOriginalName(n string) *Resource { - r.originalName = n +func (r *Resource) GetOriginalName() string { + annotations := r.GetAnnotations() + if name, ok := annotations[nameAnnotation]; 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[nameAnnotation]; !ok || overwrite { + annotations[nameAnnotation] = n + } + r.kunStr.SetAnnotations(annotations) return r } func (r *Resource) GetOriginalNs() string { - return r.originalNs + annotations := r.GetAnnotations() + if ns, ok := annotations[namespaceAnnotation]; ok { + return ns + } + ns := r.GetNamespace() + if ns == "default" { + return "" + } + return ns } -// Making this public would be bad. -func (r *Resource) setOriginalNs(n string) *Resource { - r.originalNs = n +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[namespaceAnnotation]; !ok || overwrite { + annotations[namespaceAnnotation] = n + } + r.SetAnnotations(annotations) return r } diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index 28c426211..ccaa13a20 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -695,6 +695,325 @@ spec: } } +func TestSetOriginalNameAndNs(t *testing.T) { + 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 +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) + } + assert.Equal(t, expected, string(bytes)) +} + +func TestGetOriginalName(t *testing.T) { + tests := []struct { + input string + expected string + }{ + { + // no name annotation, return the name + input: `apiVersion: apps/v1 +kind: Secret +metadata: + name: mySecret`, + expected: "mySecret", + }, + + { + // return name from name annotation + 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 + name: newName +`, + }, + + { + // 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 + 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 + name: newName + namespace: newNamespace +`, + }, + } + + 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)) + } +} + // baseResource produces a base object which used to test // patch transformation // Also the structure is matching the Deployment syntax diff --git a/api/testutils/kusttest/harness.go b/api/testutils/kusttest/harness.go index f1d8706ac..7146899af 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -126,6 +126,14 @@ func (th Harness) AssertActualEqualsExpected( th.AssertActualEqualsExpectedWithTweak(m, nil, expected) } +func (th Harness) AssertActualEqualsExpectedNoIdAnnotations(m resmap.ResMap, expected string) { + err := m.RemoveIdAnnotations() + if err != nil { + th.t.Fatalf("Err: %v", err) + } + th.AssertActualEqualsExpectedWithTweak(m, nil, expected) +} + func (th Harness) AssertActualEqualsExpectedWithTweak( m resmap.ResMap, tweaker func([]byte) []byte, expected string) { assertActualEqualsExpectedWithTweak(th, m, tweaker, expected) diff --git a/api/testutils/kusttest/harnessenhanced.go b/api/testutils/kusttest/harnessenhanced.go index baa22edb6..8216e5a83 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -109,6 +109,7 @@ func (th *HarnessEnhanced) LoadAndRunGenerator( if err != nil { th.t.Fatalf("Err: %v", err) } + rm.RemoveIdAnnotations() return rm } @@ -124,7 +125,7 @@ func (th *HarnessEnhanced) LoadAndRunTransformer( func (th *HarnessEnhanced) RunTransformerAndCheckResult( config, input, expected string) { resMap := th.LoadAndRunTransformer(config, input) - th.AssertActualEqualsExpected(resMap, expected) + th.AssertActualEqualsExpectedNoIdAnnotations(resMap, expected) } func (th *HarnessEnhanced) ErrorFromLoadAndRunTransformer( diff --git a/plugin/builtin/hashtransformer/HashTransformer.go b/plugin/builtin/hashtransformer/HashTransformer.go index eb7b6618b..c1a7dccdd 100644 --- a/plugin/builtin/hashtransformer/HashTransformer.go +++ b/plugin/builtin/hashtransformer/HashTransformer.go @@ -32,6 +32,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { if err != nil { return err } + res.SetOriginalName(res.GetName(), false) res.SetName(fmt.Sprintf("%s-%s", res.GetName(), h)) } } diff --git a/plugin/builtin/hashtransformer/HashTransformer_test.go b/plugin/builtin/hashtransformer/HashTransformer_test.go index 607200b11..3d62541ac 100644 --- a/plugin/builtin/hashtransformer/HashTransformer_test.go +++ b/plugin/builtin/hashtransformer/HashTransformer_test.go @@ -53,7 +53,7 @@ spec: image: nginx:1.7.9 `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 kind: ConfigMap metadata: diff --git a/plugin/builtin/imagetagtransformer/ImageTagTransformer_test.go b/plugin/builtin/imagetagtransformer/ImageTagTransformer_test.go index 1b7361b04..d7fb18679 100644 --- a/plugin/builtin/imagetagtransformer/ImageTagTransformer_test.go +++ b/plugin/builtin/imagetagtransformer/ImageTagTransformer_test.go @@ -52,7 +52,7 @@ spec: name: init-alpine `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment @@ -122,7 +122,7 @@ spec: name: init-alpine `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment @@ -194,7 +194,7 @@ spec: name: init-alpine `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment @@ -265,7 +265,7 @@ spec: name: init-alpine `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment @@ -337,7 +337,7 @@ spec: name: init-alpine `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment @@ -395,7 +395,7 @@ spec: containers: initContainers: `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment @@ -438,7 +438,7 @@ spec: name: my-image `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment @@ -480,7 +480,7 @@ spec: name: my-image `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 group: apps kind: Deployment diff --git a/plugin/builtin/labeltransformer/LabelTransformer_test.go b/plugin/builtin/labeltransformer/LabelTransformer_test.go index 355a2da13..f2e52f743 100644 --- a/plugin/builtin/labeltransformer/LabelTransformer_test.go +++ b/plugin/builtin/labeltransformer/LabelTransformer_test.go @@ -65,7 +65,7 @@ spec: image: nginx `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 kind: Service metadata: diff --git a/plugin/builtin/legacyordertransformer/LegacyOrderTransformer_test.go b/plugin/builtin/legacyordertransformer/LegacyOrderTransformer_test.go index e1a6bfde2..f3ab7e7ee 100644 --- a/plugin/builtin/legacyordertransformer/LegacyOrderTransformer_test.go +++ b/plugin/builtin/legacyordertransformer/LegacyOrderTransformer_test.go @@ -65,7 +65,7 @@ metadata: name: apricot `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: v1 kind: Namespace metadata: diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index f4a026193..d6c7024f1 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer.go @@ -38,6 +38,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } + r.SetOriginalNs(r.GetNamespace(), false) err := r.ApplyFilter(namespace.Filter{ Namespace: p.Namespace, FsSlice: p.FieldSpecs, diff --git a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go index 8af85632b..a76dfc75c 100644 --- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go +++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go @@ -603,7 +603,7 @@ spec: B: Y `) assert.NoError(t, err) - th.AssertActualEqualsExpected( + th.AssertActualEqualsExpectedNoIdAnnotations( resMap, // In kyaml/yaml.merge2, the empty "B: " is dropped // when patch1 and patch2 are merged, so the patch @@ -652,7 +652,7 @@ spec: B: Y `) assert.NoError(t, err) - th.AssertActualEqualsExpected( + th.AssertActualEqualsExpectedNoIdAnnotations( resMap, // This time only patch2 was applied. Same answer on the kyaml // path, but different answer on apimachinery path (B becomes "true"?) @@ -1388,7 +1388,7 @@ paths: - patch.yaml `, target) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: apps/v1 kind: Deployment metadata: @@ -1430,7 +1430,7 @@ paths: - patch.yaml `, target) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: apps/v1 kind: Deployment metadata: diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index a973cbea2..8030ef0e6 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -108,6 +108,7 @@ func (p *plugin) transformJson6902(m resmap.ResMap, patch jsonpatch.Patch) error return err } for _, res := range resources { + res.SetOriginalName(res.GetName(), false) err = res.ApplyFilter(patchjson6902.Filter{ Patch: p.Patch, }) diff --git a/plugin/builtin/patchtransformer/PatchTransformer_test.go b/plugin/builtin/patchtransformer/PatchTransformer_test.go index 8d9bf41fc..cadd90e7d 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer_test.go +++ b/plugin/builtin/patchtransformer/PatchTransformer_test.go @@ -306,7 +306,7 @@ path: patch.yaml target: name: myDeploy `, someDeploymentResources) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: apps/v1 kind: Deployment metadata: diff --git a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go index c09684047..9c755c1b3 100644 --- a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go +++ b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go @@ -6,7 +6,6 @@ package main import ( "errors" - "sigs.k8s.io/kustomize/api/filters/prefixsuffix" "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" @@ -70,8 +69,12 @@ func (p *plugin) Transform(m resmap.ResMap) error { // this will add a prefix and a suffix // to the resource even if those are // empty + r.AddNamePrefix(p.Prefix) r.AddNameSuffix(p.Suffix) + if p.Prefix != "" || p.Suffix != "" { + r.SetOriginalName(r.GetName(), false) + } } err := r.ApplyFilter(prefixsuffix.Filter{ Prefix: p.Prefix, diff --git a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go index e30cf4ea6..f6bc09860 100644 --- a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go +++ b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer_test.go @@ -62,6 +62,10 @@ metadata: apiVersion: v1 kind: Service metadata: + annotations: + config.kubernetes.io/originalName: apple + config.kubernetes.io/prefixes: baked- + config.kubernetes.io/suffixes: -pie name: baked-apple-pie spec: ports: @@ -80,6 +84,10 @@ metadata: apiVersion: v1 kind: ConfigMap metadata: + annotations: + config.kubernetes.io/originalName: cm + config.kubernetes.io/prefixes: baked- + config.kubernetes.io/suffixes: -pie name: baked-cm-pie `) @@ -126,6 +134,10 @@ metadata: apiVersion: apps/v1 kind: Deployment metadata: + annotations: + config.kubernetes.io/originalName: deployment + config.kubernetes.io/prefixes: test- + config.kubernetes.io/suffixes: null name: test-deployment spec: template: diff --git a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go index 4105e65c0..0a7e0a291 100644 --- a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go +++ b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer_test.go @@ -91,7 +91,7 @@ spec: app: app `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: apps/v1 kind: Service metadata: @@ -180,7 +180,7 @@ fieldSpecs: - path: spec/replicas create: true kind: Deployment`, rm) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: apps/v1 kind: Deployment metadata: diff --git a/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer_test.go b/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer_test.go index 612f4c163..abdd695cc 100644 --- a/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer_test.go +++ b/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer_test.go @@ -26,7 +26,7 @@ metadata: name: meatball `) - th.AssertActualEqualsExpected(m, ` + th.AssertActualEqualsExpectedNoIdAnnotations(m, ` apiVersion: apps/v1 kind: MeatBall metadata: diff --git a/plugin/someteam.example.com/v1/sedtransformer/SedTransformer_test.go b/plugin/someteam.example.com/v1/sedtransformer/SedTransformer_test.go index 35301275a..bac1b5bf7 100644 --- a/plugin/someteam.example.com/v1/sedtransformer/SedTransformer_test.go +++ b/plugin/someteam.example.com/v1/sedtransformer/SedTransformer_test.go @@ -36,7 +36,7 @@ fruit: $FRUIT vegetable: $VEGGIE `) - th.AssertActualEqualsExpected(rm, ` + th.AssertActualEqualsExpectedNoIdAnnotations(rm, ` apiVersion: apps/v1 beans: two two two two fruit: orange diff --git a/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer_test.go b/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer_test.go index 9931093fa..ff2e12048 100644 --- a/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer_test.go +++ b/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer_test.go @@ -27,7 +27,7 @@ metadata: name: meatball `) - th.AssertActualEqualsExpected(m, ` + th.AssertActualEqualsExpectedNoIdAnnotations(m, ` apiVersion: apps/v1 kind: MeatBall metadata: