Merge pull request #64390 from soltysh/builder_flags

Automatic merge from submit-queue (batch tested with PRs 64383, 64356, 64390). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Builder flags

Rebased version of https://github.com/kubernetes/kubernetes/pull/64277.

hack/import-restrictions.yaml

@@ -24,6 +24,7 @@
   # TODO this one should be tightened.  We depend on it for testing, but we should instead create our own scheme
   - k8s.io/api/core/v1
   - k8s.io/kubernetes/pkg/kubectl/genericclioptions/printers
+  - k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource
 
 - baseImportPath: "./vendor/k8s.io/apimachinery/"
   allowedImports:

pkg/kubectl/BUILD

@@ -82,7 +82,6 @@ go_library(
     srcs = [
         "apply.go",
         "autoscale.go",
-        "bash_comp_utils.go",
         "clusterrolebinding.go",
         "conditions.go",
         "configmap.go",
@@ -125,7 +124,6 @@ go_library(
         "//pkg/controller/deployment/util:go_default_library",
         "//pkg/credentialprovider:go_default_library",
         "//pkg/kubectl/apps:go_default_library",
-        "//pkg/kubectl/genericclioptions/resource:go_default_library",
         "//pkg/kubectl/util:go_default_library",
         "//pkg/kubectl/util/hash:go_default_library",
         "//pkg/kubectl/util/slice:go_default_library",

pkg/kubectl/bash_comp_utils.go

@@ -1,36 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors.
-
-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.
-*/
-
-// A set of common functions needed by cmd/kubectl and pkg/kubectl packages.
-
-package kubectl
-
-import (
-	"strings"
-
-	"github.com/spf13/cobra"
-
-	"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
-)
-
-func AddJsonFilenameFlag(cmd *cobra.Command, value *[]string, usage string) {
-	cmd.Flags().StringSliceVarP(value, "filename", "f", *value, usage)
-	annotations := make([]string, 0, len(resource.FileExtensions))
-	for _, ext := range resource.FileExtensions {
-		annotations = append(annotations, strings.TrimLeft(ext, "."))
-	}
-	cmd.Flags().SetAnnotation("filename", cobra.BashCompFilenameExt, annotations)
-}

pkg/kubectl/cmd/apply_set_last_applied.go

@@ -107,7 +107,7 @@ func NewCmdApplySetLastApplied(f cmdutil.Factory, ioStreams genericclioptions.IO
 
 	cmdutil.AddDryRunFlag(cmd)
 	cmd.Flags().BoolVar(&o.CreateAnnotation, "create-annotation", o.CreateAnnotation, "Will create 'last-applied-configuration' annotations if current objects doesn't have one")
-	kubectl.AddJsonFilenameFlag(cmd, &o.FilenameOptions.Filenames, "Filename, directory, or URL to files that contains the last-applied-configuration annotations")
+	cmdutil.AddJsonFilenameFlag(cmd.Flags(), &o.FilenameOptions.Filenames, "Filename, directory, or URL to files that contains the last-applied-configuration annotations")
 
 	return cmd
 }

pkg/kubectl/cmd/delete.go

@@ -275,7 +275,7 @@ func (o *DeleteOptions) DeleteResult(r *resource.Result) error {
 		effectiveTimeout = 168 * time.Hour
 	}
 	waitOptions := kubectlwait.WaitOptions{
-		ResourceFinder: kubectlwait.ResourceFinderForResult(o.Result),
+		ResourceFinder: genericclioptions.ResourceFinderForResult(o.Result),
 		DynamicClient:  o.DynamicClient,
 		Timeout:        effectiveTimeout,
 

pkg/kubectl/cmd/delete_flags.go

@@ -22,44 +22,13 @@ import (
 	"github.com/spf13/cobra"
 
 	"k8s.io/client-go/dynamic"
-	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
-	"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
 )
 
-type FileNameFlags struct {
-	Usage string
-
-	Filenames *[]string
-	Recursive *bool
-}
-
-func (o *FileNameFlags) ToOptions() resource.FilenameOptions {
-	options := resource.FilenameOptions{}
-
-	if o.Recursive != nil {
-		options.Recursive = *o.Recursive
-	}
-	if o.Filenames != nil {
-		options.Filenames = *o.Filenames
-	}
-
-	return options
-}
-
-func (o *FileNameFlags) AddFlags(cmd *cobra.Command) {
-	if o.Recursive != nil {
-		cmd.Flags().BoolVarP(o.Recursive, "recursive", "R", *o.Recursive, "Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests organized within the same directory.")
-	}
-	if o.Filenames != nil {
-		kubectl.AddJsonFilenameFlag(cmd, o.Filenames, "Filename, directory, or URL to files "+o.Usage)
-	}
-}
-
 // PrintFlags composes common printer flag structs
 // used for commands requiring deletion logic.
 type DeleteFlags struct {
-	FileNameFlags *FileNameFlags
+	FileNameFlags *genericclioptions.FileNameFlags
 	LabelSelector *string
 	FieldSelector *string
 
@@ -121,7 +90,7 @@ func (f *DeleteFlags) ToOptions(dynamicClient dynamic.Interface, streams generic
 }
 
 func (f *DeleteFlags) AddFlags(cmd *cobra.Command) {
-	f.FileNameFlags.AddFlags(cmd)
+	f.FileNameFlags.AddFlags(cmd.Flags())
 	if f.LabelSelector != nil {
 		cmd.Flags().StringVarP(f.LabelSelector, "selector", "l", *f.LabelSelector, "Selector (label query) to filter on, not including uninitialized ones.")
 	}
@@ -175,7 +144,7 @@ func NewDeleteCommandFlags(usage string) *DeleteFlags {
 	recursive := false
 
 	return &DeleteFlags{
-		FileNameFlags: &FileNameFlags{Usage: usage, Filenames: &filenames, Recursive: &recursive},
+		FileNameFlags: &genericclioptions.FileNameFlags{Usage: usage, Filenames: &filenames, Recursive: &recursive},
 		LabelSelector: &labelSelector,
 		FieldSelector: &fieldSelector,
 
@@ -203,7 +172,7 @@ func NewDeleteFlags(usage string) *DeleteFlags {
 	recursive := false
 
 	return &DeleteFlags{
-		FileNameFlags: &FileNameFlags{Usage: usage, Filenames: &filenames, Recursive: &recursive},
+		FileNameFlags: &genericclioptions.FileNameFlags{Usage: usage, Filenames: &filenames, Recursive: &recursive},
 
 		Cascade:     &cascade,
 		GracePeriod: &gracePeriod,

pkg/kubectl/cmd/rollingupdate.go

@@ -150,7 +150,7 @@ func NewCmdRollingUpdate(f cmdutil.Factory, ioStreams genericclioptions.IOStream
 	cmd.Flags().DurationVar(&o.Interval, "poll-interval", o.Interval, `Time delay between polling for replication controller status after the update. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`)
 	cmd.Flags().DurationVar(&o.Timeout, "timeout", o.Timeout, `Max time to wait for a replication controller to update before giving up. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`)
 	usage := "Filename or URL to file to use to create the new replication controller."
-	kubectl.AddJsonFilenameFlag(cmd, &o.FilenameOptions.Filenames, usage)
+	cmdutil.AddJsonFilenameFlag(cmd.Flags(), &o.FilenameOptions.Filenames, usage)
 	cmd.Flags().StringVar(&o.Image, "image", o.Image, i18n.T("Image to use for upgrading the replication controller. Must be distinct from the existing image (either new image or new image tag).  Can not be used with --filename/-f"))
 	cmd.Flags().StringVar(&o.DeploymentKey, "deployment-label-key", o.DeploymentKey, i18n.T("The key to use to differentiate between two different controllers, default 'deployment'.  Only relevant when --image is specified, ignored otherwise"))
 	cmd.Flags().StringVar(&o.Container, "container", o.Container, i18n.T("Container name which will have its image upgraded. Only relevant when --image is specified, ignored otherwise. Required when using --image on a multi-container pod"))

pkg/kubectl/cmd/util/helpers.go

@@ -29,6 +29,7 @@ import (
 	"github.com/evanphx/json-patch"
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -42,7 +43,6 @@ import (
 	"k8s.io/client-go/scale"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
-	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
 	"k8s.io/kubernetes/pkg/printers"
@@ -405,10 +405,19 @@ func AddValidateOptionFlags(cmd *cobra.Command, options *ValidateOptions) {
 }
 
 func AddFilenameOptionFlags(cmd *cobra.Command, options *resource.FilenameOptions, usage string) {
-	kubectl.AddJsonFilenameFlag(cmd, &options.Filenames, "Filename, directory, or URL to files "+usage)
+	AddJsonFilenameFlag(cmd.Flags(), &options.Filenames, "Filename, directory, or URL to files "+usage)
 	cmd.Flags().BoolVarP(&options.Recursive, "recursive", "R", options.Recursive, "Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests organized within the same directory.")
 }
 
+func AddJsonFilenameFlag(flags *pflag.FlagSet, value *[]string, usage string) {
+	flags.StringSliceVarP(value, "filename", "f", *value, usage)
+	annotations := make([]string, 0, len(resource.FileExtensions))
+	for _, ext := range resource.FileExtensions {
+		annotations = append(annotations, strings.TrimLeft(ext, "."))
+	}
+	flags.SetAnnotation("filename", cobra.BashCompFilenameExt, annotations)
+}
+
 // AddDryRunFlag adds dry-run flag to a command. Usually used by mutations.
 func AddDryRunFlag(cmd *cobra.Command) {
 	cmd.Flags().Bool("dry-run", false, "If true, only print the object that would be sent, without sending it.")

pkg/kubectl/cmd/wait/BUILD

@@ -2,11 +2,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = [
-        "fakeresourcefinder.go",
-        "flags.go",
-        "wait.go",
-    ],
+    srcs = ["wait.go"],
     importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/wait",
     visibility = ["//visibility:public"],
     deps = [
@@ -15,7 +11,6 @@ go_library(
         "//pkg/kubectl/genericclioptions/printers:go_default_library",
         "//pkg/kubectl/genericclioptions/resource:go_default_library",
         "//vendor/github.com/spf13/cobra:go_default_library",
-        "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",

pkg/kubectl/cmd/wait/wait.go

@@ -42,7 +42,7 @@ import (
 type WaitFlags struct {
 	RESTClientGetter     genericclioptions.RESTClientGetter
 	PrintFlags           *genericclioptions.PrintFlags
-	ResourceBuilderFlags *ResourceBuilderFlags
+	ResourceBuilderFlags *genericclioptions.ResourceBuilderFlags
 
 	Timeout      time.Duration
 	ForCondition string
@@ -55,7 +55,7 @@ func NewWaitFlags(restClientGetter genericclioptions.RESTClientGetter, streams g
 	return &WaitFlags{
 		RESTClientGetter:     restClientGetter,
 		PrintFlags:           genericclioptions.NewPrintFlags("condition met"),
-		ResourceBuilderFlags: NewResourceBuilderFlags(),
+		ResourceBuilderFlags: genericclioptions.NewResourceBuilderFlags(),
 
 		Timeout: 30 * time.Second,
 
@@ -151,7 +151,7 @@ func conditionFuncFor(condition string) (ConditionFunc, error) {
 // WaitOptions is a set of options that allows you to wait.  This is the object reflects the runtime needs of a wait
 // command, making the logic itself easy to unit test with our existing mocks.
 type WaitOptions struct {
-	ResourceFinder ResourceFinder
+	ResourceFinder genericclioptions.ResourceFinder
 	DynamicClient  dynamic.Interface
 	Timeout        time.Duration
 

pkg/kubectl/cmd/wait/wait_test.go

@@ -219,7 +219,7 @@ func TestWaitForDeletion(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			fakeClient := test.fakeClient()
 			o := &WaitOptions{
-				ResourceFinder: NewSimpleResourceFinder(test.info),
+				ResourceFinder: genericclioptions.NewSimpleResourceFinder(test.info),
 				DynamicClient:  fakeClient,
 				Timeout:        test.timeout,
 
@@ -451,7 +451,7 @@ func TestWaitForCondition(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			fakeClient := test.fakeClient()
 			o := &WaitOptions{
-				ResourceFinder: NewSimpleResourceFinder(test.info),
+				ResourceFinder: genericclioptions.NewSimpleResourceFinder(test.info),
 				DynamicClient:  fakeClient,
 				Timeout:        test.timeout,
 

pkg/kubectl/genericclioptions/BUILD

@@ -3,9 +3,12 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 go_library(
     name = "go_default_library",
     srcs = [
+        "builder_flags.go",
+        "builder_flags_fake.go",
         "config_flags.go",
         "config_flags_fake.go",
         "doc.go",
+        "filename_flags.go",
         "io_options.go",
         "json_yaml_flags.go",
         "name_flags.go",
@@ -16,6 +19,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/kubectl/genericclioptions/printers:go_default_library",
+        "//pkg/kubectl/genericclioptions/resource:go_default_library",
         "//vendor/github.com/evanphx/json-patch:go_default_library",
         "//vendor/github.com/spf13/cobra:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",

pkg/kubectl/genericclioptions/builder_flags.go

@@ -14,68 +14,82 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package wait
+package genericclioptions
 
 import (
-	"strings"
-
-	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
 )
 
 // ResourceBuilderFlags are flags for finding resources
+// TODO(juanvallejo): wire --local flag from commands through
 type ResourceBuilderFlags struct {
-	FilenameOptions resource.FilenameOptions
+	FileNameFlags *FileNameFlags
 
-	LabelSelector     string
-	FieldSelector     string
-	AllNamespaces     bool
-	Namespace         string
-	ExplicitNamespace bool
+	LabelSelector *string
+	FieldSelector *string
+	AllNamespaces *bool
 
-	// TODO add conditional support.  These are false for now.
-	All   bool
-	Local bool
+	All bool
 }
 
 // NewResourceBuilderFlags returns a default ResourceBuilderFlags
 func NewResourceBuilderFlags() *ResourceBuilderFlags {
+	filenames := []string{}
+
 	return &ResourceBuilderFlags{
-		FilenameOptions: resource.FilenameOptions{
-			Recursive: true,
+		FileNameFlags: &FileNameFlags{
+			Usage:     "identifying the resource.",
+			Filenames: &filenames,
+			Recursive: boolPtr(true),
 		},
+
+		LabelSelector: strPtr(""),
+		AllNamespaces: boolPtr(false),
 	}
 }
 
+func (o *ResourceBuilderFlags) WithFieldSelector(selector string) *ResourceBuilderFlags {
+	o.FieldSelector = &selector
+	return o
+}
+
 // AddFlags registers flags for finding resources
 func (o *ResourceBuilderFlags) AddFlags(flagset *pflag.FlagSet) {
-	flagset.StringSliceVarP(&o.FilenameOptions.Filenames, "filename", "f", o.FilenameOptions.Filenames, "Filename, directory, or URL to files identifying the resource.")
-	annotations := make([]string, 0, len(resource.FileExtensions))
-	for _, ext := range resource.FileExtensions {
-		annotations = append(annotations, strings.TrimLeft(ext, "."))
-	}
-	flagset.SetAnnotation("filename", cobra.BashCompFilenameExt, annotations)
-	flagset.BoolVar(&o.FilenameOptions.Recursive, "recursive", o.FilenameOptions.Recursive, "Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests organized within the same directory.")
+	o.FileNameFlags.AddFlags(flagset)
 
-	flagset.StringVarP(&o.LabelSelector, "selector", "l", o.LabelSelector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
-	flagset.StringVar(&o.FieldSelector, "field-selector", o.FieldSelector, "Selector (field query) to filter on, supports '=', '==', and '!='.(e.g. --field-selector key1=value1,key2=value2). The server only supports a limited number of field queries per type.")
-	flagset.BoolVar(&o.AllNamespaces, "all-namespaces", o.AllNamespaces, "If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.")
+	if o.LabelSelector != nil {
+		flagset.StringVarP(o.LabelSelector, "selector", "l", *o.LabelSelector, "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
+	}
+	if o.FieldSelector != nil {
+		flagset.StringVar(o.FieldSelector, "field-selector", *o.FieldSelector, "Selector (field query) to filter on, supports '=', '==', and '!='.(e.g. --field-selector key1=value1,key2=value2). The server only supports a limited number of field queries per type.")
+	}
+	if o.AllNamespaces != nil {
+		flagset.BoolVar(o.AllNamespaces, "all-namespaces", *o.AllNamespaces, "If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.")
+	}
 }
 
 // ToBuilder gives you back a resource finder to visit resources that are located
-func (o *ResourceBuilderFlags) ToBuilder(restClientGetter genericclioptions.RESTClientGetter, resources []string) ResourceFinder {
+func (o *ResourceBuilderFlags) ToBuilder(restClientGetter RESTClientGetter, resources []string) ResourceFinder {
 	namespace, enforceNamespace, namespaceErr := restClientGetter.ToRawKubeConfigLoader().Namespace()
 
+	builder := resource.NewBuilder(restClientGetter).
+		Unstructured().
+		NamespaceParam(namespace).DefaultNamespace().
+		ResourceTypeOrNameArgs(o.All, resources...)
+	if o.FileNameFlags != nil {
+		opts := o.FileNameFlags.ToOptions()
+		builder = builder.FilenameParam(enforceNamespace, &opts)
+	}
+	if o.LabelSelector != nil {
+		builder = builder.LabelSelectorParam(*o.LabelSelector)
+	}
+	if o.FieldSelector != nil {
+		builder = builder.FieldSelectorParam(*o.FieldSelector)
+	}
+
 	return &ResourceFindBuilderWrapper{
-		builder: resource.NewBuilder(restClientGetter).
-			Unstructured().
-			NamespaceParam(namespace).DefaultNamespace().
-			FilenameParam(enforceNamespace, &o.FilenameOptions).
-			LabelSelectorParam(o.LabelSelector).
-			FieldSelectorParam(o.FieldSelector).
-			ResourceTypeOrNameArgs(o.All, resources...).
+		builder: builder.
 			Latest().
 			Flatten().
 			AddError(namespaceErr),
@@ -112,3 +126,11 @@ func ResourceFinderForResult(result resource.Visitor) ResourceFinder {
 		return result
 	})
 }
+
+func strPtr(val string) *string {
+	return &val
+}
+
+func boolPtr(val bool) *bool {
+	return &val
+}

pkg/kubectl/genericclioptions/builder_flags_fake.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package wait
+package genericclioptions
 
 import (
 	"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"

pkg/kubectl/genericclioptions/filename_flags.go

@@ -0,0 +1,71 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+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 genericclioptions
+
+import (
+	"strings"
+
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+
+	"k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource"
+)
+
+// Usage of this struct by itself is discouraged.
+// These flags are composed by ResourceBuilderFlags
+// which should be used instead.
+type FileNameFlags struct {
+	Usage string
+
+	Filenames *[]string
+	Recursive *bool
+}
+
+func (o *FileNameFlags) ToOptions() resource.FilenameOptions {
+	options := resource.FilenameOptions{}
+
+	if o == nil {
+		return options
+	}
+
+	if o.Recursive != nil {
+		options.Recursive = *o.Recursive
+	}
+	if o.Filenames != nil {
+		options.Filenames = *o.Filenames
+	}
+
+	return options
+}
+
+func (o *FileNameFlags) AddFlags(flags *pflag.FlagSet) {
+	if o == nil {
+		return
+	}
+
+	if o.Recursive != nil {
+		flags.BoolVarP(o.Recursive, "recursive", "R", *o.Recursive, "Process the directory used in -f, --filename recursively. Useful when you want to manage related manifests organized within the same directory.")
+	}
+	if o.Filenames != nil {
+		flags.StringSliceVarP(o.Filenames, "filename", "f", *o.Filenames, o.Usage)
+		annotations := make([]string, 0, len(resource.FileExtensions))
+		for _, ext := range resource.FileExtensions {
+			annotations = append(annotations, strings.TrimLeft(ext, "."))
+		}
+		flags.SetAnnotation("filename", cobra.BashCompFilenameExt, annotations)
+	}
+}