From: Tom Clegg Date: Mon, 8 Nov 2021 19:22:20 +0000 (-0500) Subject: 17840: Check for unparsed command line arguments. X-Git-Tag: 2.4.0~159^2~5 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/40f551004ab4e5f1d8ab02ddb55dca225ee8f6ac 17840: Check for unparsed command line arguments. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/cmd/arvados-package/cmd.go b/cmd/arvados-package/cmd.go index 54f0809d64..863bbe9254 100644 --- 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 001504e203..b6bbb97995 100644 --- 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 4816f20ee7..3c4b560c97 100644 --- 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 8e638e6ecb..c852b0b545 100644 --- 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 595e4c9cad..4206ef5771 100644 --- 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 8f3a302039..a5e69387ec 100644 --- 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 b0241b3ae4..8663c6ee5f 100644 --- 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 7ae42c5317..8b2fbfdd02 100644 --- 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 e92af24075..d6d1ae0308 100644 --- 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 e67c24f65f..268aba7516 100644 --- 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 4e55964619..af63b32ab3 100644 --- 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 a3cb1341a4..159ee69b12 100644 --- 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 584db38edf..02493bf4fb 100644 --- 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 6bf2e3399e..3ff3912e8c 100644 --- 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 605ee5fc82..37ed06369b 100644 --- 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 65a2d5567a..820f352166 100644 --- 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 a9ac834a20..4c66165225 100644 --- 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 }