From 99248b8fe1fe1c28188657d811dc7baf8cd12982 Mon Sep 17 00:00:00 2001 From: Wenjia Zhang Date: Fri, 14 Sep 2018 11:31:21 -0700 Subject: [PATCH] Rewrite finalURLTemplate used only for metrics because of dynamic client change --- staging/src/k8s.io/client-go/rest/request.go | 65 ++++++- .../src/k8s.io/client-go/rest/request_test.go | 176 ++++++++++++++++-- 2 files changed, 217 insertions(+), 24 deletions(-) diff --git a/staging/src/k8s.io/client-go/rest/request.go b/staging/src/k8s.io/client-go/rest/request.go index 69ce1c7595..9bb311448a 100644 --- a/staging/src/k8s.io/client-go/rest/request.go +++ b/staging/src/k8s.io/client-go/rest/request.go @@ -455,17 +455,9 @@ func (r *Request) URL() *url.URL { // finalURLTemplate is similar to URL(), but will make all specific parameter values equal // - instead of name or namespace, "{name}" and "{namespace}" will be used, and all query -// parameters will be reset. This creates a copy of the request so as not to change the -// underlying object. This means some useful request info (like the types of field -// selectors in use) will be lost. -// TODO: preserve field selector keys +// parameters will be reset. This creates a copy of the url so as not to change the +// underlying object. func (r Request) finalURLTemplate() url.URL { - if len(r.resourceName) != 0 { - r.resourceName = "{name}" - } - if r.namespaceSet && len(r.namespace) != 0 { - r.namespace = "{namespace}" - } newParams := url.Values{} v := []string{"{value}"} for k := range r.params { @@ -473,6 +465,59 @@ func (r Request) finalURLTemplate() url.URL { } r.params = newParams url := r.URL() + segments := strings.Split(r.URL().Path, "/") + groupIndex := 0 + index := 0 + if r.URL() != nil && r.baseURL != nil && strings.Contains(r.URL().Path, r.baseURL.Path) { + groupIndex += len(strings.Split(r.baseURL.Path, "/")) + } + if groupIndex >= len(segments) { + return *url + } + + const CoreGroupPrefix = "api" + const NamedGroupPrefix = "apis" + isCoreGroup := segments[groupIndex] == CoreGroupPrefix + isNamedGroup := segments[groupIndex] == NamedGroupPrefix + if isCoreGroup { + // checking the case of core group with /api/v1/... format + index = groupIndex + 2 + } else if isNamedGroup { + // checking the case of named group with /apis/apps/v1/... format + index = groupIndex + 3 + } else { + // this should not happen that the only two possibilities are /api... and /apis..., just want to put an + // outlet here in case more API groups are added in future if ever possible: + // https://kubernetes.io/docs/concepts/overview/kubernetes-api/#api-groups + // if a wrong API groups name is encountered, return the {prefix} for url.Path + url.Path = "/{prefix}" + url.RawQuery = "" + return *url + } + //switch segLength := len(segments) - index; segLength { + switch { + // case len(segments) - index == 1: + // resource (with no name) do nothing + case len(segments)-index == 2: + // /$RESOURCE/$NAME: replace $NAME with {name} + segments[index+1] = "{name}" + case len(segments)-index == 3: + if segments[index+2] == "finalize" || segments[index+2] == "status" { + // /$RESOURCE/$NAME/$SUBRESOURCE: replace $NAME with {name} + segments[index+1] = "{name}" + } else { + // /namespace/$NAMESPACE/$RESOURCE: replace $NAMESPACE with {namespace} + segments[index+1] = "{namespace}" + } + case len(segments)-index >= 4: + segments[index+1] = "{namespace}" + // /namespace/$NAMESPACE/$RESOURCE/$NAME: replace $NAMESPACE with {namespace}, $NAME with {name} + if segments[index+3] != "finalize" && segments[index+3] != "status" { + // /$RESOURCE/$NAME/$SUBRESOURCE: replace $NAME with {name} + segments[index+3] = "{name}" + } + } + url.Path = path.Join(segments...) return *url } diff --git a/staging/src/k8s.io/client-go/rest/request_test.go b/staging/src/k8s.io/client-go/rest/request_test.go index f75ee86cbe..2660c0be5d 100755 --- a/staging/src/k8s.io/client-go/rest/request_test.go +++ b/staging/src/k8s.io/client-go/rest/request_test.go @@ -340,21 +340,169 @@ func TestResultIntoWithNoBodyReturnsErr(t *testing.T) { } func TestURLTemplate(t *testing.T) { - uri, _ := url.Parse("http://localhost") - r := NewRequest(nil, "POST", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0) - r.Prefix("pre1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0") - full := r.URL() - if full.String() != "http://localhost/pre1/namespaces/ns/r1/nm?p0=v0" { - t.Errorf("unexpected initial URL: %s", full) + uri, _ := url.Parse("http://localhost/some/base/url/path") + testCases := []struct { + Request *Request + ExpectedFullURL string + ExpectedFinalURL string + }{ + { + // non dynamic client + Request: NewRequest(nil, "POST", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("api", "v1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0"), + ExpectedFullURL: "http://localhost/some/base/url/path/api/v1/namespaces/ns/r1/nm?p0=v0", + ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D?p0=%7Bvalue%7D", + }, + { + // non dynamic client with wrong api group + Request: NewRequest(nil, "POST", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("pre1", "v1").Resource("r1").Namespace("ns").Name("nm").Param("p0", "v0"), + ExpectedFullURL: "http://localhost/some/base/url/path/pre1/v1/namespaces/ns/r1/nm?p0=v0", + ExpectedFinalURL: "http://localhost/%7Bprefix%7D", + }, + { + // dynamic client with core group + namespace + resourceResource (with name) + // /api/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/api/v1/namespaces/ns/r1/name1"), + ExpectedFullURL: "http://localhost/some/base/url/path/api/v1/namespaces/ns/r1/name1", + ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D", + }, + { + // dynamic client with named group + namespace + resourceResource (with name) + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/g1/v1/namespaces/ns/r1/name1"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/g1/v1/namespaces/ns/r1/name1", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/g1/v1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D", + }, + { + // dynamic client with core group + namespace + resourceResource (with NO name) + // /api/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/api/v1/namespaces/ns/r1"), + ExpectedFullURL: "http://localhost/some/base/url/path/api/v1/namespaces/ns/r1", + ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/namespaces/%7Bnamespace%7D/r1", + }, + { + // dynamic client with named group + namespace + resourceResource (with NO name) + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/g1/v1/namespaces/ns/r1"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/g1/v1/namespaces/ns/r1", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/g1/v1/namespaces/%7Bnamespace%7D/r1", + }, + { + // dynamic client with core group + resourceResource (with name) + // /api/$RESOURCEVERSION/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/api/v1/r1/name1"), + ExpectedFullURL: "http://localhost/some/base/url/path/api/v1/r1/name1", + ExpectedFinalURL: "http://localhost/some/base/url/path/api/v1/r1/%7Bname%7D", + }, + { + // dynamic client with named group + resourceResource (with name) + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/g1/v1/r1/name1"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/g1/v1/r1/name1", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/g1/v1/r1/%7Bname%7D", + }, + { + // dynamic client with named group + namespace + resourceResource (with name) + subresource + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME/$SUBRESOURCE + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/%7Bname%7D/finalize", + }, + { + // dynamic client with named group + namespace + resourceResource (with name) + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/%7Bname%7D", + }, + { + // dynamic client with named group + namespace + resourceResource (with NO name) + subresource + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%SUBRESOURCE + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/finalize"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/finalize", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/finalize", + }, + { + // dynamic client with named group + namespace + resourceResource (with NO name) + subresource + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%SUBRESOURCE + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces/status"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces/status", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces/status", + }, + { + // dynamic client with named group + namespace + resourceResource (with no name) + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces/namespaces"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/namespaces", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bnamespace%7D/namespaces", + }, + { + // dynamic client with named group + resourceResource (with name) + subresource + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces/finalize"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/finalize", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bname%7D/finalize", + }, + { + // dynamic client with named group + resourceResource (with name) + subresource + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces/status"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces/status", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bname%7D/status", + }, + { + // dynamic client with named group + resourceResource (with name) + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces/namespaces"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/namespaces", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces/%7Bname%7D", + }, + { + // dynamic client with named group + resourceResource (with no name) + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/$RESOURCE/%NAME + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/apis/namespaces/namespaces/namespaces"), + ExpectedFullURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces", + ExpectedFinalURL: "http://localhost/some/base/url/path/apis/namespaces/namespaces/namespaces", + }, + { + // dynamic client with wrong api group + namespace + resourceResource (with name) + subresource + // /apis/$NAMEDGROUPNAME/$RESOURCEVERSION/namespaces/$NAMESPACE/$RESOURCE/%NAME/$SUBRESOURCE + Request: NewRequest(nil, "DELETE", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil, 0). + Prefix("/pre1/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize"), + ExpectedFullURL: "http://localhost/some/base/url/path/pre1/namespaces/namespaces/namespaces/namespaces/namespaces/namespaces/finalize", + ExpectedFinalURL: "http://localhost/%7Bprefix%7D", + }, } - actualURL := r.finalURLTemplate() - actual := actualURL.String() - expected := "http://localhost/pre1/namespaces/%7Bnamespace%7D/r1/%7Bname%7D?p0=%7Bvalue%7D" - if actual != expected { - t.Errorf("unexpected URL template: %s %s", actual, expected) - } - if r.URL().String() != full.String() { - t.Errorf("creating URL template changed request: %s -> %s", full.String(), r.URL().String()) + for i, testCase := range testCases { + r := testCase.Request + full := r.URL() + if full.String() != testCase.ExpectedFullURL { + t.Errorf("%d: unexpected initial URL: %s %s", i, full, testCase.ExpectedFullURL) + } + actualURL := r.finalURLTemplate() + actual := actualURL.String() + if actual != testCase.ExpectedFinalURL { + t.Errorf("%d: unexpected URL template: %s %s", i, actual, testCase.ExpectedFinalURL) + } + if r.URL().String() != full.String() { + t.Errorf("%d, creating URL template changed request: %s -> %s", i, full.String(), r.URL().String()) + } } }