Delete some code.

Author: monopole

pkg/app/application.go

@@ -26,6 +26,7 @@ import (
 	"github.com/ghodss/yaml"
 	"github.com/golang/glog"
 
+	"github.com/kubernetes-sigs/kustomize/pkg/configmapandsecret"
 	"github.com/kubernetes-sigs/kustomize/pkg/constants"
 	"github.com/kubernetes-sigs/kustomize/pkg/crds"
 	"github.com/kubernetes-sigs/kustomize/pkg/fs"
@@ -44,13 +45,13 @@ import (
 // https://github.com/kubernetes-sigs/kustomize/blob/master/docs/glossary.md#target
 type Application struct {
 	kustomization *types.Kustomization
-	loader        loader.Loader
+	ldr           loader.Loader
 	fSys          fs.FileSystem
 }
 
 // NewApplication returns a new instance of Application primed with a Loader.
-func NewApplication(loader loader.Loader, fSys fs.FileSystem) (*Application, error) {
-	content, err := loader.Load(constants.KustomizationFileName)
+func NewApplication(ldr loader.Loader, fSys fs.FileSystem) (*Application, error) {
+	content, err := ldr.Load(constants.KustomizationFileName)
 	if err != nil {
 		return nil, err
 	}
@@ -60,7 +61,8 @@ func NewApplication(loader loader.Loader, fSys fs.FileSystem) (*Application, err
 	if err != nil {
 		return nil, err
 	}
-	return &Application{kustomization: &m, loader: loader, fSys: fSys}, nil
+
+	return &Application{kustomization: &m, ldr: ldr, fSys: fSys}, nil
 }
 
 func unmarshal(y []byte, o interface{}) error {
@@ -136,18 +138,19 @@ func (a *Application) loadCustomizedResMap() (resmap.ResMap, error) {
 	if err != nil {
 		errs.Append(errors.Wrap(err, "loadResMapFromBasesAndResources"))
 	}
-	err = crds.RegisterCRDs(a.loader, a.kustomization.CRDs)
+	err = crds.RegisterCRDs(a.ldr, a.kustomization.CRDs)
 	if err != nil {
 		errs.Append(errors.Wrap(err, "RegisterCRDs"))
 	}
-
 	cms, err := resmap.NewResMapFromConfigMapArgs(
-		a.loader, a.fSys, a.kustomization.ConfigMapGenerator)
+		configmapandsecret.NewConfigMapFactory(a.fSys, a.ldr),
+		a.kustomization.ConfigMapGenerator)
 	if err != nil {
 		errs.Append(errors.Wrap(err, "NewResMapFromConfigMapArgs"))
 	}
 	secrets, err := resmap.NewResMapFromSecretArgs(
-		a.loader.Root(), a.fSys, a.kustomization.SecretGenerator)
+		configmapandsecret.NewSecretFactory(a.fSys, a.ldr.Root()),
+		a.kustomization.SecretGenerator)
 	if err != nil {
 		errs.Append(errors.Wrap(err, "NewResMapFromSecretArgs"))
 	}
@@ -161,7 +164,7 @@ func (a *Application) loadCustomizedResMap() (resmap.ResMap, error) {
 		return nil, err
 	}
 
-	patches, err := resmap.NewResourceSliceFromPatches(a.loader, a.kustomization.Patches)
+	patches, err := resmap.NewResourceSliceFromPatches(a.ldr, a.kustomization.Patches)
 	if err != nil {
 		errs.Append(errors.Wrap(err, "NewResourceSliceFromPatches"))
 	}
@@ -192,7 +195,7 @@ func (a *Application) loadCustomizedResMap() (resmap.ResMap, error) {
 // Gets Bases and Resources as advertised.
 func (a *Application) loadResMapFromBasesAndResources() (resmap.ResMap, error) {
 	bases, errs := a.loadCustomizedBases()
-	resources, err := resmap.NewResMapFromFiles(a.loader, a.kustomization.Resources)
+	resources, err := resmap.NewResMapFromFiles(a.ldr, a.kustomization.Resources)
 	if err != nil {
 		errs.Append(errors.Wrap(err, "rawResources failed to read Resources"))
 	}
@@ -208,7 +211,7 @@ func (a *Application) loadCustomizedBases() (resmap.ResMap, *interror.Kustomizat
 	var list []resmap.ResMap
 	errs := &interror.KustomizationErrors{}
 	for _, path := range a.kustomization.Bases {
-		ldr, err := a.loader.New(path)
+		ldr, err := a.ldr.New(path)
 		if err != nil {
 			errs.Append(errors.Wrap(err, "couldn't make ldr for "+path))
 			continue
@@ -236,7 +239,7 @@ func (a *Application) loadBasesAsFlatList() ([]*Application, error) {
 	var result []*Application
 	errs := &interror.KustomizationErrors{}
 	for _, path := range a.kustomization.Bases {
-		ldr, err := a.loader.New(path)
+		ldr, err := a.ldr.New(path)
 		if err != nil {
 			errs.Append(err)
 			continue

pkg/commands/configmap.go

@@ -59,9 +59,10 @@ func newCmdAddConfigMap(fSys fs.FileSystem) *cobra.Command {
 			if err != nil {
 				return err
 			}
-
 			// Add the flagsAndArgs map to the kustomization file.
-			err = addConfigMap(kustomization, flagsAndArgs, fSys)
+			err = addConfigMap(
+				kustomization, flagsAndArgs,
+				configmapandsecret.NewConfigMapFactory(fSys, nil))
 			if err != nil {
 				return err
 			}
@@ -93,24 +94,22 @@ func newCmdAddConfigMap(fSys fs.FileSystem) *cobra.Command {
 }
 
 // addConfigMap adds a configmap to a kustomization file.
-// Note: error may leave kustomization file in an undefined state. Suggest passing a copy
-// of kustomization file.
-func addConfigMap(k *types.Kustomization, flagsAndArgs cMapFlagsAndArgs, fSys fs.FileSystem) error {
+// Note: error may leave kustomization file in an undefined state.
+// Suggest passing a copy of kustomization file.
+func addConfigMap(
+	k *types.Kustomization,
+	flagsAndArgs cMapFlagsAndArgs,
+	factory *configmapandsecret.ConfigMapFactory) error {
 	cmArgs := makeConfigMapArgs(k, flagsAndArgs.Name)
-
 	err := mergeFlagsIntoCmArgs(&cmArgs.DataSources, flagsAndArgs)
 	if err != nil {
 		return err
 	}
-
-	factory := configmapandsecret.NewConfigMapFactory(cmArgs, nil, fSys)
-
 	// Validate by trying to create corev1.configmap.
-	_, _, err = factory.MakeUnstructAndGenerateName()
+	_, _, err = factory.MakeUnstructAndGenerateName(cmArgs)
 	if err != nil {
 		return err
 	}
-
 	return nil
 }
 

pkg/configmapandsecret/configmapfactory.go

@@ -19,7 +19,6 @@ package configmapandsecret
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"path"
@@ -29,6 +28,7 @@ import (
 	"github.com/kubernetes-sigs/kustomize/pkg/hash"
 	"github.com/kubernetes-sigs/kustomize/pkg/loader"
 	"github.com/kubernetes-sigs/kustomize/pkg/types"
+	"github.com/pkg/errors"
 	"k8s.io/api/core/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -38,22 +38,20 @@ import (
 
 // ConfigMapFactory makes ConfigMaps.
 type ConfigMapFactory struct {
-	args *types.ConfigMapArgs
 	fSys fs.FileSystem
 	ldr  loader.Loader
 }
 
 // NewConfigMapFactory returns a new ConfigMapFactory.
 func NewConfigMapFactory(
-	args *types.ConfigMapArgs,
-	l loader.Loader,
-	fSys fs.FileSystem) *ConfigMapFactory {
-	return &ConfigMapFactory{args: args, ldr: l, fSys: fSys}
+	fSys fs.FileSystem, l loader.Loader) *ConfigMapFactory {
+	return &ConfigMapFactory{fSys: fSys, ldr: l}
 }
 
 // MakeUnstructAndGenerateName returns an configmap and the name appended with a hash.
-func (f *ConfigMapFactory) MakeUnstructAndGenerateName() (*unstructured.Unstructured, string, error) {
-	cm, err := f.MakeConfigMap1()
+func (f *ConfigMapFactory) MakeUnstructAndGenerateName(
+	args *types.ConfigMapArgs) (*unstructured.Unstructured, string, error) {
+	cm, err := f.MakeConfigMap1(args)
 	if err != nil {
 		return nil, "", err
 	}
@@ -76,31 +74,32 @@ func objectToUnstructured(in runtime.Object) (*unstructured.Unstructured, error)
 	return &out, err
 }
 
-func (f *ConfigMapFactory) makeFreshConfigMap() *corev1.ConfigMap {
+func (f *ConfigMapFactory) makeFreshConfigMap(
+	args *types.ConfigMapArgs) *corev1.ConfigMap {
 	cm := &corev1.ConfigMap{}
 	cm.APIVersion = "v1"
 	cm.Kind = "ConfigMap"
-	cm.Name = f.args.Name
+	cm.Name = args.Name
 	cm.Data = map[string]string{}
 	return cm
 }
 
 // MakeConfigMap1 returns a new ConfigMap, or nil and an error.
-func (f *ConfigMapFactory) MakeConfigMap1() (*corev1.ConfigMap, error) {
-	cm := f.makeFreshConfigMap()
-
-	if f.args.EnvSource != "" {
-		if err := f.handleConfigMapFromEnvFileSource(cm); err != nil {
+func (f *ConfigMapFactory) MakeConfigMap1(
+	args *types.ConfigMapArgs) (*corev1.ConfigMap, error) {
+	cm := f.makeFreshConfigMap(args)
+	if args.EnvSource != "" {
+		if err := f.handleConfigMapFromEnvFileSource(cm, args); err != nil {
 			return nil, err
 		}
 	}
-	if f.args.FileSources != nil {
-		if err := f.handleConfigMapFromFileSources(cm); err != nil {
+	if args.FileSources != nil {
+		if err := f.handleConfigMapFromFileSources(cm, args); err != nil {
 			return nil, err
 		}
 	}
-	if f.args.LiteralSources != nil {
-		if err := f.handleConfigMapFromLiteralSources(cm); err != nil {
+	if args.LiteralSources != nil {
+		if err := f.handleConfigMapFromLiteralSources(cm, args.LiteralSources); err != nil {
 			return nil, err
 		}
 	}
@@ -109,40 +108,38 @@ func (f *ConfigMapFactory) MakeConfigMap1() (*corev1.ConfigMap, error) {
 
 // MakeConfigMap2 returns a new ConfigMap, or nil and an error.
 // TODO: Get rid of the nearly duplicated code in MakeConfigMap1 vs MakeConfigMap2
-func (f *ConfigMapFactory) MakeConfigMap2() (*corev1.ConfigMap, error) {
-	var envPairs, literalPairs, filePairs []kvPair
+func (f *ConfigMapFactory) MakeConfigMap2(
+	args *types.ConfigMapArgs) (*corev1.ConfigMap, error) {
+	var all []kvPair
 	var err error
+	cm := f.makeFreshConfigMap(args)
 
-	cm := f.makeFreshConfigMap()
-
-	if f.args.EnvSource != "" {
-		envPairs, err = keyValuesFromEnvFile(f.ldr, f.args.EnvSource)
-		if err != nil {
-			return nil, fmt.Errorf(
-				"error reading keys from env source file: %s %v",
-				f.args.EnvSource, err)
-		}
+	pairs, err := keyValuesFromEnvFile(f.ldr, args.EnvSource)
+	if err != nil {
+		return nil, errors.Wrap(err, fmt.Sprintf(
+			"env source file: %s",
+			args.EnvSource))
 	}
+	all = append(all, pairs...)
 
-	literalPairs, err = keyValuesFromLiteralSources(f.args.LiteralSources)
+	pairs, err = keyValuesFromLiteralSources(args.LiteralSources)
 	if err != nil {
-		return nil, fmt.Errorf(
-			"error reading key values from literal sources: %v", err)
+		return nil, errors.Wrap(err, fmt.Sprintf(
+			"literal sources %v", args.LiteralSources))
 	}
+	all = append(all, pairs...)
 
-	filePairs, err = keyValuesFromFileSources(f.ldr, f.args.FileSources)
+	pairs, err = keyValuesFromFileSources(f.ldr, args.FileSources)
 	if err != nil {
-		return nil, fmt.Errorf(
-			"error reading key values from file sources: %v", err)
+		return nil, errors.Wrap(err, fmt.Sprintf(
+			"file sources: %v", args.FileSources))
 	}
+	all = append(all, pairs...)
 
-	allPairs := append(append(envPairs, literalPairs...), filePairs...)
-
-	// merge key value pairs from all the sources
-	for _, kv := range allPairs {
-		err = addKV(cm.Data, kv)
+	for _, kv := range all {
+		err = addKeyFromLiteralToConfigMap(cm, kv.key, kv.value)
 		if err != nil {
-			return nil, fmt.Errorf("error adding key in configmap: %v", err)
+			return nil, err
 		}
 	}
 	return cm, nil
@@ -163,13 +160,13 @@ func keyValuesFromLiteralSources(sources []string) ([]kvPair, error) {
 // handleConfigMapFromLiteralSources adds the specified literal source
 // information into the provided configMap.
 func (f *ConfigMapFactory) handleConfigMapFromLiteralSources(
-	configMap *v1.ConfigMap) error {
-	for _, literalSource := range f.args.LiteralSources {
-		keyName, value, err := ParseLiteralSource(literalSource)
+	configMap *v1.ConfigMap, sources []string) error {
+	for _, s := range sources {
+		k, v, err := ParseLiteralSource(s)
 		if err != nil {
 			return err
 		}
-		err = addKeyFromLiteralToConfigMap(configMap, keyName, value)
+		err = addKeyFromLiteralToConfigMap(configMap, k, v)
 		if err != nil {
 			return err
 		}
@@ -195,8 +192,9 @@ func keyValuesFromFileSources(ldr loader.Loader, sources []string) ([]kvPair, er
 
 // handleConfigMapFromFileSources adds the specified file source information
 // into the provided configMap
-func (f *ConfigMapFactory) handleConfigMapFromFileSources(configMap *v1.ConfigMap) error {
-	for _, fileSource := range f.args.FileSources {
+func (f *ConfigMapFactory) handleConfigMapFromFileSources(
+	configMap *v1.ConfigMap, args *types.ConfigMapArgs) error {
+	for _, fileSource := range args.FileSources {
 		keyName, filePath, err := ParseFileSource(fileSource)
 		if err != nil {
 			return err
@@ -232,6 +230,9 @@ func (f *ConfigMapFactory) handleConfigMapFromFileSources(configMap *v1.ConfigMa
 }
 
 func keyValuesFromEnvFile(l loader.Loader, path string) ([]kvPair, error) {
+	if path == "" {
+		return nil, nil
+	}
 	content, err := l.Load(path)
 	if err != nil {
 		return nil, err
@@ -241,14 +242,15 @@ func keyValuesFromEnvFile(l loader.Loader, path string) ([]kvPair, error) {
 
 // HandleConfigMapFromEnvFileSource adds the specified env file source information
 // into the provided configMap
-func (f *ConfigMapFactory) handleConfigMapFromEnvFileSource(configMap *v1.ConfigMap) error {
-	if !f.fSys.Exists(f.args.EnvSource) {
-		return fmt.Errorf("unable to read configmap env file %s", f.args.EnvSource)
+func (f *ConfigMapFactory) handleConfigMapFromEnvFileSource(
+	configMap *v1.ConfigMap, args *types.ConfigMapArgs) error {
+	if !f.fSys.Exists(args.EnvSource) {
+		return fmt.Errorf("unable to read configmap env file %s", args.EnvSource)
 	}
-	if f.fSys.IsDir(f.args.EnvSource) {
-		return fmt.Errorf("env config file %s cannot be a directory", f.args.EnvSource)
+	if f.fSys.IsDir(args.EnvSource) {
+		return fmt.Errorf("env config file %s cannot be a directory", args.EnvSource)
 	}
-	return addFromEnvFile(f.args.EnvSource, func(key, value string) error {
+	return addFromEnvFile(args.EnvSource, func(key, value string) error {
 		return addKeyFromLiteralToConfigMap(configMap, key, value)
 	})
 }
@@ -318,18 +320,3 @@ func ParseLiteralSource(source string) (keyName, value string, err error) {
 
 	return items[0], items[1], nil
 }
-
-// addKV adds key-value pair to the provided map.
-func addKV(m map[string]string, kv kvPair) error {
-	if errs := validation.IsConfigMapKey(kv.key); len(errs) != 0 {
-		return fmt.Errorf(
-			"%q is not a valid key name: %s",
-			kv.key, strings.Join(errs, ";"))
-	}
-	if _, exists := m[kv.key]; exists {
-		return fmt.Errorf(
-			"key %s already exists: %v", kv.key, m)
-	}
-	m[kv.key] = kv.value
-	return nil
-}

pkg/configmapandsecret/configmapfactory_test.go

@@ -133,11 +133,10 @@ func TestConstructConfigMap(t *testing.T) {
 		},
 	}
 
+	// TODO: all tests should use a FakeFs
+	f := NewConfigMapFactory(fs.MakeRealFS(), nil)
 	for _, tc := range testCases {
-		// TODO: all tests should use a FakeFs
-		fSys := fs.MakeRealFS()
-		f := NewConfigMapFactory(&tc.input, nil, fSys)
-		cm, err := f.MakeConfigMap1()
+		cm, err := f.MakeConfigMap1(&tc.input)
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}