From 9a13bae5108beb113dd0ca5605f4adeeac127915 Mon Sep 17 00:00:00 2001 From: Chok Yip Lau Date: Mon, 9 Aug 2021 00:05:43 -0400 Subject: [PATCH] Added support for multiple --from-env flags --- .../kubectl/pkg/cmd/create/create_secret.go | 52 +++-- .../pkg/cmd/create/create_secret_test.go | 200 +++++++++++------- 2 files changed, 154 insertions(+), 98 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret.go index 214b0c8f78e..b35c862685c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret.go @@ -84,8 +84,8 @@ var ( # Create a new secret named my-secret using a combination of a file and a literal kubectl create secret generic my-secret --from-file=ssh-privatekey=path/to/id_rsa --from-literal=passphrase=topsecret - # Create a new secret named my-secret from an env file - kubectl create secret generic my-secret --from-env-file=path/to/bar.env`)) + # Create a new secret named my-secret from env files + kubectl create secret generic my-secret --from-env-file=path/to/foo.env --from-env-file=path/to/bar.env`)) ) // CreateSecretOptions holds the options for 'create secret' sub command @@ -102,8 +102,8 @@ type CreateSecretOptions struct { FileSources []string // LiteralSources to derive the secret from (optional) LiteralSources []string - // EnvFileSource to derive the secret from (optional) - EnvFileSource string + // EnvFileSources to derive the secret from (optional) + EnvFileSources []string // AppendHash; if true, derive a hash from the Secret data and type and append it to the name AppendHash bool @@ -151,7 +151,7 @@ func NewCmdCreateSecretGeneric(f cmdutil.Factory, ioStreams genericclioptions.IO cmd.Flags().StringSliceVar(&o.FileSources, "from-file", o.FileSources, "Key files can be specified using their file path, in which case a default name will be given to them, or optionally with a name and file path, in which case the given name will be used. Specifying a directory will iterate each named file in the directory that is a valid secret key.") cmd.Flags().StringArrayVar(&o.LiteralSources, "from-literal", o.LiteralSources, "Specify a key and literal value to insert in secret (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 secret (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 secret (i.e. a Docker .env file).") cmd.Flags().StringVar(&o.Type, "type", o.Type, i18n.T("The type of secret to create")) cmd.Flags().BoolVar(&o.AppendHash, "append-hash", o.AppendHash, "Append a hash of the secret to its name.") @@ -220,7 +220,7 @@ func (o *CreateSecretOptions) 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 @@ -276,8 +276,8 @@ func (o *CreateSecretOptions) createSecret() (*corev1.Secret, error) { return nil, err } } - if len(o.EnvFileSource) > 0 { - if err := handleSecretFromEnvFileSource(secret, o.EnvFileSource); err != nil { + if len(o.EnvFileSources) > 0 { + if err := handleSecretFromEnvFileSources(secret, o.EnvFileSources); err != nil { return nil, err } } @@ -370,25 +370,31 @@ func handleSecretFromFileSources(secret *corev1.Secret, fileSources []string) er return nil } -// handleSecretFromEnvFileSource adds the specified env file source information +// handleSecretFromEnvFileSources adds the specified env files source information // into the provided secret -func handleSecretFromEnvFileSource(secret *corev1.Secret, envFileSource string) error { - fileInfo, 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 handleSecretFromEnvFileSources(secret *corev1.Secret, 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 secret file cannot be a directory") + } + err = cmdutil.AddFromEnvFile(envFileSource, func(key, value string) error { + return addKeyFromLiteralToSecret(secret, key, []byte(value)) + }) + if err != nil { + return err } } - if fileInfo.IsDir() { - return fmt.Errorf("env secret file cannot be a directory") - } - return cmdutil.AddFromEnvFile(envFileSource, func(key, value string) error { - return addKeyFromLiteralToSecret(secret, key, []byte(value)) - }) + return nil } // addKeyFromFileToSecret adds a key with the given name to a Secret, populating diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret_test.go index 1b915f8291e..c34927384f4 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_secret_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -53,12 +55,12 @@ func TestCreateSecretGeneric(t *testing.T) { secretType string fromLiteral []string fromFile []string - fromEnvFile string + fromEnvFile []string appendHash bool setup func(t *testing.T, secretGenericOptions *CreateSecretOptions) func() expected *corev1.Secret - expectErr bool + expectErr string }{ "create_secret_foo": { secretName: "foo", @@ -72,7 +74,6 @@ func TestCreateSecretGeneric(t *testing.T) { }, Data: map[string][]byte{}, }, - expectErr: false, }, "create_secret_foo_hash": { secretName: "foo", @@ -87,7 +88,6 @@ func TestCreateSecretGeneric(t *testing.T) { }, Data: map[string][]byte{}, }, - expectErr: false, }, "create_secret_foo_type": { secretName: "foo", @@ -103,7 +103,6 @@ func TestCreateSecretGeneric(t *testing.T) { Data: map[string][]byte{}, Type: "my-type", }, - expectErr: false, }, "create_secret_foo_type_hash": { secretName: "foo", @@ -120,7 +119,6 @@ func TestCreateSecretGeneric(t *testing.T) { Data: map[string][]byte{}, Type: "my-type", }, - expectErr: false, }, "create_secret_foo_two_literal": { secretName: "foo", @@ -138,7 +136,6 @@ func TestCreateSecretGeneric(t *testing.T) { "key2": []byte("value2"), }, }, - expectErr: false, }, "create_secret_foo_two_literal_hash": { secretName: "foo", @@ -157,7 +154,6 @@ func TestCreateSecretGeneric(t *testing.T) { "key2": []byte("value2"), }, }, - expectErr: false, }, "create_secret_foo_key1_=value1": { secretName: "foo", @@ -174,7 +170,6 @@ func TestCreateSecretGeneric(t *testing.T) { "key1": []byte("=value1"), }, }, - expectErr: false, }, "create_secret_foo_key1_=value1_hash": { secretName: "foo", @@ -192,11 +187,10 @@ func TestCreateSecretGeneric(t *testing.T) { "key1": []byte("=value1"), }, }, - expectErr: false, }, "create_secret_foo_from_file_foo1_foo2_secret": { secretName: "foo", - setup: setupSecretBinaryFile([]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}, "foo1", "foo2"), + setup: setupSecretBinaryFile([]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}), fromFile: []string{"foo1", "foo2"}, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -211,11 +205,10 @@ func TestCreateSecretGeneric(t *testing.T) { "foo2": []byte("hello world"), }, }, - expectErr: false, }, "create_secret_foo_from_file_foo1_foo2_hash": { secretName: "foo", - setup: setupSecretBinaryFile([]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}, "foo1", "foo2"), + setup: setupSecretBinaryFile([]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}), fromFile: []string{"foo1", "foo2"}, appendHash: true, expected: &corev1.Secret{ @@ -231,11 +224,10 @@ func TestCreateSecretGeneric(t *testing.T) { "foo2": []byte("hello world"), }, }, - expectErr: false, }, "create_secret_foo_from_file_foo1_foo2_and": { secretName: "foo", - setup: setupSecretBinaryFile([]byte{0xff, 0xfd}, "foo1", "foo2"), + setup: setupSecretBinaryFile([]byte{0xff, 0xfd}), fromFile: []string{"foo1", "foo2"}, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -250,11 +242,10 @@ func TestCreateSecretGeneric(t *testing.T) { "foo2": {0xff, 0xfd}, }, }, - expectErr: false, }, "create_secret_foo_from_file_foo1_foo2_and_hash": { secretName: "foo", - setup: setupSecretBinaryFile([]byte{0xff, 0xfd}, "foo1", "foo2"), + setup: setupSecretBinaryFile([]byte{0xff, 0xfd}), fromFile: []string{"foo1", "foo2"}, appendHash: true, expected: &corev1.Secret{ @@ -270,12 +261,11 @@ func TestCreateSecretGeneric(t *testing.T) { "foo2": {0xff, 0xfd}, }, }, - expectErr: false, }, "create_secret_valid_env_from_env_file": { secretName: "valid_env", - setup: setupSecretEnvFile("key1=value1", "#", "", "key2=value2"), - fromEnvFile: "file.env", + setup: setupSecretEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}}), + fromEnvFile: []string{"file.env"}, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -289,12 +279,11 @@ func TestCreateSecretGeneric(t *testing.T) { "key2": []byte("value2"), }, }, - expectErr: false, }, "create_secret_valid_env_from_env_file_hash": { secretName: "valid_env", - setup: setupSecretEnvFile("key1=value1", "#", "", "key2=value2"), - fromEnvFile: "file.env", + setup: setupSecretEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}}), + fromEnvFile: []string{"file.env"}, appendHash: true, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -309,16 +298,54 @@ func TestCreateSecretGeneric(t *testing.T) { "key2": []byte("value2"), }, }, - expectErr: false, + }, + "create_two_secret_valid_env_from_env_file": { + secretName: "two_valid_env", + setup: setupSecretEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}, {"key3=value3"}}), + fromEnvFile: []string{"file1.env", "file2.env"}, + expected: &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Secret", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "two_valid_env", + }, + Data: map[string][]byte{ + "key1": []byte("value1"), + "key2": []byte("value2"), + "key3": []byte("value3"), + }, + }, + }, + "create_two_secret_valid_env_from_env_file_hash": { + secretName: "two_valid_env", + setup: setupSecretEnvFile([][]string{{"key1=value1", "#", "", "key2=value2"}, {"key3=value3"}}), + fromEnvFile: []string{"file1.env", "file2.env"}, + appendHash: true, + expected: &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Secret", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "two_valid_env-gd56gct5cf", + }, + Data: map[string][]byte{ + "key1": []byte("value1"), + "key2": []byte("value2"), + "key3": []byte("value3"), + }, + }, }, "create_secret_get_env_from_env_file": { secretName: "get_env", setup: func() func(t *testing.T, secretGenericOptions *CreateSecretOptions) func() { os.Setenv("g_key1", "1") os.Setenv("g_key2", "2") - return setupSecretEnvFile("g_key1", "g_key2=") + return setupSecretEnvFile([][]string{{"g_key1", "g_key2="}}) }(), - fromEnvFile: "file.env", + fromEnvFile: []string{"file.env"}, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -332,16 +359,15 @@ func TestCreateSecretGeneric(t *testing.T) { "g_key2": []byte(""), }, }, - expectErr: false, }, "create_secret_get_env_from_env_file_hash": { secretName: "get_env", setup: func() func(t *testing.T, secretGenericOptions *CreateSecretOptions) func() { os.Setenv("g_key1", "1") os.Setenv("g_key2", "2") - return setupSecretEnvFile("g_key1", "g_key2=") + return setupSecretEnvFile([][]string{{"g_key1", "g_key2="}}) }(), - fromEnvFile: "file.env", + fromEnvFile: []string{"file.env"}, appendHash: true, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -356,12 +382,11 @@ func TestCreateSecretGeneric(t *testing.T) { "g_key2": []byte(""), }, }, - expectErr: false, }, "create_secret_value_with_space_from_env_file": { secretName: "value_with_space", - setup: setupSecretEnvFile(" key1= value1"), - fromEnvFile: "file.env", + setup: setupSecretEnvFile([][]string{{" key1= value1"}}), + fromEnvFile: []string{"file.env"}, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ APIVersion: corev1.SchemeGroupVersion.String(), @@ -374,12 +399,11 @@ func TestCreateSecretGeneric(t *testing.T) { "key1": []byte(" value1"), }, }, - expectErr: false, }, "create_secret_value_with_space_from_env_file_hash": { secretName: "valid_with_space", - setup: setupSecretEnvFile(" key1= value1"), - fromEnvFile: "file.env", + setup: setupSecretEnvFile([][]string{{" key1= value1"}}), + fromEnvFile: []string{"file.env"}, appendHash: true, expected: &corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -393,70 +417,86 @@ func TestCreateSecretGeneric(t *testing.T) { "key1": []byte(" value1"), }, }, - expectErr: false, }, "create_invalid_secret_filepath_contains_=": { secretName: "foo", fromFile: []string{"key1=/file=2"}, - expectErr: true, + expectErr: `key names or file paths cannot contain '='`, }, "create_invalid_secret_filepath_key_contains_=": { secretName: "foo", fromFile: []string{"=key=/file1"}, - expectErr: true, + expectErr: `key names or file paths cannot contain '='`, }, "create_invalid_secret_literal_key_contains_=": { - secretName: "foo", - fromFile: []string{"=key=value1"}, - expectErr: true, + secretName: "foo", + fromLiteral: []string{"=key=value1"}, + expectErr: `invalid literal source =key=value1, expected key=value`, + }, + "create_invalid_secret_literal_key_with_invalid_character": { + secretName: "foo", + fromLiteral: []string{"key#1=value1"}, + expectErr: `"key#1" is not valid key name for a Secret a valid config key must consist of alphanumeric characters, '-', '_' or '.' (e.g. 'key.name', or 'KEY_NAME', or 'key-name', regex used for validation is '[-._a-zA-Z0-9]+')`, }, "create_invalid_secret_env_key_contains_#": { secretName: "invalid_key", - setup: setupSecretEnvFile("key#1=value1"), - fromEnvFile: "file.env", - expectErr: true, + setup: setupSecretEnvFile([][]string{{"key#1=value1"}}), + fromEnvFile: []string{"file.env"}, + expectErr: `"key#1" is not a valid key name: a valid environment variable name must consist of alphabetic characters, digits, '_', '-', or '.', and must not start with a digit (e.g. 'my.env-name', or 'MY_ENV.NAME', or 'MyEnvName1', regex used for validation is '[-._a-zA-Z][-._a-zA-Z0-9]*')`, + }, + "create_invalid_secret_env_key_start_with_digit": { + secretName: "invalid_key", + setup: setupSecretEnvFile([][]string{{"1key=value1"}}), + fromEnvFile: []string{"file.env"}, + expectErr: `"1key" is not a valid key name: a valid environment variable name must consist of alphabetic characters, digits, '_', '-', or '.', and must not start with a digit (e.g. 'my.env-name', or 'MY_ENV.NAME', or 'MyEnvName1', regex used for validation is '[-._a-zA-Z][-._a-zA-Z0-9]*')`, + }, + "create_invalid_secret_env_key_with_invalid_character": { + secretName: "invalid_key", + setup: setupSecretEnvFile([][]string{{"key@=value1"}}), + fromEnvFile: []string{"file.env"}, + expectErr: `"key@" is not a valid key name: a valid environment variable name must consist of alphabetic characters, digits, '_', '-', or '.', and must not start with a digit (e.g. 'my.env-name', or 'MY_ENV.NAME', or 'MyEnvName1', regex used for validation is '[-._a-zA-Z][-._a-zA-Z0-9]*')`, }, "create_invalid_secret_duplicate_key1": { secretName: "foo", fromLiteral: []string{"key1=value1", "key1=value2"}, - expectErr: true, + expectErr: `cannot add key key1, another key by that name already exists`, }, "create_invalid_secret_no_file": { secretName: "foo", fromFile: []string{"key1=/file1"}, - expectErr: true, + expectErr: `error reading /file1: no such file or directory`, }, "create_invalid_secret_invalid_literal": { secretName: "foo", fromLiteral: []string{"key1value1"}, - expectErr: true, + expectErr: `invalid literal source key1value1, expected key=value`, }, "create_invalid_secret_invalid_filepath": { secretName: "foo", fromFile: []string{"key1==file1"}, - expectErr: true, + expectErr: `key names or file paths cannot contain '='`, }, "create_invalid_secret_no_name": { - expectErr: true, + expectErr: `name must be specified`, }, "create_invalid_secret_too_many_args": { secretName: "too_many_args", fromFile: []string{"key1=/file1"}, - fromEnvFile: "foo", - expectErr: true, + fromEnvFile: []string{"foo"}, + expectErr: `from-env-file cannot be combined with from-file or from-literal`, }, "create_invalid_secret_too_many_args_1": { secretName: "too_many_args_1", fromLiteral: []string{"key1=value1"}, - fromEnvFile: "foo", - expectErr: true, + fromEnvFile: []string{"foo"}, + expectErr: `from-env-file cannot be combined with from-file or from-literal`, }, "create_invalid_secret_too_many_args_2": { secretName: "too_many_args_2", fromFile: []string{"key1=/file1"}, fromLiteral: []string{"key1=value1"}, - fromEnvFile: "foo", - expectErr: true, + fromEnvFile: []string{"foo"}, + expectErr: `from-env-file cannot be combined with from-file or from-literal`, }, } @@ -470,7 +510,7 @@ func TestCreateSecretGeneric(t *testing.T) { AppendHash: test.appendHash, FileSources: test.fromFile, LiteralSources: test.fromLiteral, - EnvFileSource: test.fromEnvFile, + EnvFileSources: test.fromEnvFile, } if test.setup != nil { if teardown := test.setup(t, &secretOptions); teardown != nil { @@ -482,40 +522,50 @@ func TestCreateSecretGeneric(t *testing.T) { secret, err = secretOptions.createSecret() } - if !test.expectErr && err != nil { - t.Errorf("test %s, unexpected error: %v", name, err) - } - if test.expectErr && err == nil { - t.Errorf("test %s was expecting an error but no error occurred", name) - } - if !apiequality.Semantic.DeepEqual(secret, test.expected) { - t.Errorf("test %s\n expected:\n%#v\ngot:\n%#v", name, test.expected, secret) + if test.expectErr == "" { + require.NoError(t, err) + if !apiequality.Semantic.DeepEqual(secret, test.expected) { + t.Errorf("\nexpected:\n%#v\ngot:\n%#v", test.expected, secret) + } + } else { + require.Error(t, err) + require.EqualError(t, err, test.expectErr) } }) } } -func setupSecretEnvFile(lines ...string) func(*testing.T, *CreateSecretOptions) func() { +func setupSecretEnvFile(lines [][]string) func(*testing.T, *CreateSecretOptions) func() { return func(t *testing.T, secretOptions *CreateSecretOptions) func() { - f, err := ioutil.TempFile("", "cme") - if err != nil { - t.Errorf("unexpected error: %v", err) + files := []*os.File{} + filenames := secretOptions.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() + secretOptions.EnvFileSources[i] = f.Name() } - f.Close() - secretOptions.EnvFileSource = f.Name() return func() { - os.Remove(f.Name()) + for _, f := range files { + os.Remove(f.Name()) + } } } } -func setupSecretBinaryFile(data []byte, files ...string) func(*testing.T, *CreateSecretOptions) func() { +func setupSecretBinaryFile(data []byte) func(*testing.T, *CreateSecretOptions) func() { return func(t *testing.T, secretOptions *CreateSecretOptions) func() { tmp, _ := ioutil.TempDir("", "") + files := secretOptions.FileSources for i, file := range files { f := tmp + "/" + file ioutil.WriteFile(f, data, 0644)