From a64711ac8373ed64741601fe4b29435ce941a7a3 Mon Sep 17 00:00:00 2001 From: jlsong01 Date: Tue, 8 Feb 2022 19:10:12 +0800 Subject: [PATCH] add warning when kubectl set env from secret leads to uppercase Kubernetes-commit: feb84ccef4a69fc1710003c71bc496b94448f094 --- pkg/cmd/set/set_env.go | 16 ++++++---- pkg/cmd/set/set_env_test.go | 60 ++++++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/set/set_env.go b/pkg/cmd/set/set_env.go index aeb0cddd..a27dfc40 100644 --- a/pkg/cmd/set/set_env.go +++ b/pkg/cmd/set/set_env.go @@ -190,10 +190,6 @@ func validateNoOverwrites(existing []v1.EnvVar, env []v1.EnvVar) error { return nil } -func keyToEnvName(key string) string { - return strings.ToUpper(validEnvNameRegexp.ReplaceAllString(key, "_")) -} - func contains(key string, keyList []string) bool { if len(keyList) == 0 { return true @@ -207,6 +203,14 @@ func contains(key string, keyList []string) bool { return false } +func (o *EnvOptions) keyToEnvName(key string) string { + envName := strings.ToUpper(validEnvNameRegexp.ReplaceAllString(key, "_")) + if envName != key { + fmt.Fprintf(o.ErrOut, "warning: key %s transferred to %s\n", key, envName) + } + return envName +} + // Complete completes all required options func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { if o.All && len(o.Selector) > 0 { @@ -306,7 +310,7 @@ func (o *EnvOptions) RunEnv() error { for key := range from.Data { if contains(key, o.Keys) { envVar := v1.EnvVar{ - Name: keyToEnvName(key), + Name: o.keyToEnvName(key), ValueFrom: &v1.EnvVarSource{ SecretKeyRef: &v1.SecretKeySelector{ LocalObjectReference: v1.LocalObjectReference{ @@ -323,7 +327,7 @@ func (o *EnvOptions) RunEnv() error { for key := range from.Data { if contains(key, o.Keys) { envVar := v1.EnvVar{ - Name: keyToEnvName(key), + Name: o.keyToEnvName(key), ValueFrom: &v1.EnvVarSource{ ConfigMapKeyRef: &v1.ConfigMapKeySelector{ LocalObjectReference: v1.LocalObjectReference{ diff --git a/pkg/cmd/set/set_env_test.go b/pkg/cmd/set/set_env_test.go index 319196a4..8c9ad4f1 100644 --- a/pkg/cmd/set/set_env_test.go +++ b/pkg/cmd/set/set_env_test.go @@ -535,6 +535,15 @@ func TestSetEnvFromResource(t *testing.T) { }, } + mockConfigMapUpperCaseKey := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "testconfigmapuppercasekey"}, + Data: map[string]string{ + "ENV": "prod", + "TEST_KEY": "testValue", + "TEST_KEY_TWO": "testValueTwo", + }, + } + mockSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{Name: "testsecret"}, Data: map[string][]byte{ @@ -544,6 +553,15 @@ func TestSetEnvFromResource(t *testing.T) { }, } + mockSecretUpperCaseKey := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "testsecretuppercasekey"}, + Data: map[string][]byte{ + "ENV": []byte("prod"), + "TEST_KEY": []byte("testValue"), + "TEST_KEY_TWO": []byte("testValueTwo"), + }, + } + inputs := []struct { name string args []string @@ -551,6 +569,7 @@ func TestSetEnvFromResource(t *testing.T) { keys []string assertIncludes []string assertExcludes []string + warning bool }{ { name: "test from configmap", @@ -563,6 +582,20 @@ func TestSetEnvFromResource(t *testing.T) { `{"name":"TEST_KEY_TWO","valueFrom":{"configMapKeyRef":{"key":"test-key-two","name":"testconfigmap"}}}`, }, assertExcludes: []string{}, + warning: true, + }, + { + name: "test from configmap with upper case key", + args: []string{"deployment", "nginx"}, + from: "configmap/testconfigmapuppercasekey", + keys: []string{}, + assertIncludes: []string{ + `{"name":"ENV","valueFrom":{"configMapKeyRef":{"key":"ENV","name":"testconfigmapuppercasekey"}}}`, + `{"name":"TEST_KEY","valueFrom":{"configMapKeyRef":{"key":"TEST_KEY","name":"testconfigmapuppercasekey"}}}`, + `{"name":"TEST_KEY_TWO","valueFrom":{"configMapKeyRef":{"key":"TEST_KEY_TWO","name":"testconfigmapuppercasekey"}}}`, + }, + assertExcludes: []string{}, + warning: false, }, { name: "test from secret", @@ -575,6 +608,20 @@ func TestSetEnvFromResource(t *testing.T) { `{"name":"TEST_KEY_TWO","valueFrom":{"secretKeyRef":{"key":"test-key-two","name":"testsecret"}}}`, }, assertExcludes: []string{}, + warning: true, + }, + { + name: "test from secret with upper case key", + args: []string{"deployment", "nginx"}, + from: "secret/testsecretuppercasekey", + keys: []string{}, + assertIncludes: []string{ + `{"name":"ENV","valueFrom":{"secretKeyRef":{"key":"ENV","name":"testsecretuppercasekey"}}}`, + `{"name":"TEST_KEY","valueFrom":{"secretKeyRef":{"key":"TEST_KEY","name":"testsecretuppercasekey"}}}`, + `{"name":"TEST_KEY_TWO","valueFrom":{"secretKeyRef":{"key":"TEST_KEY_TWO","name":"testsecretuppercasekey"}}}`, + }, + assertExcludes: []string{}, + warning: false, }, { name: "test from configmap with keys", @@ -586,6 +633,7 @@ func TestSetEnvFromResource(t *testing.T) { `{"name":"TEST_KEY_TWO","valueFrom":{"configMapKeyRef":{"key":"test-key-two","name":"testconfigmap"}}}`, }, assertExcludes: []string{`{"name":"TEST_KEY","valueFrom":{"configMapKeyRef":{"key":"test-key","name":"testconfigmap"}}}`}, + warning: true, }, { name: "test from secret with keys", @@ -597,6 +645,7 @@ func TestSetEnvFromResource(t *testing.T) { `{"name":"TEST_KEY_TWO","valueFrom":{"secretKeyRef":{"key":"test-key-two","name":"testsecret"}}}`, }, assertExcludes: []string{`{"name":"TEST_KEY","valueFrom":{"secretKeyRef":{"key":"test-key","name":"testsecret"}}}`}, + warning: true, }, } @@ -628,8 +677,12 @@ func TestSetEnvFromResource(t *testing.T) { switch p, m := req.URL.Path, req.Method; { case p == "/namespaces/test/configmaps/testconfigmap" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: objBody(mockConfigMap)}, nil + case p == "/namespaces/test/configmaps/testconfigmapuppercasekey" && m == http.MethodGet: + return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: objBody(mockConfigMapUpperCaseKey)}, nil case p == "/namespaces/test/secrets/testsecret" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: objBody(mockSecret)}, nil + case p == "/namespaces/test/secrets/testsecretuppercasekey" && m == http.MethodGet: + return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: objBody(mockSecretUpperCaseKey)}, nil case p == "/namespaces/test/deployments/nginx" && m == http.MethodGet: return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: objBody(mockDeployment)}, nil case p == "/namespaces/test/deployments/nginx" && m == http.MethodPatch: @@ -656,7 +709,7 @@ func TestSetEnvFromResource(t *testing.T) { } outputFormat := "yaml" - streams := genericclioptions.NewTestIOStreamsDiscard() + streams, _, _, errOut := genericclioptions.NewTestIOStreams() opts := NewEnvOptions(streams) opts.From = input.from opts.Keys = input.keys @@ -666,6 +719,11 @@ func TestSetEnvFromResource(t *testing.T) { err := opts.Complete(tf, NewCmdEnv(tf, streams), input.args) assert.NoError(t, err) err = opts.RunEnv() + if input.warning { + assert.Contains(t, errOut.String(), "warning") + } else { + assert.NotContains(t, errOut.String(), "warning") + } assert.NoError(t, err) }) }