cli: use Cobra's positional argument validation

pkg/cli/cert.go

@@ -51,6 +51,7 @@ The certs directory is created if it does not exist.
 If the CA key exists and --allow-ca-key-reuse is true, the key is used.
 If the CA certificate exists and --overwrite is true, the new CA certificate is prepended to it.
 `,
+	Args: cobra.NoArgs,
 	RunE: MaybeDecorateGRPCError(runCreateCACert),
 }
 
@@ -84,6 +85,12 @@ Requires a CA cert in "<certs-dir>/ca.crt" and matching key in "--ca-key".
 If "ca.crt" contains more than one certificate, the first is used.
 Creation fails if the CA expiration time is before the desired certificate expiration.
 `,
+	Args: func(cmd *cobra.Command, args []string) error {
+		if len(args) == 0 {
+			return errors.Errorf("create-node requires at least one host name or address, none was specified")
+		}
+		return nil
+	},
 	RunE: MaybeDecorateGRPCError(runCreateNodeCert),
 }
 
@@ -119,6 +126,7 @@ Requires a CA cert in "<certs-dir>/ca.crt" and matching key in "--ca-key".
 If "ca.crt" contains more than one certificate, the first is used.
 Creation fails if the CA expiration time is before the desired certificate expiration.
 `,
+	Args: cobra.ExactArgs(1),
 	RunE: MaybeDecorateGRPCError(runCreateClientCert),
 }
 
@@ -127,10 +135,6 @@ Creation fails if the CA expiration time is before the desired certificate expir
 // TODO(marc): there is currently no way to specify which CA cert to use if more
 // than one if present.
 func runCreateClientCert(cmd *cobra.Command, args []string) error {
-	if len(args) != 1 {
-		return usageAndError(cmd)
-	}
-
 	var err error
 	var username string
 	if username, err = sql.NormalizeAndValidateUsername(args[0]); err != nil {
@@ -156,15 +160,12 @@ var listCertsCmd = &cobra.Command{
 	Long: `
 List certificates and keys found in the certificate directory.
 `,
+	Args: cobra.NoArgs,
 	RunE: MaybeDecorateGRPCError(runListCerts),
 }
 
 // runListCerts loads and lists all certs.
 func runListCerts(cmd *cobra.Command, args []string) error {
-	if len(args) != 0 {
-		return usageAndError(cmd)
-	}
-
 	cm, err := baseCfg.GetCertificateManager()
 	if err != nil {
 		return errors.Wrap(err, "could not get certificate manager")

pkg/cli/cli.go

@@ -81,6 +81,7 @@ var versionCmd = &cobra.Command{
 	Long: `
 Output build version information.
 `,
+	Args: cobra.NoArgs,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		info := build.GetInfo()
 		tw := tabwriter.NewWriter(os.Stdout, 2, 1, 2, ' ', 0)

pkg/cli/cli_test.go

@@ -2061,17 +2061,19 @@ func TestJunkPositionalArguments(t *testing.T) {
 	defer c.cleanup()
 
 	for i, test := range []string{
-		"start junk",
-		"sql junk",
-		"gen man junk",
-		"gen autocomplete junk",
-		"gen example-data intro junk",
+		"start",
+		"sql",
+		"gen man",
+		"gen autocomplete",
+		"gen example-data intro",
 	} {
-		out, err := c.RunWithCapture(test)
+		const junk = "junk"
+		line := test + " " + junk
+		out, err := c.RunWithCapture(line)
 		if err != nil {
 			t.Fatal(errors.Wrap(err, strconv.Itoa(i)))
 		}
-		exp := test + "\ninvalid arguments\n"
+		exp := fmt.Sprintf("%s\nunknown command %q for \"cockroach %s\"\n", line, junk, test)
 		if exp != out {
 			t.Errorf("expected:\n%s\ngot:\n%s", exp, out)
 		}

pkg/cli/debug.go

@@ -55,11 +55,12 @@ import (
 )
 
 var debugKeysCmd = &cobra.Command{
-	Use:   "keys [directory]",
+	Use:   "keys <directory>",
 	Short: "dump all the keys in a store",
 	Long: `
 Pretty-prints all keys in a store.
 `,
+	Args: cobra.ExactArgs(1),
 	RunE: MaybeDecorateGRPCError(runDebugKeys),
 }
 
@@ -138,10 +139,6 @@ func runDebugKeys(cmd *cobra.Command, args []string) error {
 	stopper := stop.NewStopper()
 	defer stopper.Stop(context.Background())
 
-	if len(args) != 1 {
-		return errors.New("one argument required: dir")
-	}
-
 	db, err := openExistingStore(args[0], stopper, true /* readOnly */)
 	if err != nil {
 		return err
@@ -156,24 +153,21 @@ func runDebugKeys(cmd *cobra.Command, args []string) error {
 }
 
 var debugRangeDataCmd = &cobra.Command{
-	Use:   "range-data [directory] range-id",
+	Use:   "range-data <directory> <range id>",
 	Short: "dump all the data in a range",
 	Long: `
 Pretty-prints all keys and values in a range. By default, includes unreplicated
 state like the raft HardState. With --replicated, only includes data covered by
  the consistency checker.
 `,
+	Args: cobra.ExactArgs(2),
 	RunE: MaybeDecorateGRPCError(runDebugRangeData),
 }
 
 func runDebugRangeData(cmd *cobra.Command, args []string) error {
 	stopper := stop.NewStopper()
 	defer stopper.Stop(context.Background())
 
-	if len(args) != 2 {
-		return errors.New("two arguments required: dir range_id")
-	}
-
 	db, err := openExistingStore(args[0], stopper, true /* readOnly */)
 	if err != nil {
 		return err
@@ -208,11 +202,12 @@ func runDebugRangeData(cmd *cobra.Command, args []string) error {
 }
 
 var debugRangeDescriptorsCmd = &cobra.Command{
-	Use:   "range-descriptors [directory]",
+	Use:   "range-descriptors <directory>",
 	Short: "print all range descriptors in a store",
 	Long: `
 Prints all range descriptors in a store with a history of changes.
 `,
+	Args: cobra.ExactArgs(1),
 	RunE: MaybeDecorateGRPCError(runDebugRangeDescriptors),
 }
 
@@ -424,10 +419,6 @@ func runDebugRangeDescriptors(cmd *cobra.Command, args []string) error {
 	stopper := stop.NewStopper()
 	defer stopper.Stop(context.Background())
 
-	if len(args) != 1 {
-		return errors.New("one argument required: dir")
-	}
-
 	db, err := openExistingStore(args[0], stopper, true /* readOnly */)
 	if err != nil {
 		return err
@@ -448,6 +439,7 @@ Decode a hexadecimal-encoded key and pretty-print it. For example:
 	$ decode-key BB89F902ADB43000151C2D1ED07DE6C009
 	/Table/51/1/44938288/1521140384.514565824,0
 `,
