kubectl: don't fight for stdin with exec plugins

Signed-off-by: Andrew Keesler <akeesler@vmware.com>

Kubernetes-commit: 60246f69cc0fcc6f5cfd0a36eb069187a4bd9e98
This commit is contained in:
Andrew Keesler 2021-06-14 17:16:54 -04:00 committed by Kubernetes Publisher
parent f81209afea
commit d9980eb6ca
5 changed files with 86 additions and 24 deletions

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

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

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 '..'
}

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

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