Delete kustomizationerror.

Do a longstanding TODO to remove kustomizationerror.

It wasn't used much, and it wasn't used consistently,
because it's complicated to decided when it's worth
proceeding to accumulate errors when one already knows
that one has a fatal error in the kustomization.  Its
use was blocking refactoring for simplicity and making
tests harder to write.

Removing it lets us reinstate the cyclomatic complexity
check in KustTarget.

Also added more info to the affected error messages.

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

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=

pkg/internal/error/kustomizationerror.go

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

pkg/internal/error/kustomizationerror_test.go

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

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

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

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())
 }