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) }