refactor(cli): restructure commands, option flags and validation

cmd/cli/root.go

@@ -10,6 +10,7 @@
 	"github.com/spf13/cobra"
 	cobracompletefig "github.com/withfig/autocomplete-tools/integrations/cobra"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"k8s.io/cli-runtime/pkg/genericiooptions"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
 	"github.com/akuity/kargo/internal/api"
@@ -86,7 +87,7 @@
 		},
 	}
 
-	opt.IOStreams = &genericclioptions.IOStreams{
+	opt.IOStreams = &genericiooptions.IOStreams{
 		In:     cmd.InOrStdin(),
 		Out:    os.Stdout,
 		ErrOut: os.Stderr,

go.mod

@@ -34,7 +34,6 @@ require (
 	github.com/withfig/autocomplete-tools/integrations/cobra v1.2.1
 	go.uber.org/ratelimit v0.3.0
 	golang.org/x/crypto v0.20.0
-	golang.org/x/exp v0.0.0-20230807204917-050eac23e9de
 	golang.org/x/net v0.21.0
 	golang.org/x/oauth2 v0.17.0
 	golang.org/x/sync v0.6.0
@@ -54,6 +53,7 @@ require (
 require (
 	github.com/distribution/reference v0.5.0 // indirect
 	github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
+	golang.org/x/exp v0.0.0-20230807204917-050eac23e9de // indirect
 	gopkg.in/evanphx/json-patch.v5 v5.6.0 // indirect
 	sigs.k8s.io/kustomize/api v0.16.0 // indirect
 	sigs.k8s.io/kustomize/kyaml v0.16.0 // indirect

internal/cli/cmd/apply/apply.go

@@ -1,6 +1,7 @@
 package apply
 
 import (
+	"context"
 	goerrors "errors"
 	"fmt"
 
@@ -20,104 +21,156 @@
 	kargosvcapi "github.com/akuity/kargo/pkg/api/service/v1alpha1"
 )
 
+type applyOptions struct {
+	*option.Option
+	Config config.CLIConfig
+
+	Filenames []string
+}
+
 func NewCommand(cfg config.CLIConfig, opt *option.Option) *cobra.Command {
-	var filenames []string
+	cmdOpts := &applyOptions{
+		Option: opt,
+		Config: cfg,
+	}
+
 	cmd := &cobra.Command{
 		Use:   "apply [--project=project] -f (FILENAME)",
 		Short: "Apply a resource from a file or from stdin",
 		Example: `
 # Apply a stage using the data in stage.yaml
 kargo apply -f stage.yaml
 `,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			ctx := cmd.Context()
-			if len(filenames) == 0 {
-				return errors.New("filename is required")
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			if err := cmdOpts.validate(); err != nil {
+				return err
 			}
 
-			rawManifest, err := yaml.Read(filenames)
-			if err != nil {
-				return errors.Wrap(err, "read manifests")
-			}
+			return cmdOpts.run(cmd.Context())
+		},
+	}
 
-			var printer printers.ResourcePrinter
-			if ptr.Deref(opt.PrintFlags.OutputFormat, "") != "" {
-				printer, err = opt.PrintFlags.ToPrinter()
-				if err != nil {
-					return errors.Wrap(err, "new printer")
-				}
-			}
+	// Register the option flags on the command.
+	cmdOpts.addFlags(cmd)
 
-			kargoSvcCli, err := client.GetClientFromConfig(ctx, cfg, opt)
-			if err != nil {
-				return errors.Wrap(err, "get client from config")
-			}
+	return cmd
+}
 
-			// TODO: Current implementation of apply is not the same as `kubectl` does.
-			// It actually "replaces" resource with the given file.
-			// We should provide the same implementation as `kubectl` does.
-			resp, err := kargoSvcCli.CreateOrUpdateResource(ctx,
-				connect.NewRequest(&kargosvcapi.CreateOrUpdateResourceRequest{
-					Manifest: rawManifest,
-				}))
-			if err != nil {
-				return errors.Wrap(err, "apply resource")
-			}
+// addFlags adds the flags for the apply options to the provided command.
+func (o *applyOptions) addFlags(cmd *cobra.Command) {
+	o.PrintFlags.AddFlags(cmd)
 
-			resCap := len(resp.Msg.GetResults())
-			createdRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest, 0, resCap)
-			updatedRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest, 0, resCap)
-			errs := make([]error, 0, resCap)
-			for _, r := range resp.Msg.GetResults() {
-				switch typedRes := r.GetResult().(type) {
-				case *kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest:
-					createdRes = append(createdRes, typedRes)
-				case *kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest:
-					updatedRes = append(updatedRes, typedRes)
-				case *kargosvcapi.CreateOrUpdateResourceResult_Error:
-					errs = append(errs, errors.New(typedRes.Error))
-				}
-			}
-			for _, r := range createdRes {
-				var obj unstructured.Unstructured
-				if err := sigyaml.Unmarshal(r.CreatedResourceManifest, &obj); err != nil {
-					fmt.Fprintf(opt.IOStreams.ErrOut, "%s",
-						errors.Wrap(err, "Error: unmarshal created manifest"))
-					continue
-				}
-				if printer == nil {
-					name := types.NamespacedName{
-						Namespace: obj.GetNamespace(),
-						Name:      obj.GetName(),
-					}.String()
-					fmt.Fprintf(opt.IOStreams.Out, "%s Created: %q\n", obj.GetKind(), name)
-					continue
-				}
-				_ = printer.PrintObj(&obj, opt.IOStreams.Out)
-			}
-			for _, r := range updatedRes {
-				var obj unstructured.Unstructured
-				if err := sigyaml.Unmarshal(r.UpdatedResourceManifest, &obj); err != nil {
-					fmt.Fprintf(opt.IOStreams.ErrOut, "%s",
-						errors.Wrap(err, "Error: unmarshal updated manifest"))
-					continue
-				}
-				if printer == nil {
-					name := types.NamespacedName{
-						Namespace: obj.GetNamespace(),
-						Name:      obj.GetName(),
-					}.String()
-					fmt.Fprintf(opt.IOStreams.Out, "%s Updated: %q\n", obj.GetKind(), name)
-					continue
-				}
-				_ = printer.PrintObj(&obj, opt.IOStreams.Out)
-			}
-			return goerrors.Join(errs...)
-		},
+	// TODO: Factor out server flags to a higher level (root?) as they are
+	//   common to almost all commands.
+	option.InsecureTLS(cmd.PersistentFlags(), o.Option)
+	option.LocalServer(cmd.PersistentFlags(), o.Option)
+
+	option.Filenames(cmd.Flags(), &o.Filenames, "Filename or directory to use to apply the resource(s)")
+
+	if err := cmd.MarkFlagRequired(option.FilenameFlag); err != nil {
+		panic(errors.Wrap(err, "could not mark filename flag as required"))
 	}
-	opt.PrintFlags.AddFlags(cmd)
-	option.Filenames(cmd.Flags(), &filenames, "apply")
-	option.InsecureTLS(cmd.PersistentFlags(), opt)
-	option.LocalServer(cmd.PersistentFlags(), opt)
-	return cmd
+	if err := cmd.MarkFlagFilename(option.FilenameFlag, ".yaml", ".yml"); err != nil {
+		panic(errors.Wrap(err, "could not mark filename flag as filename"))
+	}
+	if err := cmd.MarkFlagDirname(option.FilenameFlag); err != nil {
+		panic(errors.Wrap(err, "could not mark filename flag as dirname"))
+	}
+}
+
+// validate performs validation of the options. If the options are invalid, an
+// error is returned.
+func (o *applyOptions) validate() error {
+	// While the filename flag is marked as required, a user could still
+	// provide an empty string. This is a check to ensure that the flag is
+	// not empty.
+	if len(o.Filenames) == 0 {
+		return errors.New("filename is required")
+	}
+	return nil
+}
+
+// run performs the apply operation using the provided options.
+func (o *applyOptions) run(ctx context.Context) error {
+	rawManifest, err := yaml.Read(o.Filenames)
+	if err != nil {
+		return errors.Wrap(err, "read manifests")
+	}
+
+	var printer printers.ResourcePrinter
+	if ptr.Deref(o.PrintFlags.OutputFormat, "") != "" {
+		printer, err = o.PrintFlags.ToPrinter()
+		if err != nil {
+			return errors.Wrap(err, "new printer")
+		}
+	}
+
+	kargoSvcCli, err := client.GetClientFromConfig(ctx, o.Config, o.Option)
+	if err != nil {
+		return errors.Wrap(err, "get client from config")
+	}
+
+	// TODO: Current implementation of apply is not the same as `kubectl` does.
+	// It actually "replaces" resource with the given file.
+	// We should provide the same implementation as `kubectl` does.
+	resp, err := kargoSvcCli.CreateOrUpdateResource(ctx,
+		connect.NewRequest(&kargosvcapi.CreateOrUpdateResourceRequest{
+			Manifest: rawManifest,
+		}))
+	if err != nil {
+		return errors.Wrap(err, "apply resource")
+	}
+
+	resCap := len(resp.Msg.GetResults())
+	createdRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest, 0, resCap)
+	updatedRes := make([]*kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest, 0, resCap)
+	errs := make([]error, 0, resCap)
+	for _, r := range resp.Msg.GetResults() {
+		switch typedRes := r.GetResult().(type) {
+		case *kargosvcapi.CreateOrUpdateResourceResult_CreatedResourceManifest:
+			createdRes = append(createdRes, typedRes)
+		case *kargosvcapi.CreateOrUpdateResourceResult_UpdatedResourceManifest:
+			updatedRes = append(updatedRes, typedRes)
+		case *kargosvcapi.CreateOrUpdateResourceResult_Error:
+			errs = append(errs, errors.New(typedRes.Error))
+		}
+	}
+
+	for _, res := range createdRes {
+		var obj unstructured.Unstructured
+		if err := sigyaml.Unmarshal(res.CreatedResourceManifest, &obj); err != nil {
+			fmt.Fprintf(o.IOStreams.ErrOut, "%s",
+				errors.Wrap(err, "Error: unmarshal created manifest"))
+			continue
+		}
+		if printer == nil {
+			name := types.NamespacedName{
+				Namespace: obj.GetNamespace(),
+				Name:      obj.GetName(),
+			}.String()
+			fmt.Fprintf(o.IOStreams.Out, "%s Created: %q\n", obj.GetKind(), name)
+			continue
+		}
+		_ = printer.PrintObj(&obj, o.IOStreams.Out)
+	}
+
+	for _, res := range updatedRes {
+		var obj unstructured.Unstructured
+		if err := sigyaml.Unmarshal(res.UpdatedResourceManifest, &obj); err != nil {
+			fmt.Fprintf(o.IOStreams.ErrOut, "%s",
+				errors.Wrap(err, "Error: unmarshal updated manifest"))
+			continue
+		}
+		if printer == nil {
+			name := types.NamespacedName{
+				Namespace: obj.GetNamespace(),
+				Name:      obj.GetName(),
+			}.String()
+			fmt.Fprintf(o.IOStreams.Out, "%s Updated: %q\n", obj.GetKind(), name)
+			continue
+		}
+		_ = printer.PrintObj(&obj, o.IOStreams.Out)
+	}
+
+	return goerrors.Join(errs...)
 }