Merge pull request #60040 from PhilipGough/keys-from-cm

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Allow env to be updated via specific key in resource

**What this PR does / why we need it**:

This change allows users of the `oc` client to specify a list of comma-separated keys when running `oc set env` which should be imported from a resource i.e configmap or secret

This can be useful when a number of applications want to share a configuration object but don't want to pollute a resource with unused environment 



**Release note**:

```release-note
Allow kubectl env to specify which keys to import from a config map
```

pkg/kubectl/cmd/set/set_env.go

@@ -79,6 +79,9 @@ var (
 	  # Import environment from a config map with a prefix
 	  kubectl set env --from=configmap/myconfigmap --prefix=MYSQL_ deployment/myapp
 
+          # Import specific keys from a config map
+          kubectl set env --keys=my-example-key --from=configmap/myconfigmap deployment/myapp
+
 	  # Remove the environment variable ENV from container 'c1' in all deployment configs
 	  kubectl set env deployments --all --containers="c1" ENV-
 
@@ -114,6 +117,7 @@ type EnvOptions struct {
 	Output            string
 	From              string
 	Prefix            string
+	Keys              []string
 
 	Builder *resource.Builder
 	Infos   []*resource.Info
@@ -137,7 +141,8 @@ func NewCmdEnv(f cmdutil.Factory, in io.Reader, out, errout io.Writer) *cobra.Co
 		Long:    envLong,
 		Example: fmt.Sprintf(envExample),
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(options.Complete(f, cmd, args))
+			options.Complete(f, cmd)
+			cmdutil.CheckErr(options.Validate(args))
 			cmdutil.CheckErr(options.RunEnv(f))
 		},
 	}
@@ -147,6 +152,7 @@ func NewCmdEnv(f cmdutil.Factory, in io.Reader, out, errout io.Writer) *cobra.Co
 	cmd.Flags().StringP("from", "", "", "The name of a resource from which to inject environment variables")
 	cmd.Flags().StringP("prefix", "", "", "Prefix to append to variable names")
 	cmd.Flags().StringArrayVarP(&options.EnvParams, "env", "e", options.EnvParams, "Specify a key-value pair for an environment variable to set into each container.")
+	cmd.Flags().StringSliceVarP(&options.Keys, "keys", "", options.Keys, "Comma-separated list of keys to import from specified resource")
 	cmd.Flags().BoolVar(&options.List, "list", options.List, "If true, display the environment and any changes in the standard format. this flag will removed when we have kubectl view env.")
 	cmd.Flags().BoolVar(&options.Resolve, "resolve", options.Resolve, "If true, show secret or configmap references when listing variables")
 	cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on")
@@ -173,18 +179,20 @@ func keyToEnvName(key string) string {
 	return strings.ToUpper(validEnvNameRegexp.ReplaceAllString(key, "_"))
 }
 
