diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go index a8ebf6410c8..c77b79bfd88 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder.go @@ -72,9 +72,10 @@ type Builder struct { errs []error - paths []Visitor - stream bool - dir bool + paths []Visitor + stream bool + stdinInUse bool + dir bool labelSelector *string fieldSelector *string @@ -121,6 +122,8 @@ Example resource specifications include: '-f rsrc.yaml' '--filename=rsrc.json'`) +var StdinMultiUseError = errors.New("standard input cannot be used for multiple arguments") + // TODO: expand this to include other errors. func IsUsageError(err error) bool { if err == nil { @@ -362,13 +365,31 @@ func (b *Builder) URL(httpAttemptCount int, urls ...*url.URL) *Builder { // Stdin will read objects from the standard input. If ContinueOnError() is set // prior to this method being called, objects in the stream that are unrecognized -// will be ignored (but logged at V(2)). +// will be ignored (but logged at V(2)). If StdinInUse() is set prior to this method +// being called, an error will be recorded as there are multiple entities trying to use +// the single standard input stream. func (b *Builder) Stdin() *Builder { b.stream = true + if b.stdinInUse { + b.errs = append(b.errs, StdinMultiUseError) + } + b.stdinInUse = true b.paths = append(b.paths, FileVisitorForSTDIN(b.mapper, b.schema)) return b } +// StdinInUse will mark standard input as in use by this Builder, and therefore standard +// input should not be used by another entity. If Stdin() is set prior to this method +// being called, an error will be recorded as there are multiple entities trying to use +// the single standard input stream. +func (b *Builder) StdinInUse() *Builder { + if b.stdinInUse { + b.errs = append(b.errs, StdinMultiUseError) + } + b.stdinInUse = true + return b +} + // Stream will read objects from the provided reader, and if an error occurs will // include the name string in the error message. If ContinueOnError() is set // prior to this method being called, objects in the stream that are unrecognized @@ -911,9 +932,9 @@ func (b *Builder) getClient(gv schema.GroupVersion) (RESTClient, error) { case b.fakeClientFn != nil: client, err = b.fakeClientFn(gv) case b.negotiatedSerializer != nil: - client, err = b.clientConfigFn.clientForGroupVersion(gv, b.negotiatedSerializer) + client, err = b.clientConfigFn.withStdinUnavailable(b.stdinInUse).clientForGroupVersion(gv, b.negotiatedSerializer) default: - client, err = b.clientConfigFn.unstructuredClientForGroupVersion(gv) + client, err = b.clientConfigFn.withStdinUnavailable(b.stdinInUse).unstructuredClientForGroupVersion(gv) } if err != nil { diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go index 5f5fe433ae1..6a46454195d 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go @@ -18,6 +18,7 @@ package resource import ( "bytes" + "errors" "fmt" "io" "io/ioutil" @@ -1793,3 +1794,12 @@ func TestUnstructured(t *testing.T) { }) } } + +func TestStdinMultiUseError(t *testing.T) { + if got, want := newUnstructuredDefaultBuilder().Stdin().StdinInUse().Do().Err(), StdinMultiUseError; !errors.Is(got, want) { + t.Errorf("got: %q, want: %q", got, want) + } + if got, want := newUnstructuredDefaultBuilder().StdinInUse().Stdin().Do().Err(), StdinMultiUseError; !errors.Is(got, want) { + t.Errorf("got: %q, want: %q", got, want) + } +} diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/client.go b/staging/src/k8s.io/cli-runtime/pkg/resource/client.go index 46380207f3e..cd52c304313 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/client.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/client.go @@ -56,3 +56,14 @@ func (clientConfigFn ClientConfigFunc) unstructuredClientForGroupVersion(gv sche return rest.RESTClientFor(cfg) } + +func (clientConfigFn ClientConfigFunc) withStdinUnavailable(stdinUnavailable bool) ClientConfigFunc { + return func() (*rest.Config, error) { + cfg, err := clientConfigFn() + if stdinUnavailable && cfg != nil && cfg.ExecProvider != nil { + cfg.ExecProvider.StdinUnavailable = stdinUnavailable + cfg.ExecProvider.StdinUnavailableMessage = "used by stdin resource manifest reader" + } + return cfg, err + } +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/replace/replace.go b/staging/src/k8s.io/kubectl/pkg/cmd/replace/replace.go index 3a7d3224bd0..5ac2047c7fd 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/replace/replace.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/replace/replace.go @@ -306,6 +306,7 @@ func (o *ReplaceOptions) Run(f cmdutil.Factory) error { } func (o *ReplaceOptions) forceReplace() error { + stdinInUse := false for i, filename := range o.DeleteOptions.FilenameOptions.Filenames { if filename == "-" { tempDir, err := ioutil.TempDir("", "kubectl_replace_") @@ -319,17 +320,21 @@ func (o *ReplaceOptions) forceReplace() error { return err } o.DeleteOptions.FilenameOptions.Filenames[i] = tempFilename + stdinInUse = true } } - r := o.Builder(). + b := o.Builder(). Unstructured(). ContinueOnError(). NamespaceParam(o.Namespace).DefaultNamespace(). ResourceTypeOrNameArgs(false, o.BuilderArgs...).RequireObject(false). FilenameParam(o.EnforceNamespace, &o.DeleteOptions.FilenameOptions). - Flatten(). - Do() + Flatten() + if stdinInUse { + b = b.StdinInUse() + } + r := b.Do() if err := r.Err(); err != nil { return err } @@ -358,14 +363,17 @@ func (o *ReplaceOptions) forceReplace() error { return err } - r = o.Builder(). + b = o.Builder(). Unstructured(). Schema(o.Schema). ContinueOnError(). NamespaceParam(o.Namespace).DefaultNamespace(). FilenameParam(o.EnforceNamespace, &o.DeleteOptions.FilenameOptions). - Flatten(). - Do() + Flatten() + if stdinInUse { + b = b.StdinInUse() + } + r = b.Do() err = r.Err() if err != nil { return err diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse.go b/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse.go index e93fed8311d..5f46237d29d 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse.go @@ -23,7 +23,7 @@ import ( "regexp" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" ) @@ -59,28 +59,30 @@ func SplitEnvironmentFromResources(args []string) (resources, envArgs []string, // parseIntoEnvVar parses the list of key-value pairs into kubernetes EnvVar. // envVarType is for making errors more specific to user intentions. -func parseIntoEnvVar(spec []string, defaultReader io.Reader, envVarType string) ([]v1.EnvVar, []string, error) { +func parseIntoEnvVar(spec []string, defaultReader io.Reader, envVarType string) ([]v1.EnvVar, []string, bool, error) { env := []v1.EnvVar{} exists := sets.NewString() var remove []string + usedStdin := false for _, envSpec := range spec { switch { case envSpec == "-": if defaultReader == nil { - return nil, nil, fmt.Errorf("when '-' is used, STDIN must be open") + return nil, nil, usedStdin, fmt.Errorf("when '-' is used, STDIN must be open") } fileEnv, err := readEnv(defaultReader, envVarType) if err != nil { - return nil, nil, err + return nil, nil, usedStdin, err } env = append(env, fileEnv...) + usedStdin = true case strings.Contains(envSpec, "="): parts := strings.SplitN(envSpec, "=", 2) if len(parts) != 2 { - return nil, nil, fmt.Errorf("invalid %s: %v", envVarType, envSpec) + return nil, nil, usedStdin, fmt.Errorf("invalid %s: %v", envVarType, envSpec) } if errs := validation.IsEnvVarName(parts[0]); len(errs) != 0 { - return nil, nil, fmt.Errorf("%q is not a valid key name: %s", parts[0], strings.Join(errs, ";")) + return nil, nil, usedStdin, fmt.Errorf("%q is not a valid key name: %s", parts[0], strings.Join(errs, ";")) } exists.Insert(parts[0]) env = append(env, v1.EnvVar{ @@ -90,20 +92,20 @@ func parseIntoEnvVar(spec []string, defaultReader io.Reader, envVarType string) case strings.HasSuffix(envSpec, "-"): remove = append(remove, envSpec[:len(envSpec)-1]) default: - return nil, nil, fmt.Errorf("unknown %s: %v", envVarType, envSpec) + return nil, nil, usedStdin, fmt.Errorf("unknown %s: %v", envVarType, envSpec) } } for _, removeLabel := range remove { if _, found := exists[removeLabel]; found { - return nil, nil, fmt.Errorf("can not both modify and remove the same %s in the same command", envVarType) + return nil, nil, usedStdin, fmt.Errorf("can not both modify and remove the same %s in the same command", envVarType) } } - return env, remove, nil + return env, remove, usedStdin, nil } -// ParseEnv parses the elements of the first argument looking for environment variables in key=value form and, if one of those values is "-", it also scans the reader. +// ParseEnv parses the elements of the first argument looking for environment variables in key=value form and, if one of those values is "-", it also scans the reader and returns true for its third return value. // The same environment variable cannot be both modified and removed in the same command. -func ParseEnv(spec []string, defaultReader io.Reader) ([]v1.EnvVar, []string, error) { +func ParseEnv(spec []string, defaultReader io.Reader) ([]v1.EnvVar, []string, bool, error) { return parseIntoEnvVar(spec, defaultReader, "environment variable") } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse_test.go index e877e0c016d..f378312bb90 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse_test.go @@ -40,12 +40,27 @@ func ExampleSplitEnvironmentFromResources() { // Output: [resource] [ENV\=ARG ONE\=MORE DASH-] true } -func ExampleParseEnv_good() { +func ExampleParseEnv_good_with_stdin() { r := strings.NewReader("FROM=READER") ss := []string{"ENV=VARIABLE", "ENV.TEST=VARIABLE", "AND=ANOTHER", "REMOVE-", "-"} fmt.Println(ParseEnv(ss, r)) // Output: - // [{ENV VARIABLE nil} {ENV.TEST VARIABLE nil} {AND ANOTHER nil} {FROM READER nil}] [REMOVE] + // [{ENV VARIABLE nil} {ENV.TEST VARIABLE nil} {AND ANOTHER nil} {FROM READER nil}] [REMOVE] true +} + +func ExampleParseEnv_good_with_stdin_and_error() { + r := strings.NewReader("FROM=READER") + ss := []string{"-", "This not in the key=value format."} + fmt.Println(ParseEnv(ss, r)) + // Output: + // [] [] true "This not in the 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]*') +} + +func ExampleParseEnv_good_without_stdin() { + ss := []string{"ENV=VARIABLE", "ENV.TEST=VARIABLE", "AND=ANOTHER", "REMOVE-"} + fmt.Println(ParseEnv(ss, nil)) + // Output: + // [{ENV VARIABLE nil} {ENV.TEST VARIABLE nil} {AND ANOTHER nil}] [REMOVE] false } func ExampleParseEnv_bad_first() { @@ -53,7 +68,7 @@ func ExampleParseEnv_bad_first() { bad := []string{"This not in the key=value format."} fmt.Println(ParseEnv(bad, r)) // Output: - // [] [] "This not in the 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]*') + // [] [] false "This not in the 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]*') } func ExampleParseEnv_bad_second() { @@ -61,7 +76,7 @@ func ExampleParseEnv_bad_second() { bad := []string{".=VARIABLE"} fmt.Println(ParseEnv(bad, r)) // Output: - // [] [] "." is not a valid key name: must not be '.' + // [] [] false "." is not a valid key name: must not be '.' } func ExampleParseEnv_bad_third() { @@ -69,7 +84,7 @@ func ExampleParseEnv_bad_third() { bad := []string{"..=VARIABLE"} fmt.Println(ParseEnv(bad, r)) // Output: - // [] [] ".." is not a valid key name: must not be '..' + // [] [] false ".." is not a valid key name: must not be '..' } func ExampleParseEnv_bad_fourth() { @@ -77,5 +92,5 @@ func ExampleParseEnv_bad_fourth() { bad := []string{"..ENV=VARIABLE"} fmt.Println(ParseEnv(bad, r)) // Output: - // [] [] "..ENV" is not a valid key name: must not start with '..' + // [] [] false "..ENV" is not a valid key name: must not start with '..' } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env.go b/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env.go index 82a96cb27aa..7986df4357e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env.go @@ -270,7 +270,7 @@ func (o *EnvOptions) Validate() error { // RunEnv contains all the necessary functionality for the OpenShift cli env command func (o *EnvOptions) RunEnv() error { - env, remove, err := envutil.ParseEnv(append(o.EnvParams, o.envArgs...), o.In) + env, remove, envFromStdin, err := envutil.ParseEnv(append(o.EnvParams, o.envArgs...), o.In) if err != nil { return err } @@ -291,6 +291,10 @@ func (o *EnvOptions) RunEnv() error { Latest() } + if envFromStdin { + b = b.StdinInUse() + } + infos, err := b.Do().Infos() if err != nil { return err @@ -358,6 +362,10 @@ func (o *EnvOptions) RunEnv() error { Latest() } + if envFromStdin { + b = b.StdinInUse() + } + infos, err := b.Do().Infos() if err != nil { return err diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env_test.go index baa1a0188f5..4a610d892a6 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/set/set_env_test.go @@ -765,3 +765,32 @@ func TestSetEnvRemoteWithSpecificContainers(t *testing.T) { }) } } + +func TestSetEnvDoubleStdinUsage(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace("test") + defer tf.Cleanup() + + tf.Client = &fake.RESTClient{ + GroupVersion: schema.GroupVersion{Version: ""}, + NegotiatedSerializer: scheme.Codecs.WithoutConversion(), + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + t.Fatalf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req) + return nil, nil + }), + } + tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}} + + streams, bufIn, _, _ := genericclioptions.NewTestIOStreams() + bufIn.WriteString("SOME_ENV_VAR_KEY=SOME_ENV_VAR_VAL") + opts := NewEnvOptions(streams) + opts.FilenameOptions = resource.FilenameOptions{ + Filenames: []string{"-"}, + } + + err := opts.Complete(tf, NewCmdEnv(tf, streams), []string{"-"}) + assert.NoError(t, err) + err = opts.Validate() + assert.NoError(t, err) + err = opts.RunEnv() + assert.ErrorIs(t, err, resource.StdinMultiUseError) +}