github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-11 21:12:07 +00:00
Code Issues Projects Releases Wiki Activity

kubectl: don't fight for stdin with exec plugins

Browse Source 
Signed-off-by: Andrew Keesler <akeesler@vmware.com>
This commit is contained in:
Andrew Keesler 2021-06-14 17:16:54 -04:00
parent cd83d89ac9
commit 60246f69cc
No known key found for this signature in database
GPG Key ID: 27CE0444346F9413
8 changed files with 134 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
staging/src/k8s.io/cli-runtime/pkg/resource/builder.go
View File

@ -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 {

10
staging/src/k8s.io/cli-runtime/pkg/resource/builder_test.go
View File

@ -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)
}
}

11
staging/src/k8s.io/cli-runtime/pkg/resource/client.go
View File

@ -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
}
}

20
staging/src/k8s.io/kubectl/pkg/cmd/replace/replace.go
View File

@ -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

24
staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse.go vendored
View File

@ -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")
}

27
staging/src/k8s.io/kubectl/pkg/cmd/set/env/env_parse_test.go vendored
View File

@ -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] <nil>
// [{ENV VARIABLE nil} {ENV.TEST VARIABLE nil} {AND ANOTHER nil} {FROM READER nil}] [REMOVE] true <nil>
}
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 <nil>
}
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 '..'
}

10
staging/src/k8s.io/kubectl/pkg/cmd/set/set_env.go
View File

@ -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

29
staging/src/k8s.io/kubectl/pkg/cmd/set/set_env_test.go
View File

@ -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)
}