diff --git a/pkg/endpoints/apiserver_test.go b/pkg/endpoints/apiserver_test.go index 3f0ee9c5c..f4e64b5e2 100644 --- a/pkg/endpoints/apiserver_test.go +++ b/pkg/endpoints/apiserver_test.go @@ -1444,65 +1444,6 @@ func TestSelfLinkSkipsEmptyName(t *testing.T) { } } -func TestRootSelfLink(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemoveSelfLink, false)() - - storage := map[string]rest.Storage{} - simpleStorage := GetWithOptionsRootRESTStorage{ - SimpleTypedStorage: &SimpleTypedStorage{ - baseType: &genericapitesting.SimpleRoot{}, // a root scoped type - item: &genericapitesting.SimpleRoot{ - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Other: "foo", - }, - }, - takesPath: "atAPath", - } - storage["simple"] = &simpleStorage - storage["simple/sub"] = &simpleStorage - handler := handle(storage) - server := httptest.NewServer(handler) - defer server.Close() - - testCases := []struct { - url string - selfLink string - }{ - { - url: server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simple/foo", - selfLink: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simple/foo", - }, - { - url: server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simple/foo/sub", - selfLink: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/simple/foo/sub", - }, - } - - for _, test := range testCases { - resp, err := http.Get(test.url) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if resp.StatusCode != http.StatusOK { - t.Errorf("Unexpected status: %d, Expected: %d, %#v", resp.StatusCode, http.StatusOK, resp) - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - t.Logf("Data: %s", string(body)) - } - var out genericapitesting.SimpleRoot - if _, err := extractBody(resp, &out); err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if out.SelfLink != test.selfLink { - t.Errorf("unexpected self link: %#v", out.SelfLink) - } - } -} - func TestMetadata(t *testing.T) { simpleStorage := &MetadataRESTStorage{&SimpleRESTStorage{}, []string{"text/plain"}} h := handle(map[string]rest.Storage{"simple": simpleStorage}) @@ -3885,15 +3826,6 @@ type setTestSelfLinker struct { func (s *setTestSelfLinker) Namespace(runtime.Object) (string, error) { return s.namespace, s.err } func (s *setTestSelfLinker) Name(runtime.Object) (string, error) { return s.name, s.err } func (s *setTestSelfLinker) SelfLink(runtime.Object) (string, error) { return "", s.err } -func (s *setTestSelfLinker) SetSelfLink(obj runtime.Object, selfLink string) error { - if e, a := s.expectedSet, selfLink; e != a { - if !s.alternativeSet.Has(a) { - s.t.Errorf("expected '%v', got '%v'", e, a) - } - } - s.called = true - return s.err -} func TestCreate(t *testing.T) { storage := SimpleRESTStorage{ diff --git a/pkg/endpoints/handlers/namer.go b/pkg/endpoints/handlers/namer.go index 2eb0c174d..7439caa40 100644 --- a/pkg/endpoints/handlers/namer.go +++ b/pkg/endpoints/handlers/namer.go @@ -19,8 +19,6 @@ package handlers import ( "fmt" "net/http" - "net/url" - "strings" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -38,14 +36,6 @@ type ScopeNamer interface { // ObjectName returns the namespace and name from an object if they exist, or an error if the object // does not support names. ObjectName(obj runtime.Object) (namespace, name string, err error) - // SetSelfLink sets the provided URL onto the object. The method should return nil if the object - // does not support selfLinks. - SetSelfLink(obj runtime.Object, url string) error - // GenerateLink creates an encoded URI for a given runtime object that represents the canonical path - // and query. - GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) - // GenerateListLink creates an encoded URI for a list that represents the canonical path and query. - GenerateListLink(req *http.Request) (uri string, err error) } type ContextBasedNaming struct { @@ -59,10 +49,6 @@ type ContextBasedNaming struct { // ContextBasedNaming implements ScopeNamer var _ ScopeNamer = ContextBasedNaming{} -func (n ContextBasedNaming) SetSelfLink(obj runtime.Object, url string) error { - return n.SelfLinker.SetSelfLink(obj, url) -} - func (n ContextBasedNaming) Namespace(req *http.Request) (namespace string, err error) { requestInfo, ok := request.RequestInfoFrom(req.Context()) if !ok { @@ -83,55 +69,6 @@ func (n ContextBasedNaming) Name(req *http.Request) (namespace, name string, err return requestInfo.Namespace, requestInfo.Name, nil } -// fastURLPathEncode encodes the provided path as a URL path -func fastURLPathEncode(path string) string { - for _, r := range []byte(path) { - switch { - case r >= '-' && r <= '9', r >= 'A' && r <= 'Z', r >= 'a' && r <= 'z': - // characters within this range do not require escaping - default: - var u url.URL - u.Path = path - return u.EscapedPath() - } - } - return path -} - -func (n ContextBasedNaming) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) { - namespace, name, err := n.ObjectName(obj) - if err == errEmptyName && len(requestInfo.Name) > 0 { - name = requestInfo.Name - } else if err != nil { - return "", err - } - if len(namespace) == 0 && len(requestInfo.Namespace) > 0 { - namespace = requestInfo.Namespace - } - - if n.ClusterScoped { - return n.SelfLinkPathPrefix + url.QueryEscape(name) + n.SelfLinkPathSuffix, nil - } - - builder := strings.Builder{} - builder.Grow(len(n.SelfLinkPathPrefix) + len(namespace) + len(requestInfo.Resource) + len(name) + len(n.SelfLinkPathSuffix) + 8) - builder.WriteString(n.SelfLinkPathPrefix) - builder.WriteString(namespace) - builder.WriteByte('/') - builder.WriteString(requestInfo.Resource) - builder.WriteByte('/') - builder.WriteString(name) - builder.WriteString(n.SelfLinkPathSuffix) - return fastURLPathEncode(builder.String()), nil -} - -func (n ContextBasedNaming) GenerateListLink(req *http.Request) (uri string, err error) { - if len(req.URL.RawPath) > 0 { - return req.URL.RawPath, nil - } - return fastURLPathEncode(req.URL.Path), nil -} - func (n ContextBasedNaming) ObjectName(obj runtime.Object) (namespace, name string, err error) { name, err = n.SelfLinker.Name(obj) if err != nil { diff --git a/pkg/endpoints/handlers/namer_test.go b/pkg/endpoints/handlers/namer_test.go deleted file mode 100644 index 861296278..000000000 --- a/pkg/endpoints/handlers/namer_test.go +++ /dev/null @@ -1,179 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package handlers - -import ( - "math/rand" - "net/url" - "testing" - - fuzz "github.com/google/gofuzz" - - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apiserver/pkg/endpoints/request" -) - -func TestGenerateLink(t *testing.T) { - testCases := []struct { - name string - requestInfo *request.RequestInfo - obj runtime.Object - expect string - expectErr bool - clusterScoped bool - }{ - { - name: "obj has more priority than requestInfo", - requestInfo: &request.RequestInfo{ - Name: "should-not-use", - Namespace: "should-not-use", - Resource: "pod", - }, - obj: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "should-use", Namespace: "should-use"}}, - expect: "/api/v1/should-use/pod/should-use", - expectErr: false, - clusterScoped: false, - }, - { - name: "hit errEmptyName", - requestInfo: &request.RequestInfo{ - Name: "should-use", - Namespace: "should-use", - Resource: "pod", - }, - obj: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "should-not-use"}}, - expect: "/api/v1/should-use/pod/should-use", - expectErr: false, - clusterScoped: false, - }, - { - name: "use namespace of requestInfo if obj namespace is empty", - requestInfo: &request.RequestInfo{ - Name: "should-not-use", - Namespace: "should-use", - Resource: "pod", - }, - obj: &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "should-use"}}, - expect: "/api/v1/should-use/pod/should-use", - expectErr: false, - clusterScoped: false, - }, - { - name: "hit error", - requestInfo: &request.RequestInfo{ - Name: "", - Namespace: "", - Resource: "pod", - }, - obj: &v1.Pod{ObjectMeta: metav1.ObjectMeta{}}, - expect: "name must be provided", - expectErr: true, - clusterScoped: false, - }, - { - name: "cluster scoped", - requestInfo: &request.RequestInfo{ - Name: "only-name", - Namespace: "should-not-use", - Resource: "pod", - }, - obj: &v1.Pod{ObjectMeta: metav1.ObjectMeta{}}, - expect: "/api/v1/only-name", - expectErr: false, - clusterScoped: true, - }, - } - - for _, test := range testCases { - n := ContextBasedNaming{ - SelfLinker: meta.NewAccessor(), - SelfLinkPathPrefix: "/api/v1/", - ClusterScoped: test.clusterScoped, - } - uri, err := n.GenerateLink(test.requestInfo, test.obj) - - if uri != test.expect && err.Error() != test.expect { - if test.expectErr { - t.Fatalf("%s: unexpected non-error: %v", test.name, err) - } else { - t.Fatalf("%s: expected: %v, but got: %v", test.name, test.expect, uri) - } - } - } -} - -func Test_fastURLPathEncode_fuzz(t *testing.T) { - specialCases := []string{"/", "//", ".", "*", "/abc%"} - for _, test := range specialCases { - got := fastURLPathEncode(test) - u := url.URL{Path: test} - expected := u.EscapedPath() - if got != expected { - t.Errorf("%q did not match %q", got, expected) - } - } - f := fuzz.New().Funcs( - func(s *string, c fuzz.Continue) { - *s = randString(c.Rand) - }, - ) - for i := 0; i < 2000; i++ { - var test string - f.Fuzz(&test) - - got := fastURLPathEncode(test) - u := url.URL{Path: test} - expected := u.EscapedPath() - if got != expected { - t.Errorf("%q did not match %q", got, expected) - } - } -} - -// Unicode range fuzzer from github.com/google/gofuzz/fuzz.go - -type charRange struct { - first, last rune -} - -var unicodeRanges = []charRange{ - {0x00, 0x255}, - {' ', '~'}, // ASCII characters - {'\u00a0', '\u02af'}, // Multi-byte encoded characters - {'\u4e00', '\u9fff'}, // Common CJK (even longer encodings) -} - -// randString makes a random string up to 20 characters long. The returned string -// may include a variety of (valid) UTF-8 encodings. -func randString(r *rand.Rand) string { - n := r.Intn(20) - runes := make([]rune, n) - for i := range runes { - runes[i] = unicodeRanges[r.Intn(len(unicodeRanges))].choose(r) - } - return string(runes) -} - -// choose returns a random unicode character from the given range, using the -// given randomness source. -func (r *charRange) choose(rand *rand.Rand) rune { - count := int64(r.last - r.first) - return r.first + rune(rand.Int63n(count)) -} diff --git a/pkg/endpoints/handlers/response_test.go b/pkg/endpoints/handlers/response_test.go index 4cd37aa7c..9455a0613 100644 --- a/pkg/endpoints/handlers/response_test.go +++ b/pkg/endpoints/handlers/response_test.go @@ -77,11 +77,6 @@ type mockNamer struct{} func (*mockNamer) Namespace(_ *http.Request) (string, error) { return "", nil } func (*mockNamer) Name(_ *http.Request) (string, string, error) { return "", "", nil } func (*mockNamer) ObjectName(_ runtime.Object) (string, string, error) { return "", "", nil } -func (*mockNamer) SetSelfLink(_ runtime.Object, _ string) error { return nil } -func (*mockNamer) GenerateLink(_ *request.RequestInfo, _ runtime.Object) (string, error) { - return "", nil -} -func (*mockNamer) GenerateListLink(_ *http.Request) (string, error) { return "", nil } func TestCacheableObject(t *testing.T) { pomGVK := metav1.SchemeGroupVersion.WithKind("PartialObjectMetadata") diff --git a/pkg/endpoints/handlers/rest.go b/pkg/endpoints/handlers/rest.go index c5fd95e89..aba9a3f88 100644 --- a/pkg/endpoints/handlers/rest.go +++ b/pkg/endpoints/handlers/rest.go @@ -49,7 +49,6 @@ import ( "k8s.io/apiserver/pkg/registry/rest" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/warning" - "k8s.io/klog/v2" ) const ( @@ -260,18 +259,6 @@ func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime return errors.NewBadRequest(fmt.Sprintf("the object provided is unrecognized (must be of type %s): %v (%s)", objGVK.Kind, baseErr, summary)) } -// setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request -// plus the path and query generated by the provided linkFunc -func setSelfLink(obj runtime.Object, requestInfo *request.RequestInfo, namer ScopeNamer) error { - // TODO: SelfLink generation should return a full URL? - uri, err := namer.GenerateLink(requestInfo, obj) - if err != nil { - return nil - } - - return namer.SetSelfLink(obj, uri) -} - func hasUID(obj runtime.Object) (bool, error) { if obj == nil { return false, nil @@ -387,29 +374,22 @@ func setObjectSelfLink(ctx context.Context, obj runtime.Object, req *http.Reques // check via reflection, but as we move away from reflection we require that you not only carry Items but // ListMeta into order to be identified as a list. if !meta.IsListType(obj) { - requestInfo, ok := request.RequestInfoFrom(ctx) + _, ok := request.RequestInfoFrom(ctx) if !ok { return fmt.Errorf("missing requestInfo") } - return setSelfLink(obj, requestInfo, namer) + return nil } - uri, err := namer.GenerateListLink(req) - if err != nil { - return err - } - if err := namer.SetSelfLink(obj, uri); err != nil { - klog.V(4).InfoS("Unable to set self link on object", "error", err) - } - requestInfo, ok := request.RequestInfoFrom(ctx) + _, ok := request.RequestInfoFrom(ctx) if !ok { return fmt.Errorf("missing requestInfo") } count := 0 - err = meta.EachListItem(obj, func(obj runtime.Object) error { + err := meta.EachListItem(obj, func(obj runtime.Object) error { count++ - return setSelfLink(obj, requestInfo, namer) + return nil }) if count == 0 { diff --git a/pkg/endpoints/handlers/rest_test.go b/pkg/endpoints/handlers/rest_test.go index 0bbabdc24..52d169a86 100644 --- a/pkg/endpoints/handlers/rest_test.go +++ b/pkg/endpoints/handlers/rest_test.go @@ -300,22 +300,6 @@ func (p *testNamer) ObjectName(obj runtime.Object) (namespace, name string, err return p.namespace, p.name, nil } -// SetSelfLink sets the provided URL onto the object. The method should return nil if the object -// does not support selfLinks. -func (p *testNamer) SetSelfLink(obj runtime.Object, url string) error { - return errors.New("not implemented") -} - -// GenerateLink creates a path and query for a given runtime object that represents the canonical path. -func (p *testNamer) GenerateLink(requestInfo *request.RequestInfo, obj runtime.Object) (uri string, err error) { - return "", errors.New("not implemented") -} - -// GenerateListLink creates a path and query for a list that represents the canonical path. -func (p *testNamer) GenerateListLink(req *http.Request) (uri string, err error) { - return "", errors.New("not implemented") -} - type patchTestCase struct { name string