Merge pull request kubernetes#5233 from brendandburns/labels

Make label and field selector query strings versionable.
duda · Mar 16, 2015 · 2f9a41b · 2f9a41b
2 parents 361c208 + 266234f
commit 2f9a41b
Show file tree

Hide file tree

Showing 22 changed files with 92 additions and 74 deletions.
diff --git a/pkg/api/unversioned.go b/pkg/api/unversioned.go
@@ -30,3 +30,22 @@ type APIVersions struct {
 type RootPaths struct {
 	Paths []string `json:"paths"`
 }
+
+// preV1Beta3 returns true if the provided API version is an API introduced before v1beta3.
+func PreV1Beta3(version string) bool {
+	return version == "v1beta1" || version == "v1beta2"
+}
+
+func LabelSelectorQueryParam(version string) string {
+	if PreV1Beta3(version) {
+		return "labels"
+	}
+	return "label-selector"
+}
+
+func FieldSelectorQueryParam(version string) string {
+	if PreV1Beta3(version) {
+		return "fields"
+	}
+	return "field-selector"
+}
diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go
@@ -84,7 +84,7 @@ func GetResource(r RESTGetter, ctxFn ContextFunc, namer ScopeNamer, codec runtim
 }
 
 func parseSelectorQueryParams(query url.Values, version, apiResource string) (label labels.Selector, field fields.Selector, err error) {
-	labelString := query.Get("labels")
+	labelString := query.Get(api.LabelSelectorQueryParam(version))
 	label, err = labels.Parse(labelString)
 	if err != nil {
 		return nil, nil, errors.NewBadRequest(fmt.Sprintf("The 'labels' selector parameter (%s) could not be parsed: %v", labelString, err))
@@ -93,7 +93,7 @@ func parseSelectorQueryParams(query url.Values, version, apiResource string) (la
 	convertToInternalVersionFunc := func(label, value string) (newLabel, newValue string, err error) {
 		return api.Scheme.ConvertFieldLabel(version, apiResource, label, value)
 	}
-	fieldString := query.Get("fields")
+	fieldString := query.Get(api.FieldSelectorQueryParam(version))
 	field, err = fields.ParseAndTransformSelector(fieldString, convertToInternalVersionFunc)
 	if err != nil {
 		return nil, nil, errors.NewBadRequest(fmt.Sprintf("The 'fields' selector parameter (%s) could not be parsed: %v", fieldString, err))

diff --git a/pkg/apiserver/watch_test.go b/pkg/apiserver/watch_test.go
@@ -188,13 +188,13 @@ func TestWatchParamParsing(t *testing.T) {
 			fieldSelector:   "",
 			namespace:       api.NamespaceAll,
 		}, {
-			rawQuery:        "namespace=default&resourceVersion=314159&fields=Host%3D&labels=name%3Dfoo",
+			rawQuery:        "namespace=default&resourceVersion=314159&" + api.FieldSelectorQueryParam(testVersion) + "=Host%3D&" + api.LabelSelectorQueryParam(testVersion) + "=name%3Dfoo",
 			resourceVersion: "314159",
 			labelSelector:   "name=foo",
 			fieldSelector:   "Host=",
 			namespace:       api.NamespaceDefault,
 		}, {
-			rawQuery:        "namespace=watchother&fields=id%3dfoo&resourceVersion=1492",
+			rawQuery:        "namespace=watchother&" + api.FieldSelectorQueryParam(testVersion) + "=id%3dfoo&resourceVersion=1492",
 			resourceVersion: "1492",
 			labelSelector:   "",
 			fieldSelector:   "id=foo",

diff --git a/pkg/client/cache/listwatch.go b/pkg/client/cache/listwatch.go
@@ -17,6 +17,7 @@ limitations under the License.
 package cache
 
 import (
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
@@ -38,12 +39,12 @@ type ListWatch struct {
 }
 
 // NewListWatchFromClient creates a new ListWatch from the specified client, resource, namespace and field selector.
-func NewListWatchFromClient(client *client.Client, resource string, namespace string, fieldSelector labels.Selector) *ListWatch {
+func NewListWatchFromClient(c *client.Client, resource string, namespace string, fieldSelector labels.Selector) *ListWatch {
 	listFunc := func() (runtime.Object, error) {
-		return client.Get().Namespace(namespace).Resource(resource).SelectorParam("fields", fieldSelector).Do().Get()
+		return c.Get().Namespace(namespace).Resource(resource).SelectorParam(api.FieldSelectorQueryParam(c.APIVersion()), fieldSelector).Do().Get()
 	}
 	watchFunc := func(resourceVersion string) (watch.Interface, error) {
-		return client.Get().Prefix("watch").Namespace(namespace).Resource(resource).SelectorParam("fields", fieldSelector).Param("resourceVersion", resourceVersion).Watch()
+		return c.Get().Prefix("watch").Namespace(namespace).Resource(resource).SelectorParam(api.FieldSelectorQueryParam(c.APIVersion()), fieldSelector).Param("resourceVersion", resourceVersion).Watch()
 	}
 	return &ListWatch{ListFunc: listFunc, WatchFunc: watchFunc}
 }

diff --git a/pkg/client/client.go b/pkg/client/client.go
@@ -149,8 +149,3 @@ func IsTimeout(err error) bool {
 	}
 	return false
 }
-
-// preV1Beta3 returns true if the provided API version is an API introduced before v1beta3.
-func preV1Beta3(version string) bool {
-	return version == "v1beta1" || version == "v1beta2"
-}
diff --git a/pkg/client/endpoints.go b/pkg/client/endpoints.go
@@ -60,7 +60,12 @@ func (c *endpoints) Create(endpoints *api.Endpoints) (*api.Endpoints, error) {
 // List takes a selector, and returns the list of endpoints that match that selector
 func (c *endpoints) List(selector labels.Selector) (result *api.EndpointsList, err error) {
 	result = &api.EndpointsList{}
-	err = c.r.Get().Namespace(c.ns).Resource("endpoints").SelectorParam("labels", selector).Do().Into(result)
+	err = c.r.Get().
+		Namespace(c.ns).
+		Resource("endpoints").
+		SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), selector).
+		Do().
+		Into(result)
 	return
 }
 
@@ -82,8 +87,8 @@ func (c *endpoints) Watch(label, field labels.Selector, resourceVersion string)
 		Namespace(c.ns).
 		Resource("endpoints").
 		Param("resourceVersion", resourceVersion).
-		SelectorParam("labels", label).
-		SelectorParam("fields", field).
+		SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), label).
+		SelectorParam(api.FieldSelectorQueryParam(c.r.APIVersion()), field).
 		Watch()
 }
 

diff --git a/pkg/client/events.go b/pkg/client/events.go
@@ -101,8 +101,8 @@ func (e *events) List(label, field labels.Selector) (*api.EventList, error) {
 	err := e.client.Get().
 		NamespaceIfScoped(e.namespace, len(e.namespace) > 0).
 		Resource("events").
-		SelectorParam("labels", label).
-		SelectorParam("fields", field).
+		SelectorParam(api.LabelSelectorQueryParam(e.client.APIVersion()), label).
+		SelectorParam(api.FieldSelectorQueryParam(e.client.APIVersion()), field).
 		Do().
 		Into(result)
 	return result, err
@@ -131,8 +131,8 @@ func (e *events) Watch(label, field labels.Selector, resourceVersion string) (wa
 		NamespaceIfScoped(e.namespace, len(e.namespace) > 0).
 		Resource("events").
 		Param("resourceVersion", resourceVersion).
-		SelectorParam("labels", label).
-		SelectorParam("fields", field).
+		SelectorParam(api.LabelSelectorQueryParam(e.client.APIVersion()), label).
+		SelectorParam(api.FieldSelectorQueryParam(e.client.APIVersion()), field).
 		Watch()
 }
 

diff --git a/pkg/client/fake.go b/pkg/client/fake.go
@@ -21,6 +21,7 @@ import (
 	"net/url"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/version"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
@@ -120,19 +121,19 @@ type FakeRESTClient struct {
 }
 
 func (c *FakeRESTClient) Get() *Request {
-	return NewRequest(c, "GET", &url.URL{Host: "localhost"}, c.Codec, c.Legacy, c.Legacy)
+	return NewRequest(c, "GET", &url.URL{Host: "localhost"}, testapi.Version(), c.Codec, c.Legacy, c.Legacy)
 }
 
 func (c *FakeRESTClient) Put() *Request {
-	return NewRequest(c, "PUT", &url.URL{Host: "localhost"}, c.Codec, c.Legacy, c.Legacy)
+	return NewRequest(c, "PUT", &url.URL{Host: "localhost"}, testapi.Version(), c.Codec, c.Legacy, c.Legacy)
 }
 
 func (c *FakeRESTClient) Post() *Request {
-	return NewRequest(c, "POST", &url.URL{Host: "localhost"}, c.Codec, c.Legacy, c.Legacy)
+	return NewRequest(c, "POST", &url.URL{Host: "localhost"}, testapi.Version(), c.Codec, c.Legacy, c.Legacy)
 }
 
 func (c *FakeRESTClient) Delete() *Request {
-	return NewRequest(c, "DELETE", &url.URL{Host: "localhost"}, c.Codec, c.Legacy, c.Legacy)
+	return NewRequest(c, "DELETE", &url.URL{Host: "localhost"}, testapi.Version(), c.Codec, c.Legacy, c.Legacy)
 }
 
 func (c *FakeRESTClient) Do(req *http.Request) (*http.Response, error) {

diff --git a/pkg/client/limit_ranges.go b/pkg/client/limit_ranges.go
@@ -57,7 +57,7 @@ func newLimitRanges(c *Client, namespace string) *limitRanges {
 // List takes a selector, and returns the list of limitRanges that match that selector.
 func (c *limitRanges) List(selector labels.Selector) (result *api.LimitRangeList, err error) {
 	result = &api.LimitRangeList{}
-	err = c.r.Get().Namespace(c.ns).Resource("limitRanges").SelectorParam("labels", selector).Do().Into(result)
+	err = c.r.Get().Namespace(c.ns).Resource("limitRanges").SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), selector).Do().Into(result)
 	return
 }
 
@@ -102,7 +102,7 @@ func (c *limitRanges) Watch(label, field labels.Selector, resourceVersion string
 		Namespace(c.ns).
 		Resource("limitRanges").
 		Param("resourceVersion", resourceVersion).
-		SelectorParam("labels", label).
-		SelectorParam("fields", field).
+		SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), label).
+		SelectorParam(api.FieldSelectorQueryParam(c.r.APIVersion()), field).
 		Watch()
 }
diff --git a/pkg/client/minions.go b/pkg/client/minions.go
@@ -48,7 +48,7 @@ func newNodes(c *Client) *nodes {
 
 // resourceName returns node's URL resource name based on resource version.
 func (c *nodes) resourceName() string {
-	if preV1Beta3(c.r.APIVersion()) {
+	if api.PreV1Beta3(c.r.APIVersion()) {
 		return "minions"
 	}
 	return "nodes"

diff --git a/pkg/client/namespaces.go b/pkg/client/namespaces.go
@@ -58,7 +58,7 @@ func (c *namespaces) Create(namespace *api.Namespace) (*api.Namespace, error) {
 // List lists all the namespaces in the cluster.
 func (c *namespaces) List(selector labels.Selector) (*api.NamespaceList, error) {
 	result := &api.NamespaceList{}
-	err := c.r.Get().Resource("namespaces").SelectorParam("labels", selector).Do().Into(result)
+	err := c.r.Get().Resource("namespaces").SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), selector).Do().Into(result)
 	return result, err
 }
 
@@ -95,7 +95,7 @@ func (c *namespaces) Watch(label, field labels.Selector, resourceVersion string)
 		Prefix("watch").
 		Resource("namespaces").
 		Param("resourceVersion", resourceVersion).
-		SelectorParam("labels", label).
-		SelectorParam("fields", field).
+		SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), label).
+		SelectorParam(api.FieldSelectorQueryParam(c.r.APIVersion()), field).
 		Watch()
 }
diff --git a/pkg/client/pods.go b/pkg/client/pods.go
@@ -58,7 +58,7 @@ func newPods(c *Client, namespace string) *pods {
 // List takes a selector, and returns the list of pods that match that selector.
 func (c *pods) List(selector labels.Selector) (result *api.PodList, err error) {
 	result = &api.PodList{}
-	err = c.r.Get().Namespace(c.ns).Resource("pods").SelectorParam("labels", selector).Do().Into(result)
+	err = c.r.Get().Namespace(c.ns).Resource("pods").SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), selector).Do().Into(result)
 	return
 }
 

diff --git a/pkg/client/replication_controllers.go b/pkg/client/replication_controllers.go
@@ -54,7 +54,7 @@ func newReplicationControllers(c *Client, namespace string) *replicationControll
 // List takes a selector, and returns the list of replication controllers that match that selector.
 func (c *replicationControllers) List(selector labels.Selector) (result *api.ReplicationControllerList, err error) {
 	result = &api.ReplicationControllerList{}
-	err = c.r.Get().Namespace(c.ns).Resource("replicationControllers").SelectorParam("labels", selector).Do().Into(result)
+	err = c.r.Get().Namespace(c.ns).Resource("replicationControllers").SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), selector).Do().Into(result)
 	return
 }
 
