Remove dead merge conflict code.

This commit is contained in:
monopole
2021-03-17 06:35:30 -07:00
parent eb48b1b718
commit 6f6d41f17f
20 changed files with 29 additions and 604 deletions

View File

@@ -45,15 +45,6 @@ func (p *PatchStrategicMergeTransformerPlugin) Config(
return fmt.Errorf( return fmt.Errorf(
"patch appears to be empty; files=%v, Patch=%s", p.Paths, p.Patches) "patch appears to be empty; files=%v, Patch=%s", p.Paths, p.Patches)
} }
// TODO(#3723): Delete conflict detection.
// Since #1500 closed, the conflict detector in use doesn't do
// anything useful. The resmap returned by this method hasn't
// been used for many releases. Leaving code as a comment to
// aid in deletion (fixing #3723).
// _, err = h.ResmapFactory().ConflatePatches(p.loadedPatches)
// if err != nil {
// return err
// }
return nil return nil
} }

View File

@@ -233,7 +233,7 @@ map:
} }
tc.filter.Referrer = referrer tc.filter.Referrer = referrer
resMapFactory := resmap.NewFactory(factory, nil) resMapFactory := resmap.NewFactory(factory)
candidatesRes, err := factory.SliceFromBytesWithNames( candidatesRes, err := factory.SliceFromBytesWithNames(
tc.originalNames, []byte(tc.candidates)) tc.originalNames, []byte(tc.candidates))
if err != nil { if err != nil {
@@ -334,7 +334,7 @@ metadata:
} }
tc.filter.Referrer = referrer tc.filter.Referrer = referrer
resMapFactory := resmap.NewFactory(factory, nil) resMapFactory := resmap.NewFactory(factory)
candidatesRes, err := factory.SliceFromBytesWithNames( candidatesRes, err := factory.SliceFromBytesWithNames(
tc.originalNames, []byte(tc.candidates)) tc.originalNames, []byte(tc.candidates))
if err != nil { if err != nil {
@@ -751,7 +751,7 @@ ref:
} }
tc.filter.Referrer = referrer tc.filter.Referrer = referrer
resMapFactory := resmap.NewFactory(factory, nil) resMapFactory := resmap.NewFactory(factory)
candidatesRes, err := factory.SliceFromBytesWithNames( candidatesRes, err := factory.SliceFromBytesWithNames(
tc.originalNames, []byte(tc.candidates)) tc.originalNames, []byte(tc.candidates))
if err != nil { if err != nil {

View File

@@ -1,23 +0,0 @@
// Copyright 2020 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0
package conflict
import (
"sigs.k8s.io/kustomize/api/resid"
"sigs.k8s.io/kustomize/api/resource"
)
type cdFactory struct{}
var _ resource.ConflictDetectorFactory = &cdFactory{}
// NewFactory returns a new conflict detector factory.
func NewFactory() resource.ConflictDetectorFactory {
return &cdFactory{}
}
// New returns an instance of smPatchMergeOnlyDetector.
func (c cdFactory) New(_ resid.Gvk) (resource.ConflictDetector, error) {
return &smPatchMergeOnlyDetector{}, nil
}

View File

@@ -1,33 +0,0 @@
// Copyright 2020 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0
package conflict
import (
"sigs.k8s.io/kustomize/api/resource"
)
// smPatchMergeOnlyDetector ignores conflicts,
// but does real strategic merge patching.
// This is part of an effort to eliminate dependence on
// apimachinery package to allow kustomize integration
// into kubectl (#2506 and #1500)
type smPatchMergeOnlyDetector struct{}
var _ resource.ConflictDetector = &smPatchMergeOnlyDetector{}
func (c *smPatchMergeOnlyDetector) HasConflict(
_, _ *resource.Resource) (bool, error) {
return false, nil
}
// There's at least one case that doesn't work. Suppose one has a
// Deployment with a volume with the bizarre "emptyDir: {}" entry.
// If you want to get rid of this entry via a patch containing
// the entry "emptyDir: null", then the following won't work,
// because null entries are eliminated.
func (c *smPatchMergeOnlyDetector) MergePatches(
r, patch *resource.Resource) (*resource.Resource, error) {
err := r.ApplySmPatch(patch)
return r, err
}

View File

@@ -32,8 +32,7 @@ s/$BAR/bar baz/g
t.Fatal(err) t.Fatal(err)
} }
pvd := provider.NewDefaultDepProvider() pvd := provider.NewDefaultDepProvider()
rf := resmap.NewFactory( rf := resmap.NewFactory(pvd.GetResourceFactory())
pvd.GetResourceFactory(), pvd.GetConflictDetectorFactory())
pluginConfig := rf.RF().FromMap( pluginConfig := rf.RF().FromMap(
map[string]interface{}{ map[string]interface{}{
"apiVersion": "someteam.example.com/v1", "apiVersion": "someteam.example.com/v1",

View File

@@ -51,8 +51,7 @@ func TestLoader(t *testing.T) {
BuildGoPlugin("someteam.example.com", "v1", "SomeServiceGenerator") BuildGoPlugin("someteam.example.com", "v1", "SomeServiceGenerator")
defer th.Reset() defer th.Reset()
p := provider.NewDefaultDepProvider() p := provider.NewDefaultDepProvider()
rmF := resmap.NewFactory( rmF := resmap.NewFactory(p.GetResourceFactory())
p.GetResourceFactory(), p.GetConflictDetectorFactory())
fLdr, err := loader.NewLoader( fLdr, err := loader.NewLoader(
loader.RestrictionRootOnly, loader.RestrictionRootOnly,
filesys.Separator, filesys.MakeFsInMemory()) filesys.Separator, filesys.MakeFsInMemory())

View File

@@ -36,8 +36,7 @@ func makeKustTargetWithRf(
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
rf := resmap.NewFactory( rf := resmap.NewFactory(pvd.GetResourceFactory())
pvd.GetResourceFactory(), pvd.GetConflictDetectorFactory())
pc := konfig.DisabledPluginConfig() pc := konfig.DisabledPluginConfig()
return target.NewKustTarget( return target.NewKustTarget(
ldr, ldr,

View File

@@ -4,15 +4,12 @@
package target package target
import ( import (
"fmt"
"sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resmap"
) )
// multiTransformer contains a list of transformers. // multiTransformer contains a list of transformers.
type multiTransformer struct { type multiTransformer struct {
transformers []resmap.Transformer transformers []resmap.Transformer
checkConflictEnabled bool
} }
var _ resmap.Transformer = &multiTransformer{} var _ resmap.Transformer = &multiTransformer{}
@@ -21,7 +18,7 @@ var _ resmap.Transformer = &multiTransformer{}
func newMultiTransformer(t []resmap.Transformer) resmap.Transformer { func newMultiTransformer(t []resmap.Transformer) resmap.Transformer {
r := &multiTransformer{ r := &multiTransformer{
transformers: make([]resmap.Transformer, len(t)), transformers: make([]resmap.Transformer, len(t)),
checkConflictEnabled: false} }
copy(r.transformers, t) copy(r.transformers, t)
return r return r
} }
@@ -29,13 +26,6 @@ func newMultiTransformer(t []resmap.Transformer) resmap.Transformer {
// Transform applies the member transformers in order to the resources, // Transform applies the member transformers in order to the resources,
// optionally detecting and erroring on commutation conflict. // optionally detecting and erroring on commutation conflict.
func (o *multiTransformer) Transform(m resmap.ResMap) error { func (o *multiTransformer) Transform(m resmap.ResMap) error {
if o.checkConflictEnabled {
return o.transformWithCheckConflict(m)
}
return o.transform(m)
}
func (o *multiTransformer) transform(m resmap.ResMap) error {
for _, t := range o.transformers { for _, t := range o.transformers {
if err := t.Transform(m); err != nil { if err := t.Transform(m); err != nil {
return err return err
@@ -44,30 +34,3 @@ func (o *multiTransformer) transform(m resmap.ResMap) error {
} }
return nil return nil
} }
// Of the len(o.transformers)! possible transformer orderings, compare to a reversed order.
// A spot check to perform when the transformations are supposed to be commutative.
// Fail if there's a difference in the result.
func (o *multiTransformer) transformWithCheckConflict(m resmap.ResMap) error {
mcopy := m.DeepCopy()
err := o.transform(m)
if err != nil {
return err
}
o.reverseTransformers()
err = o.transform(mcopy)
if err != nil {
return err
}
err = m.ErrorIfNotEqualSets(mcopy)
if err != nil {
return fmt.Errorf("found conflict between different patches\n%v", err)
}
return nil
}
func (o *multiTransformer) reverseTransformers() {
for i, j := 0, len(o.transformers)-1; i < j; i, j = i+1, j-1 {
o.transformers[i], o.transformers[j] = o.transformers[j], o.transformers[i]
}
}

View File

@@ -52,9 +52,7 @@ func MakeKustomizer(o *Options) *Kustomizer {
// and Run can be called on each of them). // and Run can be called on each of them).
func (b *Kustomizer) Run( func (b *Kustomizer) Run(
fSys filesys.FileSystem, path string) (resmap.ResMap, error) { fSys filesys.FileSystem, path string) (resmap.ResMap, error) {
resmapFactory := resmap.NewFactory( resmapFactory := resmap.NewFactory(b.depProvider.GetResourceFactory())
b.depProvider.GetResourceFactory(),
b.depProvider.GetConflictDetectorFactory())
lr := fLdr.RestrictionNone lr := fLdr.RestrictionNone
if b.options.LoadRestrictions == types.LoadRestrictionsRootOnly { if b.options.LoadRestrictions == types.LoadRestrictionsRootOnly {
lr = fLdr.RestrictionRootOnly lr = fLdr.RestrictionRootOnly

View File

@@ -706,7 +706,7 @@ metadata:
`) `)
} }
func TestMultiplePatchesWithConflict(t *testing.T) { func TestNonCommutablePatches(t *testing.T) {
th := kusttest_test.MakeHarness(t) th := kusttest_test.MakeHarness(t)
makeCommonFilesForMultiplePatchTests(th) makeCommonFilesForMultiplePatchTests(th)
th.WriteF("overlay/staging/deployment-patch1.yaml", ` th.WriteF("overlay/staging/deployment-patch1.yaml", `

View File

@@ -6,42 +6,18 @@ package provider
import ( import (
"sigs.k8s.io/kustomize/api/hasher" "sigs.k8s.io/kustomize/api/hasher"
"sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/internal/conflict"
"sigs.k8s.io/kustomize/api/internal/validate" "sigs.k8s.io/kustomize/api/internal/validate"
"sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/resource"
) )
// DepProvider is a dependency provider, injecting different // DepProvider is a dependency provider, injecting different
// implementations depending on the context. // implementations depending on the context.
// type DepProvider struct {
// Notes on interfaces: resourceFactory *resource.Factory
//
// - resource.ConflictDetector
//
// implemented by api/internal/conflict.smPatchMergeOnlyDetector
//
// At time of writing, this doesn't report conflicts,
// but it does know how to merge patches. Conflict
// reporting isn't vital to kustomize function. It's
// rare that a person would configure one transformer
// with many patches, much less so many that it became
// hard to spot conflicts. In the case of an undetected
// conflict, the last patch applied wins, likely what
// the user wants anyway. Regardless, the effect of this
// is plainly visible and usable in the output, even if
// a conflict happened but wasn't reported as an error.
//
// - ifc.Validator
//
// implemented by api/internal/validate.FieldValidator // implemented by api/internal/validate.FieldValidator
//
// See TODO inside the validator for status. // See TODO inside the validator for status.
// At time of writing, this is a do-nothing // At time of writing, this is a do-nothing
// validator as it's not critical to kustomize function. // validator as it's not critical to kustomize function.
//
type DepProvider struct {
resourceFactory *resource.Factory
conflictDectectorFactory resource.ConflictDetectorFactory
fieldValidator ifc.Validator fieldValidator ifc.Validator
} }
@@ -49,7 +25,6 @@ func NewDepProvider() *DepProvider {
rf := resource.NewFactory(&hasher.Hasher{}) rf := resource.NewFactory(&hasher.Hasher{})
return &DepProvider{ return &DepProvider{
resourceFactory: rf, resourceFactory: rf,
conflictDectectorFactory: conflict.NewFactory(),
fieldValidator: validate.NewFieldValidator(), fieldValidator: validate.NewFieldValidator(),
} }
} }
@@ -62,10 +37,6 @@ func (dp *DepProvider) GetResourceFactory() *resource.Factory {
return dp.resourceFactory return dp.resourceFactory
} }
func (dp *DepProvider) GetConflictDetectorFactory() resource.ConflictDetectorFactory {
return dp.conflictDectectorFactory
}
func (dp *DepProvider) GetFieldValidator() ifc.Validator { func (dp *DepProvider) GetFieldValidator() ifc.Validator {
return dp.fieldValidator return dp.fieldValidator
} }

View File

@@ -16,14 +16,11 @@ import (
type Factory struct { type Factory struct {
// Makes resources. // Makes resources.
resF *resource.Factory resF *resource.Factory
// Makes ConflictDetectors.
cdf resource.ConflictDetectorFactory
} }
// NewFactory returns a new resmap.Factory. // NewFactory returns a new resmap.Factory.
func NewFactory( func NewFactory(rf *resource.Factory) *Factory {
rf *resource.Factory, cdf resource.ConflictDetectorFactory) *Factory { return &Factory{resF: rf}
return &Factory{resF: rf, cdf: cdf}
} }
// RF returns a resource.Factory. // RF returns a resource.Factory.
@@ -126,13 +123,6 @@ func (rmF *Factory) FromSecretArgs(
return rmF.FromResource(res), nil return rmF.FromResource(res), nil
} }
// ConflatePatches creates a new ResMap containing a merger of the
// incoming patches.
// Error if conflict found.
func (rmF *Factory) ConflatePatches(patches []*resource.Resource) (ResMap, error) {
return (&merginator{cdf: rmF.cdf}).ConflatePatches(patches)
}
func newResMapFromResourceSlice( func newResMapFromResourceSlice(
resources []*resource.Resource) (ResMap, error) { resources []*resource.Resource) (ResMap, error) {
result := New() result := New()

View File

@@ -13,7 +13,6 @@ import (
"sigs.k8s.io/kustomize/api/kv" "sigs.k8s.io/kustomize/api/kv"
"sigs.k8s.io/kustomize/api/loader" "sigs.k8s.io/kustomize/api/loader"
. "sigs.k8s.io/kustomize/api/resmap" . "sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/resource"
resmaptest_test "sigs.k8s.io/kustomize/api/testutils/resmaptest" resmaptest_test "sigs.k8s.io/kustomize/api/testutils/resmaptest"
valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest" valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest"
"sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/api/types"
@@ -350,56 +349,3 @@ metadata:
}) })
} }
} }
func TestConflatePatches_Empty(t *testing.T) {
rm, err := rmF.ConflatePatches([]*resource.Resource{})
assert.NoError(t, err)
assert.Equal(t, 0, rm.Size())
}
func TestConflatePatches(t *testing.T) {
var (
err error
yml []byte
r1, r2 *resource.Resource
)
r1, err = rf.FromBytes([]byte(`apiVersion: example.com/v1
kind: Foo
metadata:
name: my-foo
spec:
bar:
B:
C: Z
`))
assert.NoError(t, err)
r2, err = rf.FromBytes([]byte(`apiVersion: example.com/v1
kind: Foo
metadata:
name: my-foo
spec:
bar:
C: Z
D: W
baz:
hello: world
`))
assert.NoError(t, err)
rm, err := rmF.ConflatePatches([]*resource.Resource{r1, r2})
assert.NoError(t, err)
yml, err = rm.AsYaml()
assert.NoError(t, err)
assert.Equal(t, `apiVersion: example.com/v1
kind: Foo
metadata:
name: my-foo
spec:
bar:
C: Z
D: W
baz:
hello: world
`, string(yml))
}

View File

@@ -1,123 +0,0 @@
// Copyright 2020 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0
package resmap
import (
"fmt"
"sigs.k8s.io/kustomize/api/resource"
)
// merginator coordinates merging the resources in incoming to the result.
type merginator struct {
incoming []*resource.Resource
cdf resource.ConflictDetectorFactory
result ResMap
}
func (m *merginator) ConflatePatches(in []*resource.Resource) (ResMap, error) {
m.result = New()
m.incoming = in
for index := range m.incoming {
alreadyInResult, err := m.appendIfNoMatch(index)
if err != nil {
return nil, err
}
if alreadyInResult != nil {
// The resource at index has the same resId as a previously
// considered resource.
//
// If they conflict with each other (e.g. they both want to change
// the image name in a Deployment, but to different values),
// return an error.
//
// If they don't conflict, then merge them into a single resource,
// since they both target the same item, and we want cumulative
// behavior. E.g. say both patches modify a map. Without a merge,
// the last patch wins, replacing the entire map.
err = m.mergeWithExisting(index, alreadyInResult)
if err != nil {
return nil, err
}
}
}
return m.result, nil
}
func (m *merginator) appendIfNoMatch(index int) (*resource.Resource, error) {
candidate := m.incoming[index]
matchedResources := m.result.GetMatchingResourcesByAnyId(
candidate.OrgId().Equals)
if len(matchedResources) == 0 {
m.result.Append(candidate)
return nil, nil
}
if len(matchedResources) > 1 {
return nil, fmt.Errorf("multiple resources targeted by patch")
}
return matchedResources[0], nil
}
func (m *merginator) mergeWithExisting(
index int, alreadyInResult *resource.Resource) error {
candidate := m.incoming[index]
cd, err := m.cdf.New(candidate.OrgId().Gvk)
if err != nil {
return err
}
hasConflict, err := cd.HasConflict(candidate, alreadyInResult)
if err != nil {
return err
}
if hasConflict {
return m.makeError(cd, index)
}
merged, err := cd.MergePatches(alreadyInResult, candidate)
if err != nil {
return err
}
_, err = m.result.Replace(merged)
return err
}
// Make an error message describing the conflict.
func (m *merginator) makeError(cd resource.ConflictDetector, index int) error {
conflict, err := m.findConflict(cd, index)
if err != nil {
return err
}
if conflict == nil {
return fmt.Errorf("expected conflict for %s", m.incoming[index].OrgId())
}
conflictMap, _ := conflict.Map()
incomingIndexMap, _ := m.incoming[index].Map()
return fmt.Errorf(
"conflict between %#v at index %d and %#v",
incomingIndexMap,
index,
conflictMap,
)
}
// findConflict looks for a conflict in a resource slice.
// It returns the first conflict between the resource at index
// and some other resource. Two resources can only conflict if
// they have the same original ResId.
func (m *merginator) findConflict(
cd resource.ConflictDetector, index int) (*resource.Resource, error) {
targetId := m.incoming[index].OrgId()
for i, p := range m.incoming {
if i == index || !targetId.Equals(p.OrgId()) {
continue
}
conflict, err := cd.HasConflict(p, m.incoming[index])
if err != nil {
return nil, err
}
if conflict {
return p, nil
}
}
return nil, nil
}

View File

@@ -21,7 +21,7 @@ import (
var depProvider = provider.NewDefaultDepProvider() var depProvider = provider.NewDefaultDepProvider()
var rf = depProvider.GetResourceFactory() var rf = depProvider.GetResourceFactory()
var rmF = NewFactory(rf, depProvider.GetConflictDetectorFactory()) var rmF = NewFactory(rf)
func doAppend(t *testing.T, w ResMap, r *resource.Resource) { func doAppend(t *testing.T, w ResMap, r *resource.Resource) {
err := w.Append(r) err := w.Append(r)

View File

@@ -1,20 +0,0 @@
// Copyright 2019 The Kubernetes Authors.
// SPDX-License-Identifier: Apache-2.0
package resource
import "sigs.k8s.io/kustomize/api/resid"
// ConflictDetector detects conflicts between resources.
type ConflictDetector interface {
// HasConflict returns true if the given resources have a conflict.
HasConflict(patch1, patch2 *Resource) (bool, error)
// Merge two resources into one.
MergePatches(patch1, patch2 *Resource) (*Resource, error)
}
// ConflictDetectorFactory makes instances of ConflictDetector that know
// how to handle the given Group, Version, Kind tuple.
type ConflictDetectorFactory interface {
New(gvk resid.Gvk) (ConflictDetector, error)
}

View File

@@ -47,8 +47,7 @@ func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced {
} }
p := provider.NewDefaultDepProvider() p := provider.NewDefaultDepProvider()
resourceFactory := p.GetResourceFactory() resourceFactory := p.GetResourceFactory()
resmapFactory := resmap.NewFactory( resmapFactory := resmap.NewFactory(resourceFactory)
resourceFactory, p.GetConflictDetectorFactory())
result := &HarnessEnhanced{ result := &HarnessEnhanced{
Harness: MakeHarness(t), Harness: MakeHarness(t),

View File

@@ -15,8 +15,7 @@ import (
func main() { func main() {
var plugin resmap.Configurable var plugin resmap.Configurable
p := provider.NewDefaultDepProvider() p := provider.NewDefaultDepProvider()
resmapFactory := resmap.NewFactory( resmapFactory := resmap.NewFactory(p.GetResourceFactory())
p.GetResourceFactory(), p.GetConflictDetectorFactory())
pluginHelpers := resmap.NewPluginHelpers( pluginHelpers := resmap.NewPluginHelpers(
nil, p.GetFieldValidator(), resmapFactory) nil, p.GetFieldValidator(), resmapFactory)

View File

@@ -49,15 +49,6 @@ func (p *plugin) Config(
return fmt.Errorf( return fmt.Errorf(
"patch appears to be empty; files=%v, Patch=%s", p.Paths, p.Patches) "patch appears to be empty; files=%v, Patch=%s", p.Paths, p.Patches)
} }
// TODO(#3723): Delete conflict detection.
// Since #1500 closed, the conflict detector in use doesn't do
// anything useful. The resmap returned by this method hasn't
// been used for many releases. Leaving code as a comment to
// aid in deletion (fixing #3723).
// _, err = h.ResmapFactory().ConflatePatches(p.loadedPatches)
// if err != nil {
// return err
// }
return nil return nil
} }

View File

@@ -12,9 +12,6 @@ import (
kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
) )
// TODO(#3304): DECISION - OK to move to kyaml and not do conflict detection.
const skipConflictDetectionTests = true
func errorContains(err error, possibilities ...string) bool { func errorContains(err error, possibilities ...string) bool {
for _, x := range possibilities { for _, x := range possibilities {
if strings.Contains(err.Error(), x) { if strings.Contains(err.Error(), x) {
@@ -317,60 +314,6 @@ spec:
`) `)
} }
func TestStrategicMergeTransformerMultiplePatchesWithConflicts(t *testing.T) {
if skipConflictDetectionTests {
t.Skip("Skipping patch merge conflict tests.")
}
th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("PatchStrategicMergeTransformer")
defer th.Reset()
th.WriteF("patch1.yaml", `
apiVersion: apps/v1
metadata:
name: myDeploy
kind: Deployment
spec:
template:
spec:
containers:
- name: nginx
image: nginx:latest
env:
- name: SOMEENV
value: BAR
`)
th.WriteF("patch2.yaml", `
apiVersion: apps/v1
metadata:
name: myDeploy
kind: Deployment
spec:
template:
spec:
containers:
- name: nginx
image: nginx:1.7.9
env:
- name: ANOTHERENV
value: HELLO
- name: busybox
image: busybox
`)
_, err := th.RunTransformer(`
apiVersion: builtin
kind: PatchStrategicMergeTransformer
metadata:
name: notImportantHere
paths:
- patch1.yaml
- patch2.yaml
`, target)
if assert.Error(t, err) && !errorContains(err, "conflict") {
t.Fatalf("expected error to contain %q but get %v", "conflict", err)
}
}
func TestStrategicMergeTransformerWrongNamespace(t *testing.T) { func TestStrategicMergeTransformerWrongNamespace(t *testing.T) {
th := kusttest_test.MakeEnhancedHarness(t). th := kusttest_test.MakeEnhancedHarness(t).
@@ -558,7 +501,7 @@ spec:
`) `)
} }
func TestStrategicMergeTransformerNoSchemaMultiPatchesNoConflict(t *testing.T) { func TestStrategicMergeTransformerNoSchemaMultiPatches(t *testing.T) {
th := kusttest_test.MakeEnhancedHarness(t). th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("PatchStrategicMergeTransformer") PrepBuiltin("PatchStrategicMergeTransformer")
defer th.Reset() defer th.Reset()
@@ -652,47 +595,6 @@ spec:
`) `)
} }
func TestStrategicMergeTransformerNoSchemaMultiPatchesWithConflict(t *testing.T) {
if skipConflictDetectionTests {
t.Skip("Skipping patch merge conflict tests.")
}
th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("PatchStrategicMergeTransformer")
defer th.Reset()
th.WriteF("patch1.yaml", `
apiVersion: example.com/v1
kind: Foo
metadata:
name: my-foo
spec:
bar:
C: Z
`)
th.WriteF("patch2.yaml", `
apiVersion: example.com/v1
kind: Foo
metadata:
name: my-foo
spec:
bar:
C: NOT_Z
`)
_, err := th.RunTransformer(`
apiVersion: builtin
kind: PatchStrategicMergeTransformer
metadata:
name: notImportantHere
paths:
- patch1.yaml
- patch2.yaml
`, targetNoschema)
if assert.Error(t, err) && !errorContains(err, "conflict") {
t.Fatalf("expected error to contain %q but get %v", "conflict", err)
}
}
// simple utility function to add an namespace in a resource // simple utility function to add an namespace in a resource
// used as base, patch or expected result. Simply looks // used as base, patch or expected result. Simply looks
// for specs: in order to add namespace: xxxx before this line // for specs: in order to add namespace: xxxx before this line
@@ -1049,9 +951,8 @@ func TestMultiplePatches(t *testing.T) {
changeImagePatch(MyCRD, "nginx:latest"), changeImagePatch(MyCRD, "nginx:latest"),
}, },
errorExpected: false, errorExpected: false,
// There is no conflict detected. It should // Theses patches aren't commutable (you get a different result
// be but the JMPConflictDector ignores it. // if they are ordered differently). This is allowed without error.
// See https://github.com/kubernetes-sigs/kustomize/issues/1370
expected: expectedResultJMP("nginx:latest"), expected: expectedResultJMP("nginx:latest"),
}, },
"noschema-latest-label-1.7.9": { "noschema-latest-label-1.7.9": {
@@ -1062,9 +963,8 @@ func TestMultiplePatches(t *testing.T) {
changeImagePatch(MyCRD, "nginx:1.7.9"), changeImagePatch(MyCRD, "nginx:1.7.9"),
}, },
errorExpected: false, errorExpected: false,
// There is no conflict detected. It should // Theses patches aren't commutable (you get a different result
// be but the JMPConflictDector ignores it. // if they are ordered differently). This is allowed without error.
// See https://github.com/kubernetes-sigs/kustomize/issues/1370
expected: expectedResultJMP("nginx:1.7.9"), expected: expectedResultJMP("nginx:1.7.9"),
}, },
} }
@@ -1090,127 +990,6 @@ func TestMultiplePatches(t *testing.T) {
} }
} }
// TestMultiplePatchesWithConflict checks that the conflict are
// detected regardless of the order of the patches and regardless
// of the schema availibility (SMP vs JSON)
func TestMultiplePatchesWithConflict(t *testing.T) {
if skipConflictDetectionTests {
t.Skip("Skipping patch merge conflict tests.")
}
tests := map[string]testRecord{
"withschema-label-latest-1.7.9": {
base: baseResource(Deployment),
patch: []string{
addLabelAndEnvPatch(Deployment),
changeImagePatch(Deployment, "nginx:latest"),
changeImagePatch(Deployment, "nginx:1.7.9"),
},
errorExpected: true,
errorMsg: "conflict",
},
"withschema-latest-label-1.7.9-difforder": {
base: baseResource(Deployment),
patch: []string{
changeImagePatch(Deployment, "nginx:latest"),
addLabelAndEnvPatch(Deployment),
changeImagePatch(Deployment, "nginx:1.7.9"),
},
errorExpected: true,
errorMsg: "conflict",
},
"withschema-1.7.9-label-latest": {
base: baseResource(Deployment),
patch: []string{
changeImagePatch(Deployment, "nginx:1.7.9"),
addLabelAndEnvPatch(Deployment),
changeImagePatch(Deployment, "nginx:latest"),
},
errorExpected: true,
errorMsg: "conflict",
},
"withschema-1.7.9-latest-label": {
base: baseResource(Deployment),
patch: []string{
changeImagePatch(Deployment, "nginx:1.7.9"),
changeImagePatch(Deployment, "nginx:latest"),
addLabelAndEnvPatch(Deployment),
changeImagePatch(Deployment, "nginx:nginx"),
},
errorExpected: true,
errorMsg: "conflict",
},
"noschema-label-latest-1.7.9": {
base: baseResource(MyCRD),
patch: []string{
addLabelAndEnvPatch(MyCRD),
changeImagePatch(MyCRD, "nginx:latest"),
changeImagePatch(MyCRD, "nginx:1.7.9"),
},
errorExpected: true,
errorMsg: "conflict",
},
"noschema-1.7.9-latest-label": {
base: baseResource(MyCRD),
patch: []string{
changeImagePatch(MyCRD, "nginx:1.7.9"),
changeImagePatch(MyCRD, "nginx:latest"),
addLabelAndEnvPatch(MyCRD),
changeImagePatch(MyCRD, "nginx:nginx"),
},
errorExpected: true,
},
"noschema-label-image-container": {
base: baseResource(MyCRD),
patch: []string{
addLabelAndEnvPatch(MyCRD),
changeImagePatch(MyCRD, "nginx:latest"),
addContainerAndEnvPatch(MyCRD),
},
errorExpected: true,
errorMsg: "conflict",
},
"noschema-image-container-label": {
base: baseResource(MyCRD),
patch: []string{
changeImagePatch(MyCRD, "nginx:latest"),
addContainerAndEnvPatch(MyCRD),
addLabelAndEnvPatch(MyCRD),
},
errorExpected: true,
errorMsg: "conflict",
},
"noschema-container-label-image": {
base: baseResource(MyCRD),
patch: []string{
addContainerAndEnvPatch(MyCRD),
addLabelAndEnvPatch(MyCRD),
changeImagePatch(MyCRD, "nginx:latest"),
},
errorExpected: true,
errorMsg: "conflict",
},
}
th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("PatchStrategicMergeTransformer")
defer th.Reset()
for name, test := range tests {
t.Run(name, func(t *testing.T) {
th.ResetLoaderRoot(fmt.Sprintf("/%s", name))
for idx, patch := range test.patch {
th.WriteF(fmt.Sprintf("/%s/patch%d.yaml", name, idx), patch)
}
if test.errorExpected {
_, err := th.RunTransformer(toConfig(test.patch...), test.base)
compareExpectedError(t, name, err, test.errorMsg)
} else {
th.RunTransformerAndCheckResult(
toConfig(test.patch...), test.base, test.expected)
}
})
}
}
// TestMultipleNamespaces before the same patch // TestMultipleNamespaces before the same patch
// on two objects have the same name but in a different namespaces // on two objects have the same name but in a different namespaces
func TestMultipleNamespaces(t *testing.T) { func TestMultipleNamespaces(t *testing.T) {