-func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
+func contains(key string, keyList []string) bool {
-	if o.All && len(o.Selector) > 0 {
+	if len(keyList) == 0 {
-		return fmt.Errorf("cannot set --all and --selector at the same time")
+		return true
-	}
-	resources, envArgs, ok := envutil.SplitEnvironmentFromResources(args)
-	if !ok {
-		return cmdutil.UsageErrorf(o.Cmd, "all resources must be specified before environment changes: %s", strings.Join(args, " "))
-	}
-	if len(o.Filenames) == 0 && len(resources) < 1 {
-		return cmdutil.UsageErrorf(cmd, "one or more resources must be specified as <resource> <name> or <resource>/<name>")
 	}
 
+	for _, k := range keyList {
+		if k == key {
+			return true
+		}
+	}
+	return false
+}
+
+func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) {
 	o.UpdatePodSpecForObject = f.UpdatePodSpecForObject
 	o.ContainerSelector = cmdutil.GetFlagString(cmd, "containers")
 	o.List = cmdutil.GetFlagBool(cmd, "list")
@@ -195,18 +203,38 @@ func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri
 	o.Output = cmdutil.GetFlagString(cmd, "output")
 	o.From = cmdutil.GetFlagString(cmd, "from")
 	o.Prefix = cmdutil.GetFlagString(cmd, "prefix")
+	o.Keys = cmdutil.GetFlagStringSlice(cmd, "keys")
 	o.DryRun = cmdutil.GetDryRunFlag(cmd)
 
-	o.EnvArgs = envArgs
-	o.Resources = resources
 	o.Cmd = cmd
 
 	o.ShortOutput = cmdutil.GetFlagString(cmd, "output") == "name"
 
+}
+
+func (o *EnvOptions) Validate(args []string) error {
+	if o.All && len(o.Selector) > 0 {
+		return fmt.Errorf("cannot set --all and --selector at the same time")
+	}
+	resources, envArgs, ok := envutil.SplitEnvironmentFromResources(args)
+	if !ok {
+		return cmdutil.UsageErrorf(o.Cmd, "all resources must be specified before environment changes: %s", strings.Join(args, " "))
+	}
+
+	if len(o.Filenames) == 0 && len(resources) == 0 {
+		return cmdutil.UsageErrorf(o.Cmd, "one or more resources must be specified as <resource> <name> or <resource>/<name>")
+	}
+
 	if o.List && len(o.Output) > 0 {
 		return cmdutil.UsageErrorf(o.Cmd, "--list and --output may not be specified together")
 	}
 
+	if len(o.Keys) > 0 && len(o.From) == 0 {
+		return cmdutil.UsageErrorf(o.Cmd, "when specifying --keys, a configmap or secret must be provided with --from")
+	}
+
+	o.EnvArgs = envArgs
+	o.Resources = resources
 	return nil
 }
 
@@ -271,7 +299,9 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 							},
 						},
 					}
-					env = append(env, envVar)
+					if contains(key, o.Keys) {
+						env = append(env, envVar)
+					}
 				}
 			case *v1.ConfigMap:
 				for key := range from.Data {
@@ -286,7 +316,9 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 							},
 						},
 					}
-					env = append(env, envVar)
+					if contains(key, o.Keys) {
+						env = append(env, envVar)
+					}
 				}
 			default:
 				return fmt.Errorf("unsupported resource specified in --from")

pkg/kubectl/cmd/set/set_env_test.go

@@ -70,7 +70,8 @@ func TestSetEnvLocal(t *testing.T) {
 		Filenames: []string{"../../../../examples/storage/cassandra/cassandra-controller.yaml"}},
 		Out:   buf,
 		Local: true}
-	err := opts.Complete(f, cmd, []string{"env=prod"})
+	opts.Complete(f, cmd)
+	err := opts.Validate([]string{"env=prod"})
 	if err == nil {
 		err = opts.RunEnv(f)
 	}
@@ -107,7 +108,8 @@ func TestSetMultiResourcesEnvLocal(t *testing.T) {
 		Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}},
 		Out:   buf,
 		Local: true}
-	err := opts.Complete(f, cmd, []string{"env=prod"})
+	opts.Complete(f, cmd)
+	err := opts.Validate([]string{"env=prod"})
 	if err == nil {
 		err = opts.RunEnv(f)
 	}