@@ -99,7 +99,7 @@ func (c *replicationControllers) Watch(label, field labels.Selector, resourceVer
 		Namespace(c.ns).
 		Resource("replicationControllers").
 		Param("resourceVersion", resourceVersion).
-		SelectorParam("labels", label).
-		SelectorParam("fields", field).
+		SelectorParam(api.LabelSelectorQueryParam(c.r.APIVersion()), label).
+		SelectorParam(api.FieldSelectorQueryParam(c.r.APIVersion()), field).
 		Watch()
 }
diff --git a/pkg/client/request.go b/pkg/client/request.go
@@ -110,6 +110,8 @@ type Request struct {
 	selector     labels.Selector
 	timeout      time.Duration
 
+	apiVersion string
+
 	// output
 	err  error
 	body io.Reader
@@ -120,7 +122,7 @@ type Request struct {
 }
 
 // NewRequest creates a new request helper object for accessing runtime.Objects on a server.
-func NewRequest(client HTTPClient, verb string, baseURL *url.URL,
+func NewRequest(client HTTPClient, verb string, baseURL *url.URL, apiVersion string,
 	codec runtime.Codec, namespaceInQuery bool, preserveResourceCase bool) *Request {
 	return &Request{
 		client:  client,

diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go
@@ -50,12 +50,11 @@ func TestRequestWithErrorWontChange(t *testing.T) {
 	original := Request{err: errors.New("test")}
 	r := original
 	changed := r.Param("foo", "bar").
-		SelectorParam("labels", labels.Set{"a": "b"}.AsSelector()).
+		SelectorParam(api.LabelSelectorQueryParam(testapi.Version()), labels.Set{"a": "b"}.AsSelector()).
 		UintParam("uint", 1).
 		AbsPath("/abs").
 		Prefix("test").
 		Suffix("testing").
-		ParseSelectorParam("foo", "a=b").
 		Namespace("new").
 		Resource("foos").
 		Name("bars").
@@ -154,13 +153,6 @@ func TestRequestSetTwiceError(t *testing.T) {
 	}
 }
 
-func TestRequestParseSelectorParam(t *testing.T) {
-	r := (&Request{}).ParseSelectorParam("foo", "a=")
-	if r.err == nil || r.params != nil {
-		t.Errorf("should have set err and left params nil: %#v", r)
-	}
-}
-
 func TestRequestParam(t *testing.T) {
 	r := (&Request{}).Param("foo", "a")
 	if !api.Semantic.DeepDerivative(r.params, url.Values{"foo": []string{"a"}}) {
@@ -242,7 +234,7 @@ func TestTransformResponse(t *testing.T) {
 		{Response: &http.Response{StatusCode: 200, Body: ioutil.NopCloser(bytes.NewReader(invalid))}, Data: invalid},
 	}
 	for i, test := range testCases {
-		r := NewRequest(nil, "", uri, testapi.Codec(), true, true)
+		r := NewRequest(nil, "", uri, testapi.Version(), testapi.Codec(), true, true)
 		if test.Response.Body == nil {
 			test.Response.Body = ioutil.NopCloser(bytes.NewReader([]byte{}))
 		}
@@ -540,7 +532,7 @@ func TestRequestUpgrade(t *testing.T) {
 			Err: true,
 		},
 		{
-			Request: NewRequest(nil, "", uri, testapi.Codec(), true, true),
+			Request: NewRequest(nil, "", uri, testapi.Version(), testapi.Codec(), true, true),
 			Config: &Config{
 				Username: "u",
 				Password: "p",
@@ -549,7 +541,7 @@ func TestRequestUpgrade(t *testing.T) {
 			Err:             false,
 		},
 		{
-			Request: NewRequest(nil, "", uri, testapi.Codec(), true, true),
+			Request: NewRequest(nil, "", uri, testapi.Version(), testapi.Codec(), true, true),
 			Config: &Config{
 				BearerToken: "b",
 			},
@@ -638,7 +630,6 @@ func TestDoRequestNewWay(t *testing.T) {
 	obj, err := c.Verb("POST").
 		Prefix("foo", "bar").
 		Suffix("baz").
-		ParseSelectorParam("labels", "name=foo").
 		Timeout(time.Second).
 		Body([]byte(reqBody)).
 		Do().Get()
@@ -651,7 +642,7 @@ func TestDoRequestNewWay(t *testing.T) {
 	} else if !api.Semantic.DeepDerivative(expectedObj, obj) {
 		t.Errorf("Expected: %#v, got %#v", expectedObj, obj)
 	}
-	fakeHandler.ValidateRequest(t, "/api/v1beta2/foo/bar/baz?labels=name%3Dfoo&timeout=1s", "POST", &reqBody)
+	fakeHandler.ValidateRequest(t, "/api/v1beta2/foo/bar/baz?timeout=1s", "POST", &reqBody)
 	if fakeHandler.RequestReceived.Header["Authorization"] == nil {
 		t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived)
 	}
@@ -673,7 +664,7 @@ func TestDoRequestNewWayReader(t *testing.T) {
 		Resource("bar").
 		Name("baz").
 		Prefix("foo").
-		SelectorParam("labels", labels.Set{"name": "foo"}.AsSelector()).
+		SelectorParam(api.LabelSelectorQueryParam(c.APIVersion()), labels.Set{"name": "foo"}.AsSelector()).
 		Timeout(time.Second).
 		Body(bytes.NewBuffer(reqBodyExpected)).
 		Do().Get()
@@ -709,7 +700,7 @@ func TestDoRequestNewWayObj(t *testing.T) {
 		Suffix("baz").
 		Name("bar").
 		Resource("foo").
-		SelectorParam("labels", labels.Set{"name": "foo"}.AsSelector()).
+		SelectorParam(api.LabelSelectorQueryParam(c.APIVersion()), labels.Set{"name": "foo"}.AsSelector()).
 		Timeout(time.Second).
 		Body(reqObj).
 		Do().Get()
@@ -758,7 +749,6 @@ func TestDoRequestNewWayFile(t *testing.T) {
 	wasCreated := true
 	obj, err := c.Verb("POST").
 		Prefix("foo/bar", "baz").
-		ParseSelectorParam("labels", "name=foo").
 		Timeout(time.Second).
 		Body(file.Name()).
 		Do().WasCreated(&wasCreated).Get()
@@ -775,7 +765,7 @@ func TestDoRequestNewWayFile(t *testing.T) {
 		t.Errorf("expected object was not created")
 	}
 	tmpStr := string(reqBodyExpected)
-	fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz?labels=name%3Dfoo&timeout=1s", "POST", &tmpStr)
+	fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz?timeout=1s", "POST", &tmpStr)
 	if fakeHandler.RequestReceived.Header["Authorization"] == nil {
 		t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived)
 	}
@@ -800,7 +790,6 @@ func TestWasCreated(t *testing.T) {
 	wasCreated := false
 	obj, err := c.Verb("PUT").
 		Prefix("foo/bar", "baz").
-		ParseSelectorParam("labels", "name=foo").
 		Timeout(time.Second).
 		Body(reqBodyExpected).
 		Do().WasCreated(&wasCreated).Get()
@@ -818,7 +807,7 @@ func TestWasCreated(t *testing.T) {
 	}
 
 	tmpStr := string(reqBodyExpected)
-	fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz?labels=name%3Dfoo&timeout=1s", "PUT", &tmpStr)
+	fakeHandler.ValidateRequest(t, "/api/v1beta1/foo/bar/baz?timeout=1s", "PUT", &tmpStr)
 	if fakeHandler.RequestReceived.Header["Authorization"] == nil {
 		t.Errorf("Request is missing authorization header: %#v", *fakeHandler.RequestReceived)
 	}