From 91afef615ad918cfc364bf5e6d12a96785f2acaf Mon Sep 17 00:00:00 2001 From: PuneetPunamiya Date: Thu, 20 Oct 2022 12:48:15 +0530 Subject: [PATCH 1/3] Refactors explain command to split flags from options Signed-off-by: Puneet Punamiya ppunamiy@redhat.com Signed-off-by: Maciej Szulik --- .../k8s.io/kubectl/pkg/cmd/explain/explain.go | 125 +++++++++++------- .../kubectl/pkg/cmd/explain/explain_test.go | 14 +- 2 files changed, 86 insertions(+), 53 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go index 0932e031f24..a4bf0e19b74 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go @@ -56,7 +56,7 @@ 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`)) @@ -64,36 +64,70 @@ var ( plaintextOpenAPIV2TemplateName = "plaintext-openapiv2" ) -type ExplainOptions struct { - genericiooptions.IOStreams - - CmdParent string +type ExplainFlags struct { APIVersion string - Recursive bool - - args []string - - Mapper meta.RESTMapper - openAPIGetter openapi.OpenAPIResourcesGetter - - // Name of the template to use with the openapiv3 template renderer. + // Name of the template to use with the openapiv3 template renderer. If + // `EnableOpenAPIV3` is disabled, this does nothing OutputFormat string - - // Client capable of fetching openapi documents from the user's cluster - OpenAPIV3Client openapiclient.Client + Recursive bool + genericclioptions.IOStreams } -func NewExplainOptions(parent string, streams genericiooptions.IOStreams) *ExplainOptions { - return &ExplainOptions{ - IOStreams: streams, - CmdParent: parent, +// 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)") + + // Only enable --output as a valid flag if the feature is enabled + cmd.Flags().StringVar(&flags.OutputFormat, "output", plaintextTemplateName, "Format in which to render the schema (plaintext, plaintext-openapiv2)") +} + +// NewExplainFlags returns a default ExplainFlags +func NewExplainFlags(streams genericclioptions.IOStreams) *ExplainFlags { + return &ExplainFlags{ OutputFormat: plaintextTemplateName, + IOStreams: streams, } } +// 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 + } + + schema, err := f.OpenAPISchema() + if err != nil { + return nil, err + } + + // Only openapi v3 needs the discovery client. + openAPIV3Client, err := f.OpenAPIV3Client() + if err != nil { + return nil, err + } + + o := &ExplainOptions{ + CmdParent: parent, + Mapper: mapper, + Schema: schema, + args: args, + IOStreams: flags.IOStreams, + Recursive: flags.Recursive, + APIVersion: flags.APIVersion, + OutputFormat: plaintextTemplateName, + 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) +func NewCmdExplain(parent string, f cmdutil.Factory, streams genericclioptions.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,39 +136,18 @@ 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 - } - - // Only openapi v3 needs the discovery client. - o.OpenAPIV3Client, err = f.OpenAPIV3Client() - if err != nil { - return err - } - - // Lazy-load the OpenAPI V2 Resources, so they're not loaded when using OpenAPI V3. - o.openAPIGetter = f - o.args = args - return nil -} - func (o *ExplainOptions) Validate() error { if len(o.args) == 0 { return fmt.Errorf("You must specify the type of resource to explain. %s\n", cmdutil.SuggestAPIResources(o.CmdParent)) @@ -232,3 +245,23 @@ func (o *ExplainOptions) renderOpenAPIV2( return explain.PrintModelDescription(fieldsPath, o.Out, schema, gvk, o.Recursive) } + +type ExplainOptions struct { + genericclioptions.IOStreams + + CmdParent string + APIVersion string + Recursive bool + + args []string + + Mapper meta.RESTMapper + Schema openapi.Resources + + // Name of the template to use with the openapiv3 template renderer. If + // `EnableOpenAPIV3` is disabled, this does nothing + OutputFormat string + + // Client capable of fetching openapi documents from the user's cluster + OpenAPIV3Client openapiclient.Client +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go index 94c3ff8090b..9638fa82d30 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go +++ b/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(genericclioptions.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(genericclioptions.NewTestIOStreamsDiscard()) + + opts, err := flags.ToOptions(tf, "kubectl", []string{"foo"}) if err != nil { t.Fatalf("unexpected error %v", err) } From 3030b1dc6a445929c20dc911196746c2d8af2bac Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Thu, 23 Jan 2025 13:09:19 +0100 Subject: [PATCH 2/3] Finish extracting ExplainFlags structure Signed-off-by: Maciej Szulik --- .../k8s.io/kubectl/pkg/cmd/explain/explain.go | 95 +++++++++---------- .../kubectl/pkg/cmd/explain/explain_test.go | 4 +- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go index a4bf0e19b74..20847d0940b 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go @@ -59,37 +59,39 @@ var ( # Get the documentation of resources in different format kubectl explain deployment --output=plaintext-openapiv2`)) +) +const ( plaintextTemplateName = "plaintext" plaintextOpenAPIV2TemplateName = "plaintext-openapiv2" ) +// 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 - // Name of the template to use with the openapiv3 template renderer. If - // `EnableOpenAPIV3` is disabled, this does nothing + APIVersion string OutputFormat string Recursive bool - genericclioptions.IOStreams -} -// 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)") - - // Only enable --output as a valid flag if the feature is enabled - cmd.Flags().StringVar(&flags.OutputFormat, "output", plaintextTemplateName, "Format in which to render the schema (plaintext, plaintext-openapiv2)") + genericiooptions.IOStreams } // NewExplainFlags returns a default ExplainFlags -func NewExplainFlags(streams genericclioptions.IOStreams) *ExplainFlags { +func NewExplainFlags(streams genericiooptions.IOStreams) *ExplainFlags { return &ExplainFlags{ OutputFormat: plaintextTemplateName, IOStreams: streams, } } +// 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)") +} + // 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() @@ -97,11 +99,6 @@ func (flags *ExplainFlags) ToOptions(f cmdutil.Factory, parent string, args []st return nil, err } - schema, err := f.OpenAPISchema() - if err != nil { - return nil, err - } - // Only openapi v3 needs the discovery client. openAPIV3Client, err := f.OpenAPIV3Client() if err != nil { @@ -109,14 +106,18 @@ func (flags *ExplainFlags) ToOptions(f cmdutil.Factory, parent string, args []st } o := &ExplainOptions{ - CmdParent: parent, - Mapper: mapper, - Schema: schema, - args: args, - IOStreams: flags.IOStreams, - Recursive: flags.Recursive, - APIVersion: flags.APIVersion, - OutputFormat: plaintextTemplateName, + IOStreams: flags.IOStreams, + + Recursive: flags.Recursive, + APIVersion: flags.APIVersion, + OutputFormat: flags.OutputFormat, + + CmdParent: parent, + args: args, + + Mapper: mapper, + openAPIGetter: f, + OpenAPIV3Client: openAPIV3Client, } @@ -124,9 +125,7 @@ func (flags *ExplainFlags) ToOptions(f cmdutil.Factory, parent string, args []st } // NewCmdExplain returns a cobra command for swagger docs -func NewCmdExplain(parent string, f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command { - // o := NewExplainOptions(parent, streams) - +func NewCmdExplain(parent string, f cmdutil.Factory, streams genericiooptions.IOStreams) *cobra.Command { flags := NewExplainFlags(streams) cmd := &cobra.Command{ @@ -148,6 +147,24 @@ func NewCmdExplain(parent string, f cmdutil.Factory, streams genericclioptions.I return cmd } +type ExplainOptions struct { + genericiooptions.IOStreams + + Recursive bool + APIVersion string + // Name of the template to use with the openapiv3 template renderer. + OutputFormat string + + 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 { if len(o.args) == 0 { return fmt.Errorf("You must specify the type of resource to explain. %s\n", cmdutil.SuggestAPIResources(o.CmdParent)) @@ -245,23 +262,3 @@ func (o *ExplainOptions) renderOpenAPIV2( return explain.PrintModelDescription(fieldsPath, o.Out, schema, gvk, o.Recursive) } - -type ExplainOptions struct { - genericclioptions.IOStreams - - CmdParent string - APIVersion string - Recursive bool - - args []string - - Mapper meta.RESTMapper - Schema openapi.Resources - - // Name of the template to use with the openapiv3 template renderer. If - // `EnableOpenAPIV3` is disabled, this does nothing - OutputFormat string - - // Client capable of fetching openapi documents from the user's cluster - OpenAPIV3Client openapiclient.Client -} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go index 9638fa82d30..5d285cae2a7 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain_test.go @@ -57,7 +57,7 @@ func TestExplainInvalidArgs(t *testing.T) { tf := cmdtesting.NewTestFactory() defer tf.Cleanup() - flags := explain.NewExplainFlags(genericclioptions.NewTestIOStreamsDiscard()) + flags := explain.NewExplainFlags(genericiooptions.NewTestIOStreamsDiscard()) opts, err := flags.ToOptions(tf, "kubectl", []string{}) if err != nil { @@ -84,7 +84,7 @@ func TestExplainNotExistResource(t *testing.T) { tf := cmdtesting.NewTestFactory() defer tf.Cleanup() - flags := explain.NewExplainFlags(genericclioptions.NewTestIOStreamsDiscard()) + flags := explain.NewExplainFlags(genericiooptions.NewTestIOStreamsDiscard()) opts, err := flags.ToOptions(tf, "kubectl", []string{"foo"}) if err != nil { From 87139335b0e6ce5f5bc08d860a870fb9f16392b2 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Thu, 23 Jan 2025 13:10:06 +0100 Subject: [PATCH 3/3] Switch from using a function to just pure map in create token Signed-off-by: Maciej Szulik --- .../kubectl/pkg/cmd/create/create_token.go | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_token.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_token.go index ce8c749ea04..8c03588c7f2 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_token.go +++ b/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), }