From 88f7fb81926b87bf05b993b526eca934195725eb Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 16 Jul 2017 00:40:24 -0400 Subject: [PATCH] Remove Kube specific api constructs from restclient All callers must use VersionedParameters, which no longer has special behavior for Kube resources. Kubernetes-commit: 112e0fa9da069d147fffe5bd0638ebce4a9bba42 --- rest/BUILD | 3 - rest/request.go | 182 ++----------------------------------------- rest/request_test.go | 31 +------- 3 files changed, 8 insertions(+), 208 deletions(-) diff --git a/rest/BUILD b/rest/BUILD index ac39ce79..1e761402 100644 --- a/rest/BUILD +++ b/rest/BUILD @@ -28,7 +28,6 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/api/equality: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/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", @@ -65,8 +64,6 @@ go_library( "//vendor/k8s.io/api/core/v1: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/fields:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/streaming:go_default_library", diff --git a/rest/request.go b/rest/request.go index d8e5dcc6..2d676cec 100644 --- a/rest/request.go +++ b/rest/request.go @@ -33,16 +33,12 @@ import ( "time" "github.com/golang/glog" - "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" 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/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer/streaming" "k8s.io/apimachinery/pkg/util/net" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/watch" restclientwatch "k8s.io/client-go/rest/watch" "k8s.io/client-go/tools/metrics" @@ -287,143 +283,6 @@ func (r *Request) RequestURI(uri string) *Request { return r } -const ( - // A constant that clients can use to refer in a field selector to the object name field. - // Will be automatically emitted as the correct name for the API version. - nodeUnschedulable = "spec.unschedulable" - objectNameField = "metadata.name" - podHost = "spec.nodeName" - podStatus = "status.phase" - secretType = "type" - - eventReason = "reason" - eventSource = "source" - eventType = "type" - eventInvolvedKind = "involvedObject.kind" - eventInvolvedNamespace = "involvedObject.namespace" - eventInvolvedName = "involvedObject.name" - eventInvolvedUID = "involvedObject.uid" - eventInvolvedAPIVersion = "involvedObject.apiVersion" - eventInvolvedResourceVersion = "involvedObject.resourceVersion" - eventInvolvedFieldPath = "involvedObject.fieldPath" -) - -type clientFieldNameToAPIVersionFieldName map[string]string - -func (c clientFieldNameToAPIVersionFieldName) filterField(field, value string) (newField, newValue string, err error) { - newFieldName, ok := c[field] - if !ok { - return "", "", fmt.Errorf("%v - %v - no field mapping defined", field, value) - } - return newFieldName, value, nil -} - -type resourceTypeToFieldMapping map[string]clientFieldNameToAPIVersionFieldName - -func (r resourceTypeToFieldMapping) filterField(resourceType, field, value string) (newField, newValue string, err error) { - fMapping, ok := r[resourceType] - if !ok { - return "", "", fmt.Errorf("%v - %v - %v - no field mapping defined", resourceType, field, value) - } - return fMapping.filterField(field, value) -} - -type versionToResourceToFieldMapping map[schema.GroupVersion]resourceTypeToFieldMapping - -// filterField transforms the given field/value selector for the given groupVersion and resource -func (v versionToResourceToFieldMapping) filterField(groupVersion *schema.GroupVersion, resourceType, field, value string) (newField, newValue string, err error) { - rMapping, ok := v[*groupVersion] - if !ok { - // no groupVersion overrides registered, default to identity mapping - return field, value, nil - } - newField, newValue, err = rMapping.filterField(resourceType, field, value) - if err != nil { - // no groupVersionResource overrides registered, default to identity mapping - return field, value, nil - } - return newField, newValue, nil -} - -var fieldMappings = versionToResourceToFieldMapping{ - v1.SchemeGroupVersion: resourceTypeToFieldMapping{ - "nodes": clientFieldNameToAPIVersionFieldName{ - objectNameField: objectNameField, - nodeUnschedulable: nodeUnschedulable, - }, - "pods": clientFieldNameToAPIVersionFieldName{ - objectNameField: objectNameField, - podHost: podHost, - podStatus: podStatus, - }, - "secrets": clientFieldNameToAPIVersionFieldName{ - secretType: secretType, - }, - "serviceAccounts": clientFieldNameToAPIVersionFieldName{ - objectNameField: objectNameField, - }, - "endpoints": clientFieldNameToAPIVersionFieldName{ - objectNameField: objectNameField, - }, - "events": clientFieldNameToAPIVersionFieldName{ - objectNameField: objectNameField, - eventReason: eventReason, - eventSource: eventSource, - eventType: eventType, - eventInvolvedKind: eventInvolvedKind, - eventInvolvedNamespace: eventInvolvedNamespace, - eventInvolvedName: eventInvolvedName, - eventInvolvedUID: eventInvolvedUID, - eventInvolvedAPIVersion: eventInvolvedAPIVersion, - eventInvolvedResourceVersion: eventInvolvedResourceVersion, - eventInvolvedFieldPath: eventInvolvedFieldPath, - }, - }, -} - -// FieldsSelectorParam adds the given selector as a query parameter with the name paramName. -func (r *Request) FieldsSelectorParam(s fields.Selector) *Request { - if r.err != nil { - return r - } - if s == nil { - return r - } - if s.Empty() { - return r - } - s2, err := s.Transform(func(field, value string) (newField, newValue string, err error) { - return fieldMappings.filterField(r.content.GroupVersion, r.resource, field, value) - }) - if err != nil { - r.err = err - return r - } - return r.setParam(metav1.FieldSelectorQueryParam(r.content.GroupVersion.String()), s2.String()) -} - -// LabelsSelectorParam adds the given selector as a query parameter -func (r *Request) LabelsSelectorParam(s labels.Selector) *Request { - if r.err != nil { - return r - } - if s == nil { - return r - } - if s.Empty() { - return r - } - return r.setParam(metav1.LabelSelectorQueryParam(r.content.GroupVersion.String()), s.String()) -} - -// UintParam creates a query parameter with the given value. -func (r *Request) UintParam(paramName string, u uint64) *Request { - if r.err != nil { - return r - } - return r.setParam(paramName, strconv.FormatUint(u, 10)) -} - // Param creates a query parameter with the given string value. func (r *Request) Param(paramName, s string) *Request { if r.err != nil { @@ -435,6 +294,8 @@ func (r *Request) Param(paramName, s string) *Request { // VersionedParams will take the provided object, serialize it to a map[string][]string using the // implicit RESTClient API version and the default parameter codec, and then add those as parameters // to the request. Use this to provide versioned query parameters from client libraries. +// VersionedParams will not write query parameters that have omitempty set and are empty. If a +// parameter has already been set it is appended to (Params and VersionedParams are additive). func (r *Request) VersionedParams(obj runtime.Object, codec runtime.ParameterCodec) *Request { if r.err != nil { return r @@ -445,43 +306,10 @@ func (r *Request) VersionedParams(obj runtime.Object, codec runtime.ParameterCod return r } for k, v := range params { - for _, value := range v { - // TODO: Move it to setParam method, once we get rid of - // FieldSelectorParam & LabelSelectorParam methods. - if k == metav1.LabelSelectorQueryParam(r.content.GroupVersion.String()) && value == "" { - // Don't set an empty selector for backward compatibility. - // Since there is no way to get the difference between empty - // and unspecified string, we don't set it to avoid having - // labelSelector= param in every request. - continue - } - if k == metav1.FieldSelectorQueryParam(r.content.GroupVersion.String()) { - if len(value) == 0 { - // Don't set an empty selector for backward compatibility. - // Since there is no way to get the difference between empty - // and unspecified string, we don't set it to avoid having - // fieldSelector= param in every request. - continue - } - // TODO: Filtering should be handled somewhere else. - selector, err := fields.ParseSelector(value) - if err != nil { - r.err = fmt.Errorf("unparsable field selector: %v", err) - return r - } - filteredSelector, err := selector.Transform( - func(field, value string) (newField, newValue string, err error) { - return fieldMappings.filterField(r.content.GroupVersion, r.resource, field, value) - }) - if err != nil { - r.err = fmt.Errorf("untransformable field selector: %v", err) - return r - } - value = filteredSelector.String() - } - - r.setParam(k, value) + if r.params == nil { + r.params = make(url.Values) } + r.params[k] = append(r.params[k], v...) } return r } diff --git a/rest/request_test.go b/rest/request_test.go index 5863a970..fa016dfe 100755 --- a/rest/request_test.go +++ b/rest/request_test.go @@ -38,7 +38,6 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" 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" @@ -100,8 +99,6 @@ func TestRequestWithErrorWontChange(t *testing.T) { } r := original changed := r.Param("foo", "bar"). - LabelsSelectorParam(labels.Set{"a": "b"}.AsSelector()). - UintParam("uint", 1). AbsPath("/abs"). Prefix("test"). Suffix("testing"). @@ -257,7 +254,7 @@ func TestRequestVersionedParamsFromListOptions(t *testing.T) { "resourceVersion": []string{"1", "2"}, "timeoutSeconds": []string{"10"}, }) { - t.Errorf("should have set a param: %#v", r) + t.Errorf("should have set a param: %#v %v", r.params, r.err) } } @@ -1276,7 +1273,6 @@ func TestDoRequestNewWayReader(t *testing.T) { Resource("bar"). Name("baz"). Prefix("foo"). - LabelsSelectorParam(labels.Set{"name": "foo"}.AsSelector()). Timeout(time.Second). Body(bytes.NewBuffer(reqBodyExpected)). Do().Get() @@ -1291,7 +1287,7 @@ func TestDoRequestNewWayReader(t *testing.T) { } tmpStr := string(reqBodyExpected) requestURL := defaultResourcePathWithPrefix("foo", "bar", "", "baz") - requestURL += "?" + metav1.LabelSelectorQueryParam(v1.SchemeGroupVersion.String()) + "=name%3Dfoo&timeout=1s" + requestURL += "?timeout=1s" fakeHandler.ValidateRequest(t, requestURL, "POST", &tmpStr) } @@ -1316,7 +1312,6 @@ func TestDoRequestNewWayObj(t *testing.T) { Suffix("baz"). Name("bar"). Resource("foo"). - LabelsSelectorParam(labels.Set{"name": "foo"}.AsSelector()). Timeout(time.Second). Body(reqObj). Do().Get() @@ -1331,7 +1326,7 @@ func TestDoRequestNewWayObj(t *testing.T) { } tmpStr := string(reqBodyExpected) requestURL := defaultResourcePathWithPrefix("", "foo", "", "bar/baz") - requestURL += "?" + metav1.LabelSelectorQueryParam(v1.SchemeGroupVersion.String()) + "=name%3Dfoo&timeout=1s" + requestURL += "?timeout=1s" fakeHandler.ValidateRequest(t, requestURL, "POST", &tmpStr) } @@ -1489,26 +1484,6 @@ func TestAbsPath(t *testing.T) { } } -func TestUintParam(t *testing.T) { - table := []struct { - name string - testVal uint64 - expectStr string - }{ - {"foo", 31415, "http://localhost?foo=31415"}, - {"bar", 42, "http://localhost?bar=42"}, - {"baz", 0, "http://localhost?baz=0"}, - } - - for _, item := range table { - u, _ := url.Parse("http://localhost") - r := NewRequest(nil, "GET", u, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil).AbsPath("").UintParam(item.name, item.testVal) - if e, a := item.expectStr, r.URL().String(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - } -} - func TestUnacceptableParamNames(t *testing.T) { table := []struct { name string