Merge pull request #107934 from jlsong01/add_lowercase_secret_key_warning

add warning when kubectl set env from secret leads to uppercase

staging/src/k8s.io/kubectl/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{

staging/src/k8s.io/kubectl/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)
 		})
 	}