Merge pull request kubernetes#129784 from soltysh/refactor_explain

Refactors explain command to split flags from options

Author: k8s-ci-robot

staging/src/k8s.io/kubectl/pkg/cmd/create/create_token.go

@@ -98,14 +98,10 @@ var (
 `)
 )
 
-func boundObjectKindToAPIVersions() map[string]string {
-	kinds := map[string]string{
-		"Pod":    "v1",
-		"Secret": "v1",
-		"Node":   "v1",
-	}
-
-	return kinds
+var boundObjectKinds = map[string]string{
+	"Pod":    "v1",
+	"Secret": "v1",
+	"Node":   "v1",
 }
 
 func NewTokenOpts(ioStreams genericiooptions.IOStreams) *TokenOptions {
@@ -149,7 +145,7 @@ func NewCmdCreateToken(f cmdutil.Factory, ioStreams genericiooptions.IOStreams)
 	cmd.Flags().DurationVar(&o.Duration, "duration", o.Duration, "Requested lifetime of the issued token. If not set or if set to 0, the lifetime will be determined by the server automatically. The server may return a token with a longer or shorter lifetime.")
 
 	cmd.Flags().StringVar(&o.BoundObjectKind, "bound-object-kind", o.BoundObjectKind, "Kind of an object to bind the token to. "+
-		"Supported kinds are "+strings.Join(sets.List(sets.KeySet(boundObjectKindToAPIVersions())), ", ")+". "+
+		"Supported kinds are "+strings.Join(sets.List(sets.KeySet(boundObjectKinds)), ", ")+". "+
 		"If set, --bound-object-name must be provided.")
 	cmd.Flags().StringVar(&o.BoundObjectName, "bound-object-name", o.BoundObjectName, "Name of an object to bind the token to. "+
 		"The token will expire when the object is deleted. "+
@@ -226,8 +222,8 @@ func (o *TokenOptions) Validate() error {
 			return fmt.Errorf("--bound-object-uid can only be set if --bound-object-kind is provided")
 		}
 	} else {
-		if _, ok := boundObjectKindToAPIVersions()[o.BoundObjectKind]; !ok {
-			return fmt.Errorf("supported --bound-object-kind values are %s", strings.Join(sets.List(sets.KeySet(boundObjectKindToAPIVersions())), ", "))
+		if _, ok := boundObjectKinds[o.BoundObjectKind]; !ok {
+			return fmt.Errorf("supported --bound-object-kind values are %s", strings.Join(sets.List(sets.KeySet(boundObjectKinds)), ", "))
 		}
 		if len(o.BoundObjectName) == 0 {
 			return fmt.Errorf("--bound-object-name is required if --bound-object-kind is provided")
@@ -250,7 +246,7 @@ func (o *TokenOptions) Run() error {
 	if len(o.BoundObjectKind) > 0 {
 		request.Spec.BoundObjectRef = &authenticationv1.BoundObjectReference{
 			Kind:       o.BoundObjectKind,
-			APIVersion: boundObjectKindToAPIVersions()[o.BoundObjectKind],
+			APIVersion: boundObjectKinds[o.BoundObjectKind],
 			Name:       o.BoundObjectName,
 			UID:        types.UID(o.BoundObjectUID),
 		}

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

@@ -56,44 +56,77 @@ var (
 
 		# Get the documentation of a specific field of a resource
 		kubectl explain pods.spec.containers
-		
+
 		# Get the documentation of resources in different format
 		kubectl explain deployment --output=plaintext-openapiv2`))
+)
 
+const (
 	plaintextTemplateName          = "plaintext"
 	plaintextOpenAPIV2TemplateName = "plaintext-openapiv2"
 )
 
