Merge pull request #32599 from mikedanese/kubectl-selector

Automatic merge from submit-queue

allow kubectl -f to filter by selector

cc @kubernetes/kubectl

Fixes #32544

hack/make-rules/test-cmd.sh

@@ -992,6 +992,21 @@ __EOF__
   # Clean up
   kubectl delete pods test-pod "${kube_flags[@]}"
 
+
+  ## kubectl apply -f with label selector should only apply matching objects
+  # Pre-Condition: no POD exists
+  kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''
+  # apply
+  kubectl apply -l unique-label=bingbang -f hack/testdata/filter "${kube_flags[@]}"
+  # check right pod exists
+  kube::test::get_object_assert 'pods selector-test-pod' "{{${labels_field}.name}}" 'selector-test-pod'
+  # check wrong pod doesn't exist
+  output_message=$(! kubectl get pods selector-test-pod-dont-apply 2>&1 "${kube_flags[@]}")
+  kube::test::if_has_string "${output_message}" 'pods "selector-test-pod-dont-apply" not found'
+  # cleanup
+  kubectl delete pods selector-test-pod
+
+
   ## kubectl run should create deployments or jobs
   # Pre-Condition: no Job exists
   kube::test::get_object_assert jobs "{{range.items}}{{$id_field}}:{{end}}" ''

hack/testdata/filter/pod-apply-selector.yaml

@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: selector-test-pod
+  labels:
+    name: selector-test-pod
+    unique-label: bingbang
+spec:
+  containers:
+  - name: kubernetes-pause
+    image: gcr.io/google-containers/pause:2.0

hack/testdata/filter/pod-dont-apply.yaml

@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: selector-test-pod-dont-apply
+  labels:
+    name: selector-test-pod-dont-apply
+    unique-label: biz
+spec:
+  containers:
+  - name: kubernetes-pause
+    image: gcr.io/google-containers/pause:2.0

pkg/kubectl/cmd/apply.go

@@ -35,6 +35,11 @@ import (
 	"k8s.io/kubernetes/pkg/util/strategicpatch"
 )
 
+type ApplyOptions struct {
+	FilenameOptions resource.FilenameOptions
+	Selector        string
+}
+
 const (
 	// maxPatchRetry is the maximum number of conflicts retry for during a patch operation before returning failure
 	maxPatchRetry = 5
@@ -61,7 +66,7 @@ var (
 )
 
 func NewCmdApply(f *cmdutil.Factory, out io.Writer) *cobra.Command {
-	options := &resource.FilenameOptions{}
+	var options ApplyOptions
 
 	cmd := &cobra.Command{
 		Use:     "apply -f FILENAME",
@@ -71,15 +76,16 @@ func NewCmdApply(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 		Run: func(cmd *cobra.Command, args []string) {
 			cmdutil.CheckErr(validateArgs(cmd, args))
 			cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd))
-			cmdutil.CheckErr(RunApply(f, cmd, out, options))
+			cmdutil.CheckErr(RunApply(f, cmd, out, &options))
 		},
 	}
 
 	usage := "that contains the configuration to apply"
-	cmdutil.AddFilenameOptionFlags(cmd, options, usage)
+	cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage)
 	cmd.MarkFlagRequired("filename")
 	cmd.Flags().Bool("overwrite", true, "Automatically resolve conflicts between the modified and live configuration by using values from the modified configuration")
 	cmdutil.AddValidateFlags(cmd)
+	cmd.Flags().StringVarP(&options.Selector, "selector", "l", "", "Selector (label query) to filter on")
 	cmdutil.AddOutputFlagsForMutation(cmd)
 	cmdutil.AddRecordFlag(cmd)
 	cmdutil.AddInclude3rdPartyFlags(cmd)
