diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 741ba7762ca..b3da28844e8 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -4069,13 +4069,20 @@ run_plugins_tests() { kube::test::if_has_not_string "${output_message}" 'The first child' # plugin env - output_message=$(KUBECTL_PLUGINS_PATH=test/fixtures/pkg/kubectl/plugins kubectl plugin env 2>&1) + output_message=$(KUBECTL_PLUGINS_PATH=test/fixtures/pkg/kubectl/plugins kubectl plugin env -h 2>&1) + kube::test::if_has_string "${output_message}" "This is a flag 1" + kube::test::if_has_string "${output_message}" "This is a flag 2" + kube::test::if_has_string "${output_message}" "This is a flag 3" + output_message=$(KUBECTL_PLUGINS_PATH=test/fixtures/pkg/kubectl/plugins kubectl plugin env --test1=value1 -t value2 2>&1) kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_CURRENT_NAMESPACE' kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_CALLER' kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_DESCRIPTOR_COMMAND=./env.sh' kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_DESCRIPTOR_SHORT_DESC=The plugin envs plugin' kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_GLOBAL_FLAG_KUBECONFIG' kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_GLOBAL_FLAG_REQUEST_TIMEOUT=0' + kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_LOCAL_FLAG_TEST1=value1' + kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_LOCAL_FLAG_TEST2=value2' + kube::test::if_has_string "${output_message}" 'KUBECTL_PLUGINS_LOCAL_FLAG_TEST3=default' set +o nounset set +o errexit diff --git a/pkg/kubectl/cmd/plugin.go b/pkg/kubectl/cmd/plugin.go index 0ea15522871..b972d9bc814 100644 --- a/pkg/kubectl/cmd/plugin.go +++ b/pkg/kubectl/cmd/plugin.go @@ -114,6 +114,10 @@ func NewCmdForPlugin(f cmdutil.Factory, plugin *plugins.Plugin, runner plugins.P }, } + for _, flag := range plugin.Flags { + cmd.Flags().StringP(flag.Name, flag.Shorthand, flag.DefValue, flag.Desc) + } + for _, childPlugin := range plugin.Tree { cmd.AddCommand(NewCmdForPlugin(f, childPlugin, runner, in, out, errout)) } @@ -126,10 +130,14 @@ type flagsPluginEnvProvider struct { } func (p *flagsPluginEnvProvider) Env() (plugins.EnvList, error) { - prefix := "KUBECTL_PLUGINS_GLOBAL_FLAG_" + globalPrefix := "KUBECTL_PLUGINS_GLOBAL_FLAG_" env := plugins.EnvList{} - p.cmd.Flags().VisitAll(func(flag *pflag.Flag) { - env = append(env, plugins.FlagToEnv(flag, prefix)) + p.cmd.InheritedFlags().VisitAll(func(flag *pflag.Flag) { + env = append(env, plugins.FlagToEnv(flag, globalPrefix)) + }) + localPrefix := "KUBECTL_PLUGINS_LOCAL_FLAG_" + p.cmd.LocalFlags().VisitAll(func(flag *pflag.Flag) { + env = append(env, plugins.FlagToEnv(flag, localPrefix)) }) return env, nil } diff --git a/pkg/kubectl/plugins/loader_test.go b/pkg/kubectl/plugins/loader_test.go index 96a0b4f7255..b89ee671bfa 100644 --- a/pkg/kubectl/plugins/loader_test.go +++ b/pkg/kubectl/plugins/loader_test.go @@ -232,7 +232,10 @@ func setupValidPlugins(nPlugins, nChildren int) (string, error) { descriptor := fmt.Sprintf(` name: %[1]s shortDesc: The %[1]s test plugin -command: echo %[1]s`, name) +command: echo %[1]s +flags: + - name: %[1]s-flag + desc: A flag for %[1]s`, name) if nChildren > 0 { descriptor += ` diff --git a/pkg/kubectl/plugins/plugins.go b/pkg/kubectl/plugins/plugins.go index 4b65b98f246..e0bea1a3b46 100644 --- a/pkg/kubectl/plugins/plugins.go +++ b/pkg/kubectl/plugins/plugins.go @@ -19,6 +19,15 @@ package plugins import ( "fmt" "strings" + "unicode" +) + +var ( + IncompletePluginError = fmt.Errorf("incomplete plugin descriptor: name, shortDesc and command fields are required") + InvalidPluginNameError = fmt.Errorf("plugin name can't contain spaces") + IncompleteFlagError = fmt.Errorf("incomplete flag descriptor: name and desc fields are required") + InvalidFlagNameError = fmt.Errorf("flag name can't contain spaces") + InvalidFlagShorthandError = fmt.Errorf("flag shorthand must be only one letter") ) // Plugin is the representation of a CLI extension (plugin). @@ -31,12 +40,13 @@ type Plugin struct { // PluginDescription holds everything needed to register a // plugin as a command. Usually comes from a descriptor file. type Description struct { - Name string `json:"name"` - ShortDesc string `json:"shortDesc"` - LongDesc string `json:"longDesc,omitempty"` - Example string `json:"example,omitempty"` - Command string `json:"command"` - Tree []*Plugin `json:"tree,omitempty"` + Name string `json:"name"` + ShortDesc string `json:"shortDesc"` + LongDesc string `json:"longDesc,omitempty"` + Example string `json:"example,omitempty"` + Command string `json:"command"` + Flags []Flag `json:"flags,omitempty"` + Tree Plugins `json:"tree,omitempty"` } // PluginSource holds the location of a given plugin in the filesystem. @@ -45,17 +55,17 @@ type Source struct { DescriptorName string `json:"-"` } -var ( - IncompleteError = fmt.Errorf("incomplete plugin descriptor: name, shortDesc and command fields are required") - InvalidNameError = fmt.Errorf("plugin name can't contain spaces") -) - func (p Plugin) Validate() error { if len(p.Name) == 0 || len(p.ShortDesc) == 0 || (len(p.Command) == 0 && len(p.Tree) == 0) { - return IncompleteError + return IncompletePluginError } if strings.Index(p.Name, " ") > -1 { - return InvalidNameError + return InvalidPluginNameError + } + for _, flag := range p.Flags { + if err := flag.Validate(); err != nil { + return err + } } for _, child := range p.Tree { if err := child.Validate(); err != nil { @@ -71,3 +81,33 @@ func (p Plugin) IsValid() bool { // Plugins is a list of plugins. type Plugins []*Plugin + +// Flag describes a single flag supported by a given plugin. +type Flag struct { + Name string `json:"name"` + Shorthand string `json:"shorthand,omitempty"` + Desc string `json:"desc"` + DefValue string `json:"defValue,omitempty"` +} + +func (f Flag) Validate() error { + if len(f.Name) == 0 || len(f.Desc) == 0 { + return IncompleteFlagError + } + if strings.Index(f.Name, " ") > -1 { + return InvalidFlagNameError + } + return f.ValidateShorthand() +} + +func (f Flag) ValidateShorthand() error { + length := len(f.Shorthand) + if length == 0 || (length == 1 && unicode.IsLetter(rune(f.Shorthand[0]))) { + return nil + } + return InvalidFlagShorthandError +} + +func (f Flag) Shorthanded() bool { + return f.ValidateShorthand() == nil +} diff --git a/pkg/kubectl/plugins/plugins_test.go b/pkg/kubectl/plugins/plugins_test.go index b7cb78b34ef..1bbe44a5cd5 100644 --- a/pkg/kubectl/plugins/plugins_test.go +++ b/pkg/kubectl/plugins/plugins_test.go @@ -16,55 +16,154 @@ limitations under the License. package plugins -import ( - "strings" - "testing" -) +import "testing" func TestPlugin(t *testing.T) { tests := []struct { - plugin Plugin - expectedErr string - expectedValid bool + plugin *Plugin + expectedErr error }{ { - plugin: Plugin{ + plugin: &Plugin{ Description: Description{ Name: "test", ShortDesc: "The test", Command: "echo 1", }, }, - expectedValid: true, }, { - plugin: Plugin{ + plugin: &Plugin{ Description: Description{ Name: "test", ShortDesc: "The test", }, }, - expectedErr: "incomplete", + expectedErr: IncompletePluginError, }, { - plugin: Plugin{}, - expectedErr: "incomplete", + plugin: &Plugin{}, + expectedErr: IncompletePluginError, + }, + { + plugin: &Plugin{ + Description: Description{ + Name: "test spaces", + ShortDesc: "The test", + Command: "echo 1", + }, + }, + expectedErr: InvalidPluginNameError, + }, + { + plugin: &Plugin{ + Description: Description{ + Name: "test", + ShortDesc: "The test", + Command: "echo 1", + Flags: []Flag{ + { + Name: "aflag", + }, + }, + }, + }, + expectedErr: IncompleteFlagError, + }, + { + plugin: &Plugin{ + Description: Description{ + Name: "test", + ShortDesc: "The test", + Command: "echo 1", + Flags: []Flag{ + { + Name: "a flag", + Desc: "Invalid flag", + }, + }, + }, + }, + expectedErr: InvalidFlagNameError, + }, + { + plugin: &Plugin{ + Description: Description{ + Name: "test", + ShortDesc: "The test", + Command: "echo 1", + Flags: []Flag{ + { + Name: "aflag", + Desc: "Invalid shorthand", + Shorthand: "aa", + }, + }, + }, + }, + expectedErr: InvalidFlagShorthandError, + }, + { + plugin: &Plugin{ + Description: Description{ + Name: "test", + ShortDesc: "The test", + Command: "echo 1", + Flags: []Flag{ + { + Name: "aflag", + Desc: "Invalid shorthand", + Shorthand: "2", + }, + }, + }, + }, + expectedErr: InvalidFlagShorthandError, + }, + { + plugin: &Plugin{ + Description: Description{ + Name: "test", + ShortDesc: "The test", + Command: "echo 1", + Flags: []Flag{ + { + Name: "aflag", + Desc: "A flag", + Shorthand: "a", + }, + }, + Tree: Plugins{ + &Plugin{ + Description: Description{ + Name: "child", + ShortDesc: "The child", + LongDesc: "The child long desc", + Example: "You can use it like this but you're not supposed to", + Command: "echo 1", + Flags: []Flag{ + { + Name: "childflag", + Desc: "A child flag", + }, + { + Name: "childshorthand", + Desc: "A child shorthand flag", + Shorthand: "s", + }, + }, + }, + }, + }, + }, + }, }, } for _, test := range tests { - if is := test.plugin.IsValid(); test.expectedValid != is { - t.Errorf("%s: expected valid=%v, got %v", test.plugin.Name, test.expectedValid, is) - } err := test.plugin.Validate() - if len(test.expectedErr) > 0 { - if err == nil { - t.Errorf("%s: expected error, got none", test.plugin.Name) - } else if !strings.Contains(err.Error(), test.expectedErr) { - t.Errorf("%s: expected error containing %q, got %v", test.plugin.Name, test.expectedErr, err) - } - } else if err != nil { - t.Errorf("%s: expected no error, got %v", test.plugin.Name, err) + if err != test.expectedErr { + t.Errorf("%s: expected error %v, got %v", test.plugin.Name, test.expectedErr, err) } } } diff --git a/test/fixtures/pkg/kubectl/plugins/env/plugin.yaml b/test/fixtures/pkg/kubectl/plugins/env/plugin.yaml index f877c2f56d3..b01d44a843c 100644 --- a/test/fixtures/pkg/kubectl/plugins/env/plugin.yaml +++ b/test/fixtures/pkg/kubectl/plugins/env/plugin.yaml @@ -1,3 +1,12 @@ name: env shortDesc: "The plugin envs plugin" command: "./env.sh" +flags: + - name: "test1" + desc: "This is a flag 1" + - name: "test2" + desc: "This is a flag 2" + shorthand: "t" + - name: "test3" + desc: "This is a flag 3" + defValue: "default"