From 189c2d826eae8a875cc475f2d5a90d3752a6b299 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Mon, 12 Oct 2026 08:50:11 -0700 Subject: [PATCH] Overhaul flag and subcommand errors in kubectl-ate Previous to this errors might be helpful (dumping full help text) but they might also be spartan: ``` $ kubectl-ate dasdsd Error: unknown command "dasdsd" for "kubectl-ate" Run 'kubectl-ate --help' for usage. ``` or they might be useless: ``` $ kubectl-ate -r Error: unknown shorthand flag: 'r' in -r ``` Sometimes they used stdout, and sometimes stderr. In short, Cobra's error handling is a mess. There are bazillions of options and I could not find a combination that did something useful. So now we take control ourselves. Every error should: 1) print an error message telling you what went wrong ("missing flag", "unknown subcommand", etc) 2) dump the closest help message 3) write to stderr exclusively This also cleans up flag descriptions. Gemini helped. --- cmd/kubectl-ate/README.md | 2 +- cmd/kubectl-ate/internal/cmd/actor.go | 74 +-- cmd/kubectl-ate/internal/cmd/actortemplate.go | 37 +- cmd/kubectl-ate/internal/cmd/admin.go | 76 +-- cmd/kubectl-ate/internal/cmd/atespace.go | 11 +- cmd/kubectl-ate/internal/cmd/egress_policy.go | 32 +- .../internal/cmd/egress_policy_test.go | 10 + cmd/kubectl-ate/internal/cmd/get_test.go | 7 +- cmd/kubectl-ate/internal/cmd/revert.go | 1 + cmd/kubectl-ate/internal/cmd/revert_actor.go | 5 +- cmd/kubectl-ate/internal/cmd/root.go | 72 ++- cmd/kubectl-ate/internal/cmd/root_test.go | 13 - cmd/kubectl-ate/internal/cmd/tag.go | 42 +- cmd/kubectl-ate/internal/cmd/validation.go | 252 +++++++++ .../internal/cmd/validation_test.go | 508 ++++++++++++++++++ cmd/kubectl-ate/internal/cmd/verbs.go | 5 +- cmd/kubectl-ate/internal/cmd/worker.go | 54 +- 17 files changed, 991 insertions(+), 210 deletions(-) create mode 100644 cmd/kubectl-ate/internal/cmd/validation.go create mode 100644 cmd/kubectl-ate/internal/cmd/validation_test.go diff --git a/cmd/kubectl-ate/README.md b/cmd/kubectl-ate/README.md index 81a9528150..5b058683a5 100644 --- a/cmd/kubectl-ate/README.md +++ b/cmd/kubectl-ate/README.md @@ -288,7 +288,7 @@ kubectl ate delete egress-policy -a --uid --versio ### Logs -`kubectl ate logs` requires a resource-type subcommand; running `kubectl ate logs ` on its own prints help. The only supported resource type is `actors`: +`kubectl ate logs` requires a resource-type subcommand. The only supported resource type is `actor`: ```bash # Print the logs an actor has produced on its current worker. diff --git a/cmd/kubectl-ate/internal/cmd/actor.go b/cmd/kubectl-ate/internal/cmd/actor.go index 1f46c6e540..20b59a3ad1 100644 --- a/cmd/kubectl-ate/internal/cmd/actor.go +++ b/cmd/kubectl-ate/internal/cmd/actor.go @@ -59,28 +59,13 @@ var ( ) var getActorsCmd = &cobra.Command{ - Use: "actor ", + Use: "actor [actor-name ...]", Aliases: []string{"actors"}, Short: "List all actors or get one or more actors", + Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { - if len(args) > 0 { - // An actor is addressed by (atespace, name), so the atespace is - // mandatory and "all atespaces" is meaningless here. - if getActorAllAtespacesFlag { - return fmt.Errorf("-A/--all-atespaces cannot be used when getting actors; pass --atespace") - } - if getActorAtespaceFlag == "" { - return fmt.Errorf("--atespace is required when getting actors") - } - } else { - // Listing requires exactly one of --atespace (one atespace) or -A (all - // atespaces). There is no default atespace to fall back on. - if getActorAllAtespacesFlag && getActorAtespaceFlag != "" { - return fmt.Errorf("--atespace and -A/--all-atespaces are mutually exclusive") - } - if !getActorAllAtespacesFlag && getActorAtespaceFlag == "" { - return fmt.Errorf("specify --atespace to list one atespace, or -A/--all-atespaces for all") - } + if err := validateAtespaceFlags("actors", args, getActorAtespaceFlag, getActorAllAtespacesFlag); err != nil { + return err } ctx := cmd.Context() @@ -131,11 +116,11 @@ var getActorsCmd = &cobra.Command{ var createActorCmd = &cobra.Command{ Use: "actor ", Short: "Create an actor", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace", "template"), RunE: func(cmd *cobra.Command, args []string) error { request, err := buildCreateActorRequest(args[0], createActorAtespaceFlag, createActorTemplateFlag, createActorTagFlag) if err != nil { - return err + return wrapUsageError(err) } ctx := cmd.Context() @@ -157,7 +142,7 @@ var createActorCmd = &cobra.Command{ var deleteActorCmd = &cobra.Command{ Use: "actor ", Short: "Delete an actor", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() c, err := newAPIClient(cmd) @@ -183,7 +168,7 @@ var deleteActorCmd = &cobra.Command{ var pauseActorCmd = &cobra.Command{ Use: "actor ", Short: "Pause an actor", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiClient, err := newAPIClient(cmd) @@ -207,7 +192,7 @@ var pauseActorCmd = &cobra.Command{ var resumeActorCmd = &cobra.Command{ Use: "actor ", Short: "Resume an actor", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiClient, err := newAPIClient(cmd) @@ -231,7 +216,7 @@ var resumeActorCmd = &cobra.Command{ var suspendActorCmd = &cobra.Command{ Use: "actor ", Short: "Suspend an actor", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiClient, err := newAPIClient(cmd) @@ -255,7 +240,7 @@ var suspendActorCmd = &cobra.Command{ var logsActorsCmd = &cobra.Command{ Use: "actor ", Short: "Stream logs for a specific actor", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: runLogsActor, } @@ -563,7 +548,7 @@ func runLogsActor(cmd *cobra.Command, args []string) error { actorRef := resources.ActorRef{Atespace: logsActorAtespaceFlag, Name: args[0]} filter, err := newLogLineFilter(actorRef, logsActorSourceFlag, logsActorContainerFlag) if err != nil { - return err + return wrapUsageError(err) } apiClient, err := newAPIClient(cmd) @@ -748,38 +733,31 @@ func filterAndDisplayLogLine(line string, filter logLineFilter, w, stderr io.Wri } func init() { - getActorsCmd.Flags().StringVarP(&getActorAtespaceFlag, "atespace", "a", "", "Atespace to list/get actors in. Required when getting actors; for listing, use this or -A.") - getActorsCmd.Flags().BoolVarP(&getActorAllAtespacesFlag, "all-atespaces", "A", false, "List actors across all atespaces (listing only; mutually exclusive with --atespace)") + getActorsCmd.Flags().StringVarP(&getActorAtespaceFlag, "atespace", "a", "", "get actors in a specific atespace (mutually exclusive with --all-atespaces)") + getActorsCmd.Flags().BoolVarP(&getActorAllAtespacesFlag, "all-atespaces", "A", false, "get actors across all atespaces (mutually exclusive with --atespace)") getCmd.AddCommand(getActorsCmd) - createActorCmd.Flags().StringVar(&createActorTemplateFlag, "template", "", "The name of the ActorTemplate to derive the actor from, as /, or just to use the actor's own atespace (--atespace)") - _ = createActorCmd.MarkFlagRequired("template") - createActorCmd.Flags().StringVarP(&createActorAtespaceFlag, "atespace", "a", "", "Atespace to create the actor in") - _ = createActorCmd.MarkFlagRequired("atespace") - createActorCmd.Flags().StringVar(&createActorTagFlag, "tag", "", "The name of a Tag to initialize the actor from, as /, or just to use the actor's own atespace (--atespace)") + createActorCmd.Flags().StringVar(&createActorTemplateFlag, "template", "", "actor-template to derive the actor from, as /, or just to use the actor's own atespace (see --atespace)") + createActorCmd.Flags().StringVarP(&createActorAtespaceFlag, "atespace", "a", "", "atespace to create the actor in") + createActorCmd.Flags().StringVar(&createActorTagFlag, "tag", "", "tag to initialize the actor from, as /, or just to use the actor's own atespace (see --atespace)") createCmd.AddCommand(createActorCmd) - deleteActorCmd.Flags().StringVarP(&deleteActorAtespaceFlag, "atespace", "a", "", "Atespace the actor lives in") - _ = deleteActorCmd.MarkFlagRequired("atespace") - deleteActorCmd.Flags().BoolVar(&deleteActorAnyStateFlag, "any-state", false, "Delete the actor from any state") + deleteActorCmd.Flags().StringVarP(&deleteActorAtespaceFlag, "atespace", "a", "", "the actor's atespace") + deleteActorCmd.Flags().BoolVar(&deleteActorAnyStateFlag, "any-state", false, "delete the actor regardless of current state") deleteCmd.AddCommand(deleteActorCmd) - pauseActorCmd.Flags().StringVarP(&pauseActorAtespaceFlag, "atespace", "a", "", "Atespace the actor lives in") - _ = pauseActorCmd.MarkFlagRequired("atespace") + pauseActorCmd.Flags().StringVarP(&pauseActorAtespaceFlag, "atespace", "a", "", "the actor's atespace") pauseCmd.AddCommand(pauseActorCmd) - resumeActorCmd.Flags().StringVarP(&resumeActorAtespaceFlag, "atespace", "a", "", "Atespace the actor lives in") - _ = resumeActorCmd.MarkFlagRequired("atespace") + resumeActorCmd.Flags().StringVarP(&resumeActorAtespaceFlag, "atespace", "a", "", "the actor's atespace") resumeCmd.AddCommand(resumeActorCmd) - suspendActorCmd.Flags().StringVarP(&suspendActorAtespaceFlag, "atespace", "a", "", "Atespace the actor lives in") - _ = suspendActorCmd.MarkFlagRequired("atespace") + suspendActorCmd.Flags().StringVarP(&suspendActorAtespaceFlag, "atespace", "a", "", "the actor's atespace") suspendCmd.AddCommand(suspendActorCmd) - logsActorsCmd.Flags().BoolVarP(&logsActorFollowFlag, "follow", "f", false, "Specify if the logs should be streamed.") - logsActorsCmd.Flags().StringVarP(&logsActorAtespaceFlag, "atespace", "a", "", "Atespace the actor lives in") - _ = logsActorsCmd.MarkFlagRequired("atespace") - logsActorsCmd.Flags().StringVarP(&logsActorContainerFlag, "container", "c", "", "Show only logs from this container; use _pause for runsc diagnostics.") - logsActorsCmd.Flags().StringVar(&logsActorSourceFlag, "source", string(logSourceAll), "Which lines to show: all, containers (container output, no lifecycle events), or lifecycle (only the actor's lifecycle events).") + logsActorsCmd.Flags().BoolVarP(&logsActorFollowFlag, "follow", "f", false, "stream logs continuously") + logsActorsCmd.Flags().StringVarP(&logsActorAtespaceFlag, "atespace", "a", "", "the actor's atespace") + logsActorsCmd.Flags().StringVarP(&logsActorContainerFlag, "container", "c", "", "show only logs from a named container; use _pause for runsc diagnostics") + logsActorsCmd.Flags().StringVar(&logsActorSourceFlag, "source", string(logSourceAll), "which lines to show: all, containers (container output, no lifecycle events), or lifecycle (only the actor's lifecycle events)") logsCmd.AddCommand(logsActorsCmd) } diff --git a/cmd/kubectl-ate/internal/cmd/actortemplate.go b/cmd/kubectl-ate/internal/cmd/actortemplate.go index 2c4205ea0f..a27d48fa47 100644 --- a/cmd/kubectl-ate/internal/cmd/actortemplate.go +++ b/cmd/kubectl-ate/internal/cmd/actortemplate.go @@ -34,29 +34,14 @@ var ( ) var getActorTemplatesCmd = &cobra.Command{ - Use: "actor-template ", + Use: "actor-template [template-name ...]", Aliases: []string{"actor-templates"}, Short: "List all actor templates or get one or more actor templates", Long: "List or get actor templates.", + Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { - if len(args) > 0 { - // A template is addressed by (atespace, name), so the atespace is - // mandatory and "all atespaces" is meaningless here. - if getActorTemplateAllAtespacesFlag { - return fmt.Errorf("-A/--all-atespaces cannot be used when getting actor templates; pass --atespace") - } - if getActorTemplateAtespaceFlag == "" { - return fmt.Errorf("--atespace is required when getting actor templates") - } - } else { - // Listing requires exactly one of --atespace (one atespace) or -A (all - // atespaces). There is no default atespace to fall back on. - if getActorTemplateAllAtespacesFlag && getActorTemplateAtespaceFlag != "" { - return fmt.Errorf("--atespace and -A/--all-atespaces are mutually exclusive") - } - if !getActorTemplateAllAtespacesFlag && getActorTemplateAtespaceFlag == "" { - return fmt.Errorf("specify --atespace to list one atespace, or -A/--all-atespaces for all") - } + if err := validateAtespaceFlags("actor templates", args, getActorTemplateAtespaceFlag, getActorTemplateAllAtespacesFlag); err != nil { + return err } ctx := cmd.Context() @@ -117,7 +102,7 @@ The template's atespace and name come from the manifest's metadata. The atespace must already exist. Actor templates are immutable: there is no update; delete and recreate to change one.`, - Args: cobra.NoArgs, + Args: noArgs("the template name comes from the manifest passed to -f/--filename", "filename"), RunE: func(cmd *cobra.Command, args []string) error { data, err := readFileOrStdin(cmd.InOrStdin(), createActorTemplateFilenameFlag) if err != nil { @@ -149,7 +134,7 @@ var deleteActorTemplateCmd = &cobra.Command{ Long: `Delete an actor template. The server also deletes the template's golden actor and golden snapshot.`, - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() c, err := newAPIClient(cmd) @@ -197,15 +182,13 @@ func readFileOrStdin(in io.Reader, path string) ([]byte, error) { } func init() { - getActorTemplatesCmd.Flags().StringVarP(&getActorTemplateAtespaceFlag, "atespace", "a", "", "Atespace to list/get actor templates in. Required when getting templates; for listing, use this or -A.") - getActorTemplatesCmd.Flags().BoolVarP(&getActorTemplateAllAtespacesFlag, "all-atespaces", "A", false, "List actor templates across all atespaces (listing only; mutually exclusive with --atespace)") + getActorTemplatesCmd.Flags().StringVarP(&getActorTemplateAtespaceFlag, "atespace", "a", "", "get actor-templates in a specific atespace (mutually exclusive with --all-atespaces)") + getActorTemplatesCmd.Flags().BoolVarP(&getActorTemplateAllAtespacesFlag, "all-atespaces", "A", false, "get actor-templates across all atespaces (mutually exclusive with --atespace)") getCmd.AddCommand(getActorTemplatesCmd) - createActorTemplateCmd.Flags().StringVarP(&createActorTemplateFilenameFlag, "filename", "f", "", "Manifest file holding a single protojson-shaped ActorTemplate document; use - for stdin (required)") - _ = createActorTemplateCmd.MarkFlagRequired("filename") + createActorTemplateCmd.Flags().StringVarP(&createActorTemplateFilenameFlag, "filename", "f", "", "manifest file holding a single JSON or YAML actor-template document, or - for stdin") createCmd.AddCommand(createActorTemplateCmd) - deleteActorTemplateCmd.Flags().StringVarP(&deleteActorTemplateAtespaceFlag, "atespace", "a", "", "Atespace the actor template lives in") - _ = deleteActorTemplateCmd.MarkFlagRequired("atespace") + deleteActorTemplateCmd.Flags().StringVarP(&deleteActorTemplateAtespaceFlag, "atespace", "a", "", "the actor-template's atespace") deleteCmd.AddCommand(deleteActorTemplateCmd) } diff --git a/cmd/kubectl-ate/internal/cmd/admin.go b/cmd/kubectl-ate/internal/cmd/admin.go index af30acbdc6..3c9c0c1faf 100644 --- a/cmd/kubectl-ate/internal/cmd/admin.go +++ b/cmd/kubectl-ate/internal/cmd/admin.go @@ -50,14 +50,10 @@ var adminCmd = &cobra.Command{ var makeCaPoolCmd = &cobra.Command{ Use: "make-ca-pool", Short: "Make a new secret that contains a CA pool to be used by a signing controller", + Args: noArgs("use --name to specify the secret name", "name"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - kc, err := newKubeClient(cmd.ErrOrStderr()) - if err != nil { - return err - } - var keyType localca.KeyType switch makeCaPoolKeyTypeFlag { case "ED25519": @@ -65,7 +61,7 @@ var makeCaPoolCmd = &cobra.Command{ case "ECDSAP256": keyType = localca.KeyTypeECDSAP256 default: - return fmt.Errorf("unknown key type %q", makeCaPoolKeyTypeFlag) + return usageErrorf("unknown key type %q", makeCaPoolKeyTypeFlag) } ca, err := localca.GenerateCA( @@ -77,6 +73,11 @@ var makeCaPoolCmd = &cobra.Command{ return fmt.Errorf("while generating CA: %w", err) } + kc, err := newKubeClient(cmd.ErrOrStderr()) + if err != nil { + return err + } + pool := &localca.ConcretePool{ CAs: []*localca.CA{ca}, ActiveForSigning: makeCaPoolIDFlag, @@ -121,15 +122,16 @@ var makeCaPoolCmd = &cobra.Command{ var makeJwtPoolCmd = &cobra.Command{ Use: "make-jwt-pool", Short: "Make a new secret that contains a JWT authority pool to be used by the actor ID broker", + Args: noArgs("use --name to specify the secret name", "name"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - kc, err := newKubeClient(cmd.ErrOrStderr()) + secret, keyID, err := newJWTPoolSecret(poolSecretNamespaceFlag, poolSecretNameFlag, jwtAlgFlag, jwtKeyIDFlag) if err != nil { - return err + return wrapUsageError(err) } - secret, keyID, err := newJWTPoolSecret(poolSecretNamespaceFlag, poolSecretNameFlag, jwtAlgFlag, jwtKeyIDFlag) + kc, err := newKubeClient(cmd.ErrOrStderr()) if err != nil { return err } @@ -147,6 +149,7 @@ var makeJwtPoolCmd = &cobra.Command{ var listJwtKeysCmd = &cobra.Command{ Use: "list-jwt-keys", Short: "List the keys in a JWT authority pool secret", + Args: noArgs("use --name to specify the secret name", "name"), RunE: func(cmd *cobra.Command, args []string) error { kc, err := newKubeClient(cmd.ErrOrStderr()) if err != nil { @@ -169,16 +172,18 @@ var addJwtKeyCmd = &cobra.Command{ The key is published to relying parties but signs nothing until activate-jwt-key makes it the signing key. Activate it only once relying parties have refetched the key set.`, + Args: noArgs("use --name to specify the secret name", "name"), RunE: func(cmd *cobra.Command, args []string) error { - kc, err := newKubeClient(cmd.ErrOrStderr()) + authority, err := localjwtauthority.GenerateAuthority(jwtAlgFlag, jwtKeyIDFlag) if err != nil { - return err + return wrapUsageError(fmt.Errorf("while generating JWT authority: %w", err)) } - authority, err := localjwtauthority.GenerateAuthority(jwtAlgFlag, jwtKeyIDFlag) + kc, err := newKubeClient(cmd.ErrOrStderr()) if err != nil { - return fmt.Errorf("while generating JWT authority: %w", err) + return err } + if err := updateJWTPool(cmd.Context(), cmd.ErrOrStderr(), kc.CoreV1().Secrets(poolSecretNamespaceFlag), poolSecretNameFlag, func(pool *localjwtauthority.ConcretePool) error { return pool.AddAuthority(authority) }); err != nil { @@ -193,6 +198,7 @@ the key set.`, var activateJwtKeyCmd = &cobra.Command{ Use: "activate-jwt-key", Short: "Make a key in a JWT authority pool secret the one that signs", + Args: noArgs("use --name and --key-id to specify the secret and key", "name", "key-id"), RunE: func(cmd *cobra.Command, args []string) error { kc, err := newKubeClient(cmd.ErrOrStderr()) if err != nil { @@ -217,6 +223,7 @@ var removeJwtKeyCmd = &cobra.Command{ Tokens the key signed stop verifying once relying parties refetch the key set, so remove it only after the last of them has expired.`, + Args: noArgs("use --name and --key-id to specify the secret and key", "name", "key-id"), RunE: func(cmd *cobra.Command, args []string) error { kc, err := newKubeClient(cmd.ErrOrStderr()) if err != nil { @@ -330,41 +337,36 @@ func newJWTPoolSecret(namespace, name, algorithm, keyID string) (*corev1.Secret, } func init() { + requireSubcommand(adminCmd) rootCmd.AddCommand(adminCmd) - makeCaPoolCmd.Flags().StringVar(&makeCaPoolIDFlag, "ca-id", "", "The ID of the initial CA in the Pool") - makeCaPoolCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "Create the secret in this namespace") - makeCaPoolCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "Create the secret with this name") - makeCaPoolCmd.Flags().StringVar(&makeCaPoolKeyTypeFlag, "key-type", "ED25519", "CA key type. One of [ED25519, ECDSAP256]") - makeCaPoolCmd.Flags().DurationVar(&makeCaPoolValidityFlag, "validity", 365*24*time.Hour, "How long the CA certificate is valid for") - _ = makeCaPoolCmd.MarkFlagRequired("name") + makeCaPoolCmd.Flags().StringVar(&makeCaPoolIDFlag, "ca-id", "", "ID of the initial CA in the pool") + makeCaPoolCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "namespace of the pool secret") + makeCaPoolCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "name of the pool secret") + makeCaPoolCmd.Flags().StringVar(&makeCaPoolKeyTypeFlag, "key-type", "ED25519", "CA key type: ED25519 or ECDSAP256") + makeCaPoolCmd.Flags().DurationVar(&makeCaPoolValidityFlag, "validity", 365*24*time.Hour, "how long the CA certificate is valid for") adminCmd.AddCommand(makeCaPoolCmd) - makeJwtPoolCmd.Flags().StringVar(&jwtAlgFlag, "alg", "ES256", "Signing algorithm of the initial key. One of [ES256, RS256]; RS256 keys are 4096-bit RSA") - makeJwtPoolCmd.Flags().StringVar(&jwtKeyIDFlag, "key-id", "", "The ID of the initial JWT signing key in the pool. Defaults to the base64url SHA-256 of the key's PKIX encoding") - makeJwtPoolCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "Create the secret in this namespace") - makeJwtPoolCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "Create the secret with this name") - _ = makeJwtPoolCmd.MarkFlagRequired("name") + makeJwtPoolCmd.Flags().StringVar(&jwtAlgFlag, "alg", "ES256", "signing algorithm of the initial key: ES256 or RS256 (4096-bit RSA)") + makeJwtPoolCmd.Flags().StringVar(&jwtKeyIDFlag, "key-id", "", "ID of the initial JWT signing key in the pool (defaults to the base64url SHA-256 of the key's PKIX encoding)") + makeJwtPoolCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "namespace of the pool secret") + makeJwtPoolCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "name of the pool secret") adminCmd.AddCommand(makeJwtPoolCmd) - listJwtKeysCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "The namespace of the pool secret") - listJwtKeysCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "The name of the pool secret") - _ = listJwtKeysCmd.MarkFlagRequired("name") + listJwtKeysCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "namespace of the pool secret") + listJwtKeysCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "name of the pool secret") adminCmd.AddCommand(listJwtKeysCmd) - addJwtKeyCmd.Flags().StringVar(&jwtAlgFlag, "alg", "ES256", "Signing algorithm of the new key. One of [ES256, RS256]; RS256 keys are 4096-bit RSA") - addJwtKeyCmd.Flags().StringVar(&jwtKeyIDFlag, "key-id", "", "The ID of the new key. Defaults to the base64url SHA-256 of the key's PKIX encoding") - addJwtKeyCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "The namespace of the pool secret") - addJwtKeyCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "The name of the pool secret") - _ = addJwtKeyCmd.MarkFlagRequired("name") + addJwtKeyCmd.Flags().StringVar(&jwtAlgFlag, "alg", "ES256", "signing algorithm of the new key: ES256 or RS256 (4096-bit RSA)") + addJwtKeyCmd.Flags().StringVar(&jwtKeyIDFlag, "key-id", "", "ID of the new key (defaults to the base64url SHA-256 of the key's PKIX encoding)") + addJwtKeyCmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "namespace of the pool secret") + addJwtKeyCmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "name of the pool secret") adminCmd.AddCommand(addJwtKeyCmd) for _, cmd := range []*cobra.Command{activateJwtKeyCmd, removeJwtKeyCmd} { - cmd.Flags().StringVar(&jwtKeyIDFlag, "key-id", "", "The ID of the key") - cmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "The namespace of the pool secret") - cmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "The name of the pool secret") - _ = cmd.MarkFlagRequired("key-id") - _ = cmd.MarkFlagRequired("name") + cmd.Flags().StringVar(&jwtKeyIDFlag, "key-id", "", "ID of the key") + cmd.Flags().StringVar(&poolSecretNamespaceFlag, "secret-namespace", "default", "namespace of the pool secret") + cmd.Flags().StringVar(&poolSecretNameFlag, "name", "", "name of the pool secret") adminCmd.AddCommand(cmd) } } diff --git a/cmd/kubectl-ate/internal/cmd/atespace.go b/cmd/kubectl-ate/internal/cmd/atespace.go index 0427c02dbf..8d208bdc69 100644 --- a/cmd/kubectl-ate/internal/cmd/atespace.go +++ b/cmd/kubectl-ate/internal/cmd/atespace.go @@ -23,9 +23,10 @@ import ( ) var getAtespacesCmd = &cobra.Command{ - Use: "atespace [name ...]", + Use: "atespace [atespace-name ...]", Aliases: []string{"atespaces"}, Short: "List all atespaces or get one or more atespaces", + Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiClient, err := newAPIClient(cmd) @@ -70,9 +71,9 @@ var getAtespacesCmd = &cobra.Command{ } var createAtespaceCmd = &cobra.Command{ - Use: "atespace [name]", + Use: "atespace ", Short: "Create an atespace", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg(""), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiClient, err := newAPIClient(cmd) @@ -97,9 +98,9 @@ var createAtespaceCmd = &cobra.Command{ } var deleteAtespaceCmd = &cobra.Command{ - Use: "atespace [name]", + Use: "atespace ", Short: "Delete an empty atespace (fails if any actors remain)", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg(""), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiClient, err := newAPIClient(cmd) diff --git a/cmd/kubectl-ate/internal/cmd/egress_policy.go b/cmd/kubectl-ate/internal/cmd/egress_policy.go index b80171dcc1..fb06708e8e 100644 --- a/cmd/kubectl-ate/internal/cmd/egress_policy.go +++ b/cmd/kubectl-ate/internal/cmd/egress_policy.go @@ -45,7 +45,7 @@ var getEgressPolicyCmd = &cobra.Command{ Aliases: []string{"egress-policies"}, Short: "Get the egress policy of an actor", // TODO(#1550): accept several actors and print a list document. - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: runGetEgressPolicy, } @@ -57,7 +57,7 @@ var createEgressPolicyCmd = &cobra.Command{ The manifest is a YAML or JSON EgressPolicy, as printed by "kubectl ate get egress-policy -a -o yaml". Its metadata may be omitted.`, - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace", "filename"), RunE: runCreateEgressPolicy, } @@ -70,14 +70,14 @@ The manifest is a YAML or JSON EgressPolicy, as printed by "kubectl ate get egress-policy -a -o yaml". Its metadata.uid and metadata.version are required preconditions; the server rejects the update when they no longer match those of the stored egress policy.`, - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace", "filename"), RunE: runUpdateEgressPolicy, } var deleteEgressPolicyCmd = &cobra.Command{ Use: "egress-policy ", Short: "Delete the egress policy of an actor", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: runDeleteEgressPolicy, } @@ -416,7 +416,7 @@ func runDeleteEgressPolicy(cmd *cobra.Command, args []string) error { version: egressPolicyFlags.version, }) if err != nil { - return err + return wrapUsageError(err) } apiClient, err := newAPIClient(cmd) @@ -435,25 +435,19 @@ func runDeleteEgressPolicy(cmd *cobra.Command, args []string) error { } func init() { - getEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "Atespace the actor lives in (required)") - _ = getEgressPolicyCmd.MarkFlagRequired("atespace") + getEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "the actor's atespace") getCmd.AddCommand(getEgressPolicyCmd) - createEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "Atespace the actor lives in (required)") - createEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.filename, "filename", "f", "", "Manifest file holding one EgressPolicy; use - for stdin (required)") - _ = createEgressPolicyCmd.MarkFlagRequired("atespace") - _ = createEgressPolicyCmd.MarkFlagRequired("filename") + createEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "the actor's atespace") + createEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.filename, "filename", "f", "", "a manifest file holding a single JSON or YAML egress-policy document, or - for stdin") createCmd.AddCommand(createEgressPolicyCmd) - updateEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "Atespace the actor lives in (required)") - updateEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.filename, "filename", "f", "", "Manifest file holding one EgressPolicy; use - for stdin (required)") - _ = updateEgressPolicyCmd.MarkFlagRequired("atespace") - _ = updateEgressPolicyCmd.MarkFlagRequired("filename") + updateEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "the actor's atespace") + updateEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.filename, "filename", "f", "", "a manifest file holding a single JSON or YAML egress-policy document, or - for stdin") updateCmd.AddCommand(updateEgressPolicyCmd) - deleteEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "Atespace the actor lives in (required)") - _ = deleteEgressPolicyCmd.MarkFlagRequired("atespace") - deleteEgressPolicyCmd.Flags().StringVar(&egressPolicyFlags.uid, "uid", "", "Delete only if the policy's metadata.uid matches; from get -o yaml") - deleteEgressPolicyCmd.Flags().Int64Var(&egressPolicyFlags.version, "version", 0, "Delete only if the policy's metadata.version matches; from get -o yaml") + deleteEgressPolicyCmd.Flags().StringVarP(&egressPolicyFlags.atespace, "atespace", "a", "", "the actor's atespace") + deleteEgressPolicyCmd.Flags().StringVar(&egressPolicyFlags.uid, "uid", "", "delete only if the policy's metadata.uid matches; from get -o yaml") + deleteEgressPolicyCmd.Flags().Int64Var(&egressPolicyFlags.version, "version", 0, "delete only if the policy's metadata.version matches; from get -o yaml") deleteCmd.AddCommand(deleteEgressPolicyCmd) } diff --git a/cmd/kubectl-ate/internal/cmd/egress_policy_test.go b/cmd/kubectl-ate/internal/cmd/egress_policy_test.go index 6871aa919a..b740d5724a 100644 --- a/cmd/kubectl-ate/internal/cmd/egress_policy_test.go +++ b/cmd/kubectl-ate/internal/cmd/egress_policy_test.go @@ -27,6 +27,7 @@ import ( "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/google/go-cmp/cmp" + "github.com/spf13/cobra" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -392,6 +393,15 @@ func TestEgressPolicyManifest_RoundTrip(t *testing.T) { } func TestEgressPolicyCommandArgs(t *testing.T) { + resetFlagsRecursive(rootCmd) + t.Cleanup(func() { resetFlagsRecursive(rootCmd) }) + for _, c := range []*cobra.Command{getEgressPolicyCmd, createEgressPolicyCmd, updateEgressPolicyCmd, deleteEgressPolicyCmd} { + _ = c.Flags().Set("atespace", "team-a") + if c.Flags().Lookup("filename") != nil { + _ = c.Flags().Set("filename", "policy.yaml") + } + } + runCommandArgsTests(t, []commandArgsTest{ {name: "get", command: getEgressPolicyCmd, args: []string{"c1"}}, {name: "get requires actor", command: getEgressPolicyCmd, wantErr: true}, diff --git a/cmd/kubectl-ate/internal/cmd/get_test.go b/cmd/kubectl-ate/internal/cmd/get_test.go index 1b5aac6655..9a9410acda 100644 --- a/cmd/kubectl-ate/internal/cmd/get_test.go +++ b/cmd/kubectl-ate/internal/cmd/get_test.go @@ -35,6 +35,7 @@ func TestGetScopeBeforeConnecting(t *testing.T) { }{ {"actors", getActorsCmd, &getActorAtespaceFlag, &getActorAllAtespacesFlag}, {"actor templates", getActorTemplatesCmd, &getActorTemplateAtespaceFlag, &getActorTemplateAllAtespacesFlag}, + {"tags", getTagsCmd, &getTagAtespaceFlag, &getTagAllAtespacesFlag}, } { t.Run(resource.name, func(t *testing.T) { previousAtespace, previousAll := *resource.atespace, *resource.all @@ -54,7 +55,7 @@ func TestGetScopeBeforeConnecting(t *testing.T) { }{ { name: "get without atespace", args: []string{"counter"}, - wantErr: "--atespace is required when getting", + wantErr: "-a/--atespace is required when getting", }, { name: "get with all atespaces", args: []string{"counter"}, all: true, @@ -66,11 +67,11 @@ func TestGetScopeBeforeConnecting(t *testing.T) { }, { name: "list without scope", - wantErr: "specify --atespace to list one atespace, or -A/--all-atespaces for all", + wantErr: "-a/--atespace or -A/--all-atespaces is required", }, { name: "list with both scopes", atespace: "demo", all: true, - wantErr: "--atespace and -A/--all-atespaces are mutually exclusive", + wantErr: "-a/--atespace and -A/--all-atespaces are mutually exclusive", }, // Valid scopes reach client initialization, where the missing kubeconfig fails. { diff --git a/cmd/kubectl-ate/internal/cmd/revert.go b/cmd/kubectl-ate/internal/cmd/revert.go index f27df15fbe..9375384318 100644 --- a/cmd/kubectl-ate/internal/cmd/revert.go +++ b/cmd/kubectl-ate/internal/cmd/revert.go @@ -24,5 +24,6 @@ var revertCmd = &cobra.Command{ } func init() { + requireSubcommand(revertCmd) rootCmd.AddCommand(revertCmd) } diff --git a/cmd/kubectl-ate/internal/cmd/revert_actor.go b/cmd/kubectl-ate/internal/cmd/revert_actor.go index c419330bf4..d4890c9539 100644 --- a/cmd/kubectl-ate/internal/cmd/revert_actor.go +++ b/cmd/kubectl-ate/internal/cmd/revert_actor.go @@ -32,7 +32,7 @@ var revertActorCmd = &cobra.Command{ The actor's external snapshot is left untouched, so a later resume restores from it. Only crashed, running, or paused actors can be reverted.`, - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() apiClient, err := newAPIClient(cmd) @@ -54,7 +54,6 @@ from it. Only crashed, running, or paused actors can be reverted.`, } func init() { - revertActorCmd.Flags().StringVarP(&revertAtespaceFlag, "atespace", "a", "", "Atespace the actor lives in") - _ = revertActorCmd.MarkFlagRequired("atespace") + revertActorCmd.Flags().StringVarP(&revertAtespaceFlag, "atespace", "a", "", "the actor's atespace") revertCmd.AddCommand(revertActorCmd) } diff --git a/cmd/kubectl-ate/internal/cmd/root.go b/cmd/kubectl-ate/internal/cmd/root.go index d6563916c7..e4d1ea1a07 100644 --- a/cmd/kubectl-ate/internal/cmd/root.go +++ b/cmd/kubectl-ate/internal/cmd/root.go @@ -15,8 +15,10 @@ package cmd import ( + "errors" "fmt" "os" + "strings" "github.com/spf13/cobra" "google.golang.org/grpc/metadata" @@ -37,14 +39,18 @@ var ( ) var rootCmd = &cobra.Command{ - Use: "kubectl-ate", - Short: "A kubectl plugin for managing Agent Substrate environments", - Long: `kubectl ate is a CLI tool to manage Actor and Worker lifecycles in an Agent Substrate.`, - Version: version.String(), - SilenceUsage: true, + Use: "kubectl-ate", + Short: "A kubectl plugin for managing Agent Substrate environments", + Long: `kubectl ate is a CLI tool to manage Actor and Worker lifecycles in an Agent Substrate.`, + Version: version.String(), + SilenceUsage: true, // we will handle it ourselves + SilenceErrors: true, // we will handle it ourselves PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + if err := checkSwallowedFlags(cmd); err != nil { + return err + } if outputFmt != "table" && outputFmt != "json" && outputFmt != "yaml" { - return fmt.Errorf("invalid output format %q. Must be one of: table, json, yaml", outputFmt) + return usageErrorf("invalid output format %q. Must be one of: table, json, yaml", outputFmt) } if usePreview { cmd.SetContext(metadata.AppendToOutgoingContext(cmd.Context(), preview.Header, "true")) @@ -53,22 +59,58 @@ var rootCmd = &cobra.Command{ }, } +func executeRoot(root *cobra.Command) error { + executedCmd, err := root.ExecuteC() + if err == nil && executedCmd != nil && isGroupCommand(executedCmd) && len(executedCmd.Flags().Args()) > 0 { + err = unknownSubcommandError(executedCmd, executedCmd.Flags().Args()[0]) + } + if err != nil { + if executedCmd == nil { + executedCmd = root + } + errOut := executedCmd.ErrOrStderr() + fmt.Fprintf(errOut, "Error: %v\n", err) + var uerr *usageError + if errors.As(err, &uerr) { + fmt.Fprintf(errOut, "\n%s", executedCmd.UsageString()) + } + return err + } + return nil +} + func Execute() { - if err := rootCmd.Execute(); err != nil { + if err := executeRoot(rootCmd); err != nil { os.Exit(1) } } func init() { + requireSubcommand(rootCmd) + rootCmd.SetFlagErrorFunc(handleFlagError) + defaultHelpFunc := rootCmd.HelpFunc() + rootCmd.SetHelpFunc(func(c *cobra.Command, a []string) { + if isGroupCommand(c) && len(c.Flags().Args()) > 0 { + return + } + defaultHelpFunc(c, a) + }) + rootCmd.SetUsageTemplate(strings.Replace( + rootCmd.UsageTemplate(), + "{{if .Runnable}}", + "{{if and .Runnable (not .HasAvailableSubCommands)}}", + 1, + )) + // Persistent flags are available to this command and all subcommands - rootCmd.PersistentFlags().StringVar(&kubeconfig, "kubeconfig", "", "Path to the kubeconfig file") - rootCmd.PersistentFlags().StringVar(&k8sContext, "context", "", "The name of the kubeconfig context to use") - rootCmd.PersistentFlags().StringVar(&endpoint, "endpoint", "", "Manual override for the gRPC target (e.g., localhost:8080). If omitted, automatically port-forwards.") - rootCmd.PersistentFlags().StringVar(&tokenFile, "token-file", "", "Path to a bearer token for ate-api authentication, or - to read it from stdin. Defaults to a Kubernetes ServiceAccount token.") - rootCmd.PersistentFlags().StringVarP(&outputFmt, "output", "o", "table", "Output format. One of: table|json|yaml") - rootCmd.PersistentFlags().BoolVar(&traceEnabled, "trace", false, "Enable tracing for the request") - rootCmd.PersistentFlags().CountVarP(&verbosity, "verbose", "v", "Increase output verbosity; may be repeated (e.g. -v, -vv, -vvvvv)") - rootCmd.PersistentFlags().BoolVar(&usePreview, "preview", false, "Allow the request to use preview features the server has enabled") + rootCmd.PersistentFlags().StringVar(&kubeconfig, "kubeconfig", "", "path to the kubeconfig file") + rootCmd.PersistentFlags().StringVar(&k8sContext, "context", "", "kubeconfig context to use") + rootCmd.PersistentFlags().StringVar(&endpoint, "endpoint", "", "manual override for the gRPC target (e.g., localhost:8080); if omitted, automatically port-forwards") + rootCmd.PersistentFlags().StringVar(&tokenFile, "token-file", "", "path to a bearer token for ate-api authentication, or - for stdin (defaults to a Kubernetes ServiceAccount token)") + rootCmd.PersistentFlags().StringVarP(&outputFmt, "output", "o", "table", "output format: table, json, or yaml") + rootCmd.PersistentFlags().BoolVar(&traceEnabled, "trace", false, "enable tracing for the request") + rootCmd.PersistentFlags().CountVarP(&verbosity, "verbose", "v", "output verbosity; may be repeated (e.g. -v, -vv, -vvvvv)") + rootCmd.PersistentFlags().BoolVar(&usePreview, "preview", false, "allow the use of preview features enabled on the server") // Register --version without a -v shorthand so -v stays free for --verbose. rootCmd.Flags().Bool("version", false, "version for "+rootCmd.DisplayName()) diff --git a/cmd/kubectl-ate/internal/cmd/root_test.go b/cmd/kubectl-ate/internal/cmd/root_test.go index 569ee7103c..61f5b86fd3 100644 --- a/cmd/kubectl-ate/internal/cmd/root_test.go +++ b/cmd/kubectl-ate/internal/cmd/root_test.go @@ -60,19 +60,6 @@ func TestVersionFlag(t *testing.T) { if f := rootCmd.Flags().Lookup("version"); f != nil && f.Shorthand != "" { t.Errorf("--version shorthand = %q, want empty", f.Shorthand) } - - resetRootFlags(t) - out.Reset() - errOut.Reset() - rootCmd.SetOut(&out) - rootCmd.SetErr(&errOut) - rootCmd.SetArgs([]string{"-v"}) - if err := rootCmd.Execute(); err != nil { - t.Fatalf("Execute(-v) unexpected error: %v", err) - } - if strings.HasPrefix(out.String(), "kubectl-ate version ") { - t.Errorf("Execute(-v) printed version %q, want help output", out.String()) - } } func TestVerboseFlagCounting(t *testing.T) { diff --git a/cmd/kubectl-ate/internal/cmd/tag.go b/cmd/kubectl-ate/internal/cmd/tag.go index cd3337320b..9f34843457 100644 --- a/cmd/kubectl-ate/internal/cmd/tag.go +++ b/cmd/kubectl-ate/internal/cmd/tag.go @@ -40,15 +40,10 @@ var getTagsCmd = &cobra.Command{ Use: "tag [tag-name ...]", Aliases: []string{"tags"}, Short: "List or get tags", + Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { - if getTagAllAtespacesFlag && getTagAtespaceFlag != "" { - return fmt.Errorf("--atespace and -A/--all-atespaces are mutually exclusive") - } - if len(args) > 0 && getTagAtespaceFlag == "" { - return fmt.Errorf("--atespace is required when getting tags") - } - if len(args) == 0 && !getTagAllAtespacesFlag && getTagAtespaceFlag == "" { - return fmt.Errorf("specify --atespace , or -A/--all-atespaces") + if err := validateAtespaceFlags("tags", args, getTagAtespaceFlag, getTagAllAtespacesFlag); err != nil { + return err } ctx := cmd.Context() @@ -98,11 +93,11 @@ var createTagCmd = &cobra.Command{ Long: "Tag the external snapshot a suspended actor holds.\n\n" + "The tag gets its own copy of that snapshot, so suspending the actor\n" + "again or deleting it cannot collect what the tag names.", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace", "actor"), RunE: func(cmd *cobra.Command, args []string) error { scope, err := parseTagScope(createTagScopeFlag) if err != nil { - return err + return wrapUsageError(err) } ctx := cmd.Context() client, err := newAPIClient(cmd) @@ -128,11 +123,11 @@ var createTagCmd = &cobra.Command{ var updateTagCmd = &cobra.Command{ Use: "tag ", Short: "Publish or unpublish a tag", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace", "scope"), RunE: func(cmd *cobra.Command, args []string) error { scope, err := parseTagScope(updateTagScopeFlag) if err != nil { - return err + return wrapUsageError(err) } ctx := cmd.Context() client, err := newAPIClient(cmd) @@ -153,7 +148,7 @@ var updateTagCmd = &cobra.Command{ var deleteTagCmd = &cobra.Command{ Use: "tag ", Short: "Delete a tag", - Args: cobra.ExactArgs(1), + Args: exactlyOneArg("", "atespace"), RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() client, err := newAPIClient(cmd) @@ -202,24 +197,19 @@ func parseTagScope(value string) (ateapipb.TagScope, error) { } func init() { - getTagsCmd.Flags().StringVarP(&getTagAtespaceFlag, "atespace", "a", "", "Atespace to list/get tags in") - getTagsCmd.Flags().BoolVarP(&getTagAllAtespacesFlag, "all-atespaces", "A", false, "List tags across all atespaces") + getTagsCmd.Flags().StringVarP(&getTagAtespaceFlag, "atespace", "a", "", "get tags in a specific atespace (mutually exclusive with --all-atespaces)") + getTagsCmd.Flags().BoolVarP(&getTagAllAtespacesFlag, "all-atespaces", "A", false, "get tags across all atespaces (mutually exclusive with --atespace)") getCmd.AddCommand(getTagsCmd) - createTagCmd.Flags().StringVarP(&createTagAtespaceFlag, "atespace", "a", "", "Atespace the actor lives in; the tag is created here (required)") - _ = createTagCmd.MarkFlagRequired("atespace") - createTagCmd.Flags().StringVar(&createTagActorFlag, "actor", "", "Name of the suspended actor whose external snapshot to tag (required)") - _ = createTagCmd.MarkFlagRequired("actor") - createTagCmd.Flags().StringVar(&createTagScopeFlag, "scope", "atespace", "Tag scope: atespace or published") + createTagCmd.Flags().StringVarP(&createTagAtespaceFlag, "atespace", "a", "", "the actor's atespace; the tag is created here") + createTagCmd.Flags().StringVar(&createTagActorFlag, "actor", "", "suspended actor whose external snapshot to tag") + createTagCmd.Flags().StringVar(&createTagScopeFlag, "scope", "atespace", "tag scope: atespace or published") createCmd.AddCommand(createTagCmd) - updateTagCmd.Flags().StringVarP(&updateTagAtespaceFlag, "atespace", "a", "", "Atespace owning the tag (required)") - updateTagCmd.Flags().StringVar(&updateTagScopeFlag, "scope", "", "Tag scope: atespace or published (required)") - _ = updateTagCmd.MarkFlagRequired("atespace") - _ = updateTagCmd.MarkFlagRequired("scope") + updateTagCmd.Flags().StringVarP(&updateTagAtespaceFlag, "atespace", "a", "", "the tag's atespace") + updateTagCmd.Flags().StringVar(&updateTagScopeFlag, "scope", "", "tag scope: atespace or published") updateCmd.AddCommand(updateTagCmd) - deleteTagCmd.Flags().StringVarP(&deleteTagAtespaceFlag, "atespace", "a", "", "Atespace owning the tag (required)") - _ = deleteTagCmd.MarkFlagRequired("atespace") + deleteTagCmd.Flags().StringVarP(&deleteTagAtespaceFlag, "atespace", "a", "", "the tag's atespace") deleteCmd.AddCommand(deleteTagCmd) } diff --git a/cmd/kubectl-ate/internal/cmd/validation.go b/cmd/kubectl-ate/internal/cmd/validation.go new file mode 100644 index 0000000000..bc3e0de639 --- /dev/null +++ b/cmd/kubectl-ate/internal/cmd/validation.go @@ -0,0 +1,252 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "errors" + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// usageError marks an error caused by invalid CLI invocation (bad or missing +// flags, arguments, or subcommands), as opposed to a runtime failure. +// executeRoot prints the error and the offending command's usage to stderr. +type usageError struct { + err error +} + +func (e *usageError) Error() string { return e.err.Error() } +func (e *usageError) Unwrap() error { return e.err } + +func usageErrorf(format string, args ...any) error { + return &usageError{err: fmt.Errorf(format, args...)} +} + +func wrapUsageError(err error) error { + if err == nil { + return nil + } + var u *usageError + if errors.As(err, &u) { + return err + } + return &usageError{err: err} +} + +func formatFlagName(f *pflag.Flag) string { + if f.Shorthand != "" { + return fmt.Sprintf("-%s/--%s", f.Shorthand, f.Name) + } + return fmt.Sprintf("--%s", f.Name) +} + +// handleFlagError translates pflag errors into user-friendly usage errors. +func handleFlagError(cmd *cobra.Command, err error) error { + // When a user invokes a group command with an unknown subcommand and + // additional flags (e.g. "kubectl ate logs my-actor -a space"), Find stops + // at the group command and ParseFlags trips on the flag. Report the + // unknown subcommand rather than the flag. + if isGroupCommand(cmd) { + if args := cmd.Flags().Args(); len(args) > 0 { + return unknownSubcommandError(cmd, args[0]) + } + } + + var valReq *pflag.ValueRequiredError + if errors.As(err, &valReq) { + if f := valReq.GetFlag(); f != nil { + return usageErrorf("flag %q requires a value", formatFlagName(f)) + } + } + + var notExist *pflag.NotExistError + if errors.As(err, ¬Exist) { + return usageErrorf("unknown flag %q", specifiedNotExistFlag(notExist)) + } + + return wrapUsageError(err) +} + +func specifiedNotExistFlag(e *pflag.NotExistError) string { + if e.GetSpecifiedShortnames() != "" { + return "-" + e.GetSpecifiedName() + } + return "--" + e.GetSpecifiedName() +} + +// checkSwallowedFlags detects when a string flag consumed the next flag token +// as its argument (for example, "kubectl ate create actor my-actor -a --template foo"). +func checkSwallowedFlags(cmd *cobra.Command) error { + var swallowedErr error + cmd.Flags().Visit(func(f *pflag.Flag) { + if swallowedErr != nil || f.Value.Type() != "string" { + return + } + val := f.Value.String() + if val == "-" && (f.Name == "filename" || f.Name == "token-file") { + return + } + if strings.HasPrefix(val, "-") { + swallowedErr = usageErrorf("flag %q requires a value, got flag %q", formatFlagName(f), val) + } + }) + return swallowedErr +} + +func missingRequiredFlags(cmd *cobra.Command, requiredFlags []string) []string { + var missing []string + for _, name := range requiredFlags { + f := cmd.Flags().Lookup(name) + if f == nil { + continue + } + if !f.Changed || strings.TrimSpace(f.Value.String()) == "" { + missing = append(missing, formatFlagName(f)) + } + } + return missing +} + +func formatMissingFlagsClause(missing []string) string { + if len(missing) == 1 { + return "required flag " + missing[0] + } + return "required flags " + strings.Join(missing, ", ") +} + +func joinQuoted(items []string) string { + quoted := make([]string, len(items)) + for i, s := range items { + quoted[i] = fmt.Sprintf("%q", s) + } + return strings.Join(quoted, ", ") +} + +// exactlyOneArg validates that the command received exactly one positional +// argument (named argPlaceholder, e.g. "") and that all +// requiredFlags were set, reporting everything missing in a single error. +func exactlyOneArg(argPlaceholder string, requiredFlags ...string) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if err := checkSwallowedFlags(cmd); err != nil { + return err + } + missingFlags := missingRequiredFlags(cmd, requiredFlags) + if len(args) == 0 { + if len(missingFlags) > 0 { + return usageErrorf("missing required argument %s and %s", argPlaceholder, formatMissingFlagsClause(missingFlags)) + } + return usageErrorf("missing required argument %s", argPlaceholder) + } + if len(args) > 1 { + return usageErrorf("expected 1 %s argument, received %d (%s)", argPlaceholder, len(args), joinQuoted(args)) + } + if len(missingFlags) > 0 { + return usageErrorf("missing %s", formatMissingFlagsClause(missingFlags)) + } + return nil + } +} + +// noArgs validates that the command received no positional arguments and that +// all requiredFlags were set. If extraArgHint is non-empty, it is included in +// the error when an unexpected positional argument is passed. +func noArgs(extraArgHint string, requiredFlags ...string) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if err := checkSwallowedFlags(cmd); err != nil { + return err + } + if len(args) > 0 { + if extraArgHint != "" { + return usageErrorf("unexpected argument %q (%s)", args[0], extraArgHint) + } + return usageErrorf("%q takes no positional arguments (received %s)", cmd.CommandPath(), joinQuoted(args)) + } + if missingFlags := missingRequiredFlags(cmd, requiredFlags); len(missingFlags) > 0 { + return usageErrorf("missing %s", formatMissingFlagsClause(missingFlags)) + } + return nil + } +} + +const isSubcommandAnnotation = "kubectl-ate/is-a-subcommand" + +func isGroupCommand(cmd *cobra.Command) bool { + return cmd.Annotations[isSubcommandAnnotation] != "" +} + +// requireSubcommand configures a group command (such as root, a verb, or admin) +// so that invoking it with no subcommand or an unknown subcommand returns a +// descriptive usageError instead of exiting 0. +func requireSubcommand(cmd *cobra.Command) { + if cmd.Annotations == nil { + cmd.Annotations = map[string]string{} + } + cmd.Annotations[isSubcommandAnnotation] = "true" + cmd.SuggestionsMinimumDistance = 1 + cmd.Args = func(c *cobra.Command, args []string) error { + if len(args) == 0 { + return usageErrorf("missing required subcommand") + } + return unknownSubcommandError(c, args[0]) + } + cmd.RunE = func(c *cobra.Command, args []string) error { + return usageErrorf("missing required subcommand") + } +} + +func unknownSubcommandError(cmd *cobra.Command, arg string) error { + if cmd.SuggestionsMinimumDistance <= 0 { + cmd.SuggestionsMinimumDistance = 1 + } + if suggestions := cmd.SuggestionsFor(arg); len(suggestions) > 0 { + return usageErrorf("unknown subcommand %q (did you mean %q?)", arg, suggestions[0]) + } + var available []*cobra.Command + for _, sub := range cmd.Commands() { + if sub.IsAvailableCommand() && sub.Name() != "help" && sub.Name() != "completion" { + available = append(available, sub) + } + } + if len(available) == 1 { + return usageErrorf("unknown subcommand %q (did you mean \"%s %s\"?)", + arg, available[0].Name(), arg) + } + return usageErrorf("unknown subcommand %q", arg) +} + +// validateAtespaceFlags checks the --atespace and -A/--all-atespaces flags +// for atespace-scoped "get" commands (actors, actor-templates, tags) before +// connecting to the control plane. +func validateAtespaceFlags(resourcePlural string, args []string, atespace string, allAtespaces bool) error { + if len(args) > 0 { + if allAtespaces { + return usageErrorf("-A/--all-atespaces cannot be used when getting %s by name", resourcePlural) + } + if atespace == "" { + return usageErrorf("-a/--atespace is required when getting %s by name", resourcePlural) + } + return nil + } + if allAtespaces && atespace != "" { + return usageErrorf("-a/--atespace and -A/--all-atespaces are mutually exclusive") + } + if !allAtespaces && atespace == "" { + return usageErrorf("-a/--atespace or -A/--all-atespaces is required") + } + return nil +} diff --git a/cmd/kubectl-ate/internal/cmd/validation_test.go b/cmd/kubectl-ate/internal/cmd/validation_test.go new file mode 100644 index 0000000000..ec8dcf66f9 --- /dev/null +++ b/cmd/kubectl-ate/internal/cmd/validation_test.go @@ -0,0 +1,508 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +func resetFlagsRecursive(c *cobra.Command) { + resetFlagSet := func(fs *pflag.FlagSet) { + fs.VisitAll(func(f *pflag.Flag) { + f.Changed = false + _ = f.Value.Set(f.DefValue) + }) + } + resetFlagSet(c.LocalFlags()) + resetFlagSet(c.PersistentFlags()) + for _, sub := range c.Commands() { + resetFlagsRecursive(sub) + } +} + +func runRootCLI(t *testing.T, args ...string) (stdout, stderr string, err error) { + t.Helper() + resetFlagsRecursive(rootCmd) + var outBuf, errBuf bytes.Buffer + rootCmd.SetOut(&outBuf) + rootCmd.SetErr(&errBuf) + rootCmd.SetArgs(args) + t.Cleanup(func() { + rootCmd.SetOut(nil) + rootCmd.SetErr(nil) + rootCmd.SetArgs(nil) + resetFlagsRecursive(rootCmd) + }) + err = executeRoot(rootCmd) + return outBuf.String(), errBuf.String(), err +} + +func TestHelpOutputGoesToStdout(t *testing.T) { + tests := []struct { + name string + args []string + wantStdoutSub string + }{ + { + name: "root --help", + args: []string{"--help"}, + wantStdoutSub: "Usage:\n kubectl-ate [command]", + }, + { + name: "verb --help", + args: []string{"get", "--help"}, + wantStdoutSub: "Usage:\n kubectl-ate get [command]", + }, + { + name: "leaf -h", + args: []string{"create", "actor", "-h"}, + wantStdoutSub: "Usage:\n kubectl-ate create actor [flags]", + }, + { + name: "help subcommand", + args: []string{"help", "create", "actor"}, + wantStdoutSub: "Usage:\n kubectl-ate create actor [flags]", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + stdout, stderr, err := runRootCLI(t, tc.args...) + if err != nil { + t.Fatalf("unexpected error: %v (stderr: %s)", err, stderr) + } + if stderr != "" { + t.Errorf("expected empty stderr on --help, got:\n%s", stderr) + } + if !strings.Contains(stdout, tc.wantStdoutSub) { + t.Errorf("stdout = %q, want it to contain %q", stdout, tc.wantStdoutSub) + } + }) + } +} + +func TestCLIUsageErrorsGoToStderrWithUsage(t *testing.T) { + tests := []struct { + name string + args []string + wantErrorLine string + wantUsageLine string + }{ + // Flag value / parsing errors + { + name: "get actor -a missing argument", + args: []string{"get", "actor", "-a"}, + wantErrorLine: `Error: flag "-a/--atespace" requires a value`, + wantUsageLine: "kubectl-ate get actor [actor-name ...] [flags]", + }, + { + name: "get actor --atespace missing argument", + args: []string{"get", "actor", "--atespace"}, + wantErrorLine: `Error: flag "-a/--atespace" requires a value`, + wantUsageLine: "kubectl-ate get actor [actor-name ...] [flags]", + }, + { + name: "create actor -a missing argument", + args: []string{"create", "actor", "-a"}, + wantErrorLine: `Error: flag "-a/--atespace" requires a value`, + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "create actor-template -f missing argument", + args: []string{"create", "actor-template", "-f"}, + wantErrorLine: `Error: flag "-f/--filename" requires a value`, + wantUsageLine: "kubectl-ate create actor-template -f [flags]", + }, + { + name: "global -o missing argument", + args: []string{"get", "atespaces", "-o"}, + wantErrorLine: `Error: flag "-o/--output" requires a value`, + wantUsageLine: "kubectl-ate get atespace [atespace-name ...] [flags]", + }, + { + name: "swallowed flag after -a", + args: []string{"create", "actor", "my-actor", "-a", "--template", "counter"}, + wantErrorLine: `Error: flag "-a/--atespace" requires a value, got flag "--template"`, + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "invalid output format", + args: []string{"get", "atespaces", "-o", "xml"}, + wantErrorLine: `Error: invalid output format "xml". Must be one of: table, json, yaml`, + wantUsageLine: "kubectl-ate get atespace [atespace-name ...] [flags]", + }, + { + name: "unknown short flag on leaf", + args: []string{"get", "atespaces", "-x"}, + wantErrorLine: `Error: unknown flag "-x"`, + wantUsageLine: "kubectl-ate get atespace [atespace-name ...] [flags]", + }, + { + name: "unknown long flag on leaf", + args: []string{"get", "atespaces", "--bogus"}, + wantErrorLine: `Error: unknown flag "--bogus"`, + wantUsageLine: "kubectl-ate get atespace [atespace-name ...] [flags]", + }, + + // Combined positional-arg and required-flag errors + { + name: "create actor with only -a reports missing arg and --template", + args: []string{"create", "actor", "-a", "my-atespace"}, + wantErrorLine: "Error: missing required argument and required flag --template", + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "create actor with nothing reports missing arg and both required flags", + args: []string{"create", "actor"}, + wantErrorLine: "Error: missing required argument and required flags -a/--atespace, --template", + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "create actor with name only reports both missing flags", + args: []string{"create", "actor", "my-actor"}, + wantErrorLine: "Error: missing required flags -a/--atespace, --template", + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "create actor with name and -a reports missing --template", + args: []string{"create", "actor", "my-actor", "-a", "my-atespace"}, + wantErrorLine: "Error: missing required flag --template", + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "create actor with both flags but no name reports missing ", + args: []string{"create", "actor", "-a", "my-atespace", "--template", "counter"}, + wantErrorLine: "Error: missing required argument ", + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "create actor with too many args", + args: []string{"create", "actor", "a1", "a2", "-a", "my-atespace", "--template", "counter"}, + wantErrorLine: `Error: expected 1 argument, received 2 ("a1", "a2")`, + wantUsageLine: "kubectl-ate create actor [flags]", + }, + { + name: "delete actor with nothing", + args: []string{"delete", "actor"}, + wantErrorLine: "Error: missing required argument and required flag -a/--atespace", + wantUsageLine: "kubectl-ate delete actor [flags]", + }, + { + name: "delete actor missing -a", + args: []string{"delete", "actor", "my-actor"}, + wantErrorLine: "Error: missing required flag -a/--atespace", + wantUsageLine: "kubectl-ate delete actor [flags]", + }, + { + name: "revert actor with nothing", + args: []string{"revert", "actor"}, + wantErrorLine: "Error: missing required argument and required flag -a/--atespace", + wantUsageLine: "kubectl-ate revert actor [flags]", + }, + { + name: "logs actor with nothing", + args: []string{"logs", "actor"}, + wantErrorLine: "Error: missing required argument and required flag -a/--atespace", + wantUsageLine: "kubectl-ate logs actor [flags]", + }, + { + name: "create actor-template missing -f", + args: []string{"create", "actor-template"}, + wantErrorLine: "Error: missing required flag -f/--filename", + wantUsageLine: "kubectl-ate create actor-template -f [flags]", + }, + { + name: "create actor-template unexpected positional arg", + args: []string{"create", "actor-template", "my-tpl", "-f", "tpl.yaml"}, + wantErrorLine: `Error: unexpected argument "my-tpl" (the template name comes from the manifest passed to -f/--filename)`, + wantUsageLine: "kubectl-ate create actor-template -f [flags]", + }, + { + name: "delete actor-template with nothing", + args: []string{"delete", "actor-template"}, + wantErrorLine: "Error: missing required argument and required flag -a/--atespace", + wantUsageLine: "kubectl-ate delete actor-template [flags]", + }, + { + name: "create atespace missing name", + args: []string{"create", "atespace"}, + wantErrorLine: "Error: missing required argument ", + wantUsageLine: "kubectl-ate create atespace [flags]", + }, + { + name: "get egress-policy missing all", + args: []string{"get", "egress-policy"}, + wantErrorLine: "Error: missing required argument and required flag -a/--atespace", + wantUsageLine: "kubectl-ate get egress-policy [flags]", + }, + { + name: "get egress-policy rejects multiple args", + args: []string{"get", "egress-policy", "c1", "c2", "-a", "team-a"}, + wantErrorLine: `Error: expected 1 argument, received 2 ("c1", "c2")`, + wantUsageLine: "kubectl-ate get egress-policy [flags]", + }, + { + name: "create egress-policy missing all", + args: []string{"create", "egress-policy"}, + wantErrorLine: "Error: missing required argument and required flags -a/--atespace, -f/--filename", + wantUsageLine: "kubectl-ate create egress-policy -f [flags]", + }, + { + name: "create egress-policy rejects multiple args", + args: []string{"create", "egress-policy", "c1", "c2", "-a", "team-a", "-f", "policy.yaml"}, + wantErrorLine: `Error: expected 1 argument, received 2 ("c1", "c2")`, + wantUsageLine: "kubectl-ate create egress-policy -f [flags]", + }, + { + name: "update egress-policy missing all", + args: []string{"update", "egress-policy"}, + wantErrorLine: "Error: missing required argument and required flags -a/--atespace, -f/--filename", + wantUsageLine: "kubectl-ate update egress-policy -f [flags]", + }, + { + name: "update egress-policy rejects multiple args", + args: []string{"update", "egress-policy", "c1", "c2", "-a", "team-a", "-f", "policy.yaml"}, + wantErrorLine: `Error: expected 1 argument, received 2 ("c1", "c2")`, + wantUsageLine: "kubectl-ate update egress-policy -f [flags]", + }, + { + name: "delete egress-policy missing all", + args: []string{"delete", "egress-policy"}, + wantErrorLine: "Error: missing required argument and required flag -a/--atespace", + wantUsageLine: "kubectl-ate delete egress-policy [flags]", + }, + { + name: "delete egress-policy rejects multiple args", + args: []string{"delete", "egress-policy", "c1", "c2", "-a", "team-a"}, + wantErrorLine: `Error: expected 1 argument, received 2 ("c1", "c2")`, + wantUsageLine: "kubectl-ate delete egress-policy [flags]", + }, + { + name: "create tag missing all", + args: []string{"create", "tag"}, + wantErrorLine: "Error: missing required argument and required flags -a/--atespace, --actor", + wantUsageLine: "kubectl-ate create tag [flags]", + }, + { + name: "update tag missing scope", + args: []string{"update", "tag", "t1", "-a", "space-1"}, + wantErrorLine: "Error: missing required flag --scope", + wantUsageLine: "kubectl-ate update tag [flags]", + }, + { + name: "top workers unexpected positional arg", + args: []string{"top", "workers", "w1"}, + wantErrorLine: `Error: "kubectl-ate top worker" takes no positional arguments (received "w1")`, + wantUsageLine: "kubectl-ate top worker [flags]", + }, + { + name: "admin make-ca-pool missing --name", + args: []string{"admin", "make-ca-pool"}, + wantErrorLine: "Error: missing required flag --name", + wantUsageLine: "kubectl-ate admin make-ca-pool [flags]", + }, + { + name: "admin make-ca-pool unexpected positional arg", + args: []string{"admin", "make-ca-pool", "my-secret", "--name", "my-secret"}, + wantErrorLine: `Error: unexpected argument "my-secret" (use --name to specify the secret name)`, + wantUsageLine: "kubectl-ate admin make-ca-pool [flags]", + }, + { + name: "admin activate-jwt-key missing both required flags", + args: []string{"admin", "activate-jwt-key"}, + wantErrorLine: "Error: missing required flags --name, --key-id", + wantUsageLine: "kubectl-ate admin activate-jwt-key [flags]", + }, + + // Pre-connection flag validation (runs without dialing any cluster) + { + name: "get actor without -a or -A", + args: []string{"get", "actor"}, + wantErrorLine: "Error: -a/--atespace or -A/--all-atespaces is required", + wantUsageLine: "kubectl-ate get actor [actor-name ...] [flags]", + }, + { + name: "get actor with both -a and -A", + args: []string{"get", "actor", "-a", "space-1", "-A"}, + wantErrorLine: "Error: -a/--atespace and -A/--all-atespaces are mutually exclusive", + wantUsageLine: "kubectl-ate get actor [actor-name ...] [flags]", + }, + { + name: "get actor by name without -a", + args: []string{"get", "actor", "my-actor"}, + wantErrorLine: "Error: -a/--atespace is required when getting actors by name", + wantUsageLine: "kubectl-ate get actor [actor-name ...] [flags]", + }, + { + name: "get actor by name with -A", + args: []string{"get", "actor", "my-actor", "-A"}, + wantErrorLine: "Error: -A/--all-atespaces cannot be used when getting actors by name", + wantUsageLine: "kubectl-ate get actor [actor-name ...] [flags]", + }, + { + name: "get actor-template without -a or -A", + args: []string{"get", "actor-template"}, + wantErrorLine: "Error: -a/--atespace or -A/--all-atespaces is required", + wantUsageLine: "kubectl-ate get actor-template [template-name ...] [flags]", + }, + { + name: "get tag without -a or -A", + args: []string{"get", "tag"}, + wantErrorLine: "Error: -a/--atespace or -A/--all-atespaces is required", + wantUsageLine: "kubectl-ate get tag [tag-name ...] [flags]", + }, + { + name: "get tag by name with -A", + args: []string{"get", "tag", "t1", "-A"}, + wantErrorLine: "Error: -A/--all-atespaces cannot be used when getting tags by name", + wantUsageLine: "kubectl-ate get tag [tag-name ...] [flags]", + }, + { + name: "get workers by name with filter flag", + args: []string{"get", "workers", "w1", "-n", "default"}, + wantErrorLine: "Error: -n/--namespace cannot be used when getting workers by name", + wantUsageLine: "kubectl-ate get worker [worker-name ...] [flags]", + }, + { + name: "get workers with invalid label selector", + args: []string{"get", "workers", "-l", "===bad"}, + wantErrorLine: `Error: invalid label selector "===bad"`, + wantUsageLine: "kubectl-ate get worker [worker-name ...] [flags]", + }, + { + name: "top workers with invalid label selector", + args: []string{"top", "workers", "-l", "===bad"}, + wantErrorLine: `Error: invalid label selector "===bad"`, + wantUsageLine: "kubectl-ate top worker [flags]", + }, + { + name: "logs actor with invalid --source", + args: []string{"logs", "actor", "my-actor", "-a", "space-1", "--source=bogus"}, + wantErrorLine: `Error: invalid --source "bogus"`, + wantUsageLine: "kubectl-ate logs actor [flags]", + }, + { + name: "create tag with invalid --scope", + args: []string{"create", "tag", "t1", "-a", "space-1", "--actor", "a1", "--scope=bogus"}, + wantErrorLine: `Error: invalid scope "bogus"; must be atespace or published`, + wantUsageLine: "kubectl-ate create tag [flags]", + }, + { + name: "delete egress-policy with invalid --version", + args: []string{"delete", "egress-policy", "a1", "-a", "space-1", "--version=0"}, + wantErrorLine: "Error: --version must be at least 1, got 0", + wantUsageLine: "kubectl-ate delete egress-policy [flags]", + }, + { + name: "admin make-ca-pool with invalid --key-type", + args: []string{"admin", "make-ca-pool", "--name", "s", "--key-type", "RSA"}, + wantErrorLine: `Error: unknown key type "RSA"`, + wantUsageLine: "kubectl-ate admin make-ca-pool [flags]", + }, + + // Bare verb and unknown subcommand errors + { + name: "bare root command", + args: []string{}, + wantErrorLine: "Error: missing required subcommand", + wantUsageLine: "kubectl-ate [command]", + }, + { + name: "unknown root subcommand", + args: []string{"bogus"}, + wantErrorLine: `Error: unknown subcommand "bogus"`, + wantUsageLine: "kubectl-ate [command]", + }, + { + name: "bare get command", + args: []string{"get"}, + wantErrorLine: "Error: missing required subcommand", + wantUsageLine: "kubectl-ate get [command]", + }, + { + name: "bare create command", + args: []string{"create"}, + wantErrorLine: "Error: missing required subcommand", + wantUsageLine: "kubectl-ate create [command]", + }, + { + name: "create plural actors suggests singular actor", + args: []string{"create", "actors", "my-actor", "-a", "space-1", "--template", "counter"}, + wantErrorLine: `Error: unknown subcommand "actors" (did you mean "actor"?)`, + wantUsageLine: "kubectl-ate create [command]", + }, + { + name: "logs without actor subcommand suggests logs actor ", + args: []string{"logs", "my-actor", "-a", "space-1"}, + wantErrorLine: `Error: unknown subcommand "my-actor" (did you mean "actor my-actor"?)`, + wantUsageLine: "kubectl-ate logs [command]", + }, + { + name: "logs with unknown subcommand and -h still reports unknown subcommand", + args: []string{"logs", "asdasd", "-h"}, + wantErrorLine: `Error: unknown subcommand "asdasd" (did you mean "actor asdasd"?)`, + wantUsageLine: "kubectl-ate logs [command]", + }, + { + name: "flag passed to verb without resource type", + args: []string{"get", "-a", "space-1"}, + wantErrorLine: `Error: unknown flag "-a"`, + wantUsageLine: "kubectl-ate get [command]", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + stdout, stderr, err := runRootCLI(t, tc.args...) + if err == nil { + t.Fatalf("expected error for args %v, got nil", tc.args) + } + if stdout != "" { + t.Errorf("expected empty stdout on error, got:\n%s", stdout) + } + firstLine, _, _ := strings.Cut(stderr, "\n") + if !strings.Contains(firstLine, tc.wantErrorLine) { + t.Errorf("first stderr line = %q, want it to contain %q\nfull stderr:\n%s", firstLine, tc.wantErrorLine, stderr) + } + if !strings.Contains(stderr, "\n\nUsage:\n") { + t.Errorf("stderr missing Usage section:\n%s", stderr) + } + if !strings.Contains(stderr, tc.wantUsageLine) { + t.Errorf("stderr = %q, want usage line %q", stderr, tc.wantUsageLine) + } + }) + } +} + +func TestRuntimeErrorsDoNotPrintUsage(t *testing.T) { + stdout, stderr, err := runRootCLI(t, "create", "actor-template", "-f", "/nonexistent/path/to/manifest.yaml") + if err == nil { + t.Fatal("expected error for nonexistent manifest file, got nil") + } + if stdout != "" { + t.Errorf("expected empty stdout, got:\n%s", stdout) + } + if !strings.HasPrefix(stderr, "Error: ") { + t.Errorf("expected stderr to start with 'Error: ', got:\n%s", stderr) + } + if strings.Contains(stderr, "Usage:") { + t.Errorf("expected runtime error not to print Usage, got:\n%s", stderr) + } +} diff --git a/cmd/kubectl-ate/internal/cmd/verbs.go b/cmd/kubectl-ate/internal/cmd/verbs.go index f98f86fbc6..87d332d442 100644 --- a/cmd/kubectl-ate/internal/cmd/verbs.go +++ b/cmd/kubectl-ate/internal/cmd/verbs.go @@ -33,5 +33,8 @@ var ( ) func init() { - rootCmd.AddCommand(getCmd, createCmd, updateCmd, deleteCmd, pauseCmd, resumeCmd, suspendCmd, logsCmd, topCmd) + for _, c := range []*cobra.Command{getCmd, createCmd, updateCmd, deleteCmd, pauseCmd, resumeCmd, suspendCmd, logsCmd, topCmd} { + requireSubcommand(c) + rootCmd.AddCommand(c) + } } diff --git a/cmd/kubectl-ate/internal/cmd/worker.go b/cmd/kubectl-ate/internal/cmd/worker.go index deb5cbc1b5..cd499985dc 100644 --- a/cmd/kubectl-ate/internal/cmd/worker.go +++ b/cmd/kubectl-ate/internal/cmd/worker.go @@ -44,9 +44,10 @@ var ( ) var getWorkersCmd = &cobra.Command{ - Use: "worker ", + Use: "worker [worker-name ...]", Aliases: []string{"workers"}, Short: "List all workers or get one or more workers", + Args: cobra.ArbitraryArgs, RunE: runGetWorkers, } @@ -54,7 +55,7 @@ var topWorkersCmd = &cobra.Command{ Use: "worker", Aliases: []string{"workers"}, Short: "Display resource (CPU/Memory) usage of workers", - Args: cobra.NoArgs, + Args: noArgs(""), RunE: runTopWorkers, } @@ -177,8 +178,8 @@ type GetWorkersRunner struct { func (r *GetWorkersRunner) Run(ctx context.Context) error { if len(r.names) > 0 { - if r.namespace != "" || r.atespace != "" || r.selector != "" || r.sandboxClass != "" { - return fmt.Errorf("filter flags (--namespace, --atespace, --selector, --sandbox-class) cannot be used when getting workers by name") + if err := validateWorkerFlags(r.names, r.namespace, r.atespace, r.selector, r.sandboxClass); err != nil { + return err } workers := make([]*ateapipb.Worker, 0, len(r.names)) for _, name := range r.names { @@ -205,7 +206,32 @@ func (r *GetWorkersRunner) Run(ctx context.Context) error { return printer.PrintWorkersTo(r.out, filtered, r.outputFmt) } +func validateWorkerFlags(names []string, namespace, atespace, selector, sandboxClass string) error { + if len(names) > 0 { + switch { + case namespace != "": + return usageErrorf("-n/--namespace cannot be used when getting workers by name") + case atespace != "": + return usageErrorf("-a/--atespace cannot be used when getting workers by name") + case selector != "": + return usageErrorf("-l/--selector cannot be used when getting workers by name") + case sandboxClass != "": + return usageErrorf("--sandbox-class cannot be used when getting workers by name") + } + } + if selector != "" { + if _, err := labels.Parse(selector); err != nil { + return usageErrorf("invalid label selector %q: %w", selector, err) + } + } + return nil +} + func runGetWorkers(cmd *cobra.Command, args []string) error { + if err := validateWorkerFlags(args, getWorkerNamespaceFlag, getWorkerAtespaceFlag, getWorkerSelectorFlag, getWorkerClassFlag); err != nil { + return err + } + apiClient, err := newAPIClient(cmd) if err != nil { return fmt.Errorf("failed to connect to ate-api-server: %w", err) @@ -350,6 +376,10 @@ func extractContainerUsage(pm metricsv1beta1.PodMetrics, stderr io.Writer) (stri } func runTopWorkers(cmd *cobra.Command, args []string) error { + if err := validateWorkerFlags(nil, topWorkerNamespaceFlag, topWorkerAtespaceFlag, topWorkerSelectorFlag, topWorkerClassFlag); err != nil { + return err + } + apiClient, err := newAPIClient(cmd) if err != nil { return fmt.Errorf("failed to connect to ate-api-server: %w", err) @@ -381,15 +411,15 @@ func runTopWorkers(cmd *cobra.Command, args []string) error { } func init() { - getWorkersCmd.Flags().StringVarP(&getWorkerNamespaceFlag, "namespace", "n", "", "Scope output to a specific Kubernetes namespace") - getWorkersCmd.Flags().StringVarP(&getWorkerAtespaceFlag, "atespace", "a", "", "Filter worker pods hosting actors in a specific atespace") - getWorkersCmd.Flags().StringVarP(&getWorkerSelectorFlag, "selector", "l", "", "Filter by worker pool labels") - getWorkersCmd.Flags().StringVar(&getWorkerClassFlag, "sandbox-class", "", "Filter by sandbox class (e.g. gvisor, microvm)") + getWorkersCmd.Flags().StringVarP(&getWorkerNamespaceFlag, "namespace", "n", "", "scope output to a specific Kubernetes namespace") + getWorkersCmd.Flags().StringVarP(&getWorkerAtespaceFlag, "atespace", "a", "", "filter worker pods hosting actors in a specific atespace") + getWorkersCmd.Flags().StringVarP(&getWorkerSelectorFlag, "selector", "l", "", "filter by worker pool labels") + getWorkersCmd.Flags().StringVar(&getWorkerClassFlag, "sandbox-class", "", "filter by sandbox class (e.g. gvisor, microvm)") getCmd.AddCommand(getWorkersCmd) - topWorkersCmd.Flags().StringVarP(&topWorkerNamespaceFlag, "namespace", "n", "", "Scope output to a specific Kubernetes namespace") - topWorkersCmd.Flags().StringVarP(&topWorkerAtespaceFlag, "atespace", "a", "", "Filter worker pods hosting actors in a specific atespace") - topWorkersCmd.Flags().StringVarP(&topWorkerSelectorFlag, "selector", "l", "", "Filter by worker pool labels") - topWorkersCmd.Flags().StringVar(&topWorkerClassFlag, "sandbox-class", "", "Filter by sandbox class (e.g. gvisor, microvm)") + topWorkersCmd.Flags().StringVarP(&topWorkerNamespaceFlag, "namespace", "n", "", "scope output to a specific Kubernetes namespace") + topWorkersCmd.Flags().StringVarP(&topWorkerAtespaceFlag, "atespace", "a", "", "filter worker pods hosting actors in a specific atespace") + topWorkersCmd.Flags().StringVarP(&topWorkerSelectorFlag, "selector", "l", "", "filter by worker pool labels") + topWorkersCmd.Flags().StringVar(&topWorkerClassFlag, "sandbox-class", "", "filter by sandbox class (e.g. gvisor, microvm)") topCmd.AddCommand(topWorkersCmd) }