Merge pull request #1002 from monopole/deleteKustomizationError

Delete kustomizationerror.

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