dns

core/commoncmd/args.go

@@ -0,0 +1,91 @@
+package commoncmd
+
+import (
+	"fmt"
+	"io"
+	"strings"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+)
+
+// discardValue stands for a real flag value in the throwaway flag set
+// firstNonFlag() parses with. Parsing must not write the values cobra is
+// about to parse itself: a repeated parse would append twice to the slice
+// flags.
+type discardValue struct {
+	typ string
+}
+
+func (t discardValue) String() string   { return "" }
+func (t discardValue) Set(string) error { return nil }
+func (t discardValue) Type() string     { return t.typ }
+
+// ValidateArgs returns an error when args name a command that does not exist.
+//
+// cobra walks the command chain and, when the deepest command it reaches has
+// no Run function, prints that command help and returns no error, silently
+// dropping whatever was typed after it. A stale or mistyped command path
+// exits 0 that way, which is how the daemon scheduler and the api exec kept
+// reporting successful runs of commands renamed under their hard-coded argv.
+func ValidateArgs(root *cobra.Command, args []string) error {
+	if len(args) == 0 {
+		return nil
+	}
+	if strings.HasPrefix(args[0], "__complete") {
+		// shell completion: cobra answers with candidates, it does not run
+		return nil
+	}
+	cmd, rest, err := root.Find(args)
+	if err != nil || cmd == nil || cmd.Runnable() {
+		// an unknown first word is reported by cobra itself
+		return nil
+	}
+	name, ok := firstNonFlag(cmd, rest)
+	if !ok {
+		// no command name left: the user asked for the command list
+		return nil
+	}
+	return fmt.Errorf("unknown command %q for %q", name, cmd.CommandPath())
+}
+
+// firstNonFlag returns the first positional argument left in args once the
+// flags cmd accepts are stripped.
+func firstNonFlag(cmd *cobra.Command, args []string) (string, bool) {
+	fs := pflag.NewFlagSet(cmd.Name(), pflag.ContinueOnError)
+	fs.ParseErrorsWhitelist.UnknownFlags = true
+	fs.SetOutput(io.Discard)
+
+	add := func(f *pflag.Flag) {
+		if fs.Lookup(f.Name) != nil {
+			return
+		}
+		shorthand := f.Shorthand
+		if shorthand != "" && fs.ShorthandLookup(shorthand) != nil {
+			shorthand = ""
+		}
+		fs.AddFlag(&pflag.Flag{
+			Name:      f.Name,
+			Shorthand: shorthand,
+			Value:     discardValue{typ: f.Value.Type()},
+			DefValue:  f.DefValue,
+			// carried over so the valueless flags don't eat the next word
+			NoOptDefVal: f.NoOptDefVal,
+		})
+	}
+	// InheritedFlags() and LocalFlags() both merge the parents persistent
+	// flags into cmd.Flags() on the way, which Flags() alone does not do
+	// before the command runs.
+	cmd.InheritedFlags().VisitAll(add)
+	cmd.LocalFlags().VisitAll(add)
+	cmd.Flags().VisitAll(add)
+
+	if err := fs.Parse(args); err != nil {
+		// a flag error is cobra's to report
+		return "", false
+	}
+	if rest := fs.Args(); len(rest) > 0 {
+		return rest[0], true
+	}
+	return "", false
+}

core/commoncmd/args_test.go

@@ -0,0 +1,101 @@
+package commoncmd
+
+import (
+	"testing"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// newTestTree returns a command tree shaped like the om one: a non runnable
+// root, non runnable subsystem levels, and a runnable leaf.
+func newTestTree() (*cobra.Command, *string) {
+	var selector string
+	root := &cobra.Command{Use: "om"}
+	root.PersistentFlags().StringVar(new(string), "color", "auto", "")
+	root.PersistentFlags().StringVarP(&selector, "selector", "s", "", "")
+	root.PersistentFlags().Bool("debug", false, "")
+
+	svc := &cobra.Command{Use: "svc"}
+	instance := &cobra.Command{Use: "instance"}
+	status := &cobra.Command{
+		Use: "status",
+		RunE: func(*cobra.Command, []string) error {
+			return nil
+		},
+	}
+	status.Flags().BoolP("refresh", "r", false, "")
+
+	instance.AddCommand(status)
+	svc.AddCommand(instance)
+	root.AddCommand(svc)
+	return root, &selector
+}
+
+func TestValidateArgs(t *testing.T) {
+	for name, test := range map[string]struct {
+		args    []string
+		unknown string
+	}{
+		"a runnable leaf": {
+			args: []string{"svc", "instance", "status"},
+		},
+		"a runnable leaf and its flags": {
+			args: []string{"svc", "instance", "status", "-r"},
+		},
+		"a bare subsystem asks for the command list": {
+			args: []string{"svc", "instance"},
+		},
+		"no argument at all": {
+			args: nil,
+		},
+		"a valueless flag does not eat the next word": {
+			args: []string{"svc", "instance", "--debug"},
+		},
+		"an unknown command under a subsystem": {
+			args:    []string{"svc", "instance", "resource", "info", "push"},
+			unknown: "resource",
+		},
+		"a flag value is not mistaken for a command": {
+			args:    []string{"svc", "-s", "test/svc/s1", "instance", "resource", "info", "push"},
+			unknown: "resource",
+		},
+		"a flag value passed with an equal sign": {
+			args:    []string{"svc", "--color=no", "instance", "bogus"},
+			unknown: "bogus",
+		},
+		"a valueless flag before an unknown command": {
+			args:    []string{"svc", "instance", "--debug", "bogus"},
+			unknown: "bogus",
+		},
+		"an unknown first word is left to cobra": {
+			args: []string{"nosuchsubsystem", "instance"},
+		},
+		"shell completion never runs a command": {
+			args: []string{"__complete", "svc", "instance", "bogus"},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			root, _ := newTestTree()
+			err := ValidateArgs(root, test.args)
+			if test.unknown == "" {
+				assert.NoError(t, err)
+				return
+			}
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), `"`+test.unknown+`"`)
+		})
+	}
+}
+
+// ValidateArgs parses the args to tell the flags from the command names, and
+// cobra parses them again right after: it must not have written anything.
+func TestValidateArgsLeavesTheFlagValuesAlone(t *testing.T) {
+	root, selector := newTestTree()
+	args := []string{"svc", "-s", "test/svc/s1", "instance", "bogus"}
+
+	require.Error(t, ValidateArgs(root, args))
+	assert.Equal(t, "", *selector, "the selector flag value was written")
+	assert.False(t, root.PersistentFlags().Lookup("selector").Changed)
+}

