Simplify code base.

- In ResMap, drop concept of internal Id to Resource
   map.  The ResMap is now (just) a list, allowing only
   very particular edits.

 - Resources should now be maintained in the order
   loaded.  A later PR can adjust tests to remove the
   internal legacy sorting, and confirm order-out is
   predictable from order-in.  The PR would suppress
   the sort in tests, and reorder the output to make
   all tests pass again, and confirm that the new order
   matched depth-first input traversal.  The FromMap
   fixture function was removed from all test inputs to
   establish a predictable input order.

 - Resources now have two 'Ids', OriginalId and
   CurrentId.  The former is fixed as
   GVK-name-namespace at load time, the latter changes
   during transformations.  The latter can be used to
   narrow name references when the former maps to
   multiple resources.  We allow bases to be loaded
   more than once in a build (a diamond pattern), so
   the OriginalId is not unique across the resources
   set.  The CurrentId is (and must be) unique, but is
   constantly mutating.  Failing to make this
   distinction clear, and attempting to maintain a
   mapping from a single mutating Id to a resource was
   making the code too complex.

 - Drop prefix/suffix from ResId - the ResId is now
   immutable.  A later PR can remove the distinction
   with ItemId.

 - This PR increases coverage of ResMap is since this
   is a large refactor.  Higher level tests didn't need
   much change outside reordering of results at the
   resource level.
This commit is contained in:
Jeffrey Regan
2019-06-12 11:29:57 -07:00
parent 624aa5290e
commit 3a01a63a01
75 changed files with 2481 additions and 2962 deletions

View File

@@ -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(

View File

@@ -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
}

View File

@@ -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",