diff --git a/cmd/clicheck/check_cli_conventions.go b/cmd/clicheck/check_cli_conventions.go index 6c264b87b5..524f9f05dc 100644 --- a/cmd/clicheck/check_cli_conventions.go +++ b/cmd/clicheck/check_cli_conventions.go @@ -31,16 +31,23 @@ var ( ) func main() { - errors := []error{} + var errorCount int kubectl := cmd.NewKubectlCommand(cmdutil.NewFactory(nil), os.Stdin, ioutil.Discard, ioutil.Discard) - result := cmdsanity.CheckCmdTree(kubectl, cmdsanity.AllCmdChecks, []string{}) - errors = append(errors, result...) + errors := cmdsanity.RunCmdChecks(kubectl, cmdsanity.AllCmdChecks, []string{}) + for _, err := range errors { + errorCount++ + fmt.Fprintf(os.Stderr, " %d. %s\n", errorCount, err) + } - if len(errors) > 0 { - for i, err := range errors { - fmt.Fprintf(os.Stderr, "%d. %s\n\n", i+1, err) - } + errors = cmdsanity.RunGlobalChecks(cmdsanity.AllGlobalChecks) + for _, err := range errors { + errorCount++ + fmt.Fprintf(os.Stderr, " %d. %s\n", errorCount, err) + } + + if errorCount > 0 { + fmt.Fprintf(os.Stdout, "Found %d errors.\n", errorCount) os.Exit(1) } diff --git a/hack/verify-cli-conventions.sh b/hack/verify-cli-conventions.sh index 28a1a5c9be..d768eb0a56 100755 --- a/hack/verify-cli-conventions.sh +++ b/hack/verify-cli-conventions.sh @@ -32,9 +32,12 @@ clicheck=$(kube::util::find-binary "clicheck") if ! output=`$clicheck 2>&1` then - echo "FAILURE: CLI is not following one or more required conventions:" echo "$output" + echo + echo "FAILURE: CLI is not following one or more required conventions." exit 1 else + echo "$output" + echo echo "SUCCESS: CLI is following all tested conventions." fi diff --git a/pkg/kubectl/cmd/util/sanity/BUILD b/pkg/kubectl/cmd/util/sanity/BUILD index 1e28202aa9..0d72a12cda 100644 --- a/pkg/kubectl/cmd/util/sanity/BUILD +++ b/pkg/kubectl/cmd/util/sanity/BUILD @@ -15,6 +15,7 @@ go_library( deps = [ "//pkg/kubectl/cmd/templates:go_default_library", "//vendor/github.com/spf13/cobra:go_default_library", + "//vendor/github.com/spf13/pflag:go_default_library", ], ) diff --git a/pkg/kubectl/cmd/util/sanity/cmd_sanity.go b/pkg/kubectl/cmd/util/sanity/cmd_sanity.go index 1460d3228e..241f855b8b 100644 --- a/pkg/kubectl/cmd/util/sanity/cmd_sanity.go +++ b/pkg/kubectl/cmd/util/sanity/cmd_sanity.go @@ -19,27 +19,43 @@ package sanity import ( "fmt" "os" + "regexp" "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" ) type CmdCheck func(cmd *cobra.Command) []error +type GlobalCheck func() []error var ( AllCmdChecks = []CmdCheck{ CheckLongDesc, CheckExamples, + CheckFlags, + } + AllGlobalChecks = []GlobalCheck{ + CheckGlobalVarFlags, } ) -func CheckCmdTree(cmd *cobra.Command, checks []CmdCheck, skip []string) []error { +func RunGlobalChecks(globalChecks []GlobalCheck) []error { + fmt.Fprint(os.Stdout, "---+ RUNNING GLOBAL CHECKS\n") + errors := []error{} + for _, check := range globalChecks { + errors = append(errors, check()...) + } + return errors +} + +func RunCmdChecks(cmd *cobra.Command, cmdChecks []CmdCheck, skipCmd []string) []error { cmdPath := cmd.CommandPath() - for _, skipCmdPath := range skip { + for _, skipCmdPath := range skipCmd { if cmdPath == skipCmdPath { - fmt.Fprintf(os.Stdout, "-----+ skipping command %s\n", cmdPath) + fmt.Fprintf(os.Stdout, "---+ skipping command %s\n", cmdPath) return []error{} } } @@ -48,13 +64,13 @@ func CheckCmdTree(cmd *cobra.Command, checks []CmdCheck, skip []string) []error if cmd.HasSubCommands() { for _, subCmd := range cmd.Commands() { - errors = append(errors, CheckCmdTree(subCmd, checks, skip)...) + errors = append(errors, RunCmdChecks(subCmd, cmdChecks, skipCmd)...) } } - fmt.Fprintf(os.Stdout, "-----+ checking command %s\n", cmdPath) + fmt.Fprintf(os.Stdout, "---+ RUNNING COMMAND CHECKS on %q\n", cmdPath) - for _, check := range checks { + for _, check := range cmdChecks { if err := check(cmd); err != nil && len(err) > 0 { errors = append(errors, err...) } @@ -64,26 +80,26 @@ func CheckCmdTree(cmd *cobra.Command, checks []CmdCheck, skip []string) []error } func CheckLongDesc(cmd *cobra.Command) []error { + fmt.Fprint(os.Stdout, " ↳ checking long description\n") cmdPath := cmd.CommandPath() long := cmd.Long if len(long) > 0 { if strings.Trim(long, " \t\n") != long { - return []error{fmt.Errorf(`command %q: long description is not normalized - ↳ make sure you are calling templates.LongDesc (from pkg/cmd/templates) before assigning cmd.Long`, cmdPath)} + return []error{fmt.Errorf(`command %q: long description is not normalized, make sure you are calling templates.LongDesc (from pkg/cmd/templates) before assigning cmd.Long`, cmdPath)} } } return nil } func CheckExamples(cmd *cobra.Command) []error { + fmt.Fprint(os.Stdout, " ↳ checking examples\n") cmdPath := cmd.CommandPath() examples := cmd.Example errors := []error{} if len(examples) > 0 { for _, line := range strings.Split(examples, "\n") { if !strings.HasPrefix(line, templates.Indentation) { - errors = append(errors, fmt.Errorf(`command %q: examples are not normalized - ↳ make sure you are calling templates.Examples (from pkg/cmd/templates) before assigning cmd.Example`, cmdPath)) + errors = append(errors, fmt.Errorf(`command %q: examples are not normalized, make sure you are calling templates.Examples (from pkg/cmd/templates) before assigning cmd.Example`, cmdPath)) } if trimmed := strings.TrimSpace(line); strings.HasPrefix(trimmed, "//") { errors = append(errors, fmt.Errorf(`command %q: we use # to start comments in examples instead of //`, cmdPath)) @@ -92,3 +108,42 @@ func CheckExamples(cmd *cobra.Command) []error { } return errors } + +func CheckFlags(cmd *cobra.Command) []error { + allFlagsSlice := []*pflag.Flag{} + + cmd.Flags().VisitAll(func(f *pflag.Flag) { + allFlagsSlice = append(allFlagsSlice, f) + }) + cmd.PersistentFlags().VisitAll(func(f *pflag.Flag) { + allFlagsSlice = append(allFlagsSlice, f) + }) + + fmt.Fprintf(os.Stdout, " ↳ checking %d flags\n", len(allFlagsSlice)) + + errors := []error{} + + // check flags long names + regex, err := regexp.Compile(`^[a-z]+[a-z\-]*$`) + if err != nil { + errors = append(errors, fmt.Errorf("command %q: unable to compile regex to check flags", cmd.CommandPath())) + return errors + } + for _, flag := range allFlagsSlice { + name := flag.Name + if !regex.MatchString(name) { + errors = append(errors, fmt.Errorf("command %q: flag name %q is invalid, long form of flag names can only contain lowercase characters or dash (must match %v)", cmd.CommandPath(), name, regex)) + } + } + + return errors +} + +func CheckGlobalVarFlags() []error { + fmt.Fprint(os.Stdout, " ↳ checking flags from global vars\n") + errors := []error{} + pflag.CommandLine.VisitAll(func(f *pflag.Flag) { + errors = append(errors, fmt.Errorf("flag %q is invalid, please don't register any flag under the global variable \"CommandLine\"", f.Name)) + }) + return errors +}