+	Args: cobra.ArbitraryArgs,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		for _, arg := range args {
 			b, err := hex.DecodeString(arg)
@@ -465,11 +457,12 @@ Decode a hexadecimal-encoded key and pretty-print it. For example:
 }
 
 var debugRaftLogCmd = &cobra.Command{
-	Use:   "raft-log [directory] [range id]",
+	Use:   "raft-log <directory> <range id>",
 	Short: "print the raft log for a range",
 	Long: `
 Prints all log entries in a store for the given range.
 `,
+	Args: cobra.ExactArgs(2),
 	RunE: MaybeDecorateGRPCError(runDebugRaftLog),
 }
 
@@ -528,10 +521,6 @@ func runDebugRaftLog(cmd *cobra.Command, args []string) error {
 	stopper := stop.NewStopper()
 	defer stopper.Stop(context.Background())
 
-	if len(args) != 2 {
-		return errors.New("two arguments required: dir range_id")
-	}
-
 	db, err := openExistingStore(args[0], stopper, true /* readOnly */)
 	if err != nil {
 		return err
@@ -549,7 +538,7 @@ func runDebugRaftLog(cmd *cobra.Command, args []string) error {
 }
 
 var debugGCCmd = &cobra.Command{
-	Use:   "estimate-gc [directory] [range id]",
+	Use:   "estimate-gc <directory> [range id]",
 	Short: "find out what a GC run would do",
 	Long: `
 Sets up (but does not run) a GC collection cycle, giving insight into how much
@@ -560,6 +549,7 @@ ranges individually.
 
 Uses a hard-coded GC policy with a 24 hour TTL for old versions.
 `,
+	Args: cobra.RangeArgs(1, 2),
 	RunE: MaybeDecorateGRPCError(runDebugGCCmd),
 }
 
@@ -568,18 +558,11 @@ func runDebugGCCmd(cmd *cobra.Command, args []string) error {
 	defer stopper.Stop(context.Background())
 
 	var rangeID roachpb.RangeID
-	switch len(args) {
-
-	}
-	switch len(args) {
-	case 2:
+	if len(args) == 2 {
 		var err error
 		if rangeID, err = parseRangeID(args[1]); err != nil {
 			return err
 		}
-	case 1:
-	default:
-		return errors.New("arguments: dir [range_id]")
 	}
 
 	db, err := openExistingStore(args[0], stopper, true /* readOnly */)
@@ -641,7 +624,7 @@ func runDebugGCCmd(cmd *cobra.Command, args []string) error {
 }
 
 var debugCheckStoreCmd = &cobra.Command{
-	Use:   "check-store [directory]",
+	Use:   "check-store <directory>",
 	Short: "consistency check for a single store",
 	Long: `
 Perform local consistency checks of a single store.
@@ -650,6 +633,7 @@ Capable of detecting the following errors:
 * Raft logs that are inconsistent with their metadata
 * MVCC stats that are inconsistent with the data within the range
 `,
+	Args: cobra.ExactArgs(1),
 	RunE: MaybeDecorateGRPCError(runDebugCheckStoreCmd),
 }
 
@@ -666,10 +650,6 @@ func runDebugCheckStoreCmd(cmd *cobra.Command, args []string) error {
 
 	ctx := context.Background()
 
-	if len(args) != 1 {
-		return errors.New("one required argument: dir")
-	}
-
 	db, err := openExistingStore(args[0], stopper, true /* readOnly */)
 	if err != nil {
 		return err
@@ -812,29 +792,27 @@ var debugEnvCmd = &cobra.Command{
 	Long: `
 Output environment variables that influence configuration.
 `,
+	Args: cobra.NoArgs,
 	Run: func(cmd *cobra.Command, args []string) {
 		env := envutil.GetEnvReport()
 		fmt.Print(env)
 	},
 }
 
 var debugCompactCmd = &cobra.Command{
-	Use:   "compact [directory]",
+	Use:   "compact <directory>",
 	Short: "compact the sstables in a store",
 	Long: `
 Compact the sstables in a store.
 `,
+	Args: cobra.ExactArgs(1),
 	RunE: MaybeDecorateGRPCError(runDebugCompact),
 }
 
 func runDebugCompact(cmd *cobra.Command, args []string) error {
 	stopper := stop.NewStopper()
 	defer stopper.Stop(context.Background())
 
-	if len(args) != 1 {
-		return errors.New("one argument is required")
-	}
-
 	db, err := openExistingStore(args[0], stopper, false /* readOnly */)
 	if err != nil {
 		return err
@@ -863,7 +841,7 @@ func runDebugCompact(cmd *cobra.Command, args []string) error {
 }
 
 var debugSSTablesCmd = &cobra.Command{
-	Use:   "sstables [directory]",
+	Use:   "sstables <directory>",
 	Short: "list the sstables in a store",
 	Long: `
 
@@ -886,17 +864,14 @@ total files and 14 files that are 129 MiB in size.
 The suffixes K, M, G and T are used for terseness to represent KiB, MiB, GiB
 and TiB.
 `,
+	Args: cobra.ExactArgs(1),
 	RunE: MaybeDecorateGRPCError(runDebugSSTables),
 }
 
 func runDebugSSTables(cmd *cobra.Command, args []string) error {
 	stopper := stop.NewStopper()
 	defer stopper.Stop(context.Background())
 
-	if len(args) != 1 {
-		return errors.New("one argument is required")
-	}
-
 	db, err := openExistingStore(args[0], stopper, true /* readOnly */)
 	if err != nil {
 		return err
@@ -907,14 +882,15 @@ func runDebugSSTables(cmd *cobra.Command, args []string) error {
 }
 
 var debugGossipValuesCmd = &cobra.Command{
-	Use:   "gossip-values [directory]",
+	Use:   "gossip-values <directory>",
 	Short: "dump all the values in a node's gossip instance",
 	Long: `
 Pretty-prints the values in a node's gossip instance.
 
 Can connect to a running server to get the values or can be provided with
 a JSON file captured from a node's /_status/gossip/ debug endpoint.
 `,
+	Args: cobra.ExactArgs(1),
 	RunE: MaybeDecorateGRPCError(runDebugGossipValues),
 }
 
@@ -1020,6 +996,7 @@ var debugSyncTestCmd = &cobra.Command{
 	Short: "Run a performance test for WAL sync speed",
 	Long: `
 `,
+	Args:   cobra.MaximumNArgs(1),
 	Hidden: true,
 	RunE:   MaybeDecorateGRPCError(runDebugSyncTest),
 }
@@ -1032,9 +1009,6 @@ var syncTestOpts = synctest.Options{
 
 func runDebugSyncTest(cmd *cobra.Command, args []string) error {
 	syncTestOpts.Dir = "./testdb"
-	if len(args) > 1 {
-		return fmt.Errorf("too many arguments")
-	}
 	if len(args) == 1 {
 		syncTestOpts.Dir = args[0]
 	}

pkg/cli/demo.go

@@ -34,6 +34,7 @@ Start an in-memory, standalone, single-node CockroachDB instance, and open an
 interactive SQL prompt to it.
 `,
 	Example: `  cockroach demo`,
+	Args:    cobra.NoArgs,
 	RunE:    MaybeShoutError(MaybeDecorateGRPCError(runDemo)),
 }
 

pkg/cli/dump.go

@@ -41,14 +41,11 @@ var dumpCmd = &cobra.Command{
 Dump SQL tables of a cockroach database. If the table name
 is omitted, dump all tables in the database.
 `,
+	Args: cobra.MinimumNArgs(1),
 	RunE: MaybeDecorateGRPCError(runDump),
 }
 
 func runDump(cmd *cobra.Command, args []string) error {
-	if len(args) < 1 {
-		return usageAndError(cmd)
-	}
-
 	conn, err := getPasswordAndMakeSQLClient("cockroach dump")
 	if err != nil {
 		return err

pkg/cli/error.go

@@ -111,10 +111,3 @@ func MaybeShoutError(
 		return err
 	}
 }
-
-func usageAndError(cmd *cobra.Command) error {
-	if err := cmd.Usage(); err != nil {
-		return err
-	}
-	return errors.New("invalid arguments")
-}