From 8fb423bfabe0d53934cc94c154c7da2dc3ce1332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Tue, 17 May 2022 11:38:20 +0300 Subject: [PATCH] Set validate functions requiring no parameters for all commands Validate function is used to validate command options and should not get any additional parameter. To preserve compatibility across all kubectl commands, this PR removes all parameters in validate functions. --- .../src/k8s.io/kubectl/pkg/cmd/apply/apply.go | 12 +++---- .../pkg/cmd/apply/apply_view_last_applied.go | 4 +-- .../kubectl/pkg/cmd/config/get_contexts.go | 27 +++++++--------- staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go | 22 +++++++------ .../src/k8s.io/kubectl/pkg/cmd/cp/cp_test.go | 11 +++---- .../k8s.io/kubectl/pkg/cmd/create/create.go | 32 +++++++++---------- .../kubectl/pkg/cmd/create/create_test.go | 5 +-- .../src/k8s.io/kubectl/pkg/cmd/debug/debug.go | 4 +-- .../kubectl/pkg/cmd/debug/debug_test.go | 2 +- .../kubectl/pkg/cmd/describe/describe.go | 3 +- .../src/k8s.io/kubectl/pkg/cmd/diff/diff.go | 22 +++++++------ .../k8s.io/kubectl/pkg/cmd/explain/explain.go | 24 ++++++++------ staging/src/k8s.io/kubectl/pkg/cmd/get/get.go | 16 +++++----- .../k8s.io/kubectl/pkg/cmd/replace/replace.go | 18 +++++------ .../src/k8s.io/kubectl/pkg/cmd/scale/scale.go | 4 +-- .../k8s.io/kubectl/pkg/cmd/version/version.go | 16 ++++++---- .../kubectl/pkg/cmd/version/version_test.go | 9 ++++-- 17 files changed, 123 insertions(+), 108 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go index 2f13cf9698f..d73a9d6ad29 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go @@ -193,7 +193,7 @@ func NewCmdApply(baseName string, f cmdutil.Factory, ioStreams genericclioptions Run: func(cmd *cobra.Command, args []string) { o, err := flags.ToOptions(cmd, baseName, args) cmdutil.CheckErr(err) - cmdutil.CheckErr(o.Validate(cmd, args)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.Run()) }, } @@ -230,6 +230,10 @@ func (flags *ApplyFlags) AddFlags(cmd *cobra.Command) { // ToOptions converts from CLI inputs to runtime inputs func (flags *ApplyFlags) ToOptions(cmd *cobra.Command, baseName string, args []string) (*ApplyOptions, error) { + if len(args) != 0 { + return nil, cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args) + } + serverSideApply := cmdutil.GetServerSideApplyFlag(cmd) forceConflicts := cmdutil.GetForceConflictsFlag(cmd) dryRunStrategy, err := cmdutil.GetDryRunStrategy(cmd) @@ -344,11 +348,7 @@ func (flags *ApplyFlags) ToOptions(cmd *cobra.Command, baseName string, args []s } // Validate verifies if ApplyOptions are valid and without conflicts. -func (o *ApplyOptions) Validate(cmd *cobra.Command, args []string) error { - if len(args) != 0 { - return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args) - } - +func (o *ApplyOptions) Validate() error { if o.ForceConflicts && !o.ServerSideApply { return fmt.Errorf("--force-conflicts only works with --server-side") } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_view_last_applied.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_view_last_applied.go index 30983fe3cff..c9bf7d62eb2 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_view_last_applied.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_view_last_applied.go @@ -81,7 +81,7 @@ func NewCmdApplyViewLastApplied(f cmdutil.Factory, ioStreams genericclioptions.I ValidArgsFunction: completion.ResourceTypeAndNameCompletionFunc(f), Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(options.Complete(cmd, f, args)) - cmdutil.CheckErr(options.Validate(cmd)) + cmdutil.CheckErr(options.Validate()) cmdutil.CheckErr(options.RunApplyViewLastApplied(cmd)) }, } @@ -141,7 +141,7 @@ func (o *ViewLastAppliedOptions) Complete(cmd *cobra.Command, f cmdutil.Factory, } // Validate checks ViewLastAppliedOptions for validity. -func (o *ViewLastAppliedOptions) Validate(cmd *cobra.Command) error { +func (o *ViewLastAppliedOptions) Validate() error { return nil } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts.go index 033a179ae46..3bd5459f5d5 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/get_contexts.go @@ -43,6 +43,9 @@ type GetContextsOptions struct { showHeaders bool contextNames []string + outputFormat string + noHeaders bool + genericclioptions.IOStreams } @@ -73,22 +76,26 @@ func NewCmdConfigGetContexts(streams genericclioptions.IOStreams, configAccess c Long: getContextsLong, Example: getContextsExample, Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckErr(options.Validate(cmd)) cmdutil.CheckErr(options.Complete(cmd, args)) + cmdutil.CheckErr(options.Validate()) cmdutil.CheckErr(options.RunGetContexts()) }, } - cmd.Flags().Bool("no-headers", false, "When using the default or custom-column output format, don't print headers (default print headers).") - cmd.Flags().StringP("output", "o", "", `Output format. One of: (name).`) + cmd.Flags().BoolVar(&options.noHeaders, "no-headers", options.noHeaders, "When using the default or custom-column output format, don't print headers (default print headers).") + cmd.Flags().StringVarP(&options.outputFormat, "output", "o", options.outputFormat, `Output format. One of: (name).`) return cmd } // Complete assigns GetContextsOptions from the args. func (o *GetContextsOptions) Complete(cmd *cobra.Command, args []string) error { + supportedOutputTypes := sets.NewString("", "name") + if !supportedOutputTypes.Has(o.outputFormat) { + return fmt.Errorf("--output %v is not available in kubectl config get-contexts; resetting to default output format", o.outputFormat) + } o.contextNames = args o.nameOnly = false - if cmdutil.GetFlagString(cmd, "output") == "name" { + if o.outputFormat == "name" { o.nameOnly = true } o.showHeaders = true @@ -100,17 +107,7 @@ func (o *GetContextsOptions) Complete(cmd *cobra.Command, args []string) error { } // Validate ensures the of output format -func (o *GetContextsOptions) Validate(cmd *cobra.Command) error { - validOutputTypes := sets.NewString("", "json", "yaml", "wide", "name", "custom-columns", "custom-columns-file", "go-template", "go-template-file", "jsonpath", "jsonpath-file") - supportedOutputTypes := sets.NewString("", "name") - outputFormat := cmdutil.GetFlagString(cmd, "output") - if !validOutputTypes.Has(outputFormat) { - return fmt.Errorf("output must be one of '' or 'name': %v", outputFormat) - } - if !supportedOutputTypes.Has(outputFormat) { - cmd.Flags().Set("output", "") - return fmt.Errorf("--output %v is not available in kubectl config get-contexts; resetting to default output format", outputFormat) - } +func (o *GetContextsOptions) Validate() error { return nil } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go index 415e85a5dd2..9a90d133ade 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go @@ -77,6 +77,8 @@ type CopyOptions struct { Clientset kubernetes.Interface ExecParentCmdName string + args []string + genericclioptions.IOStreams } @@ -149,9 +151,9 @@ func NewCmdCp(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.C return comps, cobra.ShellCompDirectiveNoSpace }, Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckErr(o.Complete(f, cmd)) - cmdutil.CheckErr(o.Validate(cmd, args)) - cmdutil.CheckErr(o.Run(args)) + cmdutil.CheckErr(o.Complete(f, cmd, args)) + cmdutil.CheckErr(o.Validate()) + cmdutil.CheckErr(o.Run()) }, } cmdutil.AddContainerVarFlags(cmd, &o.Container, o.Container) @@ -198,7 +200,7 @@ func extractFileSpec(arg string) (fileSpec, error) { } // Complete completes all the required options -func (o *CopyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { +func (o *CopyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { if cmd.Parent() != nil { o.ExecParentCmdName = cmd.Parent().CommandPath() } @@ -218,24 +220,26 @@ func (o *CopyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { if err != nil { return err } + + o.args = args return nil } // Validate makes sure provided values for CopyOptions are valid -func (o *CopyOptions) Validate(cmd *cobra.Command, args []string) error { - if len(args) != 2 { +func (o *CopyOptions) Validate() error { + if len(o.args) != 2 { return fmt.Errorf("source and destination are required") } return nil } // Run performs the execution -func (o *CopyOptions) Run(args []string) error { - srcSpec, err := extractFileSpec(args[0]) +func (o *CopyOptions) Run() error { + srcSpec, err := extractFileSpec(o.args[0]) if err != nil { return err } - destSpec, err := extractFileSpec(args[1]) + destSpec, err := extractFileSpec(o.args[1]) if err != nil { return err } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp_test.go index 10a1428ca9c..dffd89f88d7 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp_test.go @@ -659,9 +659,9 @@ func TestCopyToPod(t *testing.T) { for name, test := range tests { opts := NewCopyOptions(ioStreams) - opts.Complete(tf, cmd) + opts.Complete(tf, cmd, []string{test.src, fmt.Sprintf("pod-ns/pod-name:%s", test.dest)}) t.Run(name, func(t *testing.T) { - err = opts.Run([]string{test.src, fmt.Sprintf("pod-ns/pod-name:%s", test.dest)}) + err = opts.Run() //If error is NotFound error , it indicates that the //request has been sent correctly. //Treat this as no error. @@ -723,7 +723,7 @@ func TestCopyToPodNoPreserve(t *testing.T) { PodName: "pod-name", File: newRemotePath("foo"), } - opts.Complete(tf, cmd) + opts.Complete(tf, cmd, nil) for name, test := range tests { t.Run(name, func(t *testing.T) { @@ -754,14 +754,13 @@ func TestValidate(t *testing.T) { expectedErr: true, }, } - tf := cmdtesting.NewTestFactory() ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() opts := NewCopyOptions(ioStreams) - cmd := NewCmdCp(tf, ioStreams) for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err := opts.Validate(cmd, test.args) + opts.args = test.args + err := opts.Validate() if (err != nil) != test.expectedErr { t.Errorf("expected error: %v, saw: %v, error: %v", test.expectedErr, err != nil, err) } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create.go index f3654998096..76dcb8de03f 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/create/create.go @@ -116,8 +116,8 @@ func NewCmdCreate(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cob defaultRunFunc(cmd, args) return } - cmdutil.CheckErr(o.Complete(f, cmd)) - cmdutil.CheckErr(o.ValidateArgs(cmd, args)) + cmdutil.CheckErr(o.Complete(f, cmd, args)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.RunCreate(f, cmd)) }, } @@ -160,32 +160,29 @@ func NewCmdCreate(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cob return cmd } -// ValidateArgs makes sure there is no discrepency in command options -func (o *CreateOptions) ValidateArgs(cmd *cobra.Command, args []string) error { - if len(args) != 0 { - return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args) - } +// Validate makes sure there is no discrepency in command options +func (o *CreateOptions) Validate() error { if len(o.Raw) > 0 { if o.EditBeforeCreate { - return cmdutil.UsageErrorf(cmd, "--raw and --edit are mutually exclusive") + return fmt.Errorf("--raw and --edit are mutually exclusive") } if len(o.FilenameOptions.Filenames) != 1 { - return cmdutil.UsageErrorf(cmd, "--raw can only use a single local file or stdin") + return fmt.Errorf("--raw can only use a single local file or stdin") } if strings.Index(o.FilenameOptions.Filenames[0], "http://") == 0 || strings.Index(o.FilenameOptions.Filenames[0], "https://") == 0 { - return cmdutil.UsageErrorf(cmd, "--raw cannot read from a url") + return fmt.Errorf("--raw cannot read from a url") } if o.FilenameOptions.Recursive { - return cmdutil.UsageErrorf(cmd, "--raw and --recursive are mutually exclusive") + return fmt.Errorf("--raw and --recursive are mutually exclusive") } if len(o.Selector) > 0 { - return cmdutil.UsageErrorf(cmd, "--raw and --selector (-l) are mutually exclusive") + return fmt.Errorf("--raw and --selector (-l) are mutually exclusive") } - if len(cmdutil.GetFlagString(cmd, "output")) > 0 { - return cmdutil.UsageErrorf(cmd, "--raw and --output are mutually exclusive") + if o.PrintFlags.OutputFormat != nil && len(*o.PrintFlags.OutputFormat) > 0 { + return fmt.Errorf("--raw and --output are mutually exclusive") } if _, err := url.ParseRequestURI(o.Raw); err != nil { - return cmdutil.UsageErrorf(cmd, "--raw must be a valid URL path: %v", err) + return fmt.Errorf("--raw must be a valid URL path: %v", err) } } @@ -193,7 +190,10 @@ func (o *CreateOptions) ValidateArgs(cmd *cobra.Command, args []string) error { } // Complete completes all the required options -func (o *CreateOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { +func (o *CreateOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { + if len(args) != 0 { + return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args) + } var err error o.RecordFlags.Complete(cmd) o.Recorder, err = o.RecordFlags.ToRecorder() diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_test.go index fdb94cafb49..7a6d7a992cf 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_test.go @@ -35,8 +35,9 @@ func TestExtraArgsFail(t *testing.T) { defer f.Cleanup() c := NewCmdCreate(f, genericclioptions.NewTestIOStreamsDiscard()) - options := CreateOptions{} - if options.ValidateArgs(c, []string{"rc"}) == nil { + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() + options := NewCreateOptions(ioStreams) + if options.Complete(f, c, []string{"rc"}) == nil { t.Errorf("unexpected non-error") } } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go index 1eaa4325b6d..12abb8b2a2f 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go @@ -151,7 +151,7 @@ func NewCmdDebug(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra. Example: debugExample, Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(o.Complete(f, cmd, args)) - cmdutil.CheckErr(o.Validate(cmd)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.Run(f, cmd)) }, } @@ -221,7 +221,7 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st } // Validate checks that the provided debug options are specified. -func (o *DebugOptions) Validate(cmd *cobra.Command) error { +func (o *DebugOptions) Validate() error { // Attach if o.Attach && o.attachChanged && len(o.Image) == 0 && len(o.Container) == 0 { return fmt.Errorf("you must specify --container or create a new container using --image in order to attach.") diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go index 95ec5b2f821..40f88f7709c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go @@ -1538,7 +1538,7 @@ func TestCompleteAndValidate(t *testing.T) { if gotError != nil { return } - gotError = opts.Validate(cmd) + gotError = opts.Validate() }, } cmd.SetArgs(strings.Split(tc.args, " ")) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/describe/describe.go b/staging/src/k8s.io/kubectl/pkg/cmd/describe/describe.go index 6ca80e6325c..e3e9de37c6c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/describe/describe.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/describe/describe.go @@ -110,6 +110,7 @@ func NewCmdDescribe(parent string, f cmdutil.Factory, streams genericclioptions. ValidArgsFunction: completion.ResourceTypeAndNameCompletionFunc(f), Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(o.Complete(f, cmd, args)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.Run()) }, } @@ -148,7 +149,7 @@ func (o *DescribeOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ return nil } -func (o *DescribeOptions) Validate(args []string) error { +func (o *DescribeOptions) Validate() error { return nil } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go index 6953251f240..fb43504227c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/diff/diff.go @@ -118,13 +118,6 @@ type DiffOptions struct { pruner *pruner } -func validateArgs(cmd *cobra.Command, args []string) error { - if len(args) != 0 { - return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args) - } - return nil -} - func NewDiffOptions(ioStreams genericclioptions.IOStreams) *DiffOptions { return &DiffOptions{ Diff: &DiffProgram{ @@ -143,8 +136,8 @@ func NewCmdDiff(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C Long: diffLong, Example: diffExample, Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckDiffErr(options.Complete(f, cmd)) - cmdutil.CheckDiffErr(validateArgs(cmd, args)) + cmdutil.CheckDiffErr(options.Complete(f, cmd, args)) + cmdutil.CheckDiffErr(options.Validate()) // `kubectl diff` propagates the error code from // diff or `KUBECTL_EXTERNAL_DIFF`. Also, we // don't want to print an error if diff returns @@ -605,7 +598,11 @@ func isConflict(err error) bool { return err != nil && errors.IsConflict(err) } -func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { +func (o *DiffOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { + if len(args) != 0 { + return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args) + } + var err error err = o.FilenameOptions.RequireFilenameOrKustomize() @@ -759,6 +756,11 @@ func (o *DiffOptions) Run() error { return differ.Run(o.Diff) } +// Validate makes sure provided values for DiffOptions are valid +func (o *DiffOptions) Validate() error { + return nil +} + func getObjectName(obj runtime.Object) (string, error) { gvk := obj.GetObjectKind().GroupVersionKind() metadata, err := meta.Accessor(obj) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go index 0ae86b1daf5..3da1ad54d48 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go @@ -58,6 +58,8 @@ type ExplainOptions struct { APIVersion string Recursive bool + args []string + Mapper meta.RESTMapper Schema openapi.Resources } @@ -80,9 +82,9 @@ func NewCmdExplain(parent string, f cmdutil.Factory, streams genericclioptions.I Long: explainLong + "\n\n" + cmdutil.SuggestAPIResources(parent), Example: explainExamples, Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckErr(o.Complete(f, cmd)) - cmdutil.CheckErr(o.Validate(args)) - cmdutil.CheckErr(o.Run(args)) + cmdutil.CheckErr(o.Complete(f, cmd, args)) + cmdutil.CheckErr(o.Validate()) + cmdutil.CheckErr(o.Run()) }, } cmd.Flags().BoolVar(&o.Recursive, "recursive", o.Recursive, "Print the fields of fields (Currently only 1 level deep)") @@ -90,7 +92,7 @@ func NewCmdExplain(parent string, f cmdutil.Factory, streams genericclioptions.I return cmd } -func (o *ExplainOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { +func (o *ExplainOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { var err error o.Mapper, err = f.ToRESTMapper() if err != nil { @@ -101,14 +103,16 @@ func (o *ExplainOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { if err != nil { return err } + + o.args = args return nil } -func (o *ExplainOptions) Validate(args []string) error { - if len(args) == 0 { +func (o *ExplainOptions) Validate() error { + if len(o.args) == 0 { return fmt.Errorf("You must specify the type of resource to explain. %s\n", cmdutil.SuggestAPIResources(o.CmdParent)) } - if len(args) > 1 { + if len(o.args) > 1 { return fmt.Errorf("We accept only this format: explain RESOURCE\n") } @@ -116,7 +120,7 @@ func (o *ExplainOptions) Validate(args []string) error { } // Run executes the appropriate steps to print a model's documentation -func (o *ExplainOptions) Run(args []string) error { +func (o *ExplainOptions) Run() error { recursive := o.Recursive apiVersionString := o.APIVersion @@ -124,7 +128,7 @@ func (o *ExplainOptions) Run(args []string) error { var fieldsPath []string var err error if len(apiVersionString) == 0 { - fullySpecifiedGVR, fieldsPath, err = explain.SplitAndParseResourceRequestWithMatchingPrefix(args[0], o.Mapper) + fullySpecifiedGVR, fieldsPath, err = explain.SplitAndParseResourceRequestWithMatchingPrefix(o.args[0], o.Mapper) if err != nil { return err } @@ -132,7 +136,7 @@ func (o *ExplainOptions) Run(args []string) error { // TODO: After we figured out the new syntax to separate group and resource, allow // the users to use it in explain (kubectl explain ). // Refer to issue #16039 for why we do this. Refer to PR #15808 that used "/" syntax. - fullySpecifiedGVR, fieldsPath, err = explain.SplitAndParseResourceRequest(args[0], o.Mapper) + fullySpecifiedGVR, fieldsPath, err = explain.SplitAndParseResourceRequest(o.args[0], o.Mapper) if err != nil { return err } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go b/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go index 82debee32e2..b5fd0c904d7 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go @@ -169,7 +169,7 @@ func NewCmdGet(parent string, f cmdutil.Factory, streams genericclioptions.IOStr // ValidArgsFunction is set when this function is called so that we have access to the util package Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(o.Complete(f, cmd, args)) - cmdutil.CheckErr(o.Validate(cmd)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.Run(f, cmd, args)) }, SuggestFor: []string{"list", "ps"}, @@ -303,26 +303,26 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri } // Validate checks the set of flags provided by the user. -func (o *GetOptions) Validate(cmd *cobra.Command) error { +func (o *GetOptions) Validate() error { if len(o.Raw) > 0 { if o.Watch || o.WatchOnly || len(o.LabelSelector) > 0 { return fmt.Errorf("--raw may not be specified with other flags that filter the server request or alter the output") } - if len(cmdutil.GetFlagString(cmd, "output")) > 0 { - return cmdutil.UsageErrorf(cmd, "--raw and --output are mutually exclusive") + if o.PrintFlags.OutputFormat != nil && len(*o.PrintFlags.OutputFormat) > 0 { + return fmt.Errorf("--raw and --output are mutually exclusive") } if _, err := url.ParseRequestURI(o.Raw); err != nil { - return cmdutil.UsageErrorf(cmd, "--raw must be a valid URL path: %v", err) + return fmt.Errorf("--raw must be a valid URL path: %v", err) } } - if cmdutil.GetFlagBool(cmd, "show-labels") { - outputOption := cmd.Flags().Lookup("output").Value.String() + if o.PrintFlags.HumanReadableFlags.ShowLabels != nil && *o.PrintFlags.HumanReadableFlags.ShowLabels && o.PrintFlags.OutputFormat != nil { + outputOption := *o.PrintFlags.OutputFormat if outputOption != "" && outputOption != "wide" { return fmt.Errorf("--show-labels option cannot be used with %s printer", outputOption) } } if o.OutputWatchEvents && !(o.Watch || o.WatchOnly) { - return cmdutil.UsageErrorf(cmd, "--output-watch-events option can only be used with --watch or --watch-only") + return fmt.Errorf("--output-watch-events option can only be used with --watch or --watch-only") } if len(o.Subresource) > 0 && !slice.ContainsString(supportedSubresources, o.Subresource, nil) { return fmt.Errorf("invalid subresource value: %q. Must be one of %v", o.Subresource, supportedSubresources) 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 c54c296c4b3..37478fd5f98 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/replace/replace.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/replace/replace.go @@ -123,7 +123,7 @@ func NewCmdReplace(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobr Example: replaceExample, Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(o.Complete(f, cmd, args)) - cmdutil.CheckErr(o.Validate(cmd)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.Run(f)) }, } @@ -218,7 +218,7 @@ func (o *ReplaceOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [] return nil } -func (o *ReplaceOptions) Validate(cmd *cobra.Command) error { +func (o *ReplaceOptions) Validate() error { if o.DeleteOptions.GracePeriod >= 0 && !o.DeleteOptions.ForceDeletion { return fmt.Errorf("--grace-period must have --force specified") } @@ -228,24 +228,24 @@ func (o *ReplaceOptions) Validate(cmd *cobra.Command) error { } if cmdutil.IsFilenameSliceEmpty(o.DeleteOptions.FilenameOptions.Filenames, o.DeleteOptions.FilenameOptions.Kustomize) { - return cmdutil.UsageErrorf(cmd, "Must specify --filename to replace") + return fmt.Errorf("Must specify --filename to replace") } if len(o.Raw) > 0 { if len(o.DeleteOptions.FilenameOptions.Filenames) != 1 { - return cmdutil.UsageErrorf(cmd, "--raw can only use a single local file or stdin") + return fmt.Errorf("--raw can only use a single local file or stdin") } if strings.Index(o.DeleteOptions.FilenameOptions.Filenames[0], "http://") == 0 || strings.Index(o.DeleteOptions.FilenameOptions.Filenames[0], "https://") == 0 { - return cmdutil.UsageErrorf(cmd, "--raw cannot read from a url") + return fmt.Errorf("--raw cannot read from a url") } if o.DeleteOptions.FilenameOptions.Recursive { - return cmdutil.UsageErrorf(cmd, "--raw and --recursive are mutually exclusive") + return fmt.Errorf("--raw and --recursive are mutually exclusive") } - if len(cmdutil.GetFlagString(cmd, "output")) > 0 { - return cmdutil.UsageErrorf(cmd, "--raw and --output are mutually exclusive") + if o.PrintFlags.OutputFormat != nil && len(*o.PrintFlags.OutputFormat) > 0 { + return fmt.Errorf("--raw and --output are mutually exclusive") } if _, err := url.ParseRequestURI(o.Raw); err != nil { - return cmdutil.UsageErrorf(cmd, "--raw must be a valid URL path: %v", err) + return fmt.Errorf("--raw must be a valid URL path: %v", err) } } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/scale/scale.go b/staging/src/k8s.io/kubectl/pkg/cmd/scale/scale.go index 9d9696e7d34..9571351d40e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/scale/scale.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/scale/scale.go @@ -117,7 +117,7 @@ func NewCmdScale(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobr ValidArgsFunction: completion.SpecifiedResourceTypeAndNameCompletionFunc(f, validArgs), Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(o.Complete(f, cmd, args)) - cmdutil.CheckErr(o.Validate(cmd)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.RunScale()) }, } @@ -181,7 +181,7 @@ func (o *ScaleOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st return nil } -func (o *ScaleOptions) Validate(cmd *cobra.Command) error { +func (o *ScaleOptions) Validate() error { if o.Replicas < 0 { return fmt.Errorf("The --replicas=COUNT flag is required, and COUNT must be greater than or equal to 0") } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/version/version.go b/staging/src/k8s.io/kubectl/pkg/cmd/version/version.go index 8941a5de50e..8bb5ae90860 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/version/version.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/version/version.go @@ -57,6 +57,8 @@ type Options struct { Short bool Output string + args []string + discoveryClient discovery.CachedDiscoveryInterface genericclioptions.IOStreams @@ -79,8 +81,8 @@ func NewCmdVersion(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *co Long: i18n.T("Print the client and server version information for the current context."), Example: versionExample, Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckErr(o.Complete(f, cmd)) - cmdutil.CheckErr(o.Validate(args)) + cmdutil.CheckErr(o.Complete(f, cmd, args)) + cmdutil.CheckErr(o.Validate()) cmdutil.CheckErr(o.Run()) }, } @@ -92,7 +94,7 @@ func NewCmdVersion(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *co } // Complete completes all the required options -func (o *Options) Complete(f cmdutil.Factory, cmd *cobra.Command) error { +func (o *Options) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { var err error if o.ClientOnly { return nil @@ -103,13 +105,15 @@ func (o *Options) Complete(f cmdutil.Factory, cmd *cobra.Command) error { if err != nil && !clientcmd.IsEmptyConfig(err) { return err } + + o.args = args return nil } // Validate validates the provided options -func (o *Options) Validate(args []string) error { - if len(args) != 0 { - return errors.New(fmt.Sprintf("extra arguments: %v", args)) +func (o *Options) Validate() error { + if len(o.args) != 0 { + return errors.New(fmt.Sprintf("extra arguments: %v", o.args)) } if o.Output != "" && o.Output != "yaml" && o.Output != "json" { diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/version/version_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/version/version_test.go index 9a355592243..3928bdb2e07 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/version/version_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/version/version_test.go @@ -31,13 +31,16 @@ func TestNewCmdVersionClientVersion(t *testing.T) { defer tf.Cleanup() streams, _, buf, _ := genericclioptions.NewTestIOStreams() o := NewOptions(streams) - if err := o.Complete(tf, &cobra.Command{}); err != nil { + if err := o.Complete(tf, &cobra.Command{}, nil); err != nil { t.Errorf("Unexpected error: %v", err) } - if err := o.Validate(nil); err != nil { + if err := o.Validate(); err != nil { t.Errorf("Unexpected error: %v", err) } - if err := o.Validate([]string{"extraParameter0"}); !strings.Contains(err.Error(), "extra arguments") { + if err := o.Complete(tf, &cobra.Command{}, []string{"extraParameter0"}); err != nil { + t.Errorf("Unexpected error: %v", err) + } + if err := o.Validate(); !strings.Contains(err.Error(), "extra arguments") { t.Errorf("Unexpected error: should fail to validate the args length greater than 0") } if err := o.Run(); err != nil {