From 3a01a63a01b92d1422ed11f15aff371e54bfbdc0 Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Wed, 12 Jun 2019 11:29:57 -0700 Subject: [PATCH] Simplify code base. - In ResMap, drop concept of internal Id to Resource map. The ResMap is now (just) a list, allowing only very particular edits. - Resources should now be maintained in the order loaded. A later PR can adjust tests to remove the internal legacy sorting, and confirm order-out is predictable from order-in. The PR would suppress the sort in tests, and reorder the output to make all tests pass again, and confirm that the new order matched depth-first input traversal. The FromMap fixture function was removed from all test inputs to establish a predictable input order. - Resources now have two 'Ids', OriginalId and CurrentId. The former is fixed as GVK-name-namespace at load time, the latter changes during transformations. The latter can be used to narrow name references when the former maps to multiple resources. We allow bases to be loaded more than once in a build (a diamond pattern), so the OriginalId is not unique across the resources set. The CurrentId is (and must be) unique, but is constantly mutating. Failing to make this distinction clear, and attempting to maintain a mapping from a single mutating Id to a resource was making the code too complex. - Drop prefix/suffix from ResId - the ResId is now immutable. A later PR can remove the distinction with ItemId. - This PR increases coverage of ResMap is since this is a large refactor. Higher level tests didn't need much change outside reordering of results at the resource level. --- k8sdeps/kunstruct/hasher_test.go | 3 - k8sdeps/transformer/patch/conflictdetector.go | 4 +- k8sdeps/transformer/patch/transformer.go | 69 +- k8sdeps/transformer/patch/transformer_test.go | 384 ++++--- pkg/accumulator/resaccumulator.go | 4 +- pkg/accumulator/resaccumulator_test.go | 220 ++-- pkg/gvk/gvk.go | 26 +- pkg/inventory/inventory.go | 28 +- pkg/inventory/inventory_test.go | 10 +- pkg/kusttest/kusttestharness.go | 4 + pkg/patch/transformer/factory_test.go | 150 ++- pkg/patch/transformer/patchjson6902json.go | 10 +- .../transformer/patchjson6902json_test.go | 160 ++- pkg/plugins/execplugin.go | 10 +- pkg/plugins/execplugin_test.go | 2 +- pkg/plugins/loader.go | 22 +- pkg/resid/itemid.go | 87 -- pkg/resid/itemid_test.go | 66 -- pkg/resid/resid.go | 199 +--- pkg/resid/resid_test.go | 362 +++---- pkg/resmap/factory.go | 24 - pkg/resmap/factory_test.go | 2 +- pkg/resmap/idslice.go | 4 +- pkg/resmap/resmap.go | 470 ++++----- pkg/resmap/resmap_test.go | 688 ++++++------- pkg/resmaptest/rmbuilder.go | 10 +- pkg/resource/resource.go | 103 +- pkg/resource/resource_test.go | 4 +- pkg/target/baseandoverlaysmall_test.go | 36 +- pkg/target/chartinflatorplugin_test.go | 6 +- pkg/target/customconfig_test.go | 18 +- pkg/target/generatormergeandreplace_test.go | 20 +- pkg/target/kusttarget_test.go | 112 +- pkg/target/multiplepatch_test.go | 18 +- pkg/target/resourceconflict_test.go | 132 ++- pkg/target/variableref_test.go | 16 +- pkg/transformers/image.go | 4 +- pkg/transformers/labelsandannotations.go | 2 +- pkg/transformers/labelsandannotations_test.go | 956 +++++++++--------- pkg/transformers/multitransformer.go | 1 + pkg/transformers/namereference.go | 74 +- pkg/transformers/namereference_test.go | 42 +- pkg/transformers/namespace.go | 161 ++- pkg/transformers/namespace_test.go | 354 +++---- pkg/transformers/refvars.go | 2 +- pkg/transformers/refvars_test.go | 76 +- plugin/builtin/AnnotationsTransformer.go | 1 + plugin/builtin/ConfigMapGenerator.go | 1 + plugin/builtin/HashTransformer.go | 1 + plugin/builtin/ImageTagTransformer.go | 1 + plugin/builtin/InventoryTransformer.go | 33 +- plugin/builtin/LabelTransformer.go | 1 + plugin/builtin/LegacyOrderTransformer.go | 13 +- plugin/builtin/NamespaceTransformer.go | 1 + plugin/builtin/PatchJson6902Transformer.go | 1 + plugin/builtin/PrefixSuffixTransformer.go | 76 +- plugin/builtin/ReplicaCountTransformer.go | 7 +- plugin/builtin/SecretGenerator.go | 1 + .../AnnotationsTransformer.go | 1 + .../configmapgenerator/ConfigMapGenerator.go | 1 + .../hashtransformer/HashTransformer.go | 1 + .../ImageTagTransformer.go | 1 + .../InventoryTransformer.go | 33 +- .../labeltransformer/LabelTransformer.go | 1 + .../LegacyOrderTransformer.go | 13 +- .../NamespaceTransformer.go | 1 + .../PatchJson6902Transformer.go | 1 + .../PrefixSuffixTransformer.go | 76 +- .../ReplicaCountTransformer.go | 7 +- .../secretgenerator/SecretGenerator.go | 1 + .../v1/chartinflator/ChartInflator_test.go | 6 +- .../v1/dateprefixer/DatePrefixer.go | 2 + .../SecretsFromDatabase.go | 2 + .../SomeServiceGenerator.go | 2 + .../v1/stringprefixer/StringPrefixer.go | 2 + 75 files changed, 2481 insertions(+), 2962 deletions(-) delete mode 100644 pkg/resid/itemid.go delete mode 100644 pkg/resid/itemid_test.go diff --git a/k8sdeps/kunstruct/hasher_test.go b/k8sdeps/kunstruct/hasher_test.go index aea27c399..1626701e7 100644 --- a/k8sdeps/kunstruct/hasher_test.go +++ b/k8sdeps/kunstruct/hasher_test.go @@ -9,11 +9,8 @@ import ( "testing" "k8s.io/api/core/v1" - "sigs.k8s.io/kustomize/pkg/gvk" ) -var secret = gvk.Gvk{Version: "v1", Kind: "Secret"} - func TestConfigMapHash(t *testing.T) { cases := []struct { desc string diff --git a/k8sdeps/transformer/patch/conflictdetector.go b/k8sdeps/transformer/patch/conflictdetector.go index 3d8851505..3c7b11198 100644 --- a/k8sdeps/transformer/patch/conflictdetector.go +++ b/k8sdeps/transformer/patch/conflictdetector.go @@ -40,7 +40,7 @@ func (jmp *jsonMergePatch) findConflict( if i == conflictingPatchIdx { continue } - if !patches[conflictingPatchIdx].Id().GvknEquals(patch.Id()) { + if !patches[conflictingPatchIdx].OrgId().GvknEquals(patch.OrgId()) { continue } conflict, err := mergepatch.HasConflicts( @@ -100,7 +100,7 @@ func (smp *strategicMergePatch) findConflict( if i == conflictingPatchIdx { continue } - if !patches[conflictingPatchIdx].Id().GvknEquals(patch.Id()) { + if !patches[conflictingPatchIdx].OrgId().GvknEquals(patch.OrgId()) { continue } conflict, err := strategicpatch.MergingMapsHaveConflicts( diff --git a/k8sdeps/transformer/patch/transformer.go b/k8sdeps/transformer/patch/transformer.go index 23aa3ade0..a554fe68d 100644 --- a/k8sdeps/transformer/patch/transformer.go +++ b/k8sdeps/transformer/patch/transformer.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/kustomize/pkg/gvk" + "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/transformers" @@ -36,33 +37,22 @@ func NewTransformer( } // Transform apply the patches on top of the base resources. -func (tf *transformer) Transform(baseResourceMap resmap.ResMap) error { - // Merge and then index the patches by Id. +// nolint:ineffassign +func (tf *transformer) Transform(m resmap.ResMap) error { patches, err := tf.mergePatches() if err != nil { return err } - - // Strategic merge the resources exist in both base and patches. for _, patch := range patches.Resources() { - // Merge patches with base resource. - id := patch.Id() - matchedIds := baseResourceMap.GetMatchingIds(id.GvknEquals) - if len(matchedIds) == 0 { - return fmt.Errorf("failed to find an object with %s to apply the patch", id.GvknString()) - } - if len(matchedIds) > 1 { - return fmt.Errorf("found multiple objects %#v targeted by patch %#v (ambiguous)", matchedIds, id) - } - id = matchedIds[0] - base := baseResourceMap.GetById(id) + target, err := tf.findPatchTarget(m, patch.OrgId()) merged := map[string]interface{}{} - versionedObj, err := scheme.Scheme.New(toSchemaGvk(id.Gvk())) - baseName := base.GetName() + versionedObj, err := scheme.Scheme.New( + toSchemaGvk(patch.OrgId().Gvk)) + saveName := target.GetName() switch { case runtime.IsNotRegisteredError(err): // Use JSON merge patch to handle types w/o schema - baseBytes, err := json.Marshal(base.Map()) + baseBytes, err := json.Marshal(target.Map()) if err != nil { return err } @@ -83,39 +73,56 @@ func (tf *transformer) Transform(baseResourceMap resmap.ResMap) error { default: // Use Strategic-Merge-Patch to handle types w/ schema // TODO: Change this to use the new Merge package. - // Store the name of the base object, because this name may have been munged. + // Store the name of the target object, because this name may have been munged. // Apply this name to the patched object. lookupPatchMeta, err := strategicpatch.NewPatchMetaFromStruct(versionedObj) if err != nil { return err } merged, err = strategicpatch.StrategicMergeMapPatchUsingLookupPatchMeta( - base.Map(), + target.Map(), patch.Map(), lookupPatchMeta) if err != nil { return err } } - baseResourceMap.GetById(id).SetMap(merged) - base.SetName(baseName) + target.SetMap(merged) + target.SetName(saveName) } return nil } -// mergePatches merge and index patches by Id. +func (tf *transformer) findPatchTarget( + m resmap.ResMap, id resid.ResId) (*resource.Resource, error) { + match, err := m.GetByOriginalId(id) + if err == nil { + return match, nil + } + match, err = m.GetByCurrentId(id) + if err == nil { + return match, nil + } + return nil, fmt.Errorf( + "failed to find target for patch %s", id.GvknString()) +} + +// mergePatches merge and index patches by OrgId. // It errors out if there is conflict between patches. func (tf *transformer) mergePatches() (resmap.ResMap, error) { rc := resmap.New() for ix, patch := range tf.patches { - id := patch.Id() - existing := rc.GetById(id) - if existing == nil { - rc.AppendWithId(id, patch) + id := patch.OrgId() + existing := rc.GetMatchingResourcesByOriginalId(id.GvknEquals) + if len(existing) == 0 { + rc.Append(patch) continue } + if len(existing) > 1 { + return nil, fmt.Errorf("self conflict in patches") + } - versionedObj, err := scheme.Scheme.New(toSchemaGvk(id.Gvk())) + versionedObj, err := scheme.Scheme.New(toSchemaGvk(id.Gvk)) if err != nil && !runtime.IsNotRegisteredError(err) { return nil, err } @@ -129,7 +136,7 @@ func (tf *transformer) mergePatches() (resmap.ResMap, error) { } } - conflict, err := cd.hasConflict(existing, patch) + conflict, err := cd.hasConflict(existing[0], patch) if err != nil { return nil, err } @@ -142,11 +149,11 @@ func (tf *transformer) mergePatches() (resmap.ResMap, error) { "conflict between %#v and %#v", conflictingPatch.Map(), patch.Map()) } - merged, err := cd.mergePatches(existing, patch) + merged, err := cd.mergePatches(existing[0], patch) if err != nil { return nil, err } - rc.ReplaceResource(id, merged) + rc.Replace(merged) } return rc, nil } diff --git a/k8sdeps/transformer/patch/transformer_test.go b/k8sdeps/transformer/patch/transformer_test.go index 61a4ac048..a21e2b054 100644 --- a/k8sdeps/transformer/patch/transformer_test.go +++ b/k8sdeps/transformer/patch/transformer_test.go @@ -9,45 +9,39 @@ import ( "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/pkg/gvk" - "sigs.k8s.io/kustomize/pkg/resid" - "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/resource" ) var rf = resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) -var deploy = gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"} -var foo = gvk.Gvk{Group: "example.com", Version: "v1", Kind: "Foo"} func TestOverlayRun(t *testing.T) { - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, + base := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx", - }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx", }, }, }, }, - }), - }) + }, + }).ResMap() patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", @@ -78,43 +72,40 @@ func TestOverlayRun(t *testing.T) { }, }, }, - }, - ), + }), } - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - "another-label": "foo", - }, + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", + "another-label": "foo", }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:latest", - "env": []interface{}{ - map[string]interface{}{ - "name": "SOMEENV", - "value": "BAR", - }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:latest", + "env": []interface{}{ + map[string]interface{}{ + "name": "SOMEENV", + "value": "BAR", }, }, }, }, }, }, - }), - }) + }, + }).ResMap() lt, err := NewTransformer(patch, rf) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -124,34 +115,32 @@ func TestOverlayRun(t *testing.T) { t.Fatalf("unexpected error: %v", err) } if !reflect.DeepEqual(base, expected) { - err = expected.ErrorIfNotEqualSets(base) + err = expected.ErrorIfNotEqualLists(base) t.Fatalf("actual doesn't match expected: %v", err) } } func TestMultiplePatches(t *testing.T) { - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx", - }, + base := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx", }, }, }, }, - }), - }) + }, + }).ResMap() patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", @@ -177,8 +166,7 @@ func TestMultiplePatches(t *testing.T) { }, }, }, - }, - ), + }), rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -206,45 +194,42 @@ func TestMultiplePatches(t *testing.T) { }, }, }, - }, - ), + }), } - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:latest", - "env": []interface{}{ - map[string]interface{}{ - "name": "ANOTHERENV", - "value": "HELLO", - }, - map[string]interface{}{ - "name": "SOMEENV", - "value": "BAR", - }, + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:latest", + "env": []interface{}{ + map[string]interface{}{ + "name": "ANOTHERENV", + "value": "HELLO", + }, + map[string]interface{}{ + "name": "SOMEENV", + "value": "BAR", }, }, - map[string]interface{}{ - "name": "busybox", - "image": "busybox", - }, + }, + map[string]interface{}{ + "name": "busybox", + "image": "busybox", }, }, }, }, - }), - }) + }, + }).ResMap() lt, err := NewTransformer(patch, rf) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -254,34 +239,33 @@ func TestMultiplePatches(t *testing.T) { t.Fatalf("unexpected error: %v", err) } if !reflect.DeepEqual(base, expected) { - err = expected.ErrorIfNotEqualSets(base) + err = expected.ErrorIfNotEqualLists(base) t.Fatalf("actual doesn't match expected: %v", err) } } func TestMultiplePatchesWithConflict(t *testing.T) { - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx", - }, + base := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx", }, }, }, }, - }), - }) + }, + }).ResMap() + patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", @@ -307,8 +291,7 @@ func TestMultiplePatchesWithConflict(t *testing.T) { }, }, }, - }, - ), + }), rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -327,8 +310,7 @@ func TestMultiplePatchesWithConflict(t *testing.T) { }, }, }, - }, - ), + }), } lt, err := NewTransformer(patch, rf) @@ -345,22 +327,20 @@ func TestMultiplePatchesWithConflict(t *testing.T) { } func TestNoSchemaOverlayRun(t *testing.T) { - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(foo, "my-foo"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "example.com/v1", - "kind": "Foo", - "metadata": map[string]interface{}{ - "name": "my-foo", + base := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "Foo", + "metadata": map[string]interface{}{ + "name": "my-foo", + }, + "spec": map[string]interface{}{ + "bar": map[string]interface{}{ + "A": "X", + "B": "Y", }, - "spec": map[string]interface{}{ - "bar": map[string]interface{}{ - "A": "X", - "B": "Y", - }, - }, - }), - }) + }, + }).ResMap() patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "example.com/v1", @@ -374,11 +354,10 @@ func TestNoSchemaOverlayRun(t *testing.T) { "C": "Z", }, }, - }, - ), + }), } - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(foo, "my-foo"): rf.FromMap( + expected := resmaptest_test.NewRmBuilder(t, rf). + Add( map[string]interface{}{ "apiVersion": "example.com/v1", "kind": "Foo", @@ -391,8 +370,7 @@ func TestNoSchemaOverlayRun(t *testing.T) { "C": "Z", }, }, - }), - }) + }).ResMap() lt, err := NewTransformer(patch, rf) if err != nil { @@ -402,28 +380,26 @@ func TestNoSchemaOverlayRun(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(base); err != nil { + if err = expected.ErrorIfNotEqualLists(base); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } func TestNoSchemaMultiplePatches(t *testing.T) { - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(foo, "my-foo"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "example.com/v1", - "kind": "Foo", - "metadata": map[string]interface{}{ - "name": "my-foo", + base := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "Foo", + "metadata": map[string]interface{}{ + "name": "my-foo", + }, + "spec": map[string]interface{}{ + "bar": map[string]interface{}{ + "A": "X", + "B": "Y", }, - "spec": map[string]interface{}{ - "bar": map[string]interface{}{ - "A": "X", - "B": "Y", - }, - }, - }), - }) + }, + }).ResMap() patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "example.com/v1", @@ -437,8 +413,7 @@ func TestNoSchemaMultiplePatches(t *testing.T) { "C": "Z", }, }, - }, - ), + }), rf.FromMap(map[string]interface{}{ "apiVersion": "example.com/v1", "kind": "Foo", @@ -454,29 +429,26 @@ func TestNoSchemaMultiplePatches(t *testing.T) { "hello": "world", }, }, - }, - ), + }), } - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(foo, "my-foo"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "example.com/v1", - "kind": "Foo", - "metadata": map[string]interface{}{ - "name": "my-foo", + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "Foo", + "metadata": map[string]interface{}{ + "name": "my-foo", + }, + "spec": map[string]interface{}{ + "bar": map[string]interface{}{ + "A": "X", + "C": "Z", + "D": "W", }, - "spec": map[string]interface{}{ - "bar": map[string]interface{}{ - "A": "X", - "C": "Z", - "D": "W", - }, - "baz": map[string]interface{}{ - "hello": "world", - }, + "baz": map[string]interface{}{ + "hello": "world", }, - }), - }) + }, + }).ResMap() lt, err := NewTransformer(patch, rf) if err != nil { @@ -486,28 +458,26 @@ func TestNoSchemaMultiplePatches(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(base); err != nil { + if err = expected.ErrorIfNotEqualLists(base); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } func TestNoSchemaMultiplePatchesWithConflict(t *testing.T) { - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(foo, "my-foo"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "example.com/v1", - "kind": "Foo", - "metadata": map[string]interface{}{ - "name": "my-foo", + base := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "Foo", + "metadata": map[string]interface{}{ + "name": "my-foo", + }, + "spec": map[string]interface{}{ + "bar": map[string]interface{}{ + "A": "X", + "B": "Y", }, - "spec": map[string]interface{}{ - "bar": map[string]interface{}{ - "A": "X", - "B": "Y", - }, - }, - }), - }) + }, + }).ResMap() patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "example.com/v1", diff --git a/pkg/accumulator/resaccumulator.go b/pkg/accumulator/resaccumulator.go index 547aebae7..66e4f0874 100644 --- a/pkg/accumulator/resaccumulator.go +++ b/pkg/accumulator/resaccumulator.go @@ -64,7 +64,7 @@ func (ra *ResAccumulator) GetTransformerConfig() *config.TransformerConfig { func (ra *ResAccumulator) MergeVars(incoming []types.Var) error { for _, v := range incoming { - matched := ra.resMap.GetMatchingIds( + matched := ra.resMap.GetMatchingResourcesByOriginalId( resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name).GvknEquals) if len(matched) > 1 { return fmt.Errorf( @@ -73,7 +73,7 @@ func (ra *ResAccumulator) MergeVars(incoming []types.Var) error { len(matched), v) } if len(matched) == 1 { - ra.resMap.GetById(matched[0]).AppendRefVarName(v) + matched[0].AppendRefVarName(v) } } return ra.varSet.MergeSlice(incoming) diff --git a/pkg/accumulator/resaccumulator_test.go b/pkg/accumulator/resaccumulator_test.go index 91856fbc0..4cf5cd3a6 100644 --- a/pkg/accumulator/resaccumulator_test.go +++ b/pkg/accumulator/resaccumulator_test.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package accumulator_test @@ -26,78 +13,65 @@ import ( "sigs.k8s.io/kustomize/k8sdeps/kunstruct" . "sigs.k8s.io/kustomize/pkg/accumulator" "sigs.k8s.io/kustomize/pkg/gvk" - "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/transformers/config" "sigs.k8s.io/kustomize/pkg/types" ) -func makeResAccumulator() (*ResAccumulator, *resource.Factory, error) { +func makeResAccumulator(t *testing.T) (*ResAccumulator, *resource.Factory) { ra := MakeEmptyAccumulator() err := ra.MergeConfig(config.MakeDefaultConfig()) if err != nil { - return nil, nil, err + t.Fatalf("unexpected err: %v", err) } rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) err = ra.AppendAll( - resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId( - gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, - "deploy1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "command": []interface{}{ - "myserver", - "--somebackendService $(SERVICE_ONE)", - "--yetAnother $(SERVICE_TWO)", - }, + resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "command": []interface{}{ + "myserver", + "--somebackendService $(SERVICE_ONE)", + "--yetAnother $(SERVICE_TWO)", }, }, }, }, }, - }), - resid.NewResId( - gvk.Gvk{Version: "v1", Kind: "Service"}, - "backendOne"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "backendOne", - }, - }), - resid.NewResId( - gvk.Gvk{Version: "v1", Kind: "Service"}, - "backendTwo"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "backendTwo", - }, - }), - })) - return ra, rf, err -} - -func TestResolveVarsHappy(t *testing.T) { - ra, _, err := makeResAccumulator() + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "backendOne", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "backendTwo", + }}).ResMap()) if err != nil { t.Fatalf("unexpected err: %v", err) } - err = ra.MergeVars([]types.Var{ + return ra, rf +} + +func TestResolveVarsHappy(t *testing.T) { + ra, _ := makeResAccumulator(t) + err := ra.MergeVars([]types.Var{ { Name: "SERVICE_ONE", ObjRef: types.Target{ @@ -125,11 +99,8 @@ func TestResolveVarsHappy(t *testing.T) { } func TestResolveVarsOneUnused(t *testing.T) { - ra, _, err := makeResAccumulator() - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - err = ra.MergeVars([]types.Var{ + ra, _ := makeResAccumulator(t) + err := ra.MergeVars([]types.Var{ { Name: "SERVICE_ONE", ObjRef: types.Target{ @@ -169,13 +140,10 @@ func expectLog(t *testing.T, log bytes.Buffer, expect string) { } func TestResolveVarsVarNeedsDisambiguation(t *testing.T) { - ra, rf, err := makeResAccumulator() - if err != nil { - t.Fatalf("unexpected err: %v", err) - } + ra, rf := makeResAccumulator(t) rm0 := resmap.New() - rm0.Append( + err := rm0.Append( rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -185,6 +153,9 @@ func TestResolveVarsVarNeedsDisambiguation(t *testing.T) { "namespace": "fooNamespace", }, })) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } err = ra.AppendAll(rm0) if err != nil { t.Fatalf("unexpected err: %v", err) @@ -209,11 +180,8 @@ func TestResolveVarsVarNeedsDisambiguation(t *testing.T) { } func TestResolveVarsGoodResIdBadField(t *testing.T) { - ra, _, err := makeResAccumulator() - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - err = ra.MergeVars([]types.Var{ + ra, _ := makeResAccumulator(t) + err := ra.MergeVars([]types.Var{ { Name: "SERVICE_ONE", ObjRef: types.Target{ @@ -237,11 +205,8 @@ func TestResolveVarsGoodResIdBadField(t *testing.T) { } func TestResolveVarsUnmappableVar(t *testing.T) { - ra, _, err := makeResAccumulator() - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - err = ra.MergeVars([]types.Var{ + ra, _ := makeResAccumulator(t) + err := ra.MergeVars([]types.Var{ { Name: "SERVICE_THREE", ObjRef: types.Target{ @@ -263,13 +228,9 @@ func TestResolveVarsUnmappableVar(t *testing.T) { } } -func TestResolveVarsWithNoambiguiation(t *testing.T) { - ra, rf, err := makeResAccumulator() - if err != nil { - t.Fatalf("unexpected err: %v", err) - } - - err = ra.MergeVars([]types.Var{ +func TestResolveVarsWithNoambiguation(t *testing.T) { + ra1, rf := makeResAccumulator(t) + err := ra1.MergeVars([]types.Var{ { Name: "SERVICE_ONE", ObjRef: types.Target{ @@ -283,45 +244,48 @@ func TestResolveVarsWithNoambiguiation(t *testing.T) { } // Create another accumulator having a resource with different prefix - ra1 := MakeEmptyAccumulator() - rm1 := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, - "deploy2"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy2", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "command": []interface{}{ - "myserver", - "--somebackendService $(SUB_SERVICE_ONE)", - }, + ra2 := MakeEmptyAccumulator() + + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy2", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "command": []interface{}{ + "myserver", + "--somebackendService $(SUB_SERVICE_ONE)", }, }, }, }, }, - }), - resid.NewResIdWithPrefixNamespace( - gvk.Gvk{Version: "v1", Kind: "Service"}, - "backendOne", "sub-", ""): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "backendOne", - }, - }), - }) - ra1.AppendAll(rm1) + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "backendOne", + }}).ResMap() - err = ra1.MergeVars([]types.Var{ + // Make it seem like this resource + // went through a prefix transformer. + r := m.GetByIndex(1) + r.AddNamePrefix("sub-") + r.SetName("sub-backendOne") // original name remains "backendOne" + + err = ra2.AppendAll(m) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + + err = ra2.MergeVars([]types.Var{ { Name: "SUB_SERVICE_ONE", ObjRef: types.Target{ @@ -333,12 +297,12 @@ func TestResolveVarsWithNoambiguiation(t *testing.T) { if err != nil { t.Fatalf("unexpected err: %v", err) } - err = ra.MergeAccumulator(ra1) + err = ra1.MergeAccumulator(ra2) if err != nil { t.Fatalf("unexpected err: %v", err) } - err = ra.ResolveVars() + err = ra1.ResolveVars() if err != nil { t.Fatalf("unexpected err: %v", err) } diff --git a/pkg/gvk/gvk.go b/pkg/gvk/gvk.go index 9e82bb75d..553e8d0a5 100644 --- a/pkg/gvk/gvk.go +++ b/pkg/gvk/gvk.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package gvk @@ -193,12 +180,3 @@ func (x Gvk) IsClusterKind() bool { } return false } - -// ClusterLevelGvks returns a slice of cluster-level Gvks -func ClusterLevelGvks() []Gvk { - var result []Gvk - for _, k := range clusterLevelKinds { - result = append(result, Gvk{Kind: k}) - } - return result -} diff --git a/pkg/inventory/inventory.go b/pkg/inventory/inventory.go index 29d10c6aa..ee1fedf9f 100644 --- a/pkg/inventory/inventory.go +++ b/pkg/inventory/inventory.go @@ -22,7 +22,7 @@ import ( //References are important in inventory management //because one may not delete an object before all //objects referencing it have been removed. -type Refs map[resid.ItemId][]resid.ItemId +type Refs map[resid.ResId][]resid.ResId func NewRefs() Refs { return Refs{} @@ -44,7 +44,7 @@ func (rf Refs) Merge(b Refs) Refs { // removeIfContains removes the reference relationship // a --> b // from the Refs if it exists -func (rf Refs) RemoveIfContains(a, b resid.ItemId) { +func (rf Refs) RemoveIfContains(a, b resid.ResId) { refs, ok := rf[a] if !ok { return @@ -98,15 +98,15 @@ func (a *Inventory) UpdateCurrent(curref Refs) *Inventory { return a } -func (a *Inventory) removeNewlyOrphanedItemsFromPrevious() []resid.ItemId { - var results []resid.ItemId +func (a *Inventory) removeNewlyOrphanedItemsFromPrevious() []resid.ResId { + var results []resid.ResId for item, refs := range a.Previous { if _, ok := a.Current[item]; ok { delete(a.Previous, item) continue } - var newRefs []resid.ItemId + var newRefs []resid.ResId toDelete := true for _, ref := range refs { if _, ok := a.Current[ref]; ok { @@ -124,8 +124,8 @@ func (a *Inventory) removeNewlyOrphanedItemsFromPrevious() []resid.ItemId { return results } -func (a *Inventory) removeOrphanedItemsFromPreviousThatAreNotInCurrent() []resid.ItemId { - var results []resid.ItemId +func (a *Inventory) removeOrphanedItemsFromPreviousThatAreNotInCurrent() []resid.ResId { + var results []resid.ResId for item, refs := range a.Previous { if _, ok := a.Current[item]; ok { continue @@ -159,7 +159,7 @@ func (a *Inventory) removeOrphanedItemsFromPreviousThatAreInCurrent() { // and returns a list of Items that can be pruned. // An item that can be pruned shows up only in Previous refs. // Prune also updates the Previous refs with those items removed -func (a *Inventory) Prune() []resid.ItemId { +func (a *Inventory) Prune() []resid.ResId { a.removeOrphanedItemsFromPreviousThatAreInCurrent() // These are candidates for deletion from the cluster. @@ -170,13 +170,13 @@ func (a *Inventory) Prune() []resid.ItemId { // inventory is the internal type used for serialization type inventory struct { - Current map[string][]resid.ItemId `json:"current,omitempty"` - Previous map[string][]resid.ItemId `json:"previous,omitempty"` + Current map[string][]resid.ResId `json:"current,omitempty"` + Previous map[string][]resid.ResId `json:"previous,omitempty"` } func (a *Inventory) toInternalType() inventory { - prev := map[string][]resid.ItemId{} - curr := map[string][]resid.ItemId{} + prev := map[string][]resid.ResId{} + curr := map[string][]resid.ResId{} for id, refs := range a.Current { curr[id.String()] = refs } @@ -204,8 +204,8 @@ func (a *Inventory) marshal() ([]byte, error) { func (a *Inventory) unMarshal(data []byte) error { inv := &inventory{ - Current: map[string][]resid.ItemId{}, - Previous: map[string][]resid.ItemId{}, + Current: map[string][]resid.ResId{}, + Previous: map[string][]resid.ResId{}, } err := json.Unmarshal(data, inv) if err != nil { diff --git a/pkg/inventory/inventory_test.go b/pkg/inventory/inventory_test.go index 22d7c9c51..9fcaf28a9 100644 --- a/pkg/inventory/inventory_test.go +++ b/pkg/inventory/inventory_test.go @@ -27,12 +27,12 @@ func makeRefs() (Refs, Refs) { b := resid.FromString("G2_V2_K2|ns2|nm2") c := resid.FromString("G3_V3_K3|ns3|nm3") current := NewRefs() - current[a] = []resid.ItemId{b, c} - current[b] = []resid.ItemId{} - current[c] = []resid.ItemId{} + current[a] = []resid.ResId{b, c} + current[b] = []resid.ResId{} + current[c] = []resid.ResId{} newRefs := NewRefs() - newRefs[a] = []resid.ItemId{b} - newRefs[b] = []resid.ItemId{} + newRefs[a] = []resid.ResId{b} + newRefs[b] = []resid.ResId{} return current, newRefs } diff --git a/pkg/kusttest/kusttestharness.go b/pkg/kusttest/kusttestharness.go index 8a990482c..a52b9553b 100644 --- a/pkg/kusttest/kusttestharness.go +++ b/pkg/kusttest/kusttestharness.go @@ -82,6 +82,10 @@ kind: Kustomization `+content) } +func (th *KustTestHarness) RF() *resource.Factory { + return th.rf.RF() +} + func (th *KustTestHarness) FromMap(m map[string]interface{}) *resource.Resource { return th.rf.RF().FromMap(m) } diff --git a/pkg/patch/transformer/factory_test.go b/pkg/patch/transformer/factory_test.go index 09173a855..19eeec01e 100644 --- a/pkg/patch/transformer/factory_test.go +++ b/pkg/patch/transformer/factory_test.go @@ -24,9 +24,7 @@ import ( "gopkg.in/yaml.v2" "sigs.k8s.io/kustomize/internal/loadertest" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/pkg/gvk" - "sigs.k8s.io/kustomize/pkg/resid" - "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/types" ) @@ -184,65 +182,60 @@ func TestNewPatchJson6902FactoryMulti(t *testing.T) { t.Fatal("the returned transformer should not be nil") } - id := resid.NewResId(gvk.FromKind("foo"), "some-name") - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - id: rf.FromMap( - map[string]interface{}{ - "kind": "foo", - "metadata": map[string]interface{}{ - "name": "some-name", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", + base := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "kind": "foo", + "metadata": map[string]interface{}{ + "name": "some-name", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "image": "nginx", + "name": "nginx", }, }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "nginx", + }, + }, + }, + }).ResMap() + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "kind": "foo", + "metadata": map[string]interface{}{ + "name": "some-name", + }, + "spec": map[string]interface{}{ + "replica": "3", + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "image": "nginx", + "name": "my-nginx", + "command": []interface{}{ + "arg1", + "arg2", + "arg3", }, }, }, }, }, - }), - }) - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - id: rf.FromMap( - map[string]interface{}{ - "kind": "foo", - "metadata": map[string]interface{}{ - "name": "some-name", - }, - "spec": map[string]interface{}{ - "replica": "3", - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "my-nginx", - "command": []interface{}{ - "arg1", - "arg2", - "arg3", - }, - }, - }, - }, - }, - }, - }), - }) + }, + }).ResMap() err = tr.Transform(base) if err != nil { t.Fatalf("unexpected error : %v", err) @@ -275,12 +268,12 @@ func TestNewPatchJson6902FactoryMultiConflict(t *testing.T) { jsonPatches := []byte(` - target: kind: foo - name: some-name + name: somename path: patch.json - target: kind: foo - name: some-name + name: somename path: patch.yaml `) var p []types.PatchJson6902 @@ -298,35 +291,32 @@ func TestNewPatchJson6902FactoryMultiConflict(t *testing.T) { t.Fatal("the returned transformer should not be nil") } - id := resid.NewResId(gvk.FromKind("foo"), "some-name") - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - id: rf.FromMap( - map[string]interface{}{ - "kind": "foo", - "metadata": map[string]interface{}{ - "name": "somename", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "kind": "foo", + "metadata": map[string]interface{}{ + "name": "somename", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "nginx", - }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "image": "nginx", + "name": "nginx", }, }, }, }, - }), - }) + }, + }).ResMap() - err = tr.Transform(base) + err = tr.Transform(m) if err == nil { t.Fatal("expected conflict") } diff --git a/pkg/patch/transformer/patchjson6902json.go b/pkg/patch/transformer/patchjson6902json.go index b35c5ad2b..5ecd36791 100644 --- a/pkg/patch/transformer/patchjson6902json.go +++ b/pkg/patch/transformer/patchjson6902json.go @@ -87,7 +87,7 @@ func (t *patchJson6902JSONTransformer) Transform(m resmap.ResMap) error { func (t *patchJson6902JSONTransformer) findTargetObj( m resmap.ResMap) (*resource.Resource, error) { - var matched []resid.ResId + var matched []*resource.Resource // TODO(monopole): namespace bug in json patch? // Since introduction in PR #300 // (see pkg/patch/transformer/util.go), @@ -95,10 +95,10 @@ func (t *patchJson6902JSONTransformer) findTargetObj( // rather than like an additional restriction to match // only the empty namespace. No test coverage to confirm. // Not sure if desired, keeping it for now. - if t.target.Namespace() != "" { - matched = m.GetMatchingIds(t.target.NsGvknEquals) + if t.target.Namespace != "" { + matched = m.GetMatchingResourcesByOriginalId(t.target.Equals) } else { - matched = m.GetMatchingIds(t.target.GvknEquals) + matched = m.GetMatchingResourcesByOriginalId(t.target.GvknEquals) } if len(matched) == 0 { return nil, fmt.Errorf( @@ -109,5 +109,5 @@ func (t *patchJson6902JSONTransformer) findTargetObj( "found multiple targets %v matching %v for json patch", matched, t.target) } - return m.GetById(matched[0]), nil + return matched[0], nil } diff --git a/pkg/patch/transformer/patchjson6902json_test.go b/pkg/patch/transformer/patchjson6902json_test.go index 647ac178f..3c2cb8e40 100644 --- a/pkg/patch/transformer/patchjson6902json_test.go +++ b/pkg/patch/transformer/patchjson6902json_test.go @@ -24,7 +24,7 @@ import ( "sigs.k8s.io/kustomize/k8sdeps/kunstruct" "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/resid" - "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/resource" ) @@ -33,34 +33,31 @@ var deploy = gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"} func TestJsonPatchJSONTransformer_Transform(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - id := resid.NewResId(deploy, "deploy1") - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - id: rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx", - }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx", }, }, }, }, - }), - }) + }, + }).ResMap() operations := []byte(`[ {"op": "replace", "path": "/spec/template/spec/containers/0/name", "value": "my-nginx"}, @@ -68,49 +65,50 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { {"op": "add", "path": "/spec/template/spec/containers/0/command", "value": ["arg1", "arg2", "arg3"]} ]`) - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - id: rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "replica": "3", - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "replica": "3", + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "image": "nginx", - "name": "my-nginx", - "command": []interface{}{ - "arg1", - "arg2", - "arg3", - }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "image": "nginx", + "name": "my-nginx", + "command": []interface{}{ + "arg1", + "arg2", + "arg3", }, }, }, }, }, - }), - }) - jpt, err := newPatchJson6902JSONTransformer(id, operations) + }, + }).ResMap() + + patchId := m.GetByIndex(0).OrgId() + + jpt, err := newPatchJson6902JSONTransformer(patchId, operations) if err != nil { t.Fatalf("unexpected error : %v", err) } - err = jpt.Transform(base) + err = jpt.Transform(m) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(base, expected) { - err = expected.ErrorIfNotEqualSets(base) + if !reflect.DeepEqual(m, expected) { + err = expected.ErrorIfNotEqualSets(m) t.Fatalf("actual doesn't match expected: %v", err) } } @@ -118,48 +116,48 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { func TestJsonPatchJSONTransformer_UnHappyTransform(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - id := resid.NewResId(deploy, "deploy1") - base := resmap.FromMap(map[resid.ResId]*resource.Resource{ - id: rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx", - }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx", }, }, }, }, - }), - }) + }, + }).ResMap() operations := []byte(`[ {"op": "add", "path": "/spec/template/spec/containers/0/command/", "value": ["arg1", "arg2", "arg3"]} ]`) - jpt, err := newPatchJson6902JSONTransformer(id, operations) + jpt, err := newPatchJson6902JSONTransformer( + m.GetByIndex(0).OrgId(), operations) if err != nil { t.Fatalf("unexpected error : %v", err) } - err = jpt.Transform(base) + err = jpt.Transform(m) if err == nil { t.Fatalf("expected error didn't happen") } - if !strings.HasPrefix(err.Error(), "failed to apply json patch") || !strings.Contains(err.Error(), string(operations)) { + if !strings.HasPrefix( + err.Error(), "failed to apply json patch") || + !strings.Contains(err.Error(), string(operations)) { t.Fatalf("expected error didn't happen, but got %v", err) } } diff --git a/pkg/plugins/execplugin.go b/pkg/plugins/execplugin.go index f5c341be9..5bf4187b9 100644 --- a/pkg/plugins/execplugin.go +++ b/pkg/plugins/execplugin.go @@ -192,8 +192,8 @@ func (p *ExecPlugin) getEnv() []string { // Returns a new copy of the given ResMap with the ResIds annotated in each Resource func (p *ExecPlugin) getResMapWithIdAnnotation(rm resmap.ResMap) (resmap.ResMap, error) { inputRM := rm.DeepCopy() - for id, r := range inputRM.AsMap() { - idString, err := yaml.Marshal(id) + for _, r := range inputRM.Resources() { + idString, err := yaml.Marshal(r.CurId()) if err != nil { return nil, err } @@ -230,9 +230,9 @@ func (p *ExecPlugin) updateResMapValues(output []byte, rm resmap.ResMap) error { if err != nil { return err } - res := rm.GetById(id) - if res == nil { - return fmt.Errorf("unable to find id %s in resource map", id.String()) + res, err := rm.GetByCurrentId(id) + if err != nil { + return fmt.Errorf("unable to find unique match to %s", id.String()) } // remove the annotation set by Kustomize to track the resource delete(annotations, idAnnotation) diff --git a/pkg/plugins/execplugin_test.go b/pkg/plugins/execplugin_test.go index ac04cd5ac..2c1093120 100644 --- a/pkg/plugins/execplugin_test.go +++ b/pkg/plugins/execplugin_test.go @@ -52,7 +52,7 @@ s/$BAR/bar/g p := NewExecPlugin( AbsolutePluginPath( DefaultPluginConfig(), - pluginConfig.Id())) + pluginConfig.OrgId())) yaml, err := pluginConfig.AsYAML() if err != nil { diff --git a/pkg/plugins/loader.go b/pkg/plugins/loader.go index 89a02e247..5f84614f1 100644 --- a/pkg/plugins/loader.go +++ b/pkg/plugins/loader.go @@ -53,7 +53,7 @@ func (l *Loader) LoadGenerator( } g, ok := c.(transformers.Generator) if !ok { - return nil, fmt.Errorf("plugin %s not a generator", res.Id()) + return nil, fmt.Errorf("plugin %s not a generator", res.OrgId()) } return g, nil } @@ -79,21 +79,21 @@ func (l *Loader) LoadTransformer( } t, ok := c.(transformers.Transformer) if !ok { - return nil, fmt.Errorf("plugin %s not a transformer", res.Id()) + return nil, fmt.Errorf("plugin %s not a transformer", res.OrgId()) } return t, nil } func relativePluginPath(id resid.ResId) string { return filepath.Join( - id.Gvk().Group, - id.Gvk().Version, - strings.ToLower(id.Gvk().Kind)) + id.Group, + id.Version, + strings.ToLower(id.Kind)) } func AbsolutePluginPath(pc *types.PluginConfig, id resid.ResId) string { return filepath.Join( - pc.DirectoryPath, relativePluginPath(id), id.Gvk().Kind) + pc.DirectoryPath, relativePluginPath(id), id.Kind) } func (l *Loader) absolutePluginPath(id resid.ResId) string { @@ -104,25 +104,25 @@ func (l *Loader) absolutePluginPath(id resid.ResId) string { func (l *Loader) loadAndConfigurePlugin( ldr ifc.Loader, res *resource.Resource) (c Configurable, err error) { if !l.pc.Enabled { - return nil, NotEnabledErr(res.Id().Gvk().Kind) + return nil, NotEnabledErr(res.OrgId().Kind) } if p := NewExecPlugin( - l.absolutePluginPath(res.Id())); p.isAvailable() { + l.absolutePluginPath(res.OrgId())); p.isAvailable() { c = p } else { - c, err = l.loadGoPlugin(res.Id()) + c, err = l.loadGoPlugin(res.OrgId()) if err != nil { return nil, err } } yaml, err := res.AsYAML() if err != nil { - return nil, errors.Wrapf(err, "marshalling yaml from res %s", res.Id()) + return nil, errors.Wrapf(err, "marshalling yaml from res %s", res.OrgId()) } err = c.Config(ldr, l.rf, yaml) if err != nil { return nil, errors.Wrapf( - err, "plugin %s fails configuration", res.Id()) + err, "plugin %s fails configuration", res.OrgId()) } return c, nil } diff --git a/pkg/resid/itemid.go b/pkg/resid/itemid.go deleted file mode 100644 index 4b042f565..000000000 --- a/pkg/resid/itemid.go +++ /dev/null @@ -1,87 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package resid - -import ( - "strings" - - "sigs.k8s.io/kustomize/pkg/gvk" -) - -// ItemId contains the group, version, kind, namespace -// and name of a resource -type ItemId struct { - // Gvk of the resource. - gvk.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"` - - // Name of the resource before transformation. - Name string `json:"name,omitempty" yaml:"name,omitempty"` - - // Namespace the resource belongs to. - // An untransformed resource has no namespace. - // A fully transformed resource has the namespace - // from the top most overlay. - Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` -} - -// String of ItemId based on GVK, name and namespace -func (i ItemId) String() string { - ns := i.Namespace - if ns == "" { - ns = noNamespace - } - nm := i.Name - if nm == "" { - nm = noName - } - - return strings.Join( - []string{i.Gvk.String(), ns, nm}, separator) -} - -func (i ItemId) Equals(o ItemId) bool { - return i.Name == o.Name && - i.Namespace == o.Namespace && - i.Gvk.Equals(o.Gvk) -} - -func NewItemId(g gvk.Gvk, ns, nm string) ItemId { - return ItemId{ - Gvk: g, - Namespace: ns, - Name: nm, - } -} - -func FromString(s string) ItemId { - values := strings.Split(s, separator) - g := gvk.FromString(values[0]) - - ns := values[1] - if ns == noNamespace { - ns = "" - } - nm := values[2] - if nm == noName { - nm = "" - } - return ItemId{ - Gvk: g, - Namespace: ns, - Name: nm, - } -} diff --git a/pkg/resid/itemid_test.go b/pkg/resid/itemid_test.go deleted file mode 100644 index 8b3ce4a0a..000000000 --- a/pkg/resid/itemid_test.go +++ /dev/null @@ -1,66 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package resid - -import ( - "testing" - - "sigs.k8s.io/kustomize/pkg/gvk" -) - -var itemIds = []ItemId{ - { - Namespace: "ns", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - { - Namespace: "ns", - Gvk: gvk.Gvk{Version: "v", Kind: "k"}, - Name: "nm", - }, - { - Namespace: "ns", - Gvk: gvk.Gvk{Kind: "k"}, - Name: "nm", - }, - { - Namespace: "ns", - Gvk: gvk.Gvk{}, - Name: "nm", - }, - { - Gvk: gvk.Gvk{}, - Name: "nm", - }, - { - Gvk: gvk.Gvk{}, - Name: "nm", - }, - { - Gvk: gvk.Gvk{}, - }, -} - -func TestItemIds(t *testing.T) { - for _, item := range itemIds { - newItem := FromString(item.String()) - if newItem != item { - t.Fatalf("Actual: %v, Expected: '%s'", newItem, item) - } - } -} diff --git a/pkg/resid/resid.go b/pkg/resid/resid.go index 912dfa383..1f5ed0ebd 100644 --- a/pkg/resid/resid.go +++ b/pkg/resid/resid.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package resid @@ -22,177 +9,89 @@ import ( "sigs.k8s.io/kustomize/pkg/gvk" ) -// ResId is an immutable identifier of a k8s resource object. +// ResId is an identifier of a k8s resource object. type ResId struct { - ItemId - // namePrefix of the resource. - // An untransformed resource has no prefix. - // A fully transformed resource has an arbitrary - // number of prefixes concatenated together. - Prefix string `json:"prefix,omitempty"` + // Gvk of the resource. + gvk.Gvk `json:",inline,omitempty" yaml:",inline,omitempty"` - // nameSuffix of the resource. - // An untransformed resource has no suffix. - // A fully transformed resource has an arbitrary - // number of suffixes concatenated together. - Suffix string `json:"suffix,omitempty"` + // Name of the resource before transformation. + Name string `json:"name,omitempty" yaml:"name,omitempty"` + + // Namespace the resource belongs to. + // An untransformed resource has no namespace. + // A fully transformed resource has the namespace + // from the top most overlay. + Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` } -// NewResIdWithNamespace creates new resource identifier +// NewResIdWithNamespace creates new ResId // in a given namespace. func NewResIdWithNamespace(k gvk.Gvk, n, ns string) ResId { - return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}} + return ResId{Gvk: k, Name: n, Namespace: ns} } -// NewResIdWithPrefixSuffixNamespace creates new resource identifier with a prefix, suffix and a namespace -func NewResIdWithPrefixSuffixNamespace(k gvk.Gvk, n, p, s, ns string) ResId { - return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, Prefix: p, Suffix: s} -} - -// NewResIdWithPrefixNamespace creates new resource identifier with a prefix and a namespace -func NewResIdWithPrefixNamespace(k gvk.Gvk, n, p, ns string) ResId { - return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, Prefix: p} -} - -// NewResIdWithSuffixNamespace creates new resource identifier with a suffix and a namespace -func NewResIdWithSuffixNamespace(k gvk.Gvk, n, s, ns string) ResId { - return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, Suffix: s} -} - -// NewResId creates new resource identifier +// NewResId creates new ResId. func NewResId(k gvk.Gvk, n string) ResId { - return ResId{ItemId: ItemId{Gvk: k, Name: n}} + return ResId{Gvk: k, Name: n} } -// NewResIdKindOnly creates new resource identifier +// NewResIdKindOnly creates a new ResId. func NewResIdKindOnly(k string, n string) ResId { - return ResId{ItemId: ItemId{Gvk: gvk.FromKind(k), Name: n}} + return ResId{Gvk: gvk.FromKind(k), Name: n} } const ( noNamespace = "~X" - noPrefix = "~P" noName = "~N" - noSuffix = "~S" separator = "|" ) // String of ResId based on GVK, name and prefix -func (n ResId) String() string { - ns := n.ItemId.Namespace +func (id ResId) String() string { + ns := id.Namespace if ns == "" { ns = noNamespace } - p := n.Prefix - if p == "" { - p = noPrefix - } - nm := n.ItemId.Name + nm := id.Name if nm == "" { nm = noName } - s := n.Suffix - if s == "" { - s = noSuffix - } - return strings.Join( - []string{n.ItemId.Gvk.String(), ns, p, nm, s}, separator) + []string{id.Gvk.String(), ns, nm}, separator) +} + +func FromString(s string) ResId { + values := strings.Split(s, separator) + g := gvk.FromString(values[0]) + + ns := values[1] + if ns == noNamespace { + ns = "" + } + nm := values[2] + if nm == noName { + nm = "" + } + return ResId{ + Gvk: g, + Namespace: ns, + Name: nm, + } } // GvknString of ResId based on GVK and name -func (n ResId) GvknString() string { - return n.ItemId.Gvk.String() + separator + n.ItemId.Name +func (id ResId) GvknString() string { + return id.Gvk.String() + separator + id.Name } // GvknEquals returns true if the other id matches // Group/Version/Kind/name. -func (n ResId) GvknEquals(id ResId) bool { - return n.ItemId.Name == id.ItemId.Name && n.ItemId.Gvk.Equals(id.ItemId.Gvk) +func (id ResId) GvknEquals(o ResId) bool { + return id.Name == o.Name && id.Gvk.Equals(o.Gvk) } -// NsGvknEquals returns true if the other id matches +// Equals returns true if the other id matches // namespace/Group/Version/Kind/name. -func (n ResId) NsGvknEquals(id ResId) bool { - // TODO: same a n.ItemId.Equals(id.ItemId) - return n.ItemId.Namespace == id.ItemId.Namespace && n.GvknEquals(id) -} - -// Gvk returns Group/Version/Kind of the resource. -func (n ResId) Gvk() gvk.Gvk { - return n.ItemId.Gvk -} - -// Name returns resource name. -func (n ResId) Name() string { - return n.ItemId.Name -} - -// Namespace returns resource namespace. -func (n ResId) Namespace() string { - return n.ItemId.Namespace -} - -// CopyWithNewPrefixSuffix make a new copy from current ResId -// and append a new prefix and suffix -func (n ResId) CopyWithNewPrefixSuffix(p, s string) ResId { - result := n - if p != "" { - result.Prefix = n.concatPrefix(p) - } - if s != "" { - result.Suffix = n.concatSuffix(s) - } - return result -} - -// CopyWithNewNamespace make a new copy from current ResId and set a new namespace -func (n ResId) CopyWithNewNamespace(ns string) ResId { - result := n - result.ItemId.Namespace = ns - return result -} - -// HasSameLeftmostPrefix check if two ResIds have the same -// left most prefix. -func (n ResId) HasSameLeftmostPrefix(id ResId) bool { - prefixes1 := n.prefixList() - prefixes2 := id.prefixList() - return prefixes1[0] == prefixes2[0] -} - -// HasSameRightmostSuffix check if two ResIds have the same -// right most suffix. -func (n ResId) HasSameRightmostSuffix(id ResId) bool { - suffixes1 := n.suffixList() - suffixes2 := id.suffixList() - return suffixes1[len(suffixes1)-1] == suffixes2[len(suffixes2)-1] -} - -func (n ResId) concatPrefix(p string) string { - if p == "" { - return n.Prefix - } - if n.Prefix == "" { - return p - } - return p + ":" + n.Prefix -} - -func (n ResId) concatSuffix(s string) string { - if s == "" { - return n.Suffix - } - if n.Suffix == "" { - return s - } - return n.Suffix + ":" + s -} - -func (n ResId) prefixList() []string { - return strings.Split(n.Prefix, ":") -} - -func (n ResId) suffixList() []string { - return strings.Split(n.Suffix, ":") +func (id ResId) Equals(o ResId) bool { + return id.Namespace == o.Namespace && id.GvknEquals(o) } diff --git a/pkg/resid/resid_test.go b/pkg/resid/resid_test.go index abd9c481f..c91f8273f 100644 --- a/pkg/resid/resid_test.go +++ b/pkg/resid/resid_test.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package resid @@ -28,93 +15,65 @@ var stringTests = []struct { }{ { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, - "g_v_k|ns|p|nm|s", + "g_v_k|ns|nm", }, { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{Version: "v", Kind: "k"}, + Name: "nm", }, - "~G_v_k|ns|p|nm|s", + "~G_v_k|ns|nm", }, { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{Kind: "k"}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{Kind: "k"}, + Name: "nm", }, - "~G_~V_k|ns|p|nm|s", + "~G_~V_k|ns|nm", }, { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{}, + Name: "nm", }, - "~G_~V_~K|ns|p|nm|s", + "~G_~V_~K|ns|nm", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Gvk: gvk.Gvk{}, + Name: "nm", }, - "~G_~V_~K|~X|p|nm|s", + "~G_~V_~K|~X|nm", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - Name: "nm", - }, - Suffix: "s", + Gvk: gvk.Gvk{}, + Name: "nm", }, - "~G_~V_~K|~X|~P|nm|s", + "~G_~V_~K|~X|nm", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - }, - Suffix: "s", + Gvk: gvk.Gvk{}, }, - "~G_~V_~K|~X|~P|~N|s", + "~G_~V_~K|~X|~N", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - }, + Gvk: gvk.Gvk{}, }, - "~G_~V_~K|~X|~P|~N|~S", + "~G_~V_~K|~X|~N", }, { ResId{}, - "~G_~V_~K|~X|~P|~N|~S", + "~G_~V_~K|~X|~N", }, } @@ -132,87 +91,59 @@ var gvknStringTests = []struct { }{ { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, "g_v_k|nm", }, { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{Version: "v", Kind: "k"}, + Name: "nm", }, "~G_v_k|nm", }, { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{Kind: "k"}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{Kind: "k"}, + Name: "nm", }, "~G_~V_k|nm", }, { ResId{ - ItemId: ItemId{ - Namespace: "ns", - Gvk: gvk.Gvk{}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Namespace: "ns", + Gvk: gvk.Gvk{}, + Name: "nm", }, "~G_~V_~K|nm", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - Name: "nm", - }, - Prefix: "p", - Suffix: "s", + Gvk: gvk.Gvk{}, + Name: "nm", }, "~G_~V_~K|nm", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - Name: "nm", - }, - Suffix: "s", + Gvk: gvk.Gvk{}, + Name: "nm", }, "~G_~V_~K|nm", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - }, - Suffix: "s", + Gvk: gvk.Gvk{}, }, "~G_~V_~K|", }, { ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{}, - }, + Gvk: gvk.Gvk{}, }, "~G_~V_~K|", }, @@ -238,129 +169,81 @@ var GvknEqualsTest = []struct { }{ { id1: ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "AA", - Suffix: "aa", + Namespace: "X", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, id2: ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "BB", - Suffix: "bb", + Namespace: "X", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, gVknResult: true, nSgVknResult: true, }, { id1: ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "AA", - Suffix: "aa", + Namespace: "X", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, id2: ResId{ - ItemId: ItemId{ - Namespace: "Z", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "BB", - Suffix: "bb", + Namespace: "Z", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, gVknResult: true, nSgVknResult: false, }, { id1: ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "AA", - Suffix: "aa", + Namespace: "X", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, id2: ResId{ - ItemId: ItemId{ - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "BB", - Suffix: "bb", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", }, gVknResult: true, nSgVknResult: false, }, { id1: ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "AA", - Suffix: "aa", + Namespace: "X", + Gvk: gvk.Gvk{Version: "v", Kind: "k"}, + Name: "nm", }, id2: ResId{ - ItemId: ItemId{ - Namespace: "Z", - Gvk: gvk.Gvk{Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "BB", - Suffix: "bb", + Namespace: "Z", + Gvk: gvk.Gvk{Version: "v", Kind: "k"}, + Name: "nm", }, gVknResult: true, nSgVknResult: false, }, { id1: ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Kind: "k"}, - Name: "nm", - }, - Prefix: "AA", - Suffix: "aa", + Namespace: "X", + Gvk: gvk.Gvk{Kind: "k"}, + Name: "nm", }, id2: ResId{ - ItemId: ItemId{ - Namespace: "Z", - Gvk: gvk.Gvk{Kind: "k"}, - Name: "nm", - }, - Prefix: "BB", - Suffix: "bb", + Namespace: "Z", + Gvk: gvk.Gvk{Kind: "k"}, + Name: "nm", }, gVknResult: true, nSgVknResult: false, }, { id1: ResId{ - ItemId: ItemId{ - Namespace: "X", - Name: "nm", - }, - Prefix: "AA", - Suffix: "aa", + Namespace: "X", + Name: "nm", }, id2: ResId{ - ItemId: ItemId{ - Namespace: "Z", - Name: "nm", - }, - Prefix: "BB", - Suffix: "bb", + Namespace: "Z", + Name: "nm", }, gVknResult: true, nSgVknResult: false, @@ -373,59 +256,52 @@ func TestEquals(t *testing.T) { t.Fatalf("GvknEquals(\n%v,\n%v\n) should be %v", tst.id1, tst.id2, tst.gVknResult) } - if tst.id1.NsGvknEquals(tst.id2) != tst.nSgVknResult { + if tst.id1.Equals(tst.id2) != tst.nSgVknResult { t.Fatalf("NsGvknEquals(\n%v,\n%v\n) should be %v", tst.id1, tst.id2, tst.nSgVknResult) } } } -func TestCopyWithNewPrefixSuffix(t *testing.T) { - r1 := ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "a", - Suffix: "b", - } - r2 := r1.CopyWithNewPrefixSuffix("p-", "-s") - expected := ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "p-a", - Suffix: "b-s", - } - if !r2.GvknEquals(expected) { - t.Fatalf("%v should equal %v", r2, expected) - } +var ids = []ResId{ + { + Namespace: "ns", + Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, + Name: "nm", + }, + { + Namespace: "ns", + Gvk: gvk.Gvk{Version: "v", Kind: "k"}, + Name: "nm", + }, + { + Namespace: "ns", + Gvk: gvk.Gvk{Kind: "k"}, + Name: "nm", + }, + { + Namespace: "ns", + Gvk: gvk.Gvk{}, + Name: "nm", + }, + { + Gvk: gvk.Gvk{}, + Name: "nm", + }, + { + Gvk: gvk.Gvk{}, + Name: "nm", + }, + { + Gvk: gvk.Gvk{}, + }, } -func TestCopyWithNewNamespace(t *testing.T) { - r1 := ResId{ - ItemId: ItemId{ - Namespace: "X", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "a", - Suffix: "b", - } - r2 := r1.CopyWithNewNamespace("zzz") - expected := ResId{ - ItemId: ItemId{ - Namespace: "zzz", - Gvk: gvk.Gvk{Group: "g", Version: "v", Kind: "k"}, - Name: "nm", - }, - Prefix: "a", - Suffix: "b", - } - if !r2.GvknEquals(expected) { - t.Fatalf("%v should equal %v", r2, expected) +func TestFromString(t *testing.T) { + for _, id := range ids { + newId := FromString(id.String()) + if newId != id { + t.Fatalf("Actual: %v, Expected: '%s'", newId, id) + } } } diff --git a/pkg/resmap/factory.go b/pkg/resmap/factory.go index 757a3aedb..e5bcaa42a 100644 --- a/pkg/resmap/factory.go +++ b/pkg/resmap/factory.go @@ -7,7 +7,6 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/kustomize/internal/kusterr" "sigs.k8s.io/kustomize/pkg/ifc" - "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/types" ) @@ -63,29 +62,6 @@ func (rmF *Factory) NewResMapFromBytes(b []byte) (ResMap, error) { return newResMapFromResourceSlice(resources) } -// Deprecated. -// FromMap returns a ResMap with arbitrary internal ordering, -// panicing on error. For tests only. -// See also ErrorIfNotEqualSets. -func FromMap(arg map[resid.ResId]*resource.Resource) ResMap { - result, err := fromMap(arg) - if err != nil { - panic(err) - } - return result -} - -func fromMap(arg map[resid.ResId]*resource.Resource) (ResMap, error) { - result := New() - for id, r := range arg { - err := result.AppendWithId(id, r) - if err != nil { - return nil, err - } - } - return result, nil -} - // NewResMapFromConfigMapArgs returns a Resource slice given // a configmap metadata slice from kustomization file. func (rmF *Factory) NewResMapFromConfigMapArgs( diff --git a/pkg/resmap/factory_test.go b/pkg/resmap/factory_test.go index 6dd07a3e0..308464f19 100644 --- a/pkg/resmap/factory_test.go +++ b/pkg/resmap/factory_test.go @@ -69,7 +69,7 @@ metadata: if m.Size() != 3 { t.Fatalf("result should contain 3, but got %d", m.Size()) } - if err := expected.ErrorIfNotEqualSets(m); err != nil { + if err := expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } diff --git a/pkg/resmap/idslice.go b/pkg/resmap/idslice.go index cdf759203..50b834baf 100644 --- a/pkg/resmap/idslice.go +++ b/pkg/resmap/idslice.go @@ -30,8 +30,8 @@ var _ sort.Interface = IdSlice{} func (a IdSlice) Len() int { return len(a) } func (a IdSlice) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func (a IdSlice) Less(i, j int) bool { - if !a[i].Gvk().Equals(a[j].Gvk()) { - return a[i].Gvk().IsLessThan(a[j].Gvk()) + if !a[i].Gvk.Equals(a[j].Gvk) { + return a[i].Gvk.IsLessThan(a[j].Gvk) } return a[i].String() < a[j].String() } diff --git a/pkg/resmap/resmap.go b/pkg/resmap/resmap.go index 4f901174b..e293ec12d 100644 --- a/pkg/resmap/resmap.go +++ b/pkg/resmap/resmap.go @@ -8,8 +8,8 @@ package resmap import ( "bytes" "fmt" - "github.com/pkg/errors" + "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/types" @@ -17,53 +17,17 @@ import ( ) // ResMap is an interface describing operations on the -// core kustomize data structure. +// core kustomize data structure, a list of Resources. // -// TODO: delete the commentary below when/if the issues -// discussed are addressed. +// Every Resource has two ResIds: OriginalId and CurId. // -// It's a temporary(?) interface used during a refactoring -// from a bare map (map[resid.ResId]*resource.Resource) to a -// pointer to struct (currently named *resWrangler). -// Replacing it with a ptr to struct will ease click-thrus -// to implementation during development. -// OTOH, hackery in a PR might be easier to see if the -// interface is left in place. +// A ResId is a tuple of {Namespace, Group, Version, Kind, Name}. // -// The old (bare map) ResMap had pervasive problems: +// In a ResMap, no two resources may have the same CurId, +// but they may have the same OriginalId. The latter can happen +// when mixing two or more different overlays apply different +// transformations to a common base. // -// * It was mutated inside loops over itself. -// -// Bugs introduced this way were hard to find since the -// bare map was recursively passed everywhere, sometimes -// mid loop. -// -// * Its keys (ResId) aren't opaque, and are effectively -// mutated (via copy and replace) for data storage reasons -// as a hack. -// -// ResId was modified a long time ago as a hack to -// store name transformation data (prefix and suffix), -// destabilizing the basic map concept and resulting -// in the need for silly ResId functions like -// NewResIdWithPrefixSuffixNamespace, NsGvknEquals, -// HasSameLeftmostPrefix, CopyWithNewPrefixSuffix, etc. -// plus logic to use them, and overly complex tests. -// -// If this data were stored in the Resource object -// (not in Kunstructured, but as a sibling to it next to -// GenArgs, references, etc.) then much code could be -// deleted and the remainder simplified. -// -// * It doesn't preserve (by definition) value order. -// -// Preserving order is now needed to support -// transformer plugins (they aren't commutative). -// -// One way to fix this is deprecate use of ResId as the -// key in favor of ItemId. See use of the resmap.Remove -// function to spot the places that need fixing to allow -// this. type ResMap interface { // Size reports the number of resources. Size() int @@ -73,17 +37,12 @@ type ResMap interface { // as appended. Resources() []*resource.Resource - // Append adds a Resource, automatically computing its - // associated Id. - // Error on Id collision. + // Append adds a Resource. + // Error on OrgId collision. Append(*resource.Resource) error - // AppendWithId adds a Resource with the given Id. - // Error on Id collision. - AppendWithId(resid.ResId, *resource.Resource) error - // AppendAll appends another ResMap to self, - // failing on any Id collision. + // failing on any OrgId collision. AppendAll(ResMap) error // AbsorbAll appends, replaces or merges the contents @@ -99,49 +58,56 @@ type ResMap interface { // self, then its behavior _cannot_ be merge or replace. AbsorbAll(ResMap) error - // AsMap returns (ResId, *Resource) pairs in - // arbitrary order via a generated map. - // The map is discardable, and edits to map structure - // have no impact on the ResMap. - // The Ids are copies, but the resources are pointers, - // so the resources themselves can be modified. - AsMap() map[resid.ResId]*resource.Resource - // AsYaml returns the yaml form of resources. AsYaml() ([]byte, error) - // Gets the resource with the given Id, else nil. - GetById(resid.ResId) *resource.Resource + // GetByIndex returns a resource at the given index, + // nil if out of range. + GetByIndex(int) *resource.Resource - // ReplaceResource associates a new resource with - // an _existing_ Id. - // Error if Id unknown, or if some other Id points - // to the same resource object. - ReplaceResource(resid.ResId, *resource.Resource) error + // GetIndexOfCurrentId returns the index of the resource + // with the given CurId. + // Returns error if there is more than one match. + // Returns (-1, nil) if there is no match. + GetIndexOfCurrentId(id resid.ResId) (int, error) - // AllIds returns all known Ids. - // Result order is arbitrary. + // GetMatchingResourcesByCurrentId returns the resources + // 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 + + // 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 + // an exact match, returning an error on multiple or no matches. + GetByOriginalId(resid.ResId) (*resource.Resource, error) + + // Deprecated. + // Same as GetByOriginalId. + GetById(resid.ResId) (*resource.Resource, error) + + // AllIds returns all CurrentIds. AllIds() []resid.ResId - // GetMatchingIds returns a slice of Ids that - // satisfy the given matcher function. - // Result order is arbitrary. - GetMatchingIds(IdMatcher) []resid.ResId + // Replace replaces the resource with the matching CurId. + // Error if there's no match or more than one match. + // Returns the index where the replacement happened. + Replace(*resource.Resource) (int, error) - // Remove removes the Id and the resource it points to. + // Remove removes the resource whose CurId matches the argument. + // Error if not found. Remove(resid.ResId) error // Clear removes all resources and Ids. Clear() - // SubsetThatCouldBeReferencedById returns a ResMap subset - // of self with resources that could be referenced by the - // resource represented by the argument Id. - // This is a filter; it excludes things that cannot be - // referenced by the Id's resource, e.g. objects in other - // namespaces. Cluster wide objects are never excluded. - SubsetThatCouldBeReferencedById(resid.ResId) ResMap - // SubsetThatCouldBeReferencedByResource returns a ResMap subset // of self with resources that could be referenced by the // resource argument. @@ -158,8 +124,8 @@ type ResMap interface { ShallowCopy() ResMap // ErrorIfNotEqualSets returns an error if the - // argument doesn't have the same Ids and resource - // data as self. Ordering is _not_ taken into account, + // argument doesn't have the same resources as self. + // Ordering is _not_ taken into account, // as this function was solely used in tests written // before internal resource order was maintained, // and those tests are initialized with maps which @@ -174,7 +140,7 @@ type ResMap interface { // data as self, in the same order. // Meta information is ignored; this is similar // to comparing the AsYaml() strings, but allows - // for printing pointers, etc. + // for more informed errors on not equals. ErrorIfNotEqualLists(ResMap) error // Debug prints the ResMap. @@ -190,14 +156,6 @@ type resWrangler struct { // specify in kustomizations to be maintained and // available as an option for final YAML rendering. rList []*resource.Resource - - // A map from id to an index into rList. - // At the time of writing, the ids used as keys in - // this map cannot be assumed to match the id - // generated from the resource.Id() method pointed - // to by the map's value (via rList). These keys - // have been hacked to store prefix/suffix data. - rIndex map[resid.ResId]int } func newOne() *resWrangler { @@ -209,14 +167,10 @@ func newOne() *resWrangler { // Clear implements ResMap. func (m *resWrangler) Clear() { m.rList = nil - m.rIndex = make(map[resid.ResId]int) } // Size implements ResMap. func (m *resWrangler) Size() int { - if len(m.rList) != len(m.rIndex) { - panic("class size invariant violation") - } return len(m.rList) } @@ -236,94 +190,51 @@ func (m *resWrangler) Resources() []*resource.Resource { return tmp } -// GetById implements ResMap. -func (m *resWrangler) GetById(id resid.ResId) *resource.Resource { - if i, ok := m.rIndex[id]; ok { - return m.rList[i] - } - return nil -} - // Append implements ResMap. func (m *resWrangler) Append(res *resource.Resource) error { - return m.AppendWithId(res.Id(), res) + id := res.CurId() + if r := m.GetMatchingResourcesByCurrentId(id.Equals); len(r) > 0 { + return fmt.Errorf( + "may not add resource with an already registered id: %s", id) + } + m.rList = append(m.rList, res) + return nil } // Remove implements ResMap. func (m *resWrangler) Remove(adios resid.ResId) error { tmp := newOne() - for i, r := range m.rList { - id, err := m.idMappingToIndex(i) - if err != nil { - return errors.Wrap(err, "assumption failure in remove") - } - if id != adios { - tmp.AppendWithId(id, r) + for _, r := range m.rList { + if r.CurId() != adios { + tmp.Append(r) } } if tmp.Size() != m.Size()-1 { return fmt.Errorf("id %s not found in removal", adios) } - m.rIndex = tmp.rIndex m.rList = tmp.rList return nil } -// AppendWithId implements ResMap. -func (m *resWrangler) AppendWithId(id resid.ResId, res *resource.Resource) error { - if already, ok := m.rIndex[id]; ok { - return fmt.Errorf( - "attempt to add res %s at id %s; that id already maps to %d", - res, id, already) +// Replace implements ResMap. +func (m *resWrangler) Replace(res *resource.Resource) (int, error) { + id := res.CurId() + i, err := m.GetIndexOfCurrentId(id) + if err != nil { + return -1, errors.Wrap(err, "in Replace") } - i := m.indexOfResource(res) - if i >= 0 { - return fmt.Errorf( - "attempt to add res %s that is already held", - res) + if i < 0 { + return -1, fmt.Errorf("cannot find resource with id %s to replace", id) } - m.rList = append(m.rList, res) - m.rIndex[id] = len(m.rList) - 1 - return nil -} - -// ReplaceResource implements ResMap. -func (m *resWrangler) ReplaceResource( - id resid.ResId, newGuy *resource.Resource) error { - insertAt, ok := m.rIndex[id] - if !ok { - return fmt.Errorf( - "attempt to reset resource at id %s; that id not used", id) - } - existingSpot := m.indexOfResource(newGuy) - if insertAt == existingSpot { - // Be idempotent. - return nil - } - if existingSpot >= 0 { - return fmt.Errorf( - "the new resource %s is already present", newGuy.Id()) - } - m.rList[insertAt] = newGuy - return nil -} - -// AsMap implements ResMap. -func (m *resWrangler) AsMap() map[resid.ResId]*resource.Resource { - result := make(map[resid.ResId]*resource.Resource, m.Size()) - for id, i := range m.rIndex { - result[id] = m.rList[i] - } - return result + m.rList[i] = res + return i, nil } // AllIds implements ResMap. func (m *resWrangler) AllIds() (ids []resid.ResId) { ids = make([]resid.ResId, m.Size()) - i := 0 - for id := range m.rIndex { - ids[i] = id - i++ + for i, r := range m.rList { + ids[i] = r.CurId() } return } @@ -338,7 +249,7 @@ func (m *resWrangler) Debug(title string) { } else { fmt.Println("---") } - fmt.Printf("# %d %s\n", i, m.debugIdMappingToIndex(i)) + fmt.Printf("# %d %s\n", i, r.OrgId()) blob, err := yaml.Marshal(r.Map()) if err != nil { panic(err) @@ -347,45 +258,91 @@ func (m *resWrangler) Debug(title string) { } } -func (m *resWrangler) debugIdMappingToIndex(i int) string { - id, err := m.idMappingToIndex(i) - if err != nil { - return err.Error() - } - return id.String() -} - -func (m *resWrangler) idMappingToIndex(i int) (resid.ResId, error) { - var foundId resid.ResId - found := false - for id, index := range m.rIndex { - if index == i { - if found { - return foundId, fmt.Errorf("found multiple") - } - found = true - foundId = id - } - } - if !found { - return foundId, fmt.Errorf("cannot find index %d", i) - } - return foundId, nil -} - type IdMatcher func(resid.ResId) bool -// GetMatchingIds implements ResMap. -func (m *resWrangler) GetMatchingIds(matches IdMatcher) []resid.ResId { - var result []resid.ResId - for id := range m.rIndex { - if matches(id) { - result = append(result, id) +// GetByIndex implements ResMap. +func (m *resWrangler) GetByIndex(i int) *resource.Resource { + if i < 0 || i >= m.Size() { + return nil + } + return m.rList[i] +} + +// GetIndexOfCurrentId implements ResMap. +func (m *resWrangler) GetIndexOfCurrentId(id resid.ResId) (int, error) { + count := 0 + result := -1 + for i, r := range m.rList { + if id.Equals(r.CurId()) { + count++ + result = i + } + } + if count > 1 { + return -1, fmt.Errorf("id matched %d resources", count) + } + return result, nil +} + +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() } + +// GetMatchingResourcesByCurrentId implements ResMap. +func (m *resWrangler) GetMatchingResourcesByCurrentId( + matches IdMatcher) []*resource.Resource { + return m.filteredById(matches, GetCurrentId) +} + +// GetMatchingResourcesByOriginalId implements ResMap. +func (m *resWrangler) GetMatchingResourcesByOriginalId( + matches IdMatcher) []*resource.Resource { + return m.filteredById(matches, GetOriginalId) +} + +func (m *resWrangler) filteredById( + matches IdMatcher, idGetter IdFromResource) []*resource.Resource { + var result []*resource.Resource + for _, r := range m.rList { + if matches(idGetter(r)) { + result = append(result, r) } } return result } +// GetByCurrentId implements ResMap. +func (m *resWrangler) GetByCurrentId( + id resid.ResId) (*resource.Resource, error) { + 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") +} + +type resFinder func(IdMatcher) []*resource.Resource + +func demandOneMatch( + f resFinder, id resid.ResId, s string) (*resource.Resource, error) { + r := f(id.Equals) + if len(r) == 1 { + return r[0], nil + } + if len(r) > 1 { + return nil, fmt.Errorf("multiple matches for %sId %s", s, id) + } + return nil, fmt.Errorf("no matches for %sId %s", s, id) +} + +// GetById implements ResMap. +func (m *resWrangler) GetById(id resid.ResId) (*resource.Resource, error) { + return m.GetByCurrentId(id) +} + // AsYaml implements ResMap. func (m *resWrangler) AsYaml() ([]byte, error) { firstObj := true @@ -421,17 +378,28 @@ func (m *resWrangler) ErrorIfNotEqualSets(other ResMap) error { "lists have different number of entries: %#v doesn't equal %#v", m.rList, m2.rList) } - for id, i := range m.rIndex { - r1 := m.rList[i] - r2 := m2.GetById(id) - if r2 == nil { - return fmt.Errorf("id in self missing from other; id: %s", id) + seen := make(map[int]bool) + for _, r1 := range m.rList { + id := r1.CurId() + others := m2.GetMatchingResourcesByCurrentId(id.Equals) + if len(others) < 0 { + return fmt.Errorf( + "id in self missing from other; id: %s", id) } + if len(others) > 1 { + return fmt.Errorf( + "id in self matches %d in other; id: %s", len(others), id) + } + r2 := others[0] if !r1.KunstructEqual(r2) { return fmt.Errorf( - "kuns equal mismatch: \n -- %s,\n -- %s\n\n--\n%#v\n------\n%#v\n", + "kunstruct not equal: \n -- %s,\n -- %s\n\n--\n%#v\n------\n%#v\n", r1, r2, r1, r2) } + seen[m2.indexOfResource(r2)] = true + } + if len(seen) != m.Size() { + return fmt.Errorf("counting problem %d != %d", len(seen), m.Size()) } return nil } @@ -449,10 +417,10 @@ func (m *resWrangler) ErrorIfNotEqualLists(other ResMap) error { } for i, r1 := range m.rList { r2 := m2.rList[i] - if !r1.KunstructEqual(r2) { + if !r1.Equals(r2) { return fmt.Errorf( - "Item i=%d differs:\n n1 = %s\n n2 = %s\n", - i, r1.Id(), r2.Id()) + "Item i=%d differs:\n n1 = %s\n n2 = %s\n o1 = %s\n o2 = %s\n", + i, r1.OrgId(), r2.OrgId(), r1, r2) } } return nil @@ -479,34 +447,9 @@ func (m *resWrangler) DeepCopy() ResMap { // makeCopy copies the ResMap. func (m *resWrangler) makeCopy(copier resCopier) ResMap { result := &resWrangler{} - result.rIndex = make(map[resid.ResId]int, m.Size()) result.rList = make([]*resource.Resource, m.Size()) for i, r := range m.rList { result.rList[i] = copier(r) - id, err := m.idMappingToIndex(i) - if err != nil { - panic("corrupt index map") - } - result.rIndex[id] = i - } - return result -} - -// SubsetThatCouldBeReferencedById implements ResMap. -func (m *resWrangler) SubsetThatCouldBeReferencedById(inputId resid.ResId) ResMap { - if inputId.Gvk().IsClusterKind() { - return m - } - result := New() - for id, i := range m.rIndex { - if id.Gvk().IsClusterKind() || id.Namespace() == inputId.Namespace() && - id.HasSameLeftmostPrefix(inputId) && - id.HasSameRightmostSuffix(inputId) { - err := result.AppendWithId(id, m.rList[i]) - if err != nil { - panic(err) - } - } } return result } @@ -514,13 +457,13 @@ func (m *resWrangler) SubsetThatCouldBeReferencedById(inputId resid.ResId) ResMa // SubsetThatCouldBeReferencedByResource implements ResMap. func (m *resWrangler) SubsetThatCouldBeReferencedByResource( inputRes *resource.Resource) ResMap { - inputId := inputRes.Id() - if inputId.Gvk().IsClusterKind() { + inputId := inputRes.OrgId() + if inputId.IsClusterKind() { return m } result := New() for _, r := range m.Resources() { - if r.Id().Gvk().IsClusterKind() || inputRes.InSameFuzzyNamespace(r) { + if r.OrgId().IsClusterKind() || inputRes.InSameFuzzyNamespace(r) { err := result.Append(r) if err != nil { panic(err) @@ -535,17 +478,8 @@ func (m *resWrangler) AppendAll(other ResMap) error { if other == nil { return nil } - w2, ok := other.(*resWrangler) - if !ok { - panic("bad cast") - } - for i, res := range w2.Resources() { - id, err := w2.idMappingToIndex(i) - if err != nil { - panic("map is irrecoverably corrupted; " + err.Error()) - } - err = m.AppendWithId(id, res) - if err != nil { + for _, res := range other.Resources() { + if err := m.Append(res); err != nil { return err } } @@ -557,16 +491,8 @@ func (m *resWrangler) AbsorbAll(other ResMap) error { if other == nil { return nil } - w2, ok := other.(*resWrangler) - if !ok { - panic("bad cast") - } - for i, r := range w2.Resources() { - id, err := w2.idMappingToIndex(i) - if err != nil { - panic("map is irrecoverably corrupted; " + err.Error()) - } - err = m.appendReplaceOrMerge(id, r) + for _, r := range other.Resources() { + err := m.appendReplaceOrMerge(r) if err != nil { return err } @@ -575,48 +501,52 @@ func (m *resWrangler) AbsorbAll(other ResMap) error { } func (m *resWrangler) appendReplaceOrMerge( - idForRes resid.ResId, res *resource.Resource) error { - matchedId := m.GetMatchingIds(idForRes.GvknEquals) - switch len(matchedId) { + res *resource.Resource) error { + id := res.CurId() + // Maybe also try by current id if nothing matches? + matches := m.GetMatchingResourcesByOriginalId(id.GvknEquals) + switch len(matches) { case 0: switch res.Behavior() { case types.BehaviorMerge, types.BehaviorReplace: return fmt.Errorf( - "id %#v does not exist; cannot merge or replace", idForRes) + "id %#v does not exist; cannot merge or replace", id) default: // presumably types.BehaviorCreate - err := m.AppendWithId(idForRes, res) + err := m.Append(res) if err != nil { return err } } case 1: - mId := matchedId[0] - old := m.GetById(mId) + old := matches[0] if old == nil { return fmt.Errorf("id lookup failure") } + index := m.indexOfResource(old) + if index < 0 { + return fmt.Errorf("indexing problem") + } switch res.Behavior() { case types.BehaviorReplace: res.Replace(old) - err := m.ReplaceResource(mId, res) - if err != nil { - return err - } case types.BehaviorMerge: res.Merge(old) - err := m.ReplaceResource(mId, res) - if err != nil { - return err - } default: return fmt.Errorf( - "id %#v exists; must merge or replace", idForRes) + "id %#v exists; must merge or replace", id) + } + i, err := m.Replace(res) + if err != nil { + return err + } + if i != index { + return fmt.Errorf("unexpected index in replacement") } default: return fmt.Errorf( "found multiple objects %v that could accept merge of %v", - matchedId, idForRes) + matches, id) } return nil } diff --git a/pkg/resmap/resmap_test.go b/pkg/resmap/resmap_test.go index 245d80bf6..9adacc200 100644 --- a/pkg/resmap/resmap_test.go +++ b/pkg/resmap/resmap_test.go @@ -9,7 +9,6 @@ import ( "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/resid" . "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resmaptest" @@ -55,10 +54,10 @@ func TestAppendRemove(t *testing.T) { doAppend(t, w1, makeCm(5)) doAppend(t, w1, makeCm(6)) doAppend(t, w1, makeCm(7)) - doRemove(t, w1, makeCm(1).Id()) - doRemove(t, w1, makeCm(3).Id()) - doRemove(t, w1, makeCm(5).Id()) - doRemove(t, w1, makeCm(7).Id()) + doRemove(t, w1, makeCm(1).OrgId()) + doRemove(t, w1, makeCm(3).OrgId()) + doRemove(t, w1, makeCm(5).OrgId()) + doRemove(t, w1, makeCm(7).OrgId()) w2 := New() doAppend(t, w2, makeCm(2)) @@ -79,7 +78,7 @@ func TestAppendRemove(t *testing.T) { func TestRemove(t *testing.T) { w := New() r := makeCm(1) - err := w.Remove(r.Id()) + err := w.Remove(r.OrgId()) if err == nil { t.Fatalf("expected error") } @@ -87,20 +86,20 @@ func TestRemove(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - err = w.Remove(r.Id()) + err = w.Remove(r.OrgId()) if err != nil { t.Fatalf("unexpected error: %v", err) } - err = w.Remove(r.Id()) + err = w.Remove(r.OrgId()) if err == nil { t.Fatalf("expected error") } } -func TestReplaceResource(t *testing.T) { +func TestReplace(t *testing.T) { cm5 := makeCm(5) cm700 := makeCm(700) - cm888 := makeCm(888) + otherCm5 := makeCm(5) w := New() doAppend(t, w, makeCm(1)) @@ -112,40 +111,27 @@ func TestReplaceResource(t *testing.T) { doAppend(t, w, makeCm(7)) oldSize := w.Size() - err := w.ReplaceResource(cm5.Id(), cm700) + _, err := w.Replace(otherCm5) if err != nil { t.Fatalf("unexpected error: %v", err) } if w.Size() != oldSize { t.Fatalf("unexpected size %d", w.Size()) } - if w.GetById(cm5.Id()) != cm700 { - t.Fatalf("unexpected result") + if r, err := w.GetByCurrentId(cm5.OrgId()); err != nil || r != otherCm5 { + t.Fatalf("unexpected result r=%s, err=%v", r.CurId(), err) } if err := w.Append(cm5); err == nil { t.Fatalf("expected id already there error") } - if err := w.AppendWithId(cm888.Id(), cm5); err != nil { - // Okay to add with some unused Id. - t.Fatalf("unexpected error: %v", err) - } - if err := w.Append(cm700); err == nil { - t.Fatalf("expected resource already there error") - } - if err := w.Remove(cm5.Id()); err != nil { + if err := w.Remove(cm5.OrgId()); err != nil { t.Fatalf("unexpected err: %v", err) } if err := w.Append(cm700); err != nil { t.Fatalf("unexpected err: %v", err) } - if err := w.Append(cm5); err == nil { - t.Fatalf("expected err; object is still there under id 888") - } - if err := w.Remove(cm888.Id()); err != nil { - t.Fatalf("unexpected err: %v", err) - } if err := w.Append(cm5); err != nil { - t.Fatalf("unexpected err; %v", err) + t.Fatalf("unexpected err: %v", err) } } @@ -184,205 +170,249 @@ metadata: } } -func TestDemandOneGvknMatchForId(t *testing.T) { - rm1 := FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixNamespace(cmap, "cm1", "prefix1", "ns1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - }, - }), - resid.NewResIdWithPrefixNamespace(cmap, "cm2", "prefix1", "ns1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm2", - }, - }), - }) +func TestGetMatchingResourcesByCurrentId(t *testing.T) { + r1 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "alice", + }, + }) + r2 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "bob", + }, + }) + r3 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "bob", + "namespace": "happy", + }, + }) + r4 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "charlie", + "namespace": "happy", + }, + }) + r5 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "charlie", + "namespace": "happy", + }, + }) - result := rm1.GetMatchingIds( - resid.NewResIdWithPrefixNamespace(cmap, "cm2", "prefix1", "ns1").GvknEquals) + m := resmaptest_test.NewRmBuilder(t, rf). + AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).ResMap() + + result := m.GetMatchingResourcesByCurrentId( + resid.NewResId(cmap, "alice").GvknEquals) + if len(result) != 1 { + t.Fatalf("Expected single map entry but got %v", result) + } + result = m.GetMatchingResourcesByCurrentId( + resid.NewResId(cmap, "bob").GvknEquals) + if len(result) != 2 { + t.Fatalf("Expected two, got %v", result) + } + result = m.GetMatchingResourcesByCurrentId( + resid.NewResIdWithNamespace(cmap, "bob", "system").GvknEquals) + if len(result) != 2 { + t.Fatalf("Expected two but got %v", result) + } + result = m.GetMatchingResourcesByCurrentId( + resid.NewResIdWithNamespace(cmap, "bob", "happy").Equals) + if len(result) != 1 { + t.Fatalf("Expected single map entry but got %v", result) + } + result = m.GetMatchingResourcesByCurrentId( + resid.NewResId(cmap, "charlie").GvknEquals) if len(result) != 1 { t.Fatalf("Expected single map entry but got %v", result) } - // confirm that ns and prefix are not included in match - result = rm1.GetMatchingIds( - resid.NewResIdWithPrefixNamespace(cmap, "cm2", "prefix", "ns").GvknEquals) - if len(result) != 1 { - t.Fatalf("Expected single map entry but got %v", result) + // 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, + }, } - - // confirm that name is matched correctly - result = rm1.GetMatchingIds( - resid.NewResIdWithPrefixNamespace(cmap, "cm3", "prefix1", "ns1").GvknEquals) - if len(result) > 0 { - t.Fatalf("Expected no map entries but got %v", result) - } - - cmap2 := gvk.Gvk{Version: "v2", Kind: "ConfigMap"} - - // confirm that gvk is matched correctly - result = rm1.GetMatchingIds( - resid.NewResIdWithPrefixNamespace(cmap2, "cm2", "prefix1", "ns1").GvknEquals) - if len(result) > 0 { - t.Fatalf("Expected no map entries but got %v", result) + for _, tst := range tests { + result := m.GetMatchingResourcesByCurrentId(tst.matcher) + if len(result) != tst.count { + t.Fatalf("test '%s'; actual: %d, expected: %d", + tst.name, len(result), tst.count) + } } } -func TestFilterBy(t *testing.T) { +func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { + r1 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "alice", + }, + }) + r2 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "bob", + }, + }) + r3 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "bob", + "namespace": "happy", + }, + }) + r4 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "charlie", + "namespace": "happy", + }, + }) + r5 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "charlie", + "namespace": "happy", + }, + }) + r5.AddNamePrefix("little-") + r6 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "domino", + "namespace": "happy", + }, + }) + r6.AddNamePrefix("little-") + r7 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ClusterRoleBinding", + "metadata": map[string]interface{}{ + "name": "meh", + }, + }) + tests := map[string]struct { - resMap ResMap - filter resid.ResId + filter *resource.Resource expected ResMap }{ - "different namespace": { - resMap: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix", "suffix", "namespace1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }), - filter: resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix", "suffix", "namespace2"), - expected: New(), + "default namespace 1": { + filter: r2, + expected: resmaptest_test.NewRmBuilder(t, rf). + AddR(r1).AddR(r2).AddR(r7).ResMap(), }, - "different prefix": { - resMap: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix1", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }), - filter: resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix2", "suffix", "namespace"), - expected: New(), + "default namespace 2": { + filter: r1, + expected: resmaptest_test.NewRmBuilder(t, rf). + AddR(r1).AddR(r2).AddR(r7).ResMap(), }, - "different suffix": { - resMap: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix", "suffix1", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }), - filter: resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix", "suffix2", "namespace"), - expected: New(), + "happy namespace no prefix": { + filter: r3, + expected: resmaptest_test.NewRmBuilder(t, rf). + AddR(r3).AddR(r4).AddR(r7).ResMap(), }, - "same namespace, same prefix": { - resMap: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixNamespace(cmap, "config-map1", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }), - filter: resid.NewResIdWithPrefixNamespace(cmap, "config-map2", "prefix", "namespace"), - expected: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixNamespace(cmap, "config-map1", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }), + "happy namespace with prefix": { + filter: r5, + expected: resmaptest_test.NewRmBuilder(t, rf). + AddR(r5).AddR(r6).AddR(r7).ResMap(), }, - "same namespace, same suffix": { - resMap: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithSuffixNamespace(cmap, "config-map1", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }), - filter: resid.NewResIdWithSuffixNamespace(cmap, "config-map2", "suffix", "namespace"), - expected: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithSuffixNamespace(cmap, "config-map1", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }), - }, - "same namespace, same prefix, same suffix": { - resMap: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map1", "prefix", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }), - filter: resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map2", "prefix", "suffix", "namespace"), - expected: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map1", "prefix", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }), - }, - "filter by cluster-level Gvk": { - resMap: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixNamespace(cmap, "config-map", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }), - filter: resid.NewResId(gvk.Gvk{Kind: "ClusterRoleBinding"}, "cluster-role-binding"), - expected: FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithPrefixNamespace(cmap, "config-map", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }), + "cluster level": { + filter: r7, + expected: resmaptest_test.NewRmBuilder(t, rf). + AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap(), }, } - + m := resmaptest_test.NewRmBuilder(t, rf). + AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).AddR(r6).AddR(r7).ResMap() for name, test := range tests { test := test t.Run(name, func(t *testing.T) { - got := test.resMap.SubsetThatCouldBeReferencedById(test.filter) - err := test.expected.ErrorIfNotEqualSets(got) + got := m.SubsetThatCouldBeReferencedByResource(test.filter) + err := test.expected.ErrorIfNotEqualLists(got) if err != nil { - t.Fatalf("Expected %v but got back %v", test.expected, got) + test.expected.Debug("expected") + got.Debug("actual") + t.Fatalf("Expected match") } }) } @@ -410,163 +440,145 @@ func TestDeepCopy(t *testing.T) { if &rm1 == &rm2 { t.Fatal("DeepCopy returned a reference to itself instead of a copy") } - err := rm1.ErrorIfNotEqualSets(rm1) + err := rm1.ErrorIfNotEqualLists(rm1) if err != nil { t.Fatal(err) } } -func TestGetMatchingIds(t *testing.T) { - - m := FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId( - gvk.Gvk{Kind: "vegetable"}, - "bedlam"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "whatever1", - }, - }), - resid.NewResId( - gvk.Gvk{Group: "g1", Version: "v1", Kind: "vegetable"}, - "domino"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "whatever2", - }, - }), - resid.NewResIdWithPrefixNamespace( - gvk.Gvk{Kind: "vegetable"}, - "peter", "p", "happy"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "whatever3", - }, - }), - resid.NewResIdWithPrefixNamespace( - gvk.Gvk{Version: "v1", Kind: "fruit"}, - "shatterstar", "p", "happy"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "whatever4", - }, - }), - }) - - tests := []struct { - name string - matcher IdMatcher - count int - }{ - { - "match everything", - func(resid.ResId) bool { return true }, - 4, - }, - { - "match nothing", - func(resid.ResId) bool { return false }, - 0, - }, - { - "name is peter", - func(x resid.ResId) bool { return x.Name() == "peter" }, - 1, - }, - { - "happy vegetable", - func(x resid.ResId) bool { - return x.Namespace() == "happy" && - x.Gvk().Kind == "vegetable" +func TestErrorIfNotEqualSets(t *testing.T) { + r1 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", }, - 1, - }, + }) + r2 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm2", + }, + }) + r3 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm2", + "namespace": "system", + }, + }) + + m1 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).AddR(r2).AddR(r3).ResMap() + if err := m1.ErrorIfNotEqualSets(m1); err != nil { + t.Fatalf("object should equal itself %v", err) } - for _, tst := range tests { - result := m.GetMatchingIds(tst.matcher) - if len(result) != tst.count { - t.Fatalf("test '%s'; actual: %d, expected: %d", - tst.name, len(result), tst.count) - } + + m2 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).ResMap() + if err := m1.ErrorIfNotEqualSets(m2); err == nil { + t.Fatalf("%v should not equal %v %v", m1, m2, err) + } + + m3 := resmaptest_test.NewRmBuilder(t, rf).Add( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + }}).ResMap() + if err := m2.ErrorIfNotEqualSets(m3); err != nil { + t.Fatalf("%v should equal %v %v", m2, m3, err) + } + + m4 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).AddR(r2).AddR(r3).ResMap() + if err := m1.ErrorIfNotEqualSets(m4); err != nil { + t.Fatalf("expected equality between %v and %v, %v", m1, m4, err) + } + + m4 = resmaptest_test.NewRmBuilder(t, rf).AddR(r3).AddR(r1).AddR(r2).ResMap() + if err := m1.ErrorIfNotEqualSets(m4); err != nil { + t.Fatalf("expected equality between %v and %v, %v", m1, m4, err) + } + + m4 = m1.ShallowCopy() + if err := m1.ErrorIfNotEqualSets(m4); err != nil { + t.Fatalf("expected equality between %v and %v, %v", m1, m4, err) + } + m4 = m1.DeepCopy() + if err := m1.ErrorIfNotEqualSets(m4); err != nil { + t.Fatalf("expected equality between %v and %v, %v", m1, m4, err) } } -func TestErrorIfNotEqual(t *testing.T) { - rm1 := resmaptest_test.NewRmBuilder(t, rf).Add( +func TestErrorIfNotEqualLists(t *testing.T) { + r1 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", "metadata": map[string]interface{}{ "name": "cm1", }, - }).Add(map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm2", - }, - }).ResMap() + }) + r2 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm2", + }, + }) + r3 := rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm2", + "namespace": "system", + }, + }) - err := rm1.ErrorIfNotEqualSets(rm1) - if err != nil { - t.Fatalf("%v should equal itself %v", rm1, err) + m1 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).AddR(r2).AddR(r3).ResMap() + if err := m1.ErrorIfNotEqualLists(m1); err != nil { + t.Fatalf("object should equal itself %v", err) } - rm2 := resmaptest_test.NewRmBuilder(t, rf).Add( + m2 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).ResMap() + if err := m1.ErrorIfNotEqualLists(m2); err == nil { + t.Fatalf("%v should not equal %v %v", m1, m2, err) + } + + m3 := resmaptest_test.NewRmBuilder(t, rf).Add( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", "metadata": map[string]interface{}{ "name": "cm1", - }, - }).ResMap() - - // test the different number of keys path - err = rm1.ErrorIfNotEqualSets(rm2) - if err == nil { - t.Fatalf("%v should not equal %v %v", rm1, rm2, err) + }}).ResMap() + if err := m2.ErrorIfNotEqualLists(m3); err != nil { + t.Fatalf("%v should equal %v %v", m2, m3, err) } - rm3 := FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm2"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - }, - }), - }) - - // test the different key values path - err = rm2.ErrorIfNotEqualSets(rm3) - if err == nil { - t.Fatalf("%v should not equal %v %v", rm1, rm2, err) + m4 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).AddR(r2).AddR(r3).ResMap() + if err := m1.ErrorIfNotEqualLists(m4); err != nil { + t.Fatalf("expected equality between %v and %v, %v", m1, m4, err) } - rm4 := FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm3", - }, - }), - }) + m4 = resmaptest_test.NewRmBuilder(t, rf).AddR(r3).AddR(r1).AddR(r2).ResMap() + if err := m1.ErrorIfNotEqualLists(m4); err == nil { + t.Fatalf("expected inequality between %v and %v, %v", m1, m4, err) + } - // test the deepcopy path - err = rm2.ErrorIfNotEqualSets(rm4) - if err == nil { - t.Fatalf("%v should not equal %v %v", rm1, rm2, err) + m4 = m1.ShallowCopy() + if err := m1.ErrorIfNotEqualLists(m4); err != nil { + t.Fatalf("expected equality between %v and %v, %v", m1, m4, err) + } + m4 = m1.DeepCopy() + if err := m1.ErrorIfNotEqualLists(m4); err != nil { + t.Fatalf("expected equality between %v and %v, %v", m1, m4, err) } } @@ -601,7 +613,7 @@ func TestAppendAll(t *testing.T) { if err := input1.AppendAll(input2); err != nil { t.Fatalf("unexpected error: %v", err) } - if err := expected.ErrorIfNotEqualSets(input1); err != nil { + if err := expected.ErrorIfNotEqualLists(input1); err != nil { input1.Debug("1") expected.Debug("ex") t.Fatalf("%#v doesn't equal expected %#v", input1, expected) @@ -609,7 +621,7 @@ func TestAppendAll(t *testing.T) { if err := input1.AppendAll(nil); err != nil { t.Fatalf("unexpected error: %v", err) } - if err := expected.ErrorIfNotEqualSets(input1); err != nil { + if err := expected.ErrorIfNotEqualLists(input1); err != nil { t.Fatalf("%#v doesn't equal expected %#v", input1, expected) } } @@ -671,14 +683,14 @@ func TestAbsorbAll(t *testing.T) { if err := w.AbsorbAll(makeMap2(types.BehaviorMerge)); err != nil { t.Fatalf("unexpected error: %v", err) } - if err := expected.ErrorIfNotEqualSets(w); err != nil { + if err := expected.ErrorIfNotEqualLists(w); err != nil { t.Fatal(err) } w = makeMap1() if err := w.AbsorbAll(nil); err != nil { t.Fatalf("unexpected error: %v", err) } - if err := w.ErrorIfNotEqualSets(makeMap1()); err != nil { + if err := w.ErrorIfNotEqualLists(makeMap1()); err != nil { t.Fatal(err) } w = makeMap1() @@ -686,7 +698,7 @@ func TestAbsorbAll(t *testing.T) { if err := w.AbsorbAll(w2); err != nil { t.Fatalf("unexpected error: %v", err) } - if err := w2.ErrorIfNotEqualSets(w); err != nil { + if err := w2.ErrorIfNotEqualLists(w); err != nil { t.Fatal(err) } w = makeMap1() diff --git a/pkg/resmaptest/rmbuilder.go b/pkg/resmaptest/rmbuilder.go index e4303820f..6d2f55247 100644 --- a/pkg/resmaptest/rmbuilder.go +++ b/pkg/resmaptest/rmbuilder.go @@ -27,7 +27,11 @@ func NewRmBuilder(t *testing.T, rf *resource.Factory) *rmBuilder { } func (rm *rmBuilder) Add(m map[string]interface{}) *rmBuilder { - err := rm.m.Append(rm.rf.FromMap(m)) + return rm.AddR(rm.rf.FromMap(m)) +} + +func (rm *rmBuilder) AddR(r *resource.Resource) *rmBuilder { + err := rm.m.Append(r) if err != nil { rm.t.Fatalf("test setup failure: %v", err) } @@ -35,7 +39,7 @@ func (rm *rmBuilder) Add(m map[string]interface{}) *rmBuilder { } func (rm *rmBuilder) AddWithId(id resid.ResId, m map[string]interface{}) *rmBuilder { - err := rm.m.AppendWithId(id, rm.rf.FromMap(m)) + err := rm.m.Append(rm.rf.FromMap(m)) if err != nil { rm.t.Fatalf("test setup failure: %v", err) } @@ -60,7 +64,7 @@ func (rm *rmBuilder) AddWithNs(ns string, m map[string]interface{}) *rmBuilder { func (rm *rmBuilder) ReplaceResource(m map[string]interface{}) *rmBuilder { r := rm.rf.FromMap(m) - err := rm.m.ReplaceResource(r.Id(), r) + _, err := rm.m.Replace(r) if err != nil { rm.t.Fatalf("test setup failure: %v", err) } diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index a60aa0241..5f1b9b1c5 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 // Package resource implements representations of k8s API resources as "unstructured" objects. package resource @@ -36,6 +23,8 @@ type Resource struct { options *types.GenArgs refBy []resid.ResId refVarNames []string + namePrefixes []string + nameSuffixes []string } // DeepCopy returns a new copy of resource @@ -61,7 +50,33 @@ func (r *Resource) copyOtherFields(other *Resource) { r.originalNs = other.originalNs r.options = other.options r.refBy = other.copyRefBy() - r.refVarNames = other.copyRefVarNames() + r.refVarNames = copyStringSlice(other.refVarNames) + r.namePrefixes = copyStringSlice(other.namePrefixes) + r.nameSuffixes = copyStringSlice(other.nameSuffixes) +} + +func (r *Resource) Equals(o *Resource) bool { + return r.ReferencesEqual(o) && + reflect.DeepEqual(r.Kunstructured, o.Kunstructured) +} + +func (r *Resource) ReferencesEqual(o *Resource) bool { + setSelf := make(map[resid.ResId]bool) + setOther := make(map[resid.ResId]bool) + for _, ref := range o.refBy { + setOther[ref] = true + } + for _, ref := range r.refBy { + if _, ok := setOther[ref]; !ok { + return false + } + setSelf[ref] = true + } + return len(setSelf) == len(setOther) +} + +func (r *Resource) KunstructEqual(o *Resource) bool { + return reflect.DeepEqual(r.Kunstructured, o.Kunstructured) } // Merge performs merge with other resource. @@ -71,23 +86,49 @@ func (r *Resource) Merge(other *Resource) { } func (r *Resource) copyRefBy() []resid.ResId { + if r.refBy == nil { + return nil + } s := make([]resid.ResId, len(r.refBy)) copy(s, r.refBy) return s } -func (r *Resource) copyRefVarNames() []string { - s := make([]string, len(r.refVarNames)) - copy(s, r.refVarNames) - return s +func copyStringSlice(s []string) []string { + if s == nil { + return nil + } + c := make([]string, len(s)) + copy(c, s) + return c +} + +func (r *Resource) AddNamePrefix(p string) { + r.namePrefixes = append(r.namePrefixes, p) +} + +func (r *Resource) AddNameSuffix(s string) { + r.nameSuffixes = append(r.nameSuffixes, s) +} + +func (r *Resource) GetOutermostNamePrefix() string { + if len(r.namePrefixes) == 0 { + return "" + } + return r.namePrefixes[len(r.namePrefixes)-1] +} + +func (r *Resource) GetOutermostNameSuffix() string { + if len(r.nameSuffixes) == 0 { + return "" + } + return r.nameSuffixes[len(r.nameSuffixes)-1] } func (r *Resource) InSameFuzzyNamespace(o *Resource) bool { - return r.GetNamespace() == o.GetNamespace() -} - -func (r *Resource) KunstructEqual(o *Resource) bool { - return reflect.DeepEqual(r.Kunstructured, o.Kunstructured) + return r.GetNamespace() == o.GetNamespace() && + r.GetOutermostNamePrefix() == o.GetOutermostNamePrefix() && + r.GetOutermostNameSuffix() == o.GetOutermostNameSuffix() } func (r *Resource) GetOriginalName() string { @@ -146,14 +187,18 @@ func (r *Resource) GetNamespace() string { return namespace } -// Id returns the immutable ResId for the resource. -func (r *Resource) Id() resid.ResId { +// 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()) } -// FinalId returns a ResId for the resource using the mutable bits. -func (r *Resource) FinalId() resid.ResId { +// CurId returns a ResId for the resource using the +// mutable parts of the resource. +// This should be unique in any ResMap. +func (r *Resource) CurId() resid.ResId { return resid.NewResIdWithNamespace( r.GetGvk(), r.GetName(), r.GetNamespace()) } diff --git a/pkg/resource/resource_test.go b/pkg/resource/resource_test.go index 72acbd2f4..abc233e5c 100644 --- a/pkg/resource/resource_test.go +++ b/pkg/resource/resource_test.go @@ -107,8 +107,8 @@ func TestResourceId(t *testing.T) { }, } for _, test := range tests { - if test.in.Id() != test.id { - t.Fatalf("Expected %v, but got %v\n", test.id, test.in.Id()) + if test.in.OrgId() != test.id { + t.Fatalf("Expected %v, but got %v\n", test.id, test.in.OrgId()) } } } diff --git a/pkg/target/baseandoverlaysmall_test.go b/pkg/target/baseandoverlaysmall_test.go index 52ef8058e..38fc5b1ba 100644 --- a/pkg/target/baseandoverlaysmall_test.go +++ b/pkg/target/baseandoverlaysmall_test.go @@ -25,9 +25,7 @@ import ( "sigs.k8s.io/kustomize/pkg/plugins" ) -// TODO(monopole): Make prefixsuffixtransformer changes -// needed to enable this test. -func disabledTestOrderPreserved(t *testing.T) { +func TestOrderPreserved(t *testing.T) { th := kusttest_test.NewKustTestHarness(t, "/app/prod") th.WriteK("/app/base", ` namePrefix: b- @@ -86,8 +84,36 @@ metadata: t.Fatalf("Err: %v", err) } th.AssertActualEqualsExpectedNoSort(m, ` -TBD -./tr `) +apiVersion: v1 +kind: Namespace +metadata: + name: p-b-myNs +--- +apiVersion: v1 +kind: Role +metadata: + name: p-b-myRole +--- +apiVersion: v1 +kind: Service +metadata: + name: p-b-myService +--- +apiVersion: v1 +kind: Deployment +metadata: + name: p-b-myDep +--- +apiVersion: v1 +kind: Service +metadata: + name: p-myService2 +--- +apiVersion: v1 +kind: Namespace +metadata: + name: p-myNs2 +`) } func TestBaseInResourceList(t *testing.T) { diff --git a/pkg/target/chartinflatorplugin_test.go b/pkg/target/chartinflatorplugin_test.go index 580eefd08..f744976c4 100644 --- a/pkg/target/chartinflatorplugin_test.go +++ b/pkg/target/chartinflatorplugin_test.go @@ -61,7 +61,7 @@ kind: Secret metadata: labels: app: release-name-minecraft - chart: minecraft-1.0.1 + chart: minecraft-1.0.3 heritage: Tiller release: release-name name: LOOOOOOOONG-release-name-minecraft @@ -72,7 +72,7 @@ kind: Service metadata: labels: app: release-name-minecraft - chart: minecraft-1.0.1 + chart: minecraft-1.0.3 heritage: Tiller release: release-name name: LOOOOOOOONG-release-name-minecraft @@ -93,7 +93,7 @@ metadata: volume.alpha.kubernetes.io/storage-class: default labels: app: release-name-minecraft - chart: minecraft-1.0.1 + chart: minecraft-1.0.3 heritage: Tiller release: release-name name: LOOOOOOOONG-release-name-minecraft-datadir diff --git a/pkg/target/customconfig_test.go b/pkg/target/customconfig_test.go index 078f3502c..79c84d660 100644 --- a/pkg/target/customconfig_test.go +++ b/pkg/target/customconfig_test.go @@ -312,6 +312,15 @@ spec: location: SE --- kind: Gorilla +metadata: + labels: + movie: planetOfTheApes + name: o-ursus +spec: + diet: heston + location: Arizona +--- +kind: Gorilla metadata: labels: app: myApp @@ -320,14 +329,5 @@ metadata: spec: diet: bambooshoots location: SW ---- -kind: Gorilla -metadata: - labels: - movie: planetOfTheApes - name: o-ursus -spec: - diet: heston - location: Arizona `) } diff --git a/pkg/target/generatormergeandreplace_test.go b/pkg/target/generatormergeandreplace_test.go index c6140814b..f22038019 100644 --- a/pkg/target/generatormergeandreplace_test.go +++ b/pkg/target/generatormergeandreplace_test.go @@ -374,6 +374,16 @@ secretGenerator: } th.AssertActualEqualsExpected(m, ` apiVersion: v1 +data: + hello: world +kind: ConfigMap +metadata: + labels: + env: staging + team: override-foo + name: staging-configmap-in-overlay-k7cbc75tg8 +--- +apiVersion: v1 data: foo: override-bar kind: ConfigMap @@ -388,16 +398,6 @@ metadata: name: staging-team-foo-configmap-in-base-gh9d7t85gb --- apiVersion: v1 -data: - hello: world -kind: ConfigMap -metadata: - labels: - env: staging - team: override-foo - name: staging-configmap-in-overlay-k7cbc75tg8 ---- -apiVersion: v1 data: password: c29tZXB3 proxy: aGFwcm94eQ== diff --git a/pkg/target/kusttarget_test.go b/pkg/target/kusttarget_test.go index 52e88fdae..657d68665 100644 --- a/pkg/target/kusttarget_test.go +++ b/pkg/target/kusttarget_test.go @@ -14,7 +14,6 @@ import ( "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/ifc" "sigs.k8s.io/kustomize/pkg/kusttest" - "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resource" . "sigs.k8s.io/kustomize/pkg/target" @@ -80,64 +79,53 @@ func TestResources(t *testing.T) { th.WriteF("/whatever/namespace.yaml", namespaceContent) th.WriteF("/whatever/jsonpatch.json", jsonpatchContent) - expected := resmap.New() - expected.AppendWithId( - resid.NewResIdWithPrefixSuffixNamespace( - gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, - "dply1", "foo-", "-bar", "ns1"), th.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "foo-dply1-bar", - "namespace": "ns1", - "labels": map[string]interface{}{ + resources := []*resource.Resource{ + th.RF().FromMapWithName("dply1", map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo-dply1-bar", + "namespace": "ns1", + "labels": map[string]interface{}{ + "app": "nginx", + }, + "annotations": map[string]interface{}{ + "note": "This is a test annotation", + }, + }, + "spec": map[string]interface{}{ + "replica": "3", + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ "app": "nginx", }, - "annotations": map[string]interface{}{ - "note": "This is a test annotation", - }, }, - "spec": map[string]interface{}{ - "replica": "3", - "selector": map[string]interface{}{ - "matchLabels": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "note": "This is a test annotation", + }, + "labels": map[string]interface{}{ "app": "nginx", }, }, - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "annotations": map[string]interface{}{ - "note": "This is a test annotation", - }, - "labels": map[string]interface{}{ - "app": "nginx", - }, - }, - }, }, - })) - expected.AppendWithId( - resid.NewResIdWithPrefixSuffixNamespace( - gvk.Gvk{Version: "v1", Kind: "Namespace"}, - "ns1", "foo-", "-bar", ""), th.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Namespace", - "metadata": map[string]interface{}{ - "name": "foo-ns1-bar", - "labels": map[string]interface{}{ - "app": "nginx", - }, - "annotations": map[string]interface{}{ - "note": "This is a test annotation", - }, + }, + }), + th.RF().FromMapWithName("ns1", map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": "foo-ns1-bar", + "labels": map[string]interface{}{ + "app": "nginx", }, - })) - expected.AppendWithId( - resid.NewResIdWithPrefixSuffixNamespace( - gvk.Gvk{Version: "v1", Kind: "ConfigMap"}, - "literalConfigMap", "foo-", "-bar", "ns1"), th.FromMap( + "annotations": map[string]interface{}{ + "note": "This is a test annotation", + }, + }, + }), + th.RF().FromMapWithName("literalConfigMap", map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -155,11 +143,8 @@ func TestResources(t *testing.T) { "DB_USERNAME": "admin", "DB_PASSWORD": "somepw", }, - })) - expected.AppendWithId( - resid.NewResIdWithPrefixSuffixNamespace( - gvk.Gvk{Version: "v1", Kind: "Secret"}, - "secret", "foo-", "-bar", "ns1"), th.FromMap( + }), + th.RF().FromMapWithName("secret", map[string]interface{}{ "apiVersion": "v1", "kind": "Secret", @@ -178,13 +163,22 @@ func TestResources(t *testing.T) { "DB_USERNAME": base64.StdEncoding.EncodeToString([]byte("admin")), "DB_PASSWORD": base64.StdEncoding.EncodeToString([]byte("somepw")), }, - })) + }), + } + + expected := resmap.New() + for _, r := range resources { + if err := expected.Append(r); err != nil { + t.Fatalf("unexpected error %v", err) + } + } + actual, err := th.MakeKustTarget().MakeCustomizedResMap() if err != nil { t.Fatalf("unexpected Resources error %v", err) } - if err = expected.ErrorIfNotEqualSets(actual); err != nil { + if err = expected.ErrorIfNotEqualLists(actual); err != nil { t.Fatalf("unexpected inequality: %v", err) } } @@ -215,7 +209,7 @@ func TestResourceNotFound(t *testing.T) { func findSecret(m resmap.ResMap) *resource.Resource { for _, r := range m.Resources() { - if r.Id().Gvk().Kind == "Secret" { + if r.OrgId().Kind == "Secret" { return r } } diff --git a/pkg/target/multiplepatch_test.go b/pkg/target/multiplepatch_test.go index 5fce30b1f..73c2a7c47 100644 --- a/pkg/target/multiplepatch_test.go +++ b/pkg/target/multiplepatch_test.go @@ -149,6 +149,15 @@ spec: } th.AssertActualEqualsExpected(m, ` apiVersion: v1 +data: + hello: world +kind: ConfigMap +metadata: + labels: + env: staging + name: staging-configmap-in-overlay-k7cbc75tg8 +--- +apiVersion: v1 data: foo: bar kind: ConfigMap @@ -163,15 +172,6 @@ metadata: name: staging-team-foo-configmap-in-base-g7k6gt2889 --- apiVersion: v1 -data: - hello: world -kind: ConfigMap -metadata: - labels: - env: staging - name: staging-configmap-in-overlay-k7cbc75tg8 ---- -apiVersion: v1 kind: Service metadata: annotations: diff --git a/pkg/target/resourceconflict_test.go b/pkg/target/resourceconflict_test.go index 0fd745564..127975204 100644 --- a/pkg/target/resourceconflict_test.go +++ b/pkg/target/resourceconflict_test.go @@ -10,14 +10,19 @@ import ( "sigs.k8s.io/kustomize/pkg/kusttest" ) -func writeCombinedOverlays(th *kusttest_test.KustTestHarness) { - // Base +func writeBase(th *kusttest_test.KustTestHarness) { th.WriteK("/app/base", ` resources: - serviceaccount.yaml - rolebinding.yaml -namePrefix: base- -nameSuffix: -suffix +namePrefix: pfx- +nameSuffix: -sfx +`) + th.WriteF("/app/base/serviceaccount.yaml", ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: serviceaccount `) th.WriteF("/app/base/rolebinding.yaml", ` apiVersion: rbac.authorization.k8s.io/v1beta1 @@ -32,13 +37,9 @@ subjects: - kind: ServiceAccount name: serviceaccount `) - th.WriteF("/app/base/serviceaccount.yaml", ` -apiVersion: v1 -kind: ServiceAccount -metadata: - name: serviceaccount -`) +} +func writeMidOverlays(th *kusttest_test.KustTestHarness) { // Mid-level overlays th.WriteK("/app/overlays/a", ` bases: @@ -52,7 +53,9 @@ bases: namePrefix: b- nameSuffix: -suffixB `) +} +func writeTopOverlay(th *kusttest_test.KustTestHarness) { // Top overlay, combining the mid-level overlays th.WriteK("/app/combined", ` bases: @@ -61,9 +64,9 @@ bases: `) } -func TestMultibasesNoConflict(t *testing.T) { - th := kusttest_test.NewKustTestHarness(t, "/app/combined") - writeCombinedOverlays(th) +func TestBase(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/base") + writeBase(th) m, err := th.MakeKustTarget().MakeCustomizedResMap() if err != nil { t.Fatalf("Unexpected err: %v", err) @@ -72,42 +75,129 @@ func TestMultibasesNoConflict(t *testing.T) { apiVersion: v1 kind: ServiceAccount metadata: - name: a-base-serviceaccount-suffix-suffixA + name: pfx-serviceaccount-sfx +--- +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: RoleBinding +metadata: + name: pfx-rolebinding-sfx +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: role +subjects: +- kind: ServiceAccount + name: pfx-serviceaccount-sfx +`) +} + +func TestMidLevelA(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/overlays/a") + writeBase(th) + writeMidOverlays(th) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Unexpected err: %v", err) + } + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: a-pfx-serviceaccount-sfx-suffixA +--- +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: RoleBinding +metadata: + name: a-pfx-rolebinding-sfx-suffixA +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: role +subjects: +- kind: ServiceAccount + name: a-pfx-serviceaccount-sfx-suffixA +`) +} + +func TestMidLevelB(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/overlays/b") + writeBase(th) + writeMidOverlays(th) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Unexpected err: %v", err) + } + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: b-pfx-serviceaccount-sfx-suffixB +--- +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: RoleBinding +metadata: + name: b-pfx-rolebinding-sfx-suffixB +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: role +subjects: +- kind: ServiceAccount + name: b-pfx-serviceaccount-sfx-suffixB +`) +} + +func TestMultibasesNoConflict(t *testing.T) { + th := kusttest_test.NewKustTestHarness(t, "/app/combined") + writeBase(th) + writeMidOverlays(th) + writeTopOverlay(th) + m, err := th.MakeKustTarget().MakeCustomizedResMap() + if err != nil { + t.Fatalf("Unexpected err: %v", err) + } + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: ServiceAccount +metadata: + name: a-pfx-serviceaccount-sfx-suffixA --- apiVersion: v1 kind: ServiceAccount metadata: - name: b-base-serviceaccount-suffix-suffixB + name: b-pfx-serviceaccount-sfx-suffixB --- apiVersion: rbac.authorization.k8s.io/v1beta1 kind: RoleBinding metadata: - name: a-base-rolebinding-suffix-suffixA + name: a-pfx-rolebinding-sfx-suffixA roleRef: apiGroup: rbac.authorization.k8s.io kind: Role name: role subjects: - kind: ServiceAccount - name: a-base-serviceaccount-suffix-suffixA + name: a-pfx-serviceaccount-sfx-suffixA --- apiVersion: rbac.authorization.k8s.io/v1beta1 kind: RoleBinding metadata: - name: b-base-rolebinding-suffix-suffixB + name: b-pfx-rolebinding-sfx-suffixB roleRef: apiGroup: rbac.authorization.k8s.io kind: Role name: role subjects: - kind: ServiceAccount - name: b-base-serviceaccount-suffix-suffixB + name: b-pfx-serviceaccount-sfx-suffixB `) } func TestMultibasesWithConflict(t *testing.T) { th := kusttest_test.NewKustTestHarness(t, "/app/combined") - writeCombinedOverlays(th) + writeBase(th) + writeMidOverlays(th) + writeTopOverlay(th) th.WriteK("/app/overlays/a", ` bases: @@ -131,7 +221,7 @@ metadata: t.Fatalf("Expected resource conflict.") } if !strings.Contains( - err.Error(), "Multiple matches for name ~G_v1_ServiceAccount") { + err.Error(), "multiple matches for ~G_v1_ServiceAccount") { t.Fatalf("Unexpected err: %v", err) } } diff --git a/pkg/target/variableref_test.go b/pkg/target/variableref_test.go index 23182b17f..8b6e3e102 100644 --- a/pkg/target/variableref_test.go +++ b/pkg/target/variableref_test.go @@ -411,10 +411,16 @@ metadata: apiVersion: v1 kind: Service metadata: + annotations: + prometheus.io/path: _status/vars + prometheus.io/port: "8080" + prometheus.io/scrape: "true" + service.alpha.kubernetes.io/tolerate-unready-endpoints: "true" labels: app: cockroachdb - name: dev-base-cockroachdb-public + name: dev-base-cockroachdb spec: + clusterIP: None ports: - name: grpc port: 26257 @@ -428,16 +434,10 @@ spec: apiVersion: v1 kind: Service metadata: - annotations: - prometheus.io/path: _status/vars - prometheus.io/port: "8080" - prometheus.io/scrape: "true" - service.alpha.kubernetes.io/tolerate-unready-endpoints: "true" labels: app: cockroachdb - name: dev-base-cockroachdb + name: dev-base-cockroachdb-public spec: - clusterIP: None ports: - name: grpc port: 26257 diff --git a/pkg/transformers/image.go b/pkg/transformers/image.go index 0ddcca92e..eba54ad56 100644 --- a/pkg/transformers/image.go +++ b/pkg/transformers/image.go @@ -46,7 +46,7 @@ func (pt *imageTransformer) Transform(m resmap.ResMap) error { } for _, r := range m.Resources() { for _, path := range pt.fieldSpecs { - if !r.Id().Gvk().IsSelected(&path.Gvk) { + if !r.OrgId().IsSelected(&path.Gvk) { continue } err := MutateField(r.Map(), path.PathSlice(), false, pt.mutateImage) @@ -55,7 +55,7 @@ func (pt *imageTransformer) Transform(m resmap.ResMap) error { } } // Kept for backward compatibility - if err := pt.findAndReplaceImage(r.Map()); err != nil && r.Id().Kind != `CustomResourceDefinition` { + if err := pt.findAndReplaceImage(r.Map()); err != nil && r.OrgId().Kind != `CustomResourceDefinition` { return err } } diff --git a/pkg/transformers/labelsandannotations.go b/pkg/transformers/labelsandannotations.go index 5d92ac1ef..b76651a3b 100644 --- a/pkg/transformers/labelsandannotations.go +++ b/pkg/transformers/labelsandannotations.go @@ -61,7 +61,7 @@ func NewMapTransformer( func (o *mapTransformer) Transform(m resmap.ResMap) error { for _, r := range m.Resources() { for _, path := range o.fieldSpecs { - if !r.Id().Gvk().IsSelected(&path.Gvk) { + if !r.OrgId().IsSelected(&path.Gvk) { continue } err := MutateField( diff --git a/pkg/transformers/labelsandannotations_test.go b/pkg/transformers/labelsandannotations_test.go index ac85426fe..fd3ef0fb9 100644 --- a/pkg/transformers/labelsandannotations_test.go +++ b/pkg/transformers/labelsandannotations_test.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package transformers @@ -20,412 +7,385 @@ import ( "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/pkg/gvk" - "sigs.k8s.io/kustomize/pkg/resid" - "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/transformers/config" ) -var service = gvk.Gvk{Version: "v1", Kind: "Service"} -var cmap = gvk.Gvk{Version: "v1", Kind: "ConfigMap"} -var ns = gvk.Gvk{Version: "v1", Kind: "Namespace"} -var deploy = gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"} -var crd = gvk.Gvk{Group: "apiextensions.k8s.io", Version: "v1beta1", Kind: "CustomResourceDefinition"} -var job = gvk.Gvk{Group: "batch", Version: "v1", Kind: "Job"} -var cronjob = gvk.Gvk{Group: "batch", Version: "v1beta1", Kind: "CronJob"} -var pv = gvk.Gvk{Version: "v1", Kind: "PersistentVolume"} -var cr = gvk.Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole"} -var crb = gvk.Gvk{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRoleBinding"} -var sa = gvk.Gvk{Version: "v1", Kind: "ServiceAccount"} var rf = resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()) var defaultTransformerConfig = config.MakeDefaultConfig() func TestLabelsRun(t *testing.T) { - m := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - }, - }), - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "group": "apps", - "apiVersion": "v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, - }, - }, - }, - }, - }), - resid.NewResId(service, "svc1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "svc1", - }, - "spec": map[string]interface{}{ - "ports": []interface{}{ - map[string]interface{}{ - "name": "port1", - "port": "12345", - }, - }, - }, - }), - resid.NewResId(job, "job1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1", - "kind": "Job", - "metadata": map[string]interface{}{ - "name": "job1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, - }, - }, - }, - }, - }), - resid.NewResId(job, "job2"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1", - "kind": "Job", - "metadata": map[string]interface{}{ - "name": "job2", - }, - "spec": map[string]interface{}{ - "selector": map[string]interface{}{ - "matchLabels": map[string]interface{}{ + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + }, + }). + Add(map[string]interface{}{ + "group": "apps", + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ "old-label": "old-value", }, }, - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", }, }, }, }, - }), - resid.NewResId(cronjob, "cronjob1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1beta1", - "kind": "CronJob", - "metadata": map[string]interface{}{ - "name": "cronjob1", + }, + }). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "svc1", + }, + "spec": map[string]interface{}{ + "ports": []interface{}{ + map[string]interface{}{ + "name": "port1", + "port": "12345", + }, }, - "spec": map[string]interface{}{ - "schedule": "* 23 * * *", - "jobTemplate": map[string]interface{}{ - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1", + "kind": "Job", + "metadata": map[string]interface{}{ + "name": "job1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", + }, + }, + }, + }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1", + "kind": "Job", + "metadata": map[string]interface{}{ + "name": "job2", + }, + "spec": map[string]interface{}{ + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ + "old-label": "old-value", + }, + }, + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", + }, + }, + }, + }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1beta1", + "kind": "CronJob", + "metadata": map[string]interface{}{ + "name": "cronjob1", + }, + "spec": map[string]interface{}{ + "schedule": "* 23 * * *", + "jobTemplate": map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", }, }, }, }, }, }, - }), - resid.NewResId(cronjob, "cronjob2"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1beta1", - "kind": "CronJob", - "metadata": map[string]interface{}{ - "name": "cronjob2", - }, - "spec": map[string]interface{}{ - "schedule": "* 23 * * *", - "jobTemplate": map[string]interface{}{ - "spec": map[string]interface{}{ - "selector": map[string]interface{}{ - "matchLabels": map[string]interface{}{ - "old-label": "old-value", - }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1beta1", + "kind": "CronJob", + "metadata": map[string]interface{}{ + "name": "cronjob2", + }, + "spec": map[string]interface{}{ + "schedule": "* 23 * * *", + "jobTemplate": map[string]interface{}{ + "spec": map[string]interface{}{ + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ + "old-label": "old-value", }, - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, + }, + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", }, }, }, }, }, }, - }), - }) - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - "labels": map[string]interface{}{ + }, + }).ResMap() + + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + }). + Add(map[string]interface{}{ + "group": "apps", + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ "label-key1": "label-value1", "label-key2": "label-value2", }, }, - }), - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "group": "apps", - "apiVersion": "v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "selector": map[string]interface{}{ - "matchLabels": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", "label-key1": "label-value1", "label-key2": "label-value2", }, }, - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", + }, + }, + }, + }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "svc1", + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "ports": []interface{}{ + map[string]interface{}{ + "name": "port1", + "port": "12345", + }, + }, + "selector": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1", + "kind": "Job", + "metadata": map[string]interface{}{ + "name": "job1", + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", + }, + }, + }, + }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1", + "kind": "Job", + "metadata": map[string]interface{}{ + "name": "job2", + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + "old-label": "old-value", + }, + }, + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", + }, + }, + }, + }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1beta1", + "kind": "CronJob", + "metadata": map[string]interface{}{ + "name": "cronjob1", + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "schedule": "* 23 * * *", + "jobTemplate": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", + }, + }, + }, + }, + }, + }, + }, + }). + Add(map[string]interface{}{ + "apiVersion": "batch/v1beta1", + "kind": "CronJob", + "metadata": map[string]interface{}{ + "name": "cronjob2", + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "schedule": "* 23 * * *", + "jobTemplate": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "label-key1": "label-value1", + "label-key2": "label-value2", + }, + }, + "spec": map[string]interface{}{ + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ "old-label": "old-value", "label-key1": "label-value1", "label-key2": "label-value2", }, }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, - }, - }, - }, - }, - }), - resid.NewResId(service, "svc1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "svc1", - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "ports": []interface{}{ - map[string]interface{}{ - "name": "port1", - "port": "12345", - }, - }, - "selector": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - }), - resid.NewResId(job, "job1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1", - "kind": "Job", - "metadata": map[string]interface{}{ - "name": "job1", - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, - }, - }, - }, - }, - }), - resid.NewResId(job, "job2"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1", - "kind": "Job", - "metadata": map[string]interface{}{ - "name": "job2", - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "selector": map[string]interface{}{ - "matchLabels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - "old-label": "old-value", - }, - }, - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, - }, - }, - }, - }, - }), - resid.NewResId(cronjob, "cronjob1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1beta1", - "kind": "CronJob", - "metadata": map[string]interface{}{ - "name": "cronjob1", - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "schedule": "* 23 * * *", - "jobTemplate": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, - }, - }, - }, - }, - }, - }, - }), - resid.NewResId(cronjob, "cronjob2"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "batch/v1beta1", - "kind": "CronJob", - "metadata": map[string]interface{}{ - "name": "cronjob2", - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "schedule": "* 23 * * *", - "jobTemplate": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "selector": map[string]interface{}{ - "matchLabels": map[string]interface{}{ - "old-label": "old-value", + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ "label-key1": "label-value1", "label-key2": "label-value2", }, }, - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "label-key1": "label-value1", - "label-key2": "label-value2", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", }, }, }, }, }, }, - }), - }) + }, + }).ResMap() + lt, err := NewLabelsMapTransformer( map[string]string{"label-key1": "label-value1", "label-key2": "label-value2"}, defaultTransformerConfig.CommonLabels) @@ -436,132 +396,125 @@ func TestLabelsRun(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(m); err != nil { + if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } func TestAnnotationsRun(t *testing.T) { - m := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - }, - }), - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "group": "apps", - "apiVersion": "v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "labels": map[string]interface{}{ - "old-label": "old-value", - }, + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + }, + }). + Add(map[string]interface{}{ + "group": "apps", + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", }, }, }, }, - }), - resid.NewResId(service, "svc1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "svc1", + }, + }). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "svc1", + }, + "spec": map[string]interface{}{ + "ports": []interface{}{ + map[string]interface{}{ + "name": "port1", + "port": "12345", + }, }, - "spec": map[string]interface{}{ - "ports": []interface{}{ - map[string]interface{}{ - "name": "port1", - "port": "12345", + }, + }).ResMap() + + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + "annotations": map[string]interface{}{ + "anno-key1": "anno-value1", + "anno-key2": "anno-value2", + }, + }, + }). + Add(map[string]interface{}{ + "group": "apps", + "apiVersion": "v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + "annotations": map[string]interface{}{ + "anno-key1": "anno-value1", + "anno-key2": "anno-value2", + }, + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "anno-key1": "anno-value1", + "anno-key2": "anno-value2", + }, + "labels": map[string]interface{}{ + "old-label": "old-value", }, }, - }, - }), - }) - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - "annotations": map[string]interface{}{ - "anno-key1": "anno-value1", - "anno-key2": "anno-value2", - }, - }, - }), - resid.NewResId(deploy, "deploy1"): rf.FromMap( - map[string]interface{}{ - "group": "apps", - "apiVersion": "v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "deploy1", - "annotations": map[string]interface{}{ - "anno-key1": "anno-value1", - "anno-key2": "anno-value2", - }, - }, - "spec": map[string]interface{}{ - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "annotations": map[string]interface{}{ - "anno-key1": "anno-value1", - "anno-key2": "anno-value2", - }, - "labels": map[string]interface{}{ - "old-label": "old-value", - }, - }, - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": "nginx", - "image": "nginx:1.7.9", - }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx:1.7.9", }, }, }, }, - }), - resid.NewResId(service, "svc1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "svc1", - "annotations": map[string]interface{}{ - "anno-key1": "anno-value1", - "anno-key2": "anno-value2", + }, + }). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "svc1", + "annotations": map[string]interface{}{ + "anno-key1": "anno-value1", + "anno-key2": "anno-value2", + }, + }, + "spec": map[string]interface{}{ + "ports": []interface{}{ + map[string]interface{}{ + "name": "port1", + "port": "12345", }, }, - "spec": map[string]interface{}{ - "ports": []interface{}{ - map[string]interface{}{ - "name": "port1", - "port": "12345", - }, - }, - }, - }), - }) + }, + }).ResMap() at, err := NewAnnotationsMapTransformer( map[string]string{"anno-key1": "anno-value1", "anno-key2": "anno-value2"}, defaultTransformerConfig.CommonAnnotations) @@ -572,38 +525,34 @@ func TestAnnotationsRun(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(m); err != nil { + if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } -func TestAnnotaionsRunWithNullValue(t *testing.T) { - m := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - "annotations": nil, - }, - }), - }) +func TestAnnotationsRunWithNullValue(t *testing.T) { + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + "annotations": nil, + }, + }).ResMap() - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - "annotations": map[string]interface{}{ - "anno-key1": "anno-value1", - "anno-key2": "anno-value2", - }, + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + "annotations": map[string]interface{}{ + "anno-key1": "anno-value1", + "anno-key2": "anno-value2", }, - }), - }) + }, + }).ResMap() at, err := NewAnnotationsMapTransformer( map[string]string{"anno-key1": "anno-value1", "anno-key2": "anno-value2"}, @@ -615,8 +564,7 @@ func TestAnnotaionsRunWithNullValue(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(m); err != nil { + if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } - } diff --git a/pkg/transformers/multitransformer.go b/pkg/transformers/multitransformer.go index dc446c6f7..cd78e3802 100644 --- a/pkg/transformers/multitransformer.go +++ b/pkg/transformers/multitransformer.go @@ -18,6 +18,7 @@ package transformers import ( "fmt" + "sigs.k8s.io/kustomize/pkg/resmap" ) diff --git a/pkg/transformers/namereference.go b/pkg/transformers/namereference.go index ef3ca1958..ef8c48b95 100644 --- a/pkg/transformers/namereference.go +++ b/pkg/transformers/namereference.go @@ -7,8 +7,9 @@ import ( "fmt" "log" + "sigs.k8s.io/kustomize/pkg/resource" + "sigs.k8s.io/kustomize/pkg/gvk" - "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/transformers/config" ) @@ -44,14 +45,15 @@ func NewNameReferenceTransformer(br []config.NameBackReferences) Transformer { // // - kind: Deployment // fieldSpecs: -// - path: spec/scaleTargetRef/name -// kind: HorizontalPodAutoscaler +// - kind: HorizontalPodAutoscaler +// path: spec/scaleTargetRef/name // -// saying that an HPA, via its 'spec/scaleTargetRef/name' -// field, may refer to a Deployment. This match to HPA -// means we may need to modify the value in its -// 'spec/scaleTargetRef/name' field, by searching for -// the thing it refers to, and getting its new name. +// This entry says that an HPA, via its +// 'spec/scaleTargetRef/name' field, may refer to a +// Deployment. This match to HPA means we may need to +// modify the value in its 'spec/scaleTargetRef/name' +// field, by searching for the thing it refers to, +// and getting its new name. // // As a filter, and search optimization, we compute a // subset of all resources that the HPA could refer to, @@ -76,19 +78,23 @@ func NewNameReferenceTransformer(br []config.NameBackReferences) Transformer { // func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error { // TODO: Too much looping, here and in transitive calls. - for referrer, res := range m.AsMap() { + for _, referrer := range m.Resources() { var candidates resmap.ResMap for _, target := range o.backRefs { for _, fSpec := range target.FieldSpecs { - if referrer.Gvk().IsSelected(&fSpec.Gvk) { + if referrer.OrgId().IsSelected(&fSpec.Gvk) { if candidates == nil { - candidates = m.SubsetThatCouldBeReferencedById(referrer) + candidates = m.SubsetThatCouldBeReferencedByResource(referrer) } err := MutateField( - res.Map(), + referrer.Map(), fSpec.PathSlice(), fSpec.CreateIfNotPresent, - o.getNewName( + o.getNewNameFunc( + // referrer could be an HPA instance, + // target could be Gvk for Deployment, + // candidate a list of resources "reachable" + // from the HPA. referrer, target.Gvk, candidates)) if err != nil { return err @@ -100,26 +106,28 @@ func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error { return nil } -func (o *nameReferenceTransformer) getNewName( - referrer resid.ResId, +func (o *nameReferenceTransformer) getNewNameFunc( + referrer *resource.Resource, target gvk.Gvk, referralCandidates resmap.ResMap) func(in interface{}) (interface{}, error) { return func(in interface{}) (interface{}, error) { switch in.(type) { case string: oldName, _ := in.(string) - for id, res := range referralCandidates.AsMap() { - if id.Gvk().IsSelected(&target) && id.Name() == oldName { - matchedIds := referralCandidates.GetMatchingIds(id.GvknEquals) + for _, res := range referralCandidates.Resources() { + id := res.OrgId() + if id.IsSelected(&target) && res.GetOriginalName() == oldName { + matches := referralCandidates.GetMatchingResourcesByOriginalId(id.GvknEquals) // If there's more than one match, there's no way // to know which one to pick, so emit error. - if len(matchedIds) > 1 { + if len(matches) > 1 { return nil, fmt.Errorf( - "Multiple matches for name %s:\n %v", id, matchedIds) + "string case - multiple matches for %s:\n %v", + id, getIds(matches)) } // In the resource, note that it is referenced // by the referrer. - res.AppendRefBy(referrer) + res.AppendRefBy(referrer.CurId()) // Return transformed name of the object, // complete with prefixes, hashes, etc. return res.GetName(), nil @@ -137,18 +145,20 @@ func (o *nameReferenceTransformer) getNewName( } names = append(names, name) } - for id, res := range referralCandidates.AsMap() { - indexes := indexOf(id.Name(), names) - if id.Gvk().IsSelected(&target) && len(indexes) > 0 { - matchedIds := referralCandidates.GetMatchingIds(id.GvknEquals) - if len(matchedIds) > 1 { + for _, res := range referralCandidates.Resources() { + indexes := indexOf(res.GetOriginalName(), names) + id := res.OrgId() + if id.IsSelected(&target) && len(indexes) > 0 { + matches := referralCandidates.GetMatchingResourcesByOriginalId(id.GvknEquals) + if len(matches) > 1 { return nil, fmt.Errorf( - "Multiple matches for name %s:\n %v", id, matchedIds) + "slice case - multiple matches for %s:\n %v", + id, getIds(matches)) } for _, index := range indexes { l[index] = res.GetName() } - res.AppendRefBy(referrer) + res.AppendRefBy(referrer.CurId()) return l, nil } } @@ -169,3 +179,11 @@ func indexOf(s string, slice []string) []int { } return index } + +func getIds(rs []*resource.Resource) []string { + var result []string + for _, r := range rs { + result = append(result, r.CurId().String()+"\n") + } + return result +} diff --git a/pkg/transformers/namereference_test.go b/pkg/transformers/namereference_test.go index a6844d8e2..668da6d2a 100644 --- a/pkg/transformers/namereference_test.go +++ b/pkg/transformers/namereference_test.go @@ -4,14 +4,12 @@ package transformers import ( - "sigs.k8s.io/kustomize/pkg/gvk" - "sigs.k8s.io/kustomize/pkg/resmaptest" "strings" "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/resource" ) @@ -466,7 +464,7 @@ func TestNameReferenceHappyRun(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(m); err != nil { + if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } @@ -537,7 +535,8 @@ func TestNameReferenceUnhappyRun(t *testing.T) { func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmaptest_test.NewRmBuilder(t, rf).AddWithName( + + v1 := rf.FromMapWithName( "volume1", map[string]interface{}{ "apiVersion": "v1", @@ -545,7 +544,8 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "metadata": map[string]interface{}{ "name": "someprefix-volume1", }, - }).AddWithName( + }) + c1 := rf.FromMapWithName( "claim1", map[string]interface{}{ "apiVersion": "v1", @@ -557,17 +557,10 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "spec": map[string]interface{}{ "volumeName": "volume1", }, - }).ResMap() + }) - expected := resmaptest_test.NewRmBuilder(t, rf).AddWithName( - "volume1", - map[string]interface{}{ - "apiVersion": "v1", - "kind": "PersistentVolume", - "metadata": map[string]interface{}{ - "name": "someprefix-volume1", - }, - }).AddWithName( + v2 := v1.DeepCopy() + c2 := rf.FromMapWithName( "claim1", map[string]interface{}{ "apiVersion": "v1", @@ -579,16 +572,19 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "spec": map[string]interface{}{ "volumeName": "someprefix-volume1", }, - }).ResMap() - expected.GetById( - resid.NewResId(gvk.Gvk{Version: "v1", Kind: "PersistentVolume"}, "volume1")).AppendRefBy( - resid.NewResId(gvk.Gvk{Version: "v1", Kind: "PersistentVolumeClaim"}, "claim1")) + }) + + m1 := resmaptest_test.NewRmBuilder(t, rf).AddR(v1).AddR(c1).ResMap() + nrt := NewNameReferenceTransformer(defaultTransformerConfig.NameReference) - err := nrt.Transform(m) - if err != nil { + if err := nrt.Transform(m1); err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(m); err != nil { + + m2 := resmaptest_test.NewRmBuilder(t, rf).AddR(v2).AddR(c2).ResMap() + v2.AppendRefBy(c2.CurId()) + + if err := m1.ErrorIfNotEqualLists(m2); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } diff --git a/pkg/transformers/namespace.go b/pkg/transformers/namespace.go index 990ec546b..48a165e22 100644 --- a/pkg/transformers/namespace.go +++ b/pkg/transformers/namespace.go @@ -1,96 +1,52 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package transformers import ( "sigs.k8s.io/kustomize/pkg/gvk" + "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/transformers/config" ) type namespaceTransformer struct { - namespace string - fieldSpecsToUse []config.FieldSpec - fieldSpecsToSkip []config.FieldSpec + namespace string + fieldSpecsToUse []config.FieldSpec } var _ Transformer = &namespaceTransformer{} // NewNamespaceTransformer construct a namespaceTransformer. func NewNamespaceTransformer(ns string, cf []config.FieldSpec) Transformer { - if len(ns) == 0 { - return NewNoOpTransformer() - } - var skip []config.FieldSpec - for _, g := range gvk.ClusterLevelGvks() { - skip = append(skip, config.FieldSpec{Gvk: g}) - } return &namespaceTransformer{ - namespace: ns, - fieldSpecsToUse: cf, - fieldSpecsToSkip: skip, + namespace: ns, + fieldSpecsToUse: cf, } } +const metaNamespace = "metadata/namespace" + // Transform adds the namespace. -func (o *namespaceTransformer) Transform(m resmap.ResMap) error { - mf := o.filterResmap(m) - - for id, res := range mf.AsMap() { - objMap := res.Map() - for _, path := range o.fieldSpecsToUse { - switch path.Path { - // Special casing .metadata.namespace since it is a common metadata field across all runtime.Object - // We should add namespace if it's namespaced resource; otherwise, we should not. - case "metadata/namespace": - if id.Gvk().IsSelected(&path.Gvk) && !id.Gvk().IsClusterKind() { - if len(objMap) > 0 { - err := MutateField( - objMap, path.PathSlice(), path.CreateIfNotPresent, - func(_ interface{}) (interface{}, error) { - return o.namespace, nil - }) - if err != nil { - return err - } - } - } - default: - if !id.Gvk().IsSelected(&path.Gvk) { - continue - } - // make sure the object is non empty - if len(objMap) > 0 { - err := MutateField( - objMap, path.PathSlice(), path.CreateIfNotPresent, - func(_ interface{}) (interface{}, error) { - return o.namespace, nil - }) - if err != nil { - return err - } - } - } - - if !id.Gvk().IsClusterKind() { - newid := id.CopyWithNewNamespace(o.namespace) - m.AppendWithId(newid, res) - } else { - m.AppendWithId(id, res) +func (o *namespaceTransformer) Transform(m resmap.ResMap) (err error) { + if len(o.namespace) == 0 { + return nil + } + for _, r := range m.Resources() { + id := r.OrgId() + fs, ok := o.isSelected(id) + if !ok { + continue + } + if len(r.Map()) == 0 { + // Don't mutate empty objects? + continue + } + if doIt(id, fs) { + err = o.changeNamespace(r, fs) + if err != nil { + return err } } } @@ -98,35 +54,46 @@ func (o *namespaceTransformer) Transform(m resmap.ResMap) error { return nil } -func (o *namespaceTransformer) filterResmap(m resmap.ResMap) resmap.ResMap { - mf := resmap.New() - for id, res := range m.AsMap() { - found := false - for _, path := range o.fieldSpecsToSkip { - if id.Gvk().IsSelected(&path.Gvk) { - found = true - mf.AppendWithId(id, res) - m.Remove(id) - } - } - if !found { - mf.AppendWithId(id, res) - m.Remove(id) +// Special casing metadata.namespace since +// all objects have it, even "ClusterKind" objects +// that don't exist in a namespace (the Namespace +// object itself doesn't live in a namespace). +func doIt(id resid.ResId, fs *config.FieldSpec) bool { + return fs.Path != metaNamespace || + (fs.Path == metaNamespace && !id.IsClusterKind()) +} + +func (o *namespaceTransformer) changeNamespace( + r *resource.Resource, fs *config.FieldSpec) error { + return MutateField( + r.Map(), fs.PathSlice(), fs.CreateIfNotPresent, + func(_ interface{}) (interface{}, error) { + return o.namespace, nil + }) +} + +func (o *namespaceTransformer) isSelected( + id resid.ResId) (*config.FieldSpec, bool) { + for _, fs := range o.fieldSpecsToUse { + if id.IsSelected(&fs.Gvk) { + return &fs, true } } - return mf + return nil, false } func (o *namespaceTransformer) updateClusterRoleBinding(m resmap.ResMap) { + srvAccount := gvk.Gvk{Version: "v1", Kind: "ServiceAccount"} saMap := map[string]bool{} - for id := range m.AsMap() { - if id.Gvk().Equals(gvk.Gvk{Version: "v1", Kind: "ServiceAccount"}) { - saMap[id.Name()] = true + for _, id := range m.AllIds() { + if id.Gvk.Equals(srvAccount) { + saMap[id.Name] = true } } - for id, res := range m.AsMap() { - if id.Gvk().Kind != "ClusterRoleBinding" && id.Gvk().Kind != "RoleBinding" { + for _, res := range m.Resources() { + if res.OrgId().Kind != "ClusterRoleBinding" && + res.OrgId().Kind != "RoleBinding" { continue } objMap := res.Map() @@ -138,15 +105,17 @@ func (o *namespaceTransformer) updateClusterRoleBinding(m resmap.ResMap) { subject := subjects[i].(map[string]interface{}) kind, foundk := subject["kind"] name, foundn := subject["name"] - if !foundk || !foundn || kind.(string) != "ServiceAccount" { + if !foundk || !foundn || kind.(string) != srvAccount.Kind { continue } // a ServiceAccount named “default” exists in every active namespace if name.(string) == "default" || saMap[name.(string)] { subject := subjects[i].(map[string]interface{}) - MutateField(subject, []string{"namespace"}, true, func(_ interface{}) (interface{}, error) { - return o.namespace, nil - }) + MutateField( + subject, []string{"namespace"}, + true, func(_ interface{}) (interface{}, error) { + return o.namespace, nil + }) subjects[i] = subject } } diff --git a/pkg/transformers/namespace_test.go b/pkg/transformers/namespace_test.go index b650aa0cb..92c28fb10 100644 --- a/pkg/transformers/namespace_test.go +++ b/pkg/transformers/namespace_test.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package transformers @@ -20,184 +7,148 @@ import ( "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" - "sigs.k8s.io/kustomize/pkg/resid" - "sigs.k8s.io/kustomize/pkg/resmap" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/resource" ) func TestNamespaceRun(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - }, - }), - resid.NewResId(cmap, "cm2"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm2", - "namespace": "foo", - }, - }), - resid.NewResId(cmap, "cm3"): rf.FromMap( - map[string]interface{}{}, - ), - resid.NewResId(ns, "ns1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Namespace", - "metadata": map[string]interface{}{ - "name": "ns1", - }, - }), - resid.NewResId(sa, "default"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ServiceAccount", - "metadata": map[string]interface{}{ + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm2", + "namespace": "foo", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": "ns1", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": map[string]interface{}{ + "name": "default", + "namespace": "system", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": map[string]interface{}{ + "name": "service-account", + "namespace": "system", + }}). + Add(map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": map[string]interface{}{ + "name": "manager-rolebinding", + }, + "subjects": []interface{}{ + map[string]interface{}{ + "kind": "ServiceAccount", "name": "default", "namespace": "system", }, - }), - resid.NewResId(sa, "service-account"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ServiceAccount", - "metadata": map[string]interface{}{ + map[string]interface{}{ + "kind": "ServiceAccount", "name": "service-account", "namespace": "system", }, - }), - resid.NewResId(crb, "crb"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "rbac.authorization.k8s.io/v1", - "kind": "ClusterRoleBinding", - "metadata": map[string]interface{}{ - "name": "manager-rolebinding", + map[string]interface{}{ + "kind": "ServiceAccount", + "name": "another", + "namespace": "random", }, - "subjects": []interface{}{ - map[string]interface{}{ - "kind": "ServiceAccount", - "name": "default", - "namespace": "system", - }, - map[string]interface{}{ - "kind": "ServiceAccount", - "name": "service-account", - "namespace": "system", - }, - map[string]interface{}{ - "kind": "ServiceAccount", - "name": "another", - "namespace": "random", - }, - }, - }), - resid.NewResId(crd, "crd"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apiextensions.k8s.io/v1beta1", - "kind": "CustomResourceDefinition", - "metadata": map[string]interface{}{ - "name": "crd", - }, - }), - }) - expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResIdWithNamespace(ns, "ns1", ""): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Namespace", - "metadata": map[string]interface{}{ - "name": "ns1", - }, - }), - resid.NewResIdWithNamespace(cmap, "cm1", "test"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - "namespace": "test", - }, - }), - resid.NewResIdWithNamespace(cmap, "cm2", "test"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm2", - "namespace": "test", - }, - }), - resid.NewResIdWithNamespace(cmap, "cm3", "test"): rf.FromMap( - map[string]interface{}{}, - ), - resid.NewResIdWithNamespace(sa, "default", "test"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ServiceAccount", - "metadata": map[string]interface{}{ + }}). + Add(map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1beta1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{ + "name": "crd", + }}).ResMap() + + expected := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + "namespace": "test", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm2", + "namespace": "test", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": "ns1", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": map[string]interface{}{ + "name": "default", + "namespace": "test", + }}). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": map[string]interface{}{ + "name": "service-account", + "namespace": "test", + }}). + Add(map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": map[string]interface{}{ + "name": "manager-rolebinding", + }, + "subjects": []interface{}{ + map[string]interface{}{ + "kind": "ServiceAccount", "name": "default", "namespace": "test", }, - }), - resid.NewResIdWithNamespace(sa, "service-account", "test"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ServiceAccount", - "metadata": map[string]interface{}{ + map[string]interface{}{ + "kind": "ServiceAccount", "name": "service-account", "namespace": "test", }, - }), - resid.NewResId(crb, "crb"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "rbac.authorization.k8s.io/v1", - "kind": "ClusterRoleBinding", - "metadata": map[string]interface{}{ - "name": "manager-rolebinding", + map[string]interface{}{ + "kind": "ServiceAccount", + "name": "another", + "namespace": "random", }, - "subjects": []interface{}{ - map[string]interface{}{ - "kind": "ServiceAccount", - "name": "default", - "namespace": "test", - }, - map[string]interface{}{ - "kind": "ServiceAccount", - "name": "service-account", - "namespace": "test", - }, - map[string]interface{}{ - "kind": "ServiceAccount", - "name": "another", - "namespace": "random", - }, - }, - }), - resid.NewResId(crd, "crd"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apiextensions.k8s.io/v1beta1", - "kind": "CustomResourceDefinition", - "metadata": map[string]interface{}{ - "name": "crd", - }, - }), - }) + }}). + Add(map[string]interface{}{ + "apiVersion": "apiextensions.k8s.io/v1beta1", + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{ + "name": "crd", + }}).ResMap() nst := NewNamespaceTransformer("test", defaultTransformerConfig.NameSpace) err := nst.Transform(m) if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(m); err != nil { + if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } @@ -205,45 +156,34 @@ func TestNamespaceRun(t *testing.T) { func TestNamespaceRunForClusterLevelKind(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(ns, "ns1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Namespace", - "metadata": map[string]interface{}{ - "name": "ns1", - }, - }), - resid.NewResId(crd, "crd1"): rf.FromMap( - map[string]interface{}{ - "kind": "CustomResourceDefinition", - "metadata": map[string]interface{}{ - "name": "crd1", - }, - }), - resid.NewResId(pv, "pv1"): rf.FromMap( - map[string]interface{}{ - "kind": "PersistentVolume", - "metadata": map[string]interface{}{ - "name": "pv1", - }, - }), - resid.NewResId(cr, "cr1"): rf.FromMap( - map[string]interface{}{ - "kind": "ClusterRole", - "metadata": map[string]interface{}{ - "name": "cr1", - }, - }), - resid.NewResId(crb, "crb1"): rf.FromMap( - map[string]interface{}{ - "kind": "ClusterRoleBinding", - "metadata": map[string]interface{}{ - "name": "crb1", - }, - "subjects": []interface{}{}, - }), - }) + m := resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": "ns1", + }}). + Add(map[string]interface{}{ + "kind": "CustomResourceDefinition", + "metadata": map[string]interface{}{ + "name": "crd1", + }}). + Add(map[string]interface{}{ + "kind": "PersistentVolume", + "metadata": map[string]interface{}{ + "name": "pv1", + }}). + Add(map[string]interface{}{ + "kind": "ClusterRole", + "metadata": map[string]interface{}{ + "name": "cr1", + }}). + Add(map[string]interface{}{ + "kind": "ClusterRoleBinding", + "metadata": map[string]interface{}{ + "name": "crb1", + }, + "subjects": []interface{}{}}).ResMap() expected := m.DeepCopy() @@ -253,7 +193,7 @@ func TestNamespaceRunForClusterLevelKind(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if err = expected.ErrorIfNotEqualSets(m); err != nil { + if err = expected.ErrorIfNotEqualLists(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } diff --git a/pkg/transformers/refvars.go b/pkg/transformers/refvars.go index 8366f6f42..794e87438 100644 --- a/pkg/transformers/refvars.go +++ b/pkg/transformers/refvars.go @@ -97,7 +97,7 @@ func (rv *RefVarTransformer) Transform(m resmap.ResMap) error { rv.replacementCounts, rv.varMap) for _, res := range m.Resources() { for _, fieldSpec := range rv.fieldSpecs { - if res.Id().Gvk().IsSelected(&fieldSpec.Gvk) { + if res.OrgId().IsSelected(&fieldSpec.Gvk) { if err := MutateField( res.Map(), fieldSpec.PathSlice(), false, rv.replaceVars); err != nil { diff --git a/pkg/transformers/refvars_test.go b/pkg/transformers/refvars_test.go index 6ef6c50e8..18169ad90 100644 --- a/pkg/transformers/refvars_test.go +++ b/pkg/transformers/refvars_test.go @@ -1,18 +1,5 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 package transformers @@ -20,9 +7,9 @@ import ( "reflect" "testing" - "sigs.k8s.io/kustomize/pkg/resid" + "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/resmap" - "sigs.k8s.io/kustomize/pkg/resource" + "sigs.k8s.io/kustomize/pkg/resmaptest" "sigs.k8s.io/kustomize/pkg/transformers/config" ) @@ -49,38 +36,33 @@ func TestVarRef(t *testing.T) { "BAR": "replacementForBar", }, fs: []config.FieldSpec{ - {Gvk: cmap, Path: "data"}, + {Gvk: gvk.Gvk{Version: "v1", Kind: "ConfigMap"}, Path: "data"}, }, - res: resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - }, - "data": map[string]interface{}{ - "item1": "$(FOO)", - "item2": "bla", - }, - }), - }), + res: resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + }, + "data": map[string]interface{}{ + "item1": "$(FOO)", + "item2": "bla", + }, + }).ResMap(), }, expected: expected{ - res: resmap.FromMap(map[resid.ResId]*resource.Resource{ - resid.NewResId(cmap, "cm1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "cm1", - }, - "data": map[string]interface{}{ - "item1": "replacementForFoo", - "item2": "bla", - }, - }), - }), + res: resmaptest_test.NewRmBuilder(t, rf). + Add(map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + }, + "data": map[string]interface{}{ + "item1": "replacementForFoo", + "item2": "bla", + }}).ResMap(), unused: []string{"BAR"}, }, }, @@ -101,7 +83,7 @@ func TestVarRef(t *testing.T) { a, e := tc.given.res, tc.expected.res if !reflect.DeepEqual(a, e) { - err = e.ErrorIfNotEqualSets(a) + err = e.ErrorIfNotEqualLists(a) t.Fatalf("actual doesn't match expected: \nACTUAL:\n%v\nEXPECTED:\n%v\nERR: %v", a, e, err) } diff --git a/plugin/builtin/AnnotationsTransformer.go b/plugin/builtin/AnnotationsTransformer.go index c120cb0a5..ec23eeb38 100644 --- a/plugin/builtin/AnnotationsTransformer.go +++ b/plugin/builtin/AnnotationsTransformer.go @@ -15,6 +15,7 @@ type AnnotationsTransformerPlugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewAnnotationsTransformerPlugin() *AnnotationsTransformerPlugin { return &AnnotationsTransformerPlugin{} } diff --git a/plugin/builtin/ConfigMapGenerator.go b/plugin/builtin/ConfigMapGenerator.go index 2859f23c6..73504a61b 100644 --- a/plugin/builtin/ConfigMapGenerator.go +++ b/plugin/builtin/ConfigMapGenerator.go @@ -15,6 +15,7 @@ type ConfigMapGeneratorPlugin struct { types.ConfigMapArgs } +//noinspection GoUnusedGlobalVariable func NewConfigMapGeneratorPlugin() *ConfigMapGeneratorPlugin { return &ConfigMapGeneratorPlugin{} } diff --git a/plugin/builtin/HashTransformer.go b/plugin/builtin/HashTransformer.go index cd14463f2..c577a2751 100644 --- a/plugin/builtin/HashTransformer.go +++ b/plugin/builtin/HashTransformer.go @@ -12,6 +12,7 @@ type HashTransformerPlugin struct { hasher ifc.KunstructuredHasher } +//noinspection GoUnusedGlobalVariable func NewHashTransformerPlugin() *HashTransformerPlugin { return &HashTransformerPlugin{} } diff --git a/plugin/builtin/ImageTagTransformer.go b/plugin/builtin/ImageTagTransformer.go index c27fe200e..28407fee3 100644 --- a/plugin/builtin/ImageTagTransformer.go +++ b/plugin/builtin/ImageTagTransformer.go @@ -17,6 +17,7 @@ type ImageTagTransformerPlugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewImageTagTransformerPlugin() *ImageTagTransformerPlugin { return &ImageTagTransformerPlugin{} } diff --git a/plugin/builtin/InventoryTransformer.go b/plugin/builtin/InventoryTransformer.go index ebf4c62aa..579d43837 100644 --- a/plugin/builtin/InventoryTransformer.go +++ b/plugin/builtin/InventoryTransformer.go @@ -3,7 +3,6 @@ package builtin import ( "fmt" - "strings" "sigs.k8s.io/kustomize/pkg/resource" @@ -24,6 +23,7 @@ type InventoryTransformerPlugin struct { Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewInventoryTransformerPlugin() *InventoryTransformerPlugin { return &InventoryTransformerPlugin{} } @@ -98,32 +98,33 @@ func makeInventory(m resmap.ResMap) ( inv = inventory.NewInventory() var keys []string for _, r := range m.Resources() { - ns := getNamespace(r) - item := resid.NewItemId(r.GetGvk(), ns, r.GetName()) + ns := r.GetNamespace() + item := resid.NewResIdWithNamespace(r.GetGvk(), r.GetName(), ns) if _, ok := inv.Current[item]; ok { return nil, "", fmt.Errorf( "item '%v' already in inventory", item) } - inv.Current[item] = computeRefs(r, m) + inv.Current[item], err = computeRefs(r, m) + if err != nil { + return nil, "", err + } keys = append(keys, item.String()) } h, err := hasher.SortArrayAndComputeHash(keys) return inv, h, err } -func getNamespace(r *resource.Resource) string { - ns, err := r.GetFieldValue("metadata.namespace") - if err != nil && !strings.Contains(err.Error(), "no field named") { - panic(err) - } - return ns -} - -func computeRefs(r *resource.Resource, m resmap.ResMap) (refs []resid.ItemId) { +func computeRefs( + r *resource.Resource, m resmap.ResMap) (refs []resid.ResId, err error) { for _, refid := range r.GetRefBy() { - ref := m.GetById(refid) - ns := getNamespace(ref) - refs = append(refs, resid.NewItemId(ref.GetGvk(), ns, ref.GetName())) + ref, err := m.GetByCurrentId(refid) + if err != nil { + return nil, err + } + refs = append( + refs, + resid.NewResIdWithNamespace( + ref.GetGvk(), ref.GetName(), ref.GetNamespace())) } return } diff --git a/plugin/builtin/LabelTransformer.go b/plugin/builtin/LabelTransformer.go index 96cfb2b18..bf7eb9f18 100644 --- a/plugin/builtin/LabelTransformer.go +++ b/plugin/builtin/LabelTransformer.go @@ -15,6 +15,7 @@ type LabelTransformerPlugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewLabelTransformerPlugin() *LabelTransformerPlugin { return &LabelTransformerPlugin{} } diff --git a/plugin/builtin/LegacyOrderTransformer.go b/plugin/builtin/LegacyOrderTransformer.go index 6b5c2e586..168ed04eb 100644 --- a/plugin/builtin/LegacyOrderTransformer.go +++ b/plugin/builtin/LegacyOrderTransformer.go @@ -2,6 +2,7 @@ package builtin import ( + "github.com/pkg/errors" "sigs.k8s.io/kustomize/pkg/ifc" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resource" @@ -15,6 +16,7 @@ import ( // (like ValidatingWebhookConfiguration) last. type LegacyOrderTransformerPlugin struct{} +//noinspection GoUnusedGlobalVariable func NewLegacyOrderTransformerPlugin() *LegacyOrderTransformerPlugin { return &LegacyOrderTransformerPlugin{} } @@ -25,16 +27,19 @@ func (p *LegacyOrderTransformerPlugin) Config( return nil } -func (p *LegacyOrderTransformerPlugin) Transform(m resmap.ResMap) error { +func (p *LegacyOrderTransformerPlugin) Transform(m resmap.ResMap) (err error) { resources := make([]*resource.Resource, m.Size()) ids := m.AllIds() sort.Sort(resmap.IdSlice(ids)) for i, id := range ids { - resources[i] = m.GetById(id) + resources[i], err = m.GetByCurrentId(id) + if err != nil { + return errors.Wrap(err, "expected match for sorting") + } } m.Clear() - for i, id := range ids { - m.AppendWithId(id, resources[i]) + for _, r := range resources { + m.Append(r) } return nil } diff --git a/plugin/builtin/NamespaceTransformer.go b/plugin/builtin/NamespaceTransformer.go index 9fa0124a9..0e89ca54b 100644 --- a/plugin/builtin/NamespaceTransformer.go +++ b/plugin/builtin/NamespaceTransformer.go @@ -15,6 +15,7 @@ type NamespaceTransformerPlugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewNamespaceTransformerPlugin() *NamespaceTransformerPlugin { return &NamespaceTransformerPlugin{} } diff --git a/plugin/builtin/PatchJson6902Transformer.go b/plugin/builtin/PatchJson6902Transformer.go index 79b9d7ff1..7fa22a32b 100644 --- a/plugin/builtin/PatchJson6902Transformer.go +++ b/plugin/builtin/PatchJson6902Transformer.go @@ -14,6 +14,7 @@ type PatchJson6902TransformerPlugin struct { Patches []types.PatchJson6902 `json:"patches,omitempty" yaml:"patches,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewPatchJson6902TransformerPlugin() *PatchJson6902TransformerPlugin { return &PatchJson6902TransformerPlugin{} } diff --git a/plugin/builtin/PrefixSuffixTransformer.go b/plugin/builtin/PrefixSuffixTransformer.go index 96fbbbe73..4f7482d0d 100644 --- a/plugin/builtin/PrefixSuffixTransformer.go +++ b/plugin/builtin/PrefixSuffixTransformer.go @@ -7,6 +7,7 @@ import ( "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/ifc" + "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/transformers" "sigs.k8s.io/kustomize/pkg/transformers/config" @@ -20,6 +21,7 @@ type PrefixSuffixTransformerPlugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewPrefixSuffixTransformerPlugin() *PrefixSuffixTransformerPlugin { return &PrefixSuffixTransformerPlugin{} } @@ -50,46 +52,54 @@ func (p *PrefixSuffixTransformerPlugin) Transform(m resmap.ResMap) error { if len(p.Prefix) == 0 && len(p.Suffix) == 0 { return nil } - - // Fill map "mf" with entries subject to name modification, and - // delete these entries from "m", so that for now m retains only - // the entries whose names will not be modified. - mf := resmap.New() - for id, r := range m.AsMap() { - found := false - for _, path := range prefixSuffixFieldSpecsToSkip { - if id.Gvk().IsSelected(&path.Gvk) { - found = true - break - } + for _, r := range m.Resources() { + if p.shouldSkip(r.OrgId()) { + continue } - if !found { - mf.AppendWithId(id, r) - m.Remove(id) + fs, ok := p.shouldInclude(r.OrgId()) + if !ok { + continue } - } - - for id, r := range mf.AsMap() { - objMap := r.Map() - for _, path := range p.FieldSpecs { - if !id.Gvk().IsSelected(&path.Gvk) { - continue - } - err := transformers.MutateField( - objMap, - path.PathSlice(), - path.CreateIfNotPresent, - p.addPrefixSuffix) - if err != nil { - return err - } - newId := id.CopyWithNewPrefixSuffix(p.Prefix, p.Suffix) - m.AppendWithId(newId, r) + if smellsLikeANameChange(fs) { + r.AddNamePrefix(p.Prefix) + r.AddNameSuffix(p.Suffix) + } + err := transformers.MutateField( + r.Map(), + fs.PathSlice(), + fs.CreateIfNotPresent, + p.addPrefixSuffix) + if err != nil { + return err } } return nil } +func smellsLikeANameChange(fs *config.FieldSpec) bool { + return fs.Path == "metadata/name" +} + +func (p *PrefixSuffixTransformerPlugin) shouldInclude( + id resid.ResId) (*config.FieldSpec, bool) { + for _, path := range p.FieldSpecs { + if id.IsSelected(&path.Gvk) { + return &path, true + } + } + return nil, false +} + +func (p *PrefixSuffixTransformerPlugin) shouldSkip( + id resid.ResId) bool { + for _, path := range prefixSuffixFieldSpecsToSkip { + if id.IsSelected(&path.Gvk) { + return true + } + } + return false +} + func (p *PrefixSuffixTransformerPlugin) addPrefixSuffix( in interface{}) (interface{}, error) { s, ok := in.(string) diff --git a/plugin/builtin/ReplicaCountTransformer.go b/plugin/builtin/ReplicaCountTransformer.go index 03628e47e..30a085f4e 100644 --- a/plugin/builtin/ReplicaCountTransformer.go +++ b/plugin/builtin/ReplicaCountTransformer.go @@ -22,6 +22,7 @@ type ReplicaCountTransformerPlugin struct { Replica types.Replica `json:"replica,omitempty" yaml:"replica,omitempty"` } +//noinspection GoUnusedGlobalVariable func NewReplicaCountTransformerPlugin() *ReplicaCountTransformerPlugin { return &ReplicaCountTransformerPlugin{} } @@ -35,11 +36,11 @@ func (p *ReplicaCountTransformerPlugin) Config( func (p *ReplicaCountTransformerPlugin) Transform(m resmap.ResMap) error { matcher := func(r resid.ResId) bool { - return r.ItemId.Name == p.Replica.Name + return r.Name == p.Replica.Name } - for _, id := range m.GetMatchingIds(matcher) { - kMap := m.GetById(id).Map() + for _, r := range m.GetMatchingResourcesByOriginalId(matcher) { + kMap := r.Map() specInterface, ok := kMap[fldSpec] if !ok { diff --git a/plugin/builtin/SecretGenerator.go b/plugin/builtin/SecretGenerator.go index 7cea56c36..9815bb454 100644 --- a/plugin/builtin/SecretGenerator.go +++ b/plugin/builtin/SecretGenerator.go @@ -15,6 +15,7 @@ type SecretGeneratorPlugin struct { types.SecretArgs } +//noinspection GoUnusedGlobalVariable func NewSecretGeneratorPlugin() *SecretGeneratorPlugin { return &SecretGeneratorPlugin{} } diff --git a/plugin/builtin/annotationstransformer/AnnotationsTransformer.go b/plugin/builtin/annotationstransformer/AnnotationsTransformer.go index 03f28acda..d6e2c5ad0 100644 --- a/plugin/builtin/annotationstransformer/AnnotationsTransformer.go +++ b/plugin/builtin/annotationstransformer/AnnotationsTransformer.go @@ -18,6 +18,7 @@ type plugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/builtin/configmapgenerator/ConfigMapGenerator.go b/plugin/builtin/configmapgenerator/ConfigMapGenerator.go index 6664a275f..6a64e0d6b 100644 --- a/plugin/builtin/configmapgenerator/ConfigMapGenerator.go +++ b/plugin/builtin/configmapgenerator/ConfigMapGenerator.go @@ -18,6 +18,7 @@ type plugin struct { types.ConfigMapArgs } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/builtin/hashtransformer/HashTransformer.go b/plugin/builtin/hashtransformer/HashTransformer.go index 7f02b67a9..467699101 100644 --- a/plugin/builtin/hashtransformer/HashTransformer.go +++ b/plugin/builtin/hashtransformer/HashTransformer.go @@ -15,6 +15,7 @@ type plugin struct { hasher ifc.KunstructuredHasher } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/builtin/imagetagtransformer/ImageTagTransformer.go b/plugin/builtin/imagetagtransformer/ImageTagTransformer.go index 62c7726f9..3ac9b48c3 100644 --- a/plugin/builtin/imagetagtransformer/ImageTagTransformer.go +++ b/plugin/builtin/imagetagtransformer/ImageTagTransformer.go @@ -20,6 +20,7 @@ type plugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/builtin/inventorytransformer/InventoryTransformer.go b/plugin/builtin/inventorytransformer/InventoryTransformer.go index 007f3c60f..5badffa1c 100644 --- a/plugin/builtin/inventorytransformer/InventoryTransformer.go +++ b/plugin/builtin/inventorytransformer/InventoryTransformer.go @@ -6,7 +6,6 @@ package main import ( "fmt" - "strings" "sigs.k8s.io/kustomize/pkg/resource" @@ -27,6 +26,7 @@ type plugin struct { Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( @@ -99,32 +99,33 @@ func makeInventory(m resmap.ResMap) ( inv = inventory.NewInventory() var keys []string for _, r := range m.Resources() { - ns := getNamespace(r) - item := resid.NewItemId(r.GetGvk(), ns, r.GetName()) + ns := r.GetNamespace() + item := resid.NewResIdWithNamespace(r.GetGvk(), r.GetName(), ns) if _, ok := inv.Current[item]; ok { return nil, "", fmt.Errorf( "item '%v' already in inventory", item) } - inv.Current[item] = computeRefs(r, m) + inv.Current[item], err = computeRefs(r, m) + if err != nil { + return nil, "", err + } keys = append(keys, item.String()) } h, err := hasher.SortArrayAndComputeHash(keys) return inv, h, err } -func getNamespace(r *resource.Resource) string { - ns, err := r.GetFieldValue("metadata.namespace") - if err != nil && !strings.Contains(err.Error(), "no field named") { - panic(err) - } - return ns -} - -func computeRefs(r *resource.Resource, m resmap.ResMap) (refs []resid.ItemId) { +func computeRefs( + r *resource.Resource, m resmap.ResMap) (refs []resid.ResId, err error) { for _, refid := range r.GetRefBy() { - ref := m.GetById(refid) - ns := getNamespace(ref) - refs = append(refs, resid.NewItemId(ref.GetGvk(), ns, ref.GetName())) + ref, err := m.GetByCurrentId(refid) + if err != nil { + return nil, err + } + refs = append( + refs, + resid.NewResIdWithNamespace( + ref.GetGvk(), ref.GetName(), ref.GetNamespace())) } return } diff --git a/plugin/builtin/labeltransformer/LabelTransformer.go b/plugin/builtin/labeltransformer/LabelTransformer.go index 60e469842..65e8f0bb2 100644 --- a/plugin/builtin/labeltransformer/LabelTransformer.go +++ b/plugin/builtin/labeltransformer/LabelTransformer.go @@ -18,6 +18,7 @@ type plugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/builtin/legacyordertransformer/LegacyOrderTransformer.go b/plugin/builtin/legacyordertransformer/LegacyOrderTransformer.go index 4cb807ddc..a0e9a6fce 100644 --- a/plugin/builtin/legacyordertransformer/LegacyOrderTransformer.go +++ b/plugin/builtin/legacyordertransformer/LegacyOrderTransformer.go @@ -5,6 +5,7 @@ package main import ( + "github.com/pkg/errors" "sigs.k8s.io/kustomize/pkg/ifc" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resource" @@ -18,6 +19,7 @@ import ( // (like ValidatingWebhookConfiguration) last. type plugin struct{} +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin // Nothing needed for configuration. @@ -26,16 +28,19 @@ func (p *plugin) Config( return nil } -func (p *plugin) Transform(m resmap.ResMap) error { +func (p *plugin) Transform(m resmap.ResMap) (err error) { resources := make([]*resource.Resource, m.Size()) ids := m.AllIds() sort.Sort(resmap.IdSlice(ids)) for i, id := range ids { - resources[i] = m.GetById(id) + resources[i], err = m.GetByCurrentId(id) + if err != nil { + return errors.Wrap(err, "expected match for sorting") + } } m.Clear() - for i, id := range ids { - m.AppendWithId(id, resources[i]) + for _, r := range resources { + m.Append(r) } return nil } diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index e990f5700..def43993a 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer.go @@ -18,6 +18,7 @@ type plugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go index 670768b54..1cd0027e4 100644 --- a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go +++ b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go @@ -17,6 +17,7 @@ type plugin struct { Patches []types.PatchJson6902 `json:"patches,omitempty" yaml:"patches,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go index 252ecf753..7024f3f4e 100644 --- a/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go +++ b/plugin/builtin/prefixsuffixtransformer/PrefixSuffixTransformer.go @@ -10,6 +10,7 @@ import ( "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/ifc" + "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/transformers" "sigs.k8s.io/kustomize/pkg/transformers/config" @@ -23,6 +24,7 @@ type plugin struct { FieldSpecs []config.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin // Not placed in a file yet due to lack of demand. @@ -51,46 +53,54 @@ func (p *plugin) Transform(m resmap.ResMap) error { if len(p.Prefix) == 0 && len(p.Suffix) == 0 { return nil } - - // Fill map "mf" with entries subject to name modification, and - // delete these entries from "m", so that for now m retains only - // the entries whose names will not be modified. - mf := resmap.New() - for id, r := range m.AsMap() { - found := false - for _, path := range prefixSuffixFieldSpecsToSkip { - if id.Gvk().IsSelected(&path.Gvk) { - found = true - break - } + for _, r := range m.Resources() { + if p.shouldSkip(r.OrgId()) { + continue } - if !found { - mf.AppendWithId(id, r) - m.Remove(id) + fs, ok := p.shouldInclude(r.OrgId()) + if !ok { + continue } - } - - for id, r := range mf.AsMap() { - objMap := r.Map() - for _, path := range p.FieldSpecs { - if !id.Gvk().IsSelected(&path.Gvk) { - continue - } - err := transformers.MutateField( - objMap, - path.PathSlice(), - path.CreateIfNotPresent, - p.addPrefixSuffix) - if err != nil { - return err - } - newId := id.CopyWithNewPrefixSuffix(p.Prefix, p.Suffix) - m.AppendWithId(newId, r) + if smellsLikeANameChange(fs) { + r.AddNamePrefix(p.Prefix) + r.AddNameSuffix(p.Suffix) + } + err := transformers.MutateField( + r.Map(), + fs.PathSlice(), + fs.CreateIfNotPresent, + p.addPrefixSuffix) + if err != nil { + return err } } return nil } +func smellsLikeANameChange(fs *config.FieldSpec) bool { + return fs.Path == "metadata/name" +} + +func (p *plugin) shouldInclude( + id resid.ResId) (*config.FieldSpec, bool) { + for _, path := range p.FieldSpecs { + if id.IsSelected(&path.Gvk) { + return &path, true + } + } + return nil, false +} + +func (p *plugin) shouldSkip( + id resid.ResId) bool { + for _, path := range prefixSuffixFieldSpecsToSkip { + if id.IsSelected(&path.Gvk) { + return true + } + } + return false +} + func (p *plugin) addPrefixSuffix( in interface{}) (interface{}, error) { s, ok := in.(string) diff --git a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go index f134dbf45..34bd4adf2 100644 --- a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go +++ b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go @@ -25,6 +25,7 @@ type plugin struct { Replica types.Replica `json:"replica,omitempty" yaml:"replica,omitempty"` } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( @@ -36,11 +37,11 @@ func (p *plugin) Config( func (p *plugin) Transform(m resmap.ResMap) error { matcher := func(r resid.ResId) bool { - return r.ItemId.Name == p.Replica.Name + return r.Name == p.Replica.Name } - for _, id := range m.GetMatchingIds(matcher) { - kMap := m.GetById(id).Map() + for _, r := range m.GetMatchingResourcesByOriginalId(matcher) { + kMap := r.Map() specInterface, ok := kMap[fldSpec] if !ok { diff --git a/plugin/builtin/secretgenerator/SecretGenerator.go b/plugin/builtin/secretgenerator/SecretGenerator.go index 6240ce1af..aa955304b 100644 --- a/plugin/builtin/secretgenerator/SecretGenerator.go +++ b/plugin/builtin/secretgenerator/SecretGenerator.go @@ -18,6 +18,7 @@ type plugin struct { types.SecretArgs } +//noinspection GoUnusedGlobalVariable var KustomizePlugin plugin func (p *plugin) Config( diff --git a/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go b/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go index 8765cbcb8..15325e2f8 100644 --- a/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go +++ b/plugin/someteam.example.com/v1/chartinflator/ChartInflator_test.go @@ -42,7 +42,7 @@ kind: Secret metadata: labels: app: release-name-minecraft - chart: minecraft-1.0.1 + chart: minecraft-1.0.3 heritage: Tiller release: release-name name: release-name-minecraft @@ -53,7 +53,7 @@ kind: Service metadata: labels: app: release-name-minecraft - chart: minecraft-1.0.1 + chart: minecraft-1.0.3 heritage: Tiller release: release-name name: release-name-minecraft @@ -74,7 +74,7 @@ metadata: volume.alpha.kubernetes.io/storage-class: default labels: app: release-name-minecraft - chart: minecraft-1.0.1 + chart: minecraft-1.0.3 heritage: Tiller release: release-name name: release-name-minecraft-datadir diff --git a/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer.go b/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer.go index 62348811c..7bc3bddaa 100644 --- a/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer.go +++ b/plugin/someteam.example.com/v1/dateprefixer/DatePrefixer.go @@ -19,6 +19,8 @@ type plugin struct { t transformers.Transformer } +//noinspection GoUnusedGlobalVariable +//nolint: golint var KustomizePlugin plugin func (p *plugin) makePrefixSuffixPluginConfig() ([]byte, error) { diff --git a/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go b/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go index a1fcbbbaa..be5525d10 100644 --- a/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go +++ b/plugin/someteam.example.com/v1/secretsfromdatabase/SecretsFromDatabase.go @@ -21,6 +21,8 @@ type plugin struct { Keys []string `json:"keys,omitempty" yaml:"keys,omitempty"` } +//noinspection GoUnusedGlobalVariable +//nolint: golint var KustomizePlugin plugin var database = map[string]string{ diff --git a/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go b/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go index 221c4db3d..a5c9bc35c 100644 --- a/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go +++ b/plugin/someteam.example.com/v1/someservicegenerator/SomeServiceGenerator.go @@ -19,6 +19,8 @@ type plugin struct { Port string `json:"port,omitempty" yaml:"port,omitempty"` } +//noinspection GoUnusedGlobalVariable +//nolint: golint var KustomizePlugin plugin const tmpl = ` diff --git a/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go b/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go index 8688d8204..8b0cefab4 100644 --- a/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go +++ b/plugin/someteam.example.com/v1/stringprefixer/StringPrefixer.go @@ -24,6 +24,8 @@ type metaData struct { Name string `json:"name,omitempty" yaml:"name,omitempty"` } +//noinspection GoUnusedGlobalVariable +//nolint: golint var KustomizePlugin plugin func (p *plugin) makePrefixSuffixPluginConfig(n string) ([]byte, error) {