From 99bc2b077fd208885e76281ab8865f1e85f0abb8 Mon Sep 17 00:00:00 2001 From: Chok Yip Lau Date: Thu, 29 Apr 2021 19:16:59 -0400 Subject: [PATCH] Added support for multiple --from-env flags --- .../pkg/cmd/create/create_configmap.go | 56 +++--- .../pkg/cmd/create/create_configmap_test.go | 166 +++++++++++------- 2 files changed, 134 insertions(+), 88 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap.go index 690d85222c9..5fba15e54b3 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap.go @@ -69,7 +69,7 @@ var ( kubectl create configmap my-config --from-file=path/to/bar # Create a new config map named my-config from an env file - kubectl create configmap my-config --from-env-file=path/to/bar.env`)) + kubectl create configmap my-config --from-env-file=path/to/foo.env --from-env-file=path/to/bar.env`)) ) // ConfigMapOptions holds properties for create configmap sub-command @@ -86,8 +86,8 @@ type ConfigMapOptions struct { FileSources []string // LiteralSources to derive the configMap from (optional) LiteralSources []string - // EnvFileSource to derive the configMap from (optional) - EnvFileSource string + // EnvFileSources to derive the configMap from (optional) + EnvFileSources []string // AppendHash; if true, derive a hash from the ConfigMap and append it to the name AppendHash bool @@ -106,10 +106,8 @@ type ConfigMapOptions struct { // NewConfigMapOptions creates a new *ConfigMapOptions with default value func NewConfigMapOptions(ioStreams genericclioptions.IOStreams) *ConfigMapOptions { return &ConfigMapOptions{ - FileSources: []string{}, - LiteralSources: []string{}, - PrintFlags: genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme), - IOStreams: ioStreams, + PrintFlags: genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme), + IOStreams: ioStreams, } } @@ -138,7 +136,7 @@ func NewCmdCreateConfigMap(f cmdutil.Factory, ioStreams genericclioptions.IOStre cmd.Flags().StringSliceVar(&o.FileSources, "from-file", o.FileSources, "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(&o.LiteralSources, "from-literal", o.LiteralSources, "Specify a key and literal value to insert in configmap (i.e. mykey=somevalue)") - cmd.Flags().StringVar(&o.EnvFileSource, "from-env-file", o.EnvFileSource, "Specify the path to a file to read lines of key=val pairs to create a configmap (i.e. a Docker .env file).") + cmd.Flags().StringSliceVar(&o.EnvFileSources, "from-env-file", o.EnvFileSources, "Specify the path to a file to read lines of key=val pairs to create a configmap (i.e. a Docker .env file).") cmd.Flags().BoolVar(&o.AppendHash, "append-hash", o.AppendHash, "Append a hash of the configmap to its name.") cmdutil.AddFieldManagerFlagVar(cmd, &o.FieldManager, "kubectl-create") @@ -205,7 +203,7 @@ func (o *ConfigMapOptions) Validate() error { if len(o.Name) == 0 { return fmt.Errorf("name must be specified") } - if len(o.EnvFileSource) > 0 && (len(o.FileSources) > 0 || len(o.LiteralSources) > 0) { + if len(o.EnvFileSources) > 0 && (len(o.FileSources) > 0 || len(o.LiteralSources) > 0) { return fmt.Errorf("from-env-file cannot be combined with from-file or from-literal") } return nil @@ -272,8 +270,8 @@ func (o *ConfigMapOptions) createConfigMap() (*corev1.ConfigMap, error) { return nil, err } } - if len(o.EnvFileSource) > 0 { - if err := handleConfigMapFromEnvFileSource(configMap, o.EnvFileSource); err != nil { + if len(o.EnvFileSources) > 0 { + if err := handleConfigMapFromEnvFileSources(configMap, o.EnvFileSources); err != nil { return nil, err } } @@ -351,25 +349,31 @@ func handleConfigMapFromFileSources(configMap *corev1.ConfigMap, fileSources []s return nil } -// handleConfigMapFromEnvFileSource adds the specified env file source information +// handleConfigMapFromEnvFileSources adds the specified env file source information // into the provided configMap -func handleConfigMapFromEnvFileSource(configMap *corev1.ConfigMap, envFileSource string) error { - info, err := os.Stat(envFileSource) - if err != nil { - switch err := err.(type) { - case *os.PathError: - return fmt.Errorf("error reading %s: %v", envFileSource, err.Err) - default: - return fmt.Errorf("error reading %s: %v", envFileSource, err) +func handleConfigMapFromEnvFileSources(configMap *corev1.ConfigMap, envFileSources []string) error { + for _, envFileSource := range envFileSources { + info, err := os.Stat(envFileSource) + if err != nil { + switch err := err.(type) { + case *os.PathError: + return fmt.Errorf("error reading %s: %v", envFileSource, err.Err) + default: + return fmt.Errorf("error reading %s: %v", envFileSource, err) + } + } + if info.IsDir() { + return fmt.Errorf("env config file cannot be a directory") + } + err = cmdutil.AddFromEnvFile(envFileSource, func(key, value string) error { + return addKeyFromLiteralToConfigMap(configMap, key, value) + }) + if err != nil { + return err } } - if info.IsDir() { - return fmt.Errorf("env config file cannot be a directory") - } - return cmdutil.AddFromEnvFile(envFileSource, func(key, value string) error { - return addKeyFromLiteralToConfigMap(configMap, key, value) - }) + return nil } // addKeyFromFileToConfigMap adds a key with the given name to a ConfigMap, populating diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap_test.go index e3c6c92c0a4..aa4b003ac4c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_configmap_test.go @@ -33,11 +33,11 @@ func TestCreateConfigMap(t *testing.T) { appendHash bool fromLiteral []string fromFile []string - fromEnvFile string + fromEnvFile []string setup func(t *testing.T, configMapOptions *ConfigMapOptions) func() expected *corev1.ConfigMap - expectErr bool + expectErr string }{ "create_foo_configmap": { configMapName: "foo", @@ -52,7 +52,6 @@ func TestCreateConfigMap(t *testing.T) { Data: map[string]string{}, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_hash_configmap": { configMapName: "foo", @@ -68,7 +67,6 @@ func TestCreateConfigMap(t *testing.T) { Data: map[string]string{}, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_type_configmap": { configMapName: "foo", @@ -84,7 +82,6 @@ func TestCreateConfigMap(t *testing.T) { Data: map[string]string{}, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_type_hash_configmap": { configMapName: "foo", @@ -101,7 +98,6 @@ func TestCreateConfigMap(t *testing.T) { Data: map[string]string{}, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_two_literal_configmap": { configMapName: "foo", @@ -120,7 +116,6 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_two_literal_hash_configmap": { configMapName: "foo", @@ -140,7 +135,6 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_key1_=value1_configmap": { configMapName: "foo", @@ -158,7 +152,6 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_key1_=value1_hash_configmap": { configMapName: "foo", @@ -177,11 +170,10 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_from_file_foo1_foo2_configmap": { configMapName: "foo", - setup: setupBinaryFile([]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}, "foo1", "foo2"), + setup: setupBinaryFile([]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}), fromFile: []string{"foo1", "foo2"}, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -197,11 +189,10 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_foo_from_file_foo1_foo2_and_configmap": { configMapName: "foo", - setup: setupBinaryFile([]byte{0xff, 0xfd}, "foo1", "foo2"), + setup: setupBinaryFile([]byte{0xff, 0xfd}), fromFile: []string{"foo1", "foo2"}, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -217,12 +208,11 @@ func TestCreateConfigMap(t *testing.T) { "foo2": {0xff, 0xfd}, }, }, - expectErr: false, }, "create_valid_env_from_env_file_configmap": { configMapName: "valid_env", - setup: setupEnvFile("key1=value1", "#", "", "key2=value2"), - fromEnvFile: "file.env", + setup: setupEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}}), + fromEnvFile: []string{"file.env"}, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -237,12 +227,31 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, + }, + "create_two_valid_env_from_env_file_configmap": { + configMapName: "two_valid_env", + setup: setupEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}, {"key3=value3"}}), + fromEnvFile: []string{"file1.env", "file2.env"}, + expected: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "two_valid_env", + }, + Data: map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, + BinaryData: map[string][]byte{}, + }, }, "create_valid_env_from_env_file_hash_configmap": { configMapName: "valid_env", - setup: setupEnvFile("key1=value1", "#", "", "key2=value2"), - fromEnvFile: "file.env", + setup: setupEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}}), + fromEnvFile: []string{"file.env"}, appendHash: true, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -258,16 +267,36 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, + }, + "create_two_valid_env_from_env_file_hash_configmap": { + configMapName: "two_valid_env", + setup: setupEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}, {"key3=value3"}}), + fromEnvFile: []string{"file1.env", "file2.env"}, + appendHash: true, + expected: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "two_valid_env-2m5tm82522", + }, + Data: map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, + BinaryData: map[string][]byte{}, + }, }, "create_get_env_from_env_file_configmap": { configMapName: "get_env", setup: func() func(t *testing.T, configMapOptions *ConfigMapOptions) func() { os.Setenv("g_key1", "1") os.Setenv("g_key2", "2") - return setupEnvFile("g_key1", "g_key2=") + return setupEnvFile([][]string{{"g_key1", "g_key2="}}) }(), - fromEnvFile: "file.env", + fromEnvFile: []string{"file.env"}, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -282,16 +311,15 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_get_env_from_env_file_hash_configmap": { configMapName: "get_env", setup: func() func(t *testing.T, configMapOptions *ConfigMapOptions) func() { os.Setenv("g_key1", "1") os.Setenv("g_key2", "2") - return setupEnvFile("g_key1", "g_key2=") + return setupEnvFile([][]string{{"g_key1", "g_key2="}}) }(), - fromEnvFile: "file.env", + fromEnvFile: []string{"file.env"}, appendHash: true, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -307,12 +335,11 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_value_with_space_from_env_file_configmap": { configMapName: "value_with_space", - setup: setupEnvFile("key1= value1"), - fromEnvFile: "file.env", + setup: setupEnvFile([][]string{{"key1= value1"}}), + fromEnvFile: []string{"file.env"}, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -326,12 +353,11 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_value_with_space_from_env_file_hash_configmap": { configMapName: "valid_with_space", - setup: setupEnvFile("key1= value1"), - fromEnvFile: "file.env", + setup: setupEnvFile([][]string{{"key1= value1"}}), + fromEnvFile: []string{"file.env"}, appendHash: true, expected: &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{ @@ -346,67 +372,67 @@ func TestCreateConfigMap(t *testing.T) { }, BinaryData: map[string][]byte{}, }, - expectErr: false, }, "create_invalid_configmap_filepath_contains_=": { configMapName: "foo", fromFile: []string{"key1=/file=2"}, - expectErr: true, + expectErr: `key names or file paths cannot contain '='`, }, "create_invalid_configmap_filepath_key_contains_=": { configMapName: "foo", fromFile: []string{"=key=/file1"}, - expectErr: true, + expectErr: `key names or file paths cannot contain '='`, }, "create_invalid_configmap_literal_key_contains_=": { configMapName: "foo", fromFile: []string{"=key=value1"}, - expectErr: true, + expectErr: `key names or file paths cannot contain '='`, }, "create_invalid_configmap_duplicate_key1": { configMapName: "foo", fromLiteral: []string{"key1=value1", "key1=value2"}, - expectErr: true, + expectErr: `cannot add key "key1", another key by that name already exists in Data for ConfigMap "foo"`, }, "create_invalid_configmap_no_file": { configMapName: "foo", fromFile: []string{"key1=/file1"}, - expectErr: true, + expectErr: `error reading /file1: no such file or directory`, }, "create_invalid_configmap_invalid_literal": { configMapName: "foo", fromLiteral: []string{"key1value1"}, - expectErr: true, + expectErr: `invalid literal source key1value1, expected key=value`, }, "create_invalid_configmap_invalid_filepath": { configMapName: "foo", fromFile: []string{"key1==file1"}, - expectErr: true, + expectErr: `key names or file paths cannot contain '='`, }, "create_invalid_configmap_too_many_args": { configMapName: "too_many_args", fromFile: []string{"key1=/file1"}, - fromEnvFile: "foo", - expectErr: true, + fromEnvFile: []string{"file.env"}, + expectErr: `from-env-file cannot be combined with from-file or from-literal`, }, "create_invalid_configmap_too_many_args_1": { configMapName: "too_many_args_1", fromLiteral: []string{"key1=value1"}, - fromEnvFile: "foo", - expectErr: true, + fromEnvFile: []string{"file.env"}, + expectErr: `from-env-file cannot be combined with from-file or from-literal`, }, } // run all the tests for name, test := range tests { t.Run(name, func(t *testing.T) { + var configMap *corev1.ConfigMap = nil configMapOptions := ConfigMapOptions{ Name: test.configMapName, Type: test.configMapType, AppendHash: test.appendHash, FileSources: test.fromFile, LiteralSources: test.fromLiteral, - EnvFileSource: test.fromEnvFile, + EnvFileSources: test.fromEnvFile, } if test.setup != nil { @@ -414,42 +440,58 @@ func TestCreateConfigMap(t *testing.T) { defer teardown() } } - - configMap, err := configMapOptions.createConfigMap() - if !test.expectErr && err != nil { - t.Errorf("test %s, unexpected error: %v", name, err) + err := configMapOptions.Validate() + if err == nil { + configMap, err = configMapOptions.createConfigMap() } - if test.expectErr && err == nil { - t.Errorf("test %s was expecting an error but no error occurred", name) + + if test.expectErr == "" && err != nil { + t.Errorf("unexpected error: %v", err) + } + if test.expectErr != "" && err == nil { + t.Errorf("was expecting an error but no error occurred") + } + if test.expectErr != "" && test.expectErr != err.Error() { + t.Errorf("\nexpected error:\n%s\ngot error:\n%s", test.expectErr, err.Error()) } if !apiequality.Semantic.DeepEqual(configMap, test.expected) { - t.Errorf("test %s expected:\n%#v\ngot:\n%#v", name, test.expected, configMap) + t.Errorf("\nexpected:\n%#v\ngot:\n%#v", test.expected, configMap) } }) } } -func setupEnvFile(lines ...string) func(*testing.T, *ConfigMapOptions) func() { +func setupEnvFile(lines [][]string) func(*testing.T, *ConfigMapOptions) func() { return func(t *testing.T, configMapOptions *ConfigMapOptions) func() { - f, err := ioutil.TempFile("", "cme") - if err != nil { - t.Errorf("unexpected error: %v", err) + files := []*os.File{} + filenames := configMapOptions.EnvFileSources + for _, filename := range filenames { + file, err := ioutil.TempFile("", filename) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + files = append(files, file) } - for _, l := range lines { - f.WriteString(l) - f.WriteString("\r\n") + for i, f := range files { + for _, l := range lines[i] { + f.WriteString(l) + f.WriteString("\r\n") + } + f.Close() + configMapOptions.EnvFileSources[i] = f.Name() } - f.Close() - configMapOptions.EnvFileSource = f.Name() return func() { - os.Remove(f.Name()) + for _, f := range files { + os.Remove(f.Name()) + } } } } -func setupBinaryFile(data []byte, files ...string) func(*testing.T, *ConfigMapOptions) func() { +func setupBinaryFile(data []byte) func(*testing.T, *ConfigMapOptions) func() { return func(t *testing.T, configMapOptions *ConfigMapOptions) func() { tmp, _ := ioutil.TempDir("", "") + files := configMapOptions.FileSources for i, file := range files { f := tmp + "/" + file ioutil.WriteFile(f, data, 0644)