Refactor Apply cmd to split flags from options (#102240)

* Refactor Apply cmd to split flags from options

* refactor code

* fix subcommands

staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go

@@ -46,15 +46,36 @@ import (
 	"k8s.io/kubectl/pkg/validation"
 )
 
+// ApplyFlags directly reflect the information that CLI is gathering via flags.  They will be converted to Options, which
+// reflect the runtime requirements for the command.  This structure reduces the transformation to wiring and makes
+// the logic itself easy to unit test
+type ApplyFlags struct {
+	Factory cmdutil.Factory
+
+	RecordFlags *genericclioptions.RecordFlags
+	PrintFlags  *genericclioptions.PrintFlags
+
+	DeleteFlags *delete.DeleteFlags
+
+	FieldManager   string
+	Selector       string
+	Prune          bool
+	PruneResources []pruneResource
+	All            bool
+	Overwrite      bool
+	OpenAPIPatch   bool
+	PruneWhitelist []string
+
+	genericclioptions.IOStreams
+}
+
 // ApplyOptions defines flags and other configuration parameters for the `apply` command
 type ApplyOptions struct {
-	RecordFlags *genericclioptions.RecordFlags
 	Recorder genericclioptions.Recorder
 
 	PrintFlags *genericclioptions.PrintFlags
 	ToPrinter  func(string) (printers.ResourcePrinter, error)
 
-	DeleteFlags   *delete.DeleteFlags
 	DeleteOptions *delete.DeleteOptions
 
 	ServerSideApply bool
@@ -137,9 +158,10 @@ var (
 	warningChangesOnDeletingResource     = "Warning: Detected changes to resource %[1]s which is currently being deleted.\n"
 )
 
-// NewApplyOptions creates new ApplyOptions for the `apply` command
+// NewApplyFlags returns a default ApplyFlags
-func NewApplyOptions(ioStreams genericclioptions.IOStreams) *ApplyOptions {
+func NewApplyFlags(f cmdutil.Factory, streams genericclioptions.IOStreams) *ApplyFlags {
-	return &ApplyOptions{
+	return &ApplyFlags{
+		Factory:     f,
 		RecordFlags: genericclioptions.NewRecordFlags(),
 		DeleteFlags: delete.NewDeleteFlags("that contains the configuration to apply"),
 		PrintFlags:  genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme),
@@ -147,25 +169,13 @@ func NewApplyOptions(ioStreams genericclioptions.IOStreams) *ApplyOptions {
 		Overwrite:    true,
 		OpenAPIPatch: true,
 
-		Recorder: genericclioptions.NoopRecorder{},
+		IOStreams: streams,
-
-		IOStreams: ioStreams,
-
-		objects:       []*resource.Info{},
-		objectsCached: false,
-
-		VisitedUids:       sets.NewString(),
-		VisitedNamespaces: sets.NewString(),
 	}
 }
 
 // NewCmdApply creates the `apply` command
 func NewCmdApply(baseName string, f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
-	o := NewApplyOptions(ioStreams)
+	flags := NewApplyFlags(f, ioStreams)
-
-	// Store baseName for use in printing warnings / messages involving the base command name.
-	// This is useful for downstream command that wrap this one.
-	o.cmdBaseName = baseName
 
 	cmd := &cobra.Command{
 		Use:                   "apply (-f FILENAME | -k DIRECTORY)",
@@ -174,52 +184,156 @@ func NewCmdApply(baseName string, f cmdutil.Factory, ioStreams genericclioptions
 		Long:                  applyLong,
 		Example:               applyExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(o.Complete(f, cmd))
+			o, err := flags.ToOptions(cmd, baseName, args)
-			cmdutil.CheckErr(validateArgs(cmd, args))
+			cmdutil.CheckErr(err)
-			cmdutil.CheckErr(validatePruneAll(o.Prune, o.All, o.Selector))
+			cmdutil.CheckErr(o.Validate(cmd, args))
 			cmdutil.CheckErr(o.Run())
 		},
 	}
 
-	// bind flag structs
+	flags.AddFlags(cmd)
-	o.DeleteFlags.AddFlags(cmd)
-	o.RecordFlags.AddFlags(cmd)
-	o.PrintFlags.AddFlags(cmd)
-
-	cmd.Flags().BoolVar(&o.Overwrite, "overwrite", o.Overwrite, "Automatically resolve conflicts between the modified and live configuration by using values from the modified configuration")
-	cmd.Flags().BoolVar(&o.Prune, "prune", o.Prune, "Automatically delete resource objects, that do not appear in the configs and are created by either apply or create --save-config. Should be used with either -l or --all.")
-	cmdutil.AddValidateFlags(cmd)
-	cmd.Flags().StringVarP(&o.Selector, "selector", "l", o.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
-	cmd.Flags().BoolVar(&o.All, "all", o.All, "Select all resources in the namespace of the specified resource types.")
-	cmd.Flags().StringArrayVar(&o.PruneWhitelist, "prune-whitelist", o.PruneWhitelist, "Overwrite the default whitelist with <group/version/kind> for --prune")
-	cmd.Flags().BoolVar(&o.OpenAPIPatch, "openapi-patch", o.OpenAPIPatch, "If true, use openapi to calculate diff when the openapi presents and the resource can be found in the openapi spec. Otherwise, fall back to use baked-in types.")
-	cmdutil.AddDryRunFlag(cmd)
-	cmdutil.AddServerSideApplyFlags(cmd)
-	cmdutil.AddFieldManagerFlagVar(cmd, &o.FieldManager, FieldManagerClientSideApply)
 
 	// apply subcommands
-	cmd.AddCommand(NewCmdApplyViewLastApplied(f, ioStreams))
+	cmd.AddCommand(NewCmdApplyViewLastApplied(flags.Factory, flags.IOStreams))
-	cmd.AddCommand(NewCmdApplySetLastApplied(f, ioStreams))
+	cmd.AddCommand(NewCmdApplySetLastApplied(flags.Factory, flags.IOStreams))
-	cmd.AddCommand(NewCmdApplyEditLastApplied(f, ioStreams))
+	cmd.AddCommand(NewCmdApplyEditLastApplied(flags.Factory, flags.IOStreams))
 
 	return cmd
 }
 
-// Complete verifies if ApplyOptions are valid and without conflicts.
+// AddFlags registers flags for a cli
-func (o *ApplyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
+func (flags *ApplyFlags) AddFlags(cmd *cobra.Command) {
-	var err error
+	// bind flag structs
-	o.ServerSideApply = cmdutil.GetServerSideApplyFlag(cmd)
+	flags.DeleteFlags.AddFlags(cmd)
-	o.ForceConflicts = cmdutil.GetForceConflictsFlag(cmd)
+	flags.RecordFlags.AddFlags(cmd)
-	o.DryRunStrategy, err = cmdutil.GetDryRunStrategy(cmd)
+	flags.PrintFlags.AddFlags(cmd)
+
+	cmdutil.AddValidateFlags(cmd)
+	cmdutil.AddDryRunFlag(cmd)
+	cmdutil.AddServerSideApplyFlags(cmd)
+	cmdutil.AddFieldManagerFlagVar(cmd, &flags.FieldManager, FieldManagerClientSideApply)
+
+	cmd.Flags().BoolVar(&flags.Overwrite, "overwrite", flags.Overwrite, "Automatically resolve conflicts between the modified and live configuration by using values from the modified configuration")
+	cmd.Flags().BoolVar(&flags.Prune, "prune", flags.Prune, "Automatically delete resource objects, that do not appear in the configs and are created by either apply or create --save-config. Should be used with either -l or --all.")
+	cmd.Flags().StringVarP(&flags.Selector, "selector", "l", flags.Selector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
+	cmd.Flags().BoolVar(&flags.All, "all", flags.All, "Select all resources in the namespace of the specified resource types.")
+	cmd.Flags().StringArrayVar(&flags.PruneWhitelist, "prune-whitelist", flags.PruneWhitelist, "Overwrite the default whitelist with <group/version/kind> for --prune")
+	cmd.Flags().BoolVar(&flags.OpenAPIPatch, "openapi-patch", flags.OpenAPIPatch, "If true, use openapi to calculate diff when the openapi presents and the resource can be found in the openapi spec. Otherwise, fall back to use baked-in types.")
+}
+
+// ToOptions converts from CLI inputs to runtime inputs
+func (flags *ApplyFlags) ToOptions(cmd *cobra.Command, baseName string, args []string) (*ApplyOptions, error) {
+	serverSideApply := cmdutil.GetServerSideApplyFlag(cmd)
+	forceConflicts := cmdutil.GetForceConflictsFlag(cmd)
+	dryRunStrategy, err := cmdutil.GetDryRunStrategy(cmd)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	o.DynamicClient, err = f.DynamicClient()
+
+	dynamicClient, err := flags.Factory.DynamicClient()
 	if err != nil {
-		return err
+		return nil, err
+	}
+
+	dryRunVerifier := resource.NewDryRunVerifier(dynamicClient, flags.Factory.OpenAPIGetter())
+	fieldManager := GetApplyFieldManagerFlag(cmd, serverSideApply)
+
+	// allow for a success message operation to be specified at print time
+	toPrinter := func(operation string) (printers.ResourcePrinter, error) {
+		flags.PrintFlags.NamePrintFlags.Operation = operation
+		cmdutil.PrintFlagsWithDryRunStrategy(flags.PrintFlags, dryRunStrategy)
+		return flags.PrintFlags.ToPrinter()
+	}
+
+	flags.RecordFlags.Complete(cmd)
+	recorder, err := flags.RecordFlags.ToRecorder()
+	if err != nil {
+		return nil, err
+	}
+
+	deleteOptions, err := flags.DeleteFlags.ToOptions(dynamicClient, flags.IOStreams)
+	if err != nil {
+		return nil, err
+	}
+
+	err = deleteOptions.FilenameOptions.RequireFilenameOrKustomize()
+	if err != nil {
+		return nil, err
+	}
+
+	openAPISchema, _ := flags.Factory.OpenAPISchema()
+	validator, err := flags.Factory.Validator(cmdutil.GetFlagBool(cmd, "validate"))
+	if err != nil {
+		return nil, err
+	}
+	builder := flags.Factory.NewBuilder()
+	mapper, err := flags.Factory.ToRESTMapper()
+	if err != nil {
+		return nil, err
+	}
+
+	namespace, enforceNamespace, err := flags.Factory.ToRawKubeConfigLoader().Namespace()
+	if err != nil {
+		return nil, err
+	}
+
+	if flags.Prune {
+		flags.PruneResources, err = parsePruneResources(mapper, flags.PruneWhitelist)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	o := &ApplyOptions{
+		// 	Store baseName for use in printing warnings / messages involving the base command name.
+		// 	This is useful for downstream command that wrap this one.
+		cmdBaseName: baseName,
+
+		PrintFlags: flags.PrintFlags,
+
+		DeleteOptions:   deleteOptions,
+		ToPrinter:       toPrinter,
+		ServerSideApply: serverSideApply,
+		ForceConflicts:  forceConflicts,
+		FieldManager:    fieldManager,
+		Selector:        flags.Selector,
+		DryRunStrategy:  dryRunStrategy,
+		DryRunVerifier:  dryRunVerifier,
+		Prune:           flags.Prune,
+		PruneResources:  flags.PruneResources,
+		All:             flags.All,
+		Overwrite:       flags.Overwrite,
+		OpenAPIPatch:    flags.OpenAPIPatch,
+		PruneWhitelist:  flags.PruneWhitelist,
+
+		Recorder:         recorder,
+		Namespace:        namespace,
+		EnforceNamespace: enforceNamespace,
+		Validator:        validator,
+		Builder:          builder,
+		Mapper:           mapper,
+		DynamicClient:    dynamicClient,
+		OpenAPISchema:    openAPISchema,
+
+		IOStreams: flags.IOStreams,
+
+		objects:       []*resource.Info{},
+		objectsCached: false,
+
+		VisitedUids:       sets.NewString(),
+		VisitedNamespaces: sets.NewString(),
+	}
+
+	o.PostProcessorFn = o.PrintAndPrunePostProcessor()
+
+	return o, nil
+}
+
+// Validate verifies if ApplyOptions are valid and without conflicts.
+func (o *ApplyOptions) Validate(cmd *cobra.Command, args []string) error {
+	if len(args) != 0 {
+		return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args)
 	}
-	o.DryRunVerifier = resource.NewDryRunVerifier(o.DynamicClient, f.OpenAPIGetter())
-	o.FieldManager = GetApplyFieldManagerFlag(cmd, o.ServerSideApply)
 
 	if o.ForceConflicts && !o.ServerSideApply {
 		return fmt.Errorf("--force-conflicts only works with --server-side")
@@ -229,29 +343,6 @@ func (o *ApplyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 		return fmt.Errorf("--dry-run=client doesn't work with --server-side (did you mean --dry-run=server instead?)")
 	}
 
-	// allow for a success message operation to be specified at print time
-	o.ToPrinter = func(operation string) (printers.ResourcePrinter, error) {
-		o.PrintFlags.NamePrintFlags.Operation = operation
-		cmdutil.PrintFlagsWithDryRunStrategy(o.PrintFlags, o.DryRunStrategy)
-		return o.PrintFlags.ToPrinter()
-	}
-
-	o.RecordFlags.Complete(cmd)
-	o.Recorder, err = o.RecordFlags.ToRecorder()
-	if err != nil {
-		return err
-	}
-
-	o.DeleteOptions, err = o.DeleteFlags.ToOptions(o.DynamicClient, o.IOStreams)
-	if err != nil {
-		return err
-	}
-
-	err = o.DeleteOptions.FilenameOptions.RequireFilenameOrKustomize()
-	if err != nil {
-		return err
-	}
-
 	if o.ServerSideApply && o.DeleteOptions.ForceDeletion {
 		return fmt.Errorf("--force cannot be used with --server-side")
 	}
@@ -260,48 +351,14 @@ func (o *ApplyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 		return fmt.Errorf("--dry-run=server cannot be used with --force")
 	}
 
-	o.OpenAPISchema, _ = f.OpenAPISchema()
+	if o.All && len(o.Selector) > 0 {
-	o.Validator, err = f.Validator(cmdutil.GetFlagBool(cmd, "validate"))
-	if err != nil {
-		return err
-	}
-	o.Builder = f.NewBuilder()
-	o.Mapper, err = f.ToRESTMapper()
-	if err != nil {
-		return err
-	}
-
-	o.Namespace, o.EnforceNamespace, err = f.ToRawKubeConfigLoader().Namespace()
-	if err != nil {
-		return err
-	}
-
-	if o.Prune {
-		o.PruneResources, err = parsePruneResources(o.Mapper, o.PruneWhitelist)
-		if err != nil {
-			return err
-		}
-	}
-
-	o.PostProcessorFn = o.PrintAndPrunePostProcessor()
-
-	return nil
-}
-
-func validateArgs(cmd *cobra.Command, args []string) error {
-	if len(args) != 0 {
-		return cmdutil.UsageErrorf(cmd, "Unexpected args: %v", args)
-	}
-	return nil
-}
-
-func validatePruneAll(prune, all bool, selector string) error {
-	if all && len(selector) > 0 {
 		return fmt.Errorf("cannot set --all and --selector at the same time")
 	}
-	if prune && !all && selector == "" {
+
+	if o.Prune && !o.All && o.Selector == "" {
 		return fmt.Errorf("all resources selected for prune without explicitly passing --all. To prune all resources, pass the --all flag. If you did not mean to prune all resources, specify a label selector")
 	}
+
 	return nil
 }