diff --git a/k8sdeps/transformer/patch/transformer.go b/k8sdeps/transformer/patch/transformer.go index 201cfb670..23aa3ade0 100644 --- a/k8sdeps/transformer/patch/transformer.go +++ b/k8sdeps/transformer/patch/transformer.go @@ -44,7 +44,7 @@ func (tf *transformer) Transform(baseResourceMap resmap.ResMap) error { } // Strategic merge the resources exist in both base and patches. - for _, patch := range patches { + for _, patch := range patches.Resources() { // Merge patches with base resource. id := patch.Id() matchedIds := baseResourceMap.GetMatchingIds(id.GvknEquals) @@ -55,7 +55,7 @@ func (tf *transformer) Transform(baseResourceMap resmap.ResMap) error { return fmt.Errorf("found multiple objects %#v targeted by patch %#v (ambiguous)", matchedIds, id) } id = matchedIds[0] - base := baseResourceMap[id] + base := baseResourceMap.GetById(id) merged := map[string]interface{}{} versionedObj, err := scheme.Scheme.New(toSchemaGvk(id.Gvk())) baseName := base.GetName() @@ -97,7 +97,7 @@ func (tf *transformer) Transform(baseResourceMap resmap.ResMap) error { return err } } - baseResourceMap[id].SetMap(merged) + baseResourceMap.GetById(id).SetMap(merged) base.SetName(baseName) } return nil @@ -106,12 +106,12 @@ func (tf *transformer) Transform(baseResourceMap resmap.ResMap) error { // mergePatches merge and index patches by Id. // It errors out if there is conflict between patches. func (tf *transformer) mergePatches() (resmap.ResMap, error) { - rc := resmap.ResMap{} + rc := resmap.New() for ix, patch := range tf.patches { id := patch.Id() - existing, found := rc[id] - if !found { - rc[id] = patch + existing := rc.GetById(id) + if existing == nil { + rc.AppendWithId(id, patch) continue } @@ -146,7 +146,7 @@ func (tf *transformer) mergePatches() (resmap.ResMap, error) { if err != nil { return nil, err } - rc[id] = merged + rc.ReplaceResource(id, merged) } return rc, nil } diff --git a/k8sdeps/transformer/patch/transformer_test.go b/k8sdeps/transformer/patch/transformer_test.go index a47d2d25d..370d740c1 100644 --- a/k8sdeps/transformer/patch/transformer_test.go +++ b/k8sdeps/transformer/patch/transformer_test.go @@ -21,7 +21,7 @@ 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.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(deploy, "deploy1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -47,7 +47,7 @@ func TestOverlayRun(t *testing.T) { }, }, }), - } + }) patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", @@ -81,7 +81,7 @@ func TestOverlayRun(t *testing.T) { }, ), } - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(deploy, "deploy1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -114,7 +114,7 @@ func TestOverlayRun(t *testing.T) { }, }, }), - } + }) lt, err := NewTransformer(patch, rf) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -130,7 +130,7 @@ func TestOverlayRun(t *testing.T) { } func TestMultiplePatches(t *testing.T) { - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(deploy, "deploy1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -151,7 +151,7 @@ func TestMultiplePatches(t *testing.T) { }, }, }), - } + }) patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", @@ -209,7 +209,7 @@ func TestMultiplePatches(t *testing.T) { }, ), } - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(deploy, "deploy1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -244,7 +244,7 @@ func TestMultiplePatches(t *testing.T) { }, }, }), - } + }) lt, err := NewTransformer(patch, rf) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -260,7 +260,7 @@ func TestMultiplePatches(t *testing.T) { } func TestMultiplePatchesWithConflict(t *testing.T) { - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(deploy, "deploy1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -281,7 +281,7 @@ func TestMultiplePatchesWithConflict(t *testing.T) { }, }, }), - } + }) patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "apps/v1", @@ -345,7 +345,7 @@ func TestMultiplePatchesWithConflict(t *testing.T) { } func TestNoSchemaOverlayRun(t *testing.T) { - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(foo, "my-foo"): rf.FromMap( map[string]interface{}{ "apiVersion": "example.com/v1", @@ -360,7 +360,7 @@ func TestNoSchemaOverlayRun(t *testing.T) { }, }, }), - } + }) patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "example.com/v1", @@ -377,7 +377,7 @@ func TestNoSchemaOverlayRun(t *testing.T) { }, ), } - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(foo, "my-foo"): rf.FromMap( map[string]interface{}{ "apiVersion": "example.com/v1", @@ -392,7 +392,7 @@ func TestNoSchemaOverlayRun(t *testing.T) { }, }, }), - } + }) lt, err := NewTransformer(patch, rf) if err != nil { @@ -408,7 +408,7 @@ func TestNoSchemaOverlayRun(t *testing.T) { } func TestNoSchemaMultiplePatches(t *testing.T) { - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(foo, "my-foo"): rf.FromMap( map[string]interface{}{ "apiVersion": "example.com/v1", @@ -423,7 +423,7 @@ func TestNoSchemaMultiplePatches(t *testing.T) { }, }, }), - } + }) patch := []*resource.Resource{ rf.FromMap(map[string]interface{}{ "apiVersion": "example.com/v1", @@ -457,7 +457,7 @@ func TestNoSchemaMultiplePatches(t *testing.T) { }, ), } - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(foo, "my-foo"): rf.FromMap( map[string]interface{}{ "apiVersion": "example.com/v1", @@ -476,7 +476,7 @@ func TestNoSchemaMultiplePatches(t *testing.T) { }, }, }), - } + }) lt, err := NewTransformer(patch, rf) if err != nil { @@ -492,7 +492,7 @@ func TestNoSchemaMultiplePatches(t *testing.T) { } func TestNoSchemaMultiplePatchesWithConflict(t *testing.T) { - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(foo, "my-foo"): rf.FromMap( map[string]interface{}{ "apiVersion": "example.com/v1", @@ -507,7 +507,7 @@ func TestNoSchemaMultiplePatchesWithConflict(t *testing.T) { }, }, }), - } + }) 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 48b9526eb..729ccd954 100644 --- a/pkg/accumulator/resaccumulator.go +++ b/pkg/accumulator/resaccumulator.go @@ -26,7 +26,7 @@ type ResAccumulator struct { func MakeEmptyAccumulator() *ResAccumulator { ra := &ResAccumulator{} - ra.resMap = make(resmap.ResMap) + ra.resMap = resmap.New() ra.tConfig = &config.TransformerConfig{} ra.varSet = types.VarSet{} return ra @@ -34,11 +34,7 @@ func MakeEmptyAccumulator() *ResAccumulator { // ResMap returns a copy of the internal resMap. func (ra *ResAccumulator) ResMap() resmap.ResMap { - result := make(resmap.ResMap) - for k, v := range ra.resMap { - result[k] = v - } - return result + return ra.resMap.ShallowCopy() } // Vars returns a copy of underlying vars. @@ -101,7 +97,7 @@ func (ra *ResAccumulator) makeVarReplacementMap() (map[string]string, error) { len(matched), v) } if len(matched) == 1 { - s, err := ra.resMap[matched[0]].GetFieldValue(v.FieldRef.FieldPath) + s, err := ra.resMap.GetById(matched[0]).GetFieldValue(v.FieldRef.FieldPath) if err != nil { return nil, fmt.Errorf( "field specified in var '%v' "+ diff --git a/pkg/accumulator/resaccumulator_test.go b/pkg/accumulator/resaccumulator_test.go index 0f187463b..239521752 100644 --- a/pkg/accumulator/resaccumulator_test.go +++ b/pkg/accumulator/resaccumulator_test.go @@ -41,54 +41,55 @@ func makeResAccumulator() (*ResAccumulator, *resource.Factory, error) { } rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - ra.MergeResourcesWithErrorOnIdCollision(resmap.ResMap{ - 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)", + err = ra.MergeResourcesWithErrorOnIdCollision( + 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)", + }, }, }, }, }, }, - }, - }), - 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, nil + }), + 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) { @@ -172,7 +173,8 @@ func TestResolveVarsVarNeedsDisambiguation(t *testing.T) { if err != nil { t.Fatalf("unexpected err: %v", err) } - ra.MergeResourcesWithErrorOnIdCollision(resmap.ResMap{ + + rm0 := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResIdWithPrefixNamespace( gvk.Gvk{Version: "v1", Kind: "Service"}, "backendOne", "", "fooNamespace"): rf.FromMap( @@ -180,11 +182,17 @@ func TestResolveVarsVarNeedsDisambiguation(t *testing.T) { "apiVersion": "v1", "kind": "Service", "metadata": map[string]interface{}{ - "name": "backendOne", + "name": "backendOne", + "namespace": "fooNamespace", }, }), }) + err = ra.MergeResourcesWithErrorOnIdCollision(rm0) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + err = ra.MergeVars([]types.Var{ { Name: "SERVICE_ONE", @@ -263,7 +271,7 @@ func TestResolveVarsUnmappableVar(t *testing.T) { } func find(name string, resMap resmap.ResMap) *resource.Resource { - for k, v := range resMap { + for k, v := range resMap.AsMap() { if k.Name() == name { return v } diff --git a/pkg/commands/build/build.go b/pkg/commands/build/build.go index 5a954780d..113898363 100644 --- a/pkg/commands/build/build.go +++ b/pkg/commands/build/build.go @@ -188,7 +188,7 @@ func NewCmdBuildPrune( func writeIndividualFiles( fSys fs.FileSystem, folderPath string, m resmap.ResMap) error { - for _, res := range m { + for _, res := range m.Resources() { filename := filepath.Join( folderPath, fmt.Sprintf( diff --git a/pkg/patch/transformer/factory_test.go b/pkg/patch/transformer/factory_test.go index f6855fa89..72b8c7836 100644 --- a/pkg/patch/transformer/factory_test.go +++ b/pkg/patch/transformer/factory_test.go @@ -185,7 +185,7 @@ func TestNewPatchJson6902FactoryMulti(t *testing.T) { } id := resid.NewResId(gvk.FromKind("foo"), "some-name") - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ id: rf.FromMap( map[string]interface{}{ "kind": "foo", @@ -210,8 +210,8 @@ func TestNewPatchJson6902FactoryMulti(t *testing.T) { }, }, }), - } - expected := resmap.ResMap{ + }) + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ id: rf.FromMap( map[string]interface{}{ "kind": "foo", @@ -242,7 +242,7 @@ func TestNewPatchJson6902FactoryMulti(t *testing.T) { }, }, }), - } + }) err = tr.Transform(base) if err != nil { t.Fatalf("unexpected error : %v", err) @@ -299,7 +299,7 @@ func TestNewPatchJson6902FactoryMultiConflict(t *testing.T) { } id := resid.NewResId(gvk.FromKind("foo"), "some-name") - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ id: rf.FromMap( map[string]interface{}{ "kind": "foo", @@ -324,7 +324,7 @@ func TestNewPatchJson6902FactoryMultiConflict(t *testing.T) { }, }, }), - } + }) err = tr.Transform(base) if err == nil { diff --git a/pkg/patch/transformer/patchjson6902json.go b/pkg/patch/transformer/patchjson6902json.go index 9e65a290b..b35c5ad2b 100644 --- a/pkg/patch/transformer/patchjson6902json.go +++ b/pkg/patch/transformer/patchjson6902json.go @@ -109,5 +109,5 @@ func (t *patchJson6902JSONTransformer) findTargetObj( "found multiple targets %v matching %v for json patch", matched, t.target) } - return m[matched[0]], nil + return m.GetById(matched[0]), nil } diff --git a/pkg/patch/transformer/patchjson6902json_test.go b/pkg/patch/transformer/patchjson6902json_test.go index fbad24988..48cb52e8d 100644 --- a/pkg/patch/transformer/patchjson6902json_test.go +++ b/pkg/patch/transformer/patchjson6902json_test.go @@ -34,7 +34,7 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) id := resid.NewResId(deploy, "deploy1") - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ id: rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -60,7 +60,7 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { }, }, }), - } + }) operations := []byte(`[ {"op": "replace", "path": "/spec/template/spec/containers/0/name", "value": "my-nginx"}, @@ -68,7 +68,7 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { {"op": "add", "path": "/spec/template/spec/containers/0/command", "value": ["arg1", "arg2", "arg3"]} ]`) - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ id: rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -100,7 +100,7 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { }, }, }), - } + }) jpt, err := newPatchJson6902JSONTransformer(id, operations) if err != nil { t.Fatalf("unexpected error : %v", err) @@ -119,7 +119,7 @@ func TestJsonPatchJSONTransformer_UnHappyTransform(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) id := resid.NewResId(deploy, "deploy1") - base := resmap.ResMap{ + base := resmap.FromMap(map[resid.ResId]*resource.Resource{ id: rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -145,7 +145,7 @@ func TestJsonPatchJSONTransformer_UnHappyTransform(t *testing.T) { }, }, }), - } + }) operations := []byte(`[ {"op": "add", "path": "/spec/template/spec/containers/0/command/", "value": ["arg1", "arg2", "arg3"]} diff --git a/pkg/plugins/execplugin.go b/pkg/plugins/execplugin.go index 6835be800..3b7bf2690 100644 --- a/pkg/plugins/execplugin.go +++ b/pkg/plugins/execplugin.go @@ -191,8 +191,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(p.rf.RF()) - for id, r := range inputRM { + inputRM := rm.DeepCopy() + for id, r := range inputRM.AsMap() { idString, err := yaml.Marshal(id) if err != nil { return nil, err @@ -216,7 +216,7 @@ func (p *ExecPlugin) updateResMapValues(output []byte, rm resmap.ResMap) error { if err != nil { return err } - for _, r := range outputRM { + for _, r := range outputRM.Resources() { // for each emitted Resource, find the matching Resource in the original ResMap // using its id annotations := r.GetAnnotations() @@ -230,8 +230,8 @@ func (p *ExecPlugin) updateResMapValues(output []byte, rm resmap.ResMap) error { if err != nil { return err } - res, ok := rm[id] - if !ok { + res := rm.GetById(id) + if res == nil { return fmt.Errorf("unable to find id %s in resource map", id.String()) } // remove the annotation set by Kustomize to track the resource diff --git a/pkg/plugins/loader.go b/pkg/plugins/loader.go index 7b58bb61a..cb835d4c2 100644 --- a/pkg/plugins/loader.go +++ b/pkg/plugins/loader.go @@ -35,7 +35,7 @@ func NewLoader( func (l *Loader) LoadGenerators( ldr ifc.Loader, rm resmap.ResMap) ([]transformers.Generator, error) { var result []transformers.Generator - for _, res := range rm { + for _, res := range rm.Resources() { g, err := l.LoadGenerator(ldr, res) if err != nil { return nil, err @@ -61,7 +61,7 @@ func (l *Loader) LoadGenerator( func (l *Loader) LoadTransformers( ldr ifc.Loader, rm resmap.ResMap) ([]transformers.Transformer, error) { var result []transformers.Transformer - for _, res := range rm { + for _, res := range rm.Resources() { t, err := l.LoadTransformer(ldr, res) if err != nil { return nil, err diff --git a/pkg/resid/itemid.go b/pkg/resid/itemid.go index b56d4ed1a..4b042f565 100644 --- a/pkg/resid/itemid.go +++ b/pkg/resid/itemid.go @@ -17,7 +17,6 @@ limitations under the License. package resid import ( - "fmt" "strings" "sigs.k8s.io/kustomize/pkg/gvk" @@ -54,8 +53,10 @@ func (i ItemId) String() string { []string{i.Gvk.String(), ns, nm}, separator) } -func (i ItemId) Equals(b fmt.Stringer) bool { - return i.String() == b.String() +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 { diff --git a/pkg/resid/resid.go b/pkg/resid/resid.go index fa9b9fad8..3e2ecc852 100644 --- a/pkg/resid/resid.go +++ b/pkg/resid/resid.go @@ -113,6 +113,7 @@ func (n ResId) GvknEquals(id ResId) bool { // NsGvknEquals 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) } diff --git a/pkg/resmap/factory.go b/pkg/resmap/factory.go index 54a3c02ea..e578e3a1b 100644 --- a/pkg/resmap/factory.go +++ b/pkg/resmap/factory.go @@ -1,27 +1,13 @@ -/* -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 resmap import ( - "fmt" - "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" ) @@ -41,11 +27,17 @@ func (rmF *Factory) RF() *resource.Factory { return rmF.resF } +func New() ResMap { + return newOne() +} + // FromResource returns a ResMap with one entry. func (rmF *Factory) FromResource(res *resource.Resource) ResMap { - result := ResMap{} - result[res.Id()] = res - return result + m, err := newResMapFromResourceSlice([]*resource.Resource{res}) + if err != nil { + panic(err) + } + return m } // FromFile returns a ResMap given a resource path. @@ -55,26 +47,41 @@ func (rmF *Factory) FromFile( if err != nil { return nil, err } - res, err := rmF.NewResMapFromBytes(content) + m, err := rmF.NewResMapFromBytes(content) if err != nil { return nil, kusterr.Handler(err, path) } - return res, nil + return m, nil } -// newResMapFromBytes decodes a list of objects in byte array format. +// NewResMapFromBytes decodes a list of objects in byte array format. func (rmF *Factory) NewResMapFromBytes(b []byte) (ResMap, error) { resources, err := rmF.resF.SliceFromBytes(b) if err != nil { return nil, err } - result := ResMap{} - for _, res := range resources { - id := res.Id() - if _, found := result[id]; found { - return result, fmt.Errorf("GroupVersionKindName: %#v already exists in the map", id) + return newResMapFromResourceSlice(resources) +} + +// Deprecated. +// FromMap returns a ResMap with arbitrary internal ordering, +// panicing on error. For tests only. +// See also ErrorIfNotEqual. +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 } - result[id] = res } return result, nil } @@ -136,13 +143,12 @@ func (rmF *Factory) FromSecretArgs( } func newResMapFromResourceSlice(resources []*resource.Resource) (ResMap, error) { - result := ResMap{} + result := New() for _, res := range resources { - id := res.Id() - if _, found := result[id]; found { - return nil, fmt.Errorf("duplicated %#v is not allowed", id) + err := result.Append(res) + if err != nil { + return nil, err } - result[id] = res } return result, nil } diff --git a/pkg/resmap/factory_test.go b/pkg/resmap/factory_test.go index 5b2360f95..cb0f7a8e5 100644 --- a/pkg/resmap/factory_test.go +++ b/pkg/resmap/factory_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 resmap_test @@ -21,6 +8,8 @@ import ( "reflect" "testing" + "sigs.k8s.io/kustomize/pkg/resource" + "sigs.k8s.io/kustomize/internal/loadertest" "sigs.k8s.io/kustomize/pkg/fs" "sigs.k8s.io/kustomize/pkg/gvk" @@ -57,14 +46,15 @@ metadata: if ferr := l.AddFile("/whatever/project/deployment.yaml", []byte(resourceStr)); ferr != nil { t.Fatalf("Error adding fake file: %v\n", ferr) } - expected := ResMap{resid.NewResId(deploy, "dply1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "dply1", - }, - }), + expected := FromMap(map[resid.ResId]*resource.Resource{ + resid.NewResId(deploy, "dply1"): rf.FromMap( + map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "dply1", + }, + }), resid.NewResId(deploy, "dply2"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -82,11 +72,10 @@ metadata: "namespace": "test", }, }), - } - + }) m, _ := rmF.FromFile(l, "deployment.yaml") - if len(m) != 3 { - t.Fatalf("%#v should contain 3 appResource, but got %d", m, len(m)) + if m.Size() != 3 { + t.Fatalf("result should contain 3, but got %d", m.Size()) } if err := expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) @@ -104,24 +93,23 @@ kind: ConfigMap metadata: name: cm2 `) - expected := ResMap{ - 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", - }, - }), - } + expected := New() + expected.Append(rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm1", + }, + })) + expected.Append(rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "cm2", + }, + })) m, err := rmF.NewResMapFromBytes(encoded) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -158,7 +146,7 @@ func TestNewFromConfigMaps(t *testing.T) { }, filepath: "/whatever/project/app.env", content: "DB_USERNAME=admin\nDB_PASSWORD=somepw", - expected: ResMap{ + expected: FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "envConfigMap"): rf.FromMapAndOption( map[string]interface{}{ "apiVersion": "v1", @@ -171,7 +159,7 @@ func TestNewFromConfigMaps(t *testing.T) { "DB_PASSWORD": "somepw", }, }, &types.GeneratorArgs{}, nil), - }, + }), }, { @@ -187,7 +175,7 @@ func TestNewFromConfigMaps(t *testing.T) { }, filepath: "/whatever/project/app-init.ini", content: "FOO=bar\nBAR=baz\n", - expected: ResMap{ + expected: FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "fileConfigMap"): rf.FromMapAndOption( map[string]interface{}{ "apiVersion": "v1", @@ -201,7 +189,7 @@ BAR=baz `, }, }, &types.GeneratorArgs{}, nil), - }, + }), }, { description: "construct config map from literal", @@ -215,7 +203,7 @@ BAR=baz }, }, }, - expected: ResMap{ + expected: FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "literalConfigMap"): rf.FromMapAndOption( map[string]interface{}{ "apiVersion": "v1", @@ -230,7 +218,7 @@ BAR=baz "d": "false", }, }, &types.GeneratorArgs{}, nil), - }, + }), }, // TODO: add testcase for data coming from multiple sources like @@ -275,7 +263,7 @@ func TestNewResMapFromSecretArgs(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - expected := ResMap{ + expected := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(secret, "apple"): rf.FromMapAndOption( map[string]interface{}{ "apiVersion": "v1", @@ -289,7 +277,7 @@ func TestNewResMapFromSecretArgs(t *testing.T) { "DB_PASSWORD": base64.StdEncoding.EncodeToString([]byte("somepw")), }, }, &types.GeneratorArgs{}, nil), - } + }) if !reflect.DeepEqual(actual, expected) { t.Fatalf("%#v\ndoesn't match expected:\n%#v", actual, expected) } diff --git a/pkg/resmap/resmap.go b/pkg/resmap/resmap.go index b8ddc09f4..2b8fd6b13 100644 --- a/pkg/resmap/resmap.go +++ b/pkg/resmap/resmap.go @@ -1,44 +1,348 @@ -/* -Copyright 2018 The Kubernetes Authors. +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 -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 resmap implements a map from ResId to Resource that tracks all resources in a kustomization. +// Package resmap implements a map from ResId to Resource that +// tracks all resources in a kustomization. package resmap import ( "bytes" "fmt" - "reflect" "sort" + "github.com/pkg/errors" + "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resource" "sigs.k8s.io/kustomize/pkg/types" "sigs.k8s.io/yaml" ) -// ResMap is a map from ResId to Resource. -type ResMap map[resid.ResId]*resource.Resource +// ResMap is an interface describing operations on the +// core kustomize data structure. +// +// TODO: delete the commentary below when/if the issues +// discussed are addressed. +// +// 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. +// +// The old (bare map) ResMap had pervasive problems: +// +// * 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 + + // Resources provides a discardable slice + // of resource pointers, returned in the order + // as appended. + Resources() []*resource.Resource + + // Append adds a Resource, automatically computing its + // associated Id. + // Error on Id collision. + Append(*resource.Resource) error + + // AppendWithId adds a Resource with the given Id. + // Error on Id collision. + AppendWithId(resid.ResId, *resource.Resource) error + + // AsMap returns ResId, *Resource pairs in + // arbitrary order via a 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 + + // EncodeAsYaml emits the resources as YAML in a byte slice. + // Resources are separated by `---`. + EncodeAsYaml() ([]byte, error) + + // Gets the resource with the given Id, else nil. + GetById(resid.ResId) *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 + + // AllIds returns all known Ids. + // Result order is arbitrary. + AllIds() []resid.ResId + + // GetMatchingIds returns a slice of Ids that + // satisfy the given matcher function. + // Result order is arbitrary. + GetMatchingIds(IdMatcher) []resid.ResId + + // Remove removes the Id and the resource it points to. + Remove(resid.ResId) error + + // ResourcesThatCouldReference returns a new ResMap with + // resources that _might_ reference the resource represented + // by the argument Id, excluding resources that should + // _never_ reference the Id. E.g., if the Id + // refers to a ConfigMap, the returned set may include a + // Deployment from the same namespace and exclude Deployments + // from other namespaces. Cluster wide objects are + // never excluded. + ResourcesThatCouldReference(resid.ResId) ResMap + + // DeepCopy copies the ResMap and underlying resources. + DeepCopy() ResMap + + // ShallowCopy copies the ResMap but + // not the underlying resources. + ShallowCopy() ResMap + + // ErrorIfNotEqual returns an error if the + // argument doesn't have the same Ids and resource + // data 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 + // by definition have random ordering, and will + // fail spuriously. + // TODO: rename to ErrorIfNotEqualSets + // TODO: modify tests to not use resmap.FromMap, + // TODO: - and replace this with a stricter equals. + ErrorIfNotEqual(ResMap) error + + // Debug prints the ResMap. + Debug(title string) +} + +// resWrangler holds the content manipulated by kustomize. +type resWrangler struct { + // Resource list maintained in load (append) order. + // This is important for transformers, which must + // be performed in a specific order, and for users + // who for whatever reasons wish the order they + // 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 { + result := &resWrangler{} + result.rIndex = make(map[resid.ResId]int) + return result +} + +// Size implements ResMap. +func (m *resWrangler) Size() int { + if len(m.rList) != len(m.rIndex) { + panic(errors.New("class invariant violation")) + } + return len(m.rList) +} + +func (m *resWrangler) indexOfResource(other *resource.Resource) int { + for i, r := range m.rList { + if r == other { + return i + } + } + return -1 +} + +// Resources implements ResMap. +func (m *resWrangler) Resources() []*resource.Resource { + tmp := make([]*resource.Resource, len(m.rList)) + copy(tmp, m.rList) + 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) +} + +// 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) + } + } + 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) + } + i := m.indexOfResource(res) + if i >= 0 { + return fmt.Errorf( + "attempt to add res %s that is already held", + res) + } + 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 +} + +// 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++ + } + return +} + +// Debug implements ResMap. +func (m *resWrangler) Debug(title string) { + fmt.Println("--------------------------- " + title) + firstObj := true + for i, r := range m.rList { + if firstObj { + firstObj = false + } else { + fmt.Println("---") + } + fmt.Printf("# %d %s\n", i, m.debugIdMappingToIndex(i)) + blob, err := yaml.Marshal(r.Map()) + if err != nil { + panic(err) + } + fmt.Println(string(blob)) + } +} + +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 returns a slice of ResId keys from the map -// that all satisfy the given matcher function. -func (m ResMap) GetMatchingIds(matches IdMatcher) []resid.ResId { +// GetMatchingIds implements ResMap. +func (m *resWrangler) GetMatchingIds(matches IdMatcher) []resid.ResId { var result []resid.ResId - for id := range m { + for id := range m.rIndex { if matches(id) { result = append(result, id) } @@ -46,19 +350,19 @@ func (m ResMap) GetMatchingIds(matches IdMatcher) []resid.ResId { return result } -// EncodeAsYaml encodes a ResMap to YAML; encoded objects separated by `---`. -func (m ResMap) EncodeAsYaml() ([]byte, error) { - var ids []resid.ResId - for id := range m { - ids = append(ids, id) - } +// EncodeAsYaml implements ResMap. +func (m *resWrangler) EncodeAsYaml() ([]byte, error) { + // TODO: should be able to suppress this sort + // and rely on ordering as specified in the ResMap + // internal rList. + ids := m.AllIds() sort.Sort(IdSlice(ids)) firstObj := true var b []byte buf := bytes.NewBuffer(b) for _, id := range ids { - obj := m[id] + obj := m.GetById(id) out, err := yaml.Marshal(obj.Map()) if err != nil { return nil, err @@ -79,54 +383,80 @@ func (m ResMap) EncodeAsYaml() ([]byte, error) { return buf.Bytes(), nil } -// ErrorIfNotEqual returns error if maps are not equal. -func (m ResMap) ErrorIfNotEqual(m2 ResMap) error { - if len(m) != len(m2) { - var keySet1 []resid.ResId - var keySet2 []resid.ResId - for id := range m { - keySet1 = append(keySet1, id) - } - for id := range m2 { - keySet2 = append(keySet2, id) - } - return fmt.Errorf("maps has different number of entries: %#v doesn't equals %#v", keySet1, keySet2) +// ErrorIfNotEqual implements ResMap. +func (m *resWrangler) ErrorIfNotEqual(other ResMap) error { + m2, ok := other.(*resWrangler) + if !ok { + panic(fmt.Errorf("bad cast to resmapImpl")) } - for id, obj1 := range m { - obj2, found := m2[id] - if !found { - return fmt.Errorf("%#v doesn't exist in %#v", id, m2) + if m.Size() != m2.Size() { + return fmt.Errorf( + "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) } - if !reflect.DeepEqual(obj1, obj2) { - return fmt.Errorf("%#v doesn't deep equal %#v", obj1, obj2) + if !r1.KunstructEqual(r2) { + return fmt.Errorf( + "kuns equal mismatch: \n -- %s,\n -- %s\n\n--\n%#v\n------\n%#v\n", + r1, r2, r1, r2) } } return nil } -// DeepCopy clone the resmap into a new one -func (m ResMap) DeepCopy(rf *resource.Factory) ResMap { - mcopy := make(ResMap) - for id, obj := range m { - mcopy[id] = obj.DeepCopy() - } - return mcopy +type resCopier func(r *resource.Resource) *resource.Resource + +// ShallowCopy implements ResMap. +func (m *resWrangler) ShallowCopy() ResMap { + return m.makeCopy( + func(r *resource.Resource) *resource.Resource { + return r + }) } -// FilterBy returns a subset ResMap containing ResIds with -// the same namespace and leftmost name prefix and rightmost name -// as the inputId. If inputId is a cluster level resource, this -// returns the original ResMap. -func (m ResMap) FilterBy(inputId resid.ResId) ResMap { +// DeepCopy implements ResMap. +func (m *resWrangler) DeepCopy() ResMap { + return m.makeCopy( + func(r *resource.Resource) *resource.Resource { + return r.DeepCopy() + }) +} + +// 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(fmt.Errorf("corrupt index map")) + } + result.rIndex[id] = i + } + return result +} + +// ResourcesThatCouldReference implements ResMap. +func (m *resWrangler) ResourcesThatCouldReference(inputId resid.ResId) ResMap { if inputId.Gvk().IsClusterKind() { return m } - result := ResMap{} - for id, res := range m { + result := New() + for id, i := range m.rIndex { if id.Gvk().IsClusterKind() || id.Namespace() == inputId.Namespace() && id.HasSameLeftmostPrefix(inputId) && id.HasSameRightmostSuffix(inputId) { - result[id] = res + err := result.AppendWithId(id, m.rList[i]) + if err != nil { + panic(err) + } } } return result @@ -136,16 +466,16 @@ func (m ResMap) FilterBy(inputId resid.ResId) ResMap { // key collision and skipping nil maps. // If all of the maps are nil, an empty ResMap is returned. func MergeWithErrorOnIdCollision(maps ...ResMap) (ResMap, error) { - result := ResMap{} + result := New() for _, m := range maps { if m == nil { continue } - for id, res := range m { - if _, found := result[id]; found { - return nil, fmt.Errorf("id '%q' already used", id) + for id, res := range m.AsMap() { + err := result.AppendWithId(id, res) + if err != nil { + return nil, err } - result[id] = res } } return result, nil @@ -161,26 +491,40 @@ func MergeWithErrorOnIdCollision(maps ...ResMap) (ResMap, error) { // resource X is found to be already in the combined map, then the behavior // field for X must be BehaviorMerge or BehaviorReplace. If X is not in the // map, then it's behavior cannot be merge or replace. +// nolint: gocyclo func MergeWithOverride(maps ...ResMap) (ResMap, error) { + if len(maps) == 0 { + return New(), nil + } result := maps[0] if result == nil { - result = ResMap{} + result = New() } for _, m := range maps[1:] { if m == nil { continue } - for id, r := range m { + for id, r := range m.AsMap() { matchedId := result.GetMatchingIds(id.GvknEquals) if len(matchedId) == 1 { id = matchedId[0] + old := result.GetById(id) + if old == nil { + return nil, fmt.Errorf("id lookup failure") + } switch r.Behavior() { case types.BehaviorReplace: - r.Replace(result[id]) - result[id] = r + r.Replace(old) + err := result.ReplaceResource(id, r) + if err != nil { + return nil, err + } case types.BehaviorMerge: - r.Merge(result[id]) - result[id] = r + r.Merge(old) + err := result.ReplaceResource(id, r) + if err != nil { + return nil, err + } default: return nil, fmt.Errorf("id %#v exists; must merge or replace", id) } @@ -189,7 +533,10 @@ func MergeWithOverride(maps ...ResMap) (ResMap, error) { case types.BehaviorMerge, types.BehaviorReplace: return nil, fmt.Errorf("id %#v does not exist; cannot merge or replace", id) default: - result[id] = r + err := result.AppendWithId(id, r) + if err != nil { + return nil, err + } } } else { return nil, fmt.Errorf("merge conflict, found multiple objects %v the Resmap %v can merge into", matchedId, id) diff --git a/pkg/resmap/resmap_test.go b/pkg/resmap/resmap_test.go index 05341cb03..b2498b3a0 100644 --- a/pkg/resmap/resmap_test.go +++ b/pkg/resmap/resmap_test.go @@ -1,22 +1,10 @@ -/* -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 resmap_test import ( + "fmt" "reflect" "testing" @@ -34,6 +22,134 @@ var rf = resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) var rmF = NewFactory(rf) +func doAppend(t *testing.T, w ResMap, r *resource.Resource) { + err := w.Append(r) + if err != nil { + t.Fatalf("append error: %v", err) + } +} +func doRemove(t *testing.T, w ResMap, id resid.ResId) { + err := w.Remove(id) + if err != nil { + t.Fatalf("remove error: %v", err) + } +} + +// Make a resource with a predictable name. +func makeCm(i int) *resource.Resource { + return rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": fmt.Sprintf("cm%03d", i), + }, + }) +} + +func TestAppendRemove(t *testing.T) { + w1 := New() + doAppend(t, w1, makeCm(1)) + doAppend(t, w1, makeCm(2)) + doAppend(t, w1, makeCm(3)) + doAppend(t, w1, makeCm(4)) + 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()) + + w2 := New() + doAppend(t, w2, makeCm(2)) + doAppend(t, w2, makeCm(4)) + doAppend(t, w2, makeCm(6)) + if !reflect.DeepEqual(w1, w1) { + w1.Debug("w1") + w2.Debug("w2") + t.Fatalf("mismatch") + } + + err := w2.Append(makeCm(6)) + if err == nil { + t.Fatalf("expected error") + } +} + +func TestRemove(t *testing.T) { + w := New() + r := makeCm(1) + err := w.Remove(r.Id()) + if err == nil { + t.Fatalf("expected error") + } + err = w.Append(r) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + err = w.Remove(r.Id()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + err = w.Remove(r.Id()) + if err == nil { + t.Fatalf("expected error") + } +} + +func TestReplaceResource(t *testing.T) { + cm5 := makeCm(5) + cm700 := makeCm(700) + cm888 := makeCm(888) + + w := New() + doAppend(t, w, makeCm(1)) + doAppend(t, w, makeCm(2)) + doAppend(t, w, makeCm(3)) + doAppend(t, w, makeCm(4)) + doAppend(t, w, cm5) + doAppend(t, w, makeCm(6)) + doAppend(t, w, makeCm(7)) + + oldSize := w.Size() + err := w.ReplaceResource(cm5.Id(), cm700) + 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 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 { + 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) + } +} + func TestEncodeAsYaml(t *testing.T) { encoded := []byte(`apiVersion: v1 kind: ConfigMap @@ -45,7 +161,7 @@ kind: ConfigMap metadata: name: cm2 `) - input := ResMap{ + input := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -62,7 +178,7 @@ metadata: "name": "cm2", }, }), - } + }) out, err := input.EncodeAsYaml() if err != nil { t.Fatalf("unexpected error: %v", err) @@ -73,7 +189,7 @@ metadata: } func TestDemandOneGvknMatchForId(t *testing.T) { - rm1 := ResMap{ + rm1 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResIdWithPrefixNamespace(cmap, "cm1", "prefix1", "ns1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -90,7 +206,7 @@ func TestDemandOneGvknMatchForId(t *testing.T) { "name": "cm2", }, }), - } + }) result := rm1.GetMatchingIds( resid.NewResIdWithPrefixNamespace(cmap, "cm2", "prefix1", "ns1").GvknEquals) @@ -129,135 +245,147 @@ func TestFilterBy(t *testing.T) { expected ResMap }{ "different namespace": { - resMap: ResMap{resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix", "suffix", "namespace1"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }, + 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: ResMap{}, + expected: New(), }, "different prefix": { - resMap: ResMap{resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix1", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }, + 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: ResMap{}, + expected: New(), }, "different suffix": { - resMap: ResMap{resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map", "prefix", "suffix1", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }, + 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: ResMap{}, + expected: New(), }, "same namespace, same prefix": { - resMap: ResMap{resid.NewResIdWithPrefixNamespace(cmap, "config-map1", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }, + 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: ResMap{resid.NewResIdWithPrefixNamespace(cmap, "config-map1", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }, + 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", + }, + }), + }), }, "same namespace, same suffix": { - resMap: ResMap{resid.NewResIdWithSuffixNamespace(cmap, "config-map1", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }, + 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: ResMap{resid.NewResIdWithSuffixNamespace(cmap, "config-map1", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map1", - }, - }), - }, + 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: ResMap{resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map1", "prefix", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }, + 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: ResMap{resid.NewResIdWithPrefixSuffixNamespace(cmap, "config-map1", "prefix", "suffix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }, + 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: ResMap{resid.NewResIdWithPrefixNamespace(cmap, "config-map", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }, + 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: ResMap{resid.NewResIdWithPrefixNamespace(cmap, "config-map", "prefix", "namespace"): rf.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-map", - }, - }), - }, + 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", + }, + }), + }), }, } for name, test := range tests { test := test t.Run(name, func(t *testing.T) { - got := test.resMap.FilterBy(test.filter) - if !reflect.DeepEqual(test.expected, got) { + got := test.resMap.ResourcesThatCouldReference(test.filter) + err := test.expected.ErrorIfNotEqual(got) + if err != nil { t.Fatalf("Expected %v but got back %v", test.expected, got) } }) @@ -265,7 +393,7 @@ func TestFilterBy(t *testing.T) { } func TestDeepCopy(t *testing.T) { - rm1 := ResMap{ + rm1 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -282,46 +410,63 @@ func TestDeepCopy(t *testing.T) { "name": "cm2", }, }), - } + }) - rm2 := rm1.DeepCopy(rf) + rm2 := rm1.DeepCopy() if &rm1 == &rm2 { t.Fatal("DeepCopy returned a reference to itself instead of a copy") } - - if !reflect.DeepEqual(rm1, rm2) { - t.Fatalf("%v doesn't equal it's deep copy %v", rm1, rm2) + err := rm1.ErrorIfNotEqual(rm1) + if err != nil { + t.Fatal(err) } } func TestGetMatchingIds(t *testing.T) { - // These ids used as map keys. - // They must be different to avoid overwriting - // map entries during construction. - ids := []resid.ResId{ + + m := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId( gvk.Gvk{Kind: "vegetable"}, - "bedlam"), + "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"), + "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"), + "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"), - } - - m := ResMap{} - for _, id := range ids { - // Resources values don't matter in this test. - m[id] = nil - } - if len(m) != len(ids) { - t.Fatalf("unexpected map len %d presumably due to duplicate keys", len(m)) - } + "shatterstar", "p", "happy"): rf.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "whatever4", + }, + }), + }) tests := []struct { name string @@ -362,7 +507,7 @@ func TestGetMatchingIds(t *testing.T) { } func TestErrorIfNotEqual(t *testing.T) { - rm1 := ResMap{ + rm1 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -379,14 +524,14 @@ func TestErrorIfNotEqual(t *testing.T) { "name": "cm2", }, }), - } + }) err := rm1.ErrorIfNotEqual(rm1) if err != nil { t.Fatalf("%v should equal itself %v", rm1, err) } - rm2 := ResMap{ + rm2 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -395,7 +540,7 @@ func TestErrorIfNotEqual(t *testing.T) { "name": "cm1", }, }), - } + }) // test the different number of keys path err = rm1.ErrorIfNotEqual(rm2) @@ -403,7 +548,7 @@ func TestErrorIfNotEqual(t *testing.T) { t.Fatalf("%v should not equal %v %v", rm1, rm2, err) } - rm3 := ResMap{ + rm3 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm2"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -412,7 +557,7 @@ func TestErrorIfNotEqual(t *testing.T) { "name": "cm1", }, }), - } + }) // test the different key values path err = rm2.ErrorIfNotEqual(rm3) @@ -420,7 +565,7 @@ func TestErrorIfNotEqual(t *testing.T) { t.Fatalf("%v should not equal %v %v", rm1, rm2, err) } - rm4 := ResMap{ + rm4 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -429,7 +574,7 @@ func TestErrorIfNotEqual(t *testing.T) { "name": "cm3", }, }), - } + }) // test the deepcopy path err = rm2.ErrorIfNotEqual(rm4) @@ -440,7 +585,7 @@ func TestErrorIfNotEqual(t *testing.T) { } func TestMergeWithoutOverride(t *testing.T) { - input1 := ResMap{ + input1 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(deploy, "deploy1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -449,8 +594,8 @@ func TestMergeWithoutOverride(t *testing.T) { "name": "foo-deploy1", }, }), - } - input2 := ResMap{ + }) + input2 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(statefulset, "stateful1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -459,9 +604,9 @@ func TestMergeWithoutOverride(t *testing.T) { "name": "bar-stateful", }, }), - } + }) input := []ResMap{input1, input2} - expected := ResMap{ + expected := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(deploy, "deploy1"): rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", @@ -478,12 +623,13 @@ func TestMergeWithoutOverride(t *testing.T) { "name": "bar-stateful", }, }), - } + }) merged, err := MergeWithErrorOnIdCollision(input...) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(merged, expected) { + err = expected.ErrorIfNotEqual(merged) + if err != nil { t.Fatalf("%#v doesn't equal expected %#v", merged, expected) } input3 := []ResMap{merged, nil} @@ -491,21 +637,23 @@ func TestMergeWithoutOverride(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(merged1, expected) { - t.Fatalf("%#v doesn't equal expected %#v", merged1, expected) + err = expected.ErrorIfNotEqual(merged1) + if err != nil { + t.Fatal(err) } input4 := []ResMap{nil, merged} merged2, err := MergeWithErrorOnIdCollision(input4...) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(merged2, expected) { - t.Fatalf("%#v doesn't equal expected %#v", merged2, expected) + err = expected.ErrorIfNotEqual(merged2) + if err != nil { + t.Fatal(err) } } func generateMergeFixtures(b types.GenerationBehavior) []ResMap { - input1 := ResMap{ + input1 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cmap"): rf.FromMapAndOption( map[string]interface{}{ "apiVersion": "apps/v1", @@ -520,8 +668,8 @@ func generateMergeFixtures(b types.GenerationBehavior) []ResMap { }, &types.GeneratorArgs{ Behavior: "create", }, nil), - } - input2 := ResMap{ + }) + input2 := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cmap"): rf.FromMapAndOption( map[string]interface{}{ "apiVersion": "apps/v1", @@ -537,12 +685,12 @@ func generateMergeFixtures(b types.GenerationBehavior) []ResMap { }, &types.GeneratorArgs{ Behavior: b.String(), }, nil), - } + }) return []ResMap{input1, input2} } func TestMergeWithOverride(t *testing.T) { - expected := ResMap{ + expected := FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cmap"): rf.FromMapAndOption( map[string]interface{}{ "apiVersion": "apps/v1", @@ -560,41 +708,43 @@ func TestMergeWithOverride(t *testing.T) { }, &types.GeneratorArgs{ Behavior: "create", }, nil), - } + }) merged, err := MergeWithOverride(generateMergeFixtures(types.BehaviorMerge)...) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(merged, expected) { - t.Fatalf("%#v doesn't equal expected %#v", merged, expected) + err = expected.ErrorIfNotEqual(merged) + if err != nil { + t.Fatal(err) } input3 := []ResMap{merged, nil} merged1, err := MergeWithOverride(input3...) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(merged1, expected) { - t.Fatalf("%#v doesn't equal expected %#v", merged1, expected) + err = expected.ErrorIfNotEqual(merged1) + if err != nil { + t.Fatal(err) } input4 := []ResMap{nil, merged} merged2, err := MergeWithOverride(input4...) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(merged2, expected) { - t.Fatalf("%#v doesn't equal expected %#v", merged2, expected) + err = expected.ErrorIfNotEqual(merged2) + if err != nil { + t.Fatal(err) } - inputs := generateMergeFixtures(types.BehaviorReplace) replaced, err := MergeWithOverride(inputs...) if err != nil { t.Fatalf("unexpected error: %v", err) } expectedReplaced := inputs[1] - if !reflect.DeepEqual(replaced, expectedReplaced) { - t.Fatalf("%#v doesn't equal expected %#v", replaced, expectedReplaced) + err = expectedReplaced.ErrorIfNotEqual(replaced) + if err != nil { + t.Fatal(err) } - _, err = MergeWithOverride(generateMergeFixtures(types.BehaviorUnspecified)...) if err == nil { t.Fatal("Merging with GenerationBehavior BehaviorUnspecified should return an error but does not") diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index 919268040..b95abc410 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -18,6 +18,7 @@ limitations under the License. package resource import ( + "reflect" "strings" "sigs.k8s.io/kustomize/pkg/ifc" @@ -34,6 +35,10 @@ type Resource struct { refBy []resid.ResId } +func (r *Resource) KunstructEqual(o *Resource) bool { + return reflect.DeepEqual(r.Kunstructured, o.Kunstructured) +} + // String returns resource as JSON. func (r *Resource) String() string { bs, err := r.MarshalJSON() diff --git a/pkg/target/baseandoverlaymedium_test.go b/pkg/target/baseandoverlaymedium_test.go index 9507e6d51..67d5d9543 100644 --- a/pkg/target/baseandoverlaymedium_test.go +++ b/pkg/target/baseandoverlaymedium_test.go @@ -22,21 +22,6 @@ import ( "sigs.k8s.io/kustomize/pkg/kusttest" ) -// TODO(monopole): Add a feature test example covering secret generation. - -// WARNING: These tests use a fake file system, and any attempt to use a -// feature that spawns shells will fail, because said shells expect a working -// directory corresponding to a real directory on disk - see -// these lines in secretfactory.go: -// cmd := exec.CommandContext(ctx, commands[0], commands[1:]...) -// cmd.Dir = f.wd -// Worse, the fake directory might match a real directory on the your system, -// making the failure less obvious (and maybe hurting something if your secret -// generation technique writes data to disk). So no use of secret generation -// in these particular tests. -// To eventually fix this, we could write the data to a real filesystem, and -// clean up after, or use some other trick compatible with exec. - func writeMediumBase(th *kusttest_test.KustTestHarness) { th.WriteK("/app/base", ` namePrefix: baseprefix- diff --git a/pkg/target/kusttarget_test.go b/pkg/target/kusttarget_test.go index 032bf56c8..18eeee31c 100644 --- a/pkg/target/kusttarget_test.go +++ b/pkg/target/kusttarget_test.go @@ -93,7 +93,7 @@ func TestResources(t *testing.T) { th.WriteF("/whatever/namespace.yaml", namespaceContent) th.WriteF("/whatever/jsonpatch.json", jsonpatchContent) - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResIdWithPrefixSuffixNamespace( gvk.Gvk{Group: "apps", Version: "v1", Kind: "Deployment"}, "dply1", "foo-", "-bar", "ns1"): th.FromMap( @@ -192,14 +192,13 @@ func TestResources(t *testing.T) { }, }, }), - } + }) actual, err := th.MakeKustTarget().MakeCustomizedResMap() if err != nil { t.Fatalf("unexpected Resources error %v", err) } - if !reflect.DeepEqual(actual, expected) { - err = expected.ErrorIfNotEqual(actual) + if err = expected.ErrorIfNotEqual(actual); err != nil { t.Fatalf("unexpected inequality: %v", err) } } @@ -229,7 +228,7 @@ func TestResourceNotFound(t *testing.T) { } func findSecret(m resmap.ResMap) *resource.Resource { - for id, res := range m { + for id, res := range m.AsMap() { if id.Gvk().Kind == "Secret" { return res } diff --git a/pkg/target/variableref_test.go b/pkg/target/variableref_test.go index 74b647110..6d111a442 100644 --- a/pkg/target/variableref_test.go +++ b/pkg/target/variableref_test.go @@ -720,7 +720,7 @@ spec: `) } -func TestVariableRefMounthPath(t *testing.T) { +func TestVariableRefMountPath(t *testing.T) { th := kusttest_test.NewKustTestHarness(t, "/app/base") th.WriteK("/app/base", ` resources: diff --git a/pkg/transformers/image.go b/pkg/transformers/image.go index d2e6f39c6..847b6e0b1 100644 --- a/pkg/transformers/image.go +++ b/pkg/transformers/image.go @@ -44,19 +44,18 @@ func (pt *imageTransformer) Transform(m resmap.ResMap) error { if len(pt.images) == 0 { return nil } - for id := range m { - objMap := m[id].Map() + for _, r := range m.Resources() { for _, path := range pt.fieldSpecs { - if !id.Gvk().IsSelected(&path.Gvk) { + if !r.Id().Gvk().IsSelected(&path.Gvk) { continue } - err := mutateField(objMap, path.PathSlice(), false, pt.mutateImage) + err := mutateField(r.Map(), path.PathSlice(), false, pt.mutateImage) if err != nil { return err } } // Kept for backward compatibility - if err := pt.findAndReplaceImage(objMap); err != nil && id.Gvk().Kind != `CustomResourceDefinition` { + if err := pt.findAndReplaceImage(r.Map()); err != nil && r.Id().Kind != `CustomResourceDefinition` { return err } } diff --git a/pkg/transformers/labelsandannotations.go b/pkg/transformers/labelsandannotations.go index 10c515a44..836db2e48 100644 --- a/pkg/transformers/labelsandannotations.go +++ b/pkg/transformers/labelsandannotations.go @@ -59,13 +59,14 @@ func NewMapTransformer( // Transform apply each pair in the mapTransformer to the // fields specified in mapTransformer. func (o *mapTransformer) Transform(m resmap.ResMap) error { - for id := range m { - objMap := m[id].Map() + for _, r := range m.Resources() { for _, path := range o.fieldSpecs { - if !id.Gvk().IsSelected(&path.Gvk) { + if !r.Id().Gvk().IsSelected(&path.Gvk) { continue } - err := mutateField(objMap, path.PathSlice(), path.CreateIfNotPresent, o.addMap) + err := mutateField( + r.Map(), path.PathSlice(), + path.CreateIfNotPresent, o.addMap) if err != nil { return err } diff --git a/pkg/transformers/labelsandannotations_test.go b/pkg/transformers/labelsandannotations_test.go index ae2503b6a..79f2aaa96 100644 --- a/pkg/transformers/labelsandannotations_test.go +++ b/pkg/transformers/labelsandannotations_test.go @@ -17,7 +17,6 @@ limitations under the License. package transformers import ( - "reflect" "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" @@ -34,7 +33,7 @@ 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 statefulset = gvk.Gvk{Group: "apps", Version: "v1", Kind: "StatefulSet"} -var crd = gvk.Gvk{Group: "apiwctensions.k8s.io", Version: "v1beta1", Kind: "CustomResourceDefinition"} +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"} @@ -47,7 +46,7 @@ var rf = resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()) var defaultTransformerConfig = config.MakeDefaultConfig() func TestLabelsRun(t *testing.T) { - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -198,8 +197,8 @@ func TestLabelsRun(t *testing.T) { }, }, }), - } - expected := resmap.ResMap{ + }) + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -430,7 +429,7 @@ func TestLabelsRun(t *testing.T) { }, }, }), - } + }) lt, err := NewLabelsMapTransformer( map[string]string{"label-key1": "label-value1", "label-key2": "label-value2"}, defaultTransformerConfig.CommonLabels) @@ -441,14 +440,13 @@ func TestLabelsRun(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } func TestAnnotationsRun(t *testing.T) { - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -499,8 +497,8 @@ func TestAnnotationsRun(t *testing.T) { }, }, }), - } - expected := resmap.ResMap{ + }) + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -567,7 +565,7 @@ func TestAnnotationsRun(t *testing.T) { }, }, }), - } + }) at, err := NewAnnotationsMapTransformer( map[string]string{"anno-key1": "anno-value1", "anno-key2": "anno-value2"}, defaultTransformerConfig.CommonAnnotations) @@ -578,14 +576,13 @@ func TestAnnotationsRun(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } func TestAnnotaionsRunWithNullValue(t *testing.T) { - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -595,9 +592,9 @@ func TestAnnotaionsRunWithNullValue(t *testing.T) { "annotations": nil, }, }), - } + }) - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -610,7 +607,7 @@ func TestAnnotaionsRunWithNullValue(t *testing.T) { }, }, }), - } + }) at, err := NewAnnotationsMapTransformer( map[string]string{"anno-key1": "anno-value1", "anno-key2": "anno-value2"}, @@ -622,8 +619,7 @@ func TestAnnotaionsRunWithNullValue(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(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 d5921d1a9..ed30c168a 100644 --- a/pkg/transformers/multitransformer.go +++ b/pkg/transformers/multitransformer.go @@ -18,8 +18,6 @@ package transformers import ( "fmt" - "sigs.k8s.io/kustomize/pkg/resource" - "sigs.k8s.io/kustomize/pkg/resmap" ) @@ -27,7 +25,6 @@ import ( type multiTransformer struct { transformers []Transformer checkConflictEnabled bool - rf *resource.Factory } var _ Transformer = &multiTransformer{} @@ -71,7 +68,7 @@ func (o *multiTransformer) transform(m resmap.ResMap) error { // A spot check to perform when the transformations are supposed to be commutative. // Fail if there's a difference in the result. func (o *multiTransformer) transformWithCheckConflict(m resmap.ResMap) error { - mcopy := m.DeepCopy(o.rf) + mcopy := m.DeepCopy() err := o.transform(m) if err != nil { return err diff --git a/pkg/transformers/namereference.go b/pkg/transformers/namereference.go index 8081ddeed..318d506ef 100644 --- a/pkg/transformers/namereference.go +++ b/pkg/transformers/namereference.go @@ -61,17 +61,18 @@ func NewNameReferenceTransformer(br []config.NameBackReferences) Transformer { // body of the resource object (the value in the ResMap). func (o *nameReferenceTransformer) Transform(m resmap.ResMap) error { // TODO: Too much looping. - // Even more hidden loops in FilterBy, + // Even more hidden loops in ResourcesThatCouldReference, // updateNameReference and FindByGVKN. - for id := range m { + for id, r := range m.AsMap() { for _, backRef := range o.backRefs { for _, fSpec := range backRef.FieldSpecs { if id.Gvk().IsSelected(&fSpec.Gvk) { err := mutateField( - m[id].Map(), fSpec.PathSlice(), + r.Map(), + fSpec.PathSlice(), fSpec.CreateIfNotPresent, o.updateNameReference( - id, backRef.Gvk, m.FilterBy(id))) + id, backRef.Gvk, m.ResourcesThatCouldReference(id))) if err != nil { return err } @@ -88,7 +89,7 @@ func (o *nameReferenceTransformer) updateNameReference( switch in.(type) { case string: s, _ := in.(string) - for id, res := range m { + for id, res := range m.AsMap() { if id.Gvk().IsSelected(&backRef) && id.Name() == s { matchedIds := m.GetMatchingIds(id.GvknEquals) // If there's more than one match, there's no way @@ -114,7 +115,7 @@ func (o *nameReferenceTransformer) updateNameReference( } names = append(names, name) } - for id, res := range m { + for id, res := range m.AsMap() { indexes := indexOf(id.Name(), names) if id.Gvk().IsSelected(&backRef) && len(indexes) > 0 { matchedIds := m.GetMatchingIds(id.GvknEquals) diff --git a/pkg/transformers/namereference_test.go b/pkg/transformers/namereference_test.go index c5535fec9..70ab1c090 100644 --- a/pkg/transformers/namereference_test.go +++ b/pkg/transformers/namereference_test.go @@ -17,7 +17,6 @@ limitations under the License. package transformers import ( - "reflect" "strings" "testing" @@ -30,7 +29,7 @@ import ( func TestNameReferenceHappyRun(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -278,14 +277,11 @@ func TestNameReferenceHappyRun(t *testing.T) { }, }, }), - } + }) - expected := resmap.ResMap{} - for k, v := range m { - expected[k] = v - } + expected := m.ShallowCopy() - expected[resid.NewResId(deploy, "deploy1")] = rf.FromMap( + expected.ReplaceResource(resid.NewResId(deploy, "deploy1"), rf.FromMap( map[string]interface{}{ "group": "apps", "apiVersion": "v1", @@ -365,8 +361,8 @@ func TestNameReferenceHappyRun(t *testing.T) { }, }, }, - }) - expected[resid.NewResId(statefulset, "statefulset1")] = rf.FromMap( + })) + expected.ReplaceResource(resid.NewResId(statefulset, "statefulset1"), rf.FromMap( map[string]interface{}{ "group": "apps", "apiVersion": "v1", @@ -398,8 +394,8 @@ func TestNameReferenceHappyRun(t *testing.T) { }, }, }, - }) - expected[resid.NewResId(ingress, "ingress1")] = rf.FromMap( + })) + expected.ReplaceResource(resid.NewResId(ingress, "ingress1"), rf.FromMap( map[string]interface{}{ "group": "extensions", "apiVersion": "v1beta1", @@ -418,8 +414,8 @@ func TestNameReferenceHappyRun(t *testing.T) { }, }, }, - ) - expected[resid.NewResId(crb, "crb")] = rf.FromMap( + )) + expected.ReplaceResource(resid.NewResId(crb, "crb"), rf.FromMap( map[string]interface{}{ "apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRoleBinding", @@ -433,8 +429,8 @@ func TestNameReferenceHappyRun(t *testing.T) { "namespace": "test", }, }, - }) - expected[resid.NewResId(cr, "cr")] = rf.FromMap( + })) + expected.ReplaceResource(resid.NewResId(cr, "cr"), rf.FromMap( map[string]interface{}{ "apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRole", @@ -453,8 +449,8 @@ func TestNameReferenceHappyRun(t *testing.T) { }, }, }, - }) - expected[resid.NewResId(cronjob, "cronjob1")] = rf.FromMap( + })) + expected.ReplaceResource(resid.NewResId(cronjob, "cronjob1"), rf.FromMap( map[string]interface{}{ "apiVersion": "batch/v1beta1", "kind": "CronJob", @@ -490,14 +486,13 @@ func TestNameReferenceHappyRun(t *testing.T) { }, }, }, - }) + })) nrt := NewNameReferenceTransformer(defaultTransformerConfig.NameReference) err := nrt.Transform(m) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } @@ -510,7 +505,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { expectedErr string }{ { - resMap: resmap.ResMap{ + resMap: resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cr, "cr"): rf.FromMap( map[string]interface{}{ "apiVersion": "rbac.authorization.k8s.io/v1", @@ -529,9 +524,9 @@ func TestNameReferenceUnhappyRun(t *testing.T) { }, }, }), - }, + }), expectedErr: "is expected to be string"}, - {resMap: resmap.ResMap{ + {resMap: resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cr, "cr"): rf.FromMap( map[string]interface{}{ "apiVersion": "rbac.authorization.k8s.io/v1", @@ -550,7 +545,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { }, }, }), - }, + }), expectedErr: "is expected to be either a string or a []interface{}"}, } @@ -571,7 +566,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(pv, "volume1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -593,9 +588,9 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "volumeName": "volume1", }, }), - } + }) - expected := resmap.ResMap{ + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(pv, "volume1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -617,15 +612,14 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "volumeName": "someprefix-volume1", }, }), - } - expected[resid.NewResId(pv, "volume1")].AppendRefBy(resid.NewResId(pvc, "claim1")) + }) + expected.GetById(resid.NewResId(pv, "volume1")).AppendRefBy(resid.NewResId(pvc, "claim1")) nrt := NewNameReferenceTransformer(defaultTransformerConfig.NameReference) err := nrt.Transform(m) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } diff --git a/pkg/transformers/namespace.go b/pkg/transformers/namespace.go index 6b06bf636..0f66aba86 100644 --- a/pkg/transformers/namespace.go +++ b/pkg/transformers/namespace.go @@ -50,8 +50,8 @@ func NewNamespaceTransformer(ns string, cf []config.FieldSpec) Transformer { func (o *namespaceTransformer) Transform(m resmap.ResMap) error { mf := o.filterResmap(m) - for id := range mf { - objMap := mf[id].Map() + 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 @@ -88,9 +88,9 @@ func (o *namespaceTransformer) Transform(m resmap.ResMap) error { if !id.Gvk().IsClusterKind() { newid := id.CopyWithNewNamespace(o.namespace) - m[newid] = mf[id] + m.AppendWithId(newid, res) } else { - m[id] = mf[id] + m.AppendWithId(id, res) } } } @@ -99,19 +99,19 @@ func (o *namespaceTransformer) Transform(m resmap.ResMap) error { } func (o *namespaceTransformer) filterResmap(m resmap.ResMap) resmap.ResMap { - mf := resmap.ResMap{} - for id := range m { + 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[id] = m[id] - delete(m, id) + mf.AppendWithId(id, res) + m.Remove(id) } } if !found { - mf[id] = m[id] - delete(m, id) + mf.AppendWithId(id, res) + m.Remove(id) } } return mf @@ -119,17 +119,17 @@ func (o *namespaceTransformer) filterResmap(m resmap.ResMap) resmap.ResMap { func (o *namespaceTransformer) updateClusterRoleBinding(m resmap.ResMap) { saMap := map[string]bool{} - for id := range m { + for id := range m.AsMap() { if id.Gvk().Equals(gvk.Gvk{Version: "v1", Kind: "ServiceAccount"}) { saMap[id.Name()] = true } } - for id := range m { + for id, res := range m.AsMap() { if id.Gvk().Kind != "ClusterRoleBinding" && id.Gvk().Kind != "RoleBinding" { continue } - objMap := m[id].Map() + objMap := res.Map() subjects, ok := objMap["subjects"].([]interface{}) if subjects == nil || !ok { continue diff --git a/pkg/transformers/namespace_test.go b/pkg/transformers/namespace_test.go index 34beed5d0..df9483319 100644 --- a/pkg/transformers/namespace_test.go +++ b/pkg/transformers/namespace_test.go @@ -17,7 +17,6 @@ limitations under the License. package transformers import ( - "reflect" "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" @@ -29,7 +28,7 @@ import ( func TestNamespaceRun(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -109,8 +108,8 @@ func TestNamespaceRun(t *testing.T) { "name": "crd", }, }), - } - expected := resmap.ResMap{ + }) + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResIdWithPrefixNamespace(ns, "ns1", "", ""): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -191,15 +190,14 @@ func TestNamespaceRun(t *testing.T) { "name": "crd", }, }), - } + }) nst := NewNamespaceTransformer("test", defaultTransformerConfig.NameSpace) err := nst.Transform(m) if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } @@ -207,7 +205,7 @@ func TestNamespaceRun(t *testing.T) { func TestNamespaceRunForClusterLevelKind(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(ns, "ns1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -245,9 +243,9 @@ func TestNamespaceRunForClusterLevelKind(t *testing.T) { }, "subjects": []interface{}{}, }), - } + }) - expected := m.DeepCopy(rf) + expected := m.DeepCopy() nst := NewNamespaceTransformer("test", defaultTransformerConfig.NameSpace) @@ -255,8 +253,7 @@ func TestNamespaceRunForClusterLevelKind(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } } diff --git a/pkg/transformers/prefixsuffixname.go b/pkg/transformers/prefixsuffixname.go index c4ca85f53..d74da3c89 100644 --- a/pkg/transformers/prefixsuffixname.go +++ b/pkg/transformers/prefixsuffixname.go @@ -59,12 +59,14 @@ func NewNamePrefixSuffixTransformer( } // Transform prepends the name prefix and appends the name suffix. +// TODO: this transformer breaks internal +// ordering and depends on Id hackery. Rewrite completely. func (o *namePrefixSuffixTransformer) Transform(m resmap.ResMap) error { // 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.ResMap{} - for id := range m { + mf := resmap.New() + for id, r := range m.AsMap() { found := false for _, path := range o.fieldSpecsToSkip { if id.Gvk().IsSelected(&path.Gvk) { @@ -73,13 +75,13 @@ func (o *namePrefixSuffixTransformer) Transform(m resmap.ResMap) error { } } if !found { - mf[id] = m[id] - delete(m, id) + mf.AppendWithId(id, r) + m.Remove(id) } } - for id := range mf { - objMap := mf[id].Map() + for id, r := range mf.AsMap() { + objMap := r.Map() for _, path := range o.fieldSpecsToUse { if !id.Gvk().IsSelected(&path.Gvk) { continue @@ -93,7 +95,7 @@ func (o *namePrefixSuffixTransformer) Transform(m resmap.ResMap) error { return err } newId := id.CopyWithNewPrefixSuffix(o.prefix, o.suffix) - m[newId] = mf[id] + m.AppendWithId(newId, r) } } return nil diff --git a/pkg/transformers/prefixsuffixname_test.go b/pkg/transformers/prefixsuffixname_test.go index d9fb9366c..2b1cb1653 100644 --- a/pkg/transformers/prefixsuffixname_test.go +++ b/pkg/transformers/prefixsuffixname_test.go @@ -17,7 +17,6 @@ limitations under the License. package transformers import ( - "reflect" "testing" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" @@ -29,7 +28,7 @@ import ( func TestPrefixSuffixNameRun(t *testing.T) { rf := resource.NewFactory( kunstruct.NewKunstructuredFactoryImpl()) - m := resmap.ResMap{ + m := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -54,8 +53,8 @@ func TestPrefixSuffixNameRun(t *testing.T) { "name": "crd", }, }), - } - expected := resmap.ResMap{ + }) + expected := resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResIdWithPrefixSuffix(cmap, "cm1", "someprefix-", "-somesuffix"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -80,7 +79,7 @@ func TestPrefixSuffixNameRun(t *testing.T) { "name": "crd", }, }), - } + }) npst, err := NewNamePrefixSuffixTransformer( "someprefix-", "-somesuffix", defaultTransformerConfig.NamePrefix) @@ -91,8 +90,7 @@ func TestPrefixSuffixNameRun(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !reflect.DeepEqual(m, expected) { - err = expected.ErrorIfNotEqual(m) + if err = expected.ErrorIfNotEqual(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 b985a1393..87dda4eb1 100644 --- a/pkg/transformers/refvars.go +++ b/pkg/transformers/refvars.go @@ -95,9 +95,9 @@ func (rv *RefVarTransformer) Transform(m resmap.ResMap) error { rv.replacementCounts = make(map[string]int) rv.mappingFunc = expansion.MappingFuncFor( rv.replacementCounts, rv.varMap) - for id, res := range m { + for _, res := range m.Resources() { for _, fieldSpec := range rv.fieldSpecs { - if id.Gvk().IsSelected(&fieldSpec.Gvk) { + if res.Id().Gvk().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 4ec703810..63b62c85a 100644 --- a/pkg/transformers/refvars_test.go +++ b/pkg/transformers/refvars_test.go @@ -22,6 +22,7 @@ import ( "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" ) @@ -50,7 +51,7 @@ func TestVarRef(t *testing.T) { fs: []config.FieldSpec{ {Gvk: cmap, Path: "data"}, }, - res: resmap.ResMap{ + res: resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -63,10 +64,10 @@ func TestVarRef(t *testing.T) { "item2": "bla", }, }), - }, + }), }, expected: expected{ - res: resmap.ResMap{ + res: resmap.FromMap(map[resid.ResId]*resource.Resource{ resid.NewResId(cmap, "cm1"): rf.FromMap( map[string]interface{}{ "apiVersion": "v1", @@ -79,7 +80,7 @@ func TestVarRef(t *testing.T) { "item2": "bla", }, }), - }, + }), unused: []string{"BAR"}, }, }, diff --git a/plugin/builtin/HashTransformer.go b/plugin/builtin/HashTransformer.go index ab1374e39..cd14463f2 100644 --- a/plugin/builtin/HashTransformer.go +++ b/plugin/builtin/HashTransformer.go @@ -24,7 +24,7 @@ func (p *HashTransformerPlugin) Config( // Transform appends hash to generated resources. func (p *HashTransformerPlugin) Transform(m resmap.ResMap) error { - for _, res := range m { + for _, res := range m.Resources() { if res.NeedHashSuffix() { h, err := p.hasher.Hash(res) if err != nil { diff --git a/plugin/builtin/InventoryTransformer.go b/plugin/builtin/InventoryTransformer.go index 8b17c4b28..ebf4c62aa 100644 --- a/plugin/builtin/InventoryTransformer.go +++ b/plugin/builtin/InventoryTransformer.go @@ -86,25 +86,18 @@ func (p *InventoryTransformerPlugin) Transform(m resmap.ResMap) error { } if p.Policy == types.GarbageCollect.String() { - for byeBye := range m { - delete(m, byeBye) + for _, byeBye := range m.AllIds() { + m.Remove(byeBye) } } - - id := cm.Id() - if _, ok := m[id]; ok { - return fmt.Errorf( - "id '%v' already used; use a different name", id) - } - m[id] = cm - return nil + return m.Append(cm) } func makeInventory(m resmap.ResMap) ( inv *inventory.Inventory, hash string, err error) { inv = inventory.NewInventory() var keys []string - for _, r := range m { + for _, r := range m.Resources() { ns := getNamespace(r) item := resid.NewItemId(r.GetGvk(), ns, r.GetName()) if _, ok := inv.Current[item]; ok { @@ -128,7 +121,7 @@ func getNamespace(r *resource.Resource) string { func computeRefs(r *resource.Resource, m resmap.ResMap) (refs []resid.ItemId) { for _, refid := range r.GetRefBy() { - ref := m[refid] + ref := m.GetById(refid) ns := getNamespace(ref) refs = append(refs, resid.NewItemId(ref.GetGvk(), ns, ref.GetName())) } diff --git a/plugin/builtin/ReplicaCountTransformer.go b/plugin/builtin/ReplicaCountTransformer.go index 018079189..03628e47e 100644 --- a/plugin/builtin/ReplicaCountTransformer.go +++ b/plugin/builtin/ReplicaCountTransformer.go @@ -38,8 +38,8 @@ func (p *ReplicaCountTransformerPlugin) Transform(m resmap.ResMap) error { return r.ItemId.Name == p.Replica.Name } - for _, r := range m.GetMatchingIds(matcher) { - kMap := m[r].Map() + for _, id := range m.GetMatchingIds(matcher) { + kMap := m.GetById(id).Map() specInterface, ok := kMap[fldSpec] if !ok { diff --git a/plugin/builtin/hashtransformer/HashTransformer.go b/plugin/builtin/hashtransformer/HashTransformer.go index 44ce49044..7f02b67a9 100644 --- a/plugin/builtin/hashtransformer/HashTransformer.go +++ b/plugin/builtin/hashtransformer/HashTransformer.go @@ -25,7 +25,7 @@ func (p *plugin) Config( // Transform appends hash to generated resources. func (p *plugin) Transform(m resmap.ResMap) error { - for _, res := range m { + for _, res := range m.Resources() { if res.NeedHashSuffix() { h, err := p.hasher.Hash(res) if err != nil { diff --git a/plugin/builtin/inventorytransformer/InventoryTransformer.go b/plugin/builtin/inventorytransformer/InventoryTransformer.go index 593650491..007f3c60f 100644 --- a/plugin/builtin/inventorytransformer/InventoryTransformer.go +++ b/plugin/builtin/inventorytransformer/InventoryTransformer.go @@ -87,25 +87,18 @@ func (p *plugin) Transform(m resmap.ResMap) error { } if p.Policy == types.GarbageCollect.String() { - for byeBye := range m { - delete(m, byeBye) + for _, byeBye := range m.AllIds() { + m.Remove(byeBye) } } - - id := cm.Id() - if _, ok := m[id]; ok { - return fmt.Errorf( - "id '%v' already used; use a different name", id) - } - m[id] = cm - return nil + return m.Append(cm) } func makeInventory(m resmap.ResMap) ( inv *inventory.Inventory, hash string, err error) { inv = inventory.NewInventory() var keys []string - for _, r := range m { + for _, r := range m.Resources() { ns := getNamespace(r) item := resid.NewItemId(r.GetGvk(), ns, r.GetName()) if _, ok := inv.Current[item]; ok { @@ -129,7 +122,7 @@ func getNamespace(r *resource.Resource) string { func computeRefs(r *resource.Resource, m resmap.ResMap) (refs []resid.ItemId) { for _, refid := range r.GetRefBy() { - ref := m[refid] + ref := m.GetById(refid) ns := getNamespace(ref) refs = append(refs, resid.NewItemId(ref.GetGvk(), ns, ref.GetName())) } diff --git a/plugin/builtin/inventorytransformer/InventoryTransformer_test.go b/plugin/builtin/inventorytransformer/InventoryTransformer_test.go index 0e6bf1961..d6f0d08ac 100644 --- a/plugin/builtin/inventorytransformer/InventoryTransformer_test.go +++ b/plugin/builtin/inventorytransformer/InventoryTransformer_test.go @@ -122,4 +122,3 @@ namespace: default th.AssertActualEqualsExpected(rm, inv+"---"+content) } - diff --git a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go index d50fc2af6..f134dbf45 100644 --- a/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go +++ b/plugin/builtin/replicacounttransformer/ReplicaCountTransformer.go @@ -39,8 +39,8 @@ func (p *plugin) Transform(m resmap.ResMap) error { return r.ItemId.Name == p.Replica.Name } - for _, r := range m.GetMatchingIds(matcher) { - kMap := m[r].Map() + for _, id := range m.GetMatchingIds(matcher) { + kMap := m.GetById(id).Map() specInterface, ok := kMap[fldSpec] if !ok {