core/commoncmd/cobra_helpers.go

@@ -47,6 +47,25 @@ func formatArgText(arg string) string {
 	return strings.Join(lines, "\n")
 }
 
+func init() {
+	cobra.AddTemplateFunc("hasGroupCommands", hasGroupCommands)
+}
+
+// hasGroupCommands is true when at least one command of cmd shows up in the
+// group. The usage template asks before printing a group title, so a group
+// holding no command, or only hidden ones, leaves no empty section behind.
+func hasGroupCommands(cmd *cobra.Command, groupID string) bool {
+	for _, sub := range cmd.Commands() {
+		if sub.GroupID != groupID {
+			continue
+		}
+		if sub.IsAvailableCommand() || sub.Name() == "help" {
+			return true
+		}
+	}
+	return false
+}
+
 // usageTemplate is the custom template that conditionally includes Arguments section
 // Note: We cannot use nindent as it's a Cobra internal function, so we rely on the fact
 // that the data is already properly formatted
@@ -64,10 +83,10 @@ Examples:
 {{.Example}}{{end}}{{if .HasAvailableSubCommands}}{{$cmds := .Commands}}{{if eq (len .Groups) 0}}
 
 Available Commands:{{range $cmds}}{{if (or .IsAvailableCommand (eq .Name "help"))}}
-  {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{else}}{{range $group := .Groups}}
+  {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{else}}{{range $group := .Groups}}{{if hasGroupCommands $ $group.ID}}
 
 {{.Title}}{{range $cmds}}{{if (and (eq .GroupID $group.ID) (or .IsAvailableCommand (eq .Name "help")))}}
-  {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{if not .AllChildCommandsHaveGroup}}
+  {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{if not .AllChildCommandsHaveGroup}}
 
 Additional Commands:{{range $cmds}}{{if (and (eq .GroupID "") (or .IsAvailableCommand (eq .Name "help")))}}
   {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{end}}{{if .Annotations.args}}

core/commoncmd/daemon.go

@@ -4,36 +4,41 @@ import "github.com/spf13/cobra"
 
 func NewCmdDaemon() *cobra.Command {
 	return &cobra.Command{
-		Use:   "daemon",
-		Short: "manage the daemon and its components",
+		GroupID: GroupIDSubsystems,
+		Use:     "daemon",
+		Short:   "manage the daemon and its components",
 	}
 }
 
 func NewCmdDaemonDNS() *cobra.Command {
 	return &cobra.Command{
-		Use:   "dns",
-		Short: "manage the nameserver",
+		GroupID: GroupIDSubsystems,
+		Use:     "dns",
+		Short:   "manage the nameserver",
 	}
 }
 
 func NewCmdDaemonHeartbeat() *cobra.Command {
 	return &cobra.Command{
-		Use:   "hb",
-		Short: "manage heartbeats",
+		GroupID: GroupIDSubsystems,
+		Use:     "hb",
+		Short:   "manage heartbeats",
 	}
 }
 
 func NewCmdDaemonListener() *cobra.Command {
 	return &cobra.Command{
-		Use:   "listener",
-		Short: "manage listeners",
+		GroupID: GroupIDSubsystems,
+		Use:     "listener",
+		Short:   "manage listeners",
 	}
 }
 
 func NewCmdDaemonRelay() *cobra.Command {
 	return &cobra.Command{
-		Use:   "relay",
-		Short: "manage the relay server",
+		GroupID: GroupIDSubsystems,
+		Use:     "relay",
+		Short:   "manage the relay server",
 	}
 }