diff --git a/go.mod b/go.mod index da97c1e5b..c83b51083 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/modern-go/reflect2 v0.0.0-20180228065516-1df9eeb2bb81 // indirect github.com/onsi/ginkgo v1.8.0 // indirect github.com/onsi/gomega v1.5.0 // indirect - github.com/pkg/errors v0.8.0 + github.com/pkg/errors v0.8.1 github.com/spf13/cobra v0.0.2 github.com/spf13/pflag v1.0.1 github.com/stretchr/testify v1.3.0 // indirect diff --git a/go.sum b/go.sum index 241ce5825..a12faa59e 100644 --- a/go.sum +++ b/go.sum @@ -47,8 +47,8 @@ github.com/onsi/ginkgo v1.8.0 h1:VkHVNpR4iVnU8XQR6DBm8BqYjN7CRzw+xKUbVVbbW9w= github.com/onsi/ginkgo v1.8.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/gomega v1.5.0 h1:izbySO9zDPmjJ8rDjLvkA2zJHIo+HkYXHnf7eN7SSyo= github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= -github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw= -github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= +github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/spf13/cobra v0.0.2 h1:NfkwRbgViGoyjBKsLI0QMDcuMnhM+SBg3T0cGfpvKDE= diff --git a/pkg/internal/error/kustomizationerror.go b/pkg/internal/error/kustomizationerror.go deleted file mode 100644 index 0d53ca9b8..000000000 --- a/pkg/internal/error/kustomizationerror.go +++ /dev/null @@ -1,61 +0,0 @@ -/* -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. -*/ - -package error - -import ( - "fmt" -) - -// KustomizationError represents an error with a kustomization. -type KustomizationError struct { - KustomizationPath string - ErrorMsg string -} - -func (ke KustomizationError) Error() string { - return fmt.Sprintf("Kustomization File [%s]: %s\n", ke.KustomizationPath, ke.ErrorMsg) -} - -// KustomizationErrors collects all errors. -type KustomizationErrors struct { - kErrors []error -} - -func (ke *KustomizationErrors) Error() string { - errormsg := "" - for _, e := range ke.kErrors { - errormsg += e.Error() + "\n" - } - return errormsg -} - -// Append adds error to a collection of errors. -func (ke *KustomizationErrors) Append(e error) { - ke.kErrors = append(ke.kErrors, e) -} - -// Get returns all collected errors. -func (ke *KustomizationErrors) Get() []error { - return ke.kErrors -} - -// BatchAppend adds all errors from another KustomizationErrors -func (ke *KustomizationErrors) BatchAppend(e KustomizationErrors) { - for _, err := range e.Get() { - ke.kErrors = append(ke.kErrors, err) - } -} diff --git a/pkg/internal/error/kustomizationerror_test.go b/pkg/internal/error/kustomizationerror_test.go deleted file mode 100644 index bb6ed0dba..000000000 --- a/pkg/internal/error/kustomizationerror_test.go +++ /dev/null @@ -1,89 +0,0 @@ -/* -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. -*/ - -package error - -import ( - "fmt" - "strings" - "testing" -) - -func TestKustomizationError_Error(t *testing.T) { - errorMsg := "Kustomization not found" - - me := KustomizationError{KustomizationPath: filepath, ErrorMsg: errorMsg} - - if !strings.Contains(me.Error(), filepath) { - t.Errorf("Incorrect KustomizationError.Error() message \n") - t.Errorf("Expected filepath %s, but unfound\n", filepath) - } - - if !strings.Contains(me.Error(), errorMsg) { - t.Errorf("Incorrect KustomizationError.Error() message \n") - t.Errorf("Expected errorMsg %s, but unfound\n", errorMsg) - } -} - -func TestKustomizationErrors_Error(t *testing.T) { - me := KustomizationError{KustomizationPath: filepath, ErrorMsg: "Kustomization not found"} - ce := ConfigmapError{Path: filepath, ErrorMsg: "can't find configmap name"} - pe := PatchError{KustomizationPath: filepath, PatchFilepath: filepath, ErrorMsg: "can't find patch file"} - re := ResourceError{KustomizationPath: filepath, ResourceFilepath: filepath, ErrorMsg: "can't find resource file"} - se := SecretError{KustomizationPath: filepath, ErrorMsg: "can't find secret name"} - mes := KustomizationErrors{kErrors: []error{me, ce, pe, re, se}} - expectedErrorMsg := fmt.Sprintf("%s\n%s\n%s\n%s\n%s\n", me.Error(), ce.Error(), pe.Error(), re.Error(), se.Error()) - if mes.Error() != expectedErrorMsg { - t.Errorf("Incorrect KustomizationErrors.Error() message\n") - t.Errorf(" Expected: %s\n", expectedErrorMsg) - t.Errorf(" Got: %s\n", mes.Error()) - } -} - -func TestKustomizationErrors_Get(t *testing.T) { - ce := ConfigmapError{Path: "kustomization/filepath", ErrorMsg: "can't find configmap name"} - mes := KustomizationErrors{kErrors: []error{ce}} - if len(mes.Get()) != 1 { - t.Errorf("Incorrect KustomizationErrors.Get()\n") - t.Errorf(" Expected: %v\n", []error{ce}) - t.Errorf(" Got: %s\n", mes.Get()) - } -} - -func TestKustomizationErrors_Append(t *testing.T) { - ce := ConfigmapError{Path: "kustomization/filepath", ErrorMsg: "can't find configmap name"} - pe := PatchError{KustomizationPath: "kustomization/filepath", PatchFilepath: "patch/path", ErrorMsg: "can't find patch file"} - mes := KustomizationErrors{kErrors: []error{ce}} - mes.Append(pe) - if len(mes.Get()) != 2 { - t.Errorf("Incorrect KustomizationErrors.Append()\n") - t.Errorf(" Expected: %d error\n%v/n", 2, []error{ce, pe}) - t.Errorf(" Got: %d error\n%v\n", len(mes.Get()), mes.Get()) - } -} - -func TestKustomizationErrors_BatchAppend(t *testing.T) { - ce := ConfigmapError{Path: "kustomization/filepath", ErrorMsg: "can't find configmap name"} - pe := PatchError{KustomizationPath: "kustomization/filepath", PatchFilepath: "patch/path", ErrorMsg: "can't find patch file"} - mes := KustomizationErrors{kErrors: []error{ce}} - me := KustomizationErrors{kErrors: []error{pe}} - mes.BatchAppend(me) - if len(mes.Get()) != 2 { - t.Errorf("Incorrect KustomizationErrors.Append()\n") - t.Errorf(" Expected: %d error\n%v/n", 2, []error{ce, pe}) - t.Errorf(" Got: %d error\n%v\n", len(mes.Get()), mes.Get()) - } -} diff --git a/pkg/resmap/factory.go b/pkg/resmap/factory.go index 7209d47b2..b178dbe53 100644 --- a/pkg/resmap/factory.go +++ b/pkg/resmap/factory.go @@ -41,22 +41,18 @@ func (rmF *Factory) RF() *resource.Factory { return rmF.resF } -// FromFiles returns a ResMap given a resource path slice. -func (rmF *Factory) FromFiles( - loader ifc.Loader, paths []string) (ResMap, error) { - var result []ResMap - for _, path := range paths { - content, err := loader.Load(path) - if err != nil { - return nil, errors.Wrap(err, "Load from path "+path+" failed") - } - res, err := rmF.NewResMapFromBytes(content) - if err != nil { - return nil, internal.Handler(err, path) - } - result = append(result, res) +// FromFile returns a ResMap given a resource path. +func (rmF *Factory) FromFile( + loader ifc.Loader, path string) (ResMap, error) { + content, err := loader.Load(path) + if err != nil { + return nil, err } - return MergeWithErrorOnIdCollision(result...) + res, err := rmF.NewResMapFromBytes(content) + if err != nil { + return nil, internal.Handler(err, path) + } + return res, nil } // newResMapFromBytes decodes a list of objects in byte array format. @@ -65,7 +61,6 @@ func (rmF *Factory) NewResMapFromBytes(b []byte) (ResMap, error) { if err != nil { return nil, err } - result := ResMap{} for _, res := range resources { id := res.Id() diff --git a/pkg/resmap/factory_test.go b/pkg/resmap/factory_test.go index 985179eed..30c9b6080 100644 --- a/pkg/resmap/factory_test.go +++ b/pkg/resmap/factory_test.go @@ -32,7 +32,7 @@ import ( "sigs.k8s.io/kustomize/pkg/types" ) -func TestFromFiles(t *testing.T) { +func TestFromFile(t *testing.T) { resourceStr := `apiVersion: apps/v1 kind: Deployment @@ -85,12 +85,10 @@ metadata: }), } - m, _ := rmF.FromFiles( - l, []string{"deployment.yaml"}) + m, _ := rmF.FromFile(l, "deployment.yaml") if len(m) != 3 { t.Fatalf("%#v should contain 3 appResource, but got %d", m, len(m)) } - if err := expected.ErrorIfNotEqual(m); err != nil { t.Fatalf("actual doesn't match expected: %v", err) } diff --git a/pkg/target/kusttarget.go b/pkg/target/kusttarget.go index a0641b550..f85ae7d9a 100644 --- a/pkg/target/kusttarget.go +++ b/pkg/target/kusttarget.go @@ -28,7 +28,6 @@ import ( "sigs.k8s.io/kustomize/pkg/accumulator" "sigs.k8s.io/kustomize/pkg/ifc" "sigs.k8s.io/kustomize/pkg/ifc/transformer" - interror "sigs.k8s.io/kustomize/pkg/internal/error" patchtransformer "sigs.k8s.io/kustomize/pkg/patch/transformer" "sigs.k8s.io/kustomize/pkg/pgmconfig" "sigs.k8s.io/kustomize/pkg/plugins" @@ -195,23 +194,16 @@ func (kt *KustTarget) shouldAddHashSuffixesToGeneratedResources() bool { // holding customized resources and the data/rules used // to do so. The name back references and vars are // not yet fixed. -func (kt *KustTarget) AccumulateTarget() ( // nolint: gocyclo +func (kt *KustTarget) AccumulateTarget() ( ra *accumulator.ResAccumulator, err error) { - // TODO(monopole): Get rid of the KustomizationErrors accumulator. - // It's not consistently used, and complicates tests. - errs := &interror.KustomizationErrors{} - ra, errs = kt.accumulateBases() - resources, err := kt.rFactory.FromFiles( - kt.ldr, kt.kustomization.Resources) + ra = accumulator.MakeEmptyAccumulator() + err = kt.accumulateResources(ra, kt.kustomization.Bases) if err != nil { - errs.Append(errors.Wrap(err, "rawResources failed to read Resources")) + return nil, errors.Wrap(err, "accumulating bases") } - if len(errs.Get()) > 0 { - return ra, errs - } - err = ra.MergeResourcesWithErrorOnIdCollision(resources) + err = kt.accumulateResources(ra, kt.kustomization.Resources) if err != nil { - errs.Append(errors.Wrap(err, "MergeResourcesWithErrorOnIdCollision")) + return nil, errors.Wrap(err, "accumulating resources") } tConfig, err := config.MakeTransformerConfig( kt.ldr, kt.kustomization.Configurations) @@ -220,41 +212,46 @@ func (kt *KustTarget) AccumulateTarget() ( // nolint: gocyclo } err = ra.MergeConfig(tConfig) if err != nil { - errs.Append(errors.Wrap(err, "MergeConfig")) + return nil, errors.Wrapf( + err, "merging config %v", tConfig) } err = ra.MergeVars(kt.kustomization.Vars) if err != nil { - errs.Append(errors.Wrap(err, "MergeVars")) + return nil, errors.Wrapf( + err, "merging vars %v", kt.kustomization.Vars) } crdTc, err := config.LoadConfigFromCRDs(kt.ldr, kt.kustomization.Crds) if err != nil { - errs.Append(errors.Wrap(err, "LoadCRDs")) + return nil, errors.Wrapf( + err, "loading CRDs %v", kt.kustomization.Crds) } err = ra.MergeConfig(crdTc) if err != nil { - errs.Append(errors.Wrap(err, "merge CRDs")) + return nil, errors.Wrapf( + err, "merging CRDs %v", crdTc) } - resMap, err := kt.generateConfigMapsAndSecrets(errs) + resMap, err := kt.generateConfigMapsAndSecrets() if err != nil { - errs.Append(errors.Wrap(err, "generateConfigMapsAndSecrets")) + return nil, errors.Wrap( + err, "generating legacy configMaps and secrets") } err = ra.MergeResourcesWithOverride(resMap) if err != nil { - return nil, err + return nil, errors.Wrap( + err, "merging legacy configMaps and secrets") } if kt.pluginConfig.GoEnabled { - kt.generateFromPlugins(ra, errs) - if len(errs.Get()) > 0 { - return ra, errs + err := kt.generateFromPlugins(ra) + if err != nil { + return nil, err } } patches, err := kt.rFactory.RF().SliceFromPatches( kt.ldr, kt.kustomization.PatchesStrategicMerge) if err != nil { - errs.Append(errors.Wrap(err, "SliceFromPatches")) - } - if len(errs.Get()) > 0 { - return nil, errs + return nil, errors.Wrapf( + err, "reading strategic merge patches %v", + kt.kustomization.PatchesStrategicMerge) } t, err := kt.newTransformer(patches, ra.GetTransformerConfig()) if err != nil { @@ -268,80 +265,97 @@ func (kt *KustTarget) AccumulateTarget() ( // nolint: gocyclo } func (kt *KustTarget) generateFromPlugins( - ra *accumulator.ResAccumulator, - errs *interror.KustomizationErrors) { + ra *accumulator.ResAccumulator) error { generators, err := kt.loadGeneratorPlugins() if err != nil { - errs.Append(err) - return + return errors.Wrap(err, "loading generator plugins") } for _, g := range generators { resMap, err := g.Generate() if err != nil { - errs.Append(err) - } else { - err = ra.MergeResourcesWithErrorOnIdCollision(resMap) - if err != nil { - errs.Append(errors.Wrap(err, "from plugin")) - } + return errors.Wrapf(err, "generating from %v", g) + } + err = ra.MergeResourcesWithErrorOnIdCollision(resMap) + if err != nil { + return errors.Wrapf(err, "merging from generator %v", g) } } + return nil } -func (kt *KustTarget) generateConfigMapsAndSecrets( - errs *interror.KustomizationErrors) (resmap.ResMap, error) { +func (kt *KustTarget) generateConfigMapsAndSecrets() (resmap.ResMap, error) { cms, err := kt.rFactory.NewResMapFromConfigMapArgs( kt.ldr, kt.kustomization.GeneratorOptions, kt.kustomization.ConfigMapGenerator) if err != nil { - errs.Append(errors.Wrap(err, "NewResMapFromConfigMapArgs")) + return nil, errors.Wrapf( + err, "configmapgenerator: %v", kt.kustomization.ConfigMapGenerator) } secrets, err := kt.rFactory.NewResMapFromSecretArgs( kt.ldr, kt.kustomization.GeneratorOptions, kt.kustomization.SecretGenerator) if err != nil { - errs.Append(errors.Wrap(err, "NewResMapFromSecretArgs")) + return nil, errors.Wrapf( + err, "secretgenerator: %v", kt.kustomization.SecretGenerator) } return resmap.MergeWithErrorOnIdCollision(cms, secrets) } -// accumulateBases returns a new ResAccumulator -// holding customized resources and the data/rules -// used to customized them from only the _bases_ -// of this KustTarget. -func (kt *KustTarget) accumulateBases() ( - ra *accumulator.ResAccumulator, errs *interror.KustomizationErrors) { - errs = &interror.KustomizationErrors{} - ra = accumulator.MakeEmptyAccumulator() - - for _, path := range kt.kustomization.Bases { +// accumulateResources fills the given resourceAccumulator +// with resources read from the given list of paths. +func (kt *KustTarget) accumulateResources( + ra *accumulator.ResAccumulator, paths []string) error { + for _, path := range paths { ldr, err := kt.ldr.New(path) - if err != nil { - errs.Append(errors.Wrap(err, "couldn't make loader for "+path)) - continue + if err == nil { + err = kt.accumulateDirectory(ra, ldr, path) + if err != nil { + return err + } + } else { + err = kt.accumulateFile(ra, path) + if err != nil { + return err + } } - subKt, err := NewKustTarget( - ldr, kt.rFactory, kt.tFactory, kt.pluginConfig) - if err != nil { - errs.Append(errors.Wrap(err, "couldn't make target for "+path)) - ldr.Cleanup() - continue - } - subRa, err := subKt.AccumulateTarget() - if err != nil { - errs.Append(errors.Wrap(err, "AccumulateTarget")) - ldr.Cleanup() - continue - } - err = ra.MergeAccumulator(subRa) - if err != nil { - errs.Append(errors.Wrap(err, path)) - } - ldr.Cleanup() } - return ra, errs + return nil +} + +func (kt *KustTarget) accumulateDirectory( + ra *accumulator.ResAccumulator, ldr ifc.Loader, path string) error { + defer ldr.Cleanup() + subKt, err := NewKustTarget( + ldr, kt.rFactory, kt.tFactory, kt.pluginConfig) + if err != nil { + return errors.Wrapf(err, "couldn't make target for path '%s'", path) + } + subRa, err := subKt.AccumulateTarget() + if err != nil { + return errors.Wrapf( + err, "recursed accumulation of path '%s'", path) + } + err = ra.MergeAccumulator(subRa) + if err != nil { + return errors.Wrapf( + err, "recursed merging from path '%s'", path) + } + return nil +} + +func (kt *KustTarget) accumulateFile( + ra *accumulator.ResAccumulator, path string) error { + resources, err := kt.rFactory.FromFile(kt.ldr, path) + if err != nil { + return errors.Wrapf(err, "accumulating resources from '%s'", path) + } + err = ra.MergeResourcesWithErrorOnIdCollision(resources) + if err != nil { + return errors.Wrapf(err, "merging resources from '%s'", path) + } + return nil } // newTransformer makes a Transformer that does a collection @@ -401,21 +415,21 @@ func (kt *KustTarget) newTransformer( } func (kt *KustTarget) loadTransformerPlugins() ([]transformers.Transformer, error) { - configs, err := kt.rFactory.FromFiles( - kt.ldr, kt.kustomization.Transformers) + ra := accumulator.MakeEmptyAccumulator() + err := kt.accumulateResources(ra, kt.kustomization.Transformers) if err != nil { return nil, err } return plugins.NewTransformerLoader( - kt.pluginConfig, kt.ldr, kt.rFactory).Load(configs) + kt.pluginConfig, kt.ldr, kt.rFactory).Load(ra.ResMap()) } func (kt *KustTarget) loadGeneratorPlugins() ([]transformers.Generator, error) { - configs, err := kt.rFactory.FromFiles( - kt.ldr, kt.kustomization.Generators) + ra := accumulator.MakeEmptyAccumulator() + err := kt.accumulateResources(ra, kt.kustomization.Generators) if err != nil { return nil, err } return plugins.NewGeneratorLoader( - kt.pluginConfig, kt.ldr, kt.rFactory).Load(configs) + kt.pluginConfig, kt.ldr, kt.rFactory).Load(ra.ResMap()) }