configMap factory refactor for #86

Author: monopole

pkg/commands/data_config.go renamed to pkg/commands/cmapflagsandargs.go

@@ -20,8 +20,8 @@ import (
 	"fmt"
 )
 
-// dataConfig encapsulates the options for add configmap/Secret commands.
-type dataConfig struct {
+// cMapFlagsAndArgs encapsulates the options for add configmap commands.
+type cMapFlagsAndArgs struct {
 	// Name of configMap/Secret (required)
 	Name string
 	// FileSources to derive the configMap/Secret from (optional)
@@ -34,7 +34,7 @@ type dataConfig struct {
 }
 
 // Validate validates required fields are set to support structured generation.
-func (a *dataConfig) Validate(args []string) error {
+func (a *cMapFlagsAndArgs) Validate(args []string) error {
 	if len(args) != 1 {
 		return fmt.Errorf("name must be specified once")
 	}

pkg/commands/data_config_test.go renamed to pkg/commands/cmapflagsandargs_test.go

@@ -21,15 +21,15 @@ import (
 )
 
 func TestDataConfigValidation_NoName(t *testing.T) {
-	config := dataConfig{}
+	config := cMapFlagsAndArgs{}
 
 	if config.Validate([]string{}) == nil {
 		t.Fatal("Validation should fail if no name is specified")
 	}
 }
 
 func TestDataConfigValidation_MoreThanOneName(t *testing.T) {
-	config := dataConfig{}
+	config := cMapFlagsAndArgs{}
 
 	if config.Validate([]string{"name", "othername"}) == nil {
 		t.Fatal("Validation should fail if more than one name is specified")
@@ -39,33 +39,33 @@ func TestDataConfigValidation_MoreThanOneName(t *testing.T) {
 func TestDataConfigValidation_Flags(t *testing.T) {
 	tests := []struct {
 		name       string
-		config     dataConfig
+		config     cMapFlagsAndArgs
 		shouldFail bool
 	}{
 		{
 			name: "env-file-source and literal are both set",
-			config: dataConfig{
+			config: cMapFlagsAndArgs{
 				LiteralSources: []string{"one", "two"},
 				EnvFileSource:  "three",
 			},
 			shouldFail: true,
 		},
 		{
 			name: "env-file-source and from-file are both set",
-			config: dataConfig{
+			config: cMapFlagsAndArgs{
 				FileSources:   []string{"one", "two"},
 				EnvFileSource: "three",
 			},
 			shouldFail: true,
 		},
 		{
 			name:       "we don't have any option set",
-			config:     dataConfig{},
+			config:     cMapFlagsAndArgs{},
 			shouldFail: true,
 		},
 		{
 			name: "we have from-file and literal ",
-			config: dataConfig{
+			config: cMapFlagsAndArgs{
 				LiteralSources: []string{"one", "two"},
 				FileSources:    []string{"three", "four"},
 			},

pkg/commands/configmap.go

@@ -27,8 +27,8 @@ import (
 	"github.com/kubernetes-sigs/kustomize/pkg/types"
 )
 
-func newCmdAddConfigMap(fsys fs.FileSystem) *cobra.Command {
-	var config dataConfig
+func newCmdAddConfigMap(fSys fs.FileSystem) *cobra.Command {
+	var flagsAndArgs cMapFlagsAndArgs
 	cmd := &cobra.Command{
 		Use:   "configmap NAME [--from-file=[key=]source] [--from-literal=key1=value1]",
 		Short: "Adds a configmap to the kustomization file.",
@@ -44,75 +44,77 @@ func newCmdAddConfigMap(fsys fs.FileSystem) *cobra.Command {
 	kustomize edit add configmap my-configmap --from-env-file=env/path.env
 `,
 		RunE: func(_ *cobra.Command, args []string) error {
-			err := config.Validate(args)
+			err := flagsAndArgs.Validate(args)
 			if err != nil {
 				return err
 			}
 
-			// Load in the kustomization file.
-			mf, err := newKustomizationFile(constants.KustomizationFileName, fsys)
+			// Load the kustomization file.
+			mf, err := newKustomizationFile(constants.KustomizationFileName, fSys)
 			if err != nil {
 				return err
 			}
 
-			m, err := mf.read()
+			kustomization, err := mf.read()
 			if err != nil {
 				return err
 			}
 
-			// Add the config map to the kustomization file.
-			err = addConfigMap(m, config)
+			// Add the flagsAndArgs map to the kustomization file.
+			err = addConfigMap(kustomization, flagsAndArgs, fSys)
 			if err != nil {
 				return err
 			}
 
 			// Write out the kustomization file with added configmap.
-			return mf.write(m)
+			return mf.write(kustomization)
 		},
 	}
 
 	cmd.Flags().StringSliceVar(
-		&config.FileSources,
+		&flagsAndArgs.FileSources,
 		"from-file",
 		[]string{},
 		"Key file can be specified using its file path, in which case file basename will be used as configmap "+
 			"key, or optionally with a key and file path, in which case the given key will be used.  Specifying a "+
 			"directory will iterate each named file in the directory whose basename is a valid configmap key.")
 	cmd.Flags().StringArrayVar(
-		&config.LiteralSources,
+		&flagsAndArgs.LiteralSources,
 		"from-literal",
 		[]string{},
 		"Specify a key and literal value to insert in configmap (i.e. mykey=somevalue)")
 	cmd.Flags().StringVar(
-		&config.EnvFileSource,
+		&flagsAndArgs.EnvFileSource,
 		"from-env-file",
 		"",
 		"Specify the path to a file to read lines of key=val pairs to create a configmap (i.e. a Docker .env file).")
 
 	return cmd
 }
 
-// addConfigMap updates a configmap within a kustomization file, using the data in config.
+// 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(m *types.Kustomization, config dataConfig) error {
-	cm := getOrCreateConfigMap(m, config.Name)
+func addConfigMap(k *types.Kustomization, flagsAndArgs cMapFlagsAndArgs, fSys fs.FileSystem) error {
+	cmArgs := makeConfigMapArgs(k, flagsAndArgs.Name)
 
-	err := mergeData(&cm.DataSources, config)
+	err := mergeFlagsIntoCmArgs(&cmArgs.DataSources, flagsAndArgs)
 	if err != nil {
 		return err
 	}
 
+	factory := configmapandsecret.NewConfigMapFactory(cmArgs, fSys)
+
 	// Validate by trying to create corev1.configmap.
-	_, _, err = configmapandsecret.MakeConfigmapAndGenerateName(*cm)
+	_, _, err = factory.MakeUnstructAndGenerateName()
 	if err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func getOrCreateConfigMap(m *types.Kustomization, name string) *types.ConfigMapArgs {
+func makeConfigMapArgs(m *types.Kustomization, name string) *types.ConfigMapArgs {
 	for i, v := range m.ConfigMapGenerator {
 		if name == v.Name {
 			return &m.ConfigMapGenerator[i]
@@ -124,13 +126,12 @@ func getOrCreateConfigMap(m *types.Kustomization, name string) *types.ConfigMapA
 	return &m.ConfigMapGenerator[len(m.ConfigMapGenerator)-1]
 }
 
-func mergeData(src *types.DataSources, config dataConfig) error {
-	src.LiteralSources = append(src.LiteralSources, config.LiteralSources...)
-	src.FileSources = append(src.FileSources, config.FileSources...)
-	if src.EnvSource != "" && src.EnvSource != config.EnvFileSource {
+func mergeFlagsIntoCmArgs(src *types.DataSources, flags cMapFlagsAndArgs) error {
+	src.LiteralSources = append(src.LiteralSources, flags.LiteralSources...)
+	src.FileSources = append(src.FileSources, flags.FileSources...)
+	if src.EnvSource != "" && src.EnvSource != flags.EnvFileSource {
 		return fmt.Errorf("updating existing env source '%s' not allowed", src.EnvSource)
 	}
-	src.EnvSource = config.EnvFileSource
-
+	src.EnvSource = flags.EnvFileSource
 	return nil
 }

pkg/commands/configmap_test.go

@@ -29,7 +29,7 @@ func TestNewAddConfigMapIsNotNil(t *testing.T) {
 	}
 }
 
-func TestGetOrCreateConfigMap(t *testing.T) {
+func TestMakeConfigMapArgs(t *testing.T) {
 	cmName := "test-config-name"
 
 	kustomization := &types.Kustomization{
@@ -39,35 +39,35 @@ func TestGetOrCreateConfigMap(t *testing.T) {
 	if len(kustomization.ConfigMapGenerator) != 0 {
 		t.Fatal("Initial kustomization should not have any configmaps")
 	}
-	cm := getOrCreateConfigMap(kustomization, cmName)
+	args := makeConfigMapArgs(kustomization, cmName)
 
-	if cm == nil {
-		t.Fatalf("ConfigMap should always be non-nil")
+	if args == nil {
+		t.Fatalf("args should always be non-nil")
 	}
 
 	if len(kustomization.ConfigMapGenerator) != 1 {
 		t.Fatalf("Kustomization should have newly created configmap")
 	}
 
-	if &kustomization.ConfigMapGenerator[len(kustomization.ConfigMapGenerator)-1] != cm {
-		t.Fatalf("Pointer address for newly inserted configmap should be same")
+	if &kustomization.ConfigMapGenerator[len(kustomization.ConfigMapGenerator)-1] != args {
+		t.Fatalf("Pointer address for newly inserted configmap generator should be same")
 	}
 
-	existingCM := getOrCreateConfigMap(kustomization, cmName)
+	args2 := makeConfigMapArgs(kustomization, cmName)
 
-	if existingCM != cm {
-		t.Fatalf("should have returned an existing cm with name: %v", cmName)
+	if args2 != args {
+		t.Fatalf("should have returned an existing args with name: %v", cmName)
 	}
 
 	if len(kustomization.ConfigMapGenerator) != 1 {
 		t.Fatalf("Should not insert configmap for an existing name: %v", cmName)
 	}
 }
 
-func TestMergeData_LiteralSources(t *testing.T) {
+func TestMergeFlagsIntoCmArgs_LiteralSources(t *testing.T) {
 	ds := &types.DataSources{}
 
-	err := mergeData(ds, dataConfig{LiteralSources: []string{"k1=v1"}})
+	err := mergeFlagsIntoCmArgs(ds, cMapFlagsAndArgs{LiteralSources: []string{"k1=v1"}})
 	if err != nil {
 		t.Fatalf("Merge initial literal source should not return error")
 	}
@@ -76,7 +76,7 @@ func TestMergeData_LiteralSources(t *testing.T) {
 		t.Fatalf("Initial literal source should have been added")
 	}
 
-	err = mergeData(ds, dataConfig{LiteralSources: []string{"k2=v2"}})
+	err = mergeFlagsIntoCmArgs(ds, cMapFlagsAndArgs{LiteralSources: []string{"k2=v2"}})
 	if err != nil {
 		t.Fatalf("Merge second literal source should not return error")
 	}
@@ -86,10 +86,10 @@ func TestMergeData_LiteralSources(t *testing.T) {
 	}
 }
 
-func TestMergeData_FileSources(t *testing.T) {
+func TestMergeFlagsIntoCmArgs_FileSources(t *testing.T) {
 	ds := &types.DataSources{}
 
-	err := mergeData(ds, dataConfig{FileSources: []string{"file1"}})
+	err := mergeFlagsIntoCmArgs(ds, cMapFlagsAndArgs{FileSources: []string{"file1"}})
 	if err != nil {
 		t.Fatalf("Merge initial file source should not return error")
 	}
@@ -98,7 +98,7 @@ func TestMergeData_FileSources(t *testing.T) {
 		t.Fatalf("Initial file source should have been added")
 	}
 
-	err = mergeData(ds, dataConfig{FileSources: []string{"file2"}})
+	err = mergeFlagsIntoCmArgs(ds, cMapFlagsAndArgs{FileSources: []string{"file2"}})
 	if err != nil {
 		t.Fatalf("Merge second file source should not return error")
 	}
@@ -108,12 +108,12 @@ func TestMergeData_FileSources(t *testing.T) {
 	}
 }
 
-func TestMergeData_EnvSource(t *testing.T) {
+func TestMergeFlagsIntoCmArgs_EnvSource(t *testing.T) {
 	envFileName := "env1"
 	envFileName2 := "env2"
 	ds := &types.DataSources{}
 
-	err := mergeData(ds, dataConfig{EnvFileSource: envFileName})
+	err := mergeFlagsIntoCmArgs(ds, cMapFlagsAndArgs{EnvFileSource: envFileName})
 	if err != nil {
 		t.Fatalf("Merge initial env source should not return error")
 	}
@@ -122,7 +122,7 @@ func TestMergeData_EnvSource(t *testing.T) {
 		t.Fatalf("Initial env source filename should have been added")
 	}
 
-	err = mergeData(ds, dataConfig{EnvFileSource: envFileName2})
+	err = mergeFlagsIntoCmArgs(ds, cMapFlagsAndArgs{EnvFileSource: envFileName2})
 	if err == nil {
 		t.Fatalf("Updating env source should return an error")
 	}