@@ -122,11 +124,13 @@ func TestSetMultiResourcesEnvLocal(t *testing.T) {
 }
 
 func TestSetEnvRemote(t *testing.T) {
+	out := new(bytes.Buffer)
 	inputs := []struct {
 		object                          runtime.Object
 		apiPrefix, apiGroup, apiVersion string
 		testAPIGroup                    string
 		args                            []string
+		opts                            *EnvOptions
 	}{
 		{
 			object: &extensionsv1beta1.ReplicaSet{
@@ -147,6 +151,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "extensions", apiVersion: "v1beta1",
 			args: []string{"replicaset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1beta2.ReplicaSet{
@@ -167,6 +172,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1beta2",
 			args: []string{"replicaset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1.ReplicaSet{
@@ -187,6 +193,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1",
 			args: []string{"replicaset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &extensionsv1beta1.DaemonSet{
@@ -207,6 +214,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "extensions", apiVersion: "v1beta1",
 			args: []string{"daemonset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1beta2.DaemonSet{
@@ -227,6 +235,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1beta2",
 			args: []string{"daemonset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1.DaemonSet{
@@ -247,6 +256,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1",
 			args: []string{"daemonset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &extensionsv1beta1.Deployment{
@@ -267,6 +277,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "extensions", apiVersion: "v1beta1",
 			args: []string{"deployment", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1beta1.Deployment{
@@ -287,6 +298,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1beta1",
 			args: []string{"deployment", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1beta2.Deployment{
@@ -307,6 +319,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1beta2",
 			args: []string{"deployment", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1.Deployment{
@@ -327,6 +340,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "extensions",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1",
 			args: []string{"deployment", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1beta1.StatefulSet{
@@ -347,6 +361,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "apps",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1beta1",
 			args: []string{"statefulset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1beta2.StatefulSet{
@@ -367,6 +382,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "apps",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1beta2",
 			args: []string{"statefulset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &appsv1.StatefulSet{
@@ -387,6 +403,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "apps",
 			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1",
 			args: []string{"statefulset", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &batchv1.Job{
@@ -407,6 +424,7 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "batch",
 			apiPrefix:    "/apis", apiGroup: "batch", apiVersion: "v1",
 			args: []string{"job", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
 		},
 		{
 			object: &v1.ReplicationController{
@@ -427,6 +445,54 @@ func TestSetEnvRemote(t *testing.T) {
 			testAPIGroup: "",
 			apiPrefix:    "/api", apiGroup: "", apiVersion: "v1",
 			args: []string{"replicationcontroller", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out, Local: false},
+		},
+		{
+			object: &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{Name: "nginx"},
+				Spec: appsv1.DeploymentSpec{
+					Template: v1.PodTemplateSpec{
+						Spec: v1.PodSpec{
+							Containers: []v1.Container{
+								{
+									Name:  "nginx",
+									Image: "nginx",
+								},
+							},
+						},
+					},
+				},
+			},
+			testAPIGroup: "extensions",
+			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1",
+			args: []string{"deployment", "nginx", "env=prod"},
+			opts: &EnvOptions{Out: out,
+				Local: false,
+				From:  "configmap/myconfigmap"},
+		},
+		{
+			object: &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{Name: "nginx"},
+				Spec: appsv1.DeploymentSpec{
+					Template: v1.PodTemplateSpec{
+						Spec: v1.PodSpec{
+							Containers: []v1.Container{
+								{
+									Name:  "nginx",
+									Image: "nginx",
+								},
+							},
+						},
+					},
+				},
+			},
+			testAPIGroup: "extensions",
+			apiPrefix:    "/apis", apiGroup: "apps", apiVersion: "v1",
+			args: []string{"deployment", "nginx"},
+			opts: &EnvOptions{Out: out,
+				Local: false,
+				Keys:  []string{"test-key"},
+				From:  "configmap/myconfigmap"},
 		},
 	}
 	for _, input := range inputs {
@@ -463,16 +529,16 @@ func TestSetEnvRemote(t *testing.T) {
 			}),
 			VersionedAPIPath: path.Join(input.apiPrefix, testapi.Default.GroupVersion().String()),
 		}
-		out := new(bytes.Buffer)
 		cmd := NewCmdEnv(f, out, out, out)
 		cmd.SetOutput(out)
 		cmd.Flags().Set("output", "yaml")
-		opts := EnvOptions{
+		opts := input.opts
-			Out:   out,
+		opts.Complete(f, cmd)
-			Local: false}
+		err := opts.Validate(input.args)
-		err := opts.Complete(f, cmd, input.args)
 		assert.NoError(t, err)
 		err = opts.RunEnv(f)
 		assert.NoError(t, err)
 	}
+	// TODO This global state restoration needs fixing, b/c it's wrong. Tests should not modify global state
+	testapi.Default = testapi.Groups[""]
 }