-type ExplainOptions struct {
+// ExplainFlags directly reflect the information that CLI is gathering via flags.
+// They will be converted to Options, which reflect the runtime requirements for
+// the command.
+type ExplainFlags struct {
+	APIVersion   string
+	OutputFormat string
+	Recursive    bool
+
 	genericiooptions.IOStreams
+}
 
-	CmdParent  string
-	APIVersion string
-	Recursive  bool
+// NewExplainFlags returns a default ExplainFlags
+func NewExplainFlags(streams genericiooptions.IOStreams) *ExplainFlags {
+	return &ExplainFlags{
+		OutputFormat: plaintextTemplateName,
+		IOStreams:    streams,
+	}
+}
 
-	args []string
+// AddFlags registers flags for a cli
+func (flags *ExplainFlags) AddFlags(cmd *cobra.Command) {
+	cmd.Flags().BoolVar(&flags.Recursive, "recursive", flags.Recursive, "Print the fields of fields (Currently only 1 level deep)")
+	cmd.Flags().StringVar(&flags.APIVersion, "api-version", flags.APIVersion, "Get different explanations for particular API version (API group/version)")
+	cmd.Flags().StringVar(&flags.OutputFormat, "output", plaintextTemplateName, "Format in which to render the schema (plaintext, plaintext-openapiv2)")
+}
 
-	Mapper        meta.RESTMapper
-	openAPIGetter openapi.OpenAPIResourcesGetter
+// ToOptions converts from CLI inputs to runtime input
+func (flags *ExplainFlags) ToOptions(f cmdutil.Factory, parent string, args []string) (*ExplainOptions, error) {
+	mapper, err := f.ToRESTMapper()
+	if err != nil {
+		return nil, err
+	}
 
-	// Name of the template to use with the openapiv3 template renderer.
-	OutputFormat string
+	// Only openapi v3 needs the discovery client.
+	openAPIV3Client, err := f.OpenAPIV3Client()
+	if err != nil {
+		return nil, err
+	}
 
-	// Client capable of fetching openapi documents from the user's cluster
-	OpenAPIV3Client openapiclient.Client
-}
+	o := &ExplainOptions{
+		IOStreams: flags.IOStreams,
 
-func NewExplainOptions(parent string, streams genericiooptions.IOStreams) *ExplainOptions {
-	return &ExplainOptions{
-		IOStreams:    streams,
-		CmdParent:    parent,
-		OutputFormat: plaintextTemplateName,
+		Recursive:    flags.Recursive,
+		APIVersion:   flags.APIVersion,
+		OutputFormat: flags.OutputFormat,
+
+		CmdParent: parent,
+		args:      args,
+
+		Mapper:        mapper,
+		openAPIGetter: f,
+
+		OpenAPIV3Client: openAPIV3Client,
 	}
+
+	return o, nil
 }
 
 // NewCmdExplain returns a cobra command for swagger docs
 func NewCmdExplain(parent string, f cmdutil.Factory, streams genericiooptions.IOStreams) *cobra.Command {
-	o := NewExplainOptions(parent, streams)
+	flags := NewExplainFlags(streams)
 
 	cmd := &cobra.Command{
 		Use:                   "explain TYPE [--recursive=FALSE|TRUE] [--api-version=api-version-group] [-o|--output=plaintext|plaintext-openapiv2]",
@@ -102,37 +135,34 @@ func NewCmdExplain(parent string, f cmdutil.Factory, streams genericiooptions.IO
 		Long:                  explainLong + "\n\n" + cmdutil.SuggestAPIResources(parent),
 		Example:               explainExamples,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(o.Complete(f, cmd, args))
+			o, err := flags.ToOptions(f, parent, args)
+			cmdutil.CheckErr(err)
 			cmdutil.CheckErr(o.Validate())
 			cmdutil.CheckErr(o.Run())
 		},
 	}
-	cmd.Flags().BoolVar(&o.Recursive, "recursive", o.Recursive, "When true, print the name of all the fields recursively. Otherwise, print the available fields with their description.")
-	cmd.Flags().StringVar(&o.APIVersion, "api-version", o.APIVersion, "Use given api-version (group/version) of the resource.")
 
-	// Only enable --output as a valid flag if the feature is enabled
-	cmd.Flags().StringVarP(&o.OutputFormat, "output", "o", plaintextTemplateName, "Format in which to render the schema. Valid values are: (plaintext, plaintext-openapiv2).")
+	flags.AddFlags(cmd)
 
 	return cmd
 }
 
-func (o *ExplainOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
-	var err error
-	o.Mapper, err = f.ToRESTMapper()
-	if err != nil {
-		return err
-	}
+type ExplainOptions struct {
+	genericiooptions.IOStreams
 
-	// Only openapi v3 needs the discovery client.
-	o.OpenAPIV3Client, err = f.OpenAPIV3Client()
-	if err != nil {
-		return err
-	}
+	Recursive  bool
+	APIVersion string
+	// Name of the template to use with the openapiv3 template renderer.
+	OutputFormat string
 
-	// Lazy-load the OpenAPI V2 Resources, so they're not loaded when using OpenAPI V3.
-	o.openAPIGetter = f
-	o.args = args
-	return nil
+	CmdParent string
+	args      []string
+
+	Mapper        meta.RESTMapper
+	openAPIGetter openapi.OpenAPIResourcesGetter
+
+	// Client capable of fetching openapi documents from the user's cluster
+	OpenAPIV3Client openapiclient.Client
 }
 
 func (o *ExplainOptions) Validate() error {

staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go

@@ -57,9 +57,9 @@ func TestExplainInvalidArgs(t *testing.T) {
 	tf := cmdtesting.NewTestFactory()
 	defer tf.Cleanup()
 
-	opts := explain.NewExplainOptions("kubectl", genericiooptions.NewTestIOStreamsDiscard())
-	cmd := explain.NewCmdExplain("kubectl", tf, genericiooptions.NewTestIOStreamsDiscard())
-	err := opts.Complete(tf, cmd, []string{})
+	flags := explain.NewExplainFlags(genericiooptions.NewTestIOStreamsDiscard())
+
+	opts, err := flags.ToOptions(tf, "kubectl", []string{})
 	if err != nil {
 		t.Fatalf("unexpected error %v", err)
 	}
@@ -69,7 +69,7 @@ func TestExplainInvalidArgs(t *testing.T) {
 		t.Error("unexpected non-error")
 	}
 
-	err = opts.Complete(tf, cmd, []string{"resource1", "resource2"})
+	opts, err = flags.ToOptions(tf, "kubectl", []string{"resource1", "resource2"})
 	if err != nil {
 		t.Fatalf("unexpected error %v", err)
 	}
@@ -84,9 +84,9 @@ func TestExplainNotExistResource(t *testing.T) {
 	tf := cmdtesting.NewTestFactory()
 	defer tf.Cleanup()
 
-	opts := explain.NewExplainOptions("kubectl", genericiooptions.NewTestIOStreamsDiscard())
-	cmd := explain.NewCmdExplain("kubectl", tf, genericiooptions.NewTestIOStreamsDiscard())
-	err := opts.Complete(tf, cmd, []string{"foo"})
+	flags := explain.NewExplainFlags(genericiooptions.NewTestIOStreamsDiscard())
+
+	opts, err := flags.ToOptions(tf, "kubectl", []string{"foo"})
 	if err != nil {
 		t.Fatalf("unexpected error %v", err)
 	}