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