From 905a0698faf73d01be0892048eb090d6cf0d1ad6 Mon Sep 17 00:00:00 2001 From: PhilipGough Date: Mon, 19 Feb 2018 15:22:23 +0000 Subject: [PATCH] Allow env to be updated via specific key in resource --- pkg/kubectl/cmd/set/set_env.go | 60 +++++++++++++++++----- pkg/kubectl/cmd/set/set_env_test.go | 80 ++++++++++++++++++++++++++--- 2 files changed, 119 insertions(+), 21 deletions(-) diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index 5311dcbea356c..855e706c5cd52 100644 --- a/pkg/kubectl/cmd/set/set_env.go +++ b/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 @@ -139,7 +143,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)) }, } @@ -149,6 +154,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") @@ -175,18 +181,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 { - 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, " ")) +func contains(key string, keyList []string) bool { + if len(keyList) == 0 { + return true } - if len(o.Filenames) == 0 && len(resources) < 1 { - return cmdutil.UsageErrorf(cmd, "one or more resources must be specified as or /") + + 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.Encoder = f.JSONEncoder() o.ContainerSelector = cmdutil.GetFlagString(cmd, "containers") @@ -198,19 +206,39 @@ 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.PrintObject = f.PrintObject - 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 or /") + } + 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 } @@ -275,7 +303,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 { @@ -290,7 +320,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") diff --git a/pkg/kubectl/cmd/set/set_env_test.go b/pkg/kubectl/cmd/set/set_env_test.go index 64ec338fba28f..01ef4da9eb187 100644 --- a/pkg/kubectl/cmd/set/set_env_test.go +++ b/pkg/kubectl/cmd/set/set_env_test.go @@ -72,7 +72,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) } @@ -109,7 +110,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) } @@ -124,11 +126,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{ @@ -149,6 +153,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{ @@ -169,6 +174,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{ @@ -189,6 +195,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{ @@ -209,6 +216,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{ @@ -229,6 +237,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{ @@ -249,6 +258,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{ @@ -269,6 +279,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{ @@ -289,6 +300,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{ @@ -309,6 +321,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{ @@ -329,6 +342,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{ @@ -349,6 +363,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{ @@ -369,6 +384,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{ @@ -389,6 +405,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{ @@ -409,6 +426,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{ @@ -429,6 +447,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 { @@ -465,16 +531,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{ - Out: out, - Local: false} - err := opts.Complete(f, cmd, input.args) + opts := input.opts + opts.Complete(f, cmd) + err := opts.Validate(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[""] }