diff --git a/api/builtins/PatchStrategicMergeTransformer.go b/api/builtins/PatchStrategicMergeTransformer.go index e7636a71ee..fe2693aed8 100644 --- a/api/builtins/PatchStrategicMergeTransformer.go +++ b/api/builtins/PatchStrategicMergeTransformer.go @@ -64,7 +64,7 @@ func (p *PatchStrategicMergeTransformerPlugin) Config( } func (p *PatchStrategicMergeTransformerPlugin) Transform(m resmap.ResMap) error { - patches, err := p.h.ResmapFactory().MergePatches(p.loadedPatches) + patches, err := p.h.ResmapFactory().Merge(p.loadedPatches) if err != nil { return err } diff --git a/api/internal/k8sdeps/transformer/patch/conflictdetector.go b/api/internal/k8sdeps/merge/merginator.go similarity index 79% rename from api/internal/k8sdeps/transformer/patch/conflictdetector.go rename to api/internal/k8sdeps/merge/merginator.go index 6c79794d5d..627908f23b 100644 --- a/api/internal/k8sdeps/transformer/patch/conflictdetector.go +++ b/api/internal/k8sdeps/merge/merginator.go @@ -1,7 +1,7 @@ // Copyright 2019 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 -package patch +package merge import ( "encoding/json" @@ -20,18 +20,20 @@ import ( type conflictDetector interface { hasConflict(patch1, patch2 *resource.Resource) (bool, error) - findConflict(conflictingPatchIdx int, patches []*resource.Resource) (*resource.Resource, error) + findConflict( + conflictingPatchIdx int, + patches []*resource.Resource) (*resource.Resource, error) mergePatches(patch1, patch2 *resource.Resource) (*resource.Resource, error) } type jsonMergePatch struct { - rf *resource.Factory + resourceFactory *resource.Factory } var _ conflictDetector = &jsonMergePatch{} func newJMPConflictDetector(rf *resource.Factory) conflictDetector { - return &jsonMergePatch{rf: rf} + return &jsonMergePatch{resourceFactory: rf} } func (jmp *jsonMergePatch) hasConflict( @@ -40,7 +42,8 @@ func (jmp *jsonMergePatch) hasConflict( } func (jmp *jsonMergePatch) findConflict( - conflictingPatchIdx int, patches []*resource.Resource) (*resource.Resource, error) { + conflictingPatchIdx int, + patches []*resource.Resource) (*resource.Resource, error) { for i, patch := range patches { if i == conflictingPatchIdx { continue @@ -77,7 +80,7 @@ func (jmp *jsonMergePatch) mergePatches( } mergedMap := make(map[string]interface{}) err = json.Unmarshal(mergedBytes, &mergedMap) - return jmp.rf.FromMap(mergedMap), err + return jmp.resourceFactory.FromMap(mergedMap), err } type strategicMergePatch struct { @@ -94,13 +97,15 @@ func newSMPConflictDetector( return &strategicMergePatch{lookupPatchMeta: lookupPatchMeta, rf: rf}, err } -func (smp *strategicMergePatch) hasConflict(p1, p2 *resource.Resource) (bool, error) { +func (smp *strategicMergePatch) hasConflict( + p1, p2 *resource.Resource) (bool, error) { return strategicpatch.MergingMapsHaveConflicts( p1.Map(), p2.Map(), smp.lookupPatchMeta) } func (smp *strategicMergePatch) findConflict( - conflictingPatchIdx int, patches []*resource.Resource) (*resource.Resource, error) { + conflictingPatchIdx int, + patches []*resource.Resource) (*resource.Resource, error) { for i, patch := range patches { if i == conflictingPatchIdx { continue @@ -122,10 +127,12 @@ func (smp *strategicMergePatch) findConflict( return nil, nil } -func (smp *strategicMergePatch) mergePatches(patch1, patch2 *resource.Resource) (*resource.Resource, error) { +func (smp *strategicMergePatch) mergePatches( + patch1, patch2 *resource.Resource) (*resource.Resource, error) { if hasDeleteDirectiveMarker(patch2.Map()) { if hasDeleteDirectiveMarker(patch1.Map()) { - return nil, fmt.Errorf("cannot merge patches both containing '$patch: delete' directives") + return nil, fmt.Errorf( + "cannot merge patches both containing '$patch: delete' directives") } patch1, patch2 = patch2, patch1 } @@ -134,10 +141,21 @@ func (smp *strategicMergePatch) mergePatches(patch1, patch2 *resource.Resource) return smp.rf.FromMap(mergeJSONMap), err } -// MergePatches merge and index patches by OrgId. -// It errors out if there is conflict between patches. -func MergePatches(patches []*resource.Resource, - rf *resource.Factory) (resmap.ResMap, error) { +type merginatorImpl struct { + rf *resource.Factory +} + +// NewMerginator returns a new implementation of resmap.Merginator. +func NewMerginator(rf *resource.Factory) resmap.Merginator { + return &merginatorImpl{rf: rf} +} + +var _ resmap.Merginator = (*merginatorImpl)(nil) + +// Merge merges the incoming resources into a new resmap.ResMap. +// Returns an error on conflict. +func (m *merginatorImpl) Merge( + patches []*resource.Resource) (resmap.ResMap, error) { rc := resmap.New() for ix, patch := range patches { id := patch.OrgId() @@ -156,9 +174,9 @@ func MergePatches(patches []*resource.Resource, } var cd conflictDetector if err != nil { - cd = newJMPConflictDetector(rf) + cd = newJMPConflictDetector(m.rf) } else { - cd, err = newSMPConflictDetector(versionedObj, rf) + cd, err = newSMPConflictDetector(versionedObj, m.rf) if err != nil { return nil, err } diff --git a/api/internal/k8sdeps/transformer/factory.go b/api/internal/k8sdeps/transformer/factory.go deleted file mode 100644 index 622c4a6cf5..0000000000 --- a/api/internal/k8sdeps/transformer/factory.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2019 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 - -// Package transformer provides transformer factory -package transformer - -import ( - "sigs.k8s.io/kustomize/api/internal/k8sdeps/transformer/patch" - "sigs.k8s.io/kustomize/api/resmap" - "sigs.k8s.io/kustomize/api/resource" -) - -// FactoryImpl makes patch transformer and name hash transformer -type FactoryImpl struct{} - -// NewFactoryImpl makes a new factoryImpl instance -func NewFactoryImpl() *FactoryImpl { - return &FactoryImpl{} -} - -func (p *FactoryImpl) MergePatches(patches []*resource.Resource, - rf *resource.Factory) ( - resmap.ResMap, error) { - return patch.MergePatches(patches, rf) -} diff --git a/api/internal/plugins/fnplugin/fnplugin.go b/api/internal/plugins/fnplugin/fnplugin.go index 72dbf4271f..3d0ad10dd3 100644 --- a/api/internal/plugins/fnplugin/fnplugin.go +++ b/api/internal/plugins/fnplugin/fnplugin.go @@ -173,11 +173,11 @@ func (p *FnPlugin) invokePlugin(input []byte) ([]byte, error) { // TODO(donnyxia): This is actually not used by generator and only used to bypass a kio limitation. // Need better solution. if input == nil { - yaml, err := functionConfig.String() + yml, err := functionConfig.String() if err != nil { return nil, err } - input = []byte(yaml) + input = []byte(yml) } // Configure and Execute Fn. We don't need to convert resources to ResourceList here diff --git a/api/internal/target/kusttarget.go b/api/internal/target/kusttarget.go index c8a60a80a6..d5d1626c99 100644 --- a/api/internal/target/kusttarget.go +++ b/api/internal/target/kusttarget.go @@ -27,7 +27,6 @@ type KustTarget struct { ldr ifc.Loader validator ifc.Validator rFactory *resmap.Factory - tFactory resmap.PatchFactory pLdr *loader.Loader } @@ -36,13 +35,11 @@ func NewKustTarget( ldr ifc.Loader, validator ifc.Validator, rFactory *resmap.Factory, - tFactory resmap.PatchFactory, pLdr *loader.Loader) *KustTarget { return &KustTarget{ ldr: ldr, validator: validator, rFactory: rFactory, - tFactory: tFactory, pLdr: pLdr, } } @@ -350,8 +347,7 @@ func (kt *KustTarget) accumulateComponents( func (kt *KustTarget) accumulateDirectory( ra *accumulator.ResAccumulator, ldr ifc.Loader, isComponent bool) (*accumulator.ResAccumulator, error) { defer ldr.Cleanup() - subKt := NewKustTarget( - ldr, kt.validator, kt.rFactory, kt.tFactory, kt.pLdr) + subKt := NewKustTarget(ldr, kt.validator, kt.rFactory, kt.pLdr) err := subKt.Load() if err != nil { return nil, errors.Wrapf( diff --git a/api/internal/target/maker_test.go b/api/internal/target/maker_test.go index 461ba4f8f2..7e85832dfb 100644 --- a/api/internal/target/maker_test.go +++ b/api/internal/target/maker_test.go @@ -7,7 +7,7 @@ import ( "testing" "sigs.k8s.io/kustomize/api/filesys" - "sigs.k8s.io/kustomize/api/internal/k8sdeps/transformer" + "sigs.k8s.io/kustomize/api/internal/k8sdeps/merge" pLdr "sigs.k8s.io/kustomize/api/internal/plugins/loader" "sigs.k8s.io/kustomize/api/internal/target" "sigs.k8s.io/kustomize/api/k8sdeps/kunstruct" @@ -35,17 +35,17 @@ func makeKustTargetWithRf( t *testing.T, fSys filesys.FileSystem, root string, - resFact *resource.Factory) *target.KustTarget { - rf := resmap.NewFactory(resFact, transformer.NewFactoryImpl()) - pc := konfig.DisabledPluginConfig() + resourceFactory *resource.Factory) *target.KustTarget { ldr, err := fLdr.NewLoader(fLdr.RestrictionRootOnly, root, fSys) if err != nil { t.Fatal(err) } + rf := resmap.NewFactory( + resourceFactory, merge.NewMerginator(resourceFactory)) + pc := konfig.DisabledPluginConfig() return target.NewKustTarget( ldr, valtest_test.MakeFakeValidator(), rf, - transformer.NewFactoryImpl(), pLdr.NewLoader(pc, rf)) } diff --git a/api/krusty/kustomizer.go b/api/krusty/kustomizer.go index 12671f5dee..07fbc9145f 100644 --- a/api/krusty/kustomizer.go +++ b/api/krusty/kustomizer.go @@ -8,7 +8,7 @@ import ( "sigs.k8s.io/kustomize/api/builtins" "sigs.k8s.io/kustomize/api/filesys" - "sigs.k8s.io/kustomize/api/internal/k8sdeps/transformer" + "sigs.k8s.io/kustomize/api/internal/k8sdeps/merge" pLdr "sigs.k8s.io/kustomize/api/internal/plugins/loader" "sigs.k8s.io/kustomize/api/internal/target" "sigs.k8s.io/kustomize/api/k8sdeps/kunstruct" @@ -49,11 +49,11 @@ func MakeKustomizer(fSys filesys.FileSystem, o *Options) *Kustomizer { // on any number of internal paths (e.g. the filesystem may contain // multiple overlays, and Run can be called on each of them). func (b *Kustomizer) Run(path string) (resmap.ResMap, error) { - pf := transformer.NewFactoryImpl() - rf := resmap.NewFactory( - resource.NewFactory( - kunstruct.NewKunstructuredFactoryImpl()), - pf) + resourceFactory := resource.NewFactory( + kunstruct.NewKunstructuredFactoryImpl()) + resmapFactory := resmap.NewFactory( + resourceFactory, + merge.NewMerginator(resourceFactory)) lr := fLdr.RestrictionNone if b.options.LoadRestrictions == types.LoadRestrictionsRootOnly { lr = fLdr.RestrictionRootOnly @@ -66,9 +66,8 @@ func (b *Kustomizer) Run(path string) (resmap.ResMap, error) { kt := target.NewKustTarget( ldr, validator.NewKustValidator(), - rf, - pf, - pLdr.NewLoader(b.options.PluginConfig, rf), + resmapFactory, + pLdr.NewLoader(b.options.PluginConfig, resmapFactory), ) err = kt.Load() if err != nil { @@ -84,7 +83,9 @@ func (b *Kustomizer) Run(path string) (resmap.ResMap, error) { } if b.options.AddManagedbyLabel { t := builtins.LabelTransformerPlugin{ - Labels: map[string]string{konfig.ManagedbyLabelKey: fmt.Sprintf("kustomize-%s", provenance.GetProvenance().Version)}, + Labels: map[string]string{ + konfig.ManagedbyLabelKey: fmt.Sprintf( + "kustomize-%s", provenance.GetProvenance().Version)}, FieldSpecs: []types.FieldSpec{{ Path: "metadata/labels", CreateIfNotPresent: true, diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go index 8e5b3113a6..2d8e126da0 100644 --- a/api/loader/fileloader.go +++ b/api/loader/fileloader.go @@ -199,7 +199,9 @@ func (fl *fileLoader) New(path string) (ifc.Loader, error) { } root, errDir := demandDirectoryRoot(fl.fSys, fl.root.Join(path)) if errDir != nil { - return nil, fmt.Errorf("Error loading %s with git: %v, dir: %v, get: %v", path, errGit, errDir, errGet) + return nil, fmt.Errorf( + "error loading %s with git: %v, dir: %v, get: %v", + path, errGit, errDir, errGet) } if errDir := fl.errIfGitContainmentViolation(root); errDir != nil { return nil, errDir diff --git a/api/loader/loader.go b/api/loader/loader.go index daab269436..7c9255477a 100644 --- a/api/loader/loader.go +++ b/api/loader/loader.go @@ -39,5 +39,7 @@ func NewLoader( return newLoaderAtConfirmedDir(lr, root, fSys, nil, git.ClonerUsingGitExec, getRemoteTarget), nil } - return nil, fmt.Errorf("Error creating new loader with git: %v, dir: %v, get: %v", errGit, errDir, errGet) + return nil, fmt.Errorf( + "error creating new loader with git: %v, dir: %v, get: %v", + errGit, errDir, errGet) } diff --git a/api/resmap/factory.go b/api/resmap/factory.go index 90ee58c379..37a05ab6d4 100644 --- a/api/resmap/factory.go +++ b/api/resmap/factory.go @@ -14,12 +14,12 @@ import ( // Factory makes instances of ResMap. type Factory struct { resF *resource.Factory - tf PatchFactory + pm Merginator } // NewFactory returns a new resmap.Factory. -func NewFactory(rf *resource.Factory, tf PatchFactory) *Factory { - return &Factory{resF: rf, tf: tf} +func NewFactory(rf *resource.Factory, pm Merginator) *Factory { + return &Factory{resF: rf, pm: pm} } // RF returns a resource.Factory. @@ -87,6 +87,7 @@ func (rmF *Factory) NewResMapFromConfigMapArgs( return newResMapFromResourceSlice(resources) } +// FromConfigMapArgs creates a new ResMap containing one ConfigMap. func (rmF *Factory) FromConfigMapArgs( kvLdr ifc.KvLoader, args types.ConfigMapArgs) (ResMap, error) { res, err := rmF.resF.MakeConfigMap(kvLdr, &args) @@ -111,6 +112,7 @@ func (rmF *Factory) NewResMapFromSecretArgs( return newResMapFromResourceSlice(resources) } +// FromSecretArgs creates a new ResMap containing one secret. func (rmF *Factory) FromSecretArgs( kvLdr ifc.KvLoader, args types.SecretArgs) (ResMap, error) { res, err := rmF.resF.MakeSecret(kvLdr, &args) @@ -120,12 +122,14 @@ func (rmF *Factory) FromSecretArgs( return rmF.FromResource(res), nil } -func (rmF *Factory) MergePatches(patches []*resource.Resource) ( - ResMap, error) { - return rmF.tf.MergePatches(patches, rmF.resF) +// Merge creates a new ResMap by merging incoming resources. +// Error if conflict found. +func (rmF *Factory) Merge(patches []*resource.Resource) (ResMap, error) { + return rmF.pm.Merge(patches) } -func newResMapFromResourceSlice(resources []*resource.Resource) (ResMap, error) { +func newResMapFromResourceSlice( + resources []*resource.Resource) (ResMap, error) { result := New() for _, res := range resources { err := result.Append(res) diff --git a/api/resmap/merginator.go b/api/resmap/merginator.go new file mode 100644 index 0000000000..10d7377c50 --- /dev/null +++ b/api/resmap/merginator.go @@ -0,0 +1,13 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package resmap + +import "sigs.k8s.io/kustomize/api/resource" + +// Merginator merges resources. +type Merginator interface { + // Merge creates a new ResMap by merging incoming resources. + // Error if conflict found. + Merge([]*resource.Resource) (ResMap, error) +} diff --git a/api/resmap/patchfactory.go b/api/resmap/patchfactory.go deleted file mode 100644 index 9af1ac0451..0000000000 --- a/api/resmap/patchfactory.go +++ /dev/null @@ -1,15 +0,0 @@ -/// Copyright 2019 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 - -// Package patch holds miscellaneous interfaces used by kustomize. -package resmap - -import ( - "sigs.k8s.io/kustomize/api/resource" -) - -// PatchFactory makes transformers that require k8sdeps. -type PatchFactory interface { - MergePatches(patches []*resource.Resource, - rf *resource.Factory) (ResMap, error) -} diff --git a/api/testutils/kusttest/harnessenhanced.go b/api/testutils/kusttest/harnessenhanced.go index f40320b0f9..405af5230d 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -8,7 +8,7 @@ import ( "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/ifc" - "sigs.k8s.io/kustomize/api/internal/k8sdeps/transformer" + "sigs.k8s.io/kustomize/api/internal/k8sdeps/merge" pLdr "sigs.k8s.io/kustomize/api/internal/plugins/loader" "sigs.k8s.io/kustomize/api/k8sdeps/kunstruct" "sigs.k8s.io/kustomize/api/konfig" @@ -46,16 +46,17 @@ func MakeEnhancedHarness(t *testing.T) *HarnessEnhanced { if err != nil { t.Fatal(err) } - - rf := resmap.NewFactory( - resource.NewFactory(kunstruct.NewKunstructuredFactoryImpl()), - transformer.NewFactoryImpl()) + resourceFactory := resource.NewFactory( + kunstruct.NewKunstructuredFactoryImpl()) + resmapFactory := resmap.NewFactory( + resourceFactory, + merge.NewMerginator(resourceFactory)) result := &HarnessEnhanced{ Harness: MakeHarness(t), pte: pte, - rf: rf, - pl: pLdr.NewLoader(pc, rf)} + rf: resmapFactory, + pl: pLdr.NewLoader(pc, resmapFactory)} // Point the file loader to the root ('/') of the in-memory file system. result.ResetLoaderRoot(filesys.Separator) diff --git a/api/types/kustomization_test.go b/api/types/kustomization_test.go index 57373ee5bb..0533fa432b 100644 --- a/api/types/kustomization_test.go +++ b/api/types/kustomization_test.go @@ -5,9 +5,11 @@ import ( ) func fixKustomizationPostUnmarshallingCheck(k, e *Kustomization) bool { - return (k.Kind == e.Kind && k.APIVersion == e.APIVersion && - len(k.Resources) == len(e.Resources) && k.Resources[0] == e.Resources[0] && - k.Bases == nil) + return k.Kind == e.Kind && + k.APIVersion == e.APIVersion && + len(k.Resources) == len(e.Resources) && + k.Resources[0] == e.Resources[0] && + k.Bases == nil } func TestFixKustomizationPostUnmarshalling(t *testing.T) { diff --git a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go index 4f61f031db..c8b36eff43 100644 --- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go +++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go @@ -68,7 +68,7 @@ func (p *plugin) Config( } func (p *plugin) Transform(m resmap.ResMap) error { - patches, err := p.h.ResmapFactory().MergePatches(p.loadedPatches) + patches, err := p.h.ResmapFactory().Merge(p.loadedPatches) if err != nil { return err } diff --git a/releasing/releasing/gitrunner.go b/releasing/releasing/gitrunner.go index 78033acd7d..2d2bc956e1 100644 --- a/releasing/releasing/gitrunner.go +++ b/releasing/releasing/gitrunner.go @@ -63,14 +63,14 @@ func (gr *gitRunner) DeleteWorktreeDir() error { func (gr *gitRunner) WorktreePath() (string, error) { if gr.worktreePath == "" { - return "", fmt.Errorf("Empty worktree path") + return "", fmt.Errorf("empty worktree path") } return gr.worktreePath, nil } func (gr *gitRunner) OriginalGitPath() (string, error) { if gr.originalGitPath == "" { - return "", fmt.Errorf("Empty git path") + return "", fmt.Errorf("empty git path") } return gr.originalGitPath, nil } @@ -107,7 +107,7 @@ func (gr *gitRunner) CheckRemoteExistence(remote string) error { regString := fmt.Sprintf("(?m)^\\s*%s\\s*$", remote) reg := regexp.MustCompile(regString) if !reg.MatchString(string(stdoutStderr)) { - return fmt.Errorf("Cannot find remote named %s", remote) + return fmt.Errorf("cannot find remote named %s", remote) } logDebug("Remote %s exists", remote) return nil diff --git a/releasing/releasing/moduleversion.go b/releasing/releasing/moduleversion.go index 5370262ad9..af4a5ecab9 100644 --- a/releasing/releasing/moduleversion.go +++ b/releasing/releasing/moduleversion.go @@ -29,7 +29,7 @@ func (v *moduleVersion) Bump(t string) error { } else if t == "patch" { v.patch++ } else { - return fmt.Errorf("Invalid version type: %s", t) + return fmt.Errorf("invalid version type: %s", t) } return nil } @@ -37,14 +37,14 @@ func (v *moduleVersion) Bump(t string) error { func newModuleVersionFromString(vs string) (moduleVersion, error) { v := moduleVersion{} if len(vs) < 1 { - return v, fmt.Errorf("Invalid version string %s", vs) + return v, fmt.Errorf("invalid version string %s", vs) } if vs[0] == 'v' { vs = vs[1:] } versions := strings.Split(vs, ".") if len(versions) != 3 { - return v, fmt.Errorf("Invalid version string %s", vs) + return v, fmt.Errorf("invalid version string %s", vs) } major, err := strconv.Atoi(versions[0]) if err != nil {