pass labelSelector to server side opaquely

pkg/kubectl/cmd/apply.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -356,16 +355,12 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out, errOut io.Writer, opti
 		return nil
 	}
 
-	selector, err := labels.Parse(options.Selector)
-	if err != nil {
-		return err
-	}
 	p := pruner{
 		mapper:        mapper,
 		clientFunc:    f.UnstructuredClientForMapping,
 		clientsetFunc: f.ClientSet,
 
-		selector:    selector,
+		selector:    options.Selector,
 		visitedUids: visitedUids,
 
 		cascade:     options.Cascade,
@@ -452,7 +447,7 @@ type pruner struct {
 	clientsetFunc func() (internalclientset.Interface, error)
 
 	visitedUids sets.String
-	selector    labels.Selector
+	selector    string
 
 	cascade     bool
 	dryRun      bool

pkg/kubectl/cmd/top_node.go

@@ -132,23 +132,14 @@ func (o *TopNodeOptions) Validate() error {
 	if len(o.ResourceName) > 0 && len(o.Selector) > 0 {
 		return errors.New("only one of NAME or --selector can be provided")
 	}
-	if len(o.Selector) > 0 {
-		_, err := labels.Parse(o.Selector)
-		if err != nil {
-			return err
-		}
-	}
 	return nil
 }
 
 func (o TopNodeOptions) RunTopNode() error {
 	var err error
-	selector := labels.Everything()
+	selector := labels.Everything().String()
 	if len(o.Selector) > 0 {
-		selector, err = labels.Parse(o.Selector)
+		selector = o.Selector
-		if err != nil {
-			return err
-		}
 	}
 	metrics, err := o.Client.GetNodeMetrics(o.ResourceName, selector)
 	if err != nil {
@@ -167,7 +158,7 @@ func (o TopNodeOptions) RunTopNode() error {
 		nodes = append(nodes, *node)
 	} else {
 		nodeList, err := o.NodeClient.Nodes().List(metav1.ListOptions{
-			LabelSelector: selector.String(),
+			LabelSelector: selector,
 		})
 		if err != nil {
 			return err

pkg/kubectl/cmd/util/factory.go

@@ -37,7 +37,6 @@ import (
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
@@ -296,8 +295,8 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) Factory {
 
 // GetFirstPod returns a pod matching the namespace and label selector
 // and the number of all pods that match the label selector.
-func GetFirstPod(client coreclient.PodsGetter, namespace string, selector labels.Selector, timeout time.Duration, sortBy func([]*v1.Pod) sort.Interface) (*api.Pod, int, error) {
+func GetFirstPod(client coreclient.PodsGetter, namespace string, selector string, timeout time.Duration, sortBy func([]*v1.Pod) sort.Interface) (*api.Pod, int, error) {
-	options := metav1.ListOptions{LabelSelector: selector.String()}
+	options := metav1.ListOptions{LabelSelector: selector}
 
 	podList, err := client.Pods(namespace).List(options)
 	if err != nil {

pkg/kubectl/cmd/util/factory_object_mapping.go

@@ -296,7 +296,7 @@ func (f *ring1Factory) LogsForObject(object, options runtime.Object, timeout tim
 	}
 
 	sortBy := func(pods []*v1.Pod) sort.Interface { return controller.ByLogging(pods) }
-	pod, numPods, err := GetFirstPod(clientset.Core(), namespace, selector, timeout, sortBy)
+	pod, numPods, err := GetFirstPod(clientset.Core(), namespace, selector.String(), timeout, sortBy)
 	if err != nil {
 		return nil, err
 	}
@@ -405,7 +405,7 @@ func (f *ring1Factory) AttachablePodForObject(object runtime.Object, timeout tim
 	}
 
 	sortBy := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }
-	pod, _, err := GetFirstPod(clientset.Core(), namespace, selector, timeout, sortBy)
+	pod, _, err := GetFirstPod(clientset.Core(), namespace, selector.String(), timeout, sortBy)
 	return pod, err
 }
 

pkg/kubectl/cmd/util/factory_test.go

@@ -606,7 +606,7 @@ func TestGetFirstPod(t *testing.T) {
 		}
 		selector := labels.Set(labelSet).AsSelector()
 
-		pod, numPods, err := GetFirstPod(fake.Core(), metav1.NamespaceDefault, selector, 1*time.Minute, test.sortBy)
+		pod, numPods, err := GetFirstPod(fake.Core(), metav1.NamespaceDefault, selector.String(), 1*time.Minute, test.sortBy)
 		pod.Spec.SecurityContext = nil
 		if !test.expectedErr && err != nil {
 			t.Errorf("%s: unexpected error: %v", test.name, err)

pkg/kubectl/metricsutil/metrics_client.go

@@ -97,8 +97,8 @@ func nodeMetricsUrl(name string) (string, error) {
 	return fmt.Sprintf("%s/nodes/%s", metricsRoot, name), nil
 }
 
-func (cli *HeapsterMetricsClient) GetNodeMetrics(nodeName string, selector labels.Selector) ([]metricsapi.NodeMetrics, error) {
+func (cli *HeapsterMetricsClient) GetNodeMetrics(nodeName string, selector string) ([]metricsapi.NodeMetrics, error) {
-	params := map[string]string{"labelSelector": selector.String()}
+	params := map[string]string{"labelSelector": selector}
 	path, err := nodeMetricsUrl(nodeName)
 	if err != nil {
 		return []metricsapi.NodeMetrics{}, err

pkg/kubectl/resource/builder.go

@@ -53,7 +53,7 @@ type Builder struct {
 	stream bool
 	dir    bool
 
-	selector             labels.Selector
+	selector             *string
 	selectAll            bool
 	includeUninitialized bool
 
@@ -292,14 +292,10 @@ func (b *Builder) ResourceNames(resource string, names ...string) *Builder {
 
 // SelectorParam defines a selector that should be applied to the object types to load.
 // This will not affect files loaded from disk or URL. If the parameter is empty it is
-// a no-op - to select all resources invoke `b.Selector(labels.Everything)`.
+// a no-op - to select all resources invoke `b.Selector(labels.Everything.String)`.
 func (b *Builder) SelectorParam(s string) *Builder {
-	selector, err := labels.Parse(s)
+	selector := strings.TrimSpace(s)
-	if err != nil {
+	if len(selector) == 0 {
-		b.errs = append(b.errs, fmt.Errorf("the provided selector %q is not valid: %v", s, err))
-		return b
-	}
-	if selector.Empty() {
 		return b
 	}
 	if b.selectAll {
@@ -310,8 +306,8 @@ func (b *Builder) SelectorParam(s string) *Builder {
 }
 
 // Selector accepts a selector directly, and if non nil will trigger a list action.
-func (b *Builder) Selector(selector labels.Selector) *Builder {
+func (b *Builder) Selector(selector string) *Builder {
-	b.selector = selector
+	b.selector = &selector
 	return b
 }
 
@@ -407,7 +403,8 @@ func (b *Builder) ResourceTypeOrNameArgs(allowEmptySelector bool, args ...string
 	case len(args) == 1:
 		b.ResourceTypes(SplitResourceArgument(args[0])...)
 		if b.selector == nil && allowEmptySelector {
-			b.selector = labels.Everything()
+			selector := labels.Everything().String()
+			b.selector = &selector
 		}
 	case len(args) == 0:
 	default:
@@ -595,7 +592,8 @@ func (b *Builder) visitorResult() *Result {
 	}
 
 	if b.selectAll {
-		b.selector = labels.Everything()
+		selector := labels.Everything().String()
+		b.selector = &selector
 	}
 
 	// visit items specified by paths
@@ -655,7 +653,7 @@ func (b *Builder) visitBySelector() *Result {
 		if mapping.Scope.Name() != meta.RESTScopeNameNamespace {
 			selectorNamespace = ""
 		}
-		visitors = append(visitors, NewSelector(client, mapping, selectorNamespace, b.selector, b.export, b.includeUninitialized))
+		visitors = append(visitors, NewSelector(client, mapping, selectorNamespace, *b.selector, b.export, b.includeUninitialized))
 	}
 	if b.continueOnError {
 		result.visitor = EagerVisitorList(visitors)
@@ -831,7 +829,11 @@ func (b *Builder) visitByPaths() *Result {
 		visitors = NewDecoratedVisitor(visitors, RetrieveLatest)
 	}
 	if b.selector != nil {
-		visitors = NewFilteredVisitor(visitors, FilterBySelector(b.selector))
+		selector, err := labels.Parse(*b.selector)
+		if err != nil {
+			return result.withError(fmt.Errorf("the provided selector %q is not valid: %v", b.selector, err))
+		}
+		visitors = NewFilteredVisitor(visitors, FilterBySelector(selector))
 	}
 	result.visitor = visitors
 	result.sources = b.paths
@@ -924,5 +926,5 @@ func MultipleTypesRequested(args []string) bool {
 		}
 		rKinds.Insert(rTuple.Resource)
 	}
-	return (rKinds.Len() > 1)
+	return rKinds.Len() > 1
 }

pkg/kubectl/resource/helper.go

@@ -22,7 +22,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/watch"
@@ -65,12 +64,12 @@ func (m *Helper) Get(namespace, name string, export bool) (runtime.Object, error
 }
 
 // TODO: add field selector
-func (m *Helper) List(namespace, apiVersion string, selector labels.Selector, export, includeUninitialized bool) (runtime.Object, error) {
+func (m *Helper) List(namespace, apiVersion string, selector string, export, includeUninitialized bool) (runtime.Object, error) {
 	req := m.RESTClient.Get().
 		NamespaceIfScoped(namespace, m.NamespaceScoped).
 		Resource(m.Resource).
 		VersionedParams(&metav1.ListOptions{
-			LabelSelector: selector.String(),
+			LabelSelector: selector,
 		}, metav1.ParameterCodec)
 	if export {
 		// TODO: I should be part of ListOptions
@@ -82,14 +81,14 @@ func (m *Helper) List(namespace, apiVersion string, selector labels.Selector, ex
 	return req.Do().Get()
 }
 
-func (m *Helper) Watch(namespace, resourceVersion, apiVersion string, labelSelector labels.Selector) (watch.Interface, error) {
+func (m *Helper) Watch(namespace, resourceVersion, apiVersion string, labelSelector string) (watch.Interface, error) {
 	return m.RESTClient.Get().
 		NamespaceIfScoped(namespace, m.NamespaceScoped).
 		Resource(m.Resource).
 		VersionedParams(&metav1.ListOptions{
 			ResourceVersion: resourceVersion,
 			Watch:           true,
-			LabelSelector:   labelSelector.String(),
+			LabelSelector:   labelSelector,
 		}, metav1.ParameterCodec).
 		Watch()
 }

pkg/kubectl/resource/helper_test.go

@@ -358,7 +358,7 @@ func TestHelperList(t *testing.T) {
 			RESTClient:      client,
 			NamespaceScoped: true,
 		}
-		obj, err := modifier.List("bar", api.Registry.GroupOrDie(api.GroupName).GroupVersion.String(), labels.SelectorFromSet(labels.Set{"foo": "baz"}), false, false)
+		obj, err := modifier.List("bar", api.Registry.GroupOrDie(api.GroupName).GroupVersion.String(), "foo=baz", false, false)
 		if (err != nil) != test.Err {
 			t.Errorf("unexpected error: %t %v", test.Err, err)
 		}

pkg/kubectl/resource/selector.go

@@ -21,7 +21,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/watch"
 )
 
@@ -30,13 +29,13 @@ type Selector struct {
 	Client               RESTClient
 	Mapping              *meta.RESTMapping
 	Namespace            string
-	Selector             labels.Selector
+	Selector             string
 	Export               bool
 	IncludeUninitialized bool
 }
 
 // NewSelector creates a resource selector which hides details of getting items by their label selector.
-func NewSelector(client RESTClient, mapping *meta.RESTMapping, namespace string, selector labels.Selector, export, includeUninitialized bool) *Selector {
+func NewSelector(client RESTClient, mapping *meta.RESTMapping, namespace string, selector string, export, includeUninitialized bool) *Selector {
 	return &Selector{
 		Client:               client,
 		Mapping:              mapping,
@@ -54,14 +53,14 @@ func (r *Selector) Visit(fn VisitorFunc) error {
 		if errors.IsBadRequest(err) || errors.IsNotFound(err) {
 			if se, ok := err.(*errors.StatusError); ok {
 				// modify the message without hiding this is an API error
-				if r.Selector.Empty() {
+				if len(r.Selector) == 0 {
 					se.ErrStatus.Message = fmt.Sprintf("Unable to list %q: %v", r.Mapping.Resource, se.ErrStatus.Message)
 				} else {
 					se.ErrStatus.Message = fmt.Sprintf("Unable to find %q that match the selector %q: %v", r.Mapping.Resource, r.Selector, se.ErrStatus.Message)
 				}
 				return se
 			}
-			if r.Selector.Empty() {
+			if len(r.Selector) == 0 {
 				return fmt.Errorf("Unable to list %q: %v", r.Mapping.Resource, err)
 			} else {
 				return fmt.Errorf("Unable to find %q that match the selector %q: %v", r.Mapping.Resource, r.Selector, err)

test/e2e/kubectl/kubectl.go

@@ -554,7 +554,7 @@ var _ = SIGDescribe("Kubectl client", func() {
 				ExecOrDie()
 			Expect(runOutput).ToNot(ContainSubstring("stdin closed"))
 			g := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }
-			runTestPod, _, err := util.GetFirstPod(f.InternalClientset.Core(), ns, labels.SelectorFromSet(map[string]string{"run": "run-test-3"}), 1*time.Minute, g)
+			runTestPod, _, err := util.GetFirstPod(f.InternalClientset.Core(), ns, "run=run-test-3", 1*time.Minute, g)
 			if err != nil {
 				os.Exit(1)
 			}