Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dead merge conflict code. #3734

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions api/builtins/PatchStrategicMergeTransformer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions api/filters/nameref/nameref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ map:
}
tc.filter.Referrer = referrer

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

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

resMapFactory := resmap.NewFactory(factory, nil)
resMapFactory := resmap.NewFactory(factory)
candidatesRes, err := factory.SliceFromBytesWithNames(
tc.originalNames, []byte(tc.candidates))
if err != nil {
Expand Down
23 changes: 0 additions & 23 deletions api/internal/conflict/factory.go

This file was deleted.

33 changes: 0 additions & 33 deletions api/internal/conflict/smpatchmergeonlydetector.go

This file was deleted.

3 changes: 1 addition & 2 deletions api/internal/plugins/execplugin/execplugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ s/$BAR/bar baz/g
t.Fatal(err)
}
pvd := provider.NewDefaultDepProvider()
rf := resmap.NewFactory(
pvd.GetResourceFactory(), pvd.GetConflictDetectorFactory())
rf := resmap.NewFactory(pvd.GetResourceFactory())
pluginConfig := rf.RF().FromMap(
map[string]interface{}{
"apiVersion": "someteam.example.com/v1",
Expand Down
3 changes: 1 addition & 2 deletions api/internal/plugins/loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ func TestLoader(t *testing.T) {
BuildGoPlugin("someteam.example.com", "v1", "SomeServiceGenerator")
defer th.Reset()
p := provider.NewDefaultDepProvider()
rmF := resmap.NewFactory(
p.GetResourceFactory(), p.GetConflictDetectorFactory())
rmF := resmap.NewFactory(p.GetResourceFactory())
fLdr, err := loader.NewLoader(
loader.RestrictionRootOnly,
filesys.Separator, filesys.MakeFsInMemory())
Expand Down
3 changes: 1 addition & 2 deletions api/internal/target/maker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ func makeKustTargetWithRf(
if err != nil {
t.Fatal(err)
}
rf := resmap.NewFactory(
pvd.GetResourceFactory(), pvd.GetConflictDetectorFactory())
rf := resmap.NewFactory(pvd.GetResourceFactory())
pc := konfig.DisabledPluginConfig()
return target.NewKustTarget(
ldr,
Expand Down
43 changes: 3 additions & 40 deletions api/internal/target/multitransformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,28 @@
package target

import (
"fmt"

"sigs.k8s.io/kustomize/api/resmap"
)

// multiTransformer contains a list of transformers.
type multiTransformer struct {
transformers []resmap.Transformer
checkConflictEnabled bool
transformers []resmap.Transformer
}

var _ resmap.Transformer = &multiTransformer{}

// newMultiTransformer constructs a multiTransformer.
func newMultiTransformer(t []resmap.Transformer) resmap.Transformer {
r := &multiTransformer{
transformers: make([]resmap.Transformer, len(t)),
checkConflictEnabled: false}
transformers: make([]resmap.Transformer, len(t)),
}
copy(r.transformers, t)
return r
}

// Transform applies the member transformers in order to the resources,
// optionally detecting and erroring on commutation conflict.
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 {
if err := t.Transform(m); err != nil {
return err
Expand All @@ -44,30 +34,3 @@ func (o *multiTransformer) transform(m resmap.ResMap) error {
}
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]
}
}
4 changes: 1 addition & 3 deletions api/krusty/kustomizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ func MakeKustomizer(o *Options) *Kustomizer {
// and Run can be called on each of them).
func (b *Kustomizer) Run(
fSys filesys.FileSystem, path string) (resmap.ResMap, error) {
resmapFactory := resmap.NewFactory(
b.depProvider.GetResourceFactory(),
b.depProvider.GetConflictDetectorFactory())
resmapFactory := resmap.NewFactory(b.depProvider.GetResourceFactory())
lr := fLdr.RestrictionNone
if b.options.LoadRestrictions == types.LoadRestrictionsRootOnly {
lr = fLdr.RestrictionRootOnly
Expand Down
2 changes: 1 addition & 1 deletion api/krusty/multiplepatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ metadata:
`)
}

func TestMultiplePatchesWithConflict(t *testing.T) {
func TestNonCommutablePatches(t *testing.T) {
th := kusttest_test.MakeHarness(t)
makeCommonFilesForMultiplePatchTests(th)
th.WriteF("overlay/staging/deployment-patch1.yaml", `
Expand Down
45 changes: 8 additions & 37 deletions api/provider/depprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,51 +6,26 @@ package provider
import (
"sigs.k8s.io/kustomize/api/hasher"
"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/resource"
)

// DepProvider is a dependency provider, injecting different
// implementations depending on the context.
//
// Notes on interfaces:
//
// - 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
//
// See TODO inside the validator for status.
// At time of writing, this is a do-nothing
// validator as it's not critical to kustomize function.
//
type DepProvider struct {
resourceFactory *resource.Factory
conflictDectectorFactory resource.ConflictDetectorFactory
fieldValidator ifc.Validator
resourceFactory *resource.Factory
// implemented by api/internal/validate.FieldValidator
// See TODO inside the validator for status.
// At time of writing, this is a do-nothing
// validator as it's not critical to kustomize function.
fieldValidator ifc.Validator
}

func NewDepProvider() *DepProvider {
rf := resource.NewFactory(&hasher.Hasher{})
return &DepProvider{
resourceFactory: rf,
conflictDectectorFactory: conflict.NewFactory(),
fieldValidator: validate.NewFieldValidator(),
resourceFactory: rf,
fieldValidator: validate.NewFieldValidator(),
}
}

Expand All @@ -62,10 +37,6 @@ func (dp *DepProvider) GetResourceFactory() *resource.Factory {
return dp.resourceFactory
}

func (dp *DepProvider) GetConflictDetectorFactory() resource.ConflictDetectorFactory {
return dp.conflictDectectorFactory
}

func (dp *DepProvider) GetFieldValidator() ifc.Validator {
return dp.fieldValidator
}
14 changes: 2 additions & 12 deletions api/resmap/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,11 @@ import (
type Factory struct {
// Makes resources.
resF *resource.Factory
// Makes ConflictDetectors.
cdf resource.ConflictDetectorFactory
}

// NewFactory returns a new resmap.Factory.
func NewFactory(
rf *resource.Factory, cdf resource.ConflictDetectorFactory) *Factory {
return &Factory{resF: rf, cdf: cdf}
func NewFactory(rf *resource.Factory) *Factory {
return &Factory{resF: rf}
}

// RF returns a resource.Factory.
Expand Down Expand Up @@ -126,13 +123,6 @@ func (rmF *Factory) FromSecretArgs(
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(
resources []*resource.Resource) (ResMap, error) {
result := New()
Expand Down
54 changes: 0 additions & 54 deletions api/resmap/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"sigs.k8s.io/kustomize/api/kv"
"sigs.k8s.io/kustomize/api/loader"
. "sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/resource"
resmaptest_test "sigs.k8s.io/kustomize/api/testutils/resmaptest"
valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest"
"sigs.k8s.io/kustomize/api/types"
Expand Down Expand Up @@ -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))
}
Loading