@@ -94,7 +100,7 @@ func validateArgs(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-func RunApply(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *resource.FilenameOptions) error {
+func RunApply(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *ApplyOptions) error {
 	shortOutput := cmdutil.GetFlagString(cmd, "output") == "name"
 	schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
 	if err != nil {
@@ -111,7 +117,8 @@ func RunApply(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *re
 		Schema(schema).
 		ContinueOnError().
 		NamespaceParam(cmdNamespace).DefaultNamespace().
-		FilenameParam(enforceNamespace, options).
+		FilenameParam(enforceNamespace, &options.FilenameOptions).
+		SelectorParam(options.Selector).
 		Flatten().
 		Do()
 	err = r.Err()

pkg/kubectl/resource/builder.go

@@ -538,6 +538,11 @@ func (b *Builder) visitorResult() *Result {
 		b.selector = labels.Everything()
 	}
 
+	// visit items specified by paths
+	if len(b.paths) != 0 {
+		return b.visitByPaths()
+	}
+
 	// visit selectors
 	if b.selector != nil {
 		return b.visitBySelector()
@@ -553,11 +558,6 @@ func (b *Builder) visitorResult() *Result {
 		return b.visitByName()
 	}
 
-	// visit items specified by paths
-	if len(b.paths) != 0 {
-		return b.visitByPaths()
-	}
-
 	if len(b.resources) != 0 {
 		return &Result{err: fmt.Errorf("resource(s) were provided, but no name, label selector, or --all flag specified")}
 	}
@@ -574,14 +574,6 @@ func (b *Builder) visitBySelector() *Result {
 	if len(b.resources) == 0 {
 		return &Result{err: fmt.Errorf("at least one resource must be specified to use a selector")}
 	}
-	// empty selector has different error message for paths being provided
-	if len(b.paths) != 0 {
-		if b.selector.Empty() {
-			return &Result{err: fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify a resource by arguments as well")}
-		} else {
-			return &Result{err: fmt.Errorf("a selector may not be specified when path, URL, or stdin is provided as input")}
-		}
-	}
 	mappings, err := b.resourceMappings()
 	if err != nil {
 		return &Result{err: err}
@@ -613,9 +605,6 @@ func (b *Builder) visitByResource() *Result {
 		isSingular = len(b.resourceTuples) == 1
 	}
 
-	if len(b.paths) != 0 {
-		return &Result{singular: isSingular, err: fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify a resource by arguments as well")}
-	}
 	if len(b.resources) != 0 {
 		return &Result{singular: isSingular, err: fmt.Errorf("you may not specify individual resources and bulk resources in the same call")}
 	}
@@ -718,6 +707,12 @@ func (b *Builder) visitByPaths() *Result {
 	if len(b.resources) != 0 {
 		return &Result{singular: singular, err: fmt.Errorf("when paths, URLs, or stdin is provided as input, you may not specify resource arguments as well")}
 	}
+	if len(b.names) != 0 {
+		return &Result{err: fmt.Errorf("name cannot be provided when a path is specified")}
+	}
+	if len(b.resourceTuples) != 0 {
+		return &Result{err: fmt.Errorf("resource/name arguments cannot be provided when a path is specified")}
+	}
 
 	var visitors Visitor
 	if b.continueOnError {
@@ -738,6 +733,9 @@ func (b *Builder) visitByPaths() *Result {
 		}
 		visitors = NewDecoratedVisitor(visitors, RetrieveLatest)
 	}
+	if b.selector != nil {
+		visitors = NewFilteredVisitor(visitors, FilterBySelector(b.selector))
+	}
 	return &Result{singular: singular, visitor: visitors, sources: b.paths}
 }
 

pkg/kubectl/resource/visitor.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/api/validation"
+	"k8s.io/kubernetes/pkg/labels"
 	"k8s.io/kubernetes/pkg/runtime"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/util/yaml"
@@ -639,3 +640,51 @@ func RetrieveLazy(info *Info, err error) error {
 	}
 	return nil
 }
+
+type FilterFunc func(info *Info, err error) (bool, error)
+
+type FilteredVisitor struct {
+	visitor Visitor
+	filters []FilterFunc
+}
+
+func NewFilteredVisitor(v Visitor, fn ...FilterFunc) Visitor {
+	if len(fn) == 0 {
+		return v
+	}
+	return FilteredVisitor{v, fn}
+}
+
+func (v FilteredVisitor) Visit(fn VisitorFunc) error {
+	return v.visitor.Visit(func(info *Info, err error) error {
+		if err != nil {
+			return err
+		}
+		for _, filter := range v.filters {
+			ok, err := filter(info, nil)
+			if err != nil {
+				return err
+			}
+			if !ok {
+				return nil
+			}
+		}
+		return fn(info, nil)
+	})
+}
+
+func FilterBySelector(s labels.Selector) FilterFunc {
+	return func(info *Info, err error) (bool, error) {
+		if err != nil {
+			return false, err
+		}
+		a, err := meta.Accessor(info.Object)
+		if err != nil {
+			return false, err
+		}
+		if !s.Matches(labels.Set(a.GetLabels())) {
+			return false, nil
+		}
+		return true, nil
+	}
+}