17840: Check for unparsed command line arguments.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/cmd/arvados-package/cmd.go b/cmd/arvados-package/cmd.go
index 54f0809d6413eb9bee60fb9c741b117f73e938ac..863bbe925478c76df017bf45ac637f744f34b1f9 100644 (file)
--- a/cmd/arvados-package/cmd.go
+++ b/cmd/arvados-package/cmd.go
@@ -124,7 +124,7 @@ Options:
        if err != nil {
                return opts, err
        }
-       if len(flags.Args()) > 0 {
+       if flags.NArg() != 0 {
                return opts, fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
        }
        if opts.SourceDir == "" {

diff --git a/lib/boot/cmd.go b/lib/boot/cmd.go
index 001504e203f24221ade1668af7bb4d532e6c991d..b6bbb979954ff4a9372087e836f3b7bf7af1cd3f 100644 (file)
--- a/lib/boot/cmd.go
+++ b/lib/boot/cmd.go
@@ -75,6 +75,8 @@ func (bcmd bootCommand) run(ctx context.Context, prog string, args []string, std
                return nil
        } else if err != nil {
                return err
+       } else if flags.NArg() != 0 {
+               return fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
        } else if *versionFlag {
                cmd.Version.RunCommand(prog, args, stdin, stdout, stderr)
                return nil

diff --git a/lib/cloud/cloudtest/cmd.go b/lib/cloud/cloudtest/cmd.go
index 4816f20ee714df422b185e1801dc08349bbe7a84..3c4b560c97a17a26f0b406eac5c769ff575e5b11 100644 (file)
--- a/lib/cloud/cloudtest/cmd.go
+++ b/lib/cloud/cloudtest/cmd.go
@@ -47,10 +47,8 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
                return 0
        } else if err != nil {
                return 2
-       }
-
-       if len(flags.Args()) != 0 {
-               flags.Usage()
+       } else if flags.NArg() != 0 {
+               err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
                return 2
        }
        logger := ctxlog.New(stderr, "text", "info")

diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index 8e638e6ecb4cc712a7ba08f68c09ea2c33b189fc..c852b0b545af0e91ea856a004d7cdc2dc7850b92 100644 (file)
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -44,10 +44,8 @@ func (dumpCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou
                return 0
        } else if err != nil {
                return 2
-       }
-
-       if len(flags.Args()) != 0 {
-               flags.Usage()
+       } else if flags.NArg() != 0 {
+               err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
                return 2
        }
 
@@ -98,10 +96,8 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
                return 0
        } else if err != nil {
                return 2
-       }
-
-       if len(flags.Args()) != 0 {
-               flags.Usage()
+       } else if flags.NArg() != 0 {
+               err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
                return 2
        }
 

diff --git a/lib/config/deprecated_test.go b/lib/config/deprecated_test.go
index 595e4c9cad4e849c21d6f3d6b4c11b6b6d4b51ac..4206ef57717eebc494cd3593bdf2551cf2956178 100644 (file)
--- a/lib/config/deprecated_test.go
+++ b/lib/config/deprecated_test.go
@@ -35,7 +35,9 @@ func testLoadLegacyConfig(content []byte, mungeFlag string, c *check.C) (*arvado
        ldr := testLoader(c, "Clusters: {zzzzz: {}}", nil)
        ldr.SetupFlags(flags)
        args := ldr.MungeLegacyConfigArgs(ldr.Logger, []string{"-config", tmpfile.Name()}, mungeFlag)
-       flags.Parse(args)
+       err = flags.Parse(args)
+       c.Assert(err, check.IsNil)
+       c.Assert(flags.NArg(), check.Equals, 0)
        cfg, err := ldr.Load()
        if err != nil {
                return nil, err

diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 8f3a30203911187c28b71c405a92caac8cab14e5..a5e69387ec5d877bf0f5320c9eb7d921bacbc8e4 100644 (file)
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -1699,6 +1699,10 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
        } else if err != nil {
                log.Print(err)
                return 1
+       } else if flags.NArg() != 1 {
+               fmt.Fprintf(flags.Output(), "Usage: %s [options] containerUUID\n\nOptions:\n", prog)
+               flags.PrintDefaults()
+               return 2
        }
 
        containerUUID := flags.Arg(0)

diff --git a/lib/diagnostics/cmd.go b/lib/diagnostics/cmd.go
index b0241b3ae412634f734e4632c03ba5c5c2b6cbf0..8663c6ee5fab61bb24ad6966347cbc77d22f908d 100644 (file)
--- a/lib/diagnostics/cmd.go
+++ b/lib/diagnostics/cmd.go
@@ -39,6 +39,9 @@ func (cmd Command) RunCommand(prog string, args []string, stdin io.Reader, stdou
        } else if err != nil {
                fmt.Fprintln(stderr, err)
                return 2
+       } else if f.NArg() != 0 {
+               fmt.Fprintf(stderr, "unrecognized command line arguments: %v\n", f.Args())
+               return 2
        }
        diag.logger = ctxlog.New(stdout, "text", diag.logLevel)
        diag.logger.SetFormatter(&logrus.TextFormatter{DisableTimestamp: true, DisableLevelTruncation: true, PadLevelText: true})

diff --git a/lib/install/init.go b/lib/install/init.go
index 7ae42c531750e9af78b7ffb70b9b6151db958034..8b2fbfdd026bb292a53c3d12c3da625c330cf539 100644 (file)
--- a/lib/install/init.go
+++ b/lib/install/init.go
@@ -67,7 +67,7 @@ func (initcmd *initCommand) RunCommand(prog string, args []string, stdin io.Read
                return 2
        } else if *versionFlag {
                return cmd.Version.RunCommand(prog, args, stdin, stdout, stderr)
-       } else if len(flags.Args()) > 0 {
+       } else if flags.NArg() != 0 {
                err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
                return 2
        } else if !regexp.MustCompile(`^[a-z][a-z0-9]{4}`).MatchString(initcmd.ClusterID) {

diff --git a/lib/mount/command.go b/lib/mount/command.go
index e92af24075f1b824c741f6f35b4de53e2346b7ed..d6d1ae03080bdf10b316646843e2ea84c24968c4 100644 (file)
--- a/lib/mount/command.go
+++ b/lib/mount/command.go
@@ -9,6 +9,7 @@ import (
        "io"
        "log"
        "net/http"
+
        // pprof is only imported to register its HTTP handlers
        _ "net/http/pprof"
        "os"

diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index e67c24f65f39cea4929c95fe30abbdc5ab98a901..268aba7516834c517b9cf7fbfa4984d24bf142e2 100644 (file)
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -78,6 +78,9 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
                return 0
        } else if err != nil {
                return 2
+       } else if flags.NArg() != 0 {
+               err = fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
+               return 2
        } else if *versionFlag {
                return cmd.Version.RunCommand(prog, args, stdin, stdout, stderr)
        }

diff --git a/services/arv-git-httpd/main.go b/services/arv-git-httpd/main.go
index 4e55964619acb6d1f29fe9b6ceb6ac5fbdf386cf..af63b32ab3d3eeb3fd76d1b70669b474508cf4d9 100644 (file)
--- a/services/arv-git-httpd/main.go
+++ b/services/arv-git-httpd/main.go
@@ -23,7 +23,7 @@ func main() {
                TimestampFormat: "2006-01-02T15:04:05.000000000Z07:00",
        })
 
-       flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+       flags := flag.NewFlagSet(os.Args[0], flag.ContinueOnError)
        loader := config.NewLoader(os.Stdin, logger)
        loader.SetupFlags(flags)
 
@@ -31,7 +31,16 @@ func main() {
        getVersion := flags.Bool("version", false, "print version information and exit.")
 
        args := loader.MungeLegacyConfigArgs(logger, os.Args[1:], "-legacy-git-httpd-config")
-       flags.Parse(args)
+       err := flags.Parse(args)
+       if err == flag.ErrHelp {
+               return
+       } else if err != nil {
+               logger.Error(err)
+               os.Exit(2)
+       } else if flags.NArg() != 0 {
+               logger.Errorf("unrecognized command line arguments: %v", flags.Args())
+               os.Exit(2)
+       }
 
        if *getVersion {
                fmt.Printf("arv-git-httpd %s\n", version)

diff --git a/services/crunch-dispatch-local/crunch-dispatch-local.go b/services/crunch-dispatch-local/crunch-dispatch-local.go
index a3cb1341a4677e7ecdc7c03976da7483e47c1aa5..159ee69b1215b205dd904f14449e2bbaaa53a9a8 100644 (file)
--- a/services/crunch-dispatch-local/crunch-dispatch-local.go
+++ b/services/crunch-dispatch-local/crunch-dispatch-local.go
@@ -67,7 +67,14 @@ func doMain() error {
                "Print version information and exit.")
 
        // Parse args; omit the first arg which is the command name
-       flags.Parse(os.Args[1:])
+       err := flags.Parse(os.Args[1:])
+       if err == flag.ErrHelp {
+               return nil
+       } else if err != nil {
+               return err
+       } else if flags.NArg() != 0 {
+               return fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
+       }
 
        // Print version information if requested
        if *getVersion {

diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
index 584db38edf7e93ac57ad8929ca31e04de907b78d..02493bf4fb451450e5c684ebab48af7f4676b46e 100644 (file)
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
@@ -91,13 +91,14 @@ func (disp *Dispatcher) configure(prog string, args []string) error {
                false,
                "Print version information and exit.")
 
-       args = loader.MungeLegacyConfigArgs(logrus.StandardLogger(), args, "-legacy-crunch-dispatch-slurm-config")
-
-       // Parse args; omit the first arg which is the command name
+       args = loader.MungeLegacyConfigArgs(disp.logger, args, "-legacy-crunch-dispatch-slurm-config")
        err := flags.Parse(args)
-
        if err == flag.ErrHelp {
                return nil
+       } else if err != nil {
+               return err
+       } else if flags.NArg() != 0 {
+               return fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
        }
 
        // Print version information if requested

diff --git a/services/crunchstat/crunchstat.go b/services/crunchstat/crunchstat.go
index 6bf2e3399e61b4ec172e4ddb3dd76cee37c201d4..3ff3912e8c892ab1e33f936170157eaa2886df46 100644 (file)
--- a/services/crunchstat/crunchstat.go
+++ b/services/crunchstat/crunchstat.go
@@ -32,15 +32,22 @@ func main() {
                Logger: log.New(os.Stderr, "crunchstat: ", 0),
        }
 
-       flag.StringVar(&reporter.CgroupRoot, "cgroup-root", "", "Root of cgroup tree")
-       flag.StringVar(&reporter.CgroupParent, "cgroup-parent", "", "Name of container parent under cgroup")
-       flag.StringVar(&reporter.CIDFile, "cgroup-cid", "", "Path to container id file")
-       flag.IntVar(&signalOnDeadPPID, "signal-on-dead-ppid", signalOnDeadPPID, "Signal to send child if crunchstat's parent process disappears (0 to disable)")
-       flag.DurationVar(&ppidCheckInterval, "ppid-check-interval", ppidCheckInterval, "Time between checks for parent process disappearance")
-       pollMsec := flag.Int64("poll", 1000, "Reporting interval, in milliseconds")
-       getVersion := flag.Bool("version", false, "Print version information and exit.")
-
-       flag.Parse()
+       flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+       flags.StringVar(&reporter.CgroupRoot, "cgroup-root", "", "Root of cgroup tree")
+       flags.StringVar(&reporter.CgroupParent, "cgroup-parent", "", "Name of container parent under cgroup")
+       flags.StringVar(&reporter.CIDFile, "cgroup-cid", "", "Path to container id file")
+       flags.IntVar(&signalOnDeadPPID, "signal-on-dead-ppid", signalOnDeadPPID, "Signal to send child if crunchstat's parent process disappears (0 to disable)")
+       flags.DurationVar(&ppidCheckInterval, "ppid-check-interval", ppidCheckInterval, "Time between checks for parent process disappearance")
+       pollMsec := flags.Int64("poll", 1000, "Reporting interval, in milliseconds")
+       getVersion := flags.Bool("version", false, "Print version information and exit.")
+
+       err := flags.Parse(os.Args[1:])
+       if err == flag.ErrHelp {
+               return
+       } else if err != nil {
+               reporter.Logger.Print(err)
+               os.Exit(2)
+       }
 
        // Print version information if requested
        if *getVersion {
@@ -48,6 +55,12 @@ func main() {
                return
        }
 
+       if flags.NArg() == 0 {
+               fmt.Fprintf(flags.Output(), "Usage: %s [options] program [args...]\n\nOptions:\n", os.Args[0])
+               flags.PrintDefaults()
+               os.Exit(2)
+       }
+
        reporter.Logger.Printf("crunchstat %s started", version)
 
        if reporter.CgroupRoot == "" {
@@ -58,7 +71,7 @@ func main() {
        reporter.PollPeriod = time.Duration(*pollMsec) * time.Millisecond
 
        reporter.Start()
-       err := runCommand(flag.Args(), reporter.Logger)
+       err = runCommand(flags.Args(), reporter.Logger)
        reporter.Stop()
 
        if err, ok := err.(*exec.ExitError); ok {

diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go
index 605ee5fc82b9c40682b1a1e122d114e09835fd36..37ed06369bbf139a8e89e7bd420035b8bd6ae7ee 100644 (file)
--- a/services/keep-balance/main.go
+++ b/services/keep-balance/main.go
@@ -32,7 +32,7 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
        logger := ctxlog.FromContext(context.Background())
 
        var options RunOptions
-       flags := flag.NewFlagSet(prog, flag.ExitOnError)
+       flags := flag.NewFlagSet(prog, flag.ContinueOnError)
        flags.BoolVar(&options.Once, "once", false,
                "balance once and then exit")
        flags.BoolVar(&options.CommitPulls, "commit-pulls", false,
@@ -41,9 +41,12 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
                "send trash requests (delete unreferenced old blocks, and excess replicas of overreplicated blocks)")
        flags.BoolVar(&options.CommitConfirmedFields, "commit-confirmed-fields", true,
                "update collection fields (replicas_confirmed, storage_classes_confirmed, etc.)")
-       flags.Bool("version", false, "Write version information to stdout and exit 0")
        dumpFlag := flags.Bool("dump", false, "dump details for each block to stdout")
        pprofAddr := flags.String("pprof", "", "serve Go profile data at `[addr]:port`")
+       // "show version" is implemented by service.Command, so we
+       // don't need the var here -- we just need the -version flag
+       // to pass flags.Parse().
+       flags.Bool("version", false, "Write version information to stdout and exit 0")
 
        if *pprofAddr != "" {
                go func() {
@@ -56,12 +59,13 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 
        munged := loader.MungeLegacyConfigArgs(logger, args, "-legacy-keepbalance-config")
        err := flags.Parse(munged)
-       if err != nil {
+       if err == flag.ErrHelp {
+               return 0
+       } else if err != nil {
                logger.Errorf("error parsing command line flags: %s", err)
                return 2
-       }
-       if flags.NArg() != 0 {
-               logger.Errorf("error parsing command line flags: extra arguments: %q", flags.Args())
+       } else if flags.NArg() != 0 {
+               logger.Errorf("unrecognized command line arguments: %v", flags.Args())
                return 2
        }
 
@@ -84,7 +88,7 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
        }
        flags.Visit(func(f *flag.Flag) {
                if !dropFlag[f.Name] {
-                       args = append(args, "-"+f.Name, f.Value.String())
+                       args = append(args, "-"+f.Name+"="+f.Value.String())
                }
        })
 

diff --git a/services/keep-balance/main_test.go b/services/keep-balance/main_test.go
index 65a2d5567a86505e7d6e4866aa8ffa75e3bf2deb..820f35216637626d7e543fb5781b613294f2dd91 100644 (file)
--- a/services/keep-balance/main_test.go
+++ b/services/keep-balance/main_test.go
@@ -28,6 +28,7 @@ func (s *mainSuite) TestVersionFlag(c *check.C) {
        runCommand("keep-balance", []string{"-version"}, nil, &stdout, &stderr)
        c.Check(stderr.String(), check.Equals, "")
        c.Log(stdout.String())
+       c.Check(stdout.String(), check.Matches, `keep-balance.*\(go1.*\)\n`)
 }
 
 func (s *mainSuite) TestHTTPServer(c *check.C) {

diff --git a/services/keep-web/main.go b/services/keep-web/main.go
index a9ac834a20cedf21f80fcc5e9a7742f86bb0e812..4c66165225c6f6f32e736e53a10eab0a0fda8aa3 100644 (file)
--- a/services/keep-web/main.go
+++ b/services/keep-web/main.go
@@ -58,8 +58,8 @@ func init() {
        })
 }
 
-func configure(logger log.FieldLogger, args []string) *Config {
-       flags := flag.NewFlagSet(args[0], flag.ExitOnError)
+func configure(logger log.FieldLogger, args []string) (*Config, error) {
+       flags := flag.NewFlagSet(args[0], flag.ContinueOnError)
 
        loader := config.NewLoader(os.Stdin, logger)
        loader.SetupFlags(flags)
@@ -70,12 +70,19 @@ func configure(logger log.FieldLogger, args []string) *Config {
                "print version information and exit.")
 
        args = loader.MungeLegacyConfigArgs(logger, args[1:], "-legacy-keepweb-config")
-       flags.Parse(args)
+       err := flags.Parse(args)
+       if err == flag.ErrHelp {
+               return nil, nil
+       } else if err != nil {
+               return nil, err
+       } else if flags.NArg() != 0 {
+               return nil, fmt.Errorf("unrecognized command line arguments: %v", flags.Args())
+       }
 
        // Print version information if requested
        if *getVersion {
                fmt.Printf("keep-web %s\n", version)
-               return nil
+               return nil, nil
        }
 
        arvCfg, err := loader.Load()
@@ -87,22 +94,22 @@ func configure(logger log.FieldLogger, args []string) *Config {
        if *dumpConfig {
                out, err := yaml.Marshal(cfg)
                if err != nil {
-                       log.Fatal(err)
+                       return nil, err
                }
                _, err = os.Stdout.Write(out)
-               if err != nil {
-                       log.Fatal(err)
-               }
-               return nil
+               return nil, err
        }
-       return cfg
+       return cfg, nil
 }
 
 func main() {
        logger := log.New()
 
-       cfg := configure(logger, os.Args)
-       if cfg == nil {
+       cfg, err := configure(logger, os.Args)
+       if err != nil {
+               logger.Error(err)
+               os.Exit(1)
+       } else if cfg == nil {